Skip to content

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

Draft
bolinfest wants to merge 1 commit into
pr31362from
pr31363
Draft

codex-api: route file uploads through HTTP client factory#31363
bolinfest wants to merge 1 commit into
pr31362from
pr31363

Conversation

@bolinfest

@bolinfest bolinfest commented Jul 7, 2026

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
@bolinfest bolinfest force-pushed the pr31362 branch 2 times, most recently from e1f3794 to 0797b7d 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