fix(bin): claim a merged PR only when the pr step and every step after it completed

[loom-loki]

Intent

Contribute an already-written, already-proven fix upstream to this project. Nothing to design, nothing new to implement.

THE BUG: bin/fm-crew-state.sh claimed a no-mistakes run had merged a pull request when it had not. A run reaches outcome=passed once its steps finish without failing, which INCLUDES a run whose pr and ci steps were SKIPPED. Such a run opened and merged nothing, yet the reader mapped outcome=passed to the fixed detail "run passed: PR merged/closed", asserting a merge it had never observed. Verified on no-mistakes v1.60.2: run 01M1EA5NJVP18AE7SPY5MBYW42 reported pr,skipped and ci,skipped under outcome=passed while the forge still had that branch PR open and unmerged, and the work was reported as landed.

THE FIX (deliberate design decision): state the rule GENERALLY rather than special-casing one step. The merge claim is made only when the run own pr step AND every step it reports after that one actually completed. This is deliberate: the same false-merge claim existed one trigger over, when ci rather than pr was skipped, and both are closed by the one general rule. Do NOT reintroduce a per-step special case, and do NOT hardcode any step name after pr - the steps a merge claim depends on are read from the run own step order, because axi status carries no merge signal to key on directly. A run that passed with the pull request opened but checks unverified now reports that honestly instead of claiming a merge.

Implementation: generalises the previous ci-only nm_ci_step_status into nm_step_rows/nm_step_status and adds nm_passed_pr_detail. Reading the RIGHT row is load-bearing: the findings table is emitted BEFORE the steps table, so a finding whose id is pr can be matched first and shadow the genuine pr step row, reporting the finding SEVERITY as the step status. nm_step_rows therefore reads only rows sitting INSIDE the steps table - after the steps[N]{...}: header and indented deeper than it, up to the first dedent - with the header anchored so active_steps[...] cannot open the section. nm_ci_step_status is deliberately kept as a narrow wrapper that returns only running/fixing so its caller falls through unchanged.

CHANGED AFTER THIS PULL REQUEST WAS OPENED - and thanks to the automated reviewer for the catch, it was a good one: the first version of nm_step_rows told a step row from a findings row by requiring an unquoted NUMERIC third column, since a step row duration_ms is a number while a findings row third column is a file path. The review pointed out that a finding on a numeric file path such as 123 would satisfy that predicate and shadow the genuine pr step. That shape does not occur on v1.60.2 - the encoder quotes a numeric-looking string field, so such a finding renders as "123" and falls out of the predicate - but that is an observation about one version quoting behaviour, not a guarantee the format offers, and the step a merge claim depends on is exactly the step a finding can be named after. The predicate is therefore now scoped by POSITION in the output instead of by column content, so a findings row cannot satisfy it however the encoder chooses to quote. This also removes the opposite failure the old predicate carried: had duration_ms ever rendered quoted, it would have made genuine step rows unreadable.

ACCEPTED REFINEMENTS ALREADY APPLIED IN THIS BRANCH (do not undo, do not re-litigate):

1. All five details nm_passed_pr_detail can emit end in the same suffix "merge state unknown to the run"; and the skipped-pr branch keeps its "no PR was opened or merged by the run" scoping in front of that suffix - without those words the sentence would assert a forge fact the run never observed and contradict its own next clause. Every corresponding test asserts the COMPLETE detail string rather than a fragment, so a future edit to any one detail fails the suite rather than drifting silently.
2. The active-run side of the step-row reader is pinned by a fixture BUILT FROM OBSERVED OUTPUT, not from assumption: axi status was captured mid-step on live runs of no-mistakes v1.60.2 (eb4e379, built 2026-08-29), sampled on three different running steps - three distinct step names, not three runs - and every sample renders the running step as <step>,running,0,0 in the steps table beside <step>,running,<duration>,... in a separate active_steps table whose third column is a duration. The fixture and the comment both record that this shape was observed on that version rather than inferred, because a vendor output shape is a fact about a version, not a law. Mutation-tested: break the reader so it stops matching the active run steps row and that test fails with the exact incident signature, a green PR reading as still validating.

