fix(bin): prevent idle recovery loops without stranding wakes - #4819
Merged
kunchenguid merged 2 commits intoSep 27, 2026
Merged
Conversation
jokim1
changed the title
|
Please take it live it - fixes the problem able to see the agent - thought i see a lot of roken use. |
jokim1
force-pushed
the
fm/fm-thrash-upstream-proposal
branch
from
September 27, 2026 09:55
c16b360 to
cb0af65
Compare
Owner
|
Speaking as Kun's firstmate: Verdict: Whole thread + tip vs main Contract-class: new-default (independent tip-vs-main; do not trust "fix" / restore framing). Main's VISION.md (per-rule)
NM/attestation: MATCH ( |
Contributor
Author
|
Fixes #2028 - this PR is the fix for the supervision thrash described there. |
Owner
|
Speaking as Kun's firstmate: Captain moved on without answering the close-vs-leave card for this new-default (idle recovery: empty-queue quiet + append-source reopen; FM-LEARN-4627). Skip ≠ decline. Leave open. Do not merge. Do not re-card unless the captain asks or material new evidence appears. Firstmate flag: no (hold cleared as skip). |
Owner
|
Speaking as Kun's firstmate: this is merged. Thank you @jokim1 — really appreciate you taking the time on this. |
RibatTRW
added a commit
to RibatTRW/firstmate
that referenced
this pull request
Sep 28, 2026
… no-mistakes run 36372069208) show conclusion action_required with 0 jobs and no logs because this is a fork PR (RibatTRW/firstmate) and GitHub is holding the workflow runs awaiting maintainer approval; that gate is external to the code and needs a maintainer to approve the runs. While verifying the change locally I found a real defect the approved CI run would hit: the PR's new test test_handling_delivered_rejects_a_superseded_generation failed deterministically. Invariant violated: reopen-announced (a non-successor/manual arm start) mints a fresh recovery generation only when the durable wake queue holds unrecovered work; an announced episode with an empty queue must be left untouched so idle arm starts never churn generations (the [ -s queue ] guard from kunchenguid#4819, relied on by bin/fm-watch.sh:2413 and covered by the append-reopens and announcement-bound sibling tests). The test called reopen with an empty queue and expected churn, so the fix establishes the queued-work precondition (append one wake, re-announce, re-read the generation) before asserting the reopen mints and the old confirmation mismatches. Test-only change, 14 lines in tests/fm-watch-arm.test.sh. Verified: fm-watch-arm.test.sh 25/25 ok on two consecutive runs, fm-pi-watch-extension.test.sh 55/55 ok, and shellcheck reports only one pre-existing warning outside the edited region
neel-mishra
pushed a commit
to neel-mishra/firstmate
that referenced
this pull request
Sep 28, 2026
…nguid#4819) * fix(supervision): prevent idle recovery loops without stranding wakes * no-mistakes(review): Remove unused wake-append rollback helper
neel-mishra
added a commit
to neel-mishra/firstmate
that referenced
this pull request
Sep 28, 2026
#3) * fix: grant Claude workers access to Firstmate task channels (kunchenguid#5884) * fix(bin): grant Claude workers their task-channel dirs via --add-dir Since Claude Code 2.1.257, a file-tool read (Read/Glob/Grep, and an Edit's mandatory prior read) of a path outside the working directories parks --permission-mode auto panes on a one-time interactive question, and a "Block" answer lands permissions.blockReadsOutsideWorkingDirectories in user settings, refusing the same reads even under bypass. Firstmate launches Claude with no --add-dir, so a secondmate's parent-home steering inbox and a ship or scout worker's launch record, steering inbox, brief dir, and code-root .agents/skills were all outside: workers wedged on the question the first time they read a steer. Every Claude launch, spawn and relaunch, in both permission modes, now grants exactly the task's channel directories: state/<id>.inbox for a secondmate (in the parent home), or state/operational-inbox, state/<id>.inbox, data/<id>, and the code root's .agents/skills for a ship or scout. Paths resolve to real paths and lazily created channel dirs are made before launch so the grant never names a not-yet-existing directory; the whole state/ is deliberately never granted. The grant keeps the bypass-mode launch argv changed on purpose: it also protects bypass workers against a machine-recorded Block answer. * no-mistakes(document): Consolidate Claude launch guidance in configuration reference * no-mistakes(document): Clarify Claude permission documentation reference * fix(bin): prevent idle recovery loops without stranding wakes (kunchenguid#4819) * fix(supervision): prevent idle recovery loops without stranding wakes * no-mistakes(review): Remove unused wake-append rollback helper * fix: reduce remote worker and polling helper process churn (kunchenguid#5889) * fix(bin): stop the remote-job worker busy-polling an idle queue The serving loop slept 50ms between passes and re-ran state preparation (chmod on every queue directory), the heartbeat publish, and the stale sweep on every pass. It now blocks on a worker.wake FIFO that staging, cancellation, and lane exit nudge, keeps a short fast-poll window after activity, refreshes the heartbeat at most once a second, and runs the sweep (which re-applies the queue directories' 0700 modes) at startup and then on a bounded interval. Lane-owned records are no longer re-read every pass. Measured with a fork/execve-interposing counter on a --serve worker in a disposable HOME and queue, bash 3.2, 20-second windows (the counter slows the old loop to about 5 passes a second, so real-host rates were higher): idle worker 146 forks/s, 61 execs/s -> 4.8 forks/s, 3.1 execs/s one running long job 232 forks/s, 100 execs/s -> 15 forks/s, 11 execs/s Stage-to-result latency for a no-op job, idle and back to back, stayed at about 0.8-1.2s in both versions (dominated by job execution, not pickup). * perf(bin): drop per-cycle forks from watcher, drain, and lock helpers The watcher, drain, inactive-reconcile scan, and branch-outcome reads forked small external commands on every cycle where bash can do the same work. - fm-wake-lib.sh gains fm_dirname_to, fm_basename_to, and fm_epoch_seconds_to, exact stand-ins for $(dirname --), $(basename --), and $(date +%s); the clock uses printf %(%s)T on bash 4.2+ and still forks date exactly once on stock macOS bash 3.2. - fm_lock_abs_path, fm_wake_signal_seen_path, fm_path_age, the watcher's age_of and wedge timer, and the recovery-marker line count use them or plain reads instead of dirname/basename/tr/date/wc. - window_to_task reads a meta file once instead of two grep | tail -1 | cut -d= -f2- pipelines per file per call. - fm-classify-lib.sh reads uname -s once at source time instead of in every status stat helper. - Libraries sourced every cycle derive their own directory without forking dirname, including the backend adapter siblings a subshell re-sources on each probe. tests/fm-fork-free-helpers.test.sh pins each replacement against the command it replaces on edge-case inputs, under every available bash and both the C and a UTF-8 locale; CI's stock macOS bash lane runs it under /bin/bash 3.2. Measured with a fork/execve-interposing counter in a disposable home, one tmux crew task, FM_POLL=1 (forks and execs per watcher cycle, per run otherwise): watcher cycle bash 5.3 299/138 -> 199/66 bash 3.2 341/146 -> 224/80 drain bash 5.3 492/238 -> 430/200 bash 3.2 567/250 -> 491/212 inactive scan bash 5.3 27/14 -> 17/4 bash 3.2 37/14 -> 17/4 branch-outcome bash 5.3 40/21 -> 35/16 bash 3.2 48/24 -> 38/19 * test: note the interpreter-expanded version probe for shellcheck * no-mistakes(review): Fix worker idle bounds and fork-free contributions snapshot * no-mistakes(review): Coalesce buffered worker wake nudges into one wake * no-mistakes(review): Coalesce wake nudges via pending marker so publishers never block * no-mistakes(review): Claim wake nudges atomically via noclobber pending marker * no-mistakes(review): Release abandoned wake claims only after a 30-second bound * no-mistakes(review): Drop worker wake FIFO; load path helpers side-effect free * no-mistakes(document): Document remote worker polling and preemption cadence * no-mistakes(ci): Fixed both failing CI shards: isolated remote and teardown test fixtures now include fm-path-lib.sh, which fm-wake-lib.sh requires. The three affected tests, fm-lint.sh, and git diff --check pass locally * fix: keep remote reply listeners and watcher cycles running (kunchenguid#5941) * fix: keep a successor watcher and remote-reply listeners across the gaps that dropped them A main-only supervision pass-through exited without leaving a watcher, and each remote-reply poll released its claim until the next cycle, so short-lived listeners stayed down. * no-mistakes(document): Clarify listener and supervision continuity documentation * no-mistakes(ci): Fixed the three Greptile findings: failed ingestion leaves one durable capture, failed reads exit instead of relistening, and the disposable-checkout guard rejects state paths outside the marked lab. Added behavioral tests; the remote-reply and watcher-lock suites, shell syntax checks, and git diff checks passed * no-mistakes(ci): Fixed a race in the wake-queue interruption test: it now waits for the drain to own the lock and enter handling before signaling it. The wake-queue suite, shell syntax check, and diff check pass * fix: make attended cutover outcome re-presentation check-first (kunchenguid#5925) * fix: date replayed branch outcomes and ask main to check current state first A captain outcome main never acknowledged is presented again, which after a harness or posture switch, or the first drain after the upgrade whose earlier presenter never advanced the read cursor, can be days after its situation settled. The replay read as fresh news, so a PR since merged looked ready. bin/fm-branch-outcome.sh now adds a "recordedAgo" age (minutes, hours, then days) to present and unprocessed rows, one owner of that wording for both presenters. The drain's BRANCH OUTCOMES captain lines and the Pi branch's processing request name that age and ask main to check the task's current state first; an outcome already settled needs only the acknowledgement, with nothing relayed to the captain. Nothing is adopted as processed, so a fresh home's first outcome is still presented until acknowledged. * no-mistakes(review): Absent processed marker reads 0; never adopt read cursor * no-mistakes(review): Require recordedAgo in Pi requests; report undated rows to main * no-mistakes(review): Keep recordedAgo on captain rows only in present output * no-mistakes(document): Correct cutover documentation and retire stale migration guidance * fix: keep settled branch outcomes out of main's reply to the captain A live Pi primary that took over a host-drain home received the carried-over outcomes dated and check-first, but its processing reply still told the captain about an outcome whose decision had since been answered. The request also claimed every outcome was already shown as an anchor entry in this transcript, which is false for an outcome carried over from before a restart or a switch of primary. The Pi processing request now says each outcome was recorded earlier and may already have been seen or handled, and that a settled outcome gets no captain-facing mention at all in the reply or any recap, not even that it is settled. The drain's BRANCH OUTCOMES header and the supervision docs state the same rule, and the tests check both delivered texts. * fix: scope main's outcome reply to what is still open Telling main what not to say about a settled outcome was not enough: in two live Pi trials the processing reply still told the captain that an answered decision was settled. Main now sorts the outcomes by current state first, and its reply to the captain covers only the still-open ones, written as if the settled ones had never been listed. With that framing three live Pi trials kept the settled outcome out of the reply and relayed the open one each time. The drain's BRANCH OUTCOMES header and the supervision docs use the same framing, and the tests check both delivered texts. * no-mistakes(review): Clarify that main acknowledges every presented captain outcome * no-mistakes(document): Clarify outcome cursor ownership across Pi and host * no-mistakes(ci): Fixed Pi replay by batching unprocessed captain outcomes oldest-first and acknowledging only through each batch. Verified a marker-less backlog over 1 MiB replays through all batches. A real-drain regression confirms an older keyed decision remains under OPEN DECISIONS after a newer branch row is acknowledged; the check-first instruction now names those decisions. Relevant targeted tests and branch-supervision tests passed; the full host suite timed out * no-mistakes(ci): Fixed the host drain’s check-first wording in bin/fm-wake-drain.sh; the CI fixture now passes. The full host suite passed the affected fixtures but timed out later. Syntax and diff checks passed * no-mistakes(ci): Fixed ci-1: abbreviated Pi outcome summaries now stay within 1,024 characters and include a row-specific full-outcome lookup command. The delivered instruction requires reading the full outcome before acting, relaying, or acknowledging it. The new extension-driver regression failed before the fix and passes now; the Pi and supervision-host suites pass * no-mistakes(ci): Corrected the batching sentence in docs/pi-supervision-branch.md. The cancelled CI check needs no code fix; its clean rerun passed. The Pi branch extension suite and git diff check passed * fix: restore primary rewakes after attended main-only closes (kunchenguid#5961) * fix: wake an idle Claude primary for attended main-only hand-backs An attended main-only pass-through confirmed a handling handoff for the successor it leaves running, which flipped the recovery marker to handling. The Claude Stop hook only rewakes main while that marker reads downtime, so the close reached no one and an idle primary slept with wakes queued. The pass-through now leaves the marker at downtime, and a close that turns main-only at its turn hands the consumed handoff back to downtime before it reaches main. Regression tests drive the real Stop hook around the real host on both paths and for the successor's own later close, and a new opt-in live guard proves it against an idle interactive Claude primary with a pre-fix negative control. * no-mistakes(review): Assert live lab Stop-hook registration via parsed settings JSON * no-mistakes(document): Correct supervision hand-back documentation * no-mistakes(ci): Fixed the failed downtime-write path so the Stop hook notifies main instead of silently dropping the close. Corrected the live guard’s tracked-hook check and added the requested at-turn main-only scenario. The new regression failed before the fix and passed after it; the host suite, syntax checks, and diff check pass. The credentialed live guard was not run in this CI phase because it writes outside the worktree * no-mistakes(ci): Fixed the Stop hook’s retry ordering: a crashed host gets its bounded retry before a non-crash hand-back failure is reported. The Stop-hook suite passes, including the crash regression. The host suite passed the failed-marker-write regression but timed out before completing; syntax and diff checks pass * Clarify live Claude login for opted-in tests (kunchenguid#5975) * fix(bin): ensure resumed worker launches enter their recorded worktree (kunchenguid#5916) * Fix worker launches to enter recorded worktrees * no-mistakes(review): placeholder * no-mistakes(document): Update agent-control.md worktree-refusal note to match new universal cd+assert * no-mistakes(review): Add regression tests for Orca spawn/relaunch worktree carve-outs * Fix PR relaunch and prelaunch cwd verification * no-mistakes(document): Fix docs/agent-control.md: worktree cwd check is pre-launch, not post-launch * fix: slim worktree launch change onto upstream main * fix(bin): make a Herdr task pane render before its launch is delivered A Herdr pane created with --no-focus is not rendered until its tab has been the active tab of a focused workspace once. Until then the launch still executes but `pane read` stays empty and Herdr's screen-based agent state never observes the worker, so a spawned worker's terminal reads blank and `agent prompt` stalls (Herdr issue kunchenguid#2449). The spawn now activates the task endpoint immediately before delivering the launch command and restores the exact prior focused workspace and tab right after the Enter, matching the tmux backend where `capture-pane` reads the live pane terminal directly. Adds bin/backends/herdr.sh task-rendering activation primitives, the fm-spawn wiring, an updated focus note in docs/herdr-backend.md, and a portable fake-CLI regression in tests/fm-backend-herdr.test.sh. --------- Co-authored-by: Kun Chen <3233006+kunchenguid@users.noreply.github.com> Co-authored-by: Joseph Kim <jokim1@gmail.com> Co-authored-by: Mehul Bhagwani <mehulbhagwani@gmail.com> Co-authored-by: Neel Mishra <neelmishra@Neels-Mac-Mini.local>
knowttl
pushed a commit
to knowttl/firstmate
that referenced
this pull request
Sep 29, 2026
…nguid#4819) * fix(supervision): prevent idle recovery loops without stranding wakes * no-mistakes(review): Remove unused wake-append rollback helper
RooseveltAdvisors
pushed a commit
to RooseveltAdvisors/firstmate
that referenced
this pull request
Sep 29, 2026
…nguid#4819) * fix(supervision): prevent idle recovery loops without stranding wakes * no-mistakes(review): Remove unused wake-append rollback helper
andrewesweet
pushed a commit
to andrewesweet/firstmate
that referenced
this pull request
Sep 30, 2026
…nguid#4819) * fix(supervision): prevent idle recovery loops without stranding wakes * no-mistakes(review): Remove unused wake-append rollback helper
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
3 participants
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
Resubmit the existing upstream pull request for the supervision thrash fix on a head that cleanly applies to current kunchenguid/firstmate main and satisfies the repository's required no-mistakes attestation. Preserve only the recovery-marker behavior in bin/fm-wake-lib.sh, its authoritative continuity documentation, and the two executable watch-arm regressions: repeated empty-queue arms must stay quiet while a durable append must promptly reopen recovery. Keep the proposal to one commit, reuse the existing fork branch fm/fm-thrash-upstream-proposal and existing upstream pull request, and do not include fork-only focus files or the prior mail rollback expansion.
What Changed
Risk Assessment
✅ Low: The three-file recovery-marker change is well-bounded: empty announced arms stay quiet, durable appends mint pending so a live watcher recovers, failed appends restore, and the out-of-scope mail rollback helper is gone.
Testing
Drove the real watcher/arm, wake library, and procevent binaries in throwaway fm-lab homes, plus the two new watch-arm regressions. Repeated empty-queue arms stayed quiet on the announced generation, a durable append minted a fresh pending generation and woke the live watcher, an idle Lavish source stayed quiet until its real result, and a failed append restored the previous announced marker. Labs were removed afterward.
Evidence: Combined live watcher transcript (quiet arms, append reopen, failed restore, Lavish result)
Source: Combined live watcher transcript (quiet arms, append reopen, failed restore, Lavish result)
first arm: check: rearm-resurface quiet-arm-1/2 live on announced:downtime:live-empty.1.fixture after append: check: rearm-resurface; pending:downtime:34958.1790502543.hJ34mz; queue inbox:live-fixture failed append restored announced:downtime:restore.1.fixture (append_exit=1) lavish idle arm: process-event result captured: procevent:idle-lavish:1Evidence: Live scenario summary
Source: Live scenario summary
Evidence: Live announced watcher after durable append
Source: Live announced watcher after durable append
watcher: started pid=26631 (beacon fresh) check: rearm-resurfaceEvidence: Idle Lavish source stayed quiet then woke on the real result
Source: Idle Lavish source stayed quiet then woke on the real result
watcher: started pid=58542 (beacon fresh) check: process-event result captured: procevent:idle-lavish:1Evidence: Durable queue row written by the live append
Source: Durable queue row written by the live append
1790502543 1 check inbox:live-fixture check: captain inbox note live-fixtureEvidence: Focused watch-arm regressions
Source: Focused watch-arm regressions
ok - watch-arm: an idle Lavish source stays quiet and its real result wakes promptly ok - watch-arm: appending work reopens an announced empty recoveryPipeline
Updates from git push no-mistakes
✅ **intent** - passed
✅ No issues found.
🔧 **Rebase** - 1 issue found → no changes applied ✅
docs/watcher-continuity.md- merge conflict rebasing onto refs/remotes/no-mistakes-push/fm/fm-thrash-upstream-proposal🔧 No changes applied.
✅ Re-checked - no issues remain.
🔧 **Review** - 1 issue found → auto-fixed ✅
bin/fm-wake-lib.sh:2021- User intent forbids including "the prior mail rollback expansion" and requires preserving only the recovery-marker behavior, empty-queue quiet arms, and append-reopen recovery.fm_wake_append_rollback_lockedis a new public API with no callers in this tree (mail still usesmail_rollback_wake_locked). It is not needed for those behaviors: failed appends already restore via_fm_wake_append_recovery_restore_lockedat bin/fm-wake-lib.sh:2016. The success-path token pair at bin/fm-wake-lib.sh:742-744 is left set only so this unused helper can roll back a committed row. Removefm_wake_append_rollback_locked; do not wire it into mail.sh.🔧 Fix applied.
✅ Re-checked - no issues remain.
✅ **Test** - passed
✅ No issues found.
/tmp/fm-watch-arm-focus.test.sh(extractedtest_idle_lavish_source_stays_quiet_until_resultandtest_append_wakes_live_announced_watcherfromtests/fm-watch-arm.test.sh)FM_HOME=<lab> bin/fm-watch-arm.sh --restartagainst abin/fm-lab-home.shlab: first pending recovery announced once, then two empty-queue re-arms stayed live onannounced:downtime:live-empty.1.fixturefm_wake_append check inbox:live-fixtureagainst that live announced watcher; the arm printedcheck: rearm-resurfaceand the queue retained the durable rowFailedfm_wake_appendagainst a directory standing in for.wake-queuerestoredannounced:downtime:restore.1.fixturebin/fm-procevent.sh register lavish+ reconcile + live arm: idle source produced no wake, then the real result capturedprocevent:idle-lavish:1Sourcedbin/fm-wake-lib.shand confirmedfm_wake_append_rollback_lockedis not a defined API while_fm_wake_append_recovery_restore_lockedremains✅ **Document** - passed
✅ No issues found.
✅ **Lint** - passed
✅ No issues found.
✅ **Push** - passed
✅ No issues found.