Skip to content

core: route realtime and memories through HTTP client factory#31362

Draft
bolinfest wants to merge 1 commit into
pr31361from
pr31362
Draft

core: route realtime and memories through HTTP client factory#31362
bolinfest wants to merge 1 commit into
pr31361from
pr31362

Conversation

@bolinfest

@bolinfest bolinfest commented Jul 7, 2026

Copy link
Copy Markdown
Collaborator

Why

ModelClient already carries the HttpClientFactory resolved from session configuration, but realtime call creation and memory summarization still constructed the legacy default client directly. Consequently, those first-party API requests could ignore features.respect_system_proxy even when Responses traffic honored it.

These are the final direct default-client constructions in core/src/client.rs, so they form one small migration unit on top of #31361.

What changed

  • Generalize build_responses_transport to build_api_transport.
  • Route realtime call creation through the helper using the selected provider and /realtime/calls destination.
  • Route /memories/trace_summarize through the same helper.
  • Remove the now-unused direct build_reqwest_client import.

Review guide

  1. The helper rename at the bottom of core/src/client.rs is mechanical and keeps existing Responses behavior unchanged.
  2. The realtime call path computes the route from the final provider, including api_provider_override.
  3. The memories path supplies its existing endpoint to the same API route class.

Validation

  • cargo check --tests -p codex-core
  • just fix -p codex-core

Follow-up

Direct HTTP clients outside ModelClient remain separate migration slices.


Stack created with Sapling. Best reviewed with ReviewStack.

@bolinfest bolinfest changed the base branch from main to pr31361 July 7, 2026 04:51
@bolinfest bolinfest force-pushed the pr31361 branch 2 times, most recently from 11a2bda to 1c63ae4 Compare July 7, 2026 17:35
bolinfest added a commit that referenced this pull request Jul 7, 2026
## 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`

---
[//]: # (BEGIN SAPLING FOOTER)
Stack created with [Sapling](https://sapling-scm.com). Best reviewed
with [ReviewStack](https://reviewstack.dev/openai/codex/pull/31342).
* #31431
* #31363
* #31362
* #31361
* #31442
* #31441
* __->__ #31342
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant