From 843396e36a66b2b421d0658180e3b668d610a906 Mon Sep 17 00:00:00 2001 From: connectwithclayton-cpu <278079184+connectwithclayton@users.noreply.github.com> Date: Sun, 23 Aug 2026 20:55:27 -0400 Subject: [PATCH 1/7] fix(bin): distinguish validation handoffs from completion --- AGENTS.md | 2 +- bin/fm-brief.sh | 11 +++--- bin/fm-classify-lib.sh | 40 ++++++++++++++++------ bin/fm-crew-state.sh | 17 +++++++--- bin/fm-supervise-daemon.sh | 17 +++++----- bin/fm-watch.sh | 16 ++++----- docs/configuration.md | 2 +- tests/fm-brief.test.sh | 30 +++++++++++++--- tests/fm-crew-state.test.sh | 25 ++++++++++++-- tests/fm-daemon.test.sh | 6 ++-- tests/fm-watch-triage.test.sh | 64 +++++++++++++++++++++++------------ 11 files changed, 160 insertions(+), 70 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 8b5403096da..89ed596281c 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -338,7 +338,7 @@ After an autonomous merge, give the captain a one-line full-URL or local-main ou ### Validate -For a no-mistakes ship, trigger validation on the same worker after its implementation commit, using the harness invocation owned by `harness-adapters`. +For a no-mistakes ship, treat the generated brief's non-terminal `needs-validation:` handoff as the trigger to validate on the same worker, using the harness invocation owned by `harness-adapters`. The task worker that starts a no-mistakes run drives the pipeline and owns every `no-mistakes axi run` and `no-mistakes axi respond` call through the next gate or outcome. Firstmate never invokes `no-mistakes axi respond` for a crew-owned run. Once validation starts, prefer routing new requirements to follow-up work rather than expanding the current task, unless a new requirement completely invalidates the work being validated; however, the smallest downstream changes needed to keep already accepted product or engineering behavior correct, add behavioral tests where an executable contract exists, or keep documentation accurate remain within the current task even when they touch files not named at intake, and corrections required to satisfy already accepted intent are not new requirements. diff --git a/bin/fm-brief.sh b/bin/fm-brief.sh index 116cfd23e9a..20b53710e36 100755 --- a/bin/fm-brief.sh +++ b/bin/fm-brief.sh @@ -79,6 +79,7 @@ esac # shellcheck source=bin/fm-classify-lib.sh . "$SCRIPT_DIR/fm-classify-lib.sh" PAUSED_VERB=${FM_CLASSIFY_PAUSED_VERB:-$FM_CLASSIFY_PAUSED_VERB_DEFAULT} +NEEDS_VALIDATION_VERB=${FM_CLASSIFY_NEEDS_VALIDATION_VERB:-$FM_CLASSIFY_NEEDS_VALIDATION_VERB_DEFAULT} resolve_directory_input() { local name=$1 path=$2 resolved @@ -374,6 +375,7 @@ fi # delivery mode, validated above. The generated DOD opens with the fixed # "Delivery contract: mode=" line that bin/fm-spawn.sh checks against its own # explicit --mode before launching. +STATUS_STATES="working, needs-decision, blocked, $PAUSED_VERB, done, failed" case "$MODE" in direct-PR) SETUP2="" @@ -401,14 +403,15 @@ The configured merge authority approves the ready branch, then firstmate merges EOF ;; *) # no-mistakes + STATUS_STATES="working, $NEEDS_VALIDATION_VERB, needs-decision, blocked, $PAUSED_VERB, done, failed" SETUP2=" 2. Run \`no-mistakes doctor\`; if it reports the repo is not initialized here, run \`no-mistakes init\`." RULE1='1. Never push to the default branch. Never merge a PR.' IFS= read -r -d '' DOD <> $STATUS_FILE\` - States: working, needs-decision, blocked, $PAUSED_VERB, done, failed. + States: $STATUS_STATES. Each append wakes firstmate, so report sparingly: only phase changes a supervisor would act on (setup done, bug reproduced, fix implemented, validation passed) and the needs-decision/blocked/paused/done/failed states. No step-by-step FYI progress lines; firstmate reads your pane for that. A mid-task \`working:\` line (including setup complete) is nonterminal: do not end the - turn after it; continue the same stage until a defined \`done:\` gate under Definition of done. + turn after it; continue the same stage until a stop/report gate defined under Definition of done. Use \`$PAUSED_VERB: {why}\` - distinct from \`blocked:\` - ONLY when you are deliberately idling on a known external wait you expect to clear on its own (an upstream release, a rate-limit reset, a scheduled window): firstmate then leaves your idle pane alone and rechecks it on a long diff --git a/bin/fm-classify-lib.sh b/bin/fm-classify-lib.sh index 9915ece7d29..a48d0df8705 100755 --- a/bin/fm-classify-lib.sh +++ b/bin/fm-classify-lib.sh @@ -50,6 +50,11 @@ case $- in *u*) _fm_classify_nounset=on ;; *) _fm_classify_nounset=off ;; esac [ "$_fm_classify_nounset" = on ] || set +u unset _fm_classify_nounset +# The no-mistakes implementation-handoff verb. It is captain-relevant because +# firstmate must trigger validation, but deliberately non-terminal: only the +# later done: PR checks green event reports completed validation. +FM_CLASSIFY_NEEDS_VALIDATION_VERB_DEFAULT='needs-validation' + # Captain-relevant status verbs. A status line carrying any of these is work # firstmate must see. Lines without these verbs are no-verb signals: the watcher # absorbs them only with positive provably-working evidence, while the daemon uses @@ -61,7 +66,7 @@ unset _fm_classify_nounset # verb-aware: a nonterminal working: or paused: line never becomes captain-relevant # merely because its prose contains one of those tokens (for example # "working: rebased onto merged #76"). -FM_CLASSIFY_CAPTAIN_RE_DEFAULT='done:|needs-decision:|blocked:|failed:|PR ready|checks green|ready in branch|merged' +FM_CLASSIFY_CAPTAIN_RE_DEFAULT="done:|${FM_CLASSIFY_NEEDS_VALIDATION_VERB:-$FM_CLASSIFY_NEEDS_VALIDATION_VERB_DEFAULT}:|needs-decision:|blocked:|failed:|PR ready|checks green|ready in branch|merged" # The deliberate-external-wait verb. A crew (or firstmate steering it) appends # paused: @@ -100,8 +105,9 @@ last_status_line() { } # 0 if the given (last) status line's leading verb is a real terminal captain verb -# (done, needs-decision, blocked, failed). Free-text tokens alone never count here; -# callers that need legacy free-text matching use status_is_captain_relevant. +# (done, needs-decision, blocked, failed). The captain-relevant needs-validation +# handoff is deliberately absent. Free-text tokens alone never count here; callers +# that need legacy free-text matching use status_is_captain_relevant. status_is_terminal_verb() { local line=$1 verb [ -n "$line" ] || return 1 @@ -129,12 +135,22 @@ status_is_captain_relevant() { esac if [ -z "${FM_CAPTAIN_RE+x}" ]; then case "$verb" in - done|needs-decision|blocked|failed) return 0 ;; + done|"${FM_CLASSIFY_NEEDS_VALIDATION_VERB:-$FM_CLASSIFY_NEEDS_VALIDATION_VERB_DEFAULT}"|needs-decision|blocked|failed) return 0 ;; esac fi printf '%s' "$line" | grep -qiE "${FM_CAPTAIN_RE:-$FM_CLASSIFY_CAPTAIN_RE_DEFAULT}" } +# 0 if a status line is the non-terminal no-mistakes implementation handoff. +# The marker is separate from done: so a supervisor and deterministic state +# reader can tell "trigger validation" from "validation finished" by verb alone. +status_is_validation_handoff() { # + local line=$1 verb + [ -n "$line" ] || return 1 + verb=$(status_line_verb "$line") + [ "$verb" = "${FM_CLASSIFY_NEEDS_VALIDATION_VERB:-$FM_CLASSIFY_NEEDS_VALIDATION_VERB_DEFAULT}" ] +} + # 0 if a status line's leading verb is the pause verb (paused: ). A pure # read of the line itself, so the daemon's classify_stale can reuse the last line # it already read without a fm-crew-state.sh call. Matches only the verb before the @@ -1109,10 +1125,11 @@ EOF # It is never authoritative current crew state, and consumers must not let an open # phase outrank a structured home snapshot or fm-crew-state result. _fm_status_open_activities_stream() { - local line verb key note resolve held open='' stripped pause + local line verb key note resolve held open='' stripped pause needs_validation resolve=${FM_CLASSIFY_RESOLVE_VERB:-$FM_CLASSIFY_RESOLVE_VERB_DEFAULT} held=${FM_CLASSIFY_CAPTAIN_HELD_VERB:-$FM_CLASSIFY_CAPTAIN_HELD_VERB_DEFAULT} pause=${FM_CLASSIFY_PAUSED_VERB:-$FM_CLASSIFY_PAUSED_VERB_DEFAULT} + needs_validation=${FM_CLASSIFY_NEEDS_VALIDATION_VERB:-$FM_CLASSIFY_NEEDS_VALIDATION_VERB_DEFAULT} while IFS= read -r line || [ -n "$line" ]; do stripped=${line//[[:space:]]/} [ -n "$stripped" ] || continue @@ -1125,7 +1142,7 @@ _fm_status_open_activities_stream() { [ -n "$open" ] && open="${open}"$'\n' open="${open}${key}"$'\t'"${verb}"$'\t'"${note}"$'\n' ;; - done|failed|needs-decision|blocked|"$resolve"|"$held") + done|"$needs_validation"|failed|needs-decision|blocked|"$resolve"|"$held") open=$(_fm_decision_drop "$open" "$key") [ -n "$open" ] && open="${open}"$'\n' ;; @@ -1356,11 +1373,12 @@ signal_crew_provably_working() { # ... return 0 } -# 0 (terminal/actionable) if a stale window's last status line is -# captain-relevant; 1 otherwise, including the no-status case. A 1 only means -# "non-terminal"; the always-on watcher then applies crew_is_provably_working, -# while the away-mode daemon applies its persistence recheck. -stale_is_terminal() { # +# 0 (actionable) if a stale window's last status line is captain-relevant; 1 +# otherwise, including the no-status case. Actionable includes both terminal +# results and the non-terminal needs-validation handoff. The always-on watcher +# lets authoritative active-work evidence override either stale status before +# applying its persistence recheck. +stale_has_actionable_status() { # local win=$1 state=$2 last last=$(last_status_line "$state/$(window_to_task "$win" "$state").status") [ -n "$last" ] && status_is_captain_relevant "$last" diff --git a/bin/fm-crew-state.sh b/bin/fm-crew-state.sh index df627b487f2..be86e666069 100755 --- a/bin/fm-crew-state.sh +++ b/bin/fm-crew-state.sh @@ -2,7 +2,7 @@ # fm-crew-state.sh - deterministic read of a crew's CURRENT state. # # Why this exists: state/.status is an append-only, best-effort EVENT LOG. -# Crews append only wake-worthy transitions (done/needs-decision/blocked/paused/failed) +# Crews append only wake-worthy transitions (needs-validation/done/needs-decision/blocked/paused/failed) # and nothing when they silently resume, so `tail -1` of that log reports the # last EVENT, not the current STATE. After firstmate resolves a needs-decision # or blocked and the crew resumes (responds to the gate, the pipeline fixes, it @@ -135,6 +135,10 @@ map_log_state() { # echo paused return fi + if status_is_validation_handoff "$1"; then + echo parked + return + fi case "$(status_line_verb "$1")" in working) echo working ;; needs-decision) echo parked ;; @@ -370,7 +374,7 @@ nm_ci_checks_state() { # status` answer was not this crew's own branch, attribution always failed and # the caller fell straight through to the pane/log fallback below. (The # PRIMARY cause of the 2026-07 herdr false-surface incidents turned out to be -# a separate bug in bin/fm-watch.sh's stale_is_terminal precedence - see that +# a separate bug in bin/fm-watch.sh's stale-actionable precedence - see that # file's history - but this cross-branch path was independently confirmed # dead code and is worth having actually work.) # @@ -562,10 +566,13 @@ if [ "$HAVE_RUN" = 1 ]; then fi fi - # Reconcile the status log. A needs-decision/blocked log line that the run-step - # has moved past (anything but a genuinely parked run) is deterministically - # stale: the gate resolved and the run resumed or finished. + # Reconcile the status log. A needs-validation handoff is superseded by any + # attributed validation run. A needs-decision/blocked line that the run-step has + # moved past (anything but a genuinely parked run) is deterministically stale. case "$LOG_VERB" in + "${FM_CLASSIFY_NEEDS_VALIDATION_VERB:-$FM_CLASSIFY_NEEDS_VALIDATION_VERB_DEFAULT}") + RUN_DETAIL="$RUN_DETAIL${SEP}status-log superseded by validation run" + ;; needs-decision|blocked) if [ "$RUN_STATE" != parked ]; then if [ "$RUN_STATE" = working ]; then diff --git a/bin/fm-supervise-daemon.sh b/bin/fm-supervise-daemon.sh index 86bad52b44c..aec048ae702 100755 --- a/bin/fm-supervise-daemon.sh +++ b/bin/fm-supervise-daemon.sh @@ -387,10 +387,11 @@ classify_stale() { # fi if [ -n "$last" ] && status_is_captain_relevant "$last"; then # Independent of free-text captain-relevant matching: a nonterminal progress - # verb (working:) must never take the terminal stale path. Seen-status dedupe + # verb (working:) must never take the actionable stale path. Seen-status dedupe # must not permanently suppress or clear possible-wedge aging merely because - # prose once looked captain-relevant. Real terminal verbs and legacy free-text - # captain lines without those verbs keep the terminal escalate/dedupe path. + # prose once looked captain-relevant. The non-terminal needs-validation handoff, + # real terminal verbs, and legacy free-text captain lines keep the immediate + # actionable escalate/dedupe path. if ! status_is_terminal_verb "$last"; then case "$(status_line_verb "$last")" in working|resolved|captain-held) @@ -403,10 +404,10 @@ classify_stale() { # # (seen marker matches), self-handle to avoid a duplicate in the digest. seen="$state/.subsuper-seen-status-$(_stale_key "$task")" if [ "$(cat "$seen" 2>/dev/null || true)" = "$last" ]; then - printf 'self|stale + terminal (already escalated by signal): %s' "$last" + printf 'self|stale + actionable status (already escalated by signal): %s' "$last" return fi - printf 'escalate|stale + terminal status: %s' "$last" + printf 'escalate|stale + actionable status: %s' "$last" return fi # Non-terminal (or no status): defer to the persistence recheck. The caller @@ -1267,9 +1268,9 @@ handle_wake() { # if [ "$kind" = "stale" ]; then task=$(window_to_task "$arg" "$state") last=$(last_status_line "$state/$task.status") - # Clear wedge aging only for terminal (or legacy free-text) captain lines. - # Nonterminal progress verbs keep possible-wedge markers even if free text - # once looked captain-relevant or was written into a seen marker. + # Clear wedge aging for actionable handoffs, terminal results, and legacy + # free-text captain lines. Nonterminal progress verbs keep possible-wedge + # markers even if their prose once looked captain-relevant or was seen. _clear_wedge=0 if [ -n "$last" ] && status_is_captain_relevant "$last"; then if status_is_terminal_verb "$last"; then diff --git a/bin/fm-watch.sh b/bin/fm-watch.sh index f5a714b4c75..0d86a362bd1 100755 --- a/bin/fm-watch.sh +++ b/bin/fm-watch.sh @@ -526,7 +526,7 @@ clear_write_tracking() { # # escalates once STALE_ESCALATE_SECS have elapsed. Never re-reads the crew # state (the costly check already ran once, at classification time). Shared by # both places a hash can be absorbed this way: the plain non-terminal path, -# and the stale_is_terminal-overridden path (a captain-relevant status-log +# and the stale-actionable-overridden path (a captain-relevant status-log # line that an active run/busy pane outranked). # The worktree write probe runs ONLY here, inside the at-threshold branch that is # about to escalate: at most one bounded walk per window per STALE_ESCALATE_SECS, @@ -1353,14 +1353,14 @@ EOF printf '%s' "$h" > "$sf" wake "stale: $w" fi - elif stale_is_terminal "$w" "$STATE"; then + elif stale_has_actionable_status "$w" "$STATE"; then # The log's last line is captain-relevant - but that alone is not - # proof the crew is actually done: a crew's own status log gets no + # proof the crew is actually done or awaiting action: a crew's own status log gets no # new entry once firstmate hands it to a no-mistakes validation # (AGENTS.md's sparse status-reporting contract), so the log can - # keep showing a "done:"/needs-decision/blocked leftover from + # keep showing a needs-validation:/done:/needs-decision:/blocked: leftover from # BEFORE that validation started for the run's entire (possibly - # many-minutes) duration, while stale_is_terminal - which has no + # many-minutes) duration, while stale_has_actionable_status - which has no # run-step awareness - keeps reporting it as still-current on every # poll. Root cause of the 2026-07 herdr false-surface incidents: a # validating crew was surfaced as stale every few minutes despite an @@ -1387,11 +1387,11 @@ EOF # wedge timer is running for it) - keep treating it that way # without re-reading the crew state every poll, and without # letting the still-captain-relevant log line re-surface it. - wedge_timer_check "$w" "$ssf" "stale (overridden terminal status)" "$ewf" "$task" + wedge_timer_check "$w" "$ssf" "stale (overridden actionable status)" "$ewf" "$task" fi - # else: already surfaced as genuinely terminal on a prior poll of + # else: already surfaced as genuinely actionable on a prior poll of # this same hash - nothing left to do (matches the original, - # unmodified terminal-status behavior). + # unmodified actionable-status behavior). else # Non-terminal stale: a crew gone quiet without a captain-relevant status. # Decided once per distinct stale hash (the costly state reads run only diff --git a/docs/configuration.md b/docs/configuration.md index a861b406781..0031609163e 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -700,7 +700,7 @@ FM_WATCH_CYCLE_LOG_MAX_BYTES=262144 # size cap for the arm-owned watcher lifec FM_WATCH_CYCLE_LOG_KEEP_LINES=1000 # newest complete lifecycle rows considered when the ledger is capped FM_WATCHER_STALE_GRACE=300 # defaults to FM_GUARD_GRACE; seconds a live watcher lock may have a stale beacon before re-arm errors FM_SIGNAL_GRACE=30 # seconds to coalesce nearby status and turn-end signals into one wake -FM_CAPTAIN_RE='done:|needs-decision:|blocked:|failed:|PR ready|checks green|ready in branch|merged' # captain-relevant status regex; nonterminal progress verbs remain excluded even when their prose matches +FM_CAPTAIN_RE='done:|needs-validation:|needs-decision:|blocked:|failed:|PR ready|checks green|ready in branch|merged' # captain-relevant status regex; routine nonterminal progress verbs remain excluded even when their prose matches FM_CLASSIFY_PAUSED_VERB=paused # leading status verb for a declared external wait; excluded from FM_CAPTAIN_RE and distinct from blocked FM_STALE_ESCALATE_SECS=240 # idle seconds before a provably-working stale pane escalates; stale panes whose crew is not provably working surface immediately unless they declare the pause verb FM_BUSY_TURN_MAX_SECS=3600 # maximum age of a busy pane's latest state/.turn-ended marker, or its state/.meta spawn record before any turn completes, before the same wedge escalation used for a provably-working non-busy stale takes over; inspection-only, never an automatic interrupt or restart; a declared external wait or verified captain-held transfer takes the FM_PAUSE_RESURFACE_SECS recheck below instead diff --git a/tests/fm-brief.test.sh b/tests/fm-brief.test.sh index a4342d758f0..c9041674623 100755 --- a/tests/fm-brief.test.sh +++ b/tests/fm-brief.test.sh @@ -322,7 +322,7 @@ test_faster_paths_use_configured_authority_without_stacked_review() { # Pin the specific line the bug lived on: the no-mistakes DOD's no-mistakes # reference must render as plain prose with no dangling apostrophe artifact. test_no_mistakes_dod_wording() { - local home id brief + local home id brief handoff_line validation_line final_line handoff_text legacy_handoff_text final_text home="$TMP_ROOT/wording-home" mkdir -p "$home/data" id="brief-wording-b1" @@ -345,13 +345,32 @@ test_no_mistakes_dod_wording() { "no-mistakes DOD must keep direct requirements and exclude generic scaffold boilerplate from --intent" assert_grep "exclude generic operational, status, delivery, and other scaffold boilerplate unless it is task-specific" "$brief" \ "no-mistakes DOD must exclude non-task-specific scaffold boilerplate from --intent" + assert_grep 'States: working, needs-validation, needs-decision, blocked, paused, done, failed.' "$brief" \ + "no-mistakes brief did not advertise its distinct implementation handoff state" + # shellcheck disable=SC2016 # Literal backticks and braces are generated brief output. + handoff_text='append `needs-validation: {summary}` to the status file and stop.' + # shellcheck disable=SC2016 # Literal backticks and braces are generated brief output. + legacy_handoff_text='append `done: {summary}` to the status file and stop.' + # shellcheck disable=SC2016 # Literal backticks and braces are generated brief output. + final_text='append `done: PR {url} checks green` and stop. You are finished.' + assert_grep "$handoff_text" "$brief" \ + "no-mistakes DOD did not render the non-terminal implementation handoff" + assert_no_grep "$legacy_handoff_text" "$brief" \ + "no-mistakes DOD still reused done: for the pre-validation implementation handoff" + assert_grep "$final_text" "$brief" \ + "no-mistakes DOD changed the terminal checks-green completion event" + handoff_line=$(grep -nF "$handoff_text" "$brief" | cut -d: -f1) + validation_line=$(grep -nF 'You drive no-mistakes by responding to its gates' "$brief" | cut -d: -f1) + final_line=$(grep -nF "$final_text" "$brief" | cut -d: -f1) + [ "$handoff_line" -lt "$validation_line" ] && [ "$validation_line" -lt "$final_line" ] \ + || fail "no-mistakes DOD no longer orders implementation handoff, triggered validation, and terminal PR completion" # The apostrophe in "firstmate's authority check" is now structurally safe # (no `$(...)` wrapper around the heredoc), so it renders verbatim instead of # being reworded or escaped away. test_no_heredoc_in_command_substitution # guards the structure that makes it safe. assert_grep "firstmate's authority check" "$brief" \ "no-mistakes DOD lost the apostrophe prose that the structural fix makes parse-safe" - pass "fm-brief.sh: no-mistakes DOD keeps its apostrophe prose, now parse-safe" + pass "fm-brief.sh: no-mistakes DOD keeps two-phase handoff/validation semantics and parse-safe prose" } test_ship_project_memory_wording() { @@ -669,7 +688,7 @@ test_herdr_lab_contract_applies_to_scouts_but_not_secondmates() { } test_pause_verb_override_renders_all_brief_scaffolds() { - local home kind id brief + local home kind id brief expected_states home="$TMP_ROOT/pause-verb-home" mkdir -p "$home/data" @@ -679,18 +698,21 @@ test_pause_verb_override_renders_all_brief_scaffolds() { ship) FM_HOME="$home" FM_CLASSIFY_PAUSED_VERB=awaiting \ "$ROOT/bin/fm-brief.sh" "$id" firstmate --mode no-mistakes >/dev/null 2>&1 + expected_states='States: working, needs-validation, needs-decision, blocked, awaiting, done, failed.' ;; scout) FM_HOME="$home" FM_CLASSIFY_PAUSED_VERB=awaiting \ "$ROOT/bin/fm-brief.sh" "$id" firstmate --scout >/dev/null 2>&1 + expected_states='States: working, needs-decision, blocked, awaiting, done, failed.' ;; secondmate) FM_HOME="$home" FM_CLASSIFY_PAUSED_VERB=awaiting \ "$ROOT/bin/fm-brief.sh" "$id" --secondmate --no-projects >/dev/null 2>&1 + expected_states='States: working, needs-decision, blocked, awaiting, done, failed.' ;; esac brief="$home/data/$id/brief.md" - assert_grep "States: working, needs-decision, blocked, awaiting, done, failed." "$brief" \ + assert_grep "$expected_states" "$brief" \ "$kind brief did not render the configured pause verb in its states list" # shellcheck disable=SC2016 # Literal backticks and braces must remain unexpanded. assert_grep 'Use `awaiting: {why}`' "$brief" \ diff --git a/tests/fm-crew-state.test.sh b/tests/fm-crew-state.test.sh index 602b3e5cfc3..07ee7617385 100755 --- a/tests/fm-crew-state.test.sh +++ b/tests/fm-crew-state.test.sh @@ -389,6 +389,23 @@ test_stale_blocked_superseded() { pass "stale blocked over active run is superseded" } +# A validation run is the authoritative evidence that firstmate acted on the +# non-terminal implementation handoff, so the older status line is superseded. +test_validation_handoff_superseded_by_active_run() { + reset_fakes + local d; d=$(new_case validation-handoff-superseded) + make_repo_on_branch "$d/wt" fm/feat-bv + make_fakebin "$d" >/dev/null + fm_write_meta "$d/state/feat-bv.meta" "window=fm:fm-feat-bv" "worktree=$d/wt" "kind=ship" + printf 'needs-validation: implementation committed\n' > "$d/state/feat-bv.status" + FM_FAKE_AXI_STATUS="$(run_running fm/feat-bv)" + local out; out=$(run_crew_state "$d" feat-bv) + assert_contains "$out" "state: working" "triggered validation -> working despite handoff log" + assert_contains "$out" "source: run-step" "triggered validation -> run-step source" + assert_contains "$out" "superseded by validation run" "implementation handoff not reconciled against active validation" + pass "an active validation run supersedes the non-terminal implementation handoff" +} + # (c) genuine parked run + needs-decision log AGREE -> parked, NOT superseded test_genuine_parked_not_superseded() { reset_fakes @@ -768,7 +785,7 @@ test_other_branch_run_ignored() { make_repo_on_branch "$d/wt" fm/feat-g make_fakebin "$d" >/dev/null fm_write_meta "$d/state/feat-g.meta" "window=fm:fm-feat-g" "worktree=$d/wt" "kind=ship" "harness=claude" - printf 'done: implemented, ready to validate\n' > "$d/state/feat-g.status" + printf 'needs-validation: implemented, ready to validate\n' > "$d/state/feat-g.status" FM_FAKE_AXI_STATUS="$(run_running fm/some-other)" FM_FAKE_RUNS_LIST="$(cat <<'EOF' running fm/some-other aaaaaaa 2026-07-02 22:10 @@ -779,8 +796,9 @@ EOF local out; out=$(run_crew_state "$d" feat-g) assert_not_contains "$out" "source: run-step" "another branch's run not misattributed" assert_contains "$out" "source: status-log" "no own run -> falls back to status-log" - assert_contains "$out" "state: done" "falls back to the log verb" - pass "another branch's run is ignored, falls back" + assert_contains "$out" "state: parked" "implementation handoff remains parked pending this branch's validation" + assert_not_contains "$out" "state: done" "implementation handoff must not read as terminal" + pass "another branch's run is ignored and the implementation handoff remains parked" } # (f) no run for this crew + a busy pane -> working via pane @@ -1411,6 +1429,7 @@ test_missing_run_head_falls_back_to_current_state() { test_active_run_is_authoritative test_stale_needs_decision_superseded test_stale_blocked_superseded +test_validation_handoff_superseded_by_active_run test_genuine_parked_not_superseded test_scalar_gate_parked_not_superseded test_gate_block_parked_not_superseded diff --git a/tests/fm-daemon.test.sh b/tests/fm-daemon.test.sh index 962f143464d..ba52f37b7eb 100755 --- a/tests/fm-daemon.test.sh +++ b/tests/fm-daemon.test.sh @@ -102,11 +102,11 @@ test_classify_routine_signal_self() { case "$out" in self\|*) pass "routine signal self-handles" ;; *) fail "routine signal did not self-handle: $out" ;; esac } -test_classify_terminal_signal_escalates() { +test_classify_actionable_signal_escalates() { local dir state kw out dir=$(make_supercase classify-terminal) state="$dir/state" - for kw in "done: PR https://x/y/pull/1" "needs-decision: pick A" "blocked: no perms" \ + for kw in "done: PR https://x/y/pull/1" "needs-validation: implementation committed" "needs-decision: pick A" "blocked: no perms" \ "failed: rc 2" "PR ready https://x/y/pull/2" "checks green" \ "ready in branch fm/t1" "merged"; do printf 'working\n%s\n' "$kw" > "$state/t.status" @@ -1928,7 +1928,7 @@ test_afk_start_ignores_stale_pidfile_without_lock test_afk_start_reclaims_stale_daemon_lock_reused_pid test_daemon_state_root_uses_fm_home test_classify_routine_signal_self -test_classify_terminal_signal_escalates +test_classify_actionable_signal_escalates test_classify_check_and_unknown_escalate test_stale_transient_self_records_marker test_stale_diagnostic_wedge_survives_busy_housekeeping diff --git a/tests/fm-watch-triage.test.sh b/tests/fm-watch-triage.test.sh index 1bee5cab9bb..ece3899451f 100755 --- a/tests/fm-watch-triage.test.sh +++ b/tests/fm-watch-triage.test.sh @@ -183,21 +183,27 @@ test_signal_reason_is_actionable_classifier() { signal_reason_is_actionable "$state/d.status" || fail "a failed: line was not actionable" printf 'merged\n' > "$state/e.status" signal_reason_is_actionable "$state/e.status" || fail "a legacy merged line was not actionable" + printf 'needs-validation: implementation committed\n' > "$state/f.status" + signal_reason_is_actionable "$state/f.status" || fail "a needs-validation: handoff was not actionable" pass "signal_reason_is_actionable: benign absorbed, captain verbs and coalesced batches surfaced" } -test_stale_is_terminal_classifier() { +test_stale_has_actionable_status_classifier() { local dir state dir=$(make_case classify-stale); state="$dir/state" printf 'done: ready in branch fm/x\n' > "$state/term.status" - stale_is_terminal "sess:fm-term" "$state" || fail "terminal stale status not classified terminal" + stale_has_actionable_status "sess:fm-term" "$state" || fail "terminal stale status not classified actionable" fm_write_meta "$state/herdr-term.meta" "window=default:w1:p2" "backend=herdr" printf 'done: ready in branch fm/herdr\n' > "$state/herdr-term.status" - stale_is_terminal "default:w1:p2" "$state" || fail "terminal herdr stale status not resolved through metadata" + stale_has_actionable_status "default:w1:p2" "$state" || fail "terminal herdr stale status not resolved through metadata" + printf 'needs-validation: implementation committed\n' > "$state/handoff.status" + stale_has_actionable_status "sess:fm-handoff" "$state" || fail "non-terminal validation handoff not classified actionable" + status_is_terminal_verb "$(last_status_line "$state/handoff.status")" \ + && fail "actionable validation handoff was classified terminal" printf 'working: compiling\n' > "$state/nonterm.status" - stale_is_terminal "sess:fm-nonterm" "$state" && fail "non-terminal stale classified terminal" - stale_is_terminal "sess:fm-missing" "$state" && fail "stale with no status classified terminal" - pass "stale_is_terminal: terminal status surfaces, non-terminal and no-status are benign" + stale_has_actionable_status "sess:fm-nonterm" "$state" && fail "routine non-terminal stale classified actionable" + stale_has_actionable_status "sess:fm-missing" "$state" && fail "stale with no status classified actionable" + pass "stale_has_actionable_status separates actionable handoffs/results from routine progress" } test_scan_captain_relevant_statuses_classifier() { @@ -206,10 +212,12 @@ test_scan_captain_relevant_statuses_classifier() { printf 'working: a\n' > "$state/one.status" printf 'blocked: no perms\n' > "$state/two.status" printf 'done: PR https://x/y/pull/1\n' > "$state/three.status" + printf 'needs-validation: implementation committed\n' > "$state/four.status" out=$(scan_captain_relevant_statuses "$state") printf '%s' "$out" | grep -F "two.status" >/dev/null || fail "scan missed a blocked: status" printf '%s' "$out" | grep -F "three.status" >/dev/null || fail "scan missed a done: status" printf '%s' "$out" | grep -F "one.status" >/dev/null && fail "scan surfaced a benign working: status" + printf '%s' "$out" | grep -F "four.status" >/dev/null || fail "scan missed a needs-validation: handoff" pass "scan_captain_relevant_statuses lists only captain-relevant statuses" } @@ -220,6 +228,12 @@ test_classifier_primitives() { [ "$(last_status_line "$state/x.status")" = "done: b" ] || fail "last_status_line did not return the last non-blank line" status_is_captain_relevant "done: b" || fail "done: not recognized as captain-relevant" status_is_captain_relevant "needs-decision [key=q1]: b" || fail "keyed needs-decision not recognized as captain-relevant" + status_is_captain_relevant "needs-validation: implementation committed" \ + || fail "needs-validation: handoff not recognized as captain-relevant" + status_is_validation_handoff "needs-validation: implementation committed" \ + || fail "needs-validation: handoff verb not recognized" + status_is_terminal_verb "needs-validation: implementation committed" \ + && fail "needs-validation: implementation handoff was classed as terminal" status_is_captain_relevant "working: b" && fail "working: wrongly recognized as captain-relevant" # Incident regression: free-text "merged" inside a nonterminal working: line must # not become captain-relevant (AFK false-terminal path). @@ -234,6 +248,8 @@ test_classifier_primitives() { || fail "genuine done: checks green not captain-relevant" status_is_terminal_verb "done: PR https://x/pull/76 checks green" \ || fail "done: not a terminal verb" + status_is_validation_handoff "done: PR https://x/pull/76 checks green" \ + && fail "terminal done: PR event was classed as an implementation handoff" status_is_terminal_verb "working: rebased onto merged #76" \ && fail "working: wrongly classed as terminal verb" status_is_captain_relevant "merged" || fail "legacy bare merged free-text not captain-relevant" @@ -267,6 +283,8 @@ resolved [key=phase7]: Phase 7 completed and moved to Done paused [key=legal]: awaiting external counsel resolved [key=legal]: legal item returned to the queue working [key=phase8]: Phase 8 started +working [key=phase9]: Phase 9 implementation +needs-validation [key=phase9]: Phase 9 committed EOF activity=$(status_open_activities "$state/activity.status") printf '%s' "$activity" | grep -F $'phase8\tworking\tPhase 8 started' >/dev/null \ @@ -277,6 +295,8 @@ EOF && fail "a same-key terminal event did not supersede the older working phase" printf '%s' "$activity" | grep -F $'legal\t' >/dev/null \ && fail "a keyed resolved event did not close the declared pause" + printf '%s' "$activity" | grep -F $'phase9\t' >/dev/null \ + && fail "a keyed needs-validation handoff did not close the implementation activity" printf 'working: legacy start\ndone: legacy completion\n' > "$state/legacy-activity.status" [ -z "$(status_open_activities "$state/legacy-activity.status")" ] \ || fail "a legacy terminal event did not supersede the default working phase" @@ -769,27 +789,27 @@ test_terminal_stale_surfaced() { pass "a stale pane sitting on a terminal status is surfaced (queue + exit)" } -# --- stale pane, STALE terminal status overridden by an active run: absorbed --- +# --- stale pane, STALE actionable status overridden by an active run: absorbed --- # Regression for the 2026-07 herdr false-surface incidents: a crew's own status # log gets no new entry once firstmate hands it to a no-mistakes validation # (AGENTS.md's sparse status-reporting contract), so the log keeps showing its -# pre-validation "done:" line as the LAST line for the run's entire (possibly -# many-minutes) duration. stale_is_terminal alone has no run-step awareness and +# pre-validation needs-validation: line as the LAST line for the run's entire +# (possibly many-minutes) duration. stale_has_actionable_status alone has no run-step awareness and # would treat that leftover as still-current every time the pane goes quiet, # immediately surfacing a crew that is actively validating. crew_is_provably_working # must get a chance to override a captain-relevant-but-stale status line, exactly # as it already does for a plain non-terminal one. -test_stale_terminal_status_overridden_by_active_run() { +test_stale_actionable_status_overridden_by_active_run() { local dir state fakebin out drain_out capture_file window key pane_hash sig pid dir=$(make_case terminal-stale-overridden); state="$dir/state"; fakebin="$dir/fakebin" out="$dir/watch.out"; drain_out="$dir/drain.out"; capture_file="$dir/pane.txt" window="test:fm-validating" printf 'no-mistakes axi run: validating...' > "$capture_file" printf 'window=%s\nkind=ship\n' "$window" > "$state/validating.meta" - # The crew reported done BEFORE firstmate triggered no-mistakes validation; + # The crew requested validation BEFORE firstmate triggered no-mistakes; # this line never gets superseded by a newer status-log entry while the # pipeline itself runs. - printf 'done: implementation complete, ready to validate\n' > "$state/validating.status" + printf 'needs-validation: implementation complete and committed\n' > "$state/validating.status" sig=$(seen_sig "$state/validating.status"); printf '%s' "$sig" > "$state/.seen-validating_status" key=$(printf '%s' "$window" | tr ':/.' '___') pane_hash=$(hash_text "no-mistakes axi run: validating...") @@ -798,16 +818,16 @@ test_stale_terminal_status_overridden_by_active_run() { export FM_FAKE_CREW_STATE='state: working · source: run-step · validating (running)' # Phase A: a high escalation threshold means the first sighting is absorbed, - # not surfaced, despite the captain-relevant "done:" status-log line. + # not surfaced, despite the captain-relevant needs-validation: status-log line. PATH="$fakebin:$PATH" FM_FAKE_TMUX_WINDOW="$window" FM_FAKE_TMUX_CAPTURE="$capture_file" \ FM_STATE_OVERRIDE="$state" FM_CREW_STATE_BIN="$fakebin/fm-crew-state.sh" FM_STALE_ESCALATE_SECS=999 FM_POLL=1 FM_SIGNAL_GRACE=1 \ FM_CHECK_INTERVAL=999999 FM_HEARTBEAT=999999 "$WATCH" > "$out" & pid=$! if ! wait_poll_cycle "$state" "$pid"; then - reap "$pid"; fail "watcher exited for a stale terminal-looking status the run-step overrides (should absorb): $(cat "$out")" + reap "$pid"; fail "watcher exited for a stale actionable status the run-step overrides (should absorb): $(cat "$out")" fi - [ ! -s "$out" ] || fail "the overridden stale terminal status printed a wake reason during absorb" - [ ! -s "$state/.wake-queue" ] || fail "the overridden stale terminal status enqueued a wake during absorb" + [ ! -s "$out" ] || fail "the overridden stale actionable status printed a wake reason during absorb" + [ ! -s "$state/.wake-queue" ] || fail "the overridden stale actionable status enqueued a wake during absorb" [ "$(cat "$state/.stale-$key" 2>/dev/null || true)" = "$pane_hash" ] || fail "stale suppressor not advanced on absorb" [ -s "$state/.stale-since-$key" ] || fail "stale-since escalation timer was not recorded on absorb" [ ! -e "$state/.hb-surfaced-validating" ] || fail "an absorbed wake must not mark the status line as surfaced" @@ -822,11 +842,11 @@ test_stale_terminal_status_overridden_by_active_run() { FM_STATE_OVERRIDE="$state" FM_CREW_STATE_BIN="$fakebin/fm-crew-state.sh" FM_STALE_ESCALATE_SECS=240 FM_POLL=1 FM_SIGNAL_GRACE=1 \ FM_CHECK_INTERVAL=999999 FM_HEARTBEAT=999999 "$WATCH" > "$out" & pid=$! - wait_for_exit "$pid" 100 || fail "watcher did not escalate an overridden stale terminal status past the threshold" + wait_for_exit "$pid" 100 || fail "watcher did not escalate an overridden stale actionable status past the threshold" grep -F "stale: $window" "$out" >/dev/null || fail "escalation did not print a stale wake" grep -F "possible wedge" "$out" >/dev/null || fail "escalation did not flag a possible wedge" unset FM_FAKE_CREW_STATE - pass "a stale terminal-looking status is overridden and absorbed while a run is actively working, then wedge-escalated" + pass "a stale actionable status is overridden while validation runs, then wedge-escalated" } # --- non-terminal stale, crew provably working: absorbed, then wedge-escalated --- @@ -2102,7 +2122,7 @@ test_terminal_first_sight_drops_a_finished_write_deferral_chain() { mkdir -p "$wt/src" printf 'no-mistakes axi run: validating...' > "$capture_file" printf 'window=%s\nkind=ship\nworktree=%s\n' "$window" "$wt" > "$state/chain-first.meta" - printf 'done: implementation complete, ready to validate\n' > "$state/chain-first.status" + printf 'needs-validation: implementation complete and committed\n' > "$state/chain-first.status" sig=$(seen_sig "$state/chain-first.status"); printf '%s' "$sig" > "$state/.seen-chain-first_status" key=$(printf '%s' "$window" | tr ':/.' '___') pane_hash=$(hash_text "no-mistakes axi run: validating...") @@ -2122,7 +2142,7 @@ test_terminal_first_sight_drops_a_finished_write_deferral_chain() { FM_CHECK_INTERVAL=999999 FM_HEARTBEAT=999999 "$WATCH" > "$out" & pid=$! if ! wait_poll_cycle "$state" "$pid"; then - reap "$pid"; fail "the overridden terminal status was not absorbed on first sight: $(cat "$out")" + reap "$pid"; fail "the overridden actionable status was not absorbed on first sight: $(cat "$out")" fi [ "$(cat "$state/.stale-$key" 2>/dev/null || true)" = "$pane_hash" ] \ || { reap "$pid"; fail "the first-sight absorb did not advance the stale suppressor"; } @@ -2607,7 +2627,7 @@ test_afk_paused_changed_pane_hands_off_plain_stale() { } test_signal_reason_is_actionable_classifier -test_stale_is_terminal_classifier +test_stale_has_actionable_status_classifier test_scan_captain_relevant_statuses_classifier test_classifier_primitives test_crew_is_provably_working_classifier @@ -2627,7 +2647,7 @@ test_secondmate_status_note_surfaced_despite_busy_agent test_self_announced_close_does_not_rewake_but_next_note_does test_actionable_signal_surfaced test_terminal_stale_surfaced -test_stale_terminal_status_overridden_by_active_run +test_stale_actionable_status_overridden_by_active_run test_nonterminal_stale_provably_working_absorbed_then_escalated test_wedge_escalation_marks_demand_deep_inspection_after_threshold test_wedge_escalation_resets_when_pane_becomes_active From 71e1325dde8c400a91abf937d35c2cd80a9338f2 Mon Sep 17 00:00:00 2001 From: connectwithclayton-cpu <278079184+connectwithclayton@users.noreply.github.com> Date: Sun, 23 Aug 2026 21:12:42 -0400 Subject: [PATCH 2/7] no-mistakes(review): Guard active validation handoffs; tests pass, actionlint unavailable --- bin/fm-classify-lib.sh | 27 ++++++++++++++++++++++++--- bin/fm-supervise-daemon.sh | 6 +++--- tests/fm-daemon.test.sh | 31 +++++++++++++++++++++++++++++++ 3 files changed, 58 insertions(+), 6 deletions(-) diff --git a/bin/fm-classify-lib.sh b/bin/fm-classify-lib.sh index a48d0df8705..fbd86f162ca 100755 --- a/bin/fm-classify-lib.sh +++ b/bin/fm-classify-lib.sh @@ -151,6 +151,15 @@ status_is_validation_handoff() { # [ "$verb" = "${FM_CLASSIFY_NEEDS_VALIDATION_VERB:-$FM_CLASSIFY_NEEDS_VALIDATION_VERB_DEFAULT}" ] } +status_is_actionable_now() { # + local task=$1 line=$2 + status_is_captain_relevant "$line" || return 1 + if status_is_validation_handoff "$line" && crew_is_in_active_run "$task"; then + return 1 + fi + return 0 +} + # 0 if a status line's leading verb is the pause verb (paused: ). A pure # read of the line itself, so the daemon's classify_stale can reuse the last line # it already read without a fm-crew-state.sh call. Matches only the verb before the @@ -1228,6 +1237,17 @@ crew_absorb_class() { # printf 'none' } +crew_is_in_active_run() { # + local id=$1 line state src + [ -n "$id" ] || return 1 + line=$("$FM_CREW_STATE_BIN" "$id" 2>/dev/null) || return 1 + case "$line" in state:*) ;; *) return 1 ;; esac + state=${line#state: }; state=${state%% *} + [ "$state" = working ] || return 1 + src=${line#*source: }; src=${src%% *} + [ "$src" = run-step ] +} + # 0 if crew shows POSITIVE evidence it is still working (crew_absorb_class # reports `working`). This is the "provably working" predicate at the heart of # absorb-only-when-provably-working: a no-verb turn-end or stale wake is absorbed @@ -1380,8 +1400,9 @@ signal_crew_provably_working() { # ... # applying its persistence recheck. stale_has_actionable_status() { # local win=$1 state=$2 last - last=$(last_status_line "$state/$(window_to_task "$win" "$state").status") - [ -n "$last" ] && status_is_captain_relevant "$last" + local task; task=$(window_to_task "$win" "$state") + last=$(last_status_line "$state/$task.status") + [ -n "$last" ] && status_is_actionable_now "$task" "$last" } # Print "\t\t" for every state/*.status whose last line is @@ -1394,8 +1415,8 @@ scan_captain_relevant_statuses() { # for f in "$state"/*.status; do [ -e "$f" ] || continue last=$(last_status_line "$f") - status_is_captain_relevant "$last" || continue task=$(basename "$f"); task="${task%.status}" + status_is_actionable_now "$task" "$last" || continue printf '%s\t%s\t%s\n' "$f" "$task" "$last" done return 0 diff --git a/bin/fm-supervise-daemon.sh b/bin/fm-supervise-daemon.sh index aec048ae702..d6e4991f590 100755 --- a/bin/fm-supervise-daemon.sh +++ b/bin/fm-supervise-daemon.sh @@ -345,13 +345,13 @@ classify_signal() { # last=$(last_status_line "$f") [ -n "$last" ] || continue distilled="${distilled}$(basename "$f"): ${last} | " - status_is_captain_relevant "$last" || continue + task=$(basename "$f"); task="${task%.status}" + status_is_actionable_now "$task" "$last" || continue rel=1 # Dedupe against the catch-all scan: if this status was already escalated # (seen marker matches), skip escalating again. The seen marker is the # single source of truth shared between the per-wake signal path and the # heartbeat scan. all_seen stays 1 only if EVERY relevant file was seen. - task=$(basename "$f"); task="${task%.status}" seen="$state/.subsuper-seen-status-$(_stale_key "$task")" [ "$(cat "$seen" 2>/dev/null || true)" = "$last" ] || all_seen=0 done @@ -385,7 +385,7 @@ classify_stale() { # printf 'pause|paused (awaiting external), rechecked on a long cadence: %s' "$last" return fi - if [ -n "$last" ] && status_is_captain_relevant "$last"; then + if [ -n "$last" ] && status_is_actionable_now "$task" "$last"; then # Independent of free-text captain-relevant matching: a nonterminal progress # verb (working:) must never take the actionable stale path. Seen-status dedupe # must not permanently suppress or clear possible-wedge aging merely because diff --git a/tests/fm-daemon.test.sh b/tests/fm-daemon.test.sh index ba52f37b7eb..5234fa47cee 100755 --- a/tests/fm-daemon.test.sh +++ b/tests/fm-daemon.test.sh @@ -726,6 +726,36 @@ test_heartbeat_scan_dedup() { pass "catch-all scan escalates a missed terminal once, not twice" } +test_validation_handoff_current_state_guard() { + local dir state fakebin out + dir=$(make_case validation-handoff-current-state) + state="$dir/state" + fakebin="$dir/fakebin" + printf 'needs-validation: implementation committed\n' > "$state/validation.status" + ( + export FM_CREW_STATE_BIN="$fakebin/fm-crew-state.sh" + export FM_FAKE_CREW_STATE='state: working · source: run-step · validating (running)' + out=$(FM_STATE_OVERRIDE="$state" classify_stale "sess:fm-validation" "$state") + case "$out" in + self\|*) ;; + *) fail "active validation run escalated stale handoff: $out" ;; + esac + rm -f "$state/.subsuper-last-scan" + FM_STATE_OVERRIDE="$state" FM_HEARTBEAT_SCAN_SECS=0 housekeeping "$state" + [ ! -s "$state/.subsuper-escalations" ] || fail "active validation run was escalated by heartbeat scan" + export FM_FAKE_CREW_STATE='state: unknown · source: none · inactive handoff' + out=$(FM_STATE_OVERRIDE="$state" classify_stale "sess:fm-validation" "$state") + case "$out" in + escalate\|*) ;; + *) fail "inactive validation handoff was not actionable: $out" ;; + esac + rm -f "$state/.subsuper-last-scan" + FM_STATE_OVERRIDE="$state" FM_HEARTBEAT_SCAN_SECS=0 housekeeping "$state" + [ -s "$state/.subsuper-escalations" ] || fail "inactive validation handoff was missed by heartbeat scan" + ) || return 1 + pass "active validation suppresses stale handoff paths while inactive handoff remains actionable" +} + test_handle_wake_routes_self_and_escalate() { local dir state dir=$(make_supercase handle) @@ -1958,6 +1988,7 @@ test_housekeeping_orca_persistent_stale_resolves_terminal test_escalate_batches_into_one_digest test_escalate_batch_age_uses_first_append test_heartbeat_scan_dedup +test_validation_handoff_current_state_guard test_handle_wake_routes_self_and_escalate test_inject_skip_forces_self test_is_wake_reason_distinguishes_status_stdout From f511f81134c13e88fb55bc72a357e33056c1dea0 Mon Sep 17 00:00:00 2001 From: connectwithclayton-cpu <278079184+connectwithclayton@users.noreply.github.com> Date: Sun, 23 Aug 2026 21:28:35 -0400 Subject: [PATCH 3/7] no-mistakes(review): Handle parked validation runs and retain wedge aging --- bin/fm-classify-lib.sh | 15 ++++--- bin/fm-supervise-daemon.sh | 4 +- tests/fm-daemon.test.sh | 88 +++++++++++++++++++++++++++++++------- 3 files changed, 83 insertions(+), 24 deletions(-) diff --git a/bin/fm-classify-lib.sh b/bin/fm-classify-lib.sh index fbd86f162ca..4689a959acd 100755 --- a/bin/fm-classify-lib.sh +++ b/bin/fm-classify-lib.sh @@ -154,12 +154,15 @@ status_is_validation_handoff() { # status_is_actionable_now() { # local task=$1 line=$2 status_is_captain_relevant "$line" || return 1 - if status_is_validation_handoff "$line" && crew_is_in_active_run "$task"; then - return 1 - fi + status_is_superseded_by_run_step "$task" "$line" && return 1 return 0 } +status_is_superseded_by_run_step() { # + local task=$1 line=$2 + status_is_validation_handoff "$line" && crew_has_attributed_run_step "$task" +} + # 0 if a status line's leading verb is the pause verb (paused: ). A pure # read of the line itself, so the daemon's classify_stale can reuse the last line # it already read without a fm-crew-state.sh call. Matches only the verb before the @@ -1237,13 +1240,11 @@ crew_absorb_class() { # printf 'none' } -crew_is_in_active_run() { # - local id=$1 line state src +crew_has_attributed_run_step() { # + local id=$1 line src [ -n "$id" ] || return 1 line=$("$FM_CREW_STATE_BIN" "$id" 2>/dev/null) || return 1 case "$line" in state:*) ;; *) return 1 ;; esac - state=${line#state: }; state=${state%% *} - [ "$state" = working ] || return 1 src=${line#*source: }; src=${src%% *} [ "$src" = run-step ] } diff --git a/bin/fm-supervise-daemon.sh b/bin/fm-supervise-daemon.sh index d6e4991f590..0417ec26c11 100755 --- a/bin/fm-supervise-daemon.sh +++ b/bin/fm-supervise-daemon.sh @@ -1273,7 +1273,9 @@ handle_wake() { # # markers even if their prose once looked captain-relevant or was seen. _clear_wedge=0 if [ -n "$last" ] && status_is_captain_relevant "$last"; then - if status_is_terminal_verb "$last"; then + if status_is_superseded_by_run_step "$task" "$last"; then + _clear_wedge=0 + elif status_is_terminal_verb "$last"; then _clear_wedge=1 else case "$(status_line_verb "$last")" in diff --git a/tests/fm-daemon.test.sh b/tests/fm-daemon.test.sh index 5234fa47cee..dffe3d5660e 100755 --- a/tests/fm-daemon.test.sh +++ b/tests/fm-daemon.test.sh @@ -727,35 +727,90 @@ test_heartbeat_scan_dedup() { } test_validation_handoff_current_state_guard() { - local dir state fakebin out + local dir state fakebin out current_state old_crew_state_bin old_fake_crew_state had_fake_crew_state=0 dir=$(make_case validation-handoff-current-state) state="$dir/state" fakebin="$dir/fakebin" printf 'needs-validation: implementation committed\n' > "$state/validation.status" - ( - export FM_CREW_STATE_BIN="$fakebin/fm-crew-state.sh" - export FM_FAKE_CREW_STATE='state: working · source: run-step · validating (running)' + old_crew_state_bin=$FM_CREW_STATE_BIN + if [ "${FM_FAKE_CREW_STATE+x}" = x ]; then + had_fake_crew_state=1 + old_fake_crew_state=$FM_FAKE_CREW_STATE + else + old_fake_crew_state= + fi + export FM_CREW_STATE_BIN="$fakebin/fm-crew-state.sh" + for current_state in \ + 'state: working · source: run-step · validating (running)' \ + 'state: parked · source: run-step · parked at awaiting_approval' \ + 'state: parked · source: run-step · parked at fix_review'; do + export FM_FAKE_CREW_STATE="$current_state" out=$(FM_STATE_OVERRIDE="$state" classify_stale "sess:fm-validation" "$state") case "$out" in self\|*) ;; - *) fail "active validation run escalated stale handoff: $out" ;; - esac - rm -f "$state/.subsuper-last-scan" - FM_STATE_OVERRIDE="$state" FM_HEARTBEAT_SCAN_SECS=0 housekeeping "$state" - [ ! -s "$state/.subsuper-escalations" ] || fail "active validation run was escalated by heartbeat scan" - export FM_FAKE_CREW_STATE='state: unknown · source: none · inactive handoff' - out=$(FM_STATE_OVERRIDE="$state" classify_stale "sess:fm-validation" "$state") - case "$out" in - escalate\|*) ;; - *) fail "inactive validation handoff was not actionable: $out" ;; + *) fail "attributed validation run escalated stale handoff: $out" ;; esac rm -f "$state/.subsuper-last-scan" + : > "$state/.subsuper-escalations" FM_STATE_OVERRIDE="$state" FM_HEARTBEAT_SCAN_SECS=0 housekeeping "$state" - [ -s "$state/.subsuper-escalations" ] || fail "inactive validation handoff was missed by heartbeat scan" - ) || return 1 + [ ! -s "$state/.subsuper-escalations" ] || fail "attributed validation run was escalated by heartbeat scan" + done + export FM_FAKE_CREW_STATE='state: unknown · source: none · inactive handoff' + out=$(FM_STATE_OVERRIDE="$state" classify_stale "sess:fm-validation" "$state") + case "$out" in + escalate\|*) ;; + *) fail "inactive validation handoff was not actionable: $out" ;; + esac + rm -f "$state/.subsuper-last-scan" + FM_STATE_OVERRIDE="$state" FM_HEARTBEAT_SCAN_SECS=0 housekeeping "$state" + [ -s "$state/.subsuper-escalations" ] || fail "inactive validation handoff was missed by heartbeat scan" + export FM_CREW_STATE_BIN="$old_crew_state_bin" + if [ "$had_fake_crew_state" = 1 ]; then + export FM_FAKE_CREW_STATE="$old_fake_crew_state" + else + unset FM_FAKE_CREW_STATE + fi pass "active validation suppresses stale handoff paths while inactive handoff remains actionable" } +test_validation_handoff_retains_wedge_marker() { + local dir state fakebin win key pane old_crew_state_bin old_fake_crew_state had_fake_crew_state=0 + dir=$(make_case validation-handoff-wedge-marker) + state="$dir/state" + fakebin="$dir/fakebin" + win='sess:fm-validation' + key=validation + pane="$dir/pane.txt" + fm_write_meta "$state/validation.meta" "window=$win" "backend=tmux" + printf 'needs-validation: implementation committed\n' > "$state/validation.status" + printf 'idle prompt $\n' > "$pane" + old_crew_state_bin=$FM_CREW_STATE_BIN + if [ "${FM_FAKE_CREW_STATE+x}" = x ]; then + had_fake_crew_state=1 + old_fake_crew_state=$FM_FAKE_CREW_STATE + else + old_fake_crew_state= + fi + export FM_CREW_STATE_BIN="$fakebin/fm-crew-state.sh" + export FM_FAKE_CREW_STATE='state: parked · source: run-step · parked at fix_review' + FM_STATE_OVERRIDE="$state" handle_wake "stale: $win" "$state" + [ -e "$state/.subsuper-stale-$key" ] || fail "attributed validation handoff lost wedge marker" + [ ! -s "$state/.subsuper-escalations" ] || fail "attributed validation handoff escalated immediately" + export FM_FAKE_CREW_STATE='state: unknown · source: none · validation ended quietly' + echo $(( $(date +%s) - 500 )) > "$state/.subsuper-stale-$key" + PATH="$fakebin:$PATH" FM_STATE_OVERRIDE="$state" FM_FAKE_TMUX_WINDOW="$win" \ + FM_FAKE_TMUX_CAPTURE="$pane" FM_STALE_ESCALATE_SECS=240 housekeeping "$state" + [ -s "$state/.subsuper-escalations" ] || fail "retained wedge marker did not escalate after validation ended" + [ ! -e "$state/.subsuper-stale-$key" ] || fail "retained wedge marker was not cleared after escalation" + export FM_CREW_STATE_BIN="$old_crew_state_bin" + if [ "$had_fake_crew_state" = 1 ]; then + export FM_FAKE_CREW_STATE="$old_fake_crew_state" + else + unset FM_FAKE_CREW_STATE + fi + pass "validation handoff retains wedge aging through a quiet run" +} + test_handle_wake_routes_self_and_escalate() { local dir state dir=$(make_supercase handle) @@ -1989,6 +2044,7 @@ test_escalate_batches_into_one_digest test_escalate_batch_age_uses_first_append test_heartbeat_scan_dedup test_validation_handoff_current_state_guard +test_validation_handoff_retains_wedge_marker test_handle_wake_routes_self_and_escalate test_inject_skip_forces_self test_is_wake_reason_distinguishes_status_stdout From bf081ef6a30da5c68eb9c27c07a50d70b1665487 Mon Sep 17 00:00:00 2001 From: connectwithclayton-cpu <278079184+connectwithclayton@users.noreply.github.com> Date: Sun, 23 Aug 2026 21:45:25 -0400 Subject: [PATCH 4/7] no-mistakes(review): Heartbeat scan reuse and state evidence added --- bin/fm-watch.sh | 19 ++++++--- tests/fm-wake-drain-unread-status.test.sh | 48 +++++++++++++++++++++++ tests/fm-watch-triage.test.sh | 33 ++++++++++++++++ 3 files changed, 95 insertions(+), 5 deletions(-) diff --git a/bin/fm-watch.sh b/bin/fm-watch.sh index 0d86a362bd1..e23fb21ddee 100755 --- a/bin/fm-watch.sh +++ b/bin/fm-watch.sh @@ -896,11 +896,16 @@ run_check_capture() { # heartbeat backstop enqueues its wake, so the same statuses are not re-surfaced # by the next heartbeat. mark_all_captain_relevant_surfaced() { - local f task last + local rows=${1-} f task last + if [ "$#" -eq 0 ]; then + rows=$(scan_captain_relevant_statuses "$STATE") + fi while IFS=$(printf '\t') read -r f task last; do [ -n "$f" ] || continue printf '%s' "$last" > "$(_hb_surfaced_path "$task")" - done < <(scan_captain_relevant_statuses "$STATE") + done </dev/null || true) [ "$surfaced" = "$last" ] && continue return 0 - done < <(scan_captain_relevant_statuses "$STATE") + done </dev/null 2>&1 \ + || fail "no-mistakes brief generation failed for state evidence" + + legacy_handoff='done: implementation committed' + legacy_terminal='done: PR https://example.test/pr/7 checks green' + new_handoff='needs-validation: implementation committed' + new_terminal='done: PR https://example.test/pr/7 checks green' + printf '%s\n' "$legacy_handoff" > "$state/legacy-handoff.status" + printf '%s\n' "$legacy_terminal" > "$state/legacy-terminal.status" + printf '%s\n' "$new_handoff" > "$state/new-handoff.status" + printf '%s\n' "$new_terminal" > "$state/new-terminal.status" + append_wake "$state" signal legacy-handoff.status "signal: legacy-handoff.status" \ + || fail "legacy handoff signal queueing failed" + append_wake "$state" signal legacy-terminal.status "signal: legacy-terminal.status" \ + || fail "legacy terminal signal queueing failed" + append_wake "$state" signal new-handoff.status "signal: new-handoff.status" \ + || fail "new handoff signal queueing failed" + append_wake "$state" signal new-terminal.status "signal: new-terminal.status" \ + || fail "new terminal signal queueing failed" + FM_STATE_OVERRIDE="$state" "$DRAIN" > "$out" \ + || fail "drain failed for before/after state evidence" + + assert_grep "append \`needs-validation: {summary}\` to the status file and stop." "$brief" \ + "generated brief did not emit the non-terminal handoff" + assert_grep "append \`done: PR {url} checks green\` and stop. You are finished." "$brief" \ + "generated brief did not emit terminal completion" + assert_grep 'legacy-handoff.status: done: implementation committed' "$out" \ + "drain did not expose the legacy handoff form" + assert_grep 'legacy-terminal.status: done: PR https://example.test/pr/7 checks green' "$out" \ + "drain did not expose the legacy terminal form" + assert_grep 'new-handoff.status: needs-validation: implementation committed' "$out" \ + "drain did not expose the mechanically distinct handoff form" + assert_grep 'new-terminal.status: done: PR https://example.test/pr/7 checks green' "$out" \ + "drain did not expose the terminal completion form" + printf 'before | implementation handoff: %s | terminal completion: %s\n' "$legacy_handoff" "$legacy_terminal" + printf 'after | implementation handoff: %s | terminal completion: %s\n' "$new_handoff" "$new_terminal" + pass "generated brief and drain annotations expose distinct before/after done states" +} + test_routine_working_lines_stay_silent_on_the_empty_queue() { local dir state out dir=$(make_case silent-working) @@ -318,4 +365,5 @@ test_snapshot_does_not_ack_a_later_append test_retired_task_id_starts_new_status_unread test_open_decisions_fold_is_unchanged test_empty_queue_does_not_swallow_later_signal_annotation +test_dod_states_are_distinct_in_generated_brief_and_drain_annotations test_routine_working_lines_stay_silent_on_the_empty_queue diff --git a/tests/fm-watch-triage.test.sh b/tests/fm-watch-triage.test.sh index ece3899451f..c5b19c0cd39 100755 --- a/tests/fm-watch-triage.test.sh +++ b/tests/fm-watch-triage.test.sh @@ -25,6 +25,9 @@ set -u WATCH="$ROOT/bin/fm-watch.sh" DRAIN="$ROOT/bin/fm-wake-drain.sh" +# shellcheck source=bin/fm-watch.sh +. "$WATCH" + TMP_ROOT=$(fm_test_tmproot fm-watch-triage-tests) ack_stopped_cycle() { # @@ -2534,6 +2537,35 @@ test_heartbeat_backstop_surfaces_unsurfaced_status() { pass "heartbeat backstop fail-safe surfaces a captain-relevant status the per-wake path missed" } +test_heartbeat_scan_reuses_current_state_for_marking() { + local dir state fakebin calls + dir=$(make_case heartbeat-scan-reuse); state="$dir/state"; fakebin="$dir/fakebin" + calls="$dir/crew-state-calls" + printf 'needs-validation: implementation committed\n' > "$state/needs-validation-task.status" + cat > "$fakebin/fm-crew-state.sh" <<'SH' +#!/usr/bin/env bash +set -u +calls=${FM_CREW_STATE_CALLS:?} +count=$(cat "$calls" 2>/dev/null || echo 0) +printf '%s\n' "$((count + 1))" > "$calls" +printf '%s\n' 'state: stopped · source: none · fake' +SH + chmod +x "$fakebin/fm-crew-state.sh" + FM_CREW_STATE_BIN="$fakebin/fm-crew-state.sh" + FM_CREW_STATE_CALLS="$calls" + export FM_CREW_STATE_CALLS + # shellcheck disable=SC2034 + STATE="$state" + heartbeat_scan_finds_actionable \ + || fail "heartbeat scan did not find the inactive validation handoff" + mark_all_captain_relevant_surfaced "$FM_HEARTBEAT_ACTIONABLE_ROWS" + [ "$(cat "$calls")" = 1 ] \ + || fail "heartbeat detection and marking re-read current state: $(cat "$calls")" + [ "$(cat "$state/.hb-surfaced-needs-validation-task")" = 'needs-validation: implementation committed' ] \ + || fail "heartbeat marking did not reuse the detected status row" + pass "heartbeat detection reuses one current-state verdict while marking" +} + # --- beacon stays fresh while absorbing ------------------------------------- test_beacon_stays_fresh_while_absorbing() { @@ -2683,6 +2715,7 @@ test_procevent_surface_crash_boundaries test_procevent_marker_failure_exits_and_replays test_heartbeat_no_change_absorbed test_heartbeat_backstop_surfaces_unsurfaced_status +test_heartbeat_scan_reuses_current_state_for_marking test_beacon_stays_fresh_while_absorbing test_afk_present_reverts_watcher_to_one_shot test_afk_paused_changed_pane_hands_off_plain_stale From 51e30c10d79c35b9ec2243ea45ca2e975a9ed3e7 Mon Sep 17 00:00:00 2001 From: connectwithclayton-cpu <278079184+connectwithclayton@users.noreply.github.com> Date: Sun, 23 Aug 2026 22:06:43 -0400 Subject: [PATCH 5/7] no-mistakes(review): Narrow marker semantics; parked-gate duplicates remain separately scoped --- bin/fm-classify-lib.sh | 39 +++------------ bin/fm-supervise-daemon.sh | 27 +++++------ bin/fm-watch.sh | 35 +++++--------- tests/fm-daemon.test.sh | 91 +---------------------------------- tests/fm-watch-triage.test.sh | 55 +++++---------------- 5 files changed, 46 insertions(+), 201 deletions(-) diff --git a/bin/fm-classify-lib.sh b/bin/fm-classify-lib.sh index 4689a959acd..2337a00a65f 100755 --- a/bin/fm-classify-lib.sh +++ b/bin/fm-classify-lib.sh @@ -151,18 +151,6 @@ status_is_validation_handoff() { # [ "$verb" = "${FM_CLASSIFY_NEEDS_VALIDATION_VERB:-$FM_CLASSIFY_NEEDS_VALIDATION_VERB_DEFAULT}" ] } -status_is_actionable_now() { # - local task=$1 line=$2 - status_is_captain_relevant "$line" || return 1 - status_is_superseded_by_run_step "$task" "$line" && return 1 - return 0 -} - -status_is_superseded_by_run_step() { # - local task=$1 line=$2 - status_is_validation_handoff "$line" && crew_has_attributed_run_step "$task" -} - # 0 if a status line's leading verb is the pause verb (paused: ). A pure # read of the line itself, so the daemon's classify_stale can reuse the last line # it already read without a fm-crew-state.sh call. Matches only the verb before the @@ -1240,15 +1228,6 @@ crew_absorb_class() { # printf 'none' } -crew_has_attributed_run_step() { # - local id=$1 line src - [ -n "$id" ] || return 1 - line=$("$FM_CREW_STATE_BIN" "$id" 2>/dev/null) || return 1 - case "$line" in state:*) ;; *) return 1 ;; esac - src=${line#*source: }; src=${src%% *} - [ "$src" = run-step ] -} - # 0 if crew shows POSITIVE evidence it is still working (crew_absorb_class # reports `working`). This is the "provably working" predicate at the heart of # absorb-only-when-provably-working: a no-verb turn-end or stale wake is absorbed @@ -1394,16 +1373,14 @@ signal_crew_provably_working() { # ... return 0 } -# 0 (actionable) if a stale window's last status line is captain-relevant; 1 -# otherwise, including the no-status case. Actionable includes both terminal -# results and the non-terminal needs-validation handoff. The always-on watcher -# lets authoritative active-work evidence override either stale status before -# applying its persistence recheck. -stale_has_actionable_status() { # +# 0 (terminal/actionable) if a stale window's last status line is +# captain-relevant; 1 otherwise, including the no-status case. A 1 only means +# "non-terminal"; the always-on watcher then applies crew_is_provably_working, +# while the away-mode daemon applies its persistence recheck. +stale_is_terminal() { # local win=$1 state=$2 last - local task; task=$(window_to_task "$win" "$state") - last=$(last_status_line "$state/$task.status") - [ -n "$last" ] && status_is_actionable_now "$task" "$last" + last=$(last_status_line "$state/$(window_to_task "$win" "$state").status") + [ -n "$last" ] && status_is_captain_relevant "$last" } # Print "\t\t" for every state/*.status whose last line is @@ -1416,8 +1393,8 @@ scan_captain_relevant_statuses() { # for f in "$state"/*.status; do [ -e "$f" ] || continue last=$(last_status_line "$f") + status_is_captain_relevant "$last" || continue task=$(basename "$f"); task="${task%.status}" - status_is_actionable_now "$task" "$last" || continue printf '%s\t%s\t%s\n' "$f" "$task" "$last" done return 0 diff --git a/bin/fm-supervise-daemon.sh b/bin/fm-supervise-daemon.sh index 0417ec26c11..86bad52b44c 100755 --- a/bin/fm-supervise-daemon.sh +++ b/bin/fm-supervise-daemon.sh @@ -345,13 +345,13 @@ classify_signal() { # last=$(last_status_line "$f") [ -n "$last" ] || continue distilled="${distilled}$(basename "$f"): ${last} | " - task=$(basename "$f"); task="${task%.status}" - status_is_actionable_now "$task" "$last" || continue + status_is_captain_relevant "$last" || continue rel=1 # Dedupe against the catch-all scan: if this status was already escalated # (seen marker matches), skip escalating again. The seen marker is the # single source of truth shared between the per-wake signal path and the # heartbeat scan. all_seen stays 1 only if EVERY relevant file was seen. + task=$(basename "$f"); task="${task%.status}" seen="$state/.subsuper-seen-status-$(_stale_key "$task")" [ "$(cat "$seen" 2>/dev/null || true)" = "$last" ] || all_seen=0 done @@ -385,13 +385,12 @@ classify_stale() { # printf 'pause|paused (awaiting external), rechecked on a long cadence: %s' "$last" return fi - if [ -n "$last" ] && status_is_actionable_now "$task" "$last"; then + if [ -n "$last" ] && status_is_captain_relevant "$last"; then # Independent of free-text captain-relevant matching: a nonterminal progress - # verb (working:) must never take the actionable stale path. Seen-status dedupe + # verb (working:) must never take the terminal stale path. Seen-status dedupe # must not permanently suppress or clear possible-wedge aging merely because - # prose once looked captain-relevant. The non-terminal needs-validation handoff, - # real terminal verbs, and legacy free-text captain lines keep the immediate - # actionable escalate/dedupe path. + # prose once looked captain-relevant. Real terminal verbs and legacy free-text + # captain lines without those verbs keep the terminal escalate/dedupe path. if ! status_is_terminal_verb "$last"; then case "$(status_line_verb "$last")" in working|resolved|captain-held) @@ -404,10 +403,10 @@ classify_stale() { # # (seen marker matches), self-handle to avoid a duplicate in the digest. seen="$state/.subsuper-seen-status-$(_stale_key "$task")" if [ "$(cat "$seen" 2>/dev/null || true)" = "$last" ]; then - printf 'self|stale + actionable status (already escalated by signal): %s' "$last" + printf 'self|stale + terminal (already escalated by signal): %s' "$last" return fi - printf 'escalate|stale + actionable status: %s' "$last" + printf 'escalate|stale + terminal status: %s' "$last" return fi # Non-terminal (or no status): defer to the persistence recheck. The caller @@ -1268,14 +1267,12 @@ handle_wake() { # if [ "$kind" = "stale" ]; then task=$(window_to_task "$arg" "$state") last=$(last_status_line "$state/$task.status") - # Clear wedge aging for actionable handoffs, terminal results, and legacy - # free-text captain lines. Nonterminal progress verbs keep possible-wedge - # markers even if their prose once looked captain-relevant or was seen. + # Clear wedge aging only for terminal (or legacy free-text) captain lines. + # Nonterminal progress verbs keep possible-wedge markers even if free text + # once looked captain-relevant or was written into a seen marker. _clear_wedge=0 if [ -n "$last" ] && status_is_captain_relevant "$last"; then - if status_is_superseded_by_run_step "$task" "$last"; then - _clear_wedge=0 - elif status_is_terminal_verb "$last"; then + if status_is_terminal_verb "$last"; then _clear_wedge=1 else case "$(status_line_verb "$last")" in diff --git a/bin/fm-watch.sh b/bin/fm-watch.sh index e23fb21ddee..f5a714b4c75 100755 --- a/bin/fm-watch.sh +++ b/bin/fm-watch.sh @@ -526,7 +526,7 @@ clear_write_tracking() { # # escalates once STALE_ESCALATE_SECS have elapsed. Never re-reads the crew # state (the costly check already ran once, at classification time). Shared by # both places a hash can be absorbed this way: the plain non-terminal path, -# and the stale-actionable-overridden path (a captain-relevant status-log +# and the stale_is_terminal-overridden path (a captain-relevant status-log # line that an active run/busy pane outranked). # The worktree write probe runs ONLY here, inside the at-threshold branch that is # about to escalate: at most one bounded walk per window per STALE_ESCALATE_SECS, @@ -896,16 +896,11 @@ run_check_capture() { # heartbeat backstop enqueues its wake, so the same statuses are not re-surfaced # by the next heartbeat. mark_all_captain_relevant_surfaced() { - local rows=${1-} f task last - if [ "$#" -eq 0 ]; then - rows=$(scan_captain_relevant_statuses "$STATE") - fi + local f task last while IFS=$(printf '\t') read -r f task last; do [ -n "$f" ] || continue printf '%s' "$last" > "$(_hb_surfaced_path "$task")" - done </dev/null || true) [ "$surfaced" = "$last" ] && continue return 0 - done < "$sf" wake "stale: $w" fi - elif stale_has_actionable_status "$w" "$STATE"; then + elif stale_is_terminal "$w" "$STATE"; then # The log's last line is captain-relevant - but that alone is not - # proof the crew is actually done or awaiting action: a crew's own status log gets no + # proof the crew is actually done: a crew's own status log gets no # new entry once firstmate hands it to a no-mistakes validation # (AGENTS.md's sparse status-reporting contract), so the log can - # keep showing a needs-validation:/done:/needs-decision:/blocked: leftover from + # keep showing a "done:"/needs-decision/blocked leftover from # BEFORE that validation started for the run's entire (possibly - # many-minutes) duration, while stale_has_actionable_status - which has no + # many-minutes) duration, while stale_is_terminal - which has no # run-step awareness - keeps reporting it as still-current on every # poll. Root cause of the 2026-07 herdr false-surface incidents: a # validating crew was surfaced as stale every few minutes despite an @@ -1396,11 +1387,11 @@ EOF # wedge timer is running for it) - keep treating it that way # without re-reading the crew state every poll, and without # letting the still-captain-relevant log line re-surface it. - wedge_timer_check "$w" "$ssf" "stale (overridden actionable status)" "$ewf" "$task" + wedge_timer_check "$w" "$ssf" "stale (overridden terminal status)" "$ewf" "$task" fi - # else: already surfaced as genuinely actionable on a prior poll of + # else: already surfaced as genuinely terminal on a prior poll of # this same hash - nothing left to do (matches the original, - # unmodified actionable-status behavior). + # unmodified terminal-status behavior). else # Non-terminal stale: a crew gone quiet without a captain-relevant status. # Decided once per distinct stale hash (the costly state reads run only @@ -1516,7 +1507,7 @@ EOF # heartbeat does not re-fire them (enqueue-before-suppress preserved). fm_wake_append heartbeat heartbeat heartbeat || exit 1 touch "$STATE/.last-heartbeat" - mark_all_captain_relevant_surfaced "$FM_HEARTBEAT_ACTIONABLE_ROWS" + mark_all_captain_relevant_surfaced wake "heartbeat" else touch "$STATE/.last-heartbeat" diff --git a/tests/fm-daemon.test.sh b/tests/fm-daemon.test.sh index dffe3d5660e..93d8cb47ca1 100755 --- a/tests/fm-daemon.test.sh +++ b/tests/fm-daemon.test.sh @@ -102,7 +102,7 @@ test_classify_routine_signal_self() { case "$out" in self\|*) pass "routine signal self-handles" ;; *) fail "routine signal did not self-handle: $out" ;; esac } -test_classify_actionable_signal_escalates() { +test_classify_terminal_signal_escalates() { local dir state kw out dir=$(make_supercase classify-terminal) state="$dir/state" @@ -726,91 +726,6 @@ test_heartbeat_scan_dedup() { pass "catch-all scan escalates a missed terminal once, not twice" } -test_validation_handoff_current_state_guard() { - local dir state fakebin out current_state old_crew_state_bin old_fake_crew_state had_fake_crew_state=0 - dir=$(make_case validation-handoff-current-state) - state="$dir/state" - fakebin="$dir/fakebin" - printf 'needs-validation: implementation committed\n' > "$state/validation.status" - old_crew_state_bin=$FM_CREW_STATE_BIN - if [ "${FM_FAKE_CREW_STATE+x}" = x ]; then - had_fake_crew_state=1 - old_fake_crew_state=$FM_FAKE_CREW_STATE - else - old_fake_crew_state= - fi - export FM_CREW_STATE_BIN="$fakebin/fm-crew-state.sh" - for current_state in \ - 'state: working · source: run-step · validating (running)' \ - 'state: parked · source: run-step · parked at awaiting_approval' \ - 'state: parked · source: run-step · parked at fix_review'; do - export FM_FAKE_CREW_STATE="$current_state" - out=$(FM_STATE_OVERRIDE="$state" classify_stale "sess:fm-validation" "$state") - case "$out" in - self\|*) ;; - *) fail "attributed validation run escalated stale handoff: $out" ;; - esac - rm -f "$state/.subsuper-last-scan" - : > "$state/.subsuper-escalations" - FM_STATE_OVERRIDE="$state" FM_HEARTBEAT_SCAN_SECS=0 housekeeping "$state" - [ ! -s "$state/.subsuper-escalations" ] || fail "attributed validation run was escalated by heartbeat scan" - done - export FM_FAKE_CREW_STATE='state: unknown · source: none · inactive handoff' - out=$(FM_STATE_OVERRIDE="$state" classify_stale "sess:fm-validation" "$state") - case "$out" in - escalate\|*) ;; - *) fail "inactive validation handoff was not actionable: $out" ;; - esac - rm -f "$state/.subsuper-last-scan" - FM_STATE_OVERRIDE="$state" FM_HEARTBEAT_SCAN_SECS=0 housekeeping "$state" - [ -s "$state/.subsuper-escalations" ] || fail "inactive validation handoff was missed by heartbeat scan" - export FM_CREW_STATE_BIN="$old_crew_state_bin" - if [ "$had_fake_crew_state" = 1 ]; then - export FM_FAKE_CREW_STATE="$old_fake_crew_state" - else - unset FM_FAKE_CREW_STATE - fi - pass "active validation suppresses stale handoff paths while inactive handoff remains actionable" -} - -test_validation_handoff_retains_wedge_marker() { - local dir state fakebin win key pane old_crew_state_bin old_fake_crew_state had_fake_crew_state=0 - dir=$(make_case validation-handoff-wedge-marker) - state="$dir/state" - fakebin="$dir/fakebin" - win='sess:fm-validation' - key=validation - pane="$dir/pane.txt" - fm_write_meta "$state/validation.meta" "window=$win" "backend=tmux" - printf 'needs-validation: implementation committed\n' > "$state/validation.status" - printf 'idle prompt $\n' > "$pane" - old_crew_state_bin=$FM_CREW_STATE_BIN - if [ "${FM_FAKE_CREW_STATE+x}" = x ]; then - had_fake_crew_state=1 - old_fake_crew_state=$FM_FAKE_CREW_STATE - else - old_fake_crew_state= - fi - export FM_CREW_STATE_BIN="$fakebin/fm-crew-state.sh" - export FM_FAKE_CREW_STATE='state: parked · source: run-step · parked at fix_review' - FM_STATE_OVERRIDE="$state" handle_wake "stale: $win" "$state" - [ -e "$state/.subsuper-stale-$key" ] || fail "attributed validation handoff lost wedge marker" - [ ! -s "$state/.subsuper-escalations" ] || fail "attributed validation handoff escalated immediately" - export FM_FAKE_CREW_STATE='state: unknown · source: none · validation ended quietly' - echo $(( $(date +%s) - 500 )) > "$state/.subsuper-stale-$key" - PATH="$fakebin:$PATH" FM_STATE_OVERRIDE="$state" FM_FAKE_TMUX_WINDOW="$win" \ - FM_FAKE_TMUX_CAPTURE="$pane" FM_STALE_ESCALATE_SECS=240 housekeeping "$state" - [ -s "$state/.subsuper-escalations" ] || fail "retained wedge marker did not escalate after validation ended" - [ ! -e "$state/.subsuper-stale-$key" ] || fail "retained wedge marker was not cleared after escalation" - export FM_CREW_STATE_BIN="$old_crew_state_bin" - if [ "$had_fake_crew_state" = 1 ]; then - export FM_FAKE_CREW_STATE="$old_fake_crew_state" - else - unset FM_FAKE_CREW_STATE - fi - pass "validation handoff retains wedge aging through a quiet run" -} - test_handle_wake_routes_self_and_escalate() { local dir state dir=$(make_supercase handle) @@ -2013,7 +1928,7 @@ test_afk_start_ignores_stale_pidfile_without_lock test_afk_start_reclaims_stale_daemon_lock_reused_pid test_daemon_state_root_uses_fm_home test_classify_routine_signal_self -test_classify_actionable_signal_escalates +test_classify_terminal_signal_escalates test_classify_check_and_unknown_escalate test_stale_transient_self_records_marker test_stale_diagnostic_wedge_survives_busy_housekeeping @@ -2043,8 +1958,6 @@ test_housekeeping_orca_persistent_stale_resolves_terminal test_escalate_batches_into_one_digest test_escalate_batch_age_uses_first_append test_heartbeat_scan_dedup -test_validation_handoff_current_state_guard -test_validation_handoff_retains_wedge_marker test_handle_wake_routes_self_and_escalate test_inject_skip_forces_self test_is_wake_reason_distinguishes_status_stdout diff --git a/tests/fm-watch-triage.test.sh b/tests/fm-watch-triage.test.sh index c5b19c0cd39..7060bf9f117 100755 --- a/tests/fm-watch-triage.test.sh +++ b/tests/fm-watch-triage.test.sh @@ -25,9 +25,6 @@ set -u WATCH="$ROOT/bin/fm-watch.sh" DRAIN="$ROOT/bin/fm-wake-drain.sh" -# shellcheck source=bin/fm-watch.sh -. "$WATCH" - TMP_ROOT=$(fm_test_tmproot fm-watch-triage-tests) ack_stopped_cycle() { # @@ -191,22 +188,22 @@ test_signal_reason_is_actionable_classifier() { pass "signal_reason_is_actionable: benign absorbed, captain verbs and coalesced batches surfaced" } -test_stale_has_actionable_status_classifier() { +test_stale_is_terminal_classifier() { local dir state dir=$(make_case classify-stale); state="$dir/state" printf 'done: ready in branch fm/x\n' > "$state/term.status" - stale_has_actionable_status "sess:fm-term" "$state" || fail "terminal stale status not classified actionable" + stale_is_terminal "sess:fm-term" "$state" || fail "terminal stale status not classified actionable" fm_write_meta "$state/herdr-term.meta" "window=default:w1:p2" "backend=herdr" printf 'done: ready in branch fm/herdr\n' > "$state/herdr-term.status" - stale_has_actionable_status "default:w1:p2" "$state" || fail "terminal herdr stale status not resolved through metadata" + stale_is_terminal "default:w1:p2" "$state" || fail "terminal herdr stale status not resolved through metadata" printf 'needs-validation: implementation committed\n' > "$state/handoff.status" - stale_has_actionable_status "sess:fm-handoff" "$state" || fail "non-terminal validation handoff not classified actionable" + stale_is_terminal "sess:fm-handoff" "$state" || fail "non-terminal validation handoff not classified actionable" status_is_terminal_verb "$(last_status_line "$state/handoff.status")" \ && fail "actionable validation handoff was classified terminal" printf 'working: compiling\n' > "$state/nonterm.status" - stale_has_actionable_status "sess:fm-nonterm" "$state" && fail "routine non-terminal stale classified actionable" - stale_has_actionable_status "sess:fm-missing" "$state" && fail "stale with no status classified actionable" - pass "stale_has_actionable_status separates actionable handoffs/results from routine progress" + stale_is_terminal "sess:fm-nonterm" "$state" && fail "routine non-terminal stale classified actionable" + stale_is_terminal "sess:fm-missing" "$state" && fail "stale with no status classified actionable" + pass "stale_is_terminal treats needs-validation as actionable but non-terminal" } test_scan_captain_relevant_statuses_classifier() { @@ -797,12 +794,12 @@ test_terminal_stale_surfaced() { # log gets no new entry once firstmate hands it to a no-mistakes validation # (AGENTS.md's sparse status-reporting contract), so the log keeps showing its # pre-validation needs-validation: line as the LAST line for the run's entire -# (possibly many-minutes) duration. stale_has_actionable_status alone has no run-step awareness and +# (possibly many-minutes) duration. stale_is_terminal alone has no run-step awareness and # would treat that leftover as still-current every time the pane goes quiet, # immediately surfacing a crew that is actively validating. crew_is_provably_working # must get a chance to override a captain-relevant-but-stale status line, exactly # as it already does for a plain non-terminal one. -test_stale_actionable_status_overridden_by_active_run() { +test_stale_terminal_status_overridden_by_active_run() { local dir state fakebin out drain_out capture_file window key pane_hash sig pid dir=$(make_case terminal-stale-overridden); state="$dir/state"; fakebin="$dir/fakebin" out="$dir/watch.out"; drain_out="$dir/drain.out"; capture_file="$dir/pane.txt" @@ -2537,35 +2534,6 @@ test_heartbeat_backstop_surfaces_unsurfaced_status() { pass "heartbeat backstop fail-safe surfaces a captain-relevant status the per-wake path missed" } -test_heartbeat_scan_reuses_current_state_for_marking() { - local dir state fakebin calls - dir=$(make_case heartbeat-scan-reuse); state="$dir/state"; fakebin="$dir/fakebin" - calls="$dir/crew-state-calls" - printf 'needs-validation: implementation committed\n' > "$state/needs-validation-task.status" - cat > "$fakebin/fm-crew-state.sh" <<'SH' -#!/usr/bin/env bash -set -u -calls=${FM_CREW_STATE_CALLS:?} -count=$(cat "$calls" 2>/dev/null || echo 0) -printf '%s\n' "$((count + 1))" > "$calls" -printf '%s\n' 'state: stopped · source: none · fake' -SH - chmod +x "$fakebin/fm-crew-state.sh" - FM_CREW_STATE_BIN="$fakebin/fm-crew-state.sh" - FM_CREW_STATE_CALLS="$calls" - export FM_CREW_STATE_CALLS - # shellcheck disable=SC2034 - STATE="$state" - heartbeat_scan_finds_actionable \ - || fail "heartbeat scan did not find the inactive validation handoff" - mark_all_captain_relevant_surfaced "$FM_HEARTBEAT_ACTIONABLE_ROWS" - [ "$(cat "$calls")" = 1 ] \ - || fail "heartbeat detection and marking re-read current state: $(cat "$calls")" - [ "$(cat "$state/.hb-surfaced-needs-validation-task")" = 'needs-validation: implementation committed' ] \ - || fail "heartbeat marking did not reuse the detected status row" - pass "heartbeat detection reuses one current-state verdict while marking" -} - # --- beacon stays fresh while absorbing ------------------------------------- test_beacon_stays_fresh_while_absorbing() { @@ -2659,7 +2627,7 @@ test_afk_paused_changed_pane_hands_off_plain_stale() { } test_signal_reason_is_actionable_classifier -test_stale_has_actionable_status_classifier +test_stale_is_terminal_classifier test_scan_captain_relevant_statuses_classifier test_classifier_primitives test_crew_is_provably_working_classifier @@ -2679,7 +2647,7 @@ test_secondmate_status_note_surfaced_despite_busy_agent test_self_announced_close_does_not_rewake_but_next_note_does test_actionable_signal_surfaced test_terminal_stale_surfaced -test_stale_actionable_status_overridden_by_active_run +test_stale_terminal_status_overridden_by_active_run test_nonterminal_stale_provably_working_absorbed_then_escalated test_wedge_escalation_marks_demand_deep_inspection_after_threshold test_wedge_escalation_resets_when_pane_becomes_active @@ -2715,7 +2683,6 @@ test_procevent_surface_crash_boundaries test_procevent_marker_failure_exits_and_replays test_heartbeat_no_change_absorbed test_heartbeat_backstop_surfaces_unsurfaced_status -test_heartbeat_scan_reuses_current_state_for_marking test_beacon_stays_fresh_while_absorbing test_afk_present_reverts_watcher_to_one_shot test_afk_paused_changed_pane_hands_off_plain_stale From 3db189f87d96baee41a1bb8496bb4f17e8e9ce7f Mon Sep 17 00:00:00 2001 From: connectwithclayton-cpu <278079184+connectwithclayton@users.noreply.github.com> Date: Sun, 23 Aug 2026 22:27:58 -0400 Subject: [PATCH 6/7] no-mistakes(document): Documented validation handoff classification --- docs/architecture.md | 2 +- docs/configuration.md | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/docs/architecture.md b/docs/architecture.md index cd0318c097b..4bb59aafad0 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -58,7 +58,7 @@ During no-mistakes' `ci` monitor phase, it also reads the ci step log tail becau 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. Only when no matching run exists does it consult semantic busy state; exact busy reports working, exact idle permits fallback to a status-log event whose verb maps to a recognized run-state, and unknown or a dead pane stays 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, while the generated brief's non-terminal `needs-validation:` implementation handoff reports `parked` until an attributed validation run supersedes it. The semantic branch reports working only on an exact busy verdict and names the source that produced it; an unknown verdict never becomes working, never permits the status-log fallback, and never becomes a silent idle. 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/docs/configuration.md b/docs/configuration.md index 0031609163e..82cf18c43ef 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -700,6 +700,7 @@ FM_WATCH_CYCLE_LOG_MAX_BYTES=262144 # size cap for the arm-owned watcher lifec FM_WATCH_CYCLE_LOG_KEEP_LINES=1000 # newest complete lifecycle rows considered when the ledger is capped FM_WATCHER_STALE_GRACE=300 # defaults to FM_GUARD_GRACE; seconds a live watcher lock may have a stale beacon before re-arm errors FM_SIGNAL_GRACE=30 # seconds to coalesce nearby status and turn-end signals into one wake +FM_CLASSIFY_NEEDS_VALIDATION_VERB=needs-validation # leading status verb for the non-terminal no-mistakes implementation handoff; firstmate triggers validation on the same worker FM_CAPTAIN_RE='done:|needs-validation:|needs-decision:|blocked:|failed:|PR ready|checks green|ready in branch|merged' # captain-relevant status regex; routine nonterminal progress verbs remain excluded even when their prose matches FM_CLASSIFY_PAUSED_VERB=paused # leading status verb for a declared external wait; excluded from FM_CAPTAIN_RE and distinct from blocked FM_STALE_ESCALATE_SECS=240 # idle seconds before a provably-working stale pane escalates; stale panes whose crew is not provably working surface immediately unless they declare the pause verb From 1afa3dc98682dfc7d88f3bb8a40625a9186b2276 Mon Sep 17 00:00:00 2001 From: connectwithclayton-cpu <278079184+connectwithclayton@users.noreply.github.com> Date: Mon, 24 Aug 2026 11:54:07 -0400 Subject: [PATCH 7/7] fix(classify): preserve validation handoff under overrides --- bin/fm-classify-lib.sh | 10 +++++++--- tests/fm-watch-triage.test.sh | 8 +++++++- 2 files changed, 14 insertions(+), 4 deletions(-) diff --git a/bin/fm-classify-lib.sh b/bin/fm-classify-lib.sh index 2337a00a65f..7d5ba6d6bd2 100755 --- a/bin/fm-classify-lib.sh +++ b/bin/fm-classify-lib.sh @@ -58,8 +58,9 @@ FM_CLASSIFY_NEEDS_VALIDATION_VERB_DEFAULT='needs-validation' # Captain-relevant status verbs. A status line carrying any of these is work # firstmate must see. Lines without these verbs are no-verb signals: the watcher # absorbs them only with positive provably-working evidence, while the daemon uses -# its away-mode classification. FM_CAPTAIN_RE overrides the whole set when a home -# needs a custom verb vocabulary; absent, this default applies. +# its away-mode classification. FM_CAPTAIN_RE replaces the general set when a home +# needs a custom verb vocabulary, but cannot suppress the required implementation +# handoff; absent, this default applies. # # Free-text tokens (PR ready, checks green, ready in branch, merged) exist only for # legacy lines that lack a standard terminal verb. status_is_captain_relevant is @@ -132,10 +133,13 @@ status_is_captain_relevant() { working|resolved|captain-held|"${FM_CLASSIFY_PAUSED_VERB:-$FM_CLASSIFY_PAUSED_VERB_DEFAULT}") return 1 ;; + "${FM_CLASSIFY_NEEDS_VALIDATION_VERB:-$FM_CLASSIFY_NEEDS_VALIDATION_VERB_DEFAULT}") + return 0 + ;; esac if [ -z "${FM_CAPTAIN_RE+x}" ]; then case "$verb" in - done|"${FM_CLASSIFY_NEEDS_VALIDATION_VERB:-$FM_CLASSIFY_NEEDS_VALIDATION_VERB_DEFAULT}"|needs-decision|blocked|failed) return 0 ;; + done|needs-decision|blocked|failed) return 0 ;; esac fi printf '%s' "$line" | grep -qiE "${FM_CAPTAIN_RE:-$FM_CLASSIFY_CAPTAIN_RE_DEFAULT}" diff --git a/tests/fm-watch-triage.test.sh b/tests/fm-watch-triage.test.sh index 7060bf9f117..c19978214fb 100755 --- a/tests/fm-watch-triage.test.sh +++ b/tests/fm-watch-triage.test.sh @@ -222,7 +222,7 @@ test_scan_captain_relevant_statuses_classifier() { } test_classifier_primitives() { - local dir state open activity + local dir state open activity legacy_captain_re dir=$(make_case classify-primitives); state="$dir/state" printf 'working: a\n\ndone: b\n\n' > "$state/x.status" [ "$(last_status_line "$state/x.status")" = "done: b" ] || fail "last_status_line did not return the last non-blank line" @@ -260,6 +260,12 @@ test_classifier_primitives() { [ "$(window_to_task "default:w1:p2" "$state")" = "herdr-task" ] || fail "window_to_task did not resolve opaque backend target through metadata" FM_CAPTAIN_RE='custom-verb:' status_is_captain_relevant "custom-verb: x" || fail "FM_CAPTAIN_RE override not honored" FM_CAPTAIN_RE='custom-verb:' status_is_captain_relevant "done: x" && fail "FM_CAPTAIN_RE override did not replace the default verb set" + legacy_captain_re='done:|needs-decision:|blocked:|failed:|PR ready|checks green|ready in branch|merged' + FM_CAPTAIN_RE="$legacy_captain_re" status_is_captain_relevant "needs-validation: implementation committed" \ + || fail "a legacy FM_CAPTAIN_RE override suppressed the required validation handoff" + FM_CLASSIFY_NEEDS_VALIDATION_VERB=validate-next FM_CAPTAIN_RE="$legacy_captain_re" \ + status_is_captain_relevant "validate-next: implementation committed" \ + || fail "the overridden validation handoff verb was suppressed by FM_CAPTAIN_RE" FM_CAPTAIN_RE='merged|custom-verb:' status_is_captain_relevant "working: rebased onto merged #76" \ && fail "FM_CAPTAIN_RE override bypassed working: suppression" FM_CAPTAIN_RE='checks green|custom-verb:' status_is_captain_relevant "paused: checks green pending approval" \