Skip to content

fix(watch): keep supervision alive across delivered wakes and parked lanes - #1419

Closed
mattadams-dev wants to merge 14 commits into
kunchenguid:mainfrom
mattadams-dev:fm/fm-supervision-successor-arming
Closed

mattadams-dev wants to merge 14 commits into
kunchenguid:mainfrom
mattadams-dev:fm/fm-supervision-successor-arming

Conversation

@mattadams-dev

Copy link
Copy Markdown

Intent

Fix two defects in firstmate's supervision watcher, diagnosed by the captain from a preserved 499-record capture of a live home's watcher cycle-exit ledger (committed at tests/fixtures/watch-cycle-exits/cycle-exits.log because it accumulates over days and cannot be regenerated).

FRAMING THE CAPTAIN SET, WHICH MATTERS FOR REVIEW: the watchers were never failing. All 499 records show exit_code=0 and signal=none - the watcher is one-shot BY DESIGN and completes in order to deliver a wake. Earlier attempts treated this as a supervision failure and repaired it by hand; that framing was wrong. This is a redesign, not a fourth patch.

DEFECT 1: successor=none on 499 of 499 records. Nothing armed the next watcher cycle after a wake was delivered, so supervision ended on every delivered wake and only resumed if some adapter above the arm layer happened to re-arm. The captain specified the fix shape verbatim: 'arming the successor is the first act of consuming a wake, before handling what it woke you for.' That ordering IS the design - handling first and arming afterwards leaves a window as wide as the handling turn, which is exactly what was swallowing supervision. So in bin/fm-watch-arm.sh the successor is armed and verified before print_watch_output, deliberately. The successor is intentionally NOT the arm's child (double-forked) because it must outlive the arm that delivers the wake; that is a deliberate departure from the file's existing 'the child stays our child' comment, and it is documented in the header. It is still verified through the same live-process-plus-fresh-beacon honesty gate as an owned child, so it is not the fire-and-forget shell-& pattern the header warns about. Gated on supervision actually being needed and away mode being off, so an idle home is never left with a detached watcher.

Root cause of the secondary path, recorded but deliberately NOT fixed here: after a session fork, state/.lock still names the pre-fork harness pid, which stays alive, so the forked session's Claude Stop auto-arm hook can never claim the home and is permanently inert. Evidence from the live home: .lock=17907, a live claude process. The arm-layer fix makes continuity independent of that path; the lock behaviour itself belongs to the existing fm-upstream-issue-dossier backlog item, whose first line is already 'watcher re-arm after fork'.

A third exit shape the captain observed and told me to record rather than chase is resolved as a side effect: an ATTACHED arm could only report a genuine wake as 'watcher: FAILED - cycle ended without an actionable reason', because it could not see its watcher's output. The successor now writes its one reason to state/.watch-successor-output. before releasing the singleton, and the attached arm claims and delivers it.

DEFECT 2: 112 of 499 cycles (22%) exited within 5 seconds, all on actionable-stale, all driven by lanes deliberately parked awaiting a captain decision with idle ages in the thousands of seconds. Each parked lane carried its own independently-phased standing trigger, so a fresh watcher died within seconds of every start on whichever lane was next overdue. The captain specified the fix as his own pins-for-conditions rule: 'parked-awaiting-captain is not actionable staleness - give it a check-in cadence, not a standing trigger', and explicitly said NOT to solve it by suppressing the lane, because a declared wait can stop holding: the distinction is CADENCE, not silence. So due lanes now batch into one check-in wake, rate-limited home-wide by a state/.last-parked-checkin mtime - deliberately copying the shape the slow per-task checks already use (.last-check mtime) so the cadence survives watcher restarts, which the captain pointed at as the model. A lane whose wait has actually cleared still surfaces at once, independent of the cadence, and that is covered by its own test. Away mode keeps the unbatched one-shot form because the daemon classifies one window per printed reason.

This is guard-class code, so the captain applied the fleet mutation bar: each protection is proven by the mutation that breaks exactly its own test. That matrix is recorded in docs/verification/supervision.md - six mutations, each run twice (once to see which case it kills, once with that case removed to prove nothing else breaks). One row is honestly reported as shared by two cases guarding the same pause release.

Also in the branch, because it turned out to be load-bearing rather than incidental: tests/lib.sh's fm_test_tmproot is called through a command substitution, so its array append landed in the subshell and EVERY suite leaked its entire temp root. That was harmless until an arm could leave a detached successor - then a leaked temp root kept a live watcher supervising a home no test owned any more, and the accumulated processes destabilised a timing-sensitive case (test_watch_restart_attaches_to_healthy_peer) in the same suite. Registration now goes to a named per-owner registry file that survives the subshell, and only the sourcing shell may run the cleanup. After the fix both watcher suites run with zero stray watcher processes and zero leftover temp roots.

Known pre-existing failure, NOT caused by this branch and deliberately not fixed here: tests/fm-session-start.test.sh's Herdr husk-recovery case fails identically on the unmodified base commit f7d0d0a ('the later fleet read did not confirm the relaunched Herdr endpoint'). Verified by checking out the base commit and re-running. It is unrelated to supervision successor arming.

