Skip to content

fix: stop pinned context leaking to new chat windows#2786

Draft
laileni-aws wants to merge 1 commit into
aws:mainfrom
laileni-aws:fix/pinned-context-default-mutation
Draft

fix: stop pinned context leaking to new chat windows#2786
laileni-aws wants to merge 1 commit into
aws:mainfrom
laileni-aws:fix/pinned-context-default-mutation

Conversation

@laileni-aws

Copy link
Copy Markdown
Contributor

Problem

Pinned context cannot be un-pinned: after pinning context in a chat, opening a new chat window still shows the pinned item, and un-pinning does not stick.

Root cause

DEFAULT_PINNED_CONTEXT is a shared module-level array (the default pinned context — the active file — used for tabs that have none). In ChatDatabase.addPinnedContext, when a tab had no pinned context yet, the code aliased that shared array and then mutated it:

if (!tab.tabContext.pinnedContext) {
    tab.tabContext.pinnedContext = DEFAULT_PINNED_CONTEXT   // shared reference
}
// ...
tab.tabContext.pinnedContext.unshift(context)  // mutates DEFAULT_PINNED_CONTEXT

So pinning context in one tab permanently added it to the global default. Every new chat window reads DEFAULT_PINNED_CONTEXT for its initial pinned context, so it inherited the leaked item — and un-pinning in the original tab could not remove it from the polluted default. getPinnedContext also returned the shared default array by reference.

Fix

Copy the default array instead of aliasing it:

  • addPinnedContext: initialize a tab's pinned context with [...DEFAULT_PINNED_CONTEXT].
  • getPinnedContext: return [...DEFAULT_PINNED_CONTEXT] so callers can't mutate the shared default.

Testing

  • Added a regression test in chatDb.test.ts asserting that pinning a custom context in a tab does not mutate DEFAULT_PINNED_CONTEXT and that a brand-new tab does not inherit the item.
  • The test was confirmed to fail against the previous (aliasing) implementation and pass with this change.
  • Ran the chatDb unit tests: 24 passing. prettier --check passes; eslint reports no new issues.

addPinnedContext aliased the shared module-level DEFAULT_PINNED_CONTEXT
array and then mutated it with unshift/push. Pinning context in one tab
therefore polluted the default, so every new chat window inherited that
pinned item and it could not be removed. getPinnedContext also returned
the shared default array directly.

Copy the default array in both places so pinning in a tab no longer
mutates the shared default. Adds a regression test.
@codecov-commenter

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 85.71429% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 60.08%. Comparing base (abb1096) to head (c23b9ab).

Files with missing lines Patch % Lines
...language-server/agenticChat/tools/chatDb/chatDb.ts 85.71% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2786      +/-   ##
==========================================
- Coverage   60.20%   60.08%   -0.13%     
==========================================
  Files         281      281              
  Lines       71191    71196       +5     
  Branches     4575     4549      -26     
==========================================
- Hits        42861    42776      -85     
- Misses      28241    28331      +90     
  Partials       89       89              
Flag Coverage Δ
unittests 60.08% <85.71%> (-0.13%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants