Skip to content

fix(bin): resolve fm-crew-state run by newest-on-branch, not stale match - #7

Merged
Aviator-Coding merged 4 commits into
mainfrom
fm/fm-crew-state-unknown-during-pipeline-owned-run-v2
Aug 15, 2026
Merged

Aviator-Coding merged 4 commits into
mainfrom
fm/fm-crew-state-unknown-during-pipeline-owned-run-v2

Conversation

@Aviator-Coding

Copy link
Copy Markdown
Owner

Intent

bin/fm-crew-state.sh must stop misreporting a task whose validation pipeline is healthy.

The defect (verified, not inferred): the helper resolved the wrong no-mistakes run when a branch carried more than one run. Symptom 1: state unknown / source none while axi status showed a running pipeline-owned run whose head was not in the crew worktree (pipeline commits live in its own mirror). Symptom 2 (dangerous): state failed / source run-step / run failed while axi status showed a later run on the same branch still running with a live agent. The earlier run had died; crew-state reported the dead run. Re-reading does not help: the staleness is run selection, not a race. The working discriminator is: enumerate runs, match on branch, take the most recent.

Required behavior:

  • Resolve the run by identity, not by branch alone: prefer the newest run for the branch, and never report a terminal outcome from a superseded run while a later run on the same branch is active.
  • Distinguish "no run" from "run exists, head does not match the worktree", and report the latter with branch_sync (pipeline_owned was correct and available throughout the observations).
  • Keep refusing to treat an unmatched head as authoritative for step semantics. That intent is right; only the fallback was wrong.
  • Regression coverage for (a) two runs on one branch, older terminal and newer active, and (b) a pipeline-owned branch whose head has advanced past the crew worktree.

This work is already committed as those two changes (failing-first tests, then the resolver fix).

Accepted review-gate decisions from the prior run (apply them if they reappear; they are already decided, not open product questions):

  1. branch-sync-supersedes-open-decision: Fix. Skip needs-decision/blocked supersession when RUN_SOURCE=branch-sync, because that path has no step-level evidence to supersede with. Weight this heavily: a helper that silently marks a crew's own open decision stale is the exact failure class that cost this fleet a whole night, and this helper is the declared authority for judging validation.
  2. pipeline-owned-outlives-terminal-run: Fix. Gate the branch-sync "current run" selection on the locally computed axi_terminal signal (or a non-terminal branch_sync.pipeline.status) so a terminated run falls through to the newest-run lookup and the pane/log fallback. Do not treat branch_sync.state=pipeline_owned alone as proof a run is live. Live corroboration: a cancelled run can still show branch_sync.state=pipeline_owned with pipeline.status=cancelled and next_action recover_custody; that case must not report working.
  3. branch-sync-toon-shape-unverified: Fix. Capture a real no-mistakes axi status sample from the installed binary, pin the fixture to it, and record that verification in the header the way sibling CLI-shape claims do. A hand-written fixture that makes tests pass while the real shape silently returns unknown is worse than no test.
  4. Also fix the three auto-fixes: nm-field-reads-branch-sync-block (scope run-scalar lookups to exclude the branch_sync block so a missing run head cannot bind to branch_sync.local.head); discarded-runs-lookup-on-common-path (compute newest-on-branch lookup lazily); duplicate-initialization (remove the redundant re-init). Leave runs-lookup-failure-reopens-symptom-2 as no-op.

Branch history for the PR body: this branch is a recreation named fm/fm-crew-state-unknown-during-pipeline-owned-run-v2. The original branch fm/fm-crew-state-unknown-during-pipeline-owned-run was left intact as fallback. Custody recovery on that original was unrecoverable: no-mistakes v1.48.0 axi sync --recover refused with "the terminal run has no verified head and the gate head does not descend from the recorded head; no files or refs were changed" (safety: blocked_recover_unverified_head). The two feature commits were cherry-picked in order onto current origin/main. No no-mistakes CI-fix commit existed on the original branch, so none were skipped.

