From 1c78fe6af90d97cc841b82ae9f9037bace8db54f Mon Sep 17 00:00:00 2001 From: Abe Augustijn Date: Thu, 25 Jun 2026 20:06:49 +0200 Subject: [PATCH] fix watcher supervision loop --- .agents/skills/afk/SKILL.md | 2 +- AGENTS.md | 25 ++++---- CONTRIBUTING.md | 2 +- bin/fm-guard.sh | 6 +- bin/fm-watch-loop.sh | 108 ++++++++++++++++++++++++++++++++++ bin/fm-watch.sh | 7 ++- tests/fm-watcher-lock.test.sh | 59 ++++++++++++++++++- 7 files changed, 188 insertions(+), 21 deletions(-) create mode 100755 bin/fm-watch-loop.sh diff --git a/.agents/skills/afk/SKILL.md b/.agents/skills/afk/SKILL.md index 4ed1b0d631f..43aaf972899 100644 --- a/.agents/skills/afk/SKILL.md +++ b/.agents/skills/afk/SKILL.md @@ -49,7 +49,7 @@ No `/back` is needed. The first genuine message is the return signal: distilled "while you were out" catch-up (drain `state/.wake-queue`, summarize any pending escalations from `state/.subsuper-escalations` and any `state/.subsuper-inject-wedged` marker), and resume full per-wake - responsiveness (arm `bin/fm-watch-arm.sh`). + responsiveness (start `bin/fm-watch-loop.sh`). - A message **with** the sentinel marker (`FM_INJECT_MARK`, ASCII 0x1f) -> it is a daemon escalation; stay afk and process it. - Re-invoking `/afk` while already away -> stay afk (refresh the flag); this diff --git a/AGENTS.md b/AGENTS.md index ea82a62eb25..2588f480555 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -403,18 +403,20 @@ From there the task is an ordinary ship task through its mode-specific validatio ## 8. Supervision protocol The watcher is the backbone. -Whenever at least one task is in flight, keep `bin/fm-watch.sh` running through a harness-tracked `bin/fm-watch-arm.sh` background task. +Whenever at least one task is in flight, keep `bin/fm-watch.sh` running through a harness-tracked `bin/fm-watch-loop.sh` background task. It costs zero tokens while running and exits with one reason line when something needs you. It also writes each detected wake to the durable queue at `state/.wake-queue` before advancing suppression markers such as `.seen-*`, `.stale-*`, `.last-check`, or `.last-heartbeat`. At the start of every wake-handling turn and every recovery turn, run `bin/fm-wake-drain.sh` before peeking panes, reading status files beyond the reason line, or starting new work. The printed one-shot reason line is still useful, but the drained queue is the lossless backlog. -After handling drained wakes, re-arm the watcher before you end the turn by running `bin/fm-watch-arm.sh` as a background task. -Arm or re-arm the watcher only through the harness's own tracked background mechanism - the one that survives the call and notifies you when the process exits - so the re-arm actually persists and the next wake reaches you. +After handling drained wakes, make sure durable supervision is running before you end the turn by running `bin/fm-watch-loop.sh` as a background task. +Start the watcher only through the harness's own tracked background mechanism - the one that survives the call and surfaces output - so the supervision process actually persists and the next wake reaches you. Never fire-and-forget the watcher with a shell `&` inside another call: that backgrounded child is reaped when the call returns, so supervision silently stops, and worse, the dying process reports a false "already running" that hides the gap. +`bin/fm-watch-loop.sh` is the normal supervision command. It is home-scoped, singleton-safe, restarts the one-shot watcher after every wake, and prints each wake reason as it arrives. `bin/fm-watch-arm.sh` is self-verifying: it confirms a genuinely live watcher with a fresh beacon and prints exactly one honest status line - `watcher: started ...`, `watcher: healthy ...`, or `watcher: FAILED - no live watcher with a fresh beacon` (which exits non-zero) - so treat that line, not a process count or an unverified "already running", as the source of truth for watcher state. -The watcher is singleton-safe: acquisition is race-proof, so under any number of concurrent arms at most one watcher ever holds this home's lock, and a duplicate that somehow starts self-evicts within one poll once it sees the lock no longer names it. +Use `fm-watch-arm.sh` only for harnesses that automatically re-arm tracked background tasks after every wake, or for one-shot liveness checks and forced restarts. +The watcher is singleton-safe: acquisition is race-proof, so under any number of concurrent starts at most one watcher ever holds this home's lock, and a duplicate that somehow starts self-evicts within one poll once it sees the lock no longer names it. If one is already alive with a fresh liveness beacon, another invocation exits cleanly instead of creating a duplicate watcher; if the live holder's beacon is stale, the new invocation exits with an actionable failure. -Re-arming is the primary model: just run `bin/fm-watch-arm.sh` and let the singleton lock no-op when a healthy watcher is already alive. +Continuous supervision is the primary model: just run `bin/fm-watch-loop.sh` and let the loop lock no-op when a loop is already alive. If a forced restart is ever genuinely needed, use `bin/fm-watch-arm.sh --restart`, which stops only this home's watcher (the pid recorded in this home's `state/.watch.lock`) and starts a fresh one. Never `pkill -f bin/fm-watch.sh`: that pattern matches every firstmate home's watcher, including secondmate homes that run the same script, so a broad pkill from one home kills sibling homes' watchers. Away-mode supervision is provided by the `/afk` skill and its daemon; while `state/.afk` exists, the daemon owns the watcher. @@ -423,7 +425,8 @@ After arming it, do not send idle progress updates to the captain; wait until it Empty polls, elapsed waiting time, and "still no change" are tool bookkeeping, not conversational progress. ```sh -bin/fm-watch-arm.sh # safe verified re-arm; run as harness-tracked background; no-ops if healthy +bin/fm-watch-loop.sh # continuous supervision; run as harness-tracked background; no-ops if already running +bin/fm-watch-arm.sh # one-shot verified arm; use only when the harness auto-rearms, or for liveness checks bin/fm-watch-arm.sh --restart # home-scoped forced restart; never a broad pkill bin/fm-watch.sh # the watcher itself; exits with: signal|stale|check|heartbeat bin/fm-wake-drain.sh # drain queued wake records at turn start @@ -436,7 +439,7 @@ On wake, in order of cheapness: 3. `stale:` the crewmate stopped without reporting; peek the pane (`bin/fm-peek.sh `) to diagnose. If the pane is waiting, looping, confused, or unresponsive, load `stuck-crewmate-recovery`. 4. `check:` a per-task poll fired (usually a merge); act on it. -5. `heartbeat:` review the whole fleet: skim each window's status file, peek panes that look off, check PR-ready tasks for merge, reconcile data/backlog.md, then re-arm the watcher. +5. `heartbeat:` review the whole fleet: skim each window's status file, peek panes that look off, check PR-ready tasks for merge, reconcile data/backlog.md, then ensure the watcher loop is running. A heartbeat with no captain-relevant change is internal; do not report that the fleet is unchanged. Heartbeats back off exponentially while they are the only wakes firing (600s doubling to a 2h cap - an idle fleet stops burning turns); any signal, stale, or check wake resets the cadence to the base interval. @@ -450,14 +453,14 @@ A secondmate may be sitting on its own watcher with no visible pane changes, so This exception is narrow: ordinary crewmates still trip stale detection when their pane stops changing without a busy signature. **Watcher liveness is guarded, not just disciplined.** -Arming the watcher is the last action of every wake-handling turn - but the protocol no longer relies on remembering that. +Ensuring the watcher loop is running is the last action of every wake-handling turn - but the protocol no longer relies on remembering that. While running, `fm-watch.sh` touches `state/.last-watcher-beat` every poll cycle. The supervision scripts (`fm-peek`, `fm-send`, `fm-spawn`, `fm-teardown`, `fm-pr-check`, `fm-promote`, `fm-review-diff`, `fm-fleet-sync`, `fm-update`) call `bin/fm-guard.sh` first, which warns to stderr when any task is in flight (`state/*.meta` exists) but queued wakes are pending, or that beacon is missing or older than `FM_GUARD_GRACE` (default 300s). -The no-watcher case leads with a prominent, bordered ●-marked banner (in-flight count, beacon age, and the exact one-line re-arm command) so it reads as an alarm rather than a buried stderr line you can skim past. +The no-watcher case leads with a prominent, bordered ●-marked banner (in-flight count, beacon age, and the exact one-line durable supervision command) so it reads as an alarm rather than a buried stderr line you can skim past. So the next time you touch the fleet with queued wakes or no watcher alive, the tool output itself tells you what to do - a pull-based guard that works on any harness, since it rides the script output you already read rather than a harness-specific hook. The grace window keeps normal handling (watcher briefly down between a wake and its re-arm) silent. If a guard warning says queued wakes are pending, drain them before doing anything else. -If a guard warning says watcher liveness is stale, arm `bin/fm-watch-arm.sh` after draining any queued wakes. +If a guard warning says watcher liveness is stale, start `bin/fm-watch-loop.sh` after draining any queued wakes. Watcher liveness is not enough if you are foreground-blocked. Whenever one or more tasks are in flight, do not run long foreground-blocking operations in your own session. This is about firstmate's own session: it includes a no-mistakes pipeline firstmate runs for this repo, long builds, and any other multi-minute command. @@ -478,7 +481,7 @@ Inline facts that must survive without a loaded skill: - While `state/.afk` exists, the daemon owns the watcher; do not separately arm `fm-watch-arm.sh` or `fm-watch.sh`. - If firstmate receives a marked message while afk is active, it is an internal escalation: stay afk and process it. - If the message starts with `/afk`, stay afk and refresh the flag. -- Any other unmarked message means the captain is back: clear `state/.afk`, stop the daemon, flush catch-up from `state/.wake-queue`, `state/.subsuper-escalations`, and `state/.subsuper-inject-wedged`, then re-arm normal watcher supervision. +- Any other unmarked message means the captain is back: clear `state/.afk`, stop the daemon, flush catch-up from `state/.wake-queue`, `state/.subsuper-escalations`, and `state/.subsuper-inject-wedged`, then start the normal watcher loop. - Afk never changes approval authority; PR merges, ask-user findings, destructive actions, irreversible actions, and security-sensitive choices still require the same approval they required before. - Bias ambiguous cases toward exit because a present captain beats token savings and a false exit is self-correcting. diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index b8ee85f35da..bba4781b196 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -73,7 +73,7 @@ tests/fm-secondmate-safety.test.sh # secondmate home safety, idle charter tests/fm-teardown.test.sh # fm-teardown.sh safety and reminder checks: local-only fork-remote allow, truly-unpushed refuse, merged-to-main allow, no-mistakes regression, tasks-axi reminder, --force override [ "$(readlink CLAUDE.md)" = "AGENTS.md" ] [ "$(readlink .claude/skills)" = "../.agents/skills" ] -FM_HEARTBEAT=2 FM_POLL=1 bin/fm-watch-arm.sh # watcher re-arm smoke test (prints arm status, then "heartbeat") +FM_HEARTBEAT=2 FM_POLL=1 bin/fm-watch-loop.sh # watcher loop smoke test (prints start status, then heartbeats until stopped) ``` ## Questions diff --git a/bin/fm-guard.sh b/bin/fm-guard.sh index 2cfd9bf8fdb..8ab6479147a 100755 --- a/bin/fm-guard.sh +++ b/bin/fm-guard.sh @@ -57,16 +57,16 @@ fi # bordered banner FIRST so it reads as an alarm, not a buried stderr line. if [ "$watcher_fresh" = false ]; then if "$queue_pending"; then - fix='After draining queued wakes, re-arm the watcher: run bin/fm-watch-arm.sh as the harness-tracked background task (never a shell & that gets reaped).' + fix='After draining queued wakes, start durable supervision: run bin/fm-watch-loop.sh as the harness-tracked background task (or bin/fm-watch-arm.sh only if the harness auto-rearms after every wake).' else - fix='Re-arm it NOW: run bin/fm-watch-arm.sh as the harness-tracked background task (never a shell & that gets reaped).' + fix='Start durable supervision NOW: run bin/fm-watch-loop.sh as the harness-tracked background task (or bin/fm-watch-arm.sh only if the harness auto-rearms after every wake).' fi 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" - printf '● Trust bin/fm-watch-arm.sh for the true state: it confirms a live watcher and a fresh beacon, or fails loudly.\n' + printf '● Use bin/fm-watch-loop.sh for continuous supervision; bin/fm-watch-arm.sh remains the one-shot confirm-and-wait helper.\n' printf '● %s\n' "$fix" printf '●%s\n' "$rule" } >&2 diff --git a/bin/fm-watch-loop.sh b/bin/fm-watch-loop.sh new file mode 100755 index 00000000000..65016dadf45 --- /dev/null +++ b/bin/fm-watch-loop.sh @@ -0,0 +1,108 @@ +#!/usr/bin/env bash +# Persistent firstmate watcher loop. +# +# bin/fm-watch.sh is intentionally one-shot: it exits after one wake reason. +# bin/fm-watch-arm.sh preserves that contract for harnesses that re-arm tracked +# background tasks after exit. This loop is the explicit fallback for harnesses +# that do not: run this script as one long-lived harness-tracked background task +# and it will restart the one-shot watcher after each wake. +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}" +mkdir -p "$STATE" + +# shellcheck source=bin/fm-wake-lib.sh +. "$SCRIPT_DIR/fm-wake-lib.sh" + +WATCH="$SCRIPT_DIR/fm-watch.sh" +LOOP_LOCK="$STATE/.watch-loop.lock" +LOG="$STATE/.watch-loop.log" +WATCHER_PID= +CUR_TMP= + +is_wake_reason() { + case "$1" in + signal:*|stale:*|check:*|heartbeat|heartbeat:*) return 0 ;; + *) return 1 ;; + esac +} + +log() { + printf '%s %s\n' "$(date '+%Y-%m-%dT%H:%M:%S%z')" "$*" >> "$LOG" 2>/dev/null || true +} + +cleanup() { + trap - TERM INT HUP + if [ -n "${WATCHER_PID:-}" ] && fm_pid_alive "$WATCHER_PID"; then + kill -TERM "$WATCHER_PID" 2>/dev/null || true + wait "$WATCHER_PID" 2>/dev/null || true + fi + [ -n "${CUR_TMP:-}" ] && rm -f "$CUR_TMP" 2>/dev/null || true + fm_lock_release "$LOOP_LOCK" 2>/dev/null || true + log "loop stopped" + exit 0 +} +trap cleanup TERM INT HUP + +if ! fm_lock_try_acquire "$LOOP_LOCK"; then + if [ -n "${FM_LOCK_HELD_PID:-}" ]; then + echo "watch-loop: already running pid $FM_LOCK_HELD_PID" + else + echo "watch-loop: already running" + fi + exit 0 +fi + +echo "watch-loop: started pid=${BASHPID:-$$}" +log "loop started pid ${BASHPID:-$$}" + +while :; do + CUR_TMP=$(mktemp "${TMPDIR:-/tmp}/fm-watch-loop.XXXXXX") || { + echo "watch-loop: mktemp failed; retrying" >&2 + sleep 5 + continue + } + + "$WATCH" >"$CUR_TMP" & + WATCHER_PID=$! + if wait "$WATCHER_PID"; then + rc=0 + else + rc=$? + fi + WATCHER_PID= + + reason="" + [ -e "$CUR_TMP" ] && reason=$(<"$CUR_TMP") + rm -f "$CUR_TMP" 2>/dev/null || true + CUR_TMP= + + if [ "$rc" -ne 0 ]; then + echo "watch-loop: watcher exited rc=$rc; retrying" >&2 + log "watcher exited rc=$rc reason='$reason'" + sleep "${FM_WATCH_LOOP_RETRY_SLEEP:-5}" + continue + fi + + if [ -z "$reason" ]; then + echo "watch-loop: watcher exited without a wake; retrying" >&2 + log "watcher exited without a wake" + sleep "${FM_WATCH_LOOP_RETRY_SLEEP:-5}" + continue + fi + + if is_wake_reason "$reason"; then + printf '%s\n' "$reason" + log "wake: $reason" + continue + fi + + # A singleton status line means another watcher currently owns the one-shot + # lock. Stay alive and retry instead of treating it as a wake. + echo "watch-loop: $reason" >&2 + log "non-wake watcher output: $reason" + sleep "${FM_WATCH_LOOP_RETRY_SLEEP:-5}" +done diff --git a/bin/fm-watch.sh b/bin/fm-watch.sh index bd03becd18f..96f8070a28e 100755 --- a/bin/fm-watch.sh +++ b/bin/fm-watch.sh @@ -6,9 +6,10 @@ # stale: a crewmate pane stopped changing and shows no busy signature # check: