agentHost: resolve per-chat effective working directory in session projection#323950
Merged
Conversation
…on param PART 1: remove the unreachable claude.chats legacy peer-chat migration (listLegacyChats, _readPersistedChats, _META_CHATS and the two dead fallback call sites). Copilot's copilot.chats migration and the orchestrator's generic migration are unchanged. PART 2A: drop the redundant session parameter from IAgentChats.createChat and fork. Each agent derives the parent session from the chat URI. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Restructure the shared AgentSessionEntry so a session's default (main) chat lives in the SAME per-session chat map as its peers, keyed by its default-chat URI. Send/abort/model/agent/history now resolve any chat via one uniform getChat lookup with no default-vs-peer storage split. - agentPeerChats.ts: single _chats map + _defaultChatKey; getChat is the uniform lookup; defaultChat/setDefaultChat/clearDefaultChat replace the separate default-session slot; peer accessors read the same map. - Claude/Copilot seed the default chat into the map (setDefaultChat) and read it via defaultChat; _findChat is a single uniform lookup. - Two lifecycle guards kept at the session boundary: createSession seeds the default entry; disposeChat/createChat refuse the default chat. - Codex (single-chat) unchanged; still resolves through the session URI. No protocol/state changes; behavior identical (Agent* suites green). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…ojection Generalize the mergeSessionWithDefaultChat projection so a chat's resolved session-context view (working directory, active clients, config, customizations/MCP scope, …) applies to ALL chats, not just the default one. The projection already spreads the full SessionState onto every chat, but it ignored ChatState.workingDirectory — a per-chat override the protocol allows so a subordinate/peer chat can run in its own git worktree. Operational consumers (changeset/commit/discard/git/PR/sync services + agentService) read the effective cwd via getSessionState(...)?.workingDirectory, so a chat with its own worktree incorrectly reported the session default. Layer chat.workingDirectory over the session default in the projection so no code path has to walk back to the session. Pure orchestrator-side projection: no wire-protocol, IAgent, or persisted-shape change. Docs generalized from "default chat" to per-chat effective context. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This pull request improves the agent host’s per-chat “effective session context” projection by ensuring a chat-level working directory override is reflected in the merged session+chat view. It also updates the multi-chat agent surface to be chat-addressed (deriving the parent session from the chat URI) and adjusts agent implementations, tests, and architecture docs accordingly.
Changes:
- Fix
mergeSessionWithDefaultChatto layerChatState.workingDirectoryoverSessionState.workingDirectory, making the projection reflect the effective per-chat working directory. - Update
IAgentChats.createChat/forkto be chat-addressed (drop explicitsessionparameter) and adapt agents + orchestrator plumbing to the new signature. - Add unit tests covering working directory override vs fallback behavior.
Show a summary per file
| File | Description |
|---|---|
| src/vs/platform/agentHost/test/node/mockAgent.ts | Updates mock agent chat APIs to match the new chat-addressed IAgentChats shape. |
| src/vs/platform/agentHost/test/node/copilotAgent.test.ts | Updates Copilot agent tests to use chat-addressed create/fork and revised session-entry structure. |
| src/vs/platform/agentHost/test/node/claudeAgent.test.ts | Updates Claude agent tests for chat-addressed create/fork and default-chat entry changes. |
| src/vs/platform/agentHost/test/node/agentService.test.ts | Updates dispatcher tests to derive session from chat URIs for create/fork assertions. |
| src/vs/platform/agentHost/test/node/agentHostStateManager.test.ts | Adds tests verifying per-chat working directory override behavior in session projection. |
| src/vs/platform/agentHost/node/copilot/copilotAgent.ts | Refactors Copilot agent chat creation/forking to derive session from chat URI and unifies chat storage access. |
| src/vs/platform/agentHost/node/codex/codexAgent.ts | Adjusts Codex agent to the new IAgentChats signature (still single-chat only). |
| src/vs/platform/agentHost/node/claude/claudeAgent.ts | Refactors Claude agent to derive session from chat URI, unify chat lookup, and drop legacy peer-chat catalog reading. |
| src/vs/platform/agentHost/node/agentService.ts | Updates orchestrator-side chat creation plumbing to call providers with chat URI only. |
| src/vs/platform/agentHost/node/agentPeerChats.ts | Refactors shared per-session chat container to a single uniform chat map (default + peers). |
| src/vs/platform/agentHost/MULTI_CHAT_ARCHITECTURE.md | Updates architecture documentation to reflect chat-addressed calls and legacy migration expectations. |
| src/vs/platform/agentHost/common/state/sessionState.ts | Fixes session+chat projection to apply chat working directory override; updates docs accordingly. |
| src/vs/platform/agentHost/common/agentService.ts | Updates IAgentChats interface to remove explicit session parameter for create/fork. |
Review details
- Files reviewed: 13/13 changed files
- Comments generated: 2
- Review effort level: Low
…-per-chat-context # Conflicts: # src/vs/platform/agentHost/MULTI_CHAT_ARCHITECTURE.md
Address CCR feedback: createChat/fork resolved the session via
_resolveChatTarget but passed the original chat argument, so a default-chat
call (session URI) would use the session URI instead of the resolved
default-chat channel URI. Destructure and pass the resolved { session, chat }
to match the other chat-addressed methods in the mock.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
TylerLeonhardt
approved these changes
Jul 1, 2026
TylerLeonhardt
left a comment
TylerLeonhardt
left a comment
Member
There was a problem hiding this comment.
Approving from a security perspective to not slow down but there's feedback I'll give async
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Task mc-CHATADDR-T7 — per-chat resolved session-context view
Orthogonal cleanup: every chat handle should expose its effective session context (resolved working directory, active client, auth/MCP + customization scope, config) so no operational code path has to walk back to the session to read shared state.
Finding
The per-chat context projection already largely exists:
mergeSessionWithDefaultChat(session, chat)spreads the fullSessionStateonto each chat and is already used for default, peer, and subagent chats (AgentHostStateManager.getSessionState, and the workbench handler). BecauseSessionStateownsactiveClients/config/customizations(no per-chat override), those are already resolved correctly for every chat.Delta: the protocol lets a chat override the session working directory (
ChatState.workingDirectory, e.g. to give a subordinate chat its own git worktree), but the projection did...sessionand never layered that override. Operational consumers read the effective cwd through this view —agentHostChangesetService,agentHostCommitOperationHandler,agentHostDiscardChangesOperationHandler,agentHostGitStateService,agentHostPullRequestOperationHandler,agentHostSyncOperationHandler, andagentServiceall readgetSessionState(...)?.workingDirectory— so a chat with its own worktree incorrectly reported the session default.Change
mergeSessionWithDefaultChatnow resolvesworkingDirectory: chat?.workingDirectory ?? session.workingDirectory, making it a true per-chat effective session-context projection for all chats.ISessionWithDefaultChatfrom "default chat" to per-chat effective context.Pure orchestrator-side projection — no wire-protocol,
IAgent, or persisted-shape change; confined tosrc/vs/platform/agentHost.Validation
node build/next/index.ts transpile— pass./scripts/test.sh --grep "AgentService|AgentHostStateManager|CopilotAgent|ClaudeAgent|CodexAgent"— 640 passing (3 new)npm run typecheck-client— passnpm run valid-layers-check— passCo-authored-by: Copilot 223556219+Copilot@users.noreply.github.com