Skip to content

fix(watch): treat armed PR merge polls as external waits, not wedges - #24

Merged
Aviator-Coding merged 6 commits into
mainfrom
fm/fm-pr-wait-not-a-wedge
Sep 23, 2026
Merged

Aviator-Coding merged 6 commits into
mainfrom
fm/fm-pr-wait-not-a-wedge

Conversation

@Aviator-Coding

Copy link
Copy Markdown
Owner

Intent

Stop the firstmate watcher (bin/fm-watch.sh) from escalating an idle ship worker as a possible wedge while its PR is open and its merge poll is armed.

Defect (observed four times in one night, 2026-09-22): a ship worker that reported done: PR <url> checks green and stopped has nothing left to do, yet its pane escalated as a possible wedge about every 30 minutes for as long as the captain took to merge; by the third escalation the watcher demanded a deep inspection that could only ever conclude "still waiting". Two remedies were tried and neither works: appending paused: does not help because pause_state_class decides run-step precedence BEFORE liveness (an actively-running pipeline outranks a declared wait), and the no-mistakes run stays active while it monitors for merge; exiting the agent does not help for the same reason. Raising config/stale-escalate-secs would weaken wedge detection for every real worker, so it is not acceptable.

The signal that already exists: bin/fm-pr-check.sh records pr= in state/.meta and arms a validated merge poll (state/.pr-poll, state/.pr-poll-registration), so the watcher already knows this task waits on an external event it is itself polling for.

Required behaviour:

  • A task with an armed, non-terminal, validated PR merge poll is treated as an external wait on the stale path, using the same bounded cadence handle_paused_stale already implements, not as a wedge.
  • It must stay bounded and re-surface: a PR that goes red, a merge poll that is disarmed or invalid, or a worker whose endpoint dies must still be reported. Key on the validated poll registration (fm_pr_poll_artifacts_valid), not on the mere presence of a pr= line.
  • A task without an armed poll behaves exactly as today.

Tests: extend the existing watcher tests (tests/fm-watch-triage.test.sh, which covers pause_state_class / handle_paused_stale). Cover: armed poll + idle pane -> no wedge escalation within the window and a bounded recheck after it; same pane with no poll -> escalates as today; armed poll with dead endpoint -> still reported. Each assertion was proven able to fail by breaking the new branch (nine mutations, each turned its specific test red).

Constraints: do not touch the primary checkout; run bin/fm-lint.sh; update docs/turnend-guard.md or whichever doc owns stale-escalation semantics only where a statement becomes untrue (docs/architecture.md and docs/configuration.md were updated).

Implementation decisions made deliberately:

  • pr_merge_wait_armed(task) = valid task id, no pending .pr-poll-retirement receipt, and fm_pr_poll_artifacts_valid. pr_merge_wait_holds(win, task) additionally requires: not a secondmate, raw crew state (new CREW_ABSORB_STATE side channel from crew_absorb_class in bin/fm-classify-lib.sh) is working or done, last status verb is not needs-decision/blocked/failed, and fm_backend_agent_alive reports exactly alive (unknown/unverified backends such as zellij/orca/cmux fall back to today's escalation rather than being silenced).
  • The decision lives inside pause_state_class (one owner): the no-declaration path returns paused for a holding PR wait with a cheap repeat path (poll registration re-validated every call; crew state and liveness re-read every STALE_ESCALATE_SECS); the declared-pause path lets a holding PR wait outrank the run-step precedence. No first surface is owed for a PR wait because the PR-ready report already reached firstmate.
  • The terminal-status stale branch now classifies through pause_state_class; only an armed poll routes a paused class to handle_paused_stale there, so unarmed terminal stale surfacing is byte-for-byte the same as before.
  • Pause tracking survives idle-pane repaints for an armed PR wait (top-of-loop clear, not-yet-stable branch, and hash-change branch exempt it), so the recheck is spent once per PAUSE_RESURFACE_SECS window, not once per repaint.
  • The recheck wake reason is distinct: "awaiting external - PR open with an armed merge poll ... confirm the PR is still open and green".
  • Out of scope, deliberately: the away-mode daemon's separate stale path (bin/fm-supervise-daemon.sh) is unchanged.

What Changed

  • Idle ship panes with an armed, validated PR merge poll are classified as external waits on the stale path (pr_merge_wait_holds / pause_state_class) and use handle_paused_stale's bounded PAUSE_RESURFACE_SECS cadence instead of wedge escalation, with a distinct PR-open recheck reason.
  • Hold is keyed on validated poll registration (not a bare pr= line) plus working/done crew state, non-blocked status, and a confidently alive agent; disarmed/invalid polls, dead endpoints, and tasks without an armed poll keep today's surface or escalation behavior.
  • Pause tracking survives idle-pane repaints for armed PR waits; docs and watcher triage tests cover armed vs unarmed idle, dead endpoint, and repaint recheck bounds.

Risk Assessment

⚠️ Medium: I traced the core defect scenario (a done/checks-green ship crew whose no-mistakes run keeps monitoring an open PR) and every required re-surfacing case (dead endpoint, disarmed poll, red/parked run, no-armed-poll baseline) end to end through pause_state_class, pr_merge_wait_armed/holds, and handle_paused_stale, and found the implementation matches the authoritative intent exactly, including the two previously-failed remedies (paused: and run-step precedence); tests drive the real watcher binary as a subprocess and assert on actual queue/wake side effects rather than source content, satisfying the test-quality bar. No concrete bug survived tracing, so I did not withhold a finding out of caution - there simply isn't one to report. Risk is medium rather than low only because this rewires ~140 lines of a shared, highly-branchy escalation state machine (many marker files: .paused-, .stale-, .stale-since-, .paused-rechecked-, .paused-resurfaced-*) that gates wedge detection for every ship crew in the fleet, so the blast radius of any untraced edge case is real even though static review found none.

Testing

Drove the four PR-merge-wait watcher scenarios end-to-end against real bin/fm-watch.sh (armed via real bin/fm-pr-check.sh). All four suite tests passed in 91s, and a product transcript showed armed idle waits absorbed with a paused marker and zero wakes, then a single external-wait recheck; bare pr=, dead agent, and disarmed poll still wedge-escalated.

  • Live validation: ✅ go - 5 of 5 scenarios driven live against the product
Scenario Result Live Evidence
Idle ship with armed validated PR merge poll is absorbed on bounded cadence (no wedge) and rechecks once with external-wait reason ✅ pass live ~/.no-mistakes/evidence/01M35KE9SPCGGJS3VTF6ESF6AP/pr-merge-wait-transcript.log Scenario A; test_pr_merge_wait_is_bounded_not_a_wedge
Idle ship with bare pr= and no armed poll still wedge-escalates as today ✅ pass live ~/.no-mistakes/evidence/01M35KE9SPCGGJS3VTF6ESF6AP/pr-merge-wait-transcript.log Scenario B; test_pr_merge_wait_without_armed_poll_still_escalates
Armed PR merge wait with dead agent endpoint is still reported (wedge or stale) ✅ pass live ~/.no-mistakes/evidence/01M35KE9SPCGGJS3VTF6ESF6AP/pr-merge-wait-transcript.log Scenario C; test_pr_merge_wait_dead_endpoint_still_reported
PR merge wait re-surfaces when poll is disarmed or run stops only waiting on merge (red/parked) ✅ pass live ~/.no-mistakes/evidence/01M35KE9SPCGGJS3VTF6ESF6AP/pr-merge-wait-transcript.log Scenario D; test_pr_merge_wait_resurfaces_when_it_stops_holding
Idle-pane repaints under an armed PR wait do not re-flood the bounded recheck within one PAUSE_RESURFACE window ✅ pass live test_pr_merge_wait_is_bounded_not_a_wedge repaint rounds; pr-merge-wait-all.log
Evidence: PR merge wait suite results (all four ok)

ok - an idle crew whose PR has an armed merge poll is rechecked on the bounded cadence, never wedge-escalated ok - an idle provably-working crew without an armed merge poll still wedge-escalates as before ok - an armed PR merge wait never hides a dead agent ok - a PR merge wait re-surfaces as soon as its poll is disarmed or its run stops only waiting on the merge

ok - an idle crew whose PR has an armed merge poll is rechecked on the bounded cadence, never wedge-escalated
ok - an idle provably-working crew without an armed merge poll still wedge-escalates as before
ok - an armed PR merge wait never hides a dead agent
ok - a PR merge wait re-surfaces as soon as its poll is disarmed or its run stops only waiting on the merge
Evidence: Watcher product transcript (absorb, recheck reason, wedge paths)
## Scenario A: armed poll + idle working pane (must absorb, not wedge)
poll-registration:
fm-pr-poll-registration-v2
prwait
github
https://github.com/example/repo/pull/7
github.com
example/repo
7
823e9337bd2e0238bcefa268ef8503a56eff083d18fb6c8272b078f6645cdd34
e87aae384ec62ae28f78a0779020f52f7b891ba0cafea2551b5a2c4e967870cf
16777231:19111575
16777231:19111576
meta:
window=test:fm-prwait
kind=ship
harness=grok
backend=tmux
pr=https://github.com/example/repo/pull/7
result=alive (expect alive)
paused-marker=yes
stale-since=no
wake-count=0
watch.out:
(end)
recheck-result=exit (expect exit)
recheck-watch.out:
stale: test:fm-prwait (idle 501s, awaiting external - PR open with an armed merge poll, rechecked on a long cadence not a wedge; confirm the PR is still open and green)
wake-queue:
1790117697	1	stale	test:fm-prwait	stale: test:fm-prwait (idle 501s, awaiting external - PR open with an armed merge poll, rechecked on a long cadence not a wedge; confirm the PR is still open and green)

## Scenario B: bare pr= without armed poll (must wedge)
result=exit (expect exit)
watch.out:
stale: test:fm-prwait (idle 2s, possible wedge, escalation 1)

## Scenario C: armed poll + dead agent (must still report)
result=exit (expect exit)
watch.out:
stale: test:fm-prwait (idle 1s, possible wedge, escalation 1)

## Scenario D: disarmed poll returns to wedge
armed-result=alive (expect alive)
disarmed-result=exit (expect exit)
watch.out:
stale: test:fm-prwait (idle 2s, possible wedge, escalation 1)
Evidence: Suite timing metadata

EXIT:0 SECS:91

EXIT:0 SECS:91

Pipeline

Updates from git push no-mistakes

✅ **intent** - passed

✅ No issues found.

✅ **Rebase** - passed

✅ No issues found.

⚠️ **Review** - medium risk

✅ No issues found.

✅ **Test** - passed

✅ No issues found.

  • Live validation: ✅ go - 5 of 5 scenarios driven live against the product
Scenario Result Live Evidence
Idle ship with armed validated PR merge poll is absorbed on bounded cadence (no wedge) and rechecks once with external-wait reason ✅ pass live ~/.no-mistakes/evidence/01M35KE9SPCGGJS3VTF6ESF6AP/pr-merge-wait-transcript.log Scenario A; test_pr_merge_wait_is_bounded_not_a_wedge
Idle ship with bare pr= and no armed poll still wedge-escalates as today ✅ pass live ~/.no-mistakes/evidence/01M35KE9SPCGGJS3VTF6ESF6AP/pr-merge-wait-transcript.log Scenario B; test_pr_merge_wait_without_armed_poll_still_escalates
Armed PR merge wait with dead agent endpoint is still reported (wedge or stale) ✅ pass live ~/.no-mistakes/evidence/01M35KE9SPCGGJS3VTF6ESF6AP/pr-merge-wait-transcript.log Scenario C; test_pr_merge_wait_dead_endpoint_still_reported
PR merge wait re-surfaces when poll is disarmed or run stops only waiting on merge (red/parked) ✅ pass live ~/.no-mistakes/evidence/01M35KE9SPCGGJS3VTF6ESF6AP/pr-merge-wait-transcript.log Scenario D; test_pr_merge_wait_resurfaces_when_it_stops_holding
Idle-pane repaints under an armed PR wait do not re-flood the bounded recheck within one PAUSE_RESURFACE window ✅ pass live test_pr_merge_wait_is_bounded_not_a_wedge repaint rounds; pr-merge-wait-all.log
  • tests/fm-watch-triage.test.sh focused runner: test_pr_merge_wait_is_bounded_not_a_wedge
  • test_pr_merge_wait_without_armed_poll_still_escalates
  • test_pr_merge_wait_dead_endpoint_still_reported
  • test_pr_merge_wait_resurfaces_when_it_stops_holding
  • Manual transcript drive of real bin/fm-watch.sh + bin/fm-pr-check.sh for armed absorb/recheck, bare pr= wedge, dead-agent report, and disarmed-poll return-to-wedge
✅ **Document** - passed

✅ No issues found.

✅ **Lint** - passed

✅ No issues found.

✅ **Push** - passed

✅ No issues found.

@Aviator-Coding
Aviator-Coding merged commit a3cc759 into main Sep 23, 2026
22 of 24 checks passed
@Aviator-Coding
Aviator-Coding deleted the fm/fm-pr-wait-not-a-wedge branch September 23, 2026 01:40
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