Skip to content

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
mainfrom
fm/land-run-attribution-onto-fork
Open

fix(bin): answer "cannot resolve this run head" instead of "not mine" (land of upstream #1855)#54
sbracewell64 wants to merge 2 commits into
mainfrom
fm/land-run-attribution-onto-fork

Conversation

@sbracewell64

Copy link
Copy Markdown
Owner

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_worktree becomes 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 the nm_run_claims_terminal guard, 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 in bin/fm-test-run.sh's pure-contract-unit family), plus reader and teardown regression cases.
  • docs/architecture.md attribution-boundary update and a firstmate-coding-guidelines wording correction.

Landing verification

Three-way merge screen against the fork trunk:

$ git merge-tree --write-tree fork/main HEAD; echo "exit: $?"
532562fbd6787785f8861d407d3b735693abd198
exit: 0
$ git rev-parse HEAD^{tree}
532562fbd6787785f8861d407d3b735693abd198

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.md and bin/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:

FM_TEST_SUMMARY total=39 failed=1 skipped_gate=1 duration_ms=638219
  • tests/fm-nm-run-lib.test.sh exit=0 (the new suite)
  • tests/fm-crew-state.test.sh exit=0
  • tests/fm-teardown.test.sh exit=0
  • tests/fm-pi-primary-types.test.sh gate_skip=true (Pi not installed in this environment)

Failure disclosed: tests/fm-calm-pi-extension.test.sh exit=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 of ed376cf (this PR's base, none of these changes applied) running that one script alone fails identically:

$ git checkout ed376cf && bin/fm-test-run.sh tests/fm-calm-pi-extension.test.sh
  code: 'ERR_UNKNOWN_FILE_EXTENSION'
Node.js v22.22.1
FM_TEST_END tests/fm-calm-pi-extension.test.sh exit=1
FM_TEST_SUMMARY total=1 failed=1 skipped_gate=0

This branch touches no .pi file.

`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.
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