Add a policy-aware HTTP client builder (#34630)

## What changed

- Add `HttpClientBuilder` for configuring default headers, redirects, the
  Cloudflare cookie store, and request diagnostics without exposing the
  underlying transport.
- Provide factory-backed construction for fixed destinations that respects
  outbound proxy policy, alongside explicit direct and legacy
  transport-default construction paths.
- Preserve the request-logging setting in `ReqwestTransport`, so disabling
  diagnostics also suppresses transport-level URL and request-body traces.
- Add `HEAD`, `DELETE`, and query-parameter helpers to the shared client
  wrappers.

## Testing

- Verify builder configuration survives policy-aware construction and custom
  CA fallback.
- Verify disabled request logging omits request URLs and bodies from transport
  traces.

GitOrigin-RevId: 529c33abefeb88f38ff9a0ead374d29fcc872e6a

codex-rs/http-client/src/client.rs

@@ -1,3 +1,5 @@
+//! Reusable HTTP client and request-builder wrappers.
+
 use http::Error as HttpRequestBuildError;
 use http::HeaderMap;
 use http::HeaderName;
@@ -15,6 +17,10 @@ use tracing_opentelemetry::OpenTelemetrySpanExt;
 pub type HttpError = reqwest::Error;
 pub type HttpResponse = reqwest::Response;
 
+/// Reusable HTTP client wrapper with shared tracing and request-diagnostic behavior.
+///
+/// Product callers should obtain this through [`crate::HttpClientFactory`] for a fixed
+/// destination or use [`crate::RouteAwareClientPool`] when request and redirect URLs can vary.
 #[derive(Clone, Debug)]
 pub struct HttpClient {
     inner: reqwest::Client,
@@ -23,10 +29,7 @@ pub struct HttpClient {
 
 impl HttpClient {
     pub fn new(inner: reqwest::Client) -> Self {
-        Self {
+        Self::from_parts(inner, RequestLogging::Enabled)
-            inner,
-            request_logging: RequestLogging::Enabled,
-        }
     }
 
     /// Creates a client that suppresses request URL and response-header diagnostics.
@@ -34,9 +37,13 @@ impl HttpClient {
     /// Use this for endpoints whose URLs or headers may contain credentials that are redacted by
     /// the caller above the HTTP transport boundary.
     pub fn new_without_request_logging(inner: reqwest::Client) -> Self {
+        Self::from_parts(inner, RequestLogging::Disabled)
+    }
+
+    pub(crate) fn from_parts(inner: reqwest::Client, request_logging: RequestLogging) -> Self {
         Self {
             inner,
-            request_logging: RequestLogging::Disabled,
+            request_logging,
         }
     }
 
@@ -47,6 +54,13 @@ impl HttpClient {
         self.request(Method::GET, url)
     }
 
+    pub fn head<U>(&self, url: U) -> RequestBuilder
+    where
+        U: IntoUrl,
+    {
+        self.request(Method::HEAD, url)
+    }
+
     pub fn post<U>(&self, url: U) -> RequestBuilder
     where
         U: IntoUrl,
@@ -54,6 +68,13 @@ impl HttpClient {
         self.request(Method::POST, url)
     }
 
+    pub fn delete<U>(&self, url: U) -> RequestBuilder
+    where
+        U: IntoUrl,
+    {
+        self.request(Method::DELETE, url)
+    }
+
     pub fn request<U>(&self, method: Method, url: U) -> RequestBuilder
     where
         U: IntoUrl,
@@ -130,10 +151,15 @@ impl HttpClient {
             );
         }
     }
+
+    pub(crate) const fn request_logging_enabled(&self) -> bool {
+        matches!(self.request_logging, RequestLogging::Enabled)
+    }
 }
 
-#[derive(Clone, Copy, Debug, PartialEq, Eq)]
+#[derive(Clone, Copy, Debug, Default, PartialEq, Eq)]
-enum RequestLogging {
+pub(crate) enum RequestLogging {
+    #[default]
     Enabled,
     Disabled,
 }
@@ -203,6 +229,13 @@ impl RequestBuilder {
         self.map(|builder| builder.json(value))
     }
 
+    pub fn query<T>(self, query: &T) -> Self
+    where
+        T: ?Sized + Serialize,
+    {
+        self.map(|builder| builder.query(query))
+    }
+
     pub fn body<B>(self, body: B) -> Self
     where
         B: Into<reqwest::Body>,
@@ -278,6 +311,7 @@ mod tests {
     use opentelemetry::trace::TracerProvider;
     use opentelemetry_sdk::propagation::TraceContextPropagator;
     use opentelemetry_sdk::trace::SdkTracerProvider;
+    use pretty_assertions::assert_eq;
     use tracing::trace_span;
     use tracing_subscriber::layer::SubscriberExt;
     use tracing_subscriber::util::SubscriberInitExt;

codex-rs/http-client/src/client_builder.rs

@@ -0,0 +1,248 @@
+//! HTTP client construction that makes outbound proxy policy explicit.
+//!
+//! Product traffic should normally enter through [`HttpClientFactory`] for a fixed destination or
+//! [`crate::RouteAwareClientPool`] when request and redirect URLs can vary. The direct and
+//! transport-default terminal methods exist only for narrow exceptional or legacy compatibility
+//! paths.
+
+use http::HeaderMap;
+
+use crate::BuildCustomCaTransportError;
+use crate::BuildRouteAwareHttpClientError;
+use crate::ClientRouteClass;
+use crate::HttpClient;
+use crate::HttpClientFactory;
+use crate::client::RequestLogging;
+use crate::custom_ca::build_reqwest_client_with_custom_ca;
+use crate::with_chatgpt_cloudflare_cookie_store;
+
+/// Configures an [`HttpClient`] without exposing the underlying HTTP implementation.
+///
+/// Product traffic should prefer [`HttpClientFactory::build_client`] or finish this builder with
+/// [`Self::build_respecting_outbound_proxy_policy`]. The other terminal methods deliberately
+/// bypass the factory and are restricted to documented exceptional or legacy compatibility paths.
+#[derive(Clone)]
+pub struct HttpClientBuilder {
+    default_headers: Option<HeaderMap>,
+    follow_redirects: bool,
+    chatgpt_cloudflare_cookie_store: bool,
+    request_logging: RequestLogging,
+}
+
+impl HttpClientFactory {
+    /// Builds an HTTP client for one fixed destination using the configured proxy policy.
+    ///
+    /// This is the preferred construction path for product traffic that uses a fixed destination.
+    /// Use [`crate::RouteAwareClientPool`] instead when request or redirect URLs can vary.
+    pub fn build_client(
+        &self,
+        request_url: &str,
+        route_class: ClientRouteClass,
+    ) -> Result<HttpClient, BuildRouteAwareHttpClientError> {
+        HttpClientBuilder::new().build_respecting_outbound_proxy_policy(
+            self,
+            request_url,
+            route_class,
+        )
+    }
+
+    /// Builds a policy-aware client without request URL or response-header diagnostics.
+    ///
+    /// This has the same routing guidance as [`Self::build_client`].
+    pub fn build_client_without_request_logging(
+        &self,
+        request_url: &str,
+        route_class: ClientRouteClass,
+    ) -> Result<HttpClient, BuildRouteAwareHttpClientError> {
+        HttpClientBuilder::new()
+            .without_request_logging()
+            .build_respecting_outbound_proxy_policy(self, request_url, route_class)
+    }
+}
+
+impl HttpClientBuilder {
+    pub fn new() -> Self {
+        Self::default()
+    }
+
+    pub fn default_headers(mut self, headers: HeaderMap) -> Self {
+        self.default_headers = Some(headers);
+        self
+    }
+
+    pub fn without_redirects(mut self) -> Self {
+        self.follow_redirects = false;
+        self
+    }
+
+    pub fn with_chatgpt_cloudflare_cookie_store(mut self) -> Self {
+        self.chatgpt_cloudflare_cookie_store = true;
+        self
+    }
+
+    /// Suppresses request URL and response-header diagnostics.
+    pub fn without_request_logging(mut self) -> Self {
+        self.request_logging = RequestLogging::Disabled;
+        self
+    }
+
+    /// Builds a client that honors the [`HttpClientFactory`] outbound proxy policy.
+    ///
+    /// This is the preferred terminal method for product traffic. The request URL is used to
+    /// resolve a concrete direct or proxy route when the factory is configured with
+    /// [`crate::OutboundProxyPolicy::RespectSystemProxy`].
+    pub fn build_respecting_outbound_proxy_policy(
+        self,
+        http_client_factory: &HttpClientFactory,
+        request_url: &str,
+        route_class: ClientRouteClass,
+    ) -> Result<HttpClient, BuildRouteAwareHttpClientError> {
+        let (builder, request_logging) = self.into_reqwest_parts();
+        let inner = http_client_factory.build_reqwest_client(builder, request_url, route_class)?;
+        Ok(HttpClient::from_parts(inner, request_logging))
+    }
+
+    /// Builds a client using the transport's default proxy behavior.
+    ///
+    /// # Legacy compatibility only
+    ///
+    /// This bypasses [`HttpClientFactory`] and therefore does not honor its configured outbound
+    /// proxy policy. New product traffic must use [`Self::build_respecting_outbound_proxy_policy`]
+    /// or [`HttpClientFactory::build_client`].
+    #[deprecated(
+        note = "legacy compatibility only; use HttpClientFactory::build_client or build_respecting_outbound_proxy_policy"
+    )]
+    pub fn build_with_transport_default_proxy(
+        self,
+    ) -> Result<HttpClient, BuildCustomCaTransportError> {
+        self.build_with_proxy_routing(ProxyRouting::TransportDefault)
+    }
+
+    /// Builds a client that connects directly without using a proxy.
+    ///
+    /// # Exceptional use only
+    ///
+    /// This bypasses [`HttpClientFactory`] and is appropriate only when bypassing proxy discovery
+    /// is itself required: for example, a hermetic local test fixture, a localhost callback, or
+    /// sandbox traffic whose egress routing is handled separately. Ordinary outbound product
+    /// traffic must use [`Self::build_respecting_outbound_proxy_policy`] or
+    /// [`HttpClientFactory::build_client`].
+    pub fn build_direct(self) -> Result<HttpClient, BuildCustomCaTransportError> {
+        self.build_with_proxy_routing(ProxyRouting::Direct)
+    }
+
+    /// Builds a transport-default client while preserving the legacy custom-CA fallback.
+    ///
+    /// # Legacy compatibility only
+    ///
+    /// This preserves call sites that historically logged a custom-CA error and continued with
+    /// system roots. New product traffic must propagate construction errors through
+    /// [`Self::build_respecting_outbound_proxy_policy`] or [`HttpClientFactory::build_client`].
+    #[deprecated(
+        note = "legacy custom-CA fallback only; use HttpClientFactory::build_client or build_respecting_outbound_proxy_policy"
+    )]
+    pub fn build_with_transport_default_proxy_and_custom_ca_fallback(self) -> HttpClient {
+        self.build_with_custom_ca_fallback(ProxyRouting::TransportDefault)
+    }
+
+    /// Builds a direct client while preserving the legacy custom-CA fallback.
+    ///
+    /// # Legacy compatibility only
+    ///
+    /// This combines the exceptional proxy bypass described by [`Self::build_direct`] with the
+    /// historical behavior of logging a custom-CA error and continuing with system roots.
+    #[deprecated(
+        note = "legacy custom-CA fallback only; use build_direct and propagate construction errors"
+    )]
+    pub fn build_direct_with_custom_ca_fallback(self) -> HttpClient {
+        self.build_with_custom_ca_fallback(ProxyRouting::Direct)
+    }
+
+    fn build_with_proxy_routing(
+        self,
+        proxy_routing: ProxyRouting,
+    ) -> Result<HttpClient, BuildCustomCaTransportError> {
+        let request_logging = self.request_logging;
+        build_reqwest_client_with_custom_ca(self.reqwest_builder(proxy_routing))
+            .map(|inner| HttpClient::from_parts(inner, request_logging))
+    }
+
+    fn build_with_custom_ca_fallback(self, proxy_routing: ProxyRouting) -> HttpClient {
+        self.build_with_custom_ca_fallback_using(proxy_routing, build_reqwest_client_with_custom_ca)
+    }
+
+    fn build_with_custom_ca_fallback_using(
+        self,
+        proxy_routing: ProxyRouting,
+        build_with_custom_ca: impl FnOnce(
+            reqwest::ClientBuilder,
+        )
+            -> Result<reqwest::Client, BuildCustomCaTransportError>,
+    ) -> HttpClient {
+        let request_logging = self.request_logging;
+        match build_with_custom_ca(self.clone().reqwest_builder(proxy_routing)) {
+            Ok(inner) => HttpClient::from_parts(inner, request_logging),
+            Err(error) => {
+                tracing::warn!(error = %error, "failed to build HTTP client with custom CA");
+                self.reqwest_builder(proxy_routing)
+                    .build()
+                    .map(|inner| HttpClient::from_parts(inner, request_logging))
+                    .unwrap_or_else(|fallback_error| {
+                        tracing::warn!(
+                            error = %fallback_error,
+                            "failed to build fallback HTTP client"
+                        );
+                        HttpClient::from_parts(reqwest::Client::new(), request_logging)
+                    })
+            }
+        }
+    }
+
+    fn into_reqwest_parts(self) -> (reqwest::ClientBuilder, RequestLogging) {
+        let request_logging = self.request_logging;
+        (self.base_reqwest_builder(), request_logging)
+    }
+
+    fn reqwest_builder(self, proxy_routing: ProxyRouting) -> reqwest::ClientBuilder {
+        let builder = self.base_reqwest_builder();
+        match proxy_routing {
+            ProxyRouting::TransportDefault => builder,
+            ProxyRouting::Direct => builder.no_proxy(),
+        }
+    }
+
+    fn base_reqwest_builder(self) -> reqwest::ClientBuilder {
+        let mut builder = reqwest::Client::builder();
+        if let Some(default_headers) = self.default_headers {
+            builder = builder.default_headers(default_headers);
+        }
+        if !self.follow_redirects {
+            builder = builder.redirect(reqwest::redirect::Policy::none());
+        }
+        if self.chatgpt_cloudflare_cookie_store {
+            builder = with_chatgpt_cloudflare_cookie_store(builder);
+        }
+        builder
+    }
+}
+
+impl Default for HttpClientBuilder {
+    fn default() -> Self {
+        Self {
+            default_headers: None,
+            follow_redirects: true,
+            chatgpt_cloudflare_cookie_store: false,
+            request_logging: RequestLogging::Enabled,
+        }
+    }
+}
+
+#[derive(Clone, Copy)]
+enum ProxyRouting {
+    TransportDefault,
+    Direct,
+}
+
+#[cfg(test)]
+#[path = "client_builder_tests.rs"]
+mod tests;