What Changed

  • bin/fm-crew-state.sh: replace the branch+head-match-only run lookup with a newest-on-branch resolver (nm_newest_run_for_branch, renamed from nm_runs_status_for_branch) that never falls back past an unmatched-head newest run to an older same-branch row, and gate the branch-sync "current run" path on a locally computed terminal signal (axi_terminal / bs_pipe_terminal) so a terminated run no longer reads as live via a stale branch_sync.state=pipeline_owned.
  • Add a new branch-sync RUN_SOURCE that reports working with branch_sync: <state> detail when a current run exists on the branch but its head is not in the crew worktree, instead of reporting unknown or attributing an older terminal run; also exclude branch-sync from the status-log staleness/supersede check and from log_reports_ci_ready, since that path has no step-level evidence to supersede a genuinely open decision with.
  • bin/fm-nm-run-lib.sh: scope fm_nm_field to skip the branch_sync: block so a missing run-level key can't bind to the corresponding nested branch_sync key, and add fm_nm_status_is_active, fm_nm_branch_sync_block, fm_nm_branch_sync_state, and fm_nm_branch_sync_pipeline_status helpers for reading branch_sync/pipeline state from axi status TOON output.
  • docs/architecture.md: update the fm-crew-state.sh summary to describe newest-run-on-branch attribution and the branch_sync report path.
  • tests/fm-crew-state.test.sh: add regression coverage for two runs on one branch (older terminal, newer active) and for a pipeline-owned branch whose head has advanced past the crew worktree.

Risk Assessment

✅ Low: All five accepted findings from the prior round (pipeline_active gating on axi_terminal, branch-sync skipping needs-decision/blocked supersession, fm_nm_field scoping to exclude branch_sync, verified TOON fixture header, and the duplicate-initialization/lazy-lookup cleanup) are correctly and verifiably applied in bin/fm-crew-state.sh and bin/fm-nm-run-lib.sh, the new regression tests exercise public run_crew_state output rather than source text, and the required run-selection/branch_sync behavior from the user intent is present and traced through each control-flow branch without a reachable gap.

Testing

Targeted suite tests/fm-crew-state.test.sh (55 cases, including 5 new regression tests) passes fully at the target commit, and I confirmed by running the same tests against the pre-fix script (base commit) that two of the new tests genuinely fail before the fix and pass after it, directly reproducing the two symptoms described in the user intent (superseded-run failed report, and unknown/none for a pipeline-owned run with unmatched head).

Evidence: Full fm-crew-state.test.sh run at target commit (55/55 passing)
ok - active run-step is authoritative
... (53 more) ...
ok - pipeline-owned unmatched head reports branch_sync, not unknown
ok - pipeline-owned unmatched head is not replaced by an older failed run
ok - a cancelled branch_sync.pipeline.status is not treated as a live run
ok - branch-sync does not supersede a crew's own open needs-decision
ok - missing run head is not bound via branch_sync.local.head
all fm-crew-state tests passed
Evidence: Same new tests run against pre-fix bin/fm-crew-state.sh (base commit 1a7b107) - reproduces both reported symptoms
not ok - newer active run on the same branch -> working (missing: 'state: working')
--- output ---
state: failed · source: run-step · run failed

not ok - pipeline-owned unmatched head -> working (missing: 'state: working')
--- output ---
state: unknown · source: none · no current-state source available

Pipeline

Updates from git push no-mistakes

✅ **intent** - passed

✅ No issues found.

✅ **Rebase** - passed

✅ No issues found.

