Skip to content

fix(watch): hold the declared-wait pause cadence across pane churn - #67

Merged
withally merged 1 commit into
mainfrom
fm/fm-pause-suppressor-tuning-p1
Aug 22, 2026
Merged

fix(watch): hold the declared-wait pause cadence across pane churn#67
withally merged 1 commit into
mainfrom
fm/fm-pause-suppressor-tuning-p1

Conversation

@withally

Copy link
Copy Markdown
Owner

Intent

Fix the firstmate watcher's pause-suppressor so a declared wait actually holds its long cadence.

Reported symptom (live, 2026-08-22 evening): tasks with a current paused: status line kept surfacing bare stale: wakes every few minutes (several windows cycling) instead of being absorbed by handle_paused_stale's long-cadence resurface (FM_PAUSE_RESURFACE_SECS, default 3600). Occasionally the absorb DID work (wakes arriving as "paused Ns, awaiting external - ... long cadence"), so the mechanism existed but did not hold.

Required approach: diagnose before fixing - reproduce the failure in a test harness rather than assuming a cause. Three suspects were offered as hypotheses to check, NOT as conclusions: (1) each new status append resets the .paused- suppressor so any progress line restarts the cycle; (2) watcher generation changes, e.g. after away-mode handover, discard suppressor state; (3) the stale detector surfaces before the pause classification runs on some path.

What the fix had to achieve: a CURRENT paused: latest status line must hold the 3600s cadence across status appends and watcher restarts, while NEVER suppressing a genuinely new captain-relevant status line - needs-decision, blocked and done must still wake immediately. That boundary is the safety contract and was explicitly required to carry regression tests.

Constraints stated by the user: bin/fm-watch.sh and bin/fm-classify-lib.sh own the mechanics; colocate tests per repo convention; keep bin scripts shellcheck-clean; and read and follow .agents/skills/firstmate-coding-guidelines/SKILL.md before editing, because this is firstmate's own shared tracked material (one-owner rule for contracts, mechanics in the script header rather than duplicated prose, one sentence per line in tracked Markdown, plain dash never an em dash, no agent co-author on commits, tests must exercise behavior through an executable interface and never assert implementation-source bytes).

What the diagnosis actually found (reproduced end to end against a real fm-watch.sh subprocess before changing anything: a live-agent crew with a current paused: line and a merely re-rendering pane produced 5 bare stale: wake exits across 5 watcher rounds): none of the three suspects as literally stated. pause_state_class re-read backend agent liveness on EVERY evaluation, and for an ordinary crew whose agent is still live - the normal state of a paused crew - it answered none. none is a caller's "no declared wait here" signal, so the changed-hash caller ran clear_pause_tracking and erased the cadence flag .paused-, its long-cadence throttle .paused-resurfaced-, and the per-hash suppressor .stale-. Any pane redraw (a ticking clock, a token counter, the crew's own status echo) therefore re-armed a first sighting, and the next stable hash surfaced through surface_nonterminal_stale all over again. The suppressor was keyed to the pane hash rather than to the declaration.

Deliberate design decisions in the fix, so they are not read as oversights:

  • The liveness gate is kept, not removed. It exists (from the earlier parked-crew work) to give a live agent ONE inspection per declared wait, in case the crew declared a wait while actually parked at a decision gate. The bug was that nothing recorded the inspection had been spent. The gate is now made one-shot by reading the existing .paused- marker, which already means "this key's stale is using the bounded cadence".
  • A confidently dead agent still joins the cadence with no initial surface, and a secondmate's endpoint liveness is still deliberately never read. Both prior behaviors are preserved intentionally.
  • The one permitted live-agent surface deliberately keeps the plain bare stale: <window> payload rather than gaining a declared-wait label. That surface IS the "inspect this pane" event by design, surface_nonterminal_stale is shared with other paths, and the away-mode hand-off contract asserts a plain stale identity. Relabelling it was judged scope creep for this fix.
  • The window_kind read was moved to its only remaining consumer so the common already-on-cadence poll re-reads nothing at all - not the crew state, not the window metadata, not the backend. This is a deliberate cost reduction on the hot path, not an unrelated refactor.
  • The busy-pane clear paths were left alone on purpose: an exact busy verdict means work genuinely resumed, so clearing the cadence there is correct.
  • docs/architecture.md was updated because it stated the old dead-agent-only version of this contract; under the one-owner rule the full contract lives in the bin/fm-watch.sh header and the doc carries the cross-reference. No AGENTS.md change was needed: this is situational mechanics, not something every session needs, and AGENTS.md size discipline applies.
  • bin/fm-classify-lib.sh was inspected as part of the diagnosis but needed no change; the defect was entirely in the watcher's classification, so leaving the shared classifier untouched is intentional.

Tests added, colocated in tests/fm-watch-triage.test.sh, both driving a real fm-watch.sh subprocess (no implementation-source assertions):

  • test_declared_pause_cadence_survives_pane_churn_and_restarts - the regression. Verified to FAIL on the pre-fix tree with exactly the reported symptom ("surfaced 5 stale wakes across five restarts") and to pass after. It covers pane churn across five watcher restarts and then further declaring status appends, and asserts the cadence marker survives and no wedge timer is ever started.
  • test_captain_relevant_line_breaks_an_armed_pause_cadence - the safety contract. With a fully armed cadence and a live agent, needs-decision, blocked and done each still wake immediately, exercised on the signal path (append unseen) and on the stale path (append already seen), and each must drop the cadence. Verified non-vacuous by mutation testing: a mutant that lets the cadence swallow captain-relevant lines fails it.

Explicitly out of scope by the user's later instruction: while verifying, tests/fm-wake-queue.test.sh's "a subshell reclaimed its parent's live hold" was found failing on clean HEAD as well - bin/fm-wake-lib.sh's ${BASHPID:-$$} guard degrades on macOS bash 3.2, which has no BASHPID, so a subshell reads as the same process and reclaims its parent's live lock. That finding is real and pre-existing on main, is filed as its own separate task, and the user explicitly instructed that it must NOT be folded into this change. This change stays scoped to the pause suppressor, so that unrelated failure is expected to remain.

Verification already run locally: tests/fm-watch-triage.test.sh 62 ok and 0 failures; tests/fm-daemon.test.sh, fm-watcher-lock, fm-watch-arm, fm-watch-checkpoint, fm-watch-recovery-loop, fm-guard-stale-banner, fm-pi-watch-extension and fm-crew-state all clean; bin/fm-lint.sh exit 0 with the pinned ShellCheck 0.11.0 and actionlint 1.7.12; bin/fm-doc-audience-check.sh ok.

What Changed

  • pause_state_class in bin/fm-watch.sh no longer re-reads backend agent liveness on every evaluation. The live-agent inspection is now one-shot per declaration, gated on the existing .paused-<key> marker, so a still-current paused: line keeps the long PAUSE_RESURFACE_SECS cadence across pane redraws, further declaring status appends, and watcher restarts instead of returning none and letting the caller erase the cadence flag, its throttle, and the per-hash stale suppressor.
  • The window_kind read moved into the one-shot gate, its only remaining consumer, so a poll that is already on the cadence and reconciled within the wedge window reads no crew state, window metadata, or backend at all. A confidently dead agent still joins the cadence with no initial surface, a secondmate's endpoint liveness is still never read, and the busy-pane clear paths are unchanged.
  • Added two regression tests to tests/fm-watch-triage.test.sh, both driving a real fm-watch.sh subprocess: one asserting the cadence survives pane churn across five watcher restarts and later declaring appends with no wedge timer started, and one asserting an armed cadence with a live agent still wakes immediately for needs-decision, blocked, and done on both the signal and stale paths, dropping the cadence each time. docs/architecture.md was updated to describe the declaration-owned cadence instead of the old dead-agent-only wording, with the full contract kept in the script header.

Risk Assessment

✅ Low: Narrow, well-diagnosed fix confined to one classifier function plus its header contract; the state machine traces correctly for the reported failure and for the needs-decision/blocked/done safety boundary, and both are pinned by new behavior-driven regression tests against a real fm-watch.sh subprocess.

Testing

I reproduced the reported failure before trusting the fix: driving a real fm-watch.sh subprocess over five watcher rounds against a crew with a current paused: line and a merely re-rendering pane, the base-commit watcher emitted five bare stale: wakes (the exact live symptom), while the fixed watcher emits a single inspection wake and then only long-cadence check: rearm-resurface output, with the cadence marker intact and no wedge timer. The safety boundary was exercised too - needs-decision, blocked and done each still wake immediately through a fully armed cadence on both the signal and stale paths - and I confirmed that test is not vacuous by building a swallow mutant of the watcher, which fails it. The targeted colocated suite tests/fm-watch-triage.test.sh is 62 ok / 0 failures and the adjacent away-mode suite tests/fm-daemon.test.sh is 103 ok; the known pre-existing fm-wake-queue BASHPID failure was left alone per the user's explicit out-of-scope instruction. This is a CLI/watcher change with no rendered UI surface, so the reviewer-visible evidence is the captain-facing wake transcript rather than a screenshot. All temporary mutant and trimmed-test copies were removed and the worktree is clean.

Evidence: Captain-visible wakes, pre-fix vs post-fix (same churning declared-pause fixture)

Source: Captain-visible wakes, pre-fix vs post-fix (same churning declared-pause fixture)

### PRE-FIX (base 096e5dd bin/fm-watch.sh) stale: test:fm-churn stale: test:fm-churn stale: test:fm-churn stale: test:fm-churn stale: test:fm-churn --- stale wakes: 5 --- not ok - a churning declared pause surfaced 5 stale wakes across five restarts (expected the single live-agent inspection) ### POST-FIX (90d2924 bin/fm-watch.sh) stale: test:fm-churn check: rearm-resurface check: rearm-resurface --- stale wakes: 1 --- ok - a current declared wait holds its long cadence across pane churn, declaring appends and watcher restarts ok - needs-decision, blocked and done still wake immediately through an armed declared-wait cadence

### PRE-FIX (base 096e5dd bin/fm-watch.sh)
--- captain-visible wakes after five watcher rounds over a still-paused crew (/Users/ivan/.no-mistakes/worktrees/37852af5566c/01M0N9Y3DHNGEZYT2ZS23T1T13/bin/fm-watch-prefix.sh) ---
    stale: test:fm-churn
    stale: test:fm-churn
    stale: test:fm-churn
    stale: test:fm-churn
    stale: test:fm-churn
--- stale wakes: 5 ---
not ok - a churning declared pause surfaced 5 stale wakes across five restarts (expected the single live-agent inspection)
exit=1

### POST-FIX (90d2924 bin/fm-watch.sh)
--- captain-visible wakes after five watcher rounds over a still-paused crew (/Users/ivan/.no-mistakes/worktrees/37852af5566c/01M0N9Y3DHNGEZYT2ZS23T1T13/bin/fm-watch.sh) ---
    stale: test:fm-churn
    check: rearm-resurface
    check: rearm-resurface
--- stale wakes: 1 ---
ok - a current declared wait holds its long cadence across pane churn, declaring appends and watcher restarts
ok - needs-decision, blocked and done still wake immediately through an armed declared-wait cadence
exit=0
Evidence: Safety-contract mutation check (swallow mutant fails the boundary test)

Source: Safety-contract mutation check (swallow mutant fails the boundary test)

not ok - an armed pause cadence swallowed a needs-decision line on the stale path (exit=1)

### Safety contract non-vacuity: swallow mutant of bin/fm-watch.sh
Mutation: (a) removed the 'a non-declaring latest line drops .paused-<key>' clear in the stale loop,
          (b) routed any key with an armed .paused-<key> into handle_paused_stale before stale_is_terminal.
Result of test_captain_relevant_line_breaks_an_armed_pause_cadence against that mutant:
not ok - an armed pause cadence swallowed a needs-decision line on the stale path
exit=1
Evidence: New tests in the targeted suite (tests/fm-watch-triage.test.sh, 62 ok / 0 failures)

Source: New tests in the targeted suite (tests/fm-watch-triage.test.sh, 62 ok / 0 failures)

ok - a current declared wait holds its long cadence across pane churn, declaring appends and watcher restarts ok - needs-decision, blocked and done still wake immediately through an armed declared-wait cadence

