Make sure we preserve the right session id/resource for agent calls #275973
+39
−19
Conversation
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
When I fixed the uris used to identify sessions, it revealed this bug for non-local session. The main part of this change is avoiding the call to `LocalChatSessionUri.forSession` in mainThreadChatAgent. This call can't be used for non-local chat sessions The subagents tool part of this change unfortunately makes it a lot larger
Contributor
There was a problem hiding this comment.
Pull Request Overview
This PR adds a sessionResource field to IChatAgentRequest and IToolInvocationContext interfaces, providing URI-based session identification alongside the existing string-based sessionId.
Key changes include:
- Enhanced type safety by changing
parameterstype fromObjecttoRecord<string, any>inIToolInvocation - Added
readonly sessionResource: URIto bothIChatAgentRequestandIToolInvocationContextinterfaces - Updated all creation sites of these objects to include the new
sessionResourcefield
Reviewed Changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| src/vs/workbench/contrib/chat/common/chatAgents.ts | Added sessionResource: URI field to IChatAgentRequest interface |
| src/vs/workbench/contrib/chat/common/languageModelToolsService.ts | Added sessionResource to IToolInvocationContext, improved type from Object to Record<string, any>, updated type guard, and added readonly modifiers |
| src/vs/workbench/contrib/chat/common/chatServiceImpl.ts | Updated IChatAgentRequest creation sites to include sessionResource field using model data |
| src/vs/workbench/contrib/chat/common/tools/runSubagentTool.ts | Updated subagent request to include sessionResource from invocation context |
| src/vs/workbench/contrib/chat/browser/actions/chatExecuteActions.ts | Updated agent history entry creation to include sessionResource field |
| src/vs/workbench/api/common/extHostTypeConverters.ts | Updated toolInvocationToken to include both sessionId and sessionResource with proper typing |
| src/vs/workbench/api/common/extHostLanguageModelTools.ts | Changed parameter type from IToolInvocation to Dto<IToolInvocation> for consistency |
| src/vs/workbench/api/common/extHostChatSessions.ts | Added sessionResource to mock agent request in chat session creation |
| src/vs/workbench/api/common/extHost.protocol.ts | Updated protocol signatures to use Dto<IToolInvocation> for proper serialization |
| src/vs/workbench/api/browser/mainThreadLanguageModelTools.ts | Updated to handle Dto<IToolInvocation> and added revive call for proper deserialization |
| src/vs/workbench/api/browser/mainThreadChatAgents2.ts | Simplified session retrieval to use request.sessionResource directly instead of reconstructing URI |
| src/vs/workbench/contrib/chat/test/browser/languageModelToolsService.test.ts | Updated test helper to construct sessionResource from sessionId using LocalChatSessionUri.forSession |
| src/vs/workbench/api/test/browser/mainThreadChatSessions.test.ts | Updated test mocks to include sessionResource field in agent requests |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Counterpart of #275972 that fixes the issue is a better way for November
The main part of this change is avoiding the call to
LocalChatSessionUri.forSessionin mainThreadChatAgent. This call can't be used for non-local chat sessions. Instead we want to pass around the session's resource so we don't have to try mapping between ids and resources