Skip to content

core: route Responses API through system proxy#31335

Merged
bolinfest merged 1 commit into
mainfrom
pr31335
Jul 7, 2026
Merged

core: route Responses API through system proxy#31335
bolinfest merged 1 commit into
mainfrom
pr31335

Conversation

@bolinfest

@bolinfest bolinfest commented Jul 7, 2026

Copy link
Copy Markdown
Collaborator

Why

features.respect_system_proxy already routes authentication traffic through the OS proxy APIs, but it does not affect the primary inference path. That leaves users behind OS-managed proxies unable to send normal Responses API requests even after login succeeds.

This PR is the first product-path migration onto the route-aware transport introduced in #31323 and refined in #31331. It also establishes the construction pattern for later migrations: the effective feature state is resolved once into a required HTTP client factory rather than represented by an optional per-call setting.

The scope remains limited to the two HTTP Responses endpoints; WebSockets, model discovery, memories, realtime, and file uploads remain follow-up migrations.

What changed

  • Replace the optional proxy marker with an explicit OutboundProxyPolicy::{ReqwestDefault, RespectSystemProxy} and a required HttpClientFactory. The policy has no default, and the lower-level route-aware reqwest builder is now private.
  • Have Config construct the factory from the effective feature state and require every ModelClient constructor to receive it. There is no optional setter or implicit None fallback.
  • Build HTTP clients for /responses and /responses/compact with ClientRouteClass::Api, using the complete destination URL so PAC rules can make URL-specific decisions.
  • Layer route-aware selection onto Codex's existing default headers, Cloudflare cookie store, custom CA handling, and sandbox no-proxy behavior.
  • Extend config coverage to assert that enabling the feature produces a factory with OutboundProxyPolicy::RespectSystemProxy.

Review guide

  1. http-client/src/outbound_proxy.rs defines the mandatory policy/factory boundary and keeps route resolution private.
  2. core/src/config/mod.rs, core/src/session/session.rs, and core/src/client.rs show the compile-time invariant: effective config creates the factory, and ModelClient cannot be constructed without one.
  3. login/src/auth/default_client.rs preserves existing default-client behavior while accepting the required factory for migrated routes.
  4. core/src/client.rs switches only streaming Responses and remote compaction HTTP transports to the API route class.
  5. core/src/config/config_tests.rs covers feature-to-factory resolution; http-client/src/outbound_proxy_tests.rs covers routing through an environment-selected proxy.

Test plan

  • cargo check --tests -p codex-http-client -p codex-login -p codex-core
  • just test -p codex-login
  • just test -p codex-core respect_system_proxy_feature_resolves_enabled
  • Existing compact_uses_bearer_after_agent_identity_session_fallback coverage passes with the new transport construction.
  • just bazel-lock-check

Stack created with Sapling. Best reviewed with ReviewStack.

@bolinfest bolinfest changed the base branch from main to pr31331 July 7, 2026 01:31
Base automatically changed from pr31331 to main July 7, 2026 01:34
@bolinfest bolinfest marked this pull request as ready for review July 7, 2026 01:52
@bolinfest bolinfest requested a review from a team as a code owner July 7, 2026 01:52
@bolinfest bolinfest force-pushed the pr31335 branch 4 times, most recently from c36a8e3 to a1bbf86 Compare July 7, 2026 02:37
@bolinfest bolinfest requested review from anp-oai and pakrym-oai July 7, 2026 03:14
return Ok(build_reqwest_client());
}
if is_sandboxed() {
// Preserve the sandbox's existing no-proxy policy; sandboxed command egress is routed

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't need to address here but this seems like something we'll need to reconcile before we make respect_system_proxy the default

@bolinfest bolinfest enabled auto-merge (squash) July 7, 2026 03:14
@bolinfest bolinfest merged commit 6afcf26 into main Jul 7, 2026
51 of 70 checks passed
@bolinfest bolinfest deleted the pr31335 branch July 7, 2026 03:49
@github-actions github-actions Bot locked and limited conversation to collaborators Jul 7, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants