Set HTTP_PROXY and HTTPS_PROXY from user / OS config#324209
Merged
Merged
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR adds proxy-resolution plumbing to the agent host so the Copilot SDK subprocess can inherit an accurate HTTP_PROXY / HTTPS_PROXY derived from VS Code’s proxy configuration (settings/env) and, when local, the Electron session (system proxy/PAC).
Changes:
- Add a reverse-RPC channel (renderer → agent host) to resolve proxies via
IRequestService.resolveProxy, plus a node-sideAgentHostProxyResolverthat feeds this into@vscode/proxy-agent. - Extend
CopilotAgentto resolve the effective CAPI endpoint host (token-discovered) and injectHTTP_PROXY/HTTPS_PROXYinto the SDK subprocess env; restart the client when a token change would change the resolved proxy. - Extend
ICopilotApiServicewithresolveApiEndpoint()and update affected tests/fakes accordingly.
Show a summary per file
| File | Description |
|---|---|
| src/vs/platform/agentHost/test/node/copilotAgent.test.ts | Updates test DI wiring for the new proxy resolver dependency. |
| src/vs/platform/agentHost/test/node/codex/codexProxyService.test.ts | Updates fake ICopilotApiService to include resolveApiEndpoint. |
| src/vs/platform/agentHost/test/node/claudeProxyService.test.ts | Updates fake/wrapped ICopilotApiService shapes to include resolveApiEndpoint. |
| src/vs/platform/agentHost/test/node/claudeAgent.test.ts | Updates fake ICopilotApiService to include resolveApiEndpoint. |
| src/vs/platform/agentHost/test/node/claudeAgent.integrationTest.ts | Updates stub ICopilotApiService to include resolveApiEndpoint. |
| src/vs/platform/agentHost/test/node/agentService.test.ts | Updates fake ICopilotApiService to include resolveApiEndpoint. |
| src/vs/platform/agentHost/test/node/agentHostSessionTitleController.test.ts | Updates fake ICopilotApiService to include resolveApiEndpoint. |
| src/vs/platform/agentHost/test/node/agentHostPullRequestOperationHandler.test.ts | Updates fake ICopilotApiService to include resolveApiEndpoint. |
| src/vs/platform/agentHost/test/node/agentHostCommitOperationHandler.test.ts | Updates fake ICopilotApiService to include resolveApiEndpoint. |
| src/vs/platform/agentHost/test/electron-browser/localAgentHostService.test.ts | Updates channel-registration expectations to include the new proxy channel. |
| src/vs/platform/agentHost/node/shared/copilotApiService.ts | Adds resolveApiEndpoint() and caches discovered endpoints.api per token. |
| src/vs/platform/agentHost/node/copilot/copilotAgent.ts | Resolves CAPI proxy and forwards it via HTTP_PROXY/HTTPS_PROXY; restarts client if proxy changes on token update. |
| src/vs/platform/agentHost/node/agentHostServerMain.ts | Registers IAgentHostProxyResolver in the server DI container. |
| src/vs/platform/agentHost/node/agentHostProxyResolver.ts | New node-side resolver that uses @vscode/proxy-agent + optional host proxy resolution via reverse channel. |
| src/vs/platform/agentHost/node/agentHostMain.ts | Wires the reverse proxy channel per renderer connection and registers the resolver in DI. |
| src/vs/platform/agentHost/electron-browser/localAgentHostService.ts | Registers the renderer-side proxy-resolution server channel. |
| src/vs/platform/agentHost/common/agentHostClientProxyChannel.ts | New IPC channel wrapper + renderer-side IServerChannel implementation for resolveProxy. |
Review details
Comments suppressed due to low confidence (1)
src/vs/platform/agentHost/node/agentHostMain.ts:148
- The BYOK gating comment now mentions “proxy” in a way that’s ambiguous/misleading given this file also wires a separate proxy-resolution channel unconditionally. The comment should clarify it’s referring to the BYOK LM bridge/proxy behavior, not the new AgentHostProxyResolver.
let proxyResolver: AgentHostProxyResolver | undefined;
// Gate BYOK *use* behind the opt-in `chat.agentHost.byokModels.enabled`
// setting, forwarded from the renderer as an env var. The proxy and bridge
// registry are always constructed below (so the session launcher can inject
// them), but when off they stay inert: the per-connection bridge and the
- Files reviewed: 17/17 changed files
- Comments generated: 1
- Review effort level: Low
0c46b69 to
901b467
Compare
hediet
approved these changes
Jul 3, 2026
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.
No description provided.