-
Notifications
You must be signed in to change notification settings - Fork 36.9k
Get chat session transferring working on the ChatSessionStore #283512
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.
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
base: main
Are you sure you want to change the base?
Conversation
|
|
||
| $transferActiveChatSession(toWorkspace: UriComponents): void { | ||
| async $transferActiveChatSession(toWorkspace: UriComponents): Promise<void> { | ||
| const widget = this._chatWidgetService.lastFocusedWidget; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should validate the location
| store.getTransferredSessionData(); | ||
|
|
||
| // Wait for async cleanup | ||
| await new Promise(resolve => setTimeout(resolve, 10)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
delete this AI slop
There was a problem hiding this 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 implements chat session transfer functionality between workspaces by storing session data in a persistent file-based storage system. The implementation moves away from the previous in-memory storage approach to use the ChatSessionStore for managing transferred sessions, which allows sessions to be transferred between VS Code instances/windows.
Key Changes
- Changed the API from synchronous to asynchronous (
transferActiveChatnow returnsThenable<void>) - Simplified the transfer data model to store only
sessionResourceinstead of multiple session state fields - Implemented file-based storage for transferred sessions with expiration handling (5 minutes)
Reviewed changes
Copilot reviewed 14 out of 15 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/vscode-dts/vscode.proposed.interactive.d.ts | Changed transferActiveChat API to return Thenable for async operation |
| src/vs/workbench/contrib/chat/common/chatService.ts | Updated IChatTransferredSessionData interface to use sessionResource instead of sessionId and location |
| src/vs/workbench/contrib/chat/common/chatServiceImpl.ts | Refactored to use ChatSessionStore for transfer operations, removed old storage-based implementation |
| src/vs/workbench/contrib/chat/common/chatSessionStore.ts | Implemented storeTransferSession, getTransferredSessionData, readTransferredSession, and cleanupTransferredSession methods |
| src/vs/workbench/contrib/chat/common/chatTransferService.ts | Made helper methods private (deleteWorkspaceFromTransferredList, isChatTransferredWorkspace) |
| src/vs/workbench/contrib/chat/browser/chatViewPane.ts | Updated to use sessionResource instead of sessionId/inputState for transferred sessions |
| src/vs/workbench/contrib/chat/browser/actions/chatActions.ts | Fixed timing issue by moving setInput after model restoration to prevent clearing |
| src/vs/workbench/contrib/chat/test/common/chatSessionStore.test.ts | Added comprehensive tests for session transfer functionality |
| src/vs/workbench/api/browser/mainThreadChatAgents2.ts | Updated to use simplified transfer API with sessionResource |
| src/vs/workbench/api/common/extHostChatAgents2.ts | Changed transferActiveChat to async |
| src/vs/workbench/api/common/extHost.protocol.ts | Updated protocol to return Promise |
| src/vs/workbench/api/common/extHost.api.impl.ts | Updated API implementation return type |
| src/vs/workbench/contrib/chat/test/common/mockChatModel.ts | Added mutable properties needed for testing (sessionId, customTitle, etc.) |
| src/vs/workbench/contrib/chat/test/common/mockChatService.ts | Updated mock to async method signature |
| src/vs/workbench/contrib/chat/test/browser/localAgentSessionsProvider.test.ts | Updated mock to async method signature |
| this.cleanupTransferredSession(revivedTransferData.sessionResource); | ||
| return undefined; | ||
| } | ||
| return !!LocalChatSessionUri.parseLocalSessionId(revivedTransferData.sessionResource) && revivedTransferData.sessionResource; |
Copilot
AI
Dec 15, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The expression uses a boolean AND operator with the URI, which produces unexpected behavior. If parseLocalSessionId returns null/undefined, the result is false (not undefined). If it returns a truthy string, the result is the URI (not true/false). Consider returning either "revivedTransferData.sessionResource" directly after validating it's a local session URI, or restructuring this check for clarity.
| return !!LocalChatSessionUri.parseLocalSessionId(revivedTransferData.sessionResource) && revivedTransferData.sessionResource; | |
| return LocalChatSessionUri.parseLocalSessionId(revivedTransferData.sessionResource) | |
| ? revivedTransferData.sessionResource | |
| : undefined; |
No description provided.