From fc269b66adc37f3c855df222ad80b02733355c46 Mon Sep 17 00:00:00 2001 From: Eric Traut Date: Tue, 15 Sep 2026 02:23:02 +0000 Subject: [PATCH] Add explicit daemon package replacement from the CLI (#45580) ## What changed Add `codex app-server daemon update --from-cli` to copy and pin the invoking CLI's complete package, including downgrades and local builds. Require interactive confirmation or `--yes`, and warn that restarting a running daemon may interrupt work. Validate and stage the package before stopping the daemon, recheck the selection and running state after confirmation, and restart only a previously running daemon. Migrate legacy daemon installations to the dedicated package directory while preserving the standalone CLI selection. Explicit replacements disable automatic package updates. ## Testing Add coverage for cancellation, packages changed during confirmation, version pinning, and legacy migration. Extend CLI integration tests to verify noninteractive confirmation requirements, rejection of incomplete packages without stopping the daemon, and successful replacement and restart. GitOrigin-RevId: 2d971d6f0259aa576a15a8b115ca9d0e89060fdf --- codex-rs/.config/nextest.toml | 5 + codex-rs/app-server-daemon/src/lib.rs | 2 + .../app-server-daemon/src/managed_install.rs | 17 +- .../app-server-daemon/src/prepare_install.rs | 264 +++++++++++++++--- .../src/prepare_install_tests.rs | 174 +++++++++++- .../src/prepare_install_windows.rs | 12 +- codex-rs/cli/src/daemon_install.rs | 42 +++ codex-rs/cli/src/daemon_install_tests.rs | 18 ++ codex-rs/cli/src/main.rs | 35 ++- codex-rs/cli/tests/app_server_daemon.rs | 135 +++++++-- 10 files changed, 625 insertions(+), 79 deletions(-) create mode 100644 codex-rs/cli/src/daemon_install.rs create mode 100644 codex-rs/cli/src/daemon_install_tests.rs diff --git a/codex-rs/.config/nextest.toml b/codex-rs/.config/nextest.toml index 1f73dad8e6..8a095942dd 100644 --- a/codex-rs/.config/nextest.toml +++ b/codex-rs/.config/nextest.toml @@ -5,6 +5,11 @@ slow-timeout = { period = "30s", terminate-after = 2 } retries = 1 +[[profile.default.overrides]] +# This case validates and copies both the initial and replacement debug package. +filter = 'package(codex-cli) & binary(app_server_daemon) & test(=packaged_daemon_start_and_explicit_replacement)' +slow-timeout = { period = "1m", terminate-after = 3 } + [[profile.default.overrides]] # These cases copy a full debug CLI package and launch its new executable. filter = 'package(codex-cli) & binary(app_server_daemon) & test(packaged_daemon_)' diff --git a/codex-rs/app-server-daemon/src/lib.rs b/codex-rs/app-server-daemon/src/lib.rs index 6420d93715..fdfed6eaa8 100644 --- a/codex-rs/app-server-daemon/src/lib.rs +++ b/codex-rs/app-server-daemon/src/lib.rs @@ -7,6 +7,8 @@ mod client; mod install_lock; mod managed_install; mod prepare_install; +pub use prepare_install::InstallRequest; +pub use prepare_install::update_from_cli; mod remote_control_client; mod settings; mod thread_recovery; diff --git a/codex-rs/app-server-daemon/src/managed_install.rs b/codex-rs/app-server-daemon/src/managed_install.rs index 76d9f016d1..362cbf5795 100644 --- a/codex-rs/app-server-daemon/src/managed_install.rs +++ b/codex-rs/app-server-daemon/src/managed_install.rs @@ -153,12 +153,17 @@ pub(crate) async fn managed_codex_version(codex_bin: &Path) -> Result { let mut command = Command::new(codex_bin); #[cfg(windows)] command.creation_flags(windows_sys::Win32::System::Threading::CREATE_NO_WINDOW); - let output = command.arg("--version").output().await.with_context(|| { - format!( - "failed to invoke managed Codex binary {}", - codex_bin.display() - ) - })?; + let output = command + .arg("--version") + .kill_on_drop(true) + .output() + .await + .with_context(|| { + format!( + "failed to invoke managed Codex binary {}", + codex_bin.display() + ) + })?; if !output.status.success() { return Err(anyhow!( "managed Codex binary {} exited with status {}", diff --git a/codex-rs/app-server-daemon/src/prepare_install.rs b/codex-rs/app-server-daemon/src/prepare_install.rs index cd9ad8d97f..23325ae827 100644 --- a/codex-rs/app-server-daemon/src/prepare_install.rs +++ b/codex-rs/app-server-daemon/src/prepare_install.rs @@ -1,7 +1,8 @@ -//! Prepares complete local CLI packages for a stopped daemon. Legacy standalone -//! installations are left to their installer; new releases are immutable. +//! Installs complete CLI packages after confirmation. Initial starts preserve +//! existing selections; explicit replacements migrate to dedicated packages and stay pinned. use std::path::Path; +use std::path::PathBuf; use anyhow::Context; use anyhow::Result; @@ -14,68 +15,134 @@ use crate::install_lock::acquire_install_lock; use crate::managed_install; use crate::settings::DaemonSettings; +/// The complete CLI package offered for a daemon installation. +pub struct InstallRequest { + pub source: PathBuf, + pub version: String, + pub destination: PathBuf, + pub installed_version: Option, + pub restart_required: bool, +} + /// Prepare a missing package while the caller holds the daemon operation lock. pub(super) async fn prepare(daemon: &Daemon, settings: &DaemonSettings) -> Result<()> { let source = InstallContext::current().package_layout.as_ref(); - prepare_from_package( + // Keep package replacement state out of the CLI dispatcher's async stack frame. + Box::pin(prepare_from_package( daemon, settings, + InstallMode::Missing, source.map(|layout| layout.package_dir.as_path()), &std::env::current_exe()?, - ) + |_| Ok(true), + )) .await + .map(|_| ()) +} + +/// Select this CLI's complete package and pin it, restarting only a running daemon. +/// Returns None when the user cancels without changing the installation. +pub async fn update_from_cli( + confirm: impl FnOnce(&InstallRequest) -> Result, +) -> Result> { + crate::ensure_supported_platform()?; + #[cfg(windows)] + crate::backend::windows::ensure_not_elevated()?; + let daemon = Daemon::from_environment()?; + let settings = daemon.load_settings().await?; + let source = InstallContext::current().package_layout.as_ref(); + if !Box::pin(prepare_from_package( + &daemon, + &settings, + InstallMode::Replace, + source.map(|layout| layout.package_dir.as_path()), + &std::env::current_exe()?, + confirm, + )) + .await? + { + return Ok(None); + } + let managed_codex_path = daemon.current_managed_codex_bin()?; + Ok(Some(crate::UpdateOutput { + status: crate::UpdateStatus::Updated, + installed_version: Some(managed_install::managed_codex_version(&managed_codex_path).await?), + running_version: crate::client::probe(&daemon.socket_path) + .await + .ok() + .map(|info| info.app_server_version), + managed_codex_path, + message: "The CLI package is selected and pinned.".to_string(), + })) +} + +#[derive(Clone, Copy, PartialEq, Eq)] +enum InstallMode { + Missing, + Replace, } async fn prepare_from_package( daemon: &Daemon, settings: &DaemonSettings, + mode: InstallMode, source: Option<&Path>, running_exe: &Path, -) -> Result<()> { + confirm: impl FnOnce(&InstallRequest) -> Result, +) -> Result { let home = daemon .settings_file .parent() .and_then(Path::parent) .context("daemon settings path has no Codex home")?; - let root = managed_install::package_root(home); + let previous_root = managed_install::package_root(home); + let root = home.join("packages/app-server-daemon"); anyhow::ensure!( - daemon.managed_codex_bin.starts_with(&root), + daemon.managed_codex_bin.starts_with(&previous_root), "daemon package location changed; retry the command" ); - if !root.ends_with("app-server-daemon") - || daemon.running_backend_instance(settings).await?.is_some() - || crate::client::probe(&daemon.socket_path).await.is_ok() - { - return Ok(()); - } - if !matches!(root.join("current").symlink_metadata(), Err(error) if error.kind() == std::io::ErrorKind::NotFound) - || ["daemon.pid", "daemon.stderr.log", "daemon-updater.pid", "daemon-updater.stderr.log"] - .iter().any(|name| !matches!(home.join("app-server-daemon").join(name).symlink_metadata(), Err(error) if error.kind() == std::io::ErrorKind::NotFound)) - { - daemon.ensure_managed_codex_bin()?; - return Ok(()); + if mode == InstallMode::Missing { + if previous_root != root + || daemon.running_backend_instance(settings).await?.is_some() + || crate::client::probe(&daemon.socket_path).await.is_ok() + { + return Ok(true); + } + if !matches!(root.join("current").symlink_metadata(), Err(error) if error.kind() == std::io::ErrorKind::NotFound) + || ["daemon.pid", "daemon.stderr.log", "daemon-updater.pid", "daemon-updater.stderr.log"] + .iter().any(|name| !matches!(home.join("app-server-daemon").join(name).symlink_metadata(), Err(error) if error.kind() == std::io::ErrorKind::NotFound)) + { + daemon.ensure_managed_codex_bin()?; + return Ok(true); + } + } else { + anyhow::ensure!( + previous_root.join("current").symlink_metadata().is_ok(), + "no daemon package is selected; run `codex app-server daemon start` first" + ); } std::fs::create_dir_all(&root)?; - let _install_lock = acquire_install_lock(&root).await?; - // An older CLI may have launched a legacy daemon while this CLI waited. anyhow::ensure!( - managed_install::package_root(home) == root, + managed_install::package_root(home) == previous_root, "daemon package location changed; retry the command" ); + let backend = daemon.running_backend_instance(settings).await?; anyhow::ensure!( - daemon.running_backend_instance(settings).await?.is_none() - && crate::client::probe(&daemon.socket_path).await.is_err(), - "an app server started while preparing the daemon; retry the command" + backend.is_some() || crate::client::probe(&daemon.socket_path).await.is_err(), + "app server is running but is not managed by codex app-server daemon" ); let selected = managed_install::managed_codex_bin(home); - let missing = !selected.is_file(); - if !missing { - return Ok(()); + let previous_release = previous_root.join("current").canonicalize().ok(); + if mode == InstallMode::Missing { + if selected.is_file() { + return Ok(true); + } + anyhow::ensure!( + backend.is_none() + && matches!(root.join("current").symlink_metadata(), Err(error) if error.kind() == std::io::ErrorKind::NotFound), + "the selected daemon package is incomplete; repair its installation before starting" + ); } - anyhow::ensure!( - matches!(root.join("current").symlink_metadata(), Err(error) if error.kind() == std::io::ErrorKind::NotFound), - "the selected daemon package is incomplete; repair its installation before starting" - ); let source = source.context( "this CLI has no complete local package; install a packaged Codex CLI or use the standalone installer", )?; @@ -98,12 +165,62 @@ async fn prepare_from_package( "the CLI package does not match this platform or executable" ); validate_package(source)?; - eprintln!( - "Installing daemon from CLI version {version} into {}...", - root.display() - ); - let stable = stable_version(&version).is_some(); + // A local build may replace the executable while confirmation is pending. let running_identity = managed_install::executable_identity(running_exe).await?; + if mode == InstallMode::Replace { + if !confirm(&InstallRequest { + source: source.to_path_buf(), + version: version.clone(), + destination: root.clone(), + installed_version: tokio::time::timeout( + std::time::Duration::from_secs(5), + managed_install::managed_codex_version(&selected), + ) + .await + .ok() + .and_then(Result::ok), + restart_required: backend.is_some(), + })? { + return Ok(false); + } + } else { + eprintln!( + "Installing daemon from CLI version {version} into {}...", + root.display() + ); + } + // Confirmation must not block lifecycle commands. Recheck the approved + // selection and running state once this operation owns both locks. + let _operation_lock = if mode == InstallMode::Replace { + Some(daemon.acquire_operation_lock().await?) + } else { + None // Initial startup already holds the operation lock. + }; + let current_settings = if mode == InstallMode::Replace { + Some(daemon.load_settings().await?) + } else { + None + }; + let settings = current_settings.as_ref().unwrap_or(settings); + let _install_lock = acquire_install_lock(&root).await?; + anyhow::ensure!( + managed_install::package_root(home) == previous_root, + "daemon package location changed; retry the command" + ); + if mode == InstallMode::Missing && managed_install::managed_codex_bin(home).is_file() { + return Ok(true); + } + anyhow::ensure!( + previous_root.join("current").canonicalize().ok() == previous_release, + "daemon selection changed while awaiting confirmation; retry the command" + ); + let current_backend = daemon.running_backend_instance(settings).await?; + anyhow::ensure!( + current_backend.is_some() == backend.is_some(), + "daemon running state changed while awaiting confirmation; retry the command" + ); + let backend = current_backend; + let stable = stable_version(&version).is_some(); let releases = root.join("releases"); std::fs::create_dir_all(&releases)?; let stage = tempfile::Builder::new() @@ -128,7 +245,7 @@ async fn prepare_from_package( !stable || version == binary_version, "the CLI package version does not match its executable" ); - let name = if stable { + let name = if stable && mode == InstallMode::Missing { format!("{version}-{target}") } else { format!("local-{digest}-{target}") @@ -149,7 +266,8 @@ async fn prepare_from_package( } let standalone = home.join("packages/standalone"); let canonical_source = source.canonicalize()?; - let follows_latest = stable + let follows_latest = mode == InstallMode::Missing + && stable && (standalone.join("current").canonicalize().ok().as_deref() != Some(canonical_source.as_path()) || std::fs::read_to_string(standalone.join("auto-update-version")) @@ -157,11 +275,55 @@ async fn prepare_from_package( .as_deref() == canonical_source.file_name().and_then(|name| name.to_str())); anyhow::ensure!( - managed_install::package_root(home) == root - && daemon.running_backend_instance(settings).await?.is_none() - && crate::client::probe(&daemon.socket_path).await.is_err(), - "daemon state changed while preparing its package; retry the command" + managed_install::package_root(home) == previous_root + && (backend.is_some() || crate::client::probe(&daemon.socket_path).await.is_err()), + "daemon package location or socket ownership changed while preparing its package; retry the command" ); + #[cfg(windows)] + if backend.is_some() { + crate::backend::windows::ensure_detached_launch(&release.join(entrypoint))?; + } + #[cfg(windows)] + windows::validate_selection(&root)?; + if mode == InstallMode::Replace { + let stopped = async { + crate::backend::pid_update_loop_backend(daemon.backend_paths(settings)) + .stop() + .await?; + anyhow::ensure!( + managed_install::package_root(home) == previous_root + && previous_root.join("current").canonicalize().ok() == previous_release, + "daemon selection changed while preparing its package; retry the command" + ); + if let Some(backend) = &backend { + if let Err(error) = crate::thread_recovery::discard_pending(daemon) { + eprintln!( + "warning: failed to clear stale daemon recovery before replacement: {error}" + ); + } + backend + .stop_with_grace(settings.shutdown_grace_seconds) + .await?; + } + Ok::<(), anyhow::Error>(()) + } + .await; + if let Err(error) = stopped { + if let Err(restore_error) = async { + daemon + .current_installation()? + .ensure_managed_updater(settings) + .await + } + .await + { + eprintln!( + "warning: failed to restore the daemon updater after replacement failed: {restore_error:#}" + ); + } + return Err(error); + } + } let marker = root.join("auto-update-version"); if follows_latest { let temporary = tempfile::NamedTempFile::new_in(&root)?; @@ -179,7 +341,21 @@ async fn prepare_from_package( } #[cfg(windows)] windows::select_release(&root, &release)?; - Ok(()) + if backend.is_some() { + let selected = Daemon { + pid_file: daemon.pid_file.with_file_name(crate::DAEMON_PID_FILE_NAME), + update_pid_file: daemon + .update_pid_file + .with_file_name(crate::DAEMON_UPDATE_PID_FILE_NAME), + managed_codex_bin: root.join("current").join(entrypoint), + ..daemon.clone() + }; + selected.start_managed_backend(settings).await.context( + "daemon package selected but could not start; retry with `codex app-server daemon start`", + )?; + selected.wait_until_ready().await?; + } + Ok(true) } /// Hash the complete tree and optionally copy those same bytes. Relative file diff --git a/codex-rs/app-server-daemon/src/prepare_install_tests.rs b/codex-rs/app-server-daemon/src/prepare_install_tests.rs index 99801a0856..45cfee41f7 100644 --- a/codex-rs/app-server-daemon/src/prepare_install_tests.rs +++ b/codex-rs/app-server-daemon/src/prepare_install_tests.rs @@ -1,5 +1,6 @@ //! Covers complete-package staging, conservative selection, and legacy preservation. #![cfg(unix)] +use super::InstallMode; use super::prepare_from_package; use super::validate_package; use crate::settings::DaemonSettings; @@ -67,9 +68,16 @@ async fn seeds_full_package() { let settings = DaemonSettings::default(); let old = temp.path().join("old"); let old_bin = package(&old, "0.152.0"); - prepare_from_package(&daemon, &settings, Some(&old), &old_bin) - .await - .expect("seed"); + prepare_from_package( + &daemon, + &settings, + InstallMode::Missing, + Some(&old), + &old_bin, + |_| Ok(true), + ) + .await + .expect("seed"); let standalone = home.join("packages/app-server-daemon"); let selected = std::fs::canonicalize(standalone.join("current")).expect("selected"); @@ -94,8 +102,10 @@ async fn incomplete_source_fails_without_selecting_it() { let error = prepare_from_package( &daemon(&home), &DaemonSettings::default(), + InstallMode::Missing, Some(&source), &bin, + |_| Ok(true), ) .await .expect_err("incomplete package"); @@ -117,8 +127,10 @@ async fn provisioned_macos_bundle_seeds_from_its_running_executable() { prepare_from_package( &daemon(&home), &DaemonSettings::default(), + InstallMode::Missing, Some(&source), &bundle, + |_| Ok(true), ) .await .expect("seed provisioned bundle"); @@ -151,8 +163,10 @@ async fn legacy_selection_is_not_migrated_or_refreshed() { prepare_from_package( &daemon(&home), &DaemonSettings::default(), + InstallMode::Missing, Some(&source), &new_bin, + |_| Ok(true), ) .await .unwrap(); @@ -184,8 +198,10 @@ async fn standalone_seed_preserves_explicit_pin_or_latest_channel() { prepare_from_package( &daemon(&home), &DaemonSettings::default(), + InstallMode::Missing, Some(&source), &bin, + |_| Ok(true), ) .await .unwrap(); @@ -198,6 +214,145 @@ async fn standalone_seed_preserves_explicit_pin_or_latest_channel() { } } +#[tokio::test] +async fn explicit_selection_requires_unchanged_cli_and_pins_all_versions() { + for legacy in [false, true] { + let temp = tempfile::TempDir::new().unwrap(); + let home = temp.path().join("home"); + let source = temp.path().join("source"); + let bin = package(&source, "0.152.0"); + let settings = DaemonSettings::default(); + let initial = daemon(&home); + prepare_from_package( + &initial, + &settings, + InstallMode::Missing, + Some(&source), + &bin, + |_| Ok(true), + ) + .await + .unwrap(); + if legacy { + std::fs::rename( + home.join("packages/app-server-daemon"), + home.join("packages/standalone"), + ) + .unwrap(); + let root = home.join("packages/standalone"); + let name = std::fs::read_to_string(root.join("auto-update-version")).unwrap(); + std::fs::remove_file(root.join("current")).unwrap(); + std::os::unix::fs::symlink(root.join("releases").join(name), root.join("current")) + .unwrap(); + let state = home.join("app-server-daemon"); + std::fs::create_dir_all(&state).unwrap(); + std::fs::write(state.join("app-server.stderr.log"), b"").unwrap(); + } + let initial_root = crate::managed_install::package_root(&home); + let legacy_release = initial_root.join("current").canonicalize().unwrap(); + let legacy_marker = std::fs::read(initial_root.join("auto-update-version")).unwrap(); + let error = prepare_from_package( + &daemon(&home), + &settings, + InstallMode::Replace, + Some(&source), + &bin, + |_| { + let mut rebuilt = std::fs::read(&bin)?; + rebuilt.extend_from_slice(b"# rebuilt during confirmation\n"); + std::fs::write(&bin, rebuilt)?; + Ok(true) + }, + ) + .await + .expect_err("reject a CLI rebuilt during confirmation"); + assert!( + error + .to_string() + .contains("differs from the running executable") + ); + assert_eq!( + initial_root.join("current").canonicalize().unwrap(), + legacy_release + ); + assert_eq!( + std::fs::read(initial_root.join("auto-update-version")).unwrap(), + legacy_marker + ); + let root = home.join("packages/app-server-daemon"); + let mut previous = legacy_release.clone(); + for version in [ + "0.153.0", + "0.151.0", + "0.151.0", + "0.154.0-alpha.1", + "0.0.0", + "0.0.0", + ] { + let daemon = daemon(&home); + let previous_root = crate::managed_install::package_root(&home); + package(&source, version); + std::fs::write( + source.join("codex-resources/nested/runtime"), + previous.to_string_lossy().as_bytes(), + ) + .unwrap(); + let before = std::fs::read(previous.join("bin/codex")).unwrap(); + assert!( + !prepare_from_package( + &daemon, + &settings, + InstallMode::Replace, + Some(&source), + &bin, + |request| { + assert_eq!(request.destination, root); + Ok(false) + } + ) + .await + .unwrap() + ); + assert_eq!(crate::managed_install::package_root(&home), previous_root); + assert_eq!( + previous_root.join("current").canonicalize().unwrap(), + previous + ); + prepare_from_package( + &daemon, + &settings, + InstallMode::Replace, + Some(&source), + &bin, + |_| Ok(true), + ) + .await + .unwrap(); + let selected = root.join("current").canonicalize().unwrap(); + assert_eq!(crate::managed_install::package_root(&home), root); + if legacy { + assert_eq!( + initial_root.join("current").canonicalize().unwrap(), + legacy_release + ); + assert_eq!( + std::fs::read(initial_root.join("auto-update-version")).unwrap(), + legacy_marker + ); + } + assert_ne!(selected, previous); + assert_eq!( + std::fs::read(selected.join("bin/codex")).unwrap(), + std::fs::read(&bin).unwrap() + ); + assert_eq!(std::fs::read(previous.join("bin/codex")).unwrap(), before); + assert!(!root.join("auto-update-version").exists()); + assert!(daemon.running_backend(&settings).await.unwrap().is_none()); + previous = selected; + } + } +} + #[tokio::test] async fn broken_selection_is_not_a_missing_installation() { let temp = tempfile::TempDir::new().unwrap(); @@ -208,9 +363,16 @@ async fn broken_selection_is_not_a_missing_installation() { let current = home.join("packages/app-server-daemon/current"); std::fs::create_dir_all(current.parent().unwrap()).unwrap(); std::os::unix::fs::symlink("missing-release", ¤t).unwrap(); - let error = prepare_from_package(&daemon, &DaemonSettings::default(), Some(&source), &bin) - .await - .unwrap_err(); + let error = prepare_from_package( + &daemon, + &DaemonSettings::default(), + InstallMode::Missing, + Some(&source), + &bin, + |_| panic!("broken installation must not request replacement"), + ) + .await + .unwrap_err(); assert!( error .to_string() diff --git a/codex-rs/app-server-daemon/src/prepare_install_windows.rs b/codex-rs/app-server-daemon/src/prepare_install_windows.rs index 97622bb942..5b74d0f56b 100644 --- a/codex-rs/app-server-daemon/src/prepare_install_windows.rs +++ b/codex-rs/app-server-daemon/src/prepare_install_windows.rs @@ -25,11 +25,21 @@ pub(super) fn select_release(root: &Path, release: &Path) -> Result<()> { std::fs::rename(junction, ¤t)?; return Ok(()); } + validate_selection(root)?; + retarget_junction(¤t, &release) +} + +pub(super) fn validate_selection(root: &Path) -> Result<()> { + let current = root.join("current"); + if matches!(current.symlink_metadata(), Err(error) if error.kind() == std::io::ErrorKind::NotFound) + { + return Ok(()); + } anyhow::ensure!( current.canonicalize()?.parent() == Some(root.join("releases").canonicalize()?.as_path()), "refusing to replace a daemon selection outside its releases directory" ); - retarget_junction(¤t, &release) + Ok(()) } fn retarget_junction(current: &Path, release: &Path) -> Result<()> { diff --git a/codex-rs/cli/src/daemon_install.rs b/codex-rs/cli/src/daemon_install.rs new file mode 100644 index 0000000000..dad14777a0 --- /dev/null +++ b/codex-rs/cli/src/daemon_install.rs @@ -0,0 +1,42 @@ +//! Confirmation for copying the invoking CLI package into a daemon installation. + +use std::io::IsTerminal; + +use anyhow::Result; +use codex_app_server_daemon::InstallRequest; + +pub(crate) fn confirm_install(request: &InstallRequest, yes: bool) -> Result { + eprintln!("{}", describe_install(request)); + if yes { + return Ok(true); + } + anyhow::ensure!( + std::io::stdin().is_terminal() && std::io::stderr().is_terminal(), + "daemon installation requires confirmation; rerun with --yes to copy this CLI package" + ); + let confirmed = crate::confirm("Copy this package? [y/N]: ")?; + if !confirmed { + eprintln!("Daemon installation cancelled."); + } + Ok(confirmed) +} + +fn describe_install(request: &InstallRequest) -> String { + let mut message = format!( + "Replace installed daemon version {} with CLI version {} from {}.\nThe daemon package will be installed in {}.\nThe selected package will be pinned.", + request.installed_version.as_deref().unwrap_or("unknown"), + request.version, + request.source.display(), + request.destination.display() + ); + if request.restart_required { + message.push_str( + "\nThe running daemon will restart; active or queued work may be interrupted.", + ); + } + message +} + +#[cfg(test)] +#[path = "daemon_install_tests.rs"] +mod tests; diff --git a/codex-rs/cli/src/daemon_install_tests.rs b/codex-rs/cli/src/daemon_install_tests.rs new file mode 100644 index 0000000000..c48c85cb1b --- /dev/null +++ b/codex-rs/cli/src/daemon_install_tests.rs @@ -0,0 +1,18 @@ +use super::*; + +#[test] +fn running_daemon_replacement_prompt() { + let request = InstallRequest { + source: "/cli/package".into(), + version: "0.0.0".into(), + destination: "/home/packages/app-server-daemon".into(), + installed_version: Some("0.152.0".into()), + restart_required: true, + }; + insta::assert_snapshot!(describe_install(&request), @r" + Replace installed daemon version 0.152.0 with CLI version 0.0.0 from /cli/package. + The daemon package will be installed in /home/packages/app-server-daemon. + The selected package will be pinned. + The running daemon will restart; active or queued work may be interrupted. + "); +} diff --git a/codex-rs/cli/src/main.rs b/codex-rs/cli/src/main.rs index 68de6ec0d8..4789190142 100644 --- a/codex-rs/cli/src/main.rs +++ b/codex-rs/cli/src/main.rs @@ -57,6 +57,7 @@ static ALLOCATOR: tikv_jemallocator::Jemalloc = tikv_jemallocator::Jemalloc; #[cfg(any(target_os = "macos", target_os = "windows"))] mod app_cmd; mod cloud_config; +mod daemon_install; #[cfg(any(target_os = "macos", target_os = "windows"))] mod desktop_app; mod doctor; @@ -792,8 +793,15 @@ enum AppServerDaemonSubcommand { /// Restart the local app server daemon. Restart, - /// Update the standalone installation and restart the managed daemon (may interrupt work). - Update, + /// Update the daemon package (may interrupt running work). + Update { + /// Copy and pin this CLI package. + #[arg(long)] + from_cli: bool, + /// Confirm replacing the daemon package without an interactive prompt. + #[arg(short = 'y', long, requires = "from_cli")] + yes: bool, + }, /// Enable remote control for future starts and a currently running managed daemon. EnableRemoteControl, @@ -1396,6 +1404,18 @@ async fn cli_main( AppServerDaemonSubcommand::Restart => { print_app_server_daemon_output(AppServerLifecycleCommand::Restart).await?; } + AppServerDaemonSubcommand::Update { + from_cli: true, + yes, + } => { + if let Some(output) = codex_app_server_daemon::update_from_cli(|request| { + daemon_install::confirm_install(request, yes) + }) + .await? + { + println!("{}", serde_json::to_string(&output)?); + } + } AppServerDaemonSubcommand::EnableRemoteControl => { print_app_server_remote_control_output(AppServerRemoteControlMode::Enabled) .await?; @@ -1415,7 +1435,9 @@ async fn cli_main( AppServerDaemonSubcommand::PidUpdateLoop { check_package_ownership: true, } => return Ok(()), - AppServerDaemonSubcommand::Update + AppServerDaemonSubcommand::Update { + from_cli: false, .. + } | AppServerDaemonSubcommand::PidUpdateLoop { check_package_ownership: false, } => { @@ -1428,7 +1450,10 @@ async fn cli_main( .await .map_err(anyhow::Error::from); let http_client_factory = updater_http_client_factory(config); - if matches!(daemon_cli.subcommand, AppServerDaemonSubcommand::Update) { + if matches!( + daemon_cli.subcommand, + AppServerDaemonSubcommand::Update { .. } + ) { let output = codex_app_server_daemon::update(http_client_factory).await?; println!("{}", serde_json::to_string(&output)?); @@ -2647,7 +2672,7 @@ fn app_server_subcommand_name(subcommand: Option<&AppServerSubcommand>) -> &'sta AppServerDaemonSubcommand::Bootstrap(_) => "app-server daemon bootstrap", AppServerDaemonSubcommand::Start => "app-server daemon start", AppServerDaemonSubcommand::Restart => "app-server daemon restart", - AppServerDaemonSubcommand::Update => "app-server daemon update", + AppServerDaemonSubcommand::Update { .. } => "app-server daemon update", AppServerDaemonSubcommand::EnableRemoteControl => { "app-server daemon enable-remote-control" } diff --git a/codex-rs/cli/tests/app_server_daemon.rs b/codex-rs/cli/tests/app_server_daemon.rs index 4a9b05dda3..fb923a82bc 100644 --- a/codex-rs/cli/tests/app_server_daemon.rs +++ b/codex-rs/cli/tests/app_server_daemon.rs @@ -324,11 +324,17 @@ fn manual_update_rejects_an_unowned_installation() -> Result<()> { Ok(()) } -fn packaged_daemon_launch(action: &str) -> Result<()> { +#[derive(Clone, Copy, PartialEq, Eq)] +enum InitialDaemon { + Missing, + Legacy, +} + +fn packaged_daemon_launch(action: &str, initial: InitialDaemon) -> Result<()> { use std::os::unix::fs::PermissionsExt; let mut daemon = TestDaemon::new()?; let standalone = daemon.home.path().join("packages/standalone"); - let package = if action == "start" { + let package = if action == "start" && initial == InitialDaemon::Missing { standalone.join("releases/caller") } else { daemon.home.path().join("cli-package") @@ -363,13 +369,15 @@ fn packaged_daemon_launch(action: &str) -> Result<()> { "version": env!("CARGO_PKG_VERSION"), "target": target, "entrypoint": "bin/codex" }))?, )?; - if action == "start" { + if action == "start" && initial == InitialDaemon::Missing { std::fs::remove_file(standalone.join("current"))?; std::os::unix::fs::symlink(&package, standalone.join("current"))?; } let cli_selection = standalone.join("current").canonicalize()?; let state = daemon.home.path().join("app-server-daemon"); - std::fs::remove_file(state.join("app-server.stderr.log"))?; + if initial == InitialDaemon::Missing { + std::fs::remove_file(state.join("app-server.stderr.log"))?; + } std::fs::write( state.join("settings.json"), br#"{"shutdownGraceSeconds":0}"#, @@ -384,29 +392,117 @@ fn packaged_daemon_launch(action: &str) -> Result<()> { "{}", String::from_utf8_lossy(&result.stderr) ); - assert!(String::from_utf8_lossy(&result.stderr).contains("Installing daemon from CLI version")); + assert_eq!( + String::from_utf8_lossy(&result.stderr).contains("Installing daemon from CLI version"), + initial == InitialDaemon::Missing + ); let output: Value = serde_json::from_slice(&result.stdout)?; let dedicated = daemon .home .path() .canonicalize()? .join("packages/app-server-daemon"); + let (initial_root, initial_pid_file) = match initial { + InitialDaemon::Missing => (&dedicated, "daemon.pid"), + InitialDaemon::Legacy => (&standalone, "app-server.pid"), + }; assert_eq!( output["managedCodexPath"], - dedicated + initial_root + .canonicalize()? .join("current/bin/codex") .to_str() .context("managed path is not UTF-8")? ); - assert_eq!( - std::fs::read(dedicated.join("current/codex-path/rg"))?, - b"runtime fixture" - ); assert_eq!(daemon.codex.canonicalize()?, cli_before); assert_eq!(standalone.join("current").canonicalize()?, cli_selection); - assert!(state.join("daemon.pid").exists()); - assert!(!state.join("app-server.pid").exists()); - assert!(!state.join("app-server-updater.pid").exists()); + assert!(state.join(initial_pid_file).exists()); + let legacy_updater = match initial { + InitialDaemon::Missing => { + assert!(!state.join("app-server-updater.pid").exists()); + None + } + InitialDaemon::Legacy => Some(daemon.pid("app-server-updater.pid")?), + }; + if action == "start" { + let initial_current = initial_root.join("current"); + let current = dedicated.join("current"); + let original = initial_current.canonicalize()?; + let original_pid = daemon.pid(initial_pid_file)?; + let settings_before = std::fs::read(state.join("settings.json"))?; + let refused = daemon + .command() + .args(["app-server", "daemon", "update", "--from-cli"]) + .output()?; + assert!(!refused.status.success()); + assert!(String::from_utf8_lossy(&refused.stderr).contains("rerun with --yes")); + assert_eq!(daemon.pid(initial_pid_file)?, original_pid); + assert_eq!(initial_current.canonicalize()?, original); + if initial == InitialDaemon::Legacy { + assert!(!current.exists()); + } + + std::fs::remove_file(package.join("bin/codex-code-mode-host"))?; + let invalid = daemon + .command() + .args(["app-server", "daemon", "update", "--from-cli", "--yes"]) + .output()?; + assert!(!invalid.status.success()); + assert_eq!(daemon.pid(initial_pid_file)?, original_pid); + assert_eq!(initial_current.canonicalize()?, original); + if initial == InitialDaemon::Legacy { + assert!(!current.exists()); + } + std::fs::write( + package.join("bin/codex-code-mode-host"), + b"replacement helper", + )?; + std::fs::set_permissions( + package.join("bin/codex-code-mode-host"), + std::fs::Permissions::from_mode(0o755), + )?; + let replaced = daemon + .command() + .args(["app-server", "daemon", "update", "--from-cli", "--yes"]) + .output()?; + ensure!( + replaced.status.success(), + "{}", + String::from_utf8_lossy(&replaced.stderr) + ); + let output: Value = serde_json::from_slice(&replaced.stdout)?; + assert_eq!(output["status"], "updated"); + assert_ne!(daemon.pid("daemon.pid")?, original_pid); + wait_for_exit(original_pid)?; + if let Some(pid) = legacy_updater { + wait_for_exit(pid)?; + } + assert!(!state.join("app-server.pid").exists()); + assert!(!state.join("app-server-updater.pid").exists()); + assert_eq!( + output["managedCodexPath"], + current + .join("bin/codex") + .to_str() + .context("managed path is not UTF-8")? + ); + assert_ne!(current.canonicalize()?, original); + assert!(!dedicated.join("auto-update-version").exists()); + assert_eq!(std::fs::read(state.join("settings.json"))?, settings_before); + assert_eq!( + std::fs::read(current.join("bin/codex-code-mode-host"))?, + b"replacement helper" + ); + if initial == InitialDaemon::Missing { + assert_eq!( + std::fs::read(original.join("bin/codex-code-mode-host"))?, + b"runtime fixture" + ); + } + assert_eq!(daemon.lifecycle("start")?["status"], "alreadyRunning"); + assert_eq!(daemon.lifecycle("stop")?["status"], "stopped"); + assert_eq!(standalone.join("current").canonicalize()?, cli_selection); + } if action == "bootstrap" { assert_eq!(output["autoUpdateEnabled"], false); } @@ -414,16 +510,21 @@ fn packaged_daemon_launch(action: &str) -> Result<()> { } #[test] -fn packaged_daemon_start_seeds_local_package() -> Result<()> { - packaged_daemon_launch("start") +fn packaged_daemon_start_and_explicit_replacement() -> Result<()> { + packaged_daemon_launch("start", InitialDaemon::Missing) } #[test] fn packaged_daemon_restart_seeds_local_package() -> Result<()> { - packaged_daemon_launch("restart") + packaged_daemon_launch("restart", InitialDaemon::Missing) } #[test] fn packaged_daemon_bootstrap_seeds_local_package() -> Result<()> { - packaged_daemon_launch("bootstrap") + packaged_daemon_launch("bootstrap", InitialDaemon::Missing) +} + +#[test] +fn packaged_daemon_explicit_replacement_migrates_running_legacy() -> Result<()> { + packaged_daemon_launch("start", InitialDaemon::Legacy) }