TESTS: tests/fm-crew-state.test.sh covers the skipped pr step, a completed pr step with a skipped ci step, pr as the last reported step, a run with no pr row at all, the still-valid genuinely-merged case, the active-run both-tables case above, and a NEGATIVE case for the findings-row guard. That negative case is deliberate and has been verified to fail when the guard is removed - an earlier attempt at it would have passed with the guard gone, which is exactly what it must not do. That negative case now runs for two findings-row shapes - an ordinary file column (a.go) and a bare numeric one (123) - and the numeric shape was verified to FAIL against the earlier column-content predicate, reporting the finding severity as the pr step status, and to pass once the reader was scoped to the steps table.

SCOPE CONSTRAINT (explicit, deliberate): exactly two files - bin/fm-crew-state.sh and tests/fm-crew-state.test.sh. Touching a third file is out of scope. This fix carries no dependency on any forge-specific or provider-specific support code and must not acquire one.

DELIVERY: this is a contribution to somebody else project. The pull request body must stand on its own for a maintainer with none of this context: what the bug is, how it reproduces, why the fix is stated generally rather than per-step, and what the tests prove. Do not merge. Stop when the pull request is open and its checks are green.

NOTE ON THIS RUN: a previous run of this same branch reached the push step and failed there on a transient SSH authentication error from the forge; the identical command has since succeeded repeatedly. The two review-fix commits from that run are preserved on this branch and custody was returned through the supported recovery path. Nothing else changed.

What Changed

