Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
85 changes: 69 additions & 16 deletions bin/fm-watch.sh
Original file line number Diff line number Diff line change
Expand Up @@ -549,7 +549,8 @@ inbox_steer_check() { # <window> <task>
return 0
;;
esac
tail40=$(fm_backend_capture "$backend" "$w" 40 "$(window_label "$w")" 2>/dev/null) || tail40=
watcher_capture "$backend" "$w" 40 "$(window_label "$w")" || WATCHER_CAPTURE=
tail40=$WATCHER_CAPTURE
if window_is_busy "$w" "$tail40"; then
[ "$verb" != retry ] || return 0
if ! count=$(fm_task_inbox_record_busy "$STATE" "$task" "$rec"); then
Expand Down Expand Up @@ -763,7 +764,8 @@ signal_turnend_panes_churned() { # <file> ...
[ "$hash_bytes" = 32 ] || return 1
prev=$(cat "$hash_file" 2>/dev/null) || return 1
[[ $prev =~ ^[0-9a-f]{32}$ ]] || return 1
now=$(fm_backend_capture "$backend" "$w" 40 "$label" 2>/dev/null) || return 1
watcher_capture "$backend" "$w" 40 "$label" || return 1
now=$WATCHER_CAPTURE
[ -n "$now" ] || return 1
[ "$(printf '%s' "$now" | hash_pane)" != "$prev" ] || return 1
churned_keys+=("$key")
Expand Down Expand Up @@ -869,27 +871,29 @@ secondmate_in_active_turn() { # <window> <idle>
local w=$1 idle=$2 tail40
[ -n "$w" ] || return 1
[ "$idle" -lt "$BUSY_TURN_MAX_SECS" ] || return 1
tail40=$(fm_backend_capture "$(window_backend "$w")" "$w" 40 "$(window_label "$w")" 2>/dev/null) || return 1
watcher_capture "$(window_backend "$w")" "$w" 40 "$(window_label "$w")" || return 1
tail40=$WATCHER_CAPTURE
window_is_busy "$w" "$tail40"
}

# First token of the semantic busy classification for <window>: busy, idle,
# unknown, or dead. Capture failure and a missing window are unknown, never
# idle. Empty inbox and a fresh watcher beacon are not consulted.
# Set SECONDMATE_BUSY_CLASS to the first token of the semantic busy
# classification for <window>: busy, idle, unknown, or dead. Capture failure and
# a missing window are unknown, never idle. Empty inbox and a fresh watcher
# beacon are not consulted. Call it directly, never in $(...), so its pane
# capture runs in the watcher's own shell.
SECONDMATE_BUSY_CLASS=
secondmate_busy_class() { # <window>
local w=$1 task meta tail40 verdict
SECONDMATE_BUSY_CLASS=unknown
task=$(window_to_task "$w" "$STATE")
meta="$STATE/$task.meta"
if [ -z "$w" ] || [ -z "$task" ] || [ ! -f "$meta" ]; then
printf 'unknown'
return 0
fi
tail40=$(fm_backend_capture "$(window_backend "$w")" "$w" 40 "$(window_label "$w")" 2>/dev/null) || {
printf 'unknown'
return 0
}
watcher_capture "$(window_backend "$w")" "$w" 40 "$(window_label "$w")" || return 0
tail40=$WATCHER_CAPTURE
verdict=$(fm_busy_classify_meta "$meta" "$task" "$STATE" "$tail40")
printf '%s' "${verdict%% *}"
SECONDMATE_BUSY_CLASS=${verdict%% *}
}

# 0 iff a child ring is authorized: exact idle, a live agent, and a composer
Expand All @@ -899,7 +903,8 @@ secondmate_busy_class() { # <window>
secondmate_idle_ring_safe() { # <window>
local w=$1 backend agent_state cstate
[ -n "$w" ] || return 1
[ "$(secondmate_busy_class "$w")" = idle ] || return 1
secondmate_busy_class "$w"
[ "$SECONDMATE_BUSY_CLASS" = idle ] || return 1
backend=$(window_backend "$w")
agent_state=$(fm_backend_agent_state "$backend" "$w" 2>/dev/null || true)
[ "$agent_state" = alive ] || return 1
Expand Down Expand Up @@ -2138,8 +2143,10 @@ fm_active_check_stop() {
# 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.
# polling forever while its stopper waits (fixed upstream in bash 5.3). Bash
# 3.2 holds HUP and TERM until a running command substitution's child exits, so
# fm_backend_capture pane reads go through watcher_capture instead. INT keeps
# its trap because bash ignores a direct SIGINT while a child runs.
watcher_stop_signals() {
trap - HUP TERM
trap 'exit 1' INT
Expand Down Expand Up @@ -2176,6 +2183,50 @@ run_check_capture() {
fm_check_output_cleanup
}

FM_CAPTURE_OUTPUT=
WATCHER_CAPTURE=

fm_capture_output_cleanup() {
[ -z "$FM_CAPTURE_OUTPUT" ] || rm -f -- "$FM_CAPTURE_OUTPUT"
FM_CAPTURE_OUTPUT=
}

# watcher_capture: fm_backend_capture into WATCHER_CAPTURE with its exit status.
# The read runs as a waited background process group rather than inside $(...),
# so a stop is not held for a blocked read (watcher_stop_signals). The group is
# recorded like a check's, so watcher_cleanup stops a read still in flight.
watcher_capture() { # <backend> <target> <lines> [expected-label]
local rc pgid
fm_capture_output_cleanup
WATCHER_CAPTURE=
FM_CAPTURE_OUTPUT=$(mktemp "$STATE/.fm-capture-output.XXXXXX") || return 1
FM_CHECK_SIGNAL_PENDING=
trap 'FM_CHECK_SIGNAL_PENDING=1' HUP INT TERM
# The group's stderr is redirected before the fork: bash 3.2 on macOS can
# print a harmless "child setpgid ... Operation not permitted" race from the
# child before the command's own redirections apply.
set -m
{ fm_backend_capture "$@" < /dev/null > "$FM_CAPTURE_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:]')
if [ -n "$pgid" ] && [ "$pgid" != "$FM_ACTIVE_CHECK_PGID" ]; then
fm_active_check_stop || true
fm_capture_output_cleanup
return 1
fi
wait "$FM_ACTIVE_CHECK_PID"
rc=$?
FM_ACTIVE_CHECK_PID=
fm_active_check_stop || { fm_capture_output_cleanup; return 1; }
WATCHER_CAPTURE=$(cat "$FM_CAPTURE_OUTPUT" 2>/dev/null || true)
fm_capture_output_cleanup
return "$rc"
}

# 0 when any signaled status file carries a captain-relevant event in the bytes
# appended since this watcher last classified it. The start offset is the
# classified-position field in that file's .seen-* marker, and fm-classify-lib.sh's
Expand Down Expand Up @@ -2539,6 +2590,7 @@ watcher_cleanup() {
fi
fm_active_check_stop || cleanup_status=1
fm_check_output_cleanup
fm_capture_output_cleanup
fm_custom_check_snapshot_cleanup
if [ "$owns_lock" -eq 1 ] \
&& ! fm_recovery_transition "$WATCHER_DOWNTIME_MARKER" "$transition" "$WATCH_LOCK" \
Expand Down Expand Up @@ -3016,7 +3068,8 @@ EOF
if [ "$kind" = secondmate ] && ! status_is_paused_or_captain_held "$last"; then
continue
fi
tail40=$(fm_backend_capture "$(window_backend "$w")" "$w" 40 "$(window_label "$w")" 2>/dev/null) || continue
watcher_capture "$(window_backend "$w")" "$w" 40 "$(window_label "$w")" || continue
tail40=$WATCHER_CAPTURE
h=$(printf '%s' "$tail40" | hash_pane)
hf="$STATE/.hash-$key"
cf="$STATE/.count-$key"
Expand Down
3 changes: 2 additions & 1 deletion docs/watcher-continuity.md
Original file line number Diff line number Diff line change
Expand Up @@ -402,7 +402,7 @@ Only the watcher process touches `state/.last-watcher-beat`.
No helper process can make a wedged watcher appear healthy.
An arm whose own script path sits under a disposable no-mistakes validation checkout (`.no-mistakes/worktrees/`) refuses with the typed failure line before touching any state, because a watcher started there outlives the validation step and keeps writing the real home's state from a checkout about to be deleted.
Once per poll the watcher checks that its home, its state directory, and its own code root still exist, and exits with a logged reason when one is gone, scoped to itself alone, so a torn-down temporary home or a discarded checkout never leaves an orphan watcher behind.
The watcher uses bash's native fatal handling for HUP and TERM, including during a blocked poll, so both run its EXIT cleanup.
The watcher uses bash's native fatal handling for HUP and TERM, including during a blocked check or a blocked `fm_backend_capture` pane read, so both run its EXIT cleanup and stop that read.
`watcher_stop_signals` in `bin/fm-watch.sh` owns the signal-handling rationale.
The EXIT cleanup bounds its wait for `state/.watcher-down.lock` while persisting recovery state with `FM_WATCHER_CLEANUP_LOCK_BOUND` (default 2 seconds).
Only positive decimal integers are accepted, including leading-zero forms such as `08`; empty, non-numeric, and zero values (including `00`) fall back to 2 seconds.
Expand Down Expand Up @@ -468,6 +468,7 @@ They also prove that a legacy or handoff-phase watcher marker from an absent rep
`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.
It also exercises a single TERM with a live foreign downtime-marker lock holder, retained stale singleton and subsequent arm-style recovery, including decimal `08` and zero `00` cleanup bounds.
It checks that a newly appended keyed decision is classified without rereading earlier status bytes, so signal handling can return to the watcher's beacon refresh even when the status history is long.
`tests/fm-wake-queue.test.sh` proves TERM likewise stops a watcher blocked in the drain-ring idle check's pane capture.

`tests/fm-watcher-lock.test.sh` covers:

Expand Down
85 changes: 85 additions & 0 deletions tests/fm-wake-queue.test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -914,6 +914,90 @@ test_secondmate_proven_idle_ring_lets_the_child_drain() {
pass "a proven-idle leftover row is rung so the child home can drain without a parent alarm"
}

# The idle proof that gates a drain ring reads the mate's pane. A TERM that
# lands while that read is blocked must still stop the watcher at once and run
# its cleanup, as it does for every other pane read.
test_term_stops_a_watcher_blocked_in_the_drain_ring_idle_capture() {
local dir state sub fakebin fifo out pid holder i rc orphan
dir=$(make_case secondmate-ring-blocked-capture)
state="$dir/state"
sub="$dir/secondmate"
fakebin="$dir/fakebin"
fifo="$dir/pane.fifo"
out="$dir/watch-blocked.out"
mkdir -p "$sub/state"
printf 'mate\n' > "$sub/.fm-secondmate-home"
printf 'window=firstmate:fm-mate\nkind=secondmate\nharness=claude\nbackend=tmux\nhome=%s\n' \
"$sub" > "$state/mate.meta"
printf '100\t7\tcheck\trouted\tcheck: routed row\n' > "$sub/state/.wake-queue"
install_secondmate_alive_tmux "$fakebin"
install_secondmate_stall_date "$fakebin"
"$ROOT/bin/fm-busy-event.sh" arm "$state" mate >/dev/null \
|| fail "could not arm the mate's busy contract"
"$ROOT/bin/fm-busy-event.sh" apply "$state" mate idle --current-gen \
--source claude-hook --event stop >/dev/null \
|| fail "could not mark the mate idle"

printf '1000\n' > "$dir/now"
PATH="$fakebin:$PATH" FM_FAKE_NOW_FILE="$dir/now" FM_HOME="$dir" FM_ROOT_OVERRIDE="$ROOT" \
FM_STATE_OVERRIDE="$state" FM_FAKE_TMUX_SENT="$dir/sent" \
FM_SECONDMATE_WAKE_STALL_SECS=1 FM_POLL=1 FM_SIGNAL_GRACE=0 \
FM_CHECK_INTERVAL=999999 FM_HEARTBEAT=999999 \
secondmate_stall_watch_leg "$dir" "first" progress mate "$(printf '1000\t100-7')"

# The active-turn read returns, then the idle-proof read blocks on a FIFO
# whose write end the holder keeps open without writing.
mv "$fakebin/tmux" "$fakebin/tmux-alive"
cat > "$fakebin/tmux" <<'SH'
#!/usr/bin/env bash
if [ "${1:-}" = capture-pane ]; then
n=$(( $(cat "$FM_FAKE_CAPTURE_COUNT" 2>/dev/null || echo 0) + 1 ))
printf '%s\n' "$n" > "$FM_FAKE_CAPTURE_COUNT"
[ "$n" -ge 2 ] || exit 0
exec cat "$FM_FAKE_TMUX_CAPTURE"
fi
exec "$(dirname "$0")/tmux-alive" "$@"
SH
chmod +x "$fakebin/tmux"
mkfifo "$fifo"
( exec 3> "$fifo"; : > "$dir/capture-blocked"; exec sleep 30 ) &
holder=$!

printf '1002\n' > "$dir/now"
touch "$state/.secondmate-liveness-tick"
PATH="$fakebin:$PATH" FM_FAKE_NOW_FILE="$dir/now" FM_HOME="$dir" FM_ROOT_OVERRIDE="$ROOT" \
FM_STATE_OVERRIDE="$state" FM_FAKE_TMUX_SENT="$dir/sent" \
FM_FAKE_TMUX_CAPTURE="$fifo" FM_FAKE_CAPTURE_COUNT="$dir/captures" \
FM_SECONDMATE_WAKE_STALL_SECS=1 FM_SECONDMATE_LIVENESS_SECS=99999999 \
FM_POLL=1 FM_SIGNAL_GRACE=0 FM_CHECK_INTERVAL=999999 FM_HEARTBEAT=999999 \
"$WATCH" > "$out" 2>&1 &
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; kill "$pid" 2>/dev/null || true; wait "$pid" 2>/dev/null || true
fail "the watcher never blocked inside the drain-ring idle capture: $(cat "$out")"
fi
kill "$pid" 2>/dev/null || true
wait_for_exit "$pid" 100
rc=$?
orphan=$(pgrep -f "cat $fifo" || true)
[ -z "$orphan" ] || pkill -f "cat $fifo" 2>/dev/null || true
kill "$holder" 2>/dev/null || true
wait "$holder" 2>/dev/null || true
[ "$rc" -ne 124 ] || fail "TERM did not stop a watcher blocked in the drain-ring idle capture"
[ -z "$orphan" ] || fail "a watcher stopped in the drain-ring idle capture left that capture running"
[ "$(cat "$dir/captures")" = 2 ] \
|| fail "the watcher did not block in the drain-ring idle capture: $(cat "$dir/captures") pane reads"
[ ! -e "$state/.watch.lock" ] \
|| fail "a watcher stopped in the drain-ring idle capture kept its singleton lock"
[ ! -s "$dir/sent" ] || fail "a watcher stopped before its idle proof still rang the mate"
pass "TERM stops a watcher blocked in the drain-ring idle capture and runs its cleanup"
}

# Busy and unknown panes are never typed into. Busy still defers inside the
# active-turn bound. Unknown keeps the parent alarm. Empty inbox is not idle
# proof, so the unknown fixture starts with no instruction records.
Expand Down Expand Up @@ -3432,6 +3516,7 @@ test_secondmate_reprovisioned_queue_starts_a_fresh_interval
test_secondmate_active_turn_defers_stall_until_the_turn_ends
test_secondmate_long_lived_mate_mid_turn_is_not_a_stall
test_secondmate_proven_idle_ring_lets_the_child_drain
test_term_stops_a_watcher_blocked_in_the_drain_ring_idle_capture
test_secondmate_busy_and_unknown_panes_are_not_rung
test_secondmate_genuine_stall_after_idle_ring_still_alarms
test_secondmate_stall_marker_rejects_symlink
Expand Down
Loading