fix(liveness): a process you cannot signal is not a process that is gone - #505
Conversation
_agmsg_pid_alive already read the ESRCH/EPERM distinction. Add the ps cross-check #503 settled on, so "dead" needs a source that does not depend on signalling permission at all, and a fork-free fast path, so the common answer still costs no subshell — callers poll this in loops that exist to be fork-free.
`kill -0` answers "can I signal this", not "is this running". Under a sandbox a live watcher, bridge or app-server fails it, and every shipped caller read the exit status directly: status printed live processes as stale pidfiles, session-start and codex-monitor started a second bridge and app-server beside the running ones, and the launcher reclaimed a live owner's lock — the #485 duplicate-children shape. All seventeen sites now call _agmsg_pid_alive; instance-id.sh is sourced where it was only reachable transitively, or not at all.
pid 1 is a live process this user cannot signal, so it stages the distinction directly; the suite skips where no such pid exists. A dead-pid case guards the other direction, since "assume alive" must not make everything look alive. The sweep is #500's lesson as a test: a partially-hardened file reads like a fixed one, so no shipped script may call kill -0 outside the helper.
… process `kill -0 0` succeeds because 0 addresses the caller's own process group, so a digits-only check called it alive, and callers kill what liveness reports alive: `kill 0` TERMs the group, the caller included. A pidfile holding 0 was all it took. Leading zeros go too — nothing writes them and kill(1) may read them as octal. The check is split out as _agmsg_pid_valid and also gates the two launcher sites that kill a recorded pid without asking about liveness first. Reported by review of #505.
…t a number Past INT32_MAX kill(1) rejects the ARGUMENT rather than reporting ESRCH, and everything that is not ESRCH reads as EPERM, i.e. alive. An oversized value in a pidfile was therefore alive forever: lock never reclaimed, bridge never restarted. Bounding the input is what keeps "not ESRCH" meaning "EPERM". The ESRCH tests stubbed kill and passed the literal 999, which is a RUNNING process on some CI hosts — so the new ps cross-check correctly called it alive and the tests failed there and only there. They now use a pid that is genuinely gone, and the cross-check gets a test of its own. Reported by review of #505.
A Windows process id is a DWORD, and liveness there reads the native process table through tasklist rather than kill(1)'s signed pid_t. The INT32_MAX bound sat in front of that branch, so 2147483648..4294967295 — legitimate native pids — were called dead, and a live watcher or lock owner stale. Reported by review of #505.
The comment under the ceiling selection still explained only the POSIX side, which reads as if the Windows branch were an exception to a rule rather than the other half of it. Reported by review of #505.
|
Ran this branch on Windows 11 (10.0.26200) + Git Bash (MSYS2 3.4.7, bash 5.2.15), where Windows blocker found while validatingOn Git Bash there are TWO pid namespaces. Converted call sites that hold MSYS pids (spot-checked on this branch):
Net: this branch fixes the native-pid reads (bridge pids from A shape that keeps both namespaces visible — and keeps the hot loops fork-free for the common live-MSYS-parent case, preserving #496's builtins win: case "${MSYSTEM:-}" in
MINGW*|MSYS*|CLANGARM*)
kill -0 "$pid" 2>/dev/null && return 0 # MSYS-namespace pid: builtin, no fork
MSYS_NO_PATHCONV=1 tasklist /FI "PID eq $pid" 2>/dev/null | grep -q "$pid"
return $? ;;
esacCost: a cross-namespace numeric collision can read "alive" for the wrong process — the same tolerance the pre-#505 code already had for MSYS pids, extended rather than inverted. A Windows test that pins BOTH answers (a live A second Windows gap, independent of the above (signal side)Even with liveness answered correctly, the teardown/replace paths still signal with plain The consequence is worse than "cannot stop": What you can check without a Windows machine
Verification scope: Windows only — we have no macOS/Linux measurements for this branch. 🤖 Generated with Claude Code |
2 participants
delivery.sh statusreports a running bridge and app-server as stale pidfiles under a sandbox.kill -0answers "can I signal this", not "is this running", and the two differ exactly where it matters: a failedkill -0is ESRCH (gone) or EPERM (alive, but not signalable by us). Every shipped caller read the exit status directly.The misreport is the mild symptom. The same check decides whether to start a second bridge, whether to reuse the app-server, and whether a lock's owner is dead enough to reclaim.
What changed
_agmsg_pid_aliveinscripts/lib/instance-id.shalready read the ESRCH/EPERM distinction — it was simply not what anything called. It now also cross-checksps, per the rule #503 settled on for the test helper, so concluding "dead" needs a source that does not depend on signalling permission at all. A fork-free fast path was added first: callers poll this in loops that exist to be fork-free (#466/#496), so the common answer must not cost a subshell.All 17 liveness sites now go through it.
instance-id.shis sourced where it was reachable only transitively (codex-bridge-launcher.sh, via a conditional branch ofrole-session.sh) or not at all (codex-monitor.sh,resolve-project.sh).scripts/delivery.shcodex/_delivery.shcodex/_session-start.shcodex/codex-monitor.shcodex/codex-bridge-launcher.shscripts/lib/resolve-project.shresolve-project.shwas the #500 shape exactly: its marker GC already used the EPERM-aware helper (behind adeclare -Fguard), whileagmsg_pid_is_agenttwelve lines up still used a barekill -0.What a pid is allowed to be
Review turned the validation into its own concern,
_agmsg_pid_valid, because each accepted-but-wrong value fails differently and none of them fail safe:0is not a pid.kill -0 0succeeds because 0 addresses the caller's own process group. A digits-only check called it alive, and callers kill what liveness reports alive —kill 0TERMs the group, the caller included. A corrupt pidfile holding0was all it took. Leading zeros go too: nothing writes them andkill(1)may read them as octal.kill(1)rejects the argument ("not a pid or valid job spec") instead of reporting ESRCH — and everything that is not ESRCH reads as EPERM, i.e. alive. Unbounded, an oversized value read as alive forever: its lock never reclaimed, its bridge never restarted. Bounding the input is what keeps "not ESRCH" meaning "EPERM".tasklist, notkill(1)'s signedpid_t; the POSIX bound would call a legitimate native pid dead. POSIX gets INT32_MAX, MSYSTEM gets DWORD max.Comparisons are patterns and
${#pid}only — never$(( ))or-gton the untrusted value, both of which evaluate what they are given._agmsg_pid_validalso gates the two launcher sites that kill a recorded pid without asking about liveness first ([ -n "$old_pid" ] && kill ...), which the helper alone would not have covered. Every otherkillof a file-sourced pid was already behind_agmsg_pid_alive; the one remaining (codex-monitor.sh'skill "$server_bg") is our own child from$!, not a file.Tests
0/00/000/0123, and the ceilings on both platforms — the Windows case runs withMSYSTEMset, so both halves are checkable from either host, and widening or narrowing one side alone fails.scripts/orbin/may callkill -0outside the helper — is test: close inherited bats fd 3 in all backgrounded process launches #500's lesson as a test. A partially-hardened file reads like a fixed one; this is what stops the next one.Every new test was checked against a mutation that removes the property it claims: EPERM treated as dead, the fast path deleted, the ps cross-check dropped, the pid ceiling removed, the ceiling collapsed to one number, the DWORD ceiling applied everywhere, and a fresh bare
kill -0reintroduced into a shipped script.CI caught one thing local runs could not. Two shards failed on an intermediate head while every other shard and the full local suite passed. The pre-existing ESRCH tests stubbed
killand passed the literal999— which is a running process on some hosts, so the newpscross-check correctly called it alive. The cross-check was right and the test's assumption was host-dependent; those tests now use a pid that is genuinely gone, and the cross-check got a test of its own.One existing test changed meaning rather than breaking:
marker-gc: skips when _agmsg_pid_alive is unavailablereached its guard by not sourcing the helper, which is the gap this PR closes. It now unsets the helper to reach the same guard, and a companion test pins the new guarantee that sourcingresolve-project.shalone provides it.Verification
Full bats suite locally on macOS: 810 passed, 0 failed, 8 skipped (none of them the new ones), plus every affected suite re-run after each review round.