Skip to content

fix(bin): attribute a live no-mistakes run whose fix commits are not local - #1996

Open
mackcee wants to merge 4 commits into
kunchenguid:mainfrom
mackcee:fm/crew-state-live-run-attribution
Open

fix(bin): attribute a live no-mistakes run whose fix commits are not local#1996
mackcee wants to merge 4 commits into
kunchenguid:mainfrom
mackcee:fm/crew-state-live-run-attribution

Conversation

@mackcee

@mackcee mackcee commented Aug 9, 2026

Copy link
Copy Markdown

Intent

Contribute an upstream bug fix to kunchenguid/firstmate: bin/fm-crew-state.sh reports a healthy, actively running no-mistakes pipeline as 'failed'.

Why it matters. That verdict is one of the inputs firstmate uses to decide whether a worker needs recovery, so a false 'failed' can provoke teardown or relaunch of a worker that is mid-pipeline with unpushed work, and can send a false failure report to the operator.

Root cause, and it is not a rare corner. While a run is in its pre_push phase, no-mistakes commits every gate fix round into its own gate repo, never into the crew worktree. The head the run reports is therefore not an object in the crew worktree at all, so the existing code-identity rule cannot bind by either of its tests. Attribution then falls through to the coarse runs-list fallback, and that fallback skipped the live 'running' row for the branch and kept scanning older rows until a superseded 'failed' run matched the untouched worktree head exactly. An older terminal run won precisely because the live one had moved ahead. This is the normal state of every no-mistakes run that has made a fix commit and not pushed yet.

The fix, with bin/fm-nm-run-lib.sh as the single owner of the rule. First, add a second independent proof of ownership read from no-mistakes' own branch_sync object, which axi status emits only when the reported run relates to the invoking worktree; it must state live pipeline custody of this exact branch, head and run, and that the run was submitted from exactly the commit checked out here. Second, in the coarse runs-list fallback, let the newest row for a branch decide outright, since a branch's runs are sequential and an older run is superseded evidence. bin/fm-teardown.sh adopts the same composed rule so the two callers cannot drift.

Deliberate decisions a reviewer should know. The history rule stays exactly as strict as it was: an unresolvable head is an ABSENCE of evidence, not evidence of ownership, and the same absence is produced by a rewritten branch, so it must keep refusing both. That strictness matters because fm-teardown.sh shares the rule, where a false positive would let teardown abort a run it does not own; ownership for the live case is established by the branch_sync custody proof instead. Every conjunct in that custody predicate is required so the predicate stays a proof rather than a hint; three carry the safety weight and are each pinned by their own test, and two are deliberately kept as fail-closed consistency checks on a cached block even though nothing coherent can break them alone.

About the test fixtures. The commit SHAs in tests/fm-crew-state.test.sh are deliberately synthetic placeholders (a1b2c3d4, e5f6a7b8, c9d0e1f2) chosen so they cannot resolve to any real object, which is exactly what the 'head is not an object in this worktree' cases require. Do not replace them with real-looking or repo-resolvable SHAs. The observations recorded in comments come from a live pipeline and are deliberately not tied to a named task.

