Skip to content

fix(bin): prevent silent second-mate intake stalls - #2640

Open
tiago-peixoto wants to merge 8 commits into
kunchenguid:mainfrom
tiago-peixoto:fm/firstmate-secondmate-intake-dies-silently
Open

fix(bin): prevent silent second-mate intake stalls#2640
tiago-peixoto wants to merge 8 commits into
kunchenguid:mainfrom
tiago-peixoto:fm/firstmate-secondmate-intake-dies-silently

Conversation

@tiago-peixoto

Copy link
Copy Markdown

Intent

A second mate's event intake can die permanently and silently. It did, for thirteen hours: on 2026-08-19 at 00:16 the Artemis review coordinator's registered long-poll source emitted event 102 correctly, then everything stopped - the wake stayed queued and unhandled in that coordinator's own queue, .watcher-down was set, no watcher process remained, and nothing reached the lane again: not the captain's review requests, not new heads, not check failures. The captain found it by looking at GitHub himself, which is exactly what the intake exists to prevent. Fix the two defects behind that, together, so neither assumes the other is handling it.

Defect 1 - nothing outside checks that a second mate's supervision is alive. The loop is circular: the mate is woken by its watcher, its watcher is armed by its own session, and its session runs only when woken, so once the watcher stops WITH A WAKE ALREADY QUEUED, nothing inside that home can restart it, because waking is precisely what it lost. And nothing outside looks: the only outside check was secondmate_liveness_sweep in bin/fm-bootstrap.sh, which runs at session start only and proves exactly one thing - that an agent process exists. The fleet asked "is the agent alive?" and never asked "is its wake loop working?". An idle second mate and a deaf one are indistinguishable from outside; that is the whole defect. The task required establishing what a supervision-alive check should actually ASSERT before writing one, weighing at least these candidates - a queued-but-undrained wake older than some bound, a stale watcher beacon, a .watcher-down marker nobody cleared - preferring whichever cannot itself go stale silently, and SAYING WHY the others were rejected.

Defect 2 - handling and re-arming are coupled. bin/fm-procevent.sh states it: "a handling adapter is expected to re-arm its own next source." The source is one-shot by design and exits when it emits, which is correct, so if the event is never handled the source is never re-armed and intake ends PERMANENTLY rather than falling behind. A delay is acceptable; a silent permanent stop is not. Whatever changed here had to preserve the existing guarantee that a captured result stays eligible for re-announcement until explicitly acknowledged - acknowledgement must not be weakened to solve re-arming.

Explicit refusals the captain set, which the diff must be read against: do NOT build a daemon, a poller-watcher, or a monitoring service - we have this problem precisely because a monitor with no supervision was trusted, and adding a second unsupervised monitor repeats it one level up, so prefer a check that runs on paths that already run. Do NOT reach across homes to arm anything: bin/fm-watch-arm.sh is single-home by design because reaching across homes would kill sibling homes' watchers, and that boundary stays. Do NOT claim guarantees the runner does not make - its contract explicitly disclaims at-least-once and lossless delivery - and if a fix narrows a window rather than closing it, say which window remains. Every fix needed a test proven RED against current behaviour first.

Explicitly OUT of scope and deliberately not addressed: the third defect in this path, a source refreshing its snapshot without emitting and losing a transition, recorded as artemis-pr4224-intake-miss-2026-08-14.md in the coordinator's own home. It lives in that mate's own tooling and is routed separately; it is not fixed here and must not be assumed fixed.