What Changed

  • bin/fm-watch-arm.sh now arms and verifies a successor watcher as the first act of consuming a wake, before the wake is printed, so supervision no longer ends on every delivered cycle. The successor is deliberately detached (double-forked) so it outlives the arm that delivers the wake, is still confirmed through the same live-process-plus-fresh-beacon gate as an owned child, and is armed only when supervision is actually needed and away mode is off. Each successor writes its one reason to a pid-keyed state/.watch-successor-output.<pid> file, which an attached arm claims and delivers - an attached close carrying a genuine wake is no longer reported as cycle ended without an actionable reason. The outcome is recorded per cycle in the state/.watch-cycle-exits.log ledger.
  • bin/fm-watch.sh gives parked lanes (declared external waits and captain holds) a check-in cadence instead of a standing per-cycle trigger: every lane due in one poll is batched into a single wake, rate-limited home-wide by the state/.last-parked-checkin mtime (FM_PARKED_CHECKIN_SECS, defaulting to FM_PAUSE_RESURFACE_SECS), and only the lanes carried in an emitted check-in get their per-lane cadence stamped. A lane whose pause has actually cleared still surfaces immediately, and away mode keeps the unbatched one-shot form the daemon's one-window-per-reason triage expects.
  • Supporting work: tests/lib.sh registers temp roots through a named per-owner registry file that survives the command substitution fm_test_tmproot is called through, so suites stop leaking temp roots (which, once an arm could leave a detached successor, kept live watchers supervising abandoned homes); the 499-record live cycle-exit capture is preserved at tests/fixtures/watch-cycle-exits/cycle-exits.log because it accumulates over days and cannot be regenerated; new cases in tests/fm-watcher-lock.test.sh and tests/fm-watch-triage.test.sh cover successor arming, attached-cycle delivery, the away-mode gate and the parked cadence; a pre-existing race in test_watch_restart_attaches_to_healthy_peer (the TERM-resistant peer was signalled before its handler was installed) is fixed; and docs/verification/supervision.md records the capture analysis plus an eight-mutation guard-class matrix, each mutation run at least twice, with the shared clear_pause_state row and a surviving mutant at the redundant pause-release site reported honestly.

Note on diff size: this branch is cut from a point behind the gate's main, so the delta also carries eight already-merged upstream commits (#1303, #1327, #1328, #1339, #1349, #1350, #1356, #1358). The supervision change itself is the delta from f7d0d0a.

Risk Assessment

✅ Low: The round-1 blocking defect is fixed with a test that can no longer pass over it, the two arm-layer race gaps are closed with a pid handshake I verified from source (exec preserves the pid, and nothing re-execs before the lock claim), the registry is now unpredictable and swept by every lib-sourcing suite, and only a trivial prune-reachability tidiness item remains.

Testing

I ran the two suites that own this contract repeatedly - fm-watcher-lock four times and fm-watch-triage three times, all clean - then proved the intent at the operator surface by driving the real arm and watcher against an identical staged home under base-commit bin and branch bin, capturing arm stdout, the cycle-exit ledger record, and whether any watcher still supervises the home afterwards. The base reproduces all three captured shapes exactly and the branch fixes each. I also found the away-mode half of the successor gate had no test, so I added a focused case and mutation-proved it kills exactly that protection, taking the suite to 35 cases. The change is shell and CLI only with no rendered surface, so evidence is CLI transcripts rather than screenshots. Every run ended with zero leftover test temp roots and zero stray watcher processes, confirming the tests/lib.sh registry fix. The only working-tree change left is the added test case; bin/fm-watch-arm.sh was restored byte-identical after the mutation check.

Evidence: DEFECT 1 - successor arming, base vs branch
==============================================================
  before   (/tmp/no-mistakes-evidence/01KYVWHDGTDH63YJJC091K85AP/bin-before)
==============================================================
$ fm-watch-arm.sh          # a supervision cycle delivers one wake
    watcher: started pid=3690265 (beacon fresh)
    signal: /tmp/no-mistakes-evidence/01KYVWHDGTDH63YJJC091K85AP/demo-before/state/task.status
    [arm exit 0]

-- cycle-exit ledger (state/.watch-cycle-exits.log) --
    arm_pid=3690250	watcher_pid=3690265	origin=started	started_at=1785499052	ended_at=1785499053	exit_code=0	signal=none	reason=actionable-signal	beacon_age=1	lock_before=pid:3690265|identity:linux-starttime=2814300 cmdline-hex=62617368002f746d702f6e6f2d6d697374616b65732d65766964656e63652f30314b5956574844475444483633594a4a433039314b383541502f62696e2d6265666f72652f666d2d77617463682e736800	lock_after=pid:none|identity:none	successor=none

-- was the next cycle already armed when the wake was printed? --
    NO  - nothing held the watcher singleton at wake time

-- is the home still supervised now that the arm has exited? --
    UNSUPERVISED - no watcher is running; supervision ended with the wake

==============================================================
  after   (/tmp/no-mistakes-evidence/01KYVWHDGTDH63YJJC091K85AP/bin-after)
==============================================================
$ fm-watch-arm.sh          # a supervision cycle delivers one wake
    watcher: started pid=3690957 (beacon fresh)
    signal: /tmp/no-mistakes-evidence/01KYVWHDGTDH63YJJC091K85AP/demo-after/state/task.status
    [arm exit 0]

-- cycle-exit ledger (state/.watch-cycle-exits.log) --
    arm_pid=3690942	watcher_pid=3690957	origin=started	started_at=1785499057	ended_at=1785499058	exit_code=0	signal=none	reason=actionable-signal	beacon_age=0	lock_before=pid:3690957|identity:linux-starttime=2814780 cmdline-hex=62617368002f746d702f6e6f2d6d697374616b65732d65766964656e63652f30314b5956574844475444483633594a4a433039314b383541502f62696e2d61667465722f666d2d77617463682e736800	lock_after=pid:3692810|identity:linux-starttime=2814894 cmdline-hex=62617368002f746d702f6e6f2d6d697374616b65732d65766964656e63652f30314b5956574844475444483633594a4a433039314b383541502f62696e2d61667465722f666d2d77617463682e736800	successor=started:3692810

-- was the next cycle already armed when the wake was printed? --
    YES - watcher pid 3692810 held the singleton at wake time

-- is the home still supervised now that the arm has exited? --
    SUPERVISED - watcher pid 3692810 is still running:
      3692810     551 Ss   bash /tmp/no-mistakes-evidence/01KYVWHDGTDH63YJJC091K85AP/bin-after/fm-watch.sh
Evidence: Third exit shape - attached arm delivers its successor cycle wake
==============================================================
  before
==============================================================
    (a watcher this arm does not own is supervising: pid 3801702)

$ fm-watch-arm.sh            # arm #2: attaches to watcher pid 3801702
    watcher: already running pid 3801702
    watcher: FAILED - cycle ended without an actionable reason
    [arm exit 1]

==============================================================
  after
==============================================================
$ fm-watch-arm.sh            # arm #1: delivers a wake, leaves a successor
    watcher: started pid=3807042 (beacon fresh)
    signal: /tmp/no-mistakes-evidence/01KYVWHDGTDH63YJJC091K85AP/attach2-after/state/task.status

$ fm-watch-arm.sh            # arm #2: attaches to watcher pid 3807369
    watcher: attached pid=3807369 (beacon 0s)
    signal: /tmp/no-mistakes-evidence/01KYVWHDGTDH63YJJC091K85AP/attach2-after/state/second.status
    [arm exit 0]
Evidence: DEFECT 2 - parked check-in cadence, base vs branch
==============================================================
  before   (/tmp/no-mistakes-evidence/01KYVWHDGTDH63YJJC091K85AP/bin-before)
  3 lanes parked awaiting a captain decision, idle 500s each
==============================================================
cycle 1: EXITED after 1s to deliver 1 wake reason(s):
    stale: test:fm-park1 (paused 501s, awaiting external - declared pause, rechecked on a long cadence not a wedge; confirm the wait still holds)
cycle 2: EXITED after 0s to deliver 1 wake reason(s):
    stale: test:fm-park2 (paused 501s, awaiting external - declared pause, rechecked on a long cadence not a wedge; confirm the wait still holds)
cycle 3: EXITED after 0s to deliver 1 wake reason(s):
    stale: test:fm-park3 (paused 501s, awaiting external - declared pause, rechecked on a long cadence not a wedge; confirm the wait still holds)
cycle 4: STILL SUPERVISING after 12s (stopped by the demo), printed nothing

-- lanes carried per printed wake --
    cycle 1 carried 1 parked lane(s)
    cycle 2 carried 1 parked lane(s)
    cycle 3 carried 1 parked lane(s)
    cycle 4 carried 0 parked lane(s)
-- shared cadence marker --
    state/.last-parked-checkin absent (no shared cadence; every lane is a standing trigger)
==============================================================
  after   (/tmp/no-mistakes-evidence/01KYVWHDGTDH63YJJC091K85AP/bin-after)
  3 lanes parked awaiting a captain decision, idle 500s each
==============================================================
cycle 1: EXITED after 0s to deliver 1 wake reason(s):
    stale: test:fm-park1 (paused 500s, awaiting external - declared pause, rechecked on a long cadence not a wedge; confirm the wait still holds); stale: test:fm-park2 (paused 500s, awaiting external - declared pause, rechecked on a long cadence not a wedge; confirm the wait still holds); stale: test:fm-park3 (paused 500s, awaiting external - declared pause, rechecked on a long cadence not a wedge; confirm the wait still holds)
cycle 2: STILL SUPERVISING after 12s (stopped by the demo), printed nothing
cycle 3: STILL SUPERVISING after 12s (stopped by the demo), printed nothing
cycle 4: STILL SUPERVISING after 12s (stopped by the demo), printed nothing

-- lanes carried per printed wake --
    cycle 1 carried 3 parked lane(s)
    cycle 2 carried 0 parked lane(s)
    cycle 3 carried 0 parked lane(s)
    cycle 4 carried 0 parked lane(s)
-- shared cadence marker --
    state/.last-parked-checkin present (home-wide check-in rate limit, survives restarts)
Evidence: Away-mode successor gate
=== away-mode successor gate (manual verification, this branch) ===
$ fm-watch-arm.sh   # home has in-flight work AND state/.afk present
    watcher: started pid=3873625 (beacon fresh)
    signal: /tmp/no-mistakes-evidence/01KYVWHDGTDH63YJJC091K85AP/away-gate/state/task.status
    ledger: reason=actionable-signal	beacon_age=1	lock_before=pid:3873625|identity:linux-starttime=2862717 cmdline-hex=62617368002f686f6d652f6a616d6164612f2e6e6f2d6d697374616b65732f776f726b74726565732f6335653833346561636466322f30314b5956574844475444483633594a4a433039314b383541502f62696e2f666d2d77617463682e736800	lock_after=pid:none|identity:none	successor=none
    no detached watcher left behind: the supervise-daemon keeps sole ownership of the cycle
Evidence: Mutation proof for the added away-mode case
MUTATION: remove only the away-mode gate from successor_wanted() in bin/fm-watch-arm.sh
  -  [ -e "$STATE/.afk" ] && return 1

result (tests/fm-watcher-lock.test.sh):
  ok - arm propagates an immediate watcher wake before confirmation
  ok - arm attaches to a peer watcher after child stands down and surfaces a missing successor
  ok - arm reports FAILED and exits non-zero when no fresh watcher can be confirmed
  ok - cycle-exit ledger links a verified successor and remains size-capped
  ok - a delivered wake arms its successor before the wake is handled, and supervision survives the arm
  ok - an attached arm delivers the wake its successor cycle closed on, never a false empty-close failure
  ok - no successor is armed for a home with nothing left to supervise
  not ok - an away home recorded a successor the daemon-owned cycle should not have armed

The mutation kills exactly the new case and nothing before it.
Unmutated suite: 35/35 ok (fm-watcher-lock.with-away-case.log).
Evidence: Preserved 499-record capture stats
$ awk over tests/fixtures/watch-cycle-exits/cycle-exits.log   (preserved live capture, 499 records)
records                        499
exit_code=0                    499 / 499     <- watchers never failed; one-shot by design
signal=none                    499 / 499
successor=none                 499 / 499     <- DEFECT 1: supervision ended on every delivered wake
reason=actionable-stale        293
reason=actionable-signal       198
reason=actionable-check        8
cycles ending <=5s on stale   112  (22% of 499)   <- DEFECT 2: parked-lane churn
cycles ending  <5s on stale   106  (the strict-<5 form the test asserts, >=100)
Evidence: Evidence index
# Round 2 test evidence - fm supervision successor arming

All transcripts are the real `bin/fm-watch-arm.sh` / `bin/fm-watch.sh` run against a
staged home, base commit `f7d0d0a` vs this branch, same commands both sides.

| file | shows |
| --- | --- |
| `defect1-successor-arming.txt` | DEFECT 1 before/after: base leaves the home UNSUPERVISED with `successor=none`; branch has a live successor holding the watcher singleton at the instant the wake is printed, reparented off the arm (`ppid=1`-class, own session), and still running after the arm exits. |
| `defect1-attached-delivery.txt` | The third exit shape: base reports a genuine wake as `watcher: FAILED - cycle ended without an actionable reason` (exit 1); branch's attached arm claims the successor's output and delivers `signal: ...` (exit 0). |
| `defect2-parked-cadence.txt` | DEFECT 2 before/after: base burns 3 successive sub-second cycles, one per parked lane; branch emits one batched check-in carrying all 3 lanes and the next 3 cycles keep supervising. |
| `away-mode-successor-gate.txt` | Away mode (`state/.afk`) still delivers its wake and arms no successor, so the supervise-daemon keeps sole ownership. |
| `preserved-capture-stats.txt` | The preserved 499-record capture re-derived from the committed fixture. |
| `mutation-away-gate.txt` | Mutation proof for the test added this round. |
| `fm-watcher-lock.run{1..4}.log`, `fm-watch-triage.run{1..3}.log` | Repeat suite runs (34 and 42 cases), all clean. |
| `fm-watcher-lock.final.log` | 35 cases with the added away-mode case. |

Scripts that produced the transcripts: `demo-successor-arming.sh`,
`demo-parked-cadence.sh`, `demo-attached-delivery.sh`.

