Skip to content

Fix deleting forked session showing wrong confirmation dialog #313205

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
sandy081 merged 1 commit into from
Apr 29, 2026
+9 −5
Merged

Fix deleting forked session showing wrong confirmation dialog #313205

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 9 additions & 5 deletions src/vs/sessions/contrib/copilotChatSessions/browser/copilotChatSessionsProvider.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1464,12 +1464,11 @@ export class CopilotChatSessionsProvider extends Disposable implements ISessions
}

const chatIds = this._getChatIdsInGroup(sessionId);
if (chatIds.length <= 1) {
// Only one chat — delete the entire session
return this.deleteSession(sessionId);
}

// Find the chat matching the URI
// Find the chat matching the URI first, before deciding whether to
// delete the entire session. This prevents accidentally deleting the
// whole session when the grouping cache is stale and chatIds doesn't
// include the chat being closed.
const chatId = chatIds.find(id => {
const chat = this._sessionCache.get(this._localIdFromchatId(id));
return chat && chat.resource.toString() === chatUri.toString();
Expand All @@ -1478,6 +1477,11 @@ export class CopilotChatSessionsProvider extends Disposable implements ISessions
return;
}
Comment on lines 1468 to 1478
Copy link

Copilot AI Apr 29, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The regression fix in deleteChat (returning early when the chat URI isn’t found in the current group) isn’t covered by tests. Given the severity of the original bug (accidentally deleting the whole session/worktree), please add a unit test that calls deleteChat(mainSessionId, forkChatUri) where the fork isn’t grouped under the main session (e.g. due to missing/stale sessionParentId) and asserts that deleteSession is not triggered (model sessions remain unchanged).

Copilot uses AI. Check for mistakes.

if (chatIds.length <= 1) {
// This is the only chat in the session — delete the entire session
return this.deleteSession(sessionId);
}

// Delete the underlying agent session first.
// _refreshSessionCacheMultiChat handles the removed chat gracefully:
// it detects the chat belongs to a group with remaining siblings and
Expand Down
Loading