From f40c1d6ef526640618882d87a6a6be6daa63f10f Mon Sep 17 00:00:00 2001 From: dnth Date: Wed, 23 Sep 2026 00:05:17 +0800 Subject: [PATCH 01/21] feat: custody-checked bounded auto-recovery for stalled workers When a steering inbox record stays unhandled past the re-ring ladder or its endpoint proves dead, the watcher now runs fm-stall-recovery.sh before publishing the stale wake. The helper re-proves the record is still the oldest unhandled instruction, classifies the endpoint as live-non-turning or missing, requires a clean non-run crew-state, a clean worktree with no unlanded commits, and the durable fm- Treehouse lease, then re-proves the chain immediately before invoking fm-control relaunch. A handled record reports recovered; a busy worker or a just-published relaunch defers; every unprovable or unsafe shape escalates to the ordinary stale wake with the reason appended. One automatic relaunch per stalled record is bounded by a per-record attempt counter that resets only when the inbox empties. OMP gains semantic busy wiring (omp-ext source on turn_start/turn_end/session_shutdown) so live OMP workers classify instead of falling through to unknown, and an OMP relaunch retires the prior generation's request.* doorbell receipts after every refusal gate so a stale .acked tombstone cannot suppress the replacement incarnation's doorbell. --- bin/fm-busy-lib.sh | 3 +- bin/fm-spawn.sh | 48 +- bin/fm-stall-recovery.sh | 267 ++++++++++ bin/fm-task-inbox-lib.sh | 25 +- bin/fm-watch.sh | 42 ++ docs/architecture.md | 6 +- docs/scripts.md | 1 + tests/fm-stall-recovery.test.sh | 850 ++++++++++++++++++++++++++++++++ 8 files changed, 1236 insertions(+), 6 deletions(-) create mode 100755 bin/fm-stall-recovery.sh create mode 100755 tests/fm-stall-recovery.test.sh diff --git a/bin/fm-busy-lib.sh b/bin/fm-busy-lib.sh index 100e20777a8..f94906e6c61 100755 --- a/bin/fm-busy-lib.sh +++ b/bin/fm-busy-lib.sh @@ -31,6 +31,7 @@ # pi-ext Pi/pi-signed per-task extension (agent_start/agent_settled) # opencode-plugin OpenCode per-task plugin (session.status) # claude-hook Claude lifecycle hooks (UserPromptSubmit/Stop/StopFailure/SessionEnd) +# omp-ext OMP per-task extension (turn_start/turn_end/session_shutdown) # hermes-hook Hermes lifecycle bridge (plugin-forwarded TUI events, # plus compatible shell-hook events) # codex-hook, codex-appserver reserved: Codex, gated by @@ -178,8 +179,8 @@ fm_busy_sources_for_harness() { # adapter='codex-hook codex-appserver' ;; opencode*) adapter=opencode-plugin ;; + omp) adapter=omp-ext ;; pi|pi-signed) adapter=pi-ext ;; - hermes) adapter=hermes-hook ;; kimi*) fm_busy_kimi_verified || { printf ''; return 0; } adapter='kimi-wire kimi-hook' diff --git a/bin/fm-spawn.sh b/bin/fm-spawn.sh index 3ef4e54e438..d5ec7ccded9 100755 --- a/bin/fm-spawn.sh +++ b/bin/fm-spawn.sh @@ -4261,7 +4261,7 @@ if [ "$KIND" != secondmate ]; then ;; esac case "$HARNESS" in - claude*|opencode*|pi|pi-signed) + claude*|opencode*|pi|pi-signed|omp) BUSY_GEN=$("$FM_ROOT/bin/fm-busy-event.sh" arm "$STATE_REAL" "$ID") || { echo "error: failed to arm the busy-state contract for $ID" >&2 exit 1 @@ -4399,12 +4399,23 @@ EOF omp) rm -f "$OMP_READY" "$OMP_STARTED" "$OMP_DOORBELL_READY" "$OMP_DOORBELL_FAILED" cat > "$STATE/$ID.omp-ext.ts" < + new Promise((resolve) => { + execFile("$FM_ROOT/bin/fm-busy-event.sh", [ + "apply", "$STATE_REAL", "$ID", state, + "--gen", "$BUSY_GEN", "--source", "omp-ext", "--event", event, + ], () => resolve(undefined)); + }); export default function (omp: any) { const taskInboxDoorbell = installTaskInboxDoorbell(omp, { inboxDir: "$STATE_REAL/$ID.inbox", @@ -4423,12 +4434,17 @@ export default function (omp: any) { omp.on("turn_start", () => { taskInboxDoorbell.notifyTurnStart(); execFile("touch", ["$OMP_STARTED"]); + busyEvent("busy", "turn-start"); }); omp.on("turn_end", () => { taskInboxDoorbell.notifyTurnEnd(); execFile("$TURNEND_SIGNAL", ["$STATE_REAL", "$ID", "$SPAWN_GEN"]); + busyEvent("idle", "turn-end"); + }); + omp.on("session_shutdown", () => { + taskInboxDoorbell.retire(); + busyEvent("idle", "session-shutdown"); }); - omp.on("session_shutdown", taskInboxDoorbell.retire); } EOF ;; @@ -4922,6 +4938,32 @@ sleep 0.3 if [ "$OMP_LAUNCH_TEMPLATE" -eq 1 ] && [ "$HARNESS" = omp ]; then LAUNCH="/bin/bash -c $(shell_quote "$LAUNCH")" fi +if [ "$HARNESS" = omp ] && [ "$RELAUNCH" -eq 1 ]; then + # Generation reconciliation: every request.* receipt under the doorbell + # requests dir was written for the PRIOR incarnation's doorbell. The + # extension only reconciles .ambiguous/.awaiting-turn on activate, so a + # surviving .acked/.delivered/.unproven tombstone would suppress the + # replacement worker's doorbell for the same still-unhandled record - the + # exact delivered-no-turn stall this relaunch recovers from. This runs only + # here, after every refusal gate above has passed and immediately before + # the replacement launch is submitted, so a refused relaunch never deletes + # the prior generation's receipts; the inbox records themselves live under + # state/.inbox and are untouched. + OMP_REQUESTS_DIR="$STATE/$ID.omp-doorbell-ready.requests" + if [ -d "$OMP_REQUESTS_DIR" ] && [ ! -L "$OMP_REQUESTS_DIR" ]; then + for request_artifact in "$OMP_REQUESTS_DIR"/request.*; do + [ -e "$request_artifact" ] || [ -L "$request_artifact" ] || continue + if [ -L "$request_artifact" ] || [ ! -f "$request_artifact" ]; then + echo "error: refusing OMP relaunch through unsafe request entry: $request_artifact" >&2 + exit 1 + fi + rm -f "$request_artifact" || { + echo "error: refusing OMP relaunch because a stale doorbell receipt could not be retired: $request_artifact" >&2 + exit 1 + } + done + fi +fi if [ "$BACKEND" = herdr ]; then spawn_send_text_line "$T" "$LAUNCH" || { echo "error: Herdr launch pane did not reach a proven idle shell; refusing to submit $HARNESS" >&2 diff --git a/bin/fm-stall-recovery.sh b/bin/fm-stall-recovery.sh new file mode 100755 index 00000000000..fc3990b3603 --- /dev/null +++ b/bin/fm-stall-recovery.sh @@ -0,0 +1,267 @@ +#!/usr/bin/env bash +# fm-stall-recovery.sh - custody-checked bounded auto-recovery for a stalled +# worker, invoked by bin/fm-watch.sh at the two points where a durable steering +# instruction has provably outlived its delivery budget: a spent re-ring ladder +# on an idle pane, or a positively dead/missing endpoint that bypasses the +# ladder. +# +# Usage: fm-stall-recovery.sh +# is endpoint-unavailable (dead/missing endpoint) or +# ladder-exhausted (delivery attempts spent on an idle pane). +# +# Output contract: exactly one line, "verdict= detail=". +# recovered - the instruction was already handled; nothing to do. +# deferred - no lifecycle action taken and none needed right now: the +# record was handled between the watcher's decision and this +# check, the worker is provably busy, or a relaunch was just +# published and the episode stays pending until the record is +# handled or the ladder re-escalates. +# escalate - recovery is unsafe, unproven, or exhausted; the caller keeps +# the ordinary stale wake. Any non-zero exit or missing verdict +# is also treated as escalate by the caller. +# +# What "safe" means here (every check fails closed to escalate): +# - The named record is still the oldest UNHANDLED inbox record. Transport +# receipts (.acked/.unproven/.awaiting-turn under the doorbell request dir) +# are never consulted: a stale receipt neither triggers nor suppresses +# recovery, and a record moved to handled/ at any point cancels the action. +# - The endpoint is positively classified: dead/missing takes the +# missing-endpoint path (no exit is sent; the launch owner recreates the +# endpoint), alive takes the live-non-turning path (the old agent is +# exited first). Ambiguous, unreadable, or unverified states escalate. +# - A live endpoint must also read an explicit idle busy verdict; busy +# defers (the worker may be mid-turn on the instruction) and unknown +# escalates (a probe failure is not custody proof). +# - fm-crew-state must show no active validation run: working, parked, +# blocked, and declared-paused states all escalate (a parked gate, a +# declared external wait, and a worker-declared blocker are firstmate +# business, not stall recovery). Terminal done/failed may proceed, and +# unknown may proceed only on the missing-endpoint path. +# - The recorded worktree must be clean AND hold no commit absent from every +# remote-tracking ref (local-only mode instead requires every commit +# merged into the local default branch). Unlanded work blocks automatic +# action and escalates; the check is local-only - no gh or fetch - so the +# watcher can never hang on a remote. +# - One automatic relaunch per stalled instruction: the per-record attempt +# marker under the inbox bounds retries, and an emptied inbox resets it. +# - The durable fm- worktree lease, same-worktree/branch/commits +# preservation, and the no-shared-daemon boundary are enforced by +# bin/fm-control.sh relaunch itself; this script never moves inbox +# records, never writes new inbox records, and never touches the +# no-mistakes daemon. +# +# A published relaunch is deliberately NOT reported as recovered: it is a +# pending episode. The durable instruction's terminal outcome is either its +# handled/ move (quiet) or the bounded re-escalation the reset ladder produces +# when the replacement also fails to act. +# +# Audit: every verdict appends one line to state/.stall-recovery; the +# relaunch transaction itself journals to state/.control-relaunch, and a +# note: line on state/.status records the published recovery. +# +# Tunables (env): +# FM_STALL_RECOVERY_MAX automatic relaunches per stalled record (1) +# FM_CREW_STATE_BIN crew-state executable override (tests) +# FM_STALL_RECOVERY_CONTROL_BIN lifecycle executable override (tests) +set -u + +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}" +DATA="${FM_DATA_OVERRIDE:-$FM_HOME/data}" + +# shellcheck source=bin/fm-backend.sh +. "$SCRIPT_DIR/fm-backend.sh" +# shellcheck source=bin/fm-busy-lib.sh +. "$SCRIPT_DIR/fm-busy-lib.sh" +# shellcheck source=bin/fm-worktree-clean-lib.sh +. "$SCRIPT_DIR/fm-worktree-clean-lib.sh" +# shellcheck source=bin/fm-task-inbox-lib.sh +. "$SCRIPT_DIR/fm-task-inbox-lib.sh" + +FM_CREW_STATE_BIN="${FM_CREW_STATE_BIN:-$SCRIPT_DIR/fm-crew-state.sh}" +FM_STALL_RECOVERY_CONTROL_BIN="${FM_STALL_RECOVERY_CONTROL_BIN:-$SCRIPT_DIR/fm-control.sh}" + +ID=${1:-} +RECORD=${2:-} +TRIGGER=${3:-} +JOURNAL="$STATE/$ID.stall-recovery" +STATUS_FILE="$STATE/$ID.status" + +journal() { # + { + printf 'ts=%s id=%s trigger=%s record=%s' "$(date -u +%Y-%m-%dT%H:%M:%SZ)" "$ID" "$TRIGGER" "${RECORD##*/}" + printf ' %s\n' "$*" + } >> "$JOURNAL" 2>/dev/null || true +} + +verdict() { # + journal "verdict=$1 detail=$2" + printf 'verdict=%s detail=%s\n' "$1" "$2" + exit 0 +} + +status_note() { # + [ -f "$STATUS_FILE" ] || [ -d "$STATE" ] || return 0 + printf 'note: %s\n' "$1" >> "$STATUS_FILE" 2>/dev/null || true +} + +# --- eligibility gates ------------------------------------------------------ +case "$ID" in ''|*[!A-Za-z0-9._-]*) verdict escalate "invalid task id" ;; esac +META="$STATE/$ID.meta" +[ -f "$META" ] && [ ! -L "$META" ] || verdict escalate "no task metadata" +KIND=$(fm_meta_get "$META" kind) +case "$KIND" in ''|ship|scout) ;; *) verdict escalate "kind=$KIND is not an ordinary direct report" ;; esac +[ -z "$(fm_meta_get "$META" remote_host)" ] || verdict escalate "remote placement; recover on its own host" + +# The named record must still be the oldest unhandled instruction. A record +# that moved to handled/ (or an emptied inbox) ends the episode quietly. +dir=$(fm_task_inbox_dir "$STATE" "$ID") +oldest=$(fm_task_inbox_oldest_unhandled "$STATE" "$ID" 2>/dev/null || true) +if [ -z "$oldest" ]; then + verdict recovered "inbox empty; instruction already handled" +fi +if [ "$oldest" != "$RECORD" ]; then + verdict deferred "record ${RECORD##*/} no longer the oldest unhandled (${oldest##*/} is); late handling cancels this action" +fi + +# prove_custody: re-prove every precondition for a lifecycle action against +# CURRENT state - endpoint classification, busy verdict, crew/run state, +# worktree cleanliness, landed work, and the durable fm- lease. Called +# directly (never in a command substitution) so its PATH_KIND, WT, MODE, +# PROJ, BACKEND, and TARGET bindings reach the caller; the refusal reason is +# published through the CUSTODY_DETAIL global. Returns 1 on any failed or +# unprovable check, 2 when the worker is provably busy (a defer, not an +# escalation), 0 on success. All reads are local: no gh or fetch can ever +# stall the watcher. +prove_custody() { + local state busy crew_line crew_state unpushed unmerged default_ref cand pool_state lease_holder + PATH_KIND= + CUSTODY_DETAIL= + WT=$(fm_meta_get "$META" worktree) + MODE=$(fm_meta_get "$META" mode) + PROJ=$(fm_meta_get "$META" project) + fm_backend_validate_task_endpoint "$META" "$ID" >/dev/null 2>&1 \ + || { CUSTODY_DETAIL='endpoint metadata failed validation'; return 1; } + BACKEND=$FM_BACKEND_VALIDATED_BACKEND + TARGET=$FM_BACKEND_VALIDATED_TARGET + state=$(fm_backend_agent_state "$BACKEND" "$TARGET" "$META" 2>/dev/null || printf 'unreadable') + case "$state" in + alive) PATH_KIND=live-non-turning ;; + dead|missing) PATH_KIND=missing-endpoint ;; + *) CUSTODY_DETAIL="endpoint state '$state' is not positively classified"; return 1 ;; + esac + if [ "$PATH_KIND" = live-non-turning ]; then + busy=$(fm_busy_classify_meta "$META" "$ID" "$STATE" 2>/dev/null || printf 'unknown') + case "${busy%% *}" in + idle) ;; + busy) CUSTODY_DETAIL="worker is busy ($busy); it may be mid-turn on the instruction"; return 2 ;; + *) CUSTODY_DETAIL="busy verdict '$busy' is not a custody proof"; return 1 ;; + esac + fi + crew_line=$("$FM_CREW_STATE_BIN" "$ID" 2>/dev/null || true) + crew_state=$(printf '%s' "$crew_line" | sed -n 's/^state: \([a-z-]*\).*/\1/p' | head -1) + case "$crew_state" in + working|parked|blocked|paused) + CUSTODY_DETAIL="crew-state $crew_state needs firstmate, not auto-relaunch"; return 1 ;; + done|failed) ;; + unknown) + [ "$PATH_KIND" = missing-endpoint ] \ + || { CUSTODY_DETAIL='crew-state unknown with a live endpoint is ambiguous'; return 1; } ;; + *) CUSTODY_DETAIL="crew-state '${crew_state:-unreadable}' is not a clean non-run state"; return 1 ;; + esac + [ -n "$WT" ] && [ -d "$WT" ] || { CUSTODY_DETAIL='recorded worktree missing'; return 1; } + fm_worktree_is_clean "$WT" || { CUSTODY_DETAIL='worktree has uncommitted changes'; return 1; } + if [ "$MODE" = local-only ]; then + default_ref= + for cand in main master; do + if git -C "$PROJ" show-ref --verify --quiet "refs/heads/$cand" 2>/dev/null; then + default_ref=$cand + break + fi + done + [ -n "$default_ref" ] || { CUSTODY_DETAIL='local-only task has no resolvable default branch'; return 1; } + unmerged=$(git -C "$WT" log --format=%H HEAD --not "$default_ref" -- 2>/dev/null) \ + || { CUSTODY_DETAIL="cannot inspect worktree commits against $default_ref"; return 1; } + [ -z "$unmerged" ] || { CUSTODY_DETAIL="local-only worktree has commits not merged into $default_ref"; return 1; } + else + unpushed=$(git -C "$WT" log --format=%H HEAD --not --remotes -- 2>/dev/null) \ + || { CUSTODY_DETAIL='cannot inspect worktree commits against remotes'; return 1; } + [ -z "$unpushed" ] || { CUSTODY_DETAIL='worktree has commits not on any remote-tracking ref'; return 1; } + fi + # The durable fm- lease proof mirrors bin/fm-spawn.sh's + # relaunch_worktree_lease_proven, applied to BOTH paths here because the + # launch owner only re-proves it on the gone-endpoint path. + fm_treehouse_pool_slot "$PROJ" "$WT" \ + || { CUSTODY_DETAIL='recorded worktree is not a Treehouse pool slot of the recorded project'; return 1; } + fm_treehouse_slot_owner_state "$WT" "$ID" + case "$FM_TREEHOUSE_SLOT_OWNER" in + mine|absent) ;; + other) CUSTODY_DETAIL="pool slot is claimed by task ${FM_TREEHOUSE_SLOT_OWNER_ID:-unknown}"; return 1 ;; + *) CUSTODY_DETAIL='slot-owner claim cannot be read safely'; return 1 ;; + esac + command -v jq >/dev/null 2>&1 \ + || { CUSTODY_DETAIL='jq is required to verify the durable worktree lease'; return 1; } + pool_state="$(dirname "$(dirname "$(cd "$WT" && pwd -P)")")/treehouse-state.json" + lease_holder=$(jq -r --arg p "$(cd "$WT" && pwd -P)" \ + '.worktrees[]? | select(.path == $p and .leased == true) | .lease_holder // empty' \ + "$pool_state" 2>/dev/null || true) + [ "$lease_holder" = "fm-$ID" ] \ + || { CUSTODY_DETAIL="recorded worktree has no durable Treehouse lease held by fm-$ID"; return 1; } + return 0 +} + +# Gate proof: full custody chain before any lifecycle decision. +prove_custody || case $? in + 2) verdict deferred "$CUSTODY_DETAIL" ;; + *) verdict escalate "$CUSTODY_DETAIL" ;; +esac + +# Bounded retry: one automatic relaunch per stalled instruction record. +max_attempts=${FM_STALL_RECOVERY_MAX:-1} +case "$max_attempts" in ''|*[!0-9]*) max_attempts=1 ;; esac +attempts_file="$dir/.recovery-attempts" +attempts_record= attempts_count=0 +IFS=$(printf '\t') read -r attempts_record attempts_count </dev/null || true) +EOF +[ "$attempts_record" = "${RECORD##*/}" ] || attempts_count=0 +case "$attempts_count" in ''|*[!0-9]*) attempts_count=0 ;; esac +[ "$attempts_count" -lt "$max_attempts" ] \ + || verdict escalate "automatic recovery already attempted for ${RECORD##*/}; escalating per bounded-retry policy" + +# Final re-check immediately before the lifecycle action, in strict order: +# re-prove the full custody chain (a worker can become busy, enter a run, or +# lose its worktree between the first proof and the relaunch), then re-prove +# the record itself LAST so a handled move during the custody probe still +# cancels the action. +prove_custody || case $? in + 2) verdict deferred "$CUSTODY_DETAIL" ;; + *) verdict escalate "$CUSTODY_DETAIL" ;; +esac +oldest=$(fm_task_inbox_oldest_unhandled "$STATE" "$ID" 2>/dev/null || true) +[ -n "$oldest" ] || verdict recovered "inbox emptied before relaunch; instruction handled" +[ "$oldest" = "$RECORD" ] || verdict deferred "record ${RECORD##*/} handled or superseded before relaunch" + +# --- bounded lifecycle action ------------------------------------------------ + +unhandled=$(cd "$dir" 2>/dev/null && printf '%s ' *.msg 2>/dev/null || true) +note="Stall auto-recovery ($TRIGGER, $PATH_KIND): the previous worker stopped acting on doorbells while instruction(s) ${unhandled:-${RECORD##*/}} stayed unhandled. The worktree, branch, and commits are exactly as that worker left them; nothing was discarded. Read and act on the inbox first." +printf '%s\t%s\n' "${RECORD##*/}" "$((attempts_count + 1))" > "$attempts_file" 2>/dev/null \ + || verdict escalate "cannot persist the recovery-attempt bound at $attempts_file" + +control_out=$(FM_HOME="$FM_HOME" FM_STATE_OVERRIDE="$STATE" FM_DATA_OVERRIDE="$DATA" \ + FM_CONFIG_OVERRIDE="${FM_CONFIG_OVERRIDE:-$FM_HOME/config}" \ + "$FM_STALL_RECOVERY_CONTROL_BIN" "$ID" relaunch --note "$note" 2>&1) \ + || verdict escalate "fm-control relaunch refused or failed: $(printf '%s' "$control_out" | tail -1)" + +# The replacement owns the instruction now. Reset the delivery ladder so the +# new incarnation gets the full grace-and-retry budget before the bounded +# escalation fires again; the episode stays pending until the record is +# handled or that escalation lands. A reset failure loses that bookkeeping, +# so it escalates rather than reporting a pending relaunch. +fm_task_inbox_ladder_reset "$STATE" "$ID" \ + || verdict escalate "relaunch published but the re-ring ladder could not be reset; the spent budget would escalate the new worker immediately" +status_note "stall auto-recovery relaunched the worker ($PATH_KIND, trigger $TRIGGER); instruction ${RECORD##*/} still pending until handled" +verdict deferred "relaunch published ($PATH_KIND); episode pending until ${RECORD##*/} is handled or the ladder re-escalates" diff --git a/bin/fm-task-inbox-lib.sh b/bin/fm-task-inbox-lib.sh index cd30903b2f0..23039b8c215 100644 --- a/bin/fm-task-inbox-lib.sh +++ b/bin/fm-task-inbox-lib.sh @@ -44,6 +44,10 @@ # doorbell activates (.omp-ready follows it) # .omp-doorbell-failed the reason a doorbell activation or drain retired # the ready marker, journaled by the extension +# .inbox/.recovery-attempts +# stall auto-recovery bound: "\t" - +# one automatic relaunch per stalled record, +# reset when the inbox empties (bin/fm-stall-recovery.sh) # # Record format (fm_task_inbox_write / fm_task_inbox_body): # schema=fm-task-inbox.v1 @@ -394,7 +398,7 @@ fm_task_inbox_due_action() { # local dir oldest base now grace max ladder rec_base count last dir=$(fm_task_inbox_dir "$1" "$2") if ! oldest=$(fm_task_inbox_oldest_unhandled "$1" "$2"); then - rm -f "$dir/.ring-state" "$dir/.escalated" 2>/dev/null || true + rm -f "$dir/.ring-state" "$dir/.escalated" "$dir/.recovery-attempts" 2>/dev/null || true printf 'quiet' return 0 fi @@ -476,3 +480,22 @@ fm_task_inbox_record_escalated() { # return 1 fi } + +# Reset the re-ring ladder for a task whose worker was just replaced by stall +# auto-recovery: the new incarnation gets the full grace-and-retry budget for +# the still-unhandled record instead of inheriting the wedged worker's spent +# budget and escalation marker. The .recovery-attempts bound is deliberately +# NOT cleared here - it is the per-record retry cap and resets only when the +# inbox empties (fm_task_inbox_due_action). Returns non-zero when the ladder +# files could not be cleared while the inbox still holds records, so the +# caller escalates rather than reporting a pending relaunch with lost retry +# bookkeeping. +fm_task_inbox_ladder_reset() { # + local dir + dir=$(fm_task_inbox_dir "$1" "$2") + [ -d "$dir" ] || return 0 + rm -f "$dir/.ring-state" "$dir/.escalated" 2>/dev/null || { + [ -d "$dir" ] || return 0 + return 1 + } +} diff --git a/bin/fm-watch.sh b/bin/fm-watch.sh index 35a348456ff..583677d73b8 100755 --- a/bin/fm-watch.sh +++ b/bin/fm-watch.sh @@ -347,6 +347,10 @@ inbox_steer_escalate_unavailable() { # fm_task_inbox_due_action "$STATE" "$task" >/dev/null || true return 0 fi + if inbox_steer_attempt_recovery "$w" "$task" "$rec" "endpoint-unavailable"; then + return 0 + fi + [ -z "$INBOX_RECOVERY_DETAIL" ] || reason="$reason [auto-recovery: $INBOX_RECOVERY_DETAIL]" fm_wake_append stale "$w" "$reason" || exit 1 if ! fm_task_inbox_record_escalated "$STATE" "$task" "$rec"; then echo "error: stale wake was queued for $task but its inbox escalation marker could not be written" >&2 @@ -355,6 +359,40 @@ inbox_steer_escalate_unavailable() { # wake "$reason" } +# Custody-checked bounded auto-recovery for a stalled worker, owned by +# bin/fm-stall-recovery.sh. Runs BEFORE the stale wake is published: a +# deferred verdict (record handled meanwhile, worker provably busy, or a +# relaunch just published with the episode still pending) suppresses the +# escalation entirely, while an escalate verdict - including any helper +# failure or missing verdict - keeps the ordinary stale wake with the +# helper's reason appended. Returns 0 when the wake is suppressed, 1 when the +# caller should escalate. +FM_STALL_RECOVERY_BIN="${FM_STALL_RECOVERY_BIN:-$SCRIPT_DIR/fm-stall-recovery.sh}" +INBOX_RECOVERY_DETAIL= +inbox_steer_attempt_recovery() { # + local task=$2 record=$3 trigger=$4 out rc=0 + INBOX_RECOVERY_DETAIL= + [ -x "$FM_STALL_RECOVERY_BIN" ] || { INBOX_RECOVERY_DETAIL="recovery helper missing"; return 1; } + out=$(FM_HOME="$FM_HOME" "$FM_STALL_RECOVERY_BIN" "$task" "$record" "$trigger" 2>/dev/null) || rc=$? + case "$out" in + verdict=recovered*|verdict=deferred*) + INBOX_RECOVERY_DETAIL=${out#*detail=} + triage_log "steer-inbox stall recovery: $task ${record##*/} $out" + return 0 + ;; + verdict=escalate*) + INBOX_RECOVERY_DETAIL=${out#*detail=} + triage_log "steer-inbox stall recovery escalates: $task ${record##*/} $out" + return 1 + ;; + *) + INBOX_RECOVERY_DETAIL="recovery helper returned no verdict (rc=$rc)" + triage_log "steer-inbox stall recovery failed: $task ${record##*/} rc=$rc out=${out:-none}" + return 1 + ;; + esac +} + inbox_steer_check() { # local window=$1 task=$2 action verb record count tail40 reason ring_rc local meta backend label harness omp_runtime omp_bin agent_state @@ -411,6 +449,10 @@ inbox_steer_check() { # fm_task_inbox_due_action "$STATE" "$task" >/dev/null || true return 0 fi + if inbox_steer_attempt_recovery "$window" "$task" "$record" "ladder-exhausted"; then + return 0 + fi + [ -z "$INBOX_RECOVERY_DETAIL" ] || reason="$reason [auto-recovery: $INBOX_RECOVERY_DETAIL]" fm_wake_append stale "$window" "$reason" || exit 1 if ! fm_task_inbox_record_escalated "$STATE" "$task" "$record"; then echo "error: stale wake was queued for $task but its inbox escalation marker could not be written" >&2 diff --git a/docs/architecture.md b/docs/architecture.md index 798708621c6..03f584f0527 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -138,6 +138,10 @@ For an OMP worker the loaded extension delivers the doorbell through `sendMessag The generated OMP extension publishes `.omp-ready` only after the doorbell activates; activation or drain failure retires `.omp-doorbell-ready` and durably journals the reason in `.omp-doorbell-failed`, while `fm-spawn.sh` bounded-waits for readiness and `fm-send.sh` names the missing marker or failure journal when refusing native delivery. When the runtime downgrades `triggerTurn` to append-only, the extension re-drives the instruction through `sendUserMessage` only after its bounded grace expires without any turn opening; the re-drive is itself only a request, so a nonthrowing return is never a receipt - the entry re-parks for one more bounded proof window and leaves a durable `.unproven` marker when no turn opens, allowing the next ring to publish a fresh pending request (`.omp/extensions/lib/fm-task-inbox-doorbell.ts`). Consumed OMP delivery receipts retire as durable `.pending.acked` tombstones, so later rings report delivery without republishing or sending another doorbell; the requests directory is generation-scoped and reset with the task lifecycle. +Before either stale wake publishes, `bin/fm-stall-recovery.sh` runs a custody-checked bounded auto-recovery: it re-proves the record is still the oldest unhandled instruction, classifies the endpoint as live-non-turning or missing, requires a clean non-run crew-state, a clean worktree with no unlanded commits, and the durable `fm-` Treehouse lease, then re-proves the whole chain immediately before invoking `fm-control.sh relaunch`. +The verdict is `recovered` when the record was already handled, `deferred` when the worker is provably busy or the relaunch just published (the episode stays pending until the record is handled or the reset ladder re-escalates), and `escalate` for every unprovable or unsafe shape, which keeps the ordinary stale wake with the helper's reason appended. +One automatic relaunch per stalled record is bounded by `state/.inbox/.recovery-attempts`; the bound resets only when the inbox empties. +An OMP relaunch also retires the prior generation's `request.*` doorbell receipts after every refusal gate and immediately before the replacement launch, so a stale `.acked` tombstone cannot suppress the new incarnation's doorbell. An OMP worker is reached only through its task-bound native receive adapter, never the composer, because an already-streaming session cannot be steered through editable terminal text; `fm-send.sh` reports one bounded outcome per steer - native receipt, a named durable native queue entry, or an explicit refusal - each binding the exact session and message. Normal local metadata publication, the Orca abort-recovery publication, inbox enqueue revalidation and record publication, and teardown share the per-task metadata lifecycle lock so endpoint birth, delivery, and retirement cannot cross. The remote-secondmate publisher stays outside `fm_meta_lock_path` on purpose and is serialized against retirement by the secondmate registry lock instead, which the remote teardown path acquires after the per-task metadata lock and holds across removing the route and retiring the metadata, so publication cannot interleave with the retirement body or survive a completed retirement, while any publication landing between teardown's metadata-lock acquisition and its retirement body is retired by that body after teardown re-reads the metadata fresh under the registry lock. @@ -150,7 +154,7 @@ Successful typed text sends then receive the existing `FM_SEND_SETTLE` pause so `bin/fm-busy-lib.sh` is the single owner of what "this worker is busy" means, and `bin/fm-busy-event.sh` is the only writer of the per-task records it reads. Every classification returns a verdict of busy, idle, unknown, or dead together with the source that produced it, so a consumer or a diagnostic can never confuse semantic state with a fallback. -Each converted adapter reports its own turn lifecycle through the strongest verified source the vendor exposes: Pi and pi-signed through the Firstmate-owned extension's `agent_start` and `agent_settled` confirmed by `ctx.isIdle()`, OpenCode through its plugin's semantic `session.status`, Claude through owned lifecycle hooks, and Hermes through its live TUI's busy-only composer/footer plus a plugin-forwarded lifecycle bridge for exact turn boundaries. +Each converted adapter reports its own turn lifecycle through the strongest verified source the vendor exposes: Pi and pi-signed through the Firstmate-owned extension's `agent_start` and `agent_settled` confirmed by `ctx.isIdle()`, OpenCode through its plugin's semantic `session.status`, OMP through its generated extension's `turn_start`/`turn_end`/`session_shutdown` handlers, Claude through owned lifecycle hooks, and Hermes through its live TUI's busy-only composer/footer plus a plugin-forwarded lifecycle bridge for exact turn boundaries. Kimi behind Pi inherits Pi's lifecycle. Codex and standalone Kimi classify unknown behind explicit probes until a semantic source is live-verified for them, while the Hermes and Grok rendered sources are isolated by exact harness identity. Hermes' bridged lifecycle record outranks its rendered footer in both directions - a trusted busy beats a rendered ready row and a trusted idle beats a lagging rendered busy row - and the rendered tail is read only when no valid record exists, so neither a steer nor a `C-c` interrupt is gated on a stale screen. diff --git a/docs/scripts.md b/docs/scripts.md index c7bd25d1d68..28ada5e845a 100644 --- a/docs/scripts.md +++ b/docs/scripts.md @@ -113,6 +113,7 @@ The shared no-mistakes gate refusal for fleet lifecycle entrypoints is summarize | `fm-classify-lib.sh` | Shared wake and status-span classification, ship evidence gate, durable keyed-decision folds, status cursors, and unread informational status-line selection | | `fm-send.sh` | Enqueue ordinary local task text durably, or type remote task text, slash commands, Codex dollar invocations, explicit targets, and keys through the recorded backend | | `fm-task-inbox-lib.sh` | Own sequenced steering records, handled-file acknowledgement, the constant doorbell, and the watcher retry ladder | +| `fm-stall-recovery.sh` | Custody-checked bounded auto-recovery for a stalled worker before the watcher publishes a stale wake | | `fm-busy-lib.sh` | Single owner of the semantic busy-state contract: verdicts, source attribution, and per-harness sources | | `fm-busy-event.sh` | The only writer of a task's semantic busy-state record; arms an incarnation and applies lifecycle events | | `fm-tmux-lib.sh` | Shared tmux pane primitives for composer capture, verified submit, and the submit-time busy check | diff --git a/tests/fm-stall-recovery.test.sh b/tests/fm-stall-recovery.test.sh new file mode 100755 index 00000000000..ded23494078 --- /dev/null +++ b/tests/fm-stall-recovery.test.sh @@ -0,0 +1,850 @@ +#!/usr/bin/env bash +# tests/fm-stall-recovery.test.sh +# Behavioral tests for bin/fm-stall-recovery.sh, the custody-checked bounded +# auto-recovery the watcher invokes before publishing a stale wake for an +# unhandled steering-inbox record, plus the generation reconciliation in +# bin/fm-spawn.sh's relaunch path that retires the prior incarnation's +# doorbell receipts. +# +# Contract under test (data/fm-stalled-worker-astra-investigate/report.md): +# - triggers on queued/unproven + persistent unhandled inbox records +# - distinguishes live-non-turning from missing-endpoint +# - reconciles stale .acked tombstones against handled state and generation +# - re-checks inbox AND task/run state immediately before the lifecycle +# action +# - proves a terminal outcome or escalates deliberately; publishing the +# relaunch is not recovery +# - preserves custody: durable fm- lease, same worktree/branch/commits, +# no shared-daemon restart, no inbox handled/ moves, no duplicate records +# +# The tests drive the real bin/fm-stall-recovery.sh and, for the +# missing-endpoint path, the real bin/fm-control.sh -> bin/fm-spawn.sh +# relaunch transaction against a stateful fake tmux. No real terminal server +# or agent is required. +set -u + +# shellcheck source=tests/lib.sh +. "$(dirname "${BASH_SOURCE[0]}")/lib.sh" + +RECOVERY="$ROOT/bin/fm-stall-recovery.sh" +CONTROL="$ROOT/bin/fm-control.sh" +SPAWN="$ROOT/bin/fm-spawn.sh" +TMP_ROOT=$(fm_test_tmproot fm-stall-recovery) + +# Kill every bun sleeper a case registered, then run the fixture cleanup. +fm_stall_cleanup() { + local p + if [ -f "$TMP_ROOT/agent-pids" ]; then + while IFS= read -r p; do + [ -n "$p" ] && kill "$p" 2>/dev/null || true + done < "$TMP_ROOT/agent-pids" + fi + fm_test_cleanup +} +trap fm_stall_cleanup EXIT + +command -v jq >/dev/null 2>&1 || { echo "skip: jq not found (required by the lease proof)"; exit 0; } + +# --- fake backend CLIs ------------------------------------------------------- + +make_fakebin() { # -> echoes fakebin dir + local dir=$1 fb real_bun + fb=$(fm_fakebin "$dir/fake") + + # --- stateful tmux --------------------------------------------------------- + # $FM_FAKE_TMUX_STATE holds .windows files, one + # "idnamecwdcomm" line per window. send-keys semantics model + # the real foreground transition: an /exit line returns the pane to its + # shell (comm=bash), any other delivered line means the agent is running + # (comm=bun, the OMP runtime), and kill-window actually removes the window. + cat > "$fb/tmux" <<'SH' +#!/usr/bin/env bash +set -u +STATE="${FM_FAKE_TMUX_STATE:?}" +LOG="${FM_FAKE_TMUX_LOG:-/dev/null}" +{ printf 'tmux'; for a in "$@"; do printf '\x1f%s' "$a"; done; printf '\n'; } >> "$LOG" + +winfile() { printf '%s/%s.windows' "$STATE" "$1"; } +resolve() { # -> prints "sesidnamecwdcomm" + local t=$1 ses name f line + case "$t" in + @*) + for f in "$STATE"/*.windows; do + [ -e "$f" ] || continue + line=$(awk -F '\t' -v id="$t" '$1 == id {print; exit}' "$f") + if [ -n "$line" ]; then + ses=${f##*/}; ses=${ses%.windows} + printf '%s\t%s\n' "$ses" "$line" + return 0 + fi + done + return 1 + ;; + *:*) + ses=${t%%:*}; name=${t#*:} + f=$(winfile "$ses") + [ -f "$f" ] || return 1 + line=$(awk -F '\t' -v n="$name" '$2 == n {print; exit}' "$f") + [ -n "$line" ] || return 1 + printf '%s\t%s\n' "$ses" "$line" + ;; + *) return 1 ;; + esac +} +mark_comm() { # : rewrite a window's comm field in place + local f tmp + f=$(winfile "$1") + [ -f "$f" ] || return 0 + tmp="$f.tmp.$$" + awk -F '\t' -v id="$2" -v c="$3" 'BEGIN{OFS="\t"} $1 == id {$4 = c} {print}' "$f" > "$tmp" && mv "$tmp" "$f" +} +drop_window() { # : remove a window line entirely + local f tmp + f=$(winfile "$1") + [ -f "$f" ] || return 0 + tmp="$f.tmp.$$" + awk -F '\t' -v id="$2" '$1 != id {print}' "$f" > "$tmp" && mv "$tmp" "$f" +} +omp_doorbell_emulate() { # : mirror the generated extension's handshake + [ -f "$1.omp-ext.ts" ] || return 0 + : > "$1.omp-doorbell-ready" +} +touch_omp_acks() { + grep -Fq 'FM_OMP_HARNESS=omp' "$FM_FAKE_LAUNCH_LOG" 2>/dev/null || return 0 + for extension in "${FM_FAKE_OMP_ACK_DIR:-/nonexistent}"/*.omp-ext.ts; do + [ -e "$extension" ] || continue + omp_doorbell_emulate "${extension%.omp-ext.ts}" + done + if [ -n "${FM_FAKE_OMP_ACK:-}" ]; then + while IFS= read -r ack; do + [ -z "$ack" ] && continue + : > "$ack" + case "$ack" in *.omp-started) omp_doorbell_emulate "${ack%.omp-started}" ;; esac + done <&2 + exit 1 + fi + awk -F '\t' '{print $2}' "$f" + exit 0 ;; + has-session) + ses="" + prev="" + for a in "$@"; do [ "$prev" = "-t" ] && ses=$a; prev=$a; done + [ -f "$(winfile "$ses")" ] + exit $? ;; + new-session) + ses="" + prev="" + for a in "$@"; do [ "$prev" = "-s" ] && ses=$a; prev=$a; done + [ -n "$ses" ] || exit 1 + : > "$(winfile "$ses")" + exit 0 ;; + new-window) + ses="" name="" cwd="" + prev="" + for a in "$@"; do + case "$prev" in + -t) ses=${a%%:*} ;; + -n) name=$a ;; + -c) cwd=$a ;; + esac + prev=$a + done + f=$(winfile "$ses") + if [ ! -f "$f" ]; then + printf "can't find session: %s\n" "$ses" >&2 + exit 1 + fi + n=$(( $(awk 'END{print NR}' "$f" 2>/dev/null || echo 0) + 1 )) + wid="@$n" + printf '%s\t%s\t%s\t%s\n' "$wid" "$name" "$cwd" "bash" >> "$f" + printf '%s\n' "$wid" + exit 0 ;; + display-message) + target="" fmt="" + prev="" + for a in "$@"; do + [ "$prev" = "-t" ] && target=$a + fmt=$a + prev=$a + done + if [ -z "$target" ]; then + printf 'firstmate\n' + exit 0 + fi + line=$(resolve "$target") || exit 1 + wid=$(printf '%s' "$line" | awk -F '\t' '{print $2}') + wname=$(printf '%s' "$line" | awk -F '\t' '{print $3}') + wcwd=$(printf '%s' "$line" | awk -F '\t' '{print $4}') + wcomm=$(printf '%s' "$line" | awk -F '\t' '{print $5}') + case "$fmt" in + '#{pane_current_path}') printf '%s\n' "$wcwd" ;; + '#{pane_current_command}') printf '%s\n' "$wcomm" ;; + '#{pane_pid}') printf '4242\n' ;; + '#{pane_id}') printf '%%%s\n' "${wid#@}" ;; + '#{window_id}') printf '%s\n' "$wid" ;; + *) printf '%s\n' "$wname" ;; + esac + exit 0 ;; + send-keys) + target="" + prev="" + for a in "$@"; do [ "$prev" = "-t" ] && target=$a; prev=$a; done + line=$(resolve "$target") || exit 1 + ses=${line%% *} + wid=$(printf '%s' "$line" | awk -F '\t' '{print $2}') + exit_sent=0 + if [ -n "${FM_FAKE_LAUNCH_LOG:-}" ]; then + for a in "$@"; do + case "$a" in + -*|"$target") ;; + /exit) exit_sent=1; printf '%s\n' "$a" >> "$FM_FAKE_LAUNCH_LOG" ;; + *) printf '%s\n' "$a" >> "$FM_FAKE_LAUNCH_LOG" ;; + esac + done + touch_omp_acks + else + for a in "$@"; do [ "$a" = "/exit" ] && exit_sent=1; done + fi + if [ "$exit_sent" -eq 1 ]; then + mark_comm "$ses" "$wid" "bash" + else + mark_comm "$ses" "$wid" "bun" + fi + exit 0 ;; + kill-window) + target="" + prev="" + for a in "$@"; do [ "$prev" = "-t" ] && target=$a; prev=$a; done + target=${target#=} + line=$(resolve "$target") || exit 0 + ses=${line%% *} + wid=$(printf '%s' "$line" | awk -F '\t' '{print $2}') + drop_window "$ses" "$wid" + exit 0 ;; + kill-session|set-window-option|run-shell) exit 0 ;; + *) exit 0 ;; +esac +SH + chmod +x "$fb/tmux" + + # Order-sensitive ps bound to a REAL bun process ($FM_FAKE_AGENT_PID_FILE): + # the OMP agent-state probe asks `-o args= -p ` (args first) and must + # see the bun launch line, while the idle-shell proof asks `-p -o + # comm=`/`-o args=` (pid first) and must see a bare shell. The pid must be + # real because fm_omp_process_matches reads /proc//exe. + cat > "$fb/ps" <<'SH' +#!/usr/bin/env bash +set -u +agent_pid=$(cat "${FM_FAKE_AGENT_PID_FILE:?}" 2>/dev/null || printf '4242') +case "$*" in + *"-o tpgid="*) printf '%s\n' "$agent_pid" ;; + *"-o args= -p"*) printf 'bun %s\n' "${FM_FAKE_OMP_BIN:?}" ;; + *"-o comm="*) printf 'bash\n' ;; + *"-o args="*) printf 'bash\n' ;; + *"-o stat="*) printf 'Ss\n' ;; + *"pid=,pgid=,ppid=") printf '%s %s 4242\n' "$agent_pid" "$agent_pid" ;; + *"pid=,ppid=") printf '%s 4242\n' "$agent_pid" ;; + *) exit 1 ;; +esac +SH + chmod +x "$fb/ps" + + cat > "$fb/treehouse" <<'SH' +#!/usr/bin/env bash +set -u +exit 0 +SH + chmod +x "$fb/treehouse" + + # bun resolves to the REAL bun executable: fm-spawn derives the launch + # identity from `command -v bun` and rewrites omp_bun in the metadata, so + # the recorded identity and the sleeper's /proc//exe must both be the + # real binary. + real_bun=$(command -v bun) || { echo "skip: bun not found"; exit 0; } + ln -sf "$real_bun" "$fb/bun" + + # omp keeps its `#!/usr/bin/env bun` shebang (fm_omp_process_launch_identity + # requires a bun launch identity) and is therefore real JavaScript executed + # by real bun. + cat > "$fb/omp" <<'SH' +#!/usr/bin/env bun +const arg = process.argv[2] || ""; +if (arg === "--help") { + console.log("--model=\n--thinking=\n--auto-approve\n--max-time=\n--session-dir=\n-e, --extension=\n-r, --resume=\n--prewalk native-switch\n--prewalk-into=\n--config=\n--no-prewalk"); +} else if (arg === "--version") { + console.log("omp/18.1.14"); +} else if (arg === "config") { + console.log(`{"key":"prewalk.enabled","value":${process.env.FM_FAKE_OMP_PREWALK_ENABLED || "false"},"type":"boolean"}`); +} else if (arg === "models") { + console.log('{"models":[{"provider":"openai-codex","id":"gpt-5.6-luna","selector":"openai-codex/gpt-5.6-luna","thinking":["low","medium","high","xhigh","max"]}]}'); +} +SH + chmod +x "$fb/omp" + + # fm-crew-state.sh stub: prints a canned verdict, or moves the named inbox + # record to handled/ once FM_FAKE_CREW_HANDLE_AFTER calls have happened + # (the late-ack race test). + cat > "$fb/fm-crew-state.sh" <<'SH' +#!/usr/bin/env bash +set -u +count_file="${FM_FAKE_CREW_COUNT:-/dev/null}" +n=0 +[ -f "$count_file" ] && n=$(cat "$count_file" 2>/dev/null || printf '0') +n=$((n + 1)) +[ "$count_file" = /dev/null ] || printf '%s' "$n" > "$count_file" +after=${FM_FAKE_CREW_HANDLE_AFTER:-0} +if [ "$after" -gt 0 ] && [ "$n" -ge "$after" ]; then + rec="${FM_FAKE_CREW_HANDLE_RECORD:-}" + if [ -n "$rec" ] && [ -f "$rec" ]; then + mkdir -p "${rec%/*}/handled" + mv "$rec" "${rec%/*}/handled/" + fi +fi +printf 'state: %s · source: status-log · fake crew-state\n' "${FM_FAKE_CREW_STATE:-done}" +SH + chmod +x "$fb/fm-crew-state.sh" + + # fm-control.sh stub for tests that only need to prove the lifecycle verb + # was (or was not) invoked. + cat > "$fb/fm-control.sh" <<'SH' +#!/usr/bin/env bash +set -u +{ printf 'fm-control'; for a in "$@"; do printf '\x1f%s' "$a"; done; printf '\n'; } >> "${FM_FAKE_CONTROL_LOG:?}" +exit "${FM_FAKE_CONTROL_RC:-0}" +SH + chmod +x "$fb/fm-control.sh" + + printf '%s\n' "$fb" +} + +# --- fixture helpers --------------------------------------------------------- + +# make_case [pool|flat] -> echoes +# case_dir|home|proj|wt|fakebin|launchlog|slot_dir +make_case() { + local name=$1 id=$2 shape=${3:-pool} + local case_dir home proj wt fakebin launchlog slot_dir + case_dir="$TMP_ROOT/$name" + home="$case_dir/home" + proj="$case_dir/project" + wt="$case_dir/pool/17/proj" + slot_dir="$case_dir/pool/17" + launchlog="$case_dir/launch.log" + fakebin=$(make_fakebin "$case_dir/fake") + mkdir -p "$home/data" "$home/projects" "$home/state" "$home/config" \ + "$case_dir/fake/tmux-state" + printf 'omp\n' > "$home/config/crew-harness" + mkdir -p "$home/data/$id" + printf 'Delivery contract: mode=no-mistakes\nrelaunch brief for %s\n' "$id" > "$home/data/$id/brief.md" + touch "$home/state/.last-watcher-beat" + # A real bun process stands in for the pane's agent: the OMP identity probe + # reads /proc//exe, so the pid must be a live bun. Registered for + # cleanup by the EXIT trap below. + bun -e 'setInterval(() => {}, 1000000)' >/dev/null 2>&1 & + printf '%s\n' "$!" >> "$TMP_ROOT/agent-pids" + printf '%s\n' "$!" > "$case_dir/fake/agent.pid" + case "$shape" in + pool) + mkdir -p "$case_dir/pool" + fm_git_worktree "$proj" "$wt" "wt-$name" + git -C "$proj" fetch --quiet origin + ;; + flat) + wt="$case_dir/wt" + fm_git_worktree "$proj" "$wt" "wt-$name" + git -C "$proj" fetch --quiet origin + ;; + esac + printf '%s\n' "$case_dir|$home|$proj|$wt|$fakebin|$launchlog|$slot_dir" +} + +read_case() { + IFS='|' read -r CASE_DIR HOME_DIR PROJ_DIR WT_DIR FAKEBIN_DIR LAUNCH_LOG SLOT_DIR < +write_pool_state() { + local case_dir=$1 wt=$2 holder=${3:-} + local wt_real + wt_real=$(cd "$wt" 2>/dev/null && pwd -P) + if [ -n "$holder" ]; then + jq -n --arg p "$wt_real" --arg h "$holder" \ + '{worktrees:[{name:"17", path:$p, created_at:"2026-09-17T12:53:36+08:00", leased:true, lease_id:"c300c30567691b53fee1558601cfc49f", lease_holder:$h, leased_at:"2026-09-17T12:53:36+08:00"}]}' \ + > "$case_dir/pool/treehouse-state.json" + else + jq -n --arg p "$wt_real" \ + '{worktrees:[{name:"17", path:$p, created_at:"2026-09-17T12:53:36+08:00", leased:false}]}' \ + > "$case_dir/pool/treehouse-state.json" + fi +} + +write_slot_marker() { + printf 'task=%s\nhome=%s\n' "$2" "$3" > "$1/.fm-slot-owner" +} + +write_meta() { + local file=$1 id=$2 wt=$3 proj=$4 + local omp_bin bun + omp_bin=$(cd "$FAKEBIN_DIR" && pwd -P)/omp + bun=$(readlink -f "$FAKEBIN_DIR/bun") + { + printf 'endpoint_task_id=%s\n' "$id" + printf 'worktree=%s\n' "$wt" + printf 'project=%s\n' "$proj" + printf 'harness=omp\n' + printf 'kind=ship\n' + printf 'mode=no-mistakes\n' + printf 'yolo=off\n' + printf 'tasktmp=\n' + printf 'model=openai-codex/gpt-5.6-luna\n' + printf 'effort=high\n' + printf 'spawn_gen=gen1\n' + printf 'omp_bin=%s\n' "$omp_bin" + printf 'omp_bun=%s\n' "$bun" + printf 'window=ses-%s:fm-%s\n' "$id" "$id" + printf 'backend=tmux\n' + } > "$file" +} + +# create_prior_artifacts: the prior incarnation's durable runtime files. +create_prior_artifacts() { + local state=$1 id=$2 + : > "$state/$id.status" + : > "$state/$id.omp-ext.ts" + : > "$state/$id.omp-ready" + : > "$state/$id.omp-started" + : > "$state/$id.omp-doorbell-ready" + mkdir -p "$state/$id.omp-doorbell-ready.requests" +} + +# write_inbox : one unhandled instruction record. +write_inbox() { + local state=$1 id=$2 seq=$3 + mkdir -p "$state/$id.inbox/handled" + printf 'steer: test instruction %s\n' "$seq" > "$state/$id.inbox/$seq.msg" +} + +# write_busy : a valid armed busy-state record. +write_busy() { + local state=$1 id=$2 st=$3 + printf 'gentest\n' > "$state/$id.busy-gen" + printf 'v1 gen=gentest seq=2 state=%s source=fm-spawn event=turn-end ts=1\n' "$st" \ + > "$state/$id.busy-state" +} + +# live_window : the recorded window exists with . +live_window() { + printf '@1\tfm-%s\t%s\t%s\n' "$2" "$WT_DIR" "$3" > "$1/fake/tmux-state/ses-$2.windows" +} + +# missing_window : the session exists but the window is gone. +missing_window() { + : > "$1/fake/tmux-state/ses-$2.windows" +} + +case_id() { + printf 'stall-%s-%s' "$1" "$$" +} + +# run_recovery [env assignments...] +run_recovery() { + local case_dir=$1 home=$2 id=$3 record=$4 trigger=$5 + shift 5 + RECOVERY_OUT=$(env -u HERDR_PANE_ID -u HERDR_SESSION -u ZELLIJ_SESSION_NAME \ + FM_HOME="$home" \ + FM_STATE_OVERRIDE="$home/state" FM_DATA_OVERRIDE="$home/data" \ + FM_CONFIG_OVERRIDE="$home/config" \ + FM_FAKE_TMUX_STATE="$case_dir/fake/tmux-state" \ + FM_FAKE_TMUX_LOG="$case_dir/fake/tmux.log" \ + FM_FAKE_AGENT_PID_FILE="$case_dir/fake/agent.pid" \ + FM_FAKE_OMP_BIN="$FAKEBIN_DIR/omp" \ + FM_FAKE_LAUNCH_LOG="$LAUNCH_LOG" \ + FM_FAKE_OMP_ACK="$home/state/$id.omp-started" \ + FM_FAKE_OMP_ACK_DIR="$home/state" \ + FM_FAKE_OMP_NO_PREWALK=1 \ + FM_FAKE_CONTROL_LOG="$case_dir/control.log" \ + FM_CREW_STATE_BIN="$FAKEBIN_DIR/fm-crew-state.sh" \ + FM_STALL_RECOVERY_CONTROL_BIN="${FM_STALL_RECOVERY_CONTROL_BIN:-$FAKEBIN_DIR/fm-control.sh}" \ + FM_SPAWN_NO_GUARD=1 TMUX='fake,1,0' \ + FM_OMP_LAUNCH_ACK_POLLS=20 FM_OMP_DOORBELL_ACK_POLLS=20 \ + FM_CONTROL_POLL=0.05 FM_CONTROL_EXIT_WAIT=5 FM_CONTROL_LAUNCH_WAIT=20 \ + FM_BACKEND_TMUX_IDLE_SHELL_PROOF_POLLS=10 \ + PATH="$FAKEBIN_DIR:$PATH" \ + "$@" "$RECOVERY" "$id" "$record" "$trigger" 2>&1) + RECOVERY_STATUS=$? +} + +# --- tests ------------------------------------------------------------------- + +# Missing endpoint + unhandled record + clean custody: the real fm-control +# relaunch transaction runs, the replacement window is created in the recorded +# worktree, the ladder resets, and the prior generation's doorbell receipts +# are retired so the new incarnation's doorbell is not suppressed. +test_missing_endpoint_recovers_via_control() { + local rec id record requests + id=$(case_id missing-e2e) + rec=$(make_case missing-e2e "$id" pool) + read_case "$rec" + write_pool_state "$CASE_DIR" "$WT_DIR" "fm-$id" + write_slot_marker "$SLOT_DIR" "$id" "$HOME_DIR" + write_meta "$HOME_DIR/state/$id.meta" "$id" "$WT_DIR" "$PROJ_DIR" + create_prior_artifacts "$HOME_DIR/state" "$id" + write_inbox "$HOME_DIR/state" "$id" 001 + record="$HOME_DIR/state/$id.inbox/001.msg" + requests="$HOME_DIR/state/$id.omp-doorbell-ready.requests" + : > "$requests/request.1.pending.acked" + : > "$requests/request.2.pending.unproven" + printf '001.msg\t3\t1\n' > "$HOME_DIR/state/$id.inbox/.ring-state" + printf '001.msg\n' > "$HOME_DIR/state/$id.inbox/.escalated" + missing_window "$CASE_DIR" "$id" + + FM_STALL_RECOVERY_CONTROL_BIN="$CONTROL" \ + run_recovery "$CASE_DIR" "$HOME_DIR" "$id" "$record" endpoint-unavailable + expect_code 0 "$RECOVERY_STATUS" "missing-endpoint recovery should exit 0; got: $RECOVERY_OUT" + assert_contains "$RECOVERY_OUT" "verdict=deferred" "missing-endpoint recovery did not defer pending the episode" + assert_contains "$RECOVERY_OUT" "missing-endpoint" "verdict did not name the missing-endpoint path" + assert_grep "new-window" "$CASE_DIR/fake/tmux.log" "relaunch did not create a replacement tmux window" + assert_grep "$WT_DIR" "$CASE_DIR/fake/tmux.log" "replacement window was not created in the recorded worktree" + assert_grep "phase=complete" "$HOME_DIR/state/$id.control-relaunch" "the relaunch transaction did not complete" + assert_absent "$HOME_DIR/state/$id.inbox/.escalated" "the spent escalation marker survived the ladder reset" + assert_absent "$HOME_DIR/state/$id.inbox/.ring-state" "the spent ring ladder survived the reset" + assert_absent "$requests/request.1.pending.acked" "stale .acked tombstone survived the relaunch" + assert_absent "$requests/request.2.pending.unproven" "stale .unproven receipt survived the relaunch" + assert_grep "001.msg" "$HOME_DIR/state/$id.inbox/.recovery-attempts" "the per-record attempt bound was not recorded" + assert_present "$record" "the unhandled instruction record was moved or deleted" + assert_grep "stall auto-recovery" "$HOME_DIR/state/$id.status" "no audit note was appended to the task status" + pass "missing endpoint: real relaunch publishes, ladder resets, stale receipts retire, record stays unhandled" +} + +# Live endpoint whose agent is idle (turn ended, record still unhandled): the +# live-non-turning path invokes the lifecycle verb. +test_live_non_turning_recovers() { + local rec id record + id=$(case_id live-idle) + rec=$(make_case live-idle "$id" pool) + read_case "$rec" + write_pool_state "$CASE_DIR" "$WT_DIR" "fm-$id" + write_slot_marker "$SLOT_DIR" "$id" "$HOME_DIR" + write_meta "$HOME_DIR/state/$id.meta" "$id" "$WT_DIR" "$PROJ_DIR" + create_prior_artifacts "$HOME_DIR/state" "$id" + write_inbox "$HOME_DIR/state" "$id" 001 + record="$HOME_DIR/state/$id.inbox/001.msg" + write_busy "$HOME_DIR/state" "$id" idle + live_window "$CASE_DIR" "$id" bun + + run_recovery "$CASE_DIR" "$HOME_DIR" "$id" "$record" ladder-exhausted + expect_code 0 "$RECOVERY_STATUS" "live-non-turning recovery should exit 0; got: $RECOVERY_OUT" + assert_contains "$RECOVERY_OUT" "verdict=deferred" "live-non-turning recovery did not defer pending the episode" + assert_contains "$RECOVERY_OUT" "live-non-turning" "verdict did not name the live path" + assert_grep "relaunch" "$CASE_DIR/control.log" "the lifecycle verb was not invoked" + pass "live non-turning worker: idle verdict plus clean custody publishes the relaunch" +} + +# A live endpoint with a provably busy agent defers: the worker may be +# mid-turn on the instruction. +test_live_busy_defers() { + local rec id record + id=$(case_id live-busy) + rec=$(make_case live-busy "$id" pool) + read_case "$rec" + write_pool_state "$CASE_DIR" "$WT_DIR" "fm-$id" + write_slot_marker "$SLOT_DIR" "$id" "$HOME_DIR" + write_meta "$HOME_DIR/state/$id.meta" "$id" "$WT_DIR" "$PROJ_DIR" + create_prior_artifacts "$HOME_DIR/state" "$id" + write_inbox "$HOME_DIR/state" "$id" 001 + record="$HOME_DIR/state/$id.inbox/001.msg" + write_busy "$HOME_DIR/state" "$id" busy + live_window "$CASE_DIR" "$id" bun + + run_recovery "$CASE_DIR" "$HOME_DIR" "$id" "$record" ladder-exhausted + assert_contains "$RECOVERY_OUT" "verdict=deferred" "a busy worker did not defer" + assert_absent "$CASE_DIR/control.log" "the lifecycle verb ran against a busy worker" + pass "live busy worker: recovery defers without a lifecycle action" +} + +# A live endpoint with no semantic busy proof escalates: unknown is never a +# custody proof. +test_live_unknown_busy_escalates() { + local rec id record + id=$(case_id live-unknown) + rec=$(make_case live-unknown "$id" pool) + read_case "$rec" + write_pool_state "$CASE_DIR" "$WT_DIR" "fm-$id" + write_slot_marker "$SLOT_DIR" "$id" "$HOME_DIR" + write_meta "$HOME_DIR/state/$id.meta" "$id" "$WT_DIR" "$PROJ_DIR" + create_prior_artifacts "$HOME_DIR/state" "$id" + write_inbox "$HOME_DIR/state" "$id" 001 + record="$HOME_DIR/state/$id.inbox/001.msg" + live_window "$CASE_DIR" "$id" bun + + run_recovery "$CASE_DIR" "$HOME_DIR" "$id" "$record" ladder-exhausted + assert_contains "$RECOVERY_OUT" "verdict=escalate" "an unprovable busy verdict did not escalate" + assert_absent "$CASE_DIR/control.log" "the lifecycle verb ran without a busy proof" + pass "live worker with no busy proof: recovery escalates" +} + +# A record already handled ends the episode quietly - no lifecycle action. +test_handled_record_recovers_quietly() { + local rec id record + id=$(case_id handled) + rec=$(make_case handled "$id" pool) + read_case "$rec" + write_pool_state "$CASE_DIR" "$WT_DIR" "fm-$id" + write_slot_marker "$SLOT_DIR" "$id" "$HOME_DIR" + write_meta "$HOME_DIR/state/$id.meta" "$id" "$WT_DIR" "$PROJ_DIR" + create_prior_artifacts "$HOME_DIR/state" "$id" + mkdir -p "$HOME_DIR/state/$id.inbox/handled" + printf 'steer: done\n' > "$HOME_DIR/state/$id.inbox/handled/001.msg" + record="$HOME_DIR/state/$id.inbox/001.msg" + missing_window "$CASE_DIR" "$id" + + run_recovery "$CASE_DIR" "$HOME_DIR" "$id" "$record" endpoint-unavailable + assert_contains "$RECOVERY_OUT" "verdict=recovered" "a handled record did not close the episode" + assert_absent "$CASE_DIR/control.log" "the lifecycle verb ran for a handled record" + pass "handled record: recovery reports the terminal outcome without a relaunch" +} + +# A handled move DURING the custody probe still cancels the action: the +# crew-state stub moves the record to handled/ on its second call (the final +# pre-lifecycle proof), so the post-proof record check sees an empty inbox. +test_late_handled_cancels_relaunch() { + local rec id record + id=$(case_id late-ack) + rec=$(make_case late-ack "$id" pool) + read_case "$rec" + write_pool_state "$CASE_DIR" "$WT_DIR" "fm-$id" + write_slot_marker "$SLOT_DIR" "$id" "$HOME_DIR" + write_meta "$HOME_DIR/state/$id.meta" "$id" "$WT_DIR" "$PROJ_DIR" + create_prior_artifacts "$HOME_DIR/state" "$id" + write_inbox "$HOME_DIR/state" "$id" 001 + record="$HOME_DIR/state/$id.inbox/001.msg" + missing_window "$CASE_DIR" "$id" + + FM_FAKE_CREW_HANDLE_AFTER=2 FM_FAKE_CREW_HANDLE_RECORD="$record" \ + FM_FAKE_CREW_COUNT="$CASE_DIR/crew-count" \ + run_recovery "$CASE_DIR" "$HOME_DIR" "$id" "$record" endpoint-unavailable + assert_contains "$RECOVERY_OUT" "verdict=recovered" "a late handled move did not cancel the relaunch" + assert_absent "$CASE_DIR/control.log" "the lifecycle verb ran after the record was handled" + pass "late handled move during the custody probe cancels the relaunch" +} + +# Uncommitted work in the worktree is unlanded evidence: escalate, never +# relaunch into it. +test_dirty_worktree_escalates() { + local rec id record + id=$(case_id dirty) + rec=$(make_case dirty "$id" pool) + read_case "$rec" + write_pool_state "$CASE_DIR" "$WT_DIR" "fm-$id" + write_slot_marker "$SLOT_DIR" "$id" "$HOME_DIR" + write_meta "$HOME_DIR/state/$id.meta" "$id" "$WT_DIR" "$PROJ_DIR" + create_prior_artifacts "$HOME_DIR/state" "$id" + write_inbox "$HOME_DIR/state" "$id" 001 + record="$HOME_DIR/state/$id.inbox/001.msg" + missing_window "$CASE_DIR" "$id" + printf 'uncommitted\n' > "$WT_DIR/scratch.txt" + + run_recovery "$CASE_DIR" "$HOME_DIR" "$id" "$record" endpoint-unavailable + assert_contains "$RECOVERY_OUT" "verdict=escalate" "a dirty worktree did not escalate" + assert_absent "$CASE_DIR/control.log" "the lifecycle verb ran against a dirty worktree" + pass "dirty worktree: recovery escalates rather than discarding uncommitted work" +} + +# Commits not on any remote-tracking ref are unlanded work: escalate. +test_unlanded_commits_escalate() { + local rec id record + id=$(case_id unlanded) + rec=$(make_case unlanded "$id" pool) + read_case "$rec" + write_pool_state "$CASE_DIR" "$WT_DIR" "fm-$id" + write_slot_marker "$SLOT_DIR" "$id" "$HOME_DIR" + write_meta "$HOME_DIR/state/$id.meta" "$id" "$WT_DIR" "$PROJ_DIR" + create_prior_artifacts "$HOME_DIR/state" "$id" + write_inbox "$HOME_DIR/state" "$id" 001 + record="$HOME_DIR/state/$id.inbox/001.msg" + missing_window "$CASE_DIR" "$id" + printf 'wip\n' > "$WT_DIR/wip.txt" + git -C "$WT_DIR" add wip.txt + git -C "$WT_DIR" -c user.email=t@t -c user.name=t commit -qm wip + + run_recovery "$CASE_DIR" "$HOME_DIR" "$id" "$record" endpoint-unavailable + assert_contains "$RECOVERY_OUT" "verdict=escalate" "unlanded commits did not escalate" + assert_absent "$CASE_DIR/control.log" "the lifecycle verb ran against unlanded commits" + pass "unlanded commits: recovery escalates rather than abandoning pushed-state proof" +} + +# The per-record bound: a second automatic attempt for the same record +# escalates instead of looping relaunches. +test_attempt_cap_escalates() { + local rec id record + id=$(case_id capped) + rec=$(make_case capped "$id" pool) + read_case "$rec" + write_pool_state "$CASE_DIR" "$WT_DIR" "fm-$id" + write_slot_marker "$SLOT_DIR" "$id" "$HOME_DIR" + write_meta "$HOME_DIR/state/$id.meta" "$id" "$WT_DIR" "$PROJ_DIR" + create_prior_artifacts "$HOME_DIR/state" "$id" + write_inbox "$HOME_DIR/state" "$id" 001 + record="$HOME_DIR/state/$id.inbox/001.msg" + printf '001.msg\t1\n' > "$HOME_DIR/state/$id.inbox/.recovery-attempts" + missing_window "$CASE_DIR" "$id" + + run_recovery "$CASE_DIR" "$HOME_DIR" "$id" "$record" endpoint-unavailable + assert_contains "$RECOVERY_OUT" "verdict=escalate" "a spent attempt bound did not escalate" + assert_absent "$CASE_DIR/control.log" "the lifecycle verb ran past the attempt bound" + pass "attempt bound: a second automatic relaunch for the same record escalates" +} + +# A slot leased to a different holder is not this task's worktree. +test_foreign_lease_escalates() { + local rec id record + id=$(case_id foreign-lease) + rec=$(make_case foreign-lease "$id" pool) + read_case "$rec" + write_pool_state "$CASE_DIR" "$WT_DIR" "fm-other-task" + write_slot_marker "$SLOT_DIR" "$id" "$HOME_DIR" + write_meta "$HOME_DIR/state/$id.meta" "$id" "$WT_DIR" "$PROJ_DIR" + create_prior_artifacts "$HOME_DIR/state" "$id" + write_inbox "$HOME_DIR/state" "$id" 001 + record="$HOME_DIR/state/$id.inbox/001.msg" + missing_window "$CASE_DIR" "$id" + + run_recovery "$CASE_DIR" "$HOME_DIR" "$id" "$record" endpoint-unavailable + assert_contains "$RECOVERY_OUT" "verdict=escalate" "a foreign lease did not escalate" + assert_absent "$CASE_DIR/control.log" "the lifecycle verb ran against a foreign-leased worktree" + pass "foreign lease: recovery escalates rather than relaunching into another task's slot" +} + +# A worktree outside the pool cannot prove ownership: escalate. +test_non_pool_worktree_escalates() { + local rec id record + id=$(case_id flat) + rec=$(make_case flat "$id" flat) + read_case "$rec" + write_meta "$HOME_DIR/state/$id.meta" "$id" "$WT_DIR" "$PROJ_DIR" + create_prior_artifacts "$HOME_DIR/state" "$id" + write_inbox "$HOME_DIR/state" "$id" 001 + record="$HOME_DIR/state/$id.inbox/001.msg" + missing_window "$CASE_DIR" "$id" + + run_recovery "$CASE_DIR" "$HOME_DIR" "$id" "$record" endpoint-unavailable + assert_contains "$RECOVERY_OUT" "verdict=escalate" "a non-pool worktree did not escalate" + assert_absent "$CASE_DIR/control.log" "the lifecycle verb ran against an unprovable worktree" + pass "non-pool worktree: recovery escalates without ownership proof" +} + +# A working crew-state needs firstmate, not auto-relaunch. +test_working_crew_state_escalates() { + local rec id record + id=$(case_id working) + rec=$(make_case working "$id" pool) + read_case "$rec" + write_pool_state "$CASE_DIR" "$WT_DIR" "fm-$id" + write_slot_marker "$SLOT_DIR" "$id" "$HOME_DIR" + write_meta "$HOME_DIR/state/$id.meta" "$id" "$WT_DIR" "$PROJ_DIR" + create_prior_artifacts "$HOME_DIR/state" "$id" + write_inbox "$HOME_DIR/state" "$id" 001 + record="$HOME_DIR/state/$id.inbox/001.msg" + missing_window "$CASE_DIR" "$id" + + FM_FAKE_CREW_STATE=working \ + run_recovery "$CASE_DIR" "$HOME_DIR" "$id" "$record" endpoint-unavailable + assert_contains "$RECOVERY_OUT" "verdict=escalate" "a working crew-state did not escalate" + assert_absent "$CASE_DIR/control.log" "the lifecycle verb ran against a working task" + pass "working crew-state: recovery escalates to firstmate" +} + +# A secondmate is never an ordinary direct report: escalate. +test_secondmate_kind_escalates() { + local rec id record + id=$(case_id secondmate) + rec=$(make_case secondmate "$id" pool) + read_case "$rec" + write_pool_state "$CASE_DIR" "$WT_DIR" "fm-$id" + write_slot_marker "$SLOT_DIR" "$id" "$HOME_DIR" + write_meta "$HOME_DIR/state/$id.meta" "$id" "$WT_DIR" "$PROJ_DIR" + printf 'kind=secondmate\n' >> "$HOME_DIR/state/$id.meta" + create_prior_artifacts "$HOME_DIR/state" "$id" + write_inbox "$HOME_DIR/state" "$id" 001 + record="$HOME_DIR/state/$id.inbox/001.msg" + missing_window "$CASE_DIR" "$id" + + run_recovery "$CASE_DIR" "$HOME_DIR" "$id" "$record" endpoint-unavailable + assert_contains "$RECOVERY_OUT" "verdict=escalate" "a secondmate did not escalate" + assert_absent "$CASE_DIR/control.log" "the lifecycle verb ran against a secondmate" + pass "secondmate kind: recovery escalates; secondmates recover through their own path" +} + +# A refused relaunch preserves the prior generation's receipts: the purge runs +# only after every refusal gate, so a live-agent refusal deletes nothing. +test_refused_relaunch_preserves_receipts() { + local rec id requests + id=$(case_id refuse-keep) + rec=$(make_case refuse-keep "$id" pool) + read_case "$rec" + write_pool_state "$CASE_DIR" "$WT_DIR" "fm-$id" + write_slot_marker "$SLOT_DIR" "$id" "$HOME_DIR" + write_meta "$HOME_DIR/state/$id.meta" "$id" "$WT_DIR" "$PROJ_DIR" + create_prior_artifacts "$HOME_DIR/state" "$id" + requests="$HOME_DIR/state/$id.omp-doorbell-ready.requests" + : > "$requests/request.1.pending.acked" + live_window "$CASE_DIR" "$id" bun + + SPAWN_OUT=$(env -u HERDR_PANE_ID -u HERDR_SESSION -u ZELLIJ_SESSION_NAME \ + FM_HOME="$HOME_DIR" \ + FM_STATE_OVERRIDE="$HOME_DIR/state" FM_DATA_OVERRIDE="$HOME_DIR/data" \ + FM_CONFIG_OVERRIDE="$HOME_DIR/config" \ + FM_FAKE_TMUX_STATE="$CASE_DIR/fake/tmux-state" \ + FM_FAKE_TMUX_LOG="$CASE_DIR/fake/tmux.log" \ + FM_FAKE_AGENT_PID_FILE="$CASE_DIR/fake/agent.pid" \ + FM_FAKE_OMP_BIN="$FAKEBIN_DIR/omp" \ + FM_FAKE_LAUNCH_LOG="$LAUNCH_LOG" \ + FM_FAKE_OMP_ACK="$HOME_DIR/state/$id.omp-started" \ + FM_FAKE_OMP_ACK_DIR="$HOME_DIR/state" \ + FM_FAKE_OMP_NO_PREWALK=1 \ + FM_SPAWN_NO_GUARD=1 TMUX='fake,1,0' \ + FM_OMP_LAUNCH_ACK_POLLS=20 FM_OMP_DOORBELL_ACK_POLLS=20 \ + FM_BACKEND_TMUX_IDLE_SHELL_PROOF_POLLS=10 \ + PATH="$FAKEBIN_DIR:$PATH" \ + "$SPAWN" "$id" --relaunch 2>&1) + SPAWN_STATUS=$? + [ "$SPAWN_STATUS" -ne 0 ] || fail "a live-agent relaunch should refuse; got: $SPAWN_OUT" + assert_contains "$SPAWN_OUT" "live agent" "the refusal did not name the live agent" + assert_present "$requests/request.1.pending.acked" "a refused relaunch deleted the prior generation's receipts" + pass "refused relaunch: prior doorbell receipts survive untouched" +} + +# --- run --------------------------------------------------------------------- + +test_missing_endpoint_recovers_via_control +test_live_non_turning_recovers +test_live_busy_defers +test_live_unknown_busy_escalates +test_handled_record_recovers_quietly +test_late_handled_cancels_relaunch +test_dirty_worktree_escalates +test_unlanded_commits_escalate +test_attempt_cap_escalates +test_foreign_lease_escalates +test_non_pool_worktree_escalates +test_working_crew_state_escalates +test_secondmate_kind_escalates +test_refused_relaunch_preserves_receipts + +pass "all stall-recovery tests" From 149cf4fdf74563f45353b0e0dbf1882668b84b76 Mon Sep 17 00:00:00 2001 From: dnth Date: Wed, 23 Sep 2026 22:19:24 +0800 Subject: [PATCH 02/21] fix: restore hermes busy adapter, clean shellcheck, fix bun skip - fm-busy-lib: restore the hermes-hook adapter arm dropped by the omp-ext addition; its loss broke the crew-state status fallback (serial 3 failure). - fm-stall-recovery: SC1007 empty assignment and SC2097/SC2098 env-prefix expansion on the fm-control relaunch call. - fm-stall-recovery.test: skip at top level when bun is absent; the in-function exit only ended the command-substitution subshell and left FAKEBIN_DIR holding the skip text (serial 4 failure). --- bin/fm-busy-lib.sh | 1 + bin/fm-stall-recovery.sh | 5 +++-- tests/fm-stall-recovery.test.sh | 3 ++- 3 files changed, 6 insertions(+), 3 deletions(-) diff --git a/bin/fm-busy-lib.sh b/bin/fm-busy-lib.sh index f94906e6c61..b2f9cd7e463 100755 --- a/bin/fm-busy-lib.sh +++ b/bin/fm-busy-lib.sh @@ -181,6 +181,7 @@ fm_busy_sources_for_harness() { # opencode*) adapter=opencode-plugin ;; omp) adapter=omp-ext ;; pi|pi-signed) adapter=pi-ext ;; + hermes) adapter=hermes-hook ;; kimi*) fm_busy_kimi_verified || { printf ''; return 0; } adapter='kimi-wire kimi-hook' diff --git a/bin/fm-stall-recovery.sh b/bin/fm-stall-recovery.sh index fc3990b3603..317c67d3fe1 100755 --- a/bin/fm-stall-recovery.sh +++ b/bin/fm-stall-recovery.sh @@ -222,7 +222,7 @@ esac max_attempts=${FM_STALL_RECOVERY_MAX:-1} case "$max_attempts" in ''|*[!0-9]*) max_attempts=1 ;; esac attempts_file="$dir/.recovery-attempts" -attempts_record= attempts_count=0 +attempts_record='' attempts_count=0 IFS=$(printf '\t') read -r attempts_record attempts_count </dev/null || true) EOF @@ -251,8 +251,9 @@ note="Stall auto-recovery ($TRIGGER, $PATH_KIND): the previous worker stopped ac printf '%s\t%s\n' "${RECORD##*/}" "$((attempts_count + 1))" > "$attempts_file" 2>/dev/null \ || verdict escalate "cannot persist the recovery-attempt bound at $attempts_file" +FM_CONFIG_OVERRIDE=${FM_CONFIG_OVERRIDE:-$FM_HOME/config} control_out=$(FM_HOME="$FM_HOME" FM_STATE_OVERRIDE="$STATE" FM_DATA_OVERRIDE="$DATA" \ - FM_CONFIG_OVERRIDE="${FM_CONFIG_OVERRIDE:-$FM_HOME/config}" \ + FM_CONFIG_OVERRIDE="$FM_CONFIG_OVERRIDE" \ "$FM_STALL_RECOVERY_CONTROL_BIN" "$ID" relaunch --note "$note" 2>&1) \ || verdict escalate "fm-control relaunch refused or failed: $(printf '%s' "$control_out" | tail -1)" diff --git a/tests/fm-stall-recovery.test.sh b/tests/fm-stall-recovery.test.sh index ded23494078..ff3e484937e 100755 --- a/tests/fm-stall-recovery.test.sh +++ b/tests/fm-stall-recovery.test.sh @@ -44,6 +44,7 @@ fm_stall_cleanup() { trap fm_stall_cleanup EXIT command -v jq >/dev/null 2>&1 || { echo "skip: jq not found (required by the lease proof)"; exit 0; } +command -v bun >/dev/null 2>&1 || { echo "skip: bun not found (required by the OMP identity probe)"; exit 0; } # --- fake backend CLIs ------------------------------------------------------- @@ -274,7 +275,7 @@ SH # identity from `command -v bun` and rewrites omp_bun in the metadata, so # the recorded identity and the sleeper's /proc//exe must both be the # real binary. - real_bun=$(command -v bun) || { echo "skip: bun not found"; exit 0; } + real_bun=$(command -v bun) || { echo "skip: bun not found"; exit 1; } ln -sf "$real_bun" "$fb/bun" # omp keeps its `#!/usr/bin/env bun` shebang (fm_omp_process_launch_identity From ba0772e4a301dba8cdb67df982f87af1d94f69f8 Mon Sep 17 00:00:00 2001 From: dnth Date: Wed, 23 Sep 2026 22:29:41 +0800 Subject: [PATCH 03/21] no-mistakes(document): Document stall-recovery artifacts and OMP recovery behavior --- AGENTS.md | 1 + 1 file changed, 1 insertion(+) diff --git a/AGENTS.md b/AGENTS.md index e55638ab2b7..7dbd2743f59 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -106,6 +106,7 @@ state/ volatile runtime signals; gitignored .hermes-turnend-token .hermes-session .hermes-started firstmate-owned Hermes hook registry token plus the task's stable session id and per-turn start acknowledgement; removed by teardown .omp-ext.ts .omp-ready .omp-started .omp-doorbell-ready .omp-doorbell-failed firstmate-generated OMP task extension plus its session-start and first-turn acknowledgement markers; .omp-ready publishes only after the inbox doorbell activates, and a lost handshake journals its reason to .omp-doorbell-failed (docs/architecture.md; bin/fm-task-inbox-lib.sh); removed by teardown .inbox/ durable steering inbox: sequenced firstmate instruction records the worker acknowledges by moving them into its handled/ subdirectory; written by fm-send, re-rung and escalated by the watcher, removed by teardown (bin/fm-task-inbox-lib.sh) + .stall-recovery append-only custody and bounded-attempt verdict journal for watcher-triggered stall recovery (bin/fm-stall-recovery.sh); audit evidence, never recovery authority inbox/ trusted-local orchestrator notes written by bin/fm-inbox.sh; pending mode-0600 *.note records move to handled/ on acknowledgement, and failed wake publication leaves the note durable (docs/architecture.md) inbox-results/ trusted-local terminal result envelopes and delivery state written by bin/fm-inbox-result.sh; result, posting, receipt, failure, and retry-confirmation records remain mode-0600 across restarts (docs/architecture.md) .meta written by fm-spawn: window=, endpoint_task_id=, worktree=, project=, harness=, model=, effort=, kind=, mode=, yolo=, tasktmp=; optional grok_turnend_dir=, kimi_turnend_dir=, and devin_turnend_dir= persist harness registry ownership for teardown; optional prewalk_into= records an effective OMP Prewalk target; optional allow_project_omp_extensions=1 records explicit approval for tracked project extensions on an OMP launch (docs/configuration.md "OMP project extensions"); an optional traceparent= only when trace context is enabled (docs/configuration.md "Trace context propagation"); kind=secondmate also records home= and projects=, plus remote_host=/remote_root=/remote_backend=/remote_herdr_session=/remote_target= for a remote route; a non-default runtime backend records further backend-specific fields (docs/configuration.md "Runtime backend"; bin/fm-backend.sh, section 8); fm-pr-check, including through fm-pr-merge, records one canonical pr= and the forge's pr_head= when available (GitHub pull requests and GitLab merge requests; docs/gitlab-merge-watch.md); fm-x-link appends x_request=, x_request_ts=, x_followups=, and optional x_platform=/x_reply_max_chars= for an X-mode-originated task (section 14) From 58d478bcb564d7d502857f4da7eb704b18471638 Mon Sep 17 00:00:00 2001 From: firstmate Date: Thu, 24 Sep 2026 08:27:07 +0800 Subject: [PATCH 04/21] fix: close stall-recovery custody gaps from verification review - fm-stall-recovery acquires fm-control's lifecycle lock before the final custody/record re-check and holds it across the relaunch via --lock-preheld, so a handled record or concurrent lifecycle action can never relaunch a now-productive worker or double-relaunch - fm-control --stall-record re-proves the named inbox record inside the lock immediately before the agent is touched, cancelling with exit 3 when it resolved in flight - clean-worktree and unlanded-commit gates removed from prove_custody: recovery exists to preserve exactly that work in the same worktree, branch, and commits - OMP extension serializes busy-state writes through one awaited chain so turn_end's idle can never land after a following turn_start's busy - fm-spawn writes control_relaunch_tx into the published record so post-publish failures classify the successor correctly - one-relaunch-per-record bound is a fixed invariant; the FM_STALL_RECOVERY_MAX override is removed --- bin/fm-control.sh | 75 ++++++++++-- bin/fm-spawn.sh | 34 +++++- bin/fm-stall-recovery.sh | 148 +++++++++++++++--------- tests/fm-stall-recovery.test.sh | 197 +++++++++++++++++++++++++++++--- 4 files changed, 365 insertions(+), 89 deletions(-) diff --git a/bin/fm-control.sh b/bin/fm-control.sh index f1a60ce7252..b1cb27f3f15 100755 --- a/bin/fm-control.sh +++ b/bin/fm-control.sh @@ -5,7 +5,7 @@ # Usage: fm-control.sh interrupt # fm-control.sh exit # fm-control.sh relaunch [--harness ] [--model ] -# [--effort ] +# [--effort ] [--lock-preheld] # (--note | --note-file ) # # Why this exists, and how it differs from fm-send.sh. bin/fm-send.sh is the @@ -46,6 +46,17 @@ # inherits the local copy but none of the conversation; a # secondmate reconciles its own home's records at startup, so its # standing charter is never rewritten. +# --lock-preheld is the supervised-recovery handshake: the caller +# (bin/fm-stall-recovery.sh) already holds this task's lifecycle +# lock, so fm-control verifies the lock's recorded owner is its +# own parent process instead of acquiring it. The flag never +# bypasses the lock - without a live parent holding it, the +# command refuses. +# --stall-record rides the same handshake: inside the +# lock, immediately before the agent is touched, fm-control +# re-proves the named inbox record is still the oldest unhandled +# instruction and cancels with exit 3 when it was handled or +# superseded in flight. It requires --lock-preheld. # Records a durable checkpoint and that note, exits the old agent, # then delegates the launch to its single owner, # bin/fm-spawn.sh --relaunch. A failure before publication keeps @@ -142,6 +153,8 @@ DATA="${FM_DATA_OVERRIDE:-$FM_HOME/data}" . "$SCRIPT_DIR/fm-pr-lib.sh" # shellcheck source=bin/fm-wake-lib.sh . "$SCRIPT_DIR/fm-wake-lib.sh" +# shellcheck source=bin/fm-task-inbox-lib.sh +. "$SCRIPT_DIR/fm-task-inbox-lib.sh" POLL=${FM_CONTROL_POLL:-0.5} SETTLE_WAIT=${FM_CONTROL_SETTLE_WAIT:-5} @@ -203,6 +216,8 @@ MODEL_SET=0 EFFORT_SET=0 NOTE= NOTE_SET=0 +LOCK_PREHELD=0 +STALL_RECORD= control_want_value= for control_arg in "$@"; do if [ -n "$control_want_value" ]; then @@ -214,6 +229,7 @@ for control_arg in "$@"; do model) NEW_MODEL=$control_arg; MODEL_SET=1 ;; effort) NEW_EFFORT=$control_arg; EFFORT_SET=1 ;; note) NOTE=$control_arg; NOTE_SET=1 ;; + stall_record) STALL_RECORD=$control_arg ;; note_file) [ -f "$control_arg" ] || die "--note-file '$control_arg' is not a readable file" NOTE=$(cat "$control_arg") @@ -238,6 +254,9 @@ for control_arg in "$@"; do NOTE=$(cat "${control_arg#--note-file=}") NOTE_SET=1 ;; + --lock-preheld) LOCK_PREHELD=1 ;; + --stall-record) control_want_value=stall_record ;; + --stall-record=*) STALL_RECORD=${control_arg#--stall-record=} ;; *) die "unexpected argument '$control_arg'" ;; esac done @@ -247,9 +266,14 @@ if [ -n "$control_want_value" ]; then fi if [ "$VERB" != relaunch ]; then - [ "$HARNESS_SET" = 0 ] && [ "$MODEL_SET" = 0 ] && [ "$EFFORT_SET" = 0 ] && [ "$NOTE_SET" = 0 ] \ - || die "--harness, --model, --effort, and --note apply to 'relaunch' only" + [ "$HARNESS_SET" = 0 ] && [ "$MODEL_SET" = 0 ] && [ "$EFFORT_SET" = 0 ] && [ "$NOTE_SET" = 0 ] && [ "$LOCK_PREHELD" = 0 ] && [ -z "$STALL_RECORD" ] \ + || die "--harness, --model, --effort, --note, --lock-preheld, and --stall-record apply to 'relaunch' only" fi +# The stall-record re-check is only meaningful inside the supervised-recovery +# handshake: without --lock-preheld there is no proof the caller serialized +# its own custody checks with this invocation. +[ -z "$STALL_RECORD" ] || [ "$LOCK_PREHELD" = 1 ] \ + || die "--stall-record requires --lock-preheld" [ "$HARNESS_SET" = 0 ] || [ -n "$NEW_HARNESS" ] || die "--harness requires a non-empty value" [ "$MODEL_SET" = 0 ] || [ -n "$NEW_MODEL" ] || die "--model requires a non-empty value" [ "$EFFORT_SET" = 0 ] || [ -n "$NEW_EFFORT" ] || die "--effort requires a non-empty value" @@ -275,9 +299,23 @@ ID=$RAW_ID fm_lease_guard "$ID" "lifecycle control (fm-control)" CONTROL_LOCK="$STATE/.control-$ID.lock" trap control_cleanup EXIT -fm_lock_try_acquire "$CONTROL_LOCK" \ - || die "another lifecycle action is already running for task $ID" -CONTROL_LOCK_HELD=1 +if [ "$LOCK_PREHELD" = 1 ]; then + # Supervised-recovery handshake: the direct parent (bin/fm-stall-recovery.sh) + # holds this task's lifecycle lock across this invocation, so its custody and + # inbox-record re-checks are serialized with the relaunch itself. The flag + # never bypasses the lock: it is honored only when the lock exists, its + # recorded owner pid is this process's own parent, and that parent is alive. + # CONTROL_LOCK_HELD stays 0 so the cleanup trap never releases a lock this + # process does not own. + lock_owner_pid=$(cat "$CONTROL_LOCK/pid" 2>/dev/null || true) + if [ -z "$lock_owner_pid" ] || [ "$lock_owner_pid" != "$PPID" ] || ! fm_pid_alive "$lock_owner_pid"; then + die "--lock-preheld requires the caller to hold $CONTROL_LOCK as this process's live parent (owner: ${lock_owner_pid:-none}, parent: $PPID)" + fi +else + fm_lock_try_acquire "$CONTROL_LOCK" \ + || die "another lifecycle action is already running for task $ID" + CONTROL_LOCK_HELD=1 +fi META="$STATE/$ID.meta" if [ ! -f "$META" ]; then case "$RAW_ID" in @@ -795,7 +833,7 @@ record_note() { } do_relaunch() { - local exit_result state note_line + local exit_result state note_line stall_oldest local -a spawn_args require_state_verified_backend relaunch @@ -832,6 +870,27 @@ do_relaunch() { record_note journal_write noted "${CHECKPOINT_LINES[@]}" "$note_line" + # Final stall-record re-check, inside the lifecycle lock and immediately + # before the agent is touched: the caller's own pre-invocation proof cannot + # cover the gap to this point, so a record handled or superseded in flight + # must still cancel the relaunch here. Exit 3 is the dedicated "instruction + # resolved; nothing to do" code the supervised caller maps to recovered. The + # cancelled phase is journaled first so the rollback trap leaves this record + # rather than a misleading failed:noted. + if [ -n "$STALL_RECORD" ]; then + stall_oldest=$(fm_task_inbox_oldest_unhandled "$STATE" "$ID" 2>/dev/null || true) + if [ -z "$stall_oldest" ]; then + journal_write "cancelled:record-resolved" "${CHECKPOINT_LINES[@]}" "$note_line" || true + echo "relaunch cancelled: stall record resolved (inbox empty; instruction handled)" >&2 + exit 3 + fi + if [ "${stall_oldest##*/}" != "$STALL_RECORD" ]; then + journal_write "cancelled:record-resolved" "${CHECKPOINT_LINES[@]}" "$note_line" || true + echo "relaunch cancelled: stall record $STALL_RECORD handled or superseded (${stall_oldest##*/} is now oldest)" >&2 + exit 3 + fi + fi + journal_write stopping "${CHECKPOINT_LINES[@]}" "$note_line" state=$(agent_state) if [ "$state" = missing ]; then @@ -888,7 +947,7 @@ do_relaunch() { } RELAUNCH_AGENT_CONFIRMED=1 - journal_write complete "${CHECKPOINT_LINES[@]}" "$note_line" "exit_result=$exit_result" + journal_write complete "${CHECKPOINT_LINES[@]}" "$note_line" "exit_result=$exit_result" "relaunch_tx=$RELAUNCH_TX" RELAUNCH_ACTIVE=0 echo "relaunched $ID harness=$TARGET_HARNESS from=$PRIOR_RECORDED_HARNESS model=$TARGET_MODEL effort=$TARGET_EFFORT backend=$BACKEND endpoint=$T worktree=$WT" } diff --git a/bin/fm-spawn.sh b/bin/fm-spawn.sh index d5ec7ccded9..5ead43629e5 100755 --- a/bin/fm-spawn.sh +++ b/bin/fm-spawn.sh @@ -435,6 +435,14 @@ done [ "$TRACEPARENT_SET" -eq 0 ] || [ -n "$TRACEPARENT_ARG" ] || { echo "error: --traceparent requires a non-empty value" >&2; exit 1; } if [ "$RELAUNCH" -eq 1 ]; then + # A relaunch driven by fm-control carries its transaction id so a failure + # after this record is published still classifies the successor correctly. + # The value lands verbatim in the durable record, so a malformed token is + # refused rather than corrupting it. + case "${FM_CONTROL_RELAUNCH_TX:-}" in + '') ;; + *[!A-Za-z0-9._-]*) { echo "error: FM_CONTROL_RELAUNCH_TX is not a safe metadata token" >&2; exit 1; } ;; + esac RELAUNCH_ID=${POS[0]:-} [ -n "$RELAUNCH_ID" ] || { echo "error: --relaunch requires a task id" >&2; exit 1; } RELAUNCH_META="$STATE/$RELAUNCH_ID.meta" @@ -4416,6 +4424,16 @@ const busyEvent = (state: string, event: string) => "--gen", "$BUSY_GEN", "--source", "omp-ext", "--event", event, ], () => resolve(undefined)); }); +// Busy-state writes are serialized through one chain so a turn_end's idle can +// never land after a following turn_start's busy: an out-of-order pair would +// leave a live worker falsely busy (suppressing stall recovery forever) or a +// dead one falsely idle. Handlers also await the chain so the runtime's own +// event ordering is honored end to end. +let busyChain: Promise = Promise.resolve(); +const queueBusyEvent = (state: string, event: string) => { + busyChain = busyChain.then(() => busyEvent(state, event)); + return busyChain; +}; export default function (omp: any) { const taskInboxDoorbell = installTaskInboxDoorbell(omp, { inboxDir: "$STATE_REAL/$ID.inbox", @@ -4431,19 +4449,19 @@ export default function (omp: any) { if (active) execFile("touch", ["$OMP_READY"]); }); }); - omp.on("turn_start", () => { + omp.on("turn_start", async () => { taskInboxDoorbell.notifyTurnStart(); execFile("touch", ["$OMP_STARTED"]); - busyEvent("busy", "turn-start"); + await queueBusyEvent("busy", "turn-start"); }); - omp.on("turn_end", () => { + omp.on("turn_end", async () => { taskInboxDoorbell.notifyTurnEnd(); execFile("$TURNEND_SIGNAL", ["$STATE_REAL", "$ID", "$SPAWN_GEN"]); - busyEvent("idle", "turn-end"); + await queueBusyEvent("idle", "turn-end"); }); - omp.on("session_shutdown", () => { + omp.on("session_shutdown", async () => { taskInboxDoorbell.retire(); - busyEvent("idle", "session-shutdown"); + await queueBusyEvent("idle", "session-shutdown"); }); } EOF @@ -4670,6 +4688,10 @@ SPAWN_META_LOCK_HELD=1 echo "model=${MODEL:-default}" echo "effort=${EFFORT:-default}" echo "spawn_gen=$SPAWN_GEN" + # The relaunch transaction id lets fm-control classify a post-publish + # failure as "new record published" rather than "replacement never + # launched"; only a relaunch under fm-control writes it. + [ -z "${FM_CONTROL_RELAUNCH_TX:-}" ] || echo "control_relaunch_tx=$FM_CONTROL_RELAUNCH_TX" [ -z "${GROK_AUTH_DIR:-}" ] || echo "grok_turnend_dir=$GROK_AUTH_DIR" [ -z "${KIMI_AUTH_DIR:-}" ] || echo "kimi_turnend_dir=$KIMI_AUTH_DIR" [ -z "${DEVIN_AUTH_DIR:-}" ] || echo "devin_turnend_dir=$DEVIN_AUTH_DIR" diff --git a/bin/fm-stall-recovery.sh b/bin/fm-stall-recovery.sh index 317c67d3fe1..71119a2a9a9 100755 --- a/bin/fm-stall-recovery.sh +++ b/bin/fm-stall-recovery.sh @@ -37,11 +37,11 @@ # declared external wait, and a worker-declared blocker are firstmate # business, not stall recovery). Terminal done/failed may proceed, and # unknown may proceed only on the missing-endpoint path. -# - The recorded worktree must be clean AND hold no commit absent from every -# remote-tracking ref (local-only mode instead requires every commit -# merged into the local default branch). Unlanded work blocks automatic -# action and escalates; the check is local-only - no gh or fetch - so the -# watcher can never hang on a remote. +# - The recorded worktree must exist and carry the durable fm- lease; +# uncommitted changes and unpushed commits inside it are PRESERVED, not +# rejected: the stalled worker's unlanded work is exactly what recovery +# exists to keep, and the relaunch inherits the same worktree, branch, +# and commits untouched. # - One automatic relaunch per stalled instruction: the per-record attempt # marker under the inbox bounds retries, and an emptied inbox resets it. # - The durable fm- worktree lease, same-worktree/branch/commits @@ -55,16 +55,36 @@ # handled/ move (quiet) or the bounded re-escalation the reset ladder produces # when the replacement also fails to act. # +# The final custody and inbox-record re-check runs INSIDE fm-control's +# lifecycle lock (state/.control-.lock), acquired by this process and held +# across the fm-control invocation via --lock-preheld: a record handled in the +# gap can never relaunch a now-productive worker, and a concurrent invocation +# or manual lifecycle action can never double-relaunch. The per-record attempt +# bound is checked and recorded under the same lock. +# # Audit: every verdict appends one line to state/.stall-recovery; the # relaunch transaction itself journals to state/.control-relaunch, and a # note: line on state/.status records the published recovery. # # Tunables (env): -# FM_STALL_RECOVERY_MAX automatic relaunches per stalled record (1) # FM_CREW_STATE_BIN crew-state executable override (tests) # FM_STALL_RECOVERY_CONTROL_BIN lifecycle executable override (tests) +# +# The one-relaunch-per-record bound is a fixed invariant, not a tunable. set -u +# Release the lifecycle lock on every exit path, including verdict exits. +STALL_LOCK= +STALL_LOCK_HELD=0 +# shellcheck disable=SC2329 # Registered by the EXIT trap below. +stall_cleanup() { + if [ "$STALL_LOCK_HELD" = 1 ]; then + STALL_LOCK_HELD=0 + fm_lock_release "$STALL_LOCK" 2>/dev/null || true + fi +} +trap stall_cleanup EXIT + 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}}" @@ -75,8 +95,6 @@ DATA="${FM_DATA_OVERRIDE:-$FM_HOME/data}" . "$SCRIPT_DIR/fm-backend.sh" # shellcheck source=bin/fm-busy-lib.sh . "$SCRIPT_DIR/fm-busy-lib.sh" -# shellcheck source=bin/fm-worktree-clean-lib.sh -. "$SCRIPT_DIR/fm-worktree-clean-lib.sh" # shellcheck source=bin/fm-task-inbox-lib.sh . "$SCRIPT_DIR/fm-task-inbox-lib.sh" @@ -128,19 +146,22 @@ fi # prove_custody: re-prove every precondition for a lifecycle action against # CURRENT state - endpoint classification, busy verdict, crew/run state, -# worktree cleanliness, landed work, and the durable fm- lease. Called -# directly (never in a command substitution) so its PATH_KIND, WT, MODE, -# PROJ, BACKEND, and TARGET bindings reach the caller; the refusal reason is -# published through the CUSTODY_DETAIL global. Returns 1 on any failed or -# unprovable check, 2 when the worker is provably busy (a defer, not an -# escalation), 0 on success. All reads are local: no gh or fetch can ever -# stall the watcher. +# worktree existence, and the durable fm- lease. Called directly (never in +# a command substitution) so its PATH_KIND, WT, PROJ, BACKEND, and TARGET +# bindings reach the caller; the refusal reason is published through the +# CUSTODY_DETAIL global. Returns 1 on any failed or unprovable check, 2 when +# the worker is provably busy (a defer, not an escalation), 0 on success. All +# reads are local: no gh or fetch can ever stall the watcher. +# +# Uncommitted changes and unpushed commits are deliberately NOT gates here: +# recovery exists to preserve exactly that unlanded work, and the relaunch +# inherits the same worktree, branch, and commits untouched (fm-control's +# safe_checkpoint records head and dirty state for the journal). prove_custody() { - local state busy crew_line crew_state unpushed unmerged default_ref cand pool_state lease_holder + local state busy crew_line crew_state pool_state lease_holder PATH_KIND= CUSTODY_DETAIL= WT=$(fm_meta_get "$META" worktree) - MODE=$(fm_meta_get "$META" mode) PROJ=$(fm_meta_get "$META" project) fm_backend_validate_task_endpoint "$META" "$ID" >/dev/null 2>&1 \ || { CUSTODY_DETAIL='endpoint metadata failed validation'; return 1; } @@ -172,24 +193,6 @@ prove_custody() { *) CUSTODY_DETAIL="crew-state '${crew_state:-unreadable}' is not a clean non-run state"; return 1 ;; esac [ -n "$WT" ] && [ -d "$WT" ] || { CUSTODY_DETAIL='recorded worktree missing'; return 1; } - fm_worktree_is_clean "$WT" || { CUSTODY_DETAIL='worktree has uncommitted changes'; return 1; } - if [ "$MODE" = local-only ]; then - default_ref= - for cand in main master; do - if git -C "$PROJ" show-ref --verify --quiet "refs/heads/$cand" 2>/dev/null; then - default_ref=$cand - break - fi - done - [ -n "$default_ref" ] || { CUSTODY_DETAIL='local-only task has no resolvable default branch'; return 1; } - unmerged=$(git -C "$WT" log --format=%H HEAD --not "$default_ref" -- 2>/dev/null) \ - || { CUSTODY_DETAIL="cannot inspect worktree commits against $default_ref"; return 1; } - [ -z "$unmerged" ] || { CUSTODY_DETAIL="local-only worktree has commits not merged into $default_ref"; return 1; } - else - unpushed=$(git -C "$WT" log --format=%H HEAD --not --remotes -- 2>/dev/null) \ - || { CUSTODY_DETAIL='cannot inspect worktree commits against remotes'; return 1; } - [ -z "$unpushed" ] || { CUSTODY_DETAIL='worktree has commits not on any remote-tracking ref'; return 1; } - fi # The durable fm- lease proof mirrors bin/fm-spawn.sh's # relaunch_worktree_lease_proven, applied to BOTH paths here because the # launch owner only re-proves it on the gone-endpoint path. @@ -218,24 +221,23 @@ prove_custody || case $? in *) verdict escalate "$CUSTODY_DETAIL" ;; esac -# Bounded retry: one automatic relaunch per stalled instruction record. -max_attempts=${FM_STALL_RECOVERY_MAX:-1} -case "$max_attempts" in ''|*[!0-9]*) max_attempts=1 ;; esac -attempts_file="$dir/.recovery-attempts" -attempts_record='' attempts_count=0 -IFS=$(printf '\t') read -r attempts_record attempts_count </dev/null || true) -EOF -[ "$attempts_record" = "${RECORD##*/}" ] || attempts_count=0 -case "$attempts_count" in ''|*[!0-9]*) attempts_count=0 ;; esac -[ "$attempts_count" -lt "$max_attempts" ] \ - || verdict escalate "automatic recovery already attempted for ${RECORD##*/}; escalating per bounded-retry policy" +# --- bounded lifecycle action, under fm-control's lifecycle lock ------------ +# +# The lock is acquired BEFORE the final re-check and held across the +# fm-control invocation (--lock-preheld proves the caller owns it), so a +# record handled in the gap can never relaunch a now-productive worker and a +# concurrent invocation or manual lifecycle action can never double-relaunch. +# A live holder means another lifecycle action is in flight: defer, and the +# watcher re-evaluates on the next cycle. +STALL_LOCK="$STATE/.control-$ID.lock" +fm_lock_try_acquire "$STALL_LOCK" \ + || verdict deferred "lifecycle lock for $ID is held by pid ${FM_LOCK_HELD_PID:-unknown}; another lifecycle action is in flight" +STALL_LOCK_HELD=1 -# Final re-check immediately before the lifecycle action, in strict order: -# re-prove the full custody chain (a worker can become busy, enter a run, or -# lose its worktree between the first proof and the relaunch), then re-prove -# the record itself LAST so a handled move during the custody probe still -# cancels the action. +# Final re-check inside the lock, in strict order: re-prove the full custody +# chain (a worker can become busy, enter a run, or lose its worktree between +# the first proof and the relaunch), then re-prove the record itself LAST so +# a handled move during the custody probe still cancels the action. prove_custody || case $? in 2) verdict deferred "$CUSTODY_DETAIL" ;; *) verdict escalate "$CUSTODY_DETAIL" ;; @@ -244,18 +246,50 @@ oldest=$(fm_task_inbox_oldest_unhandled "$STATE" "$ID" 2>/dev/null || true) [ -n "$oldest" ] || verdict recovered "inbox emptied before relaunch; instruction handled" [ "$oldest" = "$RECORD" ] || verdict deferred "record ${RECORD##*/} handled or superseded before relaunch" -# --- bounded lifecycle action ------------------------------------------------ +# Bounded retry: exactly one automatic relaunch per stalled instruction +# record, checked and recorded under the lock so concurrent invocations +# cannot both pass the bound. The bound is a fixed invariant - no override. +attempts_file="$dir/.recovery-attempts" +attempts_record='' attempts_count=0 +IFS=$(printf '\t') read -r attempts_record attempts_count </dev/null || true) +EOF +[ "$attempts_record" = "${RECORD##*/}" ] || attempts_count=0 +case "$attempts_count" in ''|*[!0-9]*) attempts_count=0 ;; esac +[ "$attempts_count" -lt 1 ] \ + || verdict escalate "automatic recovery already attempted for ${RECORD##*/}; escalating per bounded-retry policy" unhandled=$(cd "$dir" 2>/dev/null && printf '%s ' *.msg 2>/dev/null || true) note="Stall auto-recovery ($TRIGGER, $PATH_KIND): the previous worker stopped acting on doorbells while instruction(s) ${unhandled:-${RECORD##*/}} stayed unhandled. The worktree, branch, and commits are exactly as that worker left them; nothing was discarded. Read and act on the inbox first." printf '%s\t%s\n' "${RECORD##*/}" "$((attempts_count + 1))" > "$attempts_file" 2>/dev/null \ || verdict escalate "cannot persist the recovery-attempt bound at $attempts_file" +# fm-control must run as a direct child so --lock-preheld's owner check +# ($PPID == the lock's recorded owner pid) binds to THIS process; a command +# substitution would interpose a subshell and fail the proof. --stall-record +# hands fm-control the record basename so it re-proves the instruction is +# still the oldest unhandled record inside the lock, immediately before the +# agent is touched - the check above cannot cover the gap to that point. +# Exit 3 is fm-control's "record resolved; nothing to do" code. FM_CONFIG_OVERRIDE=${FM_CONFIG_OVERRIDE:-$FM_HOME/config} -control_out=$(FM_HOME="$FM_HOME" FM_STATE_OVERRIDE="$STATE" FM_DATA_OVERRIDE="$DATA" \ - FM_CONFIG_OVERRIDE="$FM_CONFIG_OVERRIDE" \ - "$FM_STALL_RECOVERY_CONTROL_BIN" "$ID" relaunch --note "$note" 2>&1) \ - || verdict escalate "fm-control relaunch refused or failed: $(printf '%s' "$control_out" | tail -1)" +control_out_file=$(mktemp "$STATE/.stall-recovery-control-out.XXXXXX" 2>/dev/null) \ + || verdict escalate "cannot allocate the fm-control output capture" +if FM_HOME="$FM_HOME" FM_STATE_OVERRIDE="$STATE" FM_DATA_OVERRIDE="$DATA" \ + FM_CONFIG_OVERRIDE="$FM_CONFIG_OVERRIDE" \ + "$FM_STALL_RECOVERY_CONTROL_BIN" "$ID" relaunch --lock-preheld \ + --stall-record "${RECORD##*/}" --note "$note" \ + > "$control_out_file" 2>&1; then + control_rc=0 +else + control_rc=$? +fi +control_out=$(cat "$control_out_file" 2>/dev/null || true) +rm -f "$control_out_file" +case "$control_rc" in + 0) ;; + 3) verdict recovered "record ${RECORD##*/} resolved inside the lifecycle lock before the relaunch; instruction handled" ;; + *) verdict escalate "fm-control relaunch refused or failed: $(printf '%s' "$control_out" | tail -1)" ;; +esac # The replacement owns the instruction now. Reset the delivery ladder so the # new incarnation gets the full grace-and-retry budget before the bounded diff --git a/tests/fm-stall-recovery.test.sh b/tests/fm-stall-recovery.test.sh index ff3e484937e..4fce6203219 100755 --- a/tests/fm-stall-recovery.test.sh +++ b/tests/fm-stall-recovery.test.sh @@ -329,6 +329,24 @@ exit "${FM_FAKE_CONTROL_RC:-0}" SH chmod +x "$fb/fm-control.sh" + # git forwards to the real binary, except when FM_FAKE_GIT_MOVE names an + # inbox record: the first `git -C status --porcelain` inside + # fm-control's safe_checkpoint moves that record to handled/, simulating a + # worker that acknowledges the instruction in the gap between the caller's + # pre-invocation check and fm-control's in-lock re-check. + cat > "$fb/git" <<'SH' +#!/usr/bin/env bash +set -u +if [ -n "${FM_FAKE_GIT_MOVE:-}" ] && [ "${1:-}" = "-C" ] && [ "${3:-}" = "status" ]; then + if [ -f "$FM_FAKE_GIT_MOVE" ]; then + mkdir -p "${FM_FAKE_GIT_MOVE%/*}/handled" + mv "$FM_FAKE_GIT_MOVE" "${FM_FAKE_GIT_MOVE%/*}/handled/" + fi +fi +exec /usr/bin/git "$@" +SH + chmod +x "$fb/git" + printf '%s\n' "$fb" } @@ -528,6 +546,8 @@ test_missing_endpoint_recovers_via_control() { assert_absent "$requests/request.1.pending.acked" "stale .acked tombstone survived the relaunch" assert_absent "$requests/request.2.pending.unproven" "stale .unproven receipt survived the relaunch" assert_grep "001.msg" "$HOME_DIR/state/$id.inbox/.recovery-attempts" "the per-record attempt bound was not recorded" + assert_grep "control_relaunch_tx=" "$HOME_DIR/state/$id.meta" "fm-spawn did not record the relaunch transaction id in the published metadata" + assert_grep "relaunch_tx=" "$HOME_DIR/state/$id.control-relaunch" "the relaunch journal did not record the transaction id" assert_present "$record" "the unhandled instruction record was moved or deleted" assert_grep "stall auto-recovery" "$HOME_DIR/state/$id.status" "no audit note was appended to the task status" pass "missing endpoint: real relaunch publishes, ladder resets, stale receipts retire, record stays unhandled" @@ -645,9 +665,10 @@ test_late_handled_cancels_relaunch() { pass "late handled move during the custody probe cancels the relaunch" } -# Uncommitted work in the worktree is unlanded evidence: escalate, never -# relaunch into it. -test_dirty_worktree_escalates() { +# Uncommitted work in the worktree is exactly what recovery preserves: the +# stalled worker's unlanded changes must NOT block the relaunch, and the +# replacement inherits the same worktree, branch, and dirty state untouched. +test_dirty_worktree_recovers_preserving_work() { local rec id record id=$(case_id dirty) rec=$(make_case dirty "$id" pool) @@ -661,15 +682,21 @@ test_dirty_worktree_escalates() { missing_window "$CASE_DIR" "$id" printf 'uncommitted\n' > "$WT_DIR/scratch.txt" - run_recovery "$CASE_DIR" "$HOME_DIR" "$id" "$record" endpoint-unavailable - assert_contains "$RECOVERY_OUT" "verdict=escalate" "a dirty worktree did not escalate" - assert_absent "$CASE_DIR/control.log" "the lifecycle verb ran against a dirty worktree" - pass "dirty worktree: recovery escalates rather than discarding uncommitted work" + FM_STALL_RECOVERY_CONTROL_BIN="$CONTROL" \ + run_recovery "$CASE_DIR" "$HOME_DIR" "$id" "$record" endpoint-unavailable + expect_code 0 "$RECOVERY_STATUS" "dirty-worktree recovery should exit 0; got: $RECOVERY_OUT" + assert_contains "$RECOVERY_OUT" "verdict=deferred" "a dirty worktree did not defer pending the episode" + assert_grep "new-window" "$CASE_DIR/fake/tmux.log" "relaunch did not create a replacement tmux window" + assert_grep "worktree_dirty=yes" "$HOME_DIR/state/$id.control-relaunch" "the relaunch checkpoint did not record the dirty state" + assert_grep "uncommitted" "$WT_DIR/scratch.txt" "the relaunch discarded uncommitted work" + assert_present "$record" "the unhandled instruction record was moved or deleted" + pass "dirty worktree: recovery relaunches and preserves uncommitted work in place" } -# Commits not on any remote-tracking ref are unlanded work: escalate. -test_unlanded_commits_escalate() { - local rec id record +# Commits not on any remote-tracking ref are unlanded work the replacement +# inherits: recovery must relaunch into the same branch, not escalate. +test_unlanded_commits_recover_preserving_branch() { + local rec id record head_before id=$(case_id unlanded) rec=$(make_case unlanded "$id" pool) read_case "$rec" @@ -683,15 +710,21 @@ test_unlanded_commits_escalate() { printf 'wip\n' > "$WT_DIR/wip.txt" git -C "$WT_DIR" add wip.txt git -C "$WT_DIR" -c user.email=t@t -c user.name=t commit -qm wip + head_before=$(git -C "$WT_DIR" rev-parse HEAD) - run_recovery "$CASE_DIR" "$HOME_DIR" "$id" "$record" endpoint-unavailable - assert_contains "$RECOVERY_OUT" "verdict=escalate" "unlanded commits did not escalate" - assert_absent "$CASE_DIR/control.log" "the lifecycle verb ran against unlanded commits" - pass "unlanded commits: recovery escalates rather than abandoning pushed-state proof" + FM_STALL_RECOVERY_CONTROL_BIN="$CONTROL" \ + run_recovery "$CASE_DIR" "$HOME_DIR" "$id" "$record" endpoint-unavailable + expect_code 0 "$RECOVERY_STATUS" "unlanded-commits recovery should exit 0; got: $RECOVERY_OUT" + assert_contains "$RECOVERY_OUT" "verdict=deferred" "unlanded commits did not defer pending the episode" + assert_grep "new-window" "$CASE_DIR/fake/tmux.log" "relaunch did not create a replacement tmux window" + assert_equals "$head_before" "$(git -C "$WT_DIR" rev-parse HEAD)" "the relaunch moved the branch head" + assert_grep "worktree_head=$head_before" "$HOME_DIR/state/$id.control-relaunch" "the relaunch checkpoint did not record the preserved head" + pass "unlanded commits: recovery relaunches into the same branch and preserves every commit" } # The per-record bound: a second automatic attempt for the same record -# escalates instead of looping relaunches. +# escalates instead of looping relaunches. The bound is a fixed invariant - +# FM_STALL_RECOVERY_MAX must not be able to raise it. test_attempt_cap_escalates() { local rec id record id=$(case_id capped) @@ -706,7 +739,8 @@ test_attempt_cap_escalates() { printf '001.msg\t1\n' > "$HOME_DIR/state/$id.inbox/.recovery-attempts" missing_window "$CASE_DIR" "$id" - run_recovery "$CASE_DIR" "$HOME_DIR" "$id" "$record" endpoint-unavailable + FM_STALL_RECOVERY_MAX=5 \ + run_recovery "$CASE_DIR" "$HOME_DIR" "$id" "$record" endpoint-unavailable assert_contains "$RECOVERY_OUT" "verdict=escalate" "a spent attempt bound did not escalate" assert_absent "$CASE_DIR/control.log" "the lifecycle verb ran past the attempt bound" pass "attempt bound: a second automatic relaunch for the same record escalates" @@ -831,6 +865,130 @@ test_refused_relaunch_preserves_receipts() { pass "refused relaunch: prior doorbell receipts survive untouched" } +# A lifecycle lock held by a live process means another lifecycle action is in +# flight: recovery defers instead of racing it, and the watcher re-evaluates on +# the next cycle. +test_held_lifecycle_lock_defers() { + local rec id record lockdir holder n + id=$(case_id lockheld) + rec=$(make_case lockheld "$id" pool) + read_case "$rec" + write_pool_state "$CASE_DIR" "$WT_DIR" "fm-$id" + write_slot_marker "$SLOT_DIR" "$id" "$HOME_DIR" + write_meta "$HOME_DIR/state/$id.meta" "$id" "$WT_DIR" "$PROJ_DIR" + create_prior_artifacts "$HOME_DIR/state" "$id" + write_inbox "$HOME_DIR/state" "$id" 001 + record="$HOME_DIR/state/$id.inbox/001.msg" + missing_window "$CASE_DIR" "$id" + + lockdir="$HOME_DIR/state/.control-$id.lock" + ( . "$ROOT/bin/fm-wake-lib.sh" + fm_lock_try_acquire "$lockdir" || exit 1 + : > "$HOME_DIR/state/.test-lock-ready" + sleep 60 ) & + holder=$! + n=0 + while [ ! -e "$HOME_DIR/state/.test-lock-ready" ] && [ "$n" -lt 100 ]; do + sleep 0.05; n=$((n + 1)) + done + [ -e "$HOME_DIR/state/.test-lock-ready" ] \ + || { kill "$holder" 2>/dev/null; fail "the lock holder never acquired $lockdir"; } + + run_recovery "$CASE_DIR" "$HOME_DIR" "$id" "$record" endpoint-unavailable + kill "$holder" 2>/dev/null; wait "$holder" 2>/dev/null || true + assert_contains "$RECOVERY_OUT" "verdict=deferred" "a held lifecycle lock did not defer recovery" + assert_absent "$CASE_DIR/control.log" "the lifecycle verb ran while the lock was held" + pass "held lifecycle lock: recovery defers rather than racing another lifecycle action" +} + +# The generated OMP extension must serialize busy-state writes: a turn_end's +# idle can never land after a following turn_start's busy, or a live worker +# reads falsely idle (and a dead one falsely busy suppresses recovery). The +# fake FM_ROOT wraps fm-busy-event.sh with a delay on the turn-end write, so +# an unserialized pair deterministically inverts. +test_omp_ext_serializes_busy_events() { + local rec id record fakeroot ext out tool + id=$(case_id omp-order) + rec=$(make_case omp-order "$id" pool) + read_case "$rec" + write_pool_state "$CASE_DIR" "$WT_DIR" "fm-$id" + write_slot_marker "$SLOT_DIR" "$id" "$HOME_DIR" + write_meta "$HOME_DIR/state/$id.meta" "$id" "$WT_DIR" "$PROJ_DIR" + create_prior_artifacts "$HOME_DIR/state" "$id" + write_inbox "$HOME_DIR/state" "$id" 001 + record="$HOME_DIR/state/$id.inbox/001.msg" + missing_window "$CASE_DIR" "$id" + + # Fake FM_ROOT: every bin entry is the real script except fm-busy-event.sh, + # which delays the turn-end apply so an unserialized turn-start write would + # land first and leave the worker falsely busy. + fakeroot="$CASE_DIR/fakeroot" + mkdir -p "$fakeroot/bin" "$fakeroot/.omp/extensions" + for tool in "$ROOT"/bin/*; do + [ "$(basename "$tool")" = fm-busy-event.sh ] || ln -s "$tool" "$fakeroot/bin/$(basename "$tool")" + done + ln -s "$ROOT/.omp/extensions/lib" "$fakeroot/.omp/extensions/lib" + cat > "$fakeroot/bin/fm-busy-event.sh" < { handlers[n] = fn; } }); + handlers["turn_end"](); + handlers["turn_start"](); + await new Promise((r) => setTimeout(r, 3000)); + ' || fail "driving the generated OMP extension failed" + out=$(cat "$HOME_DIR/state/$id.busy-state" 2>/dev/null || true) + case "$out" in + *"state=busy"*"event=turn-start"*) ;; + *) fail "turn_end's idle write landed after turn_start's busy (final record: '${out:-missing}'); the extension does not serialize busy events" ;; + esac + pass "OMP extension serializes busy-state writes: turn-start busy lands after turn-end idle" +} +# A record handled in the gap between the caller's pre-invocation check and +# fm-control's in-lock re-check must still cancel the relaunch: the fake git +# moves the record to handled/ during safe_checkpoint, so fm-control's own +# stall-record proof sees it resolved and exits 3 before the agent is touched. +test_in_lock_handled_record_cancels_relaunch() { + local rec id record + id=$(case_id inlock-ack) + rec=$(make_case inlock-ack "$id" pool) + read_case "$rec" + write_pool_state "$CASE_DIR" "$WT_DIR" "fm-$id" + write_slot_marker "$SLOT_DIR" "$id" "$HOME_DIR" + write_meta "$HOME_DIR/state/$id.meta" "$id" "$WT_DIR" "$PROJ_DIR" + create_prior_artifacts "$HOME_DIR/state" "$id" + write_inbox "$HOME_DIR/state" "$id" 001 + record="$HOME_DIR/state/$id.inbox/001.msg" + missing_window "$CASE_DIR" "$id" + + FM_FAKE_GIT_MOVE="$record" FM_STALL_RECOVERY_CONTROL_BIN="$CONTROL" \ + run_recovery "$CASE_DIR" "$HOME_DIR" "$id" "$record" endpoint-unavailable + expect_code 0 "$RECOVERY_STATUS" "in-lock cancellation should exit 0; got: $RECOVERY_OUT" + assert_contains "$RECOVERY_OUT" "verdict=recovered" "a record resolved inside the lock did not report recovered" + assert_no_grep "new-window" "$CASE_DIR/fake/tmux.log" "the relaunch created a window for a resolved record" + assert_grep "cancelled:record-resolved" "$HOME_DIR/state/$id.control-relaunch" "the journal did not record the in-lock cancellation" + assert_present "$HOME_DIR/state/$id.inbox/handled/001.msg" "the handled record is not in handled/" + pass "in-lock handled record: fm-control cancels the relaunch before touching the agent" +} + # --- run --------------------------------------------------------------------- test_missing_endpoint_recovers_via_control @@ -839,13 +997,16 @@ test_live_busy_defers test_live_unknown_busy_escalates test_handled_record_recovers_quietly test_late_handled_cancels_relaunch -test_dirty_worktree_escalates -test_unlanded_commits_escalate +test_dirty_worktree_recovers_preserving_work +test_unlanded_commits_recover_preserving_branch test_attempt_cap_escalates test_foreign_lease_escalates test_non_pool_worktree_escalates test_working_crew_state_escalates test_secondmate_kind_escalates test_refused_relaunch_preserves_receipts +test_held_lifecycle_lock_defers +test_in_lock_handled_record_cancels_relaunch +test_omp_ext_serializes_busy_events pass "all stall-recovery tests" From 3f6567b4d6b3683c78659d3764260136b2dc71c5 Mon Sep 17 00:00:00 2001 From: firstmate Date: Thu, 24 Sep 2026 08:33:02 +0800 Subject: [PATCH 05/21] fix: restore worker instructions on in-lock relaunch cancellation The cancelled:record-resolved journal write moved RELAUNCH_PHASE past the rollback trap's checkpoint|noted restore branch, so a record resolved inside the lifecycle lock left the brief modified despite no relaunch. Restore the brief byte-exact before journaling the cancellation, assert it in the in-lock test, and correct docs/architecture.md's stale claim that recovery requires a clean worktree with no unlanded commits. --- bin/fm-control.sh | 21 ++++++++++++--------- docs/architecture.md | 3 ++- tests/fm-stall-recovery.test.sh | 6 ++++++ 3 files changed, 20 insertions(+), 10 deletions(-) diff --git a/bin/fm-control.sh b/bin/fm-control.sh index b1cb27f3f15..6de660af415 100755 --- a/bin/fm-control.sh +++ b/bin/fm-control.sh @@ -875,18 +875,21 @@ do_relaunch() { # cover the gap to this point, so a record handled or superseded in flight # must still cancel the relaunch here. Exit 3 is the dedicated "instruction # resolved; nothing to do" code the supervised caller maps to recovered. The - # cancelled phase is journaled first so the rollback trap leaves this record - # rather than a misleading failed:noted. + # brief is restored byte-exact before the cancelled journal write so the + # rollback trap has nothing left to do and the worker's instructions stay + # untouched when no relaunch happened. if [ -n "$STALL_RECORD" ]; then stall_oldest=$(fm_task_inbox_oldest_unhandled "$STATE" "$ID" 2>/dev/null || true) - if [ -z "$stall_oldest" ]; then - journal_write "cancelled:record-resolved" "${CHECKPOINT_LINES[@]}" "$note_line" || true - echo "relaunch cancelled: stall record resolved (inbox empty; instruction handled)" >&2 - exit 3 - fi - if [ "${stall_oldest##*/}" != "$STALL_RECORD" ]; then + if [ -z "$stall_oldest" ] || [ "${stall_oldest##*/}" != "$STALL_RECORD" ]; then + if [ -n "$RELAUNCH_BRIEF" ] && [ -f "$BRIEF_PRIOR" ]; then + cp -p "$BRIEF_PRIOR" "$RELAUNCH_BRIEF" 2>/dev/null || true + fi journal_write "cancelled:record-resolved" "${CHECKPOINT_LINES[@]}" "$note_line" || true - echo "relaunch cancelled: stall record $STALL_RECORD handled or superseded (${stall_oldest##*/} is now oldest)" >&2 + if [ -z "$stall_oldest" ]; then + echo "relaunch cancelled: stall record resolved (inbox empty; instruction handled)" >&2 + else + echo "relaunch cancelled: stall record $STALL_RECORD handled or superseded (${stall_oldest##*/} is now oldest)" >&2 + fi exit 3 fi fi diff --git a/docs/architecture.md b/docs/architecture.md index 03f584f0527..6b7945db985 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -138,7 +138,8 @@ For an OMP worker the loaded extension delivers the doorbell through `sendMessag The generated OMP extension publishes `.omp-ready` only after the doorbell activates; activation or drain failure retires `.omp-doorbell-ready` and durably journals the reason in `.omp-doorbell-failed`, while `fm-spawn.sh` bounded-waits for readiness and `fm-send.sh` names the missing marker or failure journal when refusing native delivery. When the runtime downgrades `triggerTurn` to append-only, the extension re-drives the instruction through `sendUserMessage` only after its bounded grace expires without any turn opening; the re-drive is itself only a request, so a nonthrowing return is never a receipt - the entry re-parks for one more bounded proof window and leaves a durable `.unproven` marker when no turn opens, allowing the next ring to publish a fresh pending request (`.omp/extensions/lib/fm-task-inbox-doorbell.ts`). Consumed OMP delivery receipts retire as durable `.pending.acked` tombstones, so later rings report delivery without republishing or sending another doorbell; the requests directory is generation-scoped and reset with the task lifecycle. -Before either stale wake publishes, `bin/fm-stall-recovery.sh` runs a custody-checked bounded auto-recovery: it re-proves the record is still the oldest unhandled instruction, classifies the endpoint as live-non-turning or missing, requires a clean non-run crew-state, a clean worktree with no unlanded commits, and the durable `fm-` Treehouse lease, then re-proves the whole chain immediately before invoking `fm-control.sh relaunch`. +Before either stale wake publishes, `bin/fm-stall-recovery.sh` runs a custody-checked bounded auto-recovery: it re-proves the record is still the oldest unhandled instruction, classifies the endpoint as live-non-turning or missing, requires a clean non-run crew-state and the durable `fm-` Treehouse lease, then re-proves the whole chain immediately before invoking `fm-control.sh relaunch`. +Uncommitted changes and unpushed commits are deliberately preserved rather than treated as blockers: the relaunch inherits the same worktree, branch, and commits untouched. The verdict is `recovered` when the record was already handled, `deferred` when the worker is provably busy or the relaunch just published (the episode stays pending until the record is handled or the reset ladder re-escalates), and `escalate` for every unprovable or unsafe shape, which keeps the ordinary stale wake with the helper's reason appended. One automatic relaunch per stalled record is bounded by `state/.inbox/.recovery-attempts`; the bound resets only when the inbox empties. An OMP relaunch also retires the prior generation's `request.*` doorbell receipts after every refusal gate and immediately before the replacement launch, so a stale `.acked` tombstone cannot suppress the new incarnation's doorbell. diff --git a/tests/fm-stall-recovery.test.sh b/tests/fm-stall-recovery.test.sh index 4fce6203219..9871cd66411 100755 --- a/tests/fm-stall-recovery.test.sh +++ b/tests/fm-stall-recovery.test.sh @@ -962,10 +962,13 @@ SH esac pass "OMP extension serializes busy-state writes: turn-start busy lands after turn-end idle" } + # A record handled in the gap between the caller's pre-invocation check and # fm-control's in-lock re-check must still cancel the relaunch: the fake git # moves the record to handled/ during safe_checkpoint, so fm-control's own # stall-record proof sees it resolved and exits 3 before the agent is touched. +# The cancelled transaction must also leave the worker's instructions +# byte-exact - no relaunch means no progress-note append survives. test_in_lock_handled_record_cancels_relaunch() { local rec id record id=$(case_id inlock-ack) @@ -978,6 +981,7 @@ test_in_lock_handled_record_cancels_relaunch() { write_inbox "$HOME_DIR/state" "$id" 001 record="$HOME_DIR/state/$id.inbox/001.msg" missing_window "$CASE_DIR" "$id" + cp -p "$HOME_DIR/data/$id/brief.md" "$CASE_DIR/brief.orig" FM_FAKE_GIT_MOVE="$record" FM_STALL_RECOVERY_CONTROL_BIN="$CONTROL" \ run_recovery "$CASE_DIR" "$HOME_DIR" "$id" "$record" endpoint-unavailable @@ -986,6 +990,8 @@ test_in_lock_handled_record_cancels_relaunch() { assert_no_grep "new-window" "$CASE_DIR/fake/tmux.log" "the relaunch created a window for a resolved record" assert_grep "cancelled:record-resolved" "$HOME_DIR/state/$id.control-relaunch" "the journal did not record the in-lock cancellation" assert_present "$HOME_DIR/state/$id.inbox/handled/001.msg" "the handled record is not in handled/" + cmp -s "$CASE_DIR/brief.orig" "$HOME_DIR/data/$id/brief.md" \ + || fail "a cancelled relaunch left the worker's instructions modified" pass "in-lock handled record: fm-control cancels the relaunch before touching the agent" } From 1fdcf30415050ab3b59717e4ead0c41dacc3ea38 Mon Sep 17 00:00:00 2001 From: dnth Date: Thu, 24 Sep 2026 08:39:19 +0800 Subject: [PATCH 06/21] no-mistakes(review): Fixed invalid task-ID journal path traversal --- bin/fm-stall-recovery.sh | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/bin/fm-stall-recovery.sh b/bin/fm-stall-recovery.sh index 71119a2a9a9..0571fe68ed0 100755 --- a/bin/fm-stall-recovery.sh +++ b/bin/fm-stall-recovery.sh @@ -104,8 +104,7 @@ FM_STALL_RECOVERY_CONTROL_BIN="${FM_STALL_RECOVERY_CONTROL_BIN:-$SCRIPT_DIR/fm-c ID=${1:-} RECORD=${2:-} TRIGGER=${3:-} -JOURNAL="$STATE/$ID.stall-recovery" -STATUS_FILE="$STATE/$ID.status" +JOURNAL="$STATE/stall-recovery-invalid" journal() { # { @@ -127,6 +126,8 @@ status_note() { # # --- eligibility gates ------------------------------------------------------ case "$ID" in ''|*[!A-Za-z0-9._-]*) verdict escalate "invalid task id" ;; esac +JOURNAL="$STATE/$ID.stall-recovery" +STATUS_FILE="$STATE/$ID.status" META="$STATE/$ID.meta" [ -f "$META" ] && [ ! -L "$META" ] || verdict escalate "no task metadata" KIND=$(fm_meta_get "$META" kind) From 52a1d864a53562a40e492c2f80e3723c921dc1c7 Mon Sep 17 00:00:00 2001 From: dnth Date: Thu, 24 Sep 2026 08:43:52 +0800 Subject: [PATCH 07/21] no-mistakes(review): Made recovery attempt bounds fail closed and atomic --- bin/fm-stall-recovery.sh | 30 +++++++++++++++++++++++------- 1 file changed, 23 insertions(+), 7 deletions(-) diff --git a/bin/fm-stall-recovery.sh b/bin/fm-stall-recovery.sh index 0571fe68ed0..ac113cc379b 100755 --- a/bin/fm-stall-recovery.sh +++ b/bin/fm-stall-recovery.sh @@ -252,18 +252,34 @@ oldest=$(fm_task_inbox_oldest_unhandled "$STATE" "$ID" 2>/dev/null || true) # cannot both pass the bound. The bound is a fixed invariant - no override. attempts_file="$dir/.recovery-attempts" attempts_record='' attempts_count=0 -IFS=$(printf '\t') read -r attempts_record attempts_count </dev/null || true) -EOF -[ "$attempts_record" = "${RECORD##*/}" ] || attempts_count=0 -case "$attempts_count" in ''|*[!0-9]*) attempts_count=0 ;; esac +if [ -e "$attempts_file" ] || [ -L "$attempts_file" ]; then + [ -f "$attempts_file" ] && [ ! -L "$attempts_file" ] \ + || verdict escalate "recovery-attempt marker is not a regular file" + attempts_content=$(<"$attempts_file") \ + || verdict escalate "cannot read the recovery-attempt bound at $attempts_file" + IFS=$(printf '\t') read -r attempts_record attempts_count attempts_extra \ + <<< "$attempts_content" + [ "$attempts_record" = "${RECORD##*/}" ] \ + && case "$attempts_count" in ''|*[!0-9]*) false ;; *) true ;; esac \ + && [ -z "$attempts_extra" ] \ + && [ "$attempts_content" = "${attempts_record}$(printf '\t')${attempts_count}" ] \ + || verdict escalate "malformed recovery-attempt marker at $attempts_file" +fi [ "$attempts_count" -lt 1 ] \ || verdict escalate "automatic recovery already attempted for ${RECORD##*/}; escalating per bounded-retry policy" unhandled=$(cd "$dir" 2>/dev/null && printf '%s ' *.msg 2>/dev/null || true) note="Stall auto-recovery ($TRIGGER, $PATH_KIND): the previous worker stopped acting on doorbells while instruction(s) ${unhandled:-${RECORD##*/}} stayed unhandled. The worktree, branch, and commits are exactly as that worker left them; nothing was discarded. Read and act on the inbox first." -printf '%s\t%s\n' "${RECORD##*/}" "$((attempts_count + 1))" > "$attempts_file" 2>/dev/null \ - || verdict escalate "cannot persist the recovery-attempt bound at $attempts_file" +attempts_tmp=$(mktemp "$dir/.recovery-attempts.XXXXXX" 2>/dev/null) \ + || verdict escalate "cannot allocate the recovery-attempt bound at $attempts_file" +if ! printf '%s\t%s\n' "${RECORD##*/}" "$((attempts_count + 1))" > "$attempts_tmp"; then + rm -f "$attempts_tmp" + verdict escalate "cannot persist the recovery-attempt bound at $attempts_file" +fi +if ! mv -f "$attempts_tmp" "$attempts_file" 2>/dev/null; then + rm -f "$attempts_tmp" + verdict escalate "cannot publish the recovery-attempt bound at $attempts_file" +fi # fm-control must run as a direct child so --lock-preheld's owner check # ($PPID == the lock's recorded owner pid) binds to THIS process; a command From f2e11186c53600ad7c295ab65963ecabb00e0468 Mon Sep 17 00:00:00 2001 From: dnth Date: Thu, 24 Sep 2026 08:48:03 +0800 Subject: [PATCH 08/21] no-mistakes(review): Propagated overrides and failed closed on restore errors --- bin/fm-control.sh | 6 +++++- bin/fm-watch.sh | 3 ++- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/bin/fm-control.sh b/bin/fm-control.sh index 6de660af415..792ff549585 100755 --- a/bin/fm-control.sh +++ b/bin/fm-control.sh @@ -882,7 +882,11 @@ do_relaunch() { stall_oldest=$(fm_task_inbox_oldest_unhandled "$STATE" "$ID" 2>/dev/null || true) if [ -z "$stall_oldest" ] || [ "${stall_oldest##*/}" != "$STALL_RECORD" ]; then if [ -n "$RELAUNCH_BRIEF" ] && [ -f "$BRIEF_PRIOR" ]; then - cp -p "$BRIEF_PRIOR" "$RELAUNCH_BRIEF" 2>/dev/null || true + if ! cp -p "$BRIEF_PRIOR" "$RELAUNCH_BRIEF" 2>/dev/null; then + RELAUNCH_ACTIVE=0 + journal_write "failed:record-resolved" "rollback=instructions-restore-failed" "${CHECKPOINT_LINES[@]}" "$note_line" || true + die "relaunch cancelled for resolved stall record, but restoring the original instructions failed" + fi fi journal_write "cancelled:record-resolved" "${CHECKPOINT_LINES[@]}" "$note_line" || true if [ -z "$stall_oldest" ]; then diff --git a/bin/fm-watch.sh b/bin/fm-watch.sh index 583677d73b8..55ce232bc35 100755 --- a/bin/fm-watch.sh +++ b/bin/fm-watch.sh @@ -373,7 +373,8 @@ inbox_steer_attempt_recovery() { # local task=$2 record=$3 trigger=$4 out rc=0 INBOX_RECOVERY_DETAIL= [ -x "$FM_STALL_RECOVERY_BIN" ] || { INBOX_RECOVERY_DETAIL="recovery helper missing"; return 1; } - out=$(FM_HOME="$FM_HOME" "$FM_STALL_RECOVERY_BIN" "$task" "$record" "$trigger" 2>/dev/null) || rc=$? + out=$(FM_HOME="$FM_HOME" FM_STATE_OVERRIDE="$STATE" FM_DATA_OVERRIDE="$DATA" \ + "$FM_STALL_RECOVERY_BIN" "$task" "$record" "$trigger" 2>/dev/null) || rc=$? case "$out" in verdict=recovered*|verdict=deferred*) INBOX_RECOVERY_DETAIL=${out#*detail=} From 6d4eaccb3fc3d59dc572b158aadb4509c34c5fdd Mon Sep 17 00:00:00 2001 From: dnth Date: Thu, 24 Sep 2026 08:57:17 +0800 Subject: [PATCH 09/21] no-mistakes(document): Updated inbox recovery documentation and comments --- bin/fm-task-inbox-lib.sh | 11 ++++++----- bin/fm-watch.sh | 4 ++++ docs/configuration.md | 2 +- 3 files changed, 11 insertions(+), 6 deletions(-) diff --git a/bin/fm-task-inbox-lib.sh b/bin/fm-task-inbox-lib.sh index 23039b8c215..852f10353e7 100644 --- a/bin/fm-task-inbox-lib.sh +++ b/bin/fm-task-inbox-lib.sh @@ -65,11 +65,12 @@ # Re-ring ladder (fm_task_inbox_due_action): an unhandled message older than # FM_TASK_INBOX_GRACE_SECS is due one delivery attempt per grace period; an # attempt may ring or be skipped to protect proven pending composer text. After -# FM_TASK_INBOX_RING_MAX attempts without an acknowledgement it escalates. The -# caller owns the busy and recovery-grade endpoint checks: a busy pane waits, -# while a positively dead or missing endpoint skips delivery and the ladder and -# escalates directly. This library owns only the schedule and escalation -# marker. If attempt bookkeeping cannot be persisted while the record +# FM_TASK_INBOX_RING_MAX attempts without an acknowledgement it becomes due +# for the caller's custody-checked stall-recovery helper before any stale wake +# is published. The caller owns the busy and recovery-grade endpoint checks: +# a busy pane waits, while a positively dead or missing endpoint skips delivery +# and the ladder and enters that same helper. This library owns only the +# schedule and escalation marker. If attempt bookkeeping cannot be persisted while the record # remains unhandled, the caller surfaces that failure instead of retrying # silently; a concurrently removed inbox is a quiet no-op. Escalation # deliberately queues the wake before writing the diff --git a/bin/fm-watch.sh b/bin/fm-watch.sh index 55ce232bc35..7710d8023ff 100755 --- a/bin/fm-watch.sh +++ b/bin/fm-watch.sh @@ -51,6 +51,10 @@ # demand-deep-inspection marker, for human inspection # only - never an automatic interrupt, signal, or restart # of the worker or its tool process. +# The separate steering-inbox path invokes +# fm-stall-recovery.sh before publishing its stale wake; +# that helper owns its custody-checked, bounded relaunch +# exception. # An idle secondmate that is neither paused nor captain-held # is also absorbed while its home watcher beacon is fresh # within the wedge threshold; missing, stale, future-dated, diff --git a/docs/configuration.md b/docs/configuration.md index e979a94452d..d068ebd22b3 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -972,7 +972,7 @@ FM_COMPOSER_IDLE_RE= # optional override for the fleet-wide empty-composer pl FM_COMPOSER_GHOST_LUMA_MAX=128 # fleet-wide: max perceived luminance (0.299R+0.587G+0.114B, 0-255) for a TRUECOLOR foreground to count as de-emphasised ghost/placeholder text and be stripped; dim/faint (SGR 2) is stripped regardless. Assumes a dark terminal theme (bin/fm-composer-lib.sh's fm_composer_strip_ghost, shared by the tmux and herdr composer readers) GROK_HOME= # optional Grok config home for firstmate's global grok turn-end hook; defaults to ~/.grok FM_TASK_INBOX_GRACE_SECS=90 # seconds before an unhandled local task record is eligible for a watcher re-ring, and between later attempts -FM_TASK_INBOX_RING_MAX=3 # watcher doorbell attempts before one ordinary stale wake escalates the unhandled record +FM_TASK_INBOX_RING_MAX=3 # watcher doorbell attempts before custody-checked stall recovery runs; an ordinary stale wake follows only when recovery defers or escalates FM_TASK_INBOX_LOCK_WAIT_SECS=5 # bounded wait for inbox sequence allocation and fm-send's final metadata revalidation; invalid values use 5 FM_SEND_RETRIES=3 # typed-plane fm-send Enter-retry attempts after typing the line once FM_SEND_SLEEP=0.4 # seconds between typed-plane fm-send submit checks From 4ed041a7c08eacbce22922aa7ae46d605f7743d6 Mon Sep 17 00:00:00 2001 From: dnth Date: Thu, 24 Sep 2026 10:08:06 +0800 Subject: [PATCH 10/21] no-mistakes(lint): Suppress intentional background-lock subshell SC2031 warnings --- tests/fm-stall-recovery.test.sh | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/fm-stall-recovery.test.sh b/tests/fm-stall-recovery.test.sh index 9871cd66411..9e6907f9a31 100755 --- a/tests/fm-stall-recovery.test.sh +++ b/tests/fm-stall-recovery.test.sh @@ -868,6 +868,7 @@ test_refused_relaunch_preserves_receipts() { # A lifecycle lock held by a live process means another lifecycle action is in # flight: recovery defers instead of racing it, and the watcher re-evaluates on # the next cycle. +# shellcheck disable=SC2031 # This test intentionally coordinates with a background lock-holder subshell. test_held_lifecycle_lock_defers() { local rec id record lockdir holder n id=$(case_id lockheld) From 998e56ac2975cde5489df260b88485ade3288907 Mon Sep 17 00:00:00 2001 From: dnth Date: Thu, 24 Sep 2026 11:49:12 +0800 Subject: [PATCH 11/21] fix: defer stall relaunch when worker goes busy; reset attempt bound per record Astra re-verification of 0607ba7f (data/fm-159-astra-review/report.md): F1 (P1): a worker that published a valid busy event during safe_checkpoint was still interrupted - the final in-lock gate re-proved only the record's identity, then do_exit sent Escape + /exit to a now-productive worker. The supervised --stall-record path now fails closed on any busy verdict that is not a proven idle: busy returns exit 4, which do_relaunch maps to a clean cancellation (byte-exact instruction restore, cancelled:worker-busy journal phase) that fm-stall-recovery reports as a deferred episode. Manual relaunch and the plain exit verb keep interrupt-first semantics. F2 (P2): a well-formed .recovery-attempts marker naming a previously handled record was rejected as malformed, denying the next queued record its first attempt whenever the watcher never observed an empty inbox. Marker structure is now validated separately from record identity; a marker naming a different record starts the current oldest record's count at zero and is replaced atomically under the lifecycle lock. Corrupt markers still escalate, and the one-attempt bound per record is unchanged. Regressions: busy-during-checkpoint defer (no interrupt/exit/relaunch, byte-exact brief restore), next-record fresh attempt with no intervening empty-inbox observation, and corrupt-marker fail-closed escalation. --- bin/fm-control.sh | 80 +++++++++++++++++------ bin/fm-stall-recovery.sh | 17 ++++- tests/fm-stall-recovery.test.sh | 112 ++++++++++++++++++++++++++++++-- 3 files changed, 181 insertions(+), 28 deletions(-) diff --git a/bin/fm-control.sh b/bin/fm-control.sh index 792ff549585..cd4ce38aaaa 100755 --- a/bin/fm-control.sh +++ b/bin/fm-control.sh @@ -56,7 +56,10 @@ # lock, immediately before the agent is touched, fm-control # re-proves the named inbox record is still the oldest unhandled # instruction and cancels with exit 3 when it was handled or -# superseded in flight. It requires --lock-preheld. +# superseded in flight, and cancels with exit 4 when the worker +# went provably busy during the checkpoint - a productive worker +# is deferred to, never interrupted, and an unproven busy verdict +# fails closed. It requires --lock-preheld. # Records a durable checkpoint and that note, exits the old agent, # then delegates the launch to its single owner, # bin/fm-spawn.sh --relaunch. A failure before publication keeps @@ -508,9 +511,21 @@ do_exit() { missing) die "task $ID's recorded endpoint is gone, so there is no agent to stop; reconcile the task before any further control action" ;; *) die "task $ID's endpoint reads '$state' rather than a positively classified state; refusing to send a lifecycle command into an unattributed endpoint" ;; esac - # A busy agent is interrupted first before the exit command is submitted. + # A busy agent is interrupted first before the exit command is submitted - + # except on the supervised --stall-record path, where busy means the stalled + # worker started acting on the instruction between the caller's custody + # proof and this last gate. Interrupting a now-productive worker is exactly + # the harm supervised recovery exists to avoid, so busy returns 4 for the + # caller to map to a deferred episode (with the instructions restored + # byte-exact), and any verdict that is not a proven idle fails closed + # rather than exiting an agent whose state cannot be proven. Manual + # relaunch and the plain exit verb keep the interrupt-first semantics. case "$(busy_verdict)" in busy*) + if [ -n "$STALL_RECORD" ]; then + printf 'busy-deferred' + return 4 + fi cancel=$(deliver_interrupt) || return $? state=$(agent_state) case "$state" in @@ -524,6 +539,11 @@ do_exit() { *) die "task $ID's endpoint reads '$state' after interrupt delivery rather than a positively classified state; exit cannot prove whether the agent stopped" ;; esac ;; + idle*) ;; + *) + [ -z "$STALL_RECORD" ] \ + || die "task $ID's busy verdict is not a proven idle, so supervised stall recovery cannot prove the worker is still non-turning; refusing to exit an agent whose state cannot be proven" + ;; esac cmd=$(fm_control_exit_command "$HARNESS") # The submit verdict is NOT the postcondition here: a successful exit command @@ -573,6 +593,26 @@ TARGET_HARNESS=$HARNESS TARGET_MODEL= TARGET_EFFORT= +# stall_relaunch_cancel: cancel the supervised --stall-record relaunch cleanly +# after the checkpoint/note work has run. Restores the worker's instructions +# byte-exact (record_note appended the progress note), journals the named +# cancellation phase, and exits with for the supervised caller. Reads +# do_relaunch's note_line through bash's dynamic scope; only ever called from +# inside it. A restore failure is a hard error, not a clean cancel. +stall_relaunch_cancel() { # + local phase=$1 message=$2 code=$3 + if [ -n "$RELAUNCH_BRIEF" ] && [ -f "$BRIEF_PRIOR" ]; then + if ! cp -p "$BRIEF_PRIOR" "$RELAUNCH_BRIEF" 2>/dev/null; then + RELAUNCH_ACTIVE=0 + journal_write "failed:$phase" "rollback=instructions-restore-failed" "${CHECKPOINT_LINES[@]}" "$note_line" || true + die "relaunch cancelled ($phase), but restoring the original instructions failed" + fi + fi + journal_write "cancelled:$phase" "${CHECKPOINT_LINES[@]}" "$note_line" || true + echo "relaunch cancelled: $message" >&2 + exit "$code" +} + journal_write() { # [extra-line]... local phase=$1 shift @@ -833,7 +873,7 @@ record_note() { } do_relaunch() { - local exit_result state note_line stall_oldest + local exit_result exit_rc state note_line stall_oldest local -a spawn_args require_state_verified_backend relaunch @@ -880,21 +920,10 @@ do_relaunch() { # untouched when no relaunch happened. if [ -n "$STALL_RECORD" ]; then stall_oldest=$(fm_task_inbox_oldest_unhandled "$STATE" "$ID" 2>/dev/null || true) - if [ -z "$stall_oldest" ] || [ "${stall_oldest##*/}" != "$STALL_RECORD" ]; then - if [ -n "$RELAUNCH_BRIEF" ] && [ -f "$BRIEF_PRIOR" ]; then - if ! cp -p "$BRIEF_PRIOR" "$RELAUNCH_BRIEF" 2>/dev/null; then - RELAUNCH_ACTIVE=0 - journal_write "failed:record-resolved" "rollback=instructions-restore-failed" "${CHECKPOINT_LINES[@]}" "$note_line" || true - die "relaunch cancelled for resolved stall record, but restoring the original instructions failed" - fi - fi - journal_write "cancelled:record-resolved" "${CHECKPOINT_LINES[@]}" "$note_line" || true - if [ -z "$stall_oldest" ]; then - echo "relaunch cancelled: stall record resolved (inbox empty; instruction handled)" >&2 - else - echo "relaunch cancelled: stall record $STALL_RECORD handled or superseded (${stall_oldest##*/} is now oldest)" >&2 - fi - exit 3 + if [ -z "$stall_oldest" ]; then + stall_relaunch_cancel record-resolved "stall record resolved (inbox empty; instruction handled)" 3 + elif [ "${stall_oldest##*/}" != "$STALL_RECORD" ]; then + stall_relaunch_cancel record-resolved "stall record $STALL_RECORD handled or superseded (${stall_oldest##*/} is now oldest)" 3 fi fi @@ -909,7 +938,20 @@ do_relaunch() { retire_busy_incarnation exit_result=already-stopped else - exit_result=$(do_exit) + exit_rc=0 + exit_result=$(do_exit) || exit_rc=$? + case "$exit_rc" in + 0) ;; + 4) + # The worker went busy between the custody proof and the exit gate: + # it is acting on the instruction now, so the relaunch defers rather + # than interrupting a productive worker. Exit 4 is the dedicated + # "worker busy; episode stays pending" code the supervised caller maps + # to deferred. The instructions are restored byte-exact first. + stall_relaunch_cancel worker-busy "stall record $STALL_RECORD: worker went busy during the relaunch; deferring rather than interrupting a productive worker" 4 + ;; + *) exit "$exit_rc" ;; + esac fi journal_write exited "${CHECKPOINT_LINES[@]}" "$note_line" "exit_result=$exit_result" diff --git a/bin/fm-stall-recovery.sh b/bin/fm-stall-recovery.sh index ac113cc379b..804e436a98d 100755 --- a/bin/fm-stall-recovery.sh +++ b/bin/fm-stall-recovery.sh @@ -43,7 +43,9 @@ # exists to keep, and the relaunch inherits the same worktree, branch, # and commits untouched. # - One automatic relaunch per stalled instruction: the per-record attempt -# marker under the inbox bounds retries, and an emptied inbox resets it. +# marker under the inbox bounds retries; an emptied inbox resets it, and +# a well-formed marker naming a record that was since handled starts the +# new oldest record's own count at zero. # - The durable fm- worktree lease, same-worktree/branch/commits # preservation, and the no-shared-daemon boundary are enforced by # bin/fm-control.sh relaunch itself; this script never moves inbox @@ -259,11 +261,18 @@ if [ -e "$attempts_file" ] || [ -L "$attempts_file" ]; then || verdict escalate "cannot read the recovery-attempt bound at $attempts_file" IFS=$(printf '\t') read -r attempts_record attempts_count attempts_extra \ <<< "$attempts_content" - [ "$attempts_record" = "${RECORD##*/}" ] \ + # Structure is validated separately from record identity: a well-formed + # marker naming a different record is the spent bound of an instruction that + # was since handled (the watcher only clears it on an observed empty inbox, + # which a back-to-back queue never produces), so the current oldest record + # starts its own count at zero and the marker is replaced atomically below. + # Only a corrupt marker or a spent bound on THIS record escalates. + case "$attempts_record" in ''|*[!A-Za-z0-9._-]*) false ;; *) true ;; esac \ && case "$attempts_count" in ''|*[!0-9]*) false ;; *) true ;; esac \ && [ -z "$attempts_extra" ] \ && [ "$attempts_content" = "${attempts_record}$(printf '\t')${attempts_count}" ] \ || verdict escalate "malformed recovery-attempt marker at $attempts_file" + [ "$attempts_record" = "${RECORD##*/}" ] || attempts_count=0 fi [ "$attempts_count" -lt 1 ] \ || verdict escalate "automatic recovery already attempted for ${RECORD##*/}; escalating per bounded-retry policy" @@ -287,7 +296,8 @@ fi # hands fm-control the record basename so it re-proves the instruction is # still the oldest unhandled record inside the lock, immediately before the # agent is touched - the check above cannot cover the gap to that point. -# Exit 3 is fm-control's "record resolved; nothing to do" code. +# Exit 3 is fm-control's "record resolved; nothing to do" code and exit 4 its +# "worker went busy during the relaunch; defer" code. FM_CONFIG_OVERRIDE=${FM_CONFIG_OVERRIDE:-$FM_HOME/config} control_out_file=$(mktemp "$STATE/.stall-recovery-control-out.XXXXXX" 2>/dev/null) \ || verdict escalate "cannot allocate the fm-control output capture" @@ -305,6 +315,7 @@ rm -f "$control_out_file" case "$control_rc" in 0) ;; 3) verdict recovered "record ${RECORD##*/} resolved inside the lifecycle lock before the relaunch; instruction handled" ;; + 4) verdict deferred "worker went busy inside the lifecycle lock before the relaunch; it is acting on ${RECORD##*/}, so the episode stays pending until the record is handled or the ladder re-escalates" ;; *) verdict escalate "fm-control relaunch refused or failed: $(printf '%s' "$control_out" | tail -1)" ;; esac diff --git a/tests/fm-stall-recovery.test.sh b/tests/fm-stall-recovery.test.sh index 9e6907f9a31..c0dee91eaac 100755 --- a/tests/fm-stall-recovery.test.sh +++ b/tests/fm-stall-recovery.test.sh @@ -329,19 +329,25 @@ exit "${FM_FAKE_CONTROL_RC:-0}" SH chmod +x "$fb/fm-control.sh" - # git forwards to the real binary, except when FM_FAKE_GIT_MOVE names an - # inbox record: the first `git -C status --porcelain` inside - # fm-control's safe_checkpoint moves that record to handled/, simulating a + # git forwards to the real binary, with two test hooks on the first + # `git -C status --porcelain` inside fm-control's safe_checkpoint: + # FM_FAKE_GIT_MOVE names an inbox record moved to handled/, simulating a # worker that acknowledges the instruction in the gap between the caller's - # pre-invocation check and fm-control's in-lock re-check. + # pre-invocation check and fm-control's in-lock re-check; FM_FAKE_GIT_BUSY + # names a busy-state file overwritten with a valid busy record, simulating + # a worker that starts a turn on the instruction during the checkpoint. cat > "$fb/git" <<'SH' #!/usr/bin/env bash set -u -if [ -n "${FM_FAKE_GIT_MOVE:-}" ] && [ "${1:-}" = "-C" ] && [ "${3:-}" = "status" ]; then - if [ -f "$FM_FAKE_GIT_MOVE" ]; then +if [ "${1:-}" = "-C" ] && [ "${3:-}" = "status" ]; then + if [ -n "${FM_FAKE_GIT_MOVE:-}" ] && [ -f "$FM_FAKE_GIT_MOVE" ]; then mkdir -p "${FM_FAKE_GIT_MOVE%/*}/handled" mv "$FM_FAKE_GIT_MOVE" "${FM_FAKE_GIT_MOVE%/*}/handled/" fi + if [ -n "${FM_FAKE_GIT_BUSY:-}" ]; then + printf 'v1 gen=gentest seq=3 state=busy source=omp-ext event=turn-start ts=1\n' \ + > "$FM_FAKE_GIT_BUSY" + fi fi exec /usr/bin/git "$@" SH @@ -996,6 +1002,97 @@ test_in_lock_handled_record_cancels_relaunch() { pass "in-lock handled record: fm-control cancels the relaunch before touching the agent" } +# A worker that goes busy DURING the checkpoint - publishing a valid +# turn-start busy record while its instruction stays unhandled - must never +# be interrupted: the supervised relaunch cancels with the deferred verdict, +# sends no interrupt or exit keys, creates no replacement window, and leaves +# the worker's instructions byte-exact. The fake git publishes the busy +# record inside fm-control's safe_checkpoint, after every earlier custody +# proof already saw an idle worker. +test_busy_during_checkpoint_defers() { + local rec id record + id=$(case_id checkpoint-busy) + rec=$(make_case checkpoint-busy "$id" pool) + read_case "$rec" + write_pool_state "$CASE_DIR" "$WT_DIR" "fm-$id" + write_slot_marker "$SLOT_DIR" "$id" "$HOME_DIR" + write_meta "$HOME_DIR/state/$id.meta" "$id" "$WT_DIR" "$PROJ_DIR" + create_prior_artifacts "$HOME_DIR/state" "$id" + write_inbox "$HOME_DIR/state" "$id" 001 + record="$HOME_DIR/state/$id.inbox/001.msg" + write_busy "$HOME_DIR/state" "$id" idle + live_window "$CASE_DIR" "$id" bun + cp -p "$HOME_DIR/data/$id/brief.md" "$CASE_DIR/brief.orig" + + FM_FAKE_GIT_BUSY="$HOME_DIR/state/$id.busy-state" \ + FM_STALL_RECOVERY_CONTROL_BIN="$CONTROL" \ + run_recovery "$CASE_DIR" "$HOME_DIR" "$id" "$record" ladder-exhausted + expect_code 0 "$RECOVERY_STATUS" "busy-during-checkpoint recovery should exit 0; got: $RECOVERY_OUT" + assert_contains "$RECOVERY_OUT" "verdict=deferred" "a worker that went busy during the checkpoint did not defer" + assert_no_grep "Escape" "$CASE_DIR/fake/tmux.log" "an interrupt key was sent to a worker that went busy during the checkpoint" + assert_no_grep "/exit" "$CASE_DIR/fake/tmux.log" "an exit command was sent to a worker that went busy during the checkpoint" + assert_no_grep "new-window" "$CASE_DIR/fake/tmux.log" "the relaunch created a window for a worker that went busy" + assert_grep "cancelled:worker-busy" "$HOME_DIR/state/$id.control-relaunch" "the journal did not record the busy-worker cancellation" + assert_present "$record" "the unhandled instruction record was moved or deleted" + assert_grep "001.msg" "$HOME_DIR/state/$id.inbox/.recovery-attempts" "the deferred episode did not record its attempt bound" + cmp -s "$CASE_DIR/brief.orig" "$HOME_DIR/data/$id/brief.md" \ + || fail "a busy-cancelled relaunch left the worker's instructions modified" + pass "busy during checkpoint: recovery defers without interrupt, exit, or relaunch" +} + +# A well-formed attempt marker naming a record that was since handled must +# not deny the next queued record its own first attempt: structure is +# validated separately from identity, so 002.msg recovers with a fresh count +# even though the watcher never observed an empty inbox between the two. +test_next_record_gets_own_attempt() { + local rec id record + id=$(case_id next-record) + rec=$(make_case next-record "$id" pool) + read_case "$rec" + write_pool_state "$CASE_DIR" "$WT_DIR" "fm-$id" + write_slot_marker "$SLOT_DIR" "$id" "$HOME_DIR" + write_meta "$HOME_DIR/state/$id.meta" "$id" "$WT_DIR" "$PROJ_DIR" + create_prior_artifacts "$HOME_DIR/state" "$id" + write_inbox "$HOME_DIR/state" "$id" 001 + write_inbox "$HOME_DIR/state" "$id" 002 + printf '001.msg\t1\n' > "$HOME_DIR/state/$id.inbox/.recovery-attempts" + mv "$HOME_DIR/state/$id.inbox/001.msg" "$HOME_DIR/state/$id.inbox/handled/" + record="$HOME_DIR/state/$id.inbox/002.msg" + missing_window "$CASE_DIR" "$id" + + FM_STALL_RECOVERY_CONTROL_BIN="$CONTROL" \ + run_recovery "$CASE_DIR" "$HOME_DIR" "$id" "$record" endpoint-unavailable + expect_code 0 "$RECOVERY_STATUS" "next-record recovery should exit 0; got: $RECOVERY_OUT" + assert_contains "$RECOVERY_OUT" "verdict=deferred" "the next queued record did not get its own recovery attempt" + assert_grep "phase=complete" "$HOME_DIR/state/$id.control-relaunch" "the relaunch transaction did not complete for the next record" + assert_grep "002.msg" "$HOME_DIR/state/$id.inbox/.recovery-attempts" "the new record's attempt bound was not recorded" + assert_no_grep "001.msg" "$HOME_DIR/state/$id.inbox/.recovery-attempts" "the prior record's spent marker survived the atomic replace" + pass "next record: a handled record's spent marker does not consume the next record's attempt" +} + +# A corrupt attempt marker still fails closed: structure validation is +# unchanged, so an unparseable marker escalates without any lifecycle action. +test_malformed_attempt_marker_escalates() { + local rec id record + id=$(case_id bad-marker) + rec=$(make_case bad-marker "$id" pool) + read_case "$rec" + write_pool_state "$CASE_DIR" "$WT_DIR" "fm-$id" + write_slot_marker "$SLOT_DIR" "$id" "$HOME_DIR" + write_meta "$HOME_DIR/state/$id.meta" "$id" "$WT_DIR" "$PROJ_DIR" + create_prior_artifacts "$HOME_DIR/state" "$id" + write_inbox "$HOME_DIR/state" "$id" 001 + record="$HOME_DIR/state/$id.inbox/001.msg" + printf 'garbage-no-tab\n' > "$HOME_DIR/state/$id.inbox/.recovery-attempts" + missing_window "$CASE_DIR" "$id" + + run_recovery "$CASE_DIR" "$HOME_DIR" "$id" "$record" endpoint-unavailable + assert_contains "$RECOVERY_OUT" "verdict=escalate" "a corrupt attempt marker did not escalate" + assert_contains "$RECOVERY_OUT" "malformed recovery-attempt marker" "the escalation did not name the corrupt marker" + assert_absent "$CASE_DIR/control.log" "the lifecycle verb ran against a corrupt attempt marker" + pass "corrupt attempt marker: recovery escalates without a lifecycle action" +} + # --- run --------------------------------------------------------------------- test_missing_endpoint_recovers_via_control @@ -1015,5 +1112,8 @@ test_refused_relaunch_preserves_receipts test_held_lifecycle_lock_defers test_in_lock_handled_record_cancels_relaunch test_omp_ext_serializes_busy_events +test_busy_during_checkpoint_defers +test_next_record_gets_own_attempt +test_malformed_attempt_marker_escalates pass "all stall-recovery tests" From fbbf49772b078db3fc1f4ffedb66d3fae8f1c6ab Mon Sep 17 00:00:00 2001 From: dnth Date: Thu, 24 Sep 2026 12:13:22 +0800 Subject: [PATCH 12/21] no-mistakes(review): Reject recovery markers containing trailing extra lines --- bin/fm-stall-recovery.sh | 6 +++++- tests/fm-stall-recovery.test.sh | 6 +++--- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/bin/fm-stall-recovery.sh b/bin/fm-stall-recovery.sh index 804e436a98d..34d473786f9 100755 --- a/bin/fm-stall-recovery.sh +++ b/bin/fm-stall-recovery.sh @@ -257,7 +257,11 @@ attempts_record='' attempts_count=0 if [ -e "$attempts_file" ] || [ -L "$attempts_file" ]; then [ -f "$attempts_file" ] && [ ! -L "$attempts_file" ] \ || verdict escalate "recovery-attempt marker is not a regular file" - attempts_content=$(<"$attempts_file") \ + attempts_line_count=$(wc -l < "$attempts_file") \ + || verdict escalate "cannot read the recovery-attempt bound at $attempts_file" + [ "$attempts_line_count" -eq 1 ] \ + || verdict escalate "malformed recovery-attempt marker at $attempts_file" + IFS= read -r attempts_content < "$attempts_file" \ || verdict escalate "cannot read the recovery-attempt bound at $attempts_file" IFS=$(printf '\t') read -r attempts_record attempts_count attempts_extra \ <<< "$attempts_content" diff --git a/tests/fm-stall-recovery.test.sh b/tests/fm-stall-recovery.test.sh index c0dee91eaac..8c5e63371ec 100755 --- a/tests/fm-stall-recovery.test.sh +++ b/tests/fm-stall-recovery.test.sh @@ -1070,8 +1070,8 @@ test_next_record_gets_own_attempt() { pass "next record: a handled record's spent marker does not consume the next record's attempt" } -# A corrupt attempt marker still fails closed: structure validation is -# unchanged, so an unparseable marker escalates without any lifecycle action. +# A corrupt attempt marker with an extra trailing line still fails closed, +# without any lifecycle action. test_malformed_attempt_marker_escalates() { local rec id record id=$(case_id bad-marker) @@ -1083,7 +1083,7 @@ test_malformed_attempt_marker_escalates() { create_prior_artifacts "$HOME_DIR/state" "$id" write_inbox "$HOME_DIR/state" "$id" 001 record="$HOME_DIR/state/$id.inbox/001.msg" - printf 'garbage-no-tab\n' > "$HOME_DIR/state/$id.inbox/.recovery-attempts" + printf '001.msg\t0\n\n' > "$HOME_DIR/state/$id.inbox/.recovery-attempts" missing_window "$CASE_DIR" "$id" run_recovery "$CASE_DIR" "$HOME_DIR" "$id" "$record" endpoint-unavailable From d5c10425194776214ac7dc28d1ebc83c73a97c6d Mon Sep 17 00:00:00 2001 From: dnth Date: Thu, 24 Sep 2026 12:19:24 +0800 Subject: [PATCH 13/21] no-mistakes(document): Document supervised stall-recovery relaunch behavior --- docs/agent-control.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/docs/agent-control.md b/docs/agent-control.md index 05f5e7f67a6..65c2e8ee538 100644 --- a/docs/agent-control.md +++ b/docs/agent-control.md @@ -78,6 +78,8 @@ It is not deterministic across the verified adapters: codex and grok resume only Switching harness is therefore one ordinary relaunch rather than a separate mechanism. +The watcher-triggered stall-recovery path uses the same transactional relaunch under its lifecycle lock, but its final checkpoint defers instead of interrupting when the worker has become provably busy; the custody and verdict contract lives in [`architecture.md`](architecture.md#event-driven-supervision). + ### Failure and rollback - A refusal **before** the agent is stopped leaves the durable record and the instructions byte-identical. From f0b796f972be37862acf28e112df04d5ac524f6b Mon Sep 17 00:00:00 2001 From: dnth Date: Thu, 24 Sep 2026 14:01:19 +0800 Subject: [PATCH 14/21] fix: fail closed on unproven custody and non-canonical attempt markers Astra re-verification of 40421801 (data/fm-159-astra-reverify/report.md): R1 (P2): the marker line-count check accepted a canonical line followed by an unterminated suffix (001.msg1CORRUPT) because wc -l counts only newlines and read parses only the first line. The marker is now validated as exactly one canonical newline-terminated record by byte count, so any trailing byte or missing terminator escalates; the separate identity reset and atomic replacement are unchanged. R2 (P2): an unproven busy verdict at the final do_exit gate died generically, routing into the stopping rollback where the appended progress note survived even though no lifecycle action occurred. The supervised --stall-record path now returns exit 5 for unproven custody before any transport, do_relaunch routes it through stall_relaunch_cancel (byte-exact instruction restore, cancelled:custody-unproven journal phase), and fm-stall-recovery maps it to escalate - never deferral or instruction-handled. D1 (P3): docs/architecture.md now describes the per-record attempt bound, new-record replacement, and empty-inbox cleanup separately. D2 (P3): docs/configuration.md now states deferred/recovered outcomes suppress the stale wake while escalation or helper failure retains it. Regressions: unterminated marker suffix escalates; unproven custody during checkpoint escalates with no transport and byte-exact instructions. --- bin/fm-control.sh | 29 ++++++++++--- bin/fm-stall-recovery.sh | 18 +++++--- docs/architecture.md | 2 +- docs/configuration.md | 2 +- tests/fm-stall-recovery.test.sh | 73 ++++++++++++++++++++++++++++++++- 5 files changed, 108 insertions(+), 16 deletions(-) diff --git a/bin/fm-control.sh b/bin/fm-control.sh index cd4ce38aaaa..0b01e4f799f 100755 --- a/bin/fm-control.sh +++ b/bin/fm-control.sh @@ -56,10 +56,11 @@ # lock, immediately before the agent is touched, fm-control # re-proves the named inbox record is still the oldest unhandled # instruction and cancels with exit 3 when it was handled or -# superseded in flight, and cancels with exit 4 when the worker -# went provably busy during the checkpoint - a productive worker -# is deferred to, never interrupted, and an unproven busy verdict -# fails closed. It requires --lock-preheld. +# superseded in flight, cancels with exit 4 when the worker went +# provably busy during the checkpoint - a productive worker is +# deferred to, never interrupted - and cancels with exit 5 when +# custody can no longer be proven, which the supervised caller +# escalates. It requires --lock-preheld. # Records a durable checkpoint and that note, exits the old agent, # then delegates the launch to its single owner, # bin/fm-spawn.sh --relaunch. A failure before publication keeps @@ -541,8 +542,15 @@ do_exit() { ;; idle*) ;; *) - [ -z "$STALL_RECORD" ] \ - || die "task $ID's busy verdict is not a proven idle, so supervised stall recovery cannot prove the worker is still non-turning; refusing to exit an agent whose state cannot be proven" + if [ -n "$STALL_RECORD" ]; then + # Unproven custody on the supervised path fails closed BEFORE any + # transport: exit 5 is the dedicated "custody unproven; nothing was + # sent" code the caller maps to a clean cancellation that restores the + # instructions byte-exact, and the supervised caller escalates it - + # never a deferral and never an instruction-handled result. + printf 'custody-unproven' + return 5 + fi ;; esac cmd=$(fm_control_exit_command "$HARNESS") @@ -950,6 +958,15 @@ do_relaunch() { # to deferred. The instructions are restored byte-exact first. stall_relaunch_cancel worker-busy "stall record $STALL_RECORD: worker went busy during the relaunch; deferring rather than interrupting a productive worker" 4 ;; + 5) + # Custody could not be proven at the last gate (the busy verdict went + # unknown during the checkpoint): nothing was sent to the agent, so + # this is the same clean pre-action cancellation as a resolved record + # - instructions restored byte-exact, journal cancelled - but exit 5 + # tells the supervised caller to escalate rather than defer, because + # an unproven worker is not a productive one. + stall_relaunch_cancel custody-unproven "stall record $STALL_RECORD: worker custody could not be proven during the relaunch; nothing was sent to the agent" 5 + ;; *) exit "$exit_rc" ;; esac fi diff --git a/bin/fm-stall-recovery.sh b/bin/fm-stall-recovery.sh index 34d473786f9..01ee123b090 100755 --- a/bin/fm-stall-recovery.sh +++ b/bin/fm-stall-recovery.sh @@ -257,12 +257,16 @@ attempts_record='' attempts_count=0 if [ -e "$attempts_file" ] || [ -L "$attempts_file" ]; then [ -f "$attempts_file" ] && [ ! -L "$attempts_file" ] \ || verdict escalate "recovery-attempt marker is not a regular file" - attempts_line_count=$(wc -l < "$attempts_file") \ - || verdict escalate "cannot read the recovery-attempt bound at $attempts_file" - [ "$attempts_line_count" -eq 1 ] \ - || verdict escalate "malformed recovery-attempt marker at $attempts_file" + # The marker is exactly one canonical newline-terminated record: the byte + # count must equal the first line's length plus its terminator, so a + # trailing unterminated suffix, a second line, or a missing terminator all + # fail closed rather than being silently normalized away by read. IFS= read -r attempts_content < "$attempts_file" \ || verdict escalate "cannot read the recovery-attempt bound at $attempts_file" + attempts_bytes=$(wc -c < "$attempts_file") \ + || verdict escalate "cannot read the recovery-attempt bound at $attempts_file" + [ "$attempts_bytes" -eq $(( ${#attempts_content} + 1 )) ] \ + || verdict escalate "malformed recovery-attempt marker at $attempts_file" IFS=$(printf '\t') read -r attempts_record attempts_count attempts_extra \ <<< "$attempts_content" # Structure is validated separately from record identity: a well-formed @@ -300,8 +304,9 @@ fi # hands fm-control the record basename so it re-proves the instruction is # still the oldest unhandled record inside the lock, immediately before the # agent is touched - the check above cannot cover the gap to that point. -# Exit 3 is fm-control's "record resolved; nothing to do" code and exit 4 its -# "worker went busy during the relaunch; defer" code. +# Exit 3 is fm-control's "record resolved; nothing to do" code, exit 4 its +# "worker went busy during the relaunch; defer" code, and exit 5 its +# "custody unproven; nothing was sent" code. FM_CONFIG_OVERRIDE=${FM_CONFIG_OVERRIDE:-$FM_HOME/config} control_out_file=$(mktemp "$STATE/.stall-recovery-control-out.XXXXXX" 2>/dev/null) \ || verdict escalate "cannot allocate the fm-control output capture" @@ -320,6 +325,7 @@ case "$control_rc" in 0) ;; 3) verdict recovered "record ${RECORD##*/} resolved inside the lifecycle lock before the relaunch; instruction handled" ;; 4) verdict deferred "worker went busy inside the lifecycle lock before the relaunch; it is acting on ${RECORD##*/}, so the episode stays pending until the record is handled or the ladder re-escalates" ;; + 5) verdict escalate "worker custody could not be proven inside the lifecycle lock before the relaunch; nothing was sent to the agent and the instruction stays unhandled" ;; *) verdict escalate "fm-control relaunch refused or failed: $(printf '%s' "$control_out" | tail -1)" ;; esac diff --git a/docs/architecture.md b/docs/architecture.md index 6b7945db985..30f1d4509a7 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -141,7 +141,7 @@ Consumed OMP delivery receipts retire as durable `.pending.acked` tombstones, so Before either stale wake publishes, `bin/fm-stall-recovery.sh` runs a custody-checked bounded auto-recovery: it re-proves the record is still the oldest unhandled instruction, classifies the endpoint as live-non-turning or missing, requires a clean non-run crew-state and the durable `fm-` Treehouse lease, then re-proves the whole chain immediately before invoking `fm-control.sh relaunch`. Uncommitted changes and unpushed commits are deliberately preserved rather than treated as blockers: the relaunch inherits the same worktree, branch, and commits untouched. The verdict is `recovered` when the record was already handled, `deferred` when the worker is provably busy or the relaunch just published (the episode stays pending until the record is handled or the reset ladder re-escalates), and `escalate` for every unprovable or unsafe shape, which keeps the ordinary stale wake with the helper's reason appended. -One automatic relaunch per stalled record is bounded by `state/.inbox/.recovery-attempts`; the bound resets only when the inbox empties. +One automatic relaunch per stalled record is bounded by `state/.inbox/.recovery-attempts`: the bound is per-record, so a well-formed marker naming a record that was since handled is replaced atomically and the new oldest record starts its own count at zero, while an observed empty inbox clears the marker entirely. An OMP relaunch also retires the prior generation's `request.*` doorbell receipts after every refusal gate and immediately before the replacement launch, so a stale `.acked` tombstone cannot suppress the new incarnation's doorbell. An OMP worker is reached only through its task-bound native receive adapter, never the composer, because an already-streaming session cannot be steered through editable terminal text; `fm-send.sh` reports one bounded outcome per steer - native receipt, a named durable native queue entry, or an explicit refusal - each binding the exact session and message. Normal local metadata publication, the Orca abort-recovery publication, inbox enqueue revalidation and record publication, and teardown share the per-task metadata lifecycle lock so endpoint birth, delivery, and retirement cannot cross. diff --git a/docs/configuration.md b/docs/configuration.md index d068ebd22b3..f080cd42f86 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -972,7 +972,7 @@ FM_COMPOSER_IDLE_RE= # optional override for the fleet-wide empty-composer pl FM_COMPOSER_GHOST_LUMA_MAX=128 # fleet-wide: max perceived luminance (0.299R+0.587G+0.114B, 0-255) for a TRUECOLOR foreground to count as de-emphasised ghost/placeholder text and be stripped; dim/faint (SGR 2) is stripped regardless. Assumes a dark terminal theme (bin/fm-composer-lib.sh's fm_composer_strip_ghost, shared by the tmux and herdr composer readers) GROK_HOME= # optional Grok config home for firstmate's global grok turn-end hook; defaults to ~/.grok FM_TASK_INBOX_GRACE_SECS=90 # seconds before an unhandled local task record is eligible for a watcher re-ring, and between later attempts -FM_TASK_INBOX_RING_MAX=3 # watcher doorbell attempts before custody-checked stall recovery runs; an ordinary stale wake follows only when recovery defers or escalates +FM_TASK_INBOX_RING_MAX=3 # watcher doorbell attempts before custody-checked stall recovery runs; an ordinary stale wake follows only when recovery escalates or the helper fails, while deferred and recovered outcomes suppress it FM_TASK_INBOX_LOCK_WAIT_SECS=5 # bounded wait for inbox sequence allocation and fm-send's final metadata revalidation; invalid values use 5 FM_SEND_RETRIES=3 # typed-plane fm-send Enter-retry attempts after typing the line once FM_SEND_SLEEP=0.4 # seconds between typed-plane fm-send submit checks diff --git a/tests/fm-stall-recovery.test.sh b/tests/fm-stall-recovery.test.sh index 8c5e63371ec..e28a77bee32 100755 --- a/tests/fm-stall-recovery.test.sh +++ b/tests/fm-stall-recovery.test.sh @@ -329,13 +329,15 @@ exit "${FM_FAKE_CONTROL_RC:-0}" SH chmod +x "$fb/fm-control.sh" - # git forwards to the real binary, with two test hooks on the first + # git forwards to the real binary, with three test hooks on the first # `git -C status --porcelain` inside fm-control's safe_checkpoint: # FM_FAKE_GIT_MOVE names an inbox record moved to handled/, simulating a # worker that acknowledges the instruction in the gap between the caller's # pre-invocation check and fm-control's in-lock re-check; FM_FAKE_GIT_BUSY # names a busy-state file overwritten with a valid busy record, simulating - # a worker that starts a turn on the instruction during the checkpoint. + # a worker that starts a turn on the instruction during the checkpoint; + # FM_FAKE_GIT_RM names a file deleted outright, simulating the busy-state + # record becoming unavailable mid-checkpoint. cat > "$fb/git" <<'SH' #!/usr/bin/env bash set -u @@ -348,6 +350,9 @@ if [ "${1:-}" = "-C" ] && [ "${3:-}" = "status" ]; then printf 'v1 gen=gentest seq=3 state=busy source=omp-ext event=turn-start ts=1\n' \ > "$FM_FAKE_GIT_BUSY" fi + if [ -n "${FM_FAKE_GIT_RM:-}" ]; then + rm -f "$FM_FAKE_GIT_RM" + fi fi exec /usr/bin/git "$@" SH @@ -1093,6 +1098,68 @@ test_malformed_attempt_marker_escalates() { pass "corrupt attempt marker: recovery escalates without a lifecycle action" } +# A marker whose canonical line is followed by an unterminated suffix is +# corrupt: the byte count exceeds one newline-terminated record, so recovery +# escalates rather than parsing only the valid prefix and resetting the count. +test_unterminated_marker_suffix_escalates() { + local rec id record + id=$(case_id marker-suffix) + rec=$(make_case marker-suffix "$id" pool) + read_case "$rec" + write_pool_state "$CASE_DIR" "$WT_DIR" "fm-$id" + write_slot_marker "$SLOT_DIR" "$id" "$HOME_DIR" + write_meta "$HOME_DIR/state/$id.meta" "$id" "$WT_DIR" "$PROJ_DIR" + create_prior_artifacts "$HOME_DIR/state" "$id" + write_inbox "$HOME_DIR/state" "$id" 001 + write_inbox "$HOME_DIR/state" "$id" 002 + printf '001.msg\t1\n' > "$HOME_DIR/state/$id.inbox/.recovery-attempts" + printf 'CORRUPT' >> "$HOME_DIR/state/$id.inbox/.recovery-attempts" + mv "$HOME_DIR/state/$id.inbox/001.msg" "$HOME_DIR/state/$id.inbox/handled/" + record="$HOME_DIR/state/$id.inbox/002.msg" + missing_window "$CASE_DIR" "$id" + + run_recovery "$CASE_DIR" "$HOME_DIR" "$id" "$record" endpoint-unavailable + assert_contains "$RECOVERY_OUT" "verdict=escalate" "a marker with a trailing unterminated suffix did not escalate" + assert_contains "$RECOVERY_OUT" "malformed recovery-attempt marker" "the escalation did not name the corrupt marker" + assert_absent "$CASE_DIR/control.log" "the lifecycle verb ran against a corrupt attempt marker" + pass "unterminated marker suffix: recovery escalates instead of parsing the valid prefix" +} + +# When the busy-state record becomes unavailable during the checkpoint, the +# final gate can no longer prove custody: the supervised relaunch cancels +# with escalation (not deferral), sends nothing to the agent, and restores +# the worker's instructions byte-exact. The fake git deletes the busy-state +# file inside fm-control's safe_checkpoint. +test_unproven_custody_during_checkpoint_escalates() { + local rec id record + id=$(case_id custody-gone) + rec=$(make_case custody-gone "$id" pool) + read_case "$rec" + write_pool_state "$CASE_DIR" "$WT_DIR" "fm-$id" + write_slot_marker "$SLOT_DIR" "$id" "$HOME_DIR" + write_meta "$HOME_DIR/state/$id.meta" "$id" "$WT_DIR" "$PROJ_DIR" + create_prior_artifacts "$HOME_DIR/state" "$id" + write_inbox "$HOME_DIR/state" "$id" 001 + record="$HOME_DIR/state/$id.inbox/001.msg" + write_busy "$HOME_DIR/state" "$id" idle + live_window "$CASE_DIR" "$id" bun + cp -p "$HOME_DIR/data/$id/brief.md" "$CASE_DIR/brief.orig" + + FM_FAKE_GIT_RM="$HOME_DIR/state/$id.busy-state" \ + FM_STALL_RECOVERY_CONTROL_BIN="$CONTROL" \ + run_recovery "$CASE_DIR" "$HOME_DIR" "$id" "$record" ladder-exhausted + expect_code 0 "$RECOVERY_STATUS" "unproven-custody recovery should exit 0; got: $RECOVERY_OUT" + assert_contains "$RECOVERY_OUT" "verdict=escalate" "unproven custody during the checkpoint did not escalate" + assert_no_grep "Escape" "$CASE_DIR/fake/tmux.log" "an interrupt key was sent while custody was unproven" + assert_no_grep "/exit" "$CASE_DIR/fake/tmux.log" "an exit command was sent while custody was unproven" + assert_no_grep "new-window" "$CASE_DIR/fake/tmux.log" "the relaunch created a window while custody was unproven" + assert_grep "cancelled:custody-unproven" "$HOME_DIR/state/$id.control-relaunch" "the journal did not record the unproven-custody cancellation" + assert_present "$record" "the unhandled instruction record was moved or deleted" + cmp -s "$CASE_DIR/brief.orig" "$HOME_DIR/data/$id/brief.md" \ + || fail "an unproven-custody cancellation left the worker's instructions modified" + pass "unproven custody during checkpoint: recovery escalates with no transport and restored instructions" +} + # --- run --------------------------------------------------------------------- test_missing_endpoint_recovers_via_control @@ -1115,5 +1182,7 @@ test_omp_ext_serializes_busy_events test_busy_during_checkpoint_defers test_next_record_gets_own_attempt test_malformed_attempt_marker_escalates +test_unterminated_marker_suffix_escalates +test_unproven_custody_during_checkpoint_escalates pass "all stall-recovery tests" From c69e698bd621b72793fd6f1aad9a3238713618ff Mon Sep 17 00:00:00 2001 From: dnth Date: Thu, 24 Sep 2026 19:44:38 +0800 Subject: [PATCH 15/21] feat: reshape stall recovery - live workers escalate untouched, missing endpoints relaunch Captain ruling (inbox 031/036): recovery must NEVER interrupt a live worker. - LIVE-BUT-NON-TURNING (endpoint present, any busy state): detect and escalate only. No interrupt keys, no exit, no relaunch, no invalidation. - POSITIVELY ABSENT endpoint: relaunch allowed with no busy-state proof - there is no live worker to interrupt. - Unknown/unproven crew-state fails closed on every path. - Deleted the busy-record machinery this task added: OMP extension busy events, omp-ext source registration, busy verdict gate in the helper, and fm-control's supervised busy-deferred/custody-unproven exits. - Kept: missing-endpoint relaunch under the lifecycle lock with the in-lock record re-check, per-record attempt bound, relaunch_tx, and doorbell receipt retirement; shared busy-state infrastructure untouched. 19/19 stall-recovery tests pass; fm-lint clean. --- bin/fm-busy-lib.sh | 2 - bin/fm-control.sh | 58 +-------- bin/fm-spawn.sh | 36 +----- bin/fm-stall-recovery.sh | 126 ++++++++---------- bin/fm-task-inbox-lib.sh | 21 +-- bin/fm-watch.sh | 11 +- docs/agent-control.md | 2 +- docs/architecture.md | 8 +- tests/fm-stall-recovery.test.sh | 222 ++++++-------------------------- 9 files changed, 130 insertions(+), 356 deletions(-) diff --git a/bin/fm-busy-lib.sh b/bin/fm-busy-lib.sh index b2f9cd7e463..100e20777a8 100755 --- a/bin/fm-busy-lib.sh +++ b/bin/fm-busy-lib.sh @@ -31,7 +31,6 @@ # pi-ext Pi/pi-signed per-task extension (agent_start/agent_settled) # opencode-plugin OpenCode per-task plugin (session.status) # claude-hook Claude lifecycle hooks (UserPromptSubmit/Stop/StopFailure/SessionEnd) -# omp-ext OMP per-task extension (turn_start/turn_end/session_shutdown) # hermes-hook Hermes lifecycle bridge (plugin-forwarded TUI events, # plus compatible shell-hook events) # codex-hook, codex-appserver reserved: Codex, gated by @@ -179,7 +178,6 @@ fm_busy_sources_for_harness() { # adapter='codex-hook codex-appserver' ;; opencode*) adapter=opencode-plugin ;; - omp) adapter=omp-ext ;; pi|pi-signed) adapter=pi-ext ;; hermes) adapter=hermes-hook ;; kimi*) diff --git a/bin/fm-control.sh b/bin/fm-control.sh index 0b01e4f799f..770c1e5cfff 100755 --- a/bin/fm-control.sh +++ b/bin/fm-control.sh @@ -56,11 +56,7 @@ # lock, immediately before the agent is touched, fm-control # re-proves the named inbox record is still the oldest unhandled # instruction and cancels with exit 3 when it was handled or -# superseded in flight, cancels with exit 4 when the worker went -# provably busy during the checkpoint - a productive worker is -# deferred to, never interrupted - and cancels with exit 5 when -# custody can no longer be proven, which the supervised caller -# escalates. It requires --lock-preheld. +# superseded in flight. It requires --lock-preheld. # Records a durable checkpoint and that note, exits the old agent, # then delegates the launch to its single owner, # bin/fm-spawn.sh --relaunch. A failure before publication keeps @@ -512,21 +508,9 @@ do_exit() { missing) die "task $ID's recorded endpoint is gone, so there is no agent to stop; reconcile the task before any further control action" ;; *) die "task $ID's endpoint reads '$state' rather than a positively classified state; refusing to send a lifecycle command into an unattributed endpoint" ;; esac - # A busy agent is interrupted first before the exit command is submitted - - # except on the supervised --stall-record path, where busy means the stalled - # worker started acting on the instruction between the caller's custody - # proof and this last gate. Interrupting a now-productive worker is exactly - # the harm supervised recovery exists to avoid, so busy returns 4 for the - # caller to map to a deferred episode (with the instructions restored - # byte-exact), and any verdict that is not a proven idle fails closed - # rather than exiting an agent whose state cannot be proven. Manual - # relaunch and the plain exit verb keep the interrupt-first semantics. + # A busy agent is interrupted first before the exit command is submitted. case "$(busy_verdict)" in busy*) - if [ -n "$STALL_RECORD" ]; then - printf 'busy-deferred' - return 4 - fi cancel=$(deliver_interrupt) || return $? state=$(agent_state) case "$state" in @@ -540,18 +524,6 @@ do_exit() { *) die "task $ID's endpoint reads '$state' after interrupt delivery rather than a positively classified state; exit cannot prove whether the agent stopped" ;; esac ;; - idle*) ;; - *) - if [ -n "$STALL_RECORD" ]; then - # Unproven custody on the supervised path fails closed BEFORE any - # transport: exit 5 is the dedicated "custody unproven; nothing was - # sent" code the caller maps to a clean cancellation that restores the - # instructions byte-exact, and the supervised caller escalates it - - # never a deferral and never an instruction-handled result. - printf 'custody-unproven' - return 5 - fi - ;; esac cmd=$(fm_control_exit_command "$HARNESS") # The submit verdict is NOT the postcondition here: a successful exit command @@ -881,7 +853,7 @@ record_note() { } do_relaunch() { - local exit_result exit_rc state note_line stall_oldest + local exit_result state note_line stall_oldest local -a spawn_args require_state_verified_backend relaunch @@ -946,29 +918,7 @@ do_relaunch() { retire_busy_incarnation exit_result=already-stopped else - exit_rc=0 - exit_result=$(do_exit) || exit_rc=$? - case "$exit_rc" in - 0) ;; - 4) - # The worker went busy between the custody proof and the exit gate: - # it is acting on the instruction now, so the relaunch defers rather - # than interrupting a productive worker. Exit 4 is the dedicated - # "worker busy; episode stays pending" code the supervised caller maps - # to deferred. The instructions are restored byte-exact first. - stall_relaunch_cancel worker-busy "stall record $STALL_RECORD: worker went busy during the relaunch; deferring rather than interrupting a productive worker" 4 - ;; - 5) - # Custody could not be proven at the last gate (the busy verdict went - # unknown during the checkpoint): nothing was sent to the agent, so - # this is the same clean pre-action cancellation as a resolved record - # - instructions restored byte-exact, journal cancelled - but exit 5 - # tells the supervised caller to escalate rather than defer, because - # an unproven worker is not a productive one. - stall_relaunch_cancel custody-unproven "stall record $STALL_RECORD: worker custody could not be proven during the relaunch; nothing was sent to the agent" 5 - ;; - *) exit "$exit_rc" ;; - esac + exit_result=$(do_exit) fi journal_write exited "${CHECKPOINT_LINES[@]}" "$note_line" "exit_result=$exit_result" diff --git a/bin/fm-spawn.sh b/bin/fm-spawn.sh index 5ead43629e5..61154f8e1af 100755 --- a/bin/fm-spawn.sh +++ b/bin/fm-spawn.sh @@ -4269,7 +4269,7 @@ if [ "$KIND" != secondmate ]; then ;; esac case "$HARNESS" in - claude*|opencode*|pi|pi-signed|omp) + claude*|opencode*|pi|pi-signed) BUSY_GEN=$("$FM_ROOT/bin/fm-busy-event.sh" arm "$STATE_REAL" "$ID") || { echo "error: failed to arm the busy-state contract for $ID" >&2 exit 1 @@ -4407,33 +4407,12 @@ EOF omp) rm -f "$OMP_READY" "$OMP_STARTED" "$OMP_DOORBELL_READY" "$OMP_DOORBELL_FAILED" cat > "$STATE/$ID.omp-ext.ts" < - new Promise((resolve) => { - execFile("$FM_ROOT/bin/fm-busy-event.sh", [ - "apply", "$STATE_REAL", "$ID", state, - "--gen", "$BUSY_GEN", "--source", "omp-ext", "--event", event, - ], () => resolve(undefined)); - }); -// Busy-state writes are serialized through one chain so a turn_end's idle can -// never land after a following turn_start's busy: an out-of-order pair would -// leave a live worker falsely busy (suppressing stall recovery forever) or a -// dead one falsely idle. Handlers also await the chain so the runtime's own -// event ordering is honored end to end. -let busyChain: Promise = Promise.resolve(); -const queueBusyEvent = (state: string, event: string) => { - busyChain = busyChain.then(() => busyEvent(state, event)); - return busyChain; -}; export default function (omp: any) { const taskInboxDoorbell = installTaskInboxDoorbell(omp, { inboxDir: "$STATE_REAL/$ID.inbox", @@ -4449,20 +4428,15 @@ export default function (omp: any) { if (active) execFile("touch", ["$OMP_READY"]); }); }); - omp.on("turn_start", async () => { + omp.on("turn_start", () => { taskInboxDoorbell.notifyTurnStart(); execFile("touch", ["$OMP_STARTED"]); - await queueBusyEvent("busy", "turn-start"); }); - omp.on("turn_end", async () => { + omp.on("turn_end", () => { taskInboxDoorbell.notifyTurnEnd(); execFile("$TURNEND_SIGNAL", ["$STATE_REAL", "$ID", "$SPAWN_GEN"]); - await queueBusyEvent("idle", "turn-end"); - }); - omp.on("session_shutdown", async () => { - taskInboxDoorbell.retire(); - await queueBusyEvent("idle", "session-shutdown"); }); + omp.on("session_shutdown", taskInboxDoorbell.retire); } EOF ;; diff --git a/bin/fm-stall-recovery.sh b/bin/fm-stall-recovery.sh index 01ee123b090..ee5f2cf2075 100755 --- a/bin/fm-stall-recovery.sh +++ b/bin/fm-stall-recovery.sh @@ -13,39 +13,40 @@ # recovered - the instruction was already handled; nothing to do. # deferred - no lifecycle action taken and none needed right now: the # record was handled between the watcher's decision and this -# check, the worker is provably busy, or a relaunch was just -# published and the episode stays pending until the record is -# handled or the ladder re-escalates. -# escalate - recovery is unsafe, unproven, or exhausted; the caller keeps -# the ordinary stale wake. Any non-zero exit or missing verdict -# is also treated as escalate by the caller. +# check, or a relaunch was just published and the episode +# stays pending until the record is handled or the ladder +# re-escalates. +# escalate - recovery is unsafe, unproven, exhausted, or the endpoint is +# LIVE: recovery never interrupts, exits, or relaunches a live +# worker, so a live-but-non-turning session is detected and +# escalated for firstmate and nothing is sent to it. Any +# non-zero exit or missing verdict is also treated as escalate +# by the caller. # # What "safe" means here (every check fails closed to escalate): # - The named record is still the oldest UNHANDLED inbox record. Transport # receipts (.acked/.unproven/.awaiting-turn under the doorbell request dir) -# are never consulted: a stale receipt neither triggers nor suppresses -# recovery, and a record moved to handled/ at any point cancels the action. # - The endpoint is positively classified: dead/missing takes the # missing-endpoint path (no exit is sent; the launch owner recreates the -# endpoint), alive takes the live-non-turning path (the old agent is -# exited first). Ambiguous, unreadable, or unverified states escalate. -# - A live endpoint must also read an explicit idle busy verdict; busy -# defers (the worker may be mid-turn on the instruction) and unknown -# escalates (a probe failure is not custody proof). +# endpoint, and no busy-state proof is required because there is no live +# worker to interrupt). A live endpoint escalates unconditionally - +# interrupting or relaunching a session that exists is never recovery's +# call. Ambiguous, unreadable, or unverified states escalate. # - fm-crew-state must show no active validation run: working, parked, # blocked, and declared-paused states all escalate (a parked gate, a # declared external wait, and a worker-declared blocker are firstmate -# business, not stall recovery). Terminal done/failed may proceed, and -# unknown may proceed only on the missing-endpoint path. +# business, not stall recovery). Terminal done/failed may proceed on the +# missing-endpoint path; unknown or unreadable crew-state fails closed - +# missing, unknown, unproven, and stale custody all refuse and escalate. # - The recorded worktree must exist and carry the durable fm- lease; # uncommitted changes and unpushed commits inside it are PRESERVED, not # rejected: the stalled worker's unlanded work is exactly what recovery # exists to keep, and the relaunch inherits the same worktree, branch, # and commits untouched. -# - One automatic relaunch per stalled instruction: the per-record attempt -# marker under the inbox bounds retries; an emptied inbox resets it, and -# a well-formed marker naming a record that was since handled starts the -# new oldest record's own count at zero. +# - One automatic relaunch per stalled instruction on the missing-endpoint +# path: the per-record attempt marker under the inbox bounds retries; an +# emptied inbox resets it, and a well-formed marker naming a record that +# was since handled starts the new oldest record's own count at zero. # - The durable fm- worktree lease, same-worktree/branch/commits # preservation, and the no-shared-daemon boundary are enforced by # bin/fm-control.sh relaunch itself; this script never moves inbox @@ -56,13 +57,12 @@ # pending episode. The durable instruction's terminal outcome is either its # handled/ move (quiet) or the bounded re-escalation the reset ladder produces # when the replacement also fails to act. -# -# The final custody and inbox-record re-check runs INSIDE fm-control's -# lifecycle lock (state/.control-.lock), acquired by this process and held -# across the fm-control invocation via --lock-preheld: a record handled in the -# gap can never relaunch a now-productive worker, and a concurrent invocation -# or manual lifecycle action can never double-relaunch. The per-record attempt -# bound is checked and recorded under the same lock. +# The final custody and inbox-record re-check on the missing-endpoint path +# runs INSIDE fm-control's lifecycle lock (state/.control-.lock), acquired +# by this process and held across the fm-control invocation via +# --lock-preheld: a record handled in the gap can never relaunch, and a +# concurrent invocation or manual lifecycle action can never double-relaunch. +# The per-record attempt bound is checked and recorded under the same lock. # # Audit: every verdict appends one line to state/.stall-recovery; the # relaunch transaction itself journals to state/.control-relaunch, and a @@ -95,8 +95,6 @@ DATA="${FM_DATA_OVERRIDE:-$FM_HOME/data}" # shellcheck source=bin/fm-backend.sh . "$SCRIPT_DIR/fm-backend.sh" -# shellcheck source=bin/fm-busy-lib.sh -. "$SCRIPT_DIR/fm-busy-lib.sh" # shellcheck source=bin/fm-task-inbox-lib.sh . "$SCRIPT_DIR/fm-task-inbox-lib.sh" @@ -148,20 +146,22 @@ if [ "$oldest" != "$RECORD" ]; then fi # prove_custody: re-prove every precondition for a lifecycle action against -# CURRENT state - endpoint classification, busy verdict, crew/run state, -# worktree existence, and the durable fm- lease. Called directly (never in -# a command substitution) so its PATH_KIND, WT, PROJ, BACKEND, and TARGET +# CURRENT state - endpoint classification, crew/run state, worktree +# existence, and the durable fm- lease. Called directly (never in a +# command substitution) so its PATH_KIND, WT, PROJ, BACKEND, and TARGET # bindings reach the caller; the refusal reason is published through the -# CUSTODY_DETAIL global. Returns 1 on any failed or unprovable check, 2 when -# the worker is provably busy (a defer, not an escalation), 0 on success. All -# reads are local: no gh or fetch can ever stall the watcher. +# CUSTODY_DETAIL global. Returns 1 on any failed or unprovable check - +# including a LIVE endpoint, which escalates because recovery never +# interrupts a session that exists - and 0 only on the missing-endpoint path +# with every custody proof clean. All reads are local: no gh or fetch can +# ever stall the watcher. # # Uncommitted changes and unpushed commits are deliberately NOT gates here: # recovery exists to preserve exactly that unlanded work, and the relaunch # inherits the same worktree, branch, and commits untouched (fm-control's # safe_checkpoint records head and dirty state for the journal). prove_custody() { - local state busy crew_line crew_state pool_state lease_holder + local state crew_line crew_state pool_state lease_holder PATH_KIND= CUSTODY_DETAIL= WT=$(fm_meta_get "$META" worktree) @@ -172,33 +172,28 @@ prove_custody() { TARGET=$FM_BACKEND_VALIDATED_TARGET state=$(fm_backend_agent_state "$BACKEND" "$TARGET" "$META" 2>/dev/null || printf 'unreadable') case "$state" in - alive) PATH_KIND=live-non-turning ;; + alive) + # A live worker is never interrupted, exited, or relaunched by + # recovery: the session exists, so the stall is firstmate's call. + PATH_KIND=live-non-turning + CUSTODY_DETAIL='endpoint is live; recovery never interrupts a live worker - escalating for firstmate' + return 1 + ;; dead|missing) PATH_KIND=missing-endpoint ;; *) CUSTODY_DETAIL="endpoint state '$state' is not positively classified"; return 1 ;; esac - if [ "$PATH_KIND" = live-non-turning ]; then - busy=$(fm_busy_classify_meta "$META" "$ID" "$STATE" 2>/dev/null || printf 'unknown') - case "${busy%% *}" in - idle) ;; - busy) CUSTODY_DETAIL="worker is busy ($busy); it may be mid-turn on the instruction"; return 2 ;; - *) CUSTODY_DETAIL="busy verdict '$busy' is not a custody proof"; return 1 ;; - esac - fi crew_line=$("$FM_CREW_STATE_BIN" "$ID" 2>/dev/null || true) crew_state=$(printf '%s' "$crew_line" | sed -n 's/^state: \([a-z-]*\).*/\1/p' | head -1) case "$crew_state" in working|parked|blocked|paused) CUSTODY_DETAIL="crew-state $crew_state needs firstmate, not auto-relaunch"; return 1 ;; done|failed) ;; - unknown) - [ "$PATH_KIND" = missing-endpoint ] \ - || { CUSTODY_DETAIL='crew-state unknown with a live endpoint is ambiguous'; return 1; } ;; - *) CUSTODY_DETAIL="crew-state '${crew_state:-unreadable}' is not a clean non-run state"; return 1 ;; + *) CUSTODY_DETAIL="crew-state '${crew_state:-unreadable}' cannot prove a clean non-run state"; return 1 ;; esac [ -n "$WT" ] && [ -d "$WT" ] || { CUSTODY_DETAIL='recorded worktree missing'; return 1; } # The durable fm- lease proof mirrors bin/fm-spawn.sh's - # relaunch_worktree_lease_proven, applied to BOTH paths here because the - # launch owner only re-proves it on the gone-endpoint path. + # relaunch_worktree_lease_proven, applied here because the launch owner + # only re-proves it on the gone-endpoint path. fm_treehouse_pool_slot "$PROJ" "$WT" \ || { CUSTODY_DETAIL='recorded worktree is not a Treehouse pool slot of the recorded project'; return 1; } fm_treehouse_slot_owner_state "$WT" "$ID" @@ -218,11 +213,9 @@ prove_custody() { return 0 } -# Gate proof: full custody chain before any lifecycle decision. -prove_custody || case $? in - 2) verdict deferred "$CUSTODY_DETAIL" ;; - *) verdict escalate "$CUSTODY_DETAIL" ;; -esac +# Gate proof: full custody chain before any lifecycle decision. A live +# endpoint escalates here - recovery never interrupts a session that exists. +prove_custody || verdict escalate "$CUSTODY_DETAIL" # --- bounded lifecycle action, under fm-control's lifecycle lock ------------ # @@ -238,20 +231,19 @@ fm_lock_try_acquire "$STALL_LOCK" \ STALL_LOCK_HELD=1 # Final re-check inside the lock, in strict order: re-prove the full custody -# chain (a worker can become busy, enter a run, or lose its worktree between -# the first proof and the relaunch), then re-prove the record itself LAST so -# a handled move during the custody probe still cancels the action. -prove_custody || case $? in - 2) verdict deferred "$CUSTODY_DETAIL" ;; - *) verdict escalate "$CUSTODY_DETAIL" ;; -esac +# chain (a worker can come back, enter a run, or lose its worktree between +# the first proof and the relaunch - a resurrected endpoint escalates the +# same way a live one does), then re-prove the record itself LAST so a +# handled move during the custody probe still cancels the action. +prove_custody || verdict escalate "$CUSTODY_DETAIL" oldest=$(fm_task_inbox_oldest_unhandled "$STATE" "$ID" 2>/dev/null || true) [ -n "$oldest" ] || verdict recovered "inbox emptied before relaunch; instruction handled" [ "$oldest" = "$RECORD" ] || verdict deferred "record ${RECORD##*/} handled or superseded before relaunch" # Bounded retry: exactly one automatic relaunch per stalled instruction -# record, checked and recorded under the lock so concurrent invocations -# cannot both pass the bound. The bound is a fixed invariant - no override. +# record on the missing-endpoint path, checked and recorded under the lock +# so concurrent invocations cannot both pass the bound. The bound is a fixed +# invariant - no override. attempts_file="$dir/.recovery-attempts" attempts_record='' attempts_count=0 if [ -e "$attempts_file" ] || [ -L "$attempts_file" ]; then @@ -304,9 +296,7 @@ fi # hands fm-control the record basename so it re-proves the instruction is # still the oldest unhandled record inside the lock, immediately before the # agent is touched - the check above cannot cover the gap to that point. -# Exit 3 is fm-control's "record resolved; nothing to do" code, exit 4 its -# "worker went busy during the relaunch; defer" code, and exit 5 its -# "custody unproven; nothing was sent" code. +# Exit 3 is fm-control's "record resolved; nothing to do" code. FM_CONFIG_OVERRIDE=${FM_CONFIG_OVERRIDE:-$FM_HOME/config} control_out_file=$(mktemp "$STATE/.stall-recovery-control-out.XXXXXX" 2>/dev/null) \ || verdict escalate "cannot allocate the fm-control output capture" @@ -324,8 +314,6 @@ rm -f "$control_out_file" case "$control_rc" in 0) ;; 3) verdict recovered "record ${RECORD##*/} resolved inside the lifecycle lock before the relaunch; instruction handled" ;; - 4) verdict deferred "worker went busy inside the lifecycle lock before the relaunch; it is acting on ${RECORD##*/}, so the episode stays pending until the record is handled or the ladder re-escalates" ;; - 5) verdict escalate "worker custody could not be proven inside the lifecycle lock before the relaunch; nothing was sent to the agent and the instruction stays unhandled" ;; *) verdict escalate "fm-control relaunch refused or failed: $(printf '%s' "$control_out" | tail -1)" ;; esac diff --git a/bin/fm-task-inbox-lib.sh b/bin/fm-task-inbox-lib.sh index 852f10353e7..7e7e05205a1 100644 --- a/bin/fm-task-inbox-lib.sh +++ b/bin/fm-task-inbox-lib.sh @@ -46,8 +46,9 @@ # the ready marker, journaled by the extension # .inbox/.recovery-attempts # stall auto-recovery bound: "\t" - -# one automatic relaunch per stalled record, -# reset when the inbox empties (bin/fm-stall-recovery.sh) +# one automatic relaunch per stalled record on the +# missing-endpoint path, reset when the inbox +# empties (bin/fm-stall-recovery.sh) # # Record format (fm_task_inbox_write / fm_task_inbox_body): # schema=fm-task-inbox.v1 @@ -483,14 +484,14 @@ fm_task_inbox_record_escalated() { # } # Reset the re-ring ladder for a task whose worker was just replaced by stall -# auto-recovery: the new incarnation gets the full grace-and-retry budget for -# the still-unhandled record instead of inheriting the wedged worker's spent -# budget and escalation marker. The .recovery-attempts bound is deliberately -# NOT cleared here - it is the per-record retry cap and resets only when the -# inbox empties (fm_task_inbox_due_action). Returns non-zero when the ladder -# files could not be cleared while the inbox still holds records, so the -# caller escalates rather than reporting a pending relaunch with lost retry -# bookkeeping. +# auto-recovery's missing-endpoint relaunch: the new incarnation gets the full +# grace-and-retry budget for the still-unhandled record instead of inheriting +# the wedged worker's spent budget and escalation marker. The +# .recovery-attempts bound is deliberately NOT cleared here - it is the +# per-record retry cap and resets only when the inbox empties +# (fm_task_inbox_due_action). Returns non-zero when the ladder files could not +# be cleared while the inbox still holds records, so the caller escalates +# rather than reporting a pending relaunch with lost retry bookkeeping. fm_task_inbox_ladder_reset() { # local dir dir=$(fm_task_inbox_dir "$1" "$2") diff --git a/bin/fm-watch.sh b/bin/fm-watch.sh index 7710d8023ff..96c5cfe01db 100755 --- a/bin/fm-watch.sh +++ b/bin/fm-watch.sh @@ -54,7 +54,8 @@ # The separate steering-inbox path invokes # fm-stall-recovery.sh before publishing its stale wake; # that helper owns its custody-checked, bounded relaunch -# exception. +# exception for a positively missing endpoint only - a +# live worker is never interrupted and escalates here. # An idle secondmate that is neither paused nor captain-held # is also absorbed while its home watcher beacon is fresh # within the wedge threshold; missing, stale, future-dated, @@ -365,10 +366,10 @@ inbox_steer_escalate_unavailable() { # # Custody-checked bounded auto-recovery for a stalled worker, owned by # bin/fm-stall-recovery.sh. Runs BEFORE the stale wake is published: a -# deferred verdict (record handled meanwhile, worker provably busy, or a -# relaunch just published with the episode still pending) suppresses the -# escalation entirely, while an escalate verdict - including any helper -# failure or missing verdict - keeps the ordinary stale wake with the +# deferred verdict (record handled meanwhile, or a missing-endpoint relaunch +# just published with the episode still pending) suppresses the escalation +# entirely, while an escalate verdict - including a live endpoint, any helper +# failure, or a missing verdict - keeps the ordinary stale wake with the # helper's reason appended. Returns 0 when the wake is suppressed, 1 when the # caller should escalate. FM_STALL_RECOVERY_BIN="${FM_STALL_RECOVERY_BIN:-$SCRIPT_DIR/fm-stall-recovery.sh}" diff --git a/docs/agent-control.md b/docs/agent-control.md index 65c2e8ee538..b2079fa18d6 100644 --- a/docs/agent-control.md +++ b/docs/agent-control.md @@ -78,7 +78,7 @@ It is not deterministic across the verified adapters: codex and grok resume only Switching harness is therefore one ordinary relaunch rather than a separate mechanism. -The watcher-triggered stall-recovery path uses the same transactional relaunch under its lifecycle lock, but its final checkpoint defers instead of interrupting when the worker has become provably busy; the custody and verdict contract lives in [`architecture.md`](architecture.md#event-driven-supervision). +The watcher-triggered stall-recovery path uses the same transactional relaunch under its lifecycle lock, but only for a positively missing endpoint - a live worker is never interrupted and always escalates; the custody and verdict contract lives in [`architecture.md`](architecture.md#event-driven-supervision). ### Failure and rollback diff --git a/docs/architecture.md b/docs/architecture.md index 30f1d4509a7..f53e2c9d597 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -138,10 +138,10 @@ For an OMP worker the loaded extension delivers the doorbell through `sendMessag The generated OMP extension publishes `.omp-ready` only after the doorbell activates; activation or drain failure retires `.omp-doorbell-ready` and durably journals the reason in `.omp-doorbell-failed`, while `fm-spawn.sh` bounded-waits for readiness and `fm-send.sh` names the missing marker or failure journal when refusing native delivery. When the runtime downgrades `triggerTurn` to append-only, the extension re-drives the instruction through `sendUserMessage` only after its bounded grace expires without any turn opening; the re-drive is itself only a request, so a nonthrowing return is never a receipt - the entry re-parks for one more bounded proof window and leaves a durable `.unproven` marker when no turn opens, allowing the next ring to publish a fresh pending request (`.omp/extensions/lib/fm-task-inbox-doorbell.ts`). Consumed OMP delivery receipts retire as durable `.pending.acked` tombstones, so later rings report delivery without republishing or sending another doorbell; the requests directory is generation-scoped and reset with the task lifecycle. -Before either stale wake publishes, `bin/fm-stall-recovery.sh` runs a custody-checked bounded auto-recovery: it re-proves the record is still the oldest unhandled instruction, classifies the endpoint as live-non-turning or missing, requires a clean non-run crew-state and the durable `fm-` Treehouse lease, then re-proves the whole chain immediately before invoking `fm-control.sh relaunch`. +Before either stale wake publishes, `bin/fm-stall-recovery.sh` runs a custody-checked bounded auto-recovery: it re-proves the record is still the oldest unhandled instruction and classifies the endpoint. A live endpoint always escalates - recovery never interrupts, exits, or relaunches a session that exists - while a positively missing endpoint may relaunch after proving a clean non-run crew-state and the durable `fm-` Treehouse lease, re-proving the whole chain inside fm-control's lifecycle lock immediately before `fm-control.sh relaunch`. The missing-endpoint path needs no busy-state proof: there is no live worker to interrupt. Uncommitted changes and unpushed commits are deliberately preserved rather than treated as blockers: the relaunch inherits the same worktree, branch, and commits untouched. -The verdict is `recovered` when the record was already handled, `deferred` when the worker is provably busy or the relaunch just published (the episode stays pending until the record is handled or the reset ladder re-escalates), and `escalate` for every unprovable or unsafe shape, which keeps the ordinary stale wake with the helper's reason appended. -One automatic relaunch per stalled record is bounded by `state/.inbox/.recovery-attempts`: the bound is per-record, so a well-formed marker naming a record that was since handled is replaced atomically and the new oldest record starts its own count at zero, while an observed empty inbox clears the marker entirely. +The verdict is `recovered` when the record was already handled, `deferred` when the relaunch just published (the episode stays pending until the record is handled or the reset ladder re-escalates), and `escalate` for every unprovable or unsafe shape - including every live endpoint - which keeps the ordinary stale wake with the helper's reason appended. +One automatic relaunch per stalled record on the missing-endpoint path is bounded by `state/.inbox/.recovery-attempts`: the bound is per-record, so a well-formed marker naming a record that was since handled is replaced atomically and the new oldest record starts its own count at zero, while an observed empty inbox clears the marker entirely. An OMP relaunch also retires the prior generation's `request.*` doorbell receipts after every refusal gate and immediately before the replacement launch, so a stale `.acked` tombstone cannot suppress the new incarnation's doorbell. An OMP worker is reached only through its task-bound native receive adapter, never the composer, because an already-streaming session cannot be steered through editable terminal text; `fm-send.sh` reports one bounded outcome per steer - native receipt, a named durable native queue entry, or an explicit refusal - each binding the exact session and message. Normal local metadata publication, the Orca abort-recovery publication, inbox enqueue revalidation and record publication, and teardown share the per-task metadata lifecycle lock so endpoint birth, delivery, and retirement cannot cross. @@ -155,7 +155,7 @@ Successful typed text sends then receive the existing `FM_SEND_SETTLE` pause so `bin/fm-busy-lib.sh` is the single owner of what "this worker is busy" means, and `bin/fm-busy-event.sh` is the only writer of the per-task records it reads. Every classification returns a verdict of busy, idle, unknown, or dead together with the source that produced it, so a consumer or a diagnostic can never confuse semantic state with a fallback. -Each converted adapter reports its own turn lifecycle through the strongest verified source the vendor exposes: Pi and pi-signed through the Firstmate-owned extension's `agent_start` and `agent_settled` confirmed by `ctx.isIdle()`, OpenCode through its plugin's semantic `session.status`, OMP through its generated extension's `turn_start`/`turn_end`/`session_shutdown` handlers, Claude through owned lifecycle hooks, and Hermes through its live TUI's busy-only composer/footer plus a plugin-forwarded lifecycle bridge for exact turn boundaries. +Each converted adapter reports its own turn lifecycle through the strongest verified source the vendor exposes: Pi and pi-signed through the Firstmate-owned extension's `agent_start` and `agent_settled` confirmed by `ctx.isIdle()`, OpenCode through its plugin's semantic `session.status`, Claude through owned lifecycle hooks, and Hermes through its live TUI's busy-only composer/footer plus a plugin-forwarded lifecycle bridge for exact turn boundaries. Kimi behind Pi inherits Pi's lifecycle. Codex and standalone Kimi classify unknown behind explicit probes until a semantic source is live-verified for them, while the Hermes and Grok rendered sources are isolated by exact harness identity. Hermes' bridged lifecycle record outranks its rendered footer in both directions - a trusted busy beats a rendered ready row and a trusted idle beats a lagging rendered busy row - and the rendered tail is read only when no valid record exists, so neither a steer nor a `C-c` interrupt is gated on a stale screen. diff --git a/tests/fm-stall-recovery.test.sh b/tests/fm-stall-recovery.test.sh index e28a77bee32..8e3f4316d58 100755 --- a/tests/fm-stall-recovery.test.sh +++ b/tests/fm-stall-recovery.test.sh @@ -6,9 +6,12 @@ # bin/fm-spawn.sh's relaunch path that retires the prior incarnation's # doorbell receipts. # -# Contract under test (data/fm-stalled-worker-astra-investigate/report.md): +# Contract under test (captain ruling, inbox 031/036): # - triggers on queued/unproven + persistent unhandled inbox records -# - distinguishes live-non-turning from missing-endpoint +# - distinguishes live-non-turning from missing-endpoint: a LIVE endpoint +# is detected and escalated only - recovery never interrupts, exits, or +# relaunches a session that exists - while a positively missing endpoint +# may relaunch with no busy-state proof at all # - reconciles stale .acked tombstones against handled state and generation # - re-checks inbox AND task/run state immediately before the lifecycle # action @@ -329,15 +332,11 @@ exit "${FM_FAKE_CONTROL_RC:-0}" SH chmod +x "$fb/fm-control.sh" - # git forwards to the real binary, with three test hooks on the first + # git forwards to the real binary, with one test hook on the first # `git -C status --porcelain` inside fm-control's safe_checkpoint: # FM_FAKE_GIT_MOVE names an inbox record moved to handled/, simulating a # worker that acknowledges the instruction in the gap between the caller's - # pre-invocation check and fm-control's in-lock re-check; FM_FAKE_GIT_BUSY - # names a busy-state file overwritten with a valid busy record, simulating - # a worker that starts a turn on the instruction during the checkpoint; - # FM_FAKE_GIT_RM names a file deleted outright, simulating the busy-state - # record becoming unavailable mid-checkpoint. + # pre-invocation check and fm-control's in-lock re-check. cat > "$fb/git" <<'SH' #!/usr/bin/env bash set -u @@ -346,13 +345,6 @@ if [ "${1:-}" = "-C" ] && [ "${3:-}" = "status" ]; then mkdir -p "${FM_FAKE_GIT_MOVE%/*}/handled" mv "$FM_FAKE_GIT_MOVE" "${FM_FAKE_GIT_MOVE%/*}/handled/" fi - if [ -n "${FM_FAKE_GIT_BUSY:-}" ]; then - printf 'v1 gen=gentest seq=3 state=busy source=omp-ext event=turn-start ts=1\n' \ - > "$FM_FAKE_GIT_BUSY" - fi - if [ -n "${FM_FAKE_GIT_RM:-}" ]; then - rm -f "$FM_FAKE_GIT_RM" - fi fi exec /usr/bin/git "$@" SH @@ -564,9 +556,10 @@ test_missing_endpoint_recovers_via_control() { pass "missing endpoint: real relaunch publishes, ladder resets, stale receipts retire, record stays unhandled" } -# Live endpoint whose agent is idle (turn ended, record still unhandled): the -# live-non-turning path invokes the lifecycle verb. -test_live_non_turning_recovers() { +# A live endpoint is detected and escalated only: recovery never interrupts, +# exits, or relaunches a session that exists, whatever its busy state. The +# lifecycle verb is never invoked and no transport is sent. +test_live_idle_escalates() { local rec id record id=$(case_id live-idle) rec=$(make_case live-idle "$id" pool) @@ -581,16 +574,20 @@ test_live_non_turning_recovers() { live_window "$CASE_DIR" "$id" bun run_recovery "$CASE_DIR" "$HOME_DIR" "$id" "$record" ladder-exhausted - expect_code 0 "$RECOVERY_STATUS" "live-non-turning recovery should exit 0; got: $RECOVERY_OUT" - assert_contains "$RECOVERY_OUT" "verdict=deferred" "live-non-turning recovery did not defer pending the episode" - assert_contains "$RECOVERY_OUT" "live-non-turning" "verdict did not name the live path" - assert_grep "relaunch" "$CASE_DIR/control.log" "the lifecycle verb was not invoked" - pass "live non-turning worker: idle verdict plus clean custody publishes the relaunch" + assert_contains "$RECOVERY_OUT" "verdict=escalate" "a live idle worker did not escalate" + assert_contains "$RECOVERY_OUT" "live" "the escalation did not name the live endpoint" + assert_absent "$CASE_DIR/control.log" "the lifecycle verb ran against a live worker" + assert_no_grep "Escape" "$CASE_DIR/fake/tmux.log" "an interrupt key was sent to a live worker" + assert_no_grep "/exit" "$CASE_DIR/fake/tmux.log" "an exit command was sent to a live worker" + assert_no_grep "new-window" "$CASE_DIR/fake/tmux.log" "a replacement window was created for a live worker" + assert_present "$record" "the unhandled instruction record was moved or deleted" + pass "live idle worker: recovery escalates without interrupt, exit, or relaunch" } -# A live endpoint with a provably busy agent defers: the worker may be -# mid-turn on the instruction. -test_live_busy_defers() { +# A live endpoint with a provably busy agent escalates the same way: the +# busy verdict is never consulted because no lifecycle action is ever taken +# on a live worker. +test_live_busy_escalates() { local rec id record id=$(case_id live-busy) rec=$(make_case live-busy "$id" pool) @@ -605,17 +602,19 @@ test_live_busy_defers() { live_window "$CASE_DIR" "$id" bun run_recovery "$CASE_DIR" "$HOME_DIR" "$id" "$record" ladder-exhausted - assert_contains "$RECOVERY_OUT" "verdict=deferred" "a busy worker did not defer" - assert_absent "$CASE_DIR/control.log" "the lifecycle verb ran against a busy worker" - pass "live busy worker: recovery defers without a lifecycle action" + assert_contains "$RECOVERY_OUT" "verdict=escalate" "a live busy worker did not escalate" + assert_absent "$CASE_DIR/control.log" "the lifecycle verb ran against a live worker" + assert_no_grep "Escape" "$CASE_DIR/fake/tmux.log" "an interrupt key was sent to a live worker" + assert_no_grep "new-window" "$CASE_DIR/fake/tmux.log" "a replacement window was created for a live worker" + pass "live busy worker: recovery escalates without any lifecycle action" } -# A live endpoint with no semantic busy proof escalates: unknown is never a -# custody proof. -test_live_unknown_busy_escalates() { +# Unknown crew-state fails closed on the missing-endpoint path: missing, +# unknown, unproven, and stale custody all refuse and escalate. +test_unknown_crew_state_escalates() { local rec id record - id=$(case_id live-unknown) - rec=$(make_case live-unknown "$id" pool) + id=$(case_id unknown-crew) + rec=$(make_case unknown-crew "$id" pool) read_case "$rec" write_pool_state "$CASE_DIR" "$WT_DIR" "fm-$id" write_slot_marker "$SLOT_DIR" "$id" "$HOME_DIR" @@ -623,12 +622,13 @@ test_live_unknown_busy_escalates() { create_prior_artifacts "$HOME_DIR/state" "$id" write_inbox "$HOME_DIR/state" "$id" 001 record="$HOME_DIR/state/$id.inbox/001.msg" - live_window "$CASE_DIR" "$id" bun + missing_window "$CASE_DIR" "$id" - run_recovery "$CASE_DIR" "$HOME_DIR" "$id" "$record" ladder-exhausted - assert_contains "$RECOVERY_OUT" "verdict=escalate" "an unprovable busy verdict did not escalate" - assert_absent "$CASE_DIR/control.log" "the lifecycle verb ran without a busy proof" - pass "live worker with no busy proof: recovery escalates" + FM_FAKE_CREW_STATE=unknown \ + run_recovery "$CASE_DIR" "$HOME_DIR" "$id" "$record" endpoint-unavailable + assert_contains "$RECOVERY_OUT" "verdict=escalate" "unknown crew-state did not escalate" + assert_absent "$CASE_DIR/control.log" "the lifecycle verb ran with unproven crew-state" + pass "unknown crew-state: recovery escalates; unproven custody fails closed" } # A record already handled ends the episode quietly - no lifecycle action. @@ -913,68 +913,6 @@ test_held_lifecycle_lock_defers() { pass "held lifecycle lock: recovery defers rather than racing another lifecycle action" } -# The generated OMP extension must serialize busy-state writes: a turn_end's -# idle can never land after a following turn_start's busy, or a live worker -# reads falsely idle (and a dead one falsely busy suppresses recovery). The -# fake FM_ROOT wraps fm-busy-event.sh with a delay on the turn-end write, so -# an unserialized pair deterministically inverts. -test_omp_ext_serializes_busy_events() { - local rec id record fakeroot ext out tool - id=$(case_id omp-order) - rec=$(make_case omp-order "$id" pool) - read_case "$rec" - write_pool_state "$CASE_DIR" "$WT_DIR" "fm-$id" - write_slot_marker "$SLOT_DIR" "$id" "$HOME_DIR" - write_meta "$HOME_DIR/state/$id.meta" "$id" "$WT_DIR" "$PROJ_DIR" - create_prior_artifacts "$HOME_DIR/state" "$id" - write_inbox "$HOME_DIR/state" "$id" 001 - record="$HOME_DIR/state/$id.inbox/001.msg" - missing_window "$CASE_DIR" "$id" - - # Fake FM_ROOT: every bin entry is the real script except fm-busy-event.sh, - # which delays the turn-end apply so an unserialized turn-start write would - # land first and leave the worker falsely busy. - fakeroot="$CASE_DIR/fakeroot" - mkdir -p "$fakeroot/bin" "$fakeroot/.omp/extensions" - for tool in "$ROOT"/bin/*; do - [ "$(basename "$tool")" = fm-busy-event.sh ] || ln -s "$tool" "$fakeroot/bin/$(basename "$tool")" - done - ln -s "$ROOT/.omp/extensions/lib" "$fakeroot/.omp/extensions/lib" - cat > "$fakeroot/bin/fm-busy-event.sh" < { handlers[n] = fn; } }); - handlers["turn_end"](); - handlers["turn_start"](); - await new Promise((r) => setTimeout(r, 3000)); - ' || fail "driving the generated OMP extension failed" - out=$(cat "$HOME_DIR/state/$id.busy-state" 2>/dev/null || true) - case "$out" in - *"state=busy"*"event=turn-start"*) ;; - *) fail "turn_end's idle write landed after turn_start's busy (final record: '${out:-missing}'); the extension does not serialize busy events" ;; - esac - pass "OMP extension serializes busy-state writes: turn-start busy lands after turn-end idle" -} - # A record handled in the gap between the caller's pre-invocation check and # fm-control's in-lock re-check must still cancel the relaunch: the fake git # moves the record to handled/ during safe_checkpoint, so fm-control's own @@ -1007,44 +945,6 @@ test_in_lock_handled_record_cancels_relaunch() { pass "in-lock handled record: fm-control cancels the relaunch before touching the agent" } -# A worker that goes busy DURING the checkpoint - publishing a valid -# turn-start busy record while its instruction stays unhandled - must never -# be interrupted: the supervised relaunch cancels with the deferred verdict, -# sends no interrupt or exit keys, creates no replacement window, and leaves -# the worker's instructions byte-exact. The fake git publishes the busy -# record inside fm-control's safe_checkpoint, after every earlier custody -# proof already saw an idle worker. -test_busy_during_checkpoint_defers() { - local rec id record - id=$(case_id checkpoint-busy) - rec=$(make_case checkpoint-busy "$id" pool) - read_case "$rec" - write_pool_state "$CASE_DIR" "$WT_DIR" "fm-$id" - write_slot_marker "$SLOT_DIR" "$id" "$HOME_DIR" - write_meta "$HOME_DIR/state/$id.meta" "$id" "$WT_DIR" "$PROJ_DIR" - create_prior_artifacts "$HOME_DIR/state" "$id" - write_inbox "$HOME_DIR/state" "$id" 001 - record="$HOME_DIR/state/$id.inbox/001.msg" - write_busy "$HOME_DIR/state" "$id" idle - live_window "$CASE_DIR" "$id" bun - cp -p "$HOME_DIR/data/$id/brief.md" "$CASE_DIR/brief.orig" - - FM_FAKE_GIT_BUSY="$HOME_DIR/state/$id.busy-state" \ - FM_STALL_RECOVERY_CONTROL_BIN="$CONTROL" \ - run_recovery "$CASE_DIR" "$HOME_DIR" "$id" "$record" ladder-exhausted - expect_code 0 "$RECOVERY_STATUS" "busy-during-checkpoint recovery should exit 0; got: $RECOVERY_OUT" - assert_contains "$RECOVERY_OUT" "verdict=deferred" "a worker that went busy during the checkpoint did not defer" - assert_no_grep "Escape" "$CASE_DIR/fake/tmux.log" "an interrupt key was sent to a worker that went busy during the checkpoint" - assert_no_grep "/exit" "$CASE_DIR/fake/tmux.log" "an exit command was sent to a worker that went busy during the checkpoint" - assert_no_grep "new-window" "$CASE_DIR/fake/tmux.log" "the relaunch created a window for a worker that went busy" - assert_grep "cancelled:worker-busy" "$HOME_DIR/state/$id.control-relaunch" "the journal did not record the busy-worker cancellation" - assert_present "$record" "the unhandled instruction record was moved or deleted" - assert_grep "001.msg" "$HOME_DIR/state/$id.inbox/.recovery-attempts" "the deferred episode did not record its attempt bound" - cmp -s "$CASE_DIR/brief.orig" "$HOME_DIR/data/$id/brief.md" \ - || fail "a busy-cancelled relaunch left the worker's instructions modified" - pass "busy during checkpoint: recovery defers without interrupt, exit, or relaunch" -} - # A well-formed attempt marker naming a record that was since handled must # not deny the next queued record its own first attempt: structure is # validated separately from identity, so 002.msg recovers with a fresh count @@ -1125,47 +1025,12 @@ test_unterminated_marker_suffix_escalates() { pass "unterminated marker suffix: recovery escalates instead of parsing the valid prefix" } -# When the busy-state record becomes unavailable during the checkpoint, the -# final gate can no longer prove custody: the supervised relaunch cancels -# with escalation (not deferral), sends nothing to the agent, and restores -# the worker's instructions byte-exact. The fake git deletes the busy-state -# file inside fm-control's safe_checkpoint. -test_unproven_custody_during_checkpoint_escalates() { - local rec id record - id=$(case_id custody-gone) - rec=$(make_case custody-gone "$id" pool) - read_case "$rec" - write_pool_state "$CASE_DIR" "$WT_DIR" "fm-$id" - write_slot_marker "$SLOT_DIR" "$id" "$HOME_DIR" - write_meta "$HOME_DIR/state/$id.meta" "$id" "$WT_DIR" "$PROJ_DIR" - create_prior_artifacts "$HOME_DIR/state" "$id" - write_inbox "$HOME_DIR/state" "$id" 001 - record="$HOME_DIR/state/$id.inbox/001.msg" - write_busy "$HOME_DIR/state" "$id" idle - live_window "$CASE_DIR" "$id" bun - cp -p "$HOME_DIR/data/$id/brief.md" "$CASE_DIR/brief.orig" - - FM_FAKE_GIT_RM="$HOME_DIR/state/$id.busy-state" \ - FM_STALL_RECOVERY_CONTROL_BIN="$CONTROL" \ - run_recovery "$CASE_DIR" "$HOME_DIR" "$id" "$record" ladder-exhausted - expect_code 0 "$RECOVERY_STATUS" "unproven-custody recovery should exit 0; got: $RECOVERY_OUT" - assert_contains "$RECOVERY_OUT" "verdict=escalate" "unproven custody during the checkpoint did not escalate" - assert_no_grep "Escape" "$CASE_DIR/fake/tmux.log" "an interrupt key was sent while custody was unproven" - assert_no_grep "/exit" "$CASE_DIR/fake/tmux.log" "an exit command was sent while custody was unproven" - assert_no_grep "new-window" "$CASE_DIR/fake/tmux.log" "the relaunch created a window while custody was unproven" - assert_grep "cancelled:custody-unproven" "$HOME_DIR/state/$id.control-relaunch" "the journal did not record the unproven-custody cancellation" - assert_present "$record" "the unhandled instruction record was moved or deleted" - cmp -s "$CASE_DIR/brief.orig" "$HOME_DIR/data/$id/brief.md" \ - || fail "an unproven-custody cancellation left the worker's instructions modified" - pass "unproven custody during checkpoint: recovery escalates with no transport and restored instructions" -} - # --- run --------------------------------------------------------------------- test_missing_endpoint_recovers_via_control -test_live_non_turning_recovers -test_live_busy_defers -test_live_unknown_busy_escalates +test_live_idle_escalates +test_live_busy_escalates +test_unknown_crew_state_escalates test_handled_record_recovers_quietly test_late_handled_cancels_relaunch test_dirty_worktree_recovers_preserving_work @@ -1178,11 +1043,8 @@ test_secondmate_kind_escalates test_refused_relaunch_preserves_receipts test_held_lifecycle_lock_defers test_in_lock_handled_record_cancels_relaunch -test_omp_ext_serializes_busy_events -test_busy_during_checkpoint_defers test_next_record_gets_own_attempt test_malformed_attempt_marker_escalates test_unterminated_marker_suffix_escalates -test_unproven_custody_during_checkpoint_escalates pass "all stall-recovery tests" From 3ae520e74208f09326c867b8855ad6a1bc977d68 Mon Sep 17 00:00:00 2001 From: dnth Date: Thu, 24 Sep 2026 19:44:55 +0800 Subject: [PATCH 16/21] docs: scripts.md stall-recovery description matches the captain-ruled shape --- docs/scripts.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/scripts.md b/docs/scripts.md index 28ada5e845a..cbed0d043c7 100644 --- a/docs/scripts.md +++ b/docs/scripts.md @@ -113,7 +113,7 @@ The shared no-mistakes gate refusal for fleet lifecycle entrypoints is summarize | `fm-classify-lib.sh` | Shared wake and status-span classification, ship evidence gate, durable keyed-decision folds, status cursors, and unread informational status-line selection | | `fm-send.sh` | Enqueue ordinary local task text durably, or type remote task text, slash commands, Codex dollar invocations, explicit targets, and keys through the recorded backend | | `fm-task-inbox-lib.sh` | Own sequenced steering records, handled-file acknowledgement, the constant doorbell, and the watcher retry ladder | -| `fm-stall-recovery.sh` | Custody-checked bounded auto-recovery for a stalled worker before the watcher publishes a stale wake | +| `fm-stall-recovery.sh` | Custody-checked bounded auto-recovery before a stale wake: live endpoints escalate untouched, missing endpoints relaunch | | `fm-busy-lib.sh` | Single owner of the semantic busy-state contract: verdicts, source attribution, and per-harness sources | | `fm-busy-event.sh` | The only writer of a task's semantic busy-state record; arms an incarnation and applies lifecycle events | | `fm-tmux-lib.sh` | Shared tmux pane primitives for composer capture, verified submit, and the submit-time busy check | From 3166564486e53e240ea84dc03725ed643dea6d11 Mon Sep 17 00:00:00 2001 From: dnth Date: Thu, 24 Sep 2026 19:52:50 +0800 Subject: [PATCH 17/21] no-mistakes(review): Closed relaunch custody race and inbox read failures --- bin/fm-control.sh | 14 ++++++++++++-- bin/fm-stall-recovery.sh | 18 +++++++++++++----- bin/fm-task-inbox-lib.sh | 10 ++++++++-- 3 files changed, 33 insertions(+), 9 deletions(-) diff --git a/bin/fm-control.sh b/bin/fm-control.sh index 770c1e5cfff..7d59d699d57 100755 --- a/bin/fm-control.sh +++ b/bin/fm-control.sh @@ -899,7 +899,14 @@ do_relaunch() { # rollback trap has nothing left to do and the worker's instructions stay # untouched when no relaunch happened. if [ -n "$STALL_RECORD" ]; then - stall_oldest=$(fm_task_inbox_oldest_unhandled "$STATE" "$ID" 2>/dev/null || true) + if stall_oldest=$(fm_task_inbox_oldest_unhandled "$STATE" "$ID" 2>/dev/null); then + : + else + stall_oldest_rc=$? + [ "$stall_oldest_rc" -eq 1 ] \ + && stall_relaunch_cancel record-resolved "stall record resolved (inbox empty; instruction handled)" 3 + die "relaunch refused: stall inbox is unreadable inside the lifecycle lock" + fi if [ -z "$stall_oldest" ]; then stall_relaunch_cancel record-resolved "stall record resolved (inbox empty; instruction handled)" 3 elif [ "${stall_oldest##*/}" != "$STALL_RECORD" ]; then @@ -907,8 +914,11 @@ do_relaunch() { fi fi - journal_write stopping "${CHECKPOINT_LINES[@]}" "$note_line" state=$(agent_state) + if [ -n "$STALL_RECORD" ] && [ "$state" != missing ]; then + die "supervised relaunch refused: endpoint state '$state' is not positively absent" + fi + journal_write stopping "${CHECKPOINT_LINES[@]}" "$note_line" if [ "$state" = missing ]; then # The recorded endpoint is authoritatively absent, so there is no agent to # stop: the exit phase is already complete and the launch below recreates diff --git a/bin/fm-stall-recovery.sh b/bin/fm-stall-recovery.sh index ee5f2cf2075..aaefb21bf6d 100755 --- a/bin/fm-stall-recovery.sh +++ b/bin/fm-stall-recovery.sh @@ -137,9 +137,12 @@ case "$KIND" in ''|ship|scout) ;; *) verdict escalate "kind=$KIND is not an ordi # The named record must still be the oldest unhandled instruction. A record # that moved to handled/ (or an emptied inbox) ends the episode quietly. dir=$(fm_task_inbox_dir "$STATE" "$ID") -oldest=$(fm_task_inbox_oldest_unhandled "$STATE" "$ID" 2>/dev/null || true) -if [ -z "$oldest" ]; then - verdict recovered "inbox empty; instruction already handled" +if oldest=$(fm_task_inbox_oldest_unhandled "$STATE" "$ID" 2>/dev/null); then + : +else + oldest_rc=$? + [ "$oldest_rc" -eq 1 ] && verdict recovered "inbox empty; instruction already handled" + verdict escalate "inbox unreadable; refusing recovery" fi if [ "$oldest" != "$RECORD" ]; then verdict deferred "record ${RECORD##*/} no longer the oldest unhandled (${oldest##*/} is); late handling cancels this action" @@ -236,8 +239,13 @@ STALL_LOCK_HELD=1 # same way a live one does), then re-prove the record itself LAST so a # handled move during the custody probe still cancels the action. prove_custody || verdict escalate "$CUSTODY_DETAIL" -oldest=$(fm_task_inbox_oldest_unhandled "$STATE" "$ID" 2>/dev/null || true) -[ -n "$oldest" ] || verdict recovered "inbox emptied before relaunch; instruction handled" +if oldest=$(fm_task_inbox_oldest_unhandled "$STATE" "$ID" 2>/dev/null); then + : +else + oldest_rc=$? + [ "$oldest_rc" -eq 1 ] && verdict recovered "inbox emptied before relaunch; instruction handled" + verdict escalate "inbox unreadable inside lifecycle lock; refusing recovery" +fi [ "$oldest" = "$RECORD" ] || verdict deferred "record ${RECORD##*/} handled or superseded before relaunch" # Bounded retry: exactly one automatic relaunch per stalled instruction diff --git a/bin/fm-task-inbox-lib.sh b/bin/fm-task-inbox-lib.sh index 7e7e05205a1..72afd3f884b 100644 --- a/bin/fm-task-inbox-lib.sh +++ b/bin/fm-task-inbox-lib.sh @@ -377,6 +377,8 @@ fm_task_inbox_ring() { # [expected-label] [har fm_task_inbox_oldest_unhandled() { # local dir best='' best_n=0 f n dir=$(fm_task_inbox_dir "$1" "$2") + [ -e "$dir" ] || return 1 + [ -d "$dir" ] && [ -r "$dir" ] && [ -x "$dir" ] || return 2 for f in "$dir"/*.msg; do [ -e "$f" ] || continue n=$(fm_task_inbox_seq_of "${f##*/}") || continue @@ -397,9 +399,13 @@ fm_task_inbox_oldest_unhandled() { # # An empty inbox also resets the ladder bookkeeping so the next message starts # a fresh ladder. fm_task_inbox_due_action() { # - local dir oldest base now grace max ladder rec_base count last + local dir oldest base now grace max ladder rec_base count last oldest_rc dir=$(fm_task_inbox_dir "$1" "$2") - if ! oldest=$(fm_task_inbox_oldest_unhandled "$1" "$2"); then + if oldest=$(fm_task_inbox_oldest_unhandled "$1" "$2"); then + : + else + oldest_rc=$? + [ "$oldest_rc" -eq 1 ] || return "$oldest_rc" rm -f "$dir/.ring-state" "$dir/.escalated" "$dir/.recovery-attempts" 2>/dev/null || true printf 'quiet' return 0 From 0738349715c04aecca571cd1352f1ec9525af2c3 Mon Sep 17 00:00:00 2001 From: dnth Date: Thu, 24 Sep 2026 19:59:14 +0800 Subject: [PATCH 18/21] no-mistakes(review): Propagated unreadable inboxes to stale-wake escalation --- bin/fm-watch.sh | 40 ++++++++++++++++++++++++++++++++++------ 1 file changed, 34 insertions(+), 6 deletions(-) diff --git a/bin/fm-watch.sh b/bin/fm-watch.sh index 96c5cfe01db..3880c6e0b76 100755 --- a/bin/fm-watch.sh +++ b/bin/fm-watch.sh @@ -340,16 +340,28 @@ window_label() { [ -n "$task" ] && printf 'fm-%s' "$task" } +inbox_steer_inbox_error() { # + local w=$1 task=$2 reason + reason="stale: $w (the steering inbox for $task is unreadable; durable instructions cannot be verified, so inspect the inbox)" + fm_wake_append stale "$w" "$reason" || exit 1 + wake "$reason" +} + # Surface one stale wake for an unhandled steer whose endpoint is positively # dead or missing: the doorbell was never typed, so the record goes straight to # recovery instead of walking the re-ring ladder. The marker write happens after # the durable queue append, so a crash between them can only produce a rare # duplicate, never a lost wake. inbox_steer_escalate_unavailable() { # - local w=$1 task=$2 rec=$3 reason + local w=$1 task=$2 rec=$3 reason rc reason="stale: $w (unread firstmate instruction: $rec is unhandled and the worker's agent has exited or its endpoint is missing, so the doorbell was not typed; recover the worker)" if [ ! -d "${rec%/*}" ] || [ ! -f "$rec" ]; then - fm_task_inbox_due_action "$STATE" "$task" >/dev/null || true + if fm_task_inbox_due_action "$STATE" "$task" >/dev/null; then + : + else + rc=$? + [ "$rc" -eq 2 ] && inbox_steer_inbox_error "$w" "$task" + fi return 0 fi if inbox_steer_attempt_recovery "$w" "$task" "$rec" "endpoint-unavailable"; then @@ -400,9 +412,15 @@ inbox_steer_attempt_recovery() { # } inbox_steer_check() { # - local window=$1 task=$2 action verb record count tail40 reason ring_rc + local window=$1 task=$2 action verb record count tail40 reason ring_rc rc local meta backend label harness omp_runtime omp_bin agent_state - action=$(fm_task_inbox_due_action "$STATE" "$task") || return 0 + if action=$(fm_task_inbox_due_action "$STATE" "$task" 2>/dev/null); then + : + else + rc=$? + [ "$rc" -eq 2 ] && inbox_steer_inbox_error "$window" "$task" + return 0 + fi verb=${action%% *} [ "$verb" != quiet ] || return 0 record=${action#* } @@ -438,7 +456,12 @@ inbox_steer_check() { # fi if ! fm_task_inbox_record_ring "$STATE" "$task" "$record"; then if [ ! -f "$record" ]; then - fm_task_inbox_due_action "$STATE" "$task" >/dev/null || true + if fm_task_inbox_due_action "$STATE" "$task" >/dev/null; then + : + else + rc=$? + [ "$rc" -eq 2 ] && inbox_steer_inbox_error "$window" "$task" + fi return 0 fi if [ -d "${record%/*}" ]; then @@ -452,7 +475,12 @@ inbox_steer_check() { # escalate) reason="stale: $window (unread firstmate instruction: $record still unhandled after $count doorbell delivery attempts with an idle pane; inspect the worker)" if [ ! -d "${record%/*}" ] || [ ! -f "$record" ]; then - fm_task_inbox_due_action "$STATE" "$task" >/dev/null || true + if fm_task_inbox_due_action "$STATE" "$task" >/dev/null; then + : + else + rc=$? + [ "$rc" -eq 2 ] && inbox_steer_inbox_error "$window" "$task" + fi return 0 fi if inbox_steer_attempt_recovery "$window" "$task" "$record" "ladder-exhausted"; then From 01e0dfa9764882e09d6bb8eb5735e490e4044fdd Mon Sep 17 00:00:00 2001 From: dnth Date: Thu, 24 Sep 2026 20:08:14 +0800 Subject: [PATCH 19/21] no-mistakes(review): Permit dead endpoints and recheck before relaunch --- bin/fm-control.sh | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/bin/fm-control.sh b/bin/fm-control.sh index 7d59d699d57..812ea5e5b35 100755 --- a/bin/fm-control.sh +++ b/bin/fm-control.sh @@ -856,6 +856,15 @@ do_relaunch() { local exit_result state note_line stall_oldest local -a spawn_args + stall_relaunch_absence_guard() { + [ -n "$STALL_RECORD" ] || return 0 + state=$(agent_state) + case "$state" in + dead|missing) ;; + *) die "supervised relaunch refused: endpoint state '$state' is not positively absent" ;; + esac + } + require_state_verified_backend relaunch resolve_relaunch_profile @@ -914,12 +923,9 @@ do_relaunch() { fi fi - state=$(agent_state) - if [ -n "$STALL_RECORD" ] && [ "$state" != missing ]; then - die "supervised relaunch refused: endpoint state '$state' is not positively absent" - fi + stall_relaunch_absence_guard journal_write stopping "${CHECKPOINT_LINES[@]}" "$note_line" - if [ "$state" = missing ]; then + if [ "$state" = dead ] || [ "$state" = missing ]; then # The recorded endpoint is authoritatively absent, so there is no agent to # stop: the exit phase is already complete and the launch below recreates # the endpoint in the checkpointed worktree. Only a proven-missing endpoint @@ -941,6 +947,7 @@ do_relaunch() { # before arming the new one, so nothing to do here. RELAUNCH_TX="${BASHPID:-$$}.$(date -u +%Y%m%dT%H%M%SZ).$RANDOM" journal_write launching "${CHECKPOINT_LINES[@]}" "$note_line" "relaunch_tx=$RELAUNCH_TX" + stall_relaunch_absence_guard if [ "$KIND" = secondmate ] && [ "$PRIOR_HARNESS" = omp ]; then spawn_args=("$ID" --secondmate --harness "$TARGET_HARNESS") else From fd6134a161e1907e9dd694ac84fe248fe4915628 Mon Sep 17 00:00:00 2001 From: dnth Date: Thu, 24 Sep 2026 20:14:25 +0800 Subject: [PATCH 20/21] no-mistakes(document): Document absent-endpoint stall recovery behavior --- docs/agent-control.md | 2 +- docs/architecture.md | 4 ++-- docs/scripts.md | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/docs/agent-control.md b/docs/agent-control.md index b2079fa18d6..e841914b320 100644 --- a/docs/agent-control.md +++ b/docs/agent-control.md @@ -78,7 +78,7 @@ It is not deterministic across the verified adapters: codex and grok resume only Switching harness is therefore one ordinary relaunch rather than a separate mechanism. -The watcher-triggered stall-recovery path uses the same transactional relaunch under its lifecycle lock, but only for a positively missing endpoint - a live worker is never interrupted and always escalates; the custody and verdict contract lives in [`architecture.md`](architecture.md#event-driven-supervision). +The watcher-triggered stall-recovery path uses the same transactional relaunch under its lifecycle lock, but only for a positively absent endpoint (dead or missing) - a live worker is never interrupted and always escalates; the custody and verdict contract lives in [`architecture.md`](architecture.md#event-driven-supervision). ### Failure and rollback diff --git a/docs/architecture.md b/docs/architecture.md index f53e2c9d597..12f44dfe55e 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -138,10 +138,10 @@ For an OMP worker the loaded extension delivers the doorbell through `sendMessag The generated OMP extension publishes `.omp-ready` only after the doorbell activates; activation or drain failure retires `.omp-doorbell-ready` and durably journals the reason in `.omp-doorbell-failed`, while `fm-spawn.sh` bounded-waits for readiness and `fm-send.sh` names the missing marker or failure journal when refusing native delivery. When the runtime downgrades `triggerTurn` to append-only, the extension re-drives the instruction through `sendUserMessage` only after its bounded grace expires without any turn opening; the re-drive is itself only a request, so a nonthrowing return is never a receipt - the entry re-parks for one more bounded proof window and leaves a durable `.unproven` marker when no turn opens, allowing the next ring to publish a fresh pending request (`.omp/extensions/lib/fm-task-inbox-doorbell.ts`). Consumed OMP delivery receipts retire as durable `.pending.acked` tombstones, so later rings report delivery without republishing or sending another doorbell; the requests directory is generation-scoped and reset with the task lifecycle. -Before either stale wake publishes, `bin/fm-stall-recovery.sh` runs a custody-checked bounded auto-recovery: it re-proves the record is still the oldest unhandled instruction and classifies the endpoint. A live endpoint always escalates - recovery never interrupts, exits, or relaunches a session that exists - while a positively missing endpoint may relaunch after proving a clean non-run crew-state and the durable `fm-` Treehouse lease, re-proving the whole chain inside fm-control's lifecycle lock immediately before `fm-control.sh relaunch`. The missing-endpoint path needs no busy-state proof: there is no live worker to interrupt. +Before either stale wake publishes, `bin/fm-stall-recovery.sh` runs a custody-checked bounded auto-recovery: it re-proves the record is still the oldest unhandled instruction and classifies the endpoint. A live endpoint always escalates - recovery never interrupts, exits, or relaunches a session that exists - while a positively absent endpoint (dead or missing) may relaunch after proving a clean non-run crew-state and the durable `fm-` Treehouse lease, re-proving the whole chain inside fm-control's lifecycle lock immediately before `fm-control.sh relaunch`. The absent-endpoint path needs no busy-state proof: there is no live worker to interrupt. Uncommitted changes and unpushed commits are deliberately preserved rather than treated as blockers: the relaunch inherits the same worktree, branch, and commits untouched. The verdict is `recovered` when the record was already handled, `deferred` when the relaunch just published (the episode stays pending until the record is handled or the reset ladder re-escalates), and `escalate` for every unprovable or unsafe shape - including every live endpoint - which keeps the ordinary stale wake with the helper's reason appended. -One automatic relaunch per stalled record on the missing-endpoint path is bounded by `state/.inbox/.recovery-attempts`: the bound is per-record, so a well-formed marker naming a record that was since handled is replaced atomically and the new oldest record starts its own count at zero, while an observed empty inbox clears the marker entirely. +One automatic relaunch per stalled record on the absent-endpoint path is bounded by `state/.inbox/.recovery-attempts`: the bound is per-record, so a well-formed marker naming a record that was since handled is replaced atomically and the new oldest record starts its own count at zero, while an observed empty inbox clears the marker entirely. An OMP relaunch also retires the prior generation's `request.*` doorbell receipts after every refusal gate and immediately before the replacement launch, so a stale `.acked` tombstone cannot suppress the new incarnation's doorbell. An OMP worker is reached only through its task-bound native receive adapter, never the composer, because an already-streaming session cannot be steered through editable terminal text; `fm-send.sh` reports one bounded outcome per steer - native receipt, a named durable native queue entry, or an explicit refusal - each binding the exact session and message. Normal local metadata publication, the Orca abort-recovery publication, inbox enqueue revalidation and record publication, and teardown share the per-task metadata lifecycle lock so endpoint birth, delivery, and retirement cannot cross. diff --git a/docs/scripts.md b/docs/scripts.md index cbed0d043c7..8d0082ae301 100644 --- a/docs/scripts.md +++ b/docs/scripts.md @@ -113,7 +113,7 @@ The shared no-mistakes gate refusal for fleet lifecycle entrypoints is summarize | `fm-classify-lib.sh` | Shared wake and status-span classification, ship evidence gate, durable keyed-decision folds, status cursors, and unread informational status-line selection | | `fm-send.sh` | Enqueue ordinary local task text durably, or type remote task text, slash commands, Codex dollar invocations, explicit targets, and keys through the recorded backend | | `fm-task-inbox-lib.sh` | Own sequenced steering records, handled-file acknowledgement, the constant doorbell, and the watcher retry ladder | -| `fm-stall-recovery.sh` | Custody-checked bounded auto-recovery before a stale wake: live endpoints escalate untouched, missing endpoints relaunch | +| `fm-stall-recovery.sh` | Custody-checked bounded auto-recovery before a stale wake: live endpoints escalate untouched, absent endpoints relaunch | | `fm-busy-lib.sh` | Single owner of the semantic busy-state contract: verdicts, source attribution, and per-harness sources | | `fm-busy-event.sh` | The only writer of a task's semantic busy-state record; arms an incarnation and applies lifecycle events | | `fm-tmux-lib.sh` | Shared tmux pane primitives for composer capture, verified submit, and the submit-time busy check | From 55ac2c9d488e3d43cc05b8003980e234aac634f5 Mon Sep 17 00:00:00 2001 From: dnth Date: Thu, 24 Sep 2026 20:50:08 +0800 Subject: [PATCH 21/21] no-mistakes(ci): Fixed bin/fm-control.sh relaunch state handling: state is now captured for all relaunches and the custody guard runs after the stopping journal immediately before lifecycle control. This resolves both portable serial failures (the reproduced unbound-variable error). Focused dead-endpoint relaunch tests pass. The Herdr failure was an external stale-watcher/worktree-state failure, not caused by this change; no Herdr-specific code was modified --- bin/fm-control.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/bin/fm-control.sh b/bin/fm-control.sh index 812ea5e5b35..13d8e962a4e 100755 --- a/bin/fm-control.sh +++ b/bin/fm-control.sh @@ -857,8 +857,8 @@ do_relaunch() { local -a spawn_args stall_relaunch_absence_guard() { - [ -n "$STALL_RECORD" ] || return 0 state=$(agent_state) + [ -n "$STALL_RECORD" ] || return 0 case "$state" in dead|missing) ;; *) die "supervised relaunch refused: endpoint state '$state' is not positively absent" ;; @@ -923,8 +923,8 @@ do_relaunch() { fi fi - stall_relaunch_absence_guard journal_write stopping "${CHECKPOINT_LINES[@]}" "$note_line" + stall_relaunch_absence_guard if [ "$state" = dead ] || [ "$state" = missing ]; then # The recorded endpoint is authoritatively absent, so there is no agent to # stop: the exit phase is already complete and the launch below recreates