Decisions and tradeoffs made while doing the work, which a reviewer reading only the diff would not know:

  • The chosen signal is the age of the OLDEST unacknowledged row in the second mate's own state/.wake-queue. Each row carries its own append epoch inside the record, written once and never rewritten, and a row leaves the queue only through the generation-bound acknowledgement that follows handling - so it is positive evidence produced by the very mechanism that failed, and an empty queue reads as a healthy idle mate while an old row reads as a deaf one. The two rejected candidates and their reasons are recorded in bin/fm-secondmate-wake-check.sh's header: a stale liveness beacon (state/.last-watcher-beat) is written only while a watcher process runs, so "stale" and "absent" are the normal steady state of a healthy idle home and asserting on it reproduces the idle/deaf confusion one level down; and an uncleared state/.watcher-down marker is republished on every watcher close AND every durable queue append, so its mtime tracks the last append rather than the age of the outage, pending:downtime is its ordinary transient state, and its generation token carries a timestamp only as an opaque uniqueness component that docs/watcher-continuity.md deliberately does not promise as a clock.
  • The check is hosted on paths that already run - bin/fm-watch.sh's ordinary poll cycle and bin/fm-bootstrap.sh's session-start liveness sweep - modelled on the existing bin/fm-inactive-reconcile.sh adjunct, not as a process of its own. It reads only: it never writes into another home, never starts a process, and never arms a watcher anywhere. Recovery stays firstmate nudging that second mate through its own endpoint so the mate's own session arms its own watcher; secondmate-provisioning carries that procedure and the never-arm-across-homes rule.
  • Deduplication of the stall report deliberately uses the primary's own durable wake queue (a key stays queued exactly while unacknowledged) rather than a new marker file, so a stall reports once and stays quiet until the supervisor has actually handled and acknowledged it, then re-reports on its own bounded cadence if the outage persists. A home with no recorded second mate is a true no-op that generates no state at all.
  • Windows deliberately left open and documented rather than papered over: the check finds a wake loop that received work and did not consume it, not one that is dead with nothing arriving; a remote second mate's home is on another machine and is passed over rather than judged or called healthy; detection is bounded by the stall threshold plus one scan interval, so a permanent outage becomes a delayed one; and it proves the queue is not draining, not why. The scan deliberately takes NO lock in the other home, because acquiring another home's wake-queue lock would both write into that home and let a wedged foreign lock stall the scan; reading unlocked is safe for this verdict because appends only add a row at the end, so a partially written newest row cannot change which row is oldest, and malformed rows are ignored.
  • For defect 2, reconcile now retries the adapter's own autohandle for every durably captured result with no handled acknowledgement yet, on the same idempotent seam and in the same adapter-declared order that start uses. This changes nothing about acknowledgement: an application that fails still leaves its result unhandled, still announced, and still eligible for the next retry, and only the adapter's own handled call ends re-announcement. What it removes is the assumption that a handler will arrive at all.
  • That introduced a second machine-wide per-source boundary for adapter APPLICATION (fm_procevent_apply_lock_path), deliberately separate from the source lock. It cannot be the source lock, because an adapter applying a terminal capture re-arms its next source and that takes the source lock - which is exactly why the runner has always called autohandle outside it. Two callers now apply (a runner right after its own capture, and reconcile re-driving a result no handler picked up) and without this boundary both could pass the handled check before either recorded the acknowledgement, running one adapter's apply step twice; that double application was observed before the lock was added. The runner waits on the boundary for its own capture; reconcile declines a held generation and retries on a later cycle, because a reconcile that blocks on someone else's application would stall the watcher cycle it runs on.
  • The two halves are deliberately independent, and each header says so: reconcile is driven by a home's own watcher, so a home whose wake loop has stopped runs no reconcile at all - the wake-loop check is what notices that from outside, while reconcile recovers intake without waiting for a handler.
  • bin/fm-procevent.sh's usage() now derives its help from the leading comment block instead of a hardcoded sed line range. The header grew past that range, which silently truncated the published interface the help IS; this is a deliberate fix for that class of rot, not an unrelated refactor.
  • tests/fm-remote-reply.test.sh and the publish-src block of tests/fm-procevent.test.sh were updated because they encoded the OLD contract, in which reconcile only re-announced and a handler had to apply. Under the new contract reconcile applies and re-arms itself, so those blocks now assert that stronger behaviour (published=0 with the capture applied, acknowledged, and the relay re-armed) and the handler's own call is asserted as idempotent on top. Where an always-ready source would otherwise have a detached poll race the later assertions, the same retire-before-reconcile discipline the suite already uses elsewhere was applied.
  • Several test suites in this local environment fail identically on a pristine upstream/main worktree and are NOT caused by this change: tests/fm-wake-queue.test.sh, tests/fm-watch-triage.test.sh, tests/fm-on.test.sh, tests/fm-secondmate-sync.test.sh, tests/fm-secondmate-safety.test.sh, the session-bootstrap family (8 of 9 scripts, byte-identical failure list), and load-dependent flakes in tests/fm-watch-arm.test.sh and tests/fm-watch-checkpoint.test.sh. Two of them are already owned by in-flight upstream branches - fm/fm-procevent-test-flake-r1 for the procevent detached-runner claim assertion, and fm/fm-watcher-honor-paused-review-scout-r1 for the declared-pause busy-pane behaviour - so they were deliberately NOT duplicated here, to avoid conflicting with that in-flight work. They are being reported separately and are not a reason to weaken validation.