After every run: zero leftover `fm-*` test temp roots in `/tmp` and zero stray
watcher processes.
Evidence: Suite log - fm-watcher-lock 35/35
ok - simultaneous watcher starts leave exactly one live process
ok - fm_pid_identity real ps fallback is locale-invariant
ok - fm_pid_identity is locale-invariant across LC_ALL/LC_TIME
ok - /proc process identity ignores simulated btime changes
ok - /proc process identity detects pid reuse
ok - MSYS /proc process identity regression skipped on non-Windows host
ok - killed watcher stale lock is reclaimed
ok - live watcher lock with stale heartbeat is actionable
ok - guard banner leads when down with pending wakes (repair-after-drain) and stays silent when fresh
ok - concurrent fm_lock_try_acquire yields exactly one winner
ok - dead-pid stale lock is reclaimed by a single acquirer
ok - concurrent stale-lock steal yields exactly one winner
ok - live steal mutex is not reclaimed
ok - live-held lock is not stolen
ok - empty mid-acquire lock keeps a minimum grace
ok - late original claimant cannot claim a recreated lock
ok - paused mid-acquire claimant backs off to active stealer
ok - watch restart refuses to signal a reused pid
ok - watch restart attaches to a verified healthy peer and later surfaces a successor gap
ok - watcher self-evicts when the lock pid no longer names it
ok - arm turns clean self-eviction without a successor into a typed failure
ok - arm attaches to a live fresh watcher and fails loudly when that cycle has no successor
ok - attached arm signals record a classified lifecycle entry
ok - arm starts+confirms a fresh watcher on a clean lock and self-heals a dead-pid lock (never healthy off a dead pid)
ok - arm cleans child watcher and temp output on HUP
ok - arm propagates an immediate watcher wake before confirmation
ok - arm attaches to a peer watcher after child stands down and surfaces a missing successor
watcher: lock held by live pid 4162464 but heartbeat is stale for 838786621s (>300s); inspect or stop that watcher before re-arming.
ok - arm reports FAILED and exits non-zero when no fresh watcher can be confirmed
ok - cycle-exit ledger links a verified successor and remains size-capped
ok - a delivered wake arms its successor before the wake is handled, and supervision survives the arm
ok - an attached arm delivers the wake its successor cycle closed on, never a false empty-close failure
ok - no successor is armed for a home with nothing left to supervise
ok - no successor is armed in away mode, where the supervise-daemon owns the cycle
ok - the preserved 499-record capture still shows the defect, and a fresh cycle no longer reproduces it
ok - SIGSTOP distinguishes live PID from stale beacon and termination records the exit class
Evidence: Suite log - fm-watch-triage 42/42
ok - signal_reason_is_actionable: benign absorbed, captain verbs and coalesced batches surfaced
ok - stale_is_terminal: terminal status surfaces, non-terminal and no-status are benign
ok - scan_captain_relevant_statuses lists only captain-relevant statuses
ok - classifier primitives: keyed decisions and activity phases, captain relevance, window-to-task, and overrides
ok - crew_is_provably_working: only working+run-step/pane is provable; idle/finished/parked/failed/unknown surface
ok - status_is_paused: only the leading paused verb matches, and paused is not captain-relevant
ok - crew_absorb_class: working/paused/none from one read; crew_is_paused and crew_is_provably_working agree
ok - signal_crew_provably_working: benign only when every referenced crew is provably working
ok - a no-verb signal whose crew is provably working is absorbed (no exit, no queue, suppressor advanced, beacon present)
ok - a bare turn-end whose crew is provably working (busy pane) is absorbed
ok - a bare turn-end whose crew is not provably working is surfaced (the swallowed-finish fix)
ok - a no-verb working: note whose crew is idle with no running pipeline is surfaced
ok - captain-relevant signal is surfaced (queue + exit) and marked surfaced
ok - a stale pane sitting on a terminal status is surfaced (queue + exit)
ok - a stale terminal-looking status is overridden and absorbed while a run is actively working, then wedge-escalated
ok - provably-working non-terminal stale is absorbed on first sight, then wedge-escalated past the threshold
ok - consecutive wedge escalations on the same pane accumulate and demand deep inspection at the threshold
ok - a pane becoming active again resets the consecutive wedge-escalation counter
ok - a busy worker below the turn-age bound remains working with no escalation
ok - a busy worker with a stable pane hash still escalates once its completed-turn age reaches the bound
ok - a busy worker whose pane hash changes every poll still escalates once its completed-turn age reaches the bound
ok - touching a busy worker's completed-turn marker resets the age and prevents an old-age escalation
ok - repeated busy turn-age escalations reuse the existing escalation counter and demand deep inspection at the threshold
ok - the production default busy-turn-age bound is 3600s (5min under does not wedge, 66min over does)
ok - a not-provably-working non-terminal stale is surfaced immediately (never left to wait out the timer)
ok - a declared pause is absorbed on first sight, then re-surfaced as a recheck past the threshold, never wedge-escalated
ok - parked lanes batch into one check-in per cadence window, and the cadence still comes due
ok - a parked lane whose wait has cleared is surfaced at once, even inside a closed check-in cadence
ok - exited declared-pause and captain-held panes use bounded pause cadence while a live decision gate still surfaces once
ok - a declared paused secondmate re-surfaces on the bounded normal-mode cadence
ok - a non-paused secondmate retains normal stale suppression
ok - a resumed secondmate clears pause and stale tracking before stale exemption
ok - unchanged stale hashes reclassify when a crew enters or leaves pause
ok - a declared pause is periodically rechecked against authoritative active-run state
ok - a paused status overridden by authoritative working preserves its wedge timer and escalates
ok - matching non-terminal stale suppressors repair missing or corrupt stale-since timers
ok - triage log capping handles wc byte counts with leading spaces
ok - a heartbeat with no captain-relevant change is absorbed and backs off the cadence
ok - heartbeat backstop fail-safe surfaces a captain-relevant status the per-wake path missed
ok - the liveness beacon stays fresh while the watcher absorbs benign wakes (fm-guard never false-alarms)
ok - with .afk present the watcher reverts to one-shot so the daemon owns triage (no double-triage)
ok - AFK changed paused panes hand off plain stale identities for daemon-owned pause triage
- Outcome: 🔧 1 issue found → auto-fixed ✅ across 2 runs (53m7s)

Pipeline

Updates from git push no-mistakes

✅ **intent** - passed

✅ No issues found.

⏭️ **Rebase** - skipped
  • ⚠️ tests/fm-secondmate-harness.test.sh - merge conflict rebasing onto origin/main
