From 5a361b5596762acd1bb0a143187a0ed28adb24d3 Mon Sep 17 00:00:00 2001
From: jif <jif@openai.com>
Date: Tue, 27 Oct 2026 16:42:45 +0000
Subject: [PATCH] Explicitly unlock daemon PID reservation probes on Unix
 (#57512)

## Why

A concurrent fork can inherit a PID reservation probe's file descriptor and keep its lock alive after the probe closes it. This can make the app-server daemon appear to be starting even when no live startup reservation exists.

## What changed

Explicitly release the probe's lock on Unix before returning that no reservation is active, and propagate unlock failures.

## Testing

Add a regression test using a duplicated file handle to simulate fork inheritance and verify that the probe leaves the daemon inactive. Extend coverage to confirm that a held reservation still reports `Starting` when the PID file is missing, and include pruning results in release-retirement assertion failures.

GitOrigin-RevId: ecc58df1fe5cadecb6ed658352f1300c39237358
---
 codex-rs/app-server-daemon/src/backend/pid.rs | 22 +++++++++-
 .../src/backend/pid_tests.rs                  | 43 +++++++++++++++++++
 .../src/prepare_install_tests.rs              |  4 +-
 3 files changed, 66 insertions(+), 3 deletions(-)

diff --git a/codex-rs/app-server-daemon/src/backend/pid.rs b/codex-rs/app-server-daemon/src/backend/pid.rs
index b80743523c..7a196e0604 100644
--- a/codex-rs/app-server-daemon/src/backend/pid.rs
+++ b/codex-rs/app-server-daemon/src/backend/pid.rs
@@ -1,4 +1,5 @@
 //! PID reservations serialize detached launches; process identities protect stale-record cleanup.
+//! Missing-PID probes explicitly unlock so concurrent forks cannot retain their transient reservations.
 
 #[path = "pid_identity.rs"]
 mod identity;
@@ -703,7 +704,26 @@ async fn reservation_lock_is_active(path: &Path) -> Result<bool> {
                 .with_context(|| format!("failed to inspect pid lock file {}", path.display()));
         }
     };
-    Ok(!try_lock_file(&file)?)
+    probe_reservation_lock(file)
+}
+
+#[cfg(any(unix, windows))]
+fn probe_reservation_lock(file: fs::File) -> Result<bool> {
+    if !try_lock_file(&file)? {
+        return Ok(true);
+    }
+    #[cfg(unix)]
+    {
+        use std::os::fd::AsRawFd;
+
+        // A concurrent fork can inherit this descriptor before it closes. Release the
+        // probe's lock explicitly so the child cannot look like a live reservation.
+        if unsafe { libc::flock(file.as_raw_fd(), libc::LOCK_UN) } != 0 {
+            return Err(std::io::Error::last_os_error())
+                .context("failed to unlock pid reservation probe");
+        }
+    }
+    Ok(false)
 }
 
 #[cfg(not(any(unix, windows)))]
diff --git a/codex-rs/app-server-daemon/src/backend/pid_tests.rs b/codex-rs/app-server-daemon/src/backend/pid_tests.rs
index e04b230cdd..0e11df230e 100644
--- a/codex-rs/app-server-daemon/src/backend/pid_tests.rs
+++ b/codex-rs/app-server-daemon/src/backend/pid_tests.rs
@@ -67,6 +67,17 @@ async fn locked_empty_pid_file_is_treated_as_active_reservation() {
         PidFileState::Starting
     );
     assert!(pid_file.exists());
+
+    tokio::fs::remove_file(&pid_file)
+        .await
+        .expect("remove pid file");
+    assert_eq!(
+        backend
+            .read_pid_file_state()
+            .await
+            .expect("read missing pid"),
+        PidFileState::Starting
+    );
 }
 
 #[tokio::test]
@@ -89,6 +100,38 @@ async fn unlocked_empty_pid_file_is_treated_as_stale_reservation() {
     assert!(!pid_file.exists());
 }
 
+#[cfg(unix)]
+#[tokio::test]
+async fn reservation_probe_releases_lock_with_duplicated_handle() {
+    let temp_dir = TempDir::new().expect("temp dir");
+    let backend = PidBackend::new(
+        temp_dir.path().join("codex"),
+        temp_dir.path().join("app-server.pid"),
+        /*remote_control_enabled*/ false,
+    );
+    let file = std::fs::OpenOptions::new()
+        .create(true)
+        .truncate(false)
+        .write(true)
+        .open(&backend.lock_file)
+        .expect("open pid lock file");
+    // Share the open file description as fork would, without inheriting
+    // unrelated descriptors from other tests.
+    let inherited = file.try_clone().expect("clone pid lock file");
+
+    let probe = super::probe_reservation_lock(tokio::fs::File::from_std(file));
+    let running = backend.is_starting_or_running().await;
+
+    drop(inherited);
+    assert_eq!(
+        (
+            probe.expect("probe reservation"),
+            running.expect("read daemon status"),
+        ),
+        (false, false),
+    );
+}
+
 #[tokio::test]
 async fn stop_waits_for_live_reservation_to_resolve() {
     let temp_dir = TempDir::new().expect("temp dir");
diff --git a/codex-rs/app-server-daemon/src/prepare_install_tests.rs b/codex-rs/app-server-daemon/src/prepare_install_tests.rs
index ee33ca866b..48f389a21c 100644
--- a/codex-rs/app-server-daemon/src/prepare_install_tests.rs
+++ b/codex-rs/app-server-daemon/src/prepare_install_tests.rs
@@ -120,8 +120,8 @@ async fn seeds_full_package() {
     use std::os::unix::fs::PermissionsExt;
     let read_only = old.join("codex-resources/nested");
     std::fs::set_permissions(&read_only, std::fs::Permissions::from_mode(0o555)).unwrap();
-    let _ = release_pruning::prune(&daemon, &root, &settings).await;
-    assert!(!old.exists());
+    let result = release_pruning::prune(&daemon, &root, &settings).await;
+    assert!(!old.exists(), "release was not retired: {result:#?}");
     for retained in [&selected, &previous, &outside, &linked] {
         assert!(retained.join("codex-resources/nested/runtime").is_file());
     }