mirror of
https://github.com/openai/codex.git
synced 2026-10-19 08:16:48 +00:00
Recover daemon startup and updater re-exec after cwd deletion (#49819)
## Why Managed daemons and updaters can outlive their launching directory. Restarts and updater replacement need to keep working after that directory is removed. ## What changed - Share working-directory preparation between background launches and Unix updater re-execution. Fall back to the daemon state directory when the Unix working directory is missing, while retaining usable working directories and the existing Windows state-directory behavior. - Preserve configured `CODEX_HOME` paths and valid symlink aliases, and make certificate and AWS environment paths absolute before launching children. Handle both `npm_config_cafile` and `NPM_CONFIG_CAFILE`, preserving supported home-relative values. - Log warnings when scheduled daemon updates fail. ## Testing Add Unix regression coverage for restarting the daemon and updater from a deleted working directory, and for updater re-execution with preserved environment paths, `CODEX_HOME` symlink aliases, and a Linux non-Unicode home path. GitOrigin-RevId: f45aefd8ff060adc6e025badf0bb8c81f842bf88
This commit is contained in:
@@ -1,5 +1,6 @@
|
||||
//! Detached process launch and PID publication. Hold the reservation lock until
|
||||
//! the record is published, and on Windows until an updater acknowledges startup.
|
||||
//! Recover a deleted Unix cwd without changing workspace defaults for usable directories.
|
||||
|
||||
use super::PidBackend;
|
||||
#[cfg(windows)]
|
||||
@@ -133,6 +134,13 @@ impl PidBackend {
|
||||
command.env(key, value);
|
||||
}
|
||||
|
||||
crate::background_command::set_working_directory(
|
||||
command.as_std_mut(),
|
||||
self.pid_file
|
||||
.parent()
|
||||
.context("daemon pid path has no parent")?,
|
||||
)?;
|
||||
|
||||
#[cfg(unix)]
|
||||
{
|
||||
unsafe {
|
||||
@@ -149,61 +157,6 @@ impl PidBackend {
|
||||
{
|
||||
use windows_sys::Win32::System::Threading::CREATE_BREAKAWAY_FROM_JOB;
|
||||
use windows_sys::Win32::System::Threading::DETACHED_PROCESS;
|
||||
// Preserve process-scoped paths before changing cwd; CA names match CUSTOM_CA_ENV_KEYS.
|
||||
for name in [
|
||||
"CODEX_HOME",
|
||||
"CODEX_SQLITE_HOME",
|
||||
"CODEX_CA_CERTIFICATE",
|
||||
"SSL_CERT_FILE",
|
||||
"REQUESTS_CA_BUNDLE",
|
||||
"CURL_CA_BUNDLE",
|
||||
"NODE_EXTRA_CA_CERTS",
|
||||
"GIT_SSL_CAINFO",
|
||||
"CARGO_HTTP_CAINFO",
|
||||
"PIP_CERT",
|
||||
"BUNDLE_SSL_CA_CERT",
|
||||
"npm_config_cafile",
|
||||
"AWS_CONFIG_FILE",
|
||||
"AWS_SHARED_CREDENTIALS_FILE",
|
||||
"AWS_WEB_IDENTITY_TOKEN_FILE",
|
||||
"AWS_CONTAINER_AUTHORIZATION_TOKEN_FILE",
|
||||
] {
|
||||
let Some(mut value) = std::env::var_os(name) else {
|
||||
continue;
|
||||
};
|
||||
if matches!(name, "CODEX_SQLITE_HOME" | "npm_config_cafile") {
|
||||
value = value.to_str().unwrap_or_default().trim().into();
|
||||
}
|
||||
// These consumers expand `~` independently of the working directory.
|
||||
let expands_home = if name == "npm_config_cafile" {
|
||||
value
|
||||
.to_str()
|
||||
.is_some_and(|path| path.starts_with("~/") || path.starts_with("~\\"))
|
||||
} else {
|
||||
matches!(
|
||||
name,
|
||||
"CODEX_SQLITE_HOME" | "AWS_CONFIG_FILE" | "AWS_SHARED_CREDENTIALS_FILE"
|
||||
) && std::path::Path::new(&value).starts_with("~")
|
||||
};
|
||||
if value.is_empty() || expands_home {
|
||||
continue;
|
||||
}
|
||||
command.env(name, std::path::absolute(value)?);
|
||||
}
|
||||
if let Some(value) = std::env::var_os("SSL_CERT_DIR") {
|
||||
let paths = std::env::split_paths(&value)
|
||||
.filter(|path| !path.as_os_str().is_empty())
|
||||
.map(std::path::absolute)
|
||||
.collect::<std::io::Result<Vec<_>>>()?;
|
||||
command.env("SSL_CERT_DIR", std::env::join_paths(paths)?);
|
||||
}
|
||||
// A Windows process pins its working directory for its lifetime.
|
||||
// Keep both managed children out of the launching project's directory.
|
||||
command.current_dir(
|
||||
self.pid_file
|
||||
.parent()
|
||||
.context("daemon pid path has no parent")?,
|
||||
);
|
||||
// Never retry inside the parent's Job Object: that would report a
|
||||
// successful launch that dies when the terminal/SSH session closes.
|
||||
command.creation_flags(DETACHED_PROCESS | CREATE_BREAKAWAY_FROM_JOB);
|
||||
|
||||
@@ -0,0 +1,92 @@
|
||||
//! Recovers background startup after the launching directory is removed.
|
||||
//! Preserve process-scoped paths; retain usable Unix cwd defaults for clients.
|
||||
|
||||
use std::path::Path;
|
||||
use std::process::Command;
|
||||
|
||||
use anyhow::Result;
|
||||
|
||||
pub(crate) fn set_working_directory(command: &mut Command, state_dir: &Path) -> Result<()> {
|
||||
#[cfg(unix)]
|
||||
let cwd_missing = match std::env::current_dir() {
|
||||
Ok(_) => false,
|
||||
Err(err) if err.kind() == std::io::ErrorKind::NotFound => true,
|
||||
Err(err) => return Err(err.into()),
|
||||
};
|
||||
if let Ok(value) = std::env::var("CODEX_HOME")
|
||||
&& !value.is_empty()
|
||||
{
|
||||
let home = codex_utils_home_dir::find_codex_home()?;
|
||||
// Remove parent components without discarding a valid configured symlink alias.
|
||||
let absolute = std::path::absolute(value)?;
|
||||
let alias = home.join(&absolute);
|
||||
let preserved = if alias.canonicalize().ok().as_ref() == Some(&home) {
|
||||
alias.as_path()
|
||||
} else if home.as_path().to_str().is_some() {
|
||||
home.as_path()
|
||||
} else {
|
||||
// The home resolver ignores non-Unicode env values; keep the valid original alias.
|
||||
absolute.as_path()
|
||||
};
|
||||
command.env("CODEX_HOME", preserved);
|
||||
}
|
||||
// Preserve process-scoped paths before changing cwd; CA names match CUSTOM_CA_ENV_KEYS.
|
||||
for name in [
|
||||
#[cfg(windows)]
|
||||
"CODEX_SQLITE_HOME",
|
||||
"CODEX_CA_CERTIFICATE",
|
||||
"SSL_CERT_FILE",
|
||||
"REQUESTS_CA_BUNDLE",
|
||||
"CURL_CA_BUNDLE",
|
||||
"NODE_EXTRA_CA_CERTS",
|
||||
"GIT_SSL_CAINFO",
|
||||
"CARGO_HTTP_CAINFO",
|
||||
"PIP_CERT",
|
||||
"BUNDLE_SSL_CA_CERT",
|
||||
"npm_config_cafile",
|
||||
"NPM_CONFIG_CAFILE",
|
||||
"AWS_CONFIG_FILE",
|
||||
"AWS_SHARED_CREDENTIALS_FILE",
|
||||
"AWS_WEB_IDENTITY_TOKEN_FILE",
|
||||
"AWS_CONTAINER_AUTHORIZATION_TOKEN_FILE",
|
||||
] {
|
||||
let Some(mut value) = std::env::var_os(name) else {
|
||||
continue;
|
||||
};
|
||||
if matches!(
|
||||
name,
|
||||
"CODEX_SQLITE_HOME" | "npm_config_cafile" | "NPM_CONFIG_CAFILE"
|
||||
) {
|
||||
value = value.to_str().unwrap_or_default().trim().into();
|
||||
}
|
||||
// These consumers expand `~` independently of the working directory.
|
||||
let expands_home = if matches!(name, "npm_config_cafile" | "NPM_CONFIG_CAFILE") {
|
||||
value
|
||||
.to_str()
|
||||
.is_some_and(|path| path.starts_with("~/") || path.starts_with("~\\"))
|
||||
} else {
|
||||
matches!(
|
||||
name,
|
||||
"CODEX_SQLITE_HOME" | "AWS_CONFIG_FILE" | "AWS_SHARED_CREDENTIALS_FILE"
|
||||
) && std::path::Path::new(&value).starts_with("~")
|
||||
};
|
||||
if value.is_empty() || expands_home {
|
||||
continue;
|
||||
}
|
||||
command.env(name, std::path::absolute(value)?);
|
||||
}
|
||||
if let Some(value) = std::env::var_os("SSL_CERT_DIR") {
|
||||
let paths = std::env::split_paths(&value)
|
||||
.filter(|path| !path.as_os_str().is_empty())
|
||||
.map(std::path::absolute)
|
||||
.collect::<std::io::Result<Vec<_>>>()?;
|
||||
command.env("SSL_CERT_DIR", std::env::join_paths(paths)?);
|
||||
}
|
||||
#[cfg(unix)]
|
||||
if cwd_missing {
|
||||
command.current_dir(state_dir);
|
||||
}
|
||||
#[cfg(windows)]
|
||||
command.current_dir(state_dir);
|
||||
Ok(())
|
||||
}
|
||||
@@ -1,6 +1,7 @@
|
||||
//! Managed app-server lifecycle, serialized across CLI invocations and the updater.
|
||||
|
||||
mod backend;
|
||||
mod background_command;
|
||||
#[cfg(windows)]
|
||||
pub use backend::windows::DetachedLaunchRestricted;
|
||||
#[cfg(windows)]
|
||||
|
||||
@@ -226,7 +226,8 @@ async fn run_with_http(
|
||||
next_check = Instant::now();
|
||||
continue;
|
||||
}
|
||||
Ok((UpdateLoopControl::Continue, None)) | Err(_) => {}
|
||||
Ok((UpdateLoopControl::Continue, None)) => {}
|
||||
Err(err) => eprintln!("warning: scheduled daemon update failed: {err:#}"),
|
||||
Ok((UpdateLoopControl::Stop, _)) => return Ok(()),
|
||||
}
|
||||
let Some(delay) = next_update_delay(daemon).await else {
|
||||
@@ -261,7 +262,14 @@ async fn adopt_managed_updater(
|
||||
#[cfg(unix)]
|
||||
{
|
||||
let _ = listener;
|
||||
reexec_managed_updater(&managed_bin).map(|_| UpdateLoopControl::Stop)
|
||||
reexec_managed_updater(
|
||||
&managed_bin,
|
||||
daemon
|
||||
.update_pid_file
|
||||
.parent()
|
||||
.context("updater pid path has no parent")?,
|
||||
)
|
||||
.map(|_| UpdateLoopControl::Stop)
|
||||
}
|
||||
#[cfg(windows)]
|
||||
{
|
||||
@@ -511,10 +519,11 @@ async fn current_updater_identity() -> Result<ExecutableIdentity> {
|
||||
}
|
||||
|
||||
#[cfg(unix)]
|
||||
pub(crate) fn reexec_managed_updater(managed_codex_bin: &std::path::Path) -> Result<()> {
|
||||
let err = StdCommand::new(managed_codex_bin)
|
||||
.args(["app-server", "daemon", "pid-update-loop"])
|
||||
.exec();
|
||||
pub(crate) fn reexec_managed_updater(managed_codex_bin: &Path, state_dir: &Path) -> Result<()> {
|
||||
let mut command = StdCommand::new(managed_codex_bin);
|
||||
command.args(["app-server", "daemon", "pid-update-loop"]);
|
||||
crate::background_command::set_working_directory(&mut command, state_dir)?;
|
||||
let err = command.exec();
|
||||
Err(err).with_context(|| {
|
||||
format!(
|
||||
"failed to replace updater with managed Codex binary {}",
|
||||
|
||||
@@ -49,6 +49,144 @@ async fn installer_fetch_rejects_non_success_status() {
|
||||
assert_eq!(http.requested_urls(), vec![INSTALL_URL.to_string()]);
|
||||
}
|
||||
|
||||
#[cfg(unix)]
|
||||
#[test]
|
||||
fn updater_reexec_preserves_paths_and_recovers_deleted_working_directory() {
|
||||
use std::os::unix::ffi::OsStrExt;
|
||||
use std::os::unix::fs::PermissionsExt;
|
||||
|
||||
for (delete_cwd, home_name) in [
|
||||
(false, "home"),
|
||||
(true, "home"),
|
||||
(false, "alias"),
|
||||
(true, "alias"),
|
||||
// macOS filesystems reject non-Unicode names.
|
||||
#[cfg(target_os = "linux")]
|
||||
(false, "link/../home"),
|
||||
] {
|
||||
let home = TempDir::new().expect("home");
|
||||
let project = home.path().join("project");
|
||||
let state = home.path().join("state");
|
||||
let codex_home = home.path().join("home");
|
||||
std::fs::create_dir(&project).expect("project");
|
||||
std::fs::create_dir(&state).expect("state");
|
||||
std::fs::create_dir(&codex_home).expect("codex home");
|
||||
std::os::unix::fs::symlink(&codex_home, home.path().join("alias")).expect("home alias");
|
||||
if home_name == "link/../home" {
|
||||
let non_unicode = home.path().join(std::ffi::OsStr::from_bytes(b"home-\xff"));
|
||||
std::fs::create_dir_all(non_unicode.join("child")).expect("non-Unicode parent");
|
||||
std::fs::create_dir(non_unicode.join("home")).expect("non-Unicode home");
|
||||
std::os::unix::fs::symlink(non_unicode.join("child"), home.path().join("link"))
|
||||
.expect("Unicode alias");
|
||||
}
|
||||
let executable = home.path().join("updater");
|
||||
std::fs::write(
|
||||
&executable,
|
||||
b"#!/bin/sh\ntest -d \"$CODEX_HOME\" || exit 1\n{ pwd -P; printf '%s\\n' \"$CODEX_HOME\" \"$CODEX_SQLITE_HOME\" \"$AWS_CONFIG_FILE\" \"$SSL_CERT_DIR\" \"$NPM_CONFIG_CAFILE\"; } > \"$CODEX_TEST_UPDATER_OUTPUT\"\n",
|
||||
)
|
||||
.expect("updater shim");
|
||||
std::fs::set_permissions(&executable, std::fs::Permissions::from_mode(0o755))
|
||||
.expect("executable");
|
||||
let output = state.join("output");
|
||||
let project_path = project.canonicalize().expect("project path");
|
||||
let ca_file = if delete_cwd {
|
||||
project_path.join("ca.pem").into_os_string()
|
||||
} else {
|
||||
" ca.pem ".into()
|
||||
};
|
||||
let cert_dir = if delete_cwd {
|
||||
project_path.join("certs")
|
||||
} else {
|
||||
std::path::PathBuf::from("certs")
|
||||
};
|
||||
let result = std::process::Command::new(std::env::current_exe().expect("test binary"))
|
||||
.args(["--exact", "update_loop::tests::updater_reexec_child"])
|
||||
.current_dir(&project)
|
||||
.env("CODEX_TEST_UPDATER_EXECUTABLE", &executable)
|
||||
.env("CODEX_TEST_UPDATER_STATE", &state)
|
||||
.env("CODEX_TEST_UPDATER_OUTPUT", &output)
|
||||
.env("CODEX_TEST_UPDATER_DELETE_CWD", delete_cwd.to_string())
|
||||
.env("CODEX_HOME", format!("../{home_name}"))
|
||||
.env("CODEX_SQLITE_HOME", " sqlite ")
|
||||
.env("AWS_CONFIG_FILE", "~/config")
|
||||
.env("NPM_CONFIG_CAFILE", ca_file)
|
||||
.env(
|
||||
"SSL_CERT_DIR",
|
||||
std::env::join_paths([&cert_dir, &state]).unwrap(),
|
||||
)
|
||||
.output()
|
||||
.expect("spawn updater test");
|
||||
assert!(
|
||||
result.status.success(),
|
||||
"{}",
|
||||
String::from_utf8_lossy(&result.stderr)
|
||||
);
|
||||
let state = state.canonicalize().expect("state path");
|
||||
let expected = [
|
||||
if delete_cwd { &state } else { &project_path }
|
||||
.display()
|
||||
.to_string(),
|
||||
if home_name == "link/../home" {
|
||||
project_path.join(format!("../{home_name}"))
|
||||
} else {
|
||||
home.path()
|
||||
.canonicalize()
|
||||
.expect("parent path")
|
||||
.join(home_name)
|
||||
}
|
||||
.display()
|
||||
.to_string(),
|
||||
" sqlite ".to_string(),
|
||||
"~/config".to_string(),
|
||||
std::env::join_paths([project_path.join("certs"), home.path().join("state")])
|
||||
.unwrap()
|
||||
.to_string_lossy()
|
||||
.into_owned(),
|
||||
project_path.join("ca.pem").display().to_string(),
|
||||
]
|
||||
.join("\n")
|
||||
+ "\n";
|
||||
assert_eq!(
|
||||
std::fs::read_to_string(output).expect("updater output"),
|
||||
expected
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(unix)]
|
||||
#[test]
|
||||
fn updater_reexec_child() {
|
||||
use std::os::unix::process::CommandExt;
|
||||
|
||||
let Some(executable) = std::env::var_os("CODEX_TEST_UPDATER_EXECUTABLE") else {
|
||||
return;
|
||||
};
|
||||
let state = std::env::var_os("CODEX_TEST_UPDATER_STATE").expect("state path");
|
||||
if std::env::var("CODEX_TEST_UPDATER_DELETE_CWD").as_deref() == Ok("true") {
|
||||
// Model the initial background launch while the relative home is resolvable.
|
||||
let mut command = std::process::Command::new(std::env::current_exe().expect("test binary"));
|
||||
command
|
||||
.args(["--exact", "update_loop::tests::updater_reexec_child"])
|
||||
.env("CODEX_TEST_UPDATER_DELETE_CWD", "prepared");
|
||||
crate::background_command::set_working_directory(
|
||||
&mut command,
|
||||
std::path::Path::new(&state),
|
||||
)
|
||||
.expect("prepare background launch");
|
||||
panic!("failed to launch prepared updater: {}", command.exec());
|
||||
}
|
||||
if std::env::var("CODEX_TEST_UPDATER_DELETE_CWD").as_deref() == Ok("prepared") {
|
||||
std::fs::remove_dir(std::env::current_dir().expect("project cwd"))
|
||||
.expect("delete updater cwd");
|
||||
}
|
||||
super::reexec_managed_updater(
|
||||
std::path::Path::new(&executable),
|
||||
std::path::Path::new(&state),
|
||||
)
|
||||
.expect("reexec updater");
|
||||
panic!("successful exec does not return");
|
||||
}
|
||||
|
||||
struct FakeInstallerHttp {
|
||||
response: InstallerResponse,
|
||||
requested_urls: Mutex<Vec<String>>,
|
||||
|
||||
@@ -114,6 +114,63 @@ impl Drop for TestDaemon {
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn managed_daemon_restarts_from_deleted_working_directory() -> Result<()> {
|
||||
let daemon = TestDaemon::new()?;
|
||||
let project = daemon.home.path().join("project");
|
||||
std::fs::create_dir(&project)?;
|
||||
let started = daemon
|
||||
.command()
|
||||
.current_dir(&project)
|
||||
.args(["app-server", "daemon", "start"])
|
||||
.output()?;
|
||||
ensure!(
|
||||
started.status.success(),
|
||||
"{}",
|
||||
String::from_utf8_lossy(&started.stderr)
|
||||
);
|
||||
let old_pid = daemon.pid("app-server.pid")?;
|
||||
let old_updater_pid = daemon.pid("app-server-updater.pid")?;
|
||||
signal(old_updater_pid, libc::SIGTERM)?;
|
||||
wait_for_exit(old_updater_pid)?;
|
||||
|
||||
// Delete the restart caller's cwd without changing the test process's cwd.
|
||||
let restarted = Command::new("/bin/sh")
|
||||
.args([
|
||||
"-c",
|
||||
"rmdir \"$1\" && exec \"$2\" app-server daemon restart",
|
||||
"restart",
|
||||
])
|
||||
.arg(&project)
|
||||
.arg(&daemon.codex)
|
||||
.current_dir(&project)
|
||||
.env("CODEX_HOME", daemon.home.path())
|
||||
.output()?;
|
||||
ensure!(
|
||||
restarted.status.success(),
|
||||
"{}",
|
||||
String::from_utf8_lossy(&restarted.stderr)
|
||||
);
|
||||
let restarted: Value = serde_json::from_slice(&restarted.stdout)?;
|
||||
assert_eq!(restarted["status"], "restarted");
|
||||
assert_ne!(daemon.pid("app-server.pid")?, old_pid);
|
||||
assert_ne!(daemon.pid("app-server-updater.pid")?, old_updater_pid);
|
||||
assert_eq!(daemon.lifecycle("version")?["status"], "running");
|
||||
let updater_socket = daemon
|
||||
.home
|
||||
.path()
|
||||
.join("app-server-daemon/app-server-updater.sock");
|
||||
let deadline = Instant::now() + Duration::from_secs(30);
|
||||
while std::os::unix::net::UnixStream::connect(&updater_socket).is_err() {
|
||||
ensure!(
|
||||
Instant::now() < deadline,
|
||||
"replacement updater did not become ready"
|
||||
);
|
||||
std::thread::sleep(Duration::from_millis(50));
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
|
||||
fn signal(pid: u32, signal: libc::c_int) -> Result<()> {
|
||||
let raw_pid = libc::pid_t::try_from(pid).context("pid out of range")?;
|
||||
ensure!(
|
||||
|
||||
Reference in New Issue
Block a user