From 14afd7e856a13d34cc7c1436af41c3afc26783c4 Mon Sep 17 00:00:00 2001 From: Alex William Date: Tue, 8 Sep 2026 15:22:14 +0200 Subject: [PATCH 1/8] Harden supervision reliability transitions --- AGENTS.md | 3 +- bin/fm-busy-lib.sh | 50 +++-- bin/fm-classify-lib.sh | 85 +++++++- bin/fm-control.sh | 71 ++++++- bin/fm-crew-state.sh | 45 ++++- bin/fm-fleet-snapshot.sh | 43 +++- bin/fm-supervise-daemon.sh | 2 +- bin/fm-teardown.sh | 2 + bin/fm-wake-drain.sh | 25 ++- bin/fm-watch-arm.sh | 200 +++++++++++++++---- bin/fm-watch.sh | 73 ++++++- docs/agent-control.md | 2 +- docs/configuration.md | 2 +- docs/watcher-continuity.md | 4 +- tests/fm-busy-state.test.sh | 51 ++++- tests/fm-control.test.sh | 44 ++++ tests/fm-crew-state.test.sh | 56 +++++- tests/fm-daemon.test.sh | 31 +++ tests/fm-fleet-snapshot-view.test.sh | 43 ++++ tests/fm-wake-drain-outcome-backstop.test.sh | 39 ++++ tests/fm-watch-arm.test.sh | 114 ++++++++++- tests/fm-watch-triage.test.sh | 98 +++++++++ tests/wake-helpers.sh | 5 + 23 files changed, 990 insertions(+), 98 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index ca09ab7a4cf..fee5378ecb5 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -102,6 +102,7 @@ state/ runtime records and signals; gitignored .gemini-settings.json firstmate-owned per-task Gemini settings carrying the busy-state and turn-end hooks, reached through GEMINI_CLI_SYSTEM_SETTINGS_PATH so nothing is written into the project's own .gemini/; removed by teardown .muse-session muse busy-source binding (sessions root plus task worktree) written by fm-spawn; removed by teardown .cursor-session cursor busy-source binding (projects root, task worktree, prior conversations) written by fm-spawn; removed by teardown + .voluntary-exit durable record that an explicit exit stopped the agent while a PR merge poll is still armed; removed by relaunch, teardown, and when the poll sidecar is gone (bin/fm-control.sh) .reconcile-nudged epoch second of the last inventory-reconcile nudge sent to this secondmate; bin/fm-secondmate-reconcile.sh owns its per-home cooldown window .backlog-close the exact backlog transition a teardown recorded before removing the task's record, so an interrupted cleanup can still be finished at the next session start; bin/fm-backlog-transition-lib.sh owns its format and replay, and a landed transition removes it .inbox/ durable steering inbox: sequenced firstmate instruction records the worker acknowledges by moving them into its handled/ subdirectory; written by fm-send, with ordinary records re-rung and escalated by the watcher while explicit fire-and-forget records are excluded from that ladder, and removed by teardown (bin/fm-task-inbox-lib.sh) @@ -135,7 +136,7 @@ state/ runtime records and signals; gitignored .wake-queue durable queued wakes retained until post-handling acknowledgement: epochseqkindkeypayload .watcher-down private generation-bound recovery state coupling watcher downtime, durable wake presentation, and post-handling acknowledgement; never touch ..open-decisions-cursor per-task byte cursor and folded open-decision set bounding the OPEN DECISIONS scan's cost to new status-log appends; written only by fm-classify-lib.sh's status_open_decisions_incremental, removed by teardown, safe to delete (forces one full re-fold) - .status-presentation-cursor .status-presentation-lock fleet-wide per-task status identity plus independent annotation and outcome-backstop byte offsets, with a serialization lock preventing already-presented lines from replaying while preserving delayed signal annotations; owned by fm-classify-lib.sh, with each task's row retired by teardown + .status-presentation-cursor .status-presentation-lock .status-outcome-identity fleet-wide per-task status identity plus independent annotation and outcome-backstop byte offsets, a serialization lock preventing already-presented lines from replaying while preserving delayed signal annotations, and a stable terminal-result identity so a rewritten log does not re-announce the same outcome; owned by fm-classify-lib.sh, with each task's row retired by teardown .afk durable away-mode flag; present = sub-supervisor may inject escalations (set by /afk, cleared on user return) .watch.lock .wake-queue.lock watcher singleton and queue serialization locks .claude-autoarm.lock .claude-autoarm-epoch .claude-autoarm-failure-notified .claude-autoarm-failure-alarmed .turnend-claude-blocks .turnend-claude-blocks.lock Claude Stop auto-arm single-flight, epoch, failure-episode, attended-alarm, guard-budget, and budget-lock records; never touch diff --git a/bin/fm-busy-lib.sh b/bin/fm-busy-lib.sh index 27e952cab62..84e138ec23d 100755 --- a/bin/fm-busy-lib.sh +++ b/bin/fm-busy-lib.sh @@ -42,21 +42,25 @@ # fm-interrupt the legacy Claude fm-send --key Escape idle event # fm-recovery a documented recovery reset after relaunch # Classifier-only sources (never written into a record): -# endpoint-gone, herdr-native, grok-regex, rovo-regex, muse-session-log, -# cursor-transcript, missing, malformed, gen-mismatch, source-mismatch, -# kimi-unverified, codex-unverified, capture-failed, no-target +# endpoint-gone, shell-no-agent, herdr-native, grok-regex, rovo-regex, +# muse-session-log, cursor-transcript, missing, malformed, gen-mismatch, +# source-mismatch, kimi-unverified, codex-unverified, capture-failed, +# no-target # # Classification (fm_busy_classify): busy | idle | unknown | dead, always # with the producing source as the second token. Precedence: # 1. dead endpoint (fm_busy_classify_live only) -> dead endpoint-gone -# 2. standalone Kimi before verification -> unknown kimi-unverified -# 3. a valid, gen-matching, source-trusted record -> its state and source -# 4. no record at all: herdr's native busy verdict is trusted as busy +# 2. pane that exists with no agent, recovery-grade `dead` (live only) +# -> dead shell-no-agent; this structural verdict wins regardless of +# Cursor, Grok, Muse, Codex, or Kimi classifier order +# 3. standalone Kimi before verification -> unknown kimi-unverified +# 4. a valid, gen-matching, source-trusted record -> its state and source +# 5. no record at all: herdr's native busy verdict is trusted as busy # (generation state is sufficient for busy, not for idle), then the # muse session-log and cursor transcript pull sources, then the Grok/Rovo # temporary regex fallbacks classify a grok or rovo task from its # rendered tail, then unknown missing -# 5. malformed, stale, or untrusted records -> unknown, never a fallback +# 6. malformed, stale, or untrusted records -> unknown, never a fallback # Grok and Rovo are the ONLY rendered-text classifications that survive the # redesign, because neither's structured lifecycle was credited-live-verified # in the approved audit (Rovo's clean ACP stopReason lives outside the TUI @@ -987,11 +991,17 @@ fm_busy_classify() { # [tail40] printf 'unknown missing' } -# fm_busy_classify_live: fm_busy_classify behind the one process-level -# override - a gone endpoint is dead, never busy. Requires fm-backend.sh to -# be sourced for fm_backend_target_exists. -fm_busy_classify_live() { # [expected-label] - local backend=$1 target=$2 harness=$3 id=$4 state=$5 label=${6-} +# fm_busy_classify_live: fm_busy_classify behind the process-level overrides. +# A gone endpoint is dead, never busy. A pane that still exists but whose +# recovery-grade classifier reports `dead` (nothing but a shell) is +# `dead shell-no-agent`, and that structural verdict wins before Cursor, +# Grok, Muse, Codex, or Kimi classifiers run. Ambiguous, unreadable, and +# unverified agent-state results do not override harness classifiers. +# Requires fm-backend.sh to be sourced for fm_backend_target_exists. +# Optional is forwarded to the Grok/Rovo arms unchanged. +fm_busy_classify_live() { # [expected-label] [tail40] + local backend=$1 target=$2 harness=$3 id=$4 state=$5 label=${6-} tail40=${7-} + local agent_state if [ -z "$target" ]; then printf 'unknown no-target' return 0 @@ -1000,13 +1010,23 @@ fm_busy_classify_live() { # [expe printf 'dead endpoint-gone' return 0 fi - fm_busy_classify "$backend" "$target" "$harness" "$id" "$state" + if command -v fm_backend_agent_state >/dev/null 2>&1; then + agent_state=$(fm_backend_agent_state "$backend" "$target" 2>/dev/null || true) + case "$agent_state" in + dead) + printf 'dead shell-no-agent' + return 0 + ;; + esac + fi + fm_busy_classify "$backend" "$target" "$harness" "$id" "$state" "$tail40" } # fm_busy_classify_meta: classify a task from its recorded metadata, so every # consumer resolves backend, target, and harness the same way instead of # re-deriving them. Requires fm-backend.sh to be sourced. is -# optional pre-captured plain output reused by the Grok arm. +# optional pre-captured plain output reused by the Grok arm. Process-level +# overrides (gone endpoint, shell without agent) run through classify_live. fm_busy_classify_meta() { # [tail40] local meta=$1 id=$2 state=$3 tail40=${4-} backend target harness [ -f "$meta" ] || { printf 'unknown missing'; return 0; } @@ -1017,7 +1037,7 @@ fm_busy_classify_meta() { # [tail40] printf 'unknown no-target' return 0 fi - fm_busy_classify "$backend" "$target" "$harness" "$id" "$state" "$tail40" + fm_busy_classify_live "$backend" "$target" "$harness" "$id" "$state" "fm-$id" "$tail40" } # fm_busy_is_busy: boolean view for callers that only gate on provable diff --git a/bin/fm-classify-lib.sh b/bin/fm-classify-lib.sh index 8bd4fe746ad..ad1d7e0c124 100755 --- a/bin/fm-classify-lib.sh +++ b/bin/fm-classify-lib.sh @@ -982,6 +982,63 @@ EOF printf '%s' "$offset" } +# Stable identity of one captain-facing status event, independent of the +# status file's inode or byte offset. A rewritten log that still ends on the +# same terminal result keeps this identity; a genuinely new event does not. +status_terminal_event_identity() { # + local line=$1 + printf '%s' "$line" | LC_ALL=C tr -d '\r' | cksum | awk '{printf "e1:%s-%s", $1, $2}' +} + +status_outcome_identity_path() { # + printf '%s/.status-outcome-identity' "$1" +} + +status_outcome_identity_get() { # + local state=$1 task=$2 path row_task hash extra + path=$(status_outcome_identity_path "$state") + [ -f "$path" ] && [ -r "$path" ] && [ ! -L "$path" ] || return 1 + while IFS=$(printf '\t') read -r row_task hash extra; do + [ -n "$row_task" ] || continue + [ -z "$extra" ] || continue + [ -n "$hash" ] || continue + if [ "$row_task" = "$task" ]; then + printf '%s' "$hash" + return 0 + fi + done < "$path" + return 1 +} + +status_outcome_identity_commit() { # + local state=$1 snapshot=$2 path tmp row_task hash extra seen='' line task + path=$(status_outcome_identity_path "$state") + tmp="$path.tmp.$$" + : > "$tmp" || return 1 + if [ -f "$path" ] && [ -r "$path" ] && [ ! -L "$path" ]; then + while IFS=$(printf '\t') read -r row_task hash extra; do + [ -n "$row_task" ] || continue + [ -z "$extra" ] || continue + [ -n "$hash" ] || continue + case " +$snapshot +" in *$'\n'"$row_task"$'\t'*) continue ;; esac + printf '%s\t%s\n' "$row_task" "$hash" >> "$tmp" || { rm -f "$tmp"; return 1; } + done < "$path" + elif [ -e "$path" ] || [ -L "$path" ]; then + rm -f "$tmp" + return 1 + fi + while IFS=$(printf '\t') read -r task hash; do + [ -n "$task" ] || continue + [ -n "$hash" ] || continue + printf '%s\t%s\n' "$task" "$hash" >> "$tmp" || { rm -f "$tmp"; return 1; } + done < local f=$1 state task manifest data row_task ident presented row_backstop backstop extra current size [ -f "$f" ] && [ -r "$f" ] && [ ! -L "$f" ] || return 1 @@ -1148,7 +1205,7 @@ status_presentation_marker_commit() { status_retire_presentation_task() { # local state=$1 task=$2 lock manifest tmp data row_task ident offset backstop extra rc=0 found=0 - local signal_marker heartbeat_marker daemon_marker + local signal_marker heartbeat_marker daemon_marker identity_path identity_tmp identity_row identity_hash identity_extra lock="$state/.status-presentation-lock" manifest="$state/.status-presentation-cursor" tmp="$manifest.tmp.$$" @@ -1212,6 +1269,25 @@ EOF fi fi if [ "$rc" -eq 0 ]; then + identity_path=$(status_outcome_identity_path "$state") + if [ -f "$identity_path" ] && [ -r "$identity_path" ] && [ ! -L "$identity_path" ]; then + identity_tmp="$identity_path.tmp.$$" + if : > "$identity_tmp"; then + while IFS=$(printf '\t') read -r identity_row identity_hash identity_extra; do + [ -n "$identity_row" ] || continue + [ "$identity_row" = "$task" ] && continue + [ -z "$identity_extra" ] || continue + [ -n "$identity_hash" ] || continue + printf '%s\t%s\n' "$identity_row" "$identity_hash" >> "$identity_tmp" || rc=1 + done < "$identity_path" + if [ "$rc" -eq 0 ]; then + mv -f "$identity_tmp" "$identity_path" || rc=1 + fi + [ "$rc" -eq 0 ] || rm -f "$identity_tmp" + else + rc=1 + fi + fi rm -f -- "$state/$task.status" "$state/.$task.open-decisions-cursor" \ "$signal_marker" "$heartbeat_marker" "$daemon_marker" || rc=1 fi @@ -1254,7 +1330,7 @@ EOF } status_commit_presentation_snapshot() { # - local state=$1 snapshot=$2 task endpoint ident f cur_ident size tmp backstop acknowledged_task acknowledged_endpoint + local state=$1 snapshot=$2 task endpoint ident f cur_ident size tmp backstop acknowledged_task acknowledged_endpoint acknowledged_hash tmp="$state/.status-presentation-cursor.tmp.$$" : > "$tmp" || return 1 while IFS=$(printf '\t') read -r task endpoint ident; do @@ -1270,7 +1346,7 @@ status_commit_presentation_snapshot() { # [ "$cur_ident" = "$ident" ] && [ "$endpoint" -le "$size" ] \ || { rm -f "$tmp"; return 1; } backstop=$(status_outcome_backstop_cursor_offset "$f") || { rm -f "$tmp"; return 1; } - while IFS=$(printf '\t') read -r acknowledged_task acknowledged_endpoint; do + while IFS=$(printf '\t') read -r acknowledged_task acknowledged_endpoint acknowledged_hash; do if [ "$acknowledged_task" = "$task" ]; then backstop=$acknowledged_endpoint; fi done < diff --git a/bin/fm-control.sh b/bin/fm-control.sh index 21146188416..f3dfad442be 100755 --- a/bin/fm-control.sh +++ b/bin/fm-control.sh @@ -30,7 +30,10 @@ # every uncommitted change. Interrupts first when the task reads # busy, then submits the harness's exit command. Postcondition: # the backend's recovery-grade classifier reports the agent gone. -# Already-stopped is success (idempotent). +# Already-stopped is success (idempotent). When the task still has +# an armed PR merge poll, exit records state/.voluntary-exit so +# supervision treats the dead pane as an expected external wait +# instead of a repeating stale alarm, without dropping the poll. # relaunch Transactionally replace the running agent with a new one, in the # SAME endpoint and SAME worktree, on the same or a newly chosen # harness/model/effort - so switching harness is one ordinary use @@ -444,14 +447,68 @@ retire_busy_incarnation() { fi } +# state/.voluntary-exit is the durable record that an explicit exit verb +# stopped the agent while an external wait (an armed PR merge poll) still +# stands. Schema: +# schema=fm-voluntary-exit.v1 +# reason=external-wait +# wait=pr-poll +# exited_at= +# Relaunch removes it. Teardown removes it. The watcher ignores it once the +# poll sidecar is gone, so a later genuine death is not hidden. +record_voluntary_exit_wait() { + local rec tmp exited_at + [ -f "$STATE/$ID.pr-poll" ] && [ ! -L "$STATE/$ID.pr-poll" ] || { + rm -f "$STATE/$ID.voluntary-exit" + return 0 + } + rec="$STATE/$ID.voluntary-exit" + tmp="$rec.tmp.$$" + exited_at=$(date +%s) || return 1 + case "$exited_at" in ''|*[!0-9]*) return 1 ;; esac + { + printf 'schema=fm-voluntary-exit.v1\n' + printf 'reason=external-wait\n' + printf 'wait=pr-poll\n' + printf 'exited_at=%s\n' "$exited_at" + } > "$tmp" || { rm -f "$tmp"; return 1; } + chmod 600 "$tmp" 2>/dev/null || true + mv -f "$tmp" "$rec" +} + +voluntary_exit_wait_record_valid() { + local rec="$STATE/$ID.voluntary-exit" + [ -f "$rec" ] && [ -r "$rec" ] && [ ! -L "$rec" ] \ + && [ -f "$STATE/$ID.pr-poll" ] && [ ! -L "$STATE/$ID.pr-poll" ] \ + && grep -qxF 'schema=fm-voluntary-exit.v1' "$rec" \ + && grep -qxF 'reason=external-wait' "$rec" \ + && grep -qxF 'wait=pr-poll' "$rec" \ + && grep -qxE 'exited_at=[0-9]+' "$rec" \ + && [ "$(wc -l < "$rec" | tr -d '[:space:]')" = 4 ] +} + +clear_voluntary_exit_wait() { + rm -f "$STATE/$ID.voluntary-exit" +} + # do_exit: stop the running agent, preserving endpoint and worktree. Prints -# `already-stopped` or `stopped`. +# `already-stopped` or `stopped`. Pass `record-wait` from the exit verb so an +# armed PR poll is remembered as an expected external wait. Relaunch omits +# that token and clears any prior record. do_exit() { - local state cmd verdict cancel interrupt_result=not-needed + local state cmd verdict cancel interrupt_result=not-needed record_wait=${1:-} require_state_verified_backend exit state=$(agent_state) case "$state" in dead) + if [ "$record_wait" = record-wait ]; then + # Idempotence may preserve a record written by an earlier successful + # exit, but an agent already found dead was not stopped by this call. + # Never mint a voluntary-wait record that could hide that true death. + voluntary_exit_wait_record_valid || clear_voluntary_exit_wait + else + clear_voluntary_exit_wait + fi printf 'already-stopped' return 0 ;; @@ -493,6 +550,12 @@ do_exit() { # The incarnation is over: retire its busy wiring so no stale record or # orphaned generation survives the agent that produced it. retire_busy_incarnation + if [ "$record_wait" = record-wait ]; then + record_voluntary_exit_wait \ + || die "agent stopped but its external-wait record could not be persisted" + else + clear_voluntary_exit_wait + fi printf 'stopped' } @@ -871,7 +934,7 @@ case "$VERB" in echo "interrupt-delivered $ID harness=$HARNESS backend=$BACKEND verified=$proof" ;; exit) - result=$(do_exit) + result=$(do_exit record-wait) echo "$result $ID harness=$HARNESS backend=$BACKEND endpoint=$T worktree=$WT" ;; relaunch) diff --git a/bin/fm-crew-state.sh b/bin/fm-crew-state.sh index f3a99c3e3e5..65e4518512f 100755 --- a/bin/fm-crew-state.sh +++ b/bin/fm-crew-state.sh @@ -108,10 +108,28 @@ STATE="${FM_STATE_OVERRIDE:-$FM_HOME/state}" ID=${1:-} [ -n "$ID" ] || { echo "usage: fm-crew-state.sh " >&2; exit 2; } -# Fleet snapshot composition supplies its captured metadata path here so every -# state read resolves the same task generation selected by that snapshot. -META=${FM_CREW_STATE_META_OVERRIDE:-"$STATE/$ID.meta"} -LOG=${FM_CREW_STATE_STATUS_OVERRIDE:-"$STATE/$ID.status"} +# Fleet snapshot composition supplies captured metadata through these +# overrides, confined to that child process. A set override - including an +# empty value - is never treated as "use the default path": an absent path or +# a path whose basename belongs to another task is refused so a leftover +# snapshot variable cannot make every worker look missing. +fm_crew_state_apply_override() { # + local dest=$1 is_set=$2 override=$3 default_path=$4 expected=$5 + if [ "$is_set" != 1 ]; then + printf -v "$dest" '%s' "$default_path" + return 0 + fi + if [ -z "$override" ] || [ ! -f "$override" ] || [ -L "$override" ]; then + printf -v "$dest" 'missing' + return 1 + fi + if [ "$(basename "$override")" != "$expected" ]; then + printf -v "$dest" 'foreign' + return 1 + fi + printf -v "$dest" '%s' "$override" +} + NM_TIMEOUT=${FM_CREW_STATE_NM_TIMEOUT:-10} case "$NM_TIMEOUT" in ''|*[!0-9]*) NM_TIMEOUT=10 ;; esac # How many of the most recent `no-mistakes runs` rows the cross-branch fallback @@ -132,6 +150,23 @@ emit() { # [detail] # --- meta resolution -------------------------------------------------------- +_crew_meta_set=0 +_crew_status_set=0 +[ "${FM_CREW_STATE_META_OVERRIDE+x}" = x ] && _crew_meta_set=1 +[ "${FM_CREW_STATE_STATUS_OVERRIDE+x}" = x ] && _crew_status_set=1 +if ! fm_crew_state_apply_override META "$_crew_meta_set" "${FM_CREW_STATE_META_OVERRIDE-}" "$STATE/$ID.meta" "$ID.meta"; then + if [ "$META" = foreign ]; then + emit unknown none "snapshot override belongs to another task ($ID.meta)" + fi + emit unknown none "snapshot override path missing for $ID.meta" +fi +if ! fm_crew_state_apply_override LOG "$_crew_status_set" "${FM_CREW_STATE_STATUS_OVERRIDE-}" "$STATE/$ID.status" "$ID.status"; then + if [ "$LOG" = foreign ]; then + emit unknown none "snapshot override belongs to another task ($ID.status)" + fi + emit unknown none "snapshot override path missing for $ID.status" +fi + [ -f "$META" ] || emit unknown none "no metadata for $ID" meta_value() { # @@ -247,7 +282,7 @@ crew_busy_verdict() { # case "$HARNESS" in grok*) tail40=$(fm_backend_capture "$TASK_BACKEND" "$1" 40 "$EXPECTED_LABEL" 2>/dev/null) || tail40='' ;; esac - fm_busy_classify "$TASK_BACKEND" "$1" "$HARNESS" "$ID" "$STATE" "$tail40" + fm_busy_classify_live "$TASK_BACKEND" "$1" "$HARNESS" "$ID" "$STATE" "$EXPECTED_LABEL" "$tail40" } # --- no-mistakes run lookup (authoritative when a run matches this branch) -- diff --git a/bin/fm-fleet-snapshot.sh b/bin/fm-fleet-snapshot.sh index 5ef8ffc35d1..4d59f5fa1da 100755 --- a/bin/fm-fleet-snapshot.sh +++ b/bin/fm-fleet-snapshot.sh @@ -111,6 +111,9 @@ SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" FM_ROOT="${FM_ROOT_OVERRIDE:-$(cd "$SCRIPT_DIR/.." && pwd)}" FM_HOME="${FM_HOME:-${FM_ROOT_OVERRIDE:-$FM_ROOT}}" STATE="${FM_STATE_OVERRIDE:-$FM_HOME/state}" +# Snapshot overrides are child-only. Drop any inherited leak so a prior +# snapshot cannot make every later crew-state read look missing or foreign. +unset FM_CREW_STATE_META_OVERRIDE FM_CREW_STATE_STATUS_OVERRIDE DATA="${FM_DATA_OVERRIDE:-$FM_HOME/data}" CONFIG="${FM_CONFIG_OVERRIDE:-$FM_HOME/config}" PROJECTS="${FM_PROJECTS_OVERRIDE:-$FM_HOME/projects}" @@ -294,18 +297,42 @@ last_nonempty_line() { # # A local crew-state read is bounded so one slow child cannot extend this # snapshot without limit. Remote secondmate endpoint liveness is never read here. # A local read that hits the bound folds to state unknown. +# Snapshot overrides live only in this child env: they are never exported in +# this process, and an absent or foreign captured path is omitted rather than +# passed as an empty override that would make every later worker look missing. crew_state_json() { # [] [] local id=$1 captured_meta=${2:-} captured_status=${3:-} raw rest state source detail sep + local -a crew_env + crew_env=( + FM_ROOT_OVERRIDE="$FM_ROOT" + FM_HOME="$FM_HOME" + FM_STATE_OVERRIDE="$STATE" + FM_DATA_OVERRIDE="$DATA" + FM_PROJECTS_OVERRIDE="$PROJECTS" + FM_CONFIG_OVERRIDE="$CONFIG" + ) + if [ -n "$captured_meta" ] && [ -f "$captured_meta" ] && [ ! -L "$captured_meta" ] \ + && [ "$(basename "$captured_meta")" = "$id.meta" ]; then + crew_env+=(FM_CREW_STATE_META_OVERRIDE="$captured_meta") + elif [ -n "$captured_meta" ]; then + jq -n --arg raw '' --arg state unknown --arg source none \ + --arg detail "snapshot override path missing or belongs to another task ($id.meta)" \ + '{state:$state,source:$source,detail:$detail,raw:$raw}' + return 0 + fi + if [ -n "$captured_status" ] && [ -f "$captured_status" ] && [ ! -L "$captured_status" ] \ + && [ "$(basename "$captured_status")" = "$id.status" ]; then + crew_env+=(FM_CREW_STATE_STATUS_OVERRIDE="$captured_status") + elif [ -n "$captured_status" ]; then + jq -n --arg raw '' --arg state unknown --arg source none \ + --arg detail "snapshot override path missing or belongs to another task ($id.status)" \ + '{state:$state,source:$source,detail:$detail,raw:$raw}' + return 0 + fi raw=$( + unset FM_CREW_STATE_META_OVERRIDE FM_CREW_STATE_STATUS_OVERRIDE fm_run_timed "$FM_SNAPSHOT_CREW_STATE_TIMEOUT" \ - env FM_ROOT_OVERRIDE="$FM_ROOT" \ - FM_HOME="$FM_HOME" \ - FM_STATE_OVERRIDE="$STATE" \ - FM_CREW_STATE_META_OVERRIDE="$captured_meta" \ - FM_CREW_STATE_STATUS_OVERRIDE="$captured_status" \ - FM_DATA_OVERRIDE="$DATA" \ - FM_PROJECTS_OVERRIDE="$PROJECTS" \ - FM_CONFIG_OVERRIDE="$CONFIG" \ + env "${crew_env[@]}" \ "$SCRIPT_DIR/fm-crew-state.sh" "$id" 2>/dev/null || true ) raw=$(printf '%s\n' "$raw" | head -1) diff --git a/bin/fm-supervise-daemon.sh b/bin/fm-supervise-daemon.sh index 91174bc5baf..2d4aaa9dd9b 100755 --- a/bin/fm-supervise-daemon.sh +++ b/bin/fm-supervise-daemon.sh @@ -678,7 +678,7 @@ stale_window_is_busy() { # task=$(window_to_task "$win" "$state") label="fm-$task" tail40=$(fm_backend_capture "$backend" "$win" 40 "$label" 2>/dev/null) || return 2 - verdict=$(fm_busy_classify "$backend" "$win" "$harness" "$task" "$state" "$tail40") + verdict=$(fm_busy_classify_live "$backend" "$win" "$harness" "$task" "$state" "$label" "$tail40") [ "${verdict%% *}" = busy ] } diff --git a/bin/fm-teardown.sh b/bin/fm-teardown.sh index fd2db419806..deca35d89c6 100755 --- a/bin/fm-teardown.sh +++ b/bin/fm-teardown.sh @@ -2937,6 +2937,7 @@ cleanup_firstmate_home_children() { "$sub_state/$child_id.grok-turnend-token" "$sub_state/$child_id.kimi-turnend-token" \ "$sub_state/$child_id.muse-session" "$sub_state/$child_id.muse-session-current" \ "$sub_state/$child_id.cursor-session" "$sub_state/$child_id.reconcile-nudged" \ + "$sub_state/$child_id.voluntary-exit" \ "$sub_state/.$child_id.branch-outcome-index" done } @@ -3344,6 +3345,7 @@ rm -f "$STATE/$ID.turn-ended" \ "$STATE/$ID.control-relaunch" "$STATE/$ID.control-relaunch.meta-prior" \ "$STATE/$ID.control-relaunch.brief-prior" "$STATE/$ID.control-relaunch.note" \ "$STATE/$ID.reconcile-nudged" "$STATE/$ID.gemini-settings.json" \ + "$STATE/$ID.voluntary-exit" \ "$STATE/.$ID.branch-outcome-index" # The steering inbox (bin/fm-task-inbox-lib.sh) is runtime state for the # retired endpoint; teardown only runs after landing is confirmed, so any diff --git a/bin/fm-wake-drain.sh b/bin/fm-wake-drain.sh index 8268bb917fa..fbaea80e846 100755 --- a/bin/fm-wake-drain.sh +++ b/bin/fm-wake-drain.sh @@ -255,6 +255,7 @@ BRANCH_OUTCOME_INDEX_STATE=ok BRANCH_OUTCOME_INDEX_ENDPOINT= BRANCH_OUTCOME_INDEX_IDENT= STATUS_OUTCOME_BACKSTOP_ACKNOWLEDGED= +STATUS_OUTCOME_IDENTITY_ACK= outcome_index_ready_ok() { # local seq [ -f "$1" ] && [ -r "$1" ] && [ ! -L "$1" ] || return 1 @@ -303,6 +304,7 @@ EOF print_status_outcome_backstop_section() { # local snapshot=$1 task endpoint ident event event_endpoint line verb key receipt store lock ready + local event_id stored_id local output='' used=0 shown=0 omitted=0 bytes item_bytes=220 global_bytes=4000 rc=0 [ "$ACTOR" = main ] || return 0 @@ -329,15 +331,29 @@ print_status_outcome_backstop_section() { # fi STATUS_OUTCOME_BACKSTOP_ACKNOWLEDGED= + STATUS_OUTCOME_IDENTITY_ACK= while IFS=$(printf '\t') read -r task endpoint ident; do [ -n "$task" ] || continue receipt=$(status_outcome_backstop_cursor_offset "$STATE/$task.status") || { rc=1; break; } - [ "$receipt" -lt "$endpoint" ] || continue status_snapshot_latest_event "$STATE/$task.status" "$endpoint" "$ident" || continue event=$FM_STATUS_SNAPSHOT_EVENT_LINE event_endpoint=$FM_STATUS_SNAPSHOT_EVENT_ENDPOINT - [ "$receipt" -lt "$event_endpoint" ] || continue status_is_captain_relevant "$event" || continue + event_id=$(status_terminal_event_identity "$event") + stored_id=$(status_outcome_identity_get "$STATE" "$task" || true) + if [ -n "$event_id" ] && [ "$stored_id" = "$event_id" ]; then + continue + fi + if [ "$receipt" -ge "$event_endpoint" ] && [ -z "$stored_id" ]; then + # Cursor already presented these bytes before identity tracking existed. + # Remember the result without re-announcing it. A later different + # identity at the same span still presents below. + if [ -n "$event_id" ]; then + STATUS_OUTCOME_IDENTITY_ACK="$STATUS_OUTCOME_IDENTITY_ACK$task$(printf '\t')$event_id +" + fi + continue + fi verb=$(status_line_verb "$event") case "$verb" in needs-decision|blocked) @@ -373,6 +389,10 @@ print_status_outcome_backstop_section() { # " STATUS_OUTCOME_BACKSTOP_ACKNOWLEDGED="$STATUS_OUTCOME_BACKSTOP_ACKNOWLEDGED$task$(printf '\t')$event_endpoint " + if [ -n "$event_id" ]; then + STATUS_OUTCOME_IDENTITY_ACK="$STATUS_OUTCOME_IDENTITY_ACK$task$(printf '\t')$event_id +" + fi used=$((used + bytes)) shown=$((shown + 1)) done </dev/null || true + fi + if [ -n "$child_out" ]; then + rm -f "$child_out" 2>/dev/null || true + fi +} + +# shellcheck disable=SC2329 # Invoked indirectly by the signal traps below. +handle_arm_signal() { + local signal=$1 rc=$2 + trap - HUP TERM INT + if [ -n "$child" ] && fm_pid_alive "$child"; then + kill -TERM "$child" 2>/dev/null || true + wait "$child" 2>/dev/null || true + fi + cycle_log_append "$rc" "$signal" arm-interrupted none + cleanup_child + exit "$rc" +} + +install_arm_signal_traps() { + trap 'handle_arm_signal HUP 129' HUP + trap 'handle_arm_signal TERM 143' TERM + trap 'handle_arm_signal INT 130' INT +} + +# Start exactly one successor after a clean empty close. A live healthy holder +# is attached instead of forking a second loop. A successor that never becomes +# healthy, or that exits before the confirmation window, is a failed handoff. +spawn_clean_exit_successor() { + local successor successor_out deadline started_at now rc + # Every caller has already exhausted the bounded successor wait for the + # cycle that just closed. Recheck once for a winner of that final race, then + # start the replacement immediately rather than spending a second full + # confirmation window with no watcher. + if healthy_watcher; then + cycle_log_append unknown unknown unexpected-clean-exit "attached:$HEALTHY_PID" + cycle_mark_predecessor_successor "attached:$HEALTHY_PID" + report_attached + cycle_begin "$HEALTHY_PID" attached "$HEALTHY_IDENTITY" + attach_and_wait "$HEALTHY_PID" + return $? + fi + successor_out=$(mktemp "$STATE/.watch-arm-output.XXXXXX") || return 1 + # This child replaces a cycle this same tracked arm already observed. It is + # therefore a handling successor regardless of how the arm itself started; + # re-emitting downtime recovery here would manufacture a wake from the + # intentional handoff and can loop forever. + FM_WATCH_HANDLING_SUCCESSOR=1 "$WATCH" >"$successor_out" & + successor=$! + child=$successor + child_out=$successor_out + install_arm_signal_traps + cycle_begin "$successor" started "$(fm_pid_identity "$successor" 2>/dev/null || true)" + started_at=$(date +%s) + deadline=$((started_at + CONFIRM_TIMEOUT + 1)) + while :; do + if healthy_watcher; then + if [ "$HEALTHY_PID" = "$successor" ]; then + cycle_mark_predecessor_successor "started:$successor" + echo "watcher: started pid=$successor (beacon fresh)" + wait "$successor" + rc=$? + now=$(date +%s) + if [ "$rc" -eq 0 ] && [ $((now - started_at)) -lt 2 ] \ + && ! watch_output_has_wake "$successor_out"; then + print_watch_output "$successor_out" + rm -f "$successor_out" 2>/dev/null || true + child= + child_out= + cycle_log_append "$rc" none successor-immediate-clean-exit none + return 1 + fi + if [ "$rc" -eq 0 ] && watch_output_has_wake "$successor_out"; then + cycle_log_append "$rc" none "$(watch_output_reason_type "$successor_out")" none + print_watch_output "$successor_out" + rm -f "$successor_out" 2>/dev/null || true + child= + child_out= + return 0 + fi + if [ "$rc" -eq 0 ]; then + print_watch_output "$successor_out" + rm -f "$successor_out" 2>/dev/null || true + child= + child_out= + if close_unobserved_cycle; then + cycle_log_append "$rc" none clean-exit-delivered-wake none + return 0 + fi + spawn_clean_exit_successor + return $? + fi + cycle_log_append "$rc" "$(cycle_signal_name "$rc")" successor-nonzero-exit none + print_watch_output "$successor_out" + rm -f "$successor_out" 2>/dev/null || true + child= + child_out= + return "$rc" + fi + wait "$successor" 2>/dev/null || true + rm -f "$successor_out" 2>/dev/null || true + child= + child_out= + cycle_log_append unknown unknown unexpected-clean-exit "attached:$HEALTHY_PID" + cycle_mark_predecessor_successor "attached:$HEALTHY_PID" + report_attached + cycle_begin "$HEALTHY_PID" attached "$HEALTHY_IDENTITY" + attach_and_wait "$HEALTHY_PID" + return $? + fi + if ! fm_pid_alive "$successor"; then + wait "$successor" 2>/dev/null || true + print_watch_output "$successor_out" + rm -f "$successor_out" 2>/dev/null || true + child= + child_out= + cycle_log_append 1 none successor-start-failed none + return 1 + fi + [ "$(date +%s)" -ge "$deadline" ] && break + sleep 0.2 + done + if [ -n "$successor" ] && fm_pid_alive "$successor"; then + kill -TERM "$successor" 2>/dev/null || true + wait "$successor" 2>/dev/null || true + fi + rm -f "$successor_out" 2>/dev/null || true + child= + child_out= + cycle_log_append 1 none successor-start-failed none + return 1 +} + # Close a cycle whose reason line this arm could not read against the bounded # terminal-delivery ledger the watcher publishes before releasing its lock. close_unobserved_cycle() { @@ -281,10 +426,7 @@ close_unobserved_cycle() { clean_identity=$(printf '%s' "$cycle_watcher_identity" | tr '\t\r\n' ' ') i=0 while ! fm_lock_try_acquire "$WATCH_DELIVERY_LOCK"; do - [ "$i" -lt 20 ] || { - fail_unexplained_cycle - return 1 - } + [ "$i" -lt 20 ] || return 1 sleep 0.02 i=$((i + 1)) done @@ -301,7 +443,6 @@ close_unobserved_cycle() { printf '%s\n' "$reason" return 0 fi - fail_unexplained_cycle return 1 } @@ -333,7 +474,11 @@ attach_and_wait() { cycle_log_append unknown unknown attached-delivered-wake none return 0 fi + if spawn_clean_exit_successor; then + return 0 + fi cycle_log_append unknown unknown attached-cycle-ended none + fail_unexplained_cycle return 1 done } @@ -446,34 +591,7 @@ fi # stays our child for its whole life: we wait on it, so killing this arm (the # harness-tracked task) tears the watcher down too, and the watcher's eventual # wake exit propagates out so the harness re-notifies firstmate. -child= -child_out= -cleanup_child() { - if [ -n "$child" ] && fm_pid_alive "$child"; then - kill -TERM "$child" 2>/dev/null || true - fi - if [ -n "$child_out" ]; then - rm -f "$child_out" 2>/dev/null || true - fi -} - -# shellcheck disable=SC2329 # Invoked indirectly by the signal traps below. -handle_arm_signal() { - local signal=$1 rc=$2 - trap - HUP TERM INT - if [ -n "$child" ] && fm_pid_alive "$child"; then - kill -TERM "$child" 2>/dev/null || true - wait "$child" 2>/dev/null || true - fi - cycle_log_append "$rc" "$signal" arm-interrupted none - cleanup_child - exit "$rc" -} - -trap 'handle_arm_signal HUP 129' HUP -trap 'handle_arm_signal TERM 143' TERM -trap 'handle_arm_signal INT 130' INT - +install_arm_signal_traps child_out=$(mktemp "$STATE/.watch-arm-output.XXXXXX") || { echo "watcher: FAILED - no live watcher with a fresh beacon" exit 1 @@ -521,7 +639,11 @@ owned_child_finished() { cycle_log_append "$rc" "$signal" clean-exit-delivered-wake none return 0 fi + if spawn_clean_exit_successor; then + return 0 + fi cycle_log_append "$rc" "$signal" unexpected-clean-exit none + fail_unexplained_cycle return 1 fi diff --git a/bin/fm-watch.sh b/bin/fm-watch.sh index f4246480ff5..2c91ff00cdc 100755 --- a/bin/fm-watch.sh +++ b/bin/fm-watch.sh @@ -272,8 +272,8 @@ window_is_busy() { # if [ -n "$task" ] && [ -f "$meta" ]; then verdict=$(fm_busy_classify_meta "$meta" "$task" "$STATE" "$tail40") else - verdict=$(fm_busy_classify "$(window_backend "$w")" "$w" "$(window_harness "$w")" \ - "${task:-unknown}" "$STATE" "$tail40") + verdict=$(fm_busy_classify_live "$(window_backend "$w")" "$w" "$(window_harness "$w")" \ + "${task:-unknown}" "$STATE" "fm-${task:-unknown}" "$tail40") fi [ "${verdict%% *}" = busy ] } @@ -1170,6 +1170,48 @@ captain_call_stale_bound() { # stale_wait_throttled "$key" "$STALE_WAIT_DECLARATION" } +# An explicit control-plane exit while a PR merge poll is still armed. The +# dead pane is expected, the poll must keep running, and a later genuine +# death without this record must still alarm. +task_voluntary_exit_waiting() { # + local win=$1 task=$2 rec schema reason wait_kind exited_at agent_state lines + rec="$STATE/$task.voluntary-exit" + [ -f "$rec" ] && [ -r "$rec" ] && [ ! -L "$rec" ] || return 1 + schema=$(grep '^schema=' "$rec" 2>/dev/null | cut -d= -f2-) + reason=$(grep '^reason=' "$rec" 2>/dev/null | cut -d= -f2-) + wait_kind=$(grep '^wait=' "$rec" 2>/dev/null | cut -d= -f2-) + exited_at=$(grep '^exited_at=' "$rec" 2>/dev/null | cut -d= -f2-) + lines=$(wc -l < "$rec" 2>/dev/null | tr -d '[:space:]') + [ "$schema" = fm-voluntary-exit.v1 ] || return 1 + [ "$reason" = external-wait ] || return 1 + [ "$wait_kind" = pr-poll ] || return 1 + case "$exited_at" in ''|*[!0-9]*) return 1 ;; esac + [ "$lines" = 4 ] || return 1 + if [ ! -f "$STATE/$task.pr-poll" ] || [ -L "$STATE/$task.pr-poll" ]; then + rm -f "$rec" + return 1 + fi + agent_state=$(fm_backend_agent_state "$(window_backend "$win")" "$win" 2>/dev/null || printf unreadable) + # An explicit exit preserves the endpoint and leaves a bare shell. A missing + # endpoint is a new failure, not the voluntary wait this record describes. + [ "$agent_state" = dead ] || return 1 + return 0 +} + +voluntary_exit_declaration() { # + printf 'voluntary-exit:%s:%s' \ + "$(status_observed_signature "$STATE/$1.voluntary-exit" 2>/dev/null || printf missing)" \ + "$(fm_wake_signal_sig "$STATE/$1.status" || true)" +} + +voluntary_exit_stale_bound() { # + local key=$1 win=$2 task=$3 + STALE_WAIT_DECLARATION= + task_voluntary_exit_waiting "$win" "$task" || return 1 + STALE_WAIT_DECLARATION=$(voluntary_exit_declaration "$task") + stale_wait_throttled "$key" "$STALE_WAIT_DECLARATION" +} + # Surface a stale pane no classifier could resolve, so firstmate inspects it: it # may have finished through an interactive menu that wrote no status, be waiting on # a decision, or be wedged. pause_state_class deliberately answers `none` for a @@ -1203,6 +1245,9 @@ surface_nonterminal_stale() { # elif captain_call_stale_bound "$key" "$task"; then bounded=0 throttled=0 + elif [ -z "$STALE_WAIT_DECLARATION" ] && voluntary_exit_stale_bound "$key" "$win" "$task"; then + bounded=0 + throttled=0 elif [ -n "$STALE_WAIT_DECLARATION" ]; then bounded=0 fi @@ -1486,7 +1531,8 @@ EOF # is absorbed; it surfaces only an event the per-wake path absorbed by mistake - # the fail-safe backstop. heartbeat_scan_finds_actionable() { - local f task record rest endpoint ident rc found=1 sig marker + local f task record rest endpoint ident events rc found=1 sig marker + local hb_size hb_ident hb_event_id hb_stored_id FM_HEARTBEAT_SURFACE_ENDPOINTS='' for f in "$STATE"/*.status; do [ -e "$f" ] || [ -L "$f" ] || continue @@ -1503,6 +1549,22 @@ heartbeat_scan_finds_actionable() { continue fi endpoint=${record%%$'\t'*}; rest=${record#*$'\t'}; ident=${rest%%$'\t'*} + events=${rest#*$'\t'} + hb_size=$(_fm_status_file_size "$f" 2>/dev/null || true) + hb_ident=$(_fm_open_decisions_file_ident "$f" 2>/dev/null || true) + hb_size=${hb_size//[[:space:]]/} + if [ -n "$hb_size" ] && [ -n "$hb_ident" ] \ + && status_snapshot_latest_event "$f" "$hb_size" "$hb_ident" 2>/dev/null; then + hb_event_id=$(status_terminal_event_identity "$FM_STATUS_SNAPSHOT_EVENT_LINE") + hb_stored_id=$(status_outcome_identity_get "$STATE" "$task" || true) + # Suppress only when the whole newly scanned actionable span is this one + # already-presented result. If a distinct event is buried before the same + # latest line, the span must still wake supervision. + if [ "$rc" -eq 0 ] && [ "$events" = "$FM_STATUS_SNAPSHOT_EVENT_LINE" ] \ + && [ -n "$hb_event_id" ] && [ "$hb_stored_id" = "$hb_event_id" ]; then + continue + fi + fi FM_HEARTBEAT_SURFACE_ENDPOINTS="${FM_HEARTBEAT_SURFACE_ENDPOINTS}${f}"$'\t'"${endpoint}"$'\t'"${ident}"$'\n' [ "$rc" -eq 0 ] && found=0 done @@ -2121,6 +2183,11 @@ EOF rm -f "$ssf" clear_write_tracking "$key" triage_log "absorbed stale (open captain call already surfaced for this status): $w" + elif [ -z "$STALE_WAIT_DECLARATION" ] && voluntary_exit_stale_bound "$key" "$w" "$task"; then + printf '%s' "$h" > "$sf" + rm -f "$ssf" + clear_write_tracking "$key" + triage_log "absorbed stale (voluntary exit waiting on an armed PR poll): $w" else fm_wake_append stale "$w" "stale: $w" || exit 1 stale_wait_record "$key" diff --git a/docs/agent-control.md b/docs/agent-control.md index 5a22389075d..fae72ce678c 100644 --- a/docs/agent-control.md +++ b/docs/agent-control.md @@ -31,7 +31,7 @@ A recorded `harness=` is not always an exact adapter name: a task launched from | Verb | Effect | Postcondition | | --- | --- | --- | | `interrupt` | Deliver the harness's verified interrupt sequence while leaving the agent running. | Delivery succeeds while the endpoint still exists and the agent is still alive where the backend can classify that; cancellation is confirmed only from an adapter-owned acknowledgement and otherwise reports `cancel=unconfirmed`. | -| `exit` | Stop the agent, preserving the endpoint, the worktree, and every uncommitted change. | The backend's recovery-grade classifier reports the agent gone. Already-stopped is idempotent success. | +| `exit` | Stop the agent, preserving the endpoint, the worktree, and every uncommitted change. When a PR merge poll is still armed, it also writes `state/.voluntary-exit` so the dead pane is an expected external wait rather than a repeating stale alarm. | The backend's recovery-grade classifier reports the agent gone. Already-stopped is idempotent success. The merge poll keeps running. | | `relaunch` | Replace the running agent with a new one in the same endpoint and worktree, on the exact recorded adapter or an explicitly chosen harness, model, and effort. | The new agent is alive on the recorded endpoint, and the durable record names the harness that is actually running. | An exit that delivers lifecycle input but cannot prove the agent stopped fails with `exit=unconfirmed`, reports the observed agent state and any interrupt cancellation claim, and never claims that nothing changed. diff --git a/docs/configuration.md b/docs/configuration.md index ffe847b7f10..30852aa8616 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -11,7 +11,7 @@ The shared orchestrator behavior lives in [`AGENTS.md`](../AGENTS.md) - edit it This section is the single owner of the top-level operational-home layout; producer script headers and their help own exact child-file fields and mutation contracts. The tracked code root contains the shared instruction, skill, documentation, workflow, and `bin/` surfaces, while each effective `FM_HOME` contains private operational directories. `data/` holds durable private fleet records such as the project and secondmate registries, captain preferences, optional shared captain preferences, learnings, backlog, briefs, scout reports, and explicitly installed content-addressed extension packages under `data/extensions/packages/`. -`state/` holds runtime records such as task metadata, append-only status events, endpoint signals, watcher and wake-queue coordination, inactive terminal-outcome receipts under `state/terminal-outcomes/`, enabled extension working namespaces under `state/extensions/`, away-mode state, generated Relay artifacts, parent-side remote ledger copies under `state/secondmate-summary-cache/`, one-shot Bearings reconcile requests under `state/reconcile-notify/`, private secondmate config-reread generations with their retry and quarantine state, per-task steering-inbox records under `state/.inbox/` (`bin/fm-task-inbox-lib.sh`), and parent-owned secondmate pending-reply records under `state/pending-replies/` (`bin/fm-pending-reply-lib.sh`). +`state/` holds runtime records such as task metadata, append-only status events, endpoint signals, watcher and wake-queue coordination, inactive terminal-outcome receipts under `state/terminal-outcomes/`, presented terminal-result identities under `state/.status-outcome-identity` (`bin/fm-classify-lib.sh`), enabled extension working namespaces under `state/extensions/`, away-mode state, generated Relay artifacts, parent-side remote ledger copies under `state/secondmate-summary-cache/`, one-shot Bearings reconcile requests under `state/reconcile-notify/`, private secondmate config-reread generations with their retry and quarantine state, per-task steering-inbox records under `state/.inbox/` (`bin/fm-task-inbox-lib.sh`), parent-owned secondmate pending-reply records under `state/pending-replies/` (`bin/fm-pending-reply-lib.sh`), and per-task `state/.voluntary-exit` records when an explicit exit leaves a PR merge poll armed (`bin/fm-control.sh`). `config/` holds local gitignored operating choices, including explicit extension bindings under `config/extensions.d/`, and `projects/` holds the local project clones that Firstmate reads but changes only through the narrow guarded and concrete captain-approved exceptions in `AGENTS.md`. Untracked files and directories whose names begin with `scratchpad` are also gitignored, so temporary scratch does not make porcelain-based secondmate sync guards treat a home as dirty. diff --git a/docs/watcher-continuity.md b/docs/watcher-continuity.md index 19d3d15f93d..fcd208478aa 100644 --- a/docs/watcher-continuity.md +++ b/docs/watcher-continuity.md @@ -100,7 +100,9 @@ A zero/empty child return rechecks the home lock and beacon, attaches to a verif An attached arm follows verified identity-matched successors and resolves the same way when that chain ends without one, because it holds no handle on the watcher's stdout and cannot read the reason line itself. Before releasing its singleton lock after printing an actionable reason, the watcher records that reason with its PID and process identity in `state/.watch-deliveries.log`. A matching PID and identity lets an attached arm report the delivered reason and exit zero even after its durable wake was handled and acknowledged, while an unrelated queue producer or a recycled PID cannot satisfy the match. -Only a cycle with no matching delivery record emits `watcher: FAILED - cycle ended without an actionable reason` and exits nonzero. +A clean close with no matching delivery record starts exactly one successor watcher and attaches to it rather than declaring failure or leaving the fleet unsupervised. +It never starts a second concurrent loop: a live healthy holder is attached instead of forked. +Only a cycle that delivered nothing and could not attach or start a successor emits `watcher: FAILED - cycle ended without an actionable reason` and exits nonzero. The arm layer appends one tab-separated record per observed cycle to `state/.watch-cycle-exits.log`. Each record includes arm and watcher PIDs, start and end timestamps, exit code and signal, classified reason, beacon age, lock identity before and after close, and successor disposition. diff --git a/tests/fm-busy-state.test.sh b/tests/fm-busy-state.test.sh index e295871fa20..7e018d665e1 100755 --- a/tests/fm-busy-state.test.sh +++ b/tests/fm-busy-state.test.sh @@ -6,8 +6,8 @@ # explicit source attribution; missing, malformed, stale (gen-mismatch), and # untrusted (source-mismatch) semantic data classify unknown - never idle; # adapter isolation (one adapter's writer or Grok's regex can never classify -# another adapter); endpoint death is the only process-level override and -# yields dead, never busy; converted adapters never classify from rendered +# another adapter); endpoint death and a shell-without-agent pane are the +# process-level overrides and yield dead, never busy; converted adapters never classify from rendered # footer text. All hermetic over temp dirs; no real agent session is invoked. set -u @@ -375,6 +375,52 @@ test_dead_endpoint_overrides() { pass "endpoint death is the only process-level override and yields dead, never busy" } +# A pane that still exists but holds only a shell must classify dead even when +# a harness-specific arm would otherwise answer first. Ambiguous/unreadable +# agent-state must not change those normal harness verdicts. +test_shell_without_agent_overrides_harness_classifiers() { + local state out harness + state=$(new_state_dir shell-no-agent) + mkdir -p "$state" + # shellcheck disable=SC2329 + fm_backend_target_exists() { return 0; } + + for harness in claude codex opencode pi pi-signed grok kimi cursor omp muse gemini rovo; do + # shellcheck disable=SC2329 + fm_backend_agent_state() { printf 'dead'; } + out=$(fm_busy_classify_live tmux w1 "$harness" t1 "$state" fm-t1 'Ctrl+c:cancel') + [ "$out" = "dead shell-no-agent" ] \ + || fail "$harness shell-without-agent must win before its own classifier, got '$out'" + done + + # Counterproof: an alive agent keeps the normal Grok fallback. + # shellcheck disable=SC2329 + fm_backend_agent_state() { printf 'alive'; } + out=$(fm_busy_classify_live tmux w1 grok t1 "$state" fm-t1 'Ctrl+c:cancel') + [ "$out" = "busy grok-regex" ] \ + || fail "an alive grok agent must keep grok-regex, got '$out'" + + # Unreadable/ambiguous must not promote a live grok pane to dead. + # shellcheck disable=SC2329 + fm_backend_agent_state() { printf 'unreadable'; } + out=$(fm_busy_classify_live tmux w1 grok t1 "$state" fm-t1 'Ctrl+c:cancel') + [ "$out" = "busy grok-regex" ] \ + || fail "unreadable agent-state must not override grok-regex, got '$out'" + + # Codex/Kimi keep their unverified gates when the agent is alive. + # shellcheck disable=SC2329 + fm_backend_agent_state() { printf 'alive'; } + out=$(fm_busy_classify_live tmux w1 codex t1 "$state") + [ "$out" = "unknown codex-unverified" ] \ + || fail "alive codex must keep its unverified gate, got '$out'" + out=$(fm_busy_classify_live tmux w1 kimi t1 "$state") + [ "$out" = "unknown kimi-unverified" ] \ + || fail "alive kimi must keep its unverified gate, got '$out'" + + unset -f fm_backend_target_exists fm_backend_agent_state + pass "shell-without-agent wins for every verified harness without changing alive verdicts" +} + test_herdr_native_busy_only() { local state out state=$(new_state_dir herdr-native) @@ -458,6 +504,7 @@ test_codex_unverified_gate test_kimi_unverified_gate test_cursor_ignores_rendered_and_native_signals test_dead_endpoint_overrides +test_shell_without_agent_overrides_harness_classifiers test_herdr_native_busy_only test_record_read_leaves_caller_shell_intact test_boolean_view_never_promotes_unknown diff --git a/tests/fm-control.test.sh b/tests/fm-control.test.sh index c17a8589fd0..e32b4d99fee 100755 --- a/tests/fm-control.test.sh +++ b/tests/fm-control.test.sh @@ -634,6 +634,47 @@ test_already_stopped_exit_is_idempotent() { pass "fm-control exit: an already-stopped agent is idempotent success with no bytes sent" } +test_exit_records_voluntary_wait_when_a_pr_poll_is_armed() { + local dir rec + dir=$(new_case voluntary-exit-wait) + add_task "$dir" t1 claude + alive_as "$dir" claude + printf 'pr=https://example.test/owner/repo/pull/9\n' >> "$dir/home/state/t1.meta" + printf 'provider=github\nurl=https://example.test/owner/repo/pull/9\n' > "$dir/home/state/t1.pr-poll" + run_control "$dir" t1 exit >/dev/null + rec="$dir/home/state/t1.voluntary-exit" + [ -f "$rec" ] || fail "exit with an armed PR poll did not record a voluntary wait" + grep -Fx 'schema=fm-voluntary-exit.v1' "$rec" >/dev/null \ + || fail "voluntary-exit record missing schema" + grep -Fx 'wait=pr-poll' "$rec" >/dev/null \ + || fail "voluntary-exit record missing wait=pr-poll" + [ -f "$dir/home/state/t1.pr-poll" ] || fail "exit removed the PR poll it should have kept" + pass "fm-control exit: an armed PR poll is recorded as a voluntary external wait" +} + +test_exit_without_a_pr_poll_does_not_record_a_voluntary_wait() { + local dir + dir=$(new_case voluntary-exit-none) + add_task "$dir" t1 claude + alive_as "$dir" claude + run_control "$dir" t1 exit >/dev/null + [ ! -e "$dir/home/state/t1.voluntary-exit" ] \ + || fail "exit without an external wait wrote a voluntary-exit record" + pass "fm-control exit: no voluntary-wait record without an armed PR poll" +} + +test_already_dead_agent_is_not_reclassified_as_a_voluntary_wait() { + local dir + dir=$(new_case voluntary-exit-true-death) + add_task "$dir" t1 claude + alive_as "$dir" zsh + printf 'provider=github\nurl=https://example.test/owner/repo/pull/9\n' > "$dir/home/state/t1.pr-poll" + run_control "$dir" t1 exit >/dev/null + [ ! -e "$dir/home/state/t1.voluntary-exit" ] \ + || fail "an agent already found dead was masked as a voluntary wait" + pass "fm-control exit: a true pre-existing death cannot mint a voluntary-wait record" +} + test_missing_endpoint_refuses() { local dir out rc dir=$(new_case gone) @@ -900,6 +941,9 @@ test_verb_allowlist_is_closed test_resume_is_refused_with_its_reason test_relaunch_only_flags_are_rejected_on_other_verbs test_already_stopped_exit_is_idempotent +test_exit_records_voluntary_wait_when_a_pr_poll_is_armed +test_exit_without_a_pr_poll_does_not_record_a_voluntary_wait +test_already_dead_agent_is_not_reclassified_as_a_voluntary_wait test_missing_endpoint_refuses test_interrupt_refuses_when_no_agent_runs test_ambiguous_endpoint_refuses diff --git a/tests/fm-crew-state.test.sh b/tests/fm-crew-state.test.sh index 309a7008f8a..9a9cf6c06d1 100755 --- a/tests/fm-crew-state.test.sh +++ b/tests/fm-crew-state.test.sh @@ -45,6 +45,9 @@ set -u CREW_STATE="$ROOT/bin/fm-crew-state.sh" TMP_ROOT=$(fm_test_tmproot fm-crew-state) +# The test process itself may be launched from a fleet snapshot child. Each +# override case opts in explicitly below; ordinary cases start uncontaminated. +unset FM_CREW_STATE_META_OVERRIDE FM_CREW_STATE_STATUS_OVERRIDE fm_git_identity fmtest fmtest@example.invalid # A real git repo checked out on , so the helper's branch attribution @@ -182,7 +185,11 @@ make_no_timeout_toolbin() { # -> echoes toolbin path # Run the helper for one case dir. FM_FAKE_* env (run output, busy flag) are read # from the caller's environment by the fakes above. run_crew_state() { # - PATH="$1/fakebin:$PATH" FM_STATE_OVERRIDE="$1/state" "$CREW_STATE" "$2" + # Drop inherited snapshot leaks from the parent environment so a leftover + # override cannot make every hermetic case look missing. + PATH="$1/fakebin:$PATH" FM_STATE_OVERRIDE="$1/state" \ + env -u FM_CREW_STATE_META_OVERRIDE -u FM_CREW_STATE_STATUS_OVERRIDE \ + "$CREW_STATE" "$2" } new_case() { # -> echoes case dir with an empty state/ @@ -1613,7 +1620,11 @@ SH "$ROOT/bin/fm-busy-event.sh" apply "$d/state" feat-timeout busy --gen "$gen" \ --source claude-hook --event user-prompt-submit start=$SECONDS - out=$(FM_FAKE_NM_CALLS="$calls_file" PATH="$d/fakebin:$toolbin" FM_STATE_OVERRIDE="$d/state" FM_CREW_STATE_NM_TIMEOUT=1 "$CREW_STATE" feat-timeout) + out=$( + unset FM_CREW_STATE_META_OVERRIDE FM_CREW_STATE_STATUS_OVERRIDE + FM_FAKE_NM_CALLS="$calls_file" PATH="$d/fakebin:$toolbin" FM_STATE_OVERRIDE="$d/state" \ + FM_CREW_STATE_NM_TIMEOUT=1 "$CREW_STATE" feat-timeout + ) elapsed=$((SECONDS - start)) assert_contains "$out" "state: working" "timed-out no-mistakes falls back to pane" assert_contains "$out" "source: pane" "timed-out no-mistakes -> pane source" @@ -1768,6 +1779,46 @@ test_missing_meta() { pass "missing meta is handled gracefully" } +test_snapshot_override_refuses_empty_and_foreign_paths() { + reset_fakes + local d a_meta b_meta out + d=$(new_case override-guard) + make_repo_on_branch "$d/wt" fm/feat-ov + make_fakebin "$d" >/dev/null + fm_write_meta "$d/state/alpha.meta" "window=fm:fm-alpha" "worktree=$d/wt" "kind=ship" "harness=claude" + fm_write_meta "$d/state/beta.meta" "window=fm:fm-beta" "worktree=$d/wt" "kind=ship" "harness=claude" + arm_idle_record "$d/state" alpha + arm_idle_record "$d/state" beta + printf 'working: alpha is live\n' > "$d/state/alpha.status" + printf 'working: beta is live\n' > "$d/state/beta.status" + a_meta="$d/state/alpha.meta" + b_meta="$d/state/beta.meta" + + out=$(PATH="$d/fakebin:$PATH" FM_STATE_OVERRIDE="$d/state" \ + FM_CREW_STATE_META_OVERRIDE='' "$CREW_STATE" beta) + assert_contains "$out" "snapshot override path missing for beta.meta" \ + "an empty snapshot override must be refused, not read as missing metadata" + assert_not_contains "$out" "no metadata for beta" \ + "an empty override must not look like every worker is missing" + + out=$(PATH="$d/fakebin:$PATH" FM_STATE_OVERRIDE="$d/state" \ + FM_CREW_STATE_META_OVERRIDE="$a_meta" "$CREW_STATE" beta) + assert_contains "$out" "snapshot override belongs to another task (beta.meta)" \ + "a captured path for another task must be refused" + + out=$(PATH="$d/fakebin:$PATH" FM_STATE_OVERRIDE="$d/state" \ + FM_CREW_STATE_META_OVERRIDE="$b_meta" "$CREW_STATE" beta) + assert_contains "$out" "state:" "a matching captured meta path must still resolve" + + out=$( + export FM_CREW_STATE_META_OVERRIDE="$a_meta" + PATH="$d/fakebin:$PATH" FM_STATE_OVERRIDE="$d/state" "$CREW_STATE" beta + ) + assert_contains "$out" "snapshot override belongs to another task (beta.meta)" \ + "a leftover exported override from another task must not contaminate the next read" + pass "snapshot overrides refuse empty and foreign paths instead of marking every worker lost" +} + # (k) crew_is_provably_working end-to-end over the REAL fm-crew-state.sh (not a # canned fake verdict, unlike tests/fm-watch-triage.test.sh's classifier # coverage). This is the direct regression pair for the 2026-07-02 herdr @@ -2292,6 +2343,7 @@ test_remote_alive_idle_is_healthy_not_gone test_remote_unreachable_is_unknown_remote_not_dead test_remote_dead_reports_remote_verdict test_missing_meta +test_snapshot_override_refuses_empty_and_foreign_paths test_provably_working_via_runs_list_fallback test_not_provably_working_when_stopped test_usage_error diff --git a/tests/fm-daemon.test.sh b/tests/fm-daemon.test.sh index 38403028be2..0c2fee57db1 100755 --- a/tests/fm-daemon.test.sh +++ b/tests/fm-daemon.test.sh @@ -1025,6 +1025,32 @@ test_housekeeping_paused_resumed_cleared() { pass "a busy pane cannot gate the pause clear once its crew's status no longer declares the wait" } +test_stale_busy_classifier_gives_shell_death_structural_precedence() { + local dir state win pane + dir=$(make_supercase structural-death-precedence) + state="$dir/state"; win="sess:fm-shell-dead"; pane="$dir/pane.txt" + printf 'Ctrl+c:cancel\n' > "$pane" + fm_write_meta "$state/shell-dead.meta" "window=$win" "kind=ship" "harness=grok" "backend=tmux" + + if ( + fm_backend_target_exists() { return 0; } + fm_backend_agent_state() { printf 'dead'; } + fm_backend_capture() { command cat "$pane"; } + stale_window_is_busy "$win" "$state" + ); then + fail "the daemon let Grok's rendered busy marker override a shell-without-agent verdict" + fi + if ! ( + fm_backend_target_exists() { return 0; } + fm_backend_agent_state() { printf 'alive'; } + fm_backend_capture() { command cat "$pane"; } + stale_window_is_busy "$win" "$state" + ); then + fail "the daemon changed Grok's normal busy verdict while the agent was alive" + fi + pass "supervise daemon gives structural shell death precedence without changing a live harness verdict" +} + # The inverse of test_housekeeping_paused_resumed_cleared, and the first half of # issue #3149. A declared wait can legitimately hold a pane BUSY - a worker parked on # a long foreground call it keeps live for as long as the wait lasts - so a busy @@ -1291,6 +1317,8 @@ test_housekeeping_herdr_idle_busy_record_clears_stale() { [ "$2" = "default:w1:p4" ] || fail "expected herdr busy target, got $2" printf 'idle' } + fm_backend_target_exists() { return 0; } + fm_backend_agent_state() { printf 'alive'; } fm_backend_capture herdr default:w1:p4 40 >/dev/null [ "$(fm_backend_busy_state herdr default:w1:p4)" = idle ] || fail "herdr busy stub did not report idle" FM_STATE_OVERRIDE="$state" FM_STALE_ESCALATE_SECS=240 housekeeping "$state" @@ -1319,6 +1347,8 @@ test_housekeeping_herdr_resumed_stale_cleared() { [ "$2" = "default:w1:p3" ] || fail "expected herdr busy target, got $2" printf 'busy' } + fm_backend_target_exists() { return 0; } + fm_backend_agent_state() { printf 'alive'; } fm_backend_capture herdr default:w1:p3 40 >/dev/null [ "$(fm_backend_busy_state herdr default:w1:p3)" = busy ] || fail "herdr busy stub did not report busy" FM_STATE_OVERRIDE="$state" FM_STALE_ESCALATE_SECS=240 housekeeping "$state" @@ -2640,6 +2670,7 @@ test_housekeeping_resumed_stale_cleared test_housekeeping_paused_resurfaces_and_resets test_housekeeping_captain_held_resurfaces_and_resets test_housekeeping_paused_resumed_cleared +test_stale_busy_classifier_gives_shell_death_structural_precedence test_housekeeping_busy_declared_wait_matures_its_window test_housekeeping_paused_unpaused_cleared test_housekeeping_captain_held_resolved_cleared diff --git a/tests/fm-fleet-snapshot-view.test.sh b/tests/fm-fleet-snapshot-view.test.sh index 71b2acbdc18..6ffe8823357 100755 --- a/tests/fm-fleet-snapshot-view.test.sh +++ b/tests/fm-fleet-snapshot-view.test.sh @@ -1045,7 +1045,50 @@ EOF pass "home-summary excludes kind=secondmate from unowned_current and terminal_in_flight" } +test_snapshot_overrides_do_not_contaminate_later_reads() { + local home fakebin out leftover + home=$(make_home override-leak) + fakebin=$(make_fakebin "$home") + leftover="$home/captured/wrong.meta" + mkdir -p "$home/projects/alpha-worktree" "$home/captured" + fm_write_meta "$home/state/alpha.meta" \ + "window=firstmate:fm-alpha" \ + "worktree=$home/projects/alpha-worktree" \ + "project=alpha" \ + "harness=claude" \ + "kind=ship" \ + "mode=no-mistakes" + fm_write_meta "$home/state/beta.meta" \ + "window=firstmate:fm-beta" \ + "worktree=$home/projects/alpha-worktree" \ + "project=alpha" \ + "harness=claude" \ + "kind=ship" \ + "mode=no-mistakes" + record_claude_idle "$home/state" alpha + record_claude_idle "$home/state" beta + printf 'working: alpha live\n' > "$home/state/alpha.status" + printf 'working: beta live\n' > "$home/state/beta.status" + printf 'window=firstmate:fm-wrong\nkind=ship\n' > "$leftover" + + out=$( + PATH="$fakebin:$PATH" FM_HOME="$home" \ + FM_CREW_STATE_META_OVERRIDE="$leftover" \ + "$SNAPSHOT" --json + ) + printf '%s' "$out" | jq -e ' + [.tasks[] | select(.id=="alpha" or .id=="beta")] | length == 2 + ' >/dev/null || fail "snapshot dropped a task under a leftover override: $out" + printf '%s' "$out" | jq -e ' + [.tasks[] | select(.id=="alpha" or .id=="beta") | .current_state.detail // ""] + | all((contains("no metadata") | not) and (contains("another task") | not) + and (contains("snapshot override") | not)) + ' >/dev/null || fail "a leftover snapshot override contaminated later reads: $out" + pass "fleet snapshot confines override variables to each child and refuses a foreign captured path" +} + test_empty_fleet_json +test_snapshot_overrides_do_not_contaminate_later_reads test_fixture_snapshot_json test_home_summary_excludes_secondmate_from_child_inventory test_undated_captain_hold_phrasing_and_aging diff --git a/tests/fm-wake-drain-outcome-backstop.test.sh b/tests/fm-wake-drain-outcome-backstop.test.sh index 9a2a94b0c6a..c5253f4f4ef 100755 --- a/tests/fm-wake-drain-outcome-backstop.test.sh +++ b/tests/fm-wake-drain-outcome-backstop.test.sh @@ -505,7 +505,46 @@ test_backstop_output_is_bounded() { pass "the outcome backstop caps each item and its total task output deterministically" } +test_rewritten_status_file_does_not_reannounce_the_same_terminal_result() { + local dir state out retry_out new_out old + dir=$(make_case identity-stable) + state="$dir/state" + out="$dir/first.out" + retry_out="$dir/retry.out" + new_out="$dir/new.out" + old=$(( $(date +%s) - 20 )) + + printf 'done: PR https://example.test/identity/pull/1 checks green\n' > "$state/ident.status" + set_mtime "$old" "$state/ident.status" + + FM_STATE_OVERRIDE="$state" "$DRAIN" > "$out" \ + || fail "first identity drain failed" + grep -F 'ident done: PR https://example.test/identity/pull/1 checks green' "$out" >/dev/null \ + || fail "first drain did not present the terminal result: $(cat "$out")" + + # Replace the log so the inode/file identity changes while the terminal + # result stays the same. Byte-offset receipts reset; the result identity + # must not. + rm -f "$state/ident.status" + printf 'done: PR https://example.test/identity/pull/1 checks green\n' > "$state/ident.status" + set_mtime "$old" "$state/ident.status" + FM_STATE_OVERRIDE="$state" "$DRAIN" > "$retry_out" \ + || fail "rewritten-same-result drain failed" + [ ! -s "$retry_out" ] \ + || fail "the same terminal result was re-announced after a log rewrite: $(cat "$retry_out")" + + rm -f "$state/ident.status" + printf 'failed: the follow-up PR could not be opened\n' > "$state/ident.status" + set_mtime "$old" "$state/ident.status" + FM_STATE_OVERRIDE="$state" "$DRAIN" > "$new_out" \ + || fail "new-identity drain failed" + grep -F 'ident failed: the follow-up PR could not be opened' "$new_out" >/dev/null \ + || fail "a genuinely new terminal result was not re-announced: $(cat "$new_out")" + pass "a rewritten log does not re-announce the same terminal result and does announce a new identity" +} + test_uncovered_keyless_captain_events_surface_on_the_next_main_drain +test_rewritten_status_file_does_not_reannounce_the_same_terminal_result test_newer_task_outcome_and_routine_latest_events_stay_silent test_older_or_other_task_outcome_cannot_hide_a_new_captain_event test_branch_annotation_cannot_consume_the_main_resurfacing_backstop diff --git a/tests/fm-watch-arm.test.sh b/tests/fm-watch-arm.test.sh index 49d350c538f..6fb433ebe02 100755 --- a/tests/fm-watch-arm.test.sh +++ b/tests/fm-watch-arm.test.sh @@ -218,8 +218,89 @@ test_attached_arm_reports_the_delivered_wake_after_drain() { pass "watch-arm: a delivered wake consumed by the handling turn still closes the attached arm cleanly" } -test_attached_arm_still_fails_on_a_wake_it_did_not_deliver() { - local dir state fakebin out armout status +test_clean_exit_without_event_starts_a_successor() { + local dir state fakebin out armout successor_pid i lock_pid + dir=$(make_case clean-exit-successor) + state="$dir/state" + fakebin="$dir/fakebin" + out="$dir/watch.out" + armout="$dir/arm.out" + + PATH="$fakebin:$PATH" FM_STATE_OVERRIDE="$state" FM_WATCH_HANDLING_SUCCESSOR=1 \ + FM_POLL=1 FM_SIGNAL_GRACE=1 FM_CHECK_INTERVAL=999999 FM_HEARTBEAT=999999 \ + "$WATCH" > "$out" & + SEED_PID=$! + i=0 + while [ "$i" -lt 60 ]; do + [ "$(cat "$state/.watch.lock/pid" 2>/dev/null || true)" = "$SEED_PID" ] \ + && [ -e "$state/.last-watcher-beat" ] && break + sleep 0.1 + i=$((i + 1)) + done + [ "$(cat "$state/.watch.lock/pid" 2>/dev/null || true)" = "$SEED_PID" ] \ + || fail "seed watcher did not take the lock" + + PATH="$fakebin:$PATH" FM_STATE_OVERRIDE="$state" FM_ARM_ATTACH_POLL=0.1 \ + FM_ARM_CONFIRM_TIMEOUT=5 "$WATCH_ARM" > "$armout" & + ARM_PID=$! + i=0 + while [ "$i" -lt 80 ]; do + grep -qF "watcher: attached pid=$SEED_PID" "$armout" 2>/dev/null && break + sleep 0.1 + i=$((i + 1)) + done + grep -qF "watcher: attached pid=$SEED_PID" "$armout" \ + || fail "arm did not attach to the live watcher: $(cat "$armout")" + + # Force a clean self-eviction with no delivered wake and no remaining holder. + printf '999999\n' > "$state/.watch.lock/pid" + wait_for_exit "$SEED_PID" 80 \ + || fail "seed watcher did not self-evict after its lock identity changed" + + i=0 + successor_pid= + while [ "$i" -lt 80 ]; do + if grep -q '^watcher: started pid=' "$armout" 2>/dev/null; then + successor_pid=$(sed -n 's/^watcher: started pid=\([0-9][0-9]*\).*/\1/p' "$armout" | tail -1) + break + fi + grep -qF 'watcher: FAILED' "$armout" 2>/dev/null && break + sleep 0.1 + i=$((i + 1)) + done + ! grep -qF 'watcher: FAILED' "$armout" \ + || fail "a clean empty close was declared failed instead of starting a successor: $(cat "$armout")" + [ -n "$successor_pid" ] || fail "clean empty close did not start a successor: $(cat "$armout")" + is_live_non_zombie "$successor_pid" \ + || fail "the successor watcher exited before taking over: $(cat "$armout")" + i=0 + lock_pid= + while [ "$i" -lt 20 ]; do + lock_pid=$(cat "$state/.watch.lock/pid" 2>/dev/null || true) + [ "$lock_pid" = "$successor_pid" ] && break + is_live_non_zombie "$successor_pid" \ + || fail "the successor watcher exited before holding the singleton: $(cat "$armout")" + sleep 0.05 + i=$((i + 1)) + done + [ "$lock_pid" = "$successor_pid" ] \ + || fail "the successor did not become the singleton holder (lock=$lock_pid successor=$successor_pid)" + [ "$(grep -c '^watcher: started pid=' "$armout")" -eq 1 ] \ + || fail "clean empty close started more than one successor loop: $(cat "$armout")" + kill -TERM "$ARM_PID" 2>/dev/null || true + wait "$ARM_PID" 2>/dev/null || true + i=0 + while [ "$i" -lt 30 ] && is_live_non_zombie "$successor_pid"; do + sleep 0.1 + i=$((i + 1)) + done + ! is_live_non_zombie "$successor_pid" \ + || { kill -TERM "$successor_pid" 2>/dev/null || true; fail "stopping the attached arm orphaned its successor watcher"; } + pass "watch-arm: a clean empty close starts one owned successor instead of failing" +} + +test_unrelated_queue_does_not_spoof_delivery_or_block_successor() { + local dir state fakebin out armout successor_pid i dir=$(make_case attached-no-delivery) state="$dir/state" fakebin="$dir/fakebin" @@ -234,13 +315,25 @@ test_attached_arm_still_fails_on_a_wake_it_did_not_deliver() { append_wake "$state" check process-event "check: process-event result captured: fixture" kill "$SEED_PID" 2>/dev/null || true wait "$SEED_PID" 2>/dev/null || true - wait_for_exit "$ARM_PID" 120 - status=$? - grep -qF 'watcher: FAILED - cycle ended without an actionable reason' "$armout" \ - || fail "a cycle that delivered nothing must still fail loudly: $(cat "$armout")" - [ "$status" -ne 0 ] && [ "$status" -ne 124 ] \ - || fail "arm did not exit nonzero for a cycle that delivered nothing (status $status)" - pass "watch-arm: a cycle that delivered no wake of its own still fails loudly" + i=0 + successor_pid= + while [ "$i" -lt 100 ]; do + successor_pid=$(sed -n 's/^watcher: started pid=\([0-9][0-9]*\).*/\1/p' "$armout" | tail -1) + [ -n "$successor_pid" ] && break + sleep 0.1 + i=$((i + 1)) + done + [ -n "$successor_pid" ] || fail "an unrelated queued event prevented successor startup: $(cat "$armout")" + ! grep -qF 'watcher: FAILED' "$armout" \ + || fail "an unrelated queued event turned clean closure into failure: $(cat "$armout")" + if ! is_live_non_zombie "$ARM_PID" || ! is_live_non_zombie "$successor_pid"; then + fail "the replacement supervision cycle did not stay live" + fi + grep -F "check: process-event result captured: fixture" "$state/.wake-queue" >/dev/null \ + || fail "starting the successor consumed an unrelated durable event" + kill -TERM "$ARM_PID" 2>/dev/null || true + wait "$ARM_PID" 2>/dev/null || true + pass "watch-arm: an unrelated queued event cannot spoof delivery or block a clean-close successor" } test_rearm_resurfaces_durable_queue_and_remote_open_decision() { @@ -801,7 +894,8 @@ test_downtime_marker_does_not_follow_symlink() { test_attached_arm_reports_the_delivered_wake test_attached_arm_reports_the_delivered_wake_after_drain -test_attached_arm_still_fails_on_a_wake_it_did_not_deliver +test_clean_exit_without_event_starts_a_successor +test_unrelated_queue_does_not_spoof_delivery_or_block_successor test_rearm_resurfaces_durable_queue_and_remote_open_decision test_marker_publish_failure_retains_recovery_evidence test_delivery_gap_wake_is_recovered_once diff --git a/tests/fm-watch-triage.test.sh b/tests/fm-watch-triage.test.sh index 029b4a491f9..5ce23255d70 100755 --- a/tests/fm-watch-triage.test.sh +++ b/tests/fm-watch-triage.test.sh @@ -2544,6 +2544,53 @@ test_stale_churn_without_a_captain_call_still_alarms() { pass "a stale window with no open captain call keeps alarming on every new hash" } +test_voluntary_exit_waiting_on_pr_poll_bounds_stale_churn() { + local dir state out capture throttle wakes rec + dir=$(make_hold_home voluntary-pr 'done: PR https://example.invalid/pull/1 checks green' nohold) \ + || fail "could not build a delivered-PR fixture" + state="$dir/state"; out="$dir/watch.out"; capture="$dir/pane.txt" + throttle="$state/.paused-resurfaced-$(hold_key)" + printf 'provider=github\nurl=https://example.invalid/pull/1\n' > "$state/held-merge.pr-poll" + rec="$state/held-merge.voluntary-exit" + { + printf 'schema=fm-voluntary-exit.v1\n' + printf 'reason=external-wait\n' + printf 'wait=pr-poll\n' + printf 'exited_at=1\n' + } > "$rec" + + hold_watch_surface "$dir" "$out" "$capture" 'idle, elapsed 1s' \ + || fail "first sight of a voluntary-exit wait did not surface" + wakes=$(hold_stale_wakes "$state") + [ "$wakes" -eq 1 ] || fail "first sight produced $wakes wakes instead of one" + ack_stopped_cycle "$state" || fail "could not acknowledge the first surface" + [ -f "$state/held-merge.pr-poll" ] || fail "the first surface removed the PR poll" + + hold_watch_churn "$dir" "$out" "$capture" 'idle, tick' 2 \ + || fail "watcher exited during voluntary-exit pane churn instead of supervising through it" + wakes=$(hold_stale_wakes "$state") + [ "$wakes" -eq 0 ] \ + || fail "pane churn re-alarmed a voluntary-exit wait $wakes time(s) inside the re-surface window" + + [ -e "$throttle" ] || fail "absorbed churn recorded no re-surface cadence to elapse" + set_mtime "$(( $(date +%s) - 5000 ))" "$throttle" + hold_watch_surface "$dir" "$out" "$capture" 'idle, elapsed 9s' \ + || fail "voluntary-exit wait did not re-surface once its window elapsed" + wakes=$(hold_stale_wakes "$state") + [ "$wakes" -eq 1 ] \ + || fail "elapsed re-surface window produced $wakes wakes instead of one" + [ -f "$state/held-merge.pr-poll" ] || fail "re-surface removed the PR poll" + ack_stopped_cycle "$state" || fail "could not acknowledge the elapsed voluntary-exit re-surface" + + rm -f "$rec" + hold_watch_surface "$dir" "$out" "$capture" 'idle, crashed 1s' \ + || fail "a dead agent without a voluntary-exit record did not surface" + wakes=$(hold_stale_wakes "$state") + [ "$wakes" -eq 1 ] \ + || fail "removing the voluntary-exit record produced $wakes wakes instead of one true death alarm" + pass "voluntary exit with an armed PR poll surfaces once, absorbs churn, keeps the poll, and still alarms a true death" +} + # The cadence marker may never outlive the wake it claims to record. Recording it # before publishing the durable wake turned a delayed alarm into a lost one: the @@ -3398,6 +3445,7 @@ case "${1:-}" in display-message) case "$*" in *pane_current_command*) printf '%s\n' "${FM_FAKE_TMUX_CURRENT_COMMAND:-}"; exit 0 ;; + *pane_id*) printf '%%1\n'; exit 0 ;; esac ;; esac exit 1 @@ -4238,6 +4286,54 @@ test_heartbeat_backstop_surfaces_a_masked_status() { pass "the heartbeat backstop surfaces a captain event hidden behind a later routine append" } +test_heartbeat_identity_does_not_hide_a_new_buried_result() { + local dir state fakebin first out sig pid i + dir=$(make_case heartbeat-identity-buried); state="$dir/state"; fakebin="$dir/fakebin" + first="$dir/first.out"; out="$dir/watch.out" + printf 'done: PR https://example.test/pr/identity checks green\n' > "$state/miss.status" + FM_STATE_OVERRIDE="$state" "$DRAIN" > "$first" 2>/dev/null \ + || fail "initial identity drain failed" + grep -F 'miss done: PR https://example.test/pr/identity checks green' "$first" >/dev/null \ + || fail "initial terminal result was not presented" + + rm -f "$state/miss.status" + printf 'done: PR https://example.test/pr/identity checks green\n' > "$state/miss.status" + sig=$(seen_sig "$state/miss.status"); printf '%s' "$sig" > "$state/.seen-miss_status" + PATH="$fakebin:$PATH" FM_STATE_OVERRIDE="$state" FM_POLL=1 FM_SIGNAL_GRACE=1 \ + FM_CHECK_INTERVAL=999999 FM_HEARTBEAT=1 "$WATCH" > "$out" & + pid=$! + wait_poll_cycle "$state" "$pid" \ + || { reap "$pid"; fail "the same presented terminal result re-fired during heartbeat scanning"; } + i=0 + while [ "$i" -lt 100 ]; do + [ "$(cat "$state/.heartbeat-streak" 2>/dev/null || echo 0)" -ge 1 ] && break + is_live_non_zombie "$pid" || break + sleep 0.1 + i=$((i + 1)) + done + [ "$(cat "$state/.heartbeat-streak" 2>/dev/null || echo 0)" -ge 1 ] \ + || { reap "$pid"; fail "the identity fixture never reached a heartbeat scan"; } + [ ! -s "$out" ] || { reap "$pid"; fail "the same presented result printed another wake: $(cat "$out")"; } + reap "$pid" + ack_stopped_cycle "$state" || fail "could not acknowledge the intentional identity-dedupe stop" + rm -f "$state/.last-heartbeat" "$state/.heartbeat-streak" + + # Recreate the log with a genuinely new failure buried before the same latest + # result. The stable latest identity may dedupe itself, but not the new span. + rm -f "$state/miss.status" + printf 'failed: a later delivery attempt broke\ndone: PR https://example.test/pr/identity checks green\n' \ + > "$state/miss.status" + sig=$(seen_sig "$state/miss.status"); printf '%s' "$sig" > "$state/.seen-miss_status" + PATH="$fakebin:$PATH" FM_STATE_OVERRIDE="$state" FM_POLL=1 FM_SIGNAL_GRACE=1 \ + FM_CHECK_INTERVAL=999999 FM_HEARTBEAT=1 "$WATCH" > "$out" & + pid=$! + wait_for_exit "$pid" 100 \ + || fail "the stored latest identity hid a genuinely new buried terminal result" + grep -Fx "heartbeat" "$out" >/dev/null \ + || fail "the new buried terminal result did not produce the heartbeat backstop: $(cat "$out")" + pass "terminal identity dedupe never hides a genuinely new result earlier in the scanned span" +} + test_heartbeat_backstop_surfaces_unsurfaced_status() { local dir state fakebin out drain_out sig pid dir=$(make_case heartbeat-backstop); state="$dir/state"; fakebin="$dir/fakebin" @@ -4443,6 +4539,7 @@ test_absorbed_replacement_wait_does_not_inherit_the_old_throttle test_live_declared_wait_churn_honors_the_resurface_throttle test_open_captain_call_bounds_stale_churn test_stale_churn_without_a_captain_call_still_alarms +test_voluntary_exit_waiting_on_pr_poll_bounds_stale_churn test_failed_wake_append_does_not_arm_the_captain_hold_throttle test_reheld_captain_call_starts_its_own_resurface_window test_secondmate_paused_resurfaces_in_normal_mode @@ -4468,6 +4565,7 @@ test_procevent_marker_failure_exits_and_replays test_heartbeat_no_change_absorbed test_heartbeat_backstop_surfaces_unsurfaced_status test_heartbeat_backstop_surfaces_a_masked_status +test_heartbeat_identity_does_not_hide_a_new_buried_result test_beacon_stays_fresh_while_absorbing test_afk_signal_records_heartbeat_endpoint test_afk_present_reverts_watcher_to_one_shot diff --git a/tests/wake-helpers.sh b/tests/wake-helpers.sh index da83bb3dc91..41f4e63d060 100644 --- a/tests/wake-helpers.sh +++ b/tests/wake-helpers.sh @@ -95,6 +95,11 @@ fi if [ "${1:-}" = "display-message" ]; then case "$*" in *pane_current_command*) printf '%s\n' "${FM_FAKE_TMUX_CURRENT_COMMAND:-}"; exit 0 ;; + *pane_id*) + [ -n "${FM_FAKE_TMUX_WINDOW:-}" ] || exit 1 + printf '%%1\n' + exit 0 + ;; esac fi exit 1 From da5ea75044cdb19f548345cd0e07bedaf16d9909 Mon Sep 17 00:00:00 2001 From: Alex William Date: Tue, 8 Sep 2026 16:14:03 +0200 Subject: [PATCH 2/8] no-mistakes(review): bound clean-empty watcher successor retries iteratively --- bin/fm-watch-arm.sh | 118 +++++++++++++++++++++++++--------- docs/watcher-continuity.md | 3 +- tests/fm-watch-arm.test.sh | 102 +++++++++++++++++++++++++++++ tests/fm-watcher-lock.test.sh | 75 ++++++++++++++------- 4 files changed, 243 insertions(+), 55 deletions(-) diff --git a/bin/fm-watch-arm.sh b/bin/fm-watch-arm.sh index e2ecda54077..0f05b1b24cf 100755 --- a/bin/fm-watch-arm.sh +++ b/bin/fm-watch-arm.sh @@ -33,6 +33,10 @@ # - a clean cycle ended with no wake, no # verified healthy successor, and no # successor this arm could start +# watcher: FAILED - cycle ended without an actionable reason after N successor retries +# - every successor of a clean empty close +# closed clean and empty too, up to the +# bounded retry budget # It NEVER reports started/attached/healthy off a stale beacon or a dead/reused pid: a # stale-beacon or dead-pid holder either self-heals (the fresh child steals the # dead lock per the singleton self-eviction/steal path and is confirmed) or this @@ -42,9 +46,12 @@ # watcher's identity-bound delivery record: a matching record reports that wake # and exits 0. A clean empty close with no delivery record starts exactly one # successor watcher and attaches to it rather than failing or leaving the fleet -# unsupervised; it never starts a second concurrent loop. Only a cycle that -# delivered nothing AND could not attach or start a successor is the typed -# nonzero failure. Neither is ever a silent empty completion. On FAILED it exits +# unsupervised; it never starts a second concurrent loop. That retry is bounded: +# consecutive clean empty closes are retried a fixed number of times with +# exponential backoff, and the streak resets as soon as a cycle delivers a wake or +# a verified successor keeps supervising. A cycle that delivered nothing AND could +# not attach or start a successor, and an exhausted retry budget, are both the +# typed nonzero failure. Neither is ever a silent empty completion. On FAILED it exits # non-zero so the failure is loud. A live cycle already present means re-arm # attaches - do not start a second watcher. # @@ -274,10 +281,34 @@ wait_for_healthy_successor() { } fail_unexplained_cycle() { - echo "watcher: FAILED - cycle ended without an actionable reason" + local detail=${1:-} + echo "watcher: FAILED - cycle ended without an actionable reason${detail:+ $detail}" return 1 } +# A clean empty close is not a failure, but an unbroken run of them is. The arm +# retries a successor this many times, waiting CLEAN_EMPTY_BACKOFF[n] before the +# nth consecutive attempt, then fails loudly instead of replacing watchers +# forever. Only consecutive clean empty cycles count: any delivered wake or +# verified continuing successor resets the streak. +CLEAN_EMPTY_RETRIES=5 +CLEAN_EMPTY_BACKOFF="0.25 0.5 1 2 4" + +clean_empty_backoff() { # + local attempt=$1 i=0 delay + for delay in $CLEAN_EMPTY_BACKOFF; do + i=$((i + 1)) + [ "$i" -eq "$attempt" ] && { printf '%s' "$delay"; return 0; } + done + printf '%s' "$delay" +} + +# Set by attach_and_wait and spawn_clean_exit_successor to hand the next step +# back to supervise() instead of calling each other. Alternating between the two +# is a loop in one frame, so a persistently empty cycle cannot grow the stack. +ARM_NEXT= +ARM_ATTACH_PID= + # An attached arm can become the owner of a replacement child after a clean # empty cycle. Install the owning signal handler as soon as that child starts, # so interrupting the tracked arm can never orphan its replacement watcher. @@ -315,7 +346,7 @@ install_arm_signal_traps() { # is attached instead of forking a second loop. A successor that never becomes # healthy, or that exits before the confirmation window, is a failed handoff. spawn_clean_exit_successor() { - local successor successor_out deadline started_at now rc + local successor successor_out deadline started_at rc # Every caller has already exhausted the bounded successor wait for the # cycle that just closed. Recheck once for a winner of that final race, then # start the replacement immediately rather than spending a second full @@ -325,8 +356,9 @@ spawn_clean_exit_successor() { cycle_mark_predecessor_successor "attached:$HEALTHY_PID" report_attached cycle_begin "$HEALTHY_PID" attached "$HEALTHY_IDENTITY" - attach_and_wait "$HEALTHY_PID" - return $? + ARM_ATTACH_PID=$HEALTHY_PID + ARM_NEXT=attach + return 0 fi successor_out=$(mktemp "$STATE/.watch-arm-output.XXXXXX") || return 1 # This child replaces a cycle this same tracked arm already observed. It is @@ -348,16 +380,6 @@ spawn_clean_exit_successor() { echo "watcher: started pid=$successor (beacon fresh)" wait "$successor" rc=$? - now=$(date +%s) - if [ "$rc" -eq 0 ] && [ $((now - started_at)) -lt 2 ] \ - && ! watch_output_has_wake "$successor_out"; then - print_watch_output "$successor_out" - rm -f "$successor_out" 2>/dev/null || true - child= - child_out= - cycle_log_append "$rc" none successor-immediate-clean-exit none - return 1 - fi if [ "$rc" -eq 0 ] && watch_output_has_wake "$successor_out"; then cycle_log_append "$rc" none "$(watch_output_reason_type "$successor_out")" none print_watch_output "$successor_out" @@ -375,8 +397,9 @@ spawn_clean_exit_successor() { cycle_log_append "$rc" none clean-exit-delivered-wake none return 0 fi - spawn_clean_exit_successor - return $? + cycle_log_append "$rc" none unexpected-clean-exit none + ARM_NEXT=clean-empty + return 0 fi cycle_log_append "$rc" "$(cycle_signal_name "$rc")" successor-nonzero-exit none print_watch_output "$successor_out" @@ -393,8 +416,9 @@ spawn_clean_exit_successor() { cycle_mark_predecessor_successor "attached:$HEALTHY_PID" report_attached cycle_begin "$HEALTHY_PID" attached "$HEALTHY_IDENTITY" - attach_and_wait "$HEALTHY_PID" - return $? + ARM_ATTACH_PID=$HEALTHY_PID + ARM_NEXT=attach + return 0 fi if ! fm_pid_alive "$successor"; then wait "$successor" 2>/dev/null || true @@ -468,21 +492,54 @@ attach_and_wait() { attached_pid=$HEALTHY_PID cycle_begin "$attached_pid" attached "$HEALTHY_IDENTITY" report_attached + clean_empty_streak=0 continue fi if close_unobserved_cycle; then cycle_log_append unknown unknown attached-delivered-wake none return 0 fi - if spawn_clean_exit_successor; then - return 0 - fi cycle_log_append unknown unknown attached-cycle-ended none - fail_unexplained_cycle + ARM_NEXT=clean-empty return 1 done } +# The arm's whole post-confirmation life: follow a healthy holder, and answer a +# clean empty close with a bounded, backed-off run of replacement watchers. Both +# steps return here rather than invoking one another. +clean_empty_streak=0 +supervise() { # | clean-empty> + local action=$1 rc + ARM_ATTACH_PID=${2:-} + clean_empty_streak=0 + while :; do + ARM_NEXT= + if [ "$action" = attach ]; then + attach_and_wait "$ARM_ATTACH_PID" + rc=$? + else + clean_empty_streak=$((clean_empty_streak + 1)) + if [ "$clean_empty_streak" -gt "$CLEAN_EMPTY_RETRIES" ]; then + fail_unexplained_cycle "after $CLEAN_EMPTY_RETRIES successor retries" + return 1 + fi + sleep "$(clean_empty_backoff "$clean_empty_streak")" + spawn_clean_exit_successor + rc=$? + fi + case "$ARM_NEXT" in + attach) action=attach; clean_empty_streak=0 ;; + clean-empty) action=clean-empty ;; + *) + [ "$rc" -eq 0 ] && return 0 + fail_unexplained_cycle + return 1 + ;; + esac + done +} + # shellcheck disable=SC2329 # Invoked indirectly by the signal traps below. handle_attached_signal() { local signal=$1 rc=$2 @@ -583,7 +640,7 @@ if [ "$mode" = arm ] && healthy_watcher; then cycle_mark_predecessor_successor "attached:$HEALTHY_PID" cycle_begin "$HEALTHY_PID" attached "$HEALTHY_IDENTITY" report_attached - attach_and_wait "$HEALTHY_PID" + supervise attach "$HEALTHY_PID" exit $? fi @@ -628,7 +685,7 @@ owned_child_finished() { cycle_mark_predecessor_successor "attached:$HEALTHY_PID" report_attached cycle_begin "$HEALTHY_PID" attached "$HEALTHY_IDENTITY" - attach_and_wait "$HEALTHY_PID" + supervise attach "$HEALTHY_PID" return $? fi print_watch_output "$child_out" @@ -639,12 +696,9 @@ owned_child_finished() { cycle_log_append "$rc" "$signal" clean-exit-delivered-wake none return 0 fi - if spawn_clean_exit_successor; then - return 0 - fi cycle_log_append "$rc" "$signal" unexpected-clean-exit none - fail_unexplained_cycle - return 1 + supervise clean-empty + return $? fi reason_type="nonzero-exit" diff --git a/docs/watcher-continuity.md b/docs/watcher-continuity.md index fcd208478aa..8d83a6e49ff 100644 --- a/docs/watcher-continuity.md +++ b/docs/watcher-continuity.md @@ -102,7 +102,8 @@ Before releasing its singleton lock after printing an actionable reason, the wat A matching PID and identity lets an attached arm report the delivered reason and exit zero even after its durable wake was handled and acknowledged, while an unrelated queue producer or a recycled PID cannot satisfy the match. A clean close with no matching delivery record starts exactly one successor watcher and attaches to it rather than declaring failure or leaving the fleet unsupervised. It never starts a second concurrent loop: a live healthy holder is attached instead of forked. -Only a cycle that delivered nothing and could not attach or start a successor emits `watcher: FAILED - cycle ended without an actionable reason` and exits nonzero. +That replacement is bounded: consecutive clean empty closes are answered with at most five successors, waited 250ms, 500ms, 1s, 2s and 4s apart, and the streak resets as soon as a cycle delivers a wake or a verified successor keeps supervising. +Only a cycle that delivered nothing and could not attach or start a successor emits `watcher: FAILED - cycle ended without an actionable reason` and exits nonzero; an exhausted retry budget emits the same typed failure with an `after 5 successor retries` suffix. The arm layer appends one tab-separated record per observed cycle to `state/.watch-cycle-exits.log`. Each record includes arm and watcher PIDs, start and end timestamps, exit code and signal, classified reason, beacon age, lock identity before and after close, and successor disposition. diff --git a/tests/fm-watch-arm.test.sh b/tests/fm-watch-arm.test.sh index 6fb433ebe02..e0e2a3c07eb 100755 --- a/tests/fm-watch-arm.test.sh +++ b/tests/fm-watch-arm.test.sh @@ -299,6 +299,107 @@ test_clean_exit_without_event_starts_a_successor() { pass "watch-arm: a clean empty close starts one owned successor instead of failing" } +# A cycle that always closes clean and empty must not be answered with an +# endless run of replacement watchers. The arm retries a bounded number of +# successors, spaced by the documented exponential backoff, and then fails +# loudly - rather than recursing one shell frame deeper per empty cycle and +# never reporting anything. +test_persistent_clean_empty_cycles_are_bounded_and_loud() { + local dir state fakebin armout starts clobber_pid dead status i seen count + local started total leftover t2 t3 t4 t5 t6 + dir=$(make_case clean-exit-retry-bound) + state="$dir/state" + fakebin="$dir/fakebin" + armout="$dir/arm.out" + starts="$dir/started-at" + : > "$armout" + : > "$starts" + dead=$(dead_pid) + + PATH="$fakebin:$PATH" FM_STATE_OVERRIDE="$state" FM_ARM_ATTACH_POLL=0.1 \ + FM_ARM_CONFIRM_TIMEOUT=5 FM_POLL=0.2 FM_SIGNAL_GRACE=1 \ + FM_CHECK_INTERVAL=999999 FM_HEARTBEAT=999999 "$WATCH_ARM" > "$armout" & + ARM_PID=$! + + # Hand the singleton lock to a dead pid once each confirmed watcher has run a + # normal-length cycle. That watcher self-evicts and closes cleanly with no + # wake, so EVERY cycle this arm owns is a clean empty one - and each lasts long + # enough to be an ordinary cycle rather than an obvious start-up flap, which is + # exactly the condition an unbounded successor loop would ride forever. + ( + seen=0 + while :; do + count=$(grep -c '^watcher: started pid=' "$armout" 2>/dev/null || true) + case "$count" in ''|*[!0-9]*) count=0 ;; esac + if [ "$count" -gt "$seen" ]; then + started=$(sed -n 's/^watcher: started pid=\([0-9][0-9]*\).*/\1/p' "$armout" | tail -1) + sleep 2.5 + if [ -n "$started" ] \ + && [ "$(cat "$state/.watch.lock/pid" 2>/dev/null || true)" = "$started" ]; then + printf '%s\n' "$dead" > "$state/.watch.lock/pid" + seen=$count + fi + fi + sleep 0.05 + done + ) & + clobber_pid=$! + + # Stamp the wall-clock second each reported watcher appears, so the spacing + # between successive successors can be checked against the backoff schedule. + seen=0 + i=0 + while [ "$i" -lt 1500 ]; do + count=$(grep -c '^watcher: started pid=' "$armout" 2>/dev/null || true) + case "$count" in ''|*[!0-9]*) count=0 ;; esac + while [ "$seen" -lt "$count" ]; do + date +%s >> "$starts" + seen=$((seen + 1)) + done + is_live_non_zombie "$ARM_PID" || break + sleep 0.05 + i=$((i + 1)) + done + wait_for_exit "$ARM_PID" 100 + status=$? + kill -TERM "$clobber_pid" 2>/dev/null || true + wait "$clobber_pid" 2>/dev/null || true + + [ "$status" -ne 0 ] && [ "$status" -ne 124 ] \ + || fail "an endlessly empty cycle did not fail loudly (status $status): $(cat "$armout")" + grep -qF 'watcher: FAILED - cycle ended without an actionable reason after 5 successor retries' "$armout" \ + || fail "exhausted clean-empty retries omitted the typed failure: $(cat "$armout")" + + # One owned child plus exactly five bounded successor retries. + total=$(grep -c '^watcher: started pid=' "$armout" 2>/dev/null || true) + [ "$total" -eq 6 ] \ + || fail "clean empty closes were not bounded to 5 successor retries (started $total watchers): $(cat "$armout")" + + t2=$(sed -n 2p "$starts") + t3=$(sed -n 3p "$starts") + t4=$(sed -n 4p "$starts") + t5=$(sed -n 5p "$starts") + t6=$(sed -n 6p "$starts") + [ -n "$t2" ] && [ -n "$t6" ] || fail "did not observe every successor start: $(cat "$starts")" + # Every cycle costs the same ~2.5s dwell, so only a growing backoff can push + # these gaps apart: 1s, 2s and 4s before the third, fourth and fifth retry. + [ $((t4 - t3)) -ge 3 ] \ + || fail "the third successor retry did not wait its 1s backoff (gap $((t4 - t3))s)" + [ $((t5 - t4)) -ge 4 ] \ + || fail "the fourth successor retry did not wait its 2s backoff (gap $((t5 - t4))s)" + [ $((t6 - t5)) -ge 6 ] \ + || fail "the fifth successor retry did not wait its 4s backoff (gap $((t6 - t5))s)" + [ $((t6 - t2)) -ge 15 ] \ + || fail "the retry backoff did not grow across the streak (span $((t6 - t2))s)" + + leftover=$(cat "$state/.watch.lock/pid" 2>/dev/null || true) + if [ -n "$leftover" ] && [ "$leftover" != "$dead" ] && is_live_non_zombie "$leftover"; then + kill -TERM "$leftover" 2>/dev/null || true + fail "the exhausted arm left a watcher running" + fi + pass "watch-arm: endlessly empty cycles retry a bounded, backed-off successor run and then fail loudly" +} + test_unrelated_queue_does_not_spoof_delivery_or_block_successor() { local dir state fakebin out armout successor_pid i dir=$(make_case attached-no-delivery) @@ -895,6 +996,7 @@ test_downtime_marker_does_not_follow_symlink() { test_attached_arm_reports_the_delivered_wake test_attached_arm_reports_the_delivered_wake_after_drain test_clean_exit_without_event_starts_a_successor +test_persistent_clean_empty_cycles_are_bounded_and_loud test_unrelated_queue_does_not_spoof_delivery_or_block_successor test_rearm_resurfaces_durable_queue_and_remote_open_decision test_marker_publish_failure_retains_recovery_evidence diff --git a/tests/fm-watcher-lock.test.sh b/tests/fm-watcher-lock.test.sh index 77fd4fbcca3..ad8a07204c7 100755 --- a/tests/fm-watcher-lock.test.sh +++ b/tests/fm-watcher-lock.test.sh @@ -34,6 +34,46 @@ drain_and_ack() { # --recovery-generation "$generation" } +# An attached cycle that ends with no wake and no verified successor is a +# supervision gap the arm closes, not a failure it reports: it starts exactly +# one replacement watcher, follows that one, and only stops when this test does. +expect_replacement_watcher() { # + local state=$1 armout=$2 armpid=$3 i=0 successor= + while [ "$i" -lt 200 ]; do + if grep -q '^watcher: started pid=' "$armout" 2>/dev/null; then + successor=$(sed -n 's/^watcher: started pid=\([0-9][0-9]*\).*/\1/p' "$armout" | tail -1) + break + fi + grep -qF 'watcher: FAILED' "$armout" 2>/dev/null && break + sleep 0.1 + i=$((i + 1)) + done + ! grep -qF 'watcher: FAILED' "$armout" \ + || fail "a clean empty close of the attached cycle was declared failed: $(cat "$armout")" + [ -n "$successor" ] || fail "the attached cycle ended without a replacement watcher: $(cat "$armout")" + [ "$(grep -c '^watcher: started pid=' "$armout")" -eq 1 ] \ + || fail "the arm started more than one replacement loop: $(cat "$armout")" + is_live_non_zombie "$armpid" || fail "the arm stopped supervising its replacement: $(cat "$armout")" + i=0 + while [ "$i" -lt 40 ]; do + [ "$(cat "$state/.watch.lock/pid" 2>/dev/null || true)" = "$successor" ] && break + sleep 0.1 + i=$((i + 1)) + done + [ "$(cat "$state/.watch.lock/pid" 2>/dev/null || true)" = "$successor" ] \ + || fail "the replacement watcher did not become the singleton holder: $(cat "$armout")" + kill -TERM "$armpid" 2>/dev/null || true + wait "$armpid" 2>/dev/null || true + i=0 + while [ "$i" -lt 40 ] && is_live_non_zombie "$successor"; do + sleep 0.1 + i=$((i + 1)) + done + is_live_non_zombie "$successor" \ + && { kill -TERM "$successor" 2>/dev/null || true; fail "stopping the arm orphaned its replacement watcher"; } + return 0 +} + test_singleton_start() { local dir state fakebin out1 out2 pid1 pid2 live i dir=$(make_case singleton) @@ -450,7 +490,7 @@ test_watch_restart_rejects_reused_pid() { } test_watch_restart_attaches_to_healthy_peer() { - local dir state fakebin out peer_ready peer identity armpid status i + local dir state fakebin out peer_ready peer identity armpid i dir=$(make_case restart-healthy-peer) state="$dir/state" fakebin="$dir/fakebin" @@ -475,7 +515,7 @@ test_watch_restart_attaches_to_healthy_peer() { printf '%s\n' "$WATCH" > "$state/.watch.lock/watcher-path" printf '%s\n' "$identity" > "$state/.watch.lock/pid-identity" touch "$state/.last-watcher-beat" - PATH="$fakebin:$PATH" FM_HOME="$dir" FM_POLL=5 FM_SIGNAL_GRACE=1 FM_CHECK_INTERVAL=999999 FM_HEARTBEAT=999999 FM_ARM_ATTACH_POLL=0.1 FM_ARM_CONFIRM_TIMEOUT=1 "$WATCH_ARM" --restart > "$out" & + PATH="$fakebin:$PATH" FM_HOME="$dir" FM_POLL=5 FM_SIGNAL_GRACE=1 FM_CHECK_INTERVAL=999999 FM_HEARTBEAT=999999 FM_ARM_ATTACH_POLL=0.1 FM_ARM_CONFIRM_TIMEOUT=5 "$WATCH_ARM" --restart > "$out" & armpid=$! i=0 while [ "$i" -lt 80 ]; do @@ -488,11 +528,8 @@ test_watch_restart_attaches_to_healthy_peer() { is_live_non_zombie "$peer" || fail "restart killed a TERM-resistant peer unexpectedly" kill -KILL "$peer" 2>/dev/null || true wait "$peer" 2>/dev/null || true - wait_for_exit "$armpid" 80 - status=$? - [ "$status" -ne 0 ] && [ "$status" -ne 124 ] || fail "restart arm did not fail after its attached peer ended without a successor (status $status)" - grep -qF 'watcher: FAILED - cycle ended without an actionable reason' "$out" || fail "restart arm did not surface the attached cycle end" - pass "watch restart attaches to a verified healthy peer and later surfaces a successor gap" + expect_replacement_watcher "$state" "$out" "$armpid" + pass "watch restart attaches to a verified healthy peer and replaces it when that cycle ends" } test_watcher_self_evicts_on_lock_takeover() { @@ -562,7 +599,7 @@ test_arm_self_eviction_is_loud_without_successor() { } test_arm_attaches_and_waits_for_live_fresh_watcher() { - local dir state fakebin out armout i wpid armpid status + local dir state fakebin out armout i wpid armpid dir=$(make_case arm-attach) state="$dir/state" fakebin="$dir/fakebin" @@ -580,7 +617,7 @@ test_arm_attaches_and_waits_for_live_fresh_watcher() { [ "$(cat "$state/.watch.lock/pid" 2>/dev/null || true)" = "$wpid" ] || fail "seed watcher did not take the lock" # Arming must attach to the existing watcher, NOT start a second one, and NOT # exit while the seed still holds the healthy lock. - PATH="$fakebin:$PATH" FM_STATE_OVERRIDE="$state" FM_ARM_ATTACH_POLL=0.1 FM_ARM_CONFIRM_TIMEOUT=1 "$WATCH_ARM" > "$armout" & + PATH="$fakebin:$PATH" FM_STATE_OVERRIDE="$state" FM_ARM_ATTACH_POLL=0.1 FM_ARM_CONFIRM_TIMEOUT=5 "$WATCH_ARM" > "$armout" & armpid=$! i=0 while [ "$i" -lt 80 ]; do @@ -593,14 +630,11 @@ test_arm_attaches_and_waits_for_live_fresh_watcher() { ! grep -qF 'watcher: FAILED' "$armout" || fail "arm reported FAILED for a healthy watcher" [ "$(cat "$state/.watch.lock/pid" 2>/dev/null || true)" = "$wpid" ] || fail "arm disturbed the healthy watcher's lock" is_live_non_zombie "$armpid" || fail "arm exited while the seed watcher was still healthy" - # After the seed dies without a successor, the attached arm must fail loudly. + # After the seed dies without a successor, the attached arm replaces it. kill "$wpid" 2>/dev/null || true wait "$wpid" 2>/dev/null || true - wait_for_exit "$armpid" 80 - status=$? - [ "$status" -ne 0 ] && [ "$status" -ne 124 ] || fail "attached arm did not fail after seed died (status $status)" - grep -qF 'watcher: FAILED - cycle ended without an actionable reason' "$armout" || fail "attached arm did not emit the typed cycle-end failure" - pass "arm attaches to a live fresh watcher and fails loudly when that cycle has no successor" + expect_replacement_watcher "$state" "$armout" "$armpid" + pass "arm attaches to a live fresh watcher and replaces that cycle when it ends without a successor" } test_attached_arm_signal_is_recorded_in_cycle_ledger() { @@ -757,7 +791,7 @@ SH } test_arm_waits_for_peer_beacon_after_child_stands_down() { - local dir state fakebin armout peer identity armpid status i + local dir state fakebin armout peer identity armpid i dir=$(make_case arm-peer-startup-race) state="$dir/state" fakebin="$dir/fakebin" @@ -797,14 +831,11 @@ test_arm_waits_for_peer_beacon_after_child_stands_down() { grep -qF "watcher: attached pid=$peer" "$armout" || fail "arm did not wait for and attach to the peer watcher: $(cat "$armout")" ! grep -qF 'watcher: FAILED' "$armout" || fail "arm falsely reported FAILED during peer startup race" is_live_non_zombie "$armpid" || fail "arm exited while the peer was still healthy" - # After the peer dies without a successor, the attached arm must fail loudly. + # After the peer dies without a successor, the attached arm replaces it. kill "$peer" 2>/dev/null || true wait "$peer" 2>/dev/null || true - wait_for_exit "$armpid" "$ARM_FAIL_EXIT_POLLS" - status=$? - [ "$status" -ne 0 ] && [ "$status" -ne 124 ] || fail "attached arm did not fail after peer died (status $status): $(cat "$armout")" - grep -qF 'watcher: FAILED - cycle ended without an actionable reason' "$armout" || fail "peer-attached arm did not emit the typed cycle-end failure" - pass "arm attaches to a peer watcher after child stands down and surfaces a missing successor" + expect_replacement_watcher "$state" "$armout" "$armpid" + pass "arm attaches to a peer watcher after child stands down and replaces its missing successor" } test_arm_fails_loud_when_no_fresh_watcher_confirmable() { From fd6da85612810abbcb156df3e5fdba37c4917a3f Mon Sep 17 00:00:00 2001 From: Alex William Date: Tue, 8 Sep 2026 16:39:26 +0200 Subject: [PATCH 3/8] no-mistakes(review): preserve terminal status log for agent-gone panes --- bin/fm-crew-state.sh | 16 +++++++ bin/fm-fleet-snapshot.sh | 1 - tests/fm-crew-state.test.sh | 83 ++++++++++++++++++++++++++++++++++++- 3 files changed, 98 insertions(+), 2 deletions(-) diff --git a/bin/fm-crew-state.sh b/bin/fm-crew-state.sh index 65e4518512f..359be512882 100755 --- a/bin/fm-crew-state.sh +++ b/bin/fm-crew-state.sh @@ -842,8 +842,24 @@ fi # Only an exact busy verdict reports working here, and only an exact idle # verdict permits the status-log fallback below. Missing, malformed, stale, or # unverified semantic state remains unknown. +# +# A live pane whose agent is gone (`dead shell-no-agent`) is not merely an +# unreadable busy state: the crew can have finished and had its agent exit +# outside fm-control, leaving a `done`/`failed` status log that is the last +# authoritative word on the task. Only those two terminal verbs survive the +# missing agent - a `working`, `blocked`, `needs-decision`, or `paused` log +# describes an in-flight intention that the gone agent can no longer own, so +# it stays unknown. if [ "$KIND" != secondmate ]; then BUSY_VERDICT=$(crew_busy_verdict "$BACKEND_TARGET") + if [ "$BUSY_VERDICT" = 'dead shell-no-agent' ]; then + case "$(map_log_state "$LOG_LINE")" in + done|failed) + emit "$(map_log_state "$LOG_LINE")" status-log \ + "$(status_line_note "$LOG_LINE")${SEP}agent gone, pane shell remains" + ;; + esac + fi case "${BUSY_VERDICT%% *}" in busy) emit working pane "harness busy (${BUSY_VERDICT#* })" ;; idle) ;; diff --git a/bin/fm-fleet-snapshot.sh b/bin/fm-fleet-snapshot.sh index 4d59f5fa1da..46f0b378762 100755 --- a/bin/fm-fleet-snapshot.sh +++ b/bin/fm-fleet-snapshot.sh @@ -330,7 +330,6 @@ crew_state_json() { # [] [] return 0 fi raw=$( - unset FM_CREW_STATE_META_OVERRIDE FM_CREW_STATE_STATUS_OVERRIDE fm_run_timed "$FM_SNAPSHOT_CREW_STATE_TIMEOUT" \ env "${crew_env[@]}" \ "$SCRIPT_DIR/fm-crew-state.sh" "$id" 2>/dev/null || true diff --git a/tests/fm-crew-state.test.sh b/tests/fm-crew-state.test.sh index 9a9cf6c06d1..a2223acac80 100755 --- a/tests/fm-crew-state.test.sh +++ b/tests/fm-crew-state.test.sh @@ -109,15 +109,27 @@ set -u # trimmed PATH) or errors non-definitively - so even the inventory fails, with # a message that is NOT one of the definitive no-session/no-server/no-socket # responses that fm_backend_tmux_agent_state owns as death. +# FM_FAKE_TMUX_SHELL_ONLY: the window is alive and listed, but its foreground +# process group is nothing but a shell, which is exactly the input +# fm_backend_tmux_agent_state resolves to `dead` (pane there, agent gone). The +# fake `ps` beside this file serves the process group for the fake pane tty. [ "${FM_FAKE_TMUX_UNREADABLE:-0}" = 1 ] && { printf 'no current client\n' >&2; exit 1; } case "${1:-}" in list-windows) # A successful but empty inventory: it omits the crew's window, so absence # is proved by the answer rather than by an addressed call failing. Only - # reached once display-message has already failed. + # reached once display-message has already failed. Under SHELL_ONLY the + # inventory names the crew's own window, so the pane is provably present. + [ "${FM_FAKE_TMUX_SHELL_ONLY:-0}" = 1 ] && printf '%s\n' "${FM_FAKE_TMUX_WINDOWS:-}" ;; display-message) [ "${FM_FAKE_TMUX_MISSING:-0}" = 1 ] && exit 1 + if [ "${FM_FAKE_TMUX_SHELL_ONLY:-0}" = 1 ]; then + case "${!#}" in + '#{pane_tty}') printf 'fmfake0\n'; exit 0 ;; + '#{pane_current_command}') printf 'bash\n'; exit 0 ;; + esac + fi printf '%%1\n' ;; capture-pane) [ "${FM_FAKE_TMUX_MISSING:-0}" = 1 ] && exit 1 @@ -126,6 +138,22 @@ case "${1:-}" in esac exit 0 SH + local real_ps + real_ps=$(command -v ps) || fail "missing tool for the dead-shell classifier: ps" + cat > "$fb/ps" < "$fb/herdr" <<'SH' #!/usr/bin/env bash set -u @@ -216,6 +244,8 @@ reset_fakes() { FM_FAKE_BUSY_TEXT= FM_FAKE_TMUX_MISSING=0 FM_FAKE_TMUX_UNREADABLE=0 + FM_FAKE_TMUX_SHELL_ONLY=0 + FM_FAKE_TMUX_WINDOWS="" FM_FAKE_HERDR_BUSY=0 FM_FAKE_HERDR_MISSING=0 FM_FAKE_HERDR_READ_FAIL=0 @@ -224,6 +254,7 @@ reset_fakes() { FM_FAKE_CI_LOGS="" FM_FAKE_DAEMON_DOWN=0 export FM_FAKE_AXI_STATUS FM_FAKE_AXI_STATUS_RUN FM_FAKE_RUNS_LIST FM_FAKE_BUSY FM_FAKE_BUSY_TEXT FM_FAKE_TMUX_MISSING FM_FAKE_TMUX_UNREADABLE + export FM_FAKE_TMUX_SHELL_ONLY FM_FAKE_TMUX_WINDOWS export FM_FAKE_HERDR_BUSY FM_FAKE_HERDR_MISSING FM_FAKE_HERDR_READ_FAIL FM_FAKE_HERDR_HUSK FM_FAKE_HERDR_AGENT_STATUS FM_FAKE_CI_LOGS export FM_FAKE_DAEMON_DOWN } @@ -1408,6 +1439,54 @@ test_no_run_herdr_idle_agent_status_and_idle_record_stays_idle() { } # (g) no run + idle pane -> the status-log verb, as-is +# (g'') no run + a LIVE pane whose agent is gone (the real dead-shell verdict of +# fm_backend_tmux_agent_state, driven by a foreground group that is nothing but a +# shell). A crew that finished and whose agent then exited outside fm-control - +# so nothing retired its busy record - must still surface the terminal word of +# its status log, because that log is the last authoritative account of the task +# and the captain's terminal-in-flight list is built from it. +test_no_run_live_pane_agent_gone_keeps_terminal_log() { + reset_fakes + local d; d=$(new_case husk-done) + make_repo_on_branch "$d/wt" fm/feat-husk-done + make_fakebin "$d" >/dev/null + fm_write_meta "$d/state/feat-husk-done.meta" "window=fm:fm-feat-husk-done" "worktree=$d/wt" "kind=ship" "harness=claude" + printf 'done: PR https://example.invalid/pr/1 opened\n' > "$d/state/feat-husk-done.status" + FM_FAKE_AXI_STATUS="" + FM_FAKE_TMUX_SHELL_ONLY=1 + FM_FAKE_TMUX_WINDOWS='fm-feat-husk-done' + # The busy record was never retired, so it still reads idle claude-hook. + arm_idle_record "$d/state" feat-husk-done + local out; out=$(run_crew_state "$d" feat-husk-done) + assert_contains "$out" "state: done" "terminal log survives the gone agent" + assert_contains "$out" "source: status-log" "the terminal reading is attributed to the log" + assert_contains "$out" "agent gone, pane shell remains" "the detail names why the pane could not answer" + pass "live pane with no agent still reports its terminal status-log state" +} + +# The other half of the same decision: a NONTERMINAL log describes an in-flight +# intention that the departed agent can no longer own, so it must not be +# reported as the current state. +test_no_run_live_pane_agent_gone_nonterminal_log_is_unknown() { + reset_fakes + local d; d=$(new_case husk-working) + make_repo_on_branch "$d/wt" fm/feat-husk-working + make_fakebin "$d" >/dev/null + fm_write_meta "$d/state/feat-husk-working.meta" "window=fm:fm-feat-husk-working" "worktree=$d/wt" "kind=ship" "harness=claude" + printf 'working: refactoring the parser\n' > "$d/state/feat-husk-working.status" + FM_FAKE_AXI_STATUS="" + FM_FAKE_TMUX_SHELL_ONLY=1 + FM_FAKE_TMUX_WINDOWS='fm-feat-husk-working' + arm_idle_record "$d/state" feat-husk-working + local out; out=$(run_crew_state "$d" feat-husk-working) + assert_contains "$out" "state: unknown" "a nonterminal log cannot outlive its agent" + assert_contains "$out" "shell-no-agent" "the unknown verdict names the structural cause" + case "$out" in + *"state: working"*) fail "a gone agent must not keep reporting working" ;; + esac + pass "live pane with no agent reports unknown for a nonterminal status log" +} + test_no_run_idle_pane_uses_log() { reset_fakes local d; d=$(new_case idle) @@ -2326,6 +2405,8 @@ test_no_run_herdr_alive_with_failed_read_stays_live test_no_run_herdr_husk_dead_still_reads_gone test_no_run_herdr_idle_agent_status_outranked_by_record test_no_run_herdr_idle_agent_status_and_idle_record_stays_idle +test_no_run_live_pane_agent_gone_keeps_terminal_log +test_no_run_live_pane_agent_gone_nonterminal_log_is_unknown test_no_run_idle_pane_uses_log test_no_run_idle_pane_uses_keyed_log test_no_run_idle_pane_paused From fa29e0d7a5a38005ec8d39625930746d4d81be71 Mon Sep 17 00:00:00 2001 From: Alex William Date: Tue, 8 Sep 2026 17:12:00 +0200 Subject: [PATCH 4/8] no-mistakes(review): scope content-identity dedupe to terminal outcomes --- bin/fm-classify-lib.sh | 14 +++++-- tests/fm-wake-drain-outcome-backstop.test.sh | 39 ++++++++++++++++++++ 2 files changed, 50 insertions(+), 3 deletions(-) diff --git a/bin/fm-classify-lib.sh b/bin/fm-classify-lib.sh index ad1d7e0c124..8ba86297096 100755 --- a/bin/fm-classify-lib.sh +++ b/bin/fm-classify-lib.sh @@ -982,11 +982,19 @@ EOF printf '%s' "$offset" } -# Stable identity of one captain-facing status event, independent of the -# status file's inode or byte offset. A rewritten log that still ends on the -# same terminal result keeps this identity; a genuinely new event does not. +# Stable identity of one terminal status outcome (done, failed), independent of +# the status file's inode or byte offset. A rewritten log that still ends on the +# same terminal result keeps this identity; a genuinely new result does not. +# Only an outcome verb has an identity: a blocked or needs-decision line states a +# live condition that can legitimately recur, so it stays offset-sensitive and +# the same text appended later is a new event, not the one already presented. +# Empty output means "no identity", which every caller reads as "do not dedupe". status_terminal_event_identity() { # local line=$1 + case "$(status_line_verb "$line")" in + done|failed) ;; + *) return 0 ;; + esac printf '%s' "$line" | LC_ALL=C tr -d '\r' | cksum | awk '{printf "e1:%s-%s", $1, $2}' } diff --git a/tests/fm-wake-drain-outcome-backstop.test.sh b/tests/fm-wake-drain-outcome-backstop.test.sh index c5253f4f4ef..d6df7d29f96 100755 --- a/tests/fm-wake-drain-outcome-backstop.test.sh +++ b/tests/fm-wake-drain-outcome-backstop.test.sh @@ -56,6 +56,44 @@ test_uncovered_keyless_captain_events_surface_on_the_next_main_drain() { pass "a newest keyless done, blocked, or needs-decision event with no newer branch outcome surfaces on the next main drain" } +test_recurring_nonterminal_event_surfaces_again_at_its_new_position() { + local dir state first_out second_out third_out fourth_out body blocker + dir=$(make_case recurring-blocker) + state="$dir/state" + first_out="$dir/first.out" + second_out="$dir/second.out" + third_out="$dir/third.out" + fourth_out="$dir/fourth.out" + blocker='blocked [key=bad/value]: waiting on the captain to pick a provider' + + printf '%s\n' "$blocker" > "$state/recur.status" + FM_STATE_OVERRIDE="$state" "$DRAIN" > "$first_out" \ + || fail "first recurring-blocker drain failed" + body=$(backstop_body "$first_out") + case "$body" in *"recur $blocker"*) ;; *) fail "the blocker did not surface on its first drain: $body" ;; esac + + # Unchanged bytes stay silent: the byte receipt already covers them. + FM_STATE_OVERRIDE="$state" "$DRAIN" > "$second_out" \ + || fail "unchanged recurring-blocker drain failed" + [ ! -s "$second_out" ] \ + || fail "an unchanged blocker was re-announced: $(cat "$second_out")" + + # The crew worked on, then hit the very same blocker again. This is a NEW + # event at a new position, not the one already presented. + printf 'working: retrying with the other provider\n' >> "$state/recur.status" + printf '%s\n' "$blocker" >> "$state/recur.status" + FM_STATE_OVERRIDE="$state" "$DRAIN" > "$third_out" \ + || fail "recurring-blocker drain failed" + body=$(backstop_body "$third_out") + case "$body" in *"recur $blocker"*) ;; *) fail "a recurring blocker was suppressed by its earlier identical text: $body" ;; esac + + FM_STATE_OVERRIDE="$state" "$DRAIN" > "$fourth_out" \ + || fail "post-recurrence drain failed" + [ ! -s "$fourth_out" ] \ + || fail "the recurring blocker repeated after its own presentation: $(cat "$fourth_out")" + pass "an identical nonterminal captain event appended later surfaces again, then falls silent" +} + test_newer_task_outcome_and_routine_latest_events_stay_silent() { local dir state out old dir=$(make_case covered-and-routine) @@ -545,6 +583,7 @@ test_rewritten_status_file_does_not_reannounce_the_same_terminal_result() { test_uncovered_keyless_captain_events_surface_on_the_next_main_drain test_rewritten_status_file_does_not_reannounce_the_same_terminal_result +test_recurring_nonterminal_event_surfaces_again_at_its_new_position test_newer_task_outcome_and_routine_latest_events_stay_silent test_older_or_other_task_outcome_cannot_hide_a_new_captain_event test_branch_annotation_cannot_consume_the_main_resurfacing_backstop From 5fd701ef64cd35eb2c5f7209518b4a592b6353cb Mon Sep 17 00:00:00 2001 From: Alex William Date: Tue, 8 Sep 2026 17:31:25 +0200 Subject: [PATCH 5/8] no-mistakes(review): delegate snapshot override validation to fm-crew-state --- bin/fm-fleet-snapshot.sh | 23 +++++++---------------- tests/fm-crew-state.test.sh | 18 ++++++++++++++++++ 2 files changed, 25 insertions(+), 16 deletions(-) diff --git a/bin/fm-fleet-snapshot.sh b/bin/fm-fleet-snapshot.sh index 46f0b378762..dd7c2adf718 100755 --- a/bin/fm-fleet-snapshot.sh +++ b/bin/fm-fleet-snapshot.sh @@ -298,8 +298,11 @@ last_nonempty_line() { # # snapshot without limit. Remote secondmate endpoint liveness is never read here. # A local read that hits the bound folds to state unknown. # Snapshot overrides live only in this child env: they are never exported in -# this process, and an absent or foreign captured path is omitted rather than -# passed as an empty override that would make every later worker look missing. +# this process, so a captured path reaches exactly the one task it was captured +# for and nothing survives into a later read. Whether a captured path is +# acceptable is fm-crew-state.sh's own call (fm_crew_state_apply_override): it +# refuses an absent, symlinked, or foreign path for every caller, and its +# refusal line parses into this function's JSON like any other verdict. crew_state_json() { # [] [] local id=$1 captured_meta=${2:-} captured_status=${3:-} raw rest state source detail sep local -a crew_env @@ -311,23 +314,11 @@ crew_state_json() { # [] [] FM_PROJECTS_OVERRIDE="$PROJECTS" FM_CONFIG_OVERRIDE="$CONFIG" ) - if [ -n "$captured_meta" ] && [ -f "$captured_meta" ] && [ ! -L "$captured_meta" ] \ - && [ "$(basename "$captured_meta")" = "$id.meta" ]; then + if [ -n "$captured_meta" ]; then crew_env+=(FM_CREW_STATE_META_OVERRIDE="$captured_meta") - elif [ -n "$captured_meta" ]; then - jq -n --arg raw '' --arg state unknown --arg source none \ - --arg detail "snapshot override path missing or belongs to another task ($id.meta)" \ - '{state:$state,source:$source,detail:$detail,raw:$raw}' - return 0 fi - if [ -n "$captured_status" ] && [ -f "$captured_status" ] && [ ! -L "$captured_status" ] \ - && [ "$(basename "$captured_status")" = "$id.status" ]; then + if [ -n "$captured_status" ]; then crew_env+=(FM_CREW_STATE_STATUS_OVERRIDE="$captured_status") - elif [ -n "$captured_status" ]; then - jq -n --arg raw '' --arg state unknown --arg source none \ - --arg detail "snapshot override path missing or belongs to another task ($id.status)" \ - '{state:$state,source:$source,detail:$detail,raw:$raw}' - return 0 fi raw=$( fm_run_timed "$FM_SNAPSHOT_CREW_STATE_TIMEOUT" \ diff --git a/tests/fm-crew-state.test.sh b/tests/fm-crew-state.test.sh index a2223acac80..da5b01cf961 100755 --- a/tests/fm-crew-state.test.sh +++ b/tests/fm-crew-state.test.sh @@ -1885,6 +1885,24 @@ test_snapshot_override_refuses_empty_and_foreign_paths() { assert_contains "$out" "snapshot override belongs to another task (beta.meta)" \ "a captured path for another task must be refused" + # A capture that was never taken (the snapshot's temp dir has no file for + # this task) is refused as a path, not read as a torn-down worker. + out=$(PATH="$d/fakebin:$PATH" FM_STATE_OVERRIDE="$d/state" \ + FM_CREW_STATE_META_OVERRIDE="$d/captures/beta.meta" "$CREW_STATE" beta) + assert_contains "$out" "snapshot override path missing for beta.meta" \ + "a captured path that does not exist must be refused" + assert_not_contains "$out" "no metadata for beta" \ + "a missing capture must not look like every worker is missing" + + # A symlink named like this task's capture is refused: the snapshot only ever + # hands over regular files it copied itself, so a link is not its capture. + mkdir -p "$d/captures" + ln -s "$b_meta" "$d/captures/beta.meta" + out=$(PATH="$d/fakebin:$PATH" FM_STATE_OVERRIDE="$d/state" \ + FM_CREW_STATE_META_OVERRIDE="$d/captures/beta.meta" "$CREW_STATE" beta) + assert_contains "$out" "snapshot override path missing for beta.meta" \ + "a symlinked capture must be refused even when it resolves to this task" + out=$(PATH="$d/fakebin:$PATH" FM_STATE_OVERRIDE="$d/state" \ FM_CREW_STATE_META_OVERRIDE="$b_meta" "$CREW_STATE" beta) assert_contains "$out" "state:" "a matching captured meta path must still resolve" From 1eeafd1f4f1674ae90df14608ce934e0292f5c1f Mon Sep 17 00:00:00 2001 From: Alex William Date: Tue, 8 Sep 2026 17:51:17 +0200 Subject: [PATCH 6/8] no-mistakes(review): keep settled status states through a gone agent --- bin/fm-crew-state.sh | 21 ++++++----- tests/fm-crew-state.test.sh | 73 +++++++++++++++++++++++++++++++++---- 2 files changed, 78 insertions(+), 16 deletions(-) diff --git a/bin/fm-crew-state.sh b/bin/fm-crew-state.sh index 359be512882..61553e7383a 100755 --- a/bin/fm-crew-state.sh +++ b/bin/fm-crew-state.sh @@ -844,18 +844,21 @@ fi # unverified semantic state remains unknown. # # A live pane whose agent is gone (`dead shell-no-agent`) is not merely an -# unreadable busy state: the crew can have finished and had its agent exit -# outside fm-control, leaving a `done`/`failed` status log that is the last -# authoritative word on the task. Only those two terminal verbs survive the -# missing agent - a `working`, `blocked`, `needs-decision`, or `paused` log -# describes an in-flight intention that the gone agent can no longer own, so -# it stays unknown. +# unreadable busy state: the agent can have exited outside fm-control - or been +# stopped deliberately while something external is pending - leaving a status +# log that is the last authoritative word on the task. Every settled reading +# survives the missing agent with its reason: the terminal `done`/`failed`, and +# the open `blocked`, `needs-decision`, and `paused`, which describe a condition +# that outlives the agent that reported it. `working` is the one verb that does +# NOT survive: it claims an activity in progress, which nothing is performing +# once the agent is gone, so it reads unknown rather than a stale claim. if [ "$KIND" != secondmate ]; then BUSY_VERDICT=$(crew_busy_verdict "$BACKEND_TARGET") if [ "$BUSY_VERDICT" = 'dead shell-no-agent' ]; then - case "$(map_log_state "$LOG_LINE")" in - done|failed) - emit "$(map_log_state "$LOG_LINE")" status-log \ + AGENT_GONE_STATE=$(map_log_state "$LOG_LINE") + case "$AGENT_GONE_STATE" in + done|failed|blocked|parked|paused) + emit "$AGENT_GONE_STATE" status-log \ "$(status_line_note "$LOG_LINE")${SEP}agent gone, pane shell remains" ;; esac diff --git a/tests/fm-crew-state.test.sh b/tests/fm-crew-state.test.sh index da5b01cf961..b6d5216aef1 100755 --- a/tests/fm-crew-state.test.sh +++ b/tests/fm-crew-state.test.sh @@ -1464,10 +1464,42 @@ test_no_run_live_pane_agent_gone_keeps_terminal_log() { pass "live pane with no agent still reports its terminal status-log state" } -# The other half of the same decision: a NONTERMINAL log describes an in-flight -# intention that the departed agent can no longer own, so it must not be -# reported as the current state. -test_no_run_live_pane_agent_gone_nonterminal_log_is_unknown() { +# Every settled status-log reading survives the gone agent with its reason: the +# terminal pair above, and the open conditions here, which outlive the agent +# that reported them (an unanswered decision or an external wait is still true +# once the crew is stopped). One case per surviving verb. +test_no_run_live_pane_agent_gone_keeps_open_status_states() { + reset_fakes + local d out case_name line want_state want_note + while IFS='|' read -r case_name line want_state want_note; do + [ -n "$case_name" ] || continue + reset_fakes + d=$(new_case "husk-$case_name") + make_repo_on_branch "$d/wt" "fm/feat-$case_name" + make_fakebin "$d" >/dev/null + fm_write_meta "$d/state/$case_name.meta" "window=fm:fm-$case_name" "worktree=$d/wt" "kind=ship" "harness=claude" + printf '%s\n' "$line" > "$d/state/$case_name.status" + FM_FAKE_AXI_STATUS="" + FM_FAKE_TMUX_SHELL_ONLY=1 + FM_FAKE_TMUX_WINDOWS="fm-$case_name" + arm_idle_record "$d/state" "$case_name" + out=$(run_crew_state "$d" "$case_name") + assert_contains "$out" "state: $want_state" "$case_name log must survive the gone agent" + assert_contains "$out" "source: status-log" "$case_name must be attributed to the log" + assert_contains "$out" "$want_note" "$case_name must keep its reason" + assert_contains "$out" "agent gone, pane shell remains" "$case_name must name why the pane could not answer" + done <<'EOF' +husk-failed|failed: the release job could not be retried|failed|the release job could not be retried +husk-blocked|blocked [key=provider]: which provider?|blocked|which provider? +husk-parked|needs-decision: choose REST or RPC|parked|choose REST or RPC +husk-paused|paused: holding for the vendor maintenance window|paused|holding for the vendor maintenance window +EOF + pass "live pane with no agent keeps failed, blocked, needs-decision, and paused with their reasons" +} + +# The one verb that does NOT survive: `working` claims an activity in progress, +# and nothing is performing it once the agent is gone. +test_no_run_live_pane_agent_gone_stale_working_is_unknown() { reset_fakes local d; d=$(new_case husk-working) make_repo_on_branch "$d/wt" fm/feat-husk-working @@ -1479,12 +1511,37 @@ test_no_run_live_pane_agent_gone_nonterminal_log_is_unknown() { FM_FAKE_TMUX_WINDOWS='fm-feat-husk-working' arm_idle_record "$d/state" feat-husk-working local out; out=$(run_crew_state "$d" feat-husk-working) - assert_contains "$out" "state: unknown" "a nonterminal log cannot outlive its agent" + assert_contains "$out" "state: unknown" "a stale working claim cannot outlive its agent" assert_contains "$out" "shell-no-agent" "the unknown verdict names the structural cause" case "$out" in *"state: working"*) fail "a gone agent must not keep reporting working" ;; esac - pass "live pane with no agent reports unknown for a nonterminal status log" + pass "live pane with no agent reports unknown for a stale working status log" +} + +# The structural verdict outranks the harness classifiers it is defined to +# precede: an un-retired BUSY lifecycle record must not report the crew working +# when the pane holds nothing but a shell. +test_agent_gone_outranks_a_busy_lifecycle_record() { + reset_fakes + local d; d=$(new_case husk-busy-record) + make_repo_on_branch "$d/wt" fm/feat-husk-busy + make_fakebin "$d" >/dev/null + fm_write_meta "$d/state/feat-husk-busy.meta" "window=fm:fm-feat-husk-busy" "worktree=$d/wt" "kind=ship" "harness=claude" + printf 'blocked: waiting on the captain to pick a provider\n' > "$d/state/feat-husk-busy.status" + FM_FAKE_AXI_STATUS="" + FM_FAKE_TMUX_SHELL_ONLY=1 + FM_FAKE_TMUX_WINDOWS='fm-feat-husk-busy' + local gen; gen=$("$ROOT/bin/fm-busy-event.sh" arm "$d/state" feat-husk-busy) + "$ROOT/bin/fm-busy-event.sh" apply "$d/state" feat-husk-busy busy --gen "$gen" \ + --source claude-hook --event user-prompt-submit + local out; out=$(run_crew_state "$d" feat-husk-busy) + case "$out" in + *"state: working"*) fail "a busy record outranked the shell-without-agent verdict: $out" ;; + esac + assert_contains "$out" "state: blocked" "the blocker survives an un-retired busy record" + assert_contains "$out" "waiting on the captain to pick a provider" "the blocker keeps its reason" + pass "the shell-without-agent verdict outranks an un-retired busy lifecycle record" } test_no_run_idle_pane_uses_log() { @@ -2424,7 +2481,9 @@ test_no_run_herdr_husk_dead_still_reads_gone test_no_run_herdr_idle_agent_status_outranked_by_record test_no_run_herdr_idle_agent_status_and_idle_record_stays_idle test_no_run_live_pane_agent_gone_keeps_terminal_log -test_no_run_live_pane_agent_gone_nonterminal_log_is_unknown +test_no_run_live_pane_agent_gone_keeps_open_status_states +test_no_run_live_pane_agent_gone_stale_working_is_unknown +test_agent_gone_outranks_a_busy_lifecycle_record test_no_run_idle_pane_uses_log test_no_run_idle_pane_uses_keyed_log test_no_run_idle_pane_paused From 2b3b592cb0508d7e237956eddb95bab59cd5390b Mon Sep 17 00:00:00 2001 From: Alex William Date: Tue, 8 Sep 2026 18:50:50 +0200 Subject: [PATCH 7/8] no-mistakes(review): Centralize voluntary-exit validation with shared consumer regressions --- bin/fm-control.sh | 16 ++++------------ bin/fm-pr-lib.sh | 10 ++++++++++ bin/fm-watch.sh | 14 ++------------ tests/fm-control.test.sh | 25 +++++++++++++++++++++++++ tests/fm-watch-triage.test.sh | 30 ++++++++++++++++++++++++++++++ tests/voluntary-exit-fixtures.sh | 28 ++++++++++++++++++++++++++++ 6 files changed, 99 insertions(+), 24 deletions(-) create mode 100644 tests/voluntary-exit-fixtures.sh diff --git a/bin/fm-control.sh b/bin/fm-control.sh index f3dfad442be..9f6c91ce6b1 100755 --- a/bin/fm-control.sh +++ b/bin/fm-control.sh @@ -476,17 +476,6 @@ record_voluntary_exit_wait() { mv -f "$tmp" "$rec" } -voluntary_exit_wait_record_valid() { - local rec="$STATE/$ID.voluntary-exit" - [ -f "$rec" ] && [ -r "$rec" ] && [ ! -L "$rec" ] \ - && [ -f "$STATE/$ID.pr-poll" ] && [ ! -L "$STATE/$ID.pr-poll" ] \ - && grep -qxF 'schema=fm-voluntary-exit.v1' "$rec" \ - && grep -qxF 'reason=external-wait' "$rec" \ - && grep -qxF 'wait=pr-poll' "$rec" \ - && grep -qxE 'exited_at=[0-9]+' "$rec" \ - && [ "$(wc -l < "$rec" | tr -d '[:space:]')" = 4 ] -} - clear_voluntary_exit_wait() { rm -f "$STATE/$ID.voluntary-exit" } @@ -505,7 +494,10 @@ do_exit() { # Idempotence may preserve a record written by an earlier successful # exit, but an agent already found dead was not stopped by this call. # Never mint a voluntary-wait record that could hide that true death. - voluntary_exit_wait_record_valid || clear_voluntary_exit_wait + if ! { [ -f "$STATE/$ID.pr-poll" ] && [ ! -L "$STATE/$ID.pr-poll" ] \ + && fm_voluntary_exit_record_valid "$STATE" "$ID"; }; then + clear_voluntary_exit_wait + fi else clear_voluntary_exit_wait fi diff --git a/bin/fm-pr-lib.sh b/bin/fm-pr-lib.sh index d9580dc9b4a..0b888b44471 100755 --- a/bin/fm-pr-lib.sh +++ b/bin/fm-pr-lib.sh @@ -17,6 +17,16 @@ # The receipt binds the terminal observation to the canonical registration and # lets a restart finish fixed-path removal without executing state-file bytes. +fm_voluntary_exit_record_valid() { + local rec="$1/$2.voluntary-exit" + [ -f "$rec" ] && [ -r "$rec" ] && [ ! -L "$rec" ] \ + && grep -qxF 'schema=fm-voluntary-exit.v1' "$rec" \ + && grep -qxF 'reason=external-wait' "$rec" \ + && grep -qxF 'wait=pr-poll' "$rec" \ + && grep -qxE 'exited_at=[0-9]+' "$rec" \ + && [ "$(wc -l < "$rec" | tr -d '[:space:]')" = 4 ] +} + FM_PR_PROVIDER= FM_PR_URL= FM_PR_HOST= diff --git a/bin/fm-watch.sh b/bin/fm-watch.sh index 2c91ff00cdc..6dd513dcb83 100755 --- a/bin/fm-watch.sh +++ b/bin/fm-watch.sh @@ -1174,19 +1174,9 @@ captain_call_stale_bound() { # # dead pane is expected, the poll must keep running, and a later genuine # death without this record must still alarm. task_voluntary_exit_waiting() { # - local win=$1 task=$2 rec schema reason wait_kind exited_at agent_state lines + local win=$1 task=$2 rec agent_state rec="$STATE/$task.voluntary-exit" - [ -f "$rec" ] && [ -r "$rec" ] && [ ! -L "$rec" ] || return 1 - schema=$(grep '^schema=' "$rec" 2>/dev/null | cut -d= -f2-) - reason=$(grep '^reason=' "$rec" 2>/dev/null | cut -d= -f2-) - wait_kind=$(grep '^wait=' "$rec" 2>/dev/null | cut -d= -f2-) - exited_at=$(grep '^exited_at=' "$rec" 2>/dev/null | cut -d= -f2-) - lines=$(wc -l < "$rec" 2>/dev/null | tr -d '[:space:]') - [ "$schema" = fm-voluntary-exit.v1 ] || return 1 - [ "$reason" = external-wait ] || return 1 - [ "$wait_kind" = pr-poll ] || return 1 - case "$exited_at" in ''|*[!0-9]*) return 1 ;; esac - [ "$lines" = 4 ] || return 1 + fm_voluntary_exit_record_valid "$STATE" "$task" || return 1 if [ ! -f "$STATE/$task.pr-poll" ] || [ -L "$STATE/$task.pr-poll" ]; then rm -f "$rec" return 1 diff --git a/tests/fm-control.test.sh b/tests/fm-control.test.sh index e32b4d99fee..942c7d70187 100755 --- a/tests/fm-control.test.sh +++ b/tests/fm-control.test.sh @@ -920,6 +920,31 @@ test_fm_send_still_marks_the_same_secondmate_task() { pass "fm-control's arrival leaves fm-send's from-firstmate marking untouched" } +test_voluntary_exit_record_corpus() { + local variant expected dir rec + . "$ROOT/tests/voluntary-exit-fixtures.sh" + while read -r variant expected; do + dir=$(new_case "record-$variant") + add_task "$dir" t1 claude + alive_as "$dir" zsh + rec="$dir/home/state/t1.voluntary-exit" + touch "$dir/home/state/t1.pr-poll" + voluntary_exit_fixture "$rec" "$variant" + run_control "$dir" t1 exit >/dev/null || fail "[$variant] exit failed" + if [ "$expected" = yes ]; then + [ -f "$rec" ] || fail "[$variant] valid record retired" + else + [ ! -e "$rec" ] && [ ! -L "$rec" ] || fail "[$variant] invalid record retained" + fi + done < <(voluntary_exit_cases) + pass "control validates the shared voluntary-exit corpus" +} + +if [ "${FM_TEST_VOLUNTARY_EXIT_ONLY:-0}" = 1 ]; then + test_voluntary_exit_record_corpus + exit 0 +fi +test_voluntary_exit_record_corpus test_exit_types_each_harness_verified_command test_interrupt_sends_each_harness_verified_key test_opencode_interrupts_twice_and_others_once diff --git a/tests/fm-watch-triage.test.sh b/tests/fm-watch-triage.test.sh index 5ce23255d70..03c22821190 100755 --- a/tests/fm-watch-triage.test.sh +++ b/tests/fm-watch-triage.test.sh @@ -4466,6 +4466,36 @@ test_afk_paused_changed_pane_hands_off_plain_stale() { pass "AFK changed paused panes hand off plain stale identities for daemon-owned pause triage" } +test_voluntary_exit_record_corpus() { + local variant expected dir state out capture + . "$ROOT/tests/voluntary-exit-fixtures.sh" + while read -r variant expected; do + dir=$(make_case "record-$variant") + state="$dir/state"; out="$dir/watch.out"; capture="$dir/pane.txt" + mkdir -p "$dir/data" "$dir/config" + printf 'window=test:fm-held-merge\nkind=ship\nharness=grok\nbackend=tmux\n' > "$state/held-merge.meta" + printf 'done: delivered PR\n' > "$state/held-merge.status" + printf '%s' "$(seen_sig "$state/held-merge.status")" > "$state/.seen-held-merge_status" + touch "$state/held-merge.pr-poll" + voluntary_exit_fixture "$state/held-merge.voluntary-exit" "$variant" + hold_watch_surface "$dir" "$out" "$capture" 'idle first' || fail "[$variant] first surface missing" + ack_stopped_cycle "$state" || fail "[$variant] acknowledgement failed" + if [ "$expected" = yes ]; then + hold_watch_churn "$dir" "$out" "$capture" 'idle changed' 1 || fail "[$variant] valid wait re-alarmed" + [ "$(hold_stale_wakes "$state")" -eq 0 ] || fail "[$variant] valid wait queued stale" + else + hold_watch_surface "$dir" "$out" "$capture" 'idle changed' || fail "[$variant] invalid wait hid death" + [ "$(hold_stale_wakes "$state")" -eq 1 ] || fail "[$variant] death wake missing" + fi + done < <(voluntary_exit_cases) + pass "watcher validates the shared voluntary-exit corpus" +} + +if [ "${FM_TEST_VOLUNTARY_EXIT_ONLY:-0}" = 1 ]; then + test_voluntary_exit_record_corpus + exit 0 +fi +test_voluntary_exit_record_corpus test_status_span_actionable_classifier test_status_span_survives_a_later_routine_append test_status_span_respects_decision_closure diff --git a/tests/voluntary-exit-fixtures.sh b/tests/voluntary-exit-fixtures.sh new file mode 100644 index 00000000000..86528f77e28 --- /dev/null +++ b/tests/voluntary-exit-fixtures.sh @@ -0,0 +1,28 @@ +#!/usr/bin/env bash + +voluntary_exit_cases() { + printf '%s\n' 'valid yes' 'reordered yes' 'schema no' 'reason no' 'wait no' \ + 'epoch no' 'duplicate no' 'extra no' 'missing no' 'symlink no' +} + +voluntary_exit_fixture() { + local rec=$1 variant=$2 schema=fm-voluntary-exit.v1 reason=external-wait wait_kind=pr-poll epoch=1 + case "$variant" in + schema) schema=fm-voluntary-exit.v2 ;; + reason) reason=crash ;; + wait) wait_kind=ci-poll ;; + epoch) epoch=-1 ;; + missing) return 0 ;; + esac + if [ "$variant" = reordered ]; then + printf 'exited_at=123\nwait=pr-poll\nreason=external-wait\nschema=fm-voluntary-exit.v1\n' > "$rec" + else + printf 'schema=%s\nreason=%s\nwait=%s\nexited_at=%s\n' \ + "$schema" "$reason" "$wait_kind" "$epoch" > "$rec" + fi + case "$variant" in + duplicate) printf 'wait=pr-poll\n' >> "$rec" ;; + extra) printf 'unknown=value\n' >> "$rec" ;; + symlink) mv "$rec" "$rec.target"; ln -s "$rec.target" "$rec" ;; + esac +} From b86b9d2c676970f8841b43fe83057f1bde1f1aa3 Mon Sep 17 00:00:00 2001 From: Alex William Date: Tue, 8 Sep 2026 19:06:04 +0200 Subject: [PATCH 8/8] no-mistakes(document): Align supervision documentation with current recovery behavior --- bin/fm-control.sh | 6 +----- bin/fm-pr-lib.sh | 5 +++++ docs/agent-control.md | 7 ++++++- docs/architecture.md | 9 +++++---- docs/pi-supervision-branch.md | 3 +++ 5 files changed, 20 insertions(+), 10 deletions(-) diff --git a/bin/fm-control.sh b/bin/fm-control.sh index 9f6c91ce6b1..e9280b6f8bd 100755 --- a/bin/fm-control.sh +++ b/bin/fm-control.sh @@ -449,11 +449,7 @@ retire_busy_incarnation() { # state/.voluntary-exit is the durable record that an explicit exit verb # stopped the agent while an external wait (an armed PR merge poll) still -# stands. Schema: -# schema=fm-voluntary-exit.v1 -# reason=external-wait -# wait=pr-poll -# exited_at= +# stands. fm_voluntary_exit_record_valid in fm-pr-lib.sh owns its grammar. # Relaunch removes it. Teardown removes it. The watcher ignores it once the # poll sidecar is gone, so a later genuine death is not hidden. record_voluntary_exit_wait() { diff --git a/bin/fm-pr-lib.sh b/bin/fm-pr-lib.sh index 0b888b44471..adf3e58d6df 100755 --- a/bin/fm-pr-lib.sh +++ b/bin/fm-pr-lib.sh @@ -17,6 +17,11 @@ # The receipt binds the terminal observation to the canonical registration and # lets a restart finish fixed-path removal without executing state-file bytes. +# Shared grammar owner for state/.voluntary-exit: a readable regular, +# non-symlink record with exactly four newline-terminated lines, in any order: +# schema=fm-voluntary-exit.v1, reason=external-wait, wait=pr-poll, and +# exited_at=. Poll presence and agent liveness are +# consumer checks, not record grammar. fm_voluntary_exit_record_valid() { local rec="$1/$2.voluntary-exit" [ -f "$rec" ] && [ -r "$rec" ] && [ ! -L "$rec" ] \ diff --git a/docs/agent-control.md b/docs/agent-control.md index fae72ce678c..3488b55d8f8 100644 --- a/docs/agent-control.md +++ b/docs/agent-control.md @@ -31,9 +31,14 @@ A recorded `harness=` is not always an exact adapter name: a task launched from | Verb | Effect | Postcondition | | --- | --- | --- | | `interrupt` | Deliver the harness's verified interrupt sequence while leaving the agent running. | Delivery succeeds while the endpoint still exists and the agent is still alive where the backend can classify that; cancellation is confirmed only from an adapter-owned acknowledgement and otherwise reports `cancel=unconfirmed`. | -| `exit` | Stop the agent, preserving the endpoint, the worktree, and every uncommitted change. When a PR merge poll is still armed, it also writes `state/.voluntary-exit` so the dead pane is an expected external wait rather than a repeating stale alarm. | The backend's recovery-grade classifier reports the agent gone. Already-stopped is idempotent success. The merge poll keeps running. | +| `exit` | Stop the agent, preserving the endpoint, the worktree, and every uncommitted change. When this call stops a live agent with a PR merge poll armed, it records an expected external wait; an already-dead agent cannot create that record. | The backend's recovery-grade classifier reports the agent gone. Already-stopped is idempotent success. The merge poll keeps running. | | `relaunch` | Replace the running agent with a new one in the same endpoint and worktree, on the exact recorded adapter or an explicitly chosen harness, model, and effort. | The new agent is alive on the recorded endpoint, and the durable record names the harness that is actually running. | +A recorded voluntary exit keeps the merge poll running and bounds stale-pane churn to the watcher's pause recheck cadence rather than silencing supervision indefinitely. +The watcher requires the poll to remain armed and the endpoint to remain a shell without an agent; a missing endpoint is not an expected wait. +Relaunch and teardown retire the record, and the watcher retires it when the poll disappears. +`bin/fm-pr-lib.sh` owns record validation; `tests/fm-control.test.sh` and `tests/fm-watch-triage.test.sh` exercise both consumers with `tests/voluntary-exit-fixtures.sh`. + An exit that delivers lifecycle input but cannot prove the agent stopped fails with `exit=unconfirmed`, reports the observed agent state and any interrupt cancellation claim, and never claims that nothing changed. Interrupt never rewrites busy state as proof of its own success. Claude exposes no lifecycle acknowledgement for a manual interrupt, so delivery succeeds with `cancel=unconfirmed` and its adapter-owned busy state remains as observed. diff --git a/docs/architecture.md b/docs/architecture.md index 58b900786d4..37c5da59f8c 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -83,7 +83,8 @@ 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. A terminal failed run whose only failure is the ci monitor step, after every substantive step completed and the same marker reads checks green, also reports done with the run's PR URL, because a monitor whose only remaining job is to observe a human merge decision must not convert the absence of that decision into a failure verdict. In the coarse runs-ledger fallback, which has no steps table and no ci log, a terminal failed record whose daemon an explicit `daemon status` probe proves down reports unknown as unverified instead: an instrument failure must never read as work failure. -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. +Only when no matching run exists does it consult semantic busy state; exact busy reports working and exact idle permits fallback to a status-log event whose verb maps to a recognized run-state. +A surviving shell without an agent preserves status-log `done`, `failed`, `blocked`, `needs-decision` (reported as `parked`), and `paused` with their reasons, but not stale `working`; other unknown or dead verdicts remain unknown. 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. 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. @@ -173,7 +174,7 @@ Codex and standalone Kimi classify unknown behind explicit probes until a semant Missing, malformed, stale, untrusted, or unverified semantic state is unknown, never idle, and unknown is never promoted to busy either. Ordinary task-state consumers act only on an exact busy verdict, so an unreadable worker surfaces for a closer look instead of being absorbed as still-working or written off as finished. -Endpoint death is the only process-level override and yields dead; child processes, CPU, process sleep state, and marker modification times are not state signals. +Process-level death overrides follow `fm_busy_classify_live` in `bin/fm-busy-lib.sh`; CPU, process sleep state, and marker modification times are not semantic busy-state signals. `state/.turn-ended` files remain wake notifications, not current state. Each record is bound to an incarnation token minted when the task's wiring is armed, so an event from a superseded incarnation is rejected rather than applied, and a record left behind by one classifies unknown. @@ -191,10 +192,10 @@ Unknown backend names fail loudly. For compatibility, default tmux tasks do not write `backend=tmux`; every reader treats a missing `backend=` field as `tmux`. `fm-watch.sh` decides each window's busy state through the semantic contract above rather than by polling the backend for rendered text. Herdr's native `agent.get` verdict still participates, but only as evidence of activity: a native `busy` is accepted when the task has no record of its own, while a native `idle` is not, because `agent.get` reports generation state and reads idle while a worker blocks on its own long-running foreground tool call. -tmux, zellij, orca, and cmux expose no native busy primitive at all, so a task on those backends is classified purely from its adapter's own lifecycle record. +tmux, zellij, orca, and cmux expose no native busy primitive; their semantic sources and structural overrides follow the [busy-state contract](#busy-state-is-semantic-per-adapter). That poll loop is still the default event source for backends with no native push events, so this stays an extraction of the abstraction rather than a watcher rewrite. For capable Herdr sessions, the same watcher replaces its terminal sleep with a bounded native event wait that immediately surfaces `blocked`; [Push events and polling fallback](herdr-backend.md#push-events-and-polling-fallback) owns the current mechanism and capability gates, while [runtime backend verification](verification/runtime-backends.md#native-blocked-event) owns the active evidence. -The deeper session-start agent-process liveness probe is separate from that busy-state poll: tmux and Herdr have verified classifiers for secondmate recovery, Zellij remains unverified, and Orca and cmux do not support secondmate spawns. +The recovery-grade agent-process probe also supplies the live busy classifier's structural death override: tmux and Herdr have verified classifiers for secondmate recovery, Zellij remains unverified, and Orca and cmux do not support secondmate spawns. Herdr can be selected explicitly or by runtime auto-detection: Treehouse remains its worktree provider, [`herdr-backend.md`](herdr-backend.md) owns current setup, CI coverage, and safety limits, and [`verification/runtime-backends.md`](verification/runtime-backends.md#herdr) owns active empirical evidence. Herdr uses one tab per task; [Watching and task containers](herdr-backend.md#watching-and-task-containers) owns launcher-bound workspace placement, the label-only fallback, and recovery scope. Its default-on presentation projection may place one clean new task in a disposable workspace without changing endpoint authority or lifecycle ownership; [Presentation spaces](herdr-backend.md#presentation-spaces) owns that conditional design, the Herdr version floor its unconfigured default is gated behind, and its narrow home-local restored-shell cleanup at locked session start. diff --git a/docs/pi-supervision-branch.md b/docs/pi-supervision-branch.md index 76cb84a8b85..01936d243e7 100644 --- a/docs/pi-supervision-branch.md +++ b/docs/pi-supervision-branch.md @@ -85,6 +85,9 @@ Both read the same uncached ownership authority: the lock's process ancestry is Every main-actor wake drain checks each task's newest non-blank status event against the latest supervision-branch outcome that causally covers that task's status log. When that event is terminal or otherwise captain-facing and remains uncovered, the drain prints it once in `STATUS OUTCOME BACKSTOP`, even if the original queue row was already acknowledged; routine events stay silent, and valid open decisions remain owned by `OPEN DECISIONS`. The one-shot backstop cursor is independent from signal annotation, so a delayed signal can still present its status context without repeating the recovered event. +For `done` and `failed`, the drain additionally retains the latest presented result's content identity across status-log replacement; `bin/fm-classify-lib.sh` owns that identity and its teardown retirement. +The heartbeat backstop suppresses that same result only when it is the entire newly scanned actionable span, so a different buried event still wakes supervision. +`blocked` and `needs-decision` remain offset-sensitive: identical text appended later is a new event rather than a duplicate terminal result. The drain reads one fixed-size per-task outcome index instead of scanning append-only outcome history and inspects at most the final 64 KiB of each status log. Status provenance added to new outcome rows distinguishes covered and genuinely later events even within one timestamp second. Legacy outcomes predate that causal position, so equal-second migration cannot prove order and deliberately favors surfacing a plausibly later event; this can rarely duplicate an already handled legacy event.