mirror of
https://github.com/openai/codex.git
synced 2026-10-21 07:42:01 +00:00
Ensure the standalone updater runs on managed daemon starts (#43529)
## Why Starting a latest-channel standalone daemon required a separate `bootstrap` to enable automatic updates. A missing updater could also leave a running server without automatic updates until `bootstrap` was run again. ## What changed - Ensure a single supported updater after managed `start`, `restart`, and restarts caused by remote-control changes. Repeated starts recover a missing updater while reusing the running server. - Report updater failures as warnings so successful server starts and restarts still succeed. - Preserve an existing updater while the latest-channel marker and managed binary may be temporarily inconsistent during installation. Stop it when the latest-channel marker is removed. - Document automatic updater startup and the limitations of detecting out-of-band binary replacements. ## Testing Add Unix tests for updater reuse and recovery, invalid updater records, marker mismatches and release pinning, unmanaged servers, and bootstrap status for managed local builds. GitOrigin-RevId: 44148a8d65709f68def2072f542494620d7e574b
This commit is contained in:
@@ -75,10 +75,10 @@ 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. |
|
||||
| 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. |
|
||||
| Latest-channel installer has run; `start` is used | Managed binary and detached updater when supported | When supported, the platform's installer runs hourly. | When supported, the running 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 | When supported, the platform's installer runs hourly. | When supported, the running 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. |
|
||||
| Another tool updates the managed binary | A fresh start or explicit restart uses it; a running server is reused. | Yes, when a latest-channel updater is running. | An updater that was running through the change compares binary contents on its next successful installer pass and refreshes the server first. |
|
||||
|
||||
### Standalone installs
|
||||
|
||||
@@ -86,8 +86,9 @@ 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 only for a stable
|
||||
latest-channel release whose managed binary supports the updater command
|
||||
- managed `start`, `restart`, and `bootstrap` ensure a single 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
|
||||
@@ -98,20 +99,21 @@ For installs created by either platform's standalone installer:
|
||||
- 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
|
||||
- the updater loop is not reboot-persistent; it must be started again by
|
||||
rerunning `bootstrap` after a reboot
|
||||
- the updater loop is not reboot-persistent; a managed start after reboot
|
||||
starts it again
|
||||
|
||||
### Out-of-band updates
|
||||
|
||||
This daemon does not watch arbitrary executable files for replacement. If some
|
||||
other tool updates the managed binary path:
|
||||
|
||||
- without `bootstrap`, a currently running app-server remains on the old
|
||||
executable image until an explicit `restart`
|
||||
- with `bootstrap`, the detached updater loop notices the changed managed
|
||||
- an updater that was already running notices a changed managed
|
||||
binary on its next successful scheduled installer pass; if
|
||||
app-server is running, it refreshes app-server first and then refreshes itself
|
||||
once that replacement starts successfully
|
||||
- if the updater was absent during a same-version binary replacement, a later
|
||||
managed start recovers it but cannot infer the running server's previous
|
||||
executable identity; use `codex app-server daemon restart` to refresh the server
|
||||
|
||||
## Lifecycle semantics
|
||||
|
||||
|
||||
@@ -312,39 +312,38 @@ impl Daemon {
|
||||
|
||||
async fn start(&self) -> Result<LifecycleOutput> {
|
||||
let settings = self.load_settings().await?;
|
||||
if let Ok(info) = client::probe(&self.socket_path).await {
|
||||
return Ok(self
|
||||
.output(
|
||||
LifecycleStatus::AlreadyRunning,
|
||||
self.running_backend(&settings).await?,
|
||||
/*pid*/ None,
|
||||
Some(info.app_server_version),
|
||||
)
|
||||
.await);
|
||||
}
|
||||
|
||||
if self.running_backend_instance(&settings).await?.is_some() {
|
||||
let info = self.wait_until_ready().await?;
|
||||
return Ok(self
|
||||
.output(
|
||||
LifecycleStatus::AlreadyRunning,
|
||||
Some(BackendKind::Pid),
|
||||
/*pid*/ None,
|
||||
Some(info.app_server_version),
|
||||
)
|
||||
.await);
|
||||
}
|
||||
|
||||
self.ensure_managed_codex_bin()?;
|
||||
let pid = self.start_managed_backend(&settings).await?;
|
||||
let info = self.wait_until_ready().await?;
|
||||
Ok(self
|
||||
.output(
|
||||
let (status, backend, pid, info) = if let Ok(info) = client::probe(&self.socket_path).await
|
||||
{
|
||||
(
|
||||
LifecycleStatus::AlreadyRunning,
|
||||
self.running_backend(&settings).await?,
|
||||
None,
|
||||
info,
|
||||
)
|
||||
} else if self.running_backend_instance(&settings).await?.is_some() {
|
||||
(
|
||||
LifecycleStatus::AlreadyRunning,
|
||||
Some(BackendKind::Pid),
|
||||
None,
|
||||
self.wait_until_ready().await?,
|
||||
)
|
||||
} else {
|
||||
self.ensure_managed_codex_bin()?;
|
||||
let pid = self.start_managed_backend(&settings).await?;
|
||||
(
|
||||
LifecycleStatus::Started,
|
||||
Some(BackendKind::Pid),
|
||||
pid,
|
||||
Some(info.app_server_version),
|
||||
self.wait_until_ready().await?,
|
||||
)
|
||||
};
|
||||
if backend.is_some()
|
||||
&& let Err(err) = self.ensure_managed_updater(&settings).await
|
||||
{
|
||||
eprintln!("warning: failed to ensure managed updater after app-server start: {err:#}");
|
||||
}
|
||||
Ok(self
|
||||
.output(status, backend, pid, Some(info.app_server_version))
|
||||
.await)
|
||||
}
|
||||
|
||||
@@ -365,6 +364,11 @@ impl Daemon {
|
||||
|
||||
let pid = self.start_managed_backend(&settings).await?;
|
||||
let info = self.wait_until_ready().await?;
|
||||
if let Err(err) = self.ensure_managed_updater(&settings).await {
|
||||
eprintln!(
|
||||
"warning: failed to ensure managed updater after app-server restart: {err:#}"
|
||||
);
|
||||
}
|
||||
Ok(self
|
||||
.output(
|
||||
LifecycleStatus::Restarted,
|
||||
@@ -619,7 +623,13 @@ impl Daemon {
|
||||
let app_server_version = if let Some(backend) = backend {
|
||||
backend.stop().await?;
|
||||
let _ = self.start_managed_backend(&settings).await?;
|
||||
Some(self.wait_until_ready().await?.app_server_version)
|
||||
let info = self.wait_until_ready().await?;
|
||||
if let Err(err) = self.ensure_managed_updater(&settings).await {
|
||||
eprintln!(
|
||||
"warning: failed to ensure managed updater after remote-control change: {err:#}"
|
||||
);
|
||||
}
|
||||
Some(info.app_server_version)
|
||||
} else {
|
||||
None
|
||||
};
|
||||
@@ -708,13 +718,19 @@ impl Daemon {
|
||||
async fn ensure_managed_updater(&self, settings: &DaemonSettings) -> Result<bool> {
|
||||
let updater = backend::pid_update_loop_backend(self.backend_paths(settings));
|
||||
if !self.is_stable_standalone_release()? {
|
||||
updater.stop().await?;
|
||||
// An installer publishes current and the latest marker separately.
|
||||
// Keep its updater alive while that publication may be in progress.
|
||||
if !self.has_latest_selection_marker() {
|
||||
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?;
|
||||
if !self.has_latest_selection_marker() {
|
||||
updater.stop().await?;
|
||||
}
|
||||
return Ok(false);
|
||||
};
|
||||
if !managed_install::supports_daemon_update_loop(&codex_bin).await
|
||||
@@ -723,7 +739,9 @@ impl Daemon {
|
||||
.await
|
||||
.is_ok_and(|selected| selected == codex_bin)
|
||||
{
|
||||
updater.stop().await?;
|
||||
if !self.has_latest_selection_marker() {
|
||||
updater.stop().await?;
|
||||
}
|
||||
return Ok(false);
|
||||
}
|
||||
backend::pid_update_loop_backend(self.backend_paths_with_bin(settings, &codex_bin))
|
||||
@@ -744,6 +762,16 @@ impl Daemon {
|
||||
))
|
||||
}
|
||||
|
||||
fn has_latest_selection_marker(&self) -> bool {
|
||||
self.settings_file
|
||||
.parent()
|
||||
.and_then(Path::parent)
|
||||
.is_some_and(|home| {
|
||||
home.join("packages/standalone/auto-update-version")
|
||||
.is_file()
|
||||
})
|
||||
}
|
||||
|
||||
async fn is_bootstrapped(&self, settings: &DaemonSettings) -> Result<bool> {
|
||||
if !self.is_stable_standalone_release()?
|
||||
|| !managed_install::supports_daemon_update_loop(&self.managed_codex_bin).await
|
||||
@@ -970,6 +998,8 @@ mod tests {
|
||||
use super::restart_decision;
|
||||
use super::should_reexec_updater;
|
||||
use crate::client::ProbeInfo;
|
||||
#[cfg(unix)]
|
||||
use crate::settings::DaemonSettings;
|
||||
|
||||
#[test]
|
||||
fn remote_control_status_uses_camel_case_json() {
|
||||
@@ -1131,6 +1161,49 @@ mod tests {
|
||||
);
|
||||
}
|
||||
|
||||
#[cfg(unix)]
|
||||
#[tokio::test]
|
||||
async fn managed_local_backend_counts_as_bootstrapped_without_updater() {
|
||||
use std::os::unix::fs::PermissionsExt;
|
||||
|
||||
let home = TempDir::new().expect("home");
|
||||
let standalone = home.path().join("packages/standalone");
|
||||
let local_bin = standalone.join("local-main/bin/codex");
|
||||
tokio::fs::create_dir_all(local_bin.parent().expect("bin parent"))
|
||||
.await
|
||||
.expect("local bin directory");
|
||||
tokio::fs::write(&local_bin, b"#!/bin/sh\nexec sleep 30\n")
|
||||
.await
|
||||
.expect("local bin");
|
||||
std::fs::set_permissions(&local_bin, std::fs::Permissions::from_mode(0o755))
|
||||
.expect("executable local bin");
|
||||
std::os::unix::fs::symlink("local-main", standalone.join("current"))
|
||||
.expect("current local build");
|
||||
let state = home.path().join("app-server-daemon");
|
||||
let daemon = Daemon {
|
||||
socket_path: home
|
||||
.path()
|
||||
.join("app-server-control/app-server-control.sock"),
|
||||
pid_file: state.join("app-server.pid"),
|
||||
update_pid_file: state.join("app-server-updater.pid"),
|
||||
operation_lock_file: state.join("daemon.lock"),
|
||||
settings_file: state.join("settings.json"),
|
||||
managed_codex_bin: standalone.join("current/bin/codex"),
|
||||
};
|
||||
let settings = DaemonSettings::default();
|
||||
assert!(
|
||||
!daemon
|
||||
.is_bootstrapped(&settings)
|
||||
.await
|
||||
.expect("not running")
|
||||
);
|
||||
let backend = crate::backend::pid_backend(daemon.backend_paths(&settings));
|
||||
backend.start().await.expect("start local backend");
|
||||
let bootstrapped = daemon.is_bootstrapped(&settings).await;
|
||||
backend.stop().await.expect("stop local backend");
|
||||
assert!(bootstrapped.expect("managed local backend"));
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn not_ready_context_reports_daemon_app_server_before_stderr() {
|
||||
let temp_dir = TempDir::new().expect("temp dir");
|
||||
|
||||
@@ -0,0 +1,221 @@
|
||||
#![cfg(unix)]
|
||||
|
||||
use std::path::PathBuf;
|
||||
use std::process::Child;
|
||||
use std::process::Command;
|
||||
use std::process::Stdio;
|
||||
use std::time::Duration;
|
||||
use std::time::Instant;
|
||||
|
||||
use anyhow::Context;
|
||||
use anyhow::Result;
|
||||
use anyhow::ensure;
|
||||
use pretty_assertions::assert_eq;
|
||||
use serde_json::Value;
|
||||
use tempfile::TempDir;
|
||||
|
||||
struct TestDaemon {
|
||||
home: TempDir,
|
||||
codex: PathBuf,
|
||||
unmanaged: Option<Child>,
|
||||
}
|
||||
|
||||
impl TestDaemon {
|
||||
fn new() -> Result<Self> {
|
||||
let home = tempfile::Builder::new().tempdir_in("/tmp")?;
|
||||
let codex = codex_utils_cargo_bin::cargo_bin("codex")?;
|
||||
let codex_source = std::fs::canonicalize(&codex)?;
|
||||
let target = if cfg!(target_os = "macos") {
|
||||
format!("{}-apple-darwin", std::env::consts::ARCH)
|
||||
} else {
|
||||
format!("{}-unknown-linux-musl", std::env::consts::ARCH)
|
||||
};
|
||||
let standalone = home.path().join("packages/standalone");
|
||||
let release_name = format!("0.0.0-{target}");
|
||||
let managed = standalone
|
||||
.join("releases")
|
||||
.join(&release_name)
|
||||
.join("bin/codex");
|
||||
std::fs::create_dir_all(managed.parent().context("managed bin parent")?)?;
|
||||
std::fs::hard_link(&codex_source, &managed)
|
||||
.or_else(|_| std::fs::copy(&codex_source, managed).map(|_| ()))?;
|
||||
std::fs::write(standalone.join("auto-update-version"), &release_name)?;
|
||||
std::os::unix::fs::symlink(
|
||||
PathBuf::from("releases").join(release_name),
|
||||
standalone.join("current"),
|
||||
)?;
|
||||
Ok(Self {
|
||||
home,
|
||||
codex,
|
||||
unmanaged: None,
|
||||
})
|
||||
}
|
||||
|
||||
fn command(&self) -> Command {
|
||||
let mut command = Command::new(&self.codex);
|
||||
command.env("CODEX_HOME", self.home.path());
|
||||
command
|
||||
}
|
||||
|
||||
fn lifecycle(&self, action: &str) -> Result<Value> {
|
||||
let output = self
|
||||
.command()
|
||||
.args(["app-server", "daemon", action])
|
||||
.output()?;
|
||||
ensure!(
|
||||
output.status.success(),
|
||||
"daemon {action} failed: {}",
|
||||
String::from_utf8_lossy(&output.stderr)
|
||||
);
|
||||
Ok(serde_json::from_slice(&output.stdout)?)
|
||||
}
|
||||
|
||||
fn pid(&self, name: &str) -> Result<u32> {
|
||||
let record = std::fs::read(self.home.path().join("app-server-daemon").join(name))
|
||||
.with_context(|| format!("failed to read {name}"))?;
|
||||
Ok(serde_json::from_slice::<Value>(&record)?["pid"]
|
||||
.as_u64()
|
||||
.context("pid missing")? as u32)
|
||||
}
|
||||
}
|
||||
|
||||
impl Drop for TestDaemon {
|
||||
fn drop(&mut self) {
|
||||
if let Some(mut child) = self.unmanaged.take() {
|
||||
let _ = child.kill();
|
||||
let _ = child.wait();
|
||||
}
|
||||
let _ = self.lifecycle("stop");
|
||||
if let Ok(pid) = self.pid("app-server-updater.pid") {
|
||||
let _ = signal(pid, libc::SIGTERM);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
fn signal(pid: u32, signal: libc::c_int) -> Result<()> {
|
||||
let raw_pid = libc::pid_t::try_from(pid).context("pid out of range")?;
|
||||
ensure!(
|
||||
unsafe { libc::kill(raw_pid, signal) } == 0,
|
||||
"failed to signal pid {pid}: {}",
|
||||
std::io::Error::last_os_error()
|
||||
);
|
||||
Ok(())
|
||||
}
|
||||
|
||||
fn wait_for_exit(pid: u32) -> Result<()> {
|
||||
let deadline = Instant::now() + Duration::from_secs(30);
|
||||
loop {
|
||||
let output = Command::new("/bin/ps")
|
||||
.args(["-p", &pid.to_string(), "-o", "stat="])
|
||||
.output()
|
||||
.context("failed to invoke ps")?;
|
||||
let state = String::from_utf8_lossy(&output.stdout);
|
||||
if !output.status.success() || state.trim().starts_with('Z') {
|
||||
return Ok(());
|
||||
}
|
||||
ensure!(Instant::now() < deadline, "pid {pid} did not exit: {state}");
|
||||
std::thread::sleep(Duration::from_millis(50));
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn managed_starts_ensure_one_updater_and_recover_a_missing_one() -> Result<()> {
|
||||
let daemon = TestDaemon::new()?;
|
||||
assert_eq!(daemon.lifecycle("start")?["status"], "started");
|
||||
let backend_pid = daemon.pid("app-server.pid")?;
|
||||
let updater_pid = daemon.pid("app-server-updater.pid")?;
|
||||
assert_ne!(backend_pid, updater_pid);
|
||||
|
||||
assert_eq!(daemon.lifecycle("start")?["status"], "alreadyRunning");
|
||||
assert_eq!(daemon.pid("app-server.pid")?, backend_pid);
|
||||
assert_eq!(daemon.pid("app-server-updater.pid")?, updater_pid);
|
||||
|
||||
signal(updater_pid, libc::SIGTERM)?;
|
||||
wait_for_exit(updater_pid)?;
|
||||
assert_eq!(daemon.lifecycle("start")?["status"], "alreadyRunning");
|
||||
assert_eq!(daemon.pid("app-server.pid")?, backend_pid);
|
||||
let replacement_pid = daemon.pid("app-server-updater.pid")?;
|
||||
assert_ne!(replacement_pid, updater_pid);
|
||||
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);
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn managed_start_succeeds_when_updater_record_is_invalid() -> Result<()> {
|
||||
let daemon = TestDaemon::new()?;
|
||||
let state_dir = daemon.home.path().join("app-server-daemon");
|
||||
std::fs::create_dir_all(&state_dir)?;
|
||||
std::fs::write(state_dir.join("app-server-updater.pid"), "not a PID record")?;
|
||||
|
||||
assert_eq!(daemon.lifecycle("start")?["status"], "started");
|
||||
let server_pid = daemon.pid("app-server.pid")?;
|
||||
assert_eq!(daemon.lifecycle("start")?["status"], "alreadyRunning");
|
||||
assert_eq!(daemon.pid("app-server.pid")?, server_pid);
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn managed_start_keeps_updater_on_marker_mismatch_but_stops_it_for_pin() -> Result<()> {
|
||||
let daemon = TestDaemon::new()?;
|
||||
assert_eq!(daemon.lifecycle("start")?["status"], "started");
|
||||
let updater_pid = daemon.pid("app-server-updater.pid")?;
|
||||
let marker = daemon
|
||||
.home
|
||||
.path()
|
||||
.join("packages/standalone/auto-update-version");
|
||||
std::fs::write(&marker, "0.1.0-other-target")?;
|
||||
assert_eq!(daemon.lifecycle("start")?["status"], "alreadyRunning");
|
||||
assert_eq!(daemon.pid("app-server-updater.pid")?, updater_pid);
|
||||
|
||||
std::fs::remove_file(marker)?;
|
||||
|
||||
assert_eq!(daemon.lifecycle("start")?["status"], "alreadyRunning");
|
||||
wait_for_exit(updater_pid)?;
|
||||
assert!(
|
||||
!daemon
|
||||
.home
|
||||
.path()
|
||||
.join("app-server-daemon/app-server-updater.pid")
|
||||
.exists()
|
||||
);
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn start_does_not_launch_updater_for_unmanaged_app_server() -> Result<()> {
|
||||
let mut daemon = TestDaemon::new()?;
|
||||
daemon.unmanaged = Some(
|
||||
daemon
|
||||
.command()
|
||||
.args(["app-server", "--listen", "unix://"])
|
||||
.stdin(Stdio::null())
|
||||
.stdout(Stdio::null())
|
||||
.stderr(Stdio::null())
|
||||
.spawn()?,
|
||||
);
|
||||
let deadline = Instant::now() + Duration::from_secs(30);
|
||||
while !daemon
|
||||
.command()
|
||||
.args(["app-server", "daemon", "version"])
|
||||
.output()?
|
||||
.status
|
||||
.success()
|
||||
{
|
||||
ensure!(Instant::now() < deadline, "app server did not become ready");
|
||||
std::thread::sleep(Duration::from_millis(50));
|
||||
}
|
||||
|
||||
let output = daemon.lifecycle("start")?;
|
||||
assert_eq!(output["status"], "alreadyRunning");
|
||||
assert_eq!(output["backend"], Value::Null);
|
||||
assert!(
|
||||
!daemon
|
||||
.home
|
||||
.path()
|
||||
.join("app-server-daemon/app-server-updater.pid")
|
||||
.exists()
|
||||
);
|
||||
Ok(())
|
||||
}
|
||||
Reference in New Issue
Block a user