diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index bd3113e69a6..f3e5adcf030 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -446,8 +446,8 @@ jobs: snapshot_output=$(/bin/bash tests/fm-fleet-snapshot-view.test.sh) printf '%s\n' "$snapshot_output" snapshot_count=$(printf '%s\n' "$snapshot_output" | grep -c '^ok - ') - [ "$snapshot_count" -eq 19 ] || { - echo "::error::expected 19 snapshot/fleet-view tests, got $snapshot_count" + [ "$snapshot_count" -eq 21 ] || { + echo "::error::expected 21 snapshot/fleet-view tests, got $snapshot_count" exit 1 } diff --git a/bin/fm-fleet-snapshot.sh b/bin/fm-fleet-snapshot.sh index f483f6ab7a9..a906f6d5cd8 100755 --- a/bin/fm-fleet-snapshot.sh +++ b/bin/fm-fleet-snapshot.sh @@ -1545,7 +1545,27 @@ snapshot_cleanup() { snapshot_collection_cleanup cleanup_json_files } + +# Belt-and-suspenders for a prior run that was SIGKILL'd before its EXIT trap +# could run: remove task temp directories older than a few hours before creating +# this run's. The age gate keeps a live concurrent snapshot's directory safe, +# while a leaked one ages out and is reaped by a later run. A reap failure is +# never fatal to this snapshot. +snapshot_reap_aged_task_dirs() { + local root=${TMPDIR:-/tmp} minutes=${FM_SNAPSHOT_TMP_REAP_MINUTES:-180} + case "$minutes" in ''|*[!0-9]*|0) return 0 ;; esac + find "$root" -maxdepth 1 -type d -name 'fm-fleet-tasks.*' -mmin +"$minutes" \ + -exec rm -rf {} + 2>/dev/null || true +} + +snapshot_reap_aged_task_dirs +snapshot_on_signal() { # + snapshot_cleanup + exit "$1" +} trap snapshot_cleanup EXIT +trap 'snapshot_on_signal 130' INT +trap 'snapshot_on_signal 143' TERM bounded_parent_activities_json() { # local f=$1 out rc reason script diff --git a/bin/fm-watch.sh b/bin/fm-watch.sh index 950fd2d7081..570bb4d21a0 100755 --- a/bin/fm-watch.sh +++ b/bin/fm-watch.sh @@ -272,8 +272,10 @@ fi POLL=${FM_POLL:-15} # seconds between cycles # The liveness beacon is touched once per cycle, immediately before the # terminal wait below (event_wait_or_sleep) as well as at the top of the next -# one, so a healthy cycle's beacon can legitimately age up to POLL seconds -# between touches. fm_poll_derived_grace (bin/fm-wake-lib.sh, already sourced +# one, and again before each *.check.sh in the serial sweep, so a healthy +# cycle's beacon can legitimately age up to POLL seconds between touches and a +# long but healthy check sweep never reads as a stalled watcher. +# fm_poll_derived_grace (bin/fm-wake-lib.sh, already sourced # transitively above) is the single owner of the max(300, poll+60) # derivation - see docs/turnend-guard.md "Guard grace and the poll cadence". # This recomputes the library default above now that the real configured @@ -2754,6 +2756,10 @@ while :; do contribution_check_output= for c in "$STATE"/*.check.sh; do [ -e "$c" ] || continue + # A serial sweep of up to nine 30s checks can hold this watcher's beacon + # for minutes. Refresh it before each check so a healthy sweep never reads + # as a stalled watcher to the guard or a continuity supervisor. + touch "$STATE/.last-watcher-beat" is_pr_poll=0 if [ "$(basename "$c")" = x-watch.check.sh ]; then if fmx_poll_shim_valid "$c" "$FM_HOME" "$FM_ROOT" \ diff --git a/bin/fm-watcher-continuity.sh b/bin/fm-watcher-continuity.sh new file mode 100755 index 00000000000..b040714bbd0 --- /dev/null +++ b/bin/fm-watcher-continuity.sh @@ -0,0 +1,140 @@ +#!/usr/bin/env bash +# fm-watcher-continuity.sh - keep this home's watcher alive when the harness +# re-arm owner can leave a gap (long OpenCode turns, Cursor park gaps). +# +# Usage: +# fm-watcher-continuity.sh +# +# Singleton through the shared portable lock (no flock dependency). It keeps one +# handling-successor watcher running (FM_WATCH_HANDLING_SUCCESSOR=1 +# bin/fm-watch.sh) and, when another arm already owns a live watcher, attaches +# to that holder instead of starting a second one. A holder is stopped only when +# BOTH its own uptime and the age of state/.last-watcher-beat exceed the stale +# bound, so a fresh start is never killed for a beat left by a previous cycle. +# Only the watcher process touches the beat; this supervisor never does. +# +# The stale bound is floored to max(300, poll+60) because the watcher's own +# grace uses that same derivation and its serial *.check.sh sweep can +# legitimately hold the beat for minutes. A bound below the floor is exactly the +# 2026-09-30 kill loop (70 kills / 125 restarts): the supervisor TERM'd a +# healthy watcher mid-sweep, the sweep never completed, and the next watcher +# restarted the same sweep and was killed again. +# +# REPLACING THIS SCRIPT - read before touching a running supervisor: +# bash parses a whole script into memory at startup, so editing this file does +# NOT change a supervisor already running, and a stale supervisor with an older +# bound can survive beside a new one. Replace it atomically instead: write the +# new content to a temp file, `mv` it over this path, `kill -KILL` the old +# supervisor (it holds state/.watcher-continuity.lock), then start exactly one +# new holder. Never edit a running bash script in place. +# +# Env (all optional): +# FM_CONTINUITY_STALE_SECS stuck bound in seconds; floored as above +# FM_CONTINUITY_POLL_SECS seconds between liveness checks (default 5) +# FM_CONTINUITY_RESTART_SECS seconds between watcher runs (default 2) +set -euo pipefail + +SCRIPT_DIR="$(d=${BASH_SOURCE[0]%/*}; [ "$d" != "${BASH_SOURCE[0]}" ] || d=.; cd "${d:-/}" && 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}" +WATCHER="$SCRIPT_DIR/fm-watch.sh" +WATCH_LOCK="$STATE/.watch.lock" +BEAT="$STATE/.last-watcher-beat" +LOG="$STATE/.watcher-continuity.log" +LOCK="$STATE/.watcher-continuity.lock" +PIDFILE="$STATE/.watcher-continuity.pid" + +[ -x "$WATCHER" ] || { echo "fm-watcher-continuity: watcher not found: $WATCHER" >&2; exit 1; } + +# Portable lock acquisition and pid identity, plus the leaf mtime helper. +# shellcheck source=bin/fm-wake-lib.sh +FM_ROOT_OVERRIDE="$FM_ROOT" FM_HOME="$FM_HOME" FM_STATE_OVERRIDE="$STATE" \ + . "$SCRIPT_DIR/fm-wake-lib.sh" +# shellcheck source=bin/fm-lock-lib.sh +. "$SCRIPT_DIR/fm-lock-lib.sh" + +positive_int_or() { # + case "$1" in ''|*[!0-9]*|0) printf '%s\n' "$2" ;; *) printf '%s\n' "$1" ;; esac +} +POLL_SECS=$(positive_int_or "${FM_CONTINUITY_POLL_SECS:-5}" 5) +RESTART_SECS=$(positive_int_or "${FM_CONTINUITY_RESTART_SECS:-2}" 2) +STALE_FLOOR=$((POLL_SECS + 60)) +[ "$STALE_FLOOR" -ge 300 ] || STALE_FLOOR=300 +STALE_SECS=$(positive_int_or "${FM_CONTINUITY_STALE_SECS:-$STALE_FLOOR}" "$STALE_FLOOR") +[ "$STALE_SECS" -ge "$STALE_FLOOR" ] || STALE_SECS=$STALE_FLOOR + +mkdir -p "$STATE" +log() { printf '[%s] %s\n' "$(date -u +%Y-%m-%dT%H:%M:%SZ)" "$*" >> "$LOG"; } +beat_age() { + local mtime + mtime=$(fm_lock_path_mtime "$BEAT" 2>/dev/null) || { printf 'none\n'; return 0; } + printf '%s\n' "$(( $(date +%s) - mtime ))" +} +lock_pid() { cat "$WATCH_LOCK/pid" 2>/dev/null || true; } + +if ! fm_lock_try_acquire "$LOCK"; then + exit 0 +fi +printf '%s\n' "$$" > "$PIDFILE" +cleanup() { + rm -f "$PIDFILE" 2>/dev/null || true + fm_lock_release "$LOCK" 2>/dev/null || true +} +on_signal() { # + cleanup + exit "$1" +} +trap cleanup EXIT +trap 'on_signal 130' INT +trap 'on_signal 143' TERM +trap 'on_signal 129' HUP + +log "continuity supervisor acquired lock stale=${STALE_SECS}s floor=${STALE_FLOOR}s poll=${POLL_SECS}s" + +while true; do + lp=$(lock_pid) + if fm_pid_alive "$lp"; then + # Another arm owns a live watcher. Attach and watch it, never start a second. + log "attaching to existing watcher pid=$lp" + started_at=$(date +%s) + while fm_pid_alive "$lp"; do + now=$(date +%s) + up=$((now - started_at)) + age=$(beat_age) + if [ "$up" -ge "$STALE_SECS" ] && [ "$age" != "none" ] && [ "$age" -ge "$STALE_SECS" ]; then + log "watcher pid=$lp stuck (up=${up}s beat=${age}s); TERM" + kill -TERM "$lp" 2>/dev/null || true + sleep 0.5 + fm_pid_alive "$lp" && kill -KILL "$lp" 2>/dev/null || true + break + fi + sleep "$POLL_SECS" + lp=$(lock_pid) + [ -n "$lp" ] || break + done + sleep "$RESTART_SECS" + continue + fi + + FM_HOME="$FM_HOME" FM_WATCH_HANDLING_SUCCESSOR=1 "$WATCHER" >> "$STATE/.watch-restart.log" 2>&1 & + wpid=$! + started_at=$(date +%s) + log "started watcher pid=$wpid" + while fm_pid_alive "$wpid"; do + now=$(date +%s) + up=$((now - started_at)) + age=$(beat_age) + if [ "$up" -ge "$STALE_SECS" ] && [ "$age" != "none" ] && [ "$age" -ge "$STALE_SECS" ]; then + log "watcher pid=$wpid stuck (up=${up}s beat=${age}s); TERM" + kill -TERM "$wpid" 2>/dev/null || true + sleep 0.5 + fm_pid_alive "$wpid" && kill -KILL "$wpid" 2>/dev/null || true + break + fi + sleep "$POLL_SECS" + done + wait "$wpid" 2>/dev/null || true + log "watcher pid=$wpid finished" + sleep "$RESTART_SECS" +done diff --git a/docs/scripts.md b/docs/scripts.md index dbe2cde6dbb..575f6275aa2 100644 --- a/docs/scripts.md +++ b/docs/scripts.md @@ -51,6 +51,7 @@ The shared no-mistakes gate lifecycle boundary is summarized in [architecture.md | `fm-primary-scope-lib.sh` | Shared marker-or-plain-checkout primary-home predicate for tracked hooks | | `fm-session-lock-lib.sh` | Shared session-lock ownership from harness ancestry or a trusted Claude session id for fm-lock.sh and the Claude Stop auto-arm, plus the read-only lock inspection behind `fm-lock.sh status` and `fm-inbox.sh ready` | | `fm-claude-stop-autoarm.sh` | Claude Stop `asyncRewake` hook owning tokenless watcher continuity with single-flight exit-2 rewake (docs/watcher-continuity.md) | +| `fm-watcher-continuity.sh` | Belt-and-suspenders watcher supervisor for a home whose re-arm owner can leave a gap, with a floored stuck bound that never undercuts the watcher's own grace | | `fm-turnend-guard.sh` | Shared primary turn-end guard predicate so no turn ends blind (docs/turnend-guard.md) | | `fm-turnend-guard-grok.sh` | Grok Stop-hook adapter for the primary turn-end guard | | `fm-kimi-turnend-hook.sh` | Surgically install or remove Kimi's guarded global crew turn-end hook | diff --git a/tests/fm-fleet-snapshot-view.test.sh b/tests/fm-fleet-snapshot-view.test.sh index 2ce929c11ca..64dd01857aa 100755 --- a/tests/fm-fleet-snapshot-view.test.sh +++ b/tests/fm-fleet-snapshot-view.test.sh @@ -1195,9 +1195,93 @@ test_large_payloads_compose_through_files() { pass "snapshot composes >128KB payloads through files, not argv" } +test_snapshot_term_cleanup_removes_temp_dir() { + # A SIGKILL leaves a task temp dir behind because the EXIT trap never runs. + # A trapped TERM must run snapshot_cleanup and then exit, so a graceful stop + # never leaks the directory either. + local home tmp fakebin pid tempdir status i + home=$(make_home term-cleanup) + printf '## In flight\n' > "$home/data/backlog.md" + fm_write_meta "$home/state/blocker.meta" \ + "window=firstmate:fm-blocker" \ + "worktree=$home/worktree" \ + "project=alpha" \ + "harness=codex" \ + "kind=ship" \ + "mode=ship" \ + "yolo=off" + printf 'working: probe\n' > "$home/state/blocker.status" + tmp="$TMP_ROOT/term-tmp" + mkdir -p "$tmp" + fakebin=$(make_fakebin "$TMP_ROOT/term-fakebin") + # A slow backend probe holds the snapshot in its task-observation wait long + # enough to signal it with its temp dir already created. + cat > "$fakebin/tmux" <<'SH' +#!/usr/bin/env bash +sleep 3 +exit 0 +SH + chmod +x "$fakebin/tmux" + + PATH="$fakebin:$PATH" FM_HOME="$home" TMPDIR="$tmp" \ + FM_SNAPSHOT_CREW_STATE_TIMEOUT=10 "$SNAPSHOT" --json \ + > "$TMP_ROOT/term.out" 2>&1 & + pid=$! + tempdir= + i=0 + while [ "$i" -lt 50 ]; do + tempdir=$(find "$tmp" -maxdepth 1 -type d -name 'fm-fleet-tasks.*' -print 2>/dev/null | head -1) + [ -n "$tempdir" ] && break + sleep 0.1 + i=$((i + 1)) + done + if [ -z "$tempdir" ]; then + kill -KILL "$pid" 2>/dev/null || true + fail "snapshot never created its task temp dir" + fi + + kill -TERM "$pid" + i=0 + while [ "$i" -lt 50 ]; do + kill -0 "$pid" 2>/dev/null || break + sleep 0.1 + i=$((i + 1)) + done + if kill -0 "$pid" 2>/dev/null; then + kill -KILL "$pid" 2>/dev/null || true + fail "SIGTERM must run snapshot cleanup and exit, not no-op" + fi + wait "$pid" 2>/dev/null + status=$? + [ "$status" = 143 ] || fail "SIGTERM must exit 143 after cleanup (got $status)" + [ ! -e "$tempdir" ] || fail "snapshot TERM cleanup must remove its task temp dir" + pass "snapshot TERM cleanup removes its temp dir and exits" +} + +test_snapshot_startup_reap_removes_aged_temp_dirs() { + # Belt-and-suspenders for a SIGKILL'd run: a later snapshot reaps an aged + # /tmp/fm-fleet-tasks.* directory while leaving a fresh concurrent one alone. + local home tmp fakebin + home=$(make_home reap-tmp) + printf '## In flight\n' > "$home/data/backlog.md" + tmp="$TMP_ROOT/reap-tmp" + mkdir -p "$tmp/fm-fleet-tasks.aged" "$tmp/fm-fleet-tasks.fresh" + touch -t 202001010000 "$tmp/fm-fleet-tasks.aged" + fakebin=$(make_fakebin "$home") + PATH="$fakebin:$PATH" FM_HOME="$home" TMPDIR="$tmp" "$SNAPSHOT" --json \ + > /dev/null 2>&1 || fail "snapshot must succeed while reaping aged temp dirs" + [ ! -e "$tmp/fm-fleet-tasks.aged" ] \ + || fail "startup reap must remove an aged task temp dir" + [ -e "$tmp/fm-fleet-tasks.fresh" ] \ + || fail "startup reap must keep a fresh task temp dir" + pass "snapshot startup reap removes aged task temp dirs" +} + test_empty_fleet_json test_fixture_snapshot_json test_large_payloads_compose_through_files +test_snapshot_term_cleanup_removes_temp_dir +test_snapshot_startup_reap_removes_aged_temp_dirs test_home_summary_excludes_secondmate_from_child_inventory test_undated_captain_hold_phrasing_and_aging test_hold_buckets_are_total_and_text_blind diff --git a/tests/fm-watcher-continuity.test.sh b/tests/fm-watcher-continuity.test.sh new file mode 100755 index 00000000000..023682b0ca5 --- /dev/null +++ b/tests/fm-watcher-continuity.test.sh @@ -0,0 +1,76 @@ +#!/usr/bin/env bash +# tests/fm-watcher-continuity.test.sh - behavior of bin/fm-watcher-continuity.sh. +# Two contracts, both through the real script: +# - the stale bound is floored so a mis-set FM_CONTINUITY_STALE_SECS cannot +# recreate the 2026-09-30 kill loop, which TERM'd a healthy watcher whose +# beacon was merely old; +# - a graceful TERM removes the pidfile and actually exits. +set -u + +# shellcheck source=tests/lib.sh +# shellcheck disable=SC1091 +. "$(dirname "${BASH_SOURCE[0]}")/lib.sh" + +CONT="$ROOT/bin/fm-watcher-continuity.sh" +TMP_ROOT=$(fm_test_tmproot fm-watcher-continuity) + +CONT_PIDS=() +cleanup_test() { + local pid + for pid in "${CONT_PIDS[@]:-}"; do + [ -n "$pid" ] || continue + kill -KILL "$pid" 2>/dev/null || true + done + fm_test_cleanup +} +trap cleanup_test EXIT INT TERM + +wait_dead() { # : 0 dead, 1 still alive + local pid=$1 limit=${2:-50} i=0 + while [ "$i" -lt "$limit" ]; do + kill -0 "$pid" 2>/dev/null || return 0 + sleep 0.1 + i=$((i + 1)) + done + return 1 +} + +test_stale_bound_floor_and_graceful_term() { + local home state holder cpid + home="$TMP_ROOT/home" + state="$home/state" + mkdir -p "$state/.watch.lock" + # A live watcher singleton whose beacon is ancient. With the mis-set 1s bound + # applied literally the supervisor would TERM it on the first check, which is + # exactly the kill loop; the floor must hold it instead. + sleep 60 & + holder=$! + disown "$holder" 2>/dev/null || true + CONT_PIDS+=("$holder") + printf '%s\n' "$holder" > "$state/.watch.lock/pid" + touch -t 202001010000 "$state/.last-watcher-beat" + + FM_HOME="$home" FM_CONTINUITY_STALE_SECS=1 FM_CONTINUITY_POLL_SECS=1 \ + "$CONT" > "$TMP_ROOT/continuity.out" 2>&1 & + cpid=$! + CONT_PIDS+=("$cpid") + + sleep 3 + kill -0 "$holder" 2>/dev/null \ + || fail "stale bound floor must not kill a live holder with an ancient beat" + kill -0 "$cpid" 2>/dev/null || fail "continuity supervisor exited before TERM" + grep -q 'attaching to existing watcher' "$state/.watcher-continuity.log" \ + || fail "continuity supervisor must attach to a live holder instead of starting a second watcher" + grep -q 'stale=300s' "$state/.watcher-continuity.log" \ + || fail "continuity supervisor must report the floored stale bound" + + kill -TERM "$cpid" + wait_dead "$cpid" 50 \ + || fail "TERM must stop the continuity supervisor (its handler must exit)" + [ ! -e "$state/.watcher-continuity.pid" ] \ + || fail "TERM must remove the continuity pidfile" + + pass "continuity stale bound is floored and TERM stops the supervisor" +} + +test_stale_bound_floor_and_graceful_term