test: harden watcher-fixture cleanup - #117
Merged
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Intent
Close the demonstrated hostile watcher-fixture leak that contributed to the 2026-09-07 Mac kernel panic. Harden every long-lived fake child in tests/fm-pi-watch-extension.test.sh with a test-only hard deadline, fixture-root and control-parent disappearance escape, and TERM/INT handling that cannot ignore signals indefinitely; the hostile unretired-successor case must survive exactly the one TERM its Pi fallback contract tests without weakening that production contract. Add an identity-bound owned-child registry to the shared test cleanup path recording PID, process group, PID identity witness, fixture root, and release controls; release while roots exist, wait for close, then TERM and bounded KILL only a proven exact group, fail closed on ambiguity or survivors, and delete roots only after zero registered descendants. Replace the fixed 80 ms sleep with explicit close acknowledgement. Add a failure-injected hostile regression proving the old fixture leaked after root deletion and the fixed fixture does not, plus an outer-runner zero-live assertion after successful, failed, and timed-out files. Keep macOS Bash 3.2 and Linux CI compatible. Do not implement the larger published-executable identity binding, Pi arm publication, process-event/check-shim convergence, or bootstrap orphan sweep; those remain separate queued work. Preserve the existing direct PR #117 and allow the pipeline to rewrite its body with attestation.
What Changed
Risk Assessment
🚨 High: Captain, the core fail-closed cleanup invariant is still breakable: malformed or inaccessible registry state and an unverified peer can retire ownership without proving the exact group is gone, while affected long-lived fakes remain unbounded.
Testing
Completed 1 recorded test check.
Pipeline
Updates from git push no-mistakes
✅ **intent** - passed
✅ No issues found.
✅ **Rebase** - passed
✅ No issues found.
tests/fm-pi-watch-extension.test.sh:1227- The new failure test invokes only the fixed fixture. Injection throws before release, but the nested EXIT cleanup sees the root, writes the registered release control, waits for group close, and removes the root. It never runs the old fixture after root deletion, so it cannot prove the required criterion: “Add a failure-injected hostile regression proving the old fixture leaked after root deletion and the fixed fixture does not.”tests/fm-pi-watch-extension.test.sh:3545- The new process-group wrapper leaves the OpenCode test's long-livedsetIntervalchild at line 3570 unregistered. On a per-script timeout, the nested group survives the outer timeout; cleanup can release the registered arm, then refuse the still-live group as ambiguous, after which the runner deletes the registry. This violates the required criterion to harden “every long-lived fake child in tests/fm-pi-watch-extension.test.sh.”tests/fm-test-run.test.sh:1149- The outer-runner regression iterates onlyfail timeoutat line 1149. It never runs a successful file with a registered child, so the required zero-live assertion is absent for successful files: “plus an outer-runner zero-live assertion after successful, failed, and timed-out files.”tests/fm-pi-watch-extension.test.sh:2142-fm_fixture_waitingreturns false for a real control, fixture-root disappearance, control-parent disappearance, or the hard deadline, but this loop treats every false result as a wake and printssignal: streaming chain wake. If no trigger arrives before 15 seconds, or the root disappears, the fake emits a synthetic actionable wake. The same pattern exists at the late-wake and lock-handoff loops; distinguish trigger completion from escape/deadline completion.bin/fm-test-run.sh:2112- The outer runner callsowned-children-cleanupwith only the registry at line 2112. That path releases and removes child records but never removes their recorded fixture roots. In the added failure/timeout fixture,fm_test_tmprootcreates the root outsideRUN_TMPand the script's traps are disabled, whilecleanup_runremoves onlyRUN_TMP; the root therefore remains even after the child group is reaped. This leaves the required root-cleanup lifecycle incomplete: “delete roots only after zero registered descendants.”tests/lib.sh:87- The registry key ischild.$pid, andmvat line 96 overwrites an existing record. If leader A exits while a descendant keeps its group alive and the OS reuses A's PID for B, B replaces A's PID/PGID/identity record; cleanup can then remove B and delete the root while A's group remains live. Use collision-safe no-clobber or unique record names so the identity witness cannot be discarded by PID reuse.bin/fm-test-run.sh:2084-mkdir -p "$registry"runs while callers have already disabledset -e, and its status is ignored. If registry creation fails, the cleanup and zero-live helpers treat a missing registry as success, allowing a file with an unregistered surviving child to pass without the intended outer assertion. Fail closed before launching the file when the registry cannot be created.bin/fm-test-run.sh:2112- When owned-child cleanup refuses an ambiguous group or reports a group surviving KILL,run_script_boundedonly changes the result code; the EXIT cleanup still unconditionally removesRUN_TMPand the registry. A surviving process is then left without its PID/PGID/identity evidence or a safe retry handle. Preserving or handing off that registry on cleanup failure is required to make the fail-closed outcome durable.🔧 Fix: Captain: harden fixture cleanup ownership and regression coverage
6 issues (4 errors, 2 warnings) still open:
tests/lib.sh:205- An unreadable registry is treated as empty: both cleanup helpers return success after only checking-d, so a registered child can chmod the registry000, survive cleanup, and still have its root and runner evidence removed. Treat inaccessible registry state as cleanup failure.tests/lib.sh:144- The record validation accepts empty and reserved PID/PGID fields. A record with an empty PGID makes the failedkill -0 -- "-"look like a closed group, allowing the marked fixture root to be deleted without identity or group proof. Require nonempty numeric fields and reject PGIDs 0 and 1.tests/lib.sh:172- When a leader is gone, the peer fallback trusts the stored peer PGID plus identity but never rechecks the peer's current PGID or the cleanup process's own group. A peer moved from group G to H while another descendant remains in G can cause the leader record and root to be retired even though G is still live. Revalidate peer membership before accepting this fallback.tests/fm-pi-watch-extension.test.sh:1577- The addednode -e 'setInterval(...)'lock-holder children at lines 1577, 1657, and 3653 only receive registry metadata; they have no hard deadline or fixture-root/control-parent escape. In the first two copies, registration occurs beforetry/finally, so a registration failure throws and leaves the long-lived child unregistered. This contradicts the required criterion: “Harden every long-lived fake child in tests/fm-pi-watch-extension.test.sh with a test-only hard deadline, fixture-root and control-parent disappearance escape, and TERM/INT handling that cannot ignore signals indefinitely.”tests/fm-pi-watch-extension.test.sh:1312- The repaired half of the failure regression checks only root absence and an empty registry at lines 1310-1313. It never records or checks the fixed fixture's PID/process-group liveness, so a child that failed to register or whose record disappeared while its group remained live could pass. This does not prove the required criterion that the fixed fixture does not leak after root deletion.tests/fm-pi-watch-extension.test.sh:1414- Fixed 80 ms waits remain at lines 1414 and 3482 after writing the stop control. This contradicts the required criterion: “Replace the fixed 80 ms sleep with explicit close acknowledgement.” Replace these late-close teardown waits with an acknowledgement or explicitly authorize them as exceptions.bin/fm-test-run.sh --changed --exclude-family real-herdr-gated🔧 Fix: Focused failures pass; originals are concurrency flakes
1 error still open:
bin/fm-test-run.sh --changed --exclude-family real-herdr-gated✅ **Document** - passed
✅ No issues found.
✅ **Lint** - passed
✅ No issues found.
✅ **Push** - passed
✅ No issues found.