Delivery constraints: this is a permanent-fork divergence topic branched from upstream/main. The ordinary registration targets official upstream. Never force-push or rewrite this published topic branch, and do not routinely merge official upstream or fork main into it.

What Changed

  • Detect stalled local second-mate wake loops from the oldest unacknowledged wake during existing watcher and bootstrap paths, deduplicating durable alerts without cross-home writes or re-arming.
  • Retry adapter autohandling for unacknowledged process-event captures during reconciliation, using a separate validated per-source apply-lock namespace while preserving acknowledgement and re-announcement semantics.
  • Hard-bound watcher shell-outs—including reconciliation, scans, checks, backend probes, and pending-reply work—and preserve retry state when dependencies time out.

Risk Assessment

✅ Low: The prior blocking issues are resolved, and the durable queue, acknowledgement, retry, timeout, logging, and lock-namespace invariants are consistently preserved across the changed paths.

Testing

No prior baseline log was supplied; targeted automation, base RED checks, concurrency/load stress, and two end-to-end CLI/state scenarios all succeeded after two test-only macOS/load fixture fixes.

Evidence: Second-mate wake-loop end-to-end transcript
Healthy idle mate probe: ok
Live-but-deaf mate probe: stalled 4000 7
Primary scan output:
actionable: check: secondmate-wake-stall deaf: its durable wake queue has gone 4000s unconsumed (oldest sequence 7); a live agent process does not prove a live wake loop
Primary durable notification:
kind=check key=secondmate-wake-stall:deaf:7 payload=check: secondmate-wake-stall deaf: its durable wake queue has gone 4000s unconsumed (oldest sequence 7); a live agent process does not prove a live wake loop
Repeat startup scan while notification remains unacknowledged: quiet (deduplicated)
Deaf mate home changed by the external check: no
Remote route reported locally: no (left uncovered)
Evidence: Process-event reconciliation end-to-end transcript
Capture while adapter application is unavailable:
retired: evidence-src (adapter classified the captured result terminal)
not-autohandled: evidence-src (left for the handler; still unacknowledged)
captured: /var/folders/r8/cylyt7xd7t50y9x5wc05my380000gn/T/no-mistakes-evidence/01M0D9ZY3TKYFREY0BNSZZ62YX/process-event-reconcile.iqBaUq/home/state/procevent-inbox/evidence-src.1.result
After capture: registration=absent handled=absent durable-announcements=1
Reconcile while application is still unavailable:
reconciled: published=1 started=0 stopped=0 uncertain=0
Still pending: registration=absent handled=absent durable-announcements=2
Reconcile after the adapter dependency recovers, with no handler call:
reconciled: published=1 started=1 stopped=0 uncertain=0
Recovered: registration=present runner=present handled=present applied-count=1 durable-announcements=3
Next reconcile after acknowledgment:
reconciled: published=0 started=0 stopped=0 uncertain=0
After acknowledgment: applied-count=1 durable-announcements=3
Explicit acknowledgment retry: already-handled: evidence-src 1
Evidence: Regression RED proof against base
Baseline: 03bb1d8b78a8632ae2d9cea4c10868eb100e885e
Defect 1 regression test against baseline:
exit=1
tests/fm-secondmate-wake-check.test.sh: line 50: /var/folders/r8/cylyt7xd7t50y9x5wc05my380000gn/T/no-mistakes-evidence/01M0D9ZY3TKYFREY0BNSZZ62YX/red-base.AAvoMP/bin/fm-secondmate-wake-check.sh: No such file or directory
not ok - a wake queue unconsumed for thirteen hours was not reported as stalled (missing: 'stalled 46800 1')
--- output ---

Defect 2 regression test against baseline:
exit=1
      $take = $count if $take > $count;
      if ($take > 0) {
        my $offset = 0;
        while ($offset < $take) {
          my $count_written = syswrite(STDOUT, $buffer, $take - $offset, $offset);
          exit 2 unless defined $count_written;
          $offset += $count_written;
        }
        $written += $take;
      }
      $truncated = 1 if $take < $count;
    }
    exit($truncated ? 3 : 0);
  ' "$MAX_OUTPUT_BYTES" > "$out"
ok - public start never claims an inherited caller process group
ok - an unhandled result survives restart and repeat drains, and only explicit acknowledgement stops its re-announcement
ok - publication cannot race a handled acknowledgement
ok - handled acknowledgement creation is private and fails safely
grep: /var/folders/r8/cylyt7xd7t50y9x5wc05my380000gn/T//fm-procevent-tests.EPXsKF/hpublish/state/applied: No such file or directory
not ok - reconciliation announced the capture but never re-drove its application

Pipeline

Updates from git push no-mistakes

✅ **intent** - passed

✅ No issues found.

✅ **Rebase** - passed

✅ No issues found.

🔧 **Review** - 2 issues found → auto-fixed (3) ✅
  • 🚨 bin/fm-procevent.sh:575 - The intent requires that “reconcile declines a held generation and retries on a later cycle” so it cannot stall the watcher. However, nowait covers only the apply-lock acquisition; the adapter then runs synchronously. The real remote-reply adapter waits on another lifecycle lock and may perform an unbounded SSH-backed document fetch, while fm-watch.sh invokes reconcile synchronously. A held lifecycle lock or wedged remote command can therefore stop the watcher cycle indefinitely and recreate deaf intake. Bound or detach actual adapter execution at this shared reconcile boundary while preserving retries and acknowledgement semantics.
  • ⚠️ bin/fm-procevent.sh:202 - The new apply-lock path creates the claim root but does not enforce the existing non-symlink invariant used by fm_procevent_source_lock_acquire. With a pending capture and symlinked FM_PROCEVENT_CLAIM_ROOT, reconcile can create lock state through the redirected root and execute the adapter even though its later register/handled operations reject that root, potentially repeating application. Add a shared apply-lock acquisition helper that validates the source ID and requires a real, non-symlink root before either wait or try-lock.

🔧 Fix: Bound supervision calls and hardened apply locks
4 errors still open:

  • 🚨 bin/fm-watch.sh:743 - The accepted property says “nothing the supervision loop SHELLS OUT TO can stop it from completing its cycle,” with only the wake-queue lock wait left residual. fm_backend_wait_transition remains outside fm_run_timed; its Herdr path performs unbounded herdr session list and agent get calls, so a server wedged after capability probing can still stop the watcher permanently. Recommend bounding this shared event-wait boundary; otherwise explicitly add it to the residual contract.
  • 🚨 C9 explicitly requires the wake-queue-lock residual to appear “BOTH in the commit message body AND” the watcher header. The header contains it, but commit a5e7298 has only its subject and no body. Record the required residual in a delivery commit body without rewriting published history.
  • 🚨 bin/fm-watch.sh:726 - The capability probe now runs in a fresh shell that sources the canonical backend library, so the registered fm-supervision-events.test.sh overrides are never called; CAP_CALLS remains zero and its memoization contract cannot pass. Move those fixtures to a subprocess-visible seam while retaining the one-probe assertion.
  • 🚨 bin/fm-pending-reply-lib.sh:670 - The pending-reply probes now run in fresh shells, bypassing the backend fixtures in fm-pending-reply.test.sh that drive fallback-idle, harness-scoping, and observation-cache assertions. Convert them to PATH/executable or otherwise subprocess-visible fakes.

