From adee0b04fa27a8ba5d2e3612b900363cffe72930 Mon Sep 17 00:00:00 2001 From: Eric Traut Date: Mon, 7 Sep 2026 16:47:41 +0000 Subject: [PATCH] Preserve standalone release pins during daemon updates (#43521) ## Why Bootstrapping the app-server daemon should preserve an explicitly selected release, even when that version is currently `latest`. Older managed binaries should also be able to serve app-server without supporting the updater command. ## What changed - Record `latest` selections in `auto-update-version` in both standalone installers and clear the marker for explicit releases. - Start the daemon updater only for a marked stable release whose binary supports `pid-update-loop`. Existing installs without a marker require a new `latest` installation to enable automatic updates. - Recheck the selected release under the install lock so an in-flight update cannot overwrite a new pin, including installer calls from older updaters. Recheck selection before restarting app-server or replacing the updater. - Cancel Unix installer process groups and clean up their owned fallback locks when the updater stops. ## Testing Add coverage for channel markers, explicit pins of the current latest version, older updater guards, updater command support, and Unix installer cancellation with child-process and lock cleanup. GitOrigin-RevId: 4d275237bd77d896bf64dc1b85a5bca3608142c7 --- codex-rs/app-server-daemon/README.md | 18 ++- codex-rs/app-server-daemon/src/lib.rs | 81 +++++++++-- .../app-server-daemon/src/managed_install.rs | 61 ++++++++ .../src/managed_install_path_tests.rs | 61 ++++++++ codex-rs/app-server-daemon/src/update_loop.rs | 125 ++++++++++++++--- .../src/update_loop_tests.rs | 58 +++++++- scripts/install/install.ps1 | 57 ++++++++ scripts/install/install.sh | 50 ++++++- scripts/install/test_install_sh.py | 130 ++++++++++++++++++ 9 files changed, 608 insertions(+), 33 deletions(-) diff --git a/codex-rs/app-server-daemon/README.md b/codex-rs/app-server-daemon/README.md index 5417366519..226969c383 100644 --- a/codex-rs/app-server-daemon/README.md +++ b/codex-rs/app-server-daemon/README.md @@ -63,7 +63,9 @@ $codexHome = if ($env:CODEX_HOME) { $env:CODEX_HOME } else { Join-Path $HOME '.c `bootstrap` requires the standalone managed install. It records the daemon settings under `CODEX_HOME/app-server-daemon/`, starts app-server as a -pidfile-backed detached process, and launches a detached updater loop. +pidfile-backed detached process. It launches a detached updater loop when the +installer selected the stable `latest` channel and the managed binary supports +the updater command. ## Installation and update cases @@ -74,7 +76,8 @@ on Windows) and its managed binary under `CODEX_HOME/packages/standalone/current | Situation | What starts | Does this daemon fetch new binaries? | Does a running app-server eventually move to a newer binary on its own? | | --- | --- | --- | --- | | Installer has run; only `start` is used | Managed binary | No | No; explicit restart is required. | -| Installer has run; `bootstrap` is used | Managed binary and detached updater | Yes; the platform's installer runs hourly. | Yes; after a successful update, a running app-server restarts with the new binary before the updater replaces itself. | +| Latest-channel installer has run; `bootstrap` is used | Managed binary and detached updater when supported | Yes; the platform's installer runs hourly. | Yes; after a successful update, a running app-server restarts with the new binary before the updater replaces itself. | +| Installer selected an explicit release; `bootstrap` is used | Managed binary only | No; the selected release stays pinned. | No; an explicit restart uses the selected binary. | | Another tool updates the managed binary | Next start or restart uses it | Only with `bootstrap`, on its normal cadence. | With `bootstrap`, the next successful installer pass compares binary contents and refreshes a running app-server before the updater. | ### Standalone installs @@ -83,8 +86,15 @@ For installs created by either platform's standalone installer: - lifecycle commands always use the standalone managed binary path - `bootstrap` is supported -- `bootstrap` starts a detached pid-backed updater loop that fetches via - the platform's installer +- `bootstrap` starts a detached pid-backed updater loop only for a stable + latest-channel release whose managed binary supports the updater command +- the installer records the latest-channel selection alongside `current`; + selecting an explicit release clears it, even if that version is currently + latest. The updater checks the selection again while holding the install lock + so an in-flight update cannot override a new pin +- installs made before the installer recorded channel selections need one new + `latest` installation to opt into automatic updates; until then the daemon + continues to serve app-server without updating the selected release - after a successful refresh, if app-server is running and the managed binary contents changed, the updater restarts app-server with that binary first and only then replaces its own process image diff --git a/codex-rs/app-server-daemon/src/lib.rs b/codex-rs/app-server-daemon/src/lib.rs index a9cd34f062..12b2c26839 100644 --- a/codex-rs/app-server-daemon/src/lib.rs +++ b/codex-rs/app-server-daemon/src/lib.rs @@ -394,6 +394,18 @@ impl Daemon { } else { None }; + // The installer can retarget `current` while the updater waits for + // this lock or probes the running server. Never restart from a + // release that is no longer the selected latest-channel binary. + if !self.is_stable_standalone_release()? + || managed_install::resolved_managed_codex_bin(&self.managed_codex_bin) + .await + .ok() + .as_deref() + != Some(managed_codex_bin) + { + return Ok(RestartIfRunningOutcome::AlreadyCurrent); + } match restart_decision(mode, info.as_ref(), managed_version.as_deref()) { RestartDecision::NotReady => return Ok(RestartIfRunningOutcome::NotReady), RestartDecision::AlreadyCurrent => RestartIfRunningOutcome::AlreadyCurrent, @@ -401,9 +413,7 @@ impl Daemon { #[cfg(windows)] backend::windows::ensure_detached_launch(managed_codex_bin)?; backend.stop().await?; - let _ = self - .start_managed_backend_with_bin(&settings, managed_codex_bin) - .await?; + let _ = self.start_managed_backend(&settings).await?; self.wait_until_ready().await?; RestartIfRunningOutcome::Restarted } @@ -416,6 +426,15 @@ impl Daemon { RestartIfRunningOutcome::NotRunning }; + if !self.is_stable_standalone_release()? + || managed_install::resolved_managed_codex_bin(&self.managed_codex_bin) + .await + .ok() + .as_deref() + != Some(managed_codex_bin) + { + return Ok(RestartIfRunningOutcome::AlreadyCurrent); + } #[cfg(unix)] if should_reexec_updater(updater_refresh_mode, outcome) { crate::update_loop::reexec_managed_updater(managed_codex_bin)?; @@ -634,18 +653,16 @@ impl Daemon { let backend = backend::pid_backend(self.backend_paths(&settings)); backend.start().await?; - let updater = backend::pid_update_loop_backend(self.backend_paths(&settings)); - if updater.is_starting_or_running().await? { - updater.stop().await?; - } - updater.start().await?; - let info = self.wait_until_ready().await?; + backend::pid_update_loop_backend(self.backend_paths(&settings)) + .stop() + .await?; + let auto_update_enabled = self.ensure_managed_updater(&settings).await?; let managed_codex_version = self.managed_codex_version_best_effort().await; Ok(BootstrapOutput { status: BootstrapStatus::Bootstrapped, backend: BackendKind::Pid, - auto_update_enabled: true, + auto_update_enabled, remote_control_enabled: settings.remote_control_enabled, managed_codex_path: self.managed_codex_bin.clone(), managed_codex_version, @@ -688,7 +705,51 @@ impl Daemon { backend.start().await } + async fn ensure_managed_updater(&self, settings: &DaemonSettings) -> Result { + let updater = backend::pid_update_loop_backend(self.backend_paths(settings)); + if !self.is_stable_standalone_release()? { + updater.stop().await?; + return Ok(false); + } + let Ok(codex_bin) = + managed_install::resolved_managed_codex_bin(&self.managed_codex_bin).await + else { + updater.stop().await?; + return Ok(false); + }; + if !managed_install::supports_daemon_update_loop(&codex_bin).await + || !self.is_stable_standalone_release()? + || !managed_install::resolved_managed_codex_bin(&self.managed_codex_bin) + .await + .is_ok_and(|selected| selected == codex_bin) + { + updater.stop().await?; + return Ok(false); + } + backend::pid_update_loop_backend(self.backend_paths_with_bin(settings, &codex_bin)) + .start() + .await?; + Ok(true) + } + + fn is_stable_standalone_release(&self) -> Result { + let codex_home = self + .settings_file + .parent() + .and_then(Path::parent) + .context("daemon settings path has no Codex home")?; + Ok(managed_install::is_stable_standalone_release( + codex_home, + &self.managed_codex_bin, + )) + } + async fn is_bootstrapped(&self, settings: &DaemonSettings) -> Result { + if !self.is_stable_standalone_release()? + || !managed_install::supports_daemon_update_loop(&self.managed_codex_bin).await + { + return Ok(self.running_backend_instance(settings).await?.is_some()); + } let updater = backend::pid_update_loop_backend(self.backend_paths(settings)); updater.is_starting_or_running().await } diff --git a/codex-rs/app-server-daemon/src/managed_install.rs b/codex-rs/app-server-daemon/src/managed_install.rs index c0a19ac8a0..7db54c470f 100644 --- a/codex-rs/app-server-daemon/src/managed_install.rs +++ b/codex-rs/app-server-daemon/src/managed_install.rs @@ -2,6 +2,8 @@ use std::path::Path; use std::path::PathBuf; +use std::process::Stdio; +use std::time::Duration; use anyhow::Context; use anyhow::Result; @@ -10,6 +12,7 @@ use sha2::Digest; use sha2::Sha256; 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 @@ -30,6 +33,64 @@ 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 Ok(releases) = std::fs::canonicalize(standalone.join("releases")) else { + return false; + }; + let Ok(release) = std::fs::canonicalize(standalone.join("current")) else { + return false; + }; + if release.parent() != Some(releases.as_path()) { + return false; + } + let Some(release_name) = release.file_name().and_then(|name| name.to_str()) else { + return false; + }; + let targets = [ + "aarch64-apple-darwin", + "x86_64-apple-darwin", + "aarch64-unknown-linux-musl", + "x86_64-unknown-linux-musl", + "aarch64-pc-windows-msvc", + "x86_64-pc-windows-msvc", + ]; + let Some(version) = targets + .iter() + .find_map(|target| release_name.strip_suffix(&format!("-{target}"))) + else { + return false; + }; + let components: Vec<_> = version.split('.').collect(); + components.len() == 3 + && components.iter().all(|component| { + !component.is_empty() && component.bytes().all(|byte| byte.is_ascii_digit()) + }) + && std::fs::read_to_string(standalone.join("auto-update-version")) + .is_ok_and(|selected| selected == release_name) + && std::fs::canonicalize(codex_bin).is_ok_and(|bin| bin.starts_with(&release)) +} + +/// Older managed binaries can serve app-server requests without owning an updater. +pub(crate) async fn supports_daemon_update_loop(codex_bin: &Path) -> 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"]) + .stdin(Stdio::null()) + .stdout(Stdio::null()) + .stderr(Stdio::null()) + .kill_on_drop(true) + .status(), + ) + .await + .is_ok_and(|result| result.is_ok_and(|status| status.success())) +} + pub(crate) async fn resolved_managed_codex_bin(codex_bin: &Path) -> Result { fs::canonicalize(codex_bin).await.with_context(|| { format!( 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 6c7ff5b3b0..7e49e80fb8 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 @@ -19,3 +19,64 @@ fn discovers_package_and_legacy_installs() { std::fs::write(&packaged, b"packaged").expect("packaged executable"); assert_eq!(super::managed_codex_bin(home.path()), packaged); } + +#[cfg(unix)] +#[test] +fn updater_only_runs_for_stable_installer_owned_releases() { + let home = tempfile::TempDir::new().expect("home"); + let standalone = home.path().join("packages/standalone"); + let current = standalone.join("current"); + let release = standalone.join("releases/0.150.0-aarch64-apple-darwin"); + let managed = release.join("bin/codex"); + std::fs::create_dir_all(managed.parent().expect("bin parent")).expect("release"); + std::fs::write(&managed, b"stable").expect("managed bin"); + std::os::unix::fs::symlink(&release, ¤t).expect("current release"); + assert!(!super::is_stable_standalone_release(home.path(), &managed)); + let marker = standalone.join("auto-update-version"); + let release_name = release.file_name().expect("release name"); + std::fs::write(&marker, release_name.as_encoded_bytes()).expect("latest selection"); + assert!(super::is_stable_standalone_release(home.path(), &managed)); + std::fs::write(&marker, b"0.149.0-aarch64-apple-darwin").expect("stale selection"); + assert!(!super::is_stable_standalone_release(home.path(), &managed)); + std::fs::remove_file(&marker).expect("pinned selection"); + assert!(!super::is_stable_standalone_release(home.path(), &managed)); + + let alpha = standalone.join("releases/0.151.0-alpha.1-aarch64-apple-darwin"); + let alpha_managed = alpha.join("bin/codex"); + std::fs::create_dir_all(alpha_managed.parent().expect("alpha bin parent")) + .expect("alpha release"); + std::fs::write(&alpha_managed, b"alpha").expect("alpha bin"); + std::fs::remove_file(¤t).expect("remove current"); + std::os::unix::fs::symlink(alpha, ¤t).expect("current alpha"); + assert!(!super::is_stable_standalone_release( + home.path(), + &alpha_managed + )); + + let local = standalone.join("local-main"); + let local_managed = local.join("bin/codex"); + std::fs::create_dir_all(local_managed.parent().expect("local bin parent")) + .expect("local build"); + std::fs::write(&local_managed, b"local").expect("local bin"); + std::fs::remove_file(¤t).expect("remove current"); + std::os::unix::fs::symlink(local, ¤t).expect("current local build"); + assert!(!super::is_stable_standalone_release( + home.path(), + &local_managed + )); +} + +#[cfg(unix)] +#[tokio::test] +async fn older_managed_binary_does_not_claim_updater_support() { + use std::os::unix::fs::PermissionsExt; + + let temp = tempfile::TempDir::new().expect("home"); + let binary = temp.path().join("codex"); + std::fs::write(&binary, b"#!/bin/sh\nexit 2\n").expect("older binary"); + std::fs::set_permissions(&binary, std::fs::Permissions::from_mode(0o755)) + .expect("executable binary"); + assert!(!super::supports_daemon_update_loop(&binary).await); + std::fs::write(&binary, b"#!/bin/sh\nexit 0\n").expect("newer binary"); + assert!(super::supports_daemon_update_loop(&binary).await); +} diff --git a/codex-rs/app-server-daemon/src/update_loop.rs b/codex-rs/app-server-daemon/src/update_loop.rs index 06c9b01529..60ae39c977 100644 --- a/codex-rs/app-server-daemon/src/update_loop.rs +++ b/codex-rs/app-server-daemon/src/update_loop.rs @@ -1,5 +1,6 @@ //! Installs updates, validates the server restart, then transfers updater ownership. +use std::path::Path; #[cfg(unix)] use std::process::Command as StdCommand; use std::process::Stdio; @@ -98,15 +99,52 @@ async fn update_once( running_updater_identity: &ExecutableIdentity, terminate: &mut Signal, ) -> Result { + let daemon = Daemon::from_environment()?; + if !daemon.is_stable_standalone_release()? { + // An installer can be between changing current and publishing its + // latest-channel marker. Retry after the interval instead of exiting. + return Ok(UpdateLoopControl::Continue); + } + let codex_home = daemon + .settings_file + .parent() + .and_then(Path::parent) + .context("daemon settings path has no Codex home")?; + let current = std::fs::canonicalize(codex_home.join("packages/standalone/current"))?; + let previous_release = current + .file_name() + .context("managed release has no name")? + .to_string_lossy() + .into_owned(); + let script = tokio::select! { + result = fetch_installer_script(http) => result?, + _ = terminate.recv() => return Ok(UpdateLoopControl::Stop), + }; + anyhow::ensure!( + script + .windows(b"CODEX_INSTALL_IF_LATEST".len()) + .any(|window| window == b"CODEX_INSTALL_IF_LATEST"), + "standalone installer does not support guarded updates" + ); + if !daemon.is_stable_standalone_release()? { + return Ok(UpdateLoopControl::Continue); + } #[cfg(unix)] - install_latest_standalone(http).await?; + if matches!( + run_installer_script(&script, &previous_release, codex_home, terminate.recv()).await?, + UpdateLoopControl::Stop + ) { + return Ok(UpdateLoopControl::Stop); + } #[cfg(windows)] tokio::select! { - result = install_latest_standalone(http) => result?, + result = run_installer_script(&script, &previous_release) => { result?; }, _ = terminate.recv() => return Ok(UpdateLoopControl::Stop), } + if !daemon.is_stable_standalone_release()? { + return Ok(UpdateLoopControl::Continue); + } - let daemon = Daemon::from_environment()?; let managed_codex_bin = resolved_managed_codex_bin(&daemon.managed_codex_bin).await?; let managed_identity = executable_identity(&managed_codex_bin).await?; let (restart_mode, updater_refresh_mode) = @@ -134,7 +172,13 @@ async fn update_once( } RestartIfRunningOutcome::NotRunning | RestartIfRunningOutcome::NotReady - | RestartIfRunningOutcome::AlreadyCurrent => return Ok(UpdateLoopControl::Continue), + | RestartIfRunningOutcome::AlreadyCurrent => { + return Ok(if daemon.is_stable_standalone_release()? { + UpdateLoopControl::Continue + } else { + UpdateLoopControl::Stop + }); + } } } } @@ -172,13 +216,17 @@ pub(crate) fn reexec_managed_updater(managed_codex_bin: &std::path::Path) -> Res }) } -async fn install_latest_standalone(http: &impl InstallerHttp) -> Result<()> { - let script = fetch_installer_script(http).await?; - +async fn run_installer_script( + script: &[u8], + previous_release: &str, + #[cfg(unix)] codex_home: &Path, + #[cfg(unix)] terminate: impl std::future::Future>, +) -> Result { #[cfg(unix)] let mut command = { let mut command = Command::new("/bin/sh"); command.arg("-s"); + command.process_group(0); command }; #[cfg(windows)] @@ -191,6 +239,10 @@ async fn install_latest_standalone(http: &impl InstallerHttp) -> Result<()> { command }; let mut child = command + .env("CODEX_RELEASE", "latest") + .env("CODEX_INSTALL_IF_LATEST", "1") + .env("CODEX_UPDATE_FROM_RELEASE", previous_release) + .kill_on_drop(true) .stdin(Stdio::piped()) .stdout(Stdio::null()) .stderr(Stdio::null()) @@ -200,23 +252,64 @@ async fn install_latest_standalone(http: &impl InstallerHttp) -> Result<()> { .stdin .take() .context("standalone Codex updater stdin was unavailable")?; - stdin - .write_all(&script) - .await - .context("failed to pass standalone Codex updater to shell")?; + #[cfg(unix)] + let mut terminate = std::pin::pin!(terminate); + #[cfg(unix)] + let write_result = tokio::select! { + result = stdin.write_all(script) => Some(result), + _ = &mut terminate => None, + }; + #[cfg(windows)] + let write_result = Some(stdin.write_all(script).await); drop(stdin); - let status = child - .wait() - .await - .context("failed to wait for standalone Codex updater")?; + #[cfg(unix)] + if write_result.is_none() { + cancel_installer(&mut child, codex_home).await; + return Ok(UpdateLoopControl::Stop); + } + write_result + .context("installer write was cancelled")? + .context("failed to pass standalone Codex updater to shell")?; + #[cfg(unix)] + let status = tokio::select! { + result = child.wait() => result, + _ = &mut terminate => { + cancel_installer(&mut child, codex_home).await; + return Ok(UpdateLoopControl::Stop); + } + }; + #[cfg(windows)] + let status = child.wait().await; + let status = status.context("failed to wait for standalone Codex updater")?; if status.success() { - Ok(()) + Ok(UpdateLoopControl::Continue) } else { anyhow::bail!("standalone Codex updater exited with status {status}") } } +#[cfg(unix)] +async fn cancel_installer(child: &mut tokio::process::Child, codex_home: &Path) { + let Some(pid) = child.id().and_then(|pid| libc::pid_t::try_from(pid).ok()) else { + return; + }; + // Let the shell's EXIT/TERM trap release the installer lock first. + unsafe { libc::kill(-pid, libc::SIGTERM) }; + sleep(Duration::from_secs(2)).await; + // Keep the shell unreaped until after the group kill, so its PID cannot + // be reused while descendants that ignored TERM are still running. + 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"); + 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); + } + let _ = child.wait().await; +} + async fn fetch_installer_script(http: &impl InstallerHttp) -> Result> { match http.get(INSTALL_URL).await? { InstallerResponse::Success(body) => Ok(body), 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 bb2a87f028..aebe19e602 100644 --- a/codex-rs/app-server-daemon/src/update_loop_tests.rs +++ b/codex-rs/app-server-daemon/src/update_loop_tests.rs @@ -1,4 +1,6 @@ use std::sync::Mutex; +#[cfg(unix)] +use std::time::Duration; use pretty_assertions::assert_eq; @@ -93,6 +95,48 @@ impl InstallerHttp for FakeInstallerHttp { } } +#[cfg(unix)] +#[tokio::test] +async fn cancelling_installer_stops_children_and_releases_fallback_lock() { + let home = tempfile::TempDir::new().expect("home"); + let ready = home.path().join("ready"); + let delayed = home.path().join("delayed"); + let lock = home.path().join("packages/standalone/install.lock.d"); + let script = format!( + "mkdir -p '{lock}'\necho $$ > '{lock}/pid'\n(trap '' TERM; echo ready > '{ready}'; sleep 4; echo late > '{delayed}') &\nwait\n", + lock = lock.display(), + ready = ready.display(), + delayed = delayed.display(), + ); + let (cancel, cancelled) = tokio::sync::oneshot::channel(); + let ready_for_signal = ready.clone(); + let signal_sender = tokio::spawn(async move { + let deadline = tokio::time::Instant::now() + Duration::from_secs(5); + while !ready_for_signal.exists() { + assert!( + tokio::time::Instant::now() < deadline, + "installer did not start" + ); + tokio::time::sleep(Duration::from_millis(20)).await; + } + cancel.send(()).expect("cancel installer"); + }); + 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() + }), + ) + .await + .expect("installer cancellation timed out") + .expect("installer cancellation failed"); + signal_sender.await.expect("signal sender"); + assert!(matches!(result, super::UpdateLoopControl::Stop)); + assert!(!lock.exists()); + tokio::time::sleep(Duration::from_secs(3)).await; + assert!(!delayed.exists()); +} + #[cfg(windows)] #[tokio::test] async fn powershell_installer_is_noninteractive_and_reports_script_failure() { @@ -105,11 +149,21 @@ Test-Installer "# .to_vec(), )); - super::install_latest_standalone(&valid) + 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"); let failing = FakeInstallerHttp::new(InstallerResponse::Success( b"throw 'installer failed'".to_vec(), )); - assert!(super::install_latest_standalone(&failing).await.is_err()); + let script = super::fetch_installer_script(&failing) + .await + .expect("fetch failing installer"); + assert!( + super::run_installer_script(&script, "0.150.0-x86_64-pc-windows-msvc") + .await + .is_err() + ); } diff --git a/scripts/install/install.ps1 b/scripts/install/install.ps1 index 80d41a798a..229663b166 100644 --- a/scripts/install/install.ps1 +++ b/scripts/install/install.ps1 @@ -906,6 +906,7 @@ $codexHome = if ([string]::IsNullOrWhiteSpace($env:CODEX_HOME)) { $standaloneRoot = Join-Path $codexHome "packages\standalone" $releasesDir = Join-Path $standaloneRoot "releases" $currentDir = Join-Path $standaloneRoot "current" +$autoUpdateVersion = Join-Path $standaloneRoot "auto-update-version" $lockPath = Join-Path $standaloneRoot "install.lock" $defaultVisibleBinDir = Join-Path $env:LOCALAPPDATA "Programs\OpenAI\Codex\bin" @@ -943,9 +944,55 @@ $checksumMetadata = $assetSelection.ChecksumMetadata $installLayout = $assetSelection.InstallLayout $tempDir = Join-Path ([System.IO.Path]::GetTempPath()) ("codex-install-" + [System.Guid]::NewGuid().ToString("N")) New-Item -ItemType Directory -Force -Path $tempDir | Out-Null +$guardRejected = $false try { Invoke-WithInstallLock -LockPath $lockPath -Script { + $updaterRecord = Join-Path $codexHome "app-server-daemon\app-server-updater.pid" + $oldUpdaterParent = $false + if ($Release -eq "latest" -and $env:CODEX_INSTALL_IF_LATEST -ne "1" -and (Test-Path -LiteralPath $updaterRecord)) { + $updaterPid = $null + $updaterStartTime = $null + try { + $record = Get-Content -LiteralPath $updaterRecord -Raw | ConvertFrom-Json + $updaterPid = [long]$record.pid + $updaterStartTime = [string]$record.processStartTime + } catch { + # Empty or stale PID reservations must not block a manual install. + } + if ($null -ne $updaterPid -and $updaterPid -gt 0 -and -not [string]::IsNullOrEmpty($updaterStartTime)) { + $updaterProcess = Get-Process -Id $updaterPid -ErrorAction SilentlyContinue + if ($null -ne $updaterProcess -and $updaterStartTime -eq [string]$updaterProcess.StartTime.ToFileTimeUtc()) { + try { + $parentPid = (Get-CimInstance Win32_Process -Filter "ProcessId = $PID" -ErrorAction Stop).ParentProcessId + if ([long]$parentPid -le 0) { throw "Missing updater parent process." } + } catch { + try { + $parentPid = (Get-WmiObject Win32_Process -Filter "ProcessId = $PID" -ErrorAction Stop).ParentProcessId + if ([long]$parentPid -le 0) { throw "Missing updater parent process." } + } catch { + throw "Cannot verify whether the standalone installer was launched by an older updater." + } + } + $oldUpdaterParent = $updaterPid -eq $parentPid + } + } + } + if ($env:CODEX_INSTALL_IF_LATEST -eq "1" -or $oldUpdaterParent) { + $previousRelease = if ($oldUpdaterParent -and (Test-Path -LiteralPath $autoUpdateVersion)) { + [System.IO.File]::ReadAllText($autoUpdateVersion) + } else { + $env:CODEX_UPDATE_FROM_RELEASE + } + $currentTarget = if (Test-Path -LiteralPath $currentDir) { (Get-Item -LiteralPath $currentDir).Target } else { $null } + if ($Release -ne "latest" -or [string]::IsNullOrEmpty($previousRelease) -or [string]::IsNullOrEmpty($currentTarget) -or + -not (Test-Path -LiteralPath $autoUpdateVersion) -or + [System.IO.File]::ReadAllText($autoUpdateVersion) -cne $previousRelease -or + [System.IO.Path]::GetFullPath($currentTarget) -ne [System.IO.Path]::GetFullPath((Join-Path $releasesDir $previousRelease))) { + $script:guardRejected = $true + return + } + } Remove-StaleInstallArtifacts -ReleasesDir $releasesDir if (-not (Test-ReleaseIsComplete -ReleaseDir $releaseDir -ExpectedVersion $resolvedVersion -ExpectedTarget $target -Layout $installLayout)) { @@ -1012,6 +1059,15 @@ try { New-Item -ItemType Directory -Force -Path $standaloneRoot | Out-Null Ensure-Junction -LinkPath $currentDir -TargetPath $releaseDir -InstallerOwnedTargetPrefix $releasesDir + if ($Release -eq "latest") { + $tempMarker = "$autoUpdateVersion.tmp.$PID" + [System.IO.File]::WriteAllText($tempMarker, $releaseName) + Move-Item -LiteralPath $tempMarker -Destination $autoUpdateVersion -Force + } else { + if (Test-Path -LiteralPath $autoUpdateVersion) { + Remove-Item -LiteralPath $autoUpdateVersion -Force -ErrorAction Stop + } + } $visibleParent = Split-Path -Parent $visibleBinDir $currentBinDir = if ($installLayout -eq "Package") { @@ -1040,6 +1096,7 @@ try { } finally { Remove-Item -Recurse -Force $tempDir -ErrorAction SilentlyContinue } +if ($guardRejected) { return } Maybe-HandleConflictingInstall -Conflict $conflictingInstall diff --git a/scripts/install/install.sh b/scripts/install/install.sh index 9c7dfabd3d..4a2b4ece49 100755 --- a/scripts/install/install.sh +++ b/scripts/install/install.sh @@ -19,6 +19,7 @@ CODEX_HOME_DIR="${CODEX_HOME:-$HOME/.codex}" STANDALONE_ROOT="$CODEX_HOME_DIR/packages/standalone" RELEASES_DIR="$STANDALONE_ROOT/releases" CURRENT_LINK="$STANDALONE_ROOT/current" +AUTO_UPDATE_VERSION="$STANDALONE_ROOT/auto-update-version" LOCK_FILE="$STANDALONE_ROOT/install.lock" LOCK_DIR="$STANDALONE_ROOT/install.lock.d" LOCK_STALE_AFTER_SECS=600 @@ -1148,9 +1149,50 @@ cleanup() { rm -rf "$tmp_dir" fi } -trap cleanup EXIT INT TERM +trap cleanup EXIT +trap 'exit 130' INT +trap 'exit 143' TERM acquire_install_lock +updater_record="$CODEX_HOME_DIR/app-server-daemon/app-server-updater.pid" +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)" + recorded_start="$(sed -n 's/.*"processStartTime"[[:space:]]*:[[:space:]]*"\([^"]*\)".*/\1/p' "$updater_record" | head -n 1)" + if [ -r "/proc/$$/stat" ]; then + parent_pid="$(sed 's/^.*) //' "/proc/$$/stat" | awk '{ print $2 }')" + else + parent_pid="$(ps -p "$$" -o ppid= 2>/dev/null)" || parent_pid="" + parent_pid="$(printf '%s' "$parent_pid" | tr -d ' ')" + fi + if [ -n "$updater_pid" ] && [ "$updater_pid" = "$parent_pid" ]; then + actual_details="$(ps -p "$updater_pid" -o stat= -o lstart= 2>/dev/null)" || actual_details="" + actual_start="$(printf '%s' "$actual_details" | sed 's/^[^[:space:]]*[[:space:]]*//; s/[[:space:]]*$//')" + if [ -n "$recorded_start" ] && [ "$recorded_start" = "$actual_start" ]; then + old_updater_parent="true" + fi + fi + if [ "$RELEASE" = "latest" ] && [ -n "$updater_pid" ] && + { [ -z "$parent_pid" ] || { [ "$updater_pid" = "$parent_pid" ] && + { [ -z "$recorded_start" ] || [ -z "$actual_start" ]; }; }; } && + kill -0 "$updater_pid" 2>/dev/null; then + warn "Cannot verify whether an older updater launched this installer; skipping latest update." + exit 0 + fi +fi +if [ "${CODEX_INSTALL_IF_LATEST:-}" = "1" ] || [ "$old_updater_parent" = "true" ]; then + guarded_release="${CODEX_UPDATE_FROM_RELEASE:-}" + if [ "$old_updater_parent" = "true" ]; then + guarded_release="$(cat "$AUTO_UPDATE_VERSION" 2>/dev/null || true)" + fi + current_release_dir="$(cd -P "$CURRENT_LINK" 2>/dev/null && pwd)" || exit 0 + releases_dir="$(cd -P "$RELEASES_DIR" 2>/dev/null && pwd)" || exit 0 + if [ "$RELEASE" != "latest" ] || [ -z "$guarded_release" ] || + [ "$(cat "$AUTO_UPDATE_VERSION" 2>/dev/null || true)" != "$guarded_release" ] || + [ "$current_release_dir" != "$releases_dir/$guarded_release" ]; then + exit 0 + fi +fi cleanup_stale_install_artifacts if ! release_dir_is_complete "$release_dir" "$resolved_version" "$vendor_target" "$install_layout"; then @@ -1183,6 +1225,12 @@ if ! release_dir_is_complete "$release_dir" "$resolved_version" "$vendor_target" exit 1 fi update_current_link "$release_dir" +if [ "$RELEASE" = "latest" ]; then + printf '%s' "$release_name" > "$AUTO_UPDATE_VERSION.tmp.$$" + mv -f "$AUTO_UPDATE_VERSION.tmp.$$" "$AUTO_UPDATE_VERSION" +else + rm -f "$AUTO_UPDATE_VERSION" +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 02d527d608..a754ab21cf 100644 --- a/scripts/install/test_install_sh.py +++ b/scripts/install/test_install_sh.py @@ -156,6 +156,104 @@ class InstallShTest(unittest.TestCase): ], ) + def test_explicit_release_pins_even_the_current_latest_version(self) -> None: + with tempfile.TemporaryDirectory() as temp_dir: + root = Path(temp_dir) + archive, checksum, metadata = create_package_release(root) + options = dict( + metadata_json=metadata, + archive_path=archive, + checksum_path=checksum, + force_macos=True, + ) + marker = root / "codex-home/packages/standalone/auto-update-version" + latest, _ = run_installer_in(root, "latest", **options) + self.assertEqual(latest.returncode, 0, latest.stderr) + release_name = f"{VERSION}-aarch64-apple-darwin" + self.assertEqual(marker.read_text(), release_name) + + pinned, _ = run_installer_in(root, VERSION, **options) + self.assertEqual(pinned.returncode, 0, pinned.stderr) + self.assertFalse(marker.exists()) + + updater_record = ( + root / "codex-home/app-server-daemon/app-server-updater.pid" + ) + updater_record.parent.mkdir(parents=True) + updater_record.write_text( + json.dumps( + {"pid": os.getpid(), "processStartTime": process_start_time()} + ) + ) + old_updater, _ = run_installer_in( + root, "latest", old_updater_parent_pid=os.getpid(), **options + ) + self.assertEqual(old_updater.returncode, 0, old_updater.stderr) + self.assertFalse(marker.exists()) + + skipped, _ = run_installer_in( + root, "latest", update_guard_from_release=release_name, **options + ) + self.assertEqual(skipped.returncode, 0, skipped.stderr) + self.assertFalse(marker.exists()) + + updater_record.write_text( + json.dumps({"pid": os.getpid(), "processStartTime": "stale"}) + ) + latest_again, _ = run_installer_in( + root, "latest", old_updater_parent_pid=os.getpid(), **options + ) + self.assertEqual(latest_again.returncode, 0, latest_again.stderr) + self.assertEqual(marker.read_text(), release_name) + + managed = ( + root + / f"codex-home/packages/standalone/releases/{release_name}/bin/codex" + ) + managed.unlink() + guarded, _ = run_installer_in( + root, + "latest", + update_guard_from_release=release_name, + **options, + ) + self.assertEqual(guarded.returncode, 0, guarded.stderr) + self.assertTrue(managed.exists()) + + def test_uninspectable_legacy_updater_does_not_clear_pin(self) -> None: + with tempfile.TemporaryDirectory() as temp_dir: + root = Path(temp_dir) + 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) + updater_record = ( + root / "codex-home/app-server-daemon/app-server-updater.pid" + ) + updater_record.parent.mkdir(parents=True) + updater_record.write_text( + json.dumps( + {"pid": os.getpid(), "processStartTime": process_start_time()} + ) + ) + + attempted, _ = run_installer_in( + root, + "latest", + old_updater_parent_pid=os.getpid(), + fail_ps=True, + **options, + ) + self.assertEqual(attempted.returncode, 0, attempted.stderr) + self.assertFalse( + (root / "codex-home/packages/standalone/auto-update-version").exists() + ) + def test_releases_unusable_metadata_falls_back_to_github(self) -> None: unusable_metadata = { "html": "proxy error", @@ -536,6 +634,9 @@ def run_installer_in( force_macos: bool = False, use_mirror: bool | None = False, releases_mode: str = "", + update_guard_from_release: str | None = None, + old_updater_parent_pid: int | None = None, + fail_ps: bool = False, ) -> tuple[subprocess.CompletedProcess[str], list[str]]: bin_dir = root / "bin" bin_dir.mkdir(exist_ok=True) @@ -647,6 +748,19 @@ def run_installer_in( encoding="utf-8", ) fake_uname.chmod(0o755) + if old_updater_parent_pid is not None: + fake_ps = bin_dir / "ps" + fake_ps.write_text( + "#!/bin/sh\nexit 1\n" + if fail_ps + else "#!/bin/sh\n" + 'case "$*" in\n' + ' *lstart*) printf "S %s\\n" "$CODEX_TEST_PARENT_START" ;;\n' + ' *) printf "%s\\n" "$CODEX_TEST_PARENT_PID" ;;\n' + "esac\n", + encoding="utf-8", + ) + fake_ps.chmod(0o755) home = root / "home" home.mkdir(exist_ok=True) @@ -681,6 +795,15 @@ def run_installer_in( "SHELL": "/bin/sh", } ) + if update_guard_from_release is None: + env.pop("CODEX_INSTALL_IF_LATEST", None) + env.pop("CODEX_UPDATE_FROM_RELEASE", None) + else: + env["CODEX_INSTALL_IF_LATEST"] = "1" + env["CODEX_UPDATE_FROM_RELEASE"] = update_guard_from_release + if old_updater_parent_pid is not None: + env["CODEX_TEST_PARENT_PID"] = str(old_updater_parent_pid) + env["CODEX_TEST_PARENT_START"] = process_start_time() if use_mirror is None: env.pop("CODEX_INSTALLER_USE_RELEASES_OPENAI_COM", None) else: @@ -702,6 +825,13 @@ def run_installer_in( return result, requests +def process_start_time() -> str: + details = subprocess.check_output( + ["ps", "-p", str(os.getpid()), "-o", "stat=", "-o", "lstart="], text=True + ).strip() + return details.split(maxsplit=1)[1] + + def create_package_release( root: Path, *,