diff --git a/codex-rs/app-server-daemon/Cargo.toml b/codex-rs/app-server-daemon/Cargo.toml index e00b75ff67..3c98b42f68 100644 --- a/codex-rs/app-server-daemon/Cargo.toml +++ b/codex-rs/app-server-daemon/Cargo.toml @@ -33,12 +33,12 @@ tokio = { workspace = true, features = [ "signal", "time", ] } +tempfile = { workspace = true } tokio-tungstenite = { workspace = true } tracing = { workspace = true } [dev-dependencies] pretty_assertions = { workspace = true } -tempfile = { workspace = true } [target.'cfg(windows)'.dependencies] windows-sys = { version = "0.52", features = ["Win32_Foundation", "Win32_Storage_FileSystem", "Win32_System_IO", "Win32_System_JobObjects", "Win32_Security", "Win32_Security_Authorization", "Win32_System_Threading"] } diff --git a/codex-rs/app-server-daemon/src/backend/windows.rs b/codex-rs/app-server-daemon/src/backend/windows.rs index 1d2b91eaf5..157bf3343a 100644 --- a/codex-rs/app-server-daemon/src/backend/windows.rs +++ b/codex-rs/app-server-daemon/src/backend/windows.rs @@ -1,6 +1,7 @@ //! Windows process identity and file locks. Keep a process handle across shutdown //! so PID reuse can never redirect forced termination to a different process. //! Managed servers must not elevate ordinary clients sharing the account's socket. +//! Installer jobs contain extraction processes when an update is cancelled. use std::io; use std::os::windows::io::AsRawHandle; @@ -244,6 +245,18 @@ pub(crate) fn try_lock_file(file: &tokio::fs::File) -> Result { // Keep installer descendants bounded by the updater's lifetime, while allowing // app-server launches and successor updaters to break away from this job. pub(crate) fn updater_job() -> Result { + process_job(unsafe { GetCurrentProcess() }) +} + +/// Called before writing the installer script, while PowerShell is waiting on stdin. +pub(crate) fn installer_job(child: &tokio::process::Child) -> Result { + let process = child + .raw_handle() + .context("installer process handle is unavailable")?; + process_job(process as isize) +} + +fn process_job(process: isize) -> Result { let job = unsafe { CreateJobObjectW(std::ptr::null(), std::ptr::null()) }; if job == 0 { return Err(io::Error::last_os_error()).context("failed to create updater job"); @@ -260,7 +273,7 @@ pub(crate) fn updater_job() -> Result { std::mem::size_of_val(&limits) as u32, ) } == 0 - || unsafe { AssignProcessToJobObject(job, GetCurrentProcess()) } == 0 + || unsafe { AssignProcessToJobObject(job, process) } == 0 { return Err(io::Error::last_os_error()) .context("failed to contain updater installer processes"); diff --git a/codex-rs/app-server-daemon/src/install_lock.rs b/codex-rs/app-server-daemon/src/install_lock.rs new file mode 100644 index 0000000000..8a9d50e06b --- /dev/null +++ b/codex-rs/app-server-daemon/src/install_lock.rs @@ -0,0 +1,143 @@ +//! Coordinates package selection with the shell and PowerShell installers. + +use anyhow::Context; +use anyhow::Result; +use anyhow::anyhow; +use std::fs::OpenOptions; +use std::path::Path; +use std::time::Duration; +use tokio::time::sleep; + +pub(crate) struct InstallLock { + _file: Option, + directory: Option, + _directory_owner: Option, +} + +impl Drop for InstallLock { + fn drop(&mut self) { + if let Some(directory) = &self.directory { + let _ = std::fs::remove_file(directory); + } + } +} + +pub(crate) async fn acquire_install_lock(root: &Path) -> Result { + let lock = root.join("install.lock"); + let deadline = tokio::time::Instant::now() + Duration::from_secs(30); + #[cfg(unix)] + let use_lockf = { + // Match the installer so both processes coordinate on the same lock. + let choice = tokio::process::Command::new("/bin/sh") + .args(["-c", if cfg!(target_os = "macos") { + "if command -v lockf >/dev/null 2>&1; then exit 0; elif command -v flock >/dev/null 2>&1; then exit 1; else exit 2; fi" + } else { + "if command -v flock >/dev/null 2>&1; then exit 1; else exit 2; fi" + }]) + .status().await?; + match choice.code() { + Some(0) => true, + Some(1) => false, + Some(2) => { + let directory = root.join("install.lock.d"); + // Publish complete metadata atomically. Older shell installers + // immediately remove an empty lock directory as stale. + let owner = tempfile::Builder::new() + .prefix("install-lock-") + .tempdir_in(root)?; + std::fs::write(owner.path().join("pid"), std::process::id().to_string())?; + let now = std::time::SystemTime::now() + .duration_since(std::time::UNIX_EPOCH)? + .as_secs(); + std::fs::write(owner.path().join("started_at"), now.to_string())?; + loop { + match std::os::unix::fs::symlink(owner.path(), &directory) { + Ok(()) => { + return Ok(InstallLock { + _file: None, + directory: Some(directory), + _directory_owner: Some(owner), + }); + } + Err(error) if error.kind() == std::io::ErrorKind::AlreadyExists => {} + Err(error) => { + return Err(error) + .context("failed to acquire daemon installer directory lock"); + } + } + let pid = std::fs::read_to_string(directory.join("pid")) + .ok() + .and_then(|pid| pid.trim().parse::().ok()) + .filter(|pid| *pid > 0); + let dead = pid.is_none_or(|pid| unsafe { libc::kill(pid, 0) } != 0 + && std::io::Error::last_os_error().raw_os_error() == Some(libc::ESRCH)); + let age = std::fs::metadata(&directory) + .and_then(|metadata| metadata.modified()) + .ok() + .and_then(|modified| modified.elapsed().ok()) + .unwrap_or_default(); + // Allow the creator time to publish its PID; match the installer's + // ten-minute stale-lock grace instead of stealing an active lock. + if dead && age >= Duration::from_secs(600) { + std::fs::remove_dir_all(&directory)?; + continue; + } + anyhow::ensure!( + tokio::time::Instant::now() < deadline, + "timed out waiting for daemon installer lock {}", + directory.display() + ); + sleep(Duration::from_millis(50)).await; + } + } + _ => return Err(anyhow!("failed to detect daemon installer lock tools")), + } + }; + let mut options = OpenOptions::new(); + options.create(true).truncate(false).write(true); + #[cfg(windows)] + { + use std::os::windows::fs::OpenOptionsExt; + options.share_mode(0); + } + loop { + let acquired = options.open(&lock); + #[cfg(unix)] + let acquired = acquired.and_then(|file| { + { + use std::os::fd::AsRawFd; + let result = if use_lockf { + unsafe { libc::lockf(file.as_raw_fd(), libc::F_TLOCK, 0) } + } else { + unsafe { libc::flock(file.as_raw_fd(), libc::LOCK_EX | libc::LOCK_NB) } + }; + if result != 0 { + return Err(std::io::Error::last_os_error()); + } + } + Ok(file) + }); + match acquired { + Ok(file) => { + return Ok(InstallLock { + _file: Some(tokio::fs::File::from_std(file)), + directory: None, + _directory_owner: None, + }); + } + Err(error) + if error.kind() == std::io::ErrorKind::WouldBlock + || cfg!(windows) + && (error.raw_os_error() == Some(32) + || error.kind() == std::io::ErrorKind::PermissionDenied) => {} + Err(error) => return Err(error).context("failed to acquire daemon installer lock"), + } + if tokio::time::Instant::now() >= deadline { + return Err(anyhow!( + "timed out waiting for daemon installer lock {}", + lock.display() + )); + } + sleep(Duration::from_millis(50)).await; + } +} diff --git a/codex-rs/app-server-daemon/src/lib.rs b/codex-rs/app-server-daemon/src/lib.rs index ccb1be9717..ee821a3876 100644 --- a/codex-rs/app-server-daemon/src/lib.rs +++ b/codex-rs/app-server-daemon/src/lib.rs @@ -4,6 +4,7 @@ mod backend; #[cfg(windows)] use backend::windows::try_lock_file; mod client; +mod install_lock; mod managed_install; mod remote_control_client; mod settings; @@ -36,8 +37,10 @@ const START_TIMEOUT: Duration = Duration::from_secs(10); // Leave room for the longest graceful stop, forced-exit check, and restart. const OPERATION_LOCK_TIMEOUT: Duration = Duration::from_secs(MAX_SHUTDOWN_GRACE_SECONDS as u64 + 75); -const PID_FILE_NAME: &str = "app-server.pid"; -const UPDATE_PID_FILE_NAME: &str = "app-server-updater.pid"; +const LEGACY_PID_FILE_NAME: &str = "app-server.pid"; +const LEGACY_UPDATE_PID_FILE_NAME: &str = "app-server-updater.pid"; +const DAEMON_PID_FILE_NAME: &str = "daemon.pid"; +const DAEMON_UPDATE_PID_FILE_NAME: &str = "daemon-updater.pid"; const OPERATION_LOCK_FILE_NAME: &str = "daemon.lock"; const SETTINGS_FILE_NAME: &str = "settings.json"; const STATE_DIR_NAME: &str = "app-server-daemon"; @@ -270,11 +273,13 @@ pub async fn run_pid_update_loop( update_loop::run(http_client_factory).await } -pub async fn update() -> Result { +pub async fn update( + http_client_factory: codex_http_client::HttpClientFactory, +) -> Result { ensure_supported_platform()?; #[cfg(windows)] backend::windows::ensure_not_elevated()?; - update_loop::request_manual_update(&Daemon::from_environment()?).await + update_loop::request_manual_update(&Daemon::from_environment()?, http_client_factory).await } #[cfg(any(unix, windows))] @@ -289,6 +294,7 @@ fn ensure_supported_platform() -> Result<()> { )) } +#[derive(Clone)] struct Daemon { socket_path: PathBuf, pid_file: PathBuf, @@ -305,13 +311,21 @@ impl Daemon { .as_path() .to_path_buf(); let state_dir = codex_home.as_path().join(STATE_DIR_NAME); + let managed_codex_bin = managed_codex_bin(codex_home.as_path()); + // Old CLIs must not mistake a daemon-owned installation for their backend. + let (pid_file, update_pid_file) = + if managed_codex_bin.starts_with(codex_home.as_path().join("packages/standalone")) { + (LEGACY_PID_FILE_NAME, LEGACY_UPDATE_PID_FILE_NAME) + } else { + (DAEMON_PID_FILE_NAME, DAEMON_UPDATE_PID_FILE_NAME) + }; Ok(Self { socket_path, - pid_file: state_dir.join(PID_FILE_NAME), - update_pid_file: state_dir.join(UPDATE_PID_FILE_NAME), + pid_file: state_dir.join(pid_file), + update_pid_file: state_dir.join(update_pid_file), operation_lock_file: state_dir.join(OPERATION_LOCK_FILE_NAME), settings_file: state_dir.join(SETTINGS_FILE_NAME), - managed_codex_bin: managed_codex_bin(codex_home.as_path()), + managed_codex_bin, }) } @@ -324,25 +338,45 @@ impl Daemon { )) } + // Call only after taking the operation lock: an explicit update may have + // migrated the package and PID namespace while this command was waiting. + fn current_installation(&self) -> Result { + let managed_codex_bin = self.current_managed_codex_bin()?; + let home = self + .settings_file + .parent() + .and_then(Path::parent) + .context("daemon settings path has no Codex home")?; + let (pid, updater) = if managed_codex_bin.starts_with(home.join("packages/standalone")) { + (LEGACY_PID_FILE_NAME, LEGACY_UPDATE_PID_FILE_NAME) + } else { + (DAEMON_PID_FILE_NAME, DAEMON_UPDATE_PID_FILE_NAME) + }; + Ok(Self { + managed_codex_bin, + pid_file: self.pid_file.with_file_name(pid), + update_pid_file: self.update_pid_file.with_file_name(updater), + ..self.clone() + }) + } + async fn run(&self, command: LifecycleCommand) -> Result { + if command == LifecycleCommand::Version { + return self.version().await; + } + let _operation_lock = self.acquire_operation_lock().await?; + let selected = self.current_installation()?; match command { - LifecycleCommand::Start => { - let _operation_lock = self.acquire_operation_lock().await?; - self.start().await - } - LifecycleCommand::Restart => { - let _operation_lock = self.acquire_operation_lock().await?; - self.restart().await - } + LifecycleCommand::Start => selected.start().await, + LifecycleCommand::Restart => selected.restart().await, LifecycleCommand::Stop => { - let _operation_lock = self.acquire_operation_lock().await?; - let output = self.stop().await?; - if let Err(err) = thread_recovery::discard_pending(self) { + let output = selected.stop().await?; + if let Err(err) = thread_recovery::discard_pending(&selected) { eprintln!("warning: failed to clear saved threads after daemon stop: {err}"); } Ok(output) } - LifecycleCommand::Version => self.version().await, + LifecycleCommand::Version => unreachable!(), } } @@ -597,21 +631,22 @@ impl Daemon { async fn bootstrap(&self, options: BootstrapOptions) -> Result { let _operation_lock = self.acquire_operation_lock().await?; - self.bootstrap_locked(options).await + self.current_installation()?.bootstrap_locked(options).await } async fn ensure_remote_control_started(&self) -> Result { let _operation_lock = self.acquire_operation_lock().await?; - let settings = self.load_settings().await?; - if self.is_bootstrapped(&settings).await? { - let _ = self + let selected = self.current_installation()?; + let settings = selected.load_settings().await?; + if selected.is_bootstrapped(&settings).await? { + let _ = selected .set_remote_control_locked(RemoteControlMode::Enabled) .await?; - let output = self.start().await?; + let output = selected.start().await?; return Ok(RemoteControlStartOutput::Start(output)); } - let output = self + let output = selected .bootstrap_locked(BootstrapOptions { remote_control_enabled: true, }) @@ -631,7 +666,9 @@ impl Daemon { async fn set_remote_control(&self, mode: RemoteControlMode) -> Result { let _operation_lock = self.acquire_operation_lock().await?; - self.set_remote_control_locked(mode).await + self.current_installation()? + .set_remote_control_locked(mode) + .await } async fn set_remote_control_locked( @@ -851,7 +888,8 @@ impl Daemon { .parent() .and_then(Path::parent) .is_some_and(|home| { - home.join("packages/standalone/auto-update-version") + managed_install::package_root(home) + .join("auto-update-version") .is_file() }) } @@ -917,8 +955,7 @@ impl Daemon { } fn manual_update_socket_path(&self) -> PathBuf { - self.update_pid_file - .with_file_name("app-server-updater.sock") + self.update_pid_file.with_extension("sock") } async fn load_settings(&self) -> Result { @@ -1188,6 +1225,41 @@ mod tests { ); } + #[tokio::test] + async fn waiting_lifecycle_command_uses_migrated_installation() { + let home = TempDir::new().expect("home"); + let state = home.path().join("app-server-daemon"); + let legacy = home.path().join("packages/standalone/current"); + std::fs::create_dir_all(&legacy).expect("legacy selection"); + let daemon = Daemon { + socket_path: home.path().join("server.sock"), + pid_file: state.join(super::LEGACY_PID_FILE_NAME), + update_pid_file: state.join(super::LEGACY_UPDATE_PID_FILE_NAME), + operation_lock_file: state.join("daemon.lock"), + settings_file: state.join("settings.json"), + managed_codex_bin: super::managed_codex_bin(home.path()), + }; + let lock = daemon.acquire_operation_lock().await.expect("lock"); + let stop = daemon.run(super::LifecycleCommand::Stop); + tokio::pin!(stop); + assert!( + tokio::time::timeout(std::time::Duration::from_millis(20), &mut stop) + .await + .is_err() + ); + std::fs::create_dir_all(home.path().join("packages/app-server-daemon/current")) + .expect("migrate selection"); + drop(lock); + let output = stop.await.expect("stop"); + assert_eq!( + (output.status, output.managed_codex_path), + ( + LifecycleStatus::NotRunning, + super::managed_codex_bin(home.path()) + ) + ); + } + #[tokio::test] async fn stop_creates_missing_home_parent() { let temp = TempDir::new().expect("temp dir"); diff --git a/codex-rs/app-server-daemon/src/managed_install.rs b/codex-rs/app-server-daemon/src/managed_install.rs index b7b57f7f68..099c1b71c9 100644 --- a/codex-rs/app-server-daemon/src/managed_install.rs +++ b/codex-rs/app-server-daemon/src/managed_install.rs @@ -14,19 +14,63 @@ use tokio::fs; use tokio::process::Command; use tokio::time::timeout; -/// Returns the packaged executable when present, otherwise an existing legacy executable. -/// If neither exists, returns the expected packaged path on Windows and preserves the -/// historical legacy fallback on Unix. This is path selection, not existence validation: -/// launch operations reject a missing executable separately, while commands such as -/// stop can still run after the managed install has been removed. +/// Dedicated daemon packages take precedence over standalone installations. +/// Preserve legacy launch state, including logs left after a daemon is stopped; +/// settings, installer selections, and lock files alone do not prove a prior launch. +pub(crate) fn package_root(codex_home: &Path) -> PathBuf { + let dedicated = codex_home.join("packages/app-server-daemon"); + if !matches!(dedicated.join("current").symlink_metadata(), + Err(error) if error.kind() == std::io::ErrorKind::NotFound) + { + return dedicated; + } + let state = codex_home.join("app-server-daemon"); + for (package, artifacts) in [ + ( + "app-server-daemon", + [ + crate::DAEMON_PID_FILE_NAME, + "daemon.stderr.log", + crate::DAEMON_UPDATE_PID_FILE_NAME, + "daemon-updater.stderr.log", + ], + ), + ( + "standalone", + [ + crate::LEGACY_PID_FILE_NAME, + "app-server.stderr.log", + crate::LEGACY_UPDATE_PID_FILE_NAME, + "app-server-updater.stderr.log", + ], + ), + ] { + if artifacts.iter().any(|name| { + !matches!(state.join(name).symlink_metadata(), + Err(error) if error.kind() == std::io::ErrorKind::NotFound) + }) { + return codex_home.join("packages").join(package); + } + } + // Retain CLI installs until the next stage adds dedicated package seeding. + let standalone = codex_home.join("packages/standalone"); + if !matches!(standalone.join("current").symlink_metadata(), + Err(error) if error.kind() == std::io::ErrorKind::NotFound) + { + return standalone; + } + dedicated +} + +/// Resolve both packaged and legacy binaries without requiring a valid install. pub(crate) fn managed_codex_bin(codex_home: &Path) -> PathBuf { - let current = codex_home - .join("packages") - .join("standalone") - .join("current"); + let root = package_root(codex_home); + let current = root.join("current"); let packaged = current.join("bin").join(managed_codex_file_name()); let legacy = current.join(managed_codex_file_name()); - if packaged.is_file() || (cfg!(windows) && !legacy.is_file()) { + if packaged.is_file() + || !legacy.is_file() && (cfg!(windows) || root.ends_with("app-server-daemon")) + { packaged } else { legacy @@ -35,7 +79,7 @@ pub(crate) fn managed_codex_bin(codex_home: &Path) -> PathBuf { /// Only latest-channel stable releases may run the public latest-version updater. pub(crate) fn is_stable_standalone_release(codex_home: &Path, codex_bin: &Path) -> bool { - let standalone = codex_home.join("packages/standalone"); + let standalone = package_root(codex_home); let Ok(releases) = std::fs::canonicalize(standalone.join("releases")) else { return false; }; @@ -48,9 +92,15 @@ pub(crate) fn is_stable_standalone_release(codex_home: &Path, codex_bin: &Path) let Some(release_name) = release.file_name().and_then(|name| name.to_str()) else { return false; }; + // GNU packages can seed the new directory; retain legacy updater eligibility. + if standalone.ends_with("standalone") && release_name.ends_with("-gnu") { + return false; + } let targets = [ "aarch64-apple-darwin", "x86_64-apple-darwin", + "aarch64-unknown-linux-gnu", + "x86_64-unknown-linux-gnu", "aarch64-unknown-linux-musl", "x86_64-unknown-linux-musl", "aarch64-pc-windows-msvc", @@ -74,13 +124,19 @@ pub(crate) fn is_stable_standalone_release(codex_home: &Path, codex_bin: &Path) /// Older managed binaries can serve app-server requests without owning an updater. pub(crate) async fn supports_daemon_update_loop(codex_bin: &Path) -> bool { + supports_daemon_command(codex_bin, &["pid-update-loop", "--help"]).await +} + +/// Probe an internal daemon command without running a long-lived process. +pub(crate) async fn supports_daemon_command(codex_bin: &Path, args: &[&str]) -> bool { let mut command = Command::new(codex_bin); #[cfg(windows)] command.creation_flags(windows_sys::Win32::System::Threading::CREATE_NO_WINDOW); timeout( Duration::from_secs(5), command - .args(["app-server", "daemon", "pid-update-loop", "--help"]) + .args(["app-server", "daemon"]) + .args(args) .stdin(Stdio::null()) .stdout(Stdio::null()) .stderr(Stdio::null()) diff --git a/codex-rs/app-server-daemon/src/managed_install_path_tests.rs b/codex-rs/app-server-daemon/src/managed_install_path_tests.rs index 7e49e80fb8..174c634005 100644 --- a/codex-rs/app-server-daemon/src/managed_install_path_tests.rs +++ b/codex-rs/app-server-daemon/src/managed_install_path_tests.rs @@ -5,25 +5,70 @@ fn discovers_package_and_legacy_installs() { let home = tempfile::TempDir::new().expect("home"); let current = home.path().join("packages/standalone/current"); let legacy = current.join(super::managed_codex_file_name()); - let expected = if cfg!(windows) { - current.join("bin").join("codex.exe") - } else { - legacy.clone() - }; - assert_eq!(super::managed_codex_bin(home.path()), expected); + assert_eq!( + super::managed_codex_bin(home.path()), + home.path() + .join("packages/app-server-daemon/current/bin") + .join(super::managed_codex_file_name()) + ); std::fs::create_dir_all(¤t).expect("current directory"); std::fs::write(&legacy, b"legacy").expect("legacy executable"); + let state = home.path().join("app-server-daemon"); + std::fs::create_dir(&state).unwrap(); + for name in ["settings.json", "daemon.lock", "app-server.pid.lock"] { + std::fs::write(state.join(name), b"").unwrap(); + } + // A CLI install and a previous stop/status operation do not establish ownership. + assert_eq!( + super::package_root(home.path()), + home.path().join("packages/standalone") + ); + std::fs::write(state.join("app-server.stderr.log"), b"").unwrap(); assert_eq!(super::managed_codex_bin(home.path()), legacy); let packaged = current.join("bin").join(super::managed_codex_file_name()); std::fs::create_dir(current.join("bin")).expect("bin directory"); std::fs::write(&packaged, b"packaged").expect("packaged executable"); assert_eq!(super::managed_codex_bin(home.path()), packaged); + + std::fs::remove_dir_all(home.path().join("packages/standalone")).unwrap(); + assert_eq!( + super::package_root(home.path()), + home.path().join("packages/standalone") + ); + std::fs::remove_file(state.join("app-server.stderr.log")).unwrap(); + std::fs::write(state.join("app-server.pid"), b"running daemon").unwrap(); + assert_eq!( + super::package_root(home.path()), + home.path().join("packages/standalone") + ); + + std::fs::write(state.join("daemon.pid"), b"running dedicated daemon").unwrap(); + assert_eq!( + super::package_root(home.path()), + home.path().join("packages/app-server-daemon") + ); + std::fs::remove_file(state.join("daemon.pid")).unwrap(); + std::fs::write(state.join("daemon.stderr.log"), b"").unwrap(); + assert_eq!( + super::package_root(home.path()), + home.path().join("packages/app-server-daemon") + ); + + #[cfg(unix)] + { + let dedicated = home.path().join("packages/app-server-daemon"); + std::fs::write(&dedicated, b"not a directory").expect("unreadable selection"); + assert_eq!(super::package_root(home.path()), dedicated); + } } #[cfg(unix)] #[test] fn updater_only_runs_for_stable_installer_owned_releases() { let home = tempfile::TempDir::new().expect("home"); + let state = home.path().join("app-server-daemon"); + std::fs::create_dir(&state).unwrap(); + std::fs::write(state.join("app-server.stderr.log"), b"").unwrap(); let standalone = home.path().join("packages/standalone"); let current = standalone.join("current"); let release = standalone.join("releases/0.150.0-aarch64-apple-darwin"); diff --git a/codex-rs/app-server-daemon/src/manual_update.rs b/codex-rs/app-server-daemon/src/manual_update.rs index 743b6e4e6f..e822e51d52 100644 --- a/codex-rs/app-server-daemon/src/manual_update.rs +++ b/codex-rs/app-server-daemon/src/manual_update.rs @@ -126,7 +126,7 @@ pub(crate) async fn request(daemon: &Daemon) -> Result { const UNSUPPORTED_MESSAGE: &str = "This command requires a CLI-managed daemon and a stable latest-channel standalone install; update this installation with its owning installer."; -async fn unsupported(daemon: &Daemon) -> Result { +pub(super) async fn unsupported(daemon: &Daemon) -> Result { let managed_codex_path = daemon.current_managed_codex_bin()?; Ok(UpdateOutput { status: UpdateStatus::Unsupported, diff --git a/codex-rs/app-server-daemon/src/migration.rs b/codex-rs/app-server-daemon/src/migration.rs new file mode 100644 index 0000000000..e250f6df52 --- /dev/null +++ b/codex-rs/app-server-daemon/src/migration.rs @@ -0,0 +1,193 @@ +//! Moves a legacy daemon to its dedicated package only during an explicit update. +//! Preparation leaves the legacy selection intact; publishing current is the cutover. + +use std::path::Path; + +use anyhow::Context; +use anyhow::Result; +use futures::FutureExt; + +use super::InstallerHttp; +use super::InstallerMode; +use super::fetch_installer_script; +use super::run_installer_script; +use super::selected_release; +use crate::Daemon; +use crate::UpdateOutput; +use crate::UpdateStatus; +use crate::backend; +use crate::client; +use crate::install_lock::acquire_install_lock; +use crate::managed_install; + +pub(super) async fn run(http: &impl InstallerHttp, legacy: &Daemon) -> Result { + let _operation_lock = legacy.acquire_operation_lock().await?; + let previous = selected_release(legacy)?; + anyhow::ensure!( + previous.0.ends_with("standalone") && legacy.is_stable_standalone_release()?, + "daemon selection changed; retry the update" + ); + let home = previous + .0 + .parent() + .and_then(Path::parent) + .context("package root has no Codex home")?; + let root = home.join("packages/app-server-daemon"); + let settings = legacy.load_settings().await?; + let running = legacy.running_backend_instance(&settings).await?; + if running.is_none() && client::probe(&legacy.socket_path).await.is_ok() { + return super::manual_update::unsupported(legacy).await; + } + #[cfg(unix)] + let mut terminate = tokio::signal::unix::signal(tokio::signal::unix::SignalKind::terminate())?; + let cancellation = async { + #[cfg(unix)] + tokio::select! { + _ = terminate.recv() => {}, + _ = tokio::signal::ctrl_c() => {}, + } + #[cfg(windows)] + let _ = tokio::signal::ctrl_c().await; + Some(()) + }; + let mut cancellation = std::pin::pin!(cancellation); + + let script = fetch_installer_script(http).await?; + anyhow::ensure!( + script + .windows(b"CODEX_INSTALL_DEFER_SELECTION".len()) + .any(|window| window == b"CODEX_INSTALL_DEFER_SELECTION"), + "the published installer does not support daemon migration yet; the legacy installation was left unchanged" + ); + eprintln!("Preparing the daemon update in {}...", root.display()); + #[cfg(unix)] + { + if matches!( + run_installer_script( + &script, + InstallerMode::Migration, + &root, + cancellation.as_mut() + ) + .await?, + super::UpdateLoopControl::Stop + ) { + return Err(std::io::Error::from(std::io::ErrorKind::Interrupted).into()); + } + } + #[cfg(windows)] + tokio::select! { + result = run_installer_script(&script, InstallerMode::Migration, &root) => { result?; }, + _ = &mut cancellation => return Err(std::io::Error::from(std::io::ErrorKind::Interrupted).into()), + } + + let _install_lock = acquire_install_lock(&root).await?; + let pending = root.join(".migration-current"); + let release = pending + .canonicalize() + .context("installer did not prepare a daemon package")?; + let name = release + .file_name() + .context("prepared release has no name")?; + anyhow::ensure!( + release.parent() == Some(root.join("releases").canonicalize()?.as_path()) + && std::fs::read_to_string(root.join("auto-update-version"))? == name.to_string_lossy(), + "installer did not prepare a latest-channel daemon package" + ); + let entrypoint = if cfg!(windows) { + "bin/codex.exe" + } else { + "bin/codex" + }; + let binary = release.join(entrypoint); + let version = managed_install::managed_codex_version(&binary).await?; + anyhow::ensure!( + name.to_string_lossy().starts_with(&format!("{version}-")), + "prepared daemon version does not match its release" + ); + // An older production updater understands only the legacy package/PID paths. + anyhow::ensure!( + managed_install::supports_daemon_command( + &binary, + &["pid-update-loop", "--check-package-ownership"] + ) + .await, + "the latest production release does not support daemon migration yet; the legacy daemon was left running" + ); + #[cfg(windows)] + backend::windows::ensure_detached_launch(&binary)?; + anyhow::ensure!( + selected_release(legacy)? == previous, + "daemon selection changed; retry the update" + ); + if cancellation.as_mut().now_or_never().is_some() { + return Err(std::io::Error::from(std::io::ErrorKind::Interrupted).into()); + } + let cutover = async { + backend::pid_update_loop_backend(legacy.backend_paths(&settings)) + .stop() + .await?; + anyhow::ensure!( + selected_release(legacy)? == previous, + "daemon selection changed; retry the update" + ); + if let Some(running) = &running { + crate::thread_recovery::discard_pending(legacy)?; + running + .stop_with_grace(settings.shutdown_grace_seconds) + .await?; + } + std::fs::rename(&pending, root.join("current")) + .context("failed to select the new daemon package; retry the update") + } + .await; + if let Err(error) = cutover { + if let Err(restore_error) = async { + legacy + .current_installation()? + .ensure_managed_updater(&settings) + .await + } + .await + { + eprintln!( + "warning: failed to restore the legacy updater after migration failed: {restore_error:#}" + ); + } + return Err(error); + } + let selected = Daemon { + pid_file: legacy.pid_file.with_file_name(crate::DAEMON_PID_FILE_NAME), + update_pid_file: legacy + .update_pid_file + .with_file_name(crate::DAEMON_UPDATE_PID_FILE_NAME), + managed_codex_bin: root.join("current").join(entrypoint), + ..legacy.clone() + }; + let running_version = if running.is_some() { + selected.start_managed_backend(&settings).await.context( + "daemon migrated but could not start; retry with `codex app-server daemon start`", + )?; + Some( + selected + .wait_until_ready() + .await + .context( + "daemon migrated but is not ready; retry with `codex app-server daemon start`", + )? + .app_server_version, + ) + } else { + None + }; + if let Err(error) = selected.ensure_managed_updater(&settings).await { + eprintln!("warning: daemon migrated but its updater could not start: {error:#}"); + } + Ok(UpdateOutput { + status: UpdateStatus::Updated, + installed_version: Some(version), + running_version, + managed_codex_path: selected.managed_codex_bin, + message: "The daemon was updated and moved to its dedicated package. The legacy CLI package was left unchanged.".to_string(), + }) +} diff --git a/codex-rs/app-server-daemon/src/update_loop.rs b/codex-rs/app-server-daemon/src/update_loop.rs index 2fe7850df4..4513ce6855 100644 --- a/codex-rs/app-server-daemon/src/update_loop.rs +++ b/codex-rs/app-server-daemon/src/update_loop.rs @@ -38,7 +38,27 @@ use crate::settings::UpdaterSettings; #[path = "manual_update.rs"] mod manual_update; -pub(crate) use manual_update::request as request_manual_update; +#[path = "migration.rs"] +mod migration; + +pub(crate) async fn request_manual_update( + daemon: &Daemon, + http_client_factory: HttpClientFactory, +) -> Result { + if daemon + .pid_file + .file_name() + .is_some_and(|name| name == crate::LEGACY_PID_FILE_NAME) + && daemon.is_stable_standalone_release()? + { + let http = RouteAwareClientPool::new_without_request_logging( + http_client_factory, + ClientRouteClass::Other, + ); + return migration::run(&http, daemon).await; + } + manual_update::request(daemon).await +} const INITIAL_UPDATE_DELAY: Duration = Duration::from_secs(5 * 60); const RESTART_RETRY_INTERVAL: Duration = Duration::from_millis(50); @@ -277,7 +297,11 @@ async fn update_once( // latest-channel marker. Retry after the interval instead of exiting. return Ok((UpdateLoopControl::Continue, None)); } - let (_codex_home, _, previous_release) = selected_release(daemon)?; + let (package_root, _, previous_release) = selected_release(daemon)?; + let codex_home = package_root + .parent() + .and_then(Path::parent) + .context("daemon package root has no Codex home")?; let script = tokio::select! { result = fetch_installer_script(http) => result?, _ = terminate.recv() => return Ok((UpdateLoopControl::Stop, None)), @@ -288,6 +312,14 @@ async fn update_once( .any(|window| window == b"CODEX_INSTALL_IF_LATEST"), "standalone installer does not support guarded updates" ); + if package_root.ends_with("app-server-daemon") { + anyhow::ensure!( + script + .windows(b"CODEX_INSTALL_DAEMON_ONLY".len()) + .any(|window| window == b"CODEX_INSTALL_DAEMON_ONLY"), + "installer does not support daemon-owned packages" + ); + } if trigger == UpdateTrigger::Scheduled && !UpdaterSettings::load(&daemon.settings_file) .await? @@ -298,16 +330,26 @@ async fn update_once( if release_selection_unstable(daemon, trigger)? { return Ok((UpdateLoopControl::Continue, None)); } + anyhow::ensure!( + crate::managed_install::package_root(codex_home) == package_root, + "daemon package root changed during the update; retry the command" + ); #[cfg(unix)] if matches!( - run_installer_script(&script, &previous_release, _codex_home, terminate.recv()).await?, + run_installer_script( + &script, + InstallerMode::Update(&previous_release), + &package_root, + terminate.recv() + ) + .await?, UpdateLoopControl::Stop ) { return Ok((UpdateLoopControl::Stop, None)); } #[cfg(windows)] tokio::select! { - result = run_installer_script(&script, &previous_release) => { result?; }, + result = run_installer_script(&script, InstallerMode::Update(&previous_release), &package_root) => { result?; }, _ = terminate.recv() => return Ok((UpdateLoopControl::Stop, None)), } if release_selection_unstable(daemon, trigger)? { @@ -395,19 +437,20 @@ fn release_selection_unstable(daemon: &Daemon, trigger: UpdateTrigger) -> Result Ok(true) } -fn selected_release(daemon: &Daemon) -> Result<(&Path, std::path::PathBuf, String)> { +fn selected_release(daemon: &Daemon) -> Result<(std::path::PathBuf, std::path::PathBuf, String)> { let home = daemon .settings_file .parent() .and_then(Path::parent) .context("daemon settings path has no Codex home")?; - let release = std::fs::canonicalize(home.join("packages/standalone/current"))?; + let root = crate::managed_install::package_root(home); + let release = std::fs::canonicalize(root.join("current"))?; let name = release .file_name() .context("managed release has no name")? .to_string_lossy() .into_owned(); - Ok((home, release, name)) + Ok((root, release, name)) } async fn current_updater_identity() -> Result { @@ -429,10 +472,15 @@ pub(crate) fn reexec_managed_updater(managed_codex_bin: &std::path::Path) -> Res }) } +enum InstallerMode<'a> { + Update(&'a str), + Migration, +} + async fn run_installer_script( script: &[u8], - previous_release: &str, - #[cfg(unix)] codex_home: &Path, + mode: InstallerMode<'_>, + package_root: &Path, #[cfg(unix)] terminate: impl std::future::Future>, ) -> Result { #[cfg(unix)] @@ -450,10 +498,30 @@ async fn run_installer_script( .kill_on_drop(true); command }; + let (guard, previous_release, defer_selection) = match mode { + InstallerMode::Update(release) => ("1", release, "0"), + InstallerMode::Migration => ("0", "", "1"), + }; let mut child = command + .env( + "CODEX_HOME", + package_root + .parent() + .and_then(Path::parent) + .context("package root has no Codex home")?, + ) + .env("CODEX_INSTALL_DEFER_SELECTION", defer_selection) + .env( + "CODEX_INSTALL_DAEMON_ONLY", + if package_root.ends_with("app-server-daemon") { + "1" + } else { + "0" + }, + ) .env("CODEX_RELEASE", "latest") .env("CODEX_NON_INTERACTIVE", "1") - .env("CODEX_INSTALL_IF_LATEST", "1") + .env("CODEX_INSTALL_IF_LATEST", guard) .env("CODEX_UPDATE_FROM_RELEASE", previous_release) .kill_on_drop(true) .stdin(Stdio::piped()) @@ -461,6 +529,8 @@ async fn run_installer_script( .stderr(Stdio::null()) .spawn() .context("failed to invoke standalone Codex updater")?; + #[cfg(windows)] + let _installer_job = crate::backend::windows::installer_job(&child)?; let mut stdin = child .stdin .take() @@ -477,7 +547,7 @@ async fn run_installer_script( drop(stdin); #[cfg(unix)] if write_result.is_none() { - cancel_installer(&mut child, codex_home).await; + cancel_installer(&mut child, package_root).await; return Ok(UpdateLoopControl::Stop); } write_result @@ -487,7 +557,7 @@ async fn run_installer_script( let status = tokio::select! { result = child.wait() => result, _ = &mut terminate => { - cancel_installer(&mut child, codex_home).await; + cancel_installer(&mut child, package_root).await; return Ok(UpdateLoopControl::Stop); } }; @@ -503,7 +573,7 @@ async fn run_installer_script( } #[cfg(unix)] -async fn cancel_installer(child: &mut tokio::process::Child, codex_home: &Path) { +async fn cancel_installer(child: &mut tokio::process::Child, package_root: &Path) { let Some(pid) = child.id().and_then(|pid| libc::pid_t::try_from(pid).ok()) else { return; }; @@ -515,7 +585,7 @@ async fn cancel_installer(child: &mut tokio::process::Child, codex_home: &Path) unsafe { libc::kill(-pid, libc::SIGKILL) }; // A forced kill can bypass the shell trap on hosts using the mkdir lock. // The lock is ours only if its recorded owner is this still-unreaped shell. - let lock = codex_home.join("packages/standalone/install.lock.d"); + let lock = package_root.join("install.lock.d"); if std::fs::read_to_string(lock.join("pid")).is_ok_and(|owner| owner.trim() == pid.to_string()) { let _ = std::fs::remove_dir_all(lock); 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 ff2b177b86..2dfd3c40a1 100644 --- a/codex-rs/app-server-daemon/src/update_loop_tests.rs +++ b/codex-rs/app-server-daemon/src/update_loop_tests.rs @@ -54,6 +54,135 @@ struct FakeInstallerHttp { requested_urls: Mutex>, } +#[cfg(unix)] +#[tokio::test] +async fn explicit_update_migrates_running_and_stopped_installations() { + for running in [false, true] { + let home = TempDir::new().unwrap(); + let (legacy, release) = manual_update_daemon(&home); + let scheduled = FakeInstallerHttp::new(InstallerResponse::Success( + b"# CODEX_INSTALL_IF_LATEST\ntest \"$CODEX_INSTALL_DEFER_SELECTION\" = 0 && test \"$CODEX_INSTALL_DAEMON_ONLY\" = 0\n".to_vec(), + )); + super::update_once( + &scheduled, + &legacy, + &executable_identity(&legacy.managed_codex_bin) + .await + .unwrap(), + &mut test_terminate(), + super::UpdateTrigger::Scheduled, + ) + .await + .unwrap(); + assert_eq!(scheduled.requested_urls(), vec![INSTALL_URL.to_string()]); + assert_eq!( + crate::managed_install::package_root(home.path()), + home.path().join("packages/standalone") + ); + let settings = format!( + r#"{{"updater":{{"autoUpdateEnabled":{running}}},"remoteControlEnabled":true}}"# + ); + std::fs::write(&legacy.settings_file, &settings).unwrap(); + let daemon_settings = legacy.load_settings().await.unwrap(); + let old_backend = crate::backend::pid_backend(legacy.backend_paths(&daemon_settings)); + let old_updater = + crate::backend::pid_update_loop_backend(legacy.backend_paths(&daemon_settings)); + let server = if running { + old_backend.start().await.unwrap(); + old_updater.start().await.unwrap(); + Some(test_control_server(&legacy, home.path()).await) + } else { + None + }; + let legacy_bin = std::fs::read(&legacy.managed_codex_bin).unwrap(); + let dedicated = home.path().join("packages/app-server-daemon"); + let old_installer = FakeInstallerHttp::new(InstallerResponse::Success(b"exit 99".to_vec())); + assert!( + super::migration::run(&old_installer, &legacy) + .await + .unwrap_err() + .to_string() + .contains("does not support daemon migration") + ); + + let script = format!( + r#"# CODEX_INSTALL_DEFER_SELECTION +set -eu +test "$CODEX_INSTALL_DEFER_SELECTION" = 1 +test "$CODEX_INSTALL_DAEMON_ONLY" = 1 +test "$CODEX_INSTALL_IF_LATEST" = 0 +root="$CODEX_HOME/packages/app-server-daemon" +mkdir -p "$root/releases/{release}/bin" +printf '#!/bin/sh\nif [ "$1" = --version ]; then echo codex 1.0.0; else exit 2; fi\n' > "$root/releases/{release}/bin/codex" +chmod +x "$root/releases/{release}/bin/codex" +ln -sfn 'releases/{release}' "$root/.migration-current" +printf '{release}' > "$root/auto-update-version" +"# + ); + let http = FakeInstallerHttp::new(InstallerResponse::Success(script.as_bytes().to_vec())); + let error = super::migration::run(&http, &legacy).await.unwrap_err(); + assert!( + error + .to_string() + .contains("production release does not support daemon migration") + ); + assert!(!dedicated.join("current").exists()); + assert_eq!( + crate::managed_install::package_root(home.path()), + home.path().join("packages/standalone") + ); + assert_eq!( + ( + old_backend.is_starting_or_running().await.unwrap(), + old_updater.is_starting_or_running().await.unwrap() + ), + (running, running) + ); + let compatible = FakeInstallerHttp::new(InstallerResponse::Success(script.replace("else exit 2", "elif [ \"$4\" = --check-package-ownership ] || [ \"$4\" = --help ]; then exit 0; else exec sleep 30").into_bytes())); + let output = super::migration::run(&compatible, &legacy).await.unwrap(); + assert_eq!(output, UpdateOutput { + status: UpdateStatus::Updated, + installed_version: Some("1.0.0".to_string()), + running_version: (running).then(|| "1.0.0".to_string()), + managed_codex_path: dedicated.join("current/bin/codex"), + message: "The daemon was updated and moved to its dedicated package. The legacy CLI package was left unchanged.".to_string(), + }); + assert_eq!( + crate::managed_install::package_root(home.path()), + dedicated.clone() + ); + assert!(!dedicated.join(".migration-current").exists()); + assert_eq!( + std::fs::read(&legacy.managed_codex_bin).unwrap(), + legacy_bin + ); + assert_eq!( + std::fs::read_to_string(&legacy.settings_file).unwrap(), + settings + ); + let selected = Daemon { + pid_file: legacy.pid_file.with_file_name(crate::DAEMON_PID_FILE_NAME), + update_pid_file: legacy + .update_pid_file + .with_file_name(crate::DAEMON_UPDATE_PID_FILE_NAME), + managed_codex_bin: dedicated.join("current/bin/codex"), + ..legacy.clone() + }; + let new_backend = crate::backend::pid_backend(selected.backend_paths(&daemon_settings)); + assert_eq!(new_backend.is_starting_or_running().await.unwrap(), running); + assert!(!old_backend.is_starting_or_running().await.unwrap()); + assert!(!old_updater.is_starting_or_running().await.unwrap()); + let new_updater = + crate::backend::pid_update_loop_backend(selected.backend_paths(&daemon_settings)); + assert_eq!(new_updater.is_starting_or_running().await.unwrap(), running); + new_updater.stop().await.unwrap(); + new_backend.stop().await.unwrap(); + if let Some(server) = server { + server.abort(); + } + } +} + impl FakeInstallerHttp { fn new(response: InstallerResponse) -> Self { Self { @@ -108,9 +237,12 @@ async fn cancelling_installer_stops_children_and_releases_fallback_lock() { }); let result = tokio::time::timeout( Duration::from_secs(10), - super::run_installer_script(script.as_bytes(), "0.150.0-test", home.path(), async { - cancelled.await.ok() - }), + super::run_installer_script( + script.as_bytes(), + super::InstallerMode::Update("0.150.0-test"), + &home.path().join("packages/standalone"), + async { cancelled.await.ok() }, + ), ) .await .expect("installer cancellation timed out") @@ -146,6 +278,8 @@ fn manual_update_daemon(home: &TempDir) -> (Daemon, String) { .expect("current release"); std::fs::write(standalone.join("auto-update-version"), &release).expect("latest marker"); let state = home.path().join("app-server-daemon"); + std::fs::create_dir(&state).unwrap(); + std::fs::write(state.join("app-server.stderr.log"), b"").unwrap(); ( Daemon { socket_path: home.path().join("app-server-control/server.sock"), @@ -313,30 +447,19 @@ async fn unsupported_request_preserves_updater_schedule() { } #[cfg(unix)] -#[tokio::test] -async fn manual_update_restarts_managed_daemon_with_automatic_updates_disabled() { +async fn test_control_server( + daemon: &Daemon, + home: &std::path::Path, +) -> tokio::task::JoinHandle<()> { use futures::SinkExt; use futures::StreamExt; - use tokio::io::AsyncReadExt; - use tokio::io::AsyncWriteExt; - - let home = TempDir::new().expect("home"); - let (daemon, release) = manual_update_daemon(&home); - let daemon = std::sync::Arc::new(daemon); - std::fs::create_dir_all(daemon.settings_file.parent().expect("state directory")) - .expect("state directory"); - std::fs::write( - &daemon.settings_file, - r#"{"updater":{"autoUpdateEnabled":false}}"#, - ) - .expect("disabled updater"); std::fs::create_dir_all(daemon.socket_path.parent().expect("socket parent")) .expect("socket directory"); let mut listener = codex_uds::UnixListener::bind(&daemon.socket_path) .await .expect("control listener"); - let codex_home = home.path().to_path_buf(); - let server = tokio::spawn(async move { + let codex_home = home.to_path_buf(); + tokio::spawn(async move { loop { let connection = listener.accept().await.expect("control connection"); let mut websocket = tokio_tungstenite::accept_async(connection) @@ -348,7 +471,7 @@ async fn manual_update_restarts_managed_daemon_with_automatic_updates_disabled() .expect("initialize request") .expect("frame"); let version = if std::fs::read_to_string( - codex_home.join("packages/standalone/auto-update-version"), + crate::managed_install::package_root(&codex_home).join("auto-update-version"), ) .expect("selected release") .starts_with("1.1.0") @@ -369,7 +492,26 @@ async fn manual_update_restarts_managed_daemon_with_automatic_updates_disabled() .expect("initialized notification") .expect("frame"); } - }); + }) +} + +#[cfg(unix)] +#[tokio::test] +async fn manual_update_restarts_managed_daemon_with_automatic_updates_disabled() { + use tokio::io::AsyncReadExt; + use tokio::io::AsyncWriteExt; + + let home = TempDir::new().expect("home"); + let (daemon, release) = manual_update_daemon(&home); + let daemon = std::sync::Arc::new(daemon); + std::fs::create_dir_all(daemon.settings_file.parent().expect("state directory")) + .expect("state directory"); + std::fs::write( + &daemon.settings_file, + r#"{"updater":{"autoUpdateEnabled":false}}"#, + ) + .expect("disabled updater"); + let server = test_control_server(&daemon, home.path()).await; let settings = crate::settings::DaemonSettings::default(); let backend = crate::backend::pid_backend(daemon.backend_paths(&settings)); backend.start().await.expect("start daemon"); @@ -492,6 +634,7 @@ async fn powershell_installer_is_noninteractive_and_reports_script_failure() { br#" function Test-Installer { if ($env:CODEX_NON_INTERACTIVE -ne '1') { throw 'interactive installer' } + if ($env:CODEX_INSTALL_DAEMON_ONLY -ne '1') { throw 'wrong package destination' } } Test-Installer "# @@ -500,9 +643,13 @@ Test-Installer let script = super::fetch_installer_script(&valid) .await .expect("fetch installer"); - super::run_installer_script(&script, "0.150.0-x86_64-pc-windows-msvc") - .await - .expect("installer succeeds"); + super::run_installer_script( + &script, + super::InstallerMode::Update("0.150.0-x86_64-pc-windows-msvc"), + std::path::Path::new("packages/app-server-daemon"), + ) + .await + .expect("installer succeeds"); let failing = FakeInstallerHttp::new(InstallerResponse::Success( b"throw 'installer failed'".to_vec(), )); @@ -510,8 +657,75 @@ Test-Installer .await .expect("fetch failing installer"); assert!( - super::run_installer_script(&script, "0.150.0-x86_64-pc-windows-msvc") - .await - .is_err() + super::run_installer_script( + &script, + super::InstallerMode::Update("0.150.0-x86_64-pc-windows-msvc"), + std::path::Path::new("packages/app-server-daemon") + ) + .await + .is_err() ); } + +#[cfg(unix)] +#[tokio::test] +async fn update_rejects_a_package_root_change_during_download() { + struct ChangingRoot<'a>(&'a std::path::Path); + impl InstallerHttp for ChangingRoot<'_> { + async fn get(&self, _url: &str) -> anyhow::Result { + std::os::unix::fs::symlink( + self.0.join("packages/standalone"), + self.0.join("packages/app-server-daemon"), + )?; + Ok(InstallerResponse::Success( + b"# CODEX_INSTALL_IF_LATEST\nexit 99\n".to_vec(), + )) + } + } + let home = TempDir::new().unwrap(); + let (daemon, _) = manual_update_daemon(&home); + let identity = executable_identity(&daemon.managed_codex_bin) + .await + .unwrap(); + let error = manual_update_once( + &ChangingRoot(home.path()), + &daemon, + &identity, + &mut test_terminate(), + ) + .await + .unwrap_err(); + assert!(error.to_string().contains("package root changed")); +} + +#[cfg(unix)] +#[tokio::test] +async fn daemon_owned_updates_require_and_request_an_isolated_installer() { + let home = TempDir::new().unwrap(); + let (mut daemon, _) = manual_update_daemon(&home); + let root = home.path().join("packages/app-server-daemon"); + std::fs::rename(home.path().join("packages/standalone"), &root).unwrap(); + daemon.managed_codex_bin = root.join("current/codex"); + let identity = executable_identity(&daemon.managed_codex_bin) + .await + .unwrap(); + let old = FakeInstallerHttp::new(InstallerResponse::Success( + b"# CODEX_INSTALL_IF_LATEST\nexit 0\n".to_vec(), + )); + let error = manual_update_once(&old, &daemon, &identity, &mut test_terminate()) + .await + .unwrap_err(); + assert!( + error + .to_string() + .contains("does not support daemon-owned packages") + ); + let current = FakeInstallerHttp::new(InstallerResponse::Success( + b"# CODEX_INSTALL_IF_LATEST\ntest \"$CODEX_INSTALL_DAEMON_ONLY\" = 1\n".to_vec(), + )); + let output = manual_update_once(¤t, &daemon, &identity, &mut test_terminate()) + .await + .unwrap(); + assert_eq!(output.status, UpdateStatus::NoUpdate); + assert!(!home.path().join("packages/standalone").exists()); +} diff --git a/codex-rs/cli/src/doctor/background.rs b/codex-rs/cli/src/doctor/background.rs index 7a4dc516c3..b6d13de058 100644 --- a/codex-rs/cli/src/doctor/background.rs +++ b/codex-rs/cli/src/doctor/background.rs @@ -55,6 +55,16 @@ pub(super) async fn background_server_check(config: &Config) -> DoctorCheck { "update-loop pid file", &state_dir.join(UPDATE_PID_FILE_NAME), ); + push_file_detail( + &mut details, + "dedicated pid file", + &state_dir.join("daemon.pid"), + ); + push_file_detail( + &mut details, + "dedicated update-loop pid file", + &state_dir.join("daemon-updater.pid"), + ); push_configured_updater(&mut details, &state_dir.join(SETTINGS_FILE_NAME)); let socket_path = match codex_app_server::app_server_control_socket_path(&config.codex_home) { @@ -322,6 +332,7 @@ mod tests { detail.starts_with("automatic updates:") || detail.starts_with("update interval:") || detail.starts_with("update-loop pid file:") + || detail.starts_with("dedicated") }) .map(|detail| { detail diff --git a/codex-rs/cli/src/doctor/snapshots/codex__doctor__background__tests__configured_updater.snap b/codex-rs/cli/src/doctor/snapshots/codex__doctor__background__tests__configured_updater.snap index 63f0df4058..478f044f1e 100644 --- a/codex-rs/cli/src/doctor/snapshots/codex__doctor__background__tests__configured_updater.snap +++ b/codex-rs/cli/src/doctor/snapshots/codex__doctor__background__tests__configured_updater.snap @@ -1,8 +1,9 @@ --- source: cli/src/doctor/background.rs -assertion_line: 332 expression: details --- update-loop pid file: CODEX_HOME/app-server-daemon/app-server-updater.pid (missing) +dedicated pid file: CODEX_HOME/app-server-daemon/daemon.pid (missing) +dedicated update-loop pid file: CODEX_HOME/app-server-daemon/daemon-updater.pid (missing) automatic updates: disabled (configured) update interval: 90 minutes (configured) diff --git a/codex-rs/cli/src/main.rs b/codex-rs/cli/src/main.rs index 9e8e014d14..68de6ec0d8 100644 --- a/codex-rs/cli/src/main.rs +++ b/codex-rs/cli/src/main.rs @@ -809,7 +809,11 @@ enum AppServerDaemonSubcommand { /// [internal] Run the detached pid-backed standalone updater loop. #[clap(hide = true)] - PidUpdateLoop, + PidUpdateLoop { + /// Check support for daemon-owned packages without starting the updater. + #[arg(long, hide = true)] + check_package_ownership: bool, + }, } #[derive(Debug, Args)] @@ -1392,10 +1396,6 @@ async fn cli_main( AppServerDaemonSubcommand::Restart => { print_app_server_daemon_output(AppServerLifecycleCommand::Restart).await?; } - AppServerDaemonSubcommand::Update => { - let output = codex_app_server_daemon::update().await?; - println!("{}", serde_json::to_string(&output)?); - } AppServerDaemonSubcommand::EnableRemoteControl => { print_app_server_remote_control_output(AppServerRemoteControlMode::Enabled) .await?; @@ -1412,7 +1412,13 @@ async fn cli_main( AppServerDaemonSubcommand::Version => { print_app_server_daemon_output(AppServerLifecycleCommand::Version).await?; } - AppServerDaemonSubcommand::PidUpdateLoop => { + AppServerDaemonSubcommand::PidUpdateLoop { + check_package_ownership: true, + } => return Ok(()), + AppServerDaemonSubcommand::Update + | AppServerDaemonSubcommand::PidUpdateLoop { + check_package_ownership: false, + } => { let cli_overrides = root_config_overrides .parse_overrides() .map_err(anyhow::Error::msg)?; @@ -1422,7 +1428,14 @@ async fn cli_main( .await .map_err(anyhow::Error::from); let http_client_factory = updater_http_client_factory(config); - codex_app_server_daemon::run_pid_update_loop(http_client_factory).await?; + if matches!(daemon_cli.subcommand, AppServerDaemonSubcommand::Update) { + let output = + codex_app_server_daemon::update(http_client_factory).await?; + println!("{}", serde_json::to_string(&output)?); + } else { + codex_app_server_daemon::run_pid_update_loop(http_client_factory) + .await?; + } } }, Some(AppServerSubcommand::Proxy(proxy_cli)) => { @@ -2643,7 +2656,7 @@ fn app_server_subcommand_name(subcommand: Option<&AppServerSubcommand>) -> &'sta } AppServerDaemonSubcommand::Stop => "app-server daemon stop", AppServerDaemonSubcommand::Version => "app-server daemon version", - AppServerDaemonSubcommand::PidUpdateLoop => "app-server daemon pid-update-loop", + AppServerDaemonSubcommand::PidUpdateLoop { .. } => "app-server daemon pid-update-loop", }, Some(AppServerSubcommand::Proxy(_)) => "app-server proxy", Some(AppServerSubcommand::GenerateTs(_)) => "app-server generate-ts", diff --git a/codex-rs/cli/tests/app_server_daemon.rs b/codex-rs/cli/tests/app_server_daemon.rs index 79ab3b3be7..bd90b5b59c 100644 --- a/codex-rs/cli/tests/app_server_daemon.rs +++ b/codex-rs/cli/tests/app_server_daemon.rs @@ -44,6 +44,10 @@ impl TestDaemon { PathBuf::from("releases").join(release_name), standalone.join("current"), )?; + // Model a daemon that was previously launched and is currently stopped. + let state = home.path().join("app-server-daemon"); + std::fs::create_dir(&state)?; + std::fs::write(state.join("app-server.stderr.log"), b"")?; Ok(Self { home, codex, @@ -118,6 +122,29 @@ fn wait_for_exit(pid: u32) -> Result<()> { } } +#[test] +fn package_ownership_check_does_not_start_an_updater() -> Result<()> { + let daemon = TestDaemon::new()?; + let output = daemon + .command() + .args([ + "app-server", + "daemon", + "pid-update-loop", + "--check-package-ownership", + ]) + .output()?; + ensure!( + output.status.success(), + "ownership check failed: {}", + String::from_utf8_lossy(&output.stderr) + ); + let state = daemon.home.path().join("app-server-daemon"); + assert!(!state.join("app-server-updater.pid").exists()); + assert!(!state.join("daemon-updater.pid").exists()); + Ok(()) +} + #[test] fn managed_starts_ensure_one_updater_and_recover_a_missing_one() -> Result<()> { let daemon = TestDaemon::new()?; @@ -139,6 +166,22 @@ fn managed_starts_ensure_one_updater_and_recover_a_missing_one() -> Result<()> { assert_eq!(daemon.lifecycle("restart")?["status"], "restarted"); assert_ne!(daemon.pid("app-server.pid")?, backend_pid); assert_eq!(daemon.pid("app-server-updater.pid")?, replacement_pid); + daemon.lifecycle("stop")?; + assert!( + !daemon + .home + .path() + .join("app-server-daemon/app-server.pid") + .exists() + ); + assert_eq!(daemon.lifecycle("start")?["status"], "started"); + assert!( + !daemon + .home + .path() + .join("packages/app-server-daemon") + .exists() + ); Ok(()) } diff --git a/scripts/install/install.ps1 b/scripts/install/install.ps1 index 229663b166..535bcc31f5 100644 --- a/scripts/install/install.ps1 +++ b/scripts/install/install.ps1 @@ -903,9 +903,15 @@ $codexHome = if ([string]::IsNullOrWhiteSpace($env:CODEX_HOME)) { } else { $env:CODEX_HOME } -$standaloneRoot = Join-Path $codexHome "packages\standalone" +$daemonOnly = $env:CODEX_INSTALL_DAEMON_ONLY -eq "1" +$standaloneRoot = Join-Path $codexHome $(if ($daemonOnly) { "packages\app-server-daemon" } else { "packages\standalone" }) $releasesDir = Join-Path $standaloneRoot "releases" $currentDir = Join-Path $standaloneRoot "current" +$deferSelection = $env:CODEX_INSTALL_DEFER_SELECTION -eq "1" +if ($deferSelection) { + if (-not $daemonOnly) { throw "Deferred selection requires a daemon-only installation." } + $currentDir = Join-Path $standaloneRoot ".migration-current" +} $autoUpdateVersion = Join-Path $standaloneRoot "auto-update-version" $lockPath = Join-Path $standaloneRoot "install.lock" @@ -933,7 +939,7 @@ if (-not [string]::IsNullOrWhiteSpace($currentVersion) -and $currentVersion -ne Write-Step "Detected platform: $platformLabel" Write-Step "Resolved version: $resolvedVersion" -$conflictingInstall = Get-ConflictingInstall -VisibleBinDir $visibleBinDir +$conflictingInstall = if ($daemonOnly) { $null } else { Get-ConflictingInstall -VisibleBinDir $visibleBinDir } $oldStandaloneBackup = $null $checksumAsset = "codex-package_SHA256SUMS" @@ -949,6 +955,7 @@ $guardRejected = $false try { Invoke-WithInstallLock -LockPath $lockPath -Script { $updaterRecord = Join-Path $codexHome "app-server-daemon\app-server-updater.pid" + if ($daemonOnly) { $updaterRecord = Join-Path $codexHome "app-server-daemon\daemon-updater.pid" } $oldUpdaterParent = $false if ($Release -eq "latest" -and $env:CODEX_INSTALL_IF_LATEST -ne "1" -and (Test-Path -LiteralPath $updaterRecord)) { $updaterPid = $null @@ -997,6 +1004,7 @@ try { if (-not (Test-ReleaseIsComplete -ReleaseDir $releaseDir -ExpectedVersion $resolvedVersion -ExpectedTarget $target -Layout $installLayout)) { if (Test-Path -LiteralPath $releaseDir) { + if ($daemonOnly) { throw "Refusing to overwrite existing daemon release $releaseDir." } Write-WarningStep "Found incomplete existing release at $releaseDir. Reinstalling." } @@ -1058,6 +1066,9 @@ try { } New-Item -ItemType Directory -Force -Path $standaloneRoot | Out-Null + if ($deferSelection -and (Get-Item -LiteralPath (Join-Path $standaloneRoot "current") -Force -ErrorAction SilentlyContinue)) { + throw "A dedicated daemon is already selected; retry the update." + } Ensure-Junction -LinkPath $currentDir -TargetPath $releaseDir -InstallerOwnedTargetPrefix $releasesDir if ($Release -eq "latest") { $tempMarker = "$autoUpdateVersion.tmp.$PID" @@ -1069,6 +1080,7 @@ try { } } + if ($daemonOnly) { return } $visibleParent = Split-Path -Parent $visibleBinDir $currentBinDir = if ($installLayout -eq "Package") { Join-Path $currentDir "bin" @@ -1096,7 +1108,7 @@ try { } finally { Remove-Item -Recurse -Force $tempDir -ErrorAction SilentlyContinue } -if ($guardRejected) { return } +if ($guardRejected -or $daemonOnly) { return } Maybe-HandleConflictingInstall -Conflict $conflictingInstall diff --git a/scripts/install/install.sh b/scripts/install/install.sh index 4a2b4ece49..f230ac0586 100755 --- a/scripts/install/install.sh +++ b/scripts/install/install.sh @@ -4,6 +4,7 @@ set -eu RELEASE="${CODEX_RELEASE:-latest}" NON_INTERACTIVE="${CODEX_NON_INTERACTIVE:-false}" +DAEMON_ONLY="${CODEX_INSTALL_DAEMON_ONLY:-0}" DEFAULT_PREFER_RELEASES_OPENAI_COM="true" PREFER_RELEASES_OPENAI_COM="${CODEX_INSTALLER_USE_RELEASES_OPENAI_COM:-$DEFAULT_PREFER_RELEASES_OPENAI_COM}" RELEASES_BASE_URL="https://releases.openai.com/codex" @@ -17,8 +18,18 @@ BIN_PATH="$BIN_DIR/codex" CODE_MODE_HOST_BIN_PATH="$BIN_DIR/codex-code-mode-host" CODEX_HOME_DIR="${CODEX_HOME:-$HOME/.codex}" STANDALONE_ROOT="$CODEX_HOME_DIR/packages/standalone" +if [ "$DAEMON_ONLY" = "1" ]; then + STANDALONE_ROOT="$CODEX_HOME_DIR/packages/app-server-daemon" +fi RELEASES_DIR="$STANDALONE_ROOT/releases" CURRENT_LINK="$STANDALONE_ROOT/current" +if [ "${CODEX_INSTALL_DEFER_SELECTION:-0}" = "1" ]; then + if [ "$DAEMON_ONLY" != "1" ]; then + echo "Deferred selection requires a daemon-only installation." >&2 + exit 1 + fi + CURRENT_LINK="$STANDALONE_ROOT/.migration-current" +fi AUTO_UPDATE_VERSION="$STANDALONE_ROOT/auto-update-version" LOCK_FILE="$STANDALONE_ROOT/install.lock" LOCK_DIR="$STANDALONE_ROOT/install.lock.d" @@ -742,7 +753,7 @@ cleanup_stale_install_artifacts() { find "$RELEASES_DIR" -mindepth 1 -maxdepth 1 -name '.staging.*' -exec rm -rf {} + find "$STANDALONE_ROOT" -mindepth 1 -maxdepth 1 -name '.current.*' -exec rm -f {} + - if [ -d "$BIN_DIR" ]; then + if [ "$DAEMON_ONLY" != "1" ] && [ -d "$BIN_DIR" ]; then find "$BIN_DIR" -mindepth 1 -maxdepth 1 -name '.codex.*' -exec rm -f {} + fi } @@ -1140,7 +1151,9 @@ fi step "Detected platform: $platform_label" step "Resolved version: $resolved_version" -detect_conflicting_install +if [ "$DAEMON_ONLY" != "1" ]; then + detect_conflicting_install +fi tmp_dir="$(mktemp -d)" cleanup() { @@ -1154,7 +1167,15 @@ trap 'exit 130' INT trap 'exit 143' TERM acquire_install_lock +if [ "${CODEX_INSTALL_DEFER_SELECTION:-0}" = "1" ] && + { [ -e "$STANDALONE_ROOT/current" ] || [ -L "$STANDALONE_ROOT/current" ]; }; then + echo "A dedicated daemon is already selected; retry the update." >&2 + exit 1 +fi updater_record="$CODEX_HOME_DIR/app-server-daemon/app-server-updater.pid" +if [ "$DAEMON_ONLY" = "1" ]; then + updater_record="$CODEX_HOME_DIR/app-server-daemon/daemon-updater.pid" +fi old_updater_parent="false" if [ "${CODEX_INSTALL_IF_LATEST:-}" != "1" ] && [ -f "$updater_record" ]; then updater_pid="$(sed -n 's/.*"pid"[[:space:]]*:[[:space:]]*\([0-9][0-9]*\).*/\1/p' "$updater_record" | head -n 1)" @@ -1197,6 +1218,10 @@ cleanup_stale_install_artifacts if ! release_dir_is_complete "$release_dir" "$resolved_version" "$vendor_target" "$install_layout"; then if [ -e "$release_dir" ] || [ -L "$release_dir" ]; then + if [ "$DAEMON_ONLY" = "1" ]; then + echo "Refusing to overwrite existing daemon release $release_dir." >&2 + exit 1 + fi warn "Found incomplete existing release at $release_dir; reinstalling." fi @@ -1231,6 +1256,10 @@ if [ "$RELEASE" = "latest" ]; then else rm -f "$AUTO_UPDATE_VERSION" fi +if [ "$DAEMON_ONLY" = "1" ]; then + release_install_lock + exit 0 +fi update_visible_command "$release_dir" add_to_path verify_visible_command diff --git a/scripts/install/test_install_sh.py b/scripts/install/test_install_sh.py index a754ab21cf..5ac63ce4fa 100644 --- a/scripts/install/test_install_sh.py +++ b/scripts/install/test_install_sh.py @@ -156,6 +156,40 @@ class InstallShTest(unittest.TestCase): ], ) + def test_daemon_install_preserves_visible_cli_and_profile(self) -> None: + with tempfile.TemporaryDirectory() as temp_dir: + root = Path(temp_dir).resolve() + archive, checksum, metadata = create_package_release(root) + options = dict( + metadata_json=metadata, + archive_path=archive, + checksum_path=checksum, + force_macos=True, + ) + pinned, _ = run_installer_in(root, VERSION, **options) + self.assertEqual(pinned.returncode, 0, pinned.stderr) + visible = root / "install-bin/codex" + original = visible.resolve() + pending = visible.with_name(".codex.12345") + pending.symlink_to(original) + profile = root / "home/.profile" + profile.write_text("unchanged profile\n") + + installed, _ = run_installer_in(root, "latest", daemon_only=True, **options) + self.assertEqual(installed.returncode, 0, installed.stderr) + daemon = root / "codex-home/packages/app-server-daemon" + release_name = f"{VERSION}-aarch64-apple-darwin" + self.assertEqual( + (daemon / "current").resolve(), daemon / "releases" / release_name + ) + self.assertEqual((daemon / "auto-update-version").read_text(), release_name) + self.assertEqual(visible.resolve(), original) + self.assertTrue(pending.is_symlink()) + self.assertEqual(profile.read_text(), "unchanged profile\n") + self.assertFalse( + (root / "codex-home/packages/standalone/auto-update-version").exists() + ) + def test_explicit_release_pins_even_the_current_latest_version(self) -> None: with tempfile.TemporaryDirectory() as temp_dir: root = Path(temp_dir) @@ -637,6 +671,7 @@ def run_installer_in( update_guard_from_release: str | None = None, old_updater_parent_pid: int | None = None, fail_ps: bool = False, + daemon_only: bool = False, ) -> tuple[subprocess.CompletedProcess[str], list[str]]: bin_dir = root / "bin" bin_dir.mkdir(exist_ok=True) @@ -771,6 +806,7 @@ def run_installer_in( "CODEX_INSTALL_DIR": str(root / "install-bin"), "CODEX_NON_INTERACTIVE": "1", "CODEX_RELEASE": release, + "CODEX_INSTALL_DAEMON_ONLY": "1" if daemon_only else "0", "CODEX_TEST_ARCHIVE_PATH": str(archive_path or ""), "CODEX_TEST_CHECKSUM_PATH": str(checksum_path or ""), "CODEX_TEST_RELEASES_CHECKSUM_PATH": str(