Skip to content

fix(watch): prevent repeated parked-decision wakes - #24

Merged
HelloWorldSungin merged 3 commits into
mainfrom
fm/fm-parked-decision-stale-noise
Aug 4, 2026
Merged

HelloWorldSungin merged 3 commits into
mainfrom
fm/fm-parked-decision-stale-noise

Conversation

@HelloWorldSungin

Copy link
Copy Markdown
Owner

Re-landed on the fork from the already-validated branch (previously opened as kunchenguid#1633, which this account cannot merge upstream). Code unchanged.


Intent

Stop a crew that is correctly parked on a captain decision from re-waking firstmate on every idle poll.

MECHANISM (verified before coding, traced through bin/fm-watch.sh's terminal stale path): a crew parks correctly on a keyed 'needs-decision:' line and firstmate escalates. The one-shot stale suppression guard was keyed on the PANE CONTENT HASH (state/.stale-). An idle Claude pane is not static - its footer carries a live context percentage and quota readout - so the hash changes on repaint and the identical waiting state re-surfaces as a fresh stale wake. crew_is_provably_working() correctly does NOT absorb it, because the crew genuinely is not working, it is waiting; that override was built for the opposite case (an active validation run behind a stale leftover status line). On 2026-08-01 this cost roughly twenty handling turns in a single session; one pane alone escalated twenty-two consecutive times while healthy.

WHAT WAS WANTED: once firstmate has surfaced a captain-blocked crew and taken its action, that exact open decision must not re-surface as stale on pane repaint alone. Suppression must key on the OPEN-DECISION IDENTITY - bin/fm-classify-lib.sh already folds keyed open decisions via status_open_decisions, and the observed cases carry explicit [key=...] slugs - rather than on volatile pane content.

NON-NEGOTIABLE PROPERTIES, explicitly stated as what makes the task hard:

  • A genuinely NEW event must still wake normally: a new status line, a new decision key, or the decision resolving.
  • Wedge detection for a crew that stops responding must NOT be weakened. A parked crew that then dies must still surface.

ACCEPTANCE CRITERIA:

  • A parked crew whose pane hash changes across polls surfaces at most once per open decision.
  • A new keyed decision on the same pane still surfaces.
  • A parked crew that stops responding is still detected as a wedge suspect, covered by an explicit test.
  • Tests fail against the pre-change code.

WHAT I IMPLEMENTED: in bin/fm-watch.sh's stale_is_terminal branch, when the captain-relevant status is a still-open keyed decision, the one-shot suppressor now keys on a digest of status_open_decisions' open set (new helper open_decision_id, stored in state/.stale-decision-) instead of the pane hash. On the suppressed path the pane hash is still advanced, and wedge detection is preserved rather than weakened: a new helper parked_agent_is_dead routes a parked crew whose backend confidently reports its agent dead through the existing shared wedge_timer_check. DELIBERATE DECISION: only the confident 'dead' verdict from fm_backend_agent_alive counts; ambiguous, unreadable, and unverified backend reads stay absorbed, so a flaky endpoint read cannot manufacture a false wedge alarm. The accepted tradeoff is that a truly dead crew on an unreadable endpoint waits for a heartbeat instead. The marker is deliberately named .stale-decision- so it is already covered by AGENTS.md's existing '.stale-* watcher internals; never touch' entry rather than requiring a new AGENTS.md line. The stale marker file is also cleared when no decision is open, so a later identical decision cannot be wrongly suppressed.

DELIBERATE SCOPE DECISIONS:

  • bin/fm-classify-lib.sh was NOT modified at all. The existing status_open_decisions fold supplied everything needed, which is better than the brief's allowance for a small localized change there, because a parked sibling task (fm-crew-state-blind-during-fix-round) also touches that file and now has nothing to reconcile.
  • bin/fm-crew-state.sh was NOT modified; the brief explicitly forbids it (owned by that same parked task).
  • The signal-handling and poll-wait path in bin/fm-watch.sh was NOT touched; it is owned by the live branch fm-watcher-term-hup-latency. Only the stale-wake suppression path is mine.
  • The away-mode (afk) stale branch was deliberately left on its existing hash-keyed one-shot. The away-mode daemon owns triage there and has its own escalation-digest dedupe, so changing it was out of the reported mechanism's scope and would have widened risk.

TESTS: three cases added to the existing tests/fm-watch-triage.test.sh (colocated per repo convention, driving a real fm-watch.sh subprocess through the public interface, no assertions on implementation source). test_parked_decision_survives_pane_repaint proves the repeat is suppressed; test_new_keyed_decision_on_parked_pane_surfaces proves a new keyed decision on the same repainting pane still surfaces (this one is the over-suppression guard and passes both before and after by design); test_parked_decision_with_dead_agent_wedge_escalates proves a parked-then-dead crew still wedge-escalates. Verified that test 1 and test 3 fail against the pre-change bin/fm-watch.sh and that the whole 43-case suite passes with it.

DOCS: docs/architecture.md is the maintainer-architecture owner of the actionable-wake contract, so its existing stale-pane language was patched in place (not appended to) to state the open-decision-keyed suppressor and the preserved wedge escalation. bin/fm-doc-audience-check.sh and the documentation-audiences suite pass. No AGENTS.md change was needed.

VERIFICATION RUN: bin/fm-lint.sh clean; tests/fm-watch-triage.test.sh 43/43; fm-watch-checkpoint, fm-daemon, fm-wake-queue, fm-supervision-events, fm-guard-stale-banner, fm-pi-watch-extension, fm-test-isolation-proof, fm-turnend-guard, fm-transition-lib all pass.

KNOWN PRE-EXISTING ISSUE, deliberately not fixed here: tests/fm-watcher-lock.test.sh is flaky (a watcher peer startup race, ~2 failures in 5 runs) and reproduces on origin/main independently of this change. Open PR kunchenguid#1487 on branch fm/firstmate-watcher-lock-flake already fixes exactly that flake, so touching it here would collide with that live branch.

What Changed

  • Track surfaced open-decision sets so unchanged parked decisions stay suppressed across volatile pane repaints, including after signal and heartbeat wakes.
  • Clear suppression when decisions resolve, surface changed or reopened decisions normally, and preserve wedge escalation when a parked agent is confidently dead.
  • Add subprocess regression coverage and document the updated watcher and stale-escalation behavior.

Risk Assessment

✅ Low: The updated shared surfaced-status boundary closes the prior signal-to-repaint gap, reconciles resolution and reopening, and preserves new-decision and dead-agent wedge paths without material source-verifiable regressions.

Testing

Alongside the supplied author baseline, focused subprocess and push-transition tests passed on target commit 66bb1ef; direct CLI evidence shows one initial decision wake, silence after pane repaint, wakes for a new key and resolved/reopened decision, and wedge escalation after confirmed agent death. The two regression guards fail against the pre-change code as required. No visual artifact was needed because this is a background CLI watcher change, not a rendered UI change.

Evidence: Watcher behavior transcript
ok - a parked keyed decision surfaces once, not once per pane repaint
ok - a resolved keyed decision can reopen identically and surface again
ok - a new keyed decision on an already-parked pane still surfaces
ok - a parked crew that stops responding is still detected as a wedge suspect

END-USER WATCH TRANSCRIPT

1. Initial captain decision is queued:
1785809384	2	signal	parked.status	signal: /tmp/fm-watch-focused-current.DTT00T/parked-decision-repaint/state/parked.status
wake annotation: latest wake-EVENT observed at drain, not current state: parked.status: needs-decision [key=review-gate]: ship as-is or split the migration

2. Same open decision after pane footer repaint (expected no wake):
watch stdout bytes: 0
wake queue bytes: 0
open-decision marker present: yes

3. New decision key on same pane:
stale: test:fm-parked2
1785809389	2	stale	test:fm-parked2	stale: test:fm-parked2

4. Identical decision after resolution and reopen:
stale: test:fm-reopen

5. Parked agent dies and crosses wedge threshold:
stale: test:fm-parked3 (idle 500s, possible wedge, escalation 1)
Evidence: Pre-change regression proof
PRE-CHANGE REGRESSION PROOF

test_parked_decision_survives_pane_repaint (exit 1, failure expected):
not ok - a pane repaint re-surfaced an already-escalated parked decision: stale: test:fm-parked

test_parked_decision_with_dead_agent_wedge_escalates (exit 1, failure expected):
not ok - the dead parked crew was not flagged as a possible wedge
Evidence: Push-transition focused tests
ok - handle_push_transition: a blocked crew enqueues a stale wake naming its window and wakes the supervisor
ok - handle_push_transition: enqueue failure cannot commit the Herdr dedupe marker
ok - handle_push_transition: a declared-pause crew is absorbed (no fast wake), left to the poll loop's long cadence
ok - event_wait_or_sleep: herdr windows go on the event pane list, but kind=secondmate endpoints are excluded
ok - event_wait_or_sleep: one cached capability probe owns validation across bounded waits
ok - event_wait_or_sleep: a home with no push-capable window is inert (sleeps POLL, never touches the event path)
ok - event_wait_or_sleep: consecutive event-path failures disable the fast-path and revert to pure polling (fail-closed)
# fm-supervision-events.test.sh: all assertions passed

Pipeline

Updates from git push no-mistakes

✅ **intent** - passed

✅ No issues found.

✅ **Rebase** - passed

✅ No issues found.

🔧 **Review** - 1 issue found → auto-fixed ✅
  • 🚨 bin/fm-watch.sh:978 - The intent requires that “once firstmate has surfaced a captain-blocked crew and taken its action, that exact open decision must not re-surface as stale.” However, the decision marker is written only here after a stale wake. A real needs-decision status first surfaces through the earlier signal path, which calls only mark_surfaced and exits; the next pane repaint therefore finds no decision marker and emits a redundant stale wake before creating it. The new tests mask this sequence by priming .seen-*. Record and reconcile the open-decision identity at the shared surfaced-status/transition boundary, and cover signal -> repaint plus resolve -> identical reopen sequences.

🔧 Fix: Captain, reconcile surfaced open-decision markers
✅ Re-checked - no issues remain.

✅ **Test** - passed

✅ No issues found.

  • Focused real-subprocess selectors from tests/fm-watch-triage.test.sh: test_parked_decision_survives_pane_repaint, test_resolved_decision_can_reopen_identically, test_new_keyed_decision_on_parked_pane_surfaces, and test_parked_decision_with_dead_agent_wedge_escalates.
  • Manual transcript inspection of watcher stdout, durable wake records, empty repaint queue, and the open-decision marker.
  • The repaint and dead-agent selectors against base commit 1e247571aa75e00b00c7a01c4830025ecd44dc61; both failed for the expected pre-change defects.
  • tests/fm-supervision-events.test.sh for shared push-transition wake behavior.
  • git status --short after testing to confirm no working-tree artifacts remained.
✅ **Document** - passed

✅ No issues found.

✅ **Lint** - passed

✅ No issues found.

✅ **Push** - passed

✅ No issues found.

Sungin Kim added 3 commits August 1, 2026 23:14
A crew that correctly parks on a keyed `needs-decision:` is genuinely not
working, so `crew_is_provably_working` rightly refuses to absorb its stale
pane. The one-shot stale suppressor was keyed on the pane content hash, but
an idle harness pane is not static - its footer carries a live context
percentage and quota readout - so every repaint changed the hash and
re-surfaced the identical, already-escalated waiting state as a fresh stale
wake. One parked pane escalated twenty-two consecutive times while healthy,
each costing a full handling turn.

Key the terminal stale path's suppressor on the open-decision identity from
`status_open_decisions` (bin/fm-classify-lib.sh's existing keyed fold)
whenever the captain-relevant status is a still-open keyed decision, so a
repaint alone is silent. Detection is not weakened: a new status line, a new
decision key, or the decision resolving all change the open set and surface
normally, and a parked crew whose backend confidently reports its agent dead
still escalates through the shared wedge timer. Only a confident `dead`
verdict counts, so an ambiguous or unreadable backend read cannot manufacture
a wedge alarm.

Tests cover all three properties and fail against the previous behavior.
@cursor

cursor Bot commented Aug 4, 2026

Copy link
Copy Markdown

Bugbot is not enabled for your account, so this pull request was not reviewed.

Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs.

@HelloWorldSungin
HelloWorldSungin merged commit 58022b2 into main Aug 4, 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