🔧 **Review** - 5 issues found → auto-fixed ✅
  • 🚨 bin/fm-crew-state.sh:424 - pipeline_active is set to 1 whenever BRANCH_SYNC_STATE = pipeline_owned, ORed ahead of the axi_terminal/bs_pipe check ([ &#34;$BRANCH_SYNC_STATE&#34; = pipeline_owned ] || fm_nm_status_is_active &#34;$bs_pipe&#34;). That means a cancelled/failed pipeline-owned run (branch_sync.state still pipeline_owned, pipeline.status=cancelled) still sets pipeline_active=1, is selected as RUN_SOURCE=branch-sync at line 447, and unconditionally reports RUN_STATE=working at line 490-491. This directly contradicts the accepted decision 'pipeline-owned-outlives-terminal-run', which requires gating branch-sync selection on the locally computed axi_terminal signal or a non-terminal branch_sync.pipeline.status, and explicitly states 'a cancelled run can still show branch_sync.state=pipeline_owned with pipeline.status=cancelled and next_action recover_custody; that case must not report working.' No test in tests/fm-crew-state.test.sh exercises a cancelled/terminal branch_sync.pipeline.status, so this regression is unguarded.
  • 🚨 bin/fm-crew-state.sh:579 - The needs-decision/blocked status-log supersession check (case &#34;$LOG_VERB&#34; in needs-decision|blocked) ... status-log superseded ...) has no guard for RUN_SOURCE=branch-sync. Since RUN_SOURCE=branch-sync always sets RUN_STATE=working with no step-level evidence, a crew with a genuinely open needs-decision/blocked log will have that decision marked 'status-log superseded by active run' and be reported working purely because a branch-sync-matched run exists on the branch. This directly contradicts the accepted decision 'branch-sync-supersedes-open-decision', which explicitly requires skipping this supersession when RUN_SOURCE=branch-sync ('a helper that silently marks a crew's own open decision stale is the exact failure class that cost this fleet a whole night'). No test combines a needs-decision/blocked log with a branch-sync-sourced run.
  • ⚠️ bin/fm-nm-run-lib.sh:57 - fm_nm_field still scans the whole TOON blob unscoped (sed -n &#34;s/^[[:space:]]*$2:[[:space:]]*\(.*\)/\1/p&#34; | head -1), so it is not limited to the top-level run: block. nm_field head/branch/status can match the corresponding nested keys inside branch_sync (e.g. branch_sync.local.head, branch_sync.local.branch, branch_sync.pipeline.status) whenever the top-level run field is absent, since the branch_sync block follows the run block in real axi status output and sed just returns the first line-match in document order. This is the accepted 'nm-field-reads-branch-sync-block' fix (required: 'scope run-scalar lookups to exclude the branch_sync block so a missing run head cannot bind to branch_sync.local.head'), and it was not applied.
  • ⚠️ tests/fm-crew-state.test.sh:349 - The new run_running_pipeline_owned fixture hand-writes the branch_sync:/local:/pipeline: TOON shape without any note that it was verified against a real no-mistakes axi status sample, unlike sibling CLI-shape claims in this file (e.g. lines 58 and 724, 'verified against the installed v1.32.2'/'the real CLI'). This is the accepted 'branch-sync-toon-shape-unverified' fix ('Capture a real no-mistakes axi status sample from the installed binary, pin the fixture to it, and record that verification in the header'), and it was not applied - the exact anti-pattern the decision called out ('a hand-written fixture that makes tests pass while the real shape silently returns unknown is worse than no test').
  • ℹ️ bin/fm-crew-state.sh:435 - newest_status=&#34;&#34; / newest_match=&#34;&#34; are set at lines 428-429 and then redundantly reset again at lines 435-436 inside the same conditional block before being read. This is the accepted 'duplicate-initialization' cleanup ('remove the redundant re-init'), which was not applied. Purely cosmetic, no behavior impact.

🔧 Fix: Gate branch-sync on pipeline terminal state; scope nm_field; add regressions
✅ Re-checked - no issues remain.

✅ **Test** - passed

✅ No issues found.

  • bash tests/fm-crew-state.test.sh at target commit be02b06 - all 55 tests pass
  • Same test file run against pre-fix bin/fm-crew-state.sh/bin/fm-nm-run-lib.sh from base commit 1a7b107 (via a temporary detached worktree) - confirmed test_newer_active_run_not_shadowed_by_older_failed_same_branch and test_pipeline_owned_unmatched_head_not_unknown fail pre-fix and pass post-fix, reproducing Symptom 2 and Symptom 1 from the user intent
  • Confirmed test_pipeline_owned_cancelled_not_working, test_branch_sync_does_not_supersede_open_decision, and test_missing_run_head_not_bound_via_branch_sync_local_head pass at target, covering the three accepted review-gate auto-fixes
  • Verified no other test file exercises bin/fm-nm-run-lib.sh's changed nm_field scoping (only an unrelated fakebin symlink in tests/fm-gotmp.test.sh)
✅ **Document** - passed

✅ No issues found.

✅ **Lint** - passed

✅ No issues found.

✅ **Push** - passed

✅ No issues found.

Pin the 2026-08-10/12 validation-state defect before changing the
resolver: an older terminal run on the same branch can be reported as
current failed, and a pipeline-owned run whose head is not in the crew
worktree is reported as unknown.
Resolve the newest run on the crew branch before trusting axi status.
A later active run wins over an older terminal outcome on the same
branch. When that newest run is current but its head is not in the
crew worktree, report branch_sync instead of unknown or the older
run's failed state. Step semantics still require a matching head.
@Aviator-Coding
Aviator-Coding merged commit e699c8b into main Aug 15, 2026
13 checks passed
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