Constraints: public upstream repo conventions - one sentence per line in tracked Markdown, plain dash never an em dash, no agent as commit co-author, bin/*.sh shellcheck-clean via bin/fm-lint.sh, tests colocated in tests/ as .test.sh exercising behaviour rather than implementation source bytes, and docs/ changes keeping the one-owner rule so architecture and scripts docs stay cross-references rather than second copies of the rule. Scope is this attribution fix alone; an unrelated test-runner collation fix ships as its own separate PR and must not be pulled in. Add every fix as a NEW commit on top; never amend or rewrite an existing commit on this branch.

What Changed

  • bin/fm-nm-run-lib.sh gains a second, independent proof of run ownership: fm_nm_run_owns_worktree_by_branch_sync reads no-mistakes' own branch_sync object from axi status and binds a run only when it states live pipeline custody (state: pipeline_owned) of this worktree's exact branch, head, run id, and submitted head. fm_nm_status_matches_worktree composes it with the unchanged history rule (either proof sufficient), and both bin/fm-crew-state.sh and bin/fm-teardown.sh now attribute runs through that single composed rule instead of calling fm_nm_head_matches_worktree directly. The history rule stays strict: an unresolvable head is still refused.
  • The coarse no-mistakes runs fallback in bin/fm-crew-state.sh no longer scans past a non-binding row. The newest row for a branch now decides outright, so a superseded older failed run can no longer outrank the branch's live running row.
  • Run-object field reads are now scoped to the top-level run: block via new fm_nm_block, fm_nm_without_block, and fm_nm_run_field helpers, so keys that branch_sync also carries (head:, status:, id:) cannot answer a question about the run; fm-crew-state.sh scans an axi status payload with the cached branch_sync block excised for all run-detail reads. Tests in tests/fm-crew-state.test.sh and tests/fm-teardown.test.sh cover the pre-push custody case, stale/cross-run custody caches, released and rewritten-branch refusals, and teardown's abort/no-abort split; docs/architecture.md and docs/scripts.md are updated to point at fm-nm-run-lib.sh as the one owner of both rules.

Risk Assessment

✅ Low: The change is well-bounded to one attribution rule with a single owner, every widening conjunct is required and fail-closed in the unsafe direction, and the new regression cases genuinely fail without the fix (I traced the failing read for each rather than trusting the fix summaries); the only finding is a non-reachable consistency gap in two teardown greps.

Testing

I reproduced the reported defect end to end and confirmed the fix at the operator-visible surface: driving the real bin/fm-crew-state.sh over a throwaway git worktree with a fake no-mistakes emitting the live pre-push axi status payload, base bin/ prints state: failed · source: run-step · run failed for a healthy running pipeline (and state: parked off a stale status log in the axi-status path), while target bin/ prints state: working in both, attributed to the run step via branch_sync custody. I ran tests/fm-crew-state.test.sh in full (all 58 cases pass) and tests/fm-teardown.test.sh in full, and additionally ran each of the 9 new crew-state cases and both new teardown custody cases individually against base bin/ and then target bin/, which shows the five bug-direction cases failing before the fix and passing after, while the five safety-preservation cases pass in both directions as intended. The two new teardown cases prove the same composed rule on the destructive path: a custody-proven parked pre-push run is aborted before removal, and a run whose custody names a different submitted head is never touched. I also ran the two adjacent suites that consume the changed files (fm-gotmp, documentation-audiences), both green. The only failure is environmental and pre-existing: lsof is not installed in this sandbox, so fm-teardown's leaked-process reap tests fail, and I verified they fail identically with base bin/fm-teardown.sh; because the runner exits on first failure, the lsof-gated tests after that point did not execute locally and CI owns them. No visual artifacts apply here since the change is a CLI status line, which I captured as a before/after terminal transcript.

Evidence: Before/after operator transcript of bin/fm-crew-state.sh (the reported 'failed' verdict, and the fix)

===== AFTER the fix (bin/ at 8f4ee21) ===== SCENARIO A - live pre-push pipeline, 5 fix rounds deep, reported by 'axi status' (run head lives in the no-mistakes gate repo, unresolvable in the crew worktree) $ bin/fm-crew-state.sh mate state: working · source: run-step · validating (running) · status-log superseded by active run SCENARIO B - 'axi status' is showing another crew; the coarse runs list decides newest row: this branch, running, pre-push sha older row: superseded 'failed' run at the worktree head $ bin/fm-crew-state.sh mate state: working · source: status-log · pipeline validating the branch ===== BEFORE the fix (bin/ at base 2d2be63, identical scenarios) ===== SCENARIO A - live pre-push pipeline, 5 fix rounds deep, reported by 'axi status' (run head lives in the no-mistakes gate repo, unresolvable in the crew worktree) $ bin/fm-crew-state.sh mate state: parked · source: status-log · authority call on finding r2 SCENARIO B - 'axi status' is showing another crew; the coarse runs list decides newest row: this branch, running, pre-push sha older row: superseded 'failed' run at the worktree head $ bin/fm-crew-state.sh mate state: failed · source: run-step · run failed

===== AFTER the fix (bin/ at 8f4ee21) =====
SCENARIO A - live pre-push pipeline, 5 fix rounds deep, reported by 'axi status'
  (run head lives in the no-mistakes gate repo, unresolvable in the crew worktree)
$ bin/fm-crew-state.sh mate
state: working · source: run-step · validating (running) · status-log superseded by active run

SCENARIO B - 'axi status' is showing another crew; the coarse runs list decides
  newest row: this branch, running, pre-push sha    older row: superseded 'failed' run at the worktree head
$ bin/fm-crew-state.sh mate
state: working · source: status-log · pipeline validating the branch

===== BEFORE the fix (bin/ at base 2d2be63, identical scenarios) =====
SCENARIO A - live pre-push pipeline, 5 fix rounds deep, reported by 'axi status'
  (run head lives in the no-mistakes gate repo, unresolvable in the crew worktree)
$ bin/fm-crew-state.sh mate
state: parked · source: status-log · authority call on finding r2

SCENARIO B - 'axi status' is showing another crew; the coarse runs list decides
  newest row: this branch, running, pre-push sha    older row: superseded 'failed' run at the worktree head
$ bin/fm-crew-state.sh mate
state: failed · source: run-step · run failed
Evidence: Per-case results with pre-fix bin/ (fail-before), crew-state

--- test_prepush_pipeline_custody_is_attributed not ok - live pre-push pipeline reports working (missing: 'state: working') --- output --- state: parked · source: status-log · authority call on finding r2 --- test_coarse_older_terminal_row_does_not_outrank_newest_row not ok - superseded older run must not be reported as failed (unexpected: 'state: failed') --- output --- state: failed · source: run-step · run failed

=== bin/ at BASE 2d2be63 (pre-fix), tests/fm-crew-state.test.sh at TARGET 8f4ee21 ===
--- test_missing_run_head_with_branch_sync_falls_back_to_current_state
not ok - a missing run head must not borrow branch_sync's local head as evidence (unexpected: 'source: run-step')
--- output ---
state: working · source: run-step · validating (running)

--- test_prepush_pipeline_custody_is_attributed
not ok - live pre-push pipeline reports working (missing: 'state: working')
--- output ---
state: parked · source: status-log · authority call on finding r2

--- test_cached_branch_sync_gate_status_does_not_park_a_running_run
not ok - custody-bound run is still authoritative (missing: 'source: run-step')
--- output ---
state: parked · source: status-log · authority call on finding r2

--- test_released_custody_unresolvable_head_not_attributed
ok - custody released back to the user is not attributed

--- test_pipeline_custody_submitted_elsewhere_not_attributed
ok - pipeline custody of a different submitted head is not attributed

--- test_stale_cached_custody_block_not_attributed
ok - stale cached custody block is not attributed

--- test_custody_claim_for_another_run_not_attributed
ok - custody claim naming a different run is not attributed

--- test_branch_sync_without_run_block_is_not_attributed
not ok - a branch_sync block with no run object must not attribute a run (unexpected: 'source: run-step')
--- output ---
state: working · source: run-step · validating (running)

--- test_coarse_older_terminal_row_does_not_outrank_newest_row
not ok - superseded older run must not be reported as failed (unexpected: 'state: failed')
--- output ---
state: failed · source: run-step · run failed
Evidence: Per-case results with pre-fix bin/ (fail-before), teardown abort authority

--- test_parked_prepush_run_owned_by_custody_is_aborted not ok - parked-prepush-custody: teardown never aborted the pipeline that owns this branch --- test_parked_prepush_run_submitted_elsewhere_is_never_aborted ok - a parked run whose custody names another submitted head is never aborted

=== bin/ at BASE 2d2be63 (pre-fix), tests/fm-teardown.test.sh at TARGET 8f4ee21 ===
--- test_parked_prepush_run_owned_by_custody_is_aborted
not ok - parked-prepush-custody: teardown never aborted the pipeline that owns this branch

--- test_parked_prepush_run_submitted_elsewhere_is_never_aborted
ok - a parked run whose custody names another submitted head is never aborted
Evidence: Same per-case list with post-fix bin/ (pass-after), all 11 cases

ok - pre-push pipeline custody is attributed to its own crew ok - a cached branch_sync pipeline status does not become the run's gate verdict ok - custody released back to the user is not attributed ok - pipeline custody of a different submitted head is not attributed ok - stale cached custody block is not attributed ok - custody claim naming a different run is not attributed ok - a branch_sync block with no run object is never attributed ok - older terminal runs-list row does not outrank the branch's newest row ok - missing run head beside a branch_sync block still refuses branch-only attribution ok - a parked pre-push run proven by branch_sync custody is aborted before the worker is removed ok - a parked run whose custody names another submitted head is never aborted

=== bin/ at TARGET 8f4ee21 (post-fix), same per-case runs ===
--- test_missing_run_head_with_branch_sync_falls_back_to_current_state
ok - missing run head beside a branch_sync block still refuses branch-only attribution

--- test_prepush_pipeline_custody_is_attributed
ok - pre-push pipeline custody is attributed to its own crew

--- test_cached_branch_sync_gate_status_does_not_park_a_running_run
ok - a cached branch_sync pipeline status does not become the run's gate verdict

--- test_released_custody_unresolvable_head_not_attributed
ok - custody released back to the user is not attributed

--- test_pipeline_custody_submitted_elsewhere_not_attributed
ok - pipeline custody of a different submitted head is not attributed

--- test_stale_cached_custody_block_not_attributed
ok - stale cached custody block is not attributed

--- test_custody_claim_for_another_run_not_attributed
ok - custody claim naming a different run is not attributed

--- test_branch_sync_without_run_block_is_not_attributed
ok - a branch_sync block with no run object is never attributed

--- test_coarse_older_terminal_row_does_not_outrank_newest_row
ok - older terminal runs-list row does not outrank the branch's newest row

--- test_parked_prepush_run_owned_by_custody_is_aborted
ok - a parked pre-push run proven by branch_sync custody is aborted before the worker is removed

--- test_parked_prepush_run_submitted_elsewhere_is_never_aborted
ok - a parked run whose custody names another submitted head is never aborted
- Outcome: ⚠️ 1 warning across 1 run (6m40s)

Pipeline

Updates from git push no-mistakes

✅ **intent** - passed

✅ No issues found.

✅ **Rebase** - passed

✅ No issues found.

⚠️ **Review** - 1 info
  • ⚠️ bin/fm-nm-run-lib.sh:193 - fm_nm_status_matches_worktree reads the run head with the unscoped fm_nm_field "$out" head, whose sed takes the first ^[[:space:]]*head: line anywhere in the output. The same commit adds fm_nm_block precisely because "run: and branch_sync.local: both carry a bare head:, and fm_nm_field alone would always answer with the first one" - but the scoping is never applied to this read. Failure sequence: axi status returns a run object with no head: key (the shape test_missing_run_head_falls_back_to_current_state deliberately constructs and pins as "must not permit branch-only attribution") together with a branch_sync block, which is exactly the block this change makes routine. The first head: in the output is then branch_sync.local.head, which is by construction equal to git rev-parse HEAD in the invoking worktree, so fm_nm_head_matches_worktree sees run_full == local_full and returns 0. The run is attributed by the HISTORY rule with no evidence at all about the run's own head - branch name alone, the exact thing the history rule exists to refuse. That existing test cannot catch it because its fixture (run_parked | grep -v '^ head:') has no branch_sync block. This reaches the destructive caller too: fm-teardown.sh now shares this predicate and would run axi abort on a run it has not proven it owns. Fix at the shared boundary the change already built: scope the read, e.g. fm_nm_field "$(fm_nm_block "$out" 0 run)" head, falling back to the unscoped read only when no run: block exists, and extend the missing-head fixture with a branch_sync block so the contract is pinned.
  • ⚠️ bin/fm-teardown.sh:1220 - task_status_is_own_parked_run now gates no-mistakes axi abort on the widened fm_nm_status_matches_worktree, so a run whose head is not an object in this worktree can authorize an abort - something teardown previously always refused. That widening is deliberate per the intent, but it is the one direction the intent names as unsafe if wrong ("a false positive lets teardown abort a run it does not own"), and no test covers it on the teardown side: tests/fm-teardown.test.sh's only negative attribution case is a branch mismatch (parked_axi_status_toon fm/some-other-task deadbeef, line 2174), and every positive case supplies a head that binds by history, so the custody branch is never executed there. All six new cases live in tests/fm-crew-state.test.sh, which only exercises read-only reporting. Add a teardown case with the pre-push shape (unresolvable head plus a pipeline_owned branch_sync) asserting the abort fires, and its mirror with a diverging submitted_head asserting no abort log is written.

🔧 Fix: scope run-object field reads and cover teardown custody
4 issues (2 warnings, 2 infos) still open:

  • ⚠️ bin/fm-crew-state.sh:219 - nm_gate_status greps the WHOLE payload for ^[[:space:]]*(status|state):[[:space:]]*"?(awaiting_approval|fix_review)"?$, and branch_sync.pipeline.status is rendered at 4-space indent, so it matches. Verified directly: grep -E '<that regex>' over a payload whose run block says status: running and whose branch_sync.pipeline says status: awaiting_approval returns the branch_sync line. Failure sequence: line 465 reads the run's own status correctly (running) via the new scoped nm_run_field, but line 469 then reads awaiting_approval out of the cached branch_sync block, line 481's [ -n "$gate_status" ] fires, nm_gate_line_name/nm_gate_name find nothing (no gate: key, no CSV row with a gate status), so gate=$status and the crew is emitted as state: parked | source: run-step | parked at running - a false parked verdict that also suppresses the genuine status-log state and skips the status-log superseded by active run reconciliation at line 546. This is the same class of false verdict the change exists to eliminate, and the change is what makes it routinely reachable: before it, a pre-push run whose head is unresolvable was never attributed, so its branch_sync-bearing payload never reached this parser; now every pre-push fix-round payload does, and every one of them carries branch_sync.pipeline.status. The invariant this change itself establishes is that branch_sync is an untrusted CACHED block - the commit's own comment says the conjuncts exist to "reject an inconsistent or cross-repo cached answer instead of trusting it" - yet the very next thing done with that same cached block is to let it supply the run's gate verdict unchallenged. Divergence needs only a cache not refreshed across a gate resolution: the crew answers the gate, the run resumes to running/fixing, and state/submitted_head/local.head are all unchanged so every custody conjunct still passes. Fix at the boundary the commit already drew: excise the top-level branch_sync: block once (e.g. a RUN_DETAIL_OUT derived from $RUN_OUT) and run the gate/step/findings scans over that. Note scoping these greps to the run: block instead would be wrong - run_parked_in_gate_block legitimately carries status: fix_review inside the top-level gate: block, a run sibling, not inside run:.
  • ⚠️ bin/fm-nm-run-lib.sh:183 - The custody predicate requires [ "$state_field" = pipeline_owned ] exactly, but the comment 40 lines above documents the field's own vocabulary as "pipeline_owned, user_owned, target_changed, remote_missing, legacy_unbound, and the blocked_* variants" - i.e. the change's own documentation asserts that a state the predicate rejects outright is part of the same custody vocabulary. If state can be blocked_pipeline_owned or blocked_pipeline_owned_recoverable (both are present as strings in the installed v1.45.4 binary), then a live pre-push pipeline in a blocked-sync situation is refused by custody, refused by the history rule (head unresolvable), and the exact false verdict this change fixes recurs for that state. Counter-evidence is in the change too: the live-traced fixture records state: pipeline_owned alongside safety: blocked_pipeline_owned, which suggests blocked_* is a safety/next_action code rather than a state value - in which case the comment's vocabulary list is what is wrong, not the predicate. I could not resolve which from the stripped binary. This needs the author's knowledge of the CLI, and loosening the conjunct is the unsafe direction (bin/fm-teardown.sh shares the rule and would gain abort authority), so it must not be changed without that answer: either correct the comment to state that blocked_* belongs to safety, or accept the blocked_pipeline_owned* states with their own pinning test.
  • ℹ️ bin/fm-nm-run-lib.sh:120 - fm_nm_run_field's else arm falls back to the unscoped fm_nm_field whenever the payload has no top-level run: block. If a payload ever carries branch_sync: without a run: block, that arm reads head and branch straight out of branch_sync.local, which describes this worktree by construction, so fm_nm_status_matches_worktree binds on branch-name-plus-self evidence - precisely the hole this function was added to close. I found no shape that produces it (every fixture and every documented v1.45.4 status answer nests the run under run:; the script's own comment records that bare axi status always reports some run, and fm-teardown is protected incidentally because branch_sync has no id: key so its run_id guard trips first), so this is hardening, not a proven path. Worth noting that the else arm is also reached by no test in either file - the flat-shape it exists for has no known producer. Cheapest close: take the fallback only when the payload has no top-level branch_sync: block either.
  • ℹ️ The fix-round commit subject is no-mistakes(review): scope run-object field reads and cover teardown custody. It is the only no-mistakes(...) subject in the last 400 commits - the repo's history is conventional-commits with fix/feat/docs/test/perf/chore/refactor - and it names the internal validation pipeline in what the intent calls a "public upstream repo" whose stated conventions include keeping agent attribution out of commits ("no agent as commit co-author"). Flagging rather than fixing: the intent forbids amending or rewriting commits on this branch, so the only remedy is a reword or squash at merge time, which is the author's call.

🔧 Fix: excise cached branch_sync from run-detail reads
1 info still open:

  • ℹ️ bin/fm-teardown.sh:1224 - Commit 8f4ee21 establishes the invariant that the cached branch_sync block must never answer a question about the run's own detail, and applies it throughout bin/fm-crew-state.sh by deriving RUN_DETAIL_OUT once (fm-crew-state.sh:429) and routing every detail scan through it - including the two scans that are literally duplicated here: the awaiting_agent: grep (fm-crew-state.sh:478) and nm_has_gate (fm-crew-state.sh:238). task_status_is_own_parked_run adopted the scoped reads for id/branch/status (lines 1216, 1218, 1223) but left these two greps on the raw $out, which still contains branch_sync. Those two greps are exactly what decides the abort at line 1229 when status is not awaiting_approval/fix_review, so this is the caller with the destructive blast radius, not the read-only one. I could not prove a reachable path: the live-traced branch_sync shape (state, changed, local{branch,head,clean}, pipeline{run,status,phase,submitted_head,current_head,pushed_head,pushed_at,push_generation}, target, remote, relation, safety, pr_state, note, next_action{code,command}) carries neither an awaiting_agent key nor a bare gate: key, and next_action.command's --gate text cannot match ^[[:space:]]*gate:. So this is hardening, not a live defect - reported at info because it is the completion of the invariant the fix round already drew, in one line: derive detail=$(fm_nm_without_block "$out" 0 branch_sync) and grep that for awaiting_agent and gate, the same way the sibling script does. Note the third unscoped read here, outcome at line 1221, is fine as-is: teardown's teardown-eligibility fixture nests outcome: inside the run block while fm-crew-state's fixtures put it top-level, so the unscoped read is deliberate, and a spurious outcome would only make teardown refuse to abort - the safe direction.
⚠️ **Test** - 1 warning
  • ⚠️ tests/fm-teardown.test.sh:2302 - tests/fm-teardown.test.sh fails at test_leaked_worktree_process_is_reaped because lsof is not installed in this sandbox, and fm-teardown.sh's leaked-process scan requires it. I confirmed this failure is pre-existing and unrelated: it reproduces identically with base bin/fm-teardown.sh from 2d2be63, as does test_run_abort_precedes_process_reap_precedes_worktree_removal. I cannot fix it here (installing system packages is outside the worktree boundary). Consequence: the runner exits on first failure, so the ~9 lsof-gated process-reap tests after that point did not execute locally. Both new custody tests run before that point and passed. Remote CI owns the untested tail.
  • bash tests/fm-crew-state.test.sh (full file, 58 cases, all pass at target)
  • bash tests/fm-teardown.test.sh (full file; passes through both new custody cases, then hits pre-existing lsof-absent failure)
  • Per-case fail-before/pass-after for each new crew-state case run against base bin/ then target bin/: test_prepush_pipeline_custody_is_attributed, test_coarse_older_terminal_row_does_not_outrank_newest_row, test_cached_branch_sync_gate_status_does_not_park_a_running_run, test_missing_run_head_with_branch_sync_falls_back_to_current_state, test_branch_sync_without_run_block_is_not_attributed, test_released_custody_unresolvable_head_not_attributed, test_pipeline_custody_submitted_elsewhere_not_attributed, test_stale_cached_custody_block_not_attributed, test_custody_claim_for_another_run_not_attributed
  • Per-case fail-before/pass-after for test_parked_prepush_run_owned_by_custody_is_aborted and test_parked_prepush_run_submitted_elsewhere_is_never_aborted (teardown abort authority, destructive path)
  • Manual end-user transcript: real bin/fm-crew-state.sh <id> driven over a throwaway git worktree with a fake no-mistakes emitting the live pre-push axi status payload and the 3-row runs list, captured with base bin/ and with target bin/
  • bash tests/fm-gotmp.test.sh (symlinks fm-nm-run-lib.sh into its fake bin; passes)
  • bash tests/fm-documentation-audiences.test.sh (covers the changed docs/architecture.md and docs/scripts.md prose surfaces; passes)
  • Confirmed lsof-absent failures are pre-existing: git checkout 2d2be63 -- bin/fm-teardown.sh bin/fm-nm-run-lib.sh then re-ran test_leaked_worktree_process_is_reaped and test_run_abort_precedes_process_reap_precedes_worktree_removal
  • Scope checks: git diff --stat 2d2be63..8f4ee21 shows only the 7 intended files with no fm-test-run.sh collation change; three commits sit on top of base with no rewrite; added Markdown lines contain no em/en dash
  • git status --porcelain clean after all temporary base/target file swaps and driver scripts were removed
⚠️ **Document** - 1 info
  • ℹ️ docs/decision-hold-lifecycle.md:81 - Pre-existing evidence drift, not caused by this change: docs/decision-hold-lifecycle.md:80-81 records bash tests/fm-teardown.test.sh as emitting all teardown safety cases passed, but that string is absent from tests/fm-teardown.test.sh at the base commit 2d2be63 as well as at HEAD (the suite now prints per-case ok - ... lines). This change adds two teardown custody cases to that file, so the recorded output drifts further, but fixing the dated verification record is a separate consolidation and out of scope for the attribution fix. Follow-up: re-run the documented command and refresh that evidence block.
✅ **Lint** - passed

✅ No issues found.

✅ **Push** - passed

✅ No issues found.

mackcee added 4 commits August 9, 2026 05:11
…local

bin/fm-crew-state.sh reported a healthy, actively running no-mistakes
pipeline as `failed`. That verdict is one of the inputs firstmate uses to
decide whether a worker needs recovery, so it can provoke a teardown or
relaunch of a worker that is mid-pipeline with unpushed work, and it can
send a false failure report.

Root cause: a run in its pre-push phase commits every gate fix round into
no-mistakes' own gate repo, never into the crew worktree, so the head it
reports is not an object in the crew worktree at all. The code-identity
rule cannot bind by either of its tests, attribution falls through to the
coarse runs-list fallback, and that fallback skipped the live `running`
row for the branch and kept scanning until an older superseded `failed`
run matched the untouched worktree head exactly. An older terminal run
won precisely because the live one had moved ahead.

Two changes, in bin/fm-nm-run-lib.sh as the one owner of the rule:

- Add a second, independent proof of ownership taken from no-mistakes'
  own branch_sync object, which `axi status` emits only when the run
  relates to the invoking worktree. It is required to state live pipeline
  custody of this exact branch, head, and run, and that the run was
  submitted from exactly the commit checked out here. The history rule
  stays exactly as strict as it was: an unresolvable head is an absence
  of evidence, not evidence of ownership, and the same absence is
  produced by a rewritten branch. Keeping it strict also matters because
  fm-teardown.sh shares it, where a false positive lets teardown abort a
  run it does not own.

- In the coarse runs-list fallback, let the newest row for the branch
  decide outright. A branch's runs are sequential, so an older run is
  superseded evidence and can never outrank the newest one, whichever
  sha happens to be visible.

fm-teardown.sh adopts the same composed rule, so the two callers cannot
drift and an orphaned parked run whose head advanced is now abortable.

Verified live against a running pipeline (no-mistakes v1.45.4): the
helper now reports `working - validating (running)` matching `axi status`
while the unfixed helper still reports `failed`.

Six regression cases cover both directions. The custody-binds case and
the coarse superseded-row case both fail without this change. The four
refusal cases pin that a released, stale-cached, differently-submitted,
or different-run custody claim still refuses attribution; deleting each
load-bearing conjunct was checked to fail exactly its own case.
@mackcee

mackcee commented Aug 9, 2026

Copy link
Copy Markdown
Author

CI note: Behavior portable serial 1 is red on a test this PR does not touch.

The failure is in tests/fm-remote-job-orphan-reap.test.sh: not ok - could not start the fixture remote job worker, after 120 ms.

Evidence that it is unrelated to this change:

  • This PR changes seven files: bin/fm-crew-state.sh, bin/fm-nm-run-lib.sh, bin/fm-teardown.sh, docs/architecture.md, docs/scripts.md, tests/fm-crew-state.test.sh, tests/fm-teardown.test.sh. The failing test references none of them; its only bin/ dependencies are fm-remote-job-lib.sh, fm-remote-job-worker.sh and fm-remote-job-reap-orphans.sh.
  • The same test passed in the same shard on fix(bin): pin C collation for the test runner's coverage guard #1994 an hour earlier, in 2713 ms.
  • It passes locally against this PR's published head 89669cd, in 2703 ms.
  • It failed after 120 ms inside the fixture's start_worker, which launches a background worker and then locates it with pgrep -f. That is where it stopped, before any assertion about reaping ran.

Outside contributors cannot re-run jobs on this repository (rerun reports Must have admin rights to Repository), so I am recording the evidence here rather than clearing the check myself.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant