Fix deleting sub-session when it is not active#313200
Merged
Merged
Conversation
When a child chat in a multi-chat group was removed, _refreshSessionCache would first remove it from _sessionCache and invalidate grouping caches. Then _refreshSessionCacheMultiChat would try to resolve the removed chat's group ID, but since the caches were already rebuilt without the removed chat, _getGroupIdForChat fell back to the chat's own ID instead of the parent group's ID. This caused the child to be treated as a 'truly removed' standalone session rather than a group membership change, so no 'changed' event was fired for the parent group. Fix: resolve group IDs for removed sessions BEFORE removing them from the cache and invalidating grouping caches. Fixes #311987 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Fixes a multi-chat session grouping bug in the Agents window where deleting a child/sub-session could be misclassified as removing a standalone session, preventing the parent group from emitting a changed event and potentially causing incorrect UI/session removal.
Changes:
- Pre-computes group IDs for removed sessions before mutating
_sessionCache/ invalidating grouping caches. - Passes the pre-computed removed→groupId mapping into
_refreshSessionCacheMultiChat. - Updates
_refreshSessionCacheMultiChatsignature to accept the removed group ID map.
Show a summary per file
| File | Description |
|---|---|
| src/vs/sessions/contrib/copilotChatSessions/browser/copilotChatSessionsProvider.ts | Preserves correct group resolution for removed child chats by computing group IDs before cache invalidation and wiring that data into multi-chat refresh logic. |
Copilot's findings
- Files reviewed: 1/1 changed files
- Comments generated: 3
alexr00
approved these changes
Apr 29, 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.
When a child chat in a multi-chat group was removed,
_refreshSessionCachewould first remove it from_sessionCacheand invalidate grouping caches. Then_refreshSessionCacheMultiChatwould try to resolve the removed chat's group ID, but since the caches were already rebuilt without the removed chat,_getGroupIdForChatfell back to the chat's own ID instead of the parent group's ID.This caused the child to be treated as a "truly removed" standalone session rather than a group membership change, so no "changed" event was fired for the parent group — which could result in the main session being incorrectly removed.
Fix: Resolve group IDs for removed sessions before removing them from the cache and invalidating grouping caches, then pass the pre-computed map to
_refreshSessionCacheMultiChat.Fixes #311987