feat(bin): add opt-in config/wait-no-turns so a waiting worker spends no turns - #4859
Merged
kunchenguid merged 30 commits intoSep 30, 2026
Conversation
…d poll The brief told workers to background the drive call and poll `axi status` because one call "routinely outlives what your harness lets a single command run". That advice contradicts the tool it drives: `no-mistakes axi run --help` documents `--wait` with an 8m default, existing precisely "so an agent harness with a 10-minute tool cap gets a structured return instead of an unbounded hang". Following the old text, a worker could never idle - a backgrounded call returns in milliseconds, so it does not wait at all - and each attempt leaked a live timer that later fired as a paid wake. Tell workers to make one foreground call, let it block, and repeat it when it returns on elapsed wait rather than on a gate or outcome. Also drops the generalisation that told workers on any unestablished harness to assume a command cap and use the same shape, which exported the defect to harnesses with no such cap.
A worker waiting on a decision, a pipeline gate, CI, or a heavy-test slot kept taking model turns: the brief told it to list its inbox at any natural checkpoint, and six automatic senders nudged secondmates whatever their open decisions. - The ship and scout briefs gain one Waiting section: end the turn after needs-decision or blocked, and hold an external wait inside ONE blocking command bounded by the harness's own command ceiling. The checkpoint clause is deleted. Forbidding the wrong shapes is not enough on its own, so the section also names the blocking foreground `until` loop as the wait a Claude Code worker may use, because that harness can refuse a sleep-then-check command while pointing at backgrounding, which is the one shape a waiting worker must not take. - fm-send --automatic defers (exit 4, nothing written or rung) while the target has an open decision or blocker of its own; every automatic sender passes it and keeps its retry state, and the pending-reply recovery waits the same way. - The two senders that report the result classified it by matching the text of the send's captured output against `deferred:*`. fm-send runs bin/fm-guard.sh as a supervision warning, and that guard prints its worktree-tangle banner whenever the primary checkout is on a feature branch, which is exactly what a CI pull-request checkout is. The banner lands ahead of the `deferred:` line, so the match fell through and a waiting mate was reported as a failed send, with the banner as the reason. Both senders now classify on fm-send's exit status, which is the contract the deferral is actually stated in, and select the `deferred:` line out of the output rather than assuming it came first. The third root cause, a no-mistakes definition of done that backgrounded the drive call and polled axi status, is fixed by this branch's parent commit "drive no-mistakes with one foreground call, not a background poll"; this commit takes that text as is and adds the regression test. Upstream's spawn abort path no longer calls the lease-return helper at all, so the fork's missing-helper guard and its pin-feature test line are moot here and are not ported. The command ceilings each harness enforces, and the probes behind the named Claude Code wait, are recorded in docs/verification/runtime-backends.md.
…clarify worker pauses
…nd reply recovery
…s", reads the pipeline's attestation record, which says document=skipped. No file in the repository can change that record, so I did not touch the check or the PR body. As you said, the no-mistakes rerun after this run finishes will re-execute the document step and record document=completed. The one change is the documentation sentence you ordered. It adds a line to docs/remote-secondmates.md, right after the line saying the remote host runs no re-ring ladder of its own: "A fire-and-forget record, such as a reconcile ask, gets its single retry ring only on the local plane: the remote steer leg owes no re-ring, so a swallowed remote doorbell for one waits for the next ring into that inbox, and a remote-side retry is known follow-up scope." No behavior changed. Checks: tests/fm-documentation-audiences.test.sh passes (4/4) and bin/fm-lint.sh is clean. The change is left uncommitted in the working tree for the pipeline to pick up
tiago-peixoto
force-pushed
the
fm/firstmate-issue4228-wait-turn-spend
branch
from
September 18, 2026 21:57
39ed6f9 to
232c42b
Compare
tiago-peixoto
changed the title
tiago-peixoto
marked this pull request as ready for review
September 18, 2026 22:13
Owner
|
Speaking as Kun's firstmate: HEAD Attestation: MATCH (body Contract-class: new-default. Tip vs main (own inspection, not PR claims): unconfigured homes get a new always-on ship/scout VISION (tip vs main):
Outcome: waiting-captain (Firstmate flag). Otherwise ready (green CI/NM, MATCH, CLEAN, safe review, workflow-zero) but new-default — no auto-merge. This is a captain-decision hold, not waiting on the author. Do not merge this pass. |
Contributor
Author
|
Captain decision on the waiting-captain / new-default triage: Accept the new defaults. The always-on ship/scout No author code change is planned for this classification. We will not merge the upstream PR ourselves; happy to wait on your merge path once you are ready. |
Owner
|
Speaking as Kun's firstmate: HEAD Contract-class: new-default (own tip-vs-main inspection; FM-LEARN-CLAIMS / FM-LEARN-4627). Unconfigured homes get always-on ship/scout VISION.md (each rule)
Contributor captain Accept (2026-09-18): accepted these new defaults. That does not authorize auto-merge (NEVER auto-merge new-default). Kun still owns the merge word. Branch is now DIRTY — do not rebase while captain hold is open (rule 18). Firstmate flag this pass: no (already flagged; not otherwise-ready due to conflicts). Outcome: waiting-captain. No merge this pass. |
…8-wait-turn-spend # Conflicts: # bin/fm-classify-lib.sh # bin/fm-dod-lib.sh # tests/fm-brief.test.sh
…honors open decisions
Owner
|
Speaking as Kun's firstmate: HEAD Contract-class: new-default (own tip-vs-main inspection; FM-LEARN-CLAIMS / FM-LEARN-4627 — PR claims do not decide). Unconfigured homes get always-on ship/scout VISION.md (each rule)
Contributor Accept (2026-09-18): accepted these new defaults. That does not authorize auto-merge (NEVER auto-merge new-default). Kun still owns the merge word. Branch is again CLEAN / green / MATCH — otherwise ready except default-behavior → Firstmate flag yes (captain new-default gate). No rebase/merge this pass. Outcome: waiting-captain. |
…8-wait-turn-spend
…this PR's bin/fm-brief.sh nor its bin/fm-dod-lib.sh change is involved: bin/fm-dispatch-resolve.sh sources neither file. Another branch (fm-attended-cutover-smoothing-s1, run 36343879084) failed the same shard 8 check the same way, on a different case ("a rule-criterion match prints one diagnostic line, got 2"). Root cause: `fm_quota_single_provider_for_harness` in bin/fm-quota-axi-lib.sh returned from its `while read` loop as soon as it found a match. That closed the pipe while `fm_quota_single_provider_table`'s `printf` was sometimes still writing. GitHub Actions runners ignore SIGPIPE, so bash printed `fm-quota-axi-lib.sh: line 138: printf: write error: Broken pipe` to the resolver's stderr. That is the extra line. I reproduced it locally by running the test with SIGPIPE ignored: 2 of 20 runs failed, one with the resolver's diagnostic line plus two broken-pipe lines. Invariant: looking up a harness in the provider table must never make the table writer fail. The only reader of that table is this function, and all of the resolver's lookups (line 208 without stderr redirected, line 222 with it) go through it. So the fix is in that one place: read the whole table, then print the match. The same file now shows it reads the full table first, like `fm_control_harness_supported` does. Return values and output are unchanged. Verification: with SIGPIPE ignored, tests/fm-dispatch-resolve.test.sh failed 0 of 30 runs after the fix (2 of 20 before). tests/fm-dispatch-resolve.test.sh, tests/fm-brief.test.sh, tests/fm-send-inbox.test.sh, tests/fm-quota-choose.test.sh and tests/fm-quota-array-dispatch-live-e2e.test.sh all pass, and shellcheck is clean. tests/fm-procevent-quota.test.sh fails locally with or without the change ("process-event state root is not a private directory"), so that failure comes from the local environment, not from this fix. No new test was added: the existing one-diagnostic-line assertions already catch this whenever SIGPIPE is ignored, as it is in CI
Owner
|
Speaking as Kun's firstmate: whole thread re-read (body + prior triage stamps + author Accept). Tip moved since last stamp. HEAD Contract-class: new-default (own tip-vs-main; FM-LEARN-CLAIMS / FM-LEARN-4627). Unconfigured homes still get always-on ship/scout VISION.md (each rule)
Contributor Accept (2026-09-18) accepted these new defaults; that does not authorize auto-merge. Kun owns the merge word. Otherwise ready except default-behavior → Firstmate flag yes. Outcome: waiting-captain. No merge this pass. |
…Neither this PR's bin/fm-brief.sh nor its bin/fm-dod-lib.sh change is involved: bin/fm-dispatch-resolve.sh sources neither file. Another branch (fm-attended-cutover-smoothing-s1, run 36343879084) failed the same shard 8 check the same way, on a different case ("a rule-criterion match prints one diagnostic line, got 2"). Root cause: `fm_quota_single_provider_for_harness` in bin/fm-quota-axi-lib.sh returned from its `while read` loop as soon as it found a match. That closed the pipe while `fm_quota_single_provider_table`'s `printf` was sometimes still writing. GitHub Actions runners ignore SIGPIPE, so bash printed `fm-quota-axi-lib.sh: line 138: printf: write error: Broken pipe` to the resolver's stderr. That is the extra line. I reproduced it locally by running the test with SIGPIPE ignored: 2 of 20 runs failed, one with the resolver's diagnostic line plus two broken-pipe lines. Invariant: looking up a harness in the provider table must never make the table writer fail. The only reader of that table is this function, and all of the resolver's lookups (line 208 without stderr redirected, line 222 with it) go through it. So the fix is in that one place: read the whole table, then print the match. The same file now shows it reads the full table first, like `fm_control_harness_supported` does. Return values and output are unchanged. Verification: with SIGPIPE ignored, tests/fm-dispatch-resolve.test.sh failed 0 of 30 runs after the fix (2 of 20 before). tests/fm-dispatch-resolve.test.sh, tests/fm-brief.test.sh, tests/fm-send-inbox.test.sh, tests/fm-quota-choose.test.sh and tests/fm-quota-array-dispatch-live-e2e.test.sh all pass, and shellcheck is clean. tests/fm-procevent-quota.test.sh fails locally with or without the change ("process-event state root is not a private directory"), so that failure comes from the local environment, not from this fix. No new test was added: the existing one-diagnostic-line assertions already catch this whenever SIGPIPE is ignored, as it is in CI"
This reverts commit c719928.
 |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Main's recovery reads status before it fires. With the flag present, that send still waits while a mate has its own open decision.
