Skip to content

Sessions: preserve cancelled session content in list#307684

Merged
osortega merged 2 commits intomainfrom
copilot/orthodox-crawdad
Apr 3, 2026
Merged

Sessions: preserve cancelled session content in list#307684
osortega merged 2 commits intomainfrom
copilot/orthodox-crawdad

Conversation

@osortega
Copy link
Copy Markdown
Contributor

@osortega osortega commented Apr 3, 2026

Problem

When a session was stopped before the agent committed a worktree, it was removed from the sessions list entirely. This meant the user lost access to whatever content the agent had already produced — chat messages, partial responses, etc.

Fix

Instead of removing cancelled-before-commit sessions, keep them in the list with Completed status so the user can still click on them and review the chat content.

Changes

  • _waitForCommittedSession — throws CancellationError (instead of generic Error) when the response is cancelled before commit, enabling the caller to distinguish cancellation from unexpected failures.
  • _sendFirstChat catch block — on CancellationError, keeps the temp session in the cache with Completed status and fires a changed event instead of removing it. Other errors still clean up as before.
  • _sendSubsequentChat catch block — same pattern for the multi-chat path.

Tests

  • stopping a committed session keeps it in the list — verifies a session committed mid-request persists after cancellation.
  • cancelling the request before commit keeps the session with completed status — verifies cancelled sessions stay in the list with a changed event (not removed).

When a session is stopped before the agent commits a worktree, keep it
in the sessions list with Completed status instead of removing it. This
lets the user review whatever content the agent produced before
cancellation.

- Use CancellationError in _waitForCommittedSession to distinguish
  cancellation from unexpected failures
- On CancellationError in _sendFirstChat/_sendSubsequentChat, set
  Completed status and fire a changed event instead of removing
- Add tests for committed-then-stopped and cancelled-before-commit flows

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings April 3, 2026 18:15
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR updates the Copilot Chat Sessions provider to preserve sessions that are cancelled before they’ve been committed (worktree/URI swap), so users can still reopen the session and review the chat content instead of losing it.

Changes:

  • Switch pre-commit cancellation signaling to use CancellationError so callers can distinguish cancellation from unexpected failures.
  • On CancellationError, keep the temp session cached, mark it Completed, and emit a changed event instead of removing it.
  • Extend/adjust browser tests to validate “kept in list” behavior for both pre-commit cancellation and post-commit stopping.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
src/vs/sessions/contrib/copilotChatSessions/browser/copilotChatSessionsProvider.ts Handles pre-commit cancellation by throwing CancellationError and preserving the temp session instead of deleting it.
src/vs/sessions/contrib/copilotChatSessions/test/browser/copilotChatSessionsProvider.test.ts Adds tests for “stop after commit” and “cancel before commit keeps session” behaviors, plus commit-event controllable test setup.
Comments suppressed due to low confidence (1)

src/vs/sessions/contrib/copilotChatSessions/test/browser/copilotChatSessionsProvider.test.ts:839

  • The test name says the cancelled-before-commit session is kept “with completed status”, but the assertions only check that the session remains and that a changed event fired. Add an assertion that the kept session’s status is SessionStatus.Completed to actually cover the new behavior.
			await sendPromise;

			assert.strictEqual(provider.getSessions().length, 1, 'session should stay in list after cancellation');
			assert.ok(
				changes.some(e => e.changed.some(s => s.sessionId === sessionId)),
				'changed event should have fired',
			);

…variable

- Move isCanceled check from the early-exit path to after the 5s
  safety timeout so a late commit event (user stops after worktree was
  initiated but before IPC finishes) is not missed.
- Remove unused changes variable in committed-session test.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@osortega osortega marked this pull request as ready for review April 3, 2026 18:59
@osortega osortega enabled auto-merge (squash) April 3, 2026 19:05
@osortega osortega merged commit 3c60b09 into main Apr 3, 2026
19 checks passed
@osortega osortega deleted the copilot/orthodox-crawdad branch April 3, 2026 19:30
@vs-code-engineering vs-code-engineering bot added this to the 1.115.0 milestone Apr 3, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants