Repository navigation
fix(bin): bound watcher arm confirmation per startup phase - #7
Merged
Merged
Conversation
The arm confirmed a fresh watcher against one wall-clock window over the whole cold start. On Git Bash/MSYS a watcher start is dominated by process creation (about 200 forks and execs before the first beat at about 70ms each): measured on 2026-09-17 as 5-8s to the lock and 12-18s to the first beat on an idle host, and 19.9s until the arm reported started, against a 31s window. The arm's own confirmation loop forked about ten processes per 0.2s poll and measurably slowed the child it was waiting for. Under ordinary contention the window ran out and the arm tore down a healthy child on its way to its first beat. The confirmation is now bounded per observable startup phase (lock claimed, identity published, beacon published), with each newly observed phase re-arming FM_ARM_CONFIRM_TIMEOUT, so a slow but progressing child is never torn down while a stall inside one phase still fails loudly. The loop reads those phases with shell builtins and runs the forking health proof only once they say it can pass or a foreign holder must be judged. A timed-out cycle records the phase it stalled in as the ledger reason suffix and prints it before the typed failure line. tests/fm-watcher-lock.test.sh gains a regression that drives the real arm with a stand-in watcher whose phases each fit the window while their sum does not, plus a stalled-phase negative control. Its healthy-peer restart case failed on MSYS for an unrelated reason: Cygwin's kill cannot signal a native node process and terminates it instead, so the TERM-resistant peer died; the peer is now a shell that ignores TERM on every platform.
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.
Read this first: what this change does and does not prove
check: rearm-resurface, exit 2) is the watcher's own downtime-recovery wake, not a confirmation timeout. The home's recorded confirmation timeouts show a child that had claimed the lock and then stalled before publishing its identity, behind unbounded lock waits inbin/fm-wake-lib.sh. What blocks that lock-to-identity step is a separate investigation and is not addressed here.fm-adapter-arm-timeout-w1; the adapter defaults are deliberately unchanged here.tests/fm-watcher-lock.test.sh(test_watch_restart_attaches_to_healthy_peer), which the Intent names as a live reproduction, failed because of a platform-specific fixture defect: Cygwin cannot deliver a signal to a native Windows process, so the node peer was terminated, and its attach wait was undersized for a Git Bash cold start. That case now passing is NOT evidence that the arm defect is fixed.test_arm_confirmation_is_bounded_per_startup_phaseis the genuine regression proof for the per-phase bound: it fails against the base-commit arm and passes against this one. That proof covers the per-phase bound only, not the originally reported failure.Behavior portable serial 1job was cancelled at its 30-minute time limit, without a verdict. It never got past its first suite,tests/fm-watch-triage.test.sh, which loads nothing this change touches, and one case that normally takes about 5 seconds took 16m43s on that runner. The job was rerun unchanged and passed on attempt 2 in 25m29s, so all 15 checks are green. No workflow file, timeout, or shard membership was changed.bin/fm-lint.shwas handed tocmd.exe. The pinned linter was run by hand against the pipeline head and was clean, and CI enforces it.Intent
Firstmate cannot supervise unattended on Windows. Continuous supervision does
not hold in this home, so every turn end needs an operator in the loop and no
worker can safely be left running. Fix that.
This is the operational bottleneck: it blocks leaving any other work unattended,
including the other fixes now in flight.
What is already established, measured in this home on 2026-09-17/18 under Git
Bash on Windows 11. Do not re-derive it:
bin/fm-watch.shwithFM_HOMEset, in the foreground - it works correctly: it createsstate/.watch.lockas a proper symlink plus owner directory, holds it, andbeats
state/.last-watcher-beaton a normal cadence. Confirmed alive with a12-second-fresh beacon while the arm path was producing nothing.
normal path, the child does not survive. Symptom set:
state/.watch.lockisnever created, no watcher process is ever visible in a process listing, and
state/.last-watcher-beatadvances exactly once per arm and then stops. Eacharm therefore ends by asking to be re-armed instead of holding a cycle.
hook invokes it, it advances its epoch record, reports
outcome=arming,produces one beat, and exits 2 with
check: rearm-resurface- correctbehaviour given a child that did not survive.
fm_lock_try_createprobe on thisfilesystem produced a correct symlink and owner directory. Despite the surface
resemblance, this is probably NOT the same root cause as the two lock-related
failing suites on this branch.
through 20:18:59 and beacon beats until 20:36:04. This is a change of state,
not a permanent incapacity, and that is a clue worth taking seriously rather
than explaining away.
Consequence while it is broken:
bin/fm-turnend-guard.shcorrectly refuses tolet a turn end blind, and its bounded re-block budget
(
state/.turnend-claude-blocks) reached count=2 at epoch 4 - so the guard stopsprotecting once that budget is spent.
tests/fm-watcher-lock.test.shis one of five unexplained failing suites on thisbranch and asserts exactly this predicate - "restart did not attach to the
verified healthy peer: FAILED - no live supervision with a fresh beacon". Treat
it as a live reproduction of this bug rather than separate work.
What Changed
bin/fm-watch-arm.shnow bounds a freshly forked watcher's confirmation per startup phase (fork, lock, identity, beacon) instead of over the whole cold start: each newly observed phase re-armsFM_ARM_CONFIRM_TIMEOUT, the phase probes use shell builtins only with the forkingfm_watcher_healthyproof gated losslessly behind them, the deadline moved fromdate(1)to$SECONDS, and a timed-out cycle is recorded asreason=confirmation-timeout:<phase>.wait_for_healthy_successorkeeps a single whole window.tests/fm-watcher-lock.test.shadds a per-phase bound case driving a stand-in watcher whose phases each fit one window while their sum does not, asserting teardown and theidentityledger phase for a stalled child in both a clean home and one with a stale leftover beacon; the healthy-peer restart case swaps its native node peer for a TERM-ignoring shell peer (Cygwinkillcannot signal a native process) and widens its wait loop.(FM_ARM_CONFIRM_TIMEOUT + 1) x phase-count(tracked as follow-upfm-adapter-arm-timeout-w1);docs/verification/supervision.mdadds the 2026-09-17 Git Bash/MSYS arm cost measurements.Risk Assessment
✅ Low: The change is a well-bounded rewrite of one confirmation loop whose new logic I traced end to end - the forkless gate is provably lossless against fm_watcher_healthy, the phase order matches what fm-watch.sh actually publishes, the re-arm cannot oscillate because wake() always exits before stdout can outrun the beacon, and the new test discriminates the fix from its predecessor for any platform cost - leaving only residuals the author documents explicitly and one the user already accepted.
Testing
I stood the product up the way an operator does on the affected host - a real FM_HOME driven by bin/fm-watch-arm.sh under Git Bash on Windows 11 - and drove nine scenarios plus the targeted suite cases that cover the confirmation loop this change rewrote. A cold arm confirmed a watcher in 18s, kept the lock and a repeatedly advancing beacon for 90s unattended with no cycle-exit record, a second arm attached rather than duplicating, and an arm over a dead lock holder still confirmed a fresh watcher in 28s. On a genuine primary home the turn-end guard allowed an unattended stop while that supervision was live and still blocked with the "TURN WOULD END BLIND" banner once the watcher was killed, which is the operational bottleneck the intent describes. The regression proof is real red-before/green-after: the new per-phase case passes against this arm and fails against the base-commit arm with the arm killing a progressing child. The bound still holds in the adversarial direction - an 11s stall inside one 9s window produced exit 1, the typed FAILED line, ledger reason=confirmation-timeout:identity, and a reaped child. One pre-existing suite case (immediate-wake propagation) cannot run on this host because bin/fm-check-register.sh refuses custom-check registration under MSYS file-mode validation; it fails identically at the base commit, so it is a host limitation rather than anything this change introduced, and remote CI covers it. There is no UI, HTML, or rendered surface in this change - it is a shell supervision path - so the evidence is CLI transcripts, persisted lock/beacon state, and the lifecycle ledger rather than screenshots. The adapter files in this diff changed only comments (the OpenCode/Pi/omp Windows ready-budget gap was a recorded, declined deferral), so I did not drive those harnesses. Worktree left clean and all test processes and temp homes removed.
FM_HOME=<tmp> bin/fm-watch-arm.shprintedwatcher: started pid=1060332 (beacon fresh)after 18s, state/.watch.lock resolved to its owner dir,ps -pshowed the wat…watcher: started pid=1082207, child alive) and scenario-b-per-phase-case-fixed.txt (`ok…git show 29b5a0d:bin/fm-watch-arm.shreported `not ok - arm did not confirm a child that reached every startup phase inside…watcher: FAILED - no live watcher with a fresh beacon, ledgerreason=confirmation-timeout:identitywithbeacon_age=999999, stalled c…watcher: started pid=1075089, arm 2watcher: attached pid=1075089 (beacon 8s), lock pid unchanged and both arms still livewatcher: started pid=1079630 (beacon fresh)after 28s with a 3s-old beaconbin/fm-turnend-guard.sh --claudeexited 0 while the arm held the cycle; afterkill -9of watcher and arm it exited…ok - watch restart attaches to a verified healthy peer and later surfaces a successor gap(43s). Per the recorded decision this case was failing for a pla…test_arm_waits_for_peer_beacon_after_child_stands_down,test_arm_fails_loud_when_no_fresh_watcher_confirmable, `test_arm_hup_cleans_child_and_temp_out…Evidence: Scenario A - arm cold start holds a live watcher cycle unattended (verdict line, lock symlink, ps, beacon samples)
Source: Scenario A - arm cold start holds a live watcher cycle unattended (verdict line, lock symlink, ps, beacon samples)
Evidence: Scenario B - per-phase regression case against the fixed arm
Source: Scenario B - per-phase regression case against the fixed arm
Evidence: Scenario C - same case against the base-commit arm (fails before the fix)
Source: Scenario C - same case against the base-commit arm (fails before the fix)
not ok - arm did not confirm a child that reached every startup phase inside the window: watcher: FAILED - no live watcher with a fresh beacon (runner exit code: 1, elapsed 41s; arm under test = bin/fm-watch-arm.sh at base commit 29b5a0d)Evidence: Scenario D - the suite case the intent names (restart attaches to healthy peer)
Source: Scenario D - the suite case the intent names (restart attaches to healthy peer)
Evidence: Scenario E - second arm attaches to the live watcher instead of duplicating
Source: Scenario E - second arm attaches to the live watcher instead of duplicating
Evidence: Scenario F - turn end allowed unattended while supervised, blocked when the watcher dies
Source: Scenario F - turn end allowed unattended while supervised, blocked when the watcher dies
$ FM_HOME=$HOME bin/fm-watch-arm.sh watcher: started pid=1077263 (beacon fresh) $ echo <Stop payload> | bin/fm-turnend-guard.sh --claude # supervised turn end exit=0 (0 = turn may end unattended) --- adversarial: kill the watcher, let the beacon go stale, ask again --- exit=2 (2 = guard blocks the turn end) (●) TURN WOULD END BLIND - SUPERVISION IS OFF (●) 1 task(s) in flight, but no live watcher holds this home lock (last beat: 16s ago).Evidence: Scenario G - re-arm over a dead lock holder still confirms a fresh watcher
Source: Scenario G - re-arm over a dead lock holder still confirms a fresh watcher
Evidence: Scenario H - progressing child confirmed vs stalled child torn down, with the lifecycle ledger record
Source: Scenario H - progressing child confirmed vs stalled child torn down, with the lifecycle ledger record
--- H1 progressing child: FM_ARM_CONFIRM_TIMEOUT=8 per phase, 6s in EACH of two phases --- t+18s watcher: started pid=1082207 (beacon fresh) child pid=1082207 still alive: yes --- H2 stalled child: same 8s budget, 11s inside the single identity phase --- t+21s arm exit=1 watcher: FAILED - no live watcher with a fresh beacon lifecycle ledger record: watcher_pid=1082770 exit_code=1 reason=confirmation-timeout:identity beacon_age=999999 stalled child pid=1082770 still alive: noEvidence: Scenario I - other exits of the rewritten confirmation loop (stand-down, typed failure, HUP teardown)
Source: Scenario I - other exits of the rewritten confirmation loop (stand-down, typed failure, HUP teardown)
Pipeline
Updates from git push no-mistakes
... (3 earlier update rounds omitted to keep the PR body within GitHub's 65536-char limit; full history is in the run log.)
🔧 Fix applied.
4 issues (2 warnings, 2 infos) still open:
bin/fm-watch-arm.sh:647- The emitted phase set is {fork, lock, identity, beacon}, but every place that enumerates it lists only three: docs/configuration.md:1105 ("each startup phase (lock, identity, beacon)"), the header comment at bin/fm-watch-arm.sh:75-78, and docs/watcher-continuity.md:104.confirm_phaseis initialised toforkat line 580 and is only reassigned whenphase != fork(line 594), so a child that is forked and stays alive but never claims the lock times out with confirm_phase stillfork: the ledger recordsreason=confirmation-timeout:forkand stdout printswatcher: child pid=N stalled in startup phase fork for 30s. That is not a corner case here - it is the label for the intent's own primary reported symptom ("state/.watch.lock is never created, no watcher process is ever visible"). An operator reading the documented three-phase enumeration and then finding:forkin state/.watch-cycle-exits.log has no definition for it. The user kept these diagnostics on the explicit condition that they be accurate; the value is honest but undocumented. Remedy is documentation only: addfork(forked, lock not yet claimed) to the enumeration in docs/configuration.md:1105, the header comment, and docs/watcher-continuity.md:104.tests/fm-watcher-lock.test.sh:928- The fix round raised the stalled child's beacon delay fromtimeout + 3totimeout * 2(8s -> 32s) on the stated grounds that "the arm tears it down at the bound, so a wide margin there costs no wall time" (comment at lines 869-870). That justification is false. The fixture watcher is a bash script whose TERM trap (trap 'fm_lock_release "$LOCK"; exit 1' HUP INT TERM) cannot run while it is blocked in the foreground externalsleep "$FM_FAKE_WATCH_BEACON_DELAY"- bash defers a trap until the running foreground command completes - and the arm then blocks inwait "$child"at bin/fm-watch-arm.sh:645 until the child actually exits. Concrete trace: identity is observed at t~=0.6s, the phase deadline fires at t~=17.6s and the arm sends TERM, but the child is mid-sleep 32(started t~=0.4s) and does not run its trap until t~=32.4s, so the arm cannot print FAILED or append the ledger record until then. That is ~15s of dead wall time per iteration, x2 iterations of the newfor leftover in absent staleloop, added purely by the oversized margin.timeout + 3(=19) still exceeds the 17.2s deadline with margin while cutting the dead wait to ~1.8s per iteration..opencode/plugins/fm-primary-watch-arm.js:7- docs/configuration.md:1107-1108 was corrected to say the 35s Windows adapter default covers one confirmation window and not the arm's per-phase budget, but the same now-untrue claim survives verbatim in the three adapter sources:.opencode/plugins/fm-primary-watch-arm.js:7-9("35s on Windows so the budget stays above arm's MSYS confirm default (30s in bin/fm-watch-arm.sh): a slow but successful Git Bash cold start must not be SIGTERMed mid-confirmation"),.pi/extensions/fm-primary-pi-watch.ts:145, and.omp/extensions/fm-primary-omp-watch.ts:136. After this change the arm's budget for a progressing child is roughly 3 x 31s on MSYS, so 35000ms no longer stays above it and the stated guarantee no longer holds. Comment-only correction, no behavior change - the user's decision to leave the defaults alone and not make the adapters progress-aware is untouched.tests/fm-watcher-lock.test.sh:472- Informational, for the author's awareness when judging whether the operational bug is closed. The intent names tests/fm-watcher-lock.test.sh's healthy-peer restart case as "a live reproduction of this bug." That case now passes because of a test-fixture correction, not the per-phase bound: the TERM-resistant peer was changed from a node process with a SIGTERM handler tobash -c 'trap "" TERM; ...', and the wait loop was widened 80 -> 400. The reasoning is correct (MSYSkillon a native Windows process falls back to TerminateProcess, so the node peer died, --restart's stop succeeded, and the arm's own child then won the lock and reportedstartedinstead ofattached), and the predicate is preserved. But the confirmation loop this change rewrote plays no role on that path at all - the child stands down with "already running pid <peer>", exits 0, and resolution happens in owned_child_finished -> wait_for_healthy_successor, neither of which the diff touches. So that suite's failure was a platform fixture defect, and its passing is not evidence that the per-phase bound closes the reported arm-layer failure.🔧 Fix applied.
1 warning still open:
bin/fm-watch-arm.sh:602- The new gate's foreign-holder disjunct is[ -n "$lock_pid" ] && [ "$lock_pid" != "$child" ], which is true for a DEAD leftover lock pid, so the ~10-fork health proof still runs on every 0.2s poll through the entireforkphase - the exact process-creation contention this change identifies as the root cause and claims (header, lines 573-579) to have removed. Concrete sequence: a watcher is SIGKILLed or dropped by the host, so its EXIT trap never runs andstate/.watch.locksurvives withpidnaming a now-dead process. At the next turn end bin/fm-claude-stop-autoarm.sh:270 runs the arm;healthy_watcherat line 448 fails onfm_pid_alive(fm-wake-lib.sh:164) and the arm forks a child. In the confirmation looplock_pidis that dead pid on every poll until the child steals the lock, so line 603 callshealthy_watcher->fm_watcher_healthy-> 4xcat+fm_pid_identity(cat,od,tr) +fm_path_age(date, stat) each time. At the measured 5-8s to the lock on MSYS that is 25-40 polls, roughly 250-400 forks, on the same serialized MSYS process-creation path the child is using for its own ~200-fork cold start - comparable in magnitude to the child's whole startup cost, in the recovery case (crashed watcher, leftover lock) the change most needs to protect. Remedy is a forkless pre-filter on the same disjunct:{ [ -n "$lock_pid" ] && [ "$lock_pid" != "$child" ] && fm_pid_alive "$lock_pid"; }.fm_pid_alive(bin/fm-wake-lib.sh:48) is builtin-only (kill -0), andfm_watcher_healthybegins with that identical check on the same pid read from the same file (fm-wake-lib.sh:163-164), so the guard cannot change any outcome - only the fork count. This is an incomplete optimization in new code rather than a regression (the pre-change loop forked on every poll in every phase), which is why it is a warning and not an error.🔧 Fix applied.
3 issues (1 warning, 2 infos) still open:
tests/fm-watcher-lock.test.sh:888- Round 2's fix exceeded the finding that prompted it, and the excess lands on the assertion the case exists to make. The finding asked only to shrink the STALLED child's delay fromtimeout * 2(32) totimeout + 3(19) so the arm's teardown TERM is not swallowed for ~15s. The fixer kept 32 and instead replacedsleep "$DELAY"withphase_delay, an N-iterationsleep 1loop (lines 888-894) that both delays now go through. That does fix the stalled case's dead wall time, but it also rewrites the PROGRESSING case at lines 910-912, which the finding never asked to touch. There,FM_FAKE_WATCH_IDENTITY_DELAY=11andFM_FAKE_WATCH_BEACON_DELAY=11now spawn 11 externalsleepprocesses each instead of one, so the two phases the pass assertion depends on carry 22 extra process creations. Concrete failure: the assertion at line 921 requires each phase to finish insidetimeout=16(+1 rounding second), i.e. a 6s margin over the 11s delay. On Git Bash/MSYS the change's own header (bin/fm-watch-arm.sh:81-82) prices process creation at ~70ms, so 11 spawns cost ~0.8s idle; this same test file already records (lines 761-763) a measured ~5x inflation of process-creation-dominated work at 3x CPU oversubscription, which puts the added cost at ~4s against a 6s margin and flips line 921 toarm did not confirm a child that reached every startup phase inside the window. Those 22 spawns also sit on the serialized MSYS process-creation path that this very case is trying to characterize, so they perturb the measurement. The defect lives in fix-round machinery that went past the required remedy, so the smallest honest fix is to revert this fixture to the minimal prescribed form -FM_FAKE_WATCH_BEACON_DELAY=$((timeout + 3))with a plainsleep "$DELAY"for both phases - which still exceeds the ~17.2s deadline (bounding the swallowed-TERM wait to ~1.8s, exactly as the original finding computed) while leaving the progressing case's timing and fork budget untouched.bin/fm-watch-arm.sh:30- This change edited line 30 to read "It prints exactly one unambiguous status line:" followed by the enumeration at lines 31-37, and then added a second unconditionalwatcher:-prefixed line at line 646 on the timeout path (watcher: child pid=<N> stalled in startup phase <phase> for <T>s, emitted immediately beforewatcher: FAILED - no live watcher with a fresh beacon). Both lines are relayed together: bin/fm-claude-stop-autoarm.sh:369 greps^(watcher:|signal:|stale:|check:|heartbeat)into the operator failure notice, so an operator does see twowatcher:lines from one timed-out arm. Round 2 corrected the phase enumeration in docs/configuration.md:1105, docs/watcher-continuity.md:104-105, and the header's FM_ARM_CONFIRM_TIMEOUT comment, but left this claim, which the same change had just rewritten. The user kept these diagnostics on the condition that the surrounding statements stay accurate. Remedy is comment-only: amend line 30 and the enumeration at lines 31-37 so the stall line is listed as the diagnostic that precedes the FAILED verdict, or drop the "exactly one" wording. No behavior change.bin/fm-watch-arm.sh:598- Informational, for awareness of a robustness narrowing round 2 (commit 28c627e) introduced while fixing the beacon-phase label. The author's original code used[ -e "$BEAT" ] && phase=beacon, so in the common warm home - any home supervised before, whose inherited beacon is still inside GRACE=300 - the child went to phasebeaconthe moment identity appeared andhealthy_watcherthen ran on EVERY 0.2s poll. The fix correctly re-anchored the label to[ "$BEAT" -nt "$child_out" ], but that makes-ntfalse for an inherited beacon, so the full proof in a warm home now runs exactly once, on the single poll whereentered_identityis set at line 598. Traced against the measured MSYS numbers: the child claims the lock and publishes identity at +8.1s, the arm's one proof fires at ~+8.2s and normally confirms off the inherited beacon (same confirmation time as before). If that single proof fails -fm_watcher_lock_matches_pid->fm_pid_identity(bin/fm-wake-lib.sh:150) forkscatandod, and a fork failure under the MSYS process-creation contention this change exists to relieve is exactly the load condition in play - there is no retry inside the identity phase, and confirmation defers to the child's own first beat at ~+17.7s. It stays bounded and self-heals (the deadline was re-armed at the identity transition and the beacon phase then proves on every poll), so no wrong result and no unbounded wait; the cost is that a warm-home confirmation can silently slip ~9.5s on the platform the change targets. Any remedy trades directly against the fork budget that is the change's stated purpose, so this is noted rather than prescribed.🔧 Fix applied.
1 warning still open:
.opencode/plugins/fm-primary-watch-arm.js:13- The change converted the arm's confirmation budget from one whole-cold-start window into a PER-PHASE window (bin/fm-watch-arm.sh:588-645: each newly observed startup step re-armsconfirm_deadline = SECONDS + CONFIRM_TIMEOUT + 1), so the arm's total confirmation time on MSYS grows from ~31s to up to 4x31 = ~124s. The three harness adapters kept their 35000ms Windows ready budget and only had their comments rewritten. Before this change that constant carried an explicit derivation - "35s on Windows so the budget stays above arm's MSYS confirm default (30s in bin/fm-watch-arm.sh): a slow but successful Git Bash cold start must not be SIGTERMed mid-confirmation" - and that invariant is now false by construction; the new comment states the violation rather than repairing it.Concrete reachable sequence, OpenCode primary on Windows 11 / Git Bash: (1) the session stops and the plugin spawns bin/fm-watch-arm.sh, awaiting readiness behind
setTimeout(() => resolve("timeout"), ARM_READY_TIMEOUT_MS)(fm-primary-watch-arm.js:43); (2) the change's own measurements (docs/verification/supervision.md "Git Bash/MSYS arm confirmation cost, 2026-09-17") put an arm-launched child at lock +8.1s, first beacon +17.7s,watcher: started+19.9s on an IDLE host, and the change's rationale (bin/fm-watch-arm.sh:568-571) is that ordinary contention consumed the ~10s of headroom inside the old 31s window; (3) a contended start of 36s+ - squarely inside the new per-phase budget and exactly the case this change exists to permit - trips the adapter timer first; (4)retireArmrunsarmChild.kill("SIGTERM")(fm-primary-watch-arm.js:272-274), the arm's TERM trap TERMs and reaps the child (bin/fm-watch-arm.sh:480-482), and the beacon has advanced exactly once with no watcher left - the precise symptom set the User intent describes; (5) the plugin then retries up to REARM_RETRY_LIMIT=5 times, each retry paying the same cold start and dying the same way. .pi/extensions/fm-primary-pi-watch.ts:148-151 with retireArm at 894-897 and .omp/extensions/fm-primary-omp-watch.ts:823-826 are the same shape.Net effect: the change's stated guarantee (bin/fm-watch-arm.sh:26-30, "a slow platform cannot make this arm tear down a child that is still coming up") holds for the arm itself, but on the OpenCode/Pi/omp Windows paths a slow platform still gets the child torn down - by the adapter instead. This is not a silent regression (the adapter surfaces a typed restoration failure), and the Claude Stop-hook path that the intent measures is unaffected: bin/fm-claude-stop-autoarm.sh:270 runs the arm in the foreground with no per-attempt bound, relying on the hook's multi-hour timeout. Worth noting for that path: two AUTOARM_ATTEMPTS can now serialize up to ~250s of turn-end wait on MSYS in the failing case, up from ~62s - loud and bounded, not a defect, but a latency change an operator will feel.
The smallest honest remedy is to restore the derivation the change removed - raise the three Windows adapter defaults above the arm's worst-case per-phase total, or derive them from FM_ARM_CONFIRM_TIMEOUT times the phase count rather than from a single window. That changes adapter behavior and introduces a new shared constant, i.e. it EXTENDS this change rather than correcting what it already does, so the remedy - not the defect - is what needs authorization.
🔧 Fix applied.
1 warning still open:
bin/fm-watch-arm.sh:29- The header states an absolute guarantee the code does not provide: "That confirmation is bounded per startup PHASE by FM_ARM_CONFIRM_TIMEOUT ..., never over the whole cold start, so a slow platform cannot make this arm tear down a child that is still coming up" (bin/fm-watch-arm.sh:26-30). docs/watcher-continuity.md:104 repeats it as "so a slow platform never has a progressing child torn down while a stall inside one phase still fails loudly". The bound is still wall-clock; it is only re-armed per observed step (line 602-606), so a child that is genuinely progressing but needs more than CONFIRM_TIMEOUT+1 seconds inside ONE step is still TERMed at line 652 and reported as a stall.Concrete reachable sequence on the platform this change targets: the change's own measurement (docs/verification/supervision.md, "Git Bash/MSYS arm confirmation cost, 2026-09-17") puts an arm-launched child at lock pid +8.1s and first beacon +17.7s on an IDLE host, i.e. the identity->beacon step alone costs ~9.6s against a 31s window. The stated rationale for this whole change (bin/fm-watch-arm.sh:568-571) is that ordinary contention on this same host already consumed ~10s of headroom. A contention factor above ~3.2x on that one step - larger than the ~1.75x that produced the reported failure, but the same kind of event - runs the beacon window out at line 643, the arm prints the stall diagnostic and "watcher: FAILED - no live watcher with a fresh beacon", and cleanup_child TERMs a child that was coming up normally. That is precisely the outcome the sentence says cannot happen.
This is a comment/doc accuracy defect, not a behavior defect: keeping a wall-clock bound so a real stall fails loudly is a deliberate and reasonable trade, and the wider per-phase margin plus the loop's reduced fork cost are the actual fix. But an operator sizing FM_ARM_CONFIRM_TIMEOUT for a slow host reads this header as the contract and will not know a single slow phase is still fatal.
Remedy is text-only, no behavior change: restate both places as the property the code actually holds - a child that reaches each next startup step within FM_ARM_CONFIRM_TIMEOUT is never torn down, and only a child that spends a whole window inside one step is - and drop the "cannot"/"never" absolutes.
🔧 Fix applied.
3 warnings still open:
tests/fm-watcher-lock.test.sh:472- The User intent names one specific assertion as the live reproduction of this bug: "tests/fm-watcher-lock.test.sh is one of five unexplained failing suites on this branch and asserts exactly this predicate - 'restart did not attach to the verified healthy peer: FAILED - no live supervision with a fresh beacon'. Treat it as a live reproduction of this bug rather than separate work." That is test_watch_restart_attaches_to_healthy_peer (fail message at tests/fm-watcher-lock.test.sh:502). The change does not make that case pass with the production fix; it makes it pass with two test-side edits: the peer is rewritten from a node process with a SIGTERM handler tobash -c 'trap "" TERM; ...'(line 472), and its attach wait is raised from 80 to 400 polls, 8s to 40s (line 497).Both edits are independently necessary, which is the point: with the base arm code and only the production fix applied, the node peer is still terminated outright by Cygwin's kill, --restart's stop still succeeds, the arm still forks its own child and prints
watcher: started pid=<child>rather thanwatcher: attached pid=$peer, and the 8s loop still expires well before the ~20s MSYS cold start measured in docs/verification/supervision.md. So the assertion the intent designated as the repro is resolved as a platform-specific fixture defect plus an undersized wait, not by the per-phase bound in bin/fm-watch-arm.sh. The genuine regression proof lives in the newly added test_arm_confirmation_is_bounded_per_startup_phase instead.The author's re-diagnosis is plausible and specifically evidenced (Cygwin cannot deliver a signal to a native Windows process, so a TERM-ignoring shell peer is the only portable way to express "resistant peer"), and the suite is still where the regression proof was added. But the intent explicitly directed that this assertion be treated as a reproduction of the arm bug rather than separate work, and the change instead reclassifies it. Confirm the reclassification, or say which failing assertion should stand as the bug's reproduction. This is a judgement about the author's deliberate framing, so it is not something to resolve silently.
bin/fm-watch-arm.sh:277- Comment/doc accuracy, no behavior change. The change converted a fresh child's confirmation from one window into a re-armable per-phase budget, but three descriptions of FM_ARM_CONFIRM_TIMEOUT still describe the old single-window model or now describe only half of the constant's job:bin/fm-watch-arm.sh:277 - "Give a successor the same bounded confirmation window used for a fresh child." This is now false. wait_for_healthy_successor (line 280-289) still spends exactly one flat CONFIRM_TIMEOUT+1 window, while a fresh child can now legitimately consume up to four of them (fork, lock, identity, beacon). An operator tuning FM_ARM_CONFIRM_TIMEOUT reads this line as an equivalence that no longer holds; it is the same class of stale-contract statement the previous round corrected in the file header.
bin/fm-watch-arm.sh:84-88 - the constant's own comment now defines it purely as "How long a freshly forked watcher may spend inside ONE startup phase", which omits its second live use as the whole-window successor budget at line 284. The constant has two semantics after this change and the declaration states only one.
docs/configuration.md:1105 - same omission: "seconds fm-watch-arm allows a fresh watcher in each startup phase (fork, lock, identity, beacon) before reporting FAILED". The pre-change text ("seconds fm-watch-arm waits to confirm a fresh watcher before reporting FAILED") loosely covered both uses; the new text covers only the fresh-child path, so the successor-confirmation wait is now undocumented.
Remedy is text-only: restate line 277 as the successor getting a single CONFIRM_TIMEOUT window (not the same bound a fresh child gets), and add to the line 84 comment and the docs/configuration.md entry that the same value also bounds wait_for_healthy_successor as one whole window. Change no value, default, constant, or logic.
bin/fm-watch-arm.sh:655- Simplification pass. The change introduces a phase-naming observability component that no intent requirement needs: the stdout diagnosticwatcher: child pid=$child stalled in startup phase $confirm_phase for ${CONFIRM_TIMEOUT}s(bin/fm-watch-arm.sh:655), theconfirmation-timeout:$confirm_phaseledger reason suffix (line 659), and the supporting text in the file header (lines 34-38, 87-89), docs/watcher-continuity.md:107, and thestalled in startup phase identity/reason=confirmation-timeout:identityassertions in the new test (tests/fm-watcher-lock.test.sh:940-943).The stated goal is "Firstmate cannot supervise unattended on Windows ... Fix that." Unattended supervision is restored entirely by the per-phase budget and the reduced-fork poll loop; naming which phase a failed child stalled in changes no supervision outcome, no exit code, and no adapter decision (the three extensions match only /^watcher: (?:started|attached)\b/, /^watcher: FAILED/ and /^watcher: healthy\b/, and nothing outside docs and this test reads the ledger reason). Removing the diagnostic line and reverting the ledger reason to plain
confirmation-timeoutwould satisfy the intent with strictly less surface, and would also drop the internalconfirm_phasetracking from the timeout path.This is operator-visible output the author added deliberately and documented in three places, so the call belongs to the author rather than to a refactor: keep it if the phase label is wanted as diagnostics for the Windows bottleneck, or remove it as work the intent does not ask for. If it is kept, the narrower form is the ledger suffix alone, since the auto-arm operator notice (bin/fm-claude-stop-autoarm.sh:369) already replays ledger-matching lines and the notice's own first line already states the verdict.
🔧 Fix applied.
3 warnings still open:
.opencode/plugins/fm-primary-watch-arm.js:18- The change makes the arm's confirmation legitimately outlast the harness adapters' ready budget, and the adapters then kill the very child the change exists to protect. Concrete sequence on Git Bash/MSYS with an OpenCode (or Pi, or omp) primary: the arm forks a watcher that progresses fork->lock->identity->beacon in ~45s, each phase inside its own 31s window, so the arm is deliberately silent and prints no verdict line. At 35s ARM_READY_TIMEOUT_MS fires (.opencode/plugins/fm-primary-watch-arm.js:49 resolves "timeout"), ensureArm returns a non-armed status, and line 307 calls retireArm -> armChild.kill("SIGTERM") (line 278). The arm's own TERM trap (bin/fm-watch-arm.sh:491 -> handle_arm_signal) kills the progressing watcher child. The loop then retries up to REARM_RETRY_LIMIT=5, each retry paying the same cold start and being killed at 35s again, ending at setArmStatus("failed") with no watcher running. .pi/extensions/fm-primary-pi-watch.ts:154/862/871 and .omp/extensions/fm-primary-omp-watch.ts have the identical shape.This is a behavior change introduced by this diff, not pre-existing: before it, the arm's worst case was one 30s window + 1 rounding second = 31s < 35s, so the adapter always received a typed verdict line before its budget expired and never tore the arm down mid-progress. After it, the worst case is (30+1) x 4 = ~124s, exactly as the new comment at lines 7-17 states. So for the three extension-owned harnesses on Windows the change does not restore unattended supervision in the slow-start window it targets; it converts a typed
watcher: FAILEDinto an adapter-initiated SIGTERM-and-retry loop that never converges.The change documents the gap rather than closing it ("closing the gap is tracked as separate follow-up work"), with no issue or tracking reference. The intent's own path is unaffected - bin/fm-claude-stop-autoarm.sh:270 runs the arm in the Stop hook's foreground with a multi-hour timeout and no confirmation budget - so this is not a blocker for the Claude home the intent describes. The remedy is a user-facing default change (derive the win32 ready budget from (FM_ARM_CONFIRM_TIMEOUT + 1) x 4, i.e. ~130000ms, in all three adapters and docs/configuration.md:1106-1107), which extends the change beyond its stated scope, so the deferral needs the author's confirmation rather than a silent fix here. Confirm that shipping the documented gap for Pi/OpenCode/omp on Windows is intended, or authorize re-deriving the three budgets.
tests/fm-watcher-lock.test.sh:875- The change's only genuine regression proof is tuned with ~6s of slack per phase, which the suite's own documented load profile can exceed, turning the proof into a flaky test that fails the change rather than the code.With timeout=16 and delay=11, the progressing case requires each phase to finish inside timeout+1 = 17s, so the platform's own per-phase process-creation cost must stay under 17-11 = 6s. The test's comment records that cost as "~3s to reach the lock and ~1s per later phase" on an idle MSYS host. tests/fm-watcher-lock.test.sh:761-765 documents that this same suite measures 1.9-2.3s idle inflating to 9.1-13.1s at 3x CPU oversubscription - a 4-6x factor. Applying that to the 3s lock cost gives 12-18s, which alone breaks the 17s window: the arm legitimately tears down the fixture child and the case fails at line 918 ("arm did not confirm a child that reached every startup phase inside the window"). The stall case has the same exposure in the other direction: if the lock+identity steps cost more than 17s the recorded phase is
lockorfork, and thereason=confirmation-timeout:identityassertion at line 941 fails. The 400-poll (40s) wait at line 913 is a third margin against a ~25s expected runtime.Unlike the neighbouring cases, this one cannot take the deadline out of the assertion - the deadline IS the predicate - so the only lever is the ratio. The constraint set is: 2*delay > timeout+1 (so a whole-start bound still fails the child) and (timeout+1) - delay > worst per-phase platform cost. Headroom is capped at (timeout+1)/2, so doubling the slack to ~12s needs timeout=24 with delay=13 (26 > 25, headroom 12), at the cost of raising the stalled sub-case delay to 27s and the case's total runtime to roughly 85s. That is a deliberate runtime-versus-stability trade on constants the author chose with stated measurements, so it is the author's call rather than a mechanical retune.
bin/fm-watch-arm.sh:602- Simplification pass. Beyond the per-phase budget the intent requires, the change introduces a second, separable component:entered_identity(bin/fm-watch-arm.sh:602-607) schedules the forkingfm_watcher_healthyproof as a side effect of the phase-transition re-arm branch, so during theidentityphase the proof runs on exactly ONE poll and never again until the beacon appears.The fork-reduction goal the change justifies with measurement (docs/verification/supervision.md: the arm's ten-fork poll iteration cost the child ~3s to the lock and ~6s to the first beat on MSYS) is fully served by a LOSSLESS gate, which is the narrower form: run the proof whenever
lock_pidis non-empty andfm_pid_alive "$lock_pid"and the lock either names a foreign pid or names our child withpid-identitypublished. In every other statefm_watcher_healthyprovably returns 1 (empty or dead lock pid failsfm_pid_alive; our child with no identity fails the[ -n "$lock_identity" ]check infm_watcher_lock_matches_pid, bin/fm-wake-lib.sh:149), so skipping it there drops nothing.entered_identitygoes further and also suppresses the proof on repeat polls insideidentity, which is the one lossy part: it narrows the pre-existing inherited-beacon confirmation (a beacon from a previous cycle still inside GRACE, which used to be re-checked every 0.2s) to a single sampling instant, so a proof that fails transiently at that one poll - e.g.fm_pid_identity'sod/psfork losing under exactly the MSYS contention this change is about - is never retried and the arm can spend the whole identity window and report FAILED where it previously reported started.It also couples the proof schedule to phase-transition detection in a way that is not stated anywhere: a
beacon->identityregression re-fires the proof only because it happens to pass through the same re-arm branch. Droppingentered_identityin favour of the lossless predicate removes the variable, decouples proof scheduling from re-arming, and restores the old retry behaviour; the cost is re-paying the proof's forks on each poll during the identity phase (measured at 6-12s on MSYS, so roughly 30-60 polls), which is the trade the author may have intended. No intent requirement needs the one-shot form; keep it deliberately or take the narrower lossless gate.🔧 Fix applied.
1 info still open:
bin/fm-watch-arm.sh:617- Informational, no action needed. The re-arm condition deliberately excludes theforkphase ([ "$phase" != fork ]), so the child's entire pre-lock cold start - bash startup, sourcing fm-wake-lib.sh, the pre-lock migration, and any stale-lock steal via fm_lock_try_acquire's.stealsub-lock - is covered by the single initial window, while every later step gets its own. That step is also the largest one the change measured: docs/verification/supervision.md records the arm-launched lock at +8.1s of a ~19.9s start, so the fork phase is roughly 40% of the cold start against 1 of the 4 available windows. If a contended host pushes that step past one window, the arm still tears down a progressing child and recordsreason=confirmation-timeout:fork, which is the same class of failure the change exists to stop - just with about 3.8x headroom instead of the 1.55x the pre-fix whole-start bound had. This is not a hidden defect: bin/fm-watch-arm.sh:568-572 states the bound is still wall-clock per phase and that a child stalling a whole window inside one step is torn down. Subdividing the pre-lock start (or removing the wall-clock bound there) would extend the change beyond its stated scope, so recording it rather than repairing it is the right call.✅ **Test** - passed
✅ No issues found.
FM_HOME=<tmp> bin/fm-watch-arm.shprintedwatcher: started pid=1060332 (beacon fresh)after 18s, state/.watch.lock resolved to its owner dir,ps -pshowed the wat…watcher: started pid=1082207, child alive) and scenario-b-per-phase-case-fixed.txt (`ok…git show 29b5a0d:bin/fm-watch-arm.shreported `not ok - arm did not confirm a child that reached every startup phase inside…watcher: FAILED - no live watcher with a fresh beacon, ledgerreason=confirmation-timeout:identitywithbeacon_age=999999, stalled c…watcher: started pid=1075089, arm 2watcher: attached pid=1075089 (beacon 8s), lock pid unchanged and both arms still livewatcher: started pid=1079630 (beacon fresh)after 28s with a 3s-old beaconbin/fm-turnend-guard.sh --claudeexited 0 while the arm held the cycle; afterkill -9of watcher and arm it exited…ok - watch restart attaches to a verified healthy peer and later surfaces a successor gap(43s). Per the recorded decision this case was failing for a pla…test_arm_waits_for_peer_beacon_after_child_stands_down,test_arm_fails_loud_when_no_fresh_watcher_confirmable, `test_arm_hup_cleans_child_and_temp_out…Manual live run:FM_HOME=<tmp> bin/fm-watch-arm.shon Git Bash (OSTYPE=cygwin), verdict line +state/.watch.locksymlink +ps -p <child>+ 8 beacon mtime samples over 90sManual live run: secondbin/fm-watch-arm.shagainst the same supervised home (attach path, lock pid unchanged)Manual live run:kill -9the watcher, then re-arm over the leftover dead lock holderManual live run: real primary home (git init+ AGENTS.md + bin/ +state/t1.meta),printf '{"session_id":...}' | bin/fm-turnend-guard.sh --claudewith supervision live (exit 0) and after killing the watcher withFM_GUARD_GRACE=1(exit 2 + block banner)Manual live transcript: realbin/fm-watch-arm.sh+ stand-in watcher,FM_ARM_CONFIRM_TIMEOUT=8with 6s+6s progressing phases (started) and 11s single-phase stall (exit 1,reason=confirmation-timeout:identity, child reaped)test_arm_confirmation_is_bounded_per_startup_phasefrom tests/fm-watcher-lock.test.sh (fixed arm: ok, 119s)test_arm_confirmation_is_bounded_per_startup_phasewith WATCH_ARM pointed atgit show 29b5a0d:bin/fm-watch-arm.sh(pre-fix arm: not ok - red-before proof)test_watch_restart_attaches_to_healthy_peer(the suite case the intent names)test_arm_waits_for_peer_beacon_after_child_stands_down,test_arm_fails_loud_when_no_fresh_watcher_confirmable,test_arm_hup_cleans_child_and_temp_outputtest_arm_propagates_immediate_wake_before_confirmationattempted on both the target and base commit (blocked identically bybin/fm-check-register.shon MSYS)docs/configuration.md:1108- Judgment call left unresolved:FM_OMP_ARM_READY_TIMEOUT_MS(.omp/extensions/fm-primary-omp-watch.ts:146) has never been documented in docs/configuration.md, which is the authoritative owner of the environment-variable reference. This change edited that adapter's comment to record the same per-phase ready-budget gap it recorded for OpenCode and Pi, and updated the configuration.md entries forFM_OPENCODE_ARM_READY_TIMEOUT_MSandFM_PI_ARM_READY_TIMEOUT_MSto state the gap, so the operator reference now names the gap for two of the three affected adapters and is silent about the third. The omission is pre-existing rather than introduced here, so I did not add a new entry under the scope rule (only documentation this change made stale). Worth a follow-up alongside fm-adapter-arm-timeout-w1, which is the task that will re-derive all three budgets and would naturally document the omp knob in the same place.⏭️ **Lint** - skipped
✅ **Push** - passed
✅ No issues found.