Skip to content

core: test Responses WebSockets with system proxy#31442

Draft
bolinfest wants to merge 1 commit into
pr31441from
pr31442
Draft

core: test Responses WebSockets with system proxy#31442
bolinfest wants to merge 1 commit into
pr31441from
pr31442

Conversation

@bolinfest

@bolinfest bolinfest commented Jul 7, 2026

Copy link
Copy Markdown
Collaborator

Why

#31441 preserves the Responses WebSocket fast path when features.respect_system_proxy is enabled. The core integration coverage must prove that behavior using the resolved Config::respect_system_proxy value—not merely toggle the feature registry after configuration has already been loaded.

What changed

  • Let the WebSocket test harness enable selected features and mirror the resolved respect_system_proxy field used by Config::http_client_factory().
  • Record and assert the resulting OutboundProxyPolicy so the test cannot silently exercise ReqwestDefault.
  • Add an end-to-end Responses test that enables RespectSystemProxy, completes a turn over WebSocket, and verifies one handshake and one WebSocket request.

Review guide

The entire change is in core/tests/suite/client_websockets.rs: the test is near the existing provider-capability coverage, and the harness wiring is at the bottom of the file.

Test plan

  • just test -p codex-core responses_websocket_streams_with_system_proxy_feature

Stack created with Sapling. Best reviewed with ReviewStack.

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