⚠️ **Review** - 1 info
  • 🚨 bin/fm-watch.sh:357 - The parked-lane accumulator loses its record separator, so the batched check-in only ever carries one lane. parked_due=$(printf &#39;%s%s\t%s\n&#39; &#34;$parked_due&#34; &#34;$win&#34; &#34;$reason&#34;) runs through a command substitution, which strips the trailing newline; the next append then concatenates directly onto the previous line. With 3 due lanes the value is w1&lt;TAB&gt;reason1w2&lt;TAB&gt;reason2w3&lt;TAB&gt;reason3 (verified by running the exact loop in bash). flush_parked_checkin (bin/fm-watch.sh:378) therefore performs exactly one read iteration: only the first lane is enqueued via fm_wake_append stale &#34;$win&#34; &#34;$reason&#34;, only its .paused-resurfaced-&lt;key&gt; marker is stamped, the combined=&#34;$combined; $reason&#34; join never executes, and wake prints a tab-glued mash of every lane's reason attributed to the first window. This is the exact multi-lane scenario the fix targets (the capture's 112 sub-5s exits were independently-phased parked lanes), and it also silently violates the function's own documented contract that lanes not included in an emitted check-in stay unstamped and due - lanes 2..N are never stamped, so they remain permanently due. tests/fm-watch-triage.test.sh:709 does not catch it because grep -F &#34;test:fm-park$i&#34; matches the glued line and grep -c &#39;^stale:&#39; is 1 either way. Fix: preserve the newline, e.g. parked_due=&#34;${parked_due}${win}&#34;$&#39;\t&#39;&#34;${reason}&#34;$&#39;\n&#39;, and tighten the test to assert one wake-queue record per lane and a ; -joined reason.
  • ⚠️ bin/fm-watch-arm.sh:385 - When two arms are attached to the same watcher - a topology this file explicitly supports ("A live cycle already present means re-arm attaches", header line 42) - both reach the close path in attach_and_wait simultaneously. Both pass watch_output_has_wake &#34;$out&#34;, then mv -f &#34;$out&#34; &#34;$claimed&#34; succeeds for one and fails for the other. The loser falls back to claimed=$out, which no longer exists, so print_watch_output emits nothing and the function still does return 0. The arm exits 0 with completely empty stdout - precisely the "clean empty completion that an adapter could mistake for a no-op" the function comment (line 361) says it must never produce, and which the non-wake path below deliberately turns into a typed nonzero failure. Fix: on mv failure, do not treat the wake as claimed; fall through to wait_for_healthy_successor so the arm either attaches to the successor the winner just armed or fails loudly.
  • ℹ️ bin/fm-watch-arm.sh:325 - arm_successor binds its pending output file to whichever pid wins the singleton (HEALTHY_PID), not to the process it actually forked. When another watcher wins - a concurrent arm_successor from a second attached arm, or an adapter-launched successor (docs/architecture.md:63 notes Pi and OpenCode launch their own singleton successor from child-close handlers) - the arm still renames its own tmp to &lt;winner-pid&gt;. The winner's stdout goes elsewhere, so the published file ends up holding the loser watcher's watcher: already running pid N line instead of the winner's reason, while the winner's real reason (whose fd points at the unlinked tmp on the [ -e &#34;$target&#34; ] branch) never lands on disk. A later arm attaching to that pid then fails watch_output_has_wake and can report watcher: FAILED - cycle ended without an actionable reason for a genuine wake - the third exit shape this change set out to resolve. The reason still reaches state/.wake-queue, so nothing is lost durably; only the arm-layer delivery degrades. Minimal fix: capture the pid of the process this arm actually started and only publish the output file when the confirmed healthy pid matches it, recording attached:&lt;pid&gt; otherwise. Cleaner shared boundary: have fm-watch.sh write its own reason to state/.watch-successor-output.$WATCHER_PID, so the binding is authoritative regardless of who forked it.
  • ℹ️ bin/fm-watch-arm.sh:305 - Recording the residual bound of the arm-layer fix, which the intent explicitly authorizes as containment for the deferred state/.lock fork defect. A successor is armed only from an arm process, so with a permanently inert adapter (the observed forked-Claude-session case, where the Stop auto-arm hook can never claim the home) the chain is exactly one cycle deep: the detached successor covers the handling turn, wakes, writes its reason to .watch-successor-output.&lt;pid&gt; and state/.wake-queue, exits, and no further successor is armed. The blind window is bounded by the successor's own cycle rather than eliminated, so the intent's "makes continuity independent of that path" is stronger than what the arm layer alone delivers. No action requested - the durable fix is correctly deferred to the fm-upstream-issue-dossier item, and the unclaimed output file is reaped by prune_successor_outputs at FM_WATCH_SUCCESSOR_OUT_TTL.
  • ℹ️ tests/lib.sh:73 - The cleanup registry path is fully predictable (${TMPDIR:-/tmp}/fm-test-cleanup.&lt;pid&gt;) and its contents drive rm -rf on any line starting with / (tests/lib.sh:84). On a shared host with a sticky /tmp, another user can pre-create that path as a symlink; the sticky bit prevents the rm -f at line 74 from removing it, the &gt;&gt; in fm_test_tmproot then follows the symlink and writes as the test user, and fm_test_cleanup recursively deletes every absolute path it reads back. The comment's two stated requirements (subshell-visible path, nothing created for a suite that takes no temp root) are both satisfiable safely: FM_TEST_CLEANUP_REGISTRY=$(mktemp &#34;${TMPDIR:-/tmp}/fm-test-cleanup.XXXXXX&#34;) plus export gives an unpredictable, atomically-created path that subshells still see, and fm_test_cleanup already removes it so it leaves no litter.
  • ℹ️ tests/fm-kimi-harness.test.sh:22 - trap fm_test_cleanup EXIT is now installed at source time, so any suite that installs its own EXIT trap afterwards replaces it. tests/fm-kimi-harness.test.sh:22 and tests/fm-afk-pi-herdr-return-e2e.test.sh:62 both do this without calling fm_test_cleanup. Their temp roots are still removed (each rm -rf &#34;$TMP_ROOT&#34; directly), so the branch's zero-leftover-temp-roots claim holds, but the registry file itself is never unlinked, leaving one stray ${TMPDIR:-/tmp}/fm-test-cleanup.&lt;pid&gt; per run. Adding fm_test_cleanup to both cleanup functions - which the library comment at tests/lib.sh:57 already prescribes - closes it.

🔧 Fix: fix parked check-in batching and successor output binding
1 info still open:

  • ℹ️ bin/fm-watch-arm.sh:309 - prune_successor_outputs is only ever called from arm_successor (the sole call site, line 309), and it sits after the successor_wanted || { printf &#39;none&#39;; return 0; } early return on line 308. Once a home stops needing supervision - the exact state where leftover files accumulate and nothing new claims them - the sweeper becomes unreachable, so any unclaimed .watch-successor-output.&lt;pid&gt; (plus orphan .pending.*/.forkpid.* from an arm killed mid-arm_successor) stays in state/ indefinitely rather than aging out at FM_WATCH_SUCCESSOR_OUT_TTL. Not a correctness or supervision risk - the files are small, AGENTS.md:114 marks them watcher internals, and every live path still cleans up its own - but it defeats the sweeper's stated purpose in a codebase that otherwise size-caps every state artifact. Moving the prune_successor_outputs call above the successor_wanted gate makes it run on every arm cycle, including the ones that decline to arm.
🔧 **Test** - 1 issue found → auto-fixed ✅
  • ⚠️ tests/fm-watcher-lock.test.sh:447 - tests/fm-watcher-lock.test.sh:447 test_watch_restart_attaches_to_healthy_peer was flaky for a cause unrelated to the leaked-temp-root explanation recorded in docs/verification/supervision.md. The case stages a TERM-resistant peer with node -e &#39;process.on(&#34;SIGTERM&#34;, ...)&#39; and immediately launches fm-watch-arm.sh --restart, whose first act is to TERM the recorded lock pid. Nothing waited for node to finish starting, so when node lost that race the TERM landed before the handler existed, the peer died, and the arm correctly started a fresh watcher - reported as restart did not attach to the verified healthy peer. Reproduced 3 times in 18 runs on an idle 32-core host (load 0.15), with a diagnostic confirming peer DEAD (TERM landed before its handler). The case is unchanged since a323c2b, so it predates this branch. Fixed by having the peer publish a readiness marker from inside the handler-installed process and having the case wait for it before anything can signal the peer; 15/15 isolated runs and two clean full-suite runs since.
  • bash tests/fm-watcher-lock.test.sh - full suite, two consecutive clean runs (34 cases each), including test_delivered_wake_arms_its_successor_before_it_is_handled, test_attached_arm_delivers_its_successor_cycle_wake, test_successor_is_not_armed_without_supervision_need, test_preserved_capture_still_shows_the_defect_a_fresh_cycle_no_longer_has
  • bash tests/fm-watch-triage.test.sh - full suite clean, including test_parked_lanes_batch_into_one_checkin_on_a_shared_cadence, test_a_cleared_park_is_noticed_promptly_inside_a_closed_cadence, test_afk_present_reverts_watcher_to_one_shot
  • Manual E2E before/after: git archive f7d0d0a bin | tar -x -C &lt;evidence&gt;/base-tree, then ran bin/fm-watch-arm.sh and bin/fm-watch.sh from BOTH the base tree and the branch against identically staged homes (/tmp/no-mistakes-evidence/01KYVWHDGTDH63YJJC091K85AP/supervision-e2e-demo.sh), capturing arm stdout, the state/.watch-cycle-exits.log record, and ps -o pid=,ppid=,args= proof of the surviving detached successor
  • Manual E2E parked-lane cadence: three overdue parked lanes run through three watcher cycles per variant with per-lane throttles cleared between cycles, then .last-parked-checkin aged past its window, then one lane's pause verb cleared inside a closed cadence
  • Leak check after every run: ls -d /tmp/fm-watcher-lock-tests.* /tmp/fm-watch-triage* /tmp/fm-test-cleanup.* /tmp/fm-supervision-e2e.* and ps -eo pid,args | grep c5e834eacdf2.*fm-watch - all empty
  • Flake reproduction and fix verification for test_watch_restart_attaches_to_healthy_peer: 18 runs before the fix (3 failures, with a peer-liveness diagnostic), 15 isolated runs after the fix (0 failures)

🔧 Fix: wait for TERM-resistant peer handler before restarting arm
✅ Re-checked - no issues remain.

  • tests/fm-watcher-lock.test.sh - 4 consecutive full runs, 34/34 cases each, all clean
  • tests/fm-watch-triage.test.sh - 3 consecutive full runs, 42/42 cases each, all clean
  • Manual E2E defect-1 contrast via demo-successor-arming.sh against base-commit bin and branch bin
  • Manual E2E attached-delivery contrast via demo-attached-delivery.sh
  • Manual E2E defect-2 contrast via demo-parked-cadence.sh over four successive watcher cycles
  • Manual verification of the away-mode successor gate with state/.afk present
  • Added test_successor_is_not_armed_in_away_mode to tests/fm-watcher-lock.test.sh; suite now 35/35 ok
  • Mutation proof: removed only the away-mode line from successor_wanted() in bin/fm-watch-arm.sh, reran the suite, then restored the file
  • Re-derived preserved capture stats from tests/fixtures/watch-cycle-exits/cycle-exits.log
  • Leak hygiene after every run: zero fm-* test temp roots in /tmp and no test-owned watcher processes
🔧 **Document** - 1 issue found → auto-fixed (3) ✅
  • ⚠️ docs/verification/supervision.md:222 - The guard-class mutation matrix has no row for the away-mode half of the successor gate. The working tree adds test_successor_is_not_armed_in_away_mode to tests/fm-watcher-lock.test.sh (uncommitted), which guards a distinct protection - .afk suppressing the successor so a detached watcher never competes with the supervise-daemon - and the intent states the fleet mutation bar requires each protection be proven by the mutation that breaks exactly its own test. That new case also shifts the '33 cases clean' figure recorded for the three lock-suite rows, which was measured before it existed. Closing this needs the mutation matrix re-run (mutating bin/fm-watch-arm.sh and running both suites), which is outside a documentation phase, so I did not invent a row or adjust the recorded counts.

🔧 Fix: no doc changes; mutation matrix re-measurement unfinished
1 error still open:

  • 🚨 docs/verification/supervision.md:225 - The guard-class mutation matrix was NOT updated this round: the re-measurement run was still on its first mutation when I was required to finalize, so I have suite baselines but no per-mutation kill results. I made no edit rather than publish a partially re-measured table, which the captain explicitly ruled worse than an obviously old one.

WHAT IS ESTABLISHED BY MEASUREMENT: both suites were re-run clean against the current tree (HEAD 58dae2b). tests/fm-watcher-lock.test.sh emits 35 'ok -' cases, exit 0. tests/fm-watch-triage.test.sh emits 42 'ok -' cases, exit 0. Since the table's 'N cases clean' figures are baseline-minus-the-removed-case, this already proves the three lock-suite rows reading '33 cases clean' (lines 227, 228, 229, 232 - four rows total) are stale: 33 = a 34-case baseline minus one, and the baseline is now 35 because test_successor_is_not_armed_in_away_mode was added. Those rows must read 34 once confirmed by an actual mutation run. The triage figures (41 on lines 230/231, and 40 on line 233) are arithmetically consistent with the current 42-case baseline, but consistency is not measurement and they were not re-run.

WHAT IS BUILT AND READY: /tmp/no-mistakes-evidence/mutate.py implements the full matrix with the two-run method. All eight mutations are defined and every anchor was verified to match exactly once in its target file. The new away-mode row mutates successor_wanted in bin/fm-watch-arm.sh:297, replacing the '[ -e "$STATE/.afk" ] && return 1' guard with ':' while leaving fm_supervision_needed intact, so a successor is armed even in away mode. The shared-pause-release row targets clear_pause_state in bin/fm-watch.sh:398-403 (not the similar rm at line 477), because that is the release primitive both test_a_cleared_park_is_noticed_promptly_inside_a_closed_cadence and test_secondmate_unpause_clears_pause_tracking depend on. The harness removes each killed case's invocation line and re-runs, looping until the suite goes clean, so a mutation shared by more than one case is recorded honestly instead of being forced into a one-to-one claim. One run was observed working end to end before I stopped it.

TO CLOSE: run 'python3 /tmp/no-mistakes-evidence/mutate.py' (about 35 minutes; roughly 18 suite runs at ~2 minutes each) and transcribe matrix.json into the table, including the new away-mode row. Caveat: the harness lives in ephemeral /tmp and may not survive; if it is gone it must be rebuilt before the numbers can be re-measured.

SAFETY STATE: the interrupted run left mutation 1 applied to bin/fm-watch-arm.sh. I inspected the diff, confirmed it was exactly that mutation and nothing else, and reverted it. The worktree is verified clean at 58dae2b with zero modified files, so no behavioral change escaped this phase.

🔧 Fix: re-measure guard-class mutation matrix and add away-mode row
1 info still open:

  • ℹ️ bin/fm-watch.sh:477 - Surviving mutant found while re-measuring the matrix, reported rather than fixed because this is a documentation phase and closing it means adding a test.

While determining which release site the 'Stop releasing a lane whose pause verb has gone' row actually describes, I measured both candidates. The else branch inside surface_nonterminal_stale (bin/fm-watch.sh:477) - the site that literally matches that label, releasing .paused-&lt;key&gt;, .paused-rechecked-&lt;key&gt; and .paused-resurfaced-&lt;key&gt; when status_is_paused_or_captain_held is false - can be replaced with : and tests/fm-watch-triage.test.sh still passes in full: 42 ok - lines, exit 0, zero cases killed. Measured in a private copy; log at /tmp/no-mistakes-evidence/01KYVWHDGTDH63YJJC091K85AP/matrix/keep-cleared-pause-verb.run1.log.

The row was therefore published against the site that IS guarded, clear_pause_state (bin/fm-watch.sh:403), which four cases depend on. That is honest and measured, but it means the record's 'one mutation per protection' framing now has a known hole: a lane that surfaces as non-terminal stale after its declared pause verb disappears has its pause markers cleared by an untested branch.

Suggested follow-up, outside this change: a triage case driving a window through surface_nonterminal_stale with a status whose pause verb has been replaced by a non-paused one, asserting the three .paused-* markers are gone. Then the matrix can carry a genuine row for that protection instead of relying on the broader clear_pause_state mutation.

🔧 Fix: record surviving mutant at redundant pause-release site
✅ Re-checked - no issues remain.

✅ **Lint** - passed

✅ No issues found.

✅ **Push** - passed

✅ No issues found.

Captured 2026-07-31 from a live home's state/.watch-cycle-exits.log. It
accumulates over days and cannot be regenerated, so it is committed as raw
evidence before any analysis or fix is written against it.
… lanes a check-in cadence

wip checkpoint: implementation only, regression tests still to come.

The arm layer never started the next watcher cycle after delivering an
actionable wake (successor=none on 499 of 499 captured cycles), so supervision
ended on every wake and resumed only if some adapter above it happened to
re-arm. Arming is now the first act of consuming a wake.

Parked lanes each carried a standing per-cycle trigger, so a fresh watcher died
within seconds of every start on whichever lane was next overdue. Due lanes now
batch into one home-wide check-in rate-limited by .last-parked-checkin mtime.
… parked check-in cadence

Six cases across the two suites that already own this behavior:
- a delivered wake arms its successor before the wake is handled
- an attached arm delivers the wake its successor cycle closed on
- no successor is armed for a home with nothing left to supervise
- the preserved 499-record capture still shows the defect a fresh cycle no longer reproduces
- parked lanes batch into one check-in per cadence window, and the cadence still comes due
- a parked lane whose wait has cleared is surfaced at once inside a closed cadence
…adence

Corrects the ownership claim that continuity lives only in the per-harness
adapters, and records the forked-session case where Claude's Stop hook is inert
by design because state/.lock still names the live pre-fork harness.
fm_test_tmproot is called through a command substitution, so its array append
landed in the subshell and every suite leaked its entire temp root. Registration
now goes to a file that survives the subshell, and only the sourcing shell may
run the cleanup.

This became load-bearing once an arm can leave a detached successor: a leaked
temp root kept a live watcher supervising a home no test owned any more, and the
accumulated processes destabilised timing-sensitive cases in the same suite.

Also keep the away-mode daemon on the unbatched parked wake form, since it
classifies one window per printed reason.
tests/fixtures/<name>/ is the repo's fixture convention: the changed-test
selector resolves a fixture through the directory its consuming suite names,
and a file sitting directly under tests/fixtures/ has no mapping at all.
…p root

A named per-owner path is also what lets the command-substitution subshell
append to the registry its parent will read, and a suite that never takes a
temp root now leaves nothing behind.
@mattadams-dev

Copy link
Copy Markdown
Author

Apologies for the noise - this was opened against upstream by mistake and has been reopened on our own fork (mattadams-dev#3); no action needed here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant