sessions: fix duplicate session in list when new session is opened while another is active#314913
Merged
DonJayamanne merged 1 commit intomainfrom May 7, 2026
Merged
Conversation
… another is active When a new CLI session was created and the user opened another session before the new one fully graduated: 1. SessionAdded notification from the agent host arrived before _pendingSession was set, so _sessionCache already contained the committed session when the skeleton was added as _pendingSession. getSessions() then returned both, causing the duplicate row. 2. SessionsManagementService.onDidReplaceSession only fired _onDidChangeSessions when the active session was the 'from' session. If the user had navigated away, no refresh event fired and the SessionsList kept showing the duplicate indefinitely. Fix: - Clear _pendingSession before firing _onDidReplaceSession so any synchronous listener that calls getSessions() sees only the committed session (the finally block still clears it on the failure path). - Always fire _onDidChangeSessions in onDidReplaceSession; only the active-session reassignment remains conditional. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
Screenshot ChangesBase: Errored (20)Fixtures that failed to render — no screenshot was produced.
|
Contributor
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Fixes a race condition that caused duplicate entries in the sessions list when creating/opening sessions concurrently.
Changes:
- Clear
_pendingSessionbefore emitting the replace event so synchronous listeners don’t observe both skeleton + committed sessions. - Always forward a sessions change event on replace, even if the replaced session isn’t currently active.
Show a summary per file
| File | Description |
|---|---|
| src/vs/sessions/services/sessions/browser/sessionsManagementService.ts | Ensures the sessions list refreshes on replace events regardless of active session state. |
| src/vs/sessions/contrib/agentHost/browser/baseAgentHostSessionsProvider.ts | Prevents transient duplicate session IDs by clearing pending state prior to synchronous replace notification. |
Copilot's findings
- Files reviewed: 2/2 changed files
- Comments generated: 0
chrmarti
approved these changes
May 7, 2026
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
After creating a new CLI session (which starts running), opening another session causes the new session to appear twice in the sessions list.
Steps to reproduce:
Root Cause
Two interacting bugs:
Bug 1 —
baseAgentHostSessionsProvider.ts: Race betweenSessionAddedand_pendingSessionsendAndCreateChatawaitschatService.sendRequest(...). During that await the agent host materializes the session and firesSessionAdded, which runs_handleSessionAdded→ inserts the committed session into_sessionCache→ fires_onDidChangeSessions { added: [committed] }.After the await returns, the code sets
_pendingSession = skeletonand fires another_onDidChangeSessions { added: [skeleton] }.From that point until the
finallyblock runs,getSessions()returns[...sessionCache.values(), _pendingSession]— two objects with the same session ID. AnygetSessions()call during that window sees the duplicate.The
finallyblock clears_pendingSessionafter_onDidReplaceSession.fire(...), so any synchronous listener (e.g.SessionsManagementService) that callsgetSessions()while handling the replace event still sees both.Bug 2 —
sessionsManagementService.ts: Replace event not forwarded when active session differsSessionsManagementService.onDidReplaceSessiononly forwarded a_onDidChangeSessionsevent when the active session was thefromsession. If the user navigated to a different session while the new one was being created, no refresh event fired →SessionsListnever re-pulledgetSessions()→ the duplicate row stayed indefinitely.Fix
baseAgentHostSessionsProvider.ts— Clear_pendingSessionbefore firing_onDidReplaceSessionso any synchronous listener callinggetSessions()sees only the committed session. Thefinallyblock still clears it as a defensive fallback for the failure path.sessionsManagementService.ts— Always fire_onDidChangeSessionsinonDidReplaceSessionregardless of which session is active. Only the active-session reassignment (setActiveSession) remains conditional on matching thefromsession.