From 899539c03ac46301c8e860bc0096854f57625b82 Mon Sep 17 00:00:00 2001 From: Celia Chen Date: Tue, 28 Jul 2026 04:49:36 +0000 Subject: [PATCH] Reuse route-aware clients for OpenAI file uploads (#35717) ## What changed - Store a request-logging-disabled `RouteAwareClientPool` in session services and use it for file creation, blob upload, and finalization requests. - Preserve the existing system-root fallback for transport-default proxy routes, and emit a warning event when that fallback is used. - Remove the direct `reqwest` dependency from `codex-api` now that uploads use the shared HTTP abstraction. ## Testing - Add coverage for completing multiple uploads through a shared client pool. GitOrigin-RevId: 01842415cad2d349a6bd3869abb3332e8110ebce --- codex-rs/Cargo.lock | 1 - codex-rs/codex-api/Cargo.toml | 2 - codex-rs/codex-api/src/files.rs | 228 +++++++++++------- .../codex-api/tests/models_integration.rs | 7 +- codex-rs/core/src/mcp_openai_file.rs | 21 +- codex-rs/core/src/session/session.rs | 7 + codex-rs/core/src/session/tests.rs | 12 + codex-rs/core/src/state/service.rs | 3 + codex-rs/deny.toml | 1 - codex-rs/http-client/src/client_builder.rs | 6 + .../src/route_aware_client_pool.rs | 44 +++- 11 files changed, 228 insertions(+), 104 deletions(-) diff --git a/codex-rs/Cargo.lock b/codex-rs/Cargo.lock index c85717fe05..a899ae156f 100644 --- a/codex-rs/Cargo.lock +++ b/codex-rs/Cargo.lock @@ -1932,7 +1932,6 @@ dependencies = [ "http 1.4.0", "pretty_assertions", "regex-lite", - "reqwest 0.12.28", "schemars 0.8.22", "serde", "serde_json", diff --git a/codex-rs/codex-api/Cargo.toml b/codex-rs/codex-api/Cargo.toml index 52ea20920e..01cacbe2b4 100644 --- a/codex-rs/codex-api/Cargo.toml +++ b/codex-rs/codex-api/Cargo.toml @@ -16,7 +16,6 @@ codex-utils-rustls-provider = { workspace = true } codex-websocket-client = { workspace = true } futures = { workspace = true } http = { workspace = true } -reqwest = { workspace = true, features = ["json", "stream"] } schemars = { workspace = true } serde = { workspace = true, features = ["derive"] } serde_json = { workspace = true, features = ["raw_value"] } @@ -37,7 +36,6 @@ assert_matches = { workspace = true } pretty_assertions = { workspace = true } tokio-test = { workspace = true } wiremock = { workspace = true } -reqwest = { workspace = true } [lints] workspace = true diff --git a/codex-rs/codex-api/src/files.rs b/codex-rs/codex-api/src/files.rs index 8f3ceacedb..971402f385 100644 --- a/codex-rs/codex-api/src/files.rs +++ b/codex-rs/codex-api/src/files.rs @@ -2,13 +2,14 @@ use std::time::Duration; use crate::AuthProvider; use bytes::Bytes; -use codex_http_client::BuildRouteAwareHttpClientError; -use codex_http_client::ClientRouteClass; -use codex_http_client::HttpClientFactory; -use codex_http_client::OutboundProxyPolicy; +use codex_http_client::HttpResponse; +use codex_http_client::RouteAwareClientPool; +use codex_http_client::RouteAwareRequestBuilder; +use codex_http_client::RouteAwareRequestError; use futures::Stream; -use reqwest::StatusCode; -use reqwest::header::CONTENT_LENGTH; +use http::Method; +use http::StatusCode; +use http::header::CONTENT_LENGTH; use serde::Deserialize; use tokio::time::Instant; use uuid::Uuid; @@ -45,7 +46,7 @@ pub enum OpenAiFileError { Request { url: String, #[source] - source: reqwest::Error, + source: RouteAwareRequestError, }, #[error( "OpenAI file blob upload to {host} failed after {elapsed_ms} ms ({error_kind}, azure_client_request_id={azure_client_request_id}): {source}" @@ -56,7 +57,7 @@ pub enum OpenAiFileError { error_kind: &'static str, azure_client_request_id: String, #[source] - source: reqwest::Error, + source: RouteAwareRequestError, }, #[error( "OpenAI file blob upload to {host} failed with status {status} (azure_client_request_id={azure_client_request_id}, azure_request_id={azure_request_id}, azure_error_code={azure_error_code})" @@ -80,12 +81,6 @@ pub enum OpenAiFileError { #[source] source: serde_json::Error, }, - #[error("failed to build OpenAI file client for {url}: {source}")] - ClientBuild { - url: String, - #[source] - source: BuildRouteAwareHttpClientError, - }, #[error("OpenAI file upload for `{file_id}` is not ready yet")] UploadNotReady { file_id: String }, #[error("OpenAI file upload for `{file_id}` failed: {message}")] @@ -115,7 +110,7 @@ pub fn openai_file_uri(file_id: &str) -> String { pub async fn upload_openai_file( base_url: &str, auth: &dyn AuthProvider, - http_client_factory: &HttpClientFactory, + client_pool: &RouteAwareClientPool, file_name: String, file_size_bytes: u64, contents: impl Stream> + Send + 'static, @@ -129,23 +124,18 @@ pub async fn upload_openai_file( } let create_url = format!("{}/files", base_url.trim_end_matches('/')); - let create_response = authorized_request( - http_client_factory, - auth, - reqwest::Method::POST, - &create_url, - )? - .json(&serde_json::json!({ - "file_name": file_name.as_str(), - "file_size": file_size_bytes, - "use_case": OPENAI_FILE_USE_CASE, - })) - .send() - .await - .map_err(|source| OpenAiFileError::Request { - url: create_url.clone(), - source, - })?; + let create_response = authorized_request(client_pool, auth, Method::POST, &create_url) + .json(&serde_json::json!({ + "file_name": file_name.as_str(), + "file_size": file_size_bytes, + "use_case": OPENAI_FILE_USE_CASE, + })) + .send() + .await + .map_err(|source| OpenAiFileError::Request { + url: create_url.clone(), + source, + })?; let create_status = create_response.status(); let create_body = create_response.text().await.unwrap_or_default(); if !create_status.is_success() { @@ -167,13 +157,13 @@ pub async fn upload_openai_file( .unwrap_or_else(|| "unknown-host".to_string()); let azure_client_request_id = Uuid::new_v4().to_string(); let upload_started_at = Instant::now(); - let upload_response = build_reqwest_client(http_client_factory, &create_payload.upload_url)? + let upload_response = client_pool .put(&create_payload.upload_url) .timeout(OPENAI_FILE_REQUEST_TIMEOUT) .header("x-ms-blob-type", "BlockBlob") .header("x-ms-client-request-id", &azure_client_request_id) .header(CONTENT_LENGTH, file_size_bytes) - .body(reqwest::Body::wrap_stream(contents)) + .body_stream(contents) .send() .await .map_err(|source| { @@ -245,19 +235,14 @@ pub async fn upload_openai_file( ); let finalize_started_at = Instant::now(); loop { - let finalize_response = authorized_request( - http_client_factory, - auth, - reqwest::Method::POST, - &finalize_url, - )? - .json(&serde_json::json!({})) - .send() - .await - .map_err(|source| OpenAiFileError::Request { - url: finalize_url.clone(), - source, - })?; + let finalize_response = authorized_request(client_pool, auth, Method::POST, &finalize_url) + .json(&serde_json::json!({})) + .send() + .await + .map_err(|source| OpenAiFileError::Request { + url: finalize_url.clone(), + source, + })?; let finalize_status = finalize_response.status(); let finalize_body = finalize_response.text().await.unwrap_or_default(); if !finalize_status.is_success() { @@ -310,48 +295,21 @@ pub async fn upload_openai_file( } fn authorized_request( - http_client_factory: &HttpClientFactory, + client_pool: &RouteAwareClientPool, auth: &dyn AuthProvider, - method: reqwest::Method, + method: Method, url: &str, -) -> Result { +) -> RouteAwareRequestBuilder { let mut headers = http::HeaderMap::new(); auth.add_auth_headers(&mut headers); - let client = build_reqwest_client(http_client_factory, url)?; - Ok(client + client_pool .request(method, url) .timeout(OPENAI_FILE_REQUEST_TIMEOUT) - .headers(headers)) + .headers(headers) } -fn build_reqwest_client( - http_client_factory: &HttpClientFactory, - url: &str, -) -> Result { - match http_client_factory.build_reqwest_client( - reqwest::Client::builder(), - url, - ClientRouteClass::Api, - ) { - Ok(client) => Ok(client), - Err(error) - if matches!( - http_client_factory.outbound_proxy_policy(), - OutboundProxyPolicy::ReqwestDefault - ) => - { - tracing::warn!(%error, "failed to build OpenAI file upload client"); - Ok(reqwest::Client::new()) - } - Err(source) => Err(OpenAiFileError::ClientBuild { - url: url.to_string(), - source, - }), - } -} - -fn upload_response_header(response: &reqwest::Response, header: &str) -> String { +fn upload_response_header(response: &HttpResponse, header: &str) -> String { response .headers() .get(header) @@ -363,8 +321,11 @@ fn upload_response_header(response: &reqwest::Response, header: &str) -> String #[cfg(test)] mod tests { use super::*; + use codex_http_client::ClientRouteClass; + use codex_http_client::HttpClientFactory; + use codex_http_client::OutboundProxyPolicy; + use http::header::HeaderValue; use pretty_assertions::assert_eq; - use reqwest::header::HeaderValue; use std::sync::Arc; use std::sync::atomic::AtomicUsize; use std::sync::atomic::Ordering; @@ -381,14 +342,18 @@ mod tests { #[derive(Clone, Copy)] struct ChatGptTestAuth; - fn default_http_client_factory() -> HttpClientFactory { - HttpClientFactory::new(OutboundProxyPolicy::ReqwestDefault) + fn default_http_client_pool() -> RouteAwareClientPool { + RouteAwareClientPool::new_without_request_logging( + HttpClientFactory::new(OutboundProxyPolicy::ReqwestDefault), + ClientRouteClass::Api, + ) + .with_legacy_custom_ca_fallback() } impl AuthProvider for ChatGptTestAuth { - fn add_auth_headers(&self, headers: &mut reqwest::header::HeaderMap) { + fn add_auth_headers(&self, headers: &mut http::HeaderMap) { headers.insert( - reqwest::header::AUTHORIZATION, + http::header::AUTHORIZATION, HeaderValue::from_static("Bearer token"), ); headers.insert("ChatGPT-Account-ID", HeaderValue::from_static("account_id")); @@ -456,7 +421,7 @@ mod tests { let uploaded = upload_openai_file( &base_url, &chatgpt_auth(), - &default_http_client_factory(), + &default_http_client_pool(), "hello.txt".to_string(), /*file_size_bytes*/ 5, contents, @@ -475,6 +440,93 @@ mod tests { assert_eq!(finalize_attempts.load(Ordering::SeqCst), 2); } + #[tokio::test] + async fn upload_openai_file_reuses_client_pool_across_uploads() { + let server = MockServer::start().await; + let files = [ + ("first.txt", "file_1", &b"first"[..]), + ("second.txt", "file_2", &b"second"[..]), + ]; + + for (file_name, file_id, contents) in files { + Mock::given(method("POST")) + .and(path("/backend-api/files")) + .and(header("chatgpt-account-id", "account_id")) + .and(body_json(serde_json::json!({ + "file_name": file_name, + "file_size": contents.len(), + "use_case": "codex", + }))) + .respond_with(ResponseTemplate::new(200).set_body_json(serde_json::json!({ + "file_id": file_id, + "upload_url": format!("{}/upload/{file_id}", server.uri()), + }))) + .expect(1) + .mount(&server) + .await; + Mock::given(method("PUT")) + .and(path(format!("/upload/{file_id}"))) + .and(header("content-length", contents.len().to_string())) + .respond_with(ResponseTemplate::new(200)) + .expect(1) + .mount(&server) + .await; + Mock::given(method("POST")) + .and(path(format!("/backend-api/files/{file_id}/uploaded"))) + .respond_with(ResponseTemplate::new(200).set_body_json(serde_json::json!({ + "status": "success", + "download_url": format!("{}/download/{file_id}", server.uri()), + "file_name": file_name, + "mime_type": "text/plain", + "file_size_bytes": contents.len(), + }))) + .expect(1) + .mount(&server) + .await; + } + + let client_pool = default_http_client_pool(); + let mut uploaded_files = Vec::new(); + for (file_name, _, contents) in files { + let contents_stream = + futures::stream::iter([Ok::<_, std::io::Error>(Bytes::copy_from_slice(contents))]); + let uploaded = upload_openai_file( + &base_url_for(&server), + &chatgpt_auth(), + &client_pool, + file_name.to_string(), + u64::try_from(contents.len()).expect("file size should fit in a u64"), + contents_stream, + ) + .await + .expect("upload succeeds with the shared client pool"); + uploaded_files.push(uploaded); + } + + assert_eq!( + uploaded_files, + vec![ + UploadedOpenAiFile { + file_id: "file_1".to_string(), + uri: "sediment://file_1".to_string(), + download_url: format!("{}/download/file_1", server.uri()), + file_name: "first.txt".to_string(), + file_size_bytes: 5, + mime_type: Some("text/plain".to_string()), + }, + UploadedOpenAiFile { + file_id: "file_2".to_string(), + uri: "sediment://file_2".to_string(), + download_url: format!("{}/download/file_2", server.uri()), + file_name: "second.txt".to_string(), + file_size_bytes: 6, + mime_type: Some("text/plain".to_string()), + }, + ] + ); + server.verify().await; + } + #[tokio::test] async fn upload_openai_file_reports_blob_response_diagnostics_without_sas() { let server = MockServer::start().await; @@ -500,7 +552,7 @@ mod tests { let error = upload_openai_file( &base_url_for(&server), &chatgpt_auth(), - &default_http_client_factory(), + &default_http_client_pool(), "hello.txt".to_string(), /*file_size_bytes*/ 5, futures::stream::iter([Ok::<_, std::io::Error>(Bytes::from_static(b"hello"))]), @@ -537,7 +589,7 @@ mod tests { let error = upload_openai_file( &base_url_for(&server), &chatgpt_auth(), - &default_http_client_factory(), + &default_http_client_pool(), "hello.txt".to_string(), /*file_size_bytes*/ 5, futures::stream::iter([Ok::<_, std::io::Error>(Bytes::from_static(b"hello"))]), diff --git a/codex-rs/codex-api/tests/models_integration.rs b/codex-rs/codex-api/tests/models_integration.rs index 57d1c4234d..0758dcba8e 100644 --- a/codex-rs/codex-api/tests/models_integration.rs +++ b/codex-rs/codex-api/tests/models_integration.rs @@ -3,6 +3,7 @@ use codex_api::ModelsClient; use codex_api::Provider; use codex_api::RetryConfig; use codex_client::ReqwestTransport; +use codex_http_client::HttpClientBuilder; use codex_protocol::config_types::ReasoningSummary; use codex_protocol::openai_models::ConfigShellToolType; use codex_protocol::openai_models::ModelInfo; @@ -117,7 +118,11 @@ async fn models_client_hits_models_endpoint() { .mount(&server) .await; - let transport = ReqwestTransport::new(reqwest::Client::new()); + let transport = ReqwestTransport::from_http_client( + HttpClientBuilder::new() + .build_direct() + .expect("test HTTP client should build"), + ); let provider = provider(&base_url); let request_url = ModelsClient::::request_url(&provider, "0.1.0"); let client = ModelsClient::new(transport, provider, Arc::new(DummyAuth)); diff --git a/codex-rs/core/src/mcp_openai_file.rs b/codex-rs/core/src/mcp_openai_file.rs index 59ea2c751d..9decd133f1 100644 --- a/codex-rs/core/src/mcp_openai_file.rs +++ b/codex-rs/core/src/mcp_openai_file.rs @@ -15,6 +15,7 @@ use crate::session::session::Session; use crate::session::turn_context::TurnContext; use codex_api::OPENAI_FILE_UPLOAD_LIMIT_BYTES; use codex_api::upload_openai_file; +use codex_http_client::RouteAwareClientPool; use codex_login::CodexAuth; use codex_utils_path_uri::PathUri; use serde_json::Value as JsonValue; @@ -45,6 +46,7 @@ pub(crate) async fn rewrite_mcp_tool_arguments_for_openai_files( }; let Some(uploaded_value) = rewrite_argument_value_for_openai_files( turn_context, + &sess.services.openai_file_upload_client_pool, auth.as_ref(), field_name, optional_fields, @@ -66,6 +68,7 @@ pub(crate) async fn rewrite_mcp_tool_arguments_for_openai_files( async fn rewrite_argument_value_for_openai_files( turn_context: &TurnContext, + client_pool: &RouteAwareClientPool, auth: Option<&CodexAuth>, field_name: &str, optional_fields: &[String], @@ -75,6 +78,7 @@ async fn rewrite_argument_value_for_openai_files( JsonValue::String(file_path) => { let rewritten = build_uploaded_argument_value( turn_context, + client_pool, auth, field_name, /*index*/ None, @@ -92,6 +96,7 @@ async fn rewrite_argument_value_for_openai_files( }; let rewritten = build_uploaded_argument_value( turn_context, + client_pool, auth, field_name, Some(index), @@ -109,6 +114,7 @@ async fn rewrite_argument_value_for_openai_files( async fn build_uploaded_argument_value( turn_context: &TurnContext, + client_pool: &RouteAwareClientPool, auth: Option<&CodexAuth>, field_name: &str, index: Option, @@ -169,11 +175,10 @@ async fn build_uploaded_argument_value( .unwrap_or("file") .to_string(); let upload_auth = codex_model_provider::auth_provider_from_auth(auth); - let http_client_factory = turn_context.config.http_client_factory(); let uploaded = upload_openai_file( turn_context.config.chatgpt_base_url.trim_end_matches('/'), upload_auth.as_ref(), - &http_client_factory, + client_pool, file_name, metadata.size, contents, @@ -298,7 +303,7 @@ mod tests { .mount(&server) .await; - let (_, mut turn_context) = make_session_and_context().await; + let (session, mut turn_context) = make_session_and_context().await; let auth = CodexAuth::create_dummy_chatgpt_auth_for_testing(); let dir = tempdir().expect("temp dir"); let local_path = dir.path().join("file_report.csv"); @@ -313,6 +318,7 @@ mod tests { let rewritten = build_uploaded_argument_value( &turn_context, + &session.services.openai_file_upload_client_pool, Some(&auth), "file", /*index*/ None, @@ -335,7 +341,7 @@ mod tests { #[tokio::test] async fn build_uploaded_argument_value_rejects_oversized_file_before_reading() { - let (_, mut turn_context) = make_session_and_context().await; + let (session, mut turn_context) = make_session_and_context().await; let auth = CodexAuth::create_dummy_chatgpt_auth_for_testing(); let dir = tempdir().expect("temp dir"); let file_path = dir.path().join("oversized.bin"); @@ -346,6 +352,7 @@ mod tests { let error = build_uploaded_argument_value( &turn_context, + &session.services.openai_file_upload_client_pool, Some(&auth), "file", /*index*/ None, @@ -404,7 +411,7 @@ mod tests { .mount(&server) .await; - let (_, mut turn_context) = make_session_and_context().await; + let (session, mut turn_context) = make_session_and_context().await; let auth = CodexAuth::create_dummy_chatgpt_auth_for_testing(); let dir = tempdir().expect("temp dir"); let local_path = dir.path().join("file_report.csv"); @@ -418,6 +425,7 @@ mod tests { turn_context.config = Arc::new(config); let rewritten = rewrite_argument_value_for_openai_files( &turn_context, + &session.services.openai_file_upload_client_pool, Some(&auth), "file", &[], @@ -513,7 +521,7 @@ mod tests { .mount(&server) .await; - let (_, mut turn_context) = make_session_and_context().await; + let (session, mut turn_context) = make_session_and_context().await; let auth = CodexAuth::create_dummy_chatgpt_auth_for_testing(); let dir = tempdir().expect("temp dir"); tokio::fs::write(dir.path().join("one.csv"), b"one") @@ -529,6 +537,7 @@ mod tests { turn_context.config = Arc::new(config); let rewritten = rewrite_argument_value_for_openai_files( &turn_context, + &session.services.openai_file_upload_client_pool, Some(&auth), "files", &[], diff --git a/codex-rs/core/src/session/session.rs b/codex-rs/core/src/session/session.rs index 4f38ae9754..92494f026d 100644 --- a/codex-rs/core/src/session/session.rs +++ b/codex-rs/core/src/session/session.rs @@ -9,6 +9,8 @@ use crate::shell_snapshot::ShellSnapshot; use crate::skills::SkillError; use crate::state::ActiveTurn; use codex_extension_api::ExtensionDataInit; +use codex_http_client::ClientRouteClass; +use codex_http_client::RouteAwareClientPool; use codex_login::auth::AgentIdentityAuthPolicy; use codex_protocol::SessionId; use codex_protocol::capabilities::SelectedCapabilityRoot; @@ -1097,6 +1099,11 @@ impl Session { show_raw_agent_reasoning: config.show_raw_agent_reasoning, exec_policy, auth_manager: Arc::clone(&auth_manager), + openai_file_upload_client_pool: RouteAwareClientPool::new_without_request_logging( + config.http_client_factory(), + ClientRouteClass::Api, + ) + .with_legacy_custom_ca_fallback(), session_telemetry, models_manager: Arc::clone(&models_manager), tool_approvals: Mutex::new(ApprovalStore::default()), diff --git a/codex-rs/core/src/session/tests.rs b/codex-rs/core/src/session/tests.rs index 444a7d0bee..c0dea6f759 100644 --- a/codex-rs/core/src/session/tests.rs +++ b/codex-rs/core/src/session/tests.rs @@ -36,8 +36,10 @@ use codex_core_skills::HostSkillsSnapshot; use core_test_support::test_codex::local_selections; use codex_features::Feature; +use codex_http_client::ClientRouteClass; use codex_http_client::HttpClientFactory; use codex_http_client::OutboundProxyPolicy; +use codex_http_client::RouteAwareClientPool; use codex_login::CodexAuth; use codex_login::auth::AgentIdentityAuthPolicy; use codex_model_provider_info::ModelProviderInfo; @@ -5721,6 +5723,11 @@ pub(crate) async fn make_session_and_context() -> (Session, TurnContext) { show_raw_agent_reasoning: config.show_raw_agent_reasoning, exec_policy, auth_manager: auth_manager.clone(), + openai_file_upload_client_pool: RouteAwareClientPool::new_without_request_logging( + config.http_client_factory(), + ClientRouteClass::Api, + ) + .with_legacy_custom_ca_fallback(), session_telemetry: session_telemetry.clone(), models_manager: Arc::clone(&models_manager), tool_approvals: Mutex::new(ApprovalStore::default()), @@ -7894,6 +7901,11 @@ where show_raw_agent_reasoning: config.show_raw_agent_reasoning, exec_policy, auth_manager: Arc::clone(&auth_manager), + openai_file_upload_client_pool: RouteAwareClientPool::new_without_request_logging( + config.http_client_factory(), + ClientRouteClass::Api, + ) + .with_legacy_custom_ca_fallback(), session_telemetry: session_telemetry.clone(), models_manager: Arc::clone(&models_manager), tool_approvals: Mutex::new(ApprovalStore::default()), diff --git a/codex-rs/core/src/state/service.rs b/codex-rs/core/src/state/service.rs index a01b428115..463abea8af 100644 --- a/codex-rs/core/src/state/service.rs +++ b/codex-rs/core/src/state/service.rs @@ -28,6 +28,7 @@ use codex_extension_api::ExtensionData; use codex_extension_api::ExtensionDataInit; use codex_extension_api::ExtensionRegistry; use codex_hooks::Hooks; +use codex_http_client::RouteAwareClientPool; use codex_login::AuthManager; use codex_mcp::McpRuntime; use codex_models_manager::manager::SharedModelsManager; @@ -56,6 +57,8 @@ pub(crate) struct SessionServices { pub(crate) show_raw_agent_reasoning: bool, pub(crate) exec_policy: Arc, pub(crate) auth_manager: Arc, + /// Upload-only clients shared across turns without logging signed blob URLs. + pub(crate) openai_file_upload_client_pool: RouteAwareClientPool, pub(crate) models_manager: SharedModelsManager, pub(crate) session_telemetry: SessionTelemetry, pub(crate) tool_approvals: Mutex, diff --git a/codex-rs/deny.toml b/codex-rs/deny.toml index 6d939421e7..8f235a864d 100644 --- a/codex-rs/deny.toml +++ b/codex-rs/deny.toml @@ -239,7 +239,6 @@ deny = [ # Intended owner. "codex-http-client", # Temporary migration exceptions. - "codex-api", "codex-app-server", "codex-core", "codex-ollama", diff --git a/codex-rs/http-client/src/client_builder.rs b/codex-rs/http-client/src/client_builder.rs index 589a7cd0cb..79f58a7cf2 100644 --- a/codex-rs/http-client/src/client_builder.rs +++ b/codex-rs/http-client/src/client_builder.rs @@ -212,6 +212,12 @@ impl HttpClientBuilder { match build_with_custom_ca(self.clone().reqwest_builder(proxy_routing)) { Ok(inner) => HttpClient::from_parts(inner, request_logging), Err(error) => { + tracing::event!( + target: "codex_otel.log_only", + tracing::Level::WARN, + event.name = "codex.http_client.custom_ca_fallback", + "HTTP client fell back to system root certificates" + ); tracing::warn!(error = %error, "failed to build HTTP client with custom CA"); self.reqwest_builder(proxy_routing) .build() diff --git a/codex-rs/http-client/src/route_aware_client_pool.rs b/codex-rs/http-client/src/route_aware_client_pool.rs index 94088fda22..a90b8ad685 100644 --- a/codex-rs/http-client/src/route_aware_client_pool.rs +++ b/codex-rs/http-client/src/route_aware_client_pool.rs @@ -34,6 +34,13 @@ use crate::route_aware_redirect::remove_sensitive_headers; const MAX_CACHED_ROUTES: usize = 16; +#[derive(Clone, Copy, Debug, Default, Eq, PartialEq)] +enum CustomCaFallback { + #[default] + Disabled, + LegacyTransportDefault, +} + /// Reuses transport clients by resolved route while selecting a route for every request URL. /// /// Request creation stays on the pool so the URL used for PAC or system-proxy resolution cannot @@ -44,6 +51,7 @@ pub struct RouteAwareClientPool { http_client_factory: HttpClientFactory, route_class: ClientRouteClass, client_builder: HttpClientBuilder, + custom_ca_fallback: CustomCaFallback, clients: Arc>>, } @@ -312,6 +320,7 @@ impl RouteAwareClientPool { http_client_factory, route_class, client_builder, + custom_ca_fallback: CustomCaFallback::Disabled, clients: Arc::new(Mutex::new(HashMap::new())), } } @@ -328,6 +337,15 @@ impl RouteAwareClientPool { ) } + /// Preserves the legacy custom-CA fallback for transport-default proxy routes. + /// + /// Use this only when migrating a client that already continued with system roots after a + /// custom-CA construction failure. System-proxy routes still propagate construction errors. + pub fn with_legacy_custom_ca_fallback(mut self) -> Self { + self.custom_ca_fallback = CustomCaFallback::LegacyTransportDefault; + self + } + /// Creates a pool that retains the Cloudflare cookies required by ChatGPT endpoints. pub fn with_chatgpt_cloudflare_cookies( http_client_factory: HttpClientFactory, @@ -577,11 +595,27 @@ impl RouteAwareClientPool { self.client_builder.clone().without_redirects() } }; - let client = client_builder.build_for_resolved_route( - &self.http_client_factory, - self.route_class, - &route, - )?; + #[expect( + deprecated, + reason = "explicitly opted-in pools preserve the legacy custom-CA fallback" + )] + let client = match ( + self.http_client_factory.outbound_proxy_policy(), + self.custom_ca_fallback, + ) { + (OutboundProxyPolicy::ReqwestDefault, CustomCaFallback::LegacyTransportDefault) => { + client_builder.build_with_transport_default_proxy_and_custom_ca_fallback() + } + (OutboundProxyPolicy::ReqwestDefault, CustomCaFallback::Disabled) + | (OutboundProxyPolicy::RespectSystemProxy, CustomCaFallback::Disabled) + | (OutboundProxyPolicy::RespectSystemProxy, CustomCaFallback::LegacyTransportDefault) => { + client_builder.build_for_resolved_route( + &self.http_client_factory, + self.route_class, + &route, + )? + } + }; let mut clients = match self.clients.lock() { Ok(clients) => clients, Err(error) => panic!("route-aware client cache lock should not be poisoned: {error}"),