Conversation
💡 Codex Reviewcodex/codex-rs/login/Cargo.toml Line 23 in 3cff0ef This adds a new direct dependency (and a dev-dependency below), but the commit does not update codex/codex-rs/http-client/src/outbound_proxy.rs Lines 295 to 297 in 3cff0ef When system proxy resolution is unavailable, this branch now resolves ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
Why
When OS proxy resolution is enabled, an unavailable platform resolver must not silently fall back to reqwest's ambient behavior. That would create a second routing decision and make
respect_system_proxydifficult to reason about. Route resolution is also on the request hot path, so cached PAC decisions should not queue behind unrelated lookups or retain signed URLs in process-global state.What changed
NO_PROXYpolicy, or as an explicit direct route when no proxy is configured.HTTPS_PROXY, thenHTTP_PROXY, thenALL_PROXY) when system lookup is unavailable.spawn_blockingon platforms where system resolution is already synchronous and nonblocking.NO_PROXYbehavior remains intact.Review guide
http-client/src/outbound_proxy.rsfor the explicit route state machine, cache ordering, and unavailable-resolver fallback.websocket-client/src/dialer.rsfor the environment-route handoff.Validation
outbound_proxy_tests.rs.NO_PROXYhosts bypass the proxy while nonmatching hosts use it.Stack created with Sapling. Best reviewed with ReviewStack.