Skip to content

fix: let a declared pause outrank a busy pane's wedge escalation - #2215

Closed
3264studios wants to merge 8 commits into
kunchenguid:mainfrom
3264studios:fm/declared-pause-outranks-busy-wedge
Closed

fix: let a declared pause outrank a busy pane's wedge escalation#2215
3264studios wants to merge 8 commits into
kunchenguid:mainfrom
3264studios:fm/declared-pause-outranks-busy-wedge

Conversation

@3264studios

Copy link
Copy Markdown
Contributor

A crew that declares paused: <external wait> is meant to be absorbed onto the
bounded FM_PAUSE_RESURFACE_SECS cadence rather than treated as a possible wedge.
That holds while the pane is idle. It does not hold when the pane reads busy,
and a declared wait can legitimately hold a pane busy — a worker sitting on a
background monitor keeps a live foreground call for as long as the wait lasts.

Both consumers then re-escalate a healthy, correctly-declared pane every
FM_STALE_ESCALATE_SECS. Observed in normal operation as tens of repeat
"possible wedge" escalations against two declared-paused panes in a single
evening.

This is one defect at two call sites, which is why it is one PR: the watcher and
the away-mode daemon each reach wedge escalation without consulting the
declaration, and the review and documentation commits interleave across both.

Fix 1 — daemon: a declared pause outranks an enriched wedge escalation

Where it is today. bin/fm-supervise-daemon.sh:1216-1219 (on main).
handle_wake computes classify_stale's verdict at line 1216, then line 1219
unconditionally overwrites it with escalate whenever the reason matches the
watcher's enriched wedge shape:

decision=$(classify_stale "$arg" "$state")
case "$stale_detail" in
  idle\ *s,\ possible\ wedge,\ escalation\ *)
    decision="escalate|${reason#stale: }" ;;
esac ;;

The override exists so the daemon's cheaper status-log absorption cannot
re-absorb a wedge the watcher already analyzed. That is right for run-step and
pane state. It is wrong for a pause verdict, which is neither: it is the crew's
explicit declaration that the pane idles by design — the one cadence question the
wedge timer cannot answer for itself.

The override now yields to a pause verdict and nothing else. Every other
enriched wedge escalates exactly as before, including its
demand-deep-inspection detail.

Test evidence (tests/fm-daemon.test.sh, reverting only
bin/fm-supervise-daemon.sh):

Result
without the fix FAILSnot ok - paused enriched wedge escalated instead of using the pause cadence
with the fix PASSES — 101 assertions, 0 failures

Fix 2 — watcher: a declared pause outranks a busy pane's wedge timer

Where it is today. bin/fm-watch.sh:1140 and bin/fm-watch.sh:1152 (on
main). Both busy call sites hand an over-age pane straight to
wedge_timer_check with no declared-pause check:

if [ "$busy_now" -eq 0 ] && busy_turn_over_age "$task"; then
  wedge_timer_check "$w" "$ssf" "busy (no completed turn)" "$ewf"

The idle stale paths above them already divert a declared pause to the bounded
pause cadence; these two never consult it. The nearest declared-pause check
(around line 1157) is gated on [ "$busy_now" -ne 0 ], so a busy pane cannot
reach it.

Both now route through busy_wedge_or_pause, which hands a declared pause to
handle_paused_stale and everything else to the unchanged wedge timer. Absorbed,
never silenced: the bounded recheck still re-surfaces the pause once per window,
and a later non-pause status line hands the pane straight back to ordinary wedge
detection. The gate is deliberately the cheap declared-pause read rather than
pause_state_class, which resolves a busy pane to working through run-step
precedence — the one verdict that cannot separate a wedge from a declared wait
here. It is skipped while afk is active, matching the other normal-mode pause
routing, so the daemon keeps owning triage there.

Test evidence (tests/fm-watch-triage.test.sh, reverting only
bin/fm-watch.sh):

Result
without the fix FAILSnot ok - a declared-paused busy pane wedge-escalated on round 1: stale: test:fm-busy-paused (idle 501s, possible wedge, escalation 1)
with the fix PASSES — all three regressions green

