diff --git a/AGENTS.md b/AGENTS.md index 063207215a6..3e08760d2a1 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -298,7 +298,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 current-code-matched 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. +Its header owns the exact run-step-to-state mapping, including when a declared `paused:` outranks a terminal run outcome; on a parked approval or fix-review state, require the worker to follow the active gate help. A worker hand-editing, committing, aborting, or restarting during an active validation run duplicates pipeline ownership; 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. diff --git a/bin/fm-crew-state.sh b/bin/fm-crew-state.sh index 32dff236687..328c82411de 100755 --- a/bin/fm-crew-state.sh +++ b/bin/fm-crew-state.sh @@ -34,7 +34,20 @@ # 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. +# green, so a green PR is never silently read as still-validating. ALSO: +# when the run reached a TRUE terminal outcome of done or cancelled and the +# status log's last verb is a declared paused:, emit paused with source +# status-log and keep the terminal run outcome as the LEADING detail - a +# historical record must not overrule a self-declared wait. That override +# is keyed on the run's own terminal outcome, never on the mapped state, so +# the ci-green case above (mapped done while the run is still monitoring) +# stays an active run. Active and parked runs still outrank a pause line, +# and a terminal FAILED run always surfaces as failed: neither the status +# log's lines nor `axi status` carries a timestamp, so there is no way to +# prove the pause is fresher than the failure, and a pause appended before +# or during a run that then failed would otherwise mask real work failure. +# Cancelled is a supervisor abort rather than a work failure, so it stays +# overridable. # 3. Reconcile the status log: if its last line says needs-decision/blocked but # the run-step shows the run moved on, the log is deterministically stale and # is flagged superseded. A genuinely parked run plus a needs-decision log @@ -483,6 +496,11 @@ fi if [ "$HAVE_RUN" = 1 ]; then RUN_STATE=working RUN_DETAIL="" + # The run's own TERMINAL outcome, independent of the mapped RUN_STATE: + # none (still active, whatever the mapping says), done, cancelled, or failed. + # Only the run's own reported outcome/status sets this, so a mapping override + # (ci-green -> done while the run keeps monitoring) never reads as terminal. + RUN_TERMINAL=none CI_STEP_STATUS="" CI_LOG_STATE="" RUN_STATUS="" @@ -496,9 +514,9 @@ if [ "$HAVE_RUN" = 1 ]; then # coarse-vs-full distinction, so a real gate is never silently missed. case "$COARSE_STATUS" in running) RUN_STATE=working; RUN_DETAIL="validating (background run)" ;; - completed) RUN_STATE="done"; RUN_DETAIL="run completed" ;; - failed) RUN_STATE=failed; RUN_DETAIL="run failed" ;; - cancelled) RUN_STATE=failed; RUN_DETAIL="run cancelled" ;; + completed) RUN_STATE="done"; RUN_DETAIL="run completed"; RUN_TERMINAL="done" ;; + failed) RUN_STATE=failed; RUN_DETAIL="run failed"; RUN_TERMINAL=failed ;; + cancelled) RUN_STATE=failed; RUN_DETAIL="run cancelled"; RUN_TERMINAL=cancelled ;; *) RUN_STATE=unknown; RUN_DETAIL="runs list status: $COARSE_STATUS" ;; esac else @@ -512,10 +530,10 @@ if [ "$HAVE_RUN" = 1 ]; then if [ -n "$outcome" ]; then case "$outcome" in - passed) RUN_STATE="done"; RUN_DETAIL="run passed: PR merged/closed" ;; - checks-passed) RUN_STATE="done"; RUN_DETAIL="checks green: PR ready for review" ;; - failed) RUN_STATE=failed; RUN_DETAIL="run failed" ;; - cancelled) RUN_STATE=failed; RUN_DETAIL="run cancelled" ;; + passed) RUN_STATE="done"; RUN_DETAIL="run passed: PR merged/closed"; RUN_TERMINAL="done" ;; + checks-passed) RUN_STATE="done"; RUN_DETAIL="checks green: PR ready for review"; RUN_TERMINAL="done" ;; + failed) RUN_STATE=failed; RUN_DETAIL="run failed"; RUN_TERMINAL=failed ;; + cancelled) RUN_STATE=failed; RUN_DETAIL="run cancelled"; RUN_TERMINAL=cancelled ;; *) RUN_STATE=unknown; RUN_DETAIL="outcome: $outcome" ;; esac elif [ -n "$awaiting" ] || [ "$status" = awaiting_approval ] || [ "$status" = fix_review ] || [ -n "$gate_status" ] || [ "$has_gate" = 1 ]; then @@ -537,9 +555,9 @@ if [ "$HAVE_RUN" = 1 ]; then case "$status" in ci) RUN_STATE=working; RUN_DETAIL="ci running" ;; running|fixing) RUN_STATE=working; RUN_DETAIL="validating ($status)" ;; - completed) RUN_STATE="done"; RUN_DETAIL="run completed" ;; - failed) RUN_STATE=failed; RUN_DETAIL="run failed" ;; - cancelled) RUN_STATE=failed; RUN_DETAIL="run cancelled" ;; + completed) RUN_STATE="done"; RUN_DETAIL="run completed"; RUN_TERMINAL="done" ;; + failed) RUN_STATE=failed; RUN_DETAIL="run failed"; RUN_TERMINAL=failed ;; + cancelled) RUN_STATE=failed; RUN_DETAIL="run cancelled"; RUN_TERMINAL=cancelled ;; "") RUN_STATE=working; RUN_DETAIL="run active" ;; *) RUN_STATE=working; RUN_DETAIL="run active ($status)" ;; esac @@ -593,6 +611,32 @@ if [ "$HAVE_RUN" = 1 ]; then ;; esac + # Declared pause outranks a run that reached a TRUE terminal outcome, and only + # done or cancelled. Keyed on RUN_TERMINAL, not on the mapped RUN_STATE, so an + # active run the ci-green check mapped to done is still an active run and wins. + # Active/parked runs win for the same reason: the crew is mid-validation, so a + # pause line is stale. A terminal FAILED run also wins, deliberately: no + # freshness signal exists to prove the pause came AFTER the failure (status-log + # lines carry no timestamp, and `axi status` reports no completion time), and a + # pause appended before or during the run would otherwise mask a real failure + # behind a benign external wait. Cancelled is a supervisor abort, not failed + # work, so it stays overridable. Mirror the CI-green status-log override shape: + # emit from status-log with the terminal run outcome LEADING the detail, so the + # fact that the run finished survives downstream truncation, and the pause + # reason follows. + case "$RUN_TERMINAL" in + done|cancelled) + if status_is_paused "$LOG_LINE"; then + pause_note=$(status_line_note "$LOG_LINE") + if [ -n "$pause_note" ]; then + emit paused status-log "$RUN_DETAIL${SEP}$pause_note" + else + emit paused status-log "$RUN_DETAIL" + fi + fi + ;; + esac + emit "$RUN_STATE" run-step "$RUN_DETAIL" fi diff --git a/docs/architecture.md b/docs/architecture.md index d1bbb1c1ff5..588da45586a 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -32,9 +32,12 @@ Crew status files are append-only wake-event logs, not current-state fields. The script header owns the exact run-head ancestry rules. During no-mistakes' `ci` monitor phase, it also reads the ci step log tail because `axi status` reports both "still waiting on checks" and "checks green, waiting on merge" as `ci,running`. The most recent recognized ci log marker wins, so checks-green monitoring reports done while a later re-arm, failed-check, or issue marker returns the crew to working. +A second exception covers a run that already reached a terminal outcome: when that outcome is done or cancelled and the status log's last verb is a declared external wait, the crew reports `paused` from the status log with the terminal run outcome leading the detail, because a finished run record must not permanently overrule a self-declared wait. +That exception is keyed on the run's own terminal outcome rather than the mapped state, so an actively monitored checks-green run still reports done, and a terminal failed run always surfaces as failed: neither the status log nor `axi status` timestamps its events, so a pause appended before or during the run cannot be distinguished from one declared after it, and masking a real work failure behind a benign wait is the worse error. +Cancelled is a supervisor abort rather than failed work, so it stays overridable. Only when no matching run exists does it fall back to the pane busy-signature and then a status-log event whose verb maps to a recognized run-state; a dead pane without a run reports unknown instead of trusting a stale log. Decision-only events such as `resolved` never become current state or leak their prose into the current-state detail. -In that status-log fallback, a declared external wait reports the distinct `paused` state with its reason. +In that status-log fallback, a declared external wait reports the distinct `paused` state with its reason, as does the terminal done/cancelled exception above. For herdr, that pane fallback trusts a native `busy` verdict outright, but corroborates native `idle` or unknown verdicts against the recorded harness's rendered busy signature before deciding the crew is not working. For whole-fleet read-only review, `bin/fm-fleet-snapshot.sh --json` emits schema `fm-fleet-snapshot.v1` from the backlog, task metadata, current crew state, endpoint probes, PR/report pointers, scout reports, bounded current summaries from registered secondmate homes, and secondmate return-channel guidance. `bin/fm-fleet-view.sh` renders that snapshot as Markdown for humans, while `bin/fm-bearings-snapshot.sh` provides the bounded bearings projection, so both views consume one structured contract instead of reparsing raw fleet files. diff --git a/tests/fm-crew-state.test.sh b/tests/fm-crew-state.test.sh index bc0161d624f..07f151ad6c9 100755 --- a/tests/fm-crew-state.test.sh +++ b/tests/fm-crew-state.test.sh @@ -13,6 +13,9 @@ # (b) needs-decision/blocked log + resumed run = SUPERSEDED -> run-step # (c) genuine parked run + needs-decision log = NOT superseded -> run-step # (d) terminal run-step (passed/failed) is authoritative -> run-step +# (d') terminal done/cancelled run + trailing paused: -> paused (status-log), +# terminal detail leading; a terminal FAILED run, active/parked runs, and an +# actively-monitored ci-green run all still outrank a declared pause # (e) cross-branch attribution: this branch's own run found via list lookup # (f) no run + busy pane -> pane # (g) no run + idle pane falls to the status-log verb -> status-log @@ -283,6 +286,32 @@ outcome: failed EOF } +run_cancelled() { # + cat < + cat < cat </dev/null + fm_write_meta "$d/state/feat-cancel-pause.meta" "window=fm:fm-feat-cancel-pause" "worktree=$d/wt" "kind=ship" + printf 'paused: holding for upstream maintainer merge\n' > "$d/state/feat-cancel-pause.status" + FM_FAKE_AXI_STATUS="$(run_cancelled fm/feat-cancel-pause)" + FM_FAKE_BUSY=0 + local out; out=$(run_crew_state "$d" feat-cancel-pause) + assert_contains "$out" "state: paused" "cancelled + paused: -> paused" + assert_contains "$out" "source: status-log" "cancelled + paused: is status-log sourced" + assert_contains "$out" "holding for upstream maintainer merge" "pause reason is carried" + assert_contains "$out" "run cancelled" "terminal cancelled outcome survives as secondary detail" + assert_not_contains "$out" "state: failed" "cancelled must not remain primary state over paused:" + pass "terminal cancelled + trailing paused: reports paused with run cancelled detail" +} + +# A genuine run FAILURE is never absorbed by a trailing paused:. Nothing in the +# status log or `axi status` timestamps either event, so a pause appended before +# or during the run cannot be told from one declared after it - masking real work +# failure behind a benign external wait. Failure wins; cancelled (above) does not. +test_terminal_failed_plus_paused_surfaces_failure() { + reset_fakes + local d; d=$(new_case failed-paused) + make_repo_on_branch "$d/wt" fm/feat-fail-pause + make_fakebin "$d" >/dev/null + fm_write_meta "$d/state/feat-fail-pause.meta" "window=fm:fm-feat-fail-pause" "worktree=$d/wt" "kind=ship" + printf 'paused: waiting on external rate-limit reset\n' > "$d/state/feat-fail-pause.status" + FM_FAKE_AXI_STATUS="$(run_failed fm/feat-fail-pause)" + FM_FAKE_BUSY=0 + local out; out=$(run_crew_state "$d" feat-fail-pause) + assert_contains "$out" "state: failed" "failed + paused: keeps surfacing the failure" + assert_contains "$out" "source: run-step" "failed + paused: stays run-step sourced" + assert_contains "$out" "run failed" "failure detail is reported" + assert_not_contains "$out" "state: paused" "paused: must not mask a genuine run failure" + pass "terminal failed + trailing paused: still reports failed" +} + +# A bare paused: with no reason must not emit a doubled separator. +test_terminal_cancelled_plus_bare_paused() { + reset_fakes + local d; d=$(new_case cancelled-bare-paused) + make_repo_on_branch "$d/wt" fm/feat-bare-pause + make_fakebin "$d" >/dev/null + fm_write_meta "$d/state/feat-bare-pause.meta" "window=fm:fm-feat-bare-pause" "worktree=$d/wt" "kind=ship" + printf 'paused:\n' > "$d/state/feat-bare-pause.status" + FM_FAKE_AXI_STATUS="$(run_cancelled fm/feat-bare-pause)" + FM_FAKE_BUSY=0 + local out; out=$(run_crew_state "$d" feat-bare-pause) + assert_contains "$out" "state: paused" "cancelled + bare paused: -> paused" + assert_contains "$out" "run cancelled" "terminal cancelled outcome is the detail" + assert_not_contains "$out" " · · " "empty pause note must not double the separator" + pass "cancelled + bare paused: emits a single separator" +} + +# Regression for the ci-green mapping: an ACTIVE run whose ci log reads green is +# mapped to done while it keeps monitoring for merge/close. That is not a +# terminal outcome, so a trailing paused: must not take it over. +test_ci_green_monitoring_outranks_paused() { + reset_fakes + local d; d=$(new_case ci-green-outranks-pause) + make_repo_on_branch "$d/wt" fm/feat-cigreen-pause + make_fakebin "$d" >/dev/null + fm_write_meta "$d/state/feat-cigreen-pause.meta" "window=fm:fm-feat-cigreen-pause" "worktree=$d/wt" "kind=ship" + printf 'paused: awaiting maintainer merge\n' > "$d/state/feat-cigreen-pause.status" + FM_FAKE_AXI_STATUS="$(run_ci_monitoring fm/feat-cigreen-pause)" + FM_FAKE_CI_LOGS="all CI checks passed - still monitoring until merged or closed" + FM_FAKE_BUSY=0 + local out; out=$(run_crew_state "$d" feat-cigreen-pause) + assert_contains "$out" "state: done" "ci-green monitoring run outranks trailing paused:" + assert_contains "$out" "source: run-step" "ci-green monitoring stays run-step sourced" + assert_contains "$out" "still monitoring for merge/close" "ci-green detail is preserved" + assert_not_contains "$out" "state: paused" "paused: must not beat an actively monitored ci-green run" + pass "ci-green (still monitoring) run still outranks a declared pause" +} + +test_terminal_done_plus_paused() { + reset_fakes + local d; d=$(new_case done-paused) + make_repo_on_branch "$d/wt" fm/feat-done-pause + make_fakebin "$d" >/dev/null + fm_write_meta "$d/state/feat-done-pause.meta" "window=fm:fm-feat-done-pause" "worktree=$d/wt" "kind=ship" + printf 'paused: PR open and mergeable, awaiting maintainer merge\n' > "$d/state/feat-done-pause.status" + FM_FAKE_AXI_STATUS="$(run_checks_passed fm/feat-done-pause)" + FM_FAKE_BUSY=0 + local out; out=$(run_crew_state "$d" feat-done-pause) + assert_contains "$out" "state: paused" "done + paused: -> paused" + assert_contains "$out" "source: status-log" "done + paused: is status-log sourced" + assert_contains "$out" "PR open and mergeable, awaiting maintainer merge" "pause reason is carried" + assert_contains "$out" "checks green" "terminal done outcome survives as secondary detail" + assert_not_contains "$out" "state: done" "done must not remain primary state over paused:" + pass "terminal done + trailing paused: reports paused with terminal outcome detail" +} + +test_active_run_outranks_paused() { + reset_fakes + local d; d=$(new_case active-outranks-pause) + make_repo_on_branch "$d/wt" fm/feat-active-pause + make_fakebin "$d" >/dev/null + fm_write_meta "$d/state/feat-active-pause.meta" "window=fm:fm-feat-active-pause" "worktree=$d/wt" "kind=ship" + printf 'paused: holding for external wait\n' > "$d/state/feat-active-pause.status" + FM_FAKE_AXI_STATUS="$(run_running fm/feat-active-pause)" + FM_FAKE_BUSY=0 + local out; out=$(run_crew_state "$d" feat-active-pause) + assert_contains "$out" "state: working" "active run outranks trailing paused:" + assert_contains "$out" "source: run-step" "active run remains run-step sourced" + assert_not_contains "$out" "state: paused" "paused: must not beat an active run" + pass "active (working) run still outranks a declared pause" +} + +test_parked_run_outranks_paused() { + reset_fakes + local d; d=$(new_case parked-outranks-pause) + make_repo_on_branch "$d/wt" fm/feat-parked-pause + make_fakebin "$d" >/dev/null + fm_write_meta "$d/state/feat-parked-pause.meta" "window=fm:fm-feat-parked-pause" "worktree=$d/wt" "kind=ship" + printf 'paused: holding for external wait\n' > "$d/state/feat-parked-pause.status" + FM_FAKE_AXI_STATUS="$(run_parked fm/feat-parked-pause)" + FM_FAKE_BUSY=0 + local out; out=$(run_crew_state "$d" feat-parked-pause) + assert_contains "$out" "state: parked" "parked run outranks trailing paused:" + assert_contains "$out" "source: run-step" "parked run remains run-step sourced" + assert_not_contains "$out" "state: paused" "paused: must not beat a parked run" + pass "parked run still outranks a declared pause" +} + # (e) cross-branch attribution: `axi status` returns ANOTHER branch's run (the # routine case once more than one crew validates the same underlying repo # concurrently - they share ONE no-mistakes repo registration), so the helper @@ -1252,6 +1413,13 @@ test_top_level_fixing_ci_running_after_green_stays_working test_top_level_fixing_done_log_stays_working test_terminal_passed test_terminal_failed +test_terminal_cancelled_plus_paused +test_terminal_failed_plus_paused_surfaces_failure +test_terminal_cancelled_plus_bare_paused +test_terminal_done_plus_paused +test_active_run_outranks_paused +test_parked_run_outranks_paused +test_ci_green_monitoring_outranks_paused test_cross_branch_attribution_via_runs_list test_cross_branch_attribution_picks_most_recent_row test_coarse_run_does_not_probe_other_branch_ci_log_for_ready_status