From f5960fcc22b918e658bc486e53a80031d64fd41e Mon Sep 17 00:00:00 2001 From: Ahmed Ibrahim Date: Thu, 24 Sep 2026 01:33:55 +0000 Subject: [PATCH] Reduce dependency coupling in shared configuration crates (#47713) ## What changed - Move `Provider` and `RetryConfig` into `codex-client`, retaining re-exports from `codex-api`, and switch `codex-model-provider-info` to depend on `codex-client`. - Move feature-state metric emission into `codex-core`, preserving its behavior while removing the `codex-features` dependency on `codex-otel`. - Explicitly enable TLS features for `tonic` in `codex-config` and `reqwest` in `codex-http-client`. - Enable `png`'s `zlib-rs` feature directly in `codex-utils-image` to keep encoded PNG bytes stable when WebSocket dependencies are absent. GitOrigin-RevId: 91fd63ab01b55d516b3e80e3644d06af8d5a01f6 --- codex-rs/Cargo.lock | 5 +- codex-rs/codex-api/src/provider.rs | 103 +----------------------- codex-rs/codex-client/Cargo.toml | 1 + codex-rs/codex-client/src/lib.rs | 3 + codex-rs/codex-client/src/provider.rs | 103 ++++++++++++++++++++++++ codex-rs/config/Cargo.toml | 2 +- codex-rs/core/src/config/metrics.rs | 18 ++++- codex-rs/features/Cargo.toml | 1 - codex-rs/features/src/lib.rs | 19 ----- codex-rs/http-client/Cargo.toml | 2 +- codex-rs/model-provider-info/Cargo.toml | 2 +- codex-rs/model-provider-info/src/lib.rs | 4 +- codex-rs/utils/image/Cargo.toml | 6 ++ 13 files changed, 140 insertions(+), 129 deletions(-) create mode 100644 codex-rs/codex-client/src/provider.rs diff --git a/codex-rs/Cargo.lock b/codex-rs/Cargo.lock index 2fd33cc85e..4bcf45f427 100644 --- a/codex-rs/Cargo.lock +++ b/codex-rs/Cargo.lock @@ -2578,6 +2578,7 @@ dependencies = [ "rand 0.9.3", "tokio", "tracing", + "url", ] [[package]] @@ -3338,7 +3339,6 @@ name = "codex-features" version = "0.0.0" dependencies = [ "codex-network-proxy", - "codex-otel", "codex-protocol", "pretty_assertions", "schemars 0.8.22", @@ -3989,7 +3989,7 @@ dependencies = [ name = "codex-model-provider-info" version = "0.0.0" dependencies = [ - "codex-api", + "codex-client", "codex-protocol", "codex-utils-absolute-path", "codex-utils-redacted-string", @@ -4956,6 +4956,7 @@ dependencies = [ "divan", "image", "mime_guess", + "png", "thiserror 2.0.18", "tokio", ] diff --git a/codex-rs/codex-api/src/provider.rs b/codex-rs/codex-api/src/provider.rs index ea4363e5c6..97e5db155c 100644 --- a/codex-rs/codex-api/src/provider.rs +++ b/codex-rs/codex-api/src/provider.rs @@ -1,104 +1,5 @@ -use codex_client::Request; -use codex_client::RequestCompression; -use codex_client::RetryOn; -use codex_client::RetryPolicy; -use http::Method; -use http::header::HeaderMap; -use std::collections::HashMap; -use std::time::Duration; -use url::Url; - -/// High-level retry configuration for a provider. -/// -/// This is converted into a `RetryPolicy` used by `codex-client` to drive -/// transport-level retries for both unary and streaming calls. -#[derive(Debug, Clone)] -pub struct RetryConfig { - pub max_attempts: u64, - pub base_delay: Duration, - pub retry_429: bool, - pub retry_5xx: bool, - pub retry_transport: bool, -} - -impl RetryConfig { - pub fn to_policy(&self) -> RetryPolicy { - RetryPolicy { - max_attempts: self.max_attempts, - base_delay: self.base_delay, - retry_on: RetryOn { - retry_429: self.retry_429, - retry_5xx: self.retry_5xx, - retry_transport: self.retry_transport, - }, - } - } -} - -/// HTTP endpoint configuration used to talk to a concrete API deployment. -/// -/// Encapsulates base URL, default headers, query params, retry policy, and -/// stream idle timeout, plus helper methods for building requests. -#[derive(Debug, Clone)] -pub struct Provider { - pub name: String, - pub base_url: String, - pub query_params: Option>, - pub headers: HeaderMap, - pub retry: RetryConfig, - pub stream_idle_timeout: Duration, -} - -impl Provider { - pub fn url_for_path(&self, path: &str) -> String { - let base = self.base_url.trim_end_matches('/'); - let path = path.trim_start_matches('/'); - let mut url = if path.is_empty() { - base.to_string() - } else { - format!("{base}/{path}") - }; - - if let Some(params) = &self.query_params - && !params.is_empty() - { - let qs = params - .iter() - .map(|(k, v)| format!("{k}={v}")) - .collect::>() - .join("&"); - url.push('?'); - url.push_str(&qs); - } - - url - } - - pub fn build_request(&self, method: Method, path: &str) -> Request { - Request { - method, - url: self.url_for_path(path), - headers: self.headers.clone(), - body: None, - compression: RequestCompression::None, - timeout: None, - response_body_limit_bytes: None, - } - } - - pub fn websocket_url_for_path(&self, path: &str) -> Result { - let mut url = Url::parse(&self.url_for_path(path))?; - - let scheme = match url.scheme() { - "http" => "ws", - "https" => "wss", - "ws" | "wss" => return Ok(url), - _ => return Ok(url), - }; - let _ = url.set_scheme(scheme); - Ok(url) - } -} +pub use codex_client::Provider; +pub use codex_client::RetryConfig; pub fn is_azure_responses_provider(name: &str, base_url: Option<&str>) -> bool { if name.eq_ignore_ascii_case("azure") { diff --git a/codex-rs/codex-client/Cargo.toml b/codex-rs/codex-client/Cargo.toml index bc67e383a8..9ffe57d95f 100644 --- a/codex-rs/codex-client/Cargo.toml +++ b/codex-rs/codex-client/Cargo.toml @@ -12,6 +12,7 @@ http = { workspace = true } rand = { workspace = true } tokio = { workspace = true, features = ["macros", "rt", "time", "sync"] } tracing = { workspace = true } +url = { workspace = true } [lints] workspace = true diff --git a/codex-rs/codex-client/src/lib.rs b/codex-rs/codex-client/src/lib.rs index 60b8d00063..e634a2b294 100644 --- a/codex-rs/codex-client/src/lib.rs +++ b/codex-rs/codex-client/src/lib.rs @@ -1,7 +1,10 @@ +mod provider; mod retry; mod sse; mod telemetry; +pub use crate::provider::Provider; +pub use crate::provider::RetryConfig; pub use crate::retry::RetryOn; pub use crate::retry::RetryOperation; pub use crate::retry::RetryPolicy; diff --git a/codex-rs/codex-client/src/provider.rs b/codex-rs/codex-client/src/provider.rs new file mode 100644 index 0000000000..9feb92fb2c --- /dev/null +++ b/codex-rs/codex-client/src/provider.rs @@ -0,0 +1,103 @@ +//! Endpoint configuration and request construction shared by API clients. + +use crate::Request; +use crate::RequestCompression; +use crate::RetryOn; +use crate::RetryPolicy; +use http::Method; +use http::header::HeaderMap; +use std::collections::HashMap; +use std::time::Duration; +use url::Url; + +/// High-level retry configuration for a provider. +/// +/// This is converted into a `RetryPolicy` used by `codex-client` to drive +/// transport-level retries for both unary and streaming calls. +#[derive(Debug, Clone)] +pub struct RetryConfig { + pub max_attempts: u64, + pub base_delay: Duration, + pub retry_429: bool, + pub retry_5xx: bool, + pub retry_transport: bool, +} + +impl RetryConfig { + pub fn to_policy(&self) -> RetryPolicy { + RetryPolicy { + max_attempts: self.max_attempts, + base_delay: self.base_delay, + retry_on: RetryOn { + retry_429: self.retry_429, + retry_5xx: self.retry_5xx, + retry_transport: self.retry_transport, + }, + } + } +} + +/// HTTP endpoint configuration used to talk to a concrete API deployment. +/// +/// Encapsulates base URL, default headers, query params, retry policy, and +/// stream idle timeout, plus helper methods for building requests. +#[derive(Debug, Clone)] +pub struct Provider { + pub name: String, + pub base_url: String, + pub query_params: Option>, + pub headers: HeaderMap, + pub retry: RetryConfig, + pub stream_idle_timeout: Duration, +} + +impl Provider { + pub fn url_for_path(&self, path: &str) -> String { + let base = self.base_url.trim_end_matches('/'); + let path = path.trim_start_matches('/'); + let mut url = if path.is_empty() { + base.to_string() + } else { + format!("{base}/{path}") + }; + + if let Some(params) = &self.query_params + && !params.is_empty() + { + let qs = params + .iter() + .map(|(k, v)| format!("{k}={v}")) + .collect::>() + .join("&"); + url.push('?'); + url.push_str(&qs); + } + + url + } + + pub fn build_request(&self, method: Method, path: &str) -> Request { + Request { + method, + url: self.url_for_path(path), + headers: self.headers.clone(), + body: None, + compression: RequestCompression::None, + timeout: None, + response_body_limit_bytes: None, + } + } + + pub fn websocket_url_for_path(&self, path: &str) -> Result { + let mut url = Url::parse(&self.url_for_path(path))?; + + let scheme = match url.scheme() { + "http" => "ws", + "https" => "wss", + "ws" | "wss" => return Ok(url), + _ => return Ok(url), + }; + let _ = url.set_scheme(scheme); + Ok(url) + } +} diff --git a/codex-rs/config/Cargo.toml b/codex-rs/config/Cargo.toml index a6b06169a5..9be1fb4f47 100644 --- a/codex-rs/config/Cargo.toml +++ b/codex-rs/config/Cargo.toml @@ -42,7 +42,7 @@ thiserror = { workspace = true } tokio = { workspace = true, features = ["fs"] } toml = { workspace = true, features = ["preserve_order"] } toml_edit = { workspace = true } -tonic = { workspace = true } +tonic = { workspace = true, features = ["tls-native-roots", "tls-ring"] } tonic-prost = { workspace = true } tracing = { workspace = true } wildmatch = { workspace = true } diff --git a/codex-rs/core/src/config/metrics.rs b/codex-rs/core/src/config/metrics.rs index 48d3d71215..aaae887cbe 100644 --- a/codex-rs/core/src/config/metrics.rs +++ b/codex-rs/core/src/config/metrics.rs @@ -1,10 +1,26 @@ //! Metrics derived from loaded configuration at session start. use super::Config; +use codex_features::FEATURES; +use codex_features::Stage; use codex_otel::SessionTelemetry; pub(crate) fn emit_session_start_metrics(config: &Config, telemetry: &SessionTelemetry) { - config.features.emit_metrics(telemetry); + for feature in FEATURES { + if matches!(feature.stage, Stage::Removed) { + continue; + } + if config.features.enabled(feature.id) != feature.default_enabled { + telemetry.counter( + "codex.feature.state", + /*inc*/ 1, + &[ + ("feature", feature.key), + ("value", &config.features.enabled(feature.id).to_string()), + ], + ); + } + } #[cfg(windows)] crate::windows_system_config::emit_namespace_squatting_probe(); } diff --git a/codex-rs/features/Cargo.toml b/codex-rs/features/Cargo.toml index 03721949f2..edc96b64ae 100644 --- a/codex-rs/features/Cargo.toml +++ b/codex-rs/features/Cargo.toml @@ -14,7 +14,6 @@ workspace = true [dependencies] codex-network-proxy = { workspace = true } -codex-otel = { workspace = true } codex-protocol = { workspace = true } schemars = { workspace = true } serde = { workspace = true, features = ["derive"] } diff --git a/codex-rs/features/src/lib.rs b/codex-rs/features/src/lib.rs index dadf5b40ae..029a7700ca 100644 --- a/codex-rs/features/src/lib.rs +++ b/codex-rs/features/src/lib.rs @@ -3,7 +3,6 @@ //! This crate defines the feature registry plus the logic used to resolve an //! effective feature set from config-like inputs. -use codex_otel::SessionTelemetry; use codex_protocol::protocol::Event; use codex_protocol::protocol::EventMsg; use codex_protocol::protocol::WarningEvent; @@ -562,24 +561,6 @@ impl Features { self.legacy_usages.iter() } - pub fn emit_metrics(&self, otel: &SessionTelemetry) { - for feature in FEATURES { - if matches!(feature.stage, Stage::Removed) { - continue; - } - if self.enabled(feature.id) != feature.default_enabled { - otel.counter( - "codex.feature.state", - /*inc*/ 1, - &[ - ("feature", feature.key), - ("value", &self.enabled(feature.id).to_string()), - ], - ); - } - } - } - /// Apply a table of key -> bool toggles (e.g. from TOML). pub fn apply_map(&mut self, m: &BTreeMap) { for (k, v) in m { diff --git a/codex-rs/http-client/Cargo.toml b/codex-rs/http-client/Cargo.toml index 6bdc70f8b9..d59c2e78f7 100644 --- a/codex-rs/http-client/Cargo.toml +++ b/codex-rs/http-client/Cargo.toml @@ -13,7 +13,7 @@ http-body = "1.0.1" httpdate = { workspace = true } native-tls = "0.2" opentelemetry = { workspace = true } -reqwest = { workspace = true, features = ["json", "rustls-tls-native-roots", "stream"] } +reqwest = { workspace = true, features = ["json", "rustls-tls", "rustls-tls-native-roots", "stream"] } rustls = { workspace = true } rustls-native-certs = { workspace = true } rustls-pki-types = { workspace = true } diff --git a/codex-rs/model-provider-info/Cargo.toml b/codex-rs/model-provider-info/Cargo.toml index e7f48f14ad..e6a740d25d 100644 --- a/codex-rs/model-provider-info/Cargo.toml +++ b/codex-rs/model-provider-info/Cargo.toml @@ -13,7 +13,7 @@ path = "src/lib.rs" workspace = true [dependencies] -codex-api = { workspace = true } +codex-client = { workspace = true } codex-protocol = { workspace = true } codex-utils-redacted-string = { workspace = true } http = { workspace = true } diff --git a/codex-rs/model-provider-info/src/lib.rs b/codex-rs/model-provider-info/src/lib.rs index 9de61980f1..2e35b4b4c3 100644 --- a/codex-rs/model-provider-info/src/lib.rs +++ b/codex-rs/model-provider-info/src/lib.rs @@ -8,8 +8,8 @@ //! API provider construction applies the process-wide managed residency policy also //! used by default HTTP headers. -use codex_api::Provider as ApiProvider; -use codex_api::RetryConfig as ApiRetryConfig; +use codex_client::Provider as ApiProvider; +use codex_client::RetryConfig as ApiRetryConfig; use codex_protocol::auth::AuthMode; use codex_protocol::config_types::ModelProviderAuthInfo; use codex_protocol::error::CodexErr; diff --git a/codex-rs/utils/image/Cargo.toml b/codex-rs/utils/image/Cargo.toml index 7ba28f4996..3ac43b6df0 100644 --- a/codex-rs/utils/image/Cargo.toml +++ b/codex-rs/utils/image/Cargo.toml @@ -7,9 +7,15 @@ license.workspace = true [lints] workspace = true +[package.metadata.cargo-shear] +# "image" does not forward png's zlib-rs feature. This direct dependency keeps +# encoded PNG bytes stable when WebSocket dependencies are absent from a build. +ignored = ["png"] + [dependencies] base64 = { workspace = true } image = { workspace = true, features = ["jpeg", "png", "gif", "webp"] } +png = { version = "0.18.0", features = ["zlib-rs"] } codex-utils-cache = { workspace = true } mime_guess = { workspace = true } thiserror = { workspace = true }