The three regressions drive the real watcher over both busy branches (stable
hash, and the ticking footer that never repeats a hash) across repeated
over-threshold rounds, and pin the disconfirming cases alongside: a non-pause
verb still wedges, a lifted pause returns to wedge detection after a fresh timer,
and away mode still hands the wedge off.

Note on the test-conformance commit

The final commit adjusts those new regressions for main's generation-bound
downtime recovery. They stop a watcher deliberately between rounds; since that
recovery landed, an intentionally stopped cycle leaves it armed, so the next
round woke on check: rearm-resurface and exited before reaching the busy pause
divert. They now acknowledge each intentional stop with ack_stopped_cycle, the
same way the neighbouring busy-pane tests already do, with the queue assertions
ordered ahead of the acknowledgement that rewrites the queue.

Test suite

main is not green in this environment, so the bar used here is no new
failure
, not a green suite. Baseline on untouched main, full suite,
141 scripts — 8 failed:

tests/fm-backend-orca.test.sh
tests/fm-bootstrap.test.sh
tests/fm-gotmp.test.sh
tests/fm-muse-harness.test.sh
tests/fm-pi-watch-extension.test.sh
tests/fm-remote-secondmate-lifecycle-e2e.test.sh
tests/fm-remote-secondmate-trace-context.test.sh
tests/fm-teardown.test.sh

Several are negative tests asserting an operation should fail, which succeeds
in this sandbox, so they look environmental rather than like real breakage on
main. None are touched by this change.

After this branch, the same full suite reports no new failure: the identical
set, minus tests/fm-bootstrap.test.sh, which passed this time. That one is
flaky rather than a stable pre-existing failure, which is worth knowing if you
use this list as a reference.

Two caveats stated plainly, so the number is not read as stronger than it is:

  • tests/fm-watch-triage.test.sh reports a failure in both the baseline and
    the after run, for the same reason both times: it stalled under full-suite load
    and had to be unstuck, which fails the in-flight assertion. Run on its own it
    is green on both sides — 46 assertions on untouched main, and green again
    with this branch applied.
  • tests/fm-secondmate-safety.test.sh and tests/fm-watch-arm.test.sh also
    stalled under load and were unstuck the same way. Both then passed.

Those stalls are the pre-existing hang described below, not a behavior change
here: the changed path fires only when a pane is busy, over the turn-age bound,
and under a declared pause, and none of those tests construct that. A
kind=secondmate pane is skipped before the busy branches
(bin/fm-watch.sh:1076), and fm-watch-arm.test.sh never declares a pause at
all.

bin/fm-lint.sh is clean (ShellCheck 0.11.0, the pinned version).

One environment caveat worth passing on: tests/fm-watch-triage.test.sh is
timing-sensitive under a loaded machine. Under the full parallel suite it can
stall in reap, which sends SIGTERM and then waits with no timeout — if the
watcher does not exit, the file hangs indefinitely. It passes standalone. That is
pre-existing and untouched here, but it is why the per-fix evidence above was
taken from targeted runs.

A crew that declared `paused: <external wait>` was wedge-escalated to the
away-mode supervisor every ~250s for hours (2026-08-05: 19 wedge digests
across two declared-paused panes in one evening).

The watcher's wedge timer emits an enriched
"stale: <win> (idle Ns, possible wedge, escalation N)" reason on the
FM_STALE_ESCALATE_SECS cadence, and handle_wake force-escalated every reason
of that shape so the daemon's cheaper status-log absorption could not
re-absorb a wedge the watcher had already analyzed. That override also
discarded classify_stale's correct `pause` verdict, which is not run-step or
pane state at all but the crew's explicit declaration that the pane idles by
design - the one cadence question the wedge timer cannot answer for itself.

The override now yields to a `pause` verdict and nothing else. A declared
pause routes to the pause action, so housekeeping's bounded
FM_PAUSE_RESURFACE_SECS recheck re-surfaces it once per window instead of
nagging on the wedge cadence, and a forgotten pause still cannot rot
invisibly. Every other enriched wedge escalates exactly as before, including
its demand-deep-inspection detail, and a later non-paused status append
restores normal wedge detection.

This makes the code match the contract docs/architecture.md and the /afk
skill already state ("absorbed while idle and re-surfaced only on the longer
pause cadence, rather than being treated as a possible wedge"), so no
documentation change is needed.

Reproduced first as a failing regression in tests/fm-daemon.test.sh, driving
repeated enriched wedges through the real handle_wake/housekeeping pair with
the live incident's byte shape and asserting the cadence rather than a single
wake. test_stale_diagnostic_wedge_survives_busy_housekeeping's paused case
asserted the defect and now asserts the corrected contract; its working and
prior-terminal cases are unchanged.
A crew can declare `paused: <external wait>` while its pane still reads BUSY -
a worker holding a background monitor shell keeps a live foreground call for as
long as the wait lasts. busy_turn_over_age then trips BUSY_TURN_MAX_SECS and the
watcher's busy paths handed that healthy pane to wedge_timer_check, which
re-escalates every STALE_ESCALATE_SECS: ~22 repeat "possible wedge" escalations
on one declared-paused pane in one evening (2026-08-05), outside away mode.

The idle stale paths already divert a declared pause to the bounded pause
cadence; the two busy call sites never consulted it at all. Both now route
through busy_wedge_or_pause, which hands a declared pause to handle_paused_stale
and everything else to the unchanged wedge timer. The declaration is the crew's
own statement that the pane runs long by design - the one cadence question the
wedge timer cannot answer for itself - so it takes the same bounded
PAUSE_RESURFACE_SECS recheck: absorbed, never silenced, and a later non-pause
status line hands the pane straight back to ordinary wedge detection.

The gate is deliberately the cheap declared-pause read rather than
pause_state_class: it runs on every poll of an over-age pane, and
pause_state_class resolves a busy pane to `working` through crew_absorb_class's
run-step precedence - the one verdict that cannot separate a wedge from a
declared wait here. It is also skipped while afk is active, matching the other
normal-mode pause routing, so the daemon keeps owning triage and applies its own
declared-pause precedence to the enriched wedge reason instead.

A busy pane on the pause cadence also keeps its pause bookkeeping: that path's
re-surface throttle lives in the same files the busy branches otherwise retire,
so retiring them would have re-surfaced the pause on every poll instead of once
per window.

Reproduced first as a failing regression in tests/fm-watch-triage.test.sh,
driving the real watcher over both busy branches (stable hash and the ticking
footer that never repeats a hash) with the incident's byte shape and asserting
the cadence across repeated over-threshold rounds rather than a single wake. The
disconfirming cases are pinned alongside it: a non-pause verb still wedges, a
lifted pause returns to wedge detection after a fresh timer, and away mode still
hands the wedge off.

docs/architecture.md and docs/configuration.md stated the busy bound routes
unconditionally to wedge escalation, so both are corrected to match.
…gressions

The busy declared-pause regressions drive several watcher cycles against one
state directory, stopping each cycle deliberately between rounds. Since the
watcher gained generation-bound downtime recovery, an intentionally stopped
cycle leaves that recovery armed, so the next round's watcher woke on
"check: rearm-resurface" and exited before it ever reached the busy pause
divert - the test stopped exercising the cadence it exists to pin.

Acknowledge each intentional stop with ack_stopped_cycle, the same way the
neighbouring busy-pane tests already do.

The queue assertions move ahead of their acknowledgement, because the
acknowledgement rewrites the durable queue: each over-threshold round now
asserts its own absorb before acking rather than counting once after the loop,
and the bounded-recheck window asserts its single re-surface in phase C and
then proves no second one lands in phase D. That keeps "absorbed, and
re-surfaced exactly once per window" pinned on both sides instead of resting
on a count the acknowledgement could empty.

Verified by reverting bin/fm-watch.sh alone: without the fix the first
over-threshold round still fails with the live defect ("stale: <win> (idle
501s, possible wedge, escalation 1)"), and with it all three regressions pass.
@3264studios

Copy link
Copy Markdown
Contributor Author

Superseded by #3147 - closing this one.

Three things were wrong with this PR, and all three are fixed there:

The watcher fix itself also gained a real correction on the way: the away-mode
handoff was keyed on the pane capture hash, which on a pane whose harness footer
ticks every capture would have re-fired every poll rather than once. It is now
keyed on the declaration, and the regression test drives a ticking pane apart from
a static one so that cannot regress silently.

Apologies for the noise on the first attempt.

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.

2 participants