Remove conversation_id and bring back request ID logging (#7830)

codex-rs/Cargo.lock

@@ -1098,6 +1098,7 @@ dependencies = [
  "codex-apply-patch",
  "codex-arg0",
  "codex-async-utils",
+ "codex-client",
  "codex-core",
  "codex-execpolicy",
  "codex-file-search",

codex-rs/codex-client/src/default_client.rs

@@ -0,0 +1,143 @@
+use http::Error as HttpError;
+use reqwest::IntoUrl;
+use reqwest::Method;
+use reqwest::Response;
+use reqwest::header::HeaderMap;
+use reqwest::header::HeaderName;
+use reqwest::header::HeaderValue;
+use serde::Serialize;
+use std::collections::HashMap;
+use std::fmt::Display;
+use std::time::Duration;
+
+#[derive(Clone, Debug)]
+pub struct CodexHttpClient {
+    inner: reqwest::Client,
+}
+
+impl CodexHttpClient {
+    pub fn new(inner: reqwest::Client) -> Self {
+        Self { inner }
+    }
+
+    pub fn get<U>(&self, url: U) -> CodexRequestBuilder
+    where
+        U: IntoUrl,
+    {
+        self.request(Method::GET, url)
+    }
+
+    pub fn post<U>(&self, url: U) -> CodexRequestBuilder
+    where
+        U: IntoUrl,
+    {
+        self.request(Method::POST, url)
+    }
+
+    pub fn request<U>(&self, method: Method, url: U) -> CodexRequestBuilder
+    where
+        U: IntoUrl,
+    {
+        let url_str = url.as_str().to_string();
+        CodexRequestBuilder::new(self.inner.request(method.clone(), url), method, url_str)
+    }
+}
+
+#[must_use = "requests are not sent unless `send` is awaited"]
+#[derive(Debug)]
+pub struct CodexRequestBuilder {
+    builder: reqwest::RequestBuilder,
+    method: Method,
+    url: String,
+}
+
+impl CodexRequestBuilder {
+    fn new(builder: reqwest::RequestBuilder, method: Method, url: String) -> Self {
+        Self {
+            builder,
+            method,
+            url,
+        }
+    }
+
+    fn map(self, f: impl FnOnce(reqwest::RequestBuilder) -> reqwest::RequestBuilder) -> Self {
+        Self {
+            builder: f(self.builder),
+            method: self.method,
+            url: self.url,
+        }
+    }
+
+    pub fn headers(self, headers: HeaderMap) -> Self {
+        self.map(|builder| builder.headers(headers))
+    }
+
+    pub fn header<K, V>(self, key: K, value: V) -> Self
+    where
+        HeaderName: TryFrom<K>,
+        <HeaderName as TryFrom<K>>::Error: Into<HttpError>,
+        HeaderValue: TryFrom<V>,
+        <HeaderValue as TryFrom<V>>::Error: Into<HttpError>,
+    {
+        self.map(|builder| builder.header(key, value))
+    }
+
+    pub fn bearer_auth<T>(self, token: T) -> Self
+    where
+        T: Display,
+    {
+        self.map(|builder| builder.bearer_auth(token))
+    }
+
+    pub fn timeout(self, timeout: Duration) -> Self {
+        self.map(|builder| builder.timeout(timeout))
+    }
+
+    pub fn json<T>(self, value: &T) -> Self
+    where
+        T: ?Sized + Serialize,
+    {
+        self.map(|builder| builder.json(value))
+    }
+
+    pub async fn send(self) -> Result<Response, reqwest::Error> {
+        match self.builder.send().await {
+            Ok(response) => {
+                let request_ids = Self::extract_request_ids(&response);
+                tracing::debug!(
+                    method = %self.method,
+                    url = %self.url,
+                    status = %response.status(),
+                    request_ids = ?request_ids,
+                    version = ?response.version(),
+                    "Request completed"
+                );
+
+                Ok(response)
+            }
+            Err(error) => {
+                let status = error.status();
+                tracing::debug!(
+                    method = %self.method,
+                    url = %self.url,
+                    status = status.map(|s| s.as_u16()),
+                    error = %error,
+                    "Request failed"
+                );
+                Err(error)
+            }
+        }
+    }
+
+    fn extract_request_ids(response: &Response) -> HashMap<String, String> {
+        ["cf-ray", "x-request-id", "x-oai-request-id"]
+            .iter()
+            .filter_map(|&name| {
+                let header_name = HeaderName::from_static(name);
+                let value = response.headers().get(header_name)?;
+                let value = value.to_str().ok()?.to_owned();
+                Some((name.to_owned(), value))
+            })
+            .collect()
+    }
+}

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

@@ -1,3 +1,4 @@
+mod default_client;
 mod error;
 mod request;
 mod retry;
@@ -5,6 +6,8 @@ mod sse;
 mod telemetry;
 mod transport;
 
+pub use crate::default_client::CodexHttpClient;
+pub use crate::default_client::CodexRequestBuilder;
 pub use crate::error::StreamError;
 pub use crate::error::TransportError;
 pub use crate::request::Request;

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

@@ -1,3 +1,5 @@
+use crate::default_client::CodexHttpClient;
+use crate::default_client::CodexRequestBuilder;
 use crate::error::TransportError;
 use crate::request::Request;
 use crate::request::Response;
@@ -28,15 +30,17 @@ pub trait HttpTransport: Send + Sync {
 
 #[derive(Clone, Debug)]
 pub struct ReqwestTransport {
-    client: reqwest::Client,
+    client: CodexHttpClient,
 }
 
 impl ReqwestTransport {
     pub fn new(client: reqwest::Client) -> Self {
-        Self { client }
+        Self {
+            client: CodexHttpClient::new(client),
+        }
     }
 
-    fn build(&self, req: Request) -> Result<reqwest::RequestBuilder, TransportError> {
+    fn build(&self, req: Request) -> Result<CodexRequestBuilder, TransportError> {
         let mut builder = self
             .client
             .request(

codex-rs/core/Cargo.toml

@@ -23,6 +23,7 @@ codex-api = { workspace = true }
 codex-app-server-protocol = { workspace = true }
 codex-apply-patch = { workspace = true }
 codex-async-utils = { workspace = true }
+codex-client = { workspace = true }
 codex-execpolicy = { workspace = true }
 codex-file-search = { workspace = true }
 codex-git = { workspace = true }

codex-rs/core/src/auth.rs

@@ -23,7 +23,6 @@ pub use crate::auth::storage::AuthDotJson;
 use crate::auth::storage::AuthStorageBackend;
 use crate::auth::storage::create_auth_storage;
 use crate::config::Config;
-use crate::default_client::CodexHttpClient;
 use crate::error::RefreshTokenFailedError;
 use crate::error::RefreshTokenFailedReason;
 use crate::token_data::KnownPlan as InternalKnownPlan;
@@ -31,6 +30,7 @@ use crate::token_data::PlanType as InternalPlanType;
 use crate::token_data::TokenData;
 use crate::token_data::parse_id_token;
 use crate::util::try_parse_error_message;
+use codex_client::CodexHttpClient;
 use codex_protocol::account::PlanType as AccountPlanType;
 use once_cell::sync::Lazy;
 use serde_json::Value;

codex-rs/core/src/default_client.rs

@@ -1,17 +1,12 @@
 use crate::spawn::CODEX_SANDBOX_ENV_VAR;
-use http::Error as HttpError;
-use reqwest::IntoUrl;
-use reqwest::Method;
-use reqwest::Response;
-use reqwest::header::HeaderName;
 use reqwest::header::HeaderValue;
-use serde::Serialize;
-use std::collections::HashMap;
-use std::fmt::Display;
 use std::sync::LazyLock;
 use std::sync::Mutex;
 use std::sync::OnceLock;
 
+use codex_client::CodexHttpClient;
+pub use codex_client::CodexRequestBuilder;
+
 /// Set this to add a suffix to the User-Agent string.
 ///
 /// It is not ideal that we're using a global singleton for this.
@@ -31,129 +26,6 @@ pub static USER_AGENT_SUFFIX: LazyLock<Mutex<Option<String>>> = LazyLock::new(||
 pub const DEFAULT_ORIGINATOR: &str = "codex_cli_rs";
 pub const CODEX_INTERNAL_ORIGINATOR_OVERRIDE_ENV_VAR: &str = "CODEX_INTERNAL_ORIGINATOR_OVERRIDE";
 
-#[derive(Clone, Debug)]
-pub struct CodexHttpClient {
-    inner: reqwest::Client,
-}
-
-impl CodexHttpClient {
-    fn new(inner: reqwest::Client) -> Self {
-        Self { inner }
-    }
-
-    pub fn get<U>(&self, url: U) -> CodexRequestBuilder
-    where
-        U: IntoUrl,
-    {
-        self.request(Method::GET, url)
-    }
-
-    pub fn post<U>(&self, url: U) -> CodexRequestBuilder
-    where
-        U: IntoUrl,
-    {
-        self.request(Method::POST, url)
-    }
-
-    pub fn request<U>(&self, method: Method, url: U) -> CodexRequestBuilder
-    where
-        U: IntoUrl,
-    {
-        let url_str = url.as_str().to_string();
-        CodexRequestBuilder::new(self.inner.request(method.clone(), url), method, url_str)
-    }
-}
-
-#[must_use = "requests are not sent unless `send` is awaited"]
-#[derive(Debug)]
-pub struct CodexRequestBuilder {
-    builder: reqwest::RequestBuilder,
-    method: Method,
-    url: String,
-}
-
-impl CodexRequestBuilder {
-    fn new(builder: reqwest::RequestBuilder, method: Method, url: String) -> Self {
-        Self {
-            builder,
-            method,
-            url,
-        }
-    }
-
-    fn map(self, f: impl FnOnce(reqwest::RequestBuilder) -> reqwest::RequestBuilder) -> Self {
-        Self {
-            builder: f(self.builder),
-            method: self.method,
-            url: self.url,
-        }
-    }
-
-    pub fn header<K, V>(self, key: K, value: V) -> Self
-    where
-        HeaderName: TryFrom<K>,
-        <HeaderName as TryFrom<K>>::Error: Into<HttpError>,
-        HeaderValue: TryFrom<V>,
-        <HeaderValue as TryFrom<V>>::Error: Into<HttpError>,
-    {
-        self.map(|builder| builder.header(key, value))
-    }
-
-    pub fn bearer_auth<T>(self, token: T) -> Self
-    where
-        T: Display,
-    {
-        self.map(|builder| builder.bearer_auth(token))
-    }
-
-    pub fn json<T>(self, value: &T) -> Self
-    where
-        T: ?Sized + Serialize,
-    {
-        self.map(|builder| builder.json(value))
-    }
-
-    pub async fn send(self) -> Result<Response, reqwest::Error> {
-        match self.builder.send().await {
-            Ok(response) => {
-                let request_ids = Self::extract_request_ids(&response);
-                tracing::debug!(
-                    method = %self.method,
-                    url = %self.url,
-                    status = %response.status(),
-                    request_ids = ?request_ids,
-                    version = ?response.version(),
-                    "Request completed"
-                );
-
-                Ok(response)
-            }
-            Err(error) => {
-                let status = error.status();
-                tracing::debug!(
-                    method = %self.method,
-                    url = %self.url,
-                    status = status.map(|s| s.as_u16()),
-                    error = %error,
-                    "Request failed"
-                );
-                Err(error)
-            }
-        }
-    }
-
-    fn extract_request_ids(response: &Response) -> HashMap<String, String> {
-        ["cf-ray", "x-request-id", "x-oai-request-id"]
-            .iter()
-            .filter_map(|&name| {
-                let header_name = HeaderName::from_static(name);
-                let value = response.headers().get(header_name)?;
-                let value = value.to_str().ok()?.to_owned();
-                Some((name.to_owned(), value))
-            })
-            .collect()
-    }
-}
 #[derive(Debug, Clone)]
 pub struct Originator {
     pub value: String,