codex-rs/http-client/src/client_builder_tests.rs

@@ -0,0 +1,52 @@
+use super::*;
+use http::HeaderValue;
+use std::io::Read;
+use std::io::Write;
+use std::path::PathBuf;
+
+#[tokio::test]
+async fn custom_ca_fallback_preserves_builder_configuration() {
+    let listener =
+        std::net::TcpListener::bind(("127.0.0.1", 0)).expect("HTTP listener should bind");
+    let address = listener
+        .local_addr()
+        .expect("HTTP listener should have an address");
+    let server = std::thread::spawn(move || {
+        let (mut stream, _) = listener.accept().expect("HTTP listener should accept");
+        let mut request = Vec::new();
+        let mut chunk = [0_u8; 1024];
+        while !request.windows(4).any(|window| window == b"\r\n\r\n") {
+            let bytes_read = stream.read(&mut chunk).expect("HTTP request should read");
+            assert!(bytes_read > 0, "HTTP request should include headers");
+            request.extend_from_slice(&chunk[..bytes_read]);
+        }
+        stream
+            .write_all(b"HTTP/1.1 200 OK\r\nContent-Length: 0\r\nConnection: close\r\n\r\n")
+            .expect("HTTP listener should write response");
+        String::from_utf8(request).expect("HTTP request should be UTF-8")
+    });
+    let mut headers = HeaderMap::new();
+    headers.insert("x-builder-test", HeaderValue::from_static("preserved"));
+    let client = HttpClientBuilder::new()
+        .default_headers(headers)
+        .build_with_custom_ca_fallback_using(ProxyRouting::Direct, |_| {
+            Err(BuildCustomCaTransportError::InvalidCaFile {
+                source_env: "TEST_CA_ENV",
+                path: PathBuf::from("invalid-test-ca.pem"),
+                detail: "synthetic invalid CA".to_string(),
+            })
+        });
+
+    let response = client
+        .get(format!("http://{address}/fallback"))
+        .send()
+        .await
+        .expect("fallback client should send request");
+    assert!(response.status().is_success());
+    let request = server.join().expect("HTTP listener should finish");
+    assert!(
+        request
+            .lines()
+            .any(|line| line.eq_ignore_ascii_case("x-builder-test: preserved"))
+    );
+}

