Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -377,7 +377,7 @@ Require the matching `resolved` event, forbid `--yes`, and require the worker to
Resume fleet supervision immediately after the decision lands.

Judge validation by the currently attributed run step through `bin/fm-crew-state.sh`, not by shell liveness or the last status event.
Running, fixing, or CI states remain working; parked approval or fix-review states require the worker to follow the active gate help; passed or checks-passed is done; failed or cancelled is failed exactly as `bin/fm-crew-state.sh` prints it - only that state line reclassifies an orphaned ci monitor after green checks as held-for-merge done, or a terminal failed record with the daemon unreachable as unknown, never the raw run record.
Running, fixing, or CI states remain working; parked approval or fix-review states require the worker to follow the active gate help; passed, passed-with-override, or checks-passed is done; failed or cancelled is failed exactly as `bin/fm-crew-state.sh` prints it - only that state line reclassifies an orphaned ci monitor after green checks as held-for-merge done, or a terminal failed record with the daemon unreachable as unknown, never the raw run record.
A worker hand-editing, committing, aborting, or restarting during an active validation run duplicates pipeline ownership outside the supersession sequence above; steer it back to the gate response flow.
The worker reports the PR when CI first becomes green rather than waiting for merge monitoring to finish.

Expand Down
59 changes: 51 additions & 8 deletions bin/fm-crew-state.sh
Original file line number Diff line number Diff line change
Expand Up @@ -50,9 +50,15 @@
# the ledger has been asked whether a live sibling run exists.
# The run-step is AUTHORITATIVE: running/fixing -> working, ci -> working,
# awaiting_approval/fix_review -> parked (with gate findings), terminal
# passed/checks-passed -> done, failed/cancelled -> failed. EXCEPT: while
# the active step is ci, `axi status` alone cannot tell "still waiting on
# checks" from "checks green, waiting on merge" (see nm_ci_checks_state) -
# passed/checks-passed/passed-with-override -> done, failed/cancelled ->
# failed. The outcome NAME is not proof of a merge: passed and
# passed-with-override read their PR detail off the run's own pr_state
# field (nm_outcome_pr_detail) rather than asserting merged/closed from
# the outcome alone (2026-09-20 firstmate-lint-debt-blocking-prs
# incident). An unmapped terminal outcome still reads unknown rather than
# a guessed done/failed. EXCEPT: while the active step is ci, `axi
# status` alone cannot tell "still waiting on checks" from "checks green,
# waiting on merge" (see nm_ci_checks_state) -
# a ci-step log-tail check overrides working -> done once checks read
# green, so a green PR is never silently read as still-validating. And a
# terminal FAILED run whose only failure is the ci monitor step, after
Expand Down Expand Up @@ -463,6 +469,29 @@ EOF
[ "$(nm_ci_checks_state)" = green ]
}

# Honest PR-state clause for a terminal passed/passed-with-override outcome.
# The outcome NAME alone is not proof of a landing: a run can reach a passed
# outcome while still holding for the captain's merge word (2026-09-20
# firstmate-lint-debt-blocking-prs incident - outcome=passed reported
# alongside pr_state=open in the same status block, with the forge
# independently confirming merged=false, merged_at=null). Read the run's own
# pr_state field instead of asserting a merge the outcome name does not
# prove. merged, open, closed and none are each a fact the run record states;
# every other value - an absent field from an older no-mistakes, a
# not-yet-observed state, a spelling this reader does not know - is a single
# unknown, never a guess in either direction.
nm_outcome_pr_detail() {
local pr_state
pr_state=$(strip_quotes "$(nm_field pr_state)")
case "$pr_state" in
merged) printf 'PR merged' ;;
open) printf 'PR open, not yet merged' ;;
closed) printf 'PR closed, not merged' ;;
none) printf 'no PR opened' ;;
*) printf 'PR merge state unknown' ;;
esac
}

# Reclassify a terminal failed run as done (held-for-merge) when
# nm_failed_run_is_green_held_ci matches, surfacing the run's PR URL so the
# supervisor reads the concrete review-ready outcome instead of a failure.
Expand Down Expand Up @@ -513,10 +542,14 @@ nm_effective_ci_step_status() {
}

