fix(bin): stop reporting live runs as failed when their unpushed tip is unresolvable - #1855
Open
sbracewell64 wants to merge 2 commits into
Open
Conversation
`fm_nm_head_matches_worktree` resolved the run head THROUGH THE WORKTREE and treated a failed lookup as "no match". During validation no-mistakes commits its fix rounds in its own gate-repo clone and does not push until the push step, so the live run's tip is routinely an object the crew's worktree has never seen. Measured 2026-08-06 on a lane actively running its lint step: worktree HEAD d4032f0, live run head 5152b3a, `git cat-file -t 5152b3a` in the worktree "Not a valid object name". The helper's own header said the descendant case MUST match because pipeline fix commits advance the run tip past local HEAD - the implementation could not see those commits, so the documented normal case was structurally unmatchable during every fix round. The rejected run then fell through to the coarse runs-list scan, which applied the same rule per row: the live running row was skipped as unresolvable and an OLDER, genuinely failed run sitting at the worktree's own head matched and won. The answer was not "unknown" but confidently wrong, in the direction that makes a working lane look dead: `state: failed - source: run-step - run failed`. The rule is now three-valued - match, no match, unresolvable - and stays the single owner shared by both consumers: - fm-crew-state.sh: an active run on this crew's own branch whose tip cannot be resolved is attributed and reads as working, since bare `axi status` answers for the queried branch whenever that branch has a run. A TERMINAL run whose head cannot be bound is not attributed at all, because binding its head was the only thing that could tie its verdict to this worktree. The coarse scan now binds the branch's newest row and stops, rather than walking past the live row to an older one whose sha happens to match; an unresolvable newest row answers only "a run is active on this branch", never a terminal status. - fm-teardown.sh: the direction is unchanged and deliberate. Only a positive code-identity MATCH authorizes aborting a parked run; both non-zero verdicts DECLINE, so teardown never aborts a run it cannot positively attribute. The cost of declining is a run left parked for firstmate to see; the cost of guessing is killing another crew's live validation. Upstream PR 1816 does not fix this class: it owns current-run selection and applies the same visibility test, so it rejects the same live rows. Tests: new tests/fm-nm-run-lib.test.sh pins all three verdicts, including a descendant commit created in a SEPARATE clone so it is genuinely absent from the worktree's object store - the condition that produced this defect and the one no existing test covered. Four reader cases and one teardown case cover the consumers. Every case was witnessed red against the pre-fix logic or a targeted mutation before green; the two coarse/reader regressions reproduce the measured `state: failed - source: run-step - run failed` verbatim. The test fixture rules added to firstmate-coding-guidelines are not incidental: an earlier draft of these fixtures resolved a repo path to empty and ran `git -C "" reset --hard` against the checkout the tests live in.
5 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
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Intent
Fix a measured false-verdict defect in the shared no-mistakes run-attribution rule, bin/fm-nm-run-lib.sh:fm_nm_head_matches_worktree, and its two consumers (bin/fm-crew-state.sh, bin/fm-teardown.sh).
RESUMING: this branch already carries commit 078c7eff, the fix round produced by THIS pipeline's own review gate on run 01KZC6Q944NDA19ET6955Q530V (review completed, 0 findings remaining). That run then died at the test step because the agent's Claude session limit was hit mid-step ("You've hit your session limit"), not because any test failed; custody was returned with no-mistakes axi sync --recover, preserving the review fixes. Re-validating the same content.
THE DEFECT (measured live 2026-08-06). A lane actively running its lint step was reported "state: failed / source: run-step / run failed". Mechanism, traced end to end: fm_nm_head_matches_worktree resolves the run head THROUGH THE WORKTREE (git -C "$wt" rev-parse --verify "${run_head}^{commit}" || return 1). During validation, no-mistakes commits its fix rounds in its own gate-repo clone and does not push until the push step, so the live run's tip is routinely NOT an object the crew worktree has (measured: worktree HEAD d4032f0, live run head 5152b3a, "Not a valid object name"). The lookup failure returned "no match", collapsing "I cannot determine this" into "this is not mine". The helper's own header documents that the descendant case MUST match because "pipeline fix commits advanced the run tip past local HEAD" - yet the implementation cannot see those commits, so the documented normal case was structurally unmatchable during every fix round. The rejected run then fell through to the coarse runs-list scan, which applies the same rule per row: the live running row was skipped and an OLDER, genuinely FAILED run sitting at the worktree's own head matched and won (COARSE_STATUS=failed, RUN_SOURCE=coarse, RUN_STATE=failed). A working lane was reported dead.
THE FIX.
TESTS. New tests/fm-nm-run-lib.test.sh (7 cases) pins all three verdicts directly, including the case no existing test covered: a run head genuinely absent from the worktree, reproduced faithfully by cloning the fixture origin into a separate gate clone, committing there, and asserting the sha is unreachable from the worktree - the real unpushed-pipeline-tip shape. tests/fm-crew-state.test.sh gains 4 reader tests, two of which reproduced the measured "state: failed / source: run-step / run failed" verbatim before the fix. tests/fm-teardown.test.sh gains a test that a parked run with an unresolvable head is never aborted. Every test was witnessed red against the pre-fix logic or a mutation before green. bin/fm-test-run.sh registers the new file in the pure-contract-unit family. Test fixtures pin every git call to the test temp root after an earlier draft's empty repo path ran destructive git against the real checkout; that lesson is recorded in the firstmate-coding-guidelines skill's repo style rules, with the bash mechanism corrected by the review gate.
DOCS. docs/architecture.md's one-line description of the attribution boundary had become inaccurate, so it now describes the three-valued rule and the unknown-head behavior and points at bin/fm-nm-run-lib.sh as the owner.
CONSTRAINTS HONORED. Upstream PR 1816 owns current-run SELECTION and is deliberately not touched or duplicated; note that 1816 does NOT fix this class, because it applies the same worktree-visibility test and therefore rejects the same live rows. bin/fm-crew-state.sh is concurrently being changed by another lane (structured --json mode), so the reader change is kept minimal and confined to the attribution/fallback path so an ordinary rebase can reconcile.
KNOWN PRE-EXISTING FAILURE, NOT CAUSED BY THIS CHANGE: tests/fm-backend.test.sh fails at "not ok - old fm-teardown.sh (scout, report present) should succeed" with "fm-decision-hold: compatible tasks-axi is required". Verified via git stash to fail identically at the unmodified contribution base 345de4e - it is an environment tooling requirement, unrelated to this change. tests/fm-nm-run-lib.test.sh, tests/fm-crew-state.test.sh, tests/fm-teardown.test.sh, and bin/fm-lint.sh all pass at this head.
What Changed
fm_nm_head_matches_worktreeinbin/fm-nm-run-lib.shnow returns a three-valued verdict — 0 match, 1 no match, 2 unresolvable — instead of collapsing "this worktree cannot resolve the run head" into "not mine". An unresolvable head is the routine mid-validation shape (no-mistakes commits fix rounds in its own gate clone and doesn't push until the push step), and the previous two-valued rule made the documented descendant case structurally unmatchable during every fix round. Resolution stays read-only: the helper never fetches to make an absent object appear.bin/fm-crew-state.shnow attributes a branch-matched run with an unresolvable head as this worktree's live run whenever it makes no terminal claim (a newnm_run_claims_terminalguard treats either a non-empty outcome or a terminal status word as terminal, so verdict 2 can never surface as failed/done), and the coarse runs-list fallback binds only the newest row for the branch and stops — never walking past it to let an older finished run win, which is how a working lane got reported "state: failed / run failed".bin/fm-teardown.shbehavior is unchanged: both non-match verdicts still decline to abort, now with the direction documented, so teardown never kills a run it cannot positively attribute.tests/fm-nm-run-lib.test.shpins all three verdicts (including a genuinely unreachable gate-clone tip),tests/fm-crew-state.test.shgains attribution/fallback tests that reproduced the measured false "run failed" verbatim before the fix,tests/fm-teardown.test.shgains an unresolvable-head decline test — each witnessed red against the pre-fix base per the pipeline's test step — anddocs/architecture.mdnow describes the three-valued rule withbin/fm-nm-run-lib.shas its owner.Risk Assessment
✅ Low: The change keeps the attribution rule in a single owner, moves both consumers in the fail-safe direction (unknown never becomes a terminal verdict, teardown still declines), is pinned by red-then-green tests for all three verdicts including the measured incident shapes, and is byte-identical in change-owned files to the fix round already reviewed to zero findings, merely rebased onto the newer base.
Testing
Ran the three targeted suites at head (all green), witnessed the new unit and crew-state regression tests red against the pre-fix base — reproducing the measured false 'run failed' verdict verbatim — and captured a before/after transcript of the real fm-crew-state.sh CLI showing the live-lane scenario corrected from 'state: failed' to 'state: working'; teardown's decline-on-unknown behavior was additionally proven unchanged by running its suite at the base commit, and runner registration of the new test file was verified. No visual artifacts because this is a CLI/shell-library change with no rendered surface; transcripts are the end-user surface.
Evidence: Before/after fm-crew-state.sh CLI transcript (measured defect scenario)
BEFORE (base fb368dc, pre-fix): $ fm-crew-state.sh wedge state: failed · source: run-step · run failed AFTER (head d6bf74c, this change): $ fm-crew-state.sh wedge state: working · source: run-step · validating (background run)Evidence: Red witness: new unit tests vs pre-fix helper
not ok - live run tip committed in the gate-repo clone, not pushed yet: expected verdict 2, got 1Evidence: Red witness: crew-state tests vs pre-fix reader (verbatim measured defect)
not ok - an older failed row must never answer for a live run (unexpected: 'state: failed') --- output --- state: failed · source: run-step · run failedEvidence: Green run: fm-nm-run-lib unit tests at head
Evidence: Green run: fm-crew-state tests at head
Evidence: Green run: fm-teardown tests at head
Evidence: Teardown suite at pre-fix base (proves decline-on-unknown unchanged)
Evidence: Reproducible evidence harness script (before/after scenario)
Pipeline
Updates from git push no-mistakes
✅ **intent** - passed
✅ No issues found.
✅ **Rebase** - passed
✅ No issues found.
bin/fm-nm-run-lib.sh:88- The helper's contract header says "Neither caller may turn 2 into a terminal verdict: fm-crew-state.sh reports working/validating or unknown", but a verdict-2-attributed run in fm-crew-state.sh can also legitimately be reportedparked(gate present with non-terminal status — the routine mid-fix-round shape) ordonevia the ci-checks-green log override (status=ci after push while the worktree has not fetched the pushed tip). Both downstream states are correct product behavior (parked is non-terminal; checks-green is PR/branch-scoped, not object-store-scoped), so this is a comment-precision note only — the guard that matters (nm_run_claims_terminal blocking the failed/done no-outcome mapping) is correctly in place and tested.✅ **Test** - passed
✅ No issues found.
bash tests/fm-nm-run-lib.test.sh (8/8 green at head, all three verdicts incl. unpushed gate-clone tip)bash tests/fm-crew-state.test.sh (all green at head incl. 5 new attribution/fallback tests)bash tests/fm-teardown.test.sh (58 green at head incl. new unresolvable-head decline test)bin/fm-test-run.sh tests/fm-nm-run-lib.test.sh (runner accepts file, family=pure-contract-unit)red witness: bash tests/fm-nm-run-lib.test.sh against base fb368dc via git-archive tree (fails 'expected verdict 2, got 1' on the unpushed-tip case)red witness: bash tests/fm-crew-state.test.sh against base fb368dc (fails dumping the verbatim measured defect 'state: failed · source: run-step · run failed')unchanged-behavior check: bash tests/fm-teardown.test.sh against base fb368dc (new decline test passes pre-fix too, exit 0)manual scenario harness driving the real bin/fm-crew-state.sh with unpushed gate-clone tip + stale failed row, run against both base and head trees✅ **Document** - passed
✅ No issues found.
✅ **Lint** - passed
✅ No issues found.
✅ **Push** - passed
✅ No issues found.