fix: stop pinned context leaking to new chat windows#2786
Draft
laileni-aws wants to merge 1 commit into
Draft
Conversation
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 Report❌ Patch coverage is
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
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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_CONTEXTis a shared module-level array (the default pinned context — the active file — used for tabs that have none). InChatDatabase.addPinnedContext, when a tab had no pinned context yet, the code aliased that shared array and then mutated it:So pinning context in one tab permanently added it to the global default. Every new chat window reads
DEFAULT_PINNED_CONTEXTfor 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.getPinnedContextalso 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
chatDb.test.tsasserting that pinning a custom context in a tab does not mutateDEFAULT_PINNED_CONTEXTand that a brand-new tab does not inherit the item.chatDbunit tests: 24 passing.prettier --checkpasses;eslintreports no new issues.