mirror of
https://github.com/openai/codex.git
synced 2026-11-10 21:00:47 +00:00
Honor feature overrides when starting the app-server daemon (#60489)
## Why `codex app-server daemon start` ignored CLI feature overrides, while `restart` already parsed and applied them. ## What changed Parse and validate feature overrides for `start` using the same handling as `restart`, including `--enable`, `--disable`, and `-c features.<name>=<bool>`. Pass the overrides through to daemon startup so they are saved when launching a stopped daemon. Starting an already running daemon preserves its process and saved overrides. ## Testing Add coverage for applying overrides on a fresh start and preserving them on a subsequent `start`. Extend validation coverage to ensure both `start` and `restart` reject non-toggle feature keys without changing settings or the running process. GitOrigin-RevId: 9edbfdfc723146674051a9f0713e528b552eeb00
This commit is contained in:
+12
-10
@@ -1329,9 +1329,6 @@ async fn dispatch_cli(
|
||||
}
|
||||
}
|
||||
Some(AppServerSubcommand::Daemon(daemon_cli)) => match daemon_cli.subcommand {
|
||||
AppServerDaemonSubcommand::Start => {
|
||||
print_app_server_daemon_output(AppServerLifecycleCommand::Start).await?;
|
||||
}
|
||||
AppServerDaemonSubcommand::Bootstrap(bootstrap_cli) => {
|
||||
let output =
|
||||
codex_app_server_daemon::bootstrap(AppServerBootstrapOptions {
|
||||
@@ -1340,7 +1337,10 @@ async fn dispatch_cli(
|
||||
.await?;
|
||||
println!("{}", serde_json::to_string(&output)?);
|
||||
}
|
||||
AppServerDaemonSubcommand::Restart => {
|
||||
command @ (AppServerDaemonSubcommand::Start
|
||||
| AppServerDaemonSubcommand::Restart) => {
|
||||
let start = matches!(command, AppServerDaemonSubcommand::Start);
|
||||
let action = if start { "start" } else { "restart" };
|
||||
let overrides = root_config_overrides
|
||||
.parse_overrides()
|
||||
.map_err(anyhow::Error::msg)?;
|
||||
@@ -1369,18 +1369,20 @@ async fn dispatch_cli(
|
||||
let feature =
|
||||
codex_features::feature_for_key(name).ok_or_else(|| {
|
||||
anyhow::anyhow!(
|
||||
"daemon restart requires a boolean feature toggle: {name}"
|
||||
"daemon {action} 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}")
|
||||
anyhow::anyhow!("daemon {action} 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?;
|
||||
let command = if start {
|
||||
AppServerLifecycleCommand::Start { feature_overrides }
|
||||
} else {
|
||||
AppServerLifecycleCommand::Restart { feature_overrides }
|
||||
};
|
||||
print_app_server_daemon_output(command).await?;
|
||||
}
|
||||
AppServerDaemonSubcommand::Update {
|
||||
from_cli: true,
|
||||
|
||||
@@ -122,6 +122,42 @@ fn bootstrap_reports_updater_bind_failure() -> Result<()> {
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn start_applies_feature_overrides_only_when_launching() -> Result<()> {
|
||||
let daemon = TestDaemon::new()?;
|
||||
let settings_path = daemon.home.path().join("app-server-daemon/settings.json");
|
||||
std::fs::write(
|
||||
&settings_path,
|
||||
br#"{"updater":{"autoUpdateEnabled":false}}"#,
|
||||
)?;
|
||||
let mut previous_pid = None;
|
||||
for (prefer_mxc, expected_status) in [(true, "started"), (false, "alreadyRunning")] {
|
||||
let output = daemon
|
||||
.command()
|
||||
.args(["app-server", "daemon", "start", "-c"])
|
||||
.arg(format!("features.prefer_mxc={prefer_mxc}"))
|
||||
.output()?;
|
||||
ensure!(
|
||||
output.status.success(),
|
||||
"{}",
|
||||
String::from_utf8_lossy(&output.stderr)
|
||||
);
|
||||
let output: Value = serde_json::from_slice(&output.stdout)?;
|
||||
assert_eq!(output["status"], expected_status);
|
||||
let settings: Value = serde_json::from_slice(&std::fs::read(&settings_path)?)?;
|
||||
assert_eq!(
|
||||
settings["featureOverrides"],
|
||||
serde_json::json!({"prefer_mxc": true})
|
||||
);
|
||||
let pid = daemon.pid("app-server.pid")?;
|
||||
if let Some(previous_pid) = previous_pid {
|
||||
assert_eq!(pid, previous_pid);
|
||||
}
|
||||
previous_pid = Some(pid);
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn explicit_restart_applies_feature_overrides_and_preserves_saved_settings() -> Result<()> {
|
||||
let daemon = TestDaemon::new()?;
|
||||
@@ -187,23 +223,24 @@ fn explicit_restart_applies_feature_overrides_and_preserves_saved_settings() ->
|
||||
}
|
||||
// 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());
|
||||
let mut errors = String::new();
|
||||
for action in ["start", "restart"] {
|
||||
let output = daemon
|
||||
.command()
|
||||
.args(["app-server", "daemon", action, "--enable", "tool_registry"])
|
||||
.output()?;
|
||||
assert!(!output.status.success());
|
||||
errors.push_str(&String::from_utf8(output.stderr)?);
|
||||
assert_eq!(std::fs::read(&settings_path)?, saved_settings);
|
||||
assert_eq!(Some(u64::from(daemon.pid("app-server.pid")?)), previous_pid);
|
||||
}
|
||||
insta::assert_snapshot!(
|
||||
String::from_utf8(output.stderr)?,
|
||||
@"Error: daemon restart requires a boolean feature toggle: tool_registry"
|
||||
errors,
|
||||
@"
|
||||
Error: daemon start requires a boolean feature toggle: tool_registry
|
||||
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(())
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user