fix: fall back to the liveness probe when the identity probe is denied - #88
Conversation
validateProcessIdentity() collapsed "the probe was denied" into "the process is dead", the same defect #79 reported for isProcessAlive(). Inside a Codex workspace-write sandbox `spawnSync ps` fails with EPERM unconditionally, so every job that recorded a pidIdentity would be auto-reaped while still running. It is latent today only because claude-cli.mjs swallows the same ps failure at spawn time and persists pidIdentity: null. Treat EPERM/EACCES from the identity probe as "identity unknowable" and defer to isProcessAlive(), which still reports ESRCH for a dead PID. Genuine mismatches and absent PIDs stay classified as not alive, so PID-reuse detection is only given up when identity cannot be established at all. Closes #83 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: df13c13771
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (error?.code === "EPERM" || error?.code === "EACCES") { | ||
| return isProcessAlive(pid, options); |
There was a problem hiding this comment.
When the identity probe is denied and the original PID has been reused, the zero-signal check succeeds for the unrelated process, so this function incorrectly reports that its identity matches. cancelClaudeProcess() relies on this result at lines 940 and 961 before sending SIGTERM/SIGKILL to the process group, meaning cancellation can terminate an unrelated process; preserve an unknown/denied state so liveness fallback is used by the stale-job reaper without bypassing cancellation's identity check, and cover that caller behavior.
AGENTS.md reference: AGENTS.md:L5-L5
Useful? React with 👍 / 👎.
1 participant
Summary
validateProcessIdentity()collapsed "the probe was denied" into "the process is dead" — the same defect #79 reported forisProcessAlive(), at the branch #80 deliberately left alone.Inside a Codex
workspace-writesandbox,spawnSync psfails withEPERMunconditionally (not only for non-descendants, the way the zero-signal probe does), sogetProcessIdentity()always throws, the barecatchreturnsfalse, andreapStaleJobs()auto-reaps every live job that recorded apidIdentity.Why it was latent
claude-cli.mjsswallows the identicalpsfailure at spawn time and persistspidIdentity: null, so the reaper falls through toisProcessAlive()— the branch #80 fixed. That is two independent failures cancelling, not a design, and it breaks as soon as a job is spawned wherepsis permitted and reaped where it is not.Fix
Treat
EPERM/EACCESfrom the identity probe as "identity is unknowable" and defer toisProcessAlive(pid), which still reportsESRCHfor a genuinely dead PID. One liveness answer, at the same boundary #80 established:psexiting non-zero → not alive (unchanged)PID-reuse detection is only given up in the case where identity cannot be established at all, which is the alternative the issue itself proposes.
Validation
pass 34) and passes after (pass 35)identityImpl/killImpl, matching the pattern fix: preserve sandboxed background jobs on EPERM #80 introducedCloses #83
🤖 Generated with Claude Code