fix(watch): surface a secondmate whose declared work goes silent - #2536
Open
josecarvallo wants to merge 4 commits into
Open
fix(watch): surface a secondmate whose declared work goes silent#2536josecarvallo wants to merge 4 commits into
josecarvallo wants to merge 4 commits into
Conversation
A secondmate's idle endpoint is healthy by design, so the stale loop exempts it, and a `working:` line is not captain-relevant, so the heartbeat backstop absorbs it. Both are correct in isolation, and together they hid a whole class of failure: a mate can append a non-terminal plan, end its turn, and idle forever with nothing to notice. On 2026-08-16 that left 21 panes, zero active workers, and five frozen PRs until the captain spotted it on screen. A declared-work line is an intention, never proof the work was launched. The same poll now checks every secondmate for a last event that still declares work while its endpoint has been idle past FM_DECLARED_WORK_STALL_SECS, and surfaces that combination as its own `check: secondmate-stalled <id>` reason so the supervisor reconciles the mate and pushes the work forward. fm-classify-lib.sh owns the decision, so the always-on watcher and the away-mode daemon read one contract. Detection lives inside the existing single supervision cycle; no new watcher, process, or wake kind. The idle clock is the newer of the task's completed-turn marker and its last status event, so any new turn or report resets it, and the endpoint read runs only after the pure status and idle-age gates hold. The default threshold is 900s: far above the 240s wedge threshold, because an idle secondmate endpoint is healthy and a mate may think for minutes, and below the 3600s declared-pause cadence, because a declared external wait is a legitimate reason to idle while unresumed work is not. A stall re-surfaces once per window so an unhandled one cannot rot. The secondmate idle-by-default contract and the meaning of `paused:` are unchanged: a declared pause, a terminal last event, and a proven-busy endpoint each keep their existing handling and never become a stall.
…ence Re-scopes the detector after the endpoint-based approach proved unreachable. A secondmate can never carry parent-side turn-end or busy wiring: bin/fm-spawn.sh:2304-2575 excludes kind=secondmate from all of it, deliberately, so the parent cannot land on the turn boundary that keeps that mate's own primary supervision alive. The previous round's turn-marker gate therefore silenced the detector for every mate, local and remote, while its tests passed against a fixture production cannot produce. Reading the mate's rendered pane instead was rejected as a fragile surface and a false-alarm source. The signal is now the mate's own status log. A live secondmate writes status events; one that declared work and stopped quits writing them. Status files are append-only, so the log's age is the time since its last event - a durable record the parent already holds. Nothing is wired at launch, no endpoint is read, and a remote mate is covered by the same rule because its escalations are ingested into this same state directory. The wake is reconciliation, not an alarm. Silence is not proof: the mate may be doing long legitimate work with nothing supervisor-actionable to report, may have finished a phase without closing its record, or may genuinely be stopped, and from outside these are indistinguishable. So the threshold is generous at 1500s - this repo's own contract makes status appends sparse events rather than routine progress - the reason states all three readings and asks for a cheap check before acting, and it surfaces exactly once per episode, latched until a later status event ends it. Suppressing the finished-but-unclosed reading was deliberately not built; it would need a cross-home read this watcher has no business making. Tests now build the stall the way production reaches it, by backdating the status log alone, with no turn marker and no busy record. The remote case became a positive one: a remote-shaped mate with no locally readable endpoint must surface identically, which is what proves the detector needs no launch wiring. The busy-endpoint case was removed because it had to arm a record no secondmate ever has. The secondmate idle-by-default contract, the meaning of paused:, the stale exemption, the single supervision cycle, and durable wake acknowledgement are all unchanged.
…te, pin reason key
4 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Intent
Fix a defect the captain reported on 2026-08-16 after seeing no fleet movement for a long stretch, and then correctly asked why no wake-up or verification mechanism had caught it.
The incident, measured: the tcglas second mate wrote a non-terminal plan into its status stream - "working [key=aterrizaje-nueve]: ... the remaining six relaunch ONE BY ONE as the earlier ones land ..." - and ended its turn, going idle. Nothing relaunched anything. During that stretch: 21 panes, ZERO active workers, five open PRs frozen (#391, #388, #378, #374, #368). The watcher was beating every second, so supervision was alive and still nothing woke it. The captain found it by looking at his screen, which is exactly what supervision exists to prevent.
Why no detector saw it, with the code evidence:
working:line is not captain-relevant and had already been presented, so the heartbeat absorbs it correctly per its own contract.working:as progress when it was an intention. A non-terminal status describes what the worker THINKS it will do; it never proves the work was launched.So this is a class of failure, not one case: a second mate can declare pending work and sit still forever with no mechanism noticing, and the more disciplined it is about writing its plan before executing, the more invisible the stall.
What the captain asked to be built, and the constraints he set:
working:is the real case;paused:explicitly does NOT count, because it declares a legitimate external wait and already has its own re-surface mechanism), and (b) its endpoint has been idle longer than its OWN configurable threshold, and (c) that non-terminal state was not followed by any later event.staleand fromsignal, so the main firstmate reconciles and pushes the work forward instead of waiting for a notice that never comes.paused:with a declared external wait must NOT trigger this, and that must be proven by test.done:,failed:,needs-decision:,blocked:) and idle is healthy and must not trigger either.Delivery rules the captain set:
working:with declared work plus an endpoint idle past the threshold => a wake is queued; and the three negative cases above => NOTHING is queued. Without those negatives the fix is worse than the defect.paused:.Later requirement added by the captain after the implementation commit: the PR body must carry the EVIDENCE that the pre-existing test failures fail identically at the base commit e518906, because that specific claim ("these failures are pre-existing and unrelated") is not accepted without proof. Five suites fail on this machine at base commit e518906, unmodified and unrelated to this change: tests/fm-calm-pi-extension.test.sh, tests/fm-kimi-harness.test.sh, tests/fm-turnend-guard.test.sh, tests/fm-wake-queue.test.sh, and tests/fm-watch-triage.test.sh (its
test_exited_declared_pause_is_bounded_but_live_gate_surfacescase). The last one is a timing-sensitive pre-existing case whose 1.5s window is too short on this machine; because the suite aborts on first failure, it prevents the new secondmate cases from being reached in a full-suite run, so those were verified individually against real fm-watch.sh subprocesses. Fixing those five is deliberately OUT OF SCOPE for this change.Decisions and tradeoffs made while implementing, that a reviewer reading only the diff would not know:
checkwake KIND and a distinct reason stringcheck: secondmate-stalled <id>: ..., rather than inventing a fifth wake kind. Reason: the wake-kind vocabulary is fixed at signal|stale|check|heartbeat in bin/fm-wake-lib.sh and is also filtered in the away-mode daemon's handle_durable_wakes; a new kind would have to be threaded through the queue, the drain, the daemon dispatch and the docs.check:already means "act on the named poll result" and the daemon's classify_check ALWAYS escalates, so away mode cannot self-handle-and-swallow it. The captain's requirement was that the REASON be distinguishable from stale and signal, which it is.stale:reason: the mate's idle endpoint is healthy by design and remains exempt from the stale loop, so the finding is the unresumed DECLARATION, not the quiet endpoint.workingphase has no contract requiring a keyed terminal event to close it, so an open keyed phase routinely outlives the work itself - the exact false signal already documented in the registered-secondmate current-state section of docs/architecture.md. Using the fold would resurrect that known false positive as a wake source.check:wake bullet) because its token cost is paid by every session of every fleet member; the rationale and mechanics live in bin/fm-classify-lib.sh, the bin/fm-watch.sh header, docs/architecture.md, and docs/configuration.md per the knowledge-placement decision tree in firstmate-coding-guidelines.=== RE-SCOPE (captain's decision, supersedes the endpoint-based approach above) ===
Two earlier rounds tried to read the secondmate's ENDPOINT for evidence of idleness. Both failed on a fact neither the captain nor I had verified: a secondmate can NEVER carry parent-side turn-end or busy wiring, because bin/fm-spawn.sh:2304-2575 excludes kind=secondmate from all of it. That exclusion is DELIBERATE and still valid - a secondmate is launched as a PRIMARY firstmate session in its own home (bin/fm-spawn.sh:1126 loads the primary turn-end guard and watcher extensions; docs/turnend-guard.md), so parent-owned worker wiring would sit on the turn boundary that keeps that mate's own supervision alive, with a documented wedge precedent. The captain explicitly refused to revert it, and explicitly REJECTED inferring idleness from rendered pane text as a fragile surface and a false-alarm source.
So the captain re-scoped, and this is the delivered design: do not measure the endpoint at all, measure the SILENCE OF THE MATE'S OWN STATUS LOG. A live secondmate writes status events; one that declared work and went quiet stops writing them. The condition is: the LAST status event uses the non-terminal verb declaring pending work, AND that file has not grown for longer than a generous configurable threshold. Status files are append-only, so the log's own age IS the time since its last event.
Captain's conditions on the re-scope, all of which the diff satisfies:
Consequences deliberately accepted, and recorded in the code:
The captain also requires the PR body to record two things beyond the fix itself: (a) the evidence that five failing suites fail identically at base commit e518906, because that claim is not accepted without proof, and (b) the finding that until now a secondmate emitted NO turn-end and NO busy signal to its parent at all, which explains why its supervision was blind and matters more than the detector. Note that docs/architecture.md already described a secondmate "bare turn-ended signal" that cannot occur, so the documentation had assumed this signal for some time.
What Changed
bin/fm-classify-lib.sh: aworkingverb constant (FM_CLASSIFY_DECLARED_WORK_VERB), the pure decision helpersstatus_declares_work/status_work_declared_unresumed/status_declared_work_stalled, and a generousFM_DECLARED_WORK_SILENCE_SECS_DEFAULT=1500threshold — one definition sourced by both the always-on watcher and the away-mode daemon.bin/fm-watch.shto check every secondmate whose last status event still declares work while its own (append-only) status stream has stayed silent pastFM_DECLARED_WORK_SILENCE_SECS, and to queue a single-fire, latch-guarded (state/.stalled-*)check: secondmate-stalled <id>reconciliation wake for that combination; the endpoint is never read, the secondmate stale exemption is unchanged,paused:/terminal/below-threshold cases stay silent, and a non-numeric threshold is rejected loudly and falls back to the default rather than disabling the check.FM_DECLARED_WORK_SILENCE_SECS/FM_CLASSIFY_DECLARED_WORK_VERBknobs (docs/architecture.md,docs/configuration.md,AGENTS.mdstate inventory andcheck:handling), and added tests reproducing the 2026-08-16 stall plus the three negative cases (declared pause, terminal last event, below threshold), malformed-threshold fallback, remote-mate parity, single-fire behavior, and away-mode escalation of the new reason.Risk Assessment
✅ Low: The change is a well-bounded, additive, fail-safe detector that preserves all existing supervision contracts and default behavior (the verb single-sourcing is a no-op at default), gates cheaply cheapest-first, resolves every read race toward not-firing, and is comprehensively covered by behavioral tests for the positive incident and all required negatives, so it satisfies every intent constraint with no substantiable defects.</risk_rationale>
["Read full branch diff bdae21e..c1a39e9 across all 7 changed files", "Traced poll-loop control flow and key computation in bin/fm-watch.sh", "Verified age_of / last_status_line / status_line_verb semantics", "Confirmed .status writes are append-only across the tree so silence-age is sound", "Verified classify_check always-escalates plus is_wake_reason / handle_durable_wakes acceptance in fm-supervise-daemon.sh", "Confirmed fm_wake_append dedup-at-drain and enqueue-then-latch-then-wake ordering", "Confirmed BASH_SOURCE guard ordering and fm-classify-lib.sh sourced before new constants are used", "Enumerated all callers of status_is_captain_relevant and the new classifier functions to confirm no default-behavior regression", "Checked test helpers and referenced constants exist"]
Testing
Drove real fm-watch.sh and fm-wake-drain.sh subprocesses via targeted drivers (the full triage suite aborts on a pre-existing failure before the new cases, so I ran only the relevant ones, as the intent describes). The exact incident — last status
working [key=aterrizaje-nueve]: …then silence past threshold — surfaces under its owncheck: secondmate-stalledreconciliation reason, is not misclassified as stale/signal, and is queued durably; the three required negatives (paused:, terminal last event, below-threshold) queue nothing; remote mates are detected by the same status-silence rule, the wake is single-fire per episode, a malformed threshold is rejected loudly and supervision continues on the default, and away mode escalates the stall through the same shared classifier library serving both modes. Regression confirmed: the reproduction test fails at base commit bdae21e and passes at the target commit c1a39e9. This is a CLI/supervision-daemon change with no rendered UI surface, so evidence is CLI test transcripts rather than screenshots. Restored sources and removed transient drivers; worktree is clean.Evidence: New secondmate declared-work cases (9/9 ok, real fm-watch.sh subprocesses)
== reproduction: declared work then silence past threshold surfaces == ok - a secondmate whose last event declares work wakes firstmate once its own status stream goes silent past the threshold == negative: below threshold stays silent == ok - declared work inside the threshold stays silent... == negative: declared pause never stalls == ok - paused: keeps its meaning - a declared external wait never becomes a declared-work stall == negative: terminal last status never stalls == ok - an idle secondmate whose last event is terminal is healthy and never surfaces as a stall == remote mate detected by same rule == ok == single-fire per episode == ok == away mode receives the stall == ok ALL_NEW_SECONDMATE_CASES_DONEEvidence: Regression proof: reproduction test FAILS at base commit bdae21e (pre-fix)
== BASE COMMIT (pre-fix): reproduction case should FAIL == not ok - watcher never surfaced a secondmate whose declared work went silent past the threshold EXIT=1Evidence: Away-mode daemon escalation case (ok)
== away-mode: declared-work stall escalates under its own reason == ok - away mode escalates a declared-work stall under its own reason, leaving no stale or pause tracking DAEMON_CASE_DONEPipeline
Updates from git push no-mistakes
✅ **intent** - passed
✅ No issues found.
✅ **Rebase** - passed
✅ No issues found.
✅ **Review** - passed
✅ No issues found.
✅ **Test** - passed
✅ No issues found.
Targeted driver over the new watcher cases (real fm-watch.sh subprocesses):test_declared_work_stall_classifier,test_secondmate_declared_work_stall_surfaces,test_secondmate_declared_work_below_threshold_is_silent,test_secondmate_declared_work_malformed_threshold_falls_back,test_secondmate_declared_pause_never_stalls,test_secondmate_terminal_status_never_stalls,test_secondmate_remote_declared_work_stall_surfaces,test_secondmate_declared_work_stall_fires_once_per_episode,test_secondmate_declared_work_stall_surfaces_in_afk— all okAway-mode daemon casetest_handle_wake_secondmate_stall_escalates(tests/fm-daemon.test.sh) — ok: reason recognized as a wake and escalated, no stale/pause marker left behindRegression proof: swapped bin/fm-classify-lib.sh and bin/fm-watch.sh to base commit bdae21e, re-rantest_secondmate_declared_work_stall_surfaces->not ok(watcher never surfaced the stall); restored HEAD sources -> okVerified clean worktree after restoring sources withgit checkout HEAD -- bin/fm-classify-lib.sh bin/fm-watch.shand removing transient test drivers✅ **Document** - passed
✅ No issues found.
✅ **Lint** - passed
✅ No issues found.
✅ **Push** - passed
✅ No issues found.