🔧 Fix: Bound remaining supervision calls
3 errors still open:

  • 🚨 bin/fm-watch.sh:1018 - Kill-after timeouts can exit 137 when KILL is required, but the new retry branch recognizes only 124. A TERM-ignoring check can therefore have partial output treated as complete and advance .last-check. Normalize 137 to the timeout outcome before consuming output or updating bookkeeping; update the timeout fixture to emulate that status.
  • 🚨 bin/backends/herdr.sh:2941 - The event-only deadline was added to shared fm_backend_herdr_agent_status_raw, also used inside fm-crew-state and submit confirmation. In those watcher-owned subprocesses triage_log is undefined, so a 10-second timeout becomes empty output with exit 0 and the outer boundary records no required timeout log. Use an event-specific bounded probe or propagate the timeout status to the logging parent.
  • 🚨 bin/fm-procevent-lib.sh:93 - The new apply-lock namespace collides with valid source-lock names: the apply lock for remote-reply-a and source lock for remote-reply-a.apply both resolve to remote-reply-a.apply.lock, because dots are allowed in source IDs. A long application can consequently block an unrelated source's retirement or re-arm. Put source and apply locks in disjoint filename namespaces or validated subdirectories.

🔧 Fix: Normalize bounded supervision failure handling
✅ Re-checked - no issues remain.

✅ **Test** - passed

✅ No issues found.

  • bin/fm-test-run.sh tests/fm-secondmate-wake-check.test.sh tests/fm-secondmate-liveness.test.sh tests/fm-procevent.test.sh tests/fm-remote-reply.test.sh
  • bin/fm-test-run.sh tests/fm-watch-bounded-calls.test.sh tests/fm-pending-reply.test.sh tests/fm-supervision-events.test.sh after test-fixture fixes
  • 30-iteration independent-process pending-reply concurrency stress and five consecutive FM_BOUNDED_CASE=recovery_send repetitions
  • Current regression tests overlaid on base 03bb1d8b78a8632ae2d9cea4c10868eb100e885e, confirming both defects are RED before the fix
  • Manual fm-secondmate-wake-check.sh probe/scan --startup scenario with idle, stalled, remote-route, deduplication, and foreign-home integrity controls
  • Manual terminal-capture/reconciliation scenario using the real fm-procevent.sh, showing failed application remains pending, later reconciliation re-arms and acknowledges without a handler, and subsequent reconciliation is idempotent
✅ **Document** - passed

✅ No issues found.

🔧 **Lint** - 1 issue found → auto-fixed ✅
  • ⚠️ linter found issues (exit code 1)

🔧 Fix: Suppress intentional child-shell and subshell lint findings
✅ Re-checked - no issues remain.

✅ **Push** - passed

✅ No issues found.

…ng re-arm to handling

A second mate's event intake can die permanently and silently, and did for
thirteen hours. Two independent defects, fixed so neither assumes the other is
handling it.

Nothing outside a second mate checked that its supervision was alive. The loop
is circular - the mate is woken by its watcher, its watcher is armed by its own
session, and its session runs only when woken - so once the watcher stops with a
wake already queued, nothing inside that home can restart it. The only outside
check proved one thing: that an agent process exists. An idle second mate and a
deaf one answer that identically.

