-
Notifications
You must be signed in to change notification settings - Fork 37.3k
Migrate to chat session URI in terminal tool #286826
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
Conversation
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 migrates the terminal chat tool integration from using string-based session IDs to URI-based session resources. This change provides a more consistent and type-safe way to reference chat sessions across the codebase, aligning with the broader architecture for handling chat session references.
Key Changes
- Replaced
Map<string, IToolTerminal>withResourceMap<IToolTerminal>for session-terminal associations - Updated all chat session ID parameters to use
URI(chatSessionResource) instead ofstring(chatSessionId) - Maintained backward compatibility in storage layer by converting URIs to string IDs when persisting
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
runInTerminalTool.ts |
Core implementation migrated to use ResourceMap and URI-based session resources; backward compatibility maintained in storage |
terminalChatService.ts |
Service layer updated to use ResourceMap for session tracking; added new getChatSessionResourceForInstance method |
terminal.ts |
Interface definitions updated; deprecated getChatSessionIdForInstance in favor of getChatSessionResourceForInstance |
terminalChatActions.ts |
Command handler updated to accept URI or string and convert appropriately |
commandLineAnalyzer.ts |
Interface updated to use chatSessionResource instead of chatSessionId |
commandLineAutoApproveAnalyzer.ts |
Updated to pass URI instead of string to approval methods |
commandLineAutoApprover.ts |
Parameters updated to accept URI instead of string |
chatTerminalToolConfirmationSubPart.ts |
Updated to use element.sessionResource instead of element.sessionId |
languageModelToolsService.ts |
Added chatSessionResource to IToolInvocationPreparationContext; marked chatSessionId as deprecated; passed through in tool service |
runInTerminalTool.test.ts |
Tests updated to use LocalChatSessionUri.forSession for creating session resources |
commandLineFileWriteAnalyzer.test.ts |
Tests updated to use chatSessionResource: undefined |
Part of #274403
@meganrogge any test cases in particular I should do to make sure this all works that come to mind?