tiago-peixoto
changed the title
Owner
|
Speaking as Kun's firstmate: whole thread re-read (body + prior waiting-captain/waiting-ci stamps + author Accept + opt-in rework). Tip moved since last stamp ( HEAD contract-class (own tip vs main): opt-in. Tip gates VISION.md (each rule):
Outcome: |
5 tasks
Contributor
Author
|
Following up on the waiting-ci triage: the serial 2/4/5 failures were shared with main at the time. After merging current main, the new head |
Owner
|
Speaking as Kun's firstmate: whole thread re-read (body + prior waiting-captain/waiting-ci stamps + author Accept + opt-in rework + author nudge 2026-09-30 that tip HEAD contract-class: VISION.md (each rule):
Outcome: squash-merge (opt-in + green + safe). Firstmate flag: no. |
Keep the short "$FM_TASK_INBOX" steering doorbell together with the default-off config/wait-no-turns gate. Co-authored-by: Kun Chen <kunchenguid@users.noreply.github.com>
Owner
|
Speaking as Kun's firstmate: conflict resolve landed on tip Please re-raise / re-attest this tip through no-mistakes so |
Owner
|
Speaking as Kun's firstmate: whole thread re-read. After #6240 landed on main ( HEAD contract-class: VISION.md (each rule): same as prior opt-in verdict — Authority/One-captain/Scripts/Restart/Delegation/Scope align; Fleet n/a. Outcome: |
tiago-peixoto
changed the title
Contributor
Author
|
Re-attested through no-mistakes on your conflict-resolve tip: the attestation |
Owner
|
Speaking as Kun's firstmate: this is merged. Thank you @tiago-peixoto — really appreciate you taking the time on this. contract-class: opt-in — own tip-vs-main: every new waiting-worker behavior gated on local gitignored |
cloud-practitioner
added a commit
to cloud-practitioner/firstmate
that referenced
this pull request
Sep 30, 2026
* fix(bin): run no repository hook when core.hooksPath is empty (kunchenguid#6216) * fix(bin): run no repository hook when core.hooksPath is empty The per-task hook wrapper refused every commit in a repository whose own config sets core.hooksPath to the empty string, because git rev-parse --git-path hooks fails on it. Plain git reads that setting as no hooks, so the wrapper now runs none; every other lookup failure still refuses and shows git's error. Fixes kunchenguid#6171 * no-mistakes(review): Refuse commits when core.hooksPath is a valueless key * no-mistakes(document): Document empty core.hooksPath handling in commit attribution docs * no-mistakes(ci): When the wrapper refuses a commit, Git's hook-lookup error now shows up once instead of twice. That required changing one line in the wrapper, and the tests were extended so both bad-config cases would catch the duplicate. Invariant: when the wrapper refuses, Git's lookup error must appear exactly once. In the failure path, the only Git call besides the deliberate second lookup is the `git config --get --type=path core.hooksPath` check in `runtime_chain_body` (`bin/fm-git-strip-ai-trailers.sh:168`). That check prints the same error, so it was the one place to fix. I added `2>/dev/null` to it. Its exit status still decides the outcome: an empty value still runs no hook, and anything else goes on to the second lookup, which prints Git's error once, and the commit is refused. Tests (`tests/fm-git-strip-ai-trailers.test.sh`): - The unresolvable-path test (`~fm-no-such-user-6171/hooks`) now requires `failed to expand user dir` to appear exactly once in the refused commit's output. - The valueless-key test now requires `missing value for 'core.hookspath'` to appear exactly once. - Pre-existing bug in the unresolvable-path test: its `git add` ran after the bad config was set, so it failed silently (exit 128) and the "refused commit" had nothing staged. The test now stages the file before writing the config, the same way the valueless test does, so a real commit gets refused. - The empty-string test is unchanged and still passes, so an empty `core.hooksPath` still runs no hook. Verification: - With the wrapper change reverted, both new checks fail with `expected '1', got '2'`. With the change in place, the whole suite passes. - `bash -n` passes. shellcheck shows only an info-level SC1091 note about sourcing `lib.sh`, which was already there before this change. - `git status` lists only the two intended files * fix(bin): let a stale record on a reassigned slot retire records-only (kunchenguid#6213) * fix(bin): let a stale record on a reassigned slot retire records-only When a pool slot's owner claim names another task, the stale record's teardown touches nothing under the slot, so the exclusive-slot record scan no longer refuses it. Full teardowns of a slot this task still claims, or one with no claim, keep the refusal. Fixes kunchenguid#6184 * no-mistakes(document): Note claim-over-record precedence for reassigned teardown slots * fix(bin): keep the steering doorbell short under deep homes (kunchenguid#6240) * fix(bin): keep the steering doorbell short under deep homes The doorbell printed the task inbox's absolute path twice, so under a deep home it grew to about 290 characters and a Herdr submit reported it never reached the pane on every re-ring. It now names the inbox once by its short <task>.inbox name and points at the full path the worker's brief already gives, so its length no longer depends on the home's depth. Fixes kunchenguid#6120 * no-mistakes(review): Export FM_TASK_INBOX at launch and name it in doorbell * no-mistakes(ci): ci-1 (Behavior portable serial 9) was caused by this PR, and I fixed it in the test. tests/fm-claude-trust.test.sh failed with "the launch command did not carry a brief doorbell". Its claude_launch_doorbell helper stripped exactly two leading `export ...;` statements before reading the final prompt argument. This PR adds a third one (`export FM_TASK_INBOX=...`) to every launch, so the helper was reading the wrong command. The invariant: a test that parses the launch command must skip every leading export statement, however many there are. I checked every test that parses the launch this way. The only other ones are the two helpers in tests/fm-spawn-dispatch-profile.test.sh, and they already loop over all exports. The kimi and dispatch-profile exact-string checks were updated earlier in this PR. The fix makes claude_launch_doorbell use the same loop (`while [[ "$command" == export\ *\;* ]]; do command=${command#*; }; done`) and then take the last argument. The ordinary path still works: the claude spawn test and the secondmate-clone spawn test both resolve the brief record through the same helper. Verified locally: `bash tests/fm-claude-trust.test.sh` exits 0 with no failing cases. ci-2 (Behavior tests (Herdr)) was not caused by this change, and I made no code change for it. In tests/fm-backend-herdr-presentation-e2e.test.sh, the concurrent secondmate recovery failed with "herdr presentation recovery could not acquire its session lock; refusing a concurrent resume". Two reasons it is not this PR: - The same failure, in the same test and case, happened on run 36655209015 for the unrelated branch fm/fm-contributions-old-gh-compat about 14 hours earlier. - This PR's change cannot lengthen how long the lock is held. The launch is written to a file and sent to the pane as `. launch.N.sh`, so the extra export changes neither the pane submit nor the lock hold time. The cause is a race that was already there: spawn_herdr_presentation_order_lock_acquire gives up after 5 seconds, and a concurrent real-Herdr recovery can hold the lock longer. Fixing that means changing the product's lock timeout, which is outside this PR. It should be tracked separately, and a rerun of the Herdr job is expected to pass. The only file changed is tests/fm-claude-trust.test.sh * feat(bin): add opt-in config/wait-no-turns so a waiting worker spends no turns (kunchenguid#4859) * fix(dod): drive no-mistakes with one foreground call, not a background poll The brief told workers to background the drive call and poll `axi status` because one call "routinely outlives what your harness lets a single command run". That advice contradicts the tool it drives: `no-mistakes axi run --help` documents `--wait` with an 8m default, existing precisely "so an agent harness with a 10-minute tool cap gets a structured return instead of an unbounded hang". Following the old text, a worker could never idle - a backgrounded call returns in milliseconds, so it does not wait at all - and each attempt leaked a live timer that later fired as a paid wake. Tell workers to make one foreground call, let it block, and repeat it when it returns on elapsed wait rather than on a gate or outcome. Also drops the generalisation that told workers on any unestablished harness to assume a command cap and use the same shape, which exported the defect to harnesses with no such cap. * fix(bin): let a waiting worker spend no turns until it is answered A worker waiting on a decision, a pipeline gate, CI, or a heavy-test slot kept taking model turns: the brief told it to list its inbox at any natural checkpoint, and six automatic senders nudged secondmates whatever their open decisions. - The ship and scout briefs gain one Waiting section: end the turn after needs-decision or blocked, and hold an external wait inside ONE blocking command bounded by the harness's own command ceiling. The checkpoint clause is deleted. Forbidding the wrong shapes is not enough on its own, so the section also names the blocking foreground `until` loop as the wait a Claude Code worker may use, because that harness can refuse a sleep-then-check command while pointing at backgrounding, which is the one shape a waiting worker must not take. - fm-send --automatic defers (exit 4, nothing written or rung) while the target has an open decision or blocker of its own; every automatic sender passes it and keeps its retry state, and the pending-reply recovery waits the same way. - The two senders that report the result classified it by matching the text of the send's captured output against `deferred:*`. fm-send runs bin/fm-guard.sh as a supervision warning, and that guard prints its worktree-tangle banner whenever the primary checkout is on a feature branch, which is exactly what a CI pull-request checkout is. The banner lands ahead of the `deferred:` line, so the match fell through and a waiting mate was reported as a failed send, with the banner as the reason. Both senders now classify on fm-send's exit status, which is the contract the deferral is actually stated in, and select the `deferred:` line out of the output rather than assuming it came first. The third root cause, a no-mistakes definition of done that backgrounded the drive call and polled axi status, is fixed by this branch's parent commit "drive no-mistakes with one foreground call, not a background poll"; this commit takes that text as is and adds the regression test. Upstream's spawn abort path no longer calls the lease-return helper at all, so the fork's missing-helper guard and its pin-feature test line are moot here and are not ported. The command ceilings each harness enforces, and the probes behind the named Claude Code wait, are recorded in docs/verification/runtime-backends.md. * no-mistakes(review): Exempt captain holds, quiet deferred reconcile, clarify worker pauses * no-mistakes(document): Document deferred automatic nudges, rereads, and reply recovery * no-mistakes(document): Ring unlanded fire-and-forget steers exactly once more * no-mistakes(ci): The failing check, "PR must be raised via no-mistakes", reads the pipeline's attestation record, which says document=skipped. No file in the repository can change that record, so I did not touch the check or the PR body. As you said, the no-mistakes rerun after this run finishes will re-execute the document step and record document=completed. The one change is the documentation sentence you ordered. It adds a line to docs/remote-secondmates.md, right after the line saying the remote host runs no re-ring ladder of its own: "A fire-and-forget record, such as a reconcile ask, gets its single retry ring only on the local plane: the remote steer leg owes no re-ring, so a swallowed remote doorbell for one waits for the next ring into that inbox, and a remote-side retry is known follow-up scope." No behavior changed. Checks: tests/fm-documentation-audiences.test.sh passes (4/4) and bin/fm-lint.sh is clean. The change is left uncommitted in the working tree for the pipeline to pick up * no-mistakes(review): Hold automatic wakes until a mate's own decision closes * no-mistakes(document): Document watcher delivery of deferred remote re-read nudges * no-mistakes(review): Merge duplicate elapsed-wait reattach instructions in DOD * no-mistakes(test): Resolve merged default decision in remote-reply recovery fixture * no-mistakes(test): Source classify lib so config-push retry-deferred honors open decisions * no-mistakes(ci): Fixed a flaky test that also fails on main. Neither this PR's bin/fm-brief.sh nor its bin/fm-dod-lib.sh change is involved: bin/fm-dispatch-resolve.sh sources neither file. Another branch (fm-attended-cutover-smoothing-s1, run 36343879084) failed the same shard 8 check the same way, on a different case ("a rule-criterion match prints one diagnostic line, got 2"). Root cause: `fm_quota_single_provider_for_harness` in bin/fm-quota-axi-lib.sh returned from its `while read` loop as soon as it found a match. That closed the pipe while `fm_quota_single_provider_table`'s `printf` was sometimes still writing. GitHub Actions runners ignore SIGPIPE, so bash printed `fm-quota-axi-lib.sh: line 138: printf: write error: Broken pipe` to the resolver's stderr. That is the extra line. I reproduced it locally by running the test with SIGPIPE ignored: 2 of 20 runs failed, one with the resolver's diagnostic line plus two broken-pipe lines. Invariant: looking up a harness in the provider table must never make the table writer fail. The only reader of that table is this function, and all of the resolver's lookups (line 208 without stderr redirected, line 222 with it) go through it. So the fix is in that one place: read the whole table, then print the match. The same file now shows it reads the full table first, like `fm_control_harness_supported` does. Return values and output are unchanged. Verification: with SIGPIPE ignored, tests/fm-dispatch-resolve.test.sh failed 0 of 30 runs after the fix (2 of 20 before). tests/fm-dispatch-resolve.test.sh, tests/fm-brief.test.sh, tests/fm-send-inbox.test.sh, tests/fm-quota-choose.test.sh and tests/fm-quota-array-dispatch-live-e2e.test.sh all pass, and shellcheck is clean. tests/fm-procevent-quota.test.sh fails locally with or without the change ("process-event state root is not a private directory"), so that failure comes from the local environment, not from this fix. No new test was added: the existing one-diagnostic-line assertions already catch this whenever SIGPIPE is ignored, as it is in CI * Revert "no-mistakes(ci): Fixed a flaky test that also fails on main. Neither this PR's bin/fm-brief.sh nor its bin/fm-dod-lib.sh change is involved: bin/fm-dispatch-resolve.sh sources neither file. Another branch (fm-attended-cutover-smoothing-s1, run 36343879084) failed the same shard 8 check the same way, on a different case ("a rule-criterion match prints one diagnostic line, got 2"). Root cause: `fm_quota_single_provider_for_harness` in bin/fm-quota-axi-lib.sh returned from its `while read` loop as soon as it found a match. That closed the pipe while `fm_quota_single_provider_table`'s `printf` was sometimes still writing. GitHub Actions runners ignore SIGPIPE, so bash printed `fm-quota-axi-lib.sh: line 138: printf: write error: Broken pipe` to the resolver's stderr. That is the extra line. I reproduced it locally by running the test with SIGPIPE ignored: 2 of 20 runs failed, one with the resolver's diagnostic line plus two broken-pipe lines. Invariant: looking up a harness in the provider table must never make the table writer fail. The only reader of that table is this function, and all of the resolver's lookups (line 208 without stderr redirected, line 222 with it) go through it. So the fix is in that one place: read the whole table, then print the match. The same file now shows it reads the full table first, like `fm_control_harness_supported` does. Return values and output are unchanged. Verification: with SIGPIPE ignored, tests/fm-dispatch-resolve.test.sh failed 0 of 30 runs after the fix (2 of 20 before). tests/fm-dispatch-resolve.test.sh, tests/fm-brief.test.sh, tests/fm-send-inbox.test.sh, tests/fm-quota-choose.test.sh and tests/fm-quota-array-dispatch-live-e2e.test.sh all pass, and shellcheck is clean. tests/fm-procevent-quota.test.sh fails locally with or without the change ("process-event state root is not a private directory"), so that failure comes from the local environment, not from this fix. No new test was added: the existing one-diagnostic-line assertions already catch this whenever SIGPIPE is ignored, as it is in CI" This reverts commit c719928. * no-mistakes(review): Retry deferred local instruction nudges via the watcher * no-mistakes(review): Document watcher retry for deferred local instruction nudges * no-mistakes(ci): I fixed both review findings you selected (ci-1 and ci-3). I did not touch the deferral check in bin/fm-send.sh. ci-1 (bin/fm-config-push.sh, retry_deferred_rereads) - Rule that must hold: a deferred reread stays flagged until it is actually delivered. - Before the fix, the flag was removed before any of the steps that can skip a mate: the remote lock-path lookup, validate_secondmate_home, the local lock-path lookup, and the lock acquire. A skip at any of those dropped the flag, so the watcher lost track of the reread. - Now the flag is removed in one place only, when the send succeeds (rc 0). A skipped home, a busy lock, a deferred send (rc 4) or a failed send all leave it in place. The re-mark calls on a busy lock and on rc 4 were no longer needed, so I removed them. I updated the comment above the function to match. - Side effect: a send that keeps failing now stays flagged, so the watcher retries it on every poll and logs each failure. That follows your "don't clear until delivered" rule, but it replaces the old behaviour of leaving a failed send to the next config push or session start. - New test in tests/fm-secondmate-sync.test.sh: T8j "a deferred flag survives a skipped invalid home and is retried once it validates". It takes the home's marker away to make validation fail, checks that nothing is sent and the flag stays, then puts the marker back and checks that the nudge is delivered and both the flag and the retry marker are cleared. It fails on the old code and passes now. ci-3 (bin/fm-secondmate-restart.sh) - Rule that must hold: no automatic send wakes a mate that is waiting on its own open decision. - The two automatic sends in this script are the fallback reread nudge (fall_back_to_nudge) and the persist request. Both now pass --automatic. If a persist request is deferred, its correlation is discarded and the mate goes to the fallback nudge, which is also deferred, so the mate is reported as unreached. - New test in tests/fm-secondmate-restart.test.sh: T3b. It gives a mate an open needs-decision and runs a restart. It checks that both sends report as deferred, the mate's doorbell is never rung, its inbox gets no message, nothing is stopped, and the mate is reported as unreached with exit status 3. It fails on the old code and passes now. - The test marks the watcher as alive first. Without that, the watcher-down warning is printed first and becomes the reported reason instead of the deferral message. Verification - tests/fm-secondmate-sync.test.sh passes. - tests/fm-secondmate-restart.test.sh passes. - tests/fm-secondmate-harness.test.sh (the other test that exercises --retry-deferred) passes. - The fm-send-inbox test that covers automatic deferral passes. I only looked at the last lines of that run, not the whole file. - `shellcheck -x` on the four changed files is clean * Pin autoarm supervision model in secondmate restart T3b The fresh watcher beat the test writes proves a live watcher only under the autoarm model; on CI hosts with no detected harness the persistent model demands a lock-holding watcher, so the watcher-down banner became the reported reason and the deferral assertion failed. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Keep deferred secondmate nudges retryable under the inheritance lock. A bootstrap instruction nudge could write its deferral flag outside the lock the watcher retry holds, so a concurrent retry could delete a flag that had just been set. A restart fallback that is deferred now records the same marker and flag, so the watcher delivers it once the decision closes. * no-mistakes(document): Document watcher retry of deferred restart re-read nudges * Send secondmate reread and restart nudges immediately again. Deferring those nudges let a later config push drop an incomplete transfer once the decision closed. They now send as they do on main. * Make the no-turn wait opt-in behind config/wait-no-turns. Homes that do not create the file keep the previous briefs, drive text, and sends. * no-mistakes(document): Document wait-no-turns inbox wording change in configuration * no-mistakes(review): Keep checkpoint inbox check; forbid only polling while waiting * no-mistakes(ci): Fixed ci-2 (Greptile: a concurrent retry marker gets lost). The rule that was broken: the watcher may remove only the `.retry-ring` mark for the record it just processed. A newer mark written in the meantime is owed its own retry. `fm_task_inbox_clear_retry` is the one shared function that removes the mark, and I fixed it there. In `bin/fm-task-inbox-lib.sh` it now takes the record path. It compares the mark's content with that record's name and removes the mark only when they match. When the mark names a different record it returns success and leaves the mark alone. It still fails only when the processed record's own mark can't be removed. Both callers in `bin/fm-watch.sh` now pass `"$rec"`: the dead or missing pane path and the path after a retry ring. So the fix holds at both removal sites. Tests, in `tests/fm-task-inbox.test.sh`: - I added an optional `FM_RING_MARKS_RETRY` hook to the fake tmux. It writes a newer record's mark while the doorbell is being typed, which reproduces the race deterministically. - I added `test_watcher_retry_keeps_a_newer_mark`. The owed retry rings once, the newer mark survives, and a later check rings the newer record once and then clears its mark. The test fails without the fix ("the spent retry removed a newer record's mark written during its ring") and passes with it. - I updated the direct `clear_retry` call in the existing unit test to pass the record. Results: `tests/fm-task-inbox.test.sh` passes in full and `tests/fm-send-inbox.test.sh` passes 15/15. Shellcheck reports only SC1091 "not following sourced file" notices. As instructed, I didn't change the brief inbox wording * no-mistakes(document): Fix stale wait-no-turns inbox wording in inbox lib comment --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> Co-authored-by: Cursor Agent <cursoragent@cursor.com> Co-authored-by: Kun Chen <kunchenguid@users.noreply.github.com> --------- Co-authored-by: Christopher McKay <101884182+karotkriss@users.noreply.github.com> Co-authored-by: Tiago <tiagop@hey.com> Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> Co-authored-by: Cursor Agent <cursoragent@cursor.com> Co-authored-by: Kun Chen <kunchenguid@users.noreply.github.com>
cloud-practitioner
added a commit
to cloud-practitioner/firstmate
that referenced
this pull request
Oct 1, 2026
* fix(bin): run no repository hook when core.hooksPath is empty (kunchenguid#6216) * fix(bin): run no repository hook when core.hooksPath is empty The per-task hook wrapper refused every commit in a repository whose own config sets core.hooksPath to the empty string, because git rev-parse --git-path hooks fails on it. Plain git reads that setting as no hooks, so the wrapper now runs none; every other lookup failure still refuses and shows git's error. Fixes kunchenguid#6171 * no-mistakes(review): Refuse commits when core.hooksPath is a valueless key * no-mistakes(document): Document empty core.hooksPath handling in commit attribution docs * no-mistakes(ci): When the wrapper refuses a commit, Git's hook-lookup error now shows up once instead of twice. That required changing one line in the wrapper, and the tests were extended so both bad-config cases would catch the duplicate. Invariant: when the wrapper refuses, Git's lookup error must appear exactly once. In the failure path, the only Git call besides the deliberate second lookup is the `git config --get --type=path core.hooksPath` check in `runtime_chain_body` (`bin/fm-git-strip-ai-trailers.sh:168`). That check prints the same error, so it was the one place to fix. I added `2>/dev/null` to it. Its exit status still decides the outcome: an empty value still runs no hook, and anything else goes on to the second lookup, which prints Git's error once, and the commit is refused. Tests (`tests/fm-git-strip-ai-trailers.test.sh`): - The unresolvable-path test (`~fm-no-such-user-6171/hooks`) now requires `failed to expand user dir` to appear exactly once in the refused commit's output. - The valueless-key test now requires `missing value for 'core.hookspath'` to appear exactly once. - Pre-existing bug in the unresolvable-path test: its `git add` ran after the bad config was set, so it failed silently (exit 128) and the "refused commit" had nothing staged. The test now stages the file before writing the config, the same way the valueless test does, so a real commit gets refused. - The empty-string test is unchanged and still passes, so an empty `core.hooksPath` still runs no hook. Verification: - With the wrapper change reverted, both new checks fail with `expected '1', got '2'`. With the change in place, the whole suite passes. - `bash -n` passes. shellcheck shows only an info-level SC1091 note about sourcing `lib.sh`, which was already there before this change. - `git status` lists only the two intended files * fix(bin): let a stale record on a reassigned slot retire records-only (kunchenguid#6213) * fix(bin): let a stale record on a reassigned slot retire records-only When a pool slot's owner claim names another task, the stale record's teardown touches nothing under the slot, so the exclusive-slot record scan no longer refuses it. Full teardowns of a slot this task still claims, or one with no claim, keep the refusal. Fixes kunchenguid#6184 * no-mistakes(document): Note claim-over-record precedence for reassigned teardown slots * fix(bin): keep the steering doorbell short under deep homes (kunchenguid#6240) * fix(bin): keep the steering doorbell short under deep homes The doorbell printed the task inbox's absolute path twice, so under a deep home it grew to about 290 characters and a Herdr submit reported it never reached the pane on every re-ring. It now names the inbox once by its short <task>.inbox name and points at the full path the worker's brief already gives, so its length no longer depends on the home's depth. Fixes kunchenguid#6120 * no-mistakes(review): Export FM_TASK_INBOX at launch and name it in doorbell * no-mistakes(ci): ci-1 (Behavior portable serial 9) was caused by this PR, and I fixed it in the test. tests/fm-claude-trust.test.sh failed with "the launch command did not carry a brief doorbell". Its claude_launch_doorbell helper stripped exactly two leading `export ...;` statements before reading the final prompt argument. This PR adds a third one (`export FM_TASK_INBOX=...`) to every launch, so the helper was reading the wrong command. The invariant: a test that parses the launch command must skip every leading export statement, however many there are. I checked every test that parses the launch this way. The only other ones are the two helpers in tests/fm-spawn-dispatch-profile.test.sh, and they already loop over all exports. The kimi and dispatch-profile exact-string checks were updated earlier in this PR. The fix makes claude_launch_doorbell use the same loop (`while [[ "$command" == export\ *\;* ]]; do command=${command#*; }; done`) and then take the last argument. The ordinary path still works: the claude spawn test and the secondmate-clone spawn test both resolve the brief record through the same helper. Verified locally: `bash tests/fm-claude-trust.test.sh` exits 0 with no failing cases. ci-2 (Behavior tests (Herdr)) was not caused by this change, and I made no code change for it. In tests/fm-backend-herdr-presentation-e2e.test.sh, the concurrent secondmate recovery failed with "herdr presentation recovery could not acquire its session lock; refusing a concurrent resume". Two reasons it is not this PR: - The same failure, in the same test and case, happened on run 36655209015 for the unrelated branch fm/fm-contributions-old-gh-compat about 14 hours earlier. - This PR's change cannot lengthen how long the lock is held. The launch is written to a file and sent to the pane as `. launch.N.sh`, so the extra export changes neither the pane submit nor the lock hold time. The cause is a race that was already there: spawn_herdr_presentation_order_lock_acquire gives up after 5 seconds, and a concurrent real-Herdr recovery can hold the lock longer. Fixing that means changing the product's lock timeout, which is outside this PR. It should be tracked separately, and a rerun of the Herdr job is expected to pass. The only file changed is tests/fm-claude-trust.test.sh * feat(bin): add opt-in config/wait-no-turns so a waiting worker spends no turns (kunchenguid#4859) * fix(dod): drive no-mistakes with one foreground call, not a background poll The brief told workers to background the drive call and poll `axi status` because one call "routinely outlives what your harness lets a single command run". That advice contradicts the tool it drives: `no-mistakes axi run --help` documents `--wait` with an 8m default, existing precisely "so an agent harness with a 10-minute tool cap gets a structured return instead of an unbounded hang". Following the old text, a worker could never idle - a backgrounded call returns in milliseconds, so it does not wait at all - and each attempt leaked a live timer that later fired as a paid wake. Tell workers to make one foreground call, let it block, and repeat it when it returns on elapsed wait rather than on a gate or outcome. Also drops the generalisation that told workers on any unestablished harness to assume a command cap and use the same shape, which exported the defect to harnesses with no such cap. * fix(bin): let a waiting worker spend no turns until it is answered A worker waiting on a decision, a pipeline gate, CI, or a heavy-test slot kept taking model turns: the brief told it to list its inbox at any natural checkpoint, and six automatic senders nudged secondmates whatever their open decisions. - The ship and scout briefs gain one Waiting section: end the turn after needs-decision or blocked, and hold an external wait inside ONE blocking command bounded by the harness's own command ceiling. The checkpoint clause is deleted. Forbidding the wrong shapes is not enough on its own, so the section also names the blocking foreground `until` loop as the wait a Claude Code worker may use, because that harness can refuse a sleep-then-check command while pointing at backgrounding, which is the one shape a waiting worker must not take. - fm-send --automatic defers (exit 4, nothing written or rung) while the target has an open decision or blocker of its own; every automatic sender passes it and keeps its retry state, and the pending-reply recovery waits the same way. - The two senders that report the result classified it by matching the text of the send's captured output against `deferred:*`. fm-send runs bin/fm-guard.sh as a supervision warning, and that guard prints its worktree-tangle banner whenever the primary checkout is on a feature branch, which is exactly what a CI pull-request checkout is. The banner lands ahead of the `deferred:` line, so the match fell through and a waiting mate was reported as a failed send, with the banner as the reason. Both senders now classify on fm-send's exit status, which is the contract the deferral is actually stated in, and select the `deferred:` line out of the output rather than assuming it came first. The third root cause, a no-mistakes definition of done that backgrounded the drive call and polled axi status, is fixed by this branch's parent commit "drive no-mistakes with one foreground call, not a background poll"; this commit takes that text as is and adds the regression test. Upstream's spawn abort path no longer calls the lease-return helper at all, so the fork's missing-helper guard and its pin-feature test line are moot here and are not ported. The command ceilings each harness enforces, and the probes behind the named Claude Code wait, are recorded in docs/verification/runtime-backends.md. * no-mistakes(review): Exempt captain holds, quiet deferred reconcile, clarify worker pauses * no-mistakes(document): Document deferred automatic nudges, rereads, and reply recovery * no-mistakes(document): Ring unlanded fire-and-forget steers exactly once more * no-mistakes(ci): The failing check, "PR must be raised via no-mistakes", reads the pipeline's attestation record, which says document=skipped. No file in the repository can change that record, so I did not touch the check or the PR body. As you said, the no-mistakes rerun after this run finishes will re-execute the document step and record document=completed. The one change is the documentation sentence you ordered. It adds a line to docs/remote-secondmates.md, right after the line saying the remote host runs no re-ring ladder of its own: "A fire-and-forget record, such as a reconcile ask, gets its single retry ring only on the local plane: the remote steer leg owes no re-ring, so a swallowed remote doorbell for one waits for the next ring into that inbox, and a remote-side retry is known follow-up scope." No behavior changed. Checks: tests/fm-documentation-audiences.test.sh passes (4/4) and bin/fm-lint.sh is clean. The change is left uncommitted in the working tree for the pipeline to pick up * no-mistakes(review): Hold automatic wakes until a mate's own decision closes * no-mistakes(document): Document watcher delivery of deferred remote re-read nudges * no-mistakes(review): Merge duplicate elapsed-wait reattach instructions in DOD * no-mistakes(test): Resolve merged default decision in remote-reply recovery fixture * no-mistakes(test): Source classify lib so config-push retry-deferred honors open decisions * no-mistakes(ci): Fixed a flaky test that also fails on main. Neither this PR's bin/fm-brief.sh nor its bin/fm-dod-lib.sh change is involved: bin/fm-dispatch-resolve.sh sources neither file. Another branch (fm-attended-cutover-smoothing-s1, run 36343879084) failed the same shard 8 check the same way, on a different case ("a rule-criterion match prints one diagnostic line, got 2"). Root cause: `fm_quota_single_provider_for_harness` in bin/fm-quota-axi-lib.sh returned from its `while read` loop as soon as it found a match. That closed the pipe while `fm_quota_single_provider_table`'s `printf` was sometimes still writing. GitHub Actions runners ignore SIGPIPE, so bash printed `fm-quota-axi-lib.sh: line 138: printf: write error: Broken pipe` to the resolver's stderr. That is the extra line. I reproduced it locally by running the test with SIGPIPE ignored: 2 of 20 runs failed, one with the resolver's diagnostic line plus two broken-pipe lines. Invariant: looking up a harness in the provider table must never make the table writer fail. The only reader of that table is this function, and all of the resolver's lookups (line 208 without stderr redirected, line 222 with it) go through it. So the fix is in that one place: read the whole table, then print the match. The same file now shows it reads the full table first, like `fm_control_harness_supported` does. Return values and output are unchanged. Verification: with SIGPIPE ignored, tests/fm-dispatch-resolve.test.sh failed 0 of 30 runs after the fix (2 of 20 before). tests/fm-dispatch-resolve.test.sh, tests/fm-brief.test.sh, tests/fm-send-inbox.test.sh, tests/fm-quota-choose.test.sh and tests/fm-quota-array-dispatch-live-e2e.test.sh all pass, and shellcheck is clean. tests/fm-procevent-quota.test.sh fails locally with or without the change ("process-event state root is not a private directory"), so that failure comes from the local environment, not from this fix. No new test was added: the existing one-diagnostic-line assertions already catch this whenever SIGPIPE is ignored, as it is in CI * Revert "no-mistakes(ci): Fixed a flaky test that also fails on main. Neither this PR's bin/fm-brief.sh nor its bin/fm-dod-lib.sh change is involved: bin/fm-dispatch-resolve.sh sources neither file. Another branch (fm-attended-cutover-smoothing-s1, run 36343879084) failed the same shard 8 check the same way, on a different case ("a rule-criterion match prints one diagnostic line, got 2"). Root cause: `fm_quota_single_provider_for_harness` in bin/fm-quota-axi-lib.sh returned from its `while read` loop as soon as it found a match. That closed the pipe while `fm_quota_single_provider_table`'s `printf` was sometimes still writing. GitHub Actions runners ignore SIGPIPE, so bash printed `fm-quota-axi-lib.sh: line 138: printf: write error: Broken pipe` to the resolver's stderr. That is the extra line. I reproduced it locally by running the test with SIGPIPE ignored: 2 of 20 runs failed, one with the resolver's diagnostic line plus two broken-pipe lines. Invariant: looking up a harness in the provider table must never make the table writer fail. The only reader of that table is this function, and all of the resolver's lookups (line 208 without stderr redirected, line 222 with it) go through it. So the fix is in that one place: read the whole table, then print the match. The same file now shows it reads the full table first, like `fm_control_harness_supported` does. Return values and output are unchanged. Verification: with SIGPIPE ignored, tests/fm-dispatch-resolve.test.sh failed 0 of 30 runs after the fix (2 of 20 before). tests/fm-dispatch-resolve.test.sh, tests/fm-brief.test.sh, tests/fm-send-inbox.test.sh, tests/fm-quota-choose.test.sh and tests/fm-quota-array-dispatch-live-e2e.test.sh all pass, and shellcheck is clean. tests/fm-procevent-quota.test.sh fails locally with or without the change ("process-event state root is not a private directory"), so that failure comes from the local environment, not from this fix. No new test was added: the existing one-diagnostic-line assertions already catch this whenever SIGPIPE is ignored, as it is in CI" This reverts commit c719928. * no-mistakes(review): Retry deferred local instruction nudges via the watcher * no-mistakes(review): Document watcher retry for deferred local instruction nudges * no-mistakes(ci): I fixed both review findings you selected (ci-1 and ci-3). I did not touch the deferral check in bin/fm-send.sh. ci-1 (bin/fm-config-push.sh, retry_deferred_rereads) - Rule that must hold: a deferred reread stays flagged until it is actually delivered. - Before the fix, the flag was removed before any of the steps that can skip a mate: the remote lock-path lookup, validate_secondmate_home, the local lock-path lookup, and the lock acquire. A skip at any of those dropped the flag, so the watcher lost track of the reread. - Now the flag is removed in one place only, when the send succeeds (rc 0). A skipped home, a busy lock, a deferred send (rc 4) or a failed send all leave it in place. The re-mark calls on a busy lock and on rc 4 were no longer needed, so I removed them. I updated the comment above the function to match. - Side effect: a send that keeps failing now stays flagged, so the watcher retries it on every poll and logs each failure. That follows your "don't clear until delivered" rule, but it replaces the old behaviour of leaving a failed send to the next config push or session start. - New test in tests/fm-secondmate-sync.test.sh: T8j "a deferred flag survives a skipped invalid home and is retried once it validates". It takes the home's marker away to make validation fail, checks that nothing is sent and the flag stays, then puts the marker back and checks that the nudge is delivered and both the flag and the retry marker are cleared. It fails on the old code and passes now. ci-3 (bin/fm-secondmate-restart.sh) - Rule that must hold: no automatic send wakes a mate that is waiting on its own open decision. - The two automatic sends in this script are the fallback reread nudge (fall_back_to_nudge) and the persist request. Both now pass --automatic. If a persist request is deferred, its correlation is discarded and the mate goes to the fallback nudge, which is also deferred, so the mate is reported as unreached. - New test in tests/fm-secondmate-restart.test.sh: T3b. It gives a mate an open needs-decision and runs a restart. It checks that both sends report as deferred, the mate's doorbell is never rung, its inbox gets no message, nothing is stopped, and the mate is reported as unreached with exit status 3. It fails on the old code and passes now. - The test marks the watcher as alive first. Without that, the watcher-down warning is printed first and becomes the reported reason instead of the deferral message. Verification - tests/fm-secondmate-sync.test.sh passes. - tests/fm-secondmate-restart.test.sh passes. - tests/fm-secondmate-harness.test.sh (the other test that exercises --retry-deferred) passes. - The fm-send-inbox test that covers automatic deferral passes. I only looked at the last lines of that run, not the whole file. - `shellcheck -x` on the four changed files is clean * Pin autoarm supervision model in secondmate restart T3b The fresh watcher beat the test writes proves a live watcher only under the autoarm model; on CI hosts with no detected harness the persistent model demands a lock-holding watcher, so the watcher-down banner became the reported reason and the deferral assertion failed. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Keep deferred secondmate nudges retryable under the inheritance lock. A bootstrap instruction nudge could write its deferral flag outside the lock the watcher retry holds, so a concurrent retry could delete a flag that had just been set. A restart fallback that is deferred now records the same marker and flag, so the watcher delivers it once the decision closes. * no-mistakes(document): Document watcher retry of deferred restart re-read nudges * Send secondmate reread and restart nudges immediately again. Deferring those nudges let a later config push drop an incomplete transfer once the decision closed. They now send as they do on main. * Make the no-turn wait opt-in behind config/wait-no-turns. Homes that do not create the file keep the previous briefs, drive text, and sends. * no-mistakes(document): Document wait-no-turns inbox wording change in configuration * no-mistakes(review): Keep checkpoint inbox check; forbid only polling while waiting * no-mistakes(ci): Fixed ci-2 (Greptile: a concurrent retry marker gets lost). The rule that was broken: the watcher may remove only the `.retry-ring` mark for the record it just processed. A newer mark written in the meantime is owed its own retry. `fm_task_inbox_clear_retry` is the one shared function that removes the mark, and I fixed it there. In `bin/fm-task-inbox-lib.sh` it now takes the record path. It compares the mark's content with that record's name and removes the mark only when they match. When the mark names a different record it returns success and leaves the mark alone. It still fails only when the processed record's own mark can't be removed. Both callers in `bin/fm-watch.sh` now pass `"$rec"`: the dead or missing pane path and the path after a retry ring. So the fix holds at both removal sites. Tests, in `tests/fm-task-inbox.test.sh`: - I added an optional `FM_RING_MARKS_RETRY` hook to the fake tmux. It writes a newer record's mark while the doorbell is being typed, which reproduces the race deterministically. - I added `test_watcher_retry_keeps_a_newer_mark`. The owed retry rings once, the newer mark survives, and a later check rings the newer record once and then clears its mark. The test fails without the fix ("the spent retry removed a newer record's mark written during its ring") and passes with it. - I updated the direct `clear_retry` call in the existing unit test to pass the record. Results: `tests/fm-task-inbox.test.sh` passes in full and `tests/fm-send-inbox.test.sh` passes 15/15. Shellcheck reports only SC1091 "not following sourced file" notices. As instructed, I didn't change the brief inbox wording * no-mistakes(document): Fix stale wait-no-turns inbox wording in inbox lib comment --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> Co-authored-by: Cursor Agent <cursoragent@cursor.com> Co-authored-by: Kun Chen <kunchenguid@users.noreply.github.com> --------- Co-authored-by: Christopher McKay <101884182+karotkriss@users.noreply.github.com> Co-authored-by: Tiago <tiagop@hey.com> Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> Co-authored-by: Cursor Agent <cursoragent@cursor.com> Co-authored-by: Kun Chen <kunchenguid@users.noreply.github.com>
cloud-practitioner
added a commit
to cloud-practitioner/firstmate
that referenced
this pull request
Oct 1, 2026
* fix(bin): run no repository hook when core.hooksPath is empty (kunchenguid#6216) * fix(bin): run no repository hook when core.hooksPath is empty The per-task hook wrapper refused every commit in a repository whose own config sets core.hooksPath to the empty string, because git rev-parse --git-path hooks fails on it. Plain git reads that setting as no hooks, so the wrapper now runs none; every other lookup failure still refuses and shows git's error. Fixes kunchenguid#6171 * no-mistakes(review): Refuse commits when core.hooksPath is a valueless key * no-mistakes(document): Document empty core.hooksPath handling in commit attribution docs * no-mistakes(ci): When the wrapper refuses a commit, Git's hook-lookup error now shows up once instead of twice. That required changing one line in the wrapper, and the tests were extended so both bad-config cases would catch the duplicate. Invariant: when the wrapper refuses, Git's lookup error must appear exactly once. In the failure path, the only Git call besides the deliberate second lookup is the `git config --get --type=path core.hooksPath` check in `runtime_chain_body` (`bin/fm-git-strip-ai-trailers.sh:168`). That check prints the same error, so it was the one place to fix. I added `2>/dev/null` to it. Its exit status still decides the outcome: an empty value still runs no hook, and anything else goes on to the second lookup, which prints Git's error once, and the commit is refused. Tests (`tests/fm-git-strip-ai-trailers.test.sh`): - The unresolvable-path test (`~fm-no-such-user-6171/hooks`) now requires `failed to expand user dir` to appear exactly once in the refused commit's output. - The valueless-key test now requires `missing value for 'core.hookspath'` to appear exactly once. - Pre-existing bug in the unresolvable-path test: its `git add` ran after the bad config was set, so it failed silently (exit 128) and the "refused commit" had nothing staged. The test now stages the file before writing the config, the same way the valueless test does, so a real commit gets refused. - The empty-string test is unchanged and still passes, so an empty `core.hooksPath` still runs no hook. Verification: - With the wrapper change reverted, both new checks fail with `expected '1', got '2'`. With the change in place, the whole suite passes. - `bash -n` passes. shellcheck shows only an info-level SC1091 note about sourcing `lib.sh`, which was already there before this change. - `git status` lists only the two intended files * fix(bin): let a stale record on a reassigned slot retire records-only (kunchenguid#6213) * fix(bin): let a stale record on a reassigned slot retire records-only When a pool slot's owner claim names another task, the stale record's teardown touches nothing under the slot, so the exclusive-slot record scan no longer refuses it. Full teardowns of a slot this task still claims, or one with no claim, keep the refusal. Fixes kunchenguid#6184 * no-mistakes(document): Note claim-over-record precedence for reassigned teardown slots * fix(bin): keep the steering doorbell short under deep homes (kunchenguid#6240) * fix(bin): keep the steering doorbell short under deep homes The doorbell printed the task inbox's absolute path twice, so under a deep home it grew to about 290 characters and a Herdr submit reported it never reached the pane on every re-ring. It now names the inbox once by its short <task>.inbox name and points at the full path the worker's brief already gives, so its length no longer depends on the home's depth. Fixes kunchenguid#6120 * no-mistakes(review): Export FM_TASK_INBOX at launch and name it in doorbell * no-mistakes(ci): ci-1 (Behavior portable serial 9) was caused by this PR, and I fixed it in the test. tests/fm-claude-trust.test.sh failed with "the launch command did not carry a brief doorbell". Its claude_launch_doorbell helper stripped exactly two leading `export ...;` statements before reading the final prompt argument. This PR adds a third one (`export FM_TASK_INBOX=...`) to every launch, so the helper was reading the wrong command. The invariant: a test that parses the launch command must skip every leading export statement, however many there are. I checked every test that parses the launch this way. The only other ones are the two helpers in tests/fm-spawn-dispatch-profile.test.sh, and they already loop over all exports. The kimi and dispatch-profile exact-string checks were updated earlier in this PR. The fix makes claude_launch_doorbell use the same loop (`while [[ "$command" == export\ *\;* ]]; do command=${command#*; }; done`) and then take the last argument. The ordinary path still works: the claude spawn test and the secondmate-clone spawn test both resolve the brief record through the same helper. Verified locally: `bash tests/fm-claude-trust.test.sh` exits 0 with no failing cases. ci-2 (Behavior tests (Herdr)) was not caused by this change, and I made no code change for it. In tests/fm-backend-herdr-presentation-e2e.test.sh, the concurrent secondmate recovery failed with "herdr presentation recovery could not acquire its session lock; refusing a concurrent resume". Two reasons it is not this PR: - The same failure, in the same test and case, happened on run 36655209015 for the unrelated branch fm/fm-contributions-old-gh-compat about 14 hours earlier. - This PR's change cannot lengthen how long the lock is held. The launch is written to a file and sent to the pane as `. launch.N.sh`, so the extra export changes neither the pane submit nor the lock hold time. The cause is a race that was already there: spawn_herdr_presentation_order_lock_acquire gives up after 5 seconds, and a concurrent real-Herdr recovery can hold the lock longer. Fixing that means changing the product's lock timeout, which is outside this PR. It should be tracked separately, and a rerun of the Herdr job is expected to pass. The only file changed is tests/fm-claude-trust.test.sh * feat(bin): add opt-in config/wait-no-turns so a waiting worker spends no turns (kunchenguid#4859) * fix(dod): drive no-mistakes with one foreground call, not a background poll The brief told workers to background the drive call and poll `axi status` because one call "routinely outlives what your harness lets a single command run". That advice contradicts the tool it drives: `no-mistakes axi run --help` documents `--wait` with an 8m default, existing precisely "so an agent harness with a 10-minute tool cap gets a structured return instead of an unbounded hang". Following the old text, a worker could never idle - a backgrounded call returns in milliseconds, so it does not wait at all - and each attempt leaked a live timer that later fired as a paid wake. Tell workers to make one foreground call, let it block, and repeat it when it returns on elapsed wait rather than on a gate or outcome. Also drops the generalisation that told workers on any unestablished harness to assume a command cap and use the same shape, which exported the defect to harnesses with no such cap. * fix(bin): let a waiting worker spend no turns until it is answered A worker waiting on a decision, a pipeline gate, CI, or a heavy-test slot kept taking model turns: the brief told it to list its inbox at any natural checkpoint, and six automatic senders nudged secondmates whatever their open decisions. - The ship and scout briefs gain one Waiting section: end the turn after needs-decision or blocked, and hold an external wait inside ONE blocking command bounded by the harness's own command ceiling. The checkpoint clause is deleted. Forbidding the wrong shapes is not enough on its own, so the section also names the blocking foreground `until` loop as the wait a Claude Code worker may use, because that harness can refuse a sleep-then-check command while pointing at backgrounding, which is the one shape a waiting worker must not take. - fm-send --automatic defers (exit 4, nothing written or rung) while the target has an open decision or blocker of its own; every automatic sender passes it and keeps its retry state, and the pending-reply recovery waits the same way. - The two senders that report the result classified it by matching the text of the send's captured output against `deferred:*`. fm-send runs bin/fm-guard.sh as a supervision warning, and that guard prints its worktree-tangle banner whenever the primary checkout is on a feature branch, which is exactly what a CI pull-request checkout is. The banner lands ahead of the `deferred:` line, so the match fell through and a waiting mate was reported as a failed send, with the banner as the reason. Both senders now classify on fm-send's exit status, which is the contract the deferral is actually stated in, and select the `deferred:` line out of the output rather than assuming it came first. The third root cause, a no-mistakes definition of done that backgrounded the drive call and polled axi status, is fixed by this branch's parent commit "drive no-mistakes with one foreground call, not a background poll"; this commit takes that text as is and adds the regression test. Upstream's spawn abort path no longer calls the lease-return helper at all, so the fork's missing-helper guard and its pin-feature test line are moot here and are not ported. The command ceilings each harness enforces, and the probes behind the named Claude Code wait, are recorded in docs/verification/runtime-backends.md. * no-mistakes(review): Exempt captain holds, quiet deferred reconcile, clarify worker pauses * no-mistakes(document): Document deferred automatic nudges, rereads, and reply recovery * no-mistakes(document): Ring unlanded fire-and-forget steers exactly once more * no-mistakes(ci): The failing check, "PR must be raised via no-mistakes", reads the pipeline's attestation record, which says document=skipped. No file in the repository can change that record, so I did not touch the check or the PR body. As you said, the no-mistakes rerun after this run finishes will re-execute the document step and record document=completed. The one change is the documentation sentence you ordered. It adds a line to docs/remote-secondmates.md, right after the line saying the remote host runs no re-ring ladder of its own: "A fire-and-forget record, such as a reconcile ask, gets its single retry ring only on the local plane: the remote steer leg owes no re-ring, so a swallowed remote doorbell for one waits for the next ring into that inbox, and a remote-side retry is known follow-up scope." No behavior changed. Checks: tests/fm-documentation-audiences.test.sh passes (4/4) and bin/fm-lint.sh is clean. The change is left uncommitted in the working tree for the pipeline to pick up * no-mistakes(review): Hold automatic wakes until a mate's own decision closes * no-mistakes(document): Document watcher delivery of deferred remote re-read nudges * no-mistakes(review): Merge duplicate elapsed-wait reattach instructions in DOD * no-mistakes(test): Resolve merged default decision in remote-reply recovery fixture * no-mistakes(test): Source classify lib so config-push retry-deferred honors open decisions * no-mistakes(ci): Fixed a flaky test that also fails on main. Neither this PR's bin/fm-brief.sh nor its bin/fm-dod-lib.sh change is involved: bin/fm-dispatch-resolve.sh sources neither file. Another branch (fm-attended-cutover-smoothing-s1, run 36343879084) failed the same shard 8 check the same way, on a different case ("a rule-criterion match prints one diagnostic line, got 2"). Root cause: `fm_quota_single_provider_for_harness` in bin/fm-quota-axi-lib.sh returned from its `while read` loop as soon as it found a match. That closed the pipe while `fm_quota_single_provider_table`'s `printf` was sometimes still writing. GitHub Actions runners ignore SIGPIPE, so bash printed `fm-quota-axi-lib.sh: line 138: printf: write error: Broken pipe` to the resolver's stderr. That is the extra line. I reproduced it locally by running the test with SIGPIPE ignored: 2 of 20 runs failed, one with the resolver's diagnostic line plus two broken-pipe lines. Invariant: looking up a harness in the provider table must never make the table writer fail. The only reader of that table is this function, and all of the resolver's lookups (line 208 without stderr redirected, line 222 with it) go through it. So the fix is in that one place: read the whole table, then print the match. The same file now shows it reads the full table first, like `fm_control_harness_supported` does. Return values and output are unchanged. Verification: with SIGPIPE ignored, tests/fm-dispatch-resolve.test.sh failed 0 of 30 runs after the fix (2 of 20 before). tests/fm-dispatch-resolve.test.sh, tests/fm-brief.test.sh, tests/fm-send-inbox.test.sh, tests/fm-quota-choose.test.sh and tests/fm-quota-array-dispatch-live-e2e.test.sh all pass, and shellcheck is clean. tests/fm-procevent-quota.test.sh fails locally with or without the change ("process-event state root is not a private directory"), so that failure comes from the local environment, not from this fix. No new test was added: the existing one-diagnostic-line assertions already catch this whenever SIGPIPE is ignored, as it is in CI * Revert "no-mistakes(ci): Fixed a flaky test that also fails on main. Neither this PR's bin/fm-brief.sh nor its bin/fm-dod-lib.sh change is involved: bin/fm-dispatch-resolve.sh sources neither file. Another branch (fm-attended-cutover-smoothing-s1, run 36343879084) failed the same shard 8 check the same way, on a different case ("a rule-criterion match prints one diagnostic line, got 2"). Root cause: `fm_quota_single_provider_for_harness` in bin/fm-quota-axi-lib.sh returned from its `while read` loop as soon as it found a match. That closed the pipe while `fm_quota_single_provider_table`'s `printf` was sometimes still writing. GitHub Actions runners ignore SIGPIPE, so bash printed `fm-quota-axi-lib.sh: line 138: printf: write error: Broken pipe` to the resolver's stderr. That is the extra line. I reproduced it locally by running the test with SIGPIPE ignored: 2 of 20 runs failed, one with the resolver's diagnostic line plus two broken-pipe lines. Invariant: looking up a harness in the provider table must never make the table writer fail. The only reader of that table is this function, and all of the resolver's lookups (line 208 without stderr redirected, line 222 with it) go through it. So the fix is in that one place: read the whole table, then print the match. The same file now shows it reads the full table first, like `fm_control_harness_supported` does. Return values and output are unchanged. Verification: with SIGPIPE ignored, tests/fm-dispatch-resolve.test.sh failed 0 of 30 runs after the fix (2 of 20 before). tests/fm-dispatch-resolve.test.sh, tests/fm-brief.test.sh, tests/fm-send-inbox.test.sh, tests/fm-quota-choose.test.sh and tests/fm-quota-array-dispatch-live-e2e.test.sh all pass, and shellcheck is clean. tests/fm-procevent-quota.test.sh fails locally with or without the change ("process-event state root is not a private directory"), so that failure comes from the local environment, not from this fix. No new test was added: the existing one-diagnostic-line assertions already catch this whenever SIGPIPE is ignored, as it is in CI" This reverts commit c719928. * no-mistakes(review): Retry deferred local instruction nudges via the watcher * no-mistakes(review): Document watcher retry for deferred local instruction nudges * no-mistakes(ci): I fixed both review findings you selected (ci-1 and ci-3). I did not touch the deferral check in bin/fm-send.sh. ci-1 (bin/fm-config-push.sh, retry_deferred_rereads) - Rule that must hold: a deferred reread stays flagged until it is actually delivered. - Before the fix, the flag was removed before any of the steps that can skip a mate: the remote lock-path lookup, validate_secondmate_home, the local lock-path lookup, and the lock acquire. A skip at any of those dropped the flag, so the watcher lost track of the reread. - Now the flag is removed in one place only, when the send succeeds (rc 0). A skipped home, a busy lock, a deferred send (rc 4) or a failed send all leave it in place. The re-mark calls on a busy lock and on rc 4 were no longer needed, so I removed them. I updated the comment above the function to match. - Side effect: a send that keeps failing now stays flagged, so the watcher retries it on every poll and logs each failure. That follows your "don't clear until delivered" rule, but it replaces the old behaviour of leaving a failed send to the next config push or session start. - New test in tests/fm-secondmate-sync.test.sh: T8j "a deferred flag survives a skipped invalid home and is retried once it validates". It takes the home's marker away to make validation fail, checks that nothing is sent and the flag stays, then puts the marker back and checks that the nudge is delivered and both the flag and the retry marker are cleared. It fails on the old code and passes now. ci-3 (bin/fm-secondmate-restart.sh) - Rule that must hold: no automatic send wakes a mate that is waiting on its own open decision. - The two automatic sends in this script are the fallback reread nudge (fall_back_to_nudge) and the persist request. Both now pass --automatic. If a persist request is deferred, its correlation is discarded and the mate goes to the fallback nudge, which is also deferred, so the mate is reported as unreached. - New test in tests/fm-secondmate-restart.test.sh: T3b. It gives a mate an open needs-decision and runs a restart. It checks that both sends report as deferred, the mate's doorbell is never rung, its inbox gets no message, nothing is stopped, and the mate is reported as unreached with exit status 3. It fails on the old code and passes now. - The test marks the watcher as alive first. Without that, the watcher-down warning is printed first and becomes the reported reason instead of the deferral message. Verification - tests/fm-secondmate-sync.test.sh passes. - tests/fm-secondmate-restart.test.sh passes. - tests/fm-secondmate-harness.test.sh (the other test that exercises --retry-deferred) passes. - The fm-send-inbox test that covers automatic deferral passes. I only looked at the last lines of that run, not the whole file. - `shellcheck -x` on the four changed files is clean * Pin autoarm supervision model in secondmate restart T3b The fresh watcher beat the test writes proves a live watcher only under the autoarm model; on CI hosts with no detected harness the persistent model demands a lock-holding watcher, so the watcher-down banner became the reported reason and the deferral assertion failed. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Keep deferred secondmate nudges retryable under the inheritance lock. A bootstrap instruction nudge could write its deferral flag outside the lock the watcher retry holds, so a concurrent retry could delete a flag that had just been set. A restart fallback that is deferred now records the same marker and flag, so the watcher delivers it once the decision closes. * no-mistakes(document): Document watcher retry of deferred restart re-read nudges * Send secondmate reread and restart nudges immediately again. Deferring those nudges let a later config push drop an incomplete transfer once the decision closed. They now send as they do on main. * Make the no-turn wait opt-in behind config/wait-no-turns. Homes that do not create the file keep the previous briefs, drive text, and sends. * no-mistakes(document): Document wait-no-turns inbox wording change in configuration * no-mistakes(review): Keep checkpoint inbox check; forbid only polling while waiting * no-mistakes(ci): Fixed ci-2 (Greptile: a concurrent retry marker gets lost). The rule that was broken: the watcher may remove only the `.retry-ring` mark for the record it just processed. A newer mark written in the meantime is owed its own retry. `fm_task_inbox_clear_retry` is the one shared function that removes the mark, and I fixed it there. In `bin/fm-task-inbox-lib.sh` it now takes the record path. It compares the mark's content with that record's name and removes the mark only when they match. When the mark names a different record it returns success and leaves the mark alone. It still fails only when the processed record's own mark can't be removed. Both callers in `bin/fm-watch.sh` now pass `"$rec"`: the dead or missing pane path and the path after a retry ring. So the fix holds at both removal sites. Tests, in `tests/fm-task-inbox.test.sh`: - I added an optional `FM_RING_MARKS_RETRY` hook to the fake tmux. It writes a newer record's mark while the doorbell is being typed, which reproduces the race deterministically. - I added `test_watcher_retry_keeps_a_newer_mark`. The owed retry rings once, the newer mark survives, and a later check rings the newer record once and then clears its mark. The test fails without the fix ("the spent retry removed a newer record's mark written during its ring") and passes with it. - I updated the direct `clear_retry` call in the existing unit test to pass the record. Results: `tests/fm-task-inbox.test.sh` passes in full and `tests/fm-send-inbox.test.sh` passes 15/15. Shellcheck reports only SC1091 "not following sourced file" notices. As instructed, I didn't change the brief inbox wording * no-mistakes(document): Fix stale wait-no-turns inbox wording in inbox lib comment --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> Co-authored-by: Cursor Agent <cursoragent@cursor.com> Co-authored-by: Kun Chen <kunchenguid@users.noreply.github.com> --------- Co-authored-by: Christopher McKay <101884182+karotkriss@users.noreply.github.com> Co-authored-by: Tiago <tiagop@hey.com> Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> Co-authored-by: Cursor Agent <cursoragent@cursor.com> Co-authored-by: Kun Chen <kunchenguid@users.noreply.github.com>
cloud-practitioner
added a commit
to cloud-practitioner/firstmate
that referenced
this pull request
Oct 1, 2026
* fix(bin): run no repository hook when core.hooksPath is empty (kunchenguid#6216) * fix(bin): run no repository hook when core.hooksPath is empty The per-task hook wrapper refused every commit in a repository whose own config sets core.hooksPath to the empty string, because git rev-parse --git-path hooks fails on it. Plain git reads that setting as no hooks, so the wrapper now runs none; every other lookup failure still refuses and shows git's error. Fixes kunchenguid#6171 * no-mistakes(review): Refuse commits when core.hooksPath is a valueless key * no-mistakes(document): Document empty core.hooksPath handling in commit attribution docs * no-mistakes(ci): When the wrapper refuses a commit, Git's hook-lookup error now shows up once instead of twice. That required changing one line in the wrapper, and the tests were extended so both bad-config cases would catch the duplicate. Invariant: when the wrapper refuses, Git's lookup error must appear exactly once. In the failure path, the only Git call besides the deliberate second lookup is the `git config --get --type=path core.hooksPath` check in `runtime_chain_body` (`bin/fm-git-strip-ai-trailers.sh:168`). That check prints the same error, so it was the one place to fix. I added `2>/dev/null` to it. Its exit status still decides the outcome: an empty value still runs no hook, and anything else goes on to the second lookup, which prints Git's error once, and the commit is refused. Tests (`tests/fm-git-strip-ai-trailers.test.sh`): - The unresolvable-path test (`~fm-no-such-user-6171/hooks`) now requires `failed to expand user dir` to appear exactly once in the refused commit's output. - The valueless-key test now requires `missing value for 'core.hookspath'` to appear exactly once. - Pre-existing bug in the unresolvable-path test: its `git add` ran after the bad config was set, so it failed silently (exit 128) and the "refused commit" had nothing staged. The test now stages the file before writing the config, the same way the valueless test does, so a real commit gets refused. - The empty-string test is unchanged and still passes, so an empty `core.hooksPath` still runs no hook. Verification: - With the wrapper change reverted, both new checks fail with `expected '1', got '2'`. With the change in place, the whole suite passes. - `bash -n` passes. shellcheck shows only an info-level SC1091 note about sourcing `lib.sh`, which was already there before this change. - `git status` lists only the two intended files * fix(bin): let a stale record on a reassigned slot retire records-only (kunchenguid#6213) * fix(bin): let a stale record on a reassigned slot retire records-only When a pool slot's owner claim names another task, the stale record's teardown touches nothing under the slot, so the exclusive-slot record scan no longer refuses it. Full teardowns of a slot this task still claims, or one with no claim, keep the refusal. Fixes kunchenguid#6184 * no-mistakes(document): Note claim-over-record precedence for reassigned teardown slots * fix(bin): keep the steering doorbell short under deep homes (kunchenguid#6240) * fix(bin): keep the steering doorbell short under deep homes The doorbell printed the task inbox's absolute path twice, so under a deep home it grew to about 290 characters and a Herdr submit reported it never reached the pane on every re-ring. It now names the inbox once by its short <task>.inbox name and points at the full path the worker's brief already gives, so its length no longer depends on the home's depth. Fixes kunchenguid#6120 * no-mistakes(review): Export FM_TASK_INBOX at launch and name it in doorbell * no-mistakes(ci): ci-1 (Behavior portable serial 9) was caused by this PR, and I fixed it in the test. tests/fm-claude-trust.test.sh failed with "the launch command did not carry a brief doorbell". Its claude_launch_doorbell helper stripped exactly two leading `export ...;` statements before reading the final prompt argument. This PR adds a third one (`export FM_TASK_INBOX=...`) to every launch, so the helper was reading the wrong command. The invariant: a test that parses the launch command must skip every leading export statement, however many there are. I checked every test that parses the launch this way. The only other ones are the two helpers in tests/fm-spawn-dispatch-profile.test.sh, and they already loop over all exports. The kimi and dispatch-profile exact-string checks were updated earlier in this PR. The fix makes claude_launch_doorbell use the same loop (`while [[ "$command" == export\ *\;* ]]; do command=${command#*; }; done`) and then take the last argument. The ordinary path still works: the claude spawn test and the secondmate-clone spawn test both resolve the brief record through the same helper. Verified locally: `bash tests/fm-claude-trust.test.sh` exits 0 with no failing cases. ci-2 (Behavior tests (Herdr)) was not caused by this change, and I made no code change for it. In tests/fm-backend-herdr-presentation-e2e.test.sh, the concurrent secondmate recovery failed with "herdr presentation recovery could not acquire its session lock; refusing a concurrent resume". Two reasons it is not this PR: - The same failure, in the same test and case, happened on run 36655209015 for the unrelated branch fm/fm-contributions-old-gh-compat about 14 hours earlier. - This PR's change cannot lengthen how long the lock is held. The launch is written to a file and sent to the pane as `. launch.N.sh`, so the extra export changes neither the pane submit nor the lock hold time. The cause is a race that was already there: spawn_herdr_presentation_order_lock_acquire gives up after 5 seconds, and a concurrent real-Herdr recovery can hold the lock longer. Fixing that means changing the product's lock timeout, which is outside this PR. It should be tracked separately, and a rerun of the Herdr job is expected to pass. The only file changed is tests/fm-claude-trust.test.sh * feat(bin): add opt-in config/wait-no-turns so a waiting worker spends no turns (kunchenguid#4859) * fix(dod): drive no-mistakes with one foreground call, not a background poll The brief told workers to background the drive call and poll `axi status` because one call "routinely outlives what your harness lets a single command run". That advice contradicts the tool it drives: `no-mistakes axi run --help` documents `--wait` with an 8m default, existing precisely "so an agent harness with a 10-minute tool cap gets a structured return instead of an unbounded hang". Following the old text, a worker could never idle - a backgrounded call returns in milliseconds, so it does not wait at all - and each attempt leaked a live timer that later fired as a paid wake. Tell workers to make one foreground call, let it block, and repeat it when it returns on elapsed wait rather than on a gate or outcome. Also drops the generalisation that told workers on any unestablished harness to assume a command cap and use the same shape, which exported the defect to harnesses with no such cap. * fix(bin): let a waiting worker spend no turns until it is answered A worker waiting on a decision, a pipeline gate, CI, or a heavy-test slot kept taking model turns: the brief told it to list its inbox at any natural checkpoint, and six automatic senders nudged secondmates whatever their open decisions. - The ship and scout briefs gain one Waiting section: end the turn after needs-decision or blocked, and hold an external wait inside ONE blocking command bounded by the harness's own command ceiling. The checkpoint clause is deleted. Forbidding the wrong shapes is not enough on its own, so the section also names the blocking foreground `until` loop as the wait a Claude Code worker may use, because that harness can refuse a sleep-then-check command while pointing at backgrounding, which is the one shape a waiting worker must not take. - fm-send --automatic defers (exit 4, nothing written or rung) while the target has an open decision or blocker of its own; every automatic sender passes it and keeps its retry state, and the pending-reply recovery waits the same way. - The two senders that report the result classified it by matching the text of the send's captured output against `deferred:*`. fm-send runs bin/fm-guard.sh as a supervision warning, and that guard prints its worktree-tangle banner whenever the primary checkout is on a feature branch, which is exactly what a CI pull-request checkout is. The banner lands ahead of the `deferred:` line, so the match fell through and a waiting mate was reported as a failed send, with the banner as the reason. Both senders now classify on fm-send's exit status, which is the contract the deferral is actually stated in, and select the `deferred:` line out of the output rather than assuming it came first. The third root cause, a no-mistakes definition of done that backgrounded the drive call and polled axi status, is fixed by this branch's parent commit "drive no-mistakes with one foreground call, not a background poll"; this commit takes that text as is and adds the regression test. Upstream's spawn abort path no longer calls the lease-return helper at all, so the fork's missing-helper guard and its pin-feature test line are moot here and are not ported. The command ceilings each harness enforces, and the probes behind the named Claude Code wait, are recorded in docs/verification/runtime-backends.md. * no-mistakes(review): Exempt captain holds, quiet deferred reconcile, clarify worker pauses * no-mistakes(document): Document deferred automatic nudges, rereads, and reply recovery * no-mistakes(document): Ring unlanded fire-and-forget steers exactly once more * no-mistakes(ci): The failing check, "PR must be raised via no-mistakes", reads the pipeline's attestation record, which says document=skipped. No file in the repository can change that record, so I did not touch the check or the PR body. As you said, the no-mistakes rerun after this run finishes will re-execute the document step and record document=completed. The one change is the documentation sentence you ordered. It adds a line to docs/remote-secondmates.md, right after the line saying the remote host runs no re-ring ladder of its own: "A fire-and-forget record, such as a reconcile ask, gets its single retry ring only on the local plane: the remote steer leg owes no re-ring, so a swallowed remote doorbell for one waits for the next ring into that inbox, and a remote-side retry is known follow-up scope." No behavior changed. Checks: tests/fm-documentation-audiences.test.sh passes (4/4) and bin/fm-lint.sh is clean. The change is left uncommitted in the working tree for the pipeline to pick up * no-mistakes(review): Hold automatic wakes until a mate's own decision closes * no-mistakes(document): Document watcher delivery of deferred remote re-read nudges * no-mistakes(review): Merge duplicate elapsed-wait reattach instructions in DOD * no-mistakes(test): Resolve merged default decision in remote-reply recovery fixture * no-mistakes(test): Source classify lib so config-push retry-deferred honors open decisions * no-mistakes(ci): Fixed a flaky test that also fails on main. Neither this PR's bin/fm-brief.sh nor its bin/fm-dod-lib.sh change is involved: bin/fm-dispatch-resolve.sh sources neither file. Another branch (fm-attended-cutover-smoothing-s1, run 36343879084) failed the same shard 8 check the same way, on a different case ("a rule-criterion match prints one diagnostic line, got 2"). Root cause: `fm_quota_single_provider_for_harness` in bin/fm-quota-axi-lib.sh returned from its `while read` loop as soon as it found a match. That closed the pipe while `fm_quota_single_provider_table`'s `printf` was sometimes still writing. GitHub Actions runners ignore SIGPIPE, so bash printed `fm-quota-axi-lib.sh: line 138: printf: write error: Broken pipe` to the resolver's stderr. That is the extra line. I reproduced it locally by running the test with SIGPIPE ignored: 2 of 20 runs failed, one with the resolver's diagnostic line plus two broken-pipe lines. Invariant: looking up a harness in the provider table must never make the table writer fail. The only reader of that table is this function, and all of the resolver's lookups (line 208 without stderr redirected, line 222 with it) go through it. So the fix is in that one place: read the whole table, then print the match. The same file now shows it reads the full table first, like `fm_control_harness_supported` does. Return values and output are unchanged. Verification: with SIGPIPE ignored, tests/fm-dispatch-resolve.test.sh failed 0 of 30 runs after the fix (2 of 20 before). tests/fm-dispatch-resolve.test.sh, tests/fm-brief.test.sh, tests/fm-send-inbox.test.sh, tests/fm-quota-choose.test.sh and tests/fm-quota-array-dispatch-live-e2e.test.sh all pass, and shellcheck is clean. tests/fm-procevent-quota.test.sh fails locally with or without the change ("process-event state root is not a private directory"), so that failure comes from the local environment, not from this fix. No new test was added: the existing one-diagnostic-line assertions already catch this whenever SIGPIPE is ignored, as it is in CI * Revert "no-mistakes(ci): Fixed a flaky test that also fails on main. Neither this PR's bin/fm-brief.sh nor its bin/fm-dod-lib.sh change is involved: bin/fm-dispatch-resolve.sh sources neither file. Another branch (fm-attended-cutover-smoothing-s1, run 36343879084) failed the same shard 8 check the same way, on a different case ("a rule-criterion match prints one diagnostic line, got 2"). Root cause: `fm_quota_single_provider_for_harness` in bin/fm-quota-axi-lib.sh returned from its `while read` loop as soon as it found a match. That closed the pipe while `fm_quota_single_provider_table`'s `printf` was sometimes still writing. GitHub Actions runners ignore SIGPIPE, so bash printed `fm-quota-axi-lib.sh: line 138: printf: write error: Broken pipe` to the resolver's stderr. That is the extra line. I reproduced it locally by running the test with SIGPIPE ignored: 2 of 20 runs failed, one with the resolver's diagnostic line plus two broken-pipe lines. Invariant: looking up a harness in the provider table must never make the table writer fail. The only reader of that table is this function, and all of the resolver's lookups (line 208 without stderr redirected, line 222 with it) go through it. So the fix is in that one place: read the whole table, then print the match. The same file now shows it reads the full table first, like `fm_control_harness_supported` does. Return values and output are unchanged. Verification: with SIGPIPE ignored, tests/fm-dispatch-resolve.test.sh failed 0 of 30 runs after the fix (2 of 20 before). tests/fm-dispatch-resolve.test.sh, tests/fm-brief.test.sh, tests/fm-send-inbox.test.sh, tests/fm-quota-choose.test.sh and tests/fm-quota-array-dispatch-live-e2e.test.sh all pass, and shellcheck is clean. tests/fm-procevent-quota.test.sh fails locally with or without the change ("process-event state root is not a private directory"), so that failure comes from the local environment, not from this fix. No new test was added: the existing one-diagnostic-line assertions already catch this whenever SIGPIPE is ignored, as it is in CI" This reverts commit c719928. * no-mistakes(review): Retry deferred local instruction nudges via the watcher * no-mistakes(review): Document watcher retry for deferred local instruction nudges * no-mistakes(ci): I fixed both review findings you selected (ci-1 and ci-3). I did not touch the deferral check in bin/fm-send.sh. ci-1 (bin/fm-config-push.sh, retry_deferred_rereads) - Rule that must hold: a deferred reread stays flagged until it is actually delivered. - Before the fix, the flag was removed before any of the steps that can skip a mate: the remote lock-path lookup, validate_secondmate_home, the local lock-path lookup, and the lock acquire. A skip at any of those dropped the flag, so the watcher lost track of the reread. - Now the flag is removed in one place only, when the send succeeds (rc 0). A skipped home, a busy lock, a deferred send (rc 4) or a failed send all leave it in place. The re-mark calls on a busy lock and on rc 4 were no longer needed, so I removed them. I updated the comment above the function to match. - Side effect: a send that keeps failing now stays flagged, so the watcher retries it on every poll and logs each failure. That follows your "don't clear until delivered" rule, but it replaces the old behaviour of leaving a failed send to the next config push or session start. - New test in tests/fm-secondmate-sync.test.sh: T8j "a deferred flag survives a skipped invalid home and is retried once it validates". It takes the home's marker away to make validation fail, checks that nothing is sent and the flag stays, then puts the marker back and checks that the nudge is delivered and both the flag and the retry marker are cleared. It fails on the old code and passes now. ci-3 (bin/fm-secondmate-restart.sh) - Rule that must hold: no automatic send wakes a mate that is waiting on its own open decision. - The two automatic sends in this script are the fallback reread nudge (fall_back_to_nudge) and the persist request. Both now pass --automatic. If a persist request is deferred, its correlation is discarded and the mate goes to the fallback nudge, which is also deferred, so the mate is reported as unreached. - New test in tests/fm-secondmate-restart.test.sh: T3b. It gives a mate an open needs-decision and runs a restart. It checks that both sends report as deferred, the mate's doorbell is never rung, its inbox gets no message, nothing is stopped, and the mate is reported as unreached with exit status 3. It fails on the old code and passes now. - The test marks the watcher as alive first. Without that, the watcher-down warning is printed first and becomes the reported reason instead of the deferral message. Verification - tests/fm-secondmate-sync.test.sh passes. - tests/fm-secondmate-restart.test.sh passes. - tests/fm-secondmate-harness.test.sh (the other test that exercises --retry-deferred) passes. - The fm-send-inbox test that covers automatic deferral passes. I only looked at the last lines of that run, not the whole file. - `shellcheck -x` on the four changed files is clean * Pin autoarm supervision model in secondmate restart T3b The fresh watcher beat the test writes proves a live watcher only under the autoarm model; on CI hosts with no detected harness the persistent model demands a lock-holding watcher, so the watcher-down banner became the reported reason and the deferral assertion failed. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Keep deferred secondmate nudges retryable under the inheritance lock. A bootstrap instruction nudge could write its deferral flag outside the lock the watcher retry holds, so a concurrent retry could delete a flag that had just been set. A restart fallback that is deferred now records the same marker and flag, so the watcher delivers it once the decision closes. * no-mistakes(document): Document watcher retry of deferred restart re-read nudges * Send secondmate reread and restart nudges immediately again. Deferring those nudges let a later config push drop an incomplete transfer once the decision closed. They now send as they do on main. * Make the no-turn wait opt-in behind config/wait-no-turns. Homes that do not create the file keep the previous briefs, drive text, and sends. * no-mistakes(document): Document wait-no-turns inbox wording change in configuration * no-mistakes(review): Keep checkpoint inbox check; forbid only polling while waiting * no-mistakes(ci): Fixed ci-2 (Greptile: a concurrent retry marker gets lost). The rule that was broken: the watcher may remove only the `.retry-ring` mark for the record it just processed. A newer mark written in the meantime is owed its own retry. `fm_task_inbox_clear_retry` is the one shared function that removes the mark, and I fixed it there. In `bin/fm-task-inbox-lib.sh` it now takes the record path. It compares the mark's content with that record's name and removes the mark only when they match. When the mark names a different record it returns success and leaves the mark alone. It still fails only when the processed record's own mark can't be removed. Both callers in `bin/fm-watch.sh` now pass `"$rec"`: the dead or missing pane path and the path after a retry ring. So the fix holds at both removal sites. Tests, in `tests/fm-task-inbox.test.sh`: - I added an optional `FM_RING_MARKS_RETRY` hook to the fake tmux. It writes a newer record's mark while the doorbell is being typed, which reproduces the race deterministically. - I added `test_watcher_retry_keeps_a_newer_mark`. The owed retry rings once, the newer mark survives, and a later check rings the newer record once and then clears its mark. The test fails without the fix ("the spent retry removed a newer record's mark written during its ring") and passes with it. - I updated the direct `clear_retry` call in the existing unit test to pass the record. Results: `tests/fm-task-inbox.test.sh` passes in full and `tests/fm-send-inbox.test.sh` passes 15/15. Shellcheck reports only SC1091 "not following sourced file" notices. As instructed, I didn't change the brief inbox wording * no-mistakes(document): Fix stale wait-no-turns inbox wording in inbox lib comment --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> Co-authored-by: Cursor Agent <cursoragent@cursor.com> Co-authored-by: Kun Chen <kunchenguid@users.noreply.github.com> --------- Co-authored-by: Christopher McKay <101884182+karotkriss@users.noreply.github.com> Co-authored-by: Tiago <tiagop@hey.com> Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> Co-authored-by: Cursor Agent <cursoragent@cursor.com> Co-authored-by: Kun Chen <kunchenguid@users.noreply.github.com>
cloud-practitioner
added a commit
to cloud-practitioner/firstmate
that referenced
this pull request
Oct 1, 2026
…d from zsh (#17) * fix(bin): make fm_backend_source sibling checks work under zsh * no-mistakes(review): Rename zsh-special local path in fm_backend_source * merge upstream (#19) * fix(bin): run no repository hook when core.hooksPath is empty (kunchenguid#6216) * fix(bin): run no repository hook when core.hooksPath is empty The per-task hook wrapper refused every commit in a repository whose own config sets core.hooksPath to the empty string, because git rev-parse --git-path hooks fails on it. Plain git reads that setting as no hooks, so the wrapper now runs none; every other lookup failure still refuses and shows git's error. Fixes kunchenguid#6171 * no-mistakes(review): Refuse commits when core.hooksPath is a valueless key * no-mistakes(document): Document empty core.hooksPath handling in commit attribution docs * no-mistakes(ci): When the wrapper refuses a commit, Git's hook-lookup error now shows up once instead of twice. That required changing one line in the wrapper, and the tests were extended so both bad-config cases would catch the duplicate. Invariant: when the wrapper refuses, Git's lookup error must appear exactly once. In the failure path, the only Git call besides the deliberate second lookup is the `git config --get --type=path core.hooksPath` check in `runtime_chain_body` (`bin/fm-git-strip-ai-trailers.sh:168`). That check prints the same error, so it was the one place to fix. I added `2>/dev/null` to it. Its exit status still decides the outcome: an empty value still runs no hook, and anything else goes on to the second lookup, which prints Git's error once, and the commit is refused. Tests (`tests/fm-git-strip-ai-trailers.test.sh`): - The unresolvable-path test (`~fm-no-such-user-6171/hooks`) now requires `failed to expand user dir` to appear exactly once in the refused commit's output. - The valueless-key test now requires `missing value for 'core.hookspath'` to appear exactly once. - Pre-existing bug in the unresolvable-path test: its `git add` ran after the bad config was set, so it failed silently (exit 128) and the "refused commit" had nothing staged. The test now stages the file before writing the config, the same way the valueless test does, so a real commit gets refused. - The empty-string test is unchanged and still passes, so an empty `core.hooksPath` still runs no hook. Verification: - With the wrapper change reverted, both new checks fail with `expected '1', got '2'`. With the change in place, the whole suite passes. - `bash -n` passes. shellcheck shows only an info-level SC1091 note about sourcing `lib.sh`, which was already there before this change. - `git status` lists only the two intended files * fix(bin): let a stale record on a reassigned slot retire records-only (kunchenguid#6213) * fix(bin): let a stale record on a reassigned slot retire records-only When a pool slot's owner claim names another task, the stale record's teardown touches nothing under the slot, so the exclusive-slot record scan no longer refuses it. Full teardowns of a slot this task still claims, or one with no claim, keep the refusal. Fixes kunchenguid#6184 * no-mistakes(document): Note claim-over-record precedence for reassigned teardown slots * fix(bin): keep the steering doorbell short under deep homes (kunchenguid#6240) * fix(bin): keep the steering doorbell short under deep homes The doorbell printed the task inbox's absolute path twice, so under a deep home it grew to about 290 characters and a Herdr submit reported it never reached the pane on every re-ring. It now names the inbox once by its short <task>.inbox name and points at the full path the worker's brief already gives, so its length no longer depends on the home's depth. Fixes kunchenguid#6120 * no-mistakes(review): Export FM_TASK_INBOX at launch and name it in doorbell * no-mistakes(ci): ci-1 (Behavior portable serial 9) was caused by this PR, and I fixed it in the test. tests/fm-claude-trust.test.sh failed with "the launch command did not carry a brief doorbell". Its claude_launch_doorbell helper stripped exactly two leading `export ...;` statements before reading the final prompt argument. This PR adds a third one (`export FM_TASK_INBOX=...`) to every launch, so the helper was reading the wrong command. The invariant: a test that parses the launch command must skip every leading export statement, however many there are. I checked every test that parses the launch this way. The only other ones are the two helpers in tests/fm-spawn-dispatch-profile.test.sh, and they already loop over all exports. The kimi and dispatch-profile exact-string checks were updated earlier in this PR. The fix makes claude_launch_doorbell use the same loop (`while [[ "$command" == export\ *\;* ]]; do command=${command#*; }; done`) and then take the last argument. The ordinary path still works: the claude spawn test and the secondmate-clone spawn test both resolve the brief record through the same helper. Verified locally: `bash tests/fm-claude-trust.test.sh` exits 0 with no failing cases. ci-2 (Behavior tests (Herdr)) was not caused by this change, and I made no code change for it. In tests/fm-backend-herdr-presentation-e2e.test.sh, the concurrent secondmate recovery failed with "herdr presentation recovery could not acquire its session lock; refusing a concurrent resume". Two reasons it is not this PR: - The same failure, in the same test and case, happened on run 36655209015 for the unrelated branch fm/fm-contributions-old-gh-compat about 14 hours earlier. - This PR's change cannot lengthen how long the lock is held. The launch is written to a file and sent to the pane as `. launch.N.sh`, so the extra export changes neither the pane submit nor the lock hold time. The cause is a race that was already there: spawn_herdr_presentation_order_lock_acquire gives up after 5 seconds, and a concurrent real-Herdr recovery can hold the lock longer. Fixing that means changing the product's lock timeout, which is outside this PR. It should be tracked separately, and a rerun of the Herdr job is expected to pass. The only file changed is tests/fm-claude-trust.test.sh * feat(bin): add opt-in config/wait-no-turns so a waiting worker spends no turns (kunchenguid#4859) * fix(dod): drive no-mistakes with one foreground call, not a background poll The brief told workers to background the drive call and poll `axi status` because one call "routinely outlives what your harness lets a single command run". That advice contradicts the tool it drives: `no-mistakes axi run --help` documents `--wait` with an 8m default, existing precisely "so an agent harness with a 10-minute tool cap gets a structured return instead of an unbounded hang". Following the old text, a worker could never idle - a backgrounded call returns in milliseconds, so it does not wait at all - and each attempt leaked a live timer that later fired as a paid wake. Tell workers to make one foreground call, let it block, and repeat it when it returns on elapsed wait rather than on a gate or outcome. Also drops the generalisation that told workers on any unestablished harness to assume a command cap and use the same shape, which exported the defect to harnesses with no such cap. * fix(bin): let a waiting worker spend no turns until it is answered A worker waiting on a decision, a pipeline gate, CI, or a heavy-test slot kept taking model turns: the brief told it to list its inbox at any natural checkpoint, and six automatic senders nudged secondmates whatever their open decisions. - The ship and scout briefs gain one Waiting section: end the turn after needs-decision or blocked, and hold an external wait inside ONE blocking command bounded by the harness's own command ceiling. The checkpoint clause is deleted. Forbidding the wrong shapes is not enough on its own, so the section also names the blocking foreground `until` loop as the wait a Claude Code worker may use, because that harness can refuse a sleep-then-check command while pointing at backgrounding, which is the one shape a waiting worker must not take. - fm-send --automatic defers (exit 4, nothing written or rung) while the target has an open decision or blocker of its own; every automatic sender passes it and keeps its retry state, and the pending-reply recovery waits the same way. - The two senders that report the result classified it by matching the text of the send's captured output against `deferred:*`. fm-send runs bin/fm-guard.sh as a supervision warning, and that guard prints its worktree-tangle banner whenever the primary checkout is on a feature branch, which is exactly what a CI pull-request checkout is. The banner lands ahead of the `deferred:` line, so the match fell through and a waiting mate was reported as a failed send, with the banner as the reason. Both senders now classify on fm-send's exit status, which is the contract the deferral is actually stated in, and select the `deferred:` line out of the output rather than assuming it came first. The third root cause, a no-mistakes definition of done that backgrounded the drive call and polled axi status, is fixed by this branch's parent commit "drive no-mistakes with one foreground call, not a background poll"; this commit takes that text as is and adds the regression test. Upstream's spawn abort path no longer calls the lease-return helper at all, so the fork's missing-helper guard and its pin-feature test line are moot here and are not ported. The command ceilings each harness enforces, and the probes behind the named Claude Code wait, are recorded in docs/verification/runtime-backends.md. * no-mistakes(review): Exempt captain holds, quiet deferred reconcile, clarify worker pauses * no-mistakes(document): Document deferred automatic nudges, rereads, and reply recovery * no-mistakes(document): Ring unlanded fire-and-forget steers exactly once more * no-mistakes(ci): The failing check, "PR must be raised via no-mistakes", reads the pipeline's attestation record, which says document=skipped. No file in the repository can change that record, so I did not touch the check or the PR body. As you said, the no-mistakes rerun after this run finishes will re-execute the document step and record document=completed. The one change is the documentation sentence you ordered. It adds a line to docs/remote-secondmates.md, right after the line saying the remote host runs no re-ring ladder of its own: "A fire-and-forget record, such as a reconcile ask, gets its single retry ring only on the local plane: the remote steer leg owes no re-ring, so a swallowed remote doorbell for one waits for the next ring into that inbox, and a remote-side retry is known follow-up scope." No behavior changed. Checks: tests/fm-documentation-audiences.test.sh passes (4/4) and bin/fm-lint.sh is clean. The change is left uncommitted in the working tree for the pipeline to pick up * no-mistakes(review): Hold automatic wakes until a mate's own decision closes * no-mistakes(document): Document watcher delivery of deferred remote re-read nudges * no-mistakes(review): Merge duplicate elapsed-wait reattach instructions in DOD * no-mistakes(test): Resolve merged default decision in remote-reply recovery fixture * no-mistakes(test): Source classify lib so config-push retry-deferred honors open decisions * no-mistakes(ci): Fixed a flaky test that also fails on main. Neither this PR's bin/fm-brief.sh nor its bin/fm-dod-lib.sh change is involved: bin/fm-dispatch-resolve.sh sources neither file. Another branch (fm-attended-cutover-smoothing-s1, run 36343879084) failed the same shard 8 check the same way, on a different case ("a rule-criterion match prints one diagnostic line, got 2"). Root cause: `fm_quota_single_provider_for_harness` in bin/fm-quota-axi-lib.sh returned from its `while read` loop as soon as it found a match. That closed the pipe while `fm_quota_single_provider_table`'s `printf` was sometimes still writing. GitHub Actions runners ignore SIGPIPE, so bash printed `fm-quota-axi-lib.sh: line 138: printf: write error: Broken pipe` to the resolver's stderr. That is the extra line. I reproduced it locally by running the test with SIGPIPE ignored: 2 of 20 runs failed, one with the resolver's diagnostic line plus two broken-pipe lines. Invariant: looking up a harness in the provider table must never make the table writer fail. The only reader of that table is this function, and all of the resolver's lookups (line 208 without stderr redirected, line 222 with it) go through it. So the fix is in that one place: read the whole table, then print the match. The same file now shows it reads the full table first, like `fm_control_harness_supported` does. Return values and output are unchanged. Verification: with SIGPIPE ignored, tests/fm-dispatch-resolve.test.sh failed 0 of 30 runs after the fix (2 of 20 before). tests/fm-dispatch-resolve.test.sh, tests/fm-brief.test.sh, tests/fm-send-inbox.test.sh, tests/fm-quota-choose.test.sh and tests/fm-quota-array-dispatch-live-e2e.test.sh all pass, and shellcheck is clean. tests/fm-procevent-quota.test.sh fails locally with or without the change ("process-event state root is not a private directory"), so that failure comes from the local environment, not from this fix. No new test was added: the existing one-diagnostic-line assertions already catch this whenever SIGPIPE is ignored, as it is in CI * Revert "no-mistakes(ci): Fixed a flaky test that also fails on main. Neither this PR's bin/fm-brief.sh nor its bin/fm-dod-lib.sh change is involved: bin/fm-dispatch-resolve.sh sources neither file. Another branch (fm-attended-cutover-smoothing-s1, run 36343879084) failed the same shard 8 check the same way, on a different case ("a rule-criterion match prints one diagnostic line, got 2"). Root cause: `fm_quota_single_provider_for_harness` in bin/fm-quota-axi-lib.sh returned from its `while read` loop as soon as it found a match. That closed the pipe while `fm_quota_single_provider_table`'s `printf` was sometimes still writing. GitHub Actions runners ignore SIGPIPE, so bash printed `fm-quota-axi-lib.sh: line 138: printf: write error: Broken pipe` to the resolver's stderr. That is the extra line. I reproduced it locally by running the test with SIGPIPE ignored: 2 of 20 runs failed, one with the resolver's diagnostic line plus two broken-pipe lines. Invariant: looking up a harness in the provider table must never make the table writer fail. The only reader of that table is this function, and all of the resolver's lookups (line 208 without stderr redirected, line 222 with it) go through it. So the fix is in that one place: read the whole table, then print the match. The same file now shows it reads the full table first, like `fm_control_harness_supported` does. Return values and output are unchanged. Verification: with SIGPIPE ignored, tests/fm-dispatch-resolve.test.sh failed 0 of 30 runs after the fix (2 of 20 before). tests/fm-dispatch-resolve.test.sh, tests/fm-brief.test.sh, tests/fm-send-inbox.test.sh, tests/fm-quota-choose.test.sh and tests/fm-quota-array-dispatch-live-e2e.test.sh all pass, and shellcheck is clean. tests/fm-procevent-quota.test.sh fails locally with or without the change ("process-event state root is not a private directory"), so that failure comes from the local environment, not from this fix. No new test was added: the existing one-diagnostic-line assertions already catch this whenever SIGPIPE is ignored, as it is in CI" This reverts commit c719928. * no-mistakes(review): Retry deferred local instruction nudges via the watcher * no-mistakes(review): Document watcher retry for deferred local instruction nudges * no-mistakes(ci): I fixed both review findings you selected (ci-1 and ci-3). I did not touch the deferral check in bin/fm-send.sh. ci-1 (bin/fm-config-push.sh, retry_deferred_rereads) - Rule that must hold: a deferred reread stays flagged until it is actually delivered. - Before the fix, the flag was removed before any of the steps that can skip a mate: the remote lock-path lookup, validate_secondmate_home, the local lock-path lookup, and the lock acquire. A skip at any of those dropped the flag, so the watcher lost track of the reread. - Now the flag is removed in one place only, when the send succeeds (rc 0). A skipped home, a busy lock, a deferred send (rc 4) or a failed send all leave it in place. The re-mark calls on a busy lock and on rc 4 were no longer needed, so I removed them. I updated the comment above the function to match. - Side effect: a send that keeps failing now stays flagged, so the watcher retries it on every poll and logs each failure. That follows your "don't clear until delivered" rule, but it replaces the old behaviour of leaving a failed send to the next config push or session start. - New test in tests/fm-secondmate-sync.test.sh: T8j "a deferred flag survives a skipped invalid home and is retried once it validates". It takes the home's marker away to make validation fail, checks that nothing is sent and the flag stays, then puts the marker back and checks that the nudge is delivered and both the flag and the retry marker are cleared. It fails on the old code and passes now. ci-3 (bin/fm-secondmate-restart.sh) - Rule that must hold: no automatic send wakes a mate that is waiting on its own open decision. - The two automatic sends in this script are the fallback reread nudge (fall_back_to_nudge) and the persist request. Both now pass --automatic. If a persist request is deferred, its correlation is discarded and the mate goes to the fallback nudge, which is also deferred, so the mate is reported as unreached. - New test in tests/fm-secondmate-restart.test.sh: T3b. It gives a mate an open needs-decision and runs a restart. It checks that both sends report as deferred, the mate's doorbell is never rung, its inbox gets no message, nothing is stopped, and the mate is reported as unreached with exit status 3. It fails on the old code and passes now. - The test marks the watcher as alive first. Without that, the watcher-down warning is printed first and becomes the reported reason instead of the deferral message. Verification - tests/fm-secondmate-sync.test.sh passes. - tests/fm-secondmate-restart.test.sh passes. - tests/fm-secondmate-harness.test.sh (the other test that exercises --retry-deferred) passes. - The fm-send-inbox test that covers automatic deferral passes. I only looked at the last lines of that run, not the whole file. - `shellcheck -x` on the four changed files is clean * Pin autoarm supervision model in secondmate restart T3b The fresh watcher beat the test writes proves a live watcher only under the autoarm model; on CI hosts with no detected harness the persistent model demands a lock-holding watcher, so the watcher-down banner became the reported reason and the deferral assertion failed. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Keep deferred secondmate nudges retryable under the inheritance lock. A bootstrap instruction nudge could write its deferral flag outside the lock the watcher retry holds, so a concurrent retry could delete a flag that had just been set. A restart fallback that is deferred now records the same marker and flag, so the watcher delivers it once the decision closes. * no-mistakes(document): Document watcher retry of deferred restart re-read nudges * Send secondmate reread and restart nudges immediately again. Deferring those nudges let a later config push drop an incomplete transfer once the decision closed. They now send as they do on main. * Make the no-turn wait opt-in behind config/wait-no-turns. Homes that do not create the file keep the previous briefs, drive text, and sends. * no-mistakes(document): Document wait-no-turns inbox wording change in configuration * no-mistakes(review): Keep checkpoint inbox check; forbid only polling while waiting * no-mistakes(ci): Fixed ci-2 (Greptile: a concurrent retry marker gets lost). The rule that was broken: the watcher may remove only the `.retry-ring` mark for the record it just processed. A newer mark written in the meantime is owed its own retry. `fm_task_inbox_clear_retry` is the one shared function that removes the mark, and I fixed it there. In `bin/fm-task-inbox-lib.sh` it now takes the record path. It compares the mark's content with that record's name and removes the mark only when they match. When the mark names a different record it returns success and leaves the mark alone. It still fails only when the processed record's own mark can't be removed. Both callers in `bin/fm-watch.sh` now pass `"$rec"`: the dead or missing pane path and the path after a retry ring. So the fix holds at both removal sites. Tests, in `tests/fm-task-inbox.test.sh`: - I added an optional `FM_RING_MARKS_RETRY` hook to the fake tmux. It writes a newer record's mark while the doorbell is being typed, which reproduces the race deterministically. - I added `test_watcher_retry_keeps_a_newer_mark`. The owed retry rings once, the newer mark survives, and a later check rings the newer record once and then clears its mark. The test fails without the fix ("the spent retry removed a newer record's mark written during its ring") and passes with it. - I updated the direct `clear_retry` call in the existing unit test to pass the record. Results: `tests/fm-task-inbox.test.sh` passes in full and `tests/fm-send-inbox.test.sh` passes 15/15. Shellcheck reports only SC1091 "not following sourced file" notices. As instructed, I didn't change the brief inbox wording * no-mistakes(document): Fix stale wait-no-turns inbox wording in inbox lib comment --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> Co-authored-by: Cursor Agent <cursoragent@cursor.com> Co-authored-by: Kun Chen <kunchenguid@users.noreply.github.com> --------- Co-authored-by: Christopher McKay <101884182+karotkriss@users.noreply.github.com> Co-authored-by: Tiago <tiagop@hey.com> Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> Co-authored-by: Cursor Agent <cursoragent@cursor.com> Co-authored-by: Kun Chen <kunchenguid@users.noreply.github.com> * no-mistakes(review): Load backend sibling libraries under zsh and tighten zsh test --------- Co-authored-by: Christopher McKay <101884182+karotkriss@users.noreply.github.com> Co-authored-by: Tiago <tiagop@hey.com> Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> Co-authored-by: Cursor Agent <cursoragent@cursor.com> Co-authored-by: Kun Chen <kunchenguid@users.noreply.github.com>
cloud-practitioner
added a commit
to cloud-practitioner/firstmate
that referenced
this pull request
Oct 1, 2026
… smoke test (#15) * test(herdr): pin a neutral shell prompt in the control smoke pane The real-Herdr control smoke test runs its task pane in the developer's login shell. A prompt drawn with claude's composer glyph, such as a starship-style `❯` zsh prompt, sits in the viewport as a bare agent composer row holding text, so the final exit case read the composer as pending and refused with the pending-text message instead of the not-proven-empty one the test asserts. Both refusals are safe, but the assertion depended on the developer's prompt rather than on the product. Replace the pane shell with an rc-free bash using a shell-glyph prompt and clear the screen before any case runs, so the composer verdict is set by the test alone. * no-mistakes(test): Stop the smoke test's shell from writing to bash history * merge upstream (#19) * fix(bin): run no repository hook when core.hooksPath is empty (kunchenguid#6216) * fix(bin): run no repository hook when core.hooksPath is empty The per-task hook wrapper refused every commit in a repository whose own config sets core.hooksPath to the empty string, because git rev-parse --git-path hooks fails on it. Plain git reads that setting as no hooks, so the wrapper now runs none; every other lookup failure still refuses and shows git's error. Fixes kunchenguid#6171 * no-mistakes(review): Refuse commits when core.hooksPath is a valueless key * no-mistakes(document): Document empty core.hooksPath handling in commit attribution docs * no-mistakes(ci): When the wrapper refuses a commit, Git's hook-lookup error now shows up once instead of twice. That required changing one line in the wrapper, and the tests were extended so both bad-config cases would catch the duplicate. Invariant: when the wrapper refuses, Git's lookup error must appear exactly once. In the failure path, the only Git call besides the deliberate second lookup is the `git config --get --type=path core.hooksPath` check in `runtime_chain_body` (`bin/fm-git-strip-ai-trailers.sh:168`). That check prints the same error, so it was the one place to fix. I added `2>/dev/null` to it. Its exit status still decides the outcome: an empty value still runs no hook, and anything else goes on to the second lookup, which prints Git's error once, and the commit is refused. Tests (`tests/fm-git-strip-ai-trailers.test.sh`): - The unresolvable-path test (`~fm-no-such-user-6171/hooks`) now requires `failed to expand user dir` to appear exactly once in the refused commit's output. - The valueless-key test now requires `missing value for 'core.hookspath'` to appear exactly once. - Pre-existing bug in the unresolvable-path test: its `git add` ran after the bad config was set, so it failed silently (exit 128) and the "refused commit" had nothing staged. The test now stages the file before writing the config, the same way the valueless test does, so a real commit gets refused. - The empty-string test is unchanged and still passes, so an empty `core.hooksPath` still runs no hook. Verification: - With the wrapper change reverted, both new checks fail with `expected '1', got '2'`. With the change in place, the whole suite passes. - `bash -n` passes. shellcheck shows only an info-level SC1091 note about sourcing `lib.sh`, which was already there before this change. - `git status` lists only the two intended files * fix(bin): let a stale record on a reassigned slot retire records-only (kunchenguid#6213) * fix(bin): let a stale record on a reassigned slot retire records-only When a pool slot's owner claim names another task, the stale record's teardown touches nothing under the slot, so the exclusive-slot record scan no longer refuses it. Full teardowns of a slot this task still claims, or one with no claim, keep the refusal. Fixes kunchenguid#6184 * no-mistakes(document): Note claim-over-record precedence for reassigned teardown slots * fix(bin): keep the steering doorbell short under deep homes (kunchenguid#6240) * fix(bin): keep the steering doorbell short under deep homes The doorbell printed the task inbox's absolute path twice, so under a deep home it grew to about 290 characters and a Herdr submit reported it never reached the pane on every re-ring. It now names the inbox once by its short <task>.inbox name and points at the full path the worker's brief already gives, so its length no longer depends on the home's depth. Fixes kunchenguid#6120 * no-mistakes(review): Export FM_TASK_INBOX at launch and name it in doorbell * no-mistakes(ci): ci-1 (Behavior portable serial 9) was caused by this PR, and I fixed it in the test. tests/fm-claude-trust.test.sh failed with "the launch command did not carry a brief doorbell". Its claude_launch_doorbell helper stripped exactly two leading `export ...;` statements before reading the final prompt argument. This PR adds a third one (`export FM_TASK_INBOX=...`) to every launch, so the helper was reading the wrong command. The invariant: a test that parses the launch command must skip every leading export statement, however many there are. I checked every test that parses the launch this way. The only other ones are the two helpers in tests/fm-spawn-dispatch-profile.test.sh, and they already loop over all exports. The kimi and dispatch-profile exact-string checks were updated earlier in this PR. The fix makes claude_launch_doorbell use the same loop (`while [[ "$command" == export\ *\;* ]]; do command=${command#*; }; done`) and then take the last argument. The ordinary path still works: the claude spawn test and the secondmate-clone spawn test both resolve the brief record through the same helper. Verified locally: `bash tests/fm-claude-trust.test.sh` exits 0 with no failing cases. ci-2 (Behavior tests (Herdr)) was not caused by this change, and I made no code change for it. In tests/fm-backend-herdr-presentation-e2e.test.sh, the concurrent secondmate recovery failed with "herdr presentation recovery could not acquire its session lock; refusing a concurrent resume". Two reasons it is not this PR: - The same failure, in the same test and case, happened on run 36655209015 for the unrelated branch fm/fm-contributions-old-gh-compat about 14 hours earlier. - This PR's change cannot lengthen how long the lock is held. The launch is written to a file and sent to the pane as `. launch.N.sh`, so the extra export changes neither the pane submit nor the lock hold time. The cause is a race that was already there: spawn_herdr_presentation_order_lock_acquire gives up after 5 seconds, and a concurrent real-Herdr recovery can hold the lock longer. Fixing that means changing the product's lock timeout, which is outside this PR. It should be tracked separately, and a rerun of the Herdr job is expected to pass. The only file changed is tests/fm-claude-trust.test.sh * feat(bin): add opt-in config/wait-no-turns so a waiting worker spends no turns (kunchenguid#4859) * fix(dod): drive no-mistakes with one foreground call, not a background poll The brief told workers to background the drive call and poll `axi status` because one call "routinely outlives what your harness lets a single command run". That advice contradicts the tool it drives: `no-mistakes axi run --help` documents `--wait` with an 8m default, existing precisely "so an agent harness with a 10-minute tool cap gets a structured return instead of an unbounded hang". Following the old text, a worker could never idle - a backgrounded call returns in milliseconds, so it does not wait at all - and each attempt leaked a live timer that later fired as a paid wake. Tell workers to make one foreground call, let it block, and repeat it when it returns on elapsed wait rather than on a gate or outcome. Also drops the generalisation that told workers on any unestablished harness to assume a command cap and use the same shape, which exported the defect to harnesses with no such cap. * fix(bin): let a waiting worker spend no turns until it is answered A worker waiting on a decision, a pipeline gate, CI, or a heavy-test slot kept taking model turns: the brief told it to list its inbox at any natural checkpoint, and six automatic senders nudged secondmates whatever their open decisions. - The ship and scout briefs gain one Waiting section: end the turn after needs-decision or blocked, and hold an external wait inside ONE blocking command bounded by the harness's own command ceiling. The checkpoint clause is deleted. Forbidding the wrong shapes is not enough on its own, so the section also names the blocking foreground `until` loop as the wait a Claude Code worker may use, because that harness can refuse a sleep-then-check command while pointing at backgrounding, which is the one shape a waiting worker must not take. - fm-send --automatic defers (exit 4, nothing written or rung) while the target has an open decision or blocker of its own; every automatic sender passes it and keeps its retry state, and the pending-reply recovery waits the same way. - The two senders that report the result classified it by matching the text of the send's captured output against `deferred:*`. fm-send runs bin/fm-guard.sh as a supervision warning, and that guard prints its worktree-tangle banner whenever the primary checkout is on a feature branch, which is exactly what a CI pull-request checkout is. The banner lands ahead of the `deferred:` line, so the match fell through and a waiting mate was reported as a failed send, with the banner as the reason. Both senders now classify on fm-send's exit status, which is the contract the deferral is actually stated in, and select the `deferred:` line out of the output rather than assuming it came first. The third root cause, a no-mistakes definition of done that backgrounded the drive call and polled axi status, is fixed by this branch's parent commit "drive no-mistakes with one foreground call, not a background poll"; this commit takes that text as is and adds the regression test. Upstream's spawn abort path no longer calls the lease-return helper at all, so the fork's missing-helper guard and its pin-feature test line are moot here and are not ported. The command ceilings each harness enforces, and the probes behind the named Claude Code wait, are recorded in docs/verification/runtime-backends.md. * no-mistakes(review): Exempt captain holds, quiet deferred reconcile, clarify worker pauses * no-mistakes(document): Document deferred automatic nudges, rereads, and reply recovery * no-mistakes(document): Ring unlanded fire-and-forget steers exactly once more * no-mistakes(ci): The failing check, "PR must be raised via no-mistakes", reads the pipeline's attestation record, which says document=skipped. No file in the repository can change that record, so I did not touch the check or the PR body. As you said, the no-mistakes rerun after this run finishes will re-execute the document step and record document=completed. The one change is the documentation sentence you ordered. It adds a line to docs/remote-secondmates.md, right after the line saying the remote host runs no re-ring ladder of its own: "A fire-and-forget record, such as a reconcile ask, gets its single retry ring only on the local plane: the remote steer leg owes no re-ring, so a swallowed remote doorbell for one waits for the next ring into that inbox, and a remote-side retry is known follow-up scope." No behavior changed. Checks: tests/fm-documentation-audiences.test.sh passes (4/4) and bin/fm-lint.sh is clean. The change is left uncommitted in the working tree for the pipeline to pick up * no-mistakes(review): Hold automatic wakes until a mate's own decision closes * no-mistakes(document): Document watcher delivery of deferred remote re-read nudges * no-mistakes(review): Merge duplicate elapsed-wait reattach instructions in DOD * no-mistakes(test): Resolve merged default decision in remote-reply recovery fixture * no-mistakes(test): Source classify lib so config-push retry-deferred honors open decisions * no-mistakes(ci): Fixed a flaky test that also fails on main. Neither this PR's bin/fm-brief.sh nor its bin/fm-dod-lib.sh change is involved: bin/fm-dispatch-resolve.sh sources neither file. Another branch (fm-attended-cutover-smoothing-s1, run 36343879084) failed the same shard 8 check the same way, on a different case ("a rule-criterion match prints one diagnostic line, got 2"). Root cause: `fm_quota_single_provider_for_harness` in bin/fm-quota-axi-lib.sh returned from its `while read` loop as soon as it found a match. That closed the pipe while `fm_quota_single_provider_table`'s `printf` was sometimes still writing. GitHub Actions runners ignore SIGPIPE, so bash printed `fm-quota-axi-lib.sh: line 138: printf: write error: Broken pipe` to the resolver's stderr. That is the extra line. I reproduced it locally by running the test with SIGPIPE ignored: 2 of 20 runs failed, one with the resolver's diagnostic line plus two broken-pipe lines. Invariant: looking up a harness in the provider table must never make the table writer fail. The only reader of that table is this function, and all of the resolver's lookups (line 208 without stderr redirected, line 222 with it) go through it. So the fix is in that one place: read the whole table, then print the match. The same file now shows it reads the full table first, like `fm_control_harness_supported` does. Return values and output are unchanged. Verification: with SIGPIPE ignored, tests/fm-dispatch-resolve.test.sh failed 0 of 30 runs after the fix (2 of 20 before). tests/fm-dispatch-resolve.test.sh, tests/fm-brief.test.sh, tests/fm-send-inbox.test.sh, tests/fm-quota-choose.test.sh and tests/fm-quota-array-dispatch-live-e2e.test.sh all pass, and shellcheck is clean. tests/fm-procevent-quota.test.sh fails locally with or without the change ("process-event state root is not a private directory"), so that failure comes from the local environment, not from this fix. No new test was added: the existing one-diagnostic-line assertions already catch this whenever SIGPIPE is ignored, as it is in CI * Revert "no-mistakes(ci): Fixed a flaky test that also fails on main. Neither this PR's bin/fm-brief.sh nor its bin/fm-dod-lib.sh change is involved: bin/fm-dispatch-resolve.sh sources neither file. Another branch (fm-attended-cutover-smoothing-s1, run 36343879084) failed the same shard 8 check the same way, on a different case ("a rule-criterion match prints one diagnostic line, got 2"). Root cause: `fm_quota_single_provider_for_harness` in bin/fm-quota-axi-lib.sh returned from its `while read` loop as soon as it found a match. That closed the pipe while `fm_quota_single_provider_table`'s `printf` was sometimes still writing. GitHub Actions runners ignore SIGPIPE, so bash printed `fm-quota-axi-lib.sh: line 138: printf: write error: Broken pipe` to the resolver's stderr. That is the extra line. I reproduced it locally by running the test with SIGPIPE ignored: 2 of 20 runs failed, one with the resolver's diagnostic line plus two broken-pipe lines. Invariant: looking up a harness in the provider table must never make the table writer fail. The only reader of that table is this function, and all of the resolver's lookups (line 208 without stderr redirected, line 222 with it) go through it. So the fix is in that one place: read the whole table, then print the match. The same file now shows it reads the full table first, like `fm_control_harness_supported` does. Return values and output are unchanged. Verification: with SIGPIPE ignored, tests/fm-dispatch-resolve.test.sh failed 0 of 30 runs after the fix (2 of 20 before). tests/fm-dispatch-resolve.test.sh, tests/fm-brief.test.sh, tests/fm-send-inbox.test.sh, tests/fm-quota-choose.test.sh and tests/fm-quota-array-dispatch-live-e2e.test.sh all pass, and shellcheck is clean. tests/fm-procevent-quota.test.sh fails locally with or without the change ("process-event state root is not a private directory"), so that failure comes from the local environment, not from this fix. No new test was added: the existing one-diagnostic-line assertions already catch this whenever SIGPIPE is ignored, as it is in CI" This reverts commit c719928. * no-mistakes(review): Retry deferred local instruction nudges via the watcher * no-mistakes(review): Document watcher retry for deferred local instruction nudges * no-mistakes(ci): I fixed both review findings you selected (ci-1 and ci-3). I did not touch the deferral check in bin/fm-send.sh. ci-1 (bin/fm-config-push.sh, retry_deferred_rereads) - Rule that must hold: a deferred reread stays flagged until it is actually delivered. - Before the fix, the flag was removed before any of the steps that can skip a mate: the remote lock-path lookup, validate_secondmate_home, the local lock-path lookup, and the lock acquire. A skip at any of those dropped the flag, so the watcher lost track of the reread. - Now the flag is removed in one place only, when the send succeeds (rc 0). A skipped home, a busy lock, a deferred send (rc 4) or a failed send all leave it in place. The re-mark calls on a busy lock and on rc 4 were no longer needed, so I removed them. I updated the comment above the function to match. - Side effect: a send that keeps failing now stays flagged, so the watcher retries it on every poll and logs each failure. That follows your "don't clear until delivered" rule, but it replaces the old behaviour of leaving a failed send to the next config push or session start. - New test in tests/fm-secondmate-sync.test.sh: T8j "a deferred flag survives a skipped invalid home and is retried once it validates". It takes the home's marker away to make validation fail, checks that nothing is sent and the flag stays, then puts the marker back and checks that the nudge is delivered and both the flag and the retry marker are cleared. It fails on the old code and passes now. ci-3 (bin/fm-secondmate-restart.sh) - Rule that must hold: no automatic send wakes a mate that is waiting on its own open decision. - The two automatic sends in this script are the fallback reread nudge (fall_back_to_nudge) and the persist request. Both now pass --automatic. If a persist request is deferred, its correlation is discarded and the mate goes to the fallback nudge, which is also deferred, so the mate is reported as unreached. - New test in tests/fm-secondmate-restart.test.sh: T3b. It gives a mate an open needs-decision and runs a restart. It checks that both sends report as deferred, the mate's doorbell is never rung, its inbox gets no message, nothing is stopped, and the mate is reported as unreached with exit status 3. It fails on the old code and passes now. - The test marks the watcher as alive first. Without that, the watcher-down warning is printed first and becomes the reported reason instead of the deferral message. Verification - tests/fm-secondmate-sync.test.sh passes. - tests/fm-secondmate-restart.test.sh passes. - tests/fm-secondmate-harness.test.sh (the other test that exercises --retry-deferred) passes. - The fm-send-inbox test that covers automatic deferral passes. I only looked at the last lines of that run, not the whole file. - `shellcheck -x` on the four changed files is clean * Pin autoarm supervision model in secondmate restart T3b The fresh watcher beat the test writes proves a live watcher only under the autoarm model; on CI hosts with no detected harness the persistent model demands a lock-holding watcher, so the watcher-down banner became the reported reason and the deferral assertion failed. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Keep deferred secondmate nudges retryable under the inheritance lock. A bootstrap instruction nudge could write its deferral flag outside the lock the watcher retry holds, so a concurrent retry could delete a flag that had just been set. A restart fallback that is deferred now records the same marker and flag, so the watcher delivers it once the decision closes. * no-mistakes(document): Document watcher retry of deferred restart re-read nudges * Send secondmate reread and restart nudges immediately again. Deferring those nudges let a later config push drop an incomplete transfer once the decision closed. They now send as they do on main. * Make the no-turn wait opt-in behind config/wait-no-turns. Homes that do not create the file keep the previous briefs, drive text, and sends. * no-mistakes(document): Document wait-no-turns inbox wording change in configuration * no-mistakes(review): Keep checkpoint inbox check; forbid only polling while waiting * no-mistakes(ci): Fixed ci-2 (Greptile: a concurrent retry marker gets lost). The rule that was broken: the watcher may remove only the `.retry-ring` mark for the record it just processed. A newer mark written in the meantime is owed its own retry. `fm_task_inbox_clear_retry` is the one shared function that removes the mark, and I fixed it there. In `bin/fm-task-inbox-lib.sh` it now takes the record path. It compares the mark's content with that record's name and removes the mark only when they match. When the mark names a different record it returns success and leaves the mark alone. It still fails only when the processed record's own mark can't be removed. Both callers in `bin/fm-watch.sh` now pass `"$rec"`: the dead or missing pane path and the path after a retry ring. So the fix holds at both removal sites. Tests, in `tests/fm-task-inbox.test.sh`: - I added an optional `FM_RING_MARKS_RETRY` hook to the fake tmux. It writes a newer record's mark while the doorbell is being typed, which reproduces the race deterministically. - I added `test_watcher_retry_keeps_a_newer_mark`. The owed retry rings once, the newer mark survives, and a later check rings the newer record once and then clears its mark. The test fails without the fix ("the spent retry removed a newer record's mark written during its ring") and passes with it. - I updated the direct `clear_retry` call in the existing unit test to pass the record. Results: `tests/fm-task-inbox.test.sh` passes in full and `tests/fm-send-inbox.test.sh` passes 15/15. Shellcheck reports only SC1091 "not following sourced file" notices. As instructed, I didn't change the brief inbox wording * no-mistakes(document): Fix stale wait-no-turns inbox wording in inbox lib comment --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> Co-authored-by: Cursor Agent <cursoragent@cursor.com> Co-authored-by: Kun Chen <kunchenguid@users.noreply.github.com> --------- Co-authored-by: Christopher McKay <101884182+karotkriss@users.noreply.github.com> Co-authored-by: Tiago <tiagop@hey.com> Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> Co-authored-by: Cursor Agent <cursoragent@cursor.com> Co-authored-by: Kun Chen <kunchenguid@users.noreply.github.com> * no-mistakes(test): Run Herdr smoke agent under nested shell to keep registration * no-mistakes(document): Note nested-shell requirement for Herdr stale-registration guard --------- Co-authored-by: Christopher McKay <101884182+karotkriss@users.noreply.github.com> Co-authored-by: Tiago <tiagop@hey.com> Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> Co-authored-by: Cursor Agent <cursoragent@cursor.com> Co-authored-by: Kun Chen <kunchenguid@users.noreply.github.com>
cloud-practitioner
added a commit
to cloud-practitioner/firstmate
that referenced
this pull request
Oct 1, 2026
* fix(bin): run no repository hook when core.hooksPath is empty (kunchenguid#6216) * fix(bin): run no repository hook when core.hooksPath is empty The per-task hook wrapper refused every commit in a repository whose own config sets core.hooksPath to the empty string, because git rev-parse --git-path hooks fails on it. Plain git reads that setting as no hooks, so the wrapper now runs none; every other lookup failure still refuses and shows git's error. Fixes kunchenguid#6171 * no-mistakes(review): Refuse commits when core.hooksPath is a valueless key * no-mistakes(document): Document empty core.hooksPath handling in commit attribution docs * no-mistakes(ci): When the wrapper refuses a commit, Git's hook-lookup error now shows up once instead of twice. That required changing one line in the wrapper, and the tests were extended so both bad-config cases would catch the duplicate. Invariant: when the wrapper refuses, Git's lookup error must appear exactly once. In the failure path, the only Git call besides the deliberate second lookup is the `git config --get --type=path core.hooksPath` check in `runtime_chain_body` (`bin/fm-git-strip-ai-trailers.sh:168`). That check prints the same error, so it was the one place to fix. I added `2>/dev/null` to it. Its exit status still decides the outcome: an empty value still runs no hook, and anything else goes on to the second lookup, which prints Git's error once, and the commit is refused. Tests (`tests/fm-git-strip-ai-trailers.test.sh`): - The unresolvable-path test (`~fm-no-such-user-6171/hooks`) now requires `failed to expand user dir` to appear exactly once in the refused commit's output. - The valueless-key test now requires `missing value for 'core.hookspath'` to appear exactly once. - Pre-existing bug in the unresolvable-path test: its `git add` ran after the bad config was set, so it failed silently (exit 128) and the "refused commit" had nothing staged. The test now stages the file before writing the config, the same way the valueless test does, so a real commit gets refused. - The empty-string test is unchanged and still passes, so an empty `core.hooksPath` still runs no hook. Verification: - With the wrapper change reverted, both new checks fail with `expected '1', got '2'`. With the change in place, the whole suite passes. - `bash -n` passes. shellcheck shows only an info-level SC1091 note about sourcing `lib.sh`, which was already there before this change. - `git status` lists only the two intended files * fix(bin): let a stale record on a reassigned slot retire records-only (kunchenguid#6213) * fix(bin): let a stale record on a reassigned slot retire records-only When a pool slot's owner claim names another task, the stale record's teardown touches nothing under the slot, so the exclusive-slot record scan no longer refuses it. Full teardowns of a slot this task still claims, or one with no claim, keep the refusal. Fixes kunchenguid#6184 * no-mistakes(document): Note claim-over-record precedence for reassigned teardown slots * fix(bin): keep the steering doorbell short under deep homes (kunchenguid#6240) * fix(bin): keep the steering doorbell short under deep homes The doorbell printed the task inbox's absolute path twice, so under a deep home it grew to about 290 characters and a Herdr submit reported it never reached the pane on every re-ring. It now names the inbox once by its short <task>.inbox name and points at the full path the worker's brief already gives, so its length no longer depends on the home's depth. Fixes kunchenguid#6120 * no-mistakes(review): Export FM_TASK_INBOX at launch and name it in doorbell * no-mistakes(ci): ci-1 (Behavior portable serial 9) was caused by this PR, and I fixed it in the test. tests/fm-claude-trust.test.sh failed with "the launch command did not carry a brief doorbell". Its claude_launch_doorbell helper stripped exactly two leading `export ...;` statements before reading the final prompt argument. This PR adds a third one (`export FM_TASK_INBOX=...`) to every launch, so the helper was reading the wrong command. The invariant: a test that parses the launch command must skip every leading export statement, however many there are. I checked every test that parses the launch this way. The only other ones are the two helpers in tests/fm-spawn-dispatch-profile.test.sh, and they already loop over all exports. The kimi and dispatch-profile exact-string checks were updated earlier in this PR. The fix makes claude_launch_doorbell use the same loop (`while [[ "$command" == export\ *\;* ]]; do command=${command#*; }; done`) and then take the last argument. The ordinary path still works: the claude spawn test and the secondmate-clone spawn test both resolve the brief record through the same helper. Verified locally: `bash tests/fm-claude-trust.test.sh` exits 0 with no failing cases. ci-2 (Behavior tests (Herdr)) was not caused by this change, and I made no code change for it. In tests/fm-backend-herdr-presentation-e2e.test.sh, the concurrent secondmate recovery failed with "herdr presentation recovery could not acquire its session lock; refusing a concurrent resume". Two reasons it is not this PR: - The same failure, in the same test and case, happened on run 36655209015 for the unrelated branch fm/fm-contributions-old-gh-compat about 14 hours earlier. - This PR's change cannot lengthen how long the lock is held. The launch is written to a file and sent to the pane as `. launch.N.sh`, so the extra export changes neither the pane submit nor the lock hold time. The cause is a race that was already there: spawn_herdr_presentation_order_lock_acquire gives up after 5 seconds, and a concurrent real-Herdr recovery can hold the lock longer. Fixing that means changing the product's lock timeout, which is outside this PR. It should be tracked separately, and a rerun of the Herdr job is expected to pass. The only file changed is tests/fm-claude-trust.test.sh * feat(bin): add opt-in config/wait-no-turns so a waiting worker spends no turns (kunchenguid#4859) * fix(dod): drive no-mistakes with one foreground call, not a background poll The brief told workers to background the drive call and poll `axi status` because one call "routinely outlives what your harness lets a single command run". That advice contradicts the tool it drives: `no-mistakes axi run --help` documents `--wait` with an 8m default, existing precisely "so an agent harness with a 10-minute tool cap gets a structured return instead of an unbounded hang". Following the old text, a worker could never idle - a backgrounded call returns in milliseconds, so it does not wait at all - and each attempt leaked a live timer that later fired as a paid wake. Tell workers to make one foreground call, let it block, and repeat it when it returns on elapsed wait rather than on a gate or outcome. Also drops the generalisation that told workers on any unestablished harness to assume a command cap and use the same shape, which exported the defect to harnesses with no such cap. * fix(bin): let a waiting worker spend no turns until it is answered A worker waiting on a decision, a pipeline gate, CI, or a heavy-test slot kept taking model turns: the brief told it to list its inbox at any natural checkpoint, and six automatic senders nudged secondmates whatever their open decisions. - The ship and scout briefs gain one Waiting section: end the turn after needs-decision or blocked, and hold an external wait inside ONE blocking command bounded by the harness's own command ceiling. The checkpoint clause is deleted. Forbidding the wrong shapes is not enough on its own, so the section also names the blocking foreground `until` loop as the wait a Claude Code worker may use, because that harness can refuse a sleep-then-check command while pointing at backgrounding, which is the one shape a waiting worker must not take. - fm-send --automatic defers (exit 4, nothing written or rung) while the target has an open decision or blocker of its own; every automatic sender passes it and keeps its retry state, and the pending-reply recovery waits the same way. - The two senders that report the result classified it by matching the text of the send's captured output against `deferred:*`. fm-send runs bin/fm-guard.sh as a supervision warning, and that guard prints its worktree-tangle banner whenever the primary checkout is on a feature branch, which is exactly what a CI pull-request checkout is. The banner lands ahead of the `deferred:` line, so the match fell through and a waiting mate was reported as a failed send, with the banner as the reason. Both senders now classify on fm-send's exit status, which is the contract the deferral is actually stated in, and select the `deferred:` line out of the output rather than assuming it came first. The third root cause, a no-mistakes definition of done that backgrounded the drive call and polled axi status, is fixed by this branch's parent commit "drive no-mistakes with one foreground call, not a background poll"; this commit takes that text as is and adds the regression test. Upstream's spawn abort path no longer calls the lease-return helper at all, so the fork's missing-helper guard and its pin-feature test line are moot here and are not ported. The command ceilings each harness enforces, and the probes behind the named Claude Code wait, are recorded in docs/verification/runtime-backends.md. * no-mistakes(review): Exempt captain holds, quiet deferred reconcile, clarify worker pauses * no-mistakes(document): Document deferred automatic nudges, rereads, and reply recovery * no-mistakes(document): Ring unlanded fire-and-forget steers exactly once more * no-mistakes(ci): The failing check, "PR must be raised via no-mistakes", reads the pipeline's attestation record, which says document=skipped. No file in the repository can change that record, so I did not touch the check or the PR body. As you said, the no-mistakes rerun after this run finishes will re-execute the document step and record document=completed. The one change is the documentation sentence you ordered. It adds a line to docs/remote-secondmates.md, right after the line saying the remote host runs no re-ring ladder of its own: "A fire-and-forget record, such as a reconcile ask, gets its single retry ring only on the local plane: the remote steer leg owes no re-ring, so a swallowed remote doorbell for one waits for the next ring into that inbox, and a remote-side retry is known follow-up scope." No behavior changed. Checks: tests/fm-documentation-audiences.test.sh passes (4/4) and bin/fm-lint.sh is clean. The change is left uncommitted in the working tree for the pipeline to pick up * no-mistakes(review): Hold automatic wakes until a mate's own decision closes * no-mistakes(document): Document watcher delivery of deferred remote re-read nudges * no-mistakes(review): Merge duplicate elapsed-wait reattach instructions in DOD * no-mistakes(test): Resolve merged default decision in remote-reply recovery fixture * no-mistakes(test): Source classify lib so config-push retry-deferred honors open decisions * no-mistakes(ci): Fixed a flaky test that also fails on main. Neither this PR's bin/fm-brief.sh nor its bin/fm-dod-lib.sh change is involved: bin/fm-dispatch-resolve.sh sources neither file. Another branch (fm-attended-cutover-smoothing-s1, run 36343879084) failed the same shard 8 check the same way, on a different case ("a rule-criterion match prints one diagnostic line, got 2"). Root cause: `fm_quota_single_provider_for_harness` in bin/fm-quota-axi-lib.sh returned from its `while read` loop as soon as it found a match. That closed the pipe while `fm_quota_single_provider_table`'s `printf` was sometimes still writing. GitHub Actions runners ignore SIGPIPE, so bash printed `fm-quota-axi-lib.sh: line 138: printf: write error: Broken pipe` to the resolver's stderr. That is the extra line. I reproduced it locally by running the test with SIGPIPE ignored: 2 of 20 runs failed, one with the resolver's diagnostic line plus two broken-pipe lines. Invariant: looking up a harness in the provider table must never make the table writer fail. The only reader of that table is this function, and all of the resolver's lookups (line 208 without stderr redirected, line 222 with it) go through it. So the fix is in that one place: read the whole table, then print the match. The same file now shows it reads the full table first, like `fm_control_harness_supported` does. Return values and output are unchanged. Verification: with SIGPIPE ignored, tests/fm-dispatch-resolve.test.sh failed 0 of 30 runs after the fix (2 of 20 before). tests/fm-dispatch-resolve.test.sh, tests/fm-brief.test.sh, tests/fm-send-inbox.test.sh, tests/fm-quota-choose.test.sh and tests/fm-quota-array-dispatch-live-e2e.test.sh all pass, and shellcheck is clean. tests/fm-procevent-quota.test.sh fails locally with or without the change ("process-event state root is not a private directory"), so that failure comes from the local environment, not from this fix. No new test was added: the existing one-diagnostic-line assertions already catch this whenever SIGPIPE is ignored, as it is in CI * Revert "no-mistakes(ci): Fixed a flaky test that also fails on main. Neither this PR's bin/fm-brief.sh nor its bin/fm-dod-lib.sh change is involved: bin/fm-dispatch-resolve.sh sources neither file. Another branch (fm-attended-cutover-smoothing-s1, run 36343879084) failed the same shard 8 check the same way, on a different case ("a rule-criterion match prints one diagnostic line, got 2"). Root cause: `fm_quota_single_provider_for_harness` in bin/fm-quota-axi-lib.sh returned from its `while read` loop as soon as it found a match. That closed the pipe while `fm_quota_single_provider_table`'s `printf` was sometimes still writing. GitHub Actions runners ignore SIGPIPE, so bash printed `fm-quota-axi-lib.sh: line 138: printf: write error: Broken pipe` to the resolver's stderr. That is the extra line. I reproduced it locally by running the test with SIGPIPE ignored: 2 of 20 runs failed, one with the resolver's diagnostic line plus two broken-pipe lines. Invariant: looking up a harness in the provider table must never make the table writer fail. The only reader of that table is this function, and all of the resolver's lookups (line 208 without stderr redirected, line 222 with it) go through it. So the fix is in that one place: read the whole table, then print the match. The same file now shows it reads the full table first, like `fm_control_harness_supported` does. Return values and output are unchanged. Verification: with SIGPIPE ignored, tests/fm-dispatch-resolve.test.sh failed 0 of 30 runs after the fix (2 of 20 before). tests/fm-dispatch-resolve.test.sh, tests/fm-brief.test.sh, tests/fm-send-inbox.test.sh, tests/fm-quota-choose.test.sh and tests/fm-quota-array-dispatch-live-e2e.test.sh all pass, and shellcheck is clean. tests/fm-procevent-quota.test.sh fails locally with or without the change ("process-event state root is not a private directory"), so that failure comes from the local environment, not from this fix. No new test was added: the existing one-diagnostic-line assertions already catch this whenever SIGPIPE is ignored, as it is in CI" This reverts commit c719928. * no-mistakes(review): Retry deferred local instruction nudges via the watcher * no-mistakes(review): Document watcher retry for deferred local instruction nudges * no-mistakes(ci): I fixed both review findings you selected (ci-1 and ci-3). I did not touch the deferral check in bin/fm-send.sh. ci-1 (bin/fm-config-push.sh, retry_deferred_rereads) - Rule that must hold: a deferred reread stays flagged until it is actually delivered. - Before the fix, the flag was removed before any of the steps that can skip a mate: the remote lock-path lookup, validate_secondmate_home, the local lock-path lookup, and the lock acquire. A skip at any of those dropped the flag, so the watcher lost track of the reread. - Now the flag is removed in one place only, when the send succeeds (rc 0). A skipped home, a busy lock, a deferred send (rc 4) or a failed send all leave it in place. The re-mark calls on a busy lock and on rc 4 were no longer needed, so I removed them. I updated the comment above the function to match. - Side effect: a send that keeps failing now stays flagged, so the watcher retries it on every poll and logs each failure. That follows your "don't clear until delivered" rule, but it replaces the old behaviour of leaving a failed send to the next config push or session start. - New test in tests/fm-secondmate-sync.test.sh: T8j "a deferred flag survives a skipped invalid home and is retried once it validates". It takes the home's marker away to make validation fail, checks that nothing is sent and the flag stays, then puts the marker back and checks that the nudge is delivered and both the flag and the retry marker are cleared. It fails on the old code and passes now. ci-3 (bin/fm-secondmate-restart.sh) - Rule that must hold: no automatic send wakes a mate that is waiting on its own open decision. - The two automatic sends in this script are the fallback reread nudge (fall_back_to_nudge) and the persist request. Both now pass --automatic. If a persist request is deferred, its correlation is discarded and the mate goes to the fallback nudge, which is also deferred, so the mate is reported as unreached. - New test in tests/fm-secondmate-restart.test.sh: T3b. It gives a mate an open needs-decision and runs a restart. It checks that both sends report as deferred, the mate's doorbell is never rung, its inbox gets no message, nothing is stopped, and the mate is reported as unreached with exit status 3. It fails on the old code and passes now. - The test marks the watcher as alive first. Without that, the watcher-down warning is printed first and becomes the reported reason instead of the deferral message. Verification - tests/fm-secondmate-sync.test.sh passes. - tests/fm-secondmate-restart.test.sh passes. - tests/fm-secondmate-harness.test.sh (the other test that exercises --retry-deferred) passes. - The fm-send-inbox test that covers automatic deferral passes. I only looked at the last lines of that run, not the whole file. - `shellcheck -x` on the four changed files is clean * Pin autoarm supervision model in secondmate restart T3b The fresh watcher beat the test writes proves a live watcher only under the autoarm model; on CI hosts with no detected harness the persistent model demands a lock-holding watcher, so the watcher-down banner became the reported reason and the deferral assertion failed. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Keep deferred secondmate nudges retryable under the inheritance lock. A bootstrap instruction nudge could write its deferral flag outside the lock the watcher retry holds, so a concurrent retry could delete a flag that had just been set. A restart fallback that is deferred now records the same marker and flag, so the watcher delivers it once the decision closes. * no-mistakes(document): Document watcher retry of deferred restart re-read nudges * Send secondmate reread and restart nudges immediately again. Deferring those nudges let a later config push drop an incomplete transfer once the decision closed. They now send as they do on main. * Make the no-turn wait opt-in behind config/wait-no-turns. Homes that do not create the file keep the previous briefs, drive text, and sends. * no-mistakes(document): Document wait-no-turns inbox wording change in configuration * no-mistakes(review): Keep checkpoint inbox check; forbid only polling while waiting * no-mistakes(ci): Fixed ci-2 (Greptile: a concurrent retry marker gets lost). The rule that was broken: the watcher may remove only the `.retry-ring` mark for the record it just processed. A newer mark written in the meantime is owed its own retry. `fm_task_inbox_clear_retry` is the one shared function that removes the mark, and I fixed it there. In `bin/fm-task-inbox-lib.sh` it now takes the record path. It compares the mark's content with that record's name and removes the mark only when they match. When the mark names a different record it returns success and leaves the mark alone. It still fails only when the processed record's own mark can't be removed. Both callers in `bin/fm-watch.sh` now pass `"$rec"`: the dead or missing pane path and the path after a retry ring. So the fix holds at both removal sites. Tests, in `tests/fm-task-inbox.test.sh`: - I added an optional `FM_RING_MARKS_RETRY` hook to the fake tmux. It writes a newer record's mark while the doorbell is being typed, which reproduces the race deterministically. - I added `test_watcher_retry_keeps_a_newer_mark`. The owed retry rings once, the newer mark survives, and a later check rings the newer record once and then clears its mark. The test fails without the fix ("the spent retry removed a newer record's mark written during its ring") and passes with it. - I updated the direct `clear_retry` call in the existing unit test to pass the record. Results: `tests/fm-task-inbox.test.sh` passes in full and `tests/fm-send-inbox.test.sh` passes 15/15. Shellcheck reports only SC1091 "not following sourced file" notices. As instructed, I didn't change the brief inbox wording * no-mistakes(document): Fix stale wait-no-turns inbox wording in inbox lib comment --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> Co-authored-by: Cursor Agent <cursoragent@cursor.com> Co-authored-by: Kun Chen <kunchenguid@users.noreply.github.com> --------- Co-authored-by: Christopher McKay <101884182+karotkriss@users.noreply.github.com> Co-authored-by: Tiago <tiagop@hey.com> Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> Co-authored-by: Cursor Agent <cursoragent@cursor.com> Co-authored-by: Kun Chen <kunchenguid@users.noreply.github.com>
cloud-practitioner
added a commit
to cloud-practitioner/firstmate
that referenced
this pull request
Oct 1, 2026
…teardown (#16) * fix(tests): remove read-only strip hooks and staged launch dirs in test cleanup The Herdr presentation e2e suite removed its temp root with a plain rm -rf, which fails on the read-only state/<id>.git-hooks directories that the real spawn installs, so every run leaked its /tmp tree. Spawns the suite never tears down also left their /tmp/fm-<id>+<home-hash> launch directories behind. Move fm_test_remove_tree into tests/fixture-tree-helpers.sh, sourced by both tests/lib.sh and tests/herdr-test-safety.sh, and add fm_test_remove_spawn_launch_dirs, which removes only the launch directories scoped to the fixture's own homes. Use them in the Herdr suites that spawn for real, assert in the presentation suite that nothing is left behind, and cover both helpers in tests/fm-test-fixtures.test.sh. * no-mistakes(review): Fix macOS /tmp find, concurrent flake, root guards * no-mistakes(review): Reword launch-dir helper comment about per-task tmp root * no-mistakes(review): Remove staged launch dirs in shared lib.sh test cleanup * no-mistakes(review): Keep launch-dir cleanup helper safe under set -e * no-mistakes(document): Document fixture-tree-helpers.sh in CONTRIBUTING shared helpers * merge upstream (#19) * fix(bin): run no repository hook when core.hooksPath is empty (kunchenguid#6216) * fix(bin): run no repository hook when core.hooksPath is empty The per-task hook wrapper refused every commit in a repository whose own config sets core.hooksPath to the empty string, because git rev-parse --git-path hooks fails on it. Plain git reads that setting as no hooks, so the wrapper now runs none; every other lookup failure still refuses and shows git's error. Fixes kunchenguid#6171 * no-mistakes(review): Refuse commits when core.hooksPath is a valueless key * no-mistakes(document): Document empty core.hooksPath handling in commit attribution docs * no-mistakes(ci): When the wrapper refuses a commit, Git's hook-lookup error now shows up once instead of twice. That required changing one line in the wrapper, and the tests were extended so both bad-config cases would catch the duplicate. Invariant: when the wrapper refuses, Git's lookup error must appear exactly once. In the failure path, the only Git call besides the deliberate second lookup is the `git config --get --type=path core.hooksPath` check in `runtime_chain_body` (`bin/fm-git-strip-ai-trailers.sh:168`). That check prints the same error, so it was the one place to fix. I added `2>/dev/null` to it. Its exit status still decides the outcome: an empty value still runs no hook, and anything else goes on to the second lookup, which prints Git's error once, and the commit is refused. Tests (`tests/fm-git-strip-ai-trailers.test.sh`): - The unresolvable-path test (`~fm-no-such-user-6171/hooks`) now requires `failed to expand user dir` to appear exactly once in the refused commit's output. - The valueless-key test now requires `missing value for 'core.hookspath'` to appear exactly once. - Pre-existing bug in the unresolvable-path test: its `git add` ran after the bad config was set, so it failed silently (exit 128) and the "refused commit" had nothing staged. The test now stages the file before writing the config, the same way the valueless test does, so a real commit gets refused. - The empty-string test is unchanged and still passes, so an empty `core.hooksPath` still runs no hook. Verification: - With the wrapper change reverted, both new checks fail with `expected '1', got '2'`. With the change in place, the whole suite passes. - `bash -n` passes. shellcheck shows only an info-level SC1091 note about sourcing `lib.sh`, which was already there before this change. - `git status` lists only the two intended files * fix(bin): let a stale record on a reassigned slot retire records-only (kunchenguid#6213) * fix(bin): let a stale record on a reassigned slot retire records-only When a pool slot's owner claim names another task, the stale record's teardown touches nothing under the slot, so the exclusive-slot record scan no longer refuses it. Full teardowns of a slot this task still claims, or one with no claim, keep the refusal. Fixes kunchenguid#6184 * no-mistakes(document): Note claim-over-record precedence for reassigned teardown slots * fix(bin): keep the steering doorbell short under deep homes (kunchenguid#6240) * fix(bin): keep the steering doorbell short under deep homes The doorbell printed the task inbox's absolute path twice, so under a deep home it grew to about 290 characters and a Herdr submit reported it never reached the pane on every re-ring. It now names the inbox once by its short <task>.inbox name and points at the full path the worker's brief already gives, so its length no longer depends on the home's depth. Fixes kunchenguid#6120 * no-mistakes(review): Export FM_TASK_INBOX at launch and name it in doorbell * no-mistakes(ci): ci-1 (Behavior portable serial 9) was caused by this PR, and I fixed it in the test. tests/fm-claude-trust.test.sh failed with "the launch command did not carry a brief doorbell". Its claude_launch_doorbell helper stripped exactly two leading `export ...;` statements before reading the final prompt argument. This PR adds a third one (`export FM_TASK_INBOX=...`) to every launch, so the helper was reading the wrong command. The invariant: a test that parses the launch command must skip every leading export statement, however many there are. I checked every test that parses the launch this way. The only other ones are the two helpers in tests/fm-spawn-dispatch-profile.test.sh, and they already loop over all exports. The kimi and dispatch-profile exact-string checks were updated earlier in this PR. The fix makes claude_launch_doorbell use the same loop (`while [[ "$command" == export\ *\;* ]]; do command=${command#*; }; done`) and then take the last argument. The ordinary path still works: the claude spawn test and the secondmate-clone spawn test both resolve the brief record through the same helper. Verified locally: `bash tests/fm-claude-trust.test.sh` exits 0 with no failing cases. ci-2 (Behavior tests (Herdr)) was not caused by this change, and I made no code change for it. In tests/fm-backend-herdr-presentation-e2e.test.sh, the concurrent secondmate recovery failed with "herdr presentation recovery could not acquire its session lock; refusing a concurrent resume". Two reasons it is not this PR: - The same failure, in the same test and case, happened on run 36655209015 for the unrelated branch fm/fm-contributions-old-gh-compat about 14 hours earlier. - This PR's change cannot lengthen how long the lock is held. The launch is written to a file and sent to the pane as `. launch.N.sh`, so the extra export changes neither the pane submit nor the lock hold time. The cause is a race that was already there: spawn_herdr_presentation_order_lock_acquire gives up after 5 seconds, and a concurrent real-Herdr recovery can hold the lock longer. Fixing that means changing the product's lock timeout, which is outside this PR. It should be tracked separately, and a rerun of the Herdr job is expected to pass. The only file changed is tests/fm-claude-trust.test.sh * feat(bin): add opt-in config/wait-no-turns so a waiting worker spends no turns (kunchenguid#4859) * fix(dod): drive no-mistakes with one foreground call, not a background poll The brief told workers to background the drive call and poll `axi status` because one call "routinely outlives what your harness lets a single command run". That advice contradicts the tool it drives: `no-mistakes axi run --help` documents `--wait` with an 8m default, existing precisely "so an agent harness with a 10-minute tool cap gets a structured return instead of an unbounded hang". Following the old text, a worker could never idle - a backgrounded call returns in milliseconds, so it does not wait at all - and each attempt leaked a live timer that later fired as a paid wake. Tell workers to make one foreground call, let it block, and repeat it when it returns on elapsed wait rather than on a gate or outcome. Also drops the generalisation that told workers on any unestablished harness to assume a command cap and use the same shape, which exported the defect to harnesses with no such cap. * fix(bin): let a waiting worker spend no turns until it is answered A worker waiting on a decision, a pipeline gate, CI, or a heavy-test slot kept taking model turns: the brief told it to list its inbox at any natural checkpoint, and six automatic senders nudged secondmates whatever their open decisions. - The ship and scout briefs gain one Waiting section: end the turn after needs-decision or blocked, and hold an external wait inside ONE blocking command bounded by the harness's own command ceiling. The checkpoint clause is deleted. Forbidding the wrong shapes is not enough on its own, so the section also names the blocking foreground `until` loop as the wait a Claude Code worker may use, because that harness can refuse a sleep-then-check command while pointing at backgrounding, which is the one shape a waiting worker must not take. - fm-send --automatic defers (exit 4, nothing written or rung) while the target has an open decision or blocker of its own; every automatic sender passes it and keeps its retry state, and the pending-reply recovery waits the same way. - The two senders that report the result classified it by matching the text of the send's captured output against `deferred:*`. fm-send runs bin/fm-guard.sh as a supervision warning, and that guard prints its worktree-tangle banner whenever the primary checkout is on a feature branch, which is exactly what a CI pull-request checkout is. The banner lands ahead of the `deferred:` line, so the match fell through and a waiting mate was reported as a failed send, with the banner as the reason. Both senders now classify on fm-send's exit status, which is the contract the deferral is actually stated in, and select the `deferred:` line out of the output rather than assuming it came first. The third root cause, a no-mistakes definition of done that backgrounded the drive call and polled axi status, is fixed by this branch's parent commit "drive no-mistakes with one foreground call, not a background poll"; this commit takes that text as is and adds the regression test. Upstream's spawn abort path no longer calls the lease-return helper at all, so the fork's missing-helper guard and its pin-feature test line are moot here and are not ported. The command ceilings each harness enforces, and the probes behind the named Claude Code wait, are recorded in docs/verification/runtime-backends.md. * no-mistakes(review): Exempt captain holds, quiet deferred reconcile, clarify worker pauses * no-mistakes(document): Document deferred automatic nudges, rereads, and reply recovery * no-mistakes(document): Ring unlanded fire-and-forget steers exactly once more * no-mistakes(ci): The failing check, "PR must be raised via no-mistakes", reads the pipeline's attestation record, which says document=skipped. No file in the repository can change that record, so I did not touch the check or the PR body. As you said, the no-mistakes rerun after this run finishes will re-execute the document step and record document=completed. The one change is the documentation sentence you ordered. It adds a line to docs/remote-secondmates.md, right after the line saying the remote host runs no re-ring ladder of its own: "A fire-and-forget record, such as a reconcile ask, gets its single retry ring only on the local plane: the remote steer leg owes no re-ring, so a swallowed remote doorbell for one waits for the next ring into that inbox, and a remote-side retry is known follow-up scope." No behavior changed. Checks: tests/fm-documentation-audiences.test.sh passes (4/4) and bin/fm-lint.sh is clean. The change is left uncommitted in the working tree for the pipeline to pick up * no-mistakes(review): Hold automatic wakes until a mate's own decision closes * no-mistakes(document): Document watcher delivery of deferred remote re-read nudges * no-mistakes(review): Merge duplicate elapsed-wait reattach instructions in DOD * no-mistakes(test): Resolve merged default decision in remote-reply recovery fixture * no-mistakes(test): Source classify lib so config-push retry-deferred honors open decisions * no-mistakes(ci): Fixed a flaky test that also fails on main. Neither this PR's bin/fm-brief.sh nor its bin/fm-dod-lib.sh change is involved: bin/fm-dispatch-resolve.sh sources neither file. Another branch (fm-attended-cutover-smoothing-s1, run 36343879084) failed the same shard 8 check the same way, on a different case ("a rule-criterion match prints one diagnostic line, got 2"). Root cause: `fm_quota_single_provider_for_harness` in bin/fm-quota-axi-lib.sh returned from its `while read` loop as soon as it found a match. That closed the pipe while `fm_quota_single_provider_table`'s `printf` was sometimes still writing. GitHub Actions runners ignore SIGPIPE, so bash printed `fm-quota-axi-lib.sh: line 138: printf: write error: Broken pipe` to the resolver's stderr. That is the extra line. I reproduced it locally by running the test with SIGPIPE ignored: 2 of 20 runs failed, one with the resolver's diagnostic line plus two broken-pipe lines. Invariant: looking up a harness in the provider table must never make the table writer fail. The only reader of that table is this function, and all of the resolver's lookups (line 208 without stderr redirected, line 222 with it) go through it. So the fix is in that one place: read the whole table, then print the match. The same file now shows it reads the full table first, like `fm_control_harness_supported` does. Return values and output are unchanged. Verification: with SIGPIPE ignored, tests/fm-dispatch-resolve.test.sh failed 0 of 30 runs after the fix (2 of 20 before). tests/fm-dispatch-resolve.test.sh, tests/fm-brief.test.sh, tests/fm-send-inbox.test.sh, tests/fm-quota-choose.test.sh and tests/fm-quota-array-dispatch-live-e2e.test.sh all pass, and shellcheck is clean. tests/fm-procevent-quota.test.sh fails locally with or without the change ("process-event state root is not a private directory"), so that failure comes from the local environment, not from this fix. No new test was added: the existing one-diagnostic-line assertions already catch this whenever SIGPIPE is ignored, as it is in CI * Revert "no-mistakes(ci): Fixed a flaky test that also fails on main. Neither this PR's bin/fm-brief.sh nor its bin/fm-dod-lib.sh change is involved: bin/fm-dispatch-resolve.sh sources neither file. Another branch (fm-attended-cutover-smoothing-s1, run 36343879084) failed the same shard 8 check the same way, on a different case ("a rule-criterion match prints one diagnostic line, got 2"). Root cause: `fm_quota_single_provider_for_harness` in bin/fm-quota-axi-lib.sh returned from its `while read` loop as soon as it found a match. That closed the pipe while `fm_quota_single_provider_table`'s `printf` was sometimes still writing. GitHub Actions runners ignore SIGPIPE, so bash printed `fm-quota-axi-lib.sh: line 138: printf: write error: Broken pipe` to the resolver's stderr. That is the extra line. I reproduced it locally by running the test with SIGPIPE ignored: 2 of 20 runs failed, one with the resolver's diagnostic line plus two broken-pipe lines. Invariant: looking up a harness in the provider table must never make the table writer fail. The only reader of that table is this function, and all of the resolver's lookups (line 208 without stderr redirected, line 222 with it) go through it. So the fix is in that one place: read the whole table, then print the match. The same file now shows it reads the full table first, like `fm_control_harness_supported` does. Return values and output are unchanged. Verification: with SIGPIPE ignored, tests/fm-dispatch-resolve.test.sh failed 0 of 30 runs after the fix (2 of 20 before). tests/fm-dispatch-resolve.test.sh, tests/fm-brief.test.sh, tests/fm-send-inbox.test.sh, tests/fm-quota-choose.test.sh and tests/fm-quota-array-dispatch-live-e2e.test.sh all pass, and shellcheck is clean. tests/fm-procevent-quota.test.sh fails locally with or without the change ("process-event state root is not a private directory"), so that failure comes from the local environment, not from this fix. No new test was added: the existing one-diagnostic-line assertions already catch this whenever SIGPIPE is ignored, as it is in CI" This reverts commit c719928. * no-mistakes(review): Retry deferred local instruction nudges via the watcher * no-mistakes(review): Document watcher retry for deferred local instruction nudges * no-mistakes(ci): I fixed both review findings you selected (ci-1 and ci-3). I did not touch the deferral check in bin/fm-send.sh. ci-1 (bin/fm-config-push.sh, retry_deferred_rereads) - Rule that must hold: a deferred reread stays flagged until it is actually delivered. - Before the fix, the flag was removed before any of the steps that can skip a mate: the remote lock-path lookup, validate_secondmate_home, the local lock-path lookup, and the lock acquire. A skip at any of those dropped the flag, so the watcher lost track of the reread. - Now the flag is removed in one place only, when the send succeeds (rc 0). A skipped home, a busy lock, a deferred send (rc 4) or a failed send all leave it in place. The re-mark calls on a busy lock and on rc 4 were no longer needed, so I removed them. I updated the comment above the function to match. - Side effect: a send that keeps failing now stays flagged, so the watcher retries it on every poll and logs each failure. That follows your "don't clear until delivered" rule, but it replaces the old behaviour of leaving a failed send to the next config push or session start. - New test in tests/fm-secondmate-sync.test.sh: T8j "a deferred flag survives a skipped invalid home and is retried once it validates". It takes the home's marker away to make validation fail, checks that nothing is sent and the flag stays, then puts the marker back and checks that the nudge is delivered and both the flag and the retry marker are cleared. It fails on the old code and passes now. ci-3 (bin/fm-secondmate-restart.sh) - Rule that must hold: no automatic send wakes a mate that is waiting on its own open decision. - The two automatic sends in this script are the fallback reread nudge (fall_back_to_nudge) and the persist request. Both now pass --automatic. If a persist request is deferred, its correlation is discarded and the mate goes to the fallback nudge, which is also deferred, so the mate is reported as unreached. - New test in tests/fm-secondmate-restart.test.sh: T3b. It gives a mate an open needs-decision and runs a restart. It checks that both sends report as deferred, the mate's doorbell is never rung, its inbox gets no message, nothing is stopped, and the mate is reported as unreached with exit status 3. It fails on the old code and passes now. - The test marks the watcher as alive first. Without that, the watcher-down warning is printed first and becomes the reported reason instead of the deferral message. Verification - tests/fm-secondmate-sync.test.sh passes. - tests/fm-secondmate-restart.test.sh passes. - tests/fm-secondmate-harness.test.sh (the other test that exercises --retry-deferred) passes. - The fm-send-inbox test that covers automatic deferral passes. I only looked at the last lines of that run, not the whole file. - `shellcheck -x` on the four changed files is clean * Pin autoarm supervision model in secondmate restart T3b The fresh watcher beat the test writes proves a live watcher only under the autoarm model; on CI hosts with no detected harness the persistent model demands a lock-holding watcher, so the watcher-down banner became the reported reason and the deferral assertion failed. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Keep deferred secondmate nudges retryable under the inheritance lock. A bootstrap instruction nudge could write its deferral flag outside the lock the watcher retry holds, so a concurrent retry could delete a flag that had just been set. A restart fallback that is deferred now records the same marker and flag, so the watcher delivers it once the decision closes. * no-mistakes(document): Document watcher retry of deferred restart re-read nudges * Send secondmate reread and restart nudges immediately again. Deferring those nudges let a later config push drop an incomplete transfer once the decision closed. They now send as they do on main. * Make the no-turn wait opt-in behind config/wait-no-turns. Homes that do not create the file keep the previous briefs, drive text, and sends. * no-mistakes(document): Document wait-no-turns inbox wording change in configuration * no-mistakes(review): Keep checkpoint inbox check; forbid only polling while waiting * no-mistakes(ci): Fixed ci-2 (Greptile: a concurrent retry marker gets lost). The rule that was broken: the watcher may remove only the `.retry-ring` mark for the record it just processed. A newer mark written in the meantime is owed its own retry. `fm_task_inbox_clear_retry` is the one shared function that removes the mark, and I fixed it there. In `bin/fm-task-inbox-lib.sh` it now takes the record path. It compares the mark's content with that record's name and removes the mark only when they match. When the mark names a different record it returns success and leaves the mark alone. It still fails only when the processed record's own mark can't be removed. Both callers in `bin/fm-watch.sh` now pass `"$rec"`: the dead or missing pane path and the path after a retry ring. So the fix holds at both removal sites. Tests, in `tests/fm-task-inbox.test.sh`: - I added an optional `FM_RING_MARKS_RETRY` hook to the fake tmux. It writes a newer record's mark while the doorbell is being typed, which reproduces the race deterministically. - I added `test_watcher_retry_keeps_a_newer_mark`. The owed retry rings once, the newer mark survives, and a later check rings the newer record once and then clears its mark. The test fails without the fix ("the spent retry removed a newer record's mark written during its ring") and passes with it. - I updated the direct `clear_retry` call in the existing unit test to pass the record. Results: `tests/fm-task-inbox.test.sh` passes in full and `tests/fm-send-inbox.test.sh` passes 15/15. Shellcheck reports only SC1091 "not following sourced file" notices. As instructed, I didn't change the brief inbox wording * no-mistakes(document): Fix stale wait-no-turns inbox wording in inbox lib comment --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> Co-authored-by: Cursor Agent <cursoragent@cursor.com> Co-authored-by: Kun Chen <kunchenguid@users.noreply.github.com> --------- Co-authored-by: Christopher McKay <101884182+karotkriss@users.noreply.github.com> Co-authored-by: Tiago <tiagop@hey.com> Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> Co-authored-by: Cursor Agent <cursoragent@cursor.com> Co-authored-by: Kun Chen <kunchenguid@users.noreply.github.com> * no-mistakes(document): Fix shared test helper list wording in CONTRIBUTING --------- Co-authored-by: Christopher McKay <101884182+karotkriss@users.noreply.github.com> Co-authored-by: Tiago <tiagop@hey.com> Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> Co-authored-by: Cursor Agent <cursoragent@cursor.com> Co-authored-by: Kun Chen <kunchenguid@users.noreply.github.com>
Open
6 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
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
Fixes #4228.
Clear base conflicts on #4859 (branch fm/firstmate-issue4228-wait-turn-spend, head 232c42b): let a waiting worker spend no turns until it is answered, with its accepted defaults unchanged.
Do not open a second PR. Never merge upstream.
#4859: option B - drop the reread and restart deferral entirely and keep the rest; deliver through the pipeline and answer Greptile 4120379558 accordingly.
#4859: rework to opt-in behind a config flag, same pattern as 5354, and re-validate. Never merge.
What Changed
config/wait-no-turnspresence flag (local, gitignored, not inherited by secondmate homes), documented indocs/configuration.mdand the operational home layout skill. When the flag is absent, briefs, the drive text, pending-reply recovery, and steer delivery work as they do today.fm-brief.shadds a# Waitingsection to ship and scout briefs. The section tells the worker to end its turn afterneeds-decision:/blocked:, and to wait on external state with one bounded blocking command sized to its harness. The brief also adds a line saying not to poll the inbox while waiting. The no-mistakes drive text infm-dod-lib.shswitches from a backgrounded drive call to one foregroundno-mistakes axi run.status_own_open_decisionshelper infm-classify-lib.shsupplies that set and skips reserved, remote-reply-continuity, and captain-hold keys.fm-send.shfails to ring the doorbell for a fire-and-forget steer, it marks the record for one retry ring (.retry-ringinfm-task-inbox-lib.sh).fm-watch.shrings it once after the grace period, once the worker has no open decision of its own. The mark is then cleared and never escalates.Risk Assessment
✅ Low: Every behavior change checks for the config/wait-no-turns flag, and with the flag absent the output stays the same as main: the brief text, the backgrounded drive text, pending-reply recovery, and the fire-and-forget ring (no retry mark). The reread and restart deferral is gone, as option B requires. The earlier fixes the user chose are still in place: the checkpoint inbox wording and the compare-then-remove clear of the retry mark.
Testing
I built a disposable lab home and a private fm-lab tmux server running a real Claude Code pane, then drove the real product there. - Briefs: I generated real briefs with and without the flag. With the flag absent, ship and scout briefs are byte-identical to the base commit. With it present, ship and scout briefs gain the Waiting section and the foreground drive text, and all brief kinds, including the secondmate charter, add only the no-poll inbox line and keep the natural-checkpoint wording. - Fire-and-forget retry: a real fm-send to the pane while its composer held draft text wrote the retry mark only when the flag was present. Real watcherinbox_steer_checkpasses held the retry while the worker had its own open decision, rang exactly one doorbell into the Claude pane once the decision closed, and never rang again. With the flag absent there was no mark and no ring. - Pending-reply recovery: a real reply-bearing fm-send created the pending record. Recovery was held while the mate had an open decision and was delivered to the real inbox once it closed. Turn completion was recorded through the library's busy/idle observer rather than a live watcher observation. - Tests: the four targeted test files (fm-brief, fm-task-inbox, fm-send-inbox, fm-pending-reply) all passed. The lab, the tmux server and all temp directories were torn down. There is no UI surface, so the evidence is CLI transcripts and brief diffs.Evidence: Flag-absent ship/scout briefs byte-identical to base
Source: Flag-absent ship/scout briefs byte-identical to base
Evidence: Ship brief diff, flag off vs on
Source: Ship brief diff, flag off vs on
Evidence: Scout brief diff, flag off vs on
Source: Scout brief diff, flag off vs on
Evidence: Secondmate charter diff, flag off vs on (only the no-poll line added)
Source: Secondmate charter diff, flag off vs on (only the no-poll line added)
Evidence: Live fire-and-forget send, flag on: retry mark written
Source: Live fire-and-forget send, flag on: retry mark written
Evidence: Live watcher: retry held during open decision, one ring after resolve
Source: Live watcher: retry held during open decision, one ring after resolve
Evidence: Live fire-and-forget send, flag off: legacy behavior
Source: Live fire-and-forget send, flag off: legacy behavior
Evidence: Live pending-reply recovery held then sent
Source: Live pending-reply recovery held then sent
Evidence: Targeted test runs
Source: Targeted test runs
Pipeline
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.
bin/fm-lab-home.sh create $LAB+bin/fm-brief.sh t-off demo --mode no-mistakes/--scoutwith the flag absent, compared against the same briefs generated fromgit archive 23e55847(base)bin/fm-brief.shship / scout /--secondmate --no-projectswithconfig/wait-no-turnspresent vs absent (diffs captured)Realclaudelaunched intmux -L fm-labwindow lab:fm-w1 and registered as secondmate w1 in the lab homebin/fm-send.sh w1 --fire-and-forget 0123456789abcdef ...while the Claude composer held draft text, flag present (checked the retry message and.retry-ring). bin/fm-watch.sh && inbox_steer_check lab:fm-w1 w1with an openneeds-decision(no ring), then afterresolved(exactly one doorbell, mark spent), then again (no second ring)bin/fm-send.sh w1 --fire-and-forget 1123456789abcdef ...with the flag absent (no mark, legacy message, the watcher pass does not ring)bin/fm-send.sh w1 "status of phase 8"(real pending-reply record), thenfm_pending_reply_send_recoverywith the mate's decision open (held) and closed (sent to the real inbox)bash tests/fm-brief.test.shbash tests/fm-task-inbox.test.shbash tests/fm-send-inbox.test.shbash tests/fm-pending-reply.test.sh✅ **Document** - passed
✅ No issues found.
✅ **Lint** - passed
✅ No issues found.
✅ **Push** - passed
✅ No issues found.