# Root cause of the PR #252 incident (2026-07): for a repo where merge is left
# to the captain, no-mistakes' ci step (and therefore top-level status/outcome)
# stays "running" for the ENTIRE CI-monitor phase, including long after GitHub
# reports every check green - it only reaches outcome=passed once the PR is
# actually merged (or failed/cancelled if closed). `axi status`'s steps[] table
# to the captain, no-mistakes' ci step (and therefore top-level status) can
# stay "running" for the ENTIRE CI-monitor phase, including long after GitHub
# reports every check green. Do not read outcome=passed itself as proof the PR
# was merged: the 2026-09-20 firstmate-lint-debt-blocking-prs incident
# observed outcome=passed reported alongside pr_state=open, so a passed
# terminal outcome can still be holding for the captain's merge word
# (nm_outcome_pr_detail is the one owner of deriving an honest reading from
# the run's own pr_state field for that case). `axi status`'s steps[] table
# never distinguishes "still waiting on checks" from "checks green, waiting on
# merge": both read as plain `ci,running,...`. The only place that transition is
# recorded is the ci step's own log text, e.g. "all CI checks passed - still
Expand Down Expand Up @@ -676,10 +709,20 @@ if [ "$HAVE_RUN" = 1 ]; then
has_gate=0
nm_has_gate && has_gate=1

# 2026-09-20 audit of every arm below: checks-passed's "PR ready for
# review" and the failed/cancelled arms never assert a landing, so they
# stay as they were; only passed and passed-with-override previously
# overclaimed a merge from the outcome name alone. The coarse
# ledger-fallback case above and the no-outcome status fallback below
# were audited the same way and likewise left unchanged: "run
# completed"/"ci running"/"validating (...)" assert nothing about a merge.
if [ -n "$outcome" ]; then
case "$outcome" in
passed) RUN_STATE="done"; RUN_DETAIL="run passed: PR merged/closed" ;;
passed) RUN_STATE="done"; RUN_DETAIL="run passed: $(nm_outcome_pr_detail)" ;;
checks-passed) RUN_STATE="done"; RUN_DETAIL="checks green: PR ready for review" ;;
passed-with-override)
RUN_STATE="done"
RUN_DETAIL="run passed (approved past a waived check): $(nm_outcome_pr_detail)" ;;
failed)
if nm_reclassify_failed_run_as_held_green; then :; else
RUN_STATE=failed; RUN_DETAIL="run failed"
Expand Down
118 changes: 118 additions & 0 deletions tests/fm-crew-state.test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -378,6 +378,54 @@ outcome: passed
EOF
}

# The 2026-09-20 firstmate-lint-debt-blocking-prs incident shape: outcome
# passed with pr_state open, still holding for the captain's merge word.
run_passed_pr_open() { # <branch>
cat <<EOF
run:
id: "01RUN"
branch: $1
status: completed
head: "${FM_FAKE_RUN_HEAD:-abc1234}"
pr: "https://github.com/o/r/pull/1"
pr_state: open
findings: none
outcome: passed
EOF
}

# The 2026-09-20 firstmate-detect-dropped-ci-event incident shape: a real
# terminal outcome the reader had no arm for.
run_passed_with_override() { # <branch>
cat <<EOF
run:
id: "01RUN"
branch: $1
status: completed
head: "${FM_FAKE_RUN_HEAD:-abc1234}"
pr: "https://github.com/o/r/pull/1"
pr_state: open
findings: none
outcome: passed-with-override
EOF
}

# A run that finished without ever opening a PR: the record states so, which
# is a known fact and not the absent-field unknown.
run_passed_pr_none() { # <branch>
cat <<EOF
run:
id: "01RUN"
branch: $1
status: completed
head: "${FM_FAKE_RUN_HEAD:-abc1234}"
pr: ""
pr_state: none
findings: none
outcome: passed
EOF
}

