Sessions: Fix stopped session stuck in list as Working#307508
Conversation
When a session was stopped before the agent created a worktree, the temp session remained in the list permanently because the promises waiting for session commit and cache population would never resolve. - Bound _waitForCommittedSession with responseCompletePromise so it throws immediately when the response finishes without a commit - Add timeouts and DisposableStore cleanup to both wait helpers to prevent leaked listeners - Allow archive/delete on temp sessions via _cleanupTempSession fallback when _findAgentSession returns nothing Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Fixes a Sessions-window edge case where stopping a session before it commits (untitled → real URI swap) leaves a “Working…” entry stuck in the session list and makes archive/delete silently no-op. The change is focused in the Copilot chat sessions provider under vs/sessions.
Changes:
- Bound the “wait for commit” and “wait for cache population” helpers with response completion and safety timeouts, disposing listeners on all paths.
- Enabled archive/delete to clean up uncommitted temp sessions directly from the provider cache.
- Added a dedicated
_cleanupTempSessionhelper to centralize removal and event firing for temp sessions.
…thout arbitrary timeout Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Reduce both raceTimeout safety nets to 5s (commit wait and cache wait) - Revert chatSessionsService.ts JSDoc to original wording - Clarify _waitForSessionInCache comment: only runs once during init Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…ts for cancelled temp sessions - Clear _sessionGroupCache entries in _cleanupTempSession to avoid stale ISession wrapper leaks in multi-chat mode - Add tests covering delete/archive of uncommitted temp sessions and the cancellation path where commit never fires Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
| const result = await raceTimeout(sessionPromise, 5_000); | ||
| if (!result) { | ||
| throw new Error('Timed out waiting for committed session in cache'); | ||
| } |
There was a problem hiding this comment.
@osortega I intentionally did not want this timeout. This timeout just adds a bandaid to the underlying bug. One should never go into this timeout.
There was a problem hiding this comment.
@sandy081 not sure what a good workaround would be here without the timeout. Waiting forever for onDidChangeSessions to be fired seems like we could end up hanging here.
I was seeing the same behavior with _waitForCommittedSession and that was causing some issues
Fixes #306883
Problem
When a session was stopped before the agent created a worktree, it would get permanently stuck in the session list showing "New Session" with "Working..." status. Archive and delete would silently no-op on the stuck entry.
Root cause:
_waitForCommittedSession()and_waitForSessionInCache()created promises that never resolved when a request was cancelled before the session was committed (untitled → real URI swap). Since the commit event would never arrive, the temp session remained in the cache indefinitely. Meanwhile,archiveSession()anddeleteSession()relied on_findAgentSession()which returnsundefinedfor uncommitted temp sessions, causing them to silently no-op.Fix
_waitForCommittedSessionwithresponseCompletePromise— races the commit event against response completion. In normal flow the commit fires during request processing (before the response finishes), socommitPromisealways wins. If the response finishes first (cancelled before worktree creation), throws immediately so the caller's catch block cleans up the temp session.DisposableStorecleanup to both wait helpers — prevents leaked event listeners on the hanging path._waitForSessionInCacheuses a 10s safety timeout since the adapter should appear almost immediately.archiveSession()anddeleteSession()now fall through to_cleanupTempSession()when_findAgentSession()returns nothing, properly removing uncommitted sessions from the cache and firing the removal event.