Repository navigation
Conversation
On a heavily loaded host a freshly forked watcher can need longer than the arm's confirmation window (10s, 30s on MSYS) to take its lock and beat. The arm then TERMed that live child and reported confirmation-timeout, and every retry repeated the same cold start, so the Claude Stop-hook auto-arm failed after its bounded attempts and supervision stayed down. A child still consuming CPU when a window ends is now granted another window, up to FM_GUARD_GRACE after its start. A window with no CPU progress still fails as before, and a signal still bounds its wait by the first window, so owners that stop an unready arm see it exit promptly. FM_ARM_CONFIRM_TIMEOUT keeps setting the window length.
 [Medium impact] The PR appears safe to merge, with a non-blocking false-warning issue for timeouts set only in Reviews (3) · Last reviewed commit: "no-mistakes(review): Wait out the arm's ..." · Reviewed by Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
…ow stops at FM_GUARD_GRACE, even when the grace is shorter than FM_ARM_CONFIRM_TIMEOUT. The new test failed before the fix and passes after it. The full test file did not finish: it was stopped at the 10-minute limit during its last test, after 44 tests passed and none failed. Invariant: every confirmation window must end no later than confirm_limit (confirm_start + GRACE). The window end is set in two places. owned_child_progressed already capped the later windows; the first window was not capped. - **Fix (bin/fm-watch-arm.sh):** confirm_limit is now set first, and the first window_end is capped at it. The signal-path `deadline` used by handle_arm_signal is unchanged. - **Test (tests/fm-watch-arm.test.sh):** added test_arm_bounds_the_first_confirmation_window_by_a_shorter_grace next to the existing grace-bound test. It uses start_slowed_arm with a spinning watcher start, FM_ARM_CONFIRM_TIMEOUT=30 and FM_GUARD_GRACE=3. It checks that the arm ends within 20 seconds of wall-clock time, prints the typed confirmation failure, and writes a reason=confirmation-timeout ledger row. - **Why wall-clock:** a limit on the number of polls let the test pass even before the fix. The spinning child slows each poll, so 150 polls took about 32 seconds. Results: - **Before the fix:** the new test failed after 31 seconds. - **After the fix:** it passed in about 6 seconds. - **Full file:** stopped by the time limit during its last test, so it never completed. Before that, 44 tests passed and none failed. - **shellcheck:** reports only SC1091 info about sourced files and an SC2034 warning at tests/fm-watch-arm.test.sh:1824, in code this round did not touch. ci-1 and ci-2 were left unchanged as instructed: they are fork workflow runs waiting for maintainer approval
|
Speaking as Kun's firstmate: thank you @kova1max, the load-20 Mac failure is a real gap, and the base-vs-target transcript makes it easy to see. I read the diff and approved the fork CI runs for Overlap: #7835 (@WTF-Am-ID, for #7791) attacks the same slow-start problem, including this initial arm confirmation. It uses a shared milestone-progress allowance (25s without progress, 100s cap) across the host successor, hand-back and arm. This PR uses CPU time as the progress signal, capped at Notes for the decision: with defaults, a progressing start can now hold the confirmation up to contract-class: new-default (it changes the unconfigured confirmation and Stop-hook wait). Once CI is green, this goes to the captain as a decision, not to you. |
Capping the first confirmation window at FM_GUARD_GRACE made a grace shorter than FM_ARM_CONFIRM_TIMEOUT end the window early, so with a grace of 0 the arm gave up before a watcher's immediate wake (test_arm_propagates_immediate_wake_before_confirmation). The grace is the beacon-freshness threshold; the first window stays the full FM_ARM_CONFIRM_TIMEOUT budget, and the grace caps only the further windows a still-progressing start earns.
| case "${OSTYPE:-}" in | ||
| msys*|mingw*|cygwin*) window=${FM_ARM_CONFIRM_TIMEOUT:-30} ;; | ||
| *) window=${FM_ARM_CONFIRM_TIMEOUT:-10} ;; | ||
| esac | ||
| case "$window" in ''|*[!0-9]*) window=30 ;; esac | ||
| [ "$(( window + 1 ))" -le "$budget" ] || budget=$(( window + 1 )) |
There was a problem hiding this comment.
start_handling_successor reads FM_ARM_CONFIRM_TIMEOUT from the hook’s environment, but the real arm also reads config/watch.env. With FM_GUARD_GRACE=5 in the environment and FM_ARM_CONFIRM_TIMEOUT=30 only in watch.env, the hook waits about 16 seconds while the arm now allows its full 31-second first window. A successor that confirms after 20 seconds gets reported as unconfirmed even though its start is valid. The detached watcher can still finish starting, so this produces a false warning rather than stopping it.
Read the same effective timeout as the arm before calculating this wait, and cover file-based settings in the test.
Intent
"do all and create mrs to the upstream" - send each of these firstmate improvements upstream to kunchenguid/firstmate as its own PR from the public fork kova1max/firstmate.
This one: on a heavily loaded Mac (load 20+ on 10 cores) the Stop-hook watcher auto-arm failed with reason confirmation-timeout after 2 attempts because bin/fm-watch-arm.sh waits only FM_ARM_CONFIRM_TIMEOUT=10 s (30 on MSYS) for a fresh watcher beacon; supervision stayed down until a home set FM_ARM_CONFIRM_TIMEOUT=30 in config/watch.env.
What Changed
bin/fm-watch-arm.shno longer gives up on a fresh watcher after oneFM_ARM_CONFIRM_TIMEOUTwindow. If the watcher used CPU during that window, the arm opens another window (CPU time is read from/proc/<pid>/stat, or frompswhere/procis missing). This repeats untilFM_GUARD_GRACEafter the start. A watcher that used no CPU during a window, or one still unconfirmed when the grace runs out, still fails withconfirmation-timeout. The first window always gets its full budget, even when the grace is shorter. Only that first window bounds how long a signal waits.bin/fm-claude-stop-autoarm.shnow waits longer for the Claude Stop hook's handling successor to report. The limit is whichever is larger:GRACEor the arm's full first window (FM_ARM_CONFIRM_TIMEOUT+ 1), plus 5 s of startup slack. The wait also stops as soon as the successor exits, and counts it as confirmed if it already printed astarted/attachedline.New tests in
tests/fm-watch-arm.test.shandtests/fm-claude-stop-autoarm.test.shcover:docs/configuration.mdanddocs/watcher-continuity.mddescribe the new windows. They also note that the OpenCode and Pi ready timeouts are still fixed; waiting out the extended windows there is a possible follow-up. Known follow-up: the Claude Stop hook readsFM_GUARD_GRACEandFM_ARM_CONFIRM_TIMEOUTonly from its own environment, while the arm also loadsconfig/watch.env, so a larger value set only inwatch.envcan still let the arm keep confirming after the hook stops waiting; this predates this change and is left out of this PR.🤖 Generated with Claude Code
Risk Assessment
Testing
In a marked lab home I drove the real arm and a real watcher from the base and target commits. With a 16s busy watcher start and the default 10s window, the base commit failed with confirmation-timeout and the target confirmed
watcher: started. Under real load (40 CPU burners, load average 27-50) with the default window, both base and target confirmed (base after 4s, target after 7s), so plain load on this host did not reproduce the original failure. With the window cut to 1s, base failed and target confirmed in both runs. The guards held live: a stalled start fails at the first window, a start that never stops spinning is cut off at FM_GUARD_GRACE, a grace shorter than the window keeps the full first window and gives no extension, and a TERM during an extended window exits within 1s, stops the watcher and records reason=arm-interrupted. The Claude Stop-hook successor wait was checked only through its full behavior test file (all 66 tests pass), not with a live Claude primary, so that scenario is untested live. The lab home, base copy and driver files were removed afterwards.watcher: started), not killed with confirmation-timeoutwatcher: startedat 21s with no ledger rowEvidence: Slow watcher start, base vs target (default 10s window)
Source: Slow watcher start, base vs target (default 10s window)
Evidence: Real CPU load (40 burners), default window, base vs target
Source: Real CPU load (40 burners), default window, base vs target
Evidence: Real CPU load with 1s window, base vs target x2
Source: Real CPU load with 1s window, base vs target x2
Evidence: Guard scenarios: stalled, grace cap, short grace
Source: Guard scenarios: stalled, grace cap, short grace
Evidence: TERM during extended window
Source: TERM during extended window
Evidence: Claude stop-hook autoarm test run
Source: Claude stop-hook autoarm test run
Pipeline
Updates from git push no-mistakes
✅ **intent** - passed
✅ No issues found.
⏭️ **Rebase** - skipped
Step was skipped.
bin/fm-claude-stop-autoarm.sh:371- start_handling_successor now bounds its wait by GRACE+5 alone, but fm-watch-arm.sh (after ba7245d) never shortens the first confirmation window: it is always FM_ARM_CONFIRM_TIMEOUT+1 even when FM_GUARD_GRACE is smaller, and docs/configuration.md now documents that configuration as supported. Example: FM_GUARD_GRACE=10 and FM_ARM_CONFIRM_TIMEOUT=30 (the value the intent's home put in config/watch.env). The hook gives up at about 15s and adds the 'did not confirm a live watcher' banner, even though the successor arm is still validly inside its first 31s window and may print 'watcher: started' later. Before this change the hook waited FM_ARM_CONFIRM_TIMEOUT+2, so this setup worked. Fix: set the budget to max(GRACE, FM_ARM_CONFIRM_TIMEOUT+1) plus slack, matching the arm's rule that the first window is the full budget and GRACE caps only the extensions. The comment at bin/fm-claude-stop-autoarm.sh:363-366 and docs/watcher-continuity.md:175 describe the bound as GRACE-only and need the same correction.🔧 Fix applied.
2 warnings still open:
bin/fm-claude-stop-autoarm.sh:371- start_handling_successor now bounds its wait by GRACE+5 alone, but fm-watch-arm.sh (after ba7245d) never shortens the first confirmation window: it is always FM_ARM_CONFIRM_TIMEOUT+1 even when FM_GUARD_GRACE is smaller, and docs/configuration.md now documents that configuration as supported. Example: FM_GUARD_GRACE=10 and FM_ARM_CONFIRM_TIMEOUT=30 (the value the intent's home put in config/watch.env). The hook gives up at about 15s and adds the 'did not confirm a live watcher' banner, even though the successor arm is still validly inside its first 31s window and may print 'watcher: started' later. Before this change the hook waited FM_ARM_CONFIRM_TIMEOUT+2, so this setup worked. Fix: set the budget to max(GRACE, FM_ARM_CONFIRM_TIMEOUT+1) plus slack, matching the arm's rule that the first window is the full budget and GRACE caps only the extensions. The comment at bin/fm-claude-stop-autoarm.sh:363-366 and docs/watcher-continuity.md:175 describe the bound as GRACE-only and need the same correction.bin/fm-claude-stop-autoarm.sh:371- start_handling_successor works out its wait budget from GRACE (hook line 168) and FM_ARM_CONFIRM_TIMEOUT, but it reads both only from the hook's environment. The arm sources $CONFIG/watch.env (bin/fm-watch-arm.sh:141) after it receives FM_GUARD_GRACE="$GRACE", so a value in watch.env overrides what the hook passed. Concrete case: a home sets FM_GUARD_GRACE=900 in config/watch.env, which is the documented place for it (docs/configuration.md:3060). The hook's GRACE stays at the poll-derived 300, so it waits about 305s. The arm keeps granting windows to a progressing watcher until 900s, so the hook reports 'did not confirm a live watcher' while the arm is still validly confirming. The same applies to FM_ARM_CONFIRM_TIMEOUT above about 299 in watch.env. This mismatch predates this round: the base FM_ARM_CONFIRM_TIMEOUT+2 bound ignored watch.env too, and this round's max(GRACE, window+1) actually fixes the intent's own FM_ARM_CONFIRM_TIMEOUT=30-in-watch.env case. Sibling sites with the same config-source mismatch: bin/fm-claude-stop-autoarm.sh:168 (GRACE) and :373-376 (window). The smallest honest remedy is to have the hook resolve these values from watch.env the way the arm does. That would also change the hook's other GRACE-based decisions, so the remedy, not the defect, needs authorization.✅ **Test** - passed
✅ No issues found.
watcher: started), not killed with confirmation-timeoutwatcher: startedat 21s with no ledger rowbin/fm-lab-home.sh create $LAB, then the realbin/fm-watch-arm.shwith FM_HOME=$LAB from base 7da4c362 and from target ab79e99f: watcher start kept busy for 16s through BASH_ENV, default FM_ARM_CONFIRM_TIMEOUT=10Real host load from 40yesCPU burners on 10 cores (load average 27-50): base vs target arm with the default window, then with FM_ARM_CONFIRM_TIMEOUT=1, two runs eachTarget arm, stalled start (exec sleep 60), FM_ARM_CONFIRM_TIMEOUT=3Target arm, start that spins forever, FM_ARM_CONFIRM_TIMEOUT=2 FM_GUARD_GRACE=8Target arm, grace shorter than the window: FM_ARM_CONFIRM_TIMEOUT=5 FM_GUARD_GRACE=1 with a 12s busy startTarget arm sent TERM at 7s, during an extended window (FM_ARM_CONFIRM_TIMEOUT=3); checked exit time, ledger reason and that the watcher pid was gonebash tests/fm-claude-stop-autoarm.test.sh(66 ok, 0 not ok, including the 3 new successor tests)✅ **Document** - passed
✅ No issues found.
✅ **Push** - passed
✅ No issues found.