From 61a9cfde6829272d838544a6e3cc9c56dbf75874 Mon Sep 17 00:00:00 2001 From: Ahmed Ibrahim Date: Tue, 23 Dec 2025 13:47:11 -0800 Subject: [PATCH 01/12] etag --- codex-rs/codex-api/src/endpoint/models.rs | 54 +++++++++---------- .../codex-api/tests/models_integration.rs | 7 ++- codex-rs/core/src/models_manager/manager.rs | 10 +--- codex-rs/protocol/src/openai_models.rs | 2 - 4 files changed, 28 insertions(+), 45 deletions(-) diff --git a/codex-rs/codex-api/src/endpoint/models.rs b/codex-rs/codex-api/src/endpoint/models.rs index b15f07fca2a..71bc19222c8 100644 --- a/codex-rs/codex-api/src/endpoint/models.rs +++ b/codex-rs/codex-api/src/endpoint/models.rs @@ -5,6 +5,7 @@ use crate::provider::Provider; use crate::telemetry::run_with_request_telemetry; use codex_client::HttpTransport; use codex_client::RequestTelemetry; +use codex_protocol::openai_models::ModelInfo; use codex_protocol::openai_models::ModelsResponse; use http::HeaderMap; use http::Method; @@ -41,7 +42,7 @@ impl ModelsClient { &self, client_version: &str, extra_headers: HeaderMap, - ) -> Result { + ) -> Result<(Vec, Option), ApiError> { let builder = || { let mut req = self.provider.build_request(Method::GET, self.path()); req.headers.extend(extra_headers.clone()); @@ -66,7 +67,7 @@ impl ModelsClient { .and_then(|value| value.to_str().ok()) .map(ToString::to_string); - let ModelsResponse { models, etag } = serde_json::from_slice::(&resp.body) + let ModelsResponse { models } = serde_json::from_slice::(&resp.body) .map_err(|e| { ApiError::Stream(format!( "failed to decode models response: {e}; body: {}", @@ -74,9 +75,7 @@ impl ModelsClient { )) })?; - let etag = header_etag.unwrap_or(etag); - - Ok(ModelsResponse { models, etag }) + Ok((models, header_etag)) } } @@ -102,16 +101,15 @@ mod tests { struct CapturingTransport { last_request: Arc>>, body: Arc, + etag: Option, } impl Default for CapturingTransport { fn default() -> Self { Self { last_request: Arc::new(Mutex::new(None)), - body: Arc::new(ModelsResponse { - models: Vec::new(), - etag: String::new(), - }), + body: Arc::new(ModelsResponse { models: Vec::new() }), + etag: None, } } } @@ -122,8 +120,8 @@ mod tests { *self.last_request.lock().unwrap() = Some(req); let body = serde_json::to_vec(&*self.body).unwrap(); let mut headers = HeaderMap::new(); - if !self.body.etag.is_empty() { - headers.insert(ETAG, self.body.etag.parse().unwrap()); + if let Some(etag) = &self.etag { + headers.insert(ETAG, etag.parse().unwrap()); } Ok(Response { status: StatusCode::OK, @@ -166,14 +164,12 @@ mod tests { #[tokio::test] async fn appends_client_version_query() { - let response = ModelsResponse { - models: Vec::new(), - etag: String::new(), - }; + let response = ModelsResponse { models: Vec::new() }; let transport = CapturingTransport { last_request: Arc::new(Mutex::new(None)), body: Arc::new(response), + etag: None, }; let client = ModelsClient::new( @@ -182,12 +178,12 @@ mod tests { DummyAuth, ); - let result = client + let (models, _) = client .list_models("0.99.0", HeaderMap::new()) .await .expect("request should succeed"); - assert_eq!(result.models.len(), 0); + assert_eq!(models.len(), 0); let url = transport .last_request @@ -232,12 +228,12 @@ mod tests { })) .unwrap(), ], - etag: String::new(), }; let transport = CapturingTransport { last_request: Arc::new(Mutex::new(None)), body: Arc::new(response), + etag: None, }; let client = ModelsClient::new( @@ -246,27 +242,25 @@ mod tests { DummyAuth, ); - let result = client + let (models, _) = client .list_models("0.99.0", HeaderMap::new()) .await .expect("request should succeed"); - assert_eq!(result.models.len(), 1); - assert_eq!(result.models[0].slug, "gpt-test"); - assert_eq!(result.models[0].supported_in_api, true); - assert_eq!(result.models[0].priority, 1); + assert_eq!(models.len(), 1); + assert_eq!(models[0].slug, "gpt-test"); + assert_eq!(models[0].supported_in_api, true); + assert_eq!(models[0].priority, 1); } #[tokio::test] async fn list_models_includes_etag() { - let response = ModelsResponse { - models: Vec::new(), - etag: "\"abc\"".to_string(), - }; + let response = ModelsResponse { models: Vec::new() }; let transport = CapturingTransport { last_request: Arc::new(Mutex::new(None)), body: Arc::new(response), + etag: Some("\"abc\"".to_string()), }; let client = ModelsClient::new( @@ -275,12 +269,12 @@ mod tests { DummyAuth, ); - let result = client + let (models, etag) = client .list_models("0.1.0", HeaderMap::new()) .await .expect("request should succeed"); - assert_eq!(result.models.len(), 0); - assert_eq!(result.etag, "\"abc\""); + assert_eq!(models.len(), 0); + assert_eq!(etag, Some("\"abc\"".to_string())); } } diff --git a/codex-rs/codex-api/tests/models_integration.rs b/codex-rs/codex-api/tests/models_integration.rs index 115c5a829ee..8219351946b 100644 --- a/codex-rs/codex-api/tests/models_integration.rs +++ b/codex-rs/codex-api/tests/models_integration.rs @@ -88,7 +88,6 @@ async fn models_client_hits_models_endpoint() { reasoning_summary_format: ReasoningSummaryFormat::None, experimental_supported_tools: Vec::new(), }], - etag: String::new(), }; Mock::given(method("GET")) @@ -104,13 +103,13 @@ async fn models_client_hits_models_endpoint() { let transport = ReqwestTransport::new(reqwest::Client::new()); let client = ModelsClient::new(transport, provider(&base_url), DummyAuth); - let result = client + let (models, _) = client .list_models("0.1.0", HeaderMap::new()) .await .expect("models request should succeed"); - assert_eq!(result.models.len(), 1); - assert_eq!(result.models[0].slug, "gpt-test"); + assert_eq!(models.len(), 1); + assert_eq!(models[0].slug, "gpt-test"); let received = server .received_requests() diff --git a/codex-rs/core/src/models_manager/manager.rs b/codex-rs/core/src/models_manager/manager.rs index 315380ade10..83a9464c147 100644 --- a/codex-rs/core/src/models_manager/manager.rs +++ b/codex-rs/core/src/models_manager/manager.rs @@ -94,13 +94,11 @@ impl ModelsManager { let client = ModelsClient::new(transport, api_provider, api_auth); let client_version = format_client_version_to_whole(); - let ModelsResponse { models, etag } = client + let (models, etag) = client .list_models(&client_version, HeaderMap::new()) .await .map_err(map_api_error)?; - let etag = (!etag.is_empty()).then_some(etag); - self.apply_remote_models(models.clone()).await; *self.etag.write().await = etag.clone(); self.persist_cache(&models, etag).await; @@ -389,7 +387,6 @@ mod tests { &server, ModelsResponse { models: remote_models.clone(), - etag: String::new(), }, ) .await; @@ -446,7 +443,6 @@ mod tests { &server, ModelsResponse { models: remote_models.clone(), - etag: String::new(), }, ) .await; @@ -501,7 +497,6 @@ mod tests { &server, ModelsResponse { models: initial_models.clone(), - etag: String::new(), }, ) .await; @@ -542,7 +537,6 @@ mod tests { &server, ModelsResponse { models: updated_models.clone(), - etag: String::new(), }, ) .await; @@ -576,7 +570,6 @@ mod tests { &server, ModelsResponse { models: initial_models, - etag: String::new(), }, ) .await; @@ -605,7 +598,6 @@ mod tests { &server, ModelsResponse { models: refreshed_models, - etag: String::new(), }, ) .await; diff --git a/codex-rs/protocol/src/openai_models.rs b/codex-rs/protocol/src/openai_models.rs index fc7f0b8ce02..5a950b05c46 100644 --- a/codex-rs/protocol/src/openai_models.rs +++ b/codex-rs/protocol/src/openai_models.rs @@ -196,8 +196,6 @@ pub struct ModelInfo { #[derive(Debug, Serialize, Deserialize, Clone, PartialEq, Eq, TS, JsonSchema, Default)] pub struct ModelsResponse { pub models: Vec, - #[serde(default)] - pub etag: String, } // convert ModelInfo to ModelPreset From c45eada0c35911df5ff405497289a54501b57818 Mon Sep 17 00:00:00 2001 From: Ahmed Ibrahim Date: Tue, 23 Dec 2025 14:01:20 -0800 Subject: [PATCH 02/12] etag --- codex-rs/core/src/client.rs | 12 +- codex-rs/core/src/codex.rs | 4 +- codex-rs/core/src/models_manager/manager.rs | 44 +++----- .../core/src/models_manager/model_family.rs | 103 +++++++++++------- codex-rs/core/tests/common/responses.rs | 9 +- codex-rs/core/tests/suite/remote_models.rs | 6 +- 6 files changed, 94 insertions(+), 84 deletions(-) diff --git a/codex-rs/core/src/client.rs b/codex-rs/core/src/client.rs index 11a3c5c65f3..30a4858ba2f 100644 --- a/codex-rs/core/src/client.rs +++ b/codex-rs/core/src/client.rs @@ -262,7 +262,10 @@ impl ModelClient { store_override: None, conversation_id: Some(conversation_id.clone()), session_source: Some(session_source.clone()), - extra_headers: beta_feature_headers(&self.config), + extra_headers: beta_feature_headers( + &self.config, + self.get_model_family().models_etag, + ), }; let stream_result = client @@ -398,7 +401,7 @@ fn build_api_prompt(prompt: &Prompt, instructions: String, tools_json: Vec ApiHeaderMap { +fn beta_feature_headers(config: &Config, etag: Option) -> ApiHeaderMap { let enabled = FEATURES .iter() .filter_map(|spec| { @@ -416,6 +419,11 @@ fn beta_feature_headers(config: &Config) -> ApiHeaderMap { { headers.insert("x-codex-beta-features", header_value); } + if let Some(etag) = etag + && let Ok(header_value) = HeaderValue::from_str(&etag) + { + headers.insert("X-If-Models-Match", header_value); + } headers } diff --git a/codex-rs/core/src/codex.rs b/codex-rs/core/src/codex.rs index 92ce74e3f58..2bca5f6b100 100644 --- a/codex-rs/core/src/codex.rs +++ b/codex-rs/core/src/codex.rs @@ -246,7 +246,9 @@ impl Codex { let config = Arc::new(config); if config.features.enabled(Feature::RemoteModels) - && let Err(err) = models_manager.refresh_available_models(&config).await + && let Err(err) = models_manager + .attempt_refresh_available_models(&config) + .await { error!("failed to refresh available models: {err:?}"); } diff --git a/codex-rs/core/src/models_manager/manager.rs b/codex-rs/core/src/models_manager/manager.rs index 83a9464c147..5b6e8ef5ad8 100644 --- a/codex-rs/core/src/models_manager/manager.rs +++ b/codex-rs/core/src/models_manager/manager.rs @@ -77,7 +77,7 @@ impl ModelsManager { } /// Fetch the latest remote models, using the on-disk cache when still fresh. - pub async fn refresh_available_models(&self, config: &Config) -> CoreResult<()> { + pub async fn attempt_refresh_available_models(&self, config: &Config) -> CoreResult<()> { if !config.features.enabled(Feature::RemoteModels) || self.auth_manager.get_auth_mode() == Some(AuthMode::ApiKey) { @@ -86,7 +86,10 @@ impl ModelsManager { if self.try_load_cache().await { return Ok(()); } + self.refresh_available_models().await + } + pub(crate) async fn refresh_available_models(&self) -> CoreResult<()> { let auth = self.auth_manager.auth(); let api_provider = self.provider.to_api_provider(Some(AuthMode::ChatGPT))?; let api_auth = auth_provider_from_auth(auth.clone(), &self.provider).await?; @@ -106,7 +109,7 @@ impl ModelsManager { } pub async fn list_models(&self, config: &Config) -> Vec { - if let Err(err) = self.refresh_available_models(config).await { + if let Err(err) = self.attempt_refresh_available_models(config).await { error!("failed to refresh available models: {err}"); } let remote_models = self.remote_models(config).await; @@ -124,8 +127,9 @@ impl ModelsManager { /// Look up the requested model family while applying remote metadata overrides. pub async fn construct_model_family(&self, model: &str, config: &Config) -> ModelFamily { + let etag = self.etag.read().await.clone(); Self::find_family_for_model(model) - .with_remote_overrides(self.remote_models(config).await) + .with_remote_overrides(self.remote_models(config).await, etag) .with_config_overrides(config) } @@ -133,7 +137,7 @@ impl ModelsManager { if let Some(model) = model.as_ref() { return model.to_string(); } - if let Err(err) = self.refresh_available_models(config).await { + if let Err(err) = self.attempt_refresh_available_models(config).await { error!("failed to refresh available models: {err}"); } // if codex-auto-balanced exists & signed in with chatgpt mode, return it, otherwise return the default model @@ -286,26 +290,14 @@ impl ModelsManager { /// Convert a client version string to a whole version string (e.g. "1.2.3-alpha.4" -> "1.2.3") fn format_client_version_to_whole() -> String { - format_client_version_from_parts( + format!( + "{}.{}.{}", env!("CARGO_PKG_VERSION_MAJOR"), env!("CARGO_PKG_VERSION_MINOR"), - env!("CARGO_PKG_VERSION_PATCH"), + env!("CARGO_PKG_VERSION_PATCH") ) } -fn format_client_version_from_parts(major: &str, minor: &str, patch: &str) -> String { - const DEV_VERSION: &str = "0.0.0"; - const FALLBACK_VERSION: &str = "99.99.99"; - - let normalized = format!("{major}.{minor}.{patch}"); - - if normalized == DEV_VERSION { - FALLBACK_VERSION.to_string() - } else { - normalized - } -} - #[cfg(test)] mod tests { use super::cache::ModelsCache; @@ -404,7 +396,7 @@ mod tests { let manager = ModelsManager::with_provider(auth_manager, provider); manager - .refresh_available_models(&config) + .attempt_refresh_available_models(&config) .await .expect("refresh succeeds"); let cached_remote = manager.remote_models(&config).await; @@ -463,7 +455,7 @@ mod tests { let manager = ModelsManager::with_provider(auth_manager, provider); manager - .refresh_available_models(&config) + .attempt_refresh_available_models(&config) .await .expect("first refresh succeeds"); assert_eq!( @@ -474,7 +466,7 @@ mod tests { // Second call should read from cache and avoid the network. manager - .refresh_available_models(&config) + .attempt_refresh_available_models(&config) .await .expect("cached refresh succeeds"); assert_eq!( @@ -517,7 +509,7 @@ mod tests { let manager = ModelsManager::with_provider(auth_manager, provider); manager - .refresh_available_models(&config) + .attempt_refresh_available_models(&config) .await .expect("initial refresh succeeds"); @@ -542,7 +534,7 @@ mod tests { .await; manager - .refresh_available_models(&config) + .attempt_refresh_available_models(&config) .await .expect("second refresh succeeds"); assert_eq!( @@ -588,7 +580,7 @@ mod tests { manager.cache_ttl = Duration::ZERO; manager - .refresh_available_models(&config) + .attempt_refresh_available_models(&config) .await .expect("initial refresh succeeds"); @@ -603,7 +595,7 @@ mod tests { .await; manager - .refresh_available_models(&config) + .attempt_refresh_available_models(&config) .await .expect("second refresh succeeds"); diff --git a/codex-rs/core/src/models_manager/model_family.rs b/codex-rs/core/src/models_manager/model_family.rs index 9e2fb924685..90c69d680ef 100644 --- a/codex-rs/core/src/models_manager/model_family.rs +++ b/codex-rs/core/src/models_manager/model_family.rs @@ -81,6 +81,9 @@ pub struct ModelFamily { pub shell_type: ConfigShellToolType, pub truncation_policy: TruncationPolicy, + + /// The ETag of the models cache, if known. + pub models_etag: Option, } impl ModelFamily { @@ -99,12 +102,17 @@ impl ModelFamily { } self } - pub(super) fn with_remote_overrides(mut self, remote_models: Vec) -> Self { + pub(super) fn with_remote_overrides( + mut self, + remote_models: Vec, + etag: Option, + ) -> Self { for model in remote_models { if model.slug == self.slug { self.apply_remote_overrides(model); } } + self.models_etag = etag; self } @@ -186,6 +194,7 @@ macro_rules! model_family { default_verbosity: None, default_reasoning_effort: None, truncation_policy: TruncationPolicy::Bytes(10_000), + models_etag: None, }; // apply overrides @@ -427,6 +436,7 @@ fn derive_default_model_family(model: &str) -> ModelFamily { default_verbosity: None, default_reasoning_effort: None, truncation_policy: TruncationPolicy::Bytes(10_000), + models_etag: None, } } @@ -470,18 +480,21 @@ mod tests { let family = model_family!("gpt-4o-mini", "gpt-4o-mini"); assert_ne!(family.default_reasoning_effort, Some(ReasoningEffort::High)); - let updated = family.with_remote_overrides(vec![ - remote( - "gpt-4o-mini", - ReasoningEffort::High, - ConfigShellToolType::ShellCommand, - ), - remote( - "other-model", - ReasoningEffort::Low, - ConfigShellToolType::UnifiedExec, - ), - ]); + let updated = family.with_remote_overrides( + vec![ + remote( + "gpt-4o-mini", + ReasoningEffort::High, + ConfigShellToolType::ShellCommand, + ), + remote( + "other-model", + ReasoningEffort::Low, + ConfigShellToolType::UnifiedExec, + ), + ], + None, + ); assert_eq!( updated.default_reasoning_effort, @@ -498,11 +511,14 @@ mod tests { shell_type: ConfigShellToolType::Local ); - let updated = family.clone().with_remote_overrides(vec![remote( - "other", - ReasoningEffort::High, - ConfigShellToolType::ShellCommand, - )]); + let updated = family.clone().with_remote_overrides( + vec![remote( + "other", + ReasoningEffort::High, + ConfigShellToolType::ShellCommand, + )], + None, + ); assert_eq!( updated.default_reasoning_effort, @@ -527,31 +543,34 @@ mod tests { reasoning_summary_format: ReasoningSummaryFormat::None, ); - let updated = family.with_remote_overrides(vec![ModelInfo { - slug: "gpt-5.1".to_string(), - display_name: "gpt-5.1".to_string(), - description: Some("desc".to_string()), - default_reasoning_level: ReasoningEffort::High, - supported_reasoning_levels: vec![ReasoningEffortPreset { - effort: ReasoningEffort::High, - description: "High".to_string(), + let updated = family.with_remote_overrides( + vec![ModelInfo { + slug: "gpt-5.1".to_string(), + display_name: "gpt-5.1".to_string(), + description: Some("desc".to_string()), + default_reasoning_level: ReasoningEffort::High, + supported_reasoning_levels: vec![ReasoningEffortPreset { + effort: ReasoningEffort::High, + description: "High".to_string(), + }], + shell_type: ConfigShellToolType::ShellCommand, + visibility: ModelVisibility::List, + supported_in_api: true, + priority: 10, + upgrade: None, + base_instructions: Some("Remote instructions".to_string()), + supports_reasoning_summaries: true, + support_verbosity: true, + default_verbosity: Some(Verbosity::High), + apply_patch_tool_type: Some(ApplyPatchToolType::Freeform), + truncation_policy: TruncationPolicyConfig::tokens(2_000), + supports_parallel_tool_calls: true, + context_window: Some(400_000), + reasoning_summary_format: ReasoningSummaryFormat::Experimental, + experimental_supported_tools: vec!["alpha".to_string(), "beta".to_string()], }], - shell_type: ConfigShellToolType::ShellCommand, - visibility: ModelVisibility::List, - supported_in_api: true, - priority: 10, - upgrade: None, - base_instructions: Some("Remote instructions".to_string()), - supports_reasoning_summaries: true, - support_verbosity: true, - default_verbosity: Some(Verbosity::High), - apply_patch_tool_type: Some(ApplyPatchToolType::Freeform), - truncation_policy: TruncationPolicyConfig::tokens(2_000), - supports_parallel_tool_calls: true, - context_window: Some(400_000), - reasoning_summary_format: ReasoningSummaryFormat::Experimental, - experimental_supported_tools: vec!["alpha".to_string(), "beta".to_string()], - }]); + None, + ); assert_eq!( updated.default_reasoning_effort, diff --git a/codex-rs/core/tests/common/responses.rs b/codex-rs/core/tests/common/responses.rs index b98b29625eb..8fcabdda541 100644 --- a/codex-rs/core/tests/common/responses.rs +++ b/codex-rs/core/tests/common/responses.rs @@ -677,14 +677,7 @@ pub async fn start_mock_server() -> MockServer { .await; // Provide a default `/models` response so tests remain hermetic when the client queries it. - let _ = mount_models_once( - &server, - ModelsResponse { - models: Vec::new(), - etag: String::new(), - }, - ) - .await; + let _ = mount_models_once(&server, ModelsResponse { models: Vec::new() }).await; server } diff --git a/codex-rs/core/tests/suite/remote_models.rs b/codex-rs/core/tests/suite/remote_models.rs index 15a78660982..2034f512ff5 100644 --- a/codex-rs/core/tests/suite/remote_models.rs +++ b/codex-rs/core/tests/suite/remote_models.rs @@ -91,7 +91,6 @@ async fn remote_models_remote_model_uses_unified_exec() -> Result<()> { &server, ModelsResponse { models: vec![remote_model], - etag: String::new(), }, ) .await; @@ -229,7 +228,6 @@ async fn remote_models_apply_remote_base_instructions() -> Result<()> { &server, ModelsResponse { models: vec![remote_model], - etag: String::new(), }, ) .await; @@ -307,7 +305,6 @@ async fn remote_models_preserve_builtin_presets() -> Result<()> { &server, ModelsResponse { models: vec![remote_model.clone()], - etag: String::new(), }, ) .await; @@ -327,7 +324,7 @@ async fn remote_models_preserve_builtin_presets() -> Result<()> { ); manager - .refresh_available_models(&config) + .attempt_refresh_available_models(&config) .await .expect("refresh succeeds"); @@ -365,7 +362,6 @@ async fn remote_models_hide_picker_only_models() -> Result<()> { &server, ModelsResponse { models: vec![remote_model], - etag: String::new(), }, ) .await; From 3b2d4c9c367391b24caaaafb8a15546cd05d24dd Mon Sep 17 00:00:00 2001 From: Ahmed Ibrahim Date: Tue, 23 Dec 2025 14:17:06 -0800 Subject: [PATCH 03/12] refresh --- codex-rs/codex-api/src/error.rs | 2 + codex-rs/core/src/api_bridge.rs | 3 + codex-rs/core/src/client.rs | 6 +- codex-rs/core/src/codex.rs | 21 +++- codex-rs/core/src/error.rs | 9 +- codex-rs/core/src/models_manager/manager.rs | 7 +- .../core/src/models_manager/model_family.rs | 103 +++++++----------- .../core/tests/chat_completions_payload.rs | 7 ++ codex-rs/core/tests/chat_completions_sse.rs | 6 + codex-rs/core/tests/responses_headers.rs | 16 +++ codex-rs/core/tests/suite/client.rs | 5 + 11 files changed, 116 insertions(+), 69 deletions(-) diff --git a/codex-rs/codex-api/src/error.rs b/codex-rs/codex-api/src/error.rs index 60118e87235..ef2f98a9144 100644 --- a/codex-rs/codex-api/src/error.rs +++ b/codex-rs/codex-api/src/error.rs @@ -25,6 +25,8 @@ pub enum ApiError { }, #[error("rate limit: {0}")] RateLimit(String), + #[error("models catalog has changed. Please refresh your models list.")] + ModelsCatalogChanged, } impl From for ApiError { diff --git a/codex-rs/core/src/api_bridge.rs b/codex-rs/core/src/api_bridge.rs index 79fd67d6501..ff3c80b0faa 100644 --- a/codex-rs/core/src/api_bridge.rs +++ b/codex-rs/core/src/api_bridge.rs @@ -20,6 +20,7 @@ pub(crate) fn map_api_error(err: ApiError) -> CodexErr { ApiError::ContextWindowExceeded => CodexErr::ContextWindowExceeded, ApiError::QuotaExceeded => CodexErr::QuotaExceeded, ApiError::UsageNotIncluded => CodexErr::UsageNotIncluded, + ApiError::ModelsCatalogChanged => CodexErr::ModelsCatalogChanged, ApiError::Retryable { message, delay } => CodexErr::Stream(message, delay), ApiError::Stream(msg) => CodexErr::Stream(msg, None), ApiError::Api { status, message } => CodexErr::UnexpectedStatus(UnexpectedResponseError { @@ -43,6 +44,8 @@ pub(crate) fn map_api_error(err: ApiError) -> CodexErr { } else { CodexErr::InvalidRequest(body_text) } + } else if status == http::StatusCode::PRECONDITION_FAILED { + CodexErr::ModelsCatalogChanged } else if status == http::StatusCode::INTERNAL_SERVER_ERROR { CodexErr::InternalServerError } else if status == http::StatusCode::TOO_MANY_REQUESTS { diff --git a/codex-rs/core/src/client.rs b/codex-rs/core/src/client.rs index 30a4858ba2f..54feb0a0a9b 100644 --- a/codex-rs/core/src/client.rs +++ b/codex-rs/core/src/client.rs @@ -2,6 +2,7 @@ use std::sync::Arc; use crate::api_bridge::auth_provider_from_auth; use crate::api_bridge::map_api_error; +use crate::models_manager::manager::ModelsManager; use codex_api::AggregateStreamExt; use codex_api::ChatClient as ApiChatClient; use codex_api::CompactClient as ApiCompactClient; @@ -58,6 +59,7 @@ pub struct ModelClient { config: Arc, auth_manager: Option>, model_family: ModelFamily, + models_manager: Arc, otel_manager: OtelManager, provider: ModelProviderInfo, conversation_id: ConversationId, @@ -78,6 +80,7 @@ impl ModelClient { summary: ReasoningSummaryConfig, conversation_id: ConversationId, session_source: SessionSource, + models_manager: Arc, ) -> Self { Self { config, @@ -89,6 +92,7 @@ impl ModelClient { effort, summary, session_source, + models_manager, } } @@ -264,7 +268,7 @@ impl ModelClient { session_source: Some(session_source.clone()), extra_headers: beta_feature_headers( &self.config, - self.get_model_family().models_etag, + self.models_manager.get_etag().await, ), }; diff --git a/codex-rs/core/src/codex.rs b/codex-rs/core/src/codex.rs index 2bca5f6b100..6350774b311 100644 --- a/codex-rs/core/src/codex.rs +++ b/codex-rs/core/src/codex.rs @@ -482,6 +482,7 @@ impl Session { #[allow(clippy::too_many_arguments)] fn make_turn_context( auth_manager: Option>, + models_manager: Arc, otel_manager: &OtelManager, provider: ModelProviderInfo, session_configuration: &SessionConfiguration, @@ -506,6 +507,7 @@ impl Session { session_configuration.model_reasoning_summary, conversation_id, session_configuration.session_source.clone(), + Arc::clone(&models_manager), ); let tools_config = ToolsConfig::new(&ToolsConfigParams { @@ -911,6 +913,7 @@ impl Session { .await; let mut turn_context: TurnContext = Self::make_turn_context( Some(Arc::clone(&self.services.auth_manager)), + Arc::clone(&self.services.models_manager), &self.services.otel_manager, session_configuration.provider.clone(), &session_configuration, @@ -2128,6 +2131,7 @@ async fn spawn_review_thread( per_turn_config.model_reasoning_summary, sess.conversation_id, parent_turn_context.client.get_session_source(), + Arc::clone(&sess.services.models_manager), ); let review_turn_context = TurnContext { @@ -2441,6 +2445,17 @@ async fn run_turn( let max_retries = turn_context.client.get_provider().stream_max_retries(); if retries < max_retries { retries += 1; + if matches!(e, CodexErr::ModelsCatalogChanged) { + if let Err(err) = sess + .services + .models_manager + .refresh_available_models() + .await + { + error!("failed to refresh available models: {err}"); + } + continue; + } let delay = match e { CodexErr::Stream(_, Some(delay)) => delay, _ => backoff(retries), @@ -3140,13 +3155,14 @@ mod tests { exec_policy, auth_manager: auth_manager.clone(), otel_manager: otel_manager.clone(), - models_manager, + models_manager: Arc::clone(&models_manager), tool_approvals: Mutex::new(ApprovalStore::default()), skills_manager, }; let turn_context = Session::make_turn_context( Some(Arc::clone(&auth_manager)), + models_manager.clone(), &otel_manager, session_configuration.provider.clone(), &session_configuration, @@ -3227,13 +3243,14 @@ mod tests { exec_policy, auth_manager: Arc::clone(&auth_manager), otel_manager: otel_manager.clone(), - models_manager, + models_manager: Arc::clone(&models_manager), tool_approvals: Mutex::new(ApprovalStore::default()), skills_manager, }; let turn_context = Arc::new(Session::make_turn_context( Some(Arc::clone(&auth_manager)), + models_manager.clone(), &otel_manager, session_configuration.provider.clone(), &session_configuration, diff --git a/codex-rs/core/src/error.rs b/codex-rs/core/src/error.rs index e8fa91d26e8..470a2cb2d5e 100644 --- a/codex-rs/core/src/error.rs +++ b/codex-rs/core/src/error.rs @@ -148,6 +148,9 @@ pub enum CodexErr { #[error("{0}")] RefreshTokenFailed(RefreshTokenFailedError), + #[error("Models catalog has changed. Please refresh your models list.")] + ModelsCatalogChanged, + #[error("Fatal error: {0}")] Fatal(String), @@ -455,9 +458,9 @@ impl CodexErr { CodexErr::SessionConfiguredNotFirstEvent | CodexErr::InternalServerError | CodexErr::InternalAgentDied => CodexErrorInfo::InternalServerError, - CodexErr::UnsupportedOperation(_) | CodexErr::ConversationNotFound(_) => { - CodexErrorInfo::BadRequest - } + CodexErr::UnsupportedOperation(_) + | CodexErr::ConversationNotFound(_) + | CodexErr::ModelsCatalogChanged => CodexErrorInfo::BadRequest, CodexErr::Sandbox(_) => CodexErrorInfo::SandboxError, _ => CodexErrorInfo::Other, } diff --git a/codex-rs/core/src/models_manager/manager.rs b/codex-rs/core/src/models_manager/manager.rs index 5b6e8ef5ad8..7199266ee17 100644 --- a/codex-rs/core/src/models_manager/manager.rs +++ b/codex-rs/core/src/models_manager/manager.rs @@ -127,9 +127,8 @@ impl ModelsManager { /// Look up the requested model family while applying remote metadata overrides. pub async fn construct_model_family(&self, model: &str, config: &Config) -> ModelFamily { - let etag = self.etag.read().await.clone(); Self::find_family_for_model(model) - .with_remote_overrides(self.remote_models(config).await, etag) + .with_remote_overrides(self.remote_models(config).await) .with_config_overrides(config) } @@ -156,6 +155,10 @@ impl ModelsManager { OPENAI_DEFAULT_API_MODEL.to_string() } + pub async fn get_etag(&self) -> Option { + self.etag.read().await.clone() + } + #[cfg(any(test, feature = "test-support"))] pub fn get_model_offline(model: Option<&str>) -> String { model.unwrap_or(OPENAI_DEFAULT_CHATGPT_MODEL).to_string() diff --git a/codex-rs/core/src/models_manager/model_family.rs b/codex-rs/core/src/models_manager/model_family.rs index 90c69d680ef..9e2fb924685 100644 --- a/codex-rs/core/src/models_manager/model_family.rs +++ b/codex-rs/core/src/models_manager/model_family.rs @@ -81,9 +81,6 @@ pub struct ModelFamily { pub shell_type: ConfigShellToolType, pub truncation_policy: TruncationPolicy, - - /// The ETag of the models cache, if known. - pub models_etag: Option, } impl ModelFamily { @@ -102,17 +99,12 @@ impl ModelFamily { } self } - pub(super) fn with_remote_overrides( - mut self, - remote_models: Vec, - etag: Option, - ) -> Self { + pub(super) fn with_remote_overrides(mut self, remote_models: Vec) -> Self { for model in remote_models { if model.slug == self.slug { self.apply_remote_overrides(model); } } - self.models_etag = etag; self } @@ -194,7 +186,6 @@ macro_rules! model_family { default_verbosity: None, default_reasoning_effort: None, truncation_policy: TruncationPolicy::Bytes(10_000), - models_etag: None, }; // apply overrides @@ -436,7 +427,6 @@ fn derive_default_model_family(model: &str) -> ModelFamily { default_verbosity: None, default_reasoning_effort: None, truncation_policy: TruncationPolicy::Bytes(10_000), - models_etag: None, } } @@ -480,21 +470,18 @@ mod tests { let family = model_family!("gpt-4o-mini", "gpt-4o-mini"); assert_ne!(family.default_reasoning_effort, Some(ReasoningEffort::High)); - let updated = family.with_remote_overrides( - vec![ - remote( - "gpt-4o-mini", - ReasoningEffort::High, - ConfigShellToolType::ShellCommand, - ), - remote( - "other-model", - ReasoningEffort::Low, - ConfigShellToolType::UnifiedExec, - ), - ], - None, - ); + let updated = family.with_remote_overrides(vec![ + remote( + "gpt-4o-mini", + ReasoningEffort::High, + ConfigShellToolType::ShellCommand, + ), + remote( + "other-model", + ReasoningEffort::Low, + ConfigShellToolType::UnifiedExec, + ), + ]); assert_eq!( updated.default_reasoning_effort, @@ -511,14 +498,11 @@ mod tests { shell_type: ConfigShellToolType::Local ); - let updated = family.clone().with_remote_overrides( - vec![remote( - "other", - ReasoningEffort::High, - ConfigShellToolType::ShellCommand, - )], - None, - ); + let updated = family.clone().with_remote_overrides(vec![remote( + "other", + ReasoningEffort::High, + ConfigShellToolType::ShellCommand, + )]); assert_eq!( updated.default_reasoning_effort, @@ -543,34 +527,31 @@ mod tests { reasoning_summary_format: ReasoningSummaryFormat::None, ); - let updated = family.with_remote_overrides( - vec![ModelInfo { - slug: "gpt-5.1".to_string(), - display_name: "gpt-5.1".to_string(), - description: Some("desc".to_string()), - default_reasoning_level: ReasoningEffort::High, - supported_reasoning_levels: vec![ReasoningEffortPreset { - effort: ReasoningEffort::High, - description: "High".to_string(), - }], - shell_type: ConfigShellToolType::ShellCommand, - visibility: ModelVisibility::List, - supported_in_api: true, - priority: 10, - upgrade: None, - base_instructions: Some("Remote instructions".to_string()), - supports_reasoning_summaries: true, - support_verbosity: true, - default_verbosity: Some(Verbosity::High), - apply_patch_tool_type: Some(ApplyPatchToolType::Freeform), - truncation_policy: TruncationPolicyConfig::tokens(2_000), - supports_parallel_tool_calls: true, - context_window: Some(400_000), - reasoning_summary_format: ReasoningSummaryFormat::Experimental, - experimental_supported_tools: vec!["alpha".to_string(), "beta".to_string()], + let updated = family.with_remote_overrides(vec![ModelInfo { + slug: "gpt-5.1".to_string(), + display_name: "gpt-5.1".to_string(), + description: Some("desc".to_string()), + default_reasoning_level: ReasoningEffort::High, + supported_reasoning_levels: vec![ReasoningEffortPreset { + effort: ReasoningEffort::High, + description: "High".to_string(), }], - None, - ); + shell_type: ConfigShellToolType::ShellCommand, + visibility: ModelVisibility::List, + supported_in_api: true, + priority: 10, + upgrade: None, + base_instructions: Some("Remote instructions".to_string()), + supports_reasoning_summaries: true, + support_verbosity: true, + default_verbosity: Some(Verbosity::High), + apply_patch_tool_type: Some(ApplyPatchToolType::Freeform), + truncation_policy: TruncationPolicyConfig::tokens(2_000), + supports_parallel_tool_calls: true, + context_window: Some(400_000), + reasoning_summary_format: ReasoningSummaryFormat::Experimental, + experimental_supported_tools: vec!["alpha".to_string(), "beta".to_string()], + }]); assert_eq!( updated.default_reasoning_effort, diff --git a/codex-rs/core/tests/chat_completions_payload.rs b/codex-rs/core/tests/chat_completions_payload.rs index 8af5df21695..a7e140c3e9e 100644 --- a/codex-rs/core/tests/chat_completions_payload.rs +++ b/codex-rs/core/tests/chat_completions_payload.rs @@ -3,6 +3,7 @@ use std::sync::Arc; use codex_app_server_protocol::AuthMode; +use codex_core::AuthManager; use codex_core::ContentItem; use codex_core::LocalShellAction; use codex_core::LocalShellExecAction; @@ -12,6 +13,7 @@ use codex_core::ModelProviderInfo; use codex_core::Prompt; use codex_core::ResponseItem; use codex_core::WireApi; +use codex_core::auth::AuthCredentialsStoreMode; use codex_core::models_manager::manager::ModelsManager; use codex_otel::otel_manager::OtelManager; use codex_protocol::ConversationId; @@ -98,6 +100,11 @@ async fn run_request(input: Vec) -> Value { summary, conversation_id, SessionSource::Exec, + Arc::new(ModelsManager::new(Arc::new(AuthManager::new( + config.codex_home.clone(), + false, + AuthCredentialsStoreMode::File, + )))), ); let mut prompt = Prompt::default(); diff --git a/codex-rs/core/tests/chat_completions_sse.rs b/codex-rs/core/tests/chat_completions_sse.rs index 4f05838279a..7c3743b2301 100644 --- a/codex-rs/core/tests/chat_completions_sse.rs +++ b/codex-rs/core/tests/chat_completions_sse.rs @@ -1,5 +1,6 @@ use assert_matches::assert_matches; use codex_core::AuthManager; +use codex_core::auth::AuthCredentialsStoreMode; use std::sync::Arc; use tracing_test::traced_test; @@ -99,6 +100,11 @@ async fn run_stream_with_bytes(sse_body: &[u8]) -> Vec { summary, conversation_id, SessionSource::Exec, + Arc::new(ModelsManager::new(Arc::new(AuthManager::new( + config.codex_home.clone(), + false, + AuthCredentialsStoreMode::File, + )))), ); let mut prompt = Prompt::default(); diff --git a/codex-rs/core/tests/responses_headers.rs b/codex-rs/core/tests/responses_headers.rs index c406fdbc879..151979ea281 100644 --- a/codex-rs/core/tests/responses_headers.rs +++ b/codex-rs/core/tests/responses_headers.rs @@ -10,6 +10,7 @@ use codex_core::Prompt; use codex_core::ResponseEvent; use codex_core::ResponseItem; use codex_core::WireApi; +use codex_core::auth::AuthCredentialsStoreMode; use codex_core::models_manager::manager::ModelsManager; use codex_otel::otel_manager::OtelManager; use codex_protocol::ConversationId; @@ -92,6 +93,11 @@ async fn responses_stream_includes_subagent_header_on_review() { summary, conversation_id, session_source, + Arc::new(ModelsManager::new(Arc::new(AuthManager::new( + config.codex_home.clone(), + false, + AuthCredentialsStoreMode::File, + )))), ); let mut prompt = Prompt::default(); @@ -187,6 +193,11 @@ async fn responses_stream_includes_subagent_header_on_other() { summary, conversation_id, session_source, + Arc::new(ModelsManager::new(Arc::new(AuthManager::new( + config.codex_home.clone(), + false, + AuthCredentialsStoreMode::File, + )))), ); let mut prompt = Prompt::default(); @@ -281,6 +292,11 @@ async fn responses_respects_model_family_overrides_from_config() { summary, conversation_id, session_source, + Arc::new(ModelsManager::new(Arc::new(AuthManager::new( + config.codex_home.clone(), + false, + AuthCredentialsStoreMode::File, + )))), ); let mut prompt = Prompt::default(); diff --git a/codex-rs/core/tests/suite/client.rs b/codex-rs/core/tests/suite/client.rs index a22027f99fb..b4bbe6e6db8 100644 --- a/codex-rs/core/tests/suite/client.rs +++ b/codex-rs/core/tests/suite/client.rs @@ -1152,6 +1152,11 @@ async fn azure_responses_request_includes_store_and_reasoning_ids() { summary, conversation_id, SessionSource::Exec, + Arc::new(ModelsManager::new(Arc::new(AuthManager::new( + config.codex_home.clone(), + false, + AuthCredentialsStoreMode::File, + )))), ); let mut prompt = Prompt::default(); From eb0e209474d7ee9d2fd0d6996d845d9846171b1c Mon Sep 17 00:00:00 2001 From: Ahmed Ibrahim Date: Tue, 23 Dec 2025 14:17:34 -0800 Subject: [PATCH 04/12] refresh --- codex-rs/core/src/codex.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/codex-rs/core/src/codex.rs b/codex-rs/core/src/codex.rs index 6350774b311..bff5fde745d 100644 --- a/codex-rs/core/src/codex.rs +++ b/codex-rs/core/src/codex.rs @@ -3162,7 +3162,7 @@ mod tests { let turn_context = Session::make_turn_context( Some(Arc::clone(&auth_manager)), - models_manager.clone(), + models_manager, &otel_manager, session_configuration.provider.clone(), &session_configuration, @@ -3250,7 +3250,7 @@ mod tests { let turn_context = Arc::new(Session::make_turn_context( Some(Arc::clone(&auth_manager)), - models_manager.clone(), + models_manager, &otel_manager, session_configuration.provider.clone(), &session_configuration, From cdc14b327bdaec69b2cc268c1e194bb8caaf4b00 Mon Sep 17 00:00:00 2001 From: Ahmed Ibrahim Date: Tue, 23 Dec 2025 14:21:43 -0800 Subject: [PATCH 05/12] refresh --- codex-rs/core/src/compact.rs | 11 +++++++++++ codex-rs/core/src/models_manager/manager.rs | 3 +++ 2 files changed, 14 insertions(+) diff --git a/codex-rs/core/src/compact.rs b/codex-rs/core/src/compact.rs index 77b9303e920..ced9d5d7099 100644 --- a/codex-rs/core/src/compact.rs +++ b/codex-rs/core/src/compact.rs @@ -137,6 +137,17 @@ async fn run_compact_task_inner( Err(e) => { if retries < max_retries { retries += 1; + if matches!(e, CodexErr::ModelsCatalogChanged) { + if let Err(err) = sess + .services + .models_manager + .refresh_available_models() + .await + { + error!("failed to refresh available models: {err}"); + } + continue; + } let delay = backoff(retries); sess.notify_stream_error( turn_context.as_ref(), diff --git a/codex-rs/core/src/models_manager/manager.rs b/codex-rs/core/src/models_manager/manager.rs index 7199266ee17..cae1c9eac84 100644 --- a/codex-rs/core/src/models_manager/manager.rs +++ b/codex-rs/core/src/models_manager/manager.rs @@ -90,6 +90,9 @@ impl ModelsManager { } pub(crate) async fn refresh_available_models(&self) -> CoreResult<()> { + if self.auth_manager.get_auth_mode() == Some(AuthMode::ApiKey) { + return Ok(()); + } let auth = self.auth_manager.auth(); let api_provider = self.provider.to_api_provider(Some(AuthMode::ChatGPT))?; let api_auth = auth_provider_from_auth(auth.clone(), &self.provider).await?; From 97bcefd67c27ce90d499f3962319d82bae08ec2d Mon Sep 17 00:00:00 2001 From: Ahmed Ibrahim Date: Tue, 23 Dec 2025 14:56:22 -0800 Subject: [PATCH 06/12] tests --- codex-rs/core/tests/common/responses.rs | 19 ++ codex-rs/core/tests/suite/mod.rs | 2 + .../core/tests/suite/models_etag_responses.rs | 215 ++++++++++++++++++ 3 files changed, 236 insertions(+) create mode 100644 codex-rs/core/tests/suite/models_etag_responses.rs diff --git a/codex-rs/core/tests/common/responses.rs b/codex-rs/core/tests/common/responses.rs index 8fcabdda541..39347714096 100644 --- a/codex-rs/core/tests/common/responses.rs +++ b/codex-rs/core/tests/common/responses.rs @@ -670,6 +670,25 @@ pub async fn mount_models_once(server: &MockServer, body: ModelsResponse) -> Mod models_mock } +pub async fn mount_models_once_with_etag( + server: &MockServer, + body: ModelsResponse, + etag: &str, +) -> ModelsMock { + let (mock, models_mock) = models_mock(); + mock.respond_with( + ResponseTemplate::new(200) + .insert_header("content-type", "application/json") + // ModelsClient reads the ETag header, not a JSON field. + .insert_header("ETag", etag) + .set_body_json(body.clone()), + ) + .up_to_n_times(1) + .mount(server) + .await; + models_mock +} + pub async fn start_mock_server() -> MockServer { let server = MockServer::builder() .body_print_limit(BodyPrintLimit::Limited(80_000)) diff --git a/codex-rs/core/tests/suite/mod.rs b/codex-rs/core/tests/suite/mod.rs index 242d1c3219e..55a4b4cd9ef 100644 --- a/codex-rs/core/tests/suite/mod.rs +++ b/codex-rs/core/tests/suite/mod.rs @@ -37,6 +37,8 @@ mod list_models; mod live_cli; mod model_overrides; mod model_tools; +#[cfg(not(target_os = "windows"))] +mod models_etag_responses; mod otel; mod prompt_caching; mod quota_exceeded; diff --git a/codex-rs/core/tests/suite/models_etag_responses.rs b/codex-rs/core/tests/suite/models_etag_responses.rs new file mode 100644 index 00000000000..106de92dacc --- /dev/null +++ b/codex-rs/core/tests/suite/models_etag_responses.rs @@ -0,0 +1,215 @@ +#![cfg(not(target_os = "windows"))] + +use std::sync::Arc; + +use anyhow::Result; +use codex_core::CodexAuth; +use codex_core::features::Feature; +use codex_core::protocol::AskForApproval; +use codex_core::protocol::EventMsg; +use codex_core::protocol::Op; +use codex_core::protocol::SandboxPolicy; +use codex_protocol::config_types::ReasoningSummary; +use codex_protocol::openai_models::ModelsResponse; +use codex_protocol::user_input::UserInput; +use core_test_support::responses; +use core_test_support::responses::ev_assistant_message; +use core_test_support::responses::ev_completed; +use core_test_support::responses::ev_local_shell_call; +use core_test_support::responses::ev_response_created; +use core_test_support::responses::sse; +use core_test_support::skip_if_no_network; +use core_test_support::test_codex::test_codex; +use pretty_assertions::assert_eq; +use tokio::time::Duration; +use wiremock::Match; +use wiremock::MockServer; +use wiremock::Request; +use wiremock::ResponseTemplate; +use wiremock::matchers::header; + +#[derive(Clone)] +struct ToolOutputWithEtagMatcher { + expected_etag: &'static str, + call_id: &'static str, +} + +impl Match for ToolOutputWithEtagMatcher { + fn matches(&self, request: &Request) -> bool { + let header_matches = request + .headers + .get("x-if-models-match") + .and_then(|v| v.to_str().ok()) + == Some(self.expected_etag); + + let body = String::from_utf8_lossy(&request.body); + header_matches && body.contains("\"function_call_output\"") && body.contains(self.call_id) + } +} + +async fn wait_for_task_complete_without_errors(codex: &codex_core::CodexConversation) { + loop { + // Allow a bit more time to accommodate tool execution + retries. + let ev = tokio::time::timeout(Duration::from_secs(10), codex.next_event()) + .await + .expect("timeout waiting for event") + .expect("stream ended unexpectedly"); + + if matches!(ev.msg, EventMsg::Error(_)) { + panic!("unexpected error event: {:?}", ev.msg); + } + + if matches!(ev.msg, EventMsg::TaskComplete(_)) { + break; + } + } +} + +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn refresh_models_etag_on_412_and_retry_tool_output_request() -> Result<()> { + skip_if_no_network!(Ok(())); + + const ETAG_1: &str = "\"models-etag-1\""; + const ETAG_2: &str = "\"models-etag-2\""; + const CALL_ID: &str = "local-shell-call-1"; + + let server = MockServer::start().await; + + // 1) On spawn, Codex fetches /models and stores the ETag. + let spawn_models_mock = responses::mount_models_once_with_etag( + &server, + ModelsResponse { models: Vec::new() }, + ETAG_1, + ) + .await; + + let auth = CodexAuth::create_dummy_chatgpt_auth_for_testing(); + let mut builder = test_codex() + .with_auth(auth) + .with_model("gpt-5") + .with_config(|config| { + config.features.enable(Feature::RemoteModels); + // Keep this test deterministic: no request retries, and a small stream retry budget. + config.model_provider.request_max_retries = Some(0); + config.model_provider.stream_max_retries = Some(1); + }); + + let test = builder.build(&server).await?; + let codex = Arc::clone(&test.codex); + let cwd = Arc::clone(&test.cwd); + let session_model = test.session_configured.model.clone(); + + assert_eq!(spawn_models_mock.requests().len(), 1); + assert_eq!(spawn_models_mock.single_request_path(), "/v1/models"); + + // 2) If a /responses follow-up is rejected with 412, Codex refreshes /models to get a new tag. + let refresh_models_mock = responses::mount_models_once_with_etag( + &server, + ModelsResponse { models: Vec::new() }, + ETAG_2, + ) + .await; + + // First /responses request (user message) succeeds and returns a tool call. + let first_response = sse(vec![ + ev_response_created("resp-1"), + ev_local_shell_call(CALL_ID, "completed", vec!["/bin/echo", "etag ok"]), + ev_completed("resp-1"), + ]); + let user_turn_mock = responses::mount_sse_once_match( + &server, + header("X-If-Models-Match", ETAG_1), + first_response, + ) + .await; + + // Second /responses request (tool output) is rejected with 412 due to stale models catalog. + let precondition_failed_mock = responses::mount_response_once_match( + &server, + ToolOutputWithEtagMatcher { + expected_etag: ETAG_1, + call_id: CALL_ID, + }, + ResponseTemplate::new(412) + .insert_header("content-type", "application/json") + .set_body_string( + r#"{"error":{"message":"models changed","type":"precondition_failed"}}"#, + ), + ) + .await; + + // Third /responses request retries the same tool output and now includes the new ETag. + let completion_response = sse(vec![ + ev_response_created("resp-2"), + ev_assistant_message("msg-1", "done"), + ev_completed("resp-2"), + ]); + let retried_tool_output_mock = responses::mount_sse_once_match( + &server, + ToolOutputWithEtagMatcher { + expected_etag: ETAG_2, + call_id: CALL_ID, + }, + completion_response, + ) + .await; + + codex + .submit(Op::UserTurn { + items: vec![UserInput::Text { + text: "please run a tool".into(), + }], + final_output_json_schema: None, + cwd: cwd.path().to_path_buf(), + approval_policy: AskForApproval::Never, + sandbox_policy: SandboxPolicy::DangerFullAccess, + model: session_model, + effort: None, + summary: ReasoningSummary::Auto, + }) + .await?; + + wait_for_task_complete_without_errors(&codex).await; + + // Assert initial /responses includes ETag from spawn /models call. + let user_req = user_turn_mock.single_request(); + assert_eq!( + user_req.header("X-If-Models-Match"), + Some(ETAG_1.to_string()) + ); + + // Assert the tool output request was rejected with 412 and used the old ETag. + let failed_req = precondition_failed_mock.single_request(); + assert_eq!( + failed_req.header("X-If-Models-Match"), + Some(ETAG_1.to_string()) + ); + let _ = failed_req.function_call_output(CALL_ID); + + // Assert /models was refreshed exactly once after the 412. + assert_eq!(refresh_models_mock.requests().len(), 1); + assert_eq!(refresh_models_mock.single_request_path(), "/v1/models"); + let refresh_req = refresh_models_mock + .requests() + .into_iter() + .next() + .expect("one request"); + // Ensure Codex includes client_version on refresh. (This is a stable signal that we're using the /models client.) + assert!( + refresh_req + .url + .query_pairs() + .any(|(k, _)| k == "client_version"), + "expected /models refresh to include client_version query param" + ); + + // Assert the retried tool output /responses request used the new ETag. + let retried_req = retried_tool_output_mock.single_request(); + assert_eq!( + retried_req.header("X-If-Models-Match"), + Some(ETAG_2.to_string()) + ); + let _ = retried_req.function_call_output(CALL_ID); + + Ok(()) +} From de0beb0bfb2734bfbfdb7d47dd09d18a66f6f563 Mon Sep 17 00:00:00 2001 From: Ahmed Ibrahim Date: Tue, 23 Dec 2025 15:53:39 -0800 Subject: [PATCH 07/12] fix --- codex-rs/codex-api/src/common.rs | 1 + codex-rs/codex-api/src/endpoint/chat.rs | 3 + codex-rs/codex-api/src/error.rs | 2 - codex-rs/codex-api/src/sse/responses.rs | 8 ++ codex-rs/core/src/api_bridge.rs | 3 - codex-rs/core/src/client.rs | 16 +-- codex-rs/core/src/codex.rs | 24 ++-- codex-rs/core/src/compact.rs | 11 -- codex-rs/core/src/error.rs | 9 +- codex-rs/core/src/models_manager/manager.rs | 15 ++- .../core/tests/chat_completions_payload.rs | 7 -- codex-rs/core/tests/chat_completions_sse.rs | 6 - codex-rs/core/tests/responses_headers.rs | 16 --- codex-rs/core/tests/suite/client.rs | 5 - .../core/tests/suite/models_etag_responses.rs | 115 ++++-------------- codex-rs/otel/src/otel_manager.rs | 1 + 16 files changed, 59 insertions(+), 183 deletions(-) diff --git a/codex-rs/codex-api/src/common.rs b/codex-rs/codex-api/src/common.rs index 19e82de3321..db1524d2709 100644 --- a/codex-rs/codex-api/src/common.rs +++ b/codex-rs/codex-api/src/common.rs @@ -59,6 +59,7 @@ pub enum ResponseEvent { summary_index: i64, }, RateLimits(RateLimitSnapshot), + ModelsEtag(String), } #[derive(Debug, Serialize, Clone)] diff --git a/codex-rs/codex-api/src/endpoint/chat.rs b/codex-rs/codex-api/src/endpoint/chat.rs index 4ad133dda49..b7fa0572f0e 100644 --- a/codex-rs/codex-api/src/endpoint/chat.rs +++ b/codex-rs/codex-api/src/endpoint/chat.rs @@ -152,6 +152,9 @@ impl Stream for AggregatedStream { Poll::Ready(Some(Ok(ResponseEvent::RateLimits(snapshot)))) => { return Poll::Ready(Some(Ok(ResponseEvent::RateLimits(snapshot)))); } + Poll::Ready(Some(Ok(ResponseEvent::ModelsEtag(etag)))) => { + return Poll::Ready(Some(Ok(ResponseEvent::ModelsEtag(etag)))); + } Poll::Ready(Some(Ok(ResponseEvent::Completed { response_id, token_usage, diff --git a/codex-rs/codex-api/src/error.rs b/codex-rs/codex-api/src/error.rs index ef2f98a9144..60118e87235 100644 --- a/codex-rs/codex-api/src/error.rs +++ b/codex-rs/codex-api/src/error.rs @@ -25,8 +25,6 @@ pub enum ApiError { }, #[error("rate limit: {0}")] RateLimit(String), - #[error("models catalog has changed. Please refresh your models list.")] - ModelsCatalogChanged, } impl From for ApiError { diff --git a/codex-rs/codex-api/src/sse/responses.rs b/codex-rs/codex-api/src/sse/responses.rs index 5dbec7b77a2..9d2a1be0751 100644 --- a/codex-rs/codex-api/src/sse/responses.rs +++ b/codex-rs/codex-api/src/sse/responses.rs @@ -51,11 +51,19 @@ pub fn spawn_response_stream( telemetry: Option>, ) -> ResponseStream { let rate_limits = parse_rate_limit(&stream_response.headers); + let models_etag = stream_response + .headers + .get("X-Models-Etag") + .and_then(|v| v.to_str().ok()) + .map(ToString::to_string); let (tx_event, rx_event) = mpsc::channel::>(1600); tokio::spawn(async move { if let Some(snapshot) = rate_limits { let _ = tx_event.send(Ok(ResponseEvent::RateLimits(snapshot))).await; } + if let Some(etag) = models_etag { + let _ = tx_event.send(Ok(ResponseEvent::ModelsEtag(etag))).await; + } process_sse(stream_response.bytes, tx_event, idle_timeout, telemetry).await; }); diff --git a/codex-rs/core/src/api_bridge.rs b/codex-rs/core/src/api_bridge.rs index ff3c80b0faa..79fd67d6501 100644 --- a/codex-rs/core/src/api_bridge.rs +++ b/codex-rs/core/src/api_bridge.rs @@ -20,7 +20,6 @@ pub(crate) fn map_api_error(err: ApiError) -> CodexErr { ApiError::ContextWindowExceeded => CodexErr::ContextWindowExceeded, ApiError::QuotaExceeded => CodexErr::QuotaExceeded, ApiError::UsageNotIncluded => CodexErr::UsageNotIncluded, - ApiError::ModelsCatalogChanged => CodexErr::ModelsCatalogChanged, ApiError::Retryable { message, delay } => CodexErr::Stream(message, delay), ApiError::Stream(msg) => CodexErr::Stream(msg, None), ApiError::Api { status, message } => CodexErr::UnexpectedStatus(UnexpectedResponseError { @@ -44,8 +43,6 @@ pub(crate) fn map_api_error(err: ApiError) -> CodexErr { } else { CodexErr::InvalidRequest(body_text) } - } else if status == http::StatusCode::PRECONDITION_FAILED { - CodexErr::ModelsCatalogChanged } else if status == http::StatusCode::INTERNAL_SERVER_ERROR { CodexErr::InternalServerError } else if status == http::StatusCode::TOO_MANY_REQUESTS { diff --git a/codex-rs/core/src/client.rs b/codex-rs/core/src/client.rs index 54feb0a0a9b..11a3c5c65f3 100644 --- a/codex-rs/core/src/client.rs +++ b/codex-rs/core/src/client.rs @@ -2,7 +2,6 @@ use std::sync::Arc; use crate::api_bridge::auth_provider_from_auth; use crate::api_bridge::map_api_error; -use crate::models_manager::manager::ModelsManager; use codex_api::AggregateStreamExt; use codex_api::ChatClient as ApiChatClient; use codex_api::CompactClient as ApiCompactClient; @@ -59,7 +58,6 @@ pub struct ModelClient { config: Arc, auth_manager: Option>, model_family: ModelFamily, - models_manager: Arc, otel_manager: OtelManager, provider: ModelProviderInfo, conversation_id: ConversationId, @@ -80,7 +78,6 @@ impl ModelClient { summary: ReasoningSummaryConfig, conversation_id: ConversationId, session_source: SessionSource, - models_manager: Arc, ) -> Self { Self { config, @@ -92,7 +89,6 @@ impl ModelClient { effort, summary, session_source, - models_manager, } } @@ -266,10 +262,7 @@ impl ModelClient { store_override: None, conversation_id: Some(conversation_id.clone()), session_source: Some(session_source.clone()), - extra_headers: beta_feature_headers( - &self.config, - self.models_manager.get_etag().await, - ), + extra_headers: beta_feature_headers(&self.config), }; let stream_result = client @@ -405,7 +398,7 @@ fn build_api_prompt(prompt: &Prompt, instructions: String, tools_json: Vec) -> ApiHeaderMap { +fn beta_feature_headers(config: &Config) -> ApiHeaderMap { let enabled = FEATURES .iter() .filter_map(|spec| { @@ -423,11 +416,6 @@ fn beta_feature_headers(config: &Config, etag: Option) -> ApiHeaderMap { { headers.insert("x-codex-beta-features", header_value); } - if let Some(etag) = etag - && let Ok(header_value) = HeaderValue::from_str(&etag) - { - headers.insert("X-If-Models-Match", header_value); - } headers } diff --git a/codex-rs/core/src/codex.rs b/codex-rs/core/src/codex.rs index bff5fde745d..ffa772e2d52 100644 --- a/codex-rs/core/src/codex.rs +++ b/codex-rs/core/src/codex.rs @@ -482,7 +482,6 @@ impl Session { #[allow(clippy::too_many_arguments)] fn make_turn_context( auth_manager: Option>, - models_manager: Arc, otel_manager: &OtelManager, provider: ModelProviderInfo, session_configuration: &SessionConfiguration, @@ -507,7 +506,6 @@ impl Session { session_configuration.model_reasoning_summary, conversation_id, session_configuration.session_source.clone(), - Arc::clone(&models_manager), ); let tools_config = ToolsConfig::new(&ToolsConfigParams { @@ -913,7 +911,6 @@ impl Session { .await; let mut turn_context: TurnContext = Self::make_turn_context( Some(Arc::clone(&self.services.auth_manager)), - Arc::clone(&self.services.models_manager), &self.services.otel_manager, session_configuration.provider.clone(), &session_configuration, @@ -2131,7 +2128,6 @@ async fn spawn_review_thread( per_turn_config.model_reasoning_summary, sess.conversation_id, parent_turn_context.client.get_session_source(), - Arc::clone(&sess.services.models_manager), ); let review_turn_context = TurnContext { @@ -2445,17 +2441,6 @@ async fn run_turn( let max_retries = turn_context.client.get_provider().stream_max_retries(); if retries < max_retries { retries += 1; - if matches!(e, CodexErr::ModelsCatalogChanged) { - if let Err(err) = sess - .services - .models_manager - .refresh_available_models() - .await - { - error!("failed to refresh available models: {err}"); - } - continue; - } let delay = match e { CodexErr::Stream(_, Some(delay)) => delay, _ => backoff(retries), @@ -2628,6 +2613,13 @@ async fn try_run_turn( // token usage is available to avoid duplicate TokenCount events. sess.update_rate_limits(&turn_context, snapshot).await; } + ResponseEvent::ModelsEtag(etag) => { + // Update internal state with latest models etag + sess.services + .models_manager + .handle_new_models_etag(etag) + .await; + } ResponseEvent::Completed { response_id: _, token_usage, @@ -3162,7 +3154,6 @@ mod tests { let turn_context = Session::make_turn_context( Some(Arc::clone(&auth_manager)), - models_manager, &otel_manager, session_configuration.provider.clone(), &session_configuration, @@ -3250,7 +3241,6 @@ mod tests { let turn_context = Arc::new(Session::make_turn_context( Some(Arc::clone(&auth_manager)), - models_manager, &otel_manager, session_configuration.provider.clone(), &session_configuration, diff --git a/codex-rs/core/src/compact.rs b/codex-rs/core/src/compact.rs index ced9d5d7099..77b9303e920 100644 --- a/codex-rs/core/src/compact.rs +++ b/codex-rs/core/src/compact.rs @@ -137,17 +137,6 @@ async fn run_compact_task_inner( Err(e) => { if retries < max_retries { retries += 1; - if matches!(e, CodexErr::ModelsCatalogChanged) { - if let Err(err) = sess - .services - .models_manager - .refresh_available_models() - .await - { - error!("failed to refresh available models: {err}"); - } - continue; - } let delay = backoff(retries); sess.notify_stream_error( turn_context.as_ref(), diff --git a/codex-rs/core/src/error.rs b/codex-rs/core/src/error.rs index 470a2cb2d5e..e8fa91d26e8 100644 --- a/codex-rs/core/src/error.rs +++ b/codex-rs/core/src/error.rs @@ -148,9 +148,6 @@ pub enum CodexErr { #[error("{0}")] RefreshTokenFailed(RefreshTokenFailedError), - #[error("Models catalog has changed. Please refresh your models list.")] - ModelsCatalogChanged, - #[error("Fatal error: {0}")] Fatal(String), @@ -458,9 +455,9 @@ impl CodexErr { CodexErr::SessionConfiguredNotFirstEvent | CodexErr::InternalServerError | CodexErr::InternalAgentDied => CodexErrorInfo::InternalServerError, - CodexErr::UnsupportedOperation(_) - | CodexErr::ConversationNotFound(_) - | CodexErr::ModelsCatalogChanged => CodexErrorInfo::BadRequest, + CodexErr::UnsupportedOperation(_) | CodexErr::ConversationNotFound(_) => { + CodexErrorInfo::BadRequest + } CodexErr::Sandbox(_) => CodexErrorInfo::SandboxError, _ => CodexErrorInfo::Other, } diff --git a/codex-rs/core/src/models_manager/manager.rs b/codex-rs/core/src/models_manager/manager.rs index cae1c9eac84..3c7954411dc 100644 --- a/codex-rs/core/src/models_manager/manager.rs +++ b/codex-rs/core/src/models_manager/manager.rs @@ -157,9 +157,14 @@ impl ModelsManager { } OPENAI_DEFAULT_API_MODEL.to_string() } - - pub async fn get_etag(&self) -> Option { - self.etag.read().await.clone() + pub async fn handle_new_models_etag(&self, etag: String) { + let current_etag = self.get_etag().await; + if current_etag.as_deref() == Some(etag.as_str()) { + return; + } + if let Err(err) = self.refresh_available_models().await { + error!("failed to refresh available models: {err}"); + } } #[cfg(any(test, feature = "test-support"))] @@ -173,6 +178,10 @@ impl ModelsManager { Self::find_family_for_model(model).with_config_overrides(config) } + async fn get_etag(&self) -> Option { + self.etag.read().await.clone() + } + /// Replace the cached remote models and rebuild the derived presets list. async fn apply_remote_models(&self, models: Vec) { *self.remote_models.write().await = models; diff --git a/codex-rs/core/tests/chat_completions_payload.rs b/codex-rs/core/tests/chat_completions_payload.rs index a7e140c3e9e..8af5df21695 100644 --- a/codex-rs/core/tests/chat_completions_payload.rs +++ b/codex-rs/core/tests/chat_completions_payload.rs @@ -3,7 +3,6 @@ use std::sync::Arc; use codex_app_server_protocol::AuthMode; -use codex_core::AuthManager; use codex_core::ContentItem; use codex_core::LocalShellAction; use codex_core::LocalShellExecAction; @@ -13,7 +12,6 @@ use codex_core::ModelProviderInfo; use codex_core::Prompt; use codex_core::ResponseItem; use codex_core::WireApi; -use codex_core::auth::AuthCredentialsStoreMode; use codex_core::models_manager::manager::ModelsManager; use codex_otel::otel_manager::OtelManager; use codex_protocol::ConversationId; @@ -100,11 +98,6 @@ async fn run_request(input: Vec) -> Value { summary, conversation_id, SessionSource::Exec, - Arc::new(ModelsManager::new(Arc::new(AuthManager::new( - config.codex_home.clone(), - false, - AuthCredentialsStoreMode::File, - )))), ); let mut prompt = Prompt::default(); diff --git a/codex-rs/core/tests/chat_completions_sse.rs b/codex-rs/core/tests/chat_completions_sse.rs index 7c3743b2301..4f05838279a 100644 --- a/codex-rs/core/tests/chat_completions_sse.rs +++ b/codex-rs/core/tests/chat_completions_sse.rs @@ -1,6 +1,5 @@ use assert_matches::assert_matches; use codex_core::AuthManager; -use codex_core::auth::AuthCredentialsStoreMode; use std::sync::Arc; use tracing_test::traced_test; @@ -100,11 +99,6 @@ async fn run_stream_with_bytes(sse_body: &[u8]) -> Vec { summary, conversation_id, SessionSource::Exec, - Arc::new(ModelsManager::new(Arc::new(AuthManager::new( - config.codex_home.clone(), - false, - AuthCredentialsStoreMode::File, - )))), ); let mut prompt = Prompt::default(); diff --git a/codex-rs/core/tests/responses_headers.rs b/codex-rs/core/tests/responses_headers.rs index 151979ea281..c406fdbc879 100644 --- a/codex-rs/core/tests/responses_headers.rs +++ b/codex-rs/core/tests/responses_headers.rs @@ -10,7 +10,6 @@ use codex_core::Prompt; use codex_core::ResponseEvent; use codex_core::ResponseItem; use codex_core::WireApi; -use codex_core::auth::AuthCredentialsStoreMode; use codex_core::models_manager::manager::ModelsManager; use codex_otel::otel_manager::OtelManager; use codex_protocol::ConversationId; @@ -93,11 +92,6 @@ async fn responses_stream_includes_subagent_header_on_review() { summary, conversation_id, session_source, - Arc::new(ModelsManager::new(Arc::new(AuthManager::new( - config.codex_home.clone(), - false, - AuthCredentialsStoreMode::File, - )))), ); let mut prompt = Prompt::default(); @@ -193,11 +187,6 @@ async fn responses_stream_includes_subagent_header_on_other() { summary, conversation_id, session_source, - Arc::new(ModelsManager::new(Arc::new(AuthManager::new( - config.codex_home.clone(), - false, - AuthCredentialsStoreMode::File, - )))), ); let mut prompt = Prompt::default(); @@ -292,11 +281,6 @@ async fn responses_respects_model_family_overrides_from_config() { summary, conversation_id, session_source, - Arc::new(ModelsManager::new(Arc::new(AuthManager::new( - config.codex_home.clone(), - false, - AuthCredentialsStoreMode::File, - )))), ); let mut prompt = Prompt::default(); diff --git a/codex-rs/core/tests/suite/client.rs b/codex-rs/core/tests/suite/client.rs index b4bbe6e6db8..a22027f99fb 100644 --- a/codex-rs/core/tests/suite/client.rs +++ b/codex-rs/core/tests/suite/client.rs @@ -1152,11 +1152,6 @@ async fn azure_responses_request_includes_store_and_reasoning_ids() { summary, conversation_id, SessionSource::Exec, - Arc::new(ModelsManager::new(Arc::new(AuthManager::new( - config.codex_home.clone(), - false, - AuthCredentialsStoreMode::File, - )))), ); let mut prompt = Prompt::default(); diff --git a/codex-rs/core/tests/suite/models_etag_responses.rs b/codex-rs/core/tests/suite/models_etag_responses.rs index 106de92dacc..2abed718702 100644 --- a/codex-rs/core/tests/suite/models_etag_responses.rs +++ b/codex-rs/core/tests/suite/models_etag_responses.rs @@ -18,55 +18,15 @@ use core_test_support::responses::ev_completed; use core_test_support::responses::ev_local_shell_call; use core_test_support::responses::ev_response_created; use core_test_support::responses::sse; +use core_test_support::responses::sse_response; use core_test_support::skip_if_no_network; use core_test_support::test_codex::test_codex; +use core_test_support::wait_for_event; use pretty_assertions::assert_eq; -use tokio::time::Duration; -use wiremock::Match; use wiremock::MockServer; -use wiremock::Request; -use wiremock::ResponseTemplate; -use wiremock::matchers::header; - -#[derive(Clone)] -struct ToolOutputWithEtagMatcher { - expected_etag: &'static str, - call_id: &'static str, -} - -impl Match for ToolOutputWithEtagMatcher { - fn matches(&self, request: &Request) -> bool { - let header_matches = request - .headers - .get("x-if-models-match") - .and_then(|v| v.to_str().ok()) - == Some(self.expected_etag); - - let body = String::from_utf8_lossy(&request.body); - header_matches && body.contains("\"function_call_output\"") && body.contains(self.call_id) - } -} - -async fn wait_for_task_complete_without_errors(codex: &codex_core::CodexConversation) { - loop { - // Allow a bit more time to accommodate tool execution + retries. - let ev = tokio::time::timeout(Duration::from_secs(10), codex.next_event()) - .await - .expect("timeout waiting for event") - .expect("stream ended unexpectedly"); - - if matches!(ev.msg, EventMsg::Error(_)) { - panic!("unexpected error event: {:?}", ev.msg); - } - - if matches!(ev.msg, EventMsg::TaskComplete(_)) { - break; - } - } -} #[tokio::test(flavor = "multi_thread", worker_threads = 2)] -async fn refresh_models_etag_on_412_and_retry_tool_output_request() -> Result<()> { +async fn refresh_models_on_models_etag_mismatch_and_avoid_duplicate_models_fetch() -> Result<()> { skip_if_no_network!(Ok(())); const ETAG_1: &str = "\"models-etag-1\""; @@ -102,7 +62,7 @@ async fn refresh_models_etag_on_412_and_retry_tool_output_request() -> Result<() assert_eq!(spawn_models_mock.requests().len(), 1); assert_eq!(spawn_models_mock.single_request_path(), "/v1/models"); - // 2) If a /responses follow-up is rejected with 412, Codex refreshes /models to get a new tag. + // 2) If the server sends a different X-Models-Etag on /responses, Codex refreshes /models. let refresh_models_mock = responses::mount_models_once_with_etag( &server, ModelsResponse { models: Vec::new() }, @@ -111,46 +71,28 @@ async fn refresh_models_etag_on_412_and_retry_tool_output_request() -> Result<() .await; // First /responses request (user message) succeeds and returns a tool call. - let first_response = sse(vec![ + // It also includes a mismatched X-Models-Etag, which should trigger a /models refresh. + let first_response_body = sse(vec![ ev_response_created("resp-1"), ev_local_shell_call(CALL_ID, "completed", vec!["/bin/echo", "etag ok"]), ev_completed("resp-1"), ]); - let user_turn_mock = responses::mount_sse_once_match( - &server, - header("X-If-Models-Match", ETAG_1), - first_response, - ) - .await; - - // Second /responses request (tool output) is rejected with 412 due to stale models catalog. - let precondition_failed_mock = responses::mount_response_once_match( + let user_turn_mock = responses::mount_response_once( &server, - ToolOutputWithEtagMatcher { - expected_etag: ETAG_1, - call_id: CALL_ID, - }, - ResponseTemplate::new(412) - .insert_header("content-type", "application/json") - .set_body_string( - r#"{"error":{"message":"models changed","type":"precondition_failed"}}"#, - ), + sse_response(first_response_body).insert_header("X-Models-Etag", ETAG_2), ) .await; - // Third /responses request retries the same tool output and now includes the new ETag. - let completion_response = sse(vec![ + // Second /responses request (tool output) includes the same X-Models-Etag; Codex should not + // refetch /models again after it has already refreshed the catalog. + let completion_response_body = sse(vec![ ev_response_created("resp-2"), ev_assistant_message("msg-1", "done"), ev_completed("resp-2"), ]); - let retried_tool_output_mock = responses::mount_sse_once_match( + let tool_output_mock = responses::mount_response_once( &server, - ToolOutputWithEtagMatcher { - expected_etag: ETAG_2, - call_id: CALL_ID, - }, - completion_response, + sse_response(completion_response_body).insert_header("X-Models-Etag", ETAG_2), ) .await; @@ -169,24 +111,13 @@ async fn refresh_models_etag_on_412_and_retry_tool_output_request() -> Result<() }) .await?; - wait_for_task_complete_without_errors(&codex).await; + let _ = wait_for_event(&codex, |ev| matches!(ev, EventMsg::TaskComplete(_))).await; - // Assert initial /responses includes ETag from spawn /models call. + // Assert /responses requests no longer send the conditional models header. let user_req = user_turn_mock.single_request(); - assert_eq!( - user_req.header("X-If-Models-Match"), - Some(ETAG_1.to_string()) - ); + assert_eq!(user_req.header("X-If-Models-Match"), None); - // Assert the tool output request was rejected with 412 and used the old ETag. - let failed_req = precondition_failed_mock.single_request(); - assert_eq!( - failed_req.header("X-If-Models-Match"), - Some(ETAG_1.to_string()) - ); - let _ = failed_req.function_call_output(CALL_ID); - - // Assert /models was refreshed exactly once after the 412. + // Assert /models was refreshed exactly once after the X-Models-Etag mismatch. assert_eq!(refresh_models_mock.requests().len(), 1); assert_eq!(refresh_models_mock.single_request_path(), "/v1/models"); let refresh_req = refresh_models_mock @@ -203,13 +134,11 @@ async fn refresh_models_etag_on_412_and_retry_tool_output_request() -> Result<() "expected /models refresh to include client_version query param" ); - // Assert the retried tool output /responses request used the new ETag. - let retried_req = retried_tool_output_mock.single_request(); - assert_eq!( - retried_req.header("X-If-Models-Match"), - Some(ETAG_2.to_string()) - ); - let _ = retried_req.function_call_output(CALL_ID); + // Assert the tool output /responses request succeeded and did not trigger another /models fetch. + let tool_req = tool_output_mock.single_request(); + assert_eq!(tool_req.header("X-If-Models-Match"), None); + let _ = tool_req.function_call_output(CALL_ID); + assert_eq!(refresh_models_mock.requests().len(), 1); Ok(()) } diff --git a/codex-rs/otel/src/otel_manager.rs b/codex-rs/otel/src/otel_manager.rs index 33750d83c5f..38d69001cdc 100644 --- a/codex-rs/otel/src/otel_manager.rs +++ b/codex-rs/otel/src/otel_manager.rs @@ -511,6 +511,7 @@ impl OtelManager { "reasoning_summary_part_added".into() } ResponseEvent::RateLimits(_) => "rate_limits".into(), + ResponseEvent::ModelsEtag(_) => "models_etag".into(), } } From b77ceffe6d60077cb483dc9bfa07928940f5a5bb Mon Sep 17 00:00:00 2001 From: Ahmed Ibrahim Date: Tue, 23 Dec 2025 16:10:24 -0800 Subject: [PATCH 08/12] fix --- codex-rs/core/tests/suite/models_etag_responses.rs | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/codex-rs/core/tests/suite/models_etag_responses.rs b/codex-rs/core/tests/suite/models_etag_responses.rs index 2abed718702..cd5f37bab7a 100644 --- a/codex-rs/core/tests/suite/models_etag_responses.rs +++ b/codex-rs/core/tests/suite/models_etag_responses.rs @@ -77,7 +77,7 @@ async fn refresh_models_on_models_etag_mismatch_and_avoid_duplicate_models_fetch ev_local_shell_call(CALL_ID, "completed", vec!["/bin/echo", "etag ok"]), ev_completed("resp-1"), ]); - let user_turn_mock = responses::mount_response_once( + responses::mount_response_once( &server, sse_response(first_response_body).insert_header("X-Models-Etag", ETAG_2), ) @@ -113,10 +113,6 @@ async fn refresh_models_on_models_etag_mismatch_and_avoid_duplicate_models_fetch let _ = wait_for_event(&codex, |ev| matches!(ev, EventMsg::TaskComplete(_))).await; - // Assert /responses requests no longer send the conditional models header. - let user_req = user_turn_mock.single_request(); - assert_eq!(user_req.header("X-If-Models-Match"), None); - // Assert /models was refreshed exactly once after the X-Models-Etag mismatch. assert_eq!(refresh_models_mock.requests().len(), 1); assert_eq!(refresh_models_mock.single_request_path(), "/v1/models"); From da56cce66f32449e7779fd7376aedfda271a97cc Mon Sep 17 00:00:00 2001 From: Ahmed Ibrahim Date: Tue, 23 Dec 2025 16:11:07 -0800 Subject: [PATCH 09/12] fix --- codex-rs/core/tests/suite/models_etag_responses.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/codex-rs/core/tests/suite/models_etag_responses.rs b/codex-rs/core/tests/suite/models_etag_responses.rs index cd5f37bab7a..24f0655cecb 100644 --- a/codex-rs/core/tests/suite/models_etag_responses.rs +++ b/codex-rs/core/tests/suite/models_etag_responses.rs @@ -132,7 +132,6 @@ async fn refresh_models_on_models_etag_mismatch_and_avoid_duplicate_models_fetch // Assert the tool output /responses request succeeded and did not trigger another /models fetch. let tool_req = tool_output_mock.single_request(); - assert_eq!(tool_req.header("X-If-Models-Match"), None); let _ = tool_req.function_call_output(CALL_ID); assert_eq!(refresh_models_mock.requests().len(), 1); From e635c28919082ce5fd8cf617d9d22ce6375f9ed1 Mon Sep 17 00:00:00 2001 From: Ahmed Ibrahim Date: Tue, 23 Dec 2025 16:11:30 -0800 Subject: [PATCH 10/12] fix --- codex-rs/core/tests/suite/mod.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/codex-rs/core/tests/suite/mod.rs b/codex-rs/core/tests/suite/mod.rs index 55a4b4cd9ef..63784bd4032 100644 --- a/codex-rs/core/tests/suite/mod.rs +++ b/codex-rs/core/tests/suite/mod.rs @@ -37,7 +37,6 @@ mod list_models; mod live_cli; mod model_overrides; mod model_tools; -#[cfg(not(target_os = "windows"))] mod models_etag_responses; mod otel; mod prompt_caching; From 54845469e10f47371216cdca6291743dcf97d5a9 Mon Sep 17 00:00:00 2001 From: Ahmed Ibrahim Date: Tue, 23 Dec 2025 16:13:21 -0800 Subject: [PATCH 11/12] fix --- codex-rs/core/src/models_manager/manager.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/codex-rs/core/src/models_manager/manager.rs b/codex-rs/core/src/models_manager/manager.rs index 3c7954411dc..0879b94eddc 100644 --- a/codex-rs/core/src/models_manager/manager.rs +++ b/codex-rs/core/src/models_manager/manager.rs @@ -159,7 +159,7 @@ impl ModelsManager { } pub async fn handle_new_models_etag(&self, etag: String) { let current_etag = self.get_etag().await; - if current_etag.as_deref() == Some(etag.as_str()) { + if current_etag.clone().is_some() && current_etag.as_deref() == Some(etag.as_str()) { return; } if let Err(err) = self.refresh_available_models().await { From ec720ff63639ae06dde2af57d737488129ccbd7d Mon Sep 17 00:00:00 2001 From: Ahmed Ibrahim Date: Wed, 31 Dec 2025 22:43:49 -0800 Subject: [PATCH 12/12] rename --- codex-rs/core/src/codex.rs | 7 ++---- codex-rs/core/src/models_manager/manager.rs | 28 ++++++++++----------- codex-rs/core/tests/suite/remote_models.rs | 2 +- 3 files changed, 17 insertions(+), 20 deletions(-) diff --git a/codex-rs/core/src/codex.rs b/codex-rs/core/src/codex.rs index ffa772e2d52..06974a88078 100644 --- a/codex-rs/core/src/codex.rs +++ b/codex-rs/core/src/codex.rs @@ -247,7 +247,7 @@ impl Codex { let config = Arc::new(config); if config.features.enabled(Feature::RemoteModels) && let Err(err) = models_manager - .attempt_refresh_available_models(&config) + .refresh_available_models_with_cache(&config) .await { error!("failed to refresh available models: {err:?}"); @@ -2615,10 +2615,7 @@ async fn try_run_turn( } ResponseEvent::ModelsEtag(etag) => { // Update internal state with latest models etag - sess.services - .models_manager - .handle_new_models_etag(etag) - .await; + sess.services.models_manager.refresh_if_new_etag(etag).await; } ResponseEvent::Completed { response_id: _, diff --git a/codex-rs/core/src/models_manager/manager.rs b/codex-rs/core/src/models_manager/manager.rs index 0879b94eddc..42be7dee1ff 100644 --- a/codex-rs/core/src/models_manager/manager.rs +++ b/codex-rs/core/src/models_manager/manager.rs @@ -77,7 +77,7 @@ impl ModelsManager { } /// Fetch the latest remote models, using the on-disk cache when still fresh. - pub async fn attempt_refresh_available_models(&self, config: &Config) -> CoreResult<()> { + pub async fn refresh_available_models_with_cache(&self, config: &Config) -> CoreResult<()> { if !config.features.enabled(Feature::RemoteModels) || self.auth_manager.get_auth_mode() == Some(AuthMode::ApiKey) { @@ -86,10 +86,10 @@ impl ModelsManager { if self.try_load_cache().await { return Ok(()); } - self.refresh_available_models().await + self.refresh_available_models_no_cache().await } - pub(crate) async fn refresh_available_models(&self) -> CoreResult<()> { + pub(crate) async fn refresh_available_models_no_cache(&self) -> CoreResult<()> { if self.auth_manager.get_auth_mode() == Some(AuthMode::ApiKey) { return Ok(()); } @@ -112,7 +112,7 @@ impl ModelsManager { } pub async fn list_models(&self, config: &Config) -> Vec { - if let Err(err) = self.attempt_refresh_available_models(config).await { + if let Err(err) = self.refresh_available_models_with_cache(config).await { error!("failed to refresh available models: {err}"); } let remote_models = self.remote_models(config).await; @@ -139,7 +139,7 @@ impl ModelsManager { if let Some(model) = model.as_ref() { return model.to_string(); } - if let Err(err) = self.attempt_refresh_available_models(config).await { + if let Err(err) = self.refresh_available_models_with_cache(config).await { error!("failed to refresh available models: {err}"); } // if codex-auto-balanced exists & signed in with chatgpt mode, return it, otherwise return the default model @@ -157,12 +157,12 @@ impl ModelsManager { } OPENAI_DEFAULT_API_MODEL.to_string() } - pub async fn handle_new_models_etag(&self, etag: String) { + pub async fn refresh_if_new_etag(&self, etag: String) { let current_etag = self.get_etag().await; if current_etag.clone().is_some() && current_etag.as_deref() == Some(etag.as_str()) { return; } - if let Err(err) = self.refresh_available_models().await { + if let Err(err) = self.refresh_available_models_no_cache().await { error!("failed to refresh available models: {err}"); } } @@ -411,7 +411,7 @@ mod tests { let manager = ModelsManager::with_provider(auth_manager, provider); manager - .attempt_refresh_available_models(&config) + .refresh_available_models_with_cache(&config) .await .expect("refresh succeeds"); let cached_remote = manager.remote_models(&config).await; @@ -470,7 +470,7 @@ mod tests { let manager = ModelsManager::with_provider(auth_manager, provider); manager - .attempt_refresh_available_models(&config) + .refresh_available_models_with_cache(&config) .await .expect("first refresh succeeds"); assert_eq!( @@ -481,7 +481,7 @@ mod tests { // Second call should read from cache and avoid the network. manager - .attempt_refresh_available_models(&config) + .refresh_available_models_with_cache(&config) .await .expect("cached refresh succeeds"); assert_eq!( @@ -524,7 +524,7 @@ mod tests { let manager = ModelsManager::with_provider(auth_manager, provider); manager - .attempt_refresh_available_models(&config) + .refresh_available_models_with_cache(&config) .await .expect("initial refresh succeeds"); @@ -549,7 +549,7 @@ mod tests { .await; manager - .attempt_refresh_available_models(&config) + .refresh_available_models_with_cache(&config) .await .expect("second refresh succeeds"); assert_eq!( @@ -595,7 +595,7 @@ mod tests { manager.cache_ttl = Duration::ZERO; manager - .attempt_refresh_available_models(&config) + .refresh_available_models_with_cache(&config) .await .expect("initial refresh succeeds"); @@ -610,7 +610,7 @@ mod tests { .await; manager - .attempt_refresh_available_models(&config) + .refresh_available_models_with_cache(&config) .await .expect("second refresh succeeds"); diff --git a/codex-rs/core/tests/suite/remote_models.rs b/codex-rs/core/tests/suite/remote_models.rs index 2034f512ff5..7cd3fd63cb2 100644 --- a/codex-rs/core/tests/suite/remote_models.rs +++ b/codex-rs/core/tests/suite/remote_models.rs @@ -324,7 +324,7 @@ async fn remote_models_preserve_builtin_presets() -> Result<()> { ); manager - .attempt_refresh_available_models(&config) + .refresh_available_models_with_cache(&config) .await .expect("refresh succeeds");