From 7d4c7a0767cdf81ff76c8fa5a05c22929568e3c6 Mon Sep 17 00:00:00 2001 From: andrewgu-oai Date: Wed, 30 Sep 2026 22:37:42 +0000 Subject: [PATCH] Enable API-key model discovery by default (#49807) ## What changed - Mark `api_key_model_discovery` as stable and enable it by default, while preserving explicit user opt-outs and provider catalog requirements. - Fall back to bundled models when discovery is disabled at runtime, including when a remote catalog has already been loaded. ## Testing Extend model-list coverage for default enablement, app overrides, and user configuration precedence. Add a regression assertion that runtime opt-outs hide loaded catalogs without additional fetches, and update daemon compatibility tests for the new default. GitOrigin-RevId: feaf732757792cc58063f21a5327d0e633f079f4 --- .../app-server/tests/suite/v2/model_list.rs | 23 +++++++++++------- codex-rs/cli/tests/daemon_startup.rs | 8 +++---- codex-rs/features/src/lib.rs | 4 ++-- .../src/api_key_discovery_tests.rs | 16 +++++++++++-- codex-rs/models-manager/src/manager.rs | 24 +++++++++++++------ .../tui/tests/suite/daemon_compatibility.rs | 10 ++++---- codex-rs/tui/tests/suite/directory_trust.rs | 2 +- codex-rs/tui/tests/suite/reconnect.rs | 2 +- ...ompatibility__daemon_feature_mismatch.snap | 2 +- ...mpatibility__daemon_override_mismatch.snap | 2 +- 10 files changed, 59 insertions(+), 34 deletions(-) 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 d21bebfc37..cf9021742a 100644 --- a/codex-rs/app-server/tests/suite/v2/model_list.rs +++ b/codex-rs/app-server/tests/suite/v2/model_list.rs @@ -39,15 +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, 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")] +#[test_case(None, None, true; "default on")] +#[test_case(None, Some(false), true; "app rollback")] +#[test_case(None, Some(true), true; "app rollout")] +#[test_case(Some(false), Some(true), true; "user opt out")] +#[test_case(Some(true), Some(false), true; "user opt in")] +#[test_case(Some(true), Some(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, + app_enablement: Option, catalog_opt_in: bool, ) -> Result<()> { let server = MockServer::start().await; @@ -115,7 +116,10 @@ requires_openai_auth = true |request_id| ClientRequest::ExperimentalFeatureEnablementSet { request_id, params: ExperimentalFeatureEnablementSetParams { - enablement: [("api_key_model_discovery".to_string(), app_enablement)].into(), + enablement: app_enablement + .into_iter() + .map(|enabled| ("api_key_model_discovery".to_string(), enabled)) + .collect(), }, }, ) @@ -130,7 +134,7 @@ requires_openai_auth = true }, }) .await?; - let enabled = user_enablement.unwrap_or(app_enablement) && catalog_opt_in; + let enabled = user_enablement.or(app_enablement).unwrap_or(true) && catalog_opt_in; let expected: &[ModelPreset] = if enabled { &remote } else if catalog_opt_in { @@ -154,7 +158,8 @@ requires_openai_auth = true next_cursor: None, } ); - if !enabled { + // Only startup opt-outs prevent fetches before the app sends its enablement. + if user_enablement == Some(false) || !catalog_opt_in { assert!( server .received_requests() diff --git a/codex-rs/cli/tests/daemon_startup.rs b/codex-rs/cli/tests/daemon_startup.rs index 67eff23d4d..3f6a22e7d8 100644 --- a/codex-rs/cli/tests/daemon_startup.rs +++ b/codex-rs/cli/tests/daemon_startup.rs @@ -210,12 +210,10 @@ async fn daemon_startup(command: &str) -> Result<()> { steps.push_back(("GPT-5.6-Terra", b"\x14")); "Runningwithoutthesharedbackgroundserver:thisWindowslauncher" } else if mismatch { - args.extend(if persisted { - ["--disable".into(), "api_key_model_discovery".into()] - } else if disabling { + args.extend(if disabling && !persisted { ["--disable".into(), "auth_elicitation".into()] } else { - ["--enable".into(), "api_key_model_discovery".into()] + ["--disable".into(), "api_key_model_discovery".into()] }); let input: &[u8] = match command { "mismatch-cancel" => b"\x03", @@ -325,7 +323,7 @@ async fn daemon_startup(command: &str) -> Result<()> { let previous_pid = existing_daemon.as_ref().context("missing original daemon PID")?; ensure!((fs::read(&pid_file)? != *previous_pid) == restart); if command == "mismatch-cancel" { - ensure!(text.contains("Cannotusethesharedbackgroundserver:Thissessionrequiresapi_key_model_discoverytobeenabled.")); + ensure!(text.contains("Cannotusethesharedbackgroundserver:Thissessionrequiresapi_key_model_discoverytobedisabled.")); } else { ensure!(text.contains("Server:Localbackgroundserver") == restart); } diff --git a/codex-rs/features/src/lib.rs b/codex-rs/features/src/lib.rs index e47731d1e6..519df967a8 100644 --- a/codex-rs/features/src/lib.rs +++ b/codex-rs/features/src/lib.rs @@ -1311,8 +1311,8 @@ pub const FEATURES: &[FeatureSpec] = &[ FeatureSpec { id: Feature::ApiKeyModelDiscovery, key: "api_key_model_discovery", - stage: Stage::UnderDevelopment, - default_enabled: false, + stage: Stage::Stable, + default_enabled: true, }, FeatureSpec { id: Feature::ApiKeyCyberAccessPrograms, diff --git a/codex-rs/models-manager/src/api_key_discovery_tests.rs b/codex-rs/models-manager/src/api_key_discovery_tests.rs index e818a0c6ef..518a5a4868 100644 --- a/codex-rs/models-manager/src/api_key_discovery_tests.rs +++ b/codex-rs/models-manager/src/api_key_discovery_tests.rs @@ -1,4 +1,4 @@ -//! API-key discovery rollout must gate network requests and cached catalog authority. +//! API-key discovery opt-outs must gate network requests and cached catalog authority. use super::*; use pretty_assertions::assert_eq; @@ -34,7 +34,7 @@ async fn api_key_discovery_disabled_preserves_command_auth_discovery_and_merging } #[tokio::test] -async fn api_key_discovery_startup_flag_controls_fetches_and_cached_catalogs() { +async fn api_key_discovery_flag_controls_fetches_and_cached_catalogs() { let home = tempdir().unwrap(); let models = vec![remote_model("dynamic", "Dynamic", /*priority*/ 0)]; let endpoint = TestModelsEndpoint::without_refresh(vec![models.clone()]); @@ -70,6 +70,18 @@ async fn api_key_discovery_startup_flag_controls_fetches_and_cached_catalogs() { ); assert_eq!(endpoint.fetch_count(), 1); + // A runtime rollback must also suppress the already-loaded catalog. + manager.set_api_key_model_discovery_enabled(/*enabled*/ false); + assert_eq!( + manager + .raw_model_catalog(RefreshStrategy::Online, DEFAULT_HTTP_CLIENT_FACTORY) + .await + .models, + bundled + ); + assert_eq!(manager.try_get_remote_models().unwrap(), bundled); + assert_eq!(endpoint.fetch_count(), 1); + // A new session must honor its startup flag even when a matching disk cache exists. for enabled in [false, true] { let restarted = diff --git a/codex-rs/models-manager/src/manager.rs b/codex-rs/models-manager/src/manager.rs index af734443c0..b1efc0fe7c 100644 --- a/codex-rs/models-manager/src/manager.rs +++ b/codex-rs/models-manager/src/manager.rs @@ -429,7 +429,10 @@ impl ModelsManager for OpenAiModelsManager { fn get_remote_models(&self) -> ModelsManagerFuture<'_, Vec> { Box::pin(async move { let entry = self.remote_models.read().await; - if entry.identity.is_some() && entry.identity == self.endpoint_client.identity() { + if !self.api_key_discovery_disabled() + && entry.identity.is_some() + && entry.identity == self.endpoint_client.identity() + { entry.models.clone() } else { self.catalog_source.fallback_models().unwrap_or_default() @@ -440,7 +443,10 @@ impl ModelsManager for OpenAiModelsManager { fn try_get_remote_models(&self) -> Result, TryLockError> { let entry = self.remote_models.try_read()?; Ok( - if entry.identity.is_some() && entry.identity == self.endpoint_client.identity() { + if !self.api_key_discovery_disabled() + && entry.identity.is_some() + && entry.identity == self.endpoint_client.identity() + { entry.models.clone() } else { self.catalog_source.fallback_models().unwrap_or_default() @@ -545,11 +551,7 @@ impl OpenAiModelsManager { // 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)) - { + if self.api_key_discovery_disabled() { return Ok(()); } if !self.should_refresh_models().await { @@ -654,6 +656,14 @@ impl OpenAiModelsManager { && self.uses_api_key_auth() } + // A runtime opt-out must hide catalogs fetched before the flag was disabled, too. + fn api_key_discovery_disabled(&self) -> bool { + 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)) + } + fn uses_api_key_auth(&self) -> bool { self.endpoint_client.has_provider_api_key() || self diff --git a/codex-rs/tui/tests/suite/daemon_compatibility.rs b/codex-rs/tui/tests/suite/daemon_compatibility.rs index 5e6ea64585..4ca27c838c 100644 --- a/codex-rs/tui/tests/suite/daemon_compatibility.rs +++ b/codex-rs/tui/tests/suite/daemon_compatibility.rs @@ -48,9 +48,9 @@ async fn incompatible_daemon_falls_back_for_default_and_explicit_features() -> R } else { assert_eq!(request.method, "experimentalFeature/list"); json!({"id": request.id, "result": {"data": [{ - "name": "api_key_model_discovery", "stage": "underDevelopment", + "name": "api_key_model_discovery", "stage": "stable", "displayName": null, "description": null, "announcement": null, - "enabled": scenario == "default", "defaultEnabled": false, + "enabled": scenario == "explicit", "defaultEnabled": true, }], "nextCursor": null}}) }; socket @@ -60,7 +60,7 @@ async fn incompatible_daemon_falls_back_for_default_and_explicit_features() -> R Ok::<_, anyhow::Error>(()) }); let args = if scenario == "explicit" { - vec!["-c", "features.api_key_model_discovery=true"] + vec!["-c", "features.api_key_model_discovery=false"] } else { vec![] }; @@ -71,11 +71,11 @@ async fn incompatible_daemon_falls_back_for_default_and_explicit_features() -> R let (snapshot, warning_end) = match scenario { "default" => ( "daemon_feature_mismatch", - "api_key_model_discovery to be disabled.", + "api_key_model_discovery to be enabled.", ), "explicit" => ( "daemon_override_mismatch", - "api_key_model_discovery to be enabled.", + "api_key_model_discovery to be disabled.", ), "host policy" => ("daemon_host_policy_mismatch", "requires embedded mode."), _ => unreachable!(), diff --git a/codex-rs/tui/tests/suite/directory_trust.rs b/codex-rs/tui/tests/suite/directory_trust.rs index f44a19a9ee..0cfbfc252a 100644 --- a/codex-rs/tui/tests/suite/directory_trust.rs +++ b/codex-rs/tui/tests/suite/directory_trust.rs @@ -75,7 +75,7 @@ async fn connected_trust_cancellation_and_acceptance_control_task_creation() -> methods.lock().unwrap().push(request.method.clone()); let result = match request.method.as_str() { "initialize" => json!({"userAgent": "trust-pty"}), - "experimentalFeature/list" => json!({"data": (["code_mode_host", "auth_elicitation"].map(|name| json!({ + "experimentalFeature/list" => json!({"data": (["api_key_model_discovery", "code_mode_host", "auth_elicitation"].map(|name| json!({ "name": name, "stage": "stable", "displayName": null, "description": null, "announcement": null, "enabled": true, "defaultEnabled": true, diff --git a/codex-rs/tui/tests/suite/reconnect.rs b/codex-rs/tui/tests/suite/reconnect.rs index 0064d0c889..e62dc3dbb3 100644 --- a/codex-rs/tui/tests/suite/reconnect.rs +++ b/codex-rs/tui/tests/suite/reconnect.rs @@ -87,7 +87,7 @@ async fn automatic_reconnect_restores_draft_and_routes_new_notifications() -> Re "initialize" => json!({"userAgent": "reconnect-pty"}), // An older daemon can omit the client's default-disabled features. "experimentalFeature/list" => { - json!({"data": (["code_mode_host", "auth_elicitation"].map(|name| json!({ + json!({"data": (["api_key_model_discovery", "code_mode_host", "auth_elicitation"].map(|name| json!({ "name": name, "stage": "stable", "displayName": null, "description": null, "announcement": null, "enabled": true, "defaultEnabled": true, diff --git a/codex-rs/tui/tests/suite/snapshots/all__suite__daemon_compatibility__daemon_feature_mismatch.snap b/codex-rs/tui/tests/suite/snapshots/all__suite__daemon_compatibility__daemon_feature_mismatch.snap index 60251d073c..f4f7d9ab20 100644 --- a/codex-rs/tui/tests/suite/snapshots/all__suite__daemon_compatibility__daemon_feature_mismatch.snap +++ b/codex-rs/tui/tests/suite/snapshots/all__suite__daemon_compatibility__daemon_feature_mismatch.snap @@ -2,4 +2,4 @@ source: tui/tests/suite/daemon_compatibility.rs expression: warning --- -⚠ Running without the shared background server: This session requires api_key_model_discovery to be disabled. +⚠ Running without the shared background server: This session requires api_key_model_discovery to be enabled. diff --git a/codex-rs/tui/tests/suite/snapshots/all__suite__daemon_compatibility__daemon_override_mismatch.snap b/codex-rs/tui/tests/suite/snapshots/all__suite__daemon_compatibility__daemon_override_mismatch.snap index f4f7d9ab20..60251d073c 100644 --- a/codex-rs/tui/tests/suite/snapshots/all__suite__daemon_compatibility__daemon_override_mismatch.snap +++ b/codex-rs/tui/tests/suite/snapshots/all__suite__daemon_compatibility__daemon_override_mismatch.snap @@ -2,4 +2,4 @@ source: tui/tests/suite/daemon_compatibility.rs expression: warning --- -⚠ Running without the shared background server: This session requires api_key_model_discovery to be enabled. +⚠ Running without the shared background server: This session requires api_key_model_discovery to be disabled.