Repository navigation
agentHost: preserve Codespaces tokens in remote sessions - #340566
Conversation
Forward resolver-provided variables to server-owned remote Agent Host launches so shell tools inherit Codespaces credentials. Keep overrides outside AHP, preserve null removals and launcher-owned values, and cover both editor and Agents windows. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
🟡 Changes recommended
Concurrent connections can discard the later environment snapshot and launch with stale credentials.
1 open finding
What changed in this PR
Forwards remote resolver environments to server-owned Agent Host launches, preserving Codespaces credentials across launches and crash recovery.
Changes:
- Adds validated IPC launch environment options.
- Applies resolver overrides with appropriate precedence and null handling.
- Adds coverage for forwarding, failures, privacy, and restarts.
| File | Description |
|---|---|
src/vs/workbench/services/agentHost/browser/editorRemoteAgentHostServiceClient.ts |
Resolves and forwards remote environments. |
src/vs/workbench/services/agentHost/test/browser/editorRemoteAgentHostServiceClient.test.ts |
Tests both window types and reconnections. |
src/vs/platform/agentHost/common/agentService.ts |
Defines IPC connection options. |
src/vs/platform/agentHost/browser/agentHostIpcChannelTransport.ts |
Sends launch options during connection. |
src/vs/platform/agentHost/test/browser/agentHostIpcChannelTransport.test.ts |
Tests resolution, disposal, errors, and privacy. |
src/vs/server/node/agentHostChannel.ts |
Validates and relays launch options. |
src/vs/server/test/node/agentHostChannel.test.ts |
Tests validation and forwarding behavior. |
src/vs/server/node/serverServices.ts |
Applies overrides to lazy server launches. |
src/vs/platform/agentHost/node/nodeAgentHostStarter.ts |
Retains and merges environment snapshots. |
src/vs/platform/agentHost/test/node/nodeAgentHostStarter.test.ts |
Tests precedence, removals, and restart retention. |
🧠 Review effort: Balanced
💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Resolve each renderer connection's environment independently so a concurrent connection cannot discard refreshed Codespaces credentials. Keep process startup coalescing in ServerAgentHostManager and cover absent and stale initial tokens. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Connor Peet (connor4312)
left a comment
There was a problem hiding this comment.
Reviewed the complete merge-base diff and the environment flow through resolver, renderer transport, server channel, and process launch/recovery. One P2 finding: the new starter test introduces process-global configuration registration that breaks other unit suites. This explains the two Electron-Unit failures on Linux, macOS, and Windows; they are not unrelated CI failures.
The production forwarding approach looks reasonable, and the relevant IPC, starter, channel, and client regressions pass in CI. No P1 issues found. Please address the test isolation issue before approval.
Keep production configuration registration out of NodeAgentHostStarter imports so its unit tests do not contaminate configuration-sync and account-policy suites. Register the contribution in the remote server bootstrap before starter construction, and document that ownership. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Resolve the server bootstrap import conflict while preserving the upstream GitHub environment initialization and the configuration-registration isolation fix. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Keep transient resolver failures on the buffered IPC connection without replacing the saved server environment. Forward debug and resolver layers separately, merge them with native server casing, and preserve deletion markers through the child-process boundary. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
Addressed both additional regression findings in cb16f29:
New tests cover the buffered connection and later resolver recovery, separate-layer forwarding/validation, native host casing, and real Windows child-process replacement, empty-string, and token-removal behavior. No real credentials are read or printed by those probes. |
Fixed issues
Connor Peet (connor4312)
left a comment
There was a problem hiding this comment.
Reviewed the complete diff and relevant remote source context for security and correctness. No substantiated P1 or P2 findings remain. The prior concurrent-environment and configuration-registration issues are addressed in the current head; cross-platform Electron-Unit and compile/hygiene checks pass.
Written by Connor's agent running GPT-6.1 Sol

Summary
Forward the remote resolver's environment to server-owned Agent Host launches so shell tools can inherit Codespaces-provisioned credentials such as
GITHUB_TOKEN.Normal remote terminals and the extension host already merge
extensionHostEnv. The remote Agent Host connection did not forward it, leaving its child processes without variables supplied through that route. This affects both editor chat and the Agents window; changing the default harness can expose the gap but is not required to hit it.Changes
connectIPC call, outside AHP messages and logs.