fix(instance-id): distinguish a failed ps observation from proof of death (#954)

scripts/lib/instance-id.sh

@@ -110,7 +110,7 @@ _agmsg_pid_valid() {
 # The EPERM reading and the ps cross-check are the same as _agmsg_pid_alive's --
 # a pid we minted is still a pid a sandbox may refuse to let us signal (#505).
 _agmsg_pid_alive_local() {
-  local pid="$1" err stat
+  local pid="$1" err stat probe rc canary tstat _p _s _rest
   # The POSIX ceiling, explicitly, whatever the host. _agmsg_pid_valid widens to
   # the DWORD range when MSYSTEM is set, which is right for a number tasklist
   # will be asked about and wrong for one kill(1) will parse: past INT32_MAX kill
@@ -131,10 +131,47 @@ _agmsg_pid_alive_local() {
   esac
   # kill(2) says gone. ps does not depend on signalling permission at all, so
   # requiring it to agree is what keeps a sandbox from turning "cannot signal"
-  # into "not running".
-  stat="$(ps -o stat= -p "$pid" 2>/dev/null | tr -d ' ')"
-  [ -n "$stat" ] || return 1
-  case "$stat" in Z*) return 1 ;; esac   # exited, just not reaped yet
+  # into "not running" (#505). But an EMPTY ps result is NOT proof of death: a
+  # transient ps failure and a truly-absent pid both produce nothing, and reading
+  # that as "gone" is #954 -- callers delete files, release locks, and respawn on
+  # it. Distinguish "proof of absence" from "absence of proof" by co-observing a
+  # known-live pid -- our own $$ -- in the SAME observation. Take a FULL snapshot
+  # (no -p filter, so the target pid is never handed to ps and cannot poison the
+  # query, e.g. macOS "process id too large"), parsed with builtins so only ps is
+  # external and a stripped PATH cannot itself become the failed observation:
+  #   - $$ absent from the snapshot => ps produced nothing usable => UNKNOWN =>
+  #     assume alive, exactly as the EPERM branch above. A failed observation is
+  #     not proof of absence.
+  #   - $$ present, target absent  => ps listed us and did not list the target =>
+  #     positive proof the target is gone => dead.
+  #   - target present, zombie     => gone too.
+  # `|| rc=$?` keeps the assignment out of set -e's reach: a command-substitution
+  # assignment returns the substituted command's exit status as its OWN, so under
+  # errexit in a caller that did NOT invoke us as a condition, a non-zero ps would
+  # terminate the shell right here -- leaking a failed observation to caller death
+  # instead of the UNKNOWN => alive verdict below. The leaf helper's contract must
+  # not depend on how the caller spelled the call.
+  rc=0
+  probe="$(ps -Ao pid=,stat= 2>/dev/null)" || rc=$?
+  canary=0; tstat=""
+  while read -r _p _s _rest; do
+    if [ "$_p" = "$$" ]; then canary=1; fi
+    if [ "$_p" = "$pid" ]; then tstat="${_s:-?}"; fi
+  done <<PROBE
+$probe
+PROBE
+  if [ -n "$tstat" ]; then
+    case "$tstat" in Z*) return 1 ;; esac  # zombie: exited, not yet reaped
+    return 0                                # target present -> alive (seeing it is proof enough)
+  fi
+  # Target not listed. Trust "absent" ONLY when ps COMPLETED the snapshot (exit 0)
+  # AND that snapshot included our own $$. A non-zero exit means the listing was
+  # truncated -- ps can print part of it (even our own line) and then fail -- and a
+  # pid that would have come later proves nothing; canary presence shows only that
+  # WE were listed, never that the listing FINISHED. Anything short of a complete,
+  # self-including snapshot is UNKNOWN -> assume alive (#954), which also fails safe
+  # where "ps -Ao" is unsupported (it exits non-zero rather than lying "gone").
+  if [ "$rc" -eq 0 ] && [ "$canary" = 1 ]; then return 1; fi
   return 0
 }
 

tests/test_instance_id.bats

@@ -199,6 +199,100 @@ gone_pid() {
   _agmsg_pid_alive 42
 }
 
+# --- #954: the ps cross-check must tell "proof of absence" from "absence of
+# proof". Both halves are asserted with the SAME genuinely-dead pid, so a result
+# of "alive" can ONLY be the canary suppressing the death verdict, never the pid
+# being live -- exactly the distinction the bug erased. ---
+
+@test "pid_alive: a truly-gone pid is PROVEN dead when ps answers, and cleanup fires (#954)" {
+  skip_on_windows "POSIX kill path; Windows uses tasklist (#134)"
+  # A pid this shell minted and reaped: kill(2) returns a real ESRCH, real ps
+  # lists our own $$ but not the gone pid -> positive proof of absence.
+  sh -c 'exit 0' & local gone=$!; wait "$gone" 2>/dev/null
+  run _agmsg_pid_alive_local "$gone"
+  [ "$status" -ne 0 ] || { echo "a gone pid with a working ps read alive"; false; }
+  # The cleanup-side contract: `... || rm -f` DOES delete for a proven-gone pid.
+  local marker="$RUN_DIR/marker.$gone"; : > "$marker"
+  _agmsg_pid_alive_local "$gone" || rm -f "$marker"
+  [ ! -e "$marker" ] || { echo "cleanup did not fire on a proven-dead pid"; false; }
+}
+
+@test "pid_alive: a truly-gone pid reads ALIVE when ps cannot answer -- a failed observation is not proof, and cleanup is suppressed (#954)" {
+  skip_on_windows "POSIX kill path; Windows uses tasklist (#134)"
+  sh -c 'exit 0' & local gone=$!; wait "$gone" 2>/dev/null
+  # ps cannot answer: it emits nothing and fails. The canary ($$) is absent from
+  # the output, so the helper cannot see even itself -> observation failed. The
+  # pid is genuinely gone, so "alive" here is UNAMBIGUOUSLY the canary firing.
+  ps() { return 1; }
+  run _agmsg_pid_alive_local "$gone"
+  [ "$status" -eq 0 ] || { echo "a failed ps was read as proof of death (the #954 bug)"; false; }
+  # The cleanup-side contract: `... || rm -f` does NOT delete when we could not
+  # observe. The file a live process might still own is left intact.
+  local marker="$RUN_DIR/marker.$gone"; : > "$marker"
+  _agmsg_pid_alive_local "$gone" || rm -f "$marker"
+  [ -e "$marker" ] || { echo "cleanup fired on an UNKNOWN observation (UNKNOWN leaked to the cleanup side)"; false; }
+}
+
+@test "pid_alive: ps that omits the target but still lists the canary is proof of death (#954)" {
+  skip_on_windows "POSIX kill path; Windows uses tasklist (#134)"
+  # ps answers (lists $$, proving it ran) but does not list the target -> the
+  # target is provably gone even though ps was reachable.
+  kill() { echo "bash: kill: - No such process" >&2; return 1; }
+  ps() { printf '%s S\n' "$$"; }   # only the canary, never the queried target
+  run _agmsg_pid_alive_local 424242
+  [ "$status" -ne 0 ] || { echo "an answered ps that omits the target did not read dead"; false; }
+}
+
+@test "pid_alive: a snapshot with output but WITHOUT the canary is UNKNOWN, not death (#954)" {
+  skip_on_windows "POSIX kill path; Windows uses tasklist (#134)"
+  # The subtle leak: ps returns SOME lines but not our own $$ (a partial or garbage
+  # snapshot, or a failure that still printed something). Without the canary the
+  # observation is untrusted, so a genuinely-gone pid must STILL read alive -- a
+  # non-empty result is not itself proof the snapshot was complete. No retry count
+  # or partial output may turn this into a death verdict, and cleanup stays put.
+  sh -c 'exit 0' & local gone=$!; wait "$gone" 2>/dev/null
+  ps() { printf '999999 R\n'; }   # a line, but never $$ and never the target
+  run _agmsg_pid_alive_local "$gone"
+  [ "$status" -eq 0 ] || { echo "a canary-less snapshot was read as proof of death"; false; }
+  local marker="$RUN_DIR/marker.$gone"; : > "$marker"
+  _agmsg_pid_alive_local "$gone" || rm -f "$marker"
+  [ -e "$marker" ] || { echo "cleanup fired on a canary-less snapshot (UNKNOWN leaked to cleanup)"; false; }
+}
+
+@test "pid_alive: a ps that lists the canary but EXITS NON-ZERO is a truncated snapshot -> UNKNOWN, not death (#954)" {
+  skip_on_windows "POSIX kill path; Windows uses tasklist (#134)"
+  # The last leak: ps prints part of the snapshot -- even our own $$ -- and THEN
+  # fails. The target's line may simply never have been reached, so its absence
+  # from a truncated listing is not proof. A non-zero exit must read as UNKNOWN
+  # regardless of what partial output was captured. (This is also why an "ps -Ao"
+  # a platform does not support fails safe rather than lying "gone".)
+  sh -c 'exit 0' & local gone=$!; wait "$gone" 2>/dev/null
+  ps() { printf '%s S\n' "$$"; return 1; }   # canary printed, then ps fails
+  run _agmsg_pid_alive_local "$gone"
+  [ "$status" -eq 0 ] || { echo "a non-zero ps exit was read as proof of death despite partial output"; false; }
+  local marker="$RUN_DIR/marker.$gone"; : > "$marker"
+  _agmsg_pid_alive_local "$gone" || rm -f "$marker"
+  [ -e "$marker" ] || { echo "cleanup fired on a failed (truncated) ps snapshot"; false; }
+}
+
+@test "pid_alive: a failing ps under set -e does not terminate a non-conditional caller (#954)" {
+  skip_on_windows "POSIX kill path; Windows uses tasklist (#134)"
+  # The leaf helper's contract must not depend on caller syntax. Called as a bare
+  # statement under errexit, a ps that fails must not kill the shell before the
+  # UNKNOWN -> alive verdict: the observation failure has to surface as "alive",
+  # never as caller termination.
+  run bash -c '
+    set -e
+    source "'"$SKILL_DIR"'/scripts/lib/instance-id.sh"
+    ps() { return 1; }
+    kill() { echo "bash: kill: - No such process" >&2; return 1; }
+    _agmsg_pid_alive_local 99999999
+    echo REACHED-alive
+  '
+  [ "$status" -eq 0 ] || { echo "the caller shell died on a failing ps under set -e"; false; }
+  printf '%s\n' "$output" | grep -q REACHED-alive || { echo "did not continue past the helper call"; false; }
+}
+
 # --- agmsg_normalize_instance_id ---
 
 @test "normalize: a composite token passes through unchanged (idempotent)" {