diff --git a/bin/fm-watch.sh b/bin/fm-watch.sh index 443c32303f7..32832c46d83 100755 --- a/bin/fm-watch.sh +++ b/bin/fm-watch.sh @@ -1856,6 +1856,20 @@ fm_active_check_stop() { FM_ACTIVE_CHECK_PGID= } +# Stop-signal dispositions, installed with the EXIT trap below. HUP and TERM +# keep bash's native fatal-signal handling, which runs watcher_cleanup through +# the EXIT trap and then exits on every supported bash. A trap body such as +# 'exit 1' is not reliable for them: bash 5.2 runs a pending trap inside the +# parse of the next command substitution, the body then fails to parse ("trap: +# line 2: unexpected EOF while looking for matching `)'", or nothing at all), +# and the signal is consumed, so a stop request could leave this watcher +# polling forever while its stopper waits (fixed upstream in bash 5.3). INT +# keeps its trap because bash ignores a direct SIGINT while a child runs. +watcher_stop_signals() { + trap - HUP TERM + trap 'exit 1' INT +} + run_check_capture() { local pgid fm_check_output_cleanup @@ -1863,20 +1877,23 @@ run_check_capture() { FM_CHECK_OUTPUT=$(mktemp "$STATE/.fm-check-output.XXXXXX") || return 1 chmod 0600 "$FM_CHECK_OUTPUT" || { fm_check_output_cleanup; return 1; } FM_CHECK_SIGNAL_PENDING= + # Defer stop signals only until the check's process group is recorded for + # watcher_cleanup. Keep command substitutions out of this window: bash 5.2 + # can drop a trap that is pending when one is parsed (watcher_stop_signals). trap 'FM_CHECK_SIGNAL_PENDING=1' HUP INT TERM set -m ( FM_CHECK_OWNED_GROUP=1 run_check_process "$@" ) > "$FM_CHECK_OUTPUT" 2>/dev/null & FM_ACTIVE_CHECK_PID=$! FM_ACTIVE_CHECK_PGID=$FM_ACTIVE_CHECK_PID set +m + watcher_stop_signals + [ -z "$FM_CHECK_SIGNAL_PENDING" ] || exit 1 pgid=$(ps -o pgid= -p "$FM_ACTIVE_CHECK_PID" 2>/dev/null | tr -d '[:space:]') - trap 'exit 1' HUP INT TERM if [ -n "$pgid" ] && [ "$pgid" != "$FM_ACTIVE_CHECK_PGID" ]; then fm_active_check_stop || true fm_check_output_cleanup return 1 fi - [ -z "$FM_CHECK_SIGNAL_PENDING" ] || exit 1 wait "$FM_ACTIVE_CHECK_PID" 2>/dev/null || true FM_ACTIVE_CHECK_PID= fm_active_check_stop || return 1 @@ -2221,7 +2238,7 @@ watcher_cleanup() { return "$cleanup_status" } trap watcher_cleanup EXIT -trap 'exit 1' HUP INT TERM +watcher_stop_signals # This watcher's own pid, as recorded in the lock by fm_lock_claim (which writes # ${BASHPID:-$$} from this same main shell). Read directly, never via a command # substitution, so it matches the stored holder pid for the self-eviction check. diff --git a/docs/watcher-continuity.md b/docs/watcher-continuity.md index daaaef201b4..c7a1172468b 100644 --- a/docs/watcher-continuity.md +++ b/docs/watcher-continuity.md @@ -114,6 +114,7 @@ The file is size-capped through `FM_WATCH_CYCLE_LOG_MAX_BYTES` and `FM_WATCH_CYC The default 300-second grace is unchanged. Only the watcher process touches `state/.last-watcher-beat`; no helper process can make a wedged watcher appear healthy. +The watcher uses bash's native fatal handling for HUP and TERM, including during a blocked poll, so both run its EXIT cleanup; `watcher_stop_signals` in `bin/fm-watch.sh` owns the signal-handling rationale. ## Regression coverage @@ -122,6 +123,7 @@ The same suite covers ordinary same-process session replacement for `/new`, `/re The guard and session-start suites prove that active generation evidence tolerates a fresh-beacon handoff while a legacy or handoff-phase watcher marker from an absent replacement extension still raises the outage diagnostic. `tests/fm-watch-arm.test.sh` covers durable queue replay, real remote parent-replies ingestion into the authoritative status log, decision-only OPEN DECISIONS recovery, interrupted handling replay, generation-bound acknowledgement, a persistent live successor after recovery, a watcher close inside the handling window that must leave the printed acknowledgement valid, and the self-healing moved-generation acknowledgement that consumes its handled rows and names its remedy. `tests/fm-watch-recovery-loop.test.sh` covers the once-per-generation announcement bound with the real Pi extension against a refused handling handshake, and a handling successor that must surface a real crew event instead of going blind. +`tests/fm-watch-triage.test.sh` proves TERM stops a watcher blocked inside a poll's pane capture and still releases its lock and records an acknowledgeable stop. `tests/fm-watcher-lock.test.sh` covers verified-successor attach, recovery publication before stale-lock removal, the typed self-eviction failure, bounded and successor-linked lifecycle rows, and a SIGSTOP counterfactual that distinguishes a live PID from a stale beacon before classifying termination. `tests/fm-subagent-pretool-check.test.sh` proves Claude retains only the non-status Bash seatbelts. `tests/fm-claude-stop-autoarm.test.sh` covers the auto-arm's scope, stale and live session owners, unchanged AFK and need boundaries, single-flight, bounded failure retries, benign live-watcher cycle ends, one-notice failure episodes, exit-2 translation, and host-timeout HUP/TERM/INT translation into the same durable failure handoff. diff --git a/tests/fm-watch-triage.test.sh b/tests/fm-watch-triage.test.sh index cc947969f4f..490e431bbd1 100755 --- a/tests/fm-watch-triage.test.sh +++ b/tests/fm-watch-triage.test.sh @@ -174,7 +174,17 @@ record_pi_busy() { # --source pi-ext --event agent-start } -reap() { kill "$1" 2>/dev/null || true; wait "$1" 2>/dev/null || true; } +# Stop an owned watcher. TERM must end it through its EXIT cleanup, so one still +# alive after the file's standard 100-tick budget fails the case here, with the +# process evidence wait_for_exit prints, instead of an unbounded wait hanging +# the whole suite until the CI job timeout. +reap() { + local rc + kill "$1" 2>/dev/null || true + wait_for_exit "$1" 100 + rc=$? + [ "$rc" -ne 124 ] || fail "watcher pid $1 did not exit within 10s of TERM" +} # --- pure classifier predicates (fm-classify-lib.sh) ------------------------ @@ -4220,6 +4230,52 @@ test_wedge_escalation_resets_when_pane_becomes_active() { pass "a pane becoming active again resets the consecutive wedge-escalation counter" } +# --- a stop request is honored mid-poll -------------------------------------- +# Every stopper (the arm's signal path, the away-mode daemon, reap above) waits +# for the watcher to exit after one TERM, so TERM must end it through its EXIT +# cleanup at any point of a poll. A TERM trap body cannot promise that: bash +# defers it until the blocked command returns, and bash 5.2 can drop it outright +# when it is pending as a command substitution is parsed, which left CI watchers +# polling after reap until the job timed out. The pane capture here blocks on a +# FIFO whose writer never writes, so only a TERM honored mid-poll stops the +# watcher inside the bound; the released lock and acknowledgeable stop record +# prove its cleanup still ran. +test_term_stops_a_watcher_blocked_inside_a_poll() { + local dir state fakebin out fifo window sig pid holder i rc + dir=$(make_case term-blocked-poll); state="$dir/state"; fakebin="$dir/fakebin" + out="$dir/watch.out"; fifo="$dir/pane.fifo"; window="test:fm-blocked-capture" + mkfifo "$fifo" + printf 'window=%s\nkind=ship\n' "$window" > "$state/blocked.meta" + printf 'working: implementing\n' > "$state/blocked.status" + sig=$(seen_sig "$state/blocked.status"); printf '%s' "$sig" > "$state/.seen-blocked_status" + # Opening the write end waits for the capture to open the read end, and the + # holder then keeps it open without writing, so that capture blocks mid-poll. + ( exec 3> "$fifo"; : > "$dir/capture-blocked"; exec sleep 30 ) & + holder=$! + PATH="$fakebin:$PATH" FM_FAKE_TMUX_WINDOW="$window" FM_FAKE_TMUX_CAPTURE="$fifo" \ + FM_STATE_OVERRIDE="$state" FM_POLL=1 FM_SIGNAL_GRACE=1 \ + FM_CHECK_INTERVAL=999999 FM_HEARTBEAT=999999 "$WATCH" > "$out" & + pid=$! + i=0 + while [ ! -e "$dir/capture-blocked" ] && [ "$i" -lt 300 ]; do + sleep 0.1 + i=$((i + 1)) + done + if [ ! -e "$dir/capture-blocked" ] || ! is_live_non_zombie "$pid"; then + kill "$holder" 2>/dev/null || true; reap "$pid" + fail "the watcher never blocked inside its pane capture: $(cat "$out")" + fi + kill "$pid" 2>/dev/null || true + wait_for_exit "$pid" 100 + rc=$? + kill "$holder" 2>/dev/null || true + wait "$holder" 2>/dev/null || true + [ "$rc" -ne 124 ] || fail "TERM did not stop a watcher blocked inside a poll" + [ ! -e "$state/.watch.lock" ] || fail "a watcher stopped mid-poll kept its singleton lock, so its cleanup did not run" + ack_stopped_cycle "$state" || fail "could not acknowledge the stop of a watcher blocked inside a poll" + pass "TERM stops a watcher blocked inside a poll and still runs its cleanup" +} + # --- busy pane duration bound: a completed-turn age gate on top of busy ----- # 2026-07 hibit-agent-focus-nonsteal-r1 incident: a busy pane (herdr "working" # and/or the harness's rendered busy footer) is unconditional, unbounded proof @@ -6078,6 +6134,7 @@ test_live_and_unproven_endpoints_still_wedge_escalate test_gone_report_rearms_when_the_endpoint_comes_back test_second_death_after_a_same_window_relaunch_reports_in_full test_identical_dead_display_of_a_successor_still_reports +test_term_stops_a_watcher_blocked_inside_a_poll test_busy_pane_below_turn_age_bound_is_absorbed test_busy_pane_stable_hash_escalates_past_turn_age_bound test_busy_pane_changing_hash_escalates_past_turn_age_bound