Repository navigation
Add the full PAC-style result to resolveProxyByURL - #128
Merged
Merged
Conversation
resolveProxyByURL's url and type describe only the first entry of a resolver result, so callers that hand proxies on to another resolver, such as the Copilot runtime's resolveProxy callback, lose the fallback entries of a PAC result like 'PROXY proxy1:8080; PROXY proxy2:8080; DIRECT'. The new result field keeps them. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Like Electron's session.resolveProxy, whose results never include userinfo, the result now drops the credentials of a proxy from the http.proxy setting or an environment variable. Consumers that hand the result to other PAC-style resolvers then never pass credentials on. The url field keeps them for connecting through the proxy. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Only proxies from the http.proxy setting or an environment variable differ from what Electron's session.resolveProxy returns, so only these are rewritten, and system results pass through unchanged. They now appear as Chromium formats PAC results: PROXY, HTTPS, SOCKS5 or SOCKS (SOCKSv4) with host:port, the scheme's default port when none is given, and no credentials, path, query or fragment. A malformed one becomes DIRECT, as Chromium drops malformed proxies. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
mocha 12 requires Node ^20.19.0 || >=22.12.0 because its CLI loads an ES module with require(), so the integration tests failed on node:20.18.1 with ERR_REQUIRE_ESM. On Node 22, the test client's kerberos 2.0.1 does not compile (node-addon-api 4.3.0), so it moves to kerberos 2.2.2 (node-addon-api 6.1.0) with the same API. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Christof Marti (chrmarti)
marked this pull request as ready for review
November 4, 2026 17:01
Christof Marti (chrmarti)
enabled auto-merge
November 4, 2026 17:01
Dirk Bäumer (dbaeumer)
approved these changes
Nov 4, 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.
resolveProxyByURLnow also returnsresult: the full PAC-style resolver result, of whichurlandtypedescribe only the first entry they recognize. For example, a PAC returningPROXY proxy1:8080; HTTPS proxy2:8443; DIRECTpreviously reached callers ashttp://proxy1:8080alone.This lets VS Code hand the complete list, including its fallback entries, to resolvers that accept PAC-style results, such as the Copilot runtime's proxy resolution callback used by the agent host, so the runtime can fail over to the next proxy.
resultis the string the resolver chose:DIRECTfor localhost andnoProxymatches, thehttp.proxysetting or proxy environment variable, the system/PAC result, or the cached one. It isundefinedwhen the system resolver returned nothing.session.resolveProxy, pass through unchanged. A proxy from thehttp.proxysetting or an environment variable appears as Electron would return it, so consumers get one format:PROXY,HTTPS,SOCKS5orSOCKS(SOCKSv4, as Chromium names it in PAC results) withhost:port.DIRECT, as Chromium drops malformed proxies.urlis unchanged and keeps any credentials for connecting through the proxy.Tests:
npm run compileandnpm run test:unitpass (38 passing, 1 pending). The existingresolveProxyByURLtests now also checkresult. New tests cover:http.proxyvalues with credentials, default ports, a path, IPv6, SOCKS5, SOCKS4 and a malformed port;@in the password) and a trailing slash;CI fix (separate commit):
main's integration tests have failed since the mocha 12 update (#127) withERR_REQUIRE_ESM, because mocha 12 needs Node^20.19.0 || >=22.12.0and the Docker test clients rannode:20.18.1. The test clients now runnode:22.23.4. On Node 22 the test client'skerberos2.0.1 doesn't compile (node-addon-api4.3.0), so it moves tokerberos2.2.2 (node-addon-api6.1.0, same API). Verified locally withdocker compose run --rm test-direct-client(34 passing, 1 pending) andtest-proxy-client(39 passing).