Skip to content

codex-api: route file uploads through HTTP client factory#31363

Merged
bolinfest merged 1 commit into
mainfrom
pr31363
Jul 9, 2026
Merged

codex-api: route file uploads through HTTP client factory#31363
bolinfest merged 1 commit into
mainfrom
pr31363

Conversation

@bolinfest

@bolinfest bolinfest commented Jul 7, 2026 •
edited
Loading

Copy link
Copy Markdown
Collaborator

Why

Codex Apps file parameters use a three-step upload flow: create a file record, PUT bytes to a returned signed URL, and finalize the upload. Each step still constructed a default reqwest client, so the flow could bypass features.respect_system_proxy even after model API requests honored it.

This stack entry makes the resolved client policy a required input to the upload API and resolves each concrete destination independently.

What changed

  • Require HttpClientFactory in upload_openai_file.
  • Build clients for the create, signed upload, and finalize URLs through the shared API route policy.
  • Pass the factory derived from the turn configuration at the Apps/MCP call site.
  • Return a destination-aware ClientBuild error when enabled route selection cannot construct a client.
  • Preserve the legacy logged fallback for the feature-off ReqwestDefault policy.

Review guide

  1. codex-api/src/files.rs changes the upload API and centralizes route-aware client construction.
  2. The three request stages each supply their actual URL, including the separately hosted signed blob URL.
  3. core/src/mcp_openai_file.rs is the only production caller and supplies the turn configuration factory.

Validation

  • cargo check --tests -p codex-api -p codex-core
  • just test -p codex-api files (1 matching upload test passed; 135 tests skipped by filter)
  • just fix -p codex-api -p codex-core

Follow-up

Other direct HTTP clients remain separate migration slices.

Stack created with Sapling. Best reviewed with ReviewStack.

@bolinfest bolinfest changed the base branch from main to pr31362 July 7, 2026 04:55
This was referenced Jul 7, 2026
Merged
Merged
Merged
@bolinfest bolinfest mentioned this pull request Jul 7, 2026
Merged
@bolinfest bolinfest force-pushed the pr31362 branch 2 times, most recently from e1f3794 to 0797b7d Compare July 7, 2026 17:35
This was referenced Jul 7, 2026
Merged
Closed
bolinfest added a commit that referenced this pull request Jul 7, 2026
@bolinfest
http-client: expose WebSocket proxy prerequisites (#31342)
8139255 
## 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
@bolinfest bolinfest mentioned this pull request Jul 8, 2026
Merged
bolinfest added a commit that referenced this pull request Jul 8, 2026
@bolinfest
core: preserve Responses WebSockets with system proxy (#31441)
e621d7d 
## Why

Responses WebSockets are the normal lower-latency transport for
WebSocket-capable providers. They must not bypass an OS-selected proxy
when `features.respect_system_proxy` is enabled, but disabling
WebSockets whenever the feature is enabled would impose a substantial
performance penalty.

Merged PR #31622 introduced the reusable proxy-aware WebSocket
transport. This PR makes the Responses API its first consumer so the
existing fast path uses the same effective proxy and trust policy as
HTTP.

## What changed

- Register `codex-websocket-client` as a workspace dependency and use it
from `codex-api`.
- Feed the shared crate’s route-independent `WebSocketConnection` into
the existing Responses message pump.
- Require a configured `HttpClientFactory` for normal Responses
WebSocket connections and the CLI doctor probe, so neither path can open
a connection without consulting the effective proxy policy.
- Pass the session factory from `core` and the effective configuration
factory from `doctor`.
- Add an end-to-end Responses test that enables `RespectSystemProxy`,
asserts the resolved policy, completes a turn over WebSocket, and
verifies the connection and request counts.
- Keep the existing Responses protocol handling, ping/pong pump, and
session-scoped HTTP fallback unchanged.

The DNS, proxy, TLS, custom-CA, and Happy Eyeballs implementation and
its transport tests live in merged PR #31622. This PR deliberately
contains only the Responses integration and does not duplicate that
transport code.

## Review guide

1. `codex-rs/codex-api/src/endpoint/responses_websocket.rs` constructs
the shared connector and adapts its uniform stream to the existing pump.
2. `codex-rs/core/src/client.rs` supplies the session-scoped factory for
production Responses connections.
3. `codex-rs/cli/src/doctor.rs` supplies the effective configuration
factory to the handshake probe.
4. `codex-rs/core/tests/suite/client_websockets.rs` covers the
enabled-feature path end to end.

## Test plan

- `cargo check --tests -p codex-api -p codex-core -p codex-cli`
- `just test -p codex-api`
- `just test -p codex-core
responses_websocket_streams_with_system_proxy_feature`
- `cargo shear`
- `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/31441).
* #31637
* #31431
* #31363
* #31362
* #31361
* __->__ #31441
@bolinfest bolinfest force-pushed the pr31363 branch 2 times, most recently from 794659a to aade2c7 Compare July 8, 2026 22:31
@bolinfest bolinfest force-pushed the pr31362 branch 2 times, most recently from 962c394 to 0812d7c Compare July 9, 2026 01:24
@bolinfest bolinfest force-pushed the pr31363 branch 2 times, most recently from 5f99de1 to 989b715 Compare July 9, 2026 01:25
@github-actions github-actions Bot mentioned this pull request Jul 9, 2026
Open
@github-actions github-actions Bot mentioned this pull request Jul 9, 2026
Open
bolinfest added a commit that referenced this pull request Jul 9, 2026
@bolinfest
core: route realtime and memories through HTTP client factory (#31362)
89aeae6 
## 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.














---
[//]: # (BEGIN SAPLING FOOTER)
Stack created with [Sapling](https://sapling-scm.com). Best reviewed
with [ReviewStack](https://reviewstack.dev/openai/codex/pull/31362).
* #31637
* #31431
* #31363
* __->__ #31362
Base automatically changed from pr31362 to main July 9, 2026 08:50
@ylabo0717 ylabo0717 mentioned this pull request Jul 9, 2026
Merged
@bolinfest bolinfest marked this pull request as ready for review July 9, 2026 16:58
@bolinfest bolinfest requested a review from a team as a code owner July 9, 2026 16:58
@bolinfest bolinfest merged commit 0e19d5e into main Jul 9, 2026
51 of 70 checks passed
@bolinfest bolinfest deleted the pr31363 branch July 9, 2026 16:58
@github-actions github-actions Bot locked and limited conversation to collaborators Jul 9, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

1 more reviewer
@viyatb-oai viyatb-oai viyatb-oai approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

code-reviewed

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@bolinfest @viyatb-oai