fix(teardown): make the leaked-process reap prove what it reaped

[matthewstrud]

What this fixes

bin/fm-teardown.sh enumerated leaked processes with lsof. On a host without lsof the reap silently did nothing while reporting success. Around eight torn-down workers each left a headless browser tree alive, plus dev servers still serving hours later; the machine reached load 116 on 8 cores with 0 of 15 GB free and had to be recovered by hand, twice.

The defect is not the missing tool. It is that a guard which cannot run reports success. A guard that silently no-ops is worse than no guard, because it is trusted.

What changed

Enumeration moves into a reusable library, bin/fm-process-tree-lib.sh:

• /proc is the sole authority. No lsof path and no second enumerator — one that cannot prove coverage is the same silent-degradation bug in another costume. ps is gone from enumeration entirely; ppid, run state, uid and birth identity all come from the same /proc record, so they can never describe different processes.
• Whole-tree enumeration. Counting orphans by PPID==1 undercounts badly: an orphan's children reparent to it and read as healthy. During the incident, six apparent orphans were the visible tip of nineteen processes.
• Exact PIDs only. Every signal targets a PID bound to its birth identity, rechecked immediately before that individual signal, so a recycled PID is never adopted or hit. No pkill, no killall, no ps-grep-xargs: sibling worktree processes carry identical command lines, and one careless pattern previously killed four unrelated workers.
• Refuses instead of pretending. If coverage cannot be proved, teardown refuses before destructive cleanup, preserves the worktree and tasktmp, and says why.

Coverage and ownership are separated deliberately

Conflating them makes the guard useless in one direction or the other:

• Cannot bind a live process's birth identity → uncovered ground → refuse and name the pid.
• Cannot read a live process's cwd → weaker evidence. If ancestry reached it, it is covered and signaled normally. If it is outside our tree, there is no evidence it is ours, so the pid is disclosed and teardown proceeds.
• A different-uid process is known not to be ours → skipped before cwd-root collection so it can never cause a false refusal, though still traversed as an intermediary so our own descendants below it are found.

This distinction is not academic. An earlier revision refused on any unreadable cwd, which meant it refused on every ordinary Linux host, forever — systemd --user, (sd-pam), sshd-session and sftp-server all hide their cwd. A teardown that never cleans anything is strictly worse than the bug it guards against. Claim what you can observe, disclose what you cannot, refuse only on evidence.

Known limitations, stated rather than hidden

• A leaked process that both hides its cwd and has detached from this task's tree is not found. That needs a deliberately nondumpable process; the leak class this exists for — browser trees and dev servers — are ordinary readable processes.
• Under hidepid=1/hidepid=2, foreign /proc entries are restricted or invisible, so traversal through a foreign intermediary can undercount our own descendants, and userspace cannot detect that invisibility.

The durable fix for both is a cgroup or session ownership boundary, which belongs to the follow-up resource guard, not here.

Scope

This is piece 1 of 3, split deliberately. Not included, and coming as separate PRs: the freeze-and-grace signalling sequence, and the /tmp scratch attribution.

How this was validated — please read before merging

The validation pipeline ran and passed intent, rebase, review (no findings remaining) and test, and produced its documentation step. It was then killed by the OOM killer before it could finish, as it was on all six attempts: its peak is 3.6–4.0 GB against 3–4 GB available on this machine, including one attempt with the machine entirely to itself.

So the following were completed by hand, not by the pipeline:

• shellcheck on the changed files (library clean under full extended analysis; fm-teardown.sh and the test file clean with extended analysis disabled, because full analysis on that file exceeds this machine's memory — it does so on main's copy too, so CI will cover it)
• the local test suites
• this push and PR

This is not "pipeline-validated" end to end. Review and tests passed inside the pipeline; lint, push and PR were manual. Given that this change exists because a guard reported success it had not earned, overstating its validation would be the same defect in our own colours.

Local results: fm-teardown 68 passed / 0 failed, fm-gate-refuse 7/0, fm-backlog-atomicity 78/0.

One pre-existing failure on main is unrelated and untouched here: fm-watch-triage.test.sh — "the fixture captured no process-event result". This branch modifies neither bin/fm-watch.sh nor that test; it reproduces on main and appears to be what the unmerged #2877 addresses.

Testing notes

Covered: whole-tree reaping; the outage regression (parent exits first, reparenting verified, child asserted reaped); recycled PID not adopted; exec change preserving identity; foreign-uid cwd root skipped; foreign-uid intermediary traversed but never signaled; unreadable cwd disclosed rather than refused; refusal when a live identity cannot be read; refusal on an empty /proc snapshot; mid-scan churn converging instead of refusing.

One case deliberately runs against the real /proc and real process table of the running machine. Every other case builds a synthetic /proc containing no nondumpable processes — and a suite fully green against that synthetic world once hid a revision that refused on every real host. That test was verified non-vacuous by restoring the bug and confirming it fails.

Tests clean up unconditionally on every failure path; a test for a process-leak fix must not leak processes.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 37e1db242b

ℹ️ 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".

[chatgpt-codex-connector]

Preserve teardown support on macOS

Captain, on every supported macOS installation, /proc does not exist, so this new mandatory check always returns "/proc process-cwd enumeration is unavailable"; reap_task_worktree_processes consequently refuses before returning the worktree or removing task state. Since the README advertises macOS support and includes macOS-only backends, teardown needs a Darwin-compatible enumerator or another ownership boundary rather than unconditionally requiring Linux /proc.

Useful? React with 👍 / 👎.

[greptile-apps]

Confidence Score: 5/5

The PR appears safe to merge, with no concrete changed-code defect remaining after review.

The new teardown path binds discovered processes to /proc birth identities, verifies those identities before each signal, retries ordinary tree instability, and preserves task resources whenever coverage or final cleanup cannot be proven.

_{Reviews (1): Last reviewed commit: "no-mistakes(document): Document process-..." | Re-trigger Greptile}

[matthewstrud]

Closing: this was raised against this repository by a tooling default rather than by intent. The work belongs on the author's fork, and any future contribution here will be opened deliberately. Apologies for the noise.