run_failed() { # <branch>
cat <<EOF
run:
Expand Down Expand Up @@ -955,6 +1003,72 @@ test_terminal_passed() {
pass "terminal passed run is authoritative"
}

# Pins the 2026-09-20 firstmate-lint-debt-blocking-prs fix: outcome=passed
# must not assert a merge that pr_state disproves.
test_terminal_passed_open_pr_reads_honest_detail() {
reset_fakes
local d; d=$(new_case passed-open)
make_repo_on_branch "$d/wt" fm/feat-passed-open
make_fakebin "$d" >/dev/null
fm_write_meta "$d/state/feat-passed-open.meta" "window=fm:fm-feat-passed-open" "worktree=$d/wt" "kind=ship"
FM_FAKE_AXI_STATUS="$(run_passed_pr_open fm/feat-passed-open)"
local out; out=$(run_crew_state "$d" feat-passed-open)
assert_contains "$out" "state: done" "passed run with an open PR still reads done"
assert_contains "$out" "PR open, not yet merged" "an open PR's detail must not claim it merged"
assert_not_contains "$out" "PR merged" "an open PR must never be reported as merged"
pass "terminal passed run with an open PR reads its honest pr_state, not a guessed merge"
}

# Pins the 2026-09-20 firstmate-detect-dropped-ci-event fix: an unmapped
# terminal outcome must not degrade to unknown when the run plainly finished.
test_terminal_passed_with_override_reads_done_not_unknown() {
reset_fakes
local d; d=$(new_case passed-override)
make_repo_on_branch "$d/wt" fm/feat-passed-override
make_fakebin "$d" >/dev/null
fm_write_meta "$d/state/feat-passed-override.meta" "window=fm:fm-feat-passed-override" "worktree=$d/wt" "kind=ship"
FM_FAKE_AXI_STATUS="$(run_passed_with_override fm/feat-passed-override)"
local out; out=$(run_crew_state "$d" feat-passed-override)
assert_contains "$out" "state: done" "passed-with-override is a real terminal outcome, not unknown"
assert_not_contains "$out" "state: unknown" "a finished run must never read as unknown"
assert_contains "$out" "PR open, not yet merged" "passed-with-override still reads its honest pr_state"
pass "terminal passed-with-override run is classified done, never unknown"
}

# A run record without the pr_state field proves nothing about the PR either
# way; the detail must say so rather than assert a landing or its absence.
test_terminal_passed_absent_pr_state_reads_unknown() {
reset_fakes
local d; d=$(new_case passed-no-pr-state)
make_repo_on_branch "$d/wt" fm/feat-passed-no-pr-state
make_fakebin "$d" >/dev/null
fm_write_meta "$d/state/feat-passed-no-pr-state.meta" "window=fm:fm-feat-passed-no-pr-state" "worktree=$d/wt" "kind=ship"
FM_FAKE_AXI_STATUS="$(run_passed fm/feat-passed-no-pr-state)"
local out; out=$(run_crew_state "$d" feat-passed-no-pr-state)
assert_contains "$out" "state: done" "a passed run without pr_state still reads done"
assert_contains "$out" "PR merge state unknown" "an absent pr_state must read unknown"
assert_not_contains "$out" "no PR opened" "an absent pr_state must not claim no PR was opened"
assert_not_contains "$out" "PR merged" "an absent pr_state must not claim a merge"
pass "terminal passed run without pr_state reports the merge state as unknown"
}

# pr_state: none is the record stating no PR was ever opened - a known fact
# that must stay distinct from the absent-field unknown default.
test_terminal_passed_pr_state_none_reads_no_pr_opened() {
reset_fakes
local d; d=$(new_case passed-pr-none)
make_repo_on_branch "$d/wt" fm/feat-passed-pr-none
make_fakebin "$d" >/dev/null
fm_write_meta "$d/state/feat-passed-pr-none.meta" "window=fm:fm-feat-passed-pr-none" "worktree=$d/wt" "kind=ship"
FM_FAKE_AXI_STATUS="$(run_passed_pr_none fm/feat-passed-pr-none)"
local out; out=$(run_crew_state "$d" feat-passed-pr-none)
assert_contains "$out" "state: done" "a passed run that opened no PR still reads done"
assert_contains "$out" "no PR opened" "pr_state none is a known fact, not an unknown"
assert_not_contains "$out" "merge state unknown" "a stated none must not read as the unknown default"
assert_not_contains "$out" "PR merged" "a run that opened no PR must never report a merge"
pass "terminal passed run with pr_state none reports no PR opened, not unknown"
}

test_terminal_failed() {
reset_fakes
local d; d=$(new_case failed)
Expand Down Expand Up @@ -2684,6 +2798,10 @@ test_ci_fixing_after_green_stays_working
test_top_level_fixing_ci_running_after_green_stays_working
test_top_level_fixing_done_log_stays_working
test_terminal_passed
test_terminal_passed_open_pr_reads_honest_detail
test_terminal_passed_with_override_reads_done_not_unknown
test_terminal_passed_absent_pr_state_reads_unknown
test_terminal_passed_pr_state_none_reads_no_pr_opened
test_terminal_failed
test_terminal_failed_ci_orphan_after_green_reads_done
test_terminal_failed_ci_orphan_status_only_reads_done
Expand Down
Loading