mirror of
https://github.com/openai/codex.git
synced 2026-10-18 17:18:52 +00:00
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
This commit is contained in:
@@ -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<bool>,
|
||||
app_enablement: bool,
|
||||
app_enablement: Option<bool>,
|
||||
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()
|
||||
|
||||
@@ -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);
|
||||
}
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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 =
|
||||
|
||||
@@ -429,7 +429,10 @@ impl ModelsManager for OpenAiModelsManager {
|
||||
fn get_remote_models(&self) -> ModelsManagerFuture<'_, Vec<ModelInfo>> {
|
||||
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<Vec<ModelInfo>, 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
|
||||
|
||||
@@ -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!(),
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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,
|
||||
|
||||
+1
-1
@@ -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.
|
||||
|
||||
+1
-1
@@ -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.
|
||||
|
||||
Reference in New Issue
Block a user