mirror of
https://github.com/openai/codex.git
synced 2026-10-20 17:38:45 +00:00
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
This commit is contained in:
@@ -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::<std::collections::BTreeMap<_, _>>();
|
||||
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,
|
||||
|
||||
@@ -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!(
|
||||
|
||||
Reference in New Issue
Block a user