diff --git a/AGENTS.md b/AGENTS.md index ad401cd162c..216acc19ea0 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -539,17 +539,18 @@ Firstmate's wrapper stays narrow: `ask-user` findings return through `needs-deci That checks-green status is owed at the CI-ready return point, when `/no-mistakes` first reports CI green, not after the monitor-until-merge loop observes the PR merged or closed. Use chat for yes/no decisions; use the durable `lavish-axi` decision flow when there are multiple findings or options to triage. -Judge a validating crewmate by the run's step status, never by whether its shell is still running. -Read its current state with `bin/fm-crew-state.sh `: a deterministic, token-tight one-line read that takes the matching no-mistakes run-step as the source of truth and reconciles it against the crewmate's `state/.status` log. -Because the run-step is authoritative before pane liveness, a crewmate whose window closed after or during validation can still report `done` or `working` from its run; a missing pane becomes `unknown` only when no matching run exists. +Judge a validating crewmate with `bin/fm-crew-state.sh`, never by whether its shell is still running. +Read its current state with `bin/fm-crew-state.sh `: a deterministic, token-tight one-line read that ordinarily takes the matching no-mistakes run-step as the source of truth and reconciles it against the crewmate's `state/.status` log. +Because the run-step is ordinarily authoritative before pane liveness, a crewmate whose window closed after or during validation can still report `done` or `working` from its run; the narrow orphaned-CI-record exception and its safety boundary are owned by `docs/architecture.md` and the helper's header. That log is an append-only wake-*event* log, not a current-state field, and it goes stale the moment a resolved gate lets the run resume: after you answer a `needs-decision`/`blocked` and the crewmate silently resumes (responds to the gate, the pipeline fixes, it re-validates), the log's last line still reads `needs-decision`/`blocked` while the run-step has moved on. So never infer current state from a `tail` of that log; `bin/fm-crew-state.sh` reports the live run-step state and explicitly flags the stale log line superseded, where a raw `tail` would mislead you into re-escalating settled work. The fields below name the run-step states and outcomes it reads from `no-mistakes axi status`; run that command directly when you want the full gate findings. During the `ci` monitor phase, `bin/fm-crew-state.sh` 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`. +Its missing-worktree observation is only heuristic corroboration for that narrow orphaned-record exception, never a death verdict or authority to abort a run. A step that `axi status` renders as `quiet` is NOT evidence that anything died: a step running a configured `commands.*` shell command reports no pid, no round, and no log for its whole duration, which for this repo's test step is routinely hours. `bin/fm-crew-state.sh` appends a real liveness verdict there from `bin/fm-nm-step-liveness.sh`, which finds the step's own processes by working directory; the supervision boundary absorbs `alive` but surfaces `dead` and `unknown`, and a run must never be aborted without a `dead` verdict (`docs/postmortems/nm-quiet-test-step.md`). -- `running`/`fixing`/`ci` - the pipeline is working (a fix round, a test, or CI monitoring); `ci` stays working until the ci log's most recent recognized marker says checks passed or no checks are terminally ready, and a later re-arm or issue marker returns it to working. +- `running`/`fixing`/`ci` - the pipeline is ordinarily working (a fix round, a test, or CI monitoring); `ci` stays working until the ci log's most recent recognized marker says checks passed or no checks are terminally ready, and a later re-arm or issue marker returns it to working, subject to the orphaned-record exception above. - `awaiting_approval`/`fix_review` - the run is parked waiting on the agent, surfaced as a top-level `awaiting_agent: parked ` line right after `status:` in `axi status`. Section 8's in-flight validation-custody boundary and `crew-steering` own the correction when the crewmate has stepped away. - `outcome: passed` or `checks-passed` - an open PR reports `done` only when the remote-currentness contract owned by `bin/fm-crew-state.sh`'s header succeeds; `unknown` or `stale` always means `do not merge`. @@ -667,7 +668,7 @@ On wake, in order of cheapness: 1. Read the reason line and drain queued wake records with `bin/fm-wake-drain.sh`. 2. `signal:` read the listed status files first; a wake lists every signal that landed within the coalescing grace window (e.g. a status write plus the same turn's turn-end marker), and each is ~30 tokens and usually sufficient. - A status line is the wake *event*, not the crewmate's current state; when you need the live state - especially to confirm a `needs-decision`/`blocked`/`paused` status is still real and not already resolved-and-resumed - read it with `bin/fm-crew-state.sh `, which reconciles the authoritative run-step over the possibly-stale log line, and never `tail` the status log as the current-state source. + A status line is the wake *event*, not the crewmate's current state; when you need the live state - especially to confirm a `needs-decision`/`blocked`/`paused` status is still real and not already resolved-and-resumed - read it with `bin/fm-crew-state.sh `, which reconciles the ordinarily authoritative run-step against the possibly-stale log line, and never `tail` the status log as the current-state source. 3. `stale:` the crewmate stopped without reporting, a recognized mid-run permission prompt is waiting, or a busy pane exceeded the possible system-dialog no-progress threshold. If the reason includes `permission-prompt detected` or `permission/system-dialog suspected`, load `stuck-crewmate-recovery` before taking any ordinary recovery action and follow its permission-blocked branch. Otherwise peek the pane (`bin/fm-peek.sh `) to diagnose. diff --git a/bin/fm-crew-state.sh b/bin/fm-crew-state.sh index 45e27c50dde..c0c7234dcb4 100755 --- a/bin/fm-crew-state.sh +++ b/bin/fm-crew-state.sh @@ -35,6 +35,9 @@ # on checks" from "checks green, waiting on merge" (see nm_ci_checks_state), # so a ci-step log-tail marker supplies the ready claim before the same # remote-currentness checks decide whether it is done. +# A narrow orphan-record fallback reports a declared pause only when a +# checks-green report immediately precedes it, the ci log has no newer state, +# and the liveness probe specifically cannot find that run's worktree. # 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 @@ -433,6 +436,22 @@ log_reports_ci_ready() { esac } +# True only for the exact CI-ready-to-declared-hold transition. Requiring the +# checks-green report to be the immediately preceding non-empty event prevents an +# old ready line from surviving a later working/fixing transition and disguising +# genuinely resumed work as paused. +log_reports_ci_ready_immediately_before_pause() { + local previous + status_is_paused "$LOG_LINE" || return 1 + [ -f "$LOG" ] || return 1 + previous=$(grep -v '^[[:space:]]*$' "$LOG" 2>/dev/null | tail -2 | head -1) + [ "$(status_line_verb "$previous")" = "done" ] || return 1 + case "$(status_line_note "$previous")" in + *PR*"checks green"*|*"checks green"*PR*) return 0 ;; + *) return 1 ;; + esac +} + nm_ci_step_status() { local row rest row=$(printf '%s\n' "$RUN_OUT" | grep -E '^[[:space:]]*ci,[[:space:]]*"?(running|fixing)"?[[:space:]]*,' | head -1) @@ -654,6 +673,19 @@ nm_step_liveness() { printf '%s' "$line" } +# A missing run worktree is an unreadable liveness observation, never a death +# verdict and never permission to abort a run. This predicate uses that exact +# observation only as corroboration for the independent CI-ready report and current +# declared pause checked by the caller. +nm_ci_record_has_no_worktree() { + local observation + observation=$(nm_step_liveness) + case "$observation" in + unknown\ \(grade:\ unreadable\;\ 0\ procs\;\ *no\ worktree\ for\ this\ run*) return 0 ;; + *) return 1 ;; + esac +} + nm_runs_status_for_branch() { # local branch=$1 out row st rest br head pr field out=$(nm_run runs --limit "$FM_CREW_STATE_RUNS_LIMIT") @@ -824,6 +856,20 @@ if [ "$HAVE_RUN" = 1 ]; then fi fi + # An orphaned ci row is not positive working evidence forever. This fallback is + # intentionally conjunctive: the durable ready report must immediately precede + # the current declared pause, the ci log must be unreadable rather than showing a + # re-arm/fix, and the bounded liveness probe must specifically find no worktree + # for this run. The unreadable probe is corroboration only. On its own, or beside + # any positive working evidence, ordinary run-step precedence remains unchanged. + if [ "$RUN_STATE" = working ] && [ "$RUN_SOURCE" = full ] \ + && [ "$CI_STEP_STATUS" = running ] && [ "$CI_LOG_STATE" = unknown ] \ + && log_reports_ci_ready_immediately_before_pause \ + && nm_ci_record_has_no_worktree; then + emit paused status-log \ + "$(status_line_note "$LOG_LINE")${SEP}reported checks green immediately before pause${SEP}orphaned ci run record has no worktree" + fi + if [ "$RUN_STATE" = "done" ] && [ "$READY_CLAIM" = 1 ]; then if [ "$RUN_SOURCE" = full ] && [ "$RUN_STATUS" = completed ]; then verify_no_newer_active_run_or_emit "$CREW_BRANCH" @@ -836,9 +882,10 @@ if [ "$HAVE_RUN" = 1 ]; then # leaving it to a hand check that gets it wrong. The verdict is APPENDED as an # observation and never overrides RUN_STATE: a step momentarily between # processes would otherwise be misreported as dead, which is the very failure - # mode this exists to end. The ci step is excluded because its monitoring runs - # inside the daemon with no worktree process at all, so `dead` there is - # meaningless rather than informative. + # mode this exists to end. The ordinary ci path is excluded because its monitoring + # runs inside the daemon with no worktree process at all, so `dead` there is + # meaningless rather than informative. The narrow orphan-record check above uses + # only the distinct missing-worktree observation and never treats it as dead. if [ "$RUN_STATE" = working ] && [ "$RUN_SOURCE" = full ] && nm_step_is_quiet; then ACTIVE_STEP=$(nm_active_step_name) case "$ACTIVE_STEP" in diff --git a/docs/architecture.md b/docs/architecture.md index 56f01bd3967..67ed703861e 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -18,7 +18,7 @@ No-verb wakes, such as `working:` notes and bare turn-ended signals, are benign A crewmate that declares `paused:` for a known external wait is separately absorbed while idle and re-surfaced only on the longer pause cadence, rather than being treated as a possible wedge. A pause is a statement about the work rather than about the terminal, so wake classification honours it whether the crewmate's pane is alive, idle, or gone, and whatever its attributed no-mistakes run reports - parked, failed or cancelled, or unreadable. That classification never transfers custody of an in-flight validation run: `AGENTS.md` section 8 owns the boundary that requires the supervisor to steer a paused lane back to attending and driving its run. -The single exception is an actively `working` run-step or busy pane, which supersedes the declaration because the crewmate resumed after making it. +Positive evidence from an actively `working` run-step or busy pane supersedes the declaration because the crewmate resumed after making it, subject only to the narrow orphaned-CI-record discriminator described below. Absorption is gated on two proofs taken from one immutable read of the crewmate's current durable status stream: the pause verb carrying no failure vocabulary in its headline, and an empty keyed open/resolved fold, so a pause can never mask a still-unanswered decision. The Herdr native blocked-transition edge does not yet honour this invariant, which is a known defect tracked as `herdr-push-transition-pause-gate-h8`: on that edge a lane that owes an unanswered keyed decision can be silently absorbed and go quiet. A crewmate with no locatable status stream is refused rather than absorbed, and stopped crewmates without a declared pause surface immediately. @@ -32,11 +32,15 @@ At each drain boundary, `fm-wake-drain.sh` first intakes durable Lavish answers, Routine watcher polling, supervision no-ops, elapsed waiting time, and absorbed benign wakes stay silent. A declared external wait trades that silence for one bounded recheck per pause window, so a forgotten pause cannot remain invisible indefinitely. Crewmate status files are append-only wake-event logs, not current-state fields. -`bin/fm-crew-state.sh ` is the cheap current-state read for an actionable heartbeat review: it attributes the matching no-mistakes run, active or terminal, to the crewmate's own branch and keeps that run-step authoritative even if the pane has closed; the pause precedence above belongs to the watcher's absorb classification and does not change this general current-state precedence. +`bin/fm-crew-state.sh ` is the cheap current-state read for an actionable heartbeat review: it attributes the matching no-mistakes run, active or terminal, to the crewmate's own branch and ordinarily keeps that run-step authoritative even if the pane has closed. When a terminal run reports `outcome: passed`, the helper verifies the PR detail through a bounded `gh-axi` query instead of inferring GitHub state from the pipeline outcome. An open-PR `passed` or `checks-passed` outcome and a checks-green CI marker are classified by the exact remote-only currentness contract in `bin/fm-crew-state.sh`'s header; only `done` authorizes the PR-ready workflow. 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 supplies the ready claim while a later re-arm, failed-check, or issue marker returns the crewmate to working before remote currentness is considered. +A narrow exception exposes the current declared pause instead of a lingering `ci,running` row only when a checks-green `done` event is the immediately preceding non-empty status, the ci log is unknown with no newer re-arm or fixing marker, and the bounded liveness observation specifically says the recorded run has no worktree. +The missing-worktree observation is heuristic corroboration for the adjacent durable events, not a death verdict or authority to abort the run; if an active CI run becomes undiscoverable while those events remain and its ci log is unavailable, the heuristic can misclassify it as paused. +Positive authoritative work remains on the wedge path, as covered by `test_ci_pause_does_not_hide_a_real_active_run` and `test_paused_authoritative_working_preserves_wedge_timer`. +This alarm suppression does not repair the no-mistakes lifecycle: the run still lacks a clean terminal state at the CI-ready point and can keep monitoring a held PR after its base moves. 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. diff --git a/tests/fm-backend-tmux-smoke.test.sh b/tests/fm-backend-tmux-smoke.test.sh index e417fe01e44..ec31d0e77bf 100755 --- a/tests/fm-backend-tmux-smoke.test.sh +++ b/tests/fm-backend-tmux-smoke.test.sh @@ -6,8 +6,8 @@ # "run a real tmux smoke test (create session, send text + Enter, capture, # list, kill)" from data/fm-backend-design-d7/report.md. Every other suite in # this repo fakes tmux; this one is the one place that talks to a REAL tmux -# server, isolated on a short repo-local socket (`-S`) so it never touches the host's -# actual sessions. +# server, isolated on a short private socket (`-S`) so it never touches the +# host's actual sessions. set -u ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)" @@ -18,16 +18,26 @@ pass() { printf 'ok - %s\n' "$1"; } command -v tmux >/dev/null 2>&1 || { echo "skip: tmux not found"; exit 0; } REAL_TMUX=$(command -v tmux) -SOCKET="./.fm-backend-smoke-$$.sock" +SOCKET= +SOCKET_DIR= SHIM_DIR= trap cleanup_all EXIT cleanup_all() { - "$REAL_TMUX" -S "$SOCKET" kill-server >/dev/null 2>&1 || true - rm -f -- "$SOCKET" + if [ -n "${SOCKET:-}" ]; then + "$REAL_TMUX" -S "$SOCKET" kill-server >/dev/null 2>&1 || true + rm -f -- "$SOCKET" + fi [ -n "${SHIM_DIR:-}" ] && rm -rf "$SHIM_DIR" + [ -n "${SOCKET_DIR:-}" ] && rm -rf "$SOCKET_DIR" } +# tmux limits Unix socket paths to roughly 104 bytes on macOS. Keep the real +# smoke server under a bounded root instead of inheriting a long checkout path. +SOCKET_DIR=$(mktemp -d /tmp/fm-tmux-smoke.XXXXXX) \ + || fail "could not create a private tmux socket directory" +SOCKET="$SOCKET_DIR/fm-backend-smoke.sock" + # A `tmux` shim on PATH that transparently redirects every call to the private # socket, so bin/backends/tmux.sh's bare `tmux ...` invocations never touch the # host's real sessions. diff --git a/tests/fm-crew-state.test.sh b/tests/fm-crew-state.test.sh index 0398da413ef..570bbacfa78 100755 --- a/tests/fm-crew-state.test.sh +++ b/tests/fm-crew-state.test.sh @@ -23,9 +23,10 @@ # (i) kind=scout skips the run lookup -> pane/status-log # (j) torn-down worktree / missing meta -> unknown/none # (l) a quiet active step carries a real liveness verdict (dead / alive / -# unknown) from -# bin/fm-nm-step-liveness.sh, and the ci step is exempted because it owns -# no worktree process - the 2026-08-02 false-dead regression pair +# unknown) from bin/fm-nm-step-liveness.sh. The ordinary ci path is exempt +# because it owns no worktree process, while the green-then-paused orphan +# case uses only the probe's distinct missing-worktree observation as +# corroboration - the 2026-08-02 false-dead regression pair. # (k) crew_is_provably_working end-to-end over the REAL helper (not a canned # fake fm-crew-state.sh verdict): cross-branch attribution via the runs # list -> absorbed; genuinely no run anywhere + idle pane -> surfaced. @@ -206,6 +207,7 @@ case "${FM_FAKE_LIVENESS_MODE:-}" in dead-nonzero) printf 'liveness: dead · run: 01RUN · procs: 2\n' ;; empty-procs) printf 'liveness: unknown · run: 01RUN · procs: · grade: unreadable · missing count · doing: bash t.sh (1:00)\n' ;; argv-fields) printf 'liveness: unknown · run: 01RUN · procs: 3 · grade: present-unproven · presence established · doing: python -c "procs: x grade: bogus" (1:00)\n' ;; + no-worktree) printf 'liveness: unknown · run: 01RUN · procs: 0 · grade: unreadable · no worktree for this run under /tmp/no-mistakes/worktrees\n' ;; nonzero) exit 9 ;; timeout) sleep 30 ;; *) printf 'liveness: alive · run: 01RUN · procs: 1 · processes present\n' ;; @@ -737,6 +739,66 @@ test_ci_ready_done_log_beats_monitoring_run() { pass "ci-ready status log beats monitoring run" } +# A green PR can be deliberately held after the crewmate has returned from the +# CI-ready point. If the old ci monitor has also lost its worktree, its orphaned +# `running` row is corroborating record drift rather than positive working +# evidence. The helper must expose the current declared pause instead of letting +# that row outrank the pause forever. +test_ci_ready_pause_beats_orphaned_monitor_record() { + reset_fakes + local d out + d=$(new_case ci-ready-paused-orphan) + make_repo_on_branch "$d/wt" fm/feat-ci-ready-paused-orphan + make_fakebin "$d" >/dev/null + fm_write_meta "$d/state/feat-ci-ready-paused-orphan.meta" \ + "window=fm:fm-feat-ci-ready-paused-orphan" "worktree=$d/wt" "kind=ship" + cat > "$d/state/feat-ci-ready-paused-orphan.status" <<'EOF' +done: PR https://github.com/o/r/pull/2 checks green +paused: PR 2 is green and deliberately held pending a decision +EOF + FM_FAKE_AXI_STATUS="$(run_ci_monitoring fm/feat-ci-ready-paused-orphan)" + FM_FAKE_CI_LOGS="" + out=$(FM_CREW_STATE_NM_LIVENESS_BIN="$d/fakebin/fake-liveness" \ + FM_FAKE_LIVENESS_MODE=no-worktree run_crew_state "$d" feat-ci-ready-paused-orphan) + assert_contains "$out" "state: paused" "green held PR with an orphaned ci record -> paused" + assert_contains "$out" "source: status-log" "the declared hold remains the current-state source" + assert_contains "$out" "orphaned ci run record has no worktree" \ + "the paused verdict names the corroborating orphaned record" + assert_not_contains "$out" "state: working" "the orphaned ci record must not claim positive work" + pass "a green held PR is paused when its lingering ci record has no worktree" +} + +# The orphan discriminator is deliberately conjunctive. No ready report means +# there is no proof that the work reached the CI-ready boundary, and a positive +# process reading means the run really is active. Either one preserves working +# precedence so the watcher's ordinary wedge timer still applies. +test_ci_pause_does_not_hide_a_real_active_run() { + reset_fakes + local d out + d=$(new_case ci-paused-real-work) + make_repo_on_branch "$d/wt" fm/feat-ci-paused-real-work + make_fakebin "$d" >/dev/null + fm_write_meta "$d/state/feat-ci-paused-real-work.meta" \ + "window=fm:fm-feat-ci-paused-real-work" "worktree=$d/wt" "kind=ship" + printf 'paused: stale declaration while validation is still running\n' \ + > "$d/state/feat-ci-paused-real-work.status" + FM_FAKE_AXI_STATUS="$(run_ci_monitoring fm/feat-ci-paused-real-work)" + FM_FAKE_CI_LOGS="" + out=$(FM_CREW_STATE_NM_LIVENESS_BIN="$d/fakebin/fake-liveness" \ + FM_FAKE_LIVENESS_MODE=no-worktree run_crew_state "$d" feat-ci-paused-real-work) + assert_contains "$out" "state: working" "no ready report keeps ci running authoritative" + + cat > "$d/state/feat-ci-paused-real-work.status" <<'EOF' +done: PR https://github.com/o/r/pull/2 checks green +paused: stale declaration while validation is active again +EOF + out=$(FM_CREW_STATE_NM_LIVENESS_BIN="$d/fakebin/fake-liveness" \ + FM_FAKE_LIVENESS_MODE=alive run_crew_state "$d" feat-ci-paused-real-work) + assert_contains "$out" "state: working" "a live run still outranks an old ready report and pause" + assert_not_contains "$out" "state: paused" "positive run liveness must not be silenced by a pause" + pass "a stale pause still leaves a genuinely active ci run on the wedge path" +} + # A status-log URL is an event-log detail, not evidence that it belongs to the # current no-mistakes run. Without a PR URL on the exact run object, currentness # is unknown even when the log claims checks are green. @@ -1746,6 +1808,8 @@ test_genuine_parked_not_superseded test_scalar_gate_parked_not_superseded test_gate_block_parked_not_superseded test_ci_ready_done_log_beats_monitoring_run +test_ci_ready_pause_beats_orphaned_monitor_record +test_ci_pause_does_not_hide_a_real_active_run test_ci_ready_log_pr_url_does_not_supply_run_identity test_ci_monitoring_checks_green_surfaces_done test_top_level_ci_checks_green_surfaces_done