• Bug: bin/fm-crew-state.sh mapped a terminal outcome=passed run to the fixed detail run passed: PR merged/closed, asserting a merge it had never observed. A run reaches outcome=passed once its steps finish without failing, which includes a run whose pr and ci steps were skipped - the shape no-mistakes takes when it cannot resolve a push provider, so nothing was opened or merged. Reproduced on no-mistakes v1.60.2: run 01M1EA5NJVP18AE7SPY5MBYW42 reported pr,skipped,0,17 and ci,skipped,0,16 under outcome=passed while the forge still had that branch's pull request open and unmerged, and the work was reported as landed.
• Fix: new nm_passed_pr_detail claims a merge only when the run's own pr step and every step the run reports after it read completed; otherwise the detail names the step that stopped short and ends in merge state unknown to the run. The rule is stated generally rather than special-cased on one step because the same false claim existed one trigger over (axi run --skip=ci leaves pr completed, ci skipped, PR open), and no step name after pr is hardcoded - the steps a merge claim depends on are read from the run's own step order, since axi status carries no merge signal to key on directly. Supporting change: the ci-only nm_ci_step_status is generalised into nm_step_rows/nm_step_status, needed because the findings table is emitted before the steps table and a finding whose id is pr would otherwise shadow the real step row and report the finding's severity as the step status. nm_step_rows selects rows by their position in the output - those inside the steps[N]{...}: table, with the header anchored so active_steps[...] cannot open the section - rather than by matching a numeric third column, so no findings row can satisfy it regardless of how the encoder quotes a numeric-looking field. nm_ci_step_status remains as a narrow wrapper returning only running/fixing so its caller falls through unchanged.
• Tests: seven cases added to tests/fm-crew-state.test.sh covering a skipped pr step, a completed pr step with a skipped ci step, pr as the last reported step, a run with no pr row at all, the still-valid genuinely-merged run (fixture from real run 01M1669Y82JTHWEBSG7PR2TKNH), an active run rendering both the steps and active_steps tables (fixture built from axi status captured mid-step on live v1.60.2 runs, pinning that the numeric guard did not break the green-checks override behind the PR fix(backends): scope zellij tabs by firstmate home #252 incident), and a negative case that fails if the findings-row guard is removed, now run for both an ordinary a.go file column and a bare numeric 123 - the numeric shape fails against the earlier column-content predicate and passes once the reader is scoped to the steps table. Every terminal-detail assertion was also tightened from a substring fragment to the complete expected string.

Risk Assessment

✅ Low: A well-bounded two-file change to one read-only reporting helper that replaces an over-claiming fixed string with a conservative, generally-stated derivation, backed by fixtures copied from real observed output and a negative test for the load-bearing guard; every traced input produces a correct detail and no downstream consumer parses the changed string.

Testing

I reproduced the reported bug end-to-end before testing the fix: running bin/fm-crew-state.sh from the base commit against the real v1.60.2 axi status output for run 01M1EA5NJVP18AE7SPY5MBYW42 (pr and ci skipped, outcome passed) prints run passed: PR merged/closed, and this branch prints run passed, PR step skipped: no PR was opened or merged by the run, merge state unknown to the run; the same before/after holds for the --skip=ci shape, while the genuinely landed run still reads as merged on both. I followed the detail through to the surfaces a captain and firstmate actually see - the /bearings Underway row and the fleet-snapshot JSON - and measured the longest new detail against the board's 90-char truncation: it is 98 characters and IS clipped there, rendering as "...merge state unknown to" with the trailing "the run" cut. The "by the run" scoping and the "merge state unknown" sense both survive the cut, and the fleet-snapshot JSON firstmate consumes carries the full untruncated string, so no consumer sees a claim stronger than the run can support. An earlier revision of this section said 87 characters and survived intact; that stopped being true when the "by the run" scoping was restored, and this is the corrected measurement. Each of the five new merge-claim tests fails individually against the pre-fix reader with the exact false-merge signature and passes here, and the two invariant tests that necessarily pass both before and after are pinned by mutation instead, re-run against the implementation actually in this diff: removing the steps-table scoping makes the findings-row test fail with a finding severity reported as the pr step status, and breaking the section boundary so an active run's own steps rows stop being emitted makes the active-run test fail with a green PR reading as still validating. The original numeric-column mutation has no analogue any more, because the predicate no longer inspects column content at all. The targeted suite and the downstream fleet-snapshot view suite are green, no other suite asserts on this detail string, and every mutation below was applied to a throwaway copy of this branch's head and reverted, leaving the branch itself untouched. This is a CLI text-line change with no rendered UI surface, so the evidence is CLI transcripts rather than screenshots.

Evidence: Before/after reader transcript over the real observed axi status shapes

fm-crew-state.sh - false 'PR merged/closed' claim: BEFORE (base 8988af2) vs AFTER (d813ec0)
Each scenario feeds the reader a real `no-mistakes axi status` shape observed on v1.60.2
and runs the SAME command against both readers. The one-line output is the reader's
product interface - the line firstmate reads every heartbeat.

===============================================================================
SCENARIO: run 01M1EA5NJVP18AE7SPY5MBYW42 shape - outcome=passed, pr and ci SKIPPED (no PR ever opened; the forge still had the branch PR open and unmerged)
-------------------------------------------------------------------------------
$ no-mistakes axi status            # what the reader is given
run:
  id: "01M1EA5NJVP18AE7SPY5MBYW42"
  branch: fm/feat-prskip
  status: completed
  head: "<worktree HEAD>"
  findings: "1 awaiting, 3 auto-fix, 42 info"
  steps[9]{step,status,findings,duration_ms}:
    intent,completed,0,13
    rebase,skipped,42,521
    review,completed,3,836695
    test,completed,0,704937
    document,completed,0,939357
    lint,completed,1,16
    push,completed,0,697
    pr,skipped,0,17
    ci,skipped,0,16
outcome: passed

$ bin/fm-crew-state.sh prskip     # BEFORE (base 8988af2)
state: done · source: run-step · run passed: PR merged/closed

$ bin/fm-crew-state.sh prskip     # AFTER  (this branch d813ec0)
state: done · source: run-step · run passed, PR step skipped: no PR was opened or merged by the run, merge state unknown to the run

===============================================================================
SCENARIO: `no-mistakes axi run --skip=ci` shape - outcome=passed, pr COMPLETED (PR opened) but the later ci step SKIPPED, so the PR was left open
-------------------------------------------------------------------------------
$ no-mistakes axi status            # what the reader is given
run:
  id: "01RUNCISKIP"
  branch: fm/feat-ciskip
  status: completed
  head: "<worktree HEAD>"
  pr: "https://github.com/o/r/pull/1"
  findings: "1 auto-fix"
  steps[9]{step,status,findings,duration_ms}:
    intent,completed,0,2
    rebase,completed,0,1953
    review,completed,3,6046577
    test,completed,1,1385831
    document,completed,2,418814
    lint,completed,1,13
    push,completed,0,4179
    pr,completed,0,43885
    ci,skipped,0,16
outcome: passed

$ bin/fm-crew-state.sh ciskip     # BEFORE (base 8988af2)
state: done · source: run-step · run passed: PR merged/closed

$ bin/fm-crew-state.sh ciskip     # AFTER  (this branch d813ec0)
state: done · source: run-step · run passed, PR opened but ci step skipped: merge state unknown to the run

===============================================================================
SCENARIO: run 01M1669Y82JTHWEBSG7PR2TKNH shape - the genuinely landed run: pr completed AND every step after it completed
-------------------------------------------------------------------------------
$ no-mistakes axi status            # what the reader is given
run:
  id: "01M1669Y82JTHWEBSG7PR2TKNH"
  branch: fm/feat-merged
  status: completed
  head: "<worktree HEAD>"
  pr: "https://github.com/o/r/pull/1"
  findings: "3 awaiting, 3 auto-fix, 1 info"
  steps[9]{step,status,findings,duration_ms}:
    intent,completed,0,2
    rebase,completed,0,1953
    review,completed,3,6046577
    test,completed,1,1385831
    document,completed,2,418814
    lint,completed,1,13
    push,completed,0,4179
    pr,completed,0,43885
    ci,completed,0,31779488
outcome: passed

$ bin/fm-crew-state.sh merged     # BEFORE (base 8988af2)
state: done · source: run-step · run passed: PR merged/closed

$ bin/fm-crew-state.sh merged     # AFTER  (this branch d813ec0)
state: done · source: run-step · run passed: PR merged/closed

===============================================================================
SCENARIO: findings table emitted BEFORE the steps table, carrying a finding whose id is literally 'pr' - the steps-table scoping must not let it shadow the real pr step row
-------------------------------------------------------------------------------
$ no-mistakes axi status            # what the reader is given
run:
  id: "01RUNFIND"
  branch: fm/feat-findings
  status: completed
  head: "<worktree HEAD>"
  findings[1]{id,severity,file,line,action,description}:
    pr,warning,a.go,,auto-fix,ignored error
  steps[9]{step,status,findings,duration_ms}:
    intent,completed,0,13
    rebase,skipped,42,521
    review,completed,3,836695
    test,completed,0,704937
    document,completed,0,939357
    lint,completed,1,16
    push,completed,0,697
    pr,skipped,0,17
    ci,skipped,0,16
outcome: passed

$ bin/fm-crew-state.sh findings     # BEFORE (base 8988af2)
state: done · source: run-step · run passed: PR merged/closed

$ bin/fm-crew-state.sh findings     # AFTER  (this branch d813ec0)
state: done · source: run-step · run passed, PR step skipped: no PR was opened or merged by the run, merge state unknown to the run

===============================================================================
SCENARIO: ACTIVE ci-monitoring run rendering BOTH tables (observed shape, v1.60.2):
          steps row 'ci,running,0,0' beside active_steps row 'ci,running,1m40s,...'
          CI is green; the reader must still find the ci step and fire the
          green-checks override, not read the duration-column active_steps row.
-------------------------------------------------------------------------------
run:
  id: "01RUNACTIVE"
  branch: fm/feat-active
  status: running
  head: "<worktree HEAD>"
  pr: "https://github.com/o/r/pull/2"
  findings: none
  steps[9]{step,status,findings,duration_ms}:
    intent,completed,0,12
    rebase,completed,0,77
    review,completed,0,22991
    test,completed,0,1385831
    document,completed,0,418814
    lint,completed,0,13
    push,completed,0,4179
    pr,completed,0,43885
    ci,running,0,0
  active_steps[1]{step,status,active_for,last_activity,agent_pid,round}:
    ci,running,1m40s,"12s ago: claude producing output","12345",starting

$ no-mistakes axi logs --step ci    # ci step log tail
all CI checks passed - still monitoring until merged or closed

$ bin/fm-crew-state.sh active     # AFTER  (this branch d813ec0)
state: done · source: run-step · checks green: PR ready for review (still monitoring for merge/close)

Evidence: Corrected detail on the captain-facing /bearings row and the fleet-snapshot JSON

The corrected detail as it reaches the two surfaces downstream of the reader:
the captain-facing /bearings Underway row (doing column, truncated at 90 chars -
the longest new detail is 98, so it IS clipped there: the 'by the run' scoping
survives the cut, the trailing 'the run' does not)
and the fleet-snapshot JSON firstmate consumes.

--- crew 'prskip' (pr+ci skipped: nothing was ever opened) ----------------------------------------------------------
$ bin/fm-bearings-snapshot.sh        # captain-facing /bearings Underway row
in_flight[1]{id,kind,state,repo,doing}:
  prskip,ship,done,"","run passed, PR step skipped: no PR was opened or merged by the run, merge state unknown to…"

$ bin/fm-fleet-snapshot.sh --json | jq '.tasks[].current_state'   # what firstmate consumes
{"state":"done","source":"run-step","detail":"run passed, PR step skipped: no PR was opened or merged by the run, merge state unknown to the run"}

--- crew 'ciskip' (pr completed, ci skipped: PR opened, left open) ----------------------------------------------------------
$ bin/fm-bearings-snapshot.sh        # captain-facing /bearings Underway row
in_flight[1]{id,kind,state,repo,doing}:
  ciskip,ship,done,"","run passed, PR opened but ci step skipped: merge state unknown to the run"

$ bin/fm-fleet-snapshot.sh --json | jq '.tasks[].current_state'   # what firstmate consumes
{"state":"done","source":"run-step","detail":"run passed, PR opened but ci step skipped: merge state unknown to the run"}

--- crew 'merged' (genuinely landed: still reads as merged) ----------------------------------------------------------
$ bin/fm-bearings-snapshot.sh        # captain-facing /bearings Underway row
in_flight[1]{id,kind,state,repo,doing}:
  merged,ship,done,"","run passed: PR merged/closed"

$ bin/fm-fleet-snapshot.sh --json | jq '.tasks[].current_state'   # what firstmate consumes
{"state":"done","source":"run-step","detail":"run passed: PR merged/closed"}

Evidence: Regression tests failing pre-fix, plus the mutations re-run against this diff's implementation

=== 1. Do the new regression tests actually fail BEFORE the fix? ===============
bin/fm-crew-state.sh temporarily replaced with the base-commit (8988af2) version,
each new test then run on its own (the suite stops at its first failure).
test_terminal_passed_pr_skipped_claims_no_merge
    not ok - skipped pr step must not claim a merged PR (unexpected: 'PR merged')
    --- output ---
    state: done · source: run-step · run passed: PR merged/closed
test_terminal_passed_ci_skipped_claims_no_merge
    not ok - a PR the run never carried past opening must not read as merged (unexpected: 'merged/closed')
    --- output ---
    state: done · source: run-step · run passed: PR merged/closed
test_terminal_passed_with_pr_as_last_step_claims_no_merge
    not ok - a pr step with nothing after it must not claim a merged-or-closed PR (unexpected: 'merged/closed')
    --- output ---
    state: done · source: run-step · run passed: PR merged/closed
test_terminal_passed_without_a_pr_row_claims_no_merge
    not ok - an unreported pr step must not claim a merged-or-closed PR (unexpected: 'merged/closed')
    --- output ---
    state: done · source: run-step · run passed: PR merged/closed
test_findings_row_is_not_read_as_a_step_row
    not ok - the real skipped pr step must be read, not the pr-named finding (missing: 'PR step skipped')
    --- output ---
    state: done · source: run-step · run passed: PR merged/closed
test_terminal_passed_pr_completed_reads_as_landed
    ok - outcome=passed with a completed pr step still reports the merge
test_ci_monitoring_with_active_steps_table_still_reads_the_step_row
    ok - an active run's second active_steps table does not disturb the step-row reader

The last two pass before AND after: they are the no-regression pair (the genuinely
merged run must still read as merged; the active run must still be read at all).
They are pinned by mutation instead:

=== 2. Mutation A - stop distinguishing the steps table from the findings table ===
nm_step_rows reduced to emitting every row-shaped line, whatever section it sits in:
    { if ($0 ~ /^[[:space:]]*[^,[:space:]][^,]*,/) print }

    not ok - the real skipped pr step must be read, not the pr-named finding (file column a.go) (missing: 'PR step skipped')
    --- output ---
    state: done · source: run-step · run passed, PR step warning: merge state unknown to the run
That output IS the defect: the 'pr' FINDING's severity reported as the pr STEP's status,
for a run that opened nothing.

=== 2b. The reviewer's shape, against the predicate this PR replaces ==========
The column-content predicate restored (unquoted numeric third column), which is what
this change removes. The ordinary a.go findings row passes it; the numeric one does not:
    | grep -E "^[[:space:]]*[A-Za-z0-9_-]+,[[:space:]]*\"?[A-Za-z0-9_-]+\"?[[:space:]]*,[[:space:]]*[0-9]+[[:space:]]*,"

    not ok - the real skipped pr step must be read, not the pr-named finding (file column 123) (missing: 'PR step skipped')
    --- output ---
    state: done · source: run-step · run passed, PR step warning: merge state unknown to the run
This is exactly the shape raised in review, reproduced. It is why the predicate is now
scoped by position rather than by column content.

=== 3. Mutation B - break the section boundary so the steps rows stop being emitted ===
The section-close test widened from `RLENGTH > header_indent` to `RLENGTH > header_indent + 2`,
so the table closes immediately and no step row is emitted:

    not ok - an active_steps table must not hide the ci step's own steps row (missing: 'state: done')
    --- output ---
    state: working · source: run-step · validating (running)
That failure IS the PR #252 incident signature - a green PR reading as still validating.

=== 3b. A mutation with NO analogue, stated rather than invented ==============
The header match is anchored so `active_steps[...]` cannot open the section. Unanchoring it
breaks no test: nm_step_status takes the first matching row, the steps row precedes the
active_steps row, and both carry the same status word, so the derived state is unchanged.
    if ($0 ~ /steps\[[0-9]+\]/ && ...        # unanchored
    ok - an active run's second active_steps table does not disturb the step-row reader
    ok - a findings row is never mistaken for a step row, whatever its columns hold
The anchoring is defence in depth against a shape this version does not currently render;
it is not pinned by a test, and this section does not claim otherwise.

=== 4. Reader restored, same test green =======================================
    ok - an active run's second active_steps table does not disturb the step-row reader

Evidence: The bug and the fix, in one pair of lines

$ bin/fm-crew-state.sh prskip # BEFORE (base 8988af2)
state: done · source: run-step · run passed: PR merged/closed

$ bin/fm-crew-state.sh prskip # AFTER (this branch d813ec0)
state: done · source: run-step · run passed, PR step skipped: no PR was opened or merged by the run, merge state unknown to the run

(input: the real `no-mistakes axi status` output for run 01M1EA5NJVP18AE7SPY5MBYW42 -
`pr,skipped,0,17` and `ci,skipped,0,16` under `outcome: passed`, while the forge still
had that branch's pull request open and unmerged)

Pipeline

Updates from git push no-mistakes

✅ **intent** - passed

✅ No issues found.

✅ **Rebase** - passed

✅ No issues found.

⚠️ **Review** - 1 info

• ⚠️ bin/fm-crew-state.sh:329 - The "Verified against all 495 step rows ... all 495 are read" sentence is pre-existing context (it survives unchanged from 542edd9) and was written to evidence the REMOVED column-content predicate: it measured that 495 rows matched an unquoted-numeric-third-column regex. This commit replaces that reader with a positional one, so the equivalent property is now "all 495 rows sit inside a steps[N] section indented deeper than its header" - which the old scan did not test. The sentence now sits inside the paragraph describing the positional rule, so a reader (including the upstream maintainer this is contributed to) takes it as verification of the current reader. Neither the diff nor 0630c0d's commit message states the scan was re-run. This matters more than usual here because the change's own stated discipline (and the intent's) is that comments justifying the old rule must be rewritten rather than left to rot into false claims - this is the one evidence claim carried across the implementation swap. Remedy: either re-run the store-wide scan against the positional reader and say so, or scope the sentence to what it actually established. Flagging rather than fixing because only the author knows whether the scan was re-run.
• ℹ️ tests/fm-crew-state.test.sh:580 - The comment says "The ci status must come from the steps row ci,running,0,0, never from the active_steps row ci,running,1m40s,...", but the fixture cannot fail on the "never" half. nm_step_status does nm_step_rows | grep -E "^[[:space:]]*ci," | head -1, and in run_ci_monitoring_with_active_steps the steps row precedes the active_steps row, so head -1 selects the steps row even if active_steps rows leaked into the stream - and both rows would yield the same status word running anyway. I confirmed this by tracing an intentionally unanchored header regex through the parser: the active_steps rows get appended after the steps rows and the derived ci status is unchanged, so the test still passes. The test IS a valid positive guard (dropping the steps rows makes CI_STEP_STATUS empty, the green-checks override never fires, and state: done fails), which is what the comment's later "pins that the narrowing left the ACTIVE-run path intact" clause claims. Only the "never from the active_steps row" clause is unpinned. Given the author's stated standard that "an assertion that would pass either way is worthless here", narrow the comment to the positive property it actually pins.

🔧 Fix: correct step-row evidence and active_steps test comments
2 issues (1 warning, 1 info) still open:

• ⚠️ bin/fm-crew-state.sh:415 - The skipped-pr detail emits run passed, PR step skipped: no PR was opened or merged, merge state unknown to the run. Commit d1d81a8 dropped by the run from the first clause while making the suffix uniform across the four branches. The sentence now asserts unscoped that no PR was opened or merged, and then says merge state is unknown to the run - self-contradictory, and false in the exact scenario this change exists for: the evidence comment at lines 390-394 records run 01M1EA5NJVP18AE7SPY5MBYW42 reporting pr,skipped,0,17 under outcome=passed while the forge still had that branch's pull request OPEN and unmerged. Concrete path: a run whose push provider cannot be resolved skips pr and ci, reaches outcome=passed, and fm-crew-state renders detail: run passed, PR step skipped: no PR was opened or merged, ... for a branch whose PR is open on the forge. The file's own doc comment at line 35 keeps the correct scoping ("a skipped pr step means the run opened and merged nothing"); only the user-visible string lost it. The base text at 542edd9 (no PR was opened or merged by the run, merge state unknown to it) was accurate. Remedy is restoring the run scoping in that clause (e.g. no PR was opened or merged by the run) while keeping the uniform merge state unknown to the run suffix the tests now assert - but the suffix uniforming was a deliberate prior-round decision, so the author should confirm the wording rather than have it silently reverted.
• ℹ️ bin/fm-crew-state.sh:329 - "the whole local v1.60.2 store ... all 73 runs" can be read as "73 runs produced by v1.60.2". In the store as it stands, 51 of the 73 runs were produced by v1.57.0 and only 22 by v1.60.2. This does not weaken the evidence - I reproduced the scan and the reader reads 657 of 657 rows with zero per-run mismatches against step_results - because output shape is set by the RENDERING binary (v1.60.2 for all 73), not by the producing version. If anything the claim is broader than stated: the reader also handles rows written by an older version. Noting it so the figure is not misread; no change required.

🔧 Fix: restore run scoping in skipped-PR detail, pin full strings
1 info still open:

• ℹ️ tests/fm-crew-state.test.sh:410 - The fixture comment asserts and denies the same fact in one sentence. It says "the durations observed across those samples span both second scale (7s, 18s, 19s) and minute scale (2m34s, 3m41s ...), so the 1m40s below is a duration string that version actually renders rather than a plausible-looking one invented to fill the column", then immediately concedes "only the step name and the durations differ here". 1m40s is not in the cited observed set {7s, 18s, 19s, 2m34s, 3m41s}; what the evidence supports is the minute-scale SHAPE, not that literal string. The bin/fm-crew-state.sh counterpart added by the same commit (1cc57ee) got this right - "the minute-scale third column the test fixture carries is a shape this version actually renders" - but commit 0630c0d rewrote that block and dropped the sentence, leaving only the imprecise string-level claim in the test file. Remedy is wording only: claim the shape ("a minute-scale duration of the form this version renders") and keep the observed sample list, so the sentence stops contradicting its own next clause. No behavior is affected - the active_steps third column is now rejected structurally and is never read.

⚠️ **Test** - 1 error

• 🚨 tests failed with exit code 1
• bin/fm-test-run.sh --changed --exclude-family real-herdr-gated

⚠️ **Document** - 1 info

• ℹ️ bin/fm-crew-state.sh:333 - Out-of-scope consolidation worth a follow-up, reported rather than done. The nm_step_rows comment is now ~50 lines, over half of it a verification narrative (657 of 657 rows across 73 runs, ground-truthed against step_results; live mid-step captures on v1.60.2 eb4e379 built 2026-08-29). Under the placement tree in .agents/skills/firstmate-coding-guidelines/SKILL.md that is tier-5 material - 'active reusable verification for a current guarantee' - whose classified home is a maintainer-verification record under docs/verification/, which docs/documentation-audiences.md says is the surface allowed to carry dates, versions, exact commands and exact output. Tier 7 gives the script header the mechanics (the positional rule itself), not the evidence run. I did not move it, for three reasons: the file already carried comparable inline evidence before this change (the v1.32.2 / '360+ real run logs' paragraph at line ~466), so this change follows the file's established convention rather than introducing drift; the evidence is tightly coupled to why the parser is shaped this way, which is a legitimate reason to keep a distilled form inline; and extracting it would touch a third file, which this run's stated constraints make a stop-and-report condition. If the split is wanted, the shape would be a docs/verification/ record owning the scan and the live captures, with a one-line pointer left in the comment - not a synchronized second copy.

✅ **Lint** - passed

✅ No issues found.

✅ **Push** - passed

✅ No issues found.

[greptile-apps]

Confidence Score: 5/5

The PR appears safe to merge.

No blocking failure remains, and the previously reported numeric findings-shadowing issue is resolved by structurally restricting parsed rows to the steps table.

_{Reviews (2): Last reviewed commit: "no-mistakes(document): correct step-row ..." | Re-trigger Greptile}