codex-rs/http-client/src/lib.rs

@@ -1,7 +1,8 @@
 mod chatgpt_cloudflare_cookies;
 mod chatgpt_hosts;
+mod client;
+mod client_builder;
 mod custom_ca;
-mod default_client;
 mod error;
 mod outbound_proxy;
 mod request;
@@ -11,6 +12,11 @@ mod transport;
 
 pub use crate::chatgpt_cloudflare_cookies::with_chatgpt_cloudflare_cookie_store;
 pub use crate::chatgpt_hosts::is_allowed_chatgpt_host;
+pub use crate::client::HttpClient;
+pub use crate::client::HttpError;
+pub use crate::client::HttpResponse;
+pub use crate::client::RequestBuilder;
+pub use crate::client_builder::HttpClientBuilder;
 pub use crate::custom_ca::BuildCustomCaTransportError;
 /// Test-only subprocess hook for custom CA coverage.
 ///
@@ -22,10 +28,6 @@ pub use crate::custom_ca::build_reqwest_client_for_subprocess_tests;
 pub use crate::custom_ca::build_reqwest_client_with_custom_ca;
 pub use crate::custom_ca::build_rustls_client_config_with_custom_ca;
 pub use crate::custom_ca::maybe_build_rustls_client_config_with_custom_ca;
-pub use crate::default_client::HttpClient;
-pub use crate::default_client::HttpError;
-pub use crate::default_client::HttpResponse;
-pub use crate::default_client::RequestBuilder;
 pub use crate::error::StreamError;
 pub use crate::error::TransportError;
 pub use crate::outbound_proxy::BuildRouteAwareHttpClientError;

codex-rs/http-client/src/outbound_proxy.rs

@@ -17,7 +17,6 @@ use tokio::sync::Semaphore;
 
 use crate::custom_ca::BuildCustomCaTransportError;
 use crate::custom_ca::build_reqwest_client_with_custom_ca;
-use crate::default_client::HttpClient;
 use sha2::Digest;
 use sha2::Sha256;
 use thiserror::Error;
@@ -219,26 +218,6 @@ impl HttpClientFactory {
             .map(|decision| route_from_system_decision(&ProcessEnv, env_proxy_kind, decision))
     }
 
-    /// Builds an HTTP client for a concrete outbound route.
-    pub fn build_client(
-        &self,
-        request_url: &str,
-        route_class: ClientRouteClass,
-    ) -> Result<HttpClient, BuildRouteAwareHttpClientError> {
-        self.build_reqwest_client(reqwest::Client::builder(), request_url, route_class)
-            .map(HttpClient::new)
-    }
-
-    /// Builds a route-aware client without request URL or response-header diagnostics.
-    pub fn build_client_without_request_logging(
-        &self,
-        request_url: &str,
-        route_class: ClientRouteClass,
-    ) -> Result<HttpClient, BuildRouteAwareHttpClientError> {
-        self.build_reqwest_client(reqwest::Client::builder(), request_url, route_class)
-            .map(HttpClient::new_without_request_logging)
-    }
-
     /// Builds a reqwest client for a concrete outbound route.
     pub fn build_reqwest_client(
         &self,

codex-rs/http-client/src/outbound_proxy_tests.rs

@@ -1,6 +1,9 @@
 //! Shared outbound proxy policy tests.
 
 use super::*;
+use crate::HttpClientBuilder;
+use http::HeaderMap;
+use http::HeaderValue;
 use http::header::AUTHORIZATION;
 use http::header::COOKIE;
 use http::header::PROXY_AUTHORIZATION;
@@ -47,7 +50,7 @@ fn spawn_http_listener(
     let thread = std::thread::spawn(move || {
         let mut requests = Vec::new();
         for response in responses {
-            let deadline = Instant::now() + Duration::from_secs(2);
+            let deadline = Instant::now() + Duration::from_secs(10);
             let (mut stream, _) = loop {
                 match listener.accept() {
                     Ok(connection) => break connection,
@@ -62,7 +65,7 @@ fn spawn_http_listener(
                 }
             };
             stream
-                .set_read_timeout(Some(Duration::from_secs(2)))
+                .set_read_timeout(Some(Duration::from_secs(10)))
                 .expect("HTTP stream should get a read timeout");
             requests.push(read_http_message(&mut stream));
             stream
@@ -333,6 +336,34 @@ async fn enabled_environment_proxy_routes_request_through_proxy() {
     );
 }
 
+#[tokio::test]
+async fn route_aware_builder_preserves_default_headers() {
+    let (server_addr, server_thread) = spawn_proxy_listener();
+    let request_url = format!("http://{server_addr}/builder-check");
+    cache_system_proxy_decision(&request_url, SystemProxyDecision::Direct);
+    let mut headers = HeaderMap::new();
+    headers.insert("x-builder-test", HeaderValue::from_static("preserved"));
+    let factory = HttpClientFactory::new(OutboundProxyPolicy::RespectSystemProxy);
+    let client = HttpClientBuilder::new()
+        .default_headers(headers)
+        .build_respecting_outbound_proxy_policy(&factory, &request_url, ClientRouteClass::Api)
+        .expect("route-aware client should build");
+
+    let response = client
+        .get(&request_url)
+        .send()
+        .await
+        .expect("request should use direct route");
+    let request = only_request(server_thread, "server");
+
+    assert!(response.status().is_success());
+    assert!(
+        request
+            .lines()
+            .any(|line| line.eq_ignore_ascii_case("x-builder-test: preserved"))
+    );
+}
+
 #[tokio::test]
 async fn route_aware_pool_uses_respect_system_proxy_route_for_exact_url() {
     let (proxy_addr, proxy_thread) = spawn_proxy_listener();

codex-rs/http-client/src/transport.rs

@@ -1,5 +1,5 @@
-use crate::default_client::HttpClient;
+use crate::client::HttpClient;
-use crate::default_client::RequestBuilder;
+use crate::client::RequestBuilder;
 use crate::error::TransportError;
 use crate::request::Request;
 use crate::request::RequestBody;
@@ -45,6 +45,10 @@ impl ReqwestTransport {
         }
     }
 
+    pub fn from_http_client(client: HttpClient) -> Self {
+        Self { client }
+    }
+
     fn build(&self, req: Request) -> Result<RequestBuilder, TransportError> {
         let prepared = req.prepare_body_for_send().map_err(TransportError::Build)?;
 
@@ -80,6 +84,17 @@ impl ReqwestTransport {
             TransportError::Network(err.to_string())
         }
     }
+
+    fn trace_request(&self, req: &Request) {
+        if self.client.request_logging_enabled() && enabled!(Level::TRACE) {
+            trace!(
+                "{} to {}: {}",
+                req.method,
+                req.url,
+                request_body_for_trace(req)
+            );
+        }
+    }
 }
 
 fn request_body_for_trace(req: &Request) -> String {
@@ -95,14 +110,7 @@ fn request_body_for_trace(req: &Request) -> String {
 
 impl HttpTransport for ReqwestTransport {
     async fn execute(&self, req: Request) -> Result<Response, TransportError> {
-        if enabled!(Level::TRACE) {
+        self.trace_request(&req);
-            trace!(
-                "{} to {}: {}",
-                req.method,
-                req.url,
-                request_body_for_trace(&req)
-            );
-        }
 
         let url = req.url.clone();
         let builder = self.build(req)?;
@@ -127,14 +135,7 @@ impl HttpTransport for ReqwestTransport {
     }
 
     async fn stream(&self, req: Request) -> Result<StreamResponse, TransportError> {
-        if enabled!(Level::TRACE) {
+        self.trace_request(&req);
-            trace!(
-                "{} to {}: {}",
-                req.method,
-                req.url,
-                request_body_for_trace(&req)
-            );
-        }
 
         let url = req.url.clone();
         let builder = self.build(req)?;
@@ -160,3 +161,7 @@ impl HttpTransport for ReqwestTransport {
         })
     }
 }
+
+#[cfg(test)]
+#[path = "transport_tests.rs"]
+mod tests;

codex-rs/http-client/src/transport_tests.rs

@@ -0,0 +1,92 @@
+use super::*;
+use serde_json::json;
+use std::io::Write;
+use std::sync::Arc;
+use std::sync::Mutex;
+use std::time::Duration;
+use tracing_subscriber::Layer;
+use tracing_subscriber::layer::SubscriberExt;
+
+#[tokio::test]
+async fn enabled_request_logging_emits_transport_url_and_body() {
+    let logs = capture_transport_logs(HttpClient::new(test_reqwest_client())).await;
+
+    assert!(logs.contains("log capture sentinel"));
+    assert!(logs.contains("url-secret"));
+    assert!(logs.contains("body-secret"));
+}
+
+#[tokio::test]
+async fn disabled_request_logging_suppresses_transport_url_and_body() {
+    let logs = capture_transport_logs(HttpClient::new_without_request_logging(
+        test_reqwest_client(),
+    ))
+    .await;
+
+    assert!(logs.contains("log capture sentinel"));
+    assert!(!logs.contains("url-secret"));
+    assert!(!logs.contains("body-secret"));
+}
+
+fn test_reqwest_client() -> reqwest::Client {
+    reqwest::Client::builder()
+        .no_proxy()
+        .build()
+        .expect("HTTP client should build")
+}
+
+async fn capture_transport_logs(client: HttpClient) -> String {
+    let unavailable_server =
+        std::net::TcpListener::bind(("127.0.0.1", 0)).expect("server port should bind");
+    let server_addr = unavailable_server
+        .local_addr()
+        .expect("server listener should have an address");
+    drop(unavailable_server);
+    let transport = ReqwestTransport::from_http_client(client);
+    let log_buffer = Arc::new(Mutex::new(Vec::new()));
+    let writer_buffer = Arc::clone(&log_buffer);
+    let subscriber = tracing_subscriber::registry().with(
+        tracing_subscriber::fmt::layer()
+            .with_ansi(false)
+            .with_writer(move || TestLogWriter(Arc::clone(&writer_buffer)))
+            .with_filter(
+                tracing_subscriber::filter::Targets::new()
+                    .with_target("codex_http_client::transport", tracing::Level::TRACE),
+            ),
+    );
+    let _guard = tracing::subscriber::set_default(subscriber);
+    tracing::trace!(target: "codex_http_client::transport", "log capture sentinel");
+    let mut request = Request::new(
+        Method::POST,
+        format!("http://{server_addr}/request?token=url-secret"),
+    )
+    .with_json(&json!({"token": "body-secret"}));
+    request.timeout = Some(Duration::from_secs(1));
+
+    let _ = transport.execute(request).await;
+
+    String::from_utf8(
+        log_buffer
+            .lock()
+            .expect("log buffer should not be poisoned")
+            .clone(),
+    )
+    .expect("captured logs should be UTF-8")
+}
+
+#[derive(Clone)]
+struct TestLogWriter(Arc<Mutex<Vec<u8>>>);
+
+impl Write for TestLogWriter {
+    fn write(&mut self, buf: &[u8]) -> std::io::Result<usize> {
+        self.0
+            .lock()
+            .map_err(|_| std::io::Error::other("log buffer should not be poisoned"))?
+            .extend_from_slice(buf);
+        Ok(buf.len())
+    }
+
+    fn flush(&mut self) -> std::io::Result<()> {
+        Ok(())
+    }
+}