feat: terminate processes rooted in a worktree before removing it - #50
Conversation
git worktree remove deletes the directory out from under any build daemon still running there; the daemon reparents to init and keeps burning CPU against a path that no longer exists, invisibly, for days. Add terminateProcessesRootedIn: one ps/lsof snapshot, cwd-prefix and absolute-argv-token matching with descendant closure on cwd roots only, filtered to own-uid terminal-less processes with the caller's ancestor chain protected. SIGTERM, a short grace poll, then SIGKILL, verifying the command line before every signal so a recycled pid is never hit. Over 20 candidates means the match is not trusted: nothing is signalled. All three removal paths call it; failures never block the removal itself.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 118e353973
ℹ️ 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 (spawnError.status === 1 && typeof spawnError.stdout === 'string') { | ||
| return spawnError.stdout; | ||
| } | ||
| return ''; |
There was a problem hiding this comment.
Provide cwd discovery when lsof is unavailable
On POSIX installations without lsof—including the current minimal Linux environment and systems installed with only the documented Node/Git prerequisites (README.md:68)—this returns an empty map, so cwdRooted has no entries. Typical daemons whose argv is just something like sleep 300 contain no worktree path, meaning every removal path still deletes the worktree while leaving exactly these cwd-rooted processes running; use a platform fallback such as Linux /proc/<pid>/cwd or make lsof a validated runtime requirement rather than silently disabling the primary match.
Useful? React with 👍 / 👎.
The two entry points had grown to 93 and 64 lines, over the 60-line unit-size gate, which made the match rules and the signalling sequence hard to read as separate concerns. Extract candidate discovery, filtering, and the SIGTERM/grace/SIGKILL sequence into named helpers. Behavior is unchanged: same match rules, same ownership-versus-mention asymmetry in the descendant closure, same candidate cap, same per-signal command verification.
The audit ran without coverage data, so CRAP was computed as if no code were tested. A five-branch function scored exactly at the threshold and failed the gate while sitting far inside the cyclomatic and cognitive ceilings, which made the check fire on any new file with small private helpers regardless of how well tested it was. Generate Istanbul-format coverage before the audit and point the health analysis at it. The existing coverage script emits v8 format, which the analyzer cannot read, so this adds a parallel Istanbul run with its thresholds disabled: this step measures complexity, not coverage ratchets, which the test job already enforces.
The quality job only installed dependencies, so the suite could not resolve the built CLI and the coverage run failed on module resolution.
git worktree removedeletes the directory out from under any build daemon still running there. The daemon reparents to init and keeps burning CPU against a path that no longer exists — observed in the wild at three trees, 45 processes, three cores pinned for six days.terminateProcessesRootedIntakes oneps/lsofsnapshot and matches processes by cwd prefix and by absolute argv token, expanding the descendant closure from cwd roots only: a process that owns the directory loses its children, one that merely names the path in its arguments does not. Candidates are filtered to the caller's own uid and to terminal-less processes, so an interactive shell parked in the tree is never signalled, and the caller's own ancestor chain is protected. Signalling is SIGTERM, a short grace poll, then SIGKILL, re-verifying the command line before every signal so a recycled pid is never hit. More than twenty candidates means the match is not trusted and nothing is signalled at all.All three removal paths call it. Termination failures never block the removal itself.
Test: real processes spawned into a temp fixture — direct child, shell-forked grandchild via closure, and an out-of-tree bystander that must survive.