diff --git a/codex-rs/app-server/src/request_processors/thread_processor_tests.rs b/codex-rs/app-server/src/request_processors/thread_processor_tests.rs index 8a6922da4e..44c29bda15 100644 --- a/codex-rs/app-server/src/request_processors/thread_processor_tests.rs +++ b/codex-rs/app-server/src/request_processors/thread_processor_tests.rs @@ -594,6 +594,7 @@ mod thread_processor_behavior_tests { let session_provider = ModelProviderInfo { name: "session".to_string(), base_url: Some("http://127.0.0.1:8061/api/codex".to_string()), + model_catalog_url: None, env_key: None, env_key_instructions: None, experimental_bearer_token: None, diff --git a/codex-rs/app-server/tests/suite/v2/model_list.rs b/codex-rs/app-server/tests/suite/v2/model_list.rs index 217f0cafa7..5d26c68e78 100644 --- a/codex-rs/app-server/tests/suite/v2/model_list.rs +++ b/codex-rs/app-server/tests/suite/v2/model_list.rs @@ -39,14 +39,16 @@ use wiremock::matchers::path; const DEFAULT_TIMEOUT: Duration = Duration::from_secs(10); const INVALID_REQUEST_ERROR_CODE: i64 = -32600; -#[test_case(None, false; "default off")] -#[test_case(None, true; "app rollout")] -#[test_case(Some(false), true; "user opt out")] -#[test_case(Some(true), false; "user opt in")] +#[test_case(None, false, true; "default off")] +#[test_case(None, true, true; "app rollout")] +#[test_case(Some(false), true, true; "user opt out")] +#[test_case(Some(true), false, true; "user opt in")] +#[test_case(Some(true), false, false; "base URL without catalog opt in")] #[tokio::test] async fn api_key_model_discovery_startup_enablement_respects_user_config( user_enablement: Option, app_enablement: bool, + catalog_opt_in: bool, ) -> Result<()> { let server = MockServer::start().await; let mut remote_model = codex_models_manager::bundled_models_response()? @@ -69,9 +71,24 @@ async fn api_key_model_discovery_startup_enablement_respects_user_config( let feature_config = user_enablement .map(|enabled| format!("[features]\napi_key_model_discovery = {enabled}\n")) .unwrap_or_default(); + let catalog_config = if catalog_opt_in { + format!("model_catalog_url = \"{server_uri}/v1/models\"\n") + } else { + String::new() + }; std::fs::write( codex_home.path().join("config.toml"), - format!("openai_base_url = \"{server_uri}/v1\"\n{feature_config}"), + format!( + r#" +model_provider = "catalog-test" +{feature_config} +[model_providers.catalog-test] +name = "OpenAI" +base_url = "{server_uri}/v1" +requires_openai_auth = true +{catalog_config} +"# + ), )?; login_with_api_key( codex_home.path(), @@ -113,7 +130,7 @@ async fn api_key_model_discovery_startup_enablement_respects_user_config( }, }) .await?; - let enabled = user_enablement.unwrap_or(app_enablement); + let enabled = user_enablement.unwrap_or(app_enablement) && catalog_opt_in; let expected = if enabled { &remote } else { &bundled }; assert_eq!( response, @@ -135,7 +152,8 @@ async fn api_key_model_discovery_startup_enablement_respects_user_config( .received_requests() .await .expect("request recording is enabled") - .is_empty() + .iter() + .all(|request| request.url.path() != "/v1/models") ); } Ok(()) @@ -344,9 +362,14 @@ async fn list_models_uses_remote_catalog_as_source_of_truth( model = "mock-model" approval_policy = "never" sandbox_mode = "read-only" -openai_base_url = "{server_uri}/v1" +model_provider = "catalog-test" [features] api_key_model_discovery = true +[model_providers.catalog-test] +name = "OpenAI" +base_url = "{server_uri}/v1" +requires_openai_auth = true +model_catalog_url = "{server_uri}/v1/models" "# ), )?; diff --git a/codex-rs/codex-api/src/endpoint/models.rs b/codex-rs/codex-api/src/endpoint/models.rs index ed8909ab1f..c00bc0115e 100644 --- a/codex-rs/codex-api/src/endpoint/models.rs +++ b/codex-rs/codex-api/src/endpoint/models.rs @@ -11,6 +11,7 @@ use http::HeaderMap; use http::Method; use http::header::ETAG; use std::sync::Arc; +use url::Url; pub struct ModelsClient { session: EndpointSession, @@ -44,17 +45,36 @@ impl ModelsClient { request.url } + /// Builds a full catalog URL, preserving provider routing parameters and client version. + pub fn catalog_request_url( + provider: &Provider, + catalog_url: &str, + client_version: &str, + ) -> Result { + let invalid = |message: &str| ApiError::InvalidRequest { + message: message.to_string(), + }; + let mut url = Url::parse(catalog_url) + .map_err(|_| invalid("model_catalog_url must be an absolute URL"))?; + { + let mut query = url.query_pairs_mut(); + if let Some(params) = &provider.query_params { + query.extend_pairs(params); + } + query.append_pair("client_version", client_version); + } + Ok(url.into()) + } + + /// Fetches and decodes a catalog, optionally bounding response bytes before decoding. pub async fn list_models( &self, request_url: String, extra_headers: HeaderMap, + response_body_limit_bytes: Option, ) -> Result<(Vec, Option), ApiError> { let (body, header_etag) = self - .list_models_raw( - request_url, - extra_headers, - /*response_body_limit_bytes*/ None, - ) + .list_models_raw(request_url, extra_headers, response_body_limit_bytes) .await?; let ModelsResponse { models } = serde_json::from_slice::(&body).map_err(|e| { @@ -207,6 +227,39 @@ mod tests { } } + #[test] + fn catalog_request_url_preserves_routing_and_encodes_queries() { + let mut provider = provider("https://gateway.example/v1"); + provider.query_params = Some(std::collections::HashMap::from([( + "api-version".to_string(), + "2026 09".to_string(), + )])); + let url = ModelsClient::::catalog_request_url( + &provider, + "https://catalog.example/codex/models?deployment=one", + "1.2.3", + ) + .unwrap(); + assert_eq!( + url, + "https://catalog.example/codex/models?deployment=one&api-version=2026+09&client_version=1.2.3" + ); + } + + #[test] + fn catalog_request_url_rejects_relative_urls() { + let provider = provider("https://gateway.example/v1"); + for catalog_url in ["/codex/models", "codex/models"] { + let error = ModelsClient::::catalog_request_url( + &provider, + catalog_url, + "1.2.3", + ) + .unwrap_err(); + assert!(matches!(error, ApiError::InvalidRequest { .. })); + } + } + #[tokio::test] async fn response_body_limit_survives_auth_retry_without_limiting_other_models_requests() { let transport = CapturingTransport::default(); @@ -217,7 +270,7 @@ mod tests { let client = ModelsClient::new(transport.clone(), provider, auth.clone()); client - .list_models_raw( + .list_models( request_url.clone(), HeaderMap::new(), /*response_body_limit_bytes*/ Some(64), @@ -233,7 +286,11 @@ mod tests { } let (models, _) = client - .list_models(request_url, HeaderMap::new()) + .list_models( + request_url, + HeaderMap::new(), + /*response_body_limit_bytes*/ None, + ) .await .expect("ordinary request on the same client should remain unbounded"); assert!(models.is_empty()); @@ -268,7 +325,11 @@ mod tests { let client = ModelsClient::new(transport.clone(), provider, Arc::new(DummyAuth)); let (models, _) = client - .list_models(request_url, HeaderMap::new()) + .list_models( + request_url, + HeaderMap::new(), + /*response_body_limit_bytes*/ None, + ) .await .expect("request should succeed"); @@ -327,7 +388,11 @@ mod tests { let client = ModelsClient::new(transport, provider, Arc::new(DummyAuth)); let (models, _) = client - .list_models(request_url, HeaderMap::new()) + .list_models( + request_url, + HeaderMap::new(), + /*response_body_limit_bytes*/ None, + ) .await .expect("request should succeed"); @@ -352,7 +417,11 @@ mod tests { let client = ModelsClient::new(transport, provider, Arc::new(DummyAuth)); let (models, etag) = client - .list_models(request_url, HeaderMap::new()) + .list_models( + request_url, + HeaderMap::new(), + /*response_body_limit_bytes*/ None, + ) .await .expect("request should succeed"); diff --git a/codex-rs/codex-api/tests/models_integration.rs b/codex-rs/codex-api/tests/models_integration.rs index 2227437618..ed43d8aaed 100644 --- a/codex-rs/codex-api/tests/models_integration.rs +++ b/codex-rs/codex-api/tests/models_integration.rs @@ -81,7 +81,11 @@ async fn invalid_models_response_reports_bounded_decode_metadata() { let client = ModelsClient::new(transport, provider, Arc::new(DummyAuth)); let error = client - .list_models(request_url, HeaderMap::new()) + .list_models( + request_url, + HeaderMap::new(), + /*response_body_limit_bytes*/ None, + ) .await .expect_err("invalid catalog should fail to decode"); @@ -187,7 +191,11 @@ async fn models_client_hits_models_endpoint() { let client = ModelsClient::new(transport, provider, Arc::new(DummyAuth)); let (models, _) = client - .list_models(request_url, HeaderMap::new()) + .list_models( + request_url, + HeaderMap::new(), + /*response_body_limit_bytes*/ None, + ) .await .expect("models request should succeed"); diff --git a/codex-rs/config/src/thread_config.rs b/codex-rs/config/src/thread_config.rs index 654605c85a..61965e3431 100644 --- a/codex-rs/config/src/thread_config.rs +++ b/codex-rs/config/src/thread_config.rs @@ -298,6 +298,7 @@ mod tests { ModelProviderInfo { name: name.to_string(), base_url: Some("http://127.0.0.1:8061/api/codex".to_string()), + model_catalog_url: None, env_key: None, env_key_instructions: None, experimental_bearer_token: None, diff --git a/codex-rs/config/src/thread_config/proto/codex.thread_config.v1.proto b/codex-rs/config/src/thread_config/proto/codex.thread_config.v1.proto index 5bcff76986..b9f89c95c6 100644 --- a/codex-rs/config/src/thread_config/proto/codex.thread_config.v1.proto +++ b/codex-rs/config/src/thread_config/proto/codex.thread_config.v1.proto @@ -49,6 +49,7 @@ message ModelProvider { bool requires_openai_auth = 16; bool supports_websockets = 17; bool supports_standalone_web_search = 18; + optional string model_catalog_url = 19; } message StringMap { diff --git a/codex-rs/config/src/thread_config/proto/codex.thread_config.v1.rs b/codex-rs/config/src/thread_config/proto/codex.thread_config.v1.rs index 05b1cf0eca..6ffc7bbdf6 100644 --- a/codex-rs/config/src/thread_config/proto/codex.thread_config.v1.rs +++ b/codex-rs/config/src/thread_config/proto/codex.thread_config.v1.rs @@ -77,6 +77,8 @@ pub struct ModelProvider { pub supports_websockets: bool, #[prost(bool, tag = "18")] pub supports_standalone_web_search: bool, + #[prost(string, optional, tag = "19")] + pub model_catalog_url: ::core::option::Option<::prost::alloc::string::String>, } #[derive(Clone, PartialEq, ::prost::Message)] pub struct StringMap { diff --git a/codex-rs/config/src/thread_config/remote.rs b/codex-rs/config/src/thread_config/remote.rs index 8acb6e49f9..16a937b414 100644 --- a/codex-rs/config/src/thread_config/remote.rs +++ b/codex-rs/config/src/thread_config/remote.rs @@ -173,6 +173,7 @@ fn model_provider_from_proto( let info = ModelProviderInfo { name: provider.name, base_url: provider.base_url, + model_catalog_url: provider.model_catalog_url.map(Into::into), env_key: provider.env_key, env_key_instructions: provider.env_key_instructions, experimental_bearer_token: provider.experimental_bearer_token.map(Into::into), @@ -205,6 +206,7 @@ fn model_provider_to_proto( let ModelProviderInfo { name, base_url, + model_catalog_url, env_key, env_key_instructions, experimental_bearer_token, @@ -228,6 +230,7 @@ fn model_provider_to_proto( id: id.into(), name, base_url, + model_catalog_url: model_catalog_url.map(RedactedString::into_inner), env_key, env_key_instructions, experimental_bearer_token: experimental_bearer_token.map(RedactedString::into_inner), @@ -473,6 +476,9 @@ mod tests { id: "local".to_string(), name: "Local".to_string(), base_url: Some("http://127.0.0.1:8061/api/codex".to_string()), + model_catalog_url: Some( + "http://127.0.0.1:8061/api/codex/models".to_string(), + ), env_key: None, env_key_instructions: None, experimental_bearer_token: None, @@ -543,6 +549,7 @@ mod tests { ModelProviderInfo { name: "Local".to_string(), base_url: Some("http://127.0.0.1:8061/api/codex".to_string()), + model_catalog_url: Some("http://127.0.0.1:8061/api/codex/models".into()), env_key: None, env_key_instructions: None, experimental_bearer_token: None, diff --git a/codex-rs/core/config.schema.json b/codex-rs/core/config.schema.json index df68076a7b..08f1ed5774 100644 --- a/codex-rs/core/config.schema.json +++ b/codex-rs/core/config.schema.json @@ -2549,6 +2549,10 @@ "description": "Additional HTTP headers to include in requests to this provider where the (key, value) pairs are the header name and value.", "type": "object" }, + "model_catalog_url": { + "description": "Optional full URL for a Codex-native model catalog. When unset, OpenAI discovery uses the Codex backend unless `base_url` overrides the inference endpoint.", + "type": "string" + }, "name": { "default": "", "description": "Friendly display name.", diff --git a/codex-rs/core/tests/responses_headers.rs b/codex-rs/core/tests/responses_headers.rs index 3ce7110ac9..0b1b91dec3 100644 --- a/codex-rs/core/tests/responses_headers.rs +++ b/codex-rs/core/tests/responses_headers.rs @@ -74,6 +74,7 @@ async fn responses_stream_includes_subagent_header_on_review() { let provider = ModelProviderInfo { name: "mock".into(), base_url: Some(format!("{}/v1", server.uri())), + model_catalog_url: None, env_key: None, env_key_instructions: None, experimental_bearer_token: None, @@ -214,6 +215,7 @@ async fn responses_stream_includes_subagent_header_on_other() { let provider = ModelProviderInfo { name: "mock".into(), base_url: Some(format!("{}/v1", server.uri())), + model_catalog_url: None, env_key: None, env_key_instructions: None, experimental_bearer_token: None, @@ -335,6 +337,7 @@ async fn responses_respects_model_info_overrides_from_config() { let provider = ModelProviderInfo { name: "mock".into(), base_url: Some(format!("{}/v1", server.uri())), + model_catalog_url: None, env_key: None, env_key_instructions: None, experimental_bearer_token: None, diff --git a/codex-rs/core/tests/suite/client.rs b/codex-rs/core/tests/suite/client.rs index 4760c9d638..ce0ce86784 100644 --- a/codex-rs/core/tests/suite/client.rs +++ b/codex-rs/core/tests/suite/client.rs @@ -1604,6 +1604,7 @@ async fn send_provider_auth_request(server: &MockServer, auth: ModelProviderAuth let provider = ModelProviderInfo { name: "corp".into(), base_url: Some(format!("{}/v1", server.uri())), + model_catalog_url: None, env_key: None, env_key_instructions: None, experimental_bearer_token: None, @@ -3105,6 +3106,7 @@ async fn azure_responses_request_does_not_store_and_preserves_prefixed_item_ids( let provider = ModelProviderInfo { name: "azure".into(), base_url: Some(format!("{}/openai", server.uri())), + model_catalog_url: None, env_key: None, env_key_instructions: None, experimental_bearer_token: None, @@ -3733,6 +3735,7 @@ async fn azure_overrides_assign_properties_used_for_responses_url() { let provider = ModelProviderInfo { name: "custom".to_string(), base_url: Some(format!("{}/openai", server.uri())), + model_catalog_url: None, // Reuse the existing environment variable to avoid using unsafe code env_key: Some(EXISTING_ENV_VAR_WITH_NON_EMPTY_VALUE.to_string()), experimental_bearer_token: None, @@ -3818,6 +3821,7 @@ async fn env_var_overrides_loaded_auth() { let provider = ModelProviderInfo { name: ModelProviderInfo::create_openai_provider(/*base_url*/ None).name, base_url: Some(format!("{}/openai", server.uri())), + model_catalog_url: None, // Reuse the existing environment variable to avoid using unsafe code env_key: Some(EXISTING_ENV_VAR_WITH_NON_EMPTY_VALUE.to_string()), query_params: Some(std::collections::HashMap::from([( diff --git a/codex-rs/core/tests/suite/client_websockets.rs b/codex-rs/core/tests/suite/client_websockets.rs index c6a4daa55c..b9a0492e1a 100755 --- a/codex-rs/core/tests/suite/client_websockets.rs +++ b/codex-rs/core/tests/suite/client_websockets.rs @@ -2721,6 +2721,7 @@ fn websocket_provider_with_connect_timeout( ModelProviderInfo { name: "mock-ws".into(), base_url: Some(format!("{}/v1", server.uri())), + model_catalog_url: None, env_key: None, env_key_instructions: None, experimental_bearer_token: None, diff --git a/codex-rs/core/tests/suite/models_cache_auth.rs b/codex-rs/core/tests/suite/models_cache_auth.rs index 430236ce8f..4521237045 100644 --- a/codex-rs/core/tests/suite/models_cache_auth.rs +++ b/codex-rs/core/tests/suite/models_cache_auth.rs @@ -310,6 +310,11 @@ async fn auth_and_provider_switches_do_not_reuse_chatgpt_catalog() -> Result<()> .with_home(home.clone()) .with_auth(CodexAuth::from_api_key("api-key")) .with_config(|config| { + config.model_provider.model_catalog_url = config + .model_provider + .base_url + .as_ref() + .map(|base_url| format!("{base_url}/models").into()); config .features .enable(codex_features::Feature::ApiKeyModelDiscovery) diff --git a/codex-rs/core/tests/suite/remote_models.rs b/codex-rs/core/tests/suite/remote_models.rs index b5a652c5a9..dad188d70c 100644 --- a/codex-rs/core/tests/suite/remote_models.rs +++ b/codex-rs/core/tests/suite/remote_models.rs @@ -933,6 +933,11 @@ async fn remote_models_apply_legacy_instructions(auth: CodexAuth) -> Result<()> .features .enable(Feature::ApiKeyModelDiscovery) .expect("enable API-key model discovery"); + config.model_provider.model_catalog_url = config + .model_provider + .base_url + .as_ref() + .map(|base_url| format!("{base_url}/models").into()); config.update_plan_enabled = true; config.model = Some("gpt-5.2".to_string()); }); @@ -1478,3 +1483,64 @@ fn test_remote_model_with_policy( experimental_supported_tools: Vec::new(), } } + +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn model_catalog_url_supplies_conversation_model_and_instructions() -> Result<()> { + skip_if_no_network!(Ok(())); + let server = MockServer::start().await; + let instructions = "Use the gateway's reviewed conversation instructions."; + let mut model = codex_models_manager::model_info::model_info_from_slug("gateway-conversation"); + model.visibility = ModelVisibility::List; + model.supported_in_api = true; + model.support_verbosity = false; + model.used_fallback_model_metadata = false; + model + .model_messages + .as_mut() + .expect("fallback instructions") + .instructions_template = Some(instructions.to_string()); + let catalog = ModelsResponse { + models: vec![model], + }; + Mock::given(method("GET")) + .and(path("/codex/models")) + .respond_with(ResponseTemplate::new(/*s*/ 200).set_body_json(&catalog)) + .expect(1) + .mount(&server) + .await; + let response = mount_sse_once( + &server, + sse(vec![ev_response_created("resp-1"), ev_completed("resp-1")]), + ) + .await; + let catalog_url = format!("{}/codex/models", server.uri()); + let test = test_codex() + .with_auth(CodexAuth::from_api_key("gateway-api-key")) + .with_model("gateway-conversation") + .with_config(move |config| { + config.model_provider.name = "Gateway".to_string(); + config.model_provider.model_catalog_url = Some(catalog_url.into()); + config + .features + .enable(Feature::ApiKeyModelDiscovery) + .expect("enable API-key discovery"); + config.model_verbosity = Some(codex_protocol::config_types::Verbosity::High); + }) + .build_with_auto_env(&server) + .await?; + let received = test + .thread_manager + .get_models_manager() + .raw_model_catalog( + RefreshStrategy::OnlineIfUncached, + codex_core::test_support::default_http_client_factory(), + ) + .await; + assert_eq!(received.models, catalog.models); + test.submit_turn("hello").await?; + let request = response.single_request().body_json(); + assert_eq!(request["model"], "gateway-conversation"); + assert_eq!(request["instructions"], instructions); + assert_eq!(request["text"].get("verbosity"), None); + Ok(()) +} diff --git a/codex-rs/core/tests/suite/stream_error_allows_next_turn.rs b/codex-rs/core/tests/suite/stream_error_allows_next_turn.rs index d7dcf56c0b..e46734522f 100644 --- a/codex-rs/core/tests/suite/stream_error_allows_next_turn.rs +++ b/codex-rs/core/tests/suite/stream_error_allows_next_turn.rs @@ -66,6 +66,7 @@ async fn continue_after_stream_error() { let provider = ModelProviderInfo { name: "mock-openai".into(), base_url: Some(format!("{}/v1", server.uri())), + model_catalog_url: None, env_key: Some("PATH".into()), env_key_instructions: None, experimental_bearer_token: None, diff --git a/codex-rs/core/tests/suite/stream_no_completed.rs b/codex-rs/core/tests/suite/stream_no_completed.rs index e294c578c8..676b5a9b3b 100644 --- a/codex-rs/core/tests/suite/stream_no_completed.rs +++ b/codex-rs/core/tests/suite/stream_no_completed.rs @@ -48,6 +48,7 @@ async fn retries_on_early_close() { let model_provider = ModelProviderInfo { name: "openai".into(), base_url: Some(format!("{}/v1", server.uri())), + model_catalog_url: None, // Environment variable that should exist in the test environment. // ModelClient will return an error if the environment variable for the // provider is not set. diff --git a/codex-rs/ext/guardian-v2/src/async_scorer/sampler/connection_pool.rs b/codex-rs/ext/guardian-v2/src/async_scorer/sampler/connection_pool.rs index 0358d1ecc2..8fa4227171 100644 --- a/codex-rs/ext/guardian-v2/src/async_scorer/sampler/connection_pool.rs +++ b/codex-rs/ext/guardian-v2/src/async_scorer/sampler/connection_pool.rs @@ -22,6 +22,7 @@ use codex_api::TransportError; use codex_api::build_session_headers; use codex_http_client::ClientRouteClass; use codex_login::CodexAuth; +use codex_login::default_client::ClientRedirectPolicy; use codex_login::default_client::add_originator_header; use codex_login::default_client::create_client_for_route_async; use codex_login::default_client::default_headers; @@ -216,6 +217,7 @@ impl ConnectionPool { self.config.http_client_factory.clone(), url, ClientRouteClass::Api, + ClientRedirectPolicy::Default, ) .await .map_err(|error| { diff --git a/codex-rs/http-client/src/client.rs b/codex-rs/http-client/src/client.rs index 9cda749aa2..420d5ad1b7 100644 --- a/codex-rs/http-client/src/client.rs +++ b/codex-rs/http-client/src/client.rs @@ -40,6 +40,12 @@ impl HttpClient { Self::from_parts(inner, RequestLogging::Disabled) } + /// Suppresses URL and response-header diagnostics while preserving this client's routing. + pub fn without_request_logging(mut self) -> Self { + self.request_logging = RequestLogging::Disabled; + self + } + pub(crate) fn from_parts(inner: reqwest::Client, request_logging: RequestLogging) -> Self { Self { inner, diff --git a/codex-rs/http-client/src/transport.rs b/codex-rs/http-client/src/transport.rs index ca351c1f00..13bddd0489 100644 --- a/codex-rs/http-client/src/transport.rs +++ b/codex-rs/http-client/src/transport.rs @@ -85,6 +85,7 @@ impl ReqwestTransport { } fn map_error(err: reqwest::Error) -> TransportError { + let err = err.without_url(); if err.is_connect() { TransportError::Connection(err.without_url()) } else if err.is_timeout() { diff --git a/codex-rs/http-client/src/transport_tests.rs b/codex-rs/http-client/src/transport_tests.rs index a8ab11aba5..07e49f97f8 100644 --- a/codex-rs/http-client/src/transport_tests.rs +++ b/codex-rs/http-client/src/transport_tests.rs @@ -18,10 +18,9 @@ async fn enabled_request_logging_emits_transport_url_and_body() { #[tokio::test] async fn disabled_request_logging_suppresses_transport_url_and_body() { - let logs = capture_transport_logs(HttpClient::new_without_request_logging( - test_reqwest_client(), - )) - .await; + let logs = + capture_transport_logs(HttpClient::new(test_reqwest_client()).without_request_logging()) + .await; assert!(logs.contains("log capture sentinel")); assert!(!logs.contains("url-secret")); diff --git a/codex-rs/login/src/auth/default_client.rs b/codex-rs/login/src/auth/default_client.rs index d26a82fbc7..a13421397f 100644 --- a/codex-rs/login/src/auth/default_client.rs +++ b/codex-rs/login/src/auth/default_client.rs @@ -273,6 +273,7 @@ pub async fn create_client_for_route_async( http_client_factory: HttpClientFactory, request_url: String, route_class: ClientRouteClass, + redirect_policy: ClientRedirectPolicy, ) -> std::io::Result { let permit = ROUTE_AWARE_CLIENT_BUILD_PERMIT .acquire() @@ -284,7 +285,7 @@ pub async fn create_client_for_route_async( &http_client_factory, &request_url, route_class, - ClientRedirectPolicy::Default, + redirect_policy, ) .map_err(std::io::Error::from) }) diff --git a/codex-rs/login/src/auth_env_telemetry.rs b/codex-rs/login/src/auth_env_telemetry.rs index c4dc6f07b8..b2d83bf175 100644 --- a/codex-rs/login/src/auth_env_telemetry.rs +++ b/codex-rs/login/src/auth_env_telemetry.rs @@ -61,6 +61,7 @@ mod tests { let provider = ModelProviderInfo { name: "Custom".to_string(), base_url: None, + model_catalog_url: None, env_key: Some("sk-should-not-leak".to_string()), env_key_instructions: None, experimental_bearer_token: None, diff --git a/codex-rs/model-provider-info/src/lib.rs b/codex-rs/model-provider-info/src/lib.rs index 7d9e408983..3be93a6830 100644 --- a/codex-rs/model-provider-info/src/lib.rs +++ b/codex-rs/model-provider-info/src/lib.rs @@ -137,6 +137,9 @@ pub struct ModelProviderInfo { pub name: String, /// Base URL for the provider's OpenAI-compatible API. pub base_url: Option, + /// Optional full URL for a Codex-native model catalog. When unset, OpenAI discovery + /// uses the Codex backend unless `base_url` overrides the inference endpoint. + pub model_catalog_url: Option, /// Environment variable that stores the user's API key for this provider. pub env_key: Option, @@ -510,6 +513,7 @@ other non-default provider fields are not supported" ModelProviderInfo { name: OPENAI_PROVIDER_NAME.into(), base_url, + model_catalog_url: None, env_key: None, env_key_instructions: None, experimental_bearer_token: None, @@ -554,6 +558,7 @@ other non-default provider fields are not supported" // this is unset. A configured value is therefore unambiguously an // endpoint override. base_url: None, + model_catalog_url: None, env_key: None, env_key_instructions: None, experimental_bearer_token: None, @@ -734,6 +739,7 @@ pub fn create_oss_provider_with_base_url(base_url: &str, wire_api: WireApi) -> M ModelProviderInfo { name: "gpt-oss".into(), base_url: Some(base_url.into()), + model_catalog_url: None, env_key: None, env_key_instructions: None, experimental_bearer_token: None, diff --git a/codex-rs/model-provider-info/src/model_provider_info_tests.rs b/codex-rs/model-provider-info/src/model_provider_info_tests.rs index a7bcaa45dc..c0c3971936 100644 --- a/codex-rs/model-provider-info/src/model_provider_info_tests.rs +++ b/codex-rs/model-provider-info/src/model_provider_info_tests.rs @@ -62,6 +62,7 @@ base_url = "http://localhost:11434/v1" let expected_provider = ModelProviderInfo { name: "Ollama".into(), base_url: Some("http://localhost:11434/v1".into()), + model_catalog_url: None, env_key: None, env_key_instructions: None, experimental_bearer_token: None, @@ -96,6 +97,7 @@ query_params = { api-version = "2025-04-01-preview" } let expected_provider = ModelProviderInfo { name: "Azure".into(), base_url: Some("https://xxxxx.openai.azure.com/openai".into()), + model_catalog_url: None, env_key: Some("AZURE_OPENAI_API_KEY".into()), env_key_instructions: None, experimental_bearer_token: None, @@ -134,6 +136,7 @@ supports_standalone_web_search = true let expected_provider = ModelProviderInfo { name: "Example".into(), base_url: Some("https://example.com".into()), + model_catalog_url: None, env_key: Some("API_KEY".into()), env_key_instructions: None, experimental_bearer_token: None, @@ -317,6 +320,7 @@ fn test_create_amazon_bedrock_provider() { ModelProviderInfo { name: "Amazon Bedrock".to_string(), base_url: None, + model_catalog_url: None, env_key: None, env_key_instructions: None, experimental_bearer_token: None, @@ -814,3 +818,34 @@ refresh_interval_ms = 0 assert_eq!(auth.refresh_interval_ms, 0); assert_eq!(auth.refresh_interval(), None); } + +#[test] +fn model_catalog_url_deserializes_without_changing_inference_routing() { + let provider: ModelProviderInfo = toml::from_str( + r#" +name = "Gateway" +base_url = "https://gateway.example/v1" +model_catalog_url = "https://gateway.example/codex/catalog?token=catalog-secret" +"#, + ) + .unwrap(); + assert!(!format!("{provider:?}").contains("catalog-secret")); + assert_eq!( + provider, + ModelProviderInfo { + name: "Gateway".to_string(), + base_url: Some("https://gateway.example/v1".to_string()), + model_catalog_url: Some( + "https://gateway.example/codex/catalog?token=catalog-secret".into() + ), + ..ModelProviderInfo::default() + } + ); + assert_eq!( + provider + .to_api_provider(Some(AuthMode::ApiKey)) + .unwrap() + .base_url, + "https://gateway.example/v1" + ); +} diff --git a/codex-rs/model-provider/src/models_endpoint.rs b/codex-rs/model-provider/src/models_endpoint.rs index a063a90d14..93639407a9 100644 --- a/codex-rs/model-provider/src/models_endpoint.rs +++ b/codex-rs/model-provider/src/models_endpoint.rs @@ -20,6 +20,7 @@ use codex_login::AuthManager; use codex_login::CodexAuth; use codex_login::GatewayAuthManager; use codex_login::collect_auth_env_telemetry; +use codex_login::default_client::ClientRedirectPolicy; use codex_login::default_client::create_client_for_route_async; use codex_model_provider_info::CHATGPT_CODEX_BASE_URL; use codex_model_provider_info::ModelProviderInfo; @@ -41,6 +42,8 @@ use crate::combined_auth::compose_auth; const MODELS_REFRESH_TIMEOUT: Duration = Duration::from_secs(5); const MODELS_ENDPOINT: &str = "/models"; +// Bound downloads from explicitly configured catalogs before decoding or caching them. +const MAX_MODEL_CATALOG_BYTES: usize = 1024 * 1024; /// Provider-owned OpenAI-compatible `/models` endpoint. #[derive(Debug)] @@ -57,11 +60,16 @@ impl OpenAiModelsEndpoint { auth_manager: Option>, gateway_auth_manager: Option, String>>, ) -> Self { + let redirect_policy = if provider_info.model_catalog_url.is_some() { + ClientRedirectPolicy::Reject + } else { + ClientRedirectPolicy::Default + }; Self { provider_info, auth_manager, gateway_auth_manager, - transport_builder: Arc::new(RouteAwareModelsTransportBuilder), + transport_builder: Arc::new(RouteAwareModelsTransportBuilder { redirect_policy }), } } @@ -90,9 +98,10 @@ impl OpenAiModelsEndpoint { let identity = crate::models_identity::identity(&self.provider_info, auth.as_ref())?; let auth_mode = auth.as_ref().map(CodexAuth::auth_mode); let mut api_provider = self.provider_info.to_api_provider(auth_mode)?; - if auth.as_ref().is_some_and(CodexAuth::is_api_key_auth) + if (auth.as_ref().is_some_and(CodexAuth::is_api_key_auth) || self.has_provider_api_key()) && self.supports_api_key_models() && self.provider_info.base_url.is_none() + && self.provider_info.model_catalog_url.is_none() { // Codex metadata is served by the Codex backend, not the public /v1/models API. api_provider.base_url = CHATGPT_CODEX_BASE_URL.to_string(); @@ -104,8 +113,15 @@ impl OpenAiModelsEndpoint { ) .await?; let api_auth = resolved.auth; - let request_url = - ModelsClient::::request_url(&api_provider, client_version); + let request_url = match self.provider_info.model_catalog_url.as_deref() { + Some(catalog_url) => ModelsClient::::catalog_request_url( + &api_provider, + catalog_url, + client_version, + ) + .map_err(map_api_error)?, + None => ModelsClient::::request_url(&api_provider, client_version), + }; let auth_telemetry = auth_header_telemetry(api_auth.as_ref()); let agent_identity_telemetry = if let Some(CodexAuth::AgentIdentity(auth)) = auth.as_ref() { Some(agent_identity_telemetry(auth)) @@ -113,6 +129,7 @@ impl OpenAiModelsEndpoint { None }; let request_telemetry: Arc = Arc::new(ModelsRequestTelemetry { + include_response_debug: self.provider_info.model_catalog_url.is_none(), auth_mode: auth_mode.map(|mode| TelemetryAuthMode::from(mode).to_string()), auth_header_attached: auth_telemetry.attached, auth_header_name: auth_telemetry.name, @@ -126,10 +143,30 @@ impl OpenAiModelsEndpoint { .await?; let client = ModelsClient::new(transport, api_provider, api_auth) .with_telemetry(Some(request_telemetry)); + let response_body_limit_bytes = self + .provider_info + .model_catalog_url + .as_ref() + .map(|_| MAX_MODEL_CATALOG_BYTES); client - .list_models(request_url, HeaderMap::new()) + .list_models(request_url, HeaderMap::new(), response_body_limit_bytes) .await - .map_err(map_api_error) + .map_err(|mut error| { + if self.provider_info.model_catalog_url.is_some() + && let codex_api::ApiError::Transport(TransportError::Http { + url, + headers, + body, + .. + }) = &mut error + { + // Provider diagnostics may echo URL credentials or other secrets. + *url = None; + *headers = None; + *body = None; + } + map_api_error(error) + }) }) .await .map_err(|_| CodexErr::RequestTimeout)??; @@ -151,7 +188,13 @@ impl OpenAiModelsEndpoint { impl ModelsEndpointClient for OpenAiModelsEndpoint { fn supports_api_key_models(&self) -> bool { - self.provider_info.is_openai() + self.provider_info.model_catalog_url.is_some() + || (self.provider_info.is_openai() && self.provider_info.base_url.is_none()) + } + + fn has_provider_api_key(&self) -> bool { + self.provider_info.env_key.is_some() + || self.provider_info.experimental_bearer_token.is_some() } fn identity(&self) -> Option { @@ -198,7 +241,9 @@ trait ModelsTransportBuilder: fmt::Debug + Send + Sync { } #[derive(Debug)] -struct RouteAwareModelsTransportBuilder; +struct RouteAwareModelsTransportBuilder { + redirect_policy: ClientRedirectPolicy, +} impl ModelsTransportBuilder for RouteAwareModelsTransportBuilder { fn build( @@ -206,16 +251,27 @@ impl ModelsTransportBuilder for RouteAwareModelsTransportBuilder { http_client_factory: HttpClientFactory, request_url: String, ) -> ModelsTransportFuture<'_> { + let redirect_policy = self.redirect_policy; Box::pin(async move { - create_client_for_route_async(http_client_factory, request_url, ClientRouteClass::Api) - .await - .map(ReqwestTransport::from_http_client) + let client = create_client_for_route_async( + http_client_factory, + request_url, + ClientRouteClass::Api, + redirect_policy, + ) + .await?; + let client = match redirect_policy { + ClientRedirectPolicy::Default => client, + ClientRedirectPolicy::Reject => client.without_request_logging(), + }; + Ok(ReqwestTransport::from_http_client(client)) }) } } #[derive(Clone)] struct ModelsRequestTelemetry { + include_response_debug: bool, auth_mode: Option, auth_header_attached: bool, auth_header_name: Option<&'static str>, @@ -234,6 +290,7 @@ impl RequestTelemetry for ModelsRequestTelemetry { let success = status.is_some_and(|code| code.is_success()) && error.is_none(); let error_message = error.map(telemetry_transport_error_message); let response_debug = error + .filter(|_| self.include_response_debug) .map(extract_response_debug_context) .unwrap_or_default(); let status = status.map(|status| status.as_u16()); @@ -331,6 +388,8 @@ mod tests { use codex_models_manager::manager::RefreshStrategy; use codex_protocol::auth::AuthMode; use codex_protocol::config_types::ModelProviderAuthInfo; + use codex_protocol::error::CodexErrorDetails; + use codex_protocol::openai_models::ModelVisibility; use codex_protocol::openai_models::ModelsResponse; use pretty_assertions::assert_eq; use wiremock::Mock; @@ -390,7 +449,7 @@ mod tests { ( "OpenAI", Some("https://example.com/codex"), - Some("https://example.com/codex/models"), + None, "https://example.com/codex", ), ( @@ -434,6 +493,41 @@ mod tests { } } + #[tokio::test] + async fn provider_api_key_without_endpoint_overrides_uses_codex_backend() { + for auth in [ + None, + Some(CodexAuth::create_dummy_chatgpt_auth_for_testing()), + ] { + let capture = Arc::new(CaptureModelsUrl(Mutex::new(/*t*/ None))); + let auth_manager = auth.map(AuthManager::from_auth_for_testing); + let endpoint = Arc::new(OpenAiModelsEndpoint { + provider_info: ModelProviderInfo { + experimental_bearer_token: Some("provider-key".into()), + ..ModelProviderInfo::create_openai_provider(/*base_url*/ None) + }, + auth_manager: auth_manager.clone(), + gateway_auth_manager: None, + transport_builder: capture.clone(), + }); + let manager = OpenAiModelsManager::new_without_cache(endpoint, auth_manager); + manager.set_api_key_model_discovery_enabled(/*enabled*/ true); + manager + .raw_model_catalog( + RefreshStrategy::Online, + HttpClientFactory::new(OutboundProxyPolicy::ReqwestDefault), + ) + .await; + assert_eq!( + *capture.0.lock().unwrap(), + Some(format!( + "{CHATGPT_CODEX_BASE_URL}/models?client_version={}", + codex_models_manager::client_version_to_whole(), + )) + ); + } + } + fn provider_info_with_command_auth() -> ModelProviderInfo { ModelProviderInfo { auth: Some(ModelProviderAuthInfo { @@ -671,4 +765,153 @@ mod tests { vec!["Bearer token-2", "Bearer token-5", "Bearer token-9"] ); } + + #[tokio::test] + async fn explicit_catalog_reuses_auth_headers_and_query_parameters() { + for auth in [ + CodexAuth::create_dummy_chatgpt_auth_for_testing(), + CodexAuth::from_api_key("test-key"), + ] { + let server = MockServer::start().await; + let mut model = + codex_models_manager::model_info::model_info_from_slug("provider-model"); + model.visibility = ModelVisibility::List; + model.supported_in_api = true; + model.used_fallback_model_metadata = false; + let expected = ModelsResponse { + models: vec![model], + }; + Mock::given(method("GET")) + .and(path("/codex/models")) + .and(header( + "authorization", + format!("Bearer {}", auth.get_token().unwrap()), + )) + .and(header("x-provider-header", "preserved")) + .and(query_param("deployment", "one")) + .and(query_param("api-version", "2026-09")) + .respond_with(ResponseTemplate::new(/*s*/ 200).set_body_json(&expected)) + .expect(1) + .mount(&server) + .await; + let auth_manager = AuthManager::from_auth_for_testing(auth); + // The parallel residency test must not change this catalog's cache identity. + let provider = ModelProviderInfo { + model_catalog_url: Some( + format!("{}/codex/models?deployment=one", server.uri()).into(), + ), + query_params: Some(std::collections::HashMap::from([( + "api-version".to_string(), + "2026-09".into(), + )])), + http_headers: Some(std::collections::HashMap::from([ + ("x-provider-header".to_string(), "preserved".into()), + ( + codex_login::default_client::RESIDENCY_HEADER_NAME.to_string(), + "us".into(), + ), + ])), + ..ModelProviderInfo::create_openai_provider(Some(format!("{}/v1", server.uri()))) + }; + let home = tempfile::tempdir().unwrap(); + let endpoint = Arc::new(OpenAiModelsEndpoint::new( + provider, + Some(auth_manager.clone()), + /*gateway_auth_manager*/ None, + )); + let manager = OpenAiModelsManager::new( + home.path().to_path_buf(), + endpoint, + Some(auth_manager.clone()), + ); + // SIWC does not require the API-key rollout flag. + manager.set_api_key_model_discovery_enabled( + auth_manager.auth_mode() == Some(codex_protocol::auth::AuthMode::ApiKey), + ); + for _ in 0..2 { + assert_eq!( + manager + .raw_model_catalog( + RefreshStrategy::OnlineIfUncached, + HttpClientFactory::new(OutboundProxyPolicy::ReqwestDefault) + ) + .await, + expected + ); + } + } + } + + #[tokio::test] + async fn explicit_catalog_rejects_oversized_response() { + let server = MockServer::start().await; + let mut body = br#"{"models":[]}"#.to_vec(); + body.resize(MAX_MODEL_CATALOG_BYTES + 1, b' '); + Mock::given(method("GET")) + .and(path("/codex/models")) + .respond_with(ResponseTemplate::new(/*s*/ 200).set_body_bytes(body)) + .expect(1) + .mount(&server) + .await; + let endpoint = OpenAiModelsEndpoint::new( + ModelProviderInfo { + model_catalog_url: Some(format!("{}/codex/models", server.uri()).into()), + ..ModelProviderInfo::default() + }, + /*auth_manager*/ None, + /*gateway_auth_manager*/ None, + ); + let error = endpoint + .list_models( + "1.2.3", + HttpClientFactory::new(OutboundProxyPolicy::ReqwestDefault), + ) + .await + .expect_err("oversized catalog should be rejected"); + let CodexErrorDetails::InvalidRequest(message) = error.details() else { + panic!("expected a response-size error, got {error:?}"); + }; + assert_eq!( + message, + &format!("response body exceeds the {MAX_MODEL_CATALOG_BYTES} byte limit") + ); + } + + #[tokio::test] + async fn explicit_catalog_rejects_redirects_without_forwarding_provider_credentials() { + let server = MockServer::start().await; + let destination = MockServer::start().await; + Mock::given(method("GET")) + .and(path("/codex/models")) + .respond_with( + ResponseTemplate::new(/*s*/ 302) + .insert_header("location", format!("{}/codex/models", destination.uri())) + .set_body_string("catalog-secret"), + ) + .expect(1) + .mount(&server) + .await; + let endpoint = OpenAiModelsEndpoint::new( + ModelProviderInfo { + base_url: Some(server.uri()), + model_catalog_url: Some( + format!("{}/codex/models?token=catalog-secret", server.uri()).into(), + ), + experimental_bearer_token: Some("provider-key".into()), + ..ModelProviderInfo::default() + }, + /*auth_manager*/ None, + /*gateway_auth_manager*/ None, + ); + let error = endpoint + .list_models( + "1.2.3", + HttpClientFactory::new(OutboundProxyPolicy::ReqwestDefault), + ) + .await + .expect_err("redirect should be rejected"); + assert!(!error.to_string().contains("catalog-secret")); + assert!(!format!("{error:?}").contains("catalog-secret")); + assert_eq!(destination.received_requests().await.unwrap().len(), 0); + } } diff --git a/codex-rs/model-provider/src/models_identity.rs b/codex-rs/model-provider/src/models_identity.rs index 73343d9df8..409ff83a07 100644 --- a/codex-rs/model-provider/src/models_identity.rs +++ b/codex-rs/model-provider/src/models_identity.rs @@ -14,7 +14,7 @@ pub(crate) fn identity( provider_info: &ModelProviderInfo, auth: Option<&CodexAuth>, ) -> CoreResult { - let mut provider = provider_info.to_api_provider(auth.map(CodexAuth::auth_mode))?; + let mut api_provider = provider_info.to_api_provider(auth.map(CodexAuth::auth_mode))?; let mut digest = Sha256::new(); // Length-prefix every field to avoid ambiguity between adjacent values. let mut field = |value: &[u8]| { @@ -22,9 +22,15 @@ pub(crate) fn identity( digest.update(value); }; field(b"models-cache-v1"); - field(provider.name.as_bytes()); - field(provider.base_url.as_bytes()); - let mut query: Vec<_> = provider.query_params.iter().flatten().collect(); + field(api_provider.name.as_bytes()); + field(api_provider.base_url.as_bytes()); + // Preserve existing cache identities when no explicit catalog is configured. + // The API provider resolves base_url defaults; model_catalog_url stays on ModelProviderInfo. + if let Some(catalog_url) = &provider_info.model_catalog_url { + field(b"model_catalog_url"); + field(catalog_url.as_bytes()); + } + let mut query: Vec<_> = api_provider.query_params.iter().flatten().collect(); query.sort(); field(&(query.len() as u64).to_le_bytes()); for (name, value) in query { @@ -63,9 +69,9 @@ pub(crate) fn identity( // different account, so only credentials with stable owner metadata can be reused. if !has_stable_account || explicit_bearer { let headers = resolve_provider_auth(auth, provider_info)?.to_auth_headers(); - provider.headers.extend(headers); + api_provider.headers.extend(headers); } - let mut headers: Vec<_> = provider.headers.iter().collect(); + let mut headers: Vec<_> = api_provider.headers.iter().collect(); headers.sort_by(|(a, av), (b, bv)| { a.as_str() .cmp(b.as_str()) diff --git a/codex-rs/model-provider/src/models_identity_tests.rs b/codex-rs/model-provider/src/models_identity_tests.rs index 4732265de1..16f88b41ae 100644 --- a/codex-rs/model-provider/src/models_identity_tests.rs +++ b/codex-rs/model-provider/src/models_identity_tests.rs @@ -87,3 +87,17 @@ fn cache_identity_distinguishes_auth_requirements_and_unknown_plans() { provider.requires_openai_auth = false; assert_ne!(key, identity(&provider, Some(&first)).unwrap()); } + +#[test] +fn cache_identity_tracks_catalog_url() { + let auth = CodexAuth::from_api_key("test-key"); + let mut provider = + ModelProviderInfo::create_openai_provider(Some("https://gateway.example/v1".into())); + let bundled = identity(&provider, Some(&auth)).unwrap(); + provider.model_catalog_url = Some("https://gateway.example/catalog-one".into()); + let first = identity(&provider, Some(&auth)).unwrap(); + provider.model_catalog_url = Some("https://gateway.example/catalog-two".into()); + let second = identity(&provider, Some(&auth)).unwrap(); + assert_ne!(bundled, first); + assert_ne!(first, second); +} diff --git a/codex-rs/model-provider/src/provider.rs b/codex-rs/model-provider/src/provider.rs index 5f7bd4b035..08afa3e56f 100644 --- a/codex-rs/model-provider/src/provider.rs +++ b/codex-rs/model-provider/src/provider.rs @@ -679,6 +679,7 @@ mod tests { ModelProviderInfo { name: "mock".into(), base_url: Some(base_url), + model_catalog_url: None, env_key: None, env_key_instructions: None, experimental_bearer_token: None, @@ -1380,33 +1381,50 @@ printf '%s\n' '{"AccessKeyId":"exported","SecretAccessKey":"secret"}' models: remote_models.clone(), }), ) - .expect(1) + .expect(2) .mount(&server) .await; let mut provider_info = provider_for(server.uri()); provider_info.experimental_bearer_token = Some("provider-token".into()); - let provider = create_model_provider( - provider_info, - Some(AuthManager::from_auth_for_testing( - CodexAuth::create_dummy_chatgpt_auth_for_testing(), - )), - ); + provider_info.model_catalog_url = Some(format!("{}/models", server.uri()).into()); + provider_info.http_headers = Some(std::collections::HashMap::from([( + codex_login::default_client::RESIDENCY_HEADER_NAME.to_string(), + "us".into(), + )])); + for auth in [ + None, + Some(CodexAuth::create_dummy_chatgpt_auth_for_testing()), + ] { + // Disabled discovery must ignore the catalog cached by the enabled run. + for enabled in [true, false] { + let provider = create_model_provider( + provider_info.clone(), + auth.clone().map(AuthManager::from_auth_for_testing), + ); + let manager = + provider.models_manager(test_codex_home(), /*config_model_catalog*/ None); + manager.set_api_key_model_discovery_enabled(enabled); + let refresh_strategy = if enabled { + RefreshStrategy::Online + } else { + RefreshStrategy::Offline + }; + let catalog = manager + .raw_model_catalog( + refresh_strategy, + HttpClientFactory::new(OutboundProxyPolicy::ReqwestDefault), + ) + .await; - let manager = - provider.models_manager(test_codex_home(), /*config_model_catalog*/ None); - let catalog = manager - .raw_model_catalog( - RefreshStrategy::Online, - HttpClientFactory::new(OutboundProxyPolicy::ReqwestDefault), - ) - .await; - - assert!( - catalog - .models - .iter() - .any(|model| model.slug == "provider-model") - ); + assert_eq!( + catalog + .models + .iter() + .any(|model| model.slug == "provider-model"), + enabled + ); + } + } } } diff --git a/codex-rs/model-provider/src/shared_state_tests.rs b/codex-rs/model-provider/src/shared_state_tests.rs index 2bcb05bde6..09caa4cf87 100644 --- a/codex-rs/model-provider/src/shared_state_tests.rs +++ b/codex-rs/model-provider/src/shared_state_tests.rs @@ -271,6 +271,7 @@ async fn models_cache_is_reused_only_for_matching_gateway_configuration() { home.path().to_path_buf(), ); let mut info = ModelProviderInfo { + model_catalog_url: Some(format!("{}/models", server.uri()).into()), http_headers: Some(std::collections::HashMap::from([( codex_login::default_client::RESIDENCY_HEADER_NAME.to_string(), "us".into(), @@ -354,6 +355,7 @@ async fn models_observe_gateway_rotation(instance: ProviderInstance) { home.path().to_path_buf(), ); let info = ModelProviderInfo { + model_catalog_url: Some(format!("{}/models", server.uri()).into()), gateway_oauth: Some(GatewayOAuthConfig { authorization_url: format!("{}/authorize", server.uri()), token_url: format!("{}/token", server.uri()), diff --git a/codex-rs/models-manager/src/manager.rs b/codex-rs/models-manager/src/manager.rs index 88151f7735..7b4d613be4 100644 --- a/codex-rs/models-manager/src/manager.rs +++ b/codex-rs/models-manager/src/manager.rs @@ -54,6 +54,12 @@ pub trait ModelsEndpointClient: fmt::Debug + Send + Sync { false } + /// Returns whether explicit provider configuration supplies API-key authentication. + /// This takes precedence over any unrelated first-party login used by the picker. + fn has_provider_api_key(&self) -> bool { + false + } + /// Fetches the latest remote model catalog and optional ETag. fn list_models<'a>( &'a self, @@ -476,9 +482,13 @@ impl OpenAiModelsManager { refresh_strategy: RefreshStrategy, http_client_factory: &HttpClientFactory, ) -> CoreResult<()> { - // Gate cache loading as well as requests; disabled sessions use bundled API-key models. - if self.supports_api_key_discovery() - && !self.api_key_model_discovery_enabled.load(Ordering::SeqCst) + // API-key discovery must be enabled and supported before reusing a remote catalog. + // Otherwise even a matching cache from an earlier run would bypass bundled-only behavior. + // Command-auth providers retain their existing discovery behavior. + if self.uses_api_key_auth() + && !self.endpoint_client.has_command_auth() + && (!self.endpoint_client.supports_api_key_models() + || !self.api_key_model_discovery_enabled.load(Ordering::SeqCst)) { return Ok(()); } @@ -541,7 +551,12 @@ impl OpenAiModelsManager { fn supports_api_key_discovery(&self) -> bool { self.endpoint_client.supports_api_key_models() && !self.endpoint_client.has_command_auth() - && self + && self.uses_api_key_auth() + } + + fn uses_api_key_auth(&self) -> bool { + self.endpoint_client.has_provider_api_key() + || self .auth_manager .as_ref() .is_some_and(|auth_manager| auth_manager.auth_mode() == Some(AuthMode::ApiKey))