Skip to content

http-client: expose WebSocket proxy prerequisites#31342

Open
bolinfest wants to merge 1 commit into
mainfrom
pr31342
Open

http-client: expose WebSocket proxy prerequisites#31342
bolinfest wants to merge 1 commit into
mainfrom
pr31342

Conversation

@bolinfest

@bolinfest bolinfest commented Jul 7, 2026 •
edited
Loading

Copy link
Copy Markdown
Collaborator

Why

#31335 lets HTTP callers obtain proxy-aware clients from HttpClientFactory, but a non-HTTP transport such as WebSockets also needs two pieces of policy owned by codex-http-client: a concrete route decision for its destination and the same custom-CA-aware rustls trust configuration used by HTTPS.

Keeping these prerequisites in the shared abstraction means the dependent Responses WebSocket change (#31441) cannot independently reinterpret features.respect_system_proxy, PAC results, or enterprise CA settings.

What changed

  • Add a redaction-safe OutboundProxyRoute with explicit transport-default, direct, and concrete-proxy outcomes.
  • Add HttpClientFactory::resolve_proxy_route() so transports can resolve a destination through the already-selected outbound proxy policy.
  • Resolve ws:// and wss:// URLs through their HTTP equivalents so system and PAC rules apply consistently.
  • Add an always-returned rustls config builder that starts from native roots and layers in any configured Codex custom CA bundle. The existing optional builder remains available to callers that can delegate the default configuration to their transport.
  • Continue redacting proxy URLs from Debug output because they may contain credentials.

Review guide

  1. http-client/src/outbound_proxy.rs defines the transport-neutral route result and WebSocket URL normalization.
  2. http-client/src/custom_ca.rs factors the native-root/custom-CA construction so callers that perform TLS themselves can always obtain a config.
  3. http-client/src/outbound_proxy_tests.rs verifies WebSocket normalization and legacy transport-default behavior.

Test plan

  • just test -p codex-http-client outbound_proxy
  • just test -p codex-http-client custom_ca

Stack created with Sapling. Best reviewed with ReviewStack.

@bolinfest bolinfest changed the base branch from main to pr31335 July 7, 2026 02:45
@bolinfest bolinfest mentioned this pull request Jul 7, 2026
Merged
Base automatically changed from pr31335 to main July 7, 2026 03:49
bolinfest added a commit that referenced this pull request Jul 7, 2026
@bolinfest
core: route Responses API through system proxy (#31335)
6afcf26 
## 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.
- Add an integration test that loads `features.respect_system_proxy`
through `config.toml`, creates a real Codex session, and verifies that
both a normal Responses turn and remote compaction reach an isolated
local proxy.

## 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/tests/suite/responses_api_system_proxy.rs` is the behavioral
regression boundary. Its Linux subprocess deliberately sets the CGI
marker that disables reqwest's implicit environment-proxy handling, so
the test fails if session wiring or either Responses call site falls
back to the default client.

## 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.
- New Linux integration coverage:
`responses_and_compact_use_enabled_system_proxy`
- `just bazel-lock-check`

---
[//]: # (BEGIN SAPLING FOOTER)
Stack created with [Sapling](https://sapling-scm.com). Best reviewed
with [ReviewStack](https://reviewstack.dev/openai/codex/pull/31335).
* #31342
* __->__ #31335
@bolinfest bolinfest marked this pull request as ready for review July 7, 2026 03:54
@bolinfest bolinfest requested a review from a team as a code owner July 7, 2026 03:54
@bolinfest bolinfest requested a review from anp-oai July 7, 2026 03:54
@github-actions github-actions Bot mentioned this pull request Jul 7, 2026
Open
@bolinfest bolinfest removed the request for review from anp-oai July 7, 2026 04:21
This was referenced Jul 7, 2026
Draft
Draft
Draft
@bolinfest bolinfest marked this pull request as draft July 7, 2026 04:58
@bolinfest bolinfest mentioned this pull request Jul 7, 2026
Draft
@bolinfest bolinfest changed the title core: use HTTP Responses with system proxy core: preserve Responses WebSockets with system proxy Jul 7, 2026
This was referenced Jul 7, 2026
Draft
Draft
@bolinfest bolinfest changed the title core: preserve Responses WebSockets with system proxy http-client: expose WebSocket proxy prerequisites Jul 7, 2026
@bolinfest bolinfest requested review from anp-oai and pakrym-oai July 7, 2026 20:26
@bolinfest bolinfest marked this pull request as ready for review July 7, 2026 20:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@pakrym-oai pakrym-oai pakrym-oai approved these changes
@anp-oai anp-oai anp-oai approved these changes

Assignees

No one assigned

Labels

code-reviewed

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@bolinfest @pakrym-oai @anp-oai