Skip to content

fix(bin): retire stalled owned watcher child with bounded TERM/KILL - #2320

Open
mayankasthana wants to merge 18 commits into
kunchenguid:mainfrom
mayankasthana:fm/land-watcher-arm-stalled-child-retiremen-5d
Open

mayankasthana wants to merge 18 commits into
kunchenguid:mainfrom
mayankasthana:fm/land-watcher-arm-stalled-child-retiremen-5d

Conversation

@mayankasthana

@mayankasthana mayankasthana commented Aug 13, 2026 •

Copy link
Copy Markdown

Intent

Land PR #2320 for issue #2251: retire a stalled owned watcher child with a bounded TERM/KILL sequence so a wedged watcher no longer requires a session restart. This update re-applies the fix onto current main (resolving the overlap with the landed stall-bound eviction #5594), closes the successor-lock race Greptile flagged by holding the steal mutex across the stale-lock removal, reworks the lost-race regression for the current startup sequence, and regenerates the pipeline attestation for the new head.

What Changed

  • bin/fm-watch-arm.sh no longer treats a live PID as permanent health for a watcher it forked: wait_owned_child re-applies the identity-bound beacon predicate after initial readiness and, once the shared stale-beacon grace is reached, retires the child through the bounded retire_watch_child contract (TERM, then KILL to the child's isolated process group after FM_WATCH_STALL_RETIRE_TIMEOUT, default 2s, invalid or zero values falling back to 2). The arm then publishes the existing watcher-down recovery episode and exits with a typed FAILED line so a persistent adapter can run its existing bounded retry instead of a primary session restart. The lost-race stand-down path and the confirmation-timeout path now run the same bounded retirement instead of an unbounded wait.
  • clear_stale_recorded_watcher_lock takes the watcher's steal mutex and rechecks the recorded PID and identity under it before removing the stale lock, so a concurrent arm that steals the same stale lock can no longer race the removal; the release is refused (and recorded in the cycle-exit ledger) when the child survived the bound or no longer matches the expected owner.
  • bin/fm-watch.sh refreshes the handling-successor beacon while it waits out a pending downtime marker. Docs gain the FM_WATCH_STALL_RETIRE_TIMEOUT knob in docs/configuration.md and the retirement/release contract in docs/watcher-continuity.md; tests add a real-process SIGSTOP retirement counterfactual on both platforms and a bounded lost-race stand-down regression, with .claude/hooks/ added to .gitignore.

Risk Assessment

⚠️ Medium: The bounded-retirement algorithm is sound and thoroughly covered by real-process tests, but the change adds an autonomous watcher-kill path whose documented threshold (grace) disagrees with the header's own statement (the 3x stall bound) and carries an unrequired second process, so the contract worth confirming is a follow-up rather than a blocker.

Testing

Verification targeted the change's runtime surface: the watcher-arm retirement contract in bin/fm-watch-arm.sh (plus the small bin/fm-watch.sh handling-successor pre-loop block the branch carries). Three focused suites that drive real processes ran green (42 + 31 + 2 cases, no failures): fm-watcher-lock, fm-watch-arm, and fm-watch-recovery-loop. On top of those I drove the product by hand twice in disposable homes — first against the real arm and the real watcher it forks, where a SIGSTOPped, stale-beacon owned watcher was retired in 3s (SIGKILL, exit 137 in the ledger), the stale singleton lock was released, state/.watcher-down became pending:downtime:*, and the arm exited 1 with its typed line; the same home's next arm then surfaced check: rearm-resurface and exited 0, i.e. no session restart; and a healthy owned watcher was left untouched for 14s (grace 5s, retirement bound 3s) with the arm alive and the beacon advancing. Second, adversarial work on the successor-lock race: a concurrent successor that steals the lock during the arm's retirement window kept its lock untouched, and a live foreign holder of the steal mutex made the arm refuse the removal by name rather than delete a lock it could not verify. Reviewer-visible evidence is the two CLI transcripts plus the three suite logs in the evidence directory; there is no UI surface in this change, so no screenshot/video artifact applies. No product defect surfaced; the one surface I could not exercise is the harness-owned auto-arm path (a live Pi / OpenCode / Claude Stop-hook session), which this host cannot stand up — its entry point bin/fm-claude-stop-autoarm.sh is unchanged by this branch, and the arm contract it depends on is what the transcripts above exercised.

  • Live validation: ✅ go - 9 of 10 scenarios driven live against the product
Scenario Result Live Evidence
A wedged owned watcher (SIGSTOPped with a stale beacon) is retired within a bounded window and the arm fails loudly instead of waiting forever ✅ pass live live-retirement-transcript.log (S1) and bash tests/fm-watcher-lock.test.sh / test_stopped_watcher_is_retired_and_rearms_without_session_restart
After the retirement the SAME home and session re-arms and surfaces the accepted downtime, so no session restart is needed ✅ pass live live-retirement-transcript.log (S2): second arm in the same FM_HOME/state printed check: rearm-resurface and exited 0; also the recovery half of `test_stopped_watcher_is_retired_and_rearms_without_s…
Adversarial: a healthy owned watcher is NOT retired — the arm stays alive well past its grace and the child's beacon keeps advancing ✅ pass live live-retirement-transcript.log (S3): held 14s against a 5s grace and a 3s retirement bound; arm alive, watcher alive, beacon mtime advanced; also the healthy-successor half of the fm-watcher-lock reti…
Adversarial boundary on the attached path: a slow-but-live holder below the stall bound is followed and its wake reported, while a holder past the bound ends the arm with the typed stalled-holder line… ✅ pass live bash tests/fm-watch-arm.test.sh / test_attached_arm_follows_a_slow_live_holder and test_attached_arm_hands_a_stalled_holder_to_its_replacement (arm-suite.log); the watcher-side eviction is also…
Adversarial: a child that LOSES the singleton startup race but stalls before standing down is retired within a bounded wait instead of blocking the arm forever ✅ pass live bash tests/fm-watch-arm.test.sh / test_lost_race_child_stand_down_is_bounded — driven against a copied bin dir whose fm-watch.sh never exits, asserting the arm exits on its own with `stalled befor…
Adversarial: a concurrent successor that steals this home's stale lock during the arm's retirement keeps its lock — the arm never deletes a live successor's ownership ✅ pass live live-successor-lock-race.log (variant A): successor pid 11341 stole the lock mid-retirement; after the arm closed, state/.watch.lock/pid was still 11341 and the ledger recorded reason=stale-beacon-ret…
Adversarial: with the watcher lock's steal mutex held by a live foreign process, the arm refuses the stale-lock removal by name and leaves the lock intact ✅ pass live live-successor-lock-race.log (variant B): arm printed watcher: FAILED - watcher pid=11671 ... recovery state could not release stale ownership, ledger reason=stale-beacon-release-failed, lock still…
Signal path: HUP/TERM retire the owned child through the same bounded contract rather than an indefinite wait ✅ pass live bash tests/fm-watcher-lock.test.sh cases arm cleans child watcher and temp output on HUP, arm defers TERM until startup watcher can run its lock cleanup, `arm TERM bounds wait for stalled startu…
The branch's handling-successor pre-loop change does not keep a handling successor out of its supervision loop ✅ pass live bash tests/fm-watch-recovery-loop.test.sh — 2/2 ok: a resurfacing handling successor stays alive and supervises instead of going blind (recovery-loop-suite.log)
End-user path: a Pi / OpenCode / Claude Stop-hook session auto-arms through the unchanged hook and recovers a wedged watcher without the operator restarting the primary ⏸️ untested no Requires a live agent harness session (Pi, OpenCode, or a Claude Code Stop-hook session with its asyncRewake notification) to exercise bin/fm-claude-stop-autoarm.sh as the adapter that spawns the arm;…
Evidence: Live drive: wedged owned watcher retired, same-session recovery, healthy child not retired

Source: Live drive: wedged owned watcher retired, same-session recovery, healthy child not retired

===== S1: retire a wedged owned watcher within a bound (no session restart) =====
arm confirmed: watcher: started pid=98848 (beacon fresh)
owned watcher: pid=98848 pgid=98848 (arm pid=98831)
wedge: SIGSTOP + beacon backdated; beacon age 844391201s vs grace 5s
--- arm stdout ---
watcher: started pid=98848 (beacon fresh)
watcher: FAILED - watcher pid=98848 stopped advancing its beacon for 844391201s; retired the stalled cycle and released stale ownership for bounded recovery
--- end arm stdout; exit status 1 after 3s ---
observed state: watcher 98848 gone; state/.watch.lock absent; state/.watcher-down=pending:downtime:98831.1791056204.zv4700
ledger row: arm_pid=98831 watcher_pid=98848 origin=started started_at=1791056200 ended_at=1791056204 exit_code=137 signal=KILL reason=stale-beacon-retired beacon_age=844391204 lock_before=pid:98848|identity:... lock_after=pid:none|identity:none successor=none
RESULT: PASS - wedged owned watcher retired in 3s, ownership released, arm failed loudly

===== S2: the SAME session re-arms and surfaces the accepted downtime =====
--- arm stdout (exit status 0) ---
watcher: started pid=99835 (beacon fresh)
check: rearm-resurface
--- end arm stdout ---
RESULT: PASS - same home/session, no restart: the next arm surfaced the downtime and exited 0

===== S3 (adversarial): a healthy owned watcher is NOT retired =====
arm confirmed: watcher: started pid=981 (beacon fresh)
held 14s (grace 5s, retirement bound 3s) with the arm alive; beacon mtime 1791056207 -> 1791056228
before cleanup: arm alive=yes, watcher 981 alive=yes
RESULT: PASS - healthy owned watcher survived the grace window and kept beating
Evidence: Live adversarial drive of the successor-lock race (both halves)

Source: Live adversarial drive of the successor-lock race (both halves)

===== A: a concurrent successor's live lock survives the arm's stale-lock release =====
arm pid=10710 -> watcher: started pid=10727 (beacon fresh)
wedge: SIGSTOP owned watcher pid=10727, beacon age 844391247s vs grace 5s
concurrent successor pid=11341 stole the stale lock while the arm was retiring its child
state/.watch.lock/pid is now 11341
--- arm stdout (exit status 1) ---
watcher: started pid=10727 (beacon fresh)
watcher: FAILED - watcher pid=10727 stopped advancing its beacon for 844391247s; retired the stalled cycle and released stale ownership for bounded recovery
--- end arm stdout ---
after the arm closed: state/.watch.lock/pid=11341 (successor pid=11341)
RESULT: PASS - the arm retired its own stalled child without touching the successor's lock

===== B: a live steal-mutex holder makes the arm refuse the removal by name =====
foreign steal-mutex holder pid=12287 is live for the whole retirement window
--- arm stdout (exit status 1) ---
watcher: FAILED - watcher pid=11671 stopped advancing its beacon for 844391251s; recovery state could not release stale ownership
--- end arm stdout ---
RESULT: PASS - with the steal mutex held the arm refused the removal by name and left the lock intact
Evidence: fm-watcher-lock suite (42 cases, includes the stalled-watcher retirement regression)

Source: fm-watcher-lock suite (42 cases, includes the stalled-watcher retirement regression)

ok - owned arm retires a live stale watcher, releases recovery state, and preserves a healthy successor
ok - arm TERM bounds wait for stalled startup
ok - arm cleans child watcher and temp output on HUP
ok - arm defers TERM until startup watcher can run its lock cleanup
ok - live watcher lock with a beacon past the hard bound is replaced, under it is still refused
(42 ok, 0 failures)
Evidence: fm-watch-arm suite (31 cases, includes the lost-race and attached-holder bounds)

Source: fm-watch-arm suite (31 cases, includes the lost-race and attached-holder bounds)

ok - watch-arm: a lost-race child that stalls before standing down is retired instead of blocking the arm forever
ok - watch-arm: an attached arm keeps following a slow live holder and reports its wake
ok - watch-arm: an attached arm hands a holder stalled past the bound to its owner's replacement
ok - watch-arm: --stop ends only this home's watcher, publishes downtime, and reports when none runs
(31 ok, 0 failures)
Evidence: fm-watch-recovery-loop suite (handling successor still supervises)

Source: fm-watch-recovery-loop suite (handling successor still supervises)

ok - a resurfacing handling successor stays alive and supervises instead of going blind
ok - unacknowledged recovery is announced at most once per generation and the successor stays alive

Pipeline

Updates from git push no-mistakes

✅ **intent** - passed

✅ No issues found.

✅ **Rebase** - passed

✅ No issues found.

⚠️ **Review** - 7 issues (4 warnings, 3 infos)
  • ⚠️ bin/fm-watch-arm.sh:50 - The merged header now contradicts itself about how long a started (owned) arm tolerates a slow child. Lines 41-47 (added by this change) say a live identity-matched child whose beacon reaches the shared stale-beacon grace is retired with the bounded TERM/KILL sequence, and docs/watcher-continuity.md:382 documents the same trigger ("reaches the shared stale-beacon grace"). But line 50 still carries upstream's clause "as a started arm waits out a slow child, until the lock changes or the beacon reaches fm_watcher_stall_bound" — i.e. 3x grace (fm_watcher_stall_bound, bin/fm-wake-lib.sh:150-154) — for the started path. The implementation uses GRACE (1x), so the started arm now retires at ~300s what the attached path still follows to ~900s. Concretely: a watcher whose main-loop check runs 300-900s without touching state/.last-watcher-beat (FM_WATCHER_STALE_GRACE/WATCHER_STALL_BOUND tolerate exactly that) is killed by this arm although the same evidence on an attached holder is followed. Same-class stale text: line 41 "On started it waits the child and propagates the wake reason", now superseded by lines 42-47. Remedy is a comment correction (align line 50/41 with the grace-based owned-child retirement); if the 3x tolerance was actually intended for owned children, the code needs the change instead - that choice is the author's.
  • ℹ️ bin/fm-watch-arm.sh:797 - start_owned_watchdog (line 763) introduces a second state-dir artifact, "$child_out.liveness", but only the two branches inside wait_owned_child remove it (lines 962 and 970). cleanup_child (line 793) removes just $child_out, so a HUP/TERM/INT handled while the watchdog has already written the stale beacon age leaves state/.watch-arm-output.XXXXXX.liveness behind. That window is reachable: the watchdog writes the file immediately, then sleeps STALL_RETIRE_TIMEOUT, and for a TERM-resistant child the arm keeps the file for up to STALL_RETIRE_TIMEOUT+3s before its own stale branch tears it down, so a signal landing in that window takes handle_arm_signal -> cleanup_child -> exit and the file survives. The repo already asserts on exactly this name pattern (tests/fm-watcher-lock.test.sh:1163: ! ls "$state"/.watch-arm-output.*), so the leak is an intermittent failure of that assertion as well as state-dir litter. Fix: remove the liveness sibling from cleanup_child too (guarded, since watchdog_status is empty on the confirm-timeout path).
  • ⚠️ bin/fm-watch-arm.sh:762 - Component: the arm-owned liveness watchdog subshell (start_owned_watchdog/stop_owned_watchdog, lines 762-791) plus its "$child_out.liveness" status file and its TERM/KILL trap. No intent requirement needs a second process: it exists only to evaluate owned_child_has_stale_beacon and then TERM/KILL the isolated group, which is a parallel copy of a rule wait_owned_child already owns. Its original justification ("Keep the arm itself in a raw wait so an actionable child close propagates immediately") no longer holds - commit 72c5d2a replaced that raw wait with the ATTACH_POLL poll loop the arm now runs (lines 916-924), so the arm is already free to evaluate that same predicate inline and call the existing retire_watch_child, which already implements the bounded TERM/KILL/group-sweep/reap shape. Net removal: ~30 lines, one background process, one state file, and the trap/sleep/wait choreography, with no behavior the intent requires (actionable closes are already detected only to ATTACH_POLL granularity, and the status file is the sole thing arming the retire deadline). This is an implementation choice, not a defect, so it needs the author's call.

🔧 Fix applied.
7 issues (4 warnings, 3 infos) still open:

  • ⚠️ bin/fm-watch-arm.sh:50 - The merged header now contradicts itself about how long a started (owned) arm tolerates a slow child. Lines 41-47 (added by this change) say a live identity-matched child whose beacon reaches the shared stale-beacon grace is retired with the bounded TERM/KILL sequence, and docs/watcher-continuity.md:382 documents the same trigger ("reaches the shared stale-beacon grace"). But line 50 still carries upstream's clause "as a started arm waits out a slow child, until the lock changes or the beacon reaches fm_watcher_stall_bound" — i.e. 3x grace (fm_watcher_stall_bound, bin/fm-wake-lib.sh:150-154) — for the started path. The implementation uses GRACE (1x), so the started arm now retires at ~300s what the attached path still follows to ~900s. Concretely: a watcher whose main-loop check runs 300-900s without touching state/.last-watcher-beat (FM_WATCHER_STALE_GRACE/WATCHER_STALL_BOUND tolerate exactly that) is killed by this arm although the same evidence on an attached holder is followed. Same-class stale text: line 41 "On started it waits the child and propagates the wake reason", now superseded by lines 42-47. Remedy is a comment correction (align line 50/41 with the grace-based owned-child retirement); if the 3x tolerance was actually intended for owned children, the code needs the change instead - that choice is the author's.
  • ℹ️ bin/fm-watch-arm.sh:797 - start_owned_watchdog (line 763) introduces a second state-dir artifact, "$child_out.liveness", but only the two branches inside wait_owned_child remove it (lines 962 and 970). cleanup_child (line 793) removes just $child_out, so a HUP/TERM/INT handled while the watchdog has already written the stale beacon age leaves state/.watch-arm-output.XXXXXX.liveness behind. That window is reachable: the watchdog writes the file immediately, then sleeps STALL_RETIRE_TIMEOUT, and for a TERM-resistant child the arm keeps the file for up to STALL_RETIRE_TIMEOUT+3s before its own stale branch tears it down, so a signal landing in that window takes handle_arm_signal -> cleanup_child -> exit and the file survives. The repo already asserts on exactly this name pattern (tests/fm-watcher-lock.test.sh:1163: ! ls "$state"/.watch-arm-output.*), so the leak is an intermittent failure of that assertion as well as state-dir litter. Fix: remove the liveness sibling from cleanup_child too (guarded, since watchdog_status is empty on the confirm-timeout path).
  • ⚠️ bin/fm-watch-arm.sh:762 - Component: the arm-owned liveness watchdog subshell (start_owned_watchdog/stop_owned_watchdog, lines 762-791) plus its "$child_out.liveness" status file and its TERM/KILL trap. No intent requirement needs a second process: it exists only to evaluate owned_child_has_stale_beacon and then TERM/KILL the isolated group, which is a parallel copy of a rule wait_owned_child already owns. Its original justification ("Keep the arm itself in a raw wait so an actionable child close propagates immediately") no longer holds - commit 72c5d2a replaced that raw wait with the ATTACH_POLL poll loop the arm now runs (lines 916-924), so the arm is already free to evaluate that same predicate inline and call the existing retire_watch_child, which already implements the bounded TERM/KILL/group-sweep/reap shape. Net removal: ~30 lines, one background process, one state file, and the trap/sleep/wait choreography, with no behavior the intent requires (actionable closes are already detected only to ATTACH_POLL granularity, and the status file is the sole thing arming the retire deadline). This is an implementation choice, not a defect, so it needs the author's call.
  • ⚠️ bin/fm-watch.sh:2630 - Component introduced by this change that no intent requirement needs: the FM_WATCH_HANDLING_SUCCESSOR pre-loop wait (if [ "${FM_WATCH_HANDLING_SUCCESSOR:-0}" = 1 ]; then touch beat; while handling_wait -lt 600; do touch beat; fm_recovery_marker_snapshot; case pending:downtime:*;; *) break;; esac; sleep 0.05; done; [ handling_wait -lt 600 ] || WATCHER_RECOVERY_PENDING=1; fi). It is not part of the arm-retirement fix; it arrived through this branch's merges of current main (base 96876db had it at bin/fm-watch.sh:816, it was never authored by a branch commit, and upstream deleted it as a bug in 3f03533 / PR fix: bound recovery announcements and preserve supervision #2733 precisely because it could keep a handling successor out of its poll loop). The block's original rationale (keep the beacon fresh so the arm's liveness watchdog did not retire a waiting handling successor) died in this run's fix round, which removed that watchdog entirely (commit ea786da). The tree's own contract now contradicts the block's existence: docs/watcher-continuity.md:195 states "A handling successor does not re-announce. It enters its poll loop immediately and keeps scanning signals, stale panes, and checks." Effect today is mostly latent, because bin/fm-watch.sh:2462 (main's arm_check) rewrites a pending:downtime: episode to announced:downtime: before this block runs, so the case usually breaks on the first pass; the live residue is the narrow window between that arm_check and this block, where a republication resolves the episode to pending:downtime: again and the successor then holds the marker lock each iteration (bin/fm-wake-lib.sh:846-853) for up to 600 iterations - measured in ~30-55s - before it starts supervising. Remedy (the remedy, not the defect, is what needs authorisation: it deletes merge-preserved behaviour rather than adding to it): delete lines 2630-2644 and accept upstream's version of the hunk, so the successor enters the poll loop immediately as the in-tree doc and fix: bound recovery announcements and preserve supervision #2733 require. No intent requirement in the supplied goal ("re-applies the fix onto current main") needs this block, and its beacon-refresh purpose is already provided by the poll loop's own touch at bin/fm-watch.sh:2682.
  • ⚠️ bin/fm-watch-arm.sh:737 - The new retirement predicate owned_child_has_stale_beacon decides a child is stalled with [ &#34;$(fm_path_age &#34;$BEAT&#34;)&#34; -ge &#34;$GRACE&#34; ], where GRACE is the arm's fixed default GRACE=${FM_GUARD_GRACE:-300} (line 134). The watcher layer derives its own staleness from the poll cadence: WATCHER_STALE_GRACE=${FM_WATCHER_STALE_GRACE:-${FM_GUARD_GRACE:-$(fm_poll_derived_grace &#34;$POLL&#34;)}} (bin/fm-watch.sh:277), and bin/fm-wake-lib.sh:125-140 documents that a fixed 300 is known-wrong once the cadence reaches it. The arm already derives the other bound that way (STALL_BOUND=$(fm_watcher_stall_bound), line 154, = 3x the derived grace), so the two thresholds can diverge by 3x. Concrete sequence: a home configured FM_POLL=600 (a quiet/long-poll home; fm_poll_derived_grace exists for exactly this), armed by Pi/OpenCode - neither .pi/extensions/fm-primary-pi-watch.ts nor .opencode/plugins/fm-primary-watch-arm.js sets FM_GUARD_GRACE, so the arm keeps GRACE=300 while the watcher's stale grace is 660 and its stall bound is 1980. The arm forks the watcher, confirms it healthy (beacon fresh, age<300), prints "watcher: started", then polls every ATTACH_POLL; the watcher touches the beacon once per main-loop iteration (bin/fm-watch.sh:2682), so at t=300s the arm sees age>=300 while the watcher is still mid-cycle and healthy by its own standard (and would only be evicted at 1980). The arm now TERMs/KILLs it, publishes downtime and exits 1 - every cycle, forever - which is supervision churn of the same class this change exists to remove. Before this change the same GRACE fed only the readiness gate, where the same divergence was benign (the confirmation loop samples right after each touch). Sibling sites of the same invariant: the readiness gate fm_watcher_healthy &#34;$STATE&#34; &#34;$WATCH&#34; &#34;$GRACE&#34; (bin/fm-watch-arm.sh:343) and the attach-hold bound derived at line 154 (attach_and_wait, line 444) already disagree with line 737 for any FM_POLL>240 home. Earliest shared boundary: derive the grace once at line 134 the same way fm_watcher_stall_bound does (FM_WATCHER_STALE_GRACE -> FM_GUARD_GRACE -> fm_poll_derived_grace "${FM_POLL:-15}") so readiness, the owned-child retirement and the attach bound all read one value; the alternative narrower fix is to have every arm spawner pass FM_GUARD_GRACE, which the Pi/OpenCode paths do not do today. This is a correctness fix to the new predicate's input, not new state or a new subsystem.
  • ℹ️ bin/fm-watch-arm.sh:424 - Sibling sites of the contract text the previous fix round (ea786da) corrected in the header but did not finish, all of which still assert that a started arm waits out a slow child: (1) bin/fm-watch-arm.sh:424-428, the attach_and_wait comment - "while the holder is alive and the lock still names it under the same identity, it is a slow cycle, which a started arm tolerates by waiting on its child, so this arm keeps following it" - is now false: the started path retires its child once the beacon reaches GRACE (wait_owned_child, line 870), well before STALL_BOUND, so the parallel justification for the attached path no longer exists (the header's replacement rationale is that an attached arm owns no child to wait on, lines 48-51). (2) The status-line enumeration at bin/fm-watch-arm.sh:28-37, which the same round rewrote adjacent to, still omits the retirement lines this change introduces: "watcher: FAILED - watcher pid=<N> stopped advancing its beacon ..." (lines 884 and 890) and "watcher: FAILED - our child pid=<N> stalled before standing down ..." (line 945); these are the change's primary new outputs and callers grep them (bin/fm-claude-stop-autoarm.sh:386-390). (3) tests/fm-watcher-lock.test.sh:1454 and 1476 still name "the arm's own stale-beacon watchdog" (a comment used to justify FM_GUARD_GRACE=30, and a fail message), a process the same round deleted, which makes a future failure message point at a component that does not exist. Comment/contract text only; no behaviour.
  • ℹ️ tests/fm-watch-arm.test.sh:1 - The merge commit d0fc0a8 silently flipped this test's mode (git diff --summary 1f3e769..HEAD => "mode change 100755 => 100644 tests/fm-watch-arm.test.sh"); it is the only mode change in the change set and upstream main keeps 100755. No branch commit authored the flip, so it is a merge artifact rather than intent. Impact is low because bin/fm-test-run.sh runs scripts as bash &#34;$script&#34; (line 2474) and no script executes this file directly, but it diverges from every sibling test and breaks any direct ./tests/fm-watch-arm.test.sh invocation. Fix: restore mode 100755.
✅ **Test** - passed

✅ No issues found.

  • Live validation: ✅ go - 9 of 10 scenarios driven live against the product
Scenario Result Live Evidence
A wedged owned watcher (SIGSTOPped with a stale beacon) is retired within a bounded window and the arm fails loudly instead of waiting forever ✅ pass live live-retirement-transcript.log (S1) and bash tests/fm-watcher-lock.test.sh / test_stopped_watcher_is_retired_and_rearms_without_session_restart
After the retirement the SAME home and session re-arms and surfaces the accepted downtime, so no session restart is needed ✅ pass live live-retirement-transcript.log (S2): second arm in the same FM_HOME/state printed check: rearm-resurface and exited 0; also the recovery half of `test_stopped_watcher_is_retired_and_rearms_without_s…
Adversarial: a healthy owned watcher is NOT retired — the arm stays alive well past its grace and the child's beacon keeps advancing ✅ pass live live-retirement-transcript.log (S3): held 14s against a 5s grace and a 3s retirement bound; arm alive, watcher alive, beacon mtime advanced; also the healthy-successor half of the fm-watcher-lock reti…
Adversarial boundary on the attached path: a slow-but-live holder below the stall bound is followed and its wake reported, while a holder past the bound ends the arm with the typed stalled-holder line… ✅ pass live bash tests/fm-watch-arm.test.sh / test_attached_arm_follows_a_slow_live_holder and test_attached_arm_hands_a_stalled_holder_to_its_replacement (arm-suite.log); the watcher-side eviction is also…
Adversarial: a child that LOSES the singleton startup race but stalls before standing down is retired within a bounded wait instead of blocking the arm forever ✅ pass live bash tests/fm-watch-arm.test.sh / test_lost_race_child_stand_down_is_bounded — driven against a copied bin dir whose fm-watch.sh never exits, asserting the arm exits on its own with `stalled befor…
Adversarial: a concurrent successor that steals this home's stale lock during the arm's retirement keeps its lock — the arm never deletes a live successor's ownership ✅ pass live live-successor-lock-race.log (variant A): successor pid 11341 stole the lock mid-retirement; after the arm closed, state/.watch.lock/pid was still 11341 and the ledger recorded reason=stale-beacon-ret…
Adversarial: with the watcher lock's steal mutex held by a live foreign process, the arm refuses the stale-lock removal by name and leaves the lock intact ✅ pass live live-successor-lock-race.log (variant B): arm printed watcher: FAILED - watcher pid=11671 ... recovery state could not release stale ownership, ledger reason=stale-beacon-release-failed, lock still…
Signal path: HUP/TERM retire the owned child through the same bounded contract rather than an indefinite wait ✅ pass live bash tests/fm-watcher-lock.test.sh cases arm cleans child watcher and temp output on HUP, arm defers TERM until startup watcher can run its lock cleanup, `arm TERM bounds wait for stalled startu…
The branch's handling-successor pre-loop change does not keep a handling successor out of its supervision loop ✅ pass live bash tests/fm-watch-recovery-loop.test.sh — 2/2 ok: a resurfacing handling successor stays alive and supervises instead of going blind (recovery-loop-suite.log)
End-user path: a Pi / OpenCode / Claude Stop-hook session auto-arms through the unchanged hook and recovers a wedged watcher without the operator restarting the primary ⏸️ untested no Requires a live agent harness session (Pi, OpenCode, or a Claude Code Stop-hook session with its asyncRewake notification) to exercise bin/fm-claude-stop-autoarm.sh as the adapter that spawns the arm;…
  • bash tests/fm-watcher-lock.test.sh — 42 cases, all ok, including test_stopped_watcher_is_retired_and_rearms_without_session_restart, test_arm_term_bounds_wait_for_stalled_startup, test_arm_hup_cleans_child_and_temp_output, test_live_stalled_watch_lock_is_replaced_past_hard_bound
  • bash tests/fm-watch-arm.test.sh — 31 cases, all ok, including test_lost_race_child_stand_down_is_bounded, test_attached_arm_follows_a_slow_live_holder, test_attached_arm_hands_a_stalled_holder_to_its_replacement
  • bash tests/fm-watch-recovery-loop.test.sh — 2 cases, all ok (handling successor enters its poll loop and supervises)
  • bash live-retirement-drive.sh — manual live drive of the real arm + real watcher: S1 bounded retirement, S2 same-session recovery, S3 no over-retirement of a healthy child
  • bash live-successor-lock-race.sh — manual live adversarial drive: A concurrent successor's lock survives the stale-lock release; B a live steal-mutex holder forces the typed recovery state could not release stale ownership refusal
  • git status --porcelain after all runs — clean; no stray fm-watch/fm-watch-arm processes left behind
✅ **Document** - passed

✅ No issues found.

✅ **Lint** - passed

✅ No issues found.

✅ **Push** - passed

✅ No issues found.

SIGKILL is never held pending for a stopped process on Linux: the bounded
retirement's KILL kills the stopped watcher immediately, the arm's wait
reaps it before the expected-pid hardening runs, and the retirement takes
the released-lock shape (stale-beacon-retired), not the release-failed
shape. The old Linux case block asserted the opposite and failed
deterministically on the ubuntu runner; the platform-gated block had never
run during macOS local validation.
@kunchenguid

Copy link
Copy Markdown
Owner

Speaking as Kun's firstmate:

Scheduled 3:10pm PT 8/23 pass. VISION.md read in full from current main ddf74ef22f73a33bc04971626a7d8a4f0bf2fe67 (#2901). Reconfirmed. Issue #2251 is ready-for-pr (labeled 7:10am); that is a queue label, not a merge vote. No captain comment authorizing a merge. Helping this existing PR; will not open a competing one.

VISION (inspected the stalled-owned-child retirement in bin/fm-watch-arm.sh, the one-line beat refresh in bin/fm-watch.sh, docs/watcher-continuity.md, retirement tests). Per-rule: restart is a non-event aligns (a live-but-stale owned child currently wedges supervision until a session restart); peace of mind aligns (loud typed failure + bounded TERM/KILL + lock release + same-session re-arm); scripts own the mechanics aligns. The retirement is restoring intended liveness semantics, not a new captain-facing capability.

Class: corrective.

Security: none. No workflow-file / secret / injection risk. TERM/KILL is scoped to the arm-owned watcher process group after a stale-beacon predicate; fail-loud, not silent.

Overlap / HOLD: bin/fm-watch.sh also in open #2877 / #2701 / #2809 / #2796 / #2882 / #2867; bin/fm-watch-arm.sh also in #2796. Not a standing spawn-freshen/teardown/herdr hold. Help this PR rather than competing.

CI / NM: HEAD 44dbfe5face58f25e68bf52510a6d7d32b278959. GitHub mergeable=CONFLICTING / DIRTY (rebaseable=false). Ahead 10 / behind 63 vs current main. Body no-mistakes-pipeline-attestation:v1 names c213885219a6430db1237506c68deebd08b4e2b6, not THIS HEAD (later fix(tests): assert the Linux stopped-watcher retirement shape correctly). CI on this HEAD is stale (completed 2026-08-17, all SUCCESS then). Require no-mistakes SUCCESS on that same Aug 17 SHA does not satisfy a matching attestation for THIS HEAD against current main.

Workflows: already approved historically (CI completed SUCCESS on 2026-08-17). Run IDs: 32033358684 (CI), 32033358622 (Require no-mistakes). No pending first-time-fork approval.

What would help this PR land: rebase onto current main until GitHub reports MERGEABLE, regenerate no-mistakes-pipeline-attestation:v1 for the new HEAD, and let CI re-run. A cloud conflict-fix is not in play this pass — the PR is not otherwise fully auto-eligible (conflicts + NM mismatch + 63 behind). We will not open a competing PR for #2251.

Land-eligible rec: NO (merge conflicts; 63 behind; NM attestation mismatch; stale CI vs current main). Captain-flag NOW: no.

Waiting on the author to rebase off current main, clear the conflicts, and re-stamp no-mistakes for the new HEAD. Not a captain-decision hold.

@mayankasthana mayankasthana changed the title fix(bin): retire a stalled owned watcher child with a bounded TERM/KILL sequence fix(bin): retire stalled owned watcher child with bounded TERM/KILL Aug 24, 2026
@greptile-apps

greptile-apps Bot commented Aug 24, 2026 •

Copy link
Copy Markdown

RetriggerConfidence Score: 4/5

[High risk] Modifies watcher process lifecycle and signal handling.

The PR is not ready to merge until the concurrency tests can terminate and report a multiple-winner failure.

Reviews (5) · Last reviewed commit: "no-mistakes(review): Fix arm header cont..."

Comment thread bin/fm-watch-arm.sh
Comment thread bin/fm-watch-arm.sh
@kunchenguid

Copy link
Copy Markdown
Owner

Speaking as Kun's firstmate:

Scheduled 11:10am PT 8/24 pass. VISION.md read in full from current main 038d0f7ec6ba7238a151722931434dcf06ff37c4 (#2942). LAST PASS on this PR was CONFLICTING/DIRTY, 63 behind, NM mismatch, stale CI. Re-checked THIS HEAD. Helping this existing PR; no competing PR. Never messaged the captain.

VISION (re-inspected stalled-owned-child retirement in bin/fm-watch-arm.sh, bounded beat refresh in bin/fm-watch.sh, docs/watcher-continuity.md, retirement tests). Per-rule unchanged: restart-is-a-non-event aligns; peace-of-mind aligns (loud typed failure + bounded TERM/KILL + lock release + same-session re-arm); scripts-own-mechanics aligns. Restores intended liveness; not a new captain-facing capability. Authority-is-explicit aligns (timeout knob only).

Class: corrective.

Security: none. No workflow-file / secret / injection. TERM/KILL is scoped to the arm-owned watcher process group after a stale-beacon predicate; fail-loud. Greptile still flags a successor-lock race in clear_stale_recorded_watcher_lock — not a merge gate.

Overlap: bin/fm-watch.sh also in many open PRs (#2970, #2953, #2914, #2877, #2867, #2809, #2796, #2701, …); bin/fm-watch-arm.sh also in #2914 / #2796 / #2727 / #2705. Not a standing spawn-freshen / teardown / herdr / lock hold. Help this PR rather than competing.

THIS HEAD vs last pass: conflicts cleared. GitHub mergeable=MERGEABLE, mergeStateStatus=UNSTABLE. ahead 14 / behind 0 (was ahead 10 / behind 63). I will not resolve conflicts — they are already gone — and I will not conflict-fix via cloud agent because the PR is still not otherwise auto-merge-ready.

CI / NM: HEAD ef17c98afadeb8ad15cd9079bfce48c45ee8d4c6. Body no-mistakes-pipeline-attestation:v1 names 1b015442835379e86026b44967cc98114b609424, not THIS HEAD (later no-mistakes: apply CI fixes). Fork CI was action_required on the rebased HEAD; approved this pass after full diff review to help the PR.

Workflows approved this pass: CI 32757334876, Require no-mistakes 32757334813. Not green at comment time.

Land-eligible rec: NO (NM attestation mismatch vs THIS HEAD; CI not yet green). Captain-flag NOW: no.

Waiting-on-author to regenerate no-mistakes-pipeline-attestation:v1 for HEAD ef17c98 and let the just-approved CI finish. Rebase is done; restamp NM. Not a captain-decision hold.

Comment thread bin/fm-watch-arm.sh
@kunchenguid

Copy link
Copy Markdown
Owner

Speaking as Kun's firstmate: recirc. Conflicts look cleared vs last pass (now MERGEABLE). Still not auto-eligible.

class=corrective. Bounded TERM/KILL of an arm-owned stalled watcher child after a stale-beacon predicate. Restores intended liveness.

VISION.md: restart-as-non-event aligns. Honest interface aligns. Scripts align. Authority n/a. Spine aligns. Vendor aligns. Scope aligns.

This HEAD: f59a4933df66d970c3dcdd5d7ef435ac564df60f. MERGEABLE / UNSTABLE, ahead 15 / behind 0.
Attestation 1b015442… ≠ THIS HEAD. First-time-fork-style workflow approval this pass after diff review (no .github): CI 32762125971, Require no-mistakes 32762126075.

Overlap: bin/fm-watch.sh / bin/fm-watch-arm.sh with other open watcher PRs. Help this PR; no competing one.

Waiting on author for a HEAD-matching attestation and green CI on this SHA. Not a captain-decision hold.

Resolve the stalled-owned-child retirement against the landed stall-bound
eviction (kunchenguid#5594): both knobs coexist (FM_WATCH_STALL_RETIRE_TIMEOUT for the
owned-child retirement, fm_watcher_stall_bound for the attach-follow bound),
the interrupt path keeps main's cleanup-ready guard before the bounded
retirement, and the lost-race regression now arms through a copied bin dir
with a never-exiting watcher child since main no longer runs the pre-lock
check migration. clear_stale_recorded_watcher_lock now holds the steal mutex
across a final ownership recheck and the removal, so a concurrent successor
steal can no longer lose its freshly acquired lock in the verify window.
# legitimately reclaimed as a dead-pid lock - a second winner in the
# ledger. Staying alive until all losers have finished makes the
# single-winner invariant hold deterministically.
while [ "$(wc -l < "$4" 2>/dev/null || true)" -lt "$5" ]; do

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Multiple winners hang the test

If two contenders acquire the lock, both winners wait for 39 losing contributions, but only 38 contenders can contribute one. Neither winner exits, so the test hangs until an external timeout instead of reporting the lock failure. The stale-lock concurrency test has the same wait condition.

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.

2 participants