bin/fm-secondmate-wake-check.sh asks the other half: is that home still
CONSUMING its own durable wake queue? The signal is the age of the oldest
unacknowledged row. Each row carries its own append epoch, written once and
never rewritten, and leaves the queue only through the generation-bound
acknowledgement that follows handling - so it is positive evidence produced by
the mechanism that failed, and an empty queue is a healthy idle mate. The script
header records why the two alternatives were rejected: a stale liveness beacon
is the normal steady state of an idle home, and the .watcher-down marker is
republished on every queue append so its mtime tracks the last append rather
than the outage.

It runs as an adjunct to paths that already run - the watcher's ordinary poll
cycle and the session-start liveness sweep - not as a monitor of its own, and it
only reads. Arming a watcher across homes stays forbidden; recovery is firstmate
nudging that second mate through its own endpoint so it arms its own watcher.

Separately, bin/fm-procevent.sh retires a spent registration at capture for an
adapter whose result is terminal for that exact registration, while re-arming
the next source lives inside that adapter's application step. One failed
application therefore ended intake permanently rather than delaying it.
`reconcile` now retries `autohandle` for every captured result with no handled
acknowledgement yet, on the same idempotent seam and in the same
adapter-declared order. Acknowledgement is unchanged: a failed application still
leaves its result unhandled, announced, and eligible for the next retry. Because
two callers can now reach one generation, application is serialized per source
at a boundary separate from the source lock the adapter's own re-arm takes; the
runner waits for it, reconcile declines and retries rather than blocking the
watcher.

Windows that remain, deliberately: the check finds a wake loop that received
work and did not consume it, not one that is dead with nothing arriving; a
remote second mate's home is not locally readable, so those routes stay
uncovered; detection is bounded by the stall threshold plus one scan interval;
and reconcile itself runs from a home's own watcher, so a deaf home's intake
stays down until its wake loop is restored - which is exactly why the two halves
are independent.

Both fixes were proven red first: a live agent whose queue sat unconsumed for
thirteen hours was reported "already live", and a terminal capture whose
application failed left no registration and never re-armed across repeated
reconciles.
The supervision loop now bounds every call it shells out to except the wake-queue lock wait.

That wait cannot be bounded additively: fm_lock_acquire_wait is a sourced shell function running in the watcher process rather than a subprocess, so fm_run_timed cannot wrap it. Its two in-loop callers, procevent_surface_queued and resurface_after_downtime via fm_recovery_marker_arm_check, deliver an actionable wake by calling wake, which exits the watcher process. Moving those callers into a bounded subshell would swallow that wake and recreate the silent loss this change prevents.

Dead holders are already reclaimed by fm_lock_try_acquire after FM_LOCK_STALE_AFTER. The remaining gap is only a live holder that never releases, a different defect filed as separate work.

Re-derivation found two entries previously marked already bounded were wrong: fm_backend_wait_transition and run_check_process. It also found the omitted crew_absorb_class to fm-crew-state.sh call. All three are fixed in this pass.
The supervision loop is now bounded at every call it shells out to except the wake-queue lock wait.

That wait cannot be bounded additively: fm_lock_acquire_wait is a sourced shell function running in the watcher process rather than a subprocess, so fm_run_timed cannot wrap it. Its two in-loop callers, procevent_surface_queued and resurface_after_downtime via fm_recovery_marker_arm_check, deliver an actionable wake by calling wake, which exits the watcher process. Wrapping them in a bounded subshell would swallow that wake and recreate the silent loss this change prevents.

The dead-holder case is already covered because fm_lock_try_acquire reclaims an abandoned hold after FM_LOCK_STALE_AFTER. The true residual is only a live holder that never releases, which is a different defect filed as its own work.

Two entries previously marked already bounded were re-derived and found wrong: fm_backend_wait_transition and run_check_process. One call was omitted entirely: crew_absorb_class to fm-crew-state.sh. All three were fixed in this delivery.

This follow-up also normalizes kill-after status, preserves shared Herdr helper contracts, and separates apply locks from source locks.
@tiago-peixoto
tiago-peixoto force-pushed the fm/firstmate-secondmate-intake-dies-silently branch from 34a7200 to 0092a5e Compare August 19, 2026 18:45
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