diff --git a/codex-rs/app-server-daemon/src/backend/pid_start.rs b/codex-rs/app-server-daemon/src/backend/pid_start.rs index 1c286a8b14..26d90bc152 100644 --- a/codex-rs/app-server-daemon/src/backend/pid_start.rs +++ b/codex-rs/app-server-daemon/src/backend/pid_start.rs @@ -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::>>()?; - 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); diff --git a/codex-rs/app-server-daemon/src/background_command.rs b/codex-rs/app-server-daemon/src/background_command.rs new file mode 100644 index 0000000000..1be0c8133f --- /dev/null +++ b/codex-rs/app-server-daemon/src/background_command.rs @@ -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::>>()?; + 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(()) +} diff --git a/codex-rs/app-server-daemon/src/lib.rs b/codex-rs/app-server-daemon/src/lib.rs index 15ba7692b9..cc77d8b814 100644 --- a/codex-rs/app-server-daemon/src/lib.rs +++ b/codex-rs/app-server-daemon/src/lib.rs @@ -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)] diff --git a/codex-rs/app-server-daemon/src/update_loop.rs b/codex-rs/app-server-daemon/src/update_loop.rs index 8f55126878..3a07699953 100644 --- a/codex-rs/app-server-daemon/src/update_loop.rs +++ b/codex-rs/app-server-daemon/src/update_loop.rs @@ -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 { } #[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 {}", 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 0847655a30..598222a73e 100644 --- a/codex-rs/app-server-daemon/src/update_loop_tests.rs +++ b/codex-rs/app-server-daemon/src/update_loop_tests.rs @@ -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>, diff --git a/codex-rs/cli/tests/app_server_daemon.rs b/codex-rs/cli/tests/app_server_daemon.rs index 002012a95a..75807f6c9b 100644 --- a/codex-rs/cli/tests/app_server_daemon.rs +++ b/codex-rs/cli/tests/app_server_daemon.rs @@ -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!(