mirror of
https://github.com/openai/codex.git
synced 2026-08-23 13:09:46 +00:00
core: route Responses API through system proxy (#31335)
## 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
This commit is contained in:
@@ -25,9 +25,9 @@ pub use crate::error::StreamError;
|
||||
pub use crate::error::TransportError;
|
||||
pub use crate::outbound_proxy::BuildRouteAwareHttpClientError;
|
||||
pub use crate::outbound_proxy::ClientRouteClass;
|
||||
pub use crate::outbound_proxy::OutboundProxyConfig;
|
||||
pub use crate::outbound_proxy::HttpClientFactory;
|
||||
pub use crate::outbound_proxy::OutboundProxyPolicy;
|
||||
pub use crate::outbound_proxy::RouteFailureClass;
|
||||
pub use crate::outbound_proxy::build_reqwest_client_for_route;
|
||||
pub use crate::request::EncodedJsonBody;
|
||||
pub use crate::request::PreparedRequestBody;
|
||||
pub use crate::request::Request;
|
||||
|
||||
@@ -84,14 +84,54 @@ impl fmt::Display for RouteFailureClass {
|
||||
}
|
||||
}
|
||||
|
||||
/// Marker enabling fixed system/PAC/WPAD, environment, then direct routing.
|
||||
/// Resolved endpoints and platform details remain internal to the client builder.
|
||||
/// Resolved outbound proxy behavior for HTTP clients.
|
||||
///
|
||||
/// Callers must choose a policy explicitly so omitting feature resolution cannot silently select
|
||||
/// legacy behavior.
|
||||
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
|
||||
pub struct OutboundProxyConfig;
|
||||
pub enum OutboundProxyPolicy {
|
||||
/// Preserve reqwest's built-in proxy behavior.
|
||||
ReqwestDefault,
|
||||
/// Resolve system/PAC/WPAD settings, then environment settings, then direct routing.
|
||||
RespectSystemProxy,
|
||||
}
|
||||
|
||||
impl OutboundProxyConfig {
|
||||
pub const fn respect_system_proxy() -> Self {
|
||||
Self
|
||||
/// Builds route-specific HTTP clients using one resolved outbound proxy policy.
|
||||
///
|
||||
/// Construct this once from the effective application configuration and carry it with the
|
||||
/// session or component that owns outbound requests. Individual request paths should supply only
|
||||
/// their destination and route class rather than resolving feature state themselves.
|
||||
#[derive(Debug, Clone, PartialEq, Eq)]
|
||||
pub struct HttpClientFactory {
|
||||
outbound_proxy_policy: OutboundProxyPolicy,
|
||||
}
|
||||
|
||||
impl HttpClientFactory {
|
||||
/// Creates a factory from the outbound proxy policy resolved by the application.
|
||||
pub const fn new(outbound_proxy_policy: OutboundProxyPolicy) -> Self {
|
||||
Self {
|
||||
outbound_proxy_policy,
|
||||
}
|
||||
}
|
||||
|
||||
/// Returns the outbound proxy policy used for clients built by this factory.
|
||||
pub const fn outbound_proxy_policy(&self) -> OutboundProxyPolicy {
|
||||
self.outbound_proxy_policy
|
||||
}
|
||||
|
||||
/// Builds a reqwest client for a concrete outbound route.
|
||||
pub fn build_reqwest_client(
|
||||
&self,
|
||||
builder: reqwest::ClientBuilder,
|
||||
request_url: &str,
|
||||
route_class: ClientRouteClass,
|
||||
) -> Result<reqwest::Client, BuildRouteAwareHttpClientError> {
|
||||
build_reqwest_client_for_route(
|
||||
builder,
|
||||
request_url,
|
||||
route_class,
|
||||
self.outbound_proxy_policy,
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -120,18 +160,18 @@ impl From<BuildRouteAwareHttpClientError> for io::Error {
|
||||
/// a route is selected are returned without trying another route. Ordered PAC candidates are
|
||||
/// currently collapsed to one route on both Windows and macOS; later proxy or `DIRECT` candidates
|
||||
/// are not retried after a connection failure.
|
||||
pub fn build_reqwest_client_for_route(
|
||||
fn build_reqwest_client_for_route(
|
||||
builder: reqwest::ClientBuilder,
|
||||
request_url: &str,
|
||||
route_class: ClientRouteClass,
|
||||
config: Option<&OutboundProxyConfig>,
|
||||
outbound_proxy_policy: OutboundProxyPolicy,
|
||||
) -> Result<reqwest::Client, BuildRouteAwareHttpClientError> {
|
||||
let builder = configure_proxy_for_route(
|
||||
&ProcessEnv,
|
||||
builder,
|
||||
request_url,
|
||||
route_class,
|
||||
config,
|
||||
outbound_proxy_policy,
|
||||
resolve_system_proxy,
|
||||
)?;
|
||||
build_reqwest_client_with_custom_ca(builder).map_err(Into::into)
|
||||
@@ -142,10 +182,10 @@ fn configure_proxy_for_route(
|
||||
builder: reqwest::ClientBuilder,
|
||||
request_url: &str,
|
||||
route_class: ClientRouteClass,
|
||||
config: Option<&OutboundProxyConfig>,
|
||||
outbound_proxy_policy: OutboundProxyPolicy,
|
||||
resolve_system_proxy: impl FnOnce(&str, &RequestOrigin) -> SystemProxyDecision,
|
||||
) -> Result<reqwest::ClientBuilder, BuildRouteAwareHttpClientError> {
|
||||
if config.is_none() {
|
||||
if matches!(outbound_proxy_policy, OutboundProxyPolicy::ReqwestDefault) {
|
||||
return Ok(builder);
|
||||
}
|
||||
let origin = RequestOrigin::parse(request_url);
|
||||
|
||||
@@ -79,13 +79,12 @@ async fn enabled_environment_proxy_routes_request_through_proxy() {
|
||||
values: HashMap::from([("HTTP_PROXY".to_string(), format!("http://{proxy_addr}"))]),
|
||||
};
|
||||
let request_url = "http://enabled-proxy.test/proxy-check";
|
||||
let config = OutboundProxyConfig::respect_system_proxy();
|
||||
let builder = configure_proxy_for_route(
|
||||
&env,
|
||||
reqwest::Client::builder().timeout(Duration::from_secs(2)),
|
||||
request_url,
|
||||
ClientRouteClass::Auth,
|
||||
Some(&config),
|
||||
OutboundProxyPolicy::RespectSystemProxy,
|
||||
|_, _| SystemProxyDecision::Unavailable {
|
||||
failure: RouteFailureClass::ProxyResolutionUnavailable,
|
||||
},
|
||||
|
||||
Reference in New Issue
Block a user