ok - a current declared wait holds its long cadence across pane churn, declaring appends and watcher restarts
ok - needs-decision, blocked and done still wake immediately through an armed declared-wait cadence

Pipeline

Updates from git push no-mistakes

✅ **intent** - passed

✅ No issues found.

✅ **Rebase** - passed

✅ No issues found.

⚠️ **Review** - 1 info
  • ℹ️ bin/fm-watch.sh:536 - The one-shot live-agent inspection is keyed to .paused-&lt;key&gt;, which handle_paused_stale (fm-watch.sh:437) also sets from the busy path via busy_turn_bound_check - without writing .paused-rechecked-&lt;key&gt;. Reachable sequence: a crew declares paused:, its pane stays busy past BUSY_TURN_MAX_SECS, busy_turn_bound_check absorbs it and arms .paused-&lt;key&gt;; when the pane later goes non-busy and stale, pause_state_class sees on_cadence=0 so the liveness gate at line 536 is skipped entirely and the crew joins the 3600s cadence with no inspection surface. Pre-fix that crew still got its one live-agent stale surface (the old early block required a fresh recheck marker, which is absent here, so it fell through to the liveness gate). This is consistent with the intent's stated rule that the gate is spent once a caller has put the key on the cadence, and the busy verdict already contradicts the parked-at-a-decision-gate case the gate guards against, so noting only - no change recommended.
✅ **Test** - passed

✅ No issues found.

  • bash tests/fm-watch-triage.test.sh - 62 ok, 0 failures (includes the two new tests)
  • Fail-before check: ran test_declared_pause_cadence_survives_pane_churn_and_restarts against a copy of the base-commit watcher (git show 096e5dd:bin/fm-watch.sh) - failed with "surfaced 5 stale wakes across five restarts"
  • Captured the actual watcher wake output (watch.out) for the same churn scenario pre-fix vs post-fix as the end-user-visible artifact
  • Non-vacuity check: mutated bin/fm-watch.sh so an armed .paused-&lt;key&gt; is never dropped by a captain-relevant line and terminal stale routes into handle_paused_stale; test_captain_relevant_line_breaks_an_armed_pause_cadence then fails ("swallowed a needs-decision line on the stale path")
  • bash tests/fm-daemon.test.sh - 103 ok, 0 failures (away-mode handoff, adjacent risk surface)
✅ **Document** - passed

✅ No issues found.

⚠️ **Lint** - 1 warning
  • ⚠️ linter found issues (exit code 1)
✅ **Push** - passed

✅ No issues found.

A crew with a current `paused:` (or verified `captain-held`) last status line
kept surfacing bare `stale:` wakes every few minutes instead of being absorbed
on the long FM_PAUSE_RESURFACE_SECS cadence.

pause_state_class read backend agent liveness on EVERY evaluation, and for an
ordinary crew whose agent is still live - the normal state of a paused crew - it
answered `none`. `none` is a caller's "no declared wait here" signal, so the
changed-hash caller ran clear_pause_tracking and erased the cadence flag, its
long-cadence throttle, and the per-hash stale suppressor. Any pane redraw (a
ticking clock, a token counter, the crew's own status echo) therefore re-armed a
first sighting, and the next stable hash surfaced through
surface_nonterminal_stale all over again.

The liveness gate was only ever meant to give a live agent ONE inspection per
declared wait, in case it declared a wait while actually parked at a decision
gate. `.paused-<key>` already records that the key is on the bounded cadence, so
the gate now reads it and is spent once: a live agent surfaces once and then
joins the cadence, and the declaration - not the pane hash - decides how long
the cadence holds. It survives further declaring appends and watcher restarts,
and is dropped only when the last status line stops declaring a wait or the crew
is provably working again. A confidently dead agent still joins the cadence with
no initial surface, and a secondmate's endpoint liveness is still never read.

The safety boundary is unchanged and now pinned: needs-decision, blocked and
done still wake immediately through a fully armed cadence, on the signal path
when their append is unseen and on the stale path when it has been seen.

Verified against a real fm-watch.sh over five restarts with a churning idle
pane: five bare stale wakes before, one after.
@withally
withally merged commit c735ec1 into main Aug 22, 2026
13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant