diff --git a/codex-rs/cli/tests/daemon_startup.rs b/codex-rs/cli/tests/daemon_startup.rs index e55abb9164..12bcba2bcf 100644 --- a/codex-rs/cli/tests/daemon_startup.rs +++ b/codex-rs/cli/tests/daemon_startup.rs @@ -364,6 +364,17 @@ async fn daemon_startup(command: &str) -> Result<()> { ensure!(text.contains("Cannotusethesharedbackgroundserver:Thissessionrequiresapi_key_model_discoverytobedisabled.")); } else { ensure!(text.contains("Server:Localbackgroundserver") == restart); + if restart { + let settings: serde_json::Value = serde_json::from_slice( + &fs::read(home.path().join("app-server-daemon/settings.json"))? + )?; + let flag = if disabling && !persisted { + "auth_elicitation" + } else { + "api_key_model_discovery" + }; + assert_eq!(settings["featureOverrides"], serde_json::json!({(flag): false})); + } } } else if command == "start" { ensure!(text.contains("Server:Localbackgroundserver")); diff --git a/codex-rs/tui/src/daemon_recovery.rs b/codex-rs/tui/src/daemon_recovery.rs index 2c9959201e..0dd9808bfb 100644 --- a/codex-rs/tui/src/daemon_recovery.rs +++ b/codex-rs/tui/src/daemon_recovery.rs @@ -31,10 +31,15 @@ pub(super) async fn check( startup: &mut StartupDraft, target: &AppServerTarget, config: &Config, + cli_kv_overrides: &[(String, toml::Value)], managed_daemon: bool, ) -> io::Result> { let issue = match startup - .run_until(daemon_startup::compatibility_warning(target, config)) + .run_until(daemon_startup::compatibility_warning( + target, + config, + cli_kv_overrides, + )) .await? { Ok(warning) => return Ok(warning), @@ -104,7 +109,11 @@ pub(super) async fn check( }) .await?; startup - .run_until(daemon_startup::compatibility_warning(target, config)) + .run_until(daemon_startup::compatibility_warning( + target, + config, + cli_kv_overrides, + )) .await? .map_err(io::Error::other) } diff --git a/codex-rs/tui/src/daemon_startup.rs b/codex-rs/tui/src/daemon_startup.rs index 6476c94c7b..91c38d7837 100644 --- a/codex-rs/tui/src/daemon_startup.rs +++ b/codex-rs/tui/src/daemon_startup.rs @@ -5,6 +5,10 @@ //! Windows sessions use explicit embedded behavior before discovery or startup. use super::*; +use codex_app_server_client::TypedRequestError; +use codex_app_server_protocol::ClientRequest; +use codex_app_server_protocol::ConfigRequirementsReadResponse; +use codex_app_server_protocol::RequestId; use std::collections::BTreeMap; const SERVER_FEATURES: [Feature; 4] = [ @@ -155,6 +159,7 @@ pub(super) fn server_features(overrides: &[(String, toml::Value)]) -> BTreeMap Result, CompatibilityError> { let AppServerTarget::LocalDaemon { allow_embedded_fallback, @@ -165,15 +170,61 @@ pub(super) async fn compatibility_warning( }; let mut restart_features = None; let check = async { - // The feature-list RPC cannot report this process-scoped structured setting. - if !config.features.enabled(Feature::CodeModeHost) - && config.code_mode.disable_in_process_fallback - { - return Err("code-mode host fallback policy requires embedded mode".to_string()); - } let client = app_server_connection::connect(target) .await .map_err(|_| "could not connect to check daemon feature settings".to_string())?; + // File settings and defaults belong to the daemon until it restarts. + // Only explicit invocation flags can request a shared-service restart. + let mut requested = server_features(cli_kv_overrides); + if requested.is_empty() { + let _ = client.shutdown().await; + return Ok(()); + } + // Both legacy managed config and current requirements override CLI flags. + let layers = crate::config_update::read_effective_config_if_supported( + client.request_handle(), + config.cwd.as_path(), + ) + .await + .map_err(|_| "could not read daemon configuration".to_string())? + .and_then(|config| config.layers) + .unwrap_or_default(); + for layer in layers.iter().filter(|layer| { + layer["disabledReason"].is_null() + && matches!( + layer["name"]["type"].as_str(), + Some("legacyManagedConfigTomlFromFile" | "legacyManagedConfigTomlFromMdm") + ) + }) { + requested.retain(|name, _| layer["config"]["features"].get(name).is_none()); + } + match client + .request_handle() + .request_typed::( + ClientRequest::ConfigRequirementsRead { + request_id: RequestId::String("tui-daemon-requirements".to_string()), + params: None, + }, + ) + .await + { + Ok(response) => { + if let Some(required) = response.requirements.and_then(|r| r.feature_requirements) { + requested.retain(|name, _| !required.contains_key(name)); + } + } + Err(TypedRequestError::Server { source, .. }) + if source.code == -32601 + || source.code == -32600 + && source.message.contains("configRequirements/read") + && (source.message.contains("unknown variant") + || source.message.contains("unknown method")) => {} + Err(_) => return Err("could not check daemon configuration requirements".to_string()), + } + if requested.is_empty() { + let _ = client.shutdown().await; + return Ok(()); + } let (tx, rx) = tokio::sync::oneshot::channel(); crate::experimental_features::fetch( client.request_handle(), @@ -184,27 +235,17 @@ pub(super) async fn compatibility_warning( let result = rx.await; let _ = client.shutdown().await; let features = result.map_err(|_| "daemon feature check was interrupted".to_string())??; - // A previous client may have launched this daemon with overrides, even if - // this client has none. Check effective values, including defaults. - for feature in SERVER_FEATURES { - let name = feature.key(); - let enabled = config.features.enabled(feature); + for (name, enabled) in &requested { if features .iter() - .find(|feature| feature.name == name) + .find(|feature| feature.name == *name) .is_some_and(|feature| feature.enabled) - != enabled + != *enabled { - restart_features = Some( - SERVER_FEATURES - .into_iter() - .map(|feature| { - (feature.key().to_string(), config.features.enabled(feature)) - }) - .collect(), - ); - let state = if enabled { "enabled" } else { "disabled" }; - return Err(format!("This session requires {name} to be {state}")); + let state = if *enabled { "enabled" } else { "disabled" }; + let reason = format!("This session requires {name} to be {state}"); + restart_features = Some(requested); + return Err(reason); } } Ok::<(), String>(()) diff --git a/codex-rs/tui/src/daemon_startup_tests.rs b/codex-rs/tui/src/daemon_startup_tests.rs index 93f169f037..2a6a2eaf1f 100644 --- a/codex-rs/tui/src/daemon_startup_tests.rs +++ b/codex-rs/tui/src/daemon_startup_tests.rs @@ -152,6 +152,10 @@ async fn daemon_feature_compatibility_respects_required_and_optional_attachment( "matching", "stale overrides", "conflicting client", + "explicit enable", + "managed", + "legacy managed", + "old requirements server", "unsupported RPC", ] { let home = TempDir::new()?; @@ -161,19 +165,21 @@ async fn daemon_feature_compatibility_respects_required_and_optional_attachment( .build() .await?; config.features.enable(Feature::ApiKeyModelDiscovery)?; - if scenario == "conflicting client" { - config.features.disable(Feature::ApiKeyModelDiscovery)?; - } + let cli_overrides = match scenario { + "matching" | "stale overrides" => Vec::new(), + _ => vec![( + "features.api_key_model_discovery".to_string(), + toml::Value::Boolean(scenario == "explicit enable"), + )], + }; let features = [ Feature::ApiKeyModelDiscovery, Feature::CodeModeHost, Feature::AuthElicitation, Feature::McpOAuthRefreshCoordination, ].map(|feature| { - let enabled = if feature == Feature::ApiKeyModelDiscovery && scenario == "stale overrides" { + let enabled = if feature == Feature::ApiKeyModelDiscovery && matches!(scenario, "stale overrides" | "explicit enable") { false - } else if feature == Feature::ApiKeyModelDiscovery && scenario == "conflicting client" { - true } else { config.features.enabled(feature) }; @@ -195,16 +201,34 @@ async fn daemon_feature_compatibility_respects_required_and_optional_attachment( else { continue; }; - let response = if request.method == "initialize" { - json!({"id": request.id, "result": {"userAgent": "daemon-test"}}) - } else { - assert_eq!(request.method, "experimentalFeature/list"); - assert_eq!(request.params.unwrap()["threadId"], json!(null)); - if scenario == "unsupported RPC" { + let response = match request.method.as_str() { + "initialize" => { + json!({"id": request.id, "result": {"userAgent": "daemon-test"}}) + } + "config/read" => { + json!({"id": request.id, "result": {"config": {}, "layers": [{ + "name": {"type": "legacyManagedConfigTomlFromFile"}, "config": + if scenario == "legacy managed" { + json!({"features": {"api_key_model_discovery": true}}) + } else { json!({}) } + }]}}) + } + "configRequirements/read" if scenario == "old requirements server" => { + json!({"id": request.id, "error": {"code": -32600, "message": "Invalid request: unknown variant `configRequirements/read`"}}) + } + "configRequirements/read" => json!({"id": request.id, "result": { + "requirements": if scenario == "managed" { + json!({"featureRequirements": {"api_key_model_discovery": true}}) + } else { json!(null) } + }}), + "experimentalFeature/list" if scenario == "unsupported RPC" => { json!({"id": request.id, "error": {"code": -32601, "message": "method not found"}}) - } else { + } + "experimentalFeature/list" => { + assert_eq!(request.params.unwrap()["threadId"], json!(null)); json!({"id": request.id, "result": {"data": features, "nextCursor": null}}) } + _ => panic!("unexpected request: {}", request.method), }; socket .send(Message::Text(response.to_string().into())) @@ -212,15 +236,19 @@ async fn daemon_feature_compatibility_respects_required_and_optional_attachment( .unwrap(); } }); - let result = daemon_startup::compatibility_warning(&target, &config).await; + let result = + daemon_startup::compatibility_warning(&target, &config, &cli_overrides).await; server.await?; - if scenario == "matching" { + if matches!( + scenario, + "matching" | "stale overrides" | "managed" | "legacy managed" + ) { assert_eq!(result?, None); continue; } let reason = match scenario { - "stale overrides" => "This session requires api_key_model_discovery to be enabled", - "conflicting client" => { + "explicit enable" => "This session requires api_key_model_discovery to be enabled", + "conflicting client" | "old requirements server" => { "This session requires api_key_model_discovery to be disabled" } "unsupported RPC" => "Experimental feature request failed", @@ -242,7 +270,7 @@ async fn daemon_feature_compatibility_respects_required_and_optional_attachment( daemon_startup::FAILURE_HINT ) ); - if scenario == "stale overrides" { + if scenario == "explicit enable" { insta::assert_snapshot!("daemon_feature_mismatch_error", error); } } diff --git a/codex-rs/tui/src/startup_orchestration.rs b/codex-rs/tui/src/startup_orchestration.rs index 87c1c42f54..e09f18a29a 100644 --- a/codex-rs/tui/src/startup_orchestration.rs +++ b/codex-rs/tui/src/startup_orchestration.rs @@ -585,6 +585,7 @@ pub(super) async fn run_main_inner( &mut startup_draft, &app_server_target, &config, + &cli_kv_overrides, managed_daemon, ) .await? diff --git a/codex-rs/tui/tests/suite/daemon_compatibility.rs b/codex-rs/tui/tests/suite/daemon_compatibility.rs index 4ca27c838c..9a0ca72974 100644 --- a/codex-rs/tui/tests/suite/daemon_compatibility.rs +++ b/codex-rs/tui/tests/suite/daemon_compatibility.rs @@ -8,27 +8,16 @@ use anyhow::ensure; use codex_app_server_protocol::JSONRPCMessage; use futures::SinkExt; use futures::StreamExt; -use pretty_assertions::assert_eq; use serde_json::json; use tokio::net::UnixListener; use tokio_tungstenite::tungstenite::Message; #[tokio::test(flavor = "multi_thread", worker_threads = 2)] -async fn incompatible_daemon_falls_back_for_default_and_explicit_features() -> Result<()> { - for scenario in ["default", "explicit", "host policy"] { +async fn incompatible_daemon_falls_back_for_explicit_features() -> Result<()> { + for scenario in ["explicit enable", "explicit disable"] { let cwd = codex_utils_cargo_bin::repo_root()?; let home = tempfile::tempdir_in("/tmp")?; write_test_config(home.path(), &cwd)?; - if scenario == "host policy" { - let path = home.path().join("config.toml"); - let contents = std::fs::read_to_string(&path)?; - std::fs::write( - path, - format!( - "features.code_mode_host = {{enabled=false, disable_in_process_fallback=true}}\n{contents}" - ), - )?; - } let socket = codex_app_server_client::app_server_control_socket_path(home.path())?; std::fs::create_dir_all(socket.parent().unwrap())?; let listener = UnixListener::bind(socket.as_path())?; @@ -43,15 +32,22 @@ async fn incompatible_daemon_falls_back_for_default_and_explicit_features() -> R let JSONRPCMessage::Request(request) = serde_json::from_str(&text)? else { continue; }; - let response = if request.method == "initialize" { - json!({"id": request.id, "result": {"userAgent": "daemon-test/0.0.0"}}) - } else { - assert_eq!(request.method, "experimentalFeature/list"); - json!({"id": request.id, "result": {"data": [{ + let response = match request.method.as_str() { + "initialize" => { + json!({"id": request.id, "result": {"userAgent": "daemon-test/0.0.0"}}) + } + "config/read" => { + json!({"id": request.id, "result": {"config": {}, "layers": []}}) + } + "configRequirements/read" => { + json!({"id": request.id, "result": {"requirements": null}}) + } + "experimentalFeature/list" => json!({"id": request.id, "result": {"data": [{ "name": "api_key_model_discovery", "stage": "stable", "displayName": null, "description": null, "announcement": null, - "enabled": scenario == "explicit", "defaultEnabled": true, - }], "nextCursor": null}}) + "enabled": scenario == "explicit disable", "defaultEnabled": true, + }], "nextCursor": null}}), + _ => panic!("unexpected request: {}", request.method), }; socket .send(Message::Text(response.to_string().into())) @@ -59,25 +55,24 @@ async fn incompatible_daemon_falls_back_for_default_and_explicit_features() -> R } Ok::<_, anyhow::Error>(()) }); - let args = if scenario == "explicit" { + let args = if scenario == "explicit disable" { vec!["-c", "features.api_key_model_discovery=false"] } else { - vec![] + vec!["-c", "features.api_key_model_discovery=true"] }; let mut terminal = PtyCodex::start(&cwd, home, &args)?; terminal.wait_for_startup()?; terminal.wait_for_screen("warning")?; terminal.write_input(b"\x14")?; let (snapshot, warning_end) = match scenario { - "default" => ( + "explicit enable" => ( "daemon_feature_mismatch", "api_key_model_discovery to be enabled.", ), - "explicit" => ( + "explicit disable" => ( "daemon_override_mismatch", "api_key_model_discovery to be disabled.", ), - "host policy" => ("daemon_host_policy_mismatch", "requires embedded mode."), _ => unreachable!(), }; terminal.wait_for_screen(warning_end)?; @@ -94,10 +89,6 @@ async fn incompatible_daemon_falls_back_for_default_and_explicit_features() -> R terminal.write_input(b"\r")?; terminal.wait_for_screen("Model:")?; ensure!(!terminal.screen_contains("unix://")); - if scenario == "host policy" { - server.abort(); - continue; - } tokio::time::timeout(std::time::Duration::from_secs(/*secs*/ 5), server).await???; } Ok(())