diff --git a/AGENTS.md b/AGENTS.md index c481bec3442..010e8421459 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -128,7 +128,7 @@ A lock-refused session must not spawn, steer, merge, drain the wake queue, repai When the lock could not be acquired, the worktree-tangle check uses read-only advisory wording without a checkout repair command. The five MUTATING sweeps - non-executing legacy PR-check migration, fleet sync, the local secondmate fast-forward sweep, the secondmate liveness sweep, and X-mode artifact writes - run only when this session actually holds the lock from step 1. The secondmate liveness sweep deterministically guarantees every registered secondmate is actually running: it probes each live secondmate's endpoint for a real agent process (not just pane presence) and respawns only on a confident dead reading, reported as `SECONDMATE_LIVENESS:` lines (`bin/fm-bootstrap.sh`; `bin/fm-backend.sh`'s `fm_backend_agent_alive`). -3. **Wake queue** - when locked, drains the durable wake queue and prints the records prominently as this turn's first work queue, exactly as `bin/fm-wake-drain.sh` did before; a lapsed watcher chain still surfaces here via the same guard banner. +3. **Wake queue** - when locked, drains the durable wake queue and prints the records prominently as this turn's first work queue, exactly as `bin/fm-wake-drain.sh` did before; a lapsed watcher chain still surfaces here via the same guard alarm. When the lock could not be acquired, the queue is left untouched because another session owns it, and the guard's tangle/watcher-liveness alarms still print in read-only advisory mode without drain, supervision repair, or checkout repair commands. 4. **Context digest** - the full contents of `data/projects.md`, `data/secondmates.md`, `data/captain.md`, and `data/learnings.md`, each clearly delimited. A file that does not exist prints an explicit `ABSENT` marker, never confused with an empty-but-present file: absence is meaningful (`captain.md` absent means use this template's defaults, `projects.md` absent means rebuild it from the clones under `projects/`, etc.). diff --git a/bin/fm-guard.sh b/bin/fm-guard.sh index 1fbd678bb3d..024491defc6 100755 --- a/bin/fm-guard.sh +++ b/bin/fm-guard.sh @@ -9,10 +9,14 @@ # liveness beacon (state/.last-watcher-beat, touched every poll cycle) is # missing or older than FM_GUARD_GRACE seconds, prints a loud, clearly delimited # banner so the agent cannot skim past it in the tool output of whatever it was -# doing - the one channel every harness has. Normal wake handling (watcher -# briefly down between a wake and the next supervision resume) stays inside the -# grace window and stays silent. Always exits 0: the guard warns, it never -# blocks. +# doing - the one channel every harness has. The full banner is emitted once per +# distinct staleness episode in this FM_HOME (keyed to beacon mtime or absence); +# later guarded commands in the same episode print a one-line reminder instead. +# Episode state lives only under state/.guard-watcher-stale-banner (volatile, +# bounded). Independent alarms (queued wakes, worktree tangle) are never +# suppressed by that dedup. Normal wake handling (watcher briefly down between a +# wake and the next supervision resume) stays inside the grace window and stays +# silent. Always exits 0: the guard warns, it never blocks. set -u SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" @@ -26,6 +30,10 @@ READ_ONLY=${FM_GUARD_READ_ONLY:-0} case "$READ_ONLY" in 1|true|TRUE|yes|YES) READ_ONLY=1 ;; *) READ_ONLY=0 ;; esac CONTINUE_LINE=${FM_GUARD_CONTINUE_LINE:-This is a supervision warning only; the guarded operation WILL still run.} +# Volatile, home-scoped episode marker: one line = the current stale-episode key. +# Cleared when the home leaves the unhealthy state so a later episode re-arms. +STALE_BANNER_MARKER="$STATE/.guard-watcher-stale-banner" + # shellcheck source=bin/fm-wake-lib.sh . "$SCRIPT_DIR/fm-wake-lib.sh" # shellcheck source=bin/fm-tangle-lib.sh @@ -33,6 +41,78 @@ CONTINUE_LINE=${FM_GUARD_CONTINUE_LINE:-This is a supervision warning only; the # shellcheck source=bin/fm-supervision-lib.sh . "$SCRIPT_DIR/fm-supervision-lib.sh" +# Deterministic episode key from beacon state: same continuous stale beacon +# (or continuous absence) shares a key; a recovered-then-restale beacon gets a +# new mtime and therefore a new episode. +fm_guard_stale_episode_key() { + local state=$1 beat m + beat="$state/.last-watcher-beat" + if [ -e "$beat" ]; then + m=$(fm_sup_stat_mtime "$beat") + printf 'beat:%s\n' "${m:-unknown}" + else + printf 'beat:absent\n' + fi +} + +# Claim the full banner for this episode. Exit 0 = print full banner (this call +# owns the first announcement). Exit 1 = same episode already announced (print +# reminder). The shared wake lock helper owns the race-safety mechanics; the +# re-check under the lock makes concurrent claims idempotent. +fm_guard_claim_stale_banner() { + local state=$1 key=$2 + local marker="$state/.guard-watcher-stale-banner" + local lock="$state/.guard-watcher-stale-banner.lock" + local seen i + + seen=$(cat "$marker" 2>/dev/null || true) + # Strip a single trailing newline so key comparison is line-content based. + seen=${seen%$'\n'} + if [ "$seen" = "$key" ]; then + return 1 + fi + + i=0 + while [ "$i" -lt 50 ]; do + if fm_lock_try_acquire "$lock"; then + seen=$(cat "$marker" 2>/dev/null || true) + seen=${seen%$'\n'} + if [ "$seen" = "$key" ]; then + fm_lock_release "$lock" 2>/dev/null || true + return 1 + fi + # Bounded write: one line, no growth across episodes (overwrite). + printf '%s\n' "$key" > "$marker" || true + fm_lock_release "$lock" 2>/dev/null || true + return 0 + fi + seen=$(cat "$marker" 2>/dev/null || true) + seen=${seen%$'\n'} + if [ "$seen" = "$key" ]; then + return 1 + fi + # Brief yield; 0.02s is fine on macOS/Linux sleep, fall back to 1s. + sleep 0.02 2>/dev/null || sleep 1 + i=$((i + 1)) + done + # Contended past the spin budget: stay loud rather than dropping the alarm. + return 0 +} + +fm_guard_stale_banner_seen() { + local state=$1 key=$2 + local marker="$state/.guard-watcher-stale-banner" + local seen + + seen=$(cat "$marker" 2>/dev/null || true) + seen=${seen%$'\n'} + [ "$seen" = "$key" ] +} + +fm_guard_clear_stale_banner() { + rm -f "$STALE_BANNER_MARKER" 2>/dev/null || true +} + # Worktree-tangle alarm, checked FIRST and independent of in-flight tasks: the # firstmate PRIMARY checkout (FM_ROOT) must stay on its default branch. If a # crewmate's branch/commits landed here instead of in its own isolated worktree, @@ -68,43 +148,68 @@ fm_supervision_status "$STATE" "$GRACE" in_flight=$FM_SUP_IN_FLIGHT watcher_fresh=$FM_SUP_WATCHER_FRESH beacon_desc=$FM_SUP_BEACON_DESC -[ "$in_flight" -eq 0 ] && exit 0 +if [ "$in_flight" -eq 0 ]; then + # Leave the unhealthy state (no work riding on the watcher): clear so a later + # in-flight + stale combination is a fresh episode even if the beacon is still + # absent with the same key string. + [ "$READ_ONLY" -eq 1 ] || fm_guard_clear_stale_banner + exit 0 +fi [ -s "$FM_WAKE_QUEUE" ] && queue_pending=true # No fresh watcher with tasks in flight is the dangerous state: emit a prominent, -# bordered banner FIRST so it reads as an alarm, not a buried stderr line. +# bordered banner FIRST so it reads as an alarm, not a buried stderr line. Later +# calls in the same episode get a one-line reminder only. if [ "$watcher_fresh" = false ]; then - afk=0 - [ -e "$STATE/.afk" ] && afk=1 - queue_arg=0 - "$queue_pending" && queue_arg=1 - x_mode=0 - [ -f "$CONFIG/x-mode.env" ] && x_mode=1 - fix=$("$SCRIPT_DIR/fm-supervision-instructions.sh" \ - --read-only "$READ_ONLY" \ - --afk "$afk" \ - --x-mode "$x_mode" \ - --queue-pending "$queue_arg" \ - --repair-line 2>/dev/null || printf '%s\n' 'Resume supervision according to the session-start operating block.') - rule='━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━' - { - printf '●%s\n' "$rule" - printf '● WATCHER DOWN - SUPERVISION IS OFF\n' - printf '● %s task(s) in flight, but no watcher has a fresh beacon (last beat: %s, grace %ss).\n' "$in_flight" "$beacon_desc" "$GRACE" - if [ "$READ_ONLY" -eq 1 ]; then - printf '● This read-only session should report the lapse, not repair it.\n' - else - printf '● Trust the emitted supervision protocol for this harness; do not use shell & for watcher repair.\n' - fi - printf '● %s\n' "$CONTINUE_LINE" - printf '● %s\n' "$fix" - printf '●%s\n' "$rule" - } >&2 + episode_key=$(fm_guard_stale_episode_key "$STATE") + episode_key=${episode_key%$'\n'} + print_full_banner=0 + if [ "$READ_ONLY" -eq 1 ]; then + fm_guard_stale_banner_seen "$STATE" "$episode_key" || print_full_banner=1 + elif fm_guard_claim_stale_banner "$STATE" "$episode_key"; then + print_full_banner=1 + fi + if [ "$print_full_banner" -eq 1 ]; then + afk=0 + [ -e "$STATE/.afk" ] && afk=1 + queue_arg=0 + "$queue_pending" && queue_arg=1 + x_mode=0 + [ -f "$CONFIG/x-mode.env" ] && x_mode=1 + fix=$("$SCRIPT_DIR/fm-supervision-instructions.sh" \ + --read-only "$READ_ONLY" \ + --afk "$afk" \ + --x-mode "$x_mode" \ + --queue-pending "$queue_arg" \ + --repair-line 2>/dev/null || printf '%s\n' 'Resume supervision according to the session-start operating block.') + rule='━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━' + { + printf '●%s\n' "$rule" + printf '● WATCHER DOWN - SUPERVISION IS OFF\n' + printf '● %s task(s) in flight, but no watcher has a fresh beacon (last beat: %s, grace %ss).\n' "$in_flight" "$beacon_desc" "$GRACE" + if [ "$READ_ONLY" -eq 1 ]; then + printf '● This read-only session should report the lapse, not repair it.\n' + else + printf '● Trust the emitted supervision protocol for this harness; do not use shell & for watcher repair.\n' + fi + printf '● %s\n' "$CONTINUE_LINE" + printf '● %s\n' "$fix" + printf '●%s\n' "$rule" + } >&2 + else + printf 'WARNING: watcher still down (same stale episode; last beat: %s, grace %ss) - full banner already printed this episode.\n' \ + "$beacon_desc" "$GRACE" >&2 + fi +else + # Healthy again while work is still in flight: end the episode so a later + # restale re-prints the full banner. + [ "$READ_ONLY" -eq 1 ] || fm_guard_clear_stale_banner fi # Queued wakes are an independent hazard; warn whenever they are pending, even if # a watcher is alive. Kept after the banner so the no-watcher alarm reads first. +# Dedup of the watcher-down banner never suppresses this warning. if "$queue_pending"; then if [ "$READ_ONLY" -eq 1 ]; then echo "WARNING: queued wakes pending - left untouched for the session holding the fleet lock." >&2 diff --git a/bin/fm-session-start.sh b/bin/fm-session-start.sh index 1f4b51d34d2..a229956ace2 100755 --- a/bin/fm-session-start.sh +++ b/bin/fm-session-start.sh @@ -276,8 +276,8 @@ fi # --- 3. wake-drain ------------------------------------------------------- # Drained records are this turn's first work queue (AGENTS.md section 8); the # drain also runs fm-guard.sh internally on the locked path, so the -# tangle/watcher-liveness banners land right here too, ahead of the bulk -# digest below. The read-only path never touches the queue (another session +# tangle/watcher-liveness alarms land right here too, ahead of the bulk digest +# below. The read-only path never touches the queue (another session # may be actively draining it) but still runs fm-guard.sh directly with # non-mutating advisory text, so the same alarms surface without repair # commands. diff --git a/bin/fm-wake-drain.sh b/bin/fm-wake-drain.sh index b45d15da2c8..ceaf13d3b85 100755 --- a/bin/fm-wake-drain.sh +++ b/bin/fm-wake-drain.sh @@ -13,7 +13,7 @@ DRAIN_LOCK_HELD=false # every wake-handling and recovery turn, so assert watcher liveness here too. A # lapsed supervision chain then surfaces on a plain drain-and-handle turn, not # only when a guarded supervision script (fm-peek/fm-send/...) happens to run. -# Reuse fm-guard.sh's existing graced, beacon-based banner (FM_GUARD_GRACE) - do +# Reuse fm-guard.sh's existing graced, beacon-based alarm (FM_GUARD_GRACE) - do # not duplicate the beacon math. Because the watcher touches its beacon every # poll cycle, a normal fire leaves a recent beacon well inside grace and stays # silent; only a genuine stale-beyond-grace lapse with work in flight warns. Call diff --git a/docs/architecture.md b/docs/architecture.md index 1106021469a..4c47f61527a 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -52,7 +52,7 @@ On `attached` it stays live until that existing cycle ends so background-notify Its `--restart` mode signals only the watcher recorded in the current home's `state/.watch.lock`, so restarting one home cannot kill sibling secondmate watchers. A pull-based guard (`bin/fm-guard.sh`) warns through supervision tool output if the primary checkout is tangled, or if tasks are in flight and that watcher stops running or queued wakes are waiting to be drained. The drain script calls that guard after emptying the queue, which avoids repeating the queued-wakes warning for records it just consumed while still warning on stale watcher liveness. -It leads with prominent bordered banners for the tangle and no-watcher cases so they cannot be skimmed past. +It leads with a prominent bordered tangle banner, while `bin/fm-guard.sh` owns the stale-watcher banner/reminder policy so repeated guarded commands stay noisy without reprinting the full watcher-down banner in the same episode. On every verified primary harness, tracked hook integration gives the primary session a push-based backstop: when work is in flight and no identity-matched watcher lock with a fresh beacon is live, direct Stop hooks block and passive turn-end hooks force one bounded follow-up. The guard covers the main primary and genuinely marked secondmate homes, exempts child crewmate/scout worktrees, is loop-safe per harness, and is documented in [turnend-guard.md](turnend-guard.md). diff --git a/tests/fm-guard-stale-banner.test.sh b/tests/fm-guard-stale-banner.test.sh new file mode 100755 index 00000000000..862eb9c7043 --- /dev/null +++ b/tests/fm-guard-stale-banner.test.sh @@ -0,0 +1,252 @@ +#!/usr/bin/env bash +# Regression tests for fm-guard's stale-watcher banner deduplication. +# +# The first stale command in one FM_HOME must print the full actionable watcher +# banner. +# Repeated commands in that same stale episode should print only a concise +# reminder, while unrelated alarms such as queued wakes stay independent. +set -u + +# shellcheck source=tests/lib.sh +. "$(dirname "${BASH_SOURCE[0]}")/lib.sh" + +TMP_ROOT=$(fm_test_tmproot fm-guard-stale-banner) + +make_guard_case() { + local name=$1 dir home root + dir="$TMP_ROOT/$name" + home="$dir/home" + root="$dir/root" + mkdir -p "$home/state" "$home/config" "$root" + fm_write_meta "$home/state/task.meta" "window=firstmate:fm-task" "kind=ship" + printf '%s\n' "$dir" +} + +case_home() { + printf '%s/home\n' "$1" +} + +case_root() { + printf '%s/root\n' "$1" +} + +run_guard_case() { + local dir=$1 + FM_ROOT_OVERRIDE="$(case_root "$dir")" \ + FM_HOME="$(case_home "$dir")" \ + FM_GUARD_GRACE=999 \ + "$ROOT/bin/fm-guard.sh" 2>&1 +} + +run_guard_case_read_only() { + local dir=$1 + FM_ROOT_OVERRIDE="$(case_root "$dir")" \ + FM_HOME="$(case_home "$dir")" \ + FM_GUARD_GRACE=999 \ + FM_GUARD_READ_ONLY=1 \ + "$ROOT/bin/fm-guard.sh" 2>&1 +} + +count_text() { + local haystack=$1 needle=$2 + awk -v needle="$needle" 'index($0, needle) { c++ } END { print c + 0 }' < "$out_dir/$i.out" 2>&1 + ) & + pids="$pids $!" + i=$((i + 1)) + done + for pid in $pids; do + wait "$pid" 2>/dev/null || fail "concurrent guard subprocess failed" + done + all=$(cat "$out_dir"/*.out) + full=$(count_text "$all" "WATCHER DOWN - SUPERVISION IS OFF") + reminders=$(count_text "$all" "full banner already printed this episode") + [ "$full" -eq 1 ] || fail "concurrent same-episode calls printed $full full banners"$'\n'"$all" + [ "$reminders" -eq 29 ] || fail "concurrent same-episode calls printed $reminders reminders, expected 29"$'\n'"$all" + pass "fm-guard stale banner: concurrent same-episode calls claim exactly one full banner" +} + +test_home_isolation() { + local dir_a dir_b out_a1 out_a2 out_b1 + dir_a=$(make_guard_case home-a) + dir_b=$(make_guard_case home-b) + out_a1=$(run_guard_case "$dir_a") + out_b1=$(run_guard_case "$dir_b") + out_a2=$(run_guard_case "$dir_a") + [ "$(count_text "$out_a1" "WATCHER DOWN - SUPERVISION IS OFF")" -eq 1 ] \ + || fail "home A first stale call did not print a full banner: $out_a1" + [ "$(count_text "$out_b1" "WATCHER DOWN - SUPERVISION IS OFF")" -eq 1 ] \ + || fail "home B first stale call was suppressed by home A: $out_b1" + assert_contains "$out_a2" "full banner already printed this episode" \ + "home A repeated stale call did not remember its own episode" + pass "fm-guard stale banner: deduplication is isolated per FM_HOME" +} + +test_queued_wake_warning_stays_independent() { + local dir home out1 out2 + dir=$(make_guard_case queued-wake) + home=$(case_home "$dir") + out1=$(run_guard_case "$dir") + [ "$(count_text "$out1" "WATCHER DOWN - SUPERVISION IS OFF")" -eq 1 ] \ + || fail "first stale call did not print the full banner before queued wake case: $out1" + printf 'signal: %s/state/task.status\n' "$home" > "$home/state/.wake-queue" + out2=$(run_guard_case "$dir") + assert_contains "$out2" "full banner already printed this episode" \ + "same-episode stale call should still print its concise reminder" + assert_contains "$out2" "queued wakes pending" \ + "queued wake warning must not be suppressed by stale-banner deduplication" + pass "fm-guard stale banner: queued-wake warning remains independent" +} + +test_read_only_before_writable_does_not_consume_full_banner() { + local dir home marker lock out_ro out_rw + dir=$(make_guard_case read-only-before-writable) + home=$(case_home "$dir") + marker="$home/state/.guard-watcher-stale-banner" + lock="$home/state/.guard-watcher-stale-banner.lock" + + out_ro=$(run_guard_case_read_only "$dir") + [ "$(count_text "$out_ro" "WATCHER DOWN - SUPERVISION IS OFF")" -eq 1 ] \ + || fail "read-only stale call should print the advisory full banner: $out_ro" + assert_absent "$marker" "read-only stale call must not create the stale-banner marker" + assert_absent "$lock" "read-only stale call must not create the stale-banner lock" + + out_rw=$(run_guard_case "$dir") + [ "$(count_text "$out_rw" "WATCHER DOWN - SUPERVISION IS OFF")" -eq 1 ] \ + || fail "writable stale call should still receive the full banner after read-only: $out_rw" + assert_present "$marker" "writable stale call should claim the stale-banner marker" + pass "fm-guard stale banner: read-only before writable does not consume full banner" +} + +test_read_only_during_episode_observes_without_mutating_marker() { + local dir home marker before after out_ro + dir=$(make_guard_case read-only-during-episode) + home=$(case_home "$dir") + marker="$home/state/.guard-watcher-stale-banner" + + run_guard_case "$dir" >/dev/null + before=$(cat "$marker") + out_ro=$(run_guard_case_read_only "$dir") + after=$(cat "$marker") + assert_contains "$out_ro" "full banner already printed this episode" \ + "read-only stale call during a claimed episode should print the concise reminder" + [ "$after" = "$before" ] || fail "read-only stale call must not update an existing marker" + pass "fm-guard stale banner: read-only during episode observes without mutating marker" +} + +test_healthy_read_only_does_not_clear_marker() { + local dir home marker before after healthy + dir=$(make_guard_case healthy-read-only) + home=$(case_home "$dir") + marker="$home/state/.guard-watcher-stale-banner" + + run_guard_case "$dir" >/dev/null + before=$(cat "$marker") + touch "$home/state/.last-watcher-beat" + healthy=$(run_guard_case_read_only "$dir") + [ -z "$healthy" ] || fail "healthy read-only guard should stay silent, got: $healthy" + assert_present "$marker" "healthy read-only guard must not clear the stale-banner marker" + after=$(cat "$marker") + [ "$after" = "$before" ] || fail "healthy read-only guard must not update the marker" + pass "fm-guard stale banner: healthy read-only does not clear marker" +} + +test_read_only_never_mutates_stale_banner_state_files() { + local dir home marker lock before after no_work + dir=$(make_guard_case read-only-state-nonmutation) + home=$(case_home "$dir") + marker="$home/state/.guard-watcher-stale-banner" + lock="$home/state/.guard-watcher-stale-banner.lock" + printf '%s\n' "sentinel-marker" > "$marker" + + before=$(find "$home/state" -maxdepth 1 -mindepth 1 -name '.guard-watcher-stale-banner*' -print | sort) + run_guard_case_read_only "$dir" >/dev/null + after=$(find "$home/state" -maxdepth 1 -mindepth 1 -name '.guard-watcher-stale-banner*' -print | sort) + [ "$after" = "$before" ] || fail "stale read-only guard changed stale-banner state files"$'\n'"before: $before"$'\n'"after: $after" + [ "$(cat "$marker")" = "sentinel-marker" ] || fail "stale read-only guard updated the marker content" + assert_absent "$lock" "stale read-only guard must not create the stale-banner lock" + + rm -f "$home/state/task.meta" + no_work=$(run_guard_case_read_only "$dir") + [ -z "$no_work" ] || fail "read-only guard with no in-flight work should stay silent, got: $no_work" + after=$(find "$home/state" -maxdepth 1 -mindepth 1 -name '.guard-watcher-stale-banner*' -print | sort) + [ "$after" = "$before" ] || fail "no-work read-only guard changed stale-banner state files"$'\n'"before: $before"$'\n'"after: $after" + [ "$(cat "$marker")" = "sentinel-marker" ] || fail "no-work read-only guard updated the marker content" + pass "fm-guard stale banner: read-only never mutates stale-banner state files" +} + +test_first_stale_call_prints_full_banner +test_repeated_same_episode_prints_reminder_only +test_healthy_recovery_rearms_next_stale_episode +test_concurrent_same_episode_prints_one_full_banner +test_home_isolation +test_queued_wake_warning_stays_independent +test_read_only_before_writable_does_not_consume_full_banner +test_read_only_during_episode_observes_without_mutating_marker +test_healthy_read_only_does_not_clear_marker +test_read_only_never_mutates_stale_banner_state_files