From e762c05bd686dd1eb3595aef2e14cc37c739f401 Mon Sep 17 00:00:00 2001 From: jif Date: Tue, 13 Oct 2026 19:31:05 +0000 Subject: [PATCH] Apply feature overrides when restarting the app-server daemon (#53751) ## Why `codex app-server daemon restart` previously reused saved settings without applying feature flags supplied on the command line. ## What changed Apply `--enable`, `--disable`, and boolean `-c features.*` overrides during an explicit restart and persist them for subsequent restarts. Accept nested `enabled` values, normalize feature aliases, and replace conflicting saved aliases while preserving unrelated settings. Reject invalid feature toggles before stopping the daemon or saving settings. ## Testing Add integration coverage for overrides on stopped and running daemons, alias replacement, preservation of saved settings, restarts without flags, and rejection of non-toggle keys without interrupting the running daemon. GitOrigin-RevId: 14a657bc4758b9c248e363f9dac58f8887070a04 --- codex-rs/Cargo.lock | 1 + codex-rs/app-server-daemon/Cargo.toml | 1 + codex-rs/app-server-daemon/src/launch.rs | 17 +++- codex-rs/app-server-daemon/src/lib.rs | 15 +++- .../src/update_loop_tests.rs | 2 +- codex-rs/cli/src/main.rs | 41 ++++++++- codex-rs/cli/tests/app_server_daemon.rs | 86 +++++++++++++++++++ codex-rs/tui/src/app/event_dispatch.rs | 4 +- 8 files changed, 157 insertions(+), 10 deletions(-) diff --git a/codex-rs/Cargo.lock b/codex-rs/Cargo.lock index 72658b908782..ad3060d74109 100644 --- a/codex-rs/Cargo.lock +++ b/codex-rs/Cargo.lock @@ -2220,6 +2220,7 @@ dependencies = [ "blake3", "codex-app-server-protocol", "codex-app-server-transport", + "codex-features", "codex-http-client", "codex-install-context", "codex-uds", diff --git a/codex-rs/app-server-daemon/Cargo.toml b/codex-rs/app-server-daemon/Cargo.toml index 4dabe9bc273d..053d788348c4 100644 --- a/codex-rs/app-server-daemon/Cargo.toml +++ b/codex-rs/app-server-daemon/Cargo.toml @@ -16,6 +16,7 @@ workspace = true anyhow = { workspace = true } codex-app-server-protocol = { workspace = true } codex-app-server-transport = { workspace = true } +codex-features = { workspace = true } codex-http-client = { workspace = true } codex-install-context = { workspace = true } codex-utils-home-dir = { workspace = true } diff --git a/codex-rs/app-server-daemon/src/launch.rs b/codex-rs/app-server-daemon/src/launch.rs index cd283a3c9739..c36da035faab 100644 --- a/codex-rs/app-server-daemon/src/launch.rs +++ b/codex-rs/app-server-daemon/src/launch.rs @@ -39,9 +39,20 @@ pub async fn restart_with_features(features: &BTreeMap) -> Result< } impl Daemon { - pub(super) async fn restart(&self) -> Result { - self.restart_with_settings(self.load_settings().await?) - .await + pub(super) async fn restart( + &self, + features: &BTreeMap, + ) -> Result { + let mut settings = self.load_settings().await?; + // Explicit requests replace saved aliases as well as canonical keys. + settings.feature_overrides.retain(|name, _| { + let canonical = codex_features::feature_for_key(name) + .map(codex_features::Feature::key) + .unwrap_or(name); + !features.contains_key(canonical) + }); + settings.feature_overrides.extend(features.clone()); + self.restart_with_settings(settings).await } pub(super) async fn restart_with_features_locked( diff --git a/codex-rs/app-server-daemon/src/lib.rs b/codex-rs/app-server-daemon/src/lib.rs index 0fd3ed9c2765..7b19c165676c 100644 --- a/codex-rs/app-server-daemon/src/lib.rs +++ b/codex-rs/app-server-daemon/src/lib.rs @@ -59,10 +59,12 @@ const OPERATION_LOCK_FILE_NAME: &str = "daemon.lock"; const SETTINGS_FILE_NAME: &str = "settings.json"; const STATE_DIR_NAME: &str = "app-server-daemon"; -#[derive(Debug, Clone, Copy, PartialEq, Eq)] +#[derive(Debug, Clone, PartialEq, Eq)] pub enum LifecycleCommand { Start, - Restart, + Restart { + feature_overrides: BTreeMap, + }, Stop, Version, } @@ -228,7 +230,10 @@ enum RestartDecision { pub async fn run(command: LifecycleCommand) -> Result { ensure_supported_platform()?; #[cfg(windows)] - if matches!(command, LifecycleCommand::Start | LifecycleCommand::Restart) { + if matches!( + command, + LifecycleCommand::Start | LifecycleCommand::Restart { .. } + ) { backend::windows::ensure_not_elevated()?; } // Keep daemon package preparation off callers' async stack frames. @@ -394,7 +399,9 @@ impl Daemon { let selected = self.current_installation()?; match command { LifecycleCommand::Start => selected.start(&BTreeMap::new()).await, - LifecycleCommand::Restart => selected.restart().await, + LifecycleCommand::Restart { feature_overrides } => { + selected.restart(&feature_overrides).await + } LifecycleCommand::Stop => { let output = selected.stop().await?; if let Err(err) = thread_recovery::discard_pending(&selected) { diff --git a/codex-rs/app-server-daemon/src/update_loop_tests.rs b/codex-rs/app-server-daemon/src/update_loop_tests.rs index 8ccfc1680347..a7894b81f62a 100644 --- a/codex-rs/app-server-daemon/src/update_loop_tests.rs +++ b/codex-rs/app-server-daemon/src/update_loop_tests.rs @@ -869,7 +869,7 @@ async fn daemon_start_and_restart_preserve_launch_features() { ); assert_eq!(std::fs::read_to_string(&args_path).unwrap(), expected); std::fs::remove_file(&args_path).unwrap(); - let restarted = daemon.restart().await; + let restarted = daemon.restart(&std::collections::BTreeMap::new()).await; let deadline = tokio::time::Instant::now() + Duration::from_secs(/*secs*/ 10); while std::fs::read_to_string(&args_path).ok().as_deref() != Some(expected) && tokio::time::Instant::now() < deadline diff --git a/codex-rs/cli/src/main.rs b/codex-rs/cli/src/main.rs index 9d540c3124ee..c1537bfa00e7 100644 --- a/codex-rs/cli/src/main.rs +++ b/codex-rs/cli/src/main.rs @@ -1308,7 +1308,46 @@ async fn cli_main( println!("{}", serde_json::to_string(&output)?); } AppServerDaemonSubcommand::Restart => { - print_app_server_daemon_output(AppServerLifecycleCommand::Restart).await?; + let overrides = root_config_overrides + .parse_overrides() + .map_err(anyhow::Error::msg)?; + let layer = codex_config::build_cli_overrides_layer(&overrides); + let features = layer + .get("features") + .and_then(toml::Value::as_table) + .into_iter() + .flatten() + .map(|(name, value)| (name.as_str(), value)) + .collect::>(); + let mut feature_overrides = std::collections::BTreeMap::new(); + // Match Features::apply_map's alias precedence. + for (&name, &value) in &features { + if name == "imagegenext" && features.contains_key("image_generation") { + continue; + } + let value = if let Some(table) = value.as_table() { + let Some(enabled) = table.get("enabled") else { + continue; + }; + enabled + } else { + value + }; + let feature = + codex_features::feature_for_key(name).ok_or_else(|| { + anyhow::anyhow!( + "daemon restart requires a boolean feature toggle: {name}" + ) + })?; + let enabled = value.as_bool().ok_or_else(|| { + anyhow::anyhow!("daemon restart requires a boolean enablement for features.{name}") + })?; + feature_overrides.insert(feature.key().to_string(), enabled); + } + print_app_server_daemon_output(AppServerLifecycleCommand::Restart { + feature_overrides, + }) + .await?; } AppServerDaemonSubcommand::Update { from_cli: true, diff --git a/codex-rs/cli/tests/app_server_daemon.rs b/codex-rs/cli/tests/app_server_daemon.rs index f36f03ad24cc..e05bedffe7c6 100644 --- a/codex-rs/cli/tests/app_server_daemon.rs +++ b/codex-rs/cli/tests/app_server_daemon.rs @@ -171,6 +171,92 @@ fn managed_daemon_restarts_from_deleted_working_directory() -> Result<()> { Ok(()) } +#[test] +fn explicit_restart_applies_feature_overrides_and_preserves_saved_settings() -> Result<()> { + let daemon = TestDaemon::new()?; + let settings_path = daemon.home.path().join("app-server-daemon/settings.json"); + std::fs::write( + &settings_path, + serde_json::to_vec(&serde_json::json!({ + "shutdownGraceSeconds": 0, + "updater": { "autoUpdateEnabled": false }, + "featureOverrides": { + "api_key_model_discovery": false, + "auth_elicitation": true, + "connectors": true, + "apps": true + } + }))?, + )?; + let mut previous_pid = None; + // Start from a stopped daemon, change running settings, and restart again + // with omitted flags. Every explicit restart replaces the process. + for (args, discovery) in [ + ( + vec![ + "--enable", + "api_key_model_discovery", + "--disable", + "connectors", + "-c", + "features.network_proxy.enabled=false", + ], + true, + ), + (vec!["--disable", "api_key_model_discovery"], false), + (vec!["-c", "features.api_key_model_discovery=true"], true), + (vec![], true), + ] { + let output = daemon + .command() + .args(["app-server", "daemon", "restart"]) + .args(args) + .output()?; + ensure!( + output.status.success(), + "{}", + String::from_utf8_lossy(&output.stderr) + ); + let output: Value = serde_json::from_slice(&output.stdout)?; + assert_eq!(output["status"], "restarted"); + let pid = output["pid"].as_u64().context("restarted daemon PID")?; + assert_ne!(Some(pid), previous_pid); + previous_pid = Some(pid); + let settings: Value = serde_json::from_slice(&std::fs::read(&settings_path)?)?; + assert_eq!( + settings["featureOverrides"], + serde_json::json!({ + "api_key_model_discovery": discovery, + "auth_elicitation": true, + "apps": false, + "network_proxy": false + }) + ); + assert_eq!(settings["updater"]["autoUpdateEnabled"], false); + } + // Non-toggle config keys must fail before stopping the daemon or saving settings. + let saved_settings = std::fs::read(&settings_path)?; + let output = daemon + .command() + .args([ + "app-server", + "daemon", + "restart", + "--enable", + "tool_registry", + ]) + .output()?; + assert!(!output.status.success()); + insta::assert_snapshot!( + String::from_utf8(output.stderr)?, + @"Error: daemon restart requires a boolean feature toggle: tool_registry" + ); + assert_eq!(std::fs::read(&settings_path)?, saved_settings); + assert_eq!(Some(u64::from(daemon.pid("app-server.pid")?)), previous_pid); + assert_eq!(daemon.lifecycle("version")?["status"], "running"); + Ok(()) +} + fn signal(pid: u32, signal: libc::c_int) -> Result<()> { let raw_pid = libc::pid_t::try_from(pid).context("pid out of range")?; ensure!( diff --git a/codex-rs/tui/src/app/event_dispatch.rs b/codex-rs/tui/src/app/event_dispatch.rs index a7f7f1e82781..e5bd89312b2c 100644 --- a/codex-rs/tui/src/app/event_dispatch.rs +++ b/codex-rs/tui/src/app/event_dispatch.rs @@ -166,7 +166,9 @@ impl App { && self.daemon_restart.is_none() { self.daemon_restart = Some(tokio::spawn(codex_app_server_daemon::run( - codex_app_server_daemon::LifecycleCommand::Restart, + codex_app_server_daemon::LifecycleCommand::Restart { + feature_overrides: Default::default(), + }, ))); } }