fix(bin): answer "cannot resolve this run head" instead of "not mine" (land of upstream #1855) - #54
Open
sbracewell64 wants to merge 2 commits into
Open
fix(bin): answer "cannot resolve this run head" instead of "not mine" (land of upstream #1855)#54sbracewell64 wants to merge 2 commits into
sbracewell64 wants to merge 2 commits into
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.
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
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.
Lands the three-valued run-attribution fix onto the fork trunk.
The contribution exists upstream as kunchenguid#1855, whose venue checks never executed (cross-fork approval hold), so its head is pipeline-validated but venue-unverified. This branch was cut fresh from the fork trunk and carries only that contribution's own two commits; the upstream PR branch was left untouched.
What changes
bin/fm-nm-run-lib.sh:fm_nm_head_matches_worktreebecomes three-valued - 0 MATCH, 1 NO MATCH, 2 UNRESOLVABLE - so "I cannot determine this" is never coerced into a terminal verdict. A validating run commits its fix rounds in no-mistakes' 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; calling that "no match" made the normal fix-round case structurally unmatchable, and the rejected run then fell through to a coarser scan where an older, genuinely failed run at the worktree's own head won attribution. A working lane read as dead.bin/fm-crew-state.sh: adds thenm_run_claims_terminalguard, so an active own-branch run with an unresolvable head reads working rather than failed, and the coarse runs-list scan binds the branch's newest row instead of walking past it to an older matching one.bin/fm-teardown.sh: unchanged in direction - only a positive MATCH authorizes aborting a run, so teardown still declines to abort a run it cannot positively attribute.tests/fm-nm-run-lib.test.sh(new, registered inbin/fm-test-run.sh's pure-contract-unit family), plus reader and teardown regression cases.docs/architecture.mdattribution-boundary update and afirstmate-coding-guidelineswording correction.Landing verification
Three-way merge screen against the fork trunk:
Merge result equals this branch's own tree, so nothing on the trunk is lost. Two conflicts were resolved against the current trunk (
firstmate-coding-guidelines/SKILL.mdandbin/fm-test-run.sh): both were additive-vs-additive, and the trunk's own entries (fm_test_reap,fm-launch-lib.test.sh) were kept alongside the contribution's additions. The diff against the trunk matches the contribution's own diffstat exactly - 9 files, 508 insertions, 35 deletions.Test evidence
bin/fm-lint.sh- clean (ShellCheck 0.11.0, pinned 0.11.0), exit 0.bin/fm-test-run.sh --changed --base fork/main- 39 scripts, 824 passing cases, 1 failing, 1 gate skip:tests/fm-nm-run-lib.test.shexit=0 (the new suite)tests/fm-crew-state.test.shexit=0tests/fm-teardown.test.shexit=0tests/fm-pi-primary-types.test.shgate_skip=true (Pi not installed in this environment)Failure disclosed:
tests/fm-calm-pi-extension.test.shexit=1 -TypeError [ERR_UNKNOWN_FILE_EXTENSION]: Unknown file extension ".ts"under Node v22.22.1, an environment without a TypeScript loader. Proven pre-existing on the unmodified trunk: a clean checkout ofed376cf(this PR's base, none of these changes applied) running that one script alone fails identically:This branch touches no
.pifile.