diff --git a/.gitignore b/.gitignore index dd0a8f1df1..7c39cc1546 100644 --- a/.gitignore +++ b/.gitignore @@ -4,6 +4,7 @@ data/ scratchpad* .no-mistakes/ .lavish/ +.claude/hooks/ .fm-secondmate-home .fm-secondmate-parent .DS_Store diff --git a/bin/fm-watch-arm.sh b/bin/fm-watch-arm.sh index d134f51940..d2167c8897 100755 --- a/bin/fm-watch-arm.sh +++ b/bin/fm-watch-arm.sh @@ -35,8 +35,12 @@ # It NEVER reports started/attached/healthy off a stale beacon or a dead/reused pid: a # stale-beacon or dead-pid holder either self-heals (the fresh child steals the # dead lock per the singleton self-eviction/steal path and is confirmed) or this -# returns the FAILED line. On started it waits the child and propagates the wake -# reason; on attached it stays live across identity-matched successors. A cycle +# returns the FAILED line. After started it keeps verifying the owned child instead +# of reducing liveness to process existence: a live identity-matched child whose +# beacon reaches the shared grace is retired with a bounded TERM/KILL sequence, +# its stale ownership is released through the watcher-down recovery transition, +# and this arm fails loudly so a persistent adapter can retry without a primary +# session restart. On attached it stays live across identity-matched successors. A cycle # that ends with no reason line and no healthy successor is resolved against the # watcher's identity-bound delivery record: a matching record reports that wake # and exits 0, and only a cycle that delivered nothing is the typed nonzero @@ -77,8 +81,15 @@ case "${OSTYPE:-}" in *) ARM_CONFIRM_DEFAULT=10 ;; esac CONFIRM_TIMEOUT=${FM_ARM_CONFIRM_TIMEOUT:-$ARM_CONFIRM_DEFAULT} -# Poll interval while attached to an existing healthy watcher. +# Poll interval while attached to an existing healthy watcher; also the +# cadence for the owned-child wait loop and the watchdog's stale-beacon recheck. ATTACH_POLL=${FM_ARM_ATTACH_POLL:-0.5} +# Seconds allowed for a stalled owned watcher to retire after TERM before its +# isolated process group receives KILL. This is deliberately much shorter than +# the liveness grace: once that grace has elapsed, keeping stale singleton +# ownership longer cannot restore supervision. +STALL_RETIRE_TIMEOUT=${FM_WATCH_STALL_RETIRE_TIMEOUT:-2} +case "$STALL_RETIRE_TIMEOUT" in ''|*[!0-9]*|0) STALL_RETIRE_TIMEOUT=2 ;; esac CYCLE_LOG="$STATE/.watch-cycle-exits.log" CYCLE_LOG_LOCK="$STATE/.watch-cycle-exits.lock" CYCLE_LOG_MAX_BYTES=${FM_WATCH_CYCLE_LOG_MAX_BYTES:-262144} @@ -223,7 +234,12 @@ cycle_mark_predecessor_successor() { } clear_stale_recorded_watcher_lock() { - local lock_home lock_path lock_identity + local expected_pid=${1:-} lock_pid lock_home lock_path lock_identity + lock_pid=$(cat "$WATCH_LOCK/pid" 2>/dev/null || true) + if [ -n "$expected_pid" ]; then + [ "$lock_pid" = "$expected_pid" ] || return 0 + fm_pid_alive "$lock_pid" && return 1 + fi lock_home=$(cat "$WATCH_LOCK/fm-home" 2>/dev/null || true) lock_path=$(cat "$WATCH_LOCK/watcher-path" 2>/dev/null || true) lock_identity=$(cat "$WATCH_LOCK/pid-identity" 2>/dev/null || true) @@ -447,11 +463,139 @@ fi # harness-tracked task) tears the watcher down too, and the watcher's eventual # wake exit propagates out so the harness re-notifies firstmate. child= +child_group= child_out= -cleanup_child() { - if [ -n "$child" ] && fm_pid_alive "$child"; then - kill -TERM "$child" 2>/dev/null || true +watchdog_pid= +watchdog_status= + +watch_child_running() { + local proc_root stat state_line + local -a stat_fields + [ -n "$child" ] || return 1 + fm_pid_alive "$child" || return 1 + proc_root=${FM_PROC_ROOT_OVERRIDE:-/proc} + if [ -r "$proc_root/$child/stat" ]; then + state_line=$(cat "$proc_root/$child/stat" 2>/dev/null) || return 0 + read -r -a stat_fields <<< "${state_line##*)}" + stat=${stat_fields[0]:-} + else + stat=$(ps -p "$child" -o stat= 2>/dev/null | sed 's/^[[:space:]]*//' || true) + fi + case "$stat" in + Z*) return 1 ;; + esac + return 0 +} + +signal_watch_child() { # + local signal=$1 + if [ -n "$child_group" ]; then + kill -"$signal" -- "-$child_group" 2>/dev/null || true + elif [ -n "$child" ]; then + kill -"$signal" "$child" 2>/dev/null || true + fi +} + +# Retire the owned watcher without ever waiting indefinitely on the same child +# that caused the liveness failure. WATCH_CHILD_RC records the reaped status, or +# 124 if even KILL could not make the direct child waitable inside the bound. +# TERM is sent first and the child is given STALL_RETIRE_TIMEOUT to exit on its +# own, so a healthy watcher keeps the chance to persist recovery through its own +# EXIT trap. KILL escalates to the whole isolated process group only when the +# child is STILL alive after that bound: a routine teardown of a watcher that +# honored TERM never lands a group-wide SIGKILL on a fresh-beacon holder. +WATCH_CHILD_RC=0 +retire_watch_child() { + local deadline + WATCH_CHILD_RC=0 + [ -n "$child" ] || return 0 + if watch_child_running; then + signal_watch_child TERM + deadline=$(( $(date +%s) + STALL_RETIRE_TIMEOUT + 1 )) + while watch_child_running && [ "$(date +%s)" -lt "$deadline" ]; do + sleep 0.05 + done + fi + if watch_child_running; then + # The watcher did not exit in the TERM bound: sweep the whole isolated group + # so a hung backend helper is not orphaned next to the stalled lock holder. + signal_watch_child KILL + deadline=$(( $(date +%s) + 2 )) + while watch_child_running && [ "$(date +%s)" -lt "$deadline" ]; do + sleep 0.05 + done fi + if watch_child_running; then + WATCH_CHILD_RC=124 + return 1 + fi + if wait "$child" 2>/dev/null; then + WATCH_CHILD_RC=0 + else + WATCH_CHILD_RC=$? + fi + child= + child_group= + return 0 +} + +owned_child_has_stale_beacon() { + local lock_pid age + age=$(fm_path_age "$BEAT") + [ "$age" -ge "$GRACE" ] || return 1 + lock_pid=$(cat "$WATCH_LOCK/pid" 2>/dev/null || true) + [ "$lock_pid" = "$child" ] || return 1 + fm_watcher_lock_matches_pid "$STATE" "$WATCH" "$child" "$FM_HOME" || return 1 + return 0 +} + +stop_owned_watchdog() { + [ -n "$watchdog_pid" ] || return 0 + kill -TERM "$watchdog_pid" 2>/dev/null || true + wait "$watchdog_pid" 2>/dev/null || true + watchdog_pid= +} + +# Keep the arm following the child through a short poll so an actionable child +# close still propagates promptly, and once the watchdog has retired the +# stalled group the same poll is bounded by the retire deadline below instead +# of hanging on a child KILL cannot make waitable. A separate arm-owned +# watchdog performs only the stale-beacon check and retires the isolated +# watcher group when the shared grace expires. +start_owned_watchdog() { + watchdog_status="$child_out.liveness" + rm -f "$watchdog_status" 2>/dev/null || true + ( + watchdog_sleep_pid= + # shellcheck disable=SC2329 # Invoked indirectly by the signal trap below. + stop_watchdog_sleep() { + [ -z "$watchdog_sleep_pid" ] || kill -TERM "$watchdog_sleep_pid" 2>/dev/null || true + exit 0 + } + trap stop_watchdog_sleep HUP TERM INT + while fm_pid_alive "$child"; do + if owned_child_has_stale_beacon; then + fm_path_age "$BEAT" > "$watchdog_status" + signal_watch_child TERM + sleep "$STALL_RETIRE_TIMEOUT" + signal_watch_child KILL + exit 0 + fi + # Wait through a background sleep so the arm's stop signal interrupts the + # wait immediately; a foreground sleep defers Bash's trap and delays every + # healthy actionable close by the full polling interval. + sleep "$ATTACH_POLL" & + watchdog_sleep_pid=$! + wait "$watchdog_sleep_pid" 2>/dev/null || true + watchdog_sleep_pid= + done + ) & + watchdog_pid=$! +} + +cleanup_child() { + stop_owned_watchdog + retire_watch_child || true if [ -n "$child_out" ]; then rm -f "$child_out" 2>/dev/null || true fi @@ -461,10 +605,6 @@ cleanup_child() { handle_arm_signal() { local signal=$1 rc=$2 trap - HUP TERM INT - if [ -n "$child" ] && fm_pid_alive "$child"; then - kill -TERM "$child" 2>/dev/null || true - wait "$child" 2>/dev/null || true - fi cycle_log_append "$rc" "$signal" arm-interrupted none cleanup_child exit "$rc" @@ -478,12 +618,21 @@ child_out=$(mktemp "$STATE/.watch-arm-output.XXXXXX") || { echo "watcher: FAILED - no live watcher with a fresh beacon" exit 1 } +# Give the owned watcher a separate process group. The stale-beacon path can +# then retire a hung backend helper together with the watcher instead of killing +# only the lock holder and orphaning the subprocess it was blocked on. +monitor_was_on=0 +case $- in *m*) monitor_was_on=1 ;; esac +set -m if [ -n "${FM_WATCH_PREDECESSOR_ARM_PID:-}" ]; then - FM_WATCH_HANDLING_SUCCESSOR=1 "$WATCH" >"$child_out" & + ( set +m; export FM_WATCH_HANDLING_SUCCESSOR=1; exec "$WATCH" ) >"$child_out" & else - "$WATCH" >"$child_out" & + ( set +m; exec "$WATCH" ) >"$child_out" & fi child=$! +[ "$monitor_was_on" -eq 1 ] || set +m +child_group=$(ps -p "$child" -o pgid= 2>/dev/null | tr -d '[:space:]' || true) +[ "$child_group" = "$child" ] || child_group= cycle_begin "$child" started "$(fm_pid_identity "$child" 2>/dev/null || true)" child_done=0 @@ -540,6 +689,75 @@ owned_child_finished() { return "$status" } +# Follow a watcher this arm actually forked while rechecking the same strict +# identity+beacon predicate used for initial readiness. The old raw `wait` +# could never observe an alive-but-stalled watcher, so Pi/OpenCode kept an arm +# claim forever and every repair call became an ownership no-op. +wait_owned_child() { + local stalled_pid age rc retire_deadline + stalled_pid=$child + start_owned_watchdog + rc=124 + retire_deadline= + while watch_child_running; do + if [ -z "$retire_deadline" ] && [ -s "$watchdog_status" ]; then + retire_deadline=$(( $(date +%s) + STALL_RETIRE_TIMEOUT + 3 )) + fi + if [ -n "$retire_deadline" ] && [ "$(date +%s)" -ge "$retire_deadline" ]; then + break + fi + sleep "$ATTACH_POLL" + done + if watch_child_running; then + WATCH_CHILD_RC=124 + elif wait "$child" 2>/dev/null; then + rc=0 + else + rc=$? + fi + stop_owned_watchdog + if [ -s "$watchdog_status" ]; then + age=$(cat "$watchdog_status" 2>/dev/null || fm_path_age "$BEAT") + signal_watch_child KILL + # If the deadline broke the loop with the child still running, it was never + # reaped by the `elif wait` above. Give the KILL a bounded moment to drop the + # process into a reapable state so the arm does not leave a zombie or a lock + # holder behind before returning. + bail_deadline=$(( $(date +%s) + 2 )) + while fm_pid_alive "$child" && [ "$(date +%s)" -lt "$bail_deadline" ]; do + sleep 0.05 + done + # Only reap when the child is dead or a reapable zombie; a child that truly + # survives SIGKILL (an uninterruptible D-state wait) must not block this arm + # forever on `wait` - that would reintroduce the unbounded hang the bounded + # retirement exists to prevent. Reaping a zombie always returns immediately, + # so a normal retire is unchanged. A still-alive child then fails the stale + # release below, keeping its lock exactly as the retirement contract records. + if [ -n "$child" ] && { ! fm_pid_alive "$child" || ! watch_child_running; }; then + wait "$child" 2>/dev/null || true + fi + child= + child_group= + if ! fm_recovery_marker_publish "$STATE/.watcher-down" downtime \ + || ! clear_stale_recorded_watcher_lock "$stalled_pid"; then + cycle_log_append "$rc" "$(cycle_signal_name "$rc")" stale-beacon-release-failed none + echo "watcher: FAILED - watcher pid=$stalled_pid stopped advancing its beacon for ${age}s; recovery state could not release stale ownership" + return 1 + fi + cycle_log_append "$rc" "$(cycle_signal_name "$rc")" stale-beacon-retired none + rm -f "$child_out" "$watchdog_status" 2>/dev/null || true + child= + child_group= + child_out= + watchdog_status= + echo "watcher: FAILED - watcher pid=$stalled_pid stopped advancing its beacon for ${age}s; retired the stalled cycle and released stale ownership for bounded recovery" + return 1 + fi + rm -f "$watchdog_status" 2>/dev/null || true + watchdog_status= + owned_child_finished "$rc" +} + # Verify the outcome: poll until this child is the confirmed healthy watcher, or # until some other watcher legitimately holds the singleton (a startup race), or # until the child gives up. Only then print the honest line. @@ -552,7 +770,6 @@ while :; do cycle_refresh_lock_before if ! handling_generation=$(handling_successor_generation); then cleanup_child - wait "$child" 2>/dev/null || true cycle_log_append 1 none handling-handoff-failed none echo "watcher: FAILED - established successor could not inspect handling state" exit 1 @@ -563,12 +780,25 @@ while :; do else echo "watcher: started pid=$child (beacon fresh)" fi - wait "$child" - rc=$? - owned_child_finished "$rc" + wait_owned_child exit $? fi - # Another watcher won the singleton; our child stood down. + # Another watcher won the singleton; our child stood down. Give it a + # bounded chance to exit so a live-but-stalled child can never block the + # arm in wait forever, mirroring the won-race retirement above. + if watch_child_running; then + retire_deadline=$(( $(date +%s) + STALL_RETIRE_TIMEOUT + CONFIRM_TIMEOUT )) + while watch_child_running && [ "$(date +%s)" -lt "$retire_deadline" ]; do + sleep "$ATTACH_POLL" + done + fi + if watch_child_running; then + stalled_pid=$child + cleanup_child + cycle_log_append "$WATCH_CHILD_RC" "$(cycle_signal_name "$WATCH_CHILD_RC")" child-stand-down-stalled none + echo "watcher: FAILED - our child pid=$stalled_pid stalled before standing down; retired the stalled child" + exit 1 + fi wait "$child" rc=$? owned_child_finished "$rc" @@ -588,8 +818,7 @@ done trap - HUP TERM INT print_watch_output "$child_out" cleanup_child -wait "$child" 2>/dev/null -rc=$? +rc=$WATCH_CHILD_RC cycle_log_append "$rc" "$(cycle_signal_name "$rc")" confirmation-timeout none echo "watcher: FAILED - no live watcher with a fresh beacon" exit 1 diff --git a/bin/fm-watch.sh b/bin/fm-watch.sh index b3ef034481..fe6e171d31 100755 --- a/bin/fm-watch.sh +++ b/bin/fm-watch.sh @@ -1115,6 +1115,22 @@ resurface_after_downtime() { wake "check: rearm-resurface" } +if [ "${FM_WATCH_HANDLING_SUCCESSOR:-0}" = 1 ]; then + touch "$STATE/.last-watcher-beat" + handling_wait=0 + while [ "$handling_wait" -lt 600 ]; do + touch "$STATE/.last-watcher-beat" + fm_recovery_marker_snapshot "$WATCHER_DOWNTIME_MARKER" || true + case "$FM_RECOVERY_MARKER_TOKEN" in + pending:downtime:*) ;; + *) break ;; + esac + sleep 0.05 + handling_wait=$((handling_wait + 1)) + done + [ "$handling_wait" -lt 600 ] || WATCHER_RECOVERY_PENDING=1 +fi + while :; do # Self-eviction: if the singleton lock no longer names this process, a second # watcher has taken over (e.g. a transient duplicate from a racy arm). Stand diff --git a/docs/configuration.md b/docs/configuration.md index df27bcbda2..19b8a81700 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -693,7 +693,8 @@ FM_CLAUDE_AUTOARM_SYNC_WAIT_MS=800 # milliseconds the --claude turn-end guard FM_CLAUDE_AUTOARM_EPOCH_FRESH=15 # seconds a recorded auto-arm outcome remains eligible for the current event epoch's recovery or failure decision FM_CLAUDE_TURNEND_BLOCK_BUDGET=3 # consecutive --claude guard re-blocks before the verified one-time attended fail-open; safely below Claude Code's 8-block override FM_ARM_CONFIRM_TIMEOUT=10 # seconds fm-watch-arm waits to confirm a fresh watcher before reporting FAILED; default 30 on Git Bash/MSYS -FM_ARM_ATTACH_POLL=0.5 # seconds between checks while fm-watch-arm is attached to an existing healthy watcher cycle +FM_ARM_ATTACH_POLL=0.5 # seconds between checks while fm-watch-arm follows a healthy watcher cycle: attached to a peer, or polling its forked child's liveness and the watchdog's stale-beacon recheck +FM_WATCH_STALL_RETIRE_TIMEOUT=2 # seconds an owned live-but-stale watcher gets after TERM before fm-watch-arm kills its isolated process group and releases stale ownership; a child that survives the bound keeps its lock, and invalid or zero values use 2 FM_OPENCODE_ARM_READY_TIMEOUT_MS=12000 # milliseconds the OpenCode primary watcher plugin waits for an arm attempt to report started, healthy, wake, or failure; default 35000 on Windows to stay above the MSYS confirm budget FM_PI_ARM_READY_TIMEOUT_MS=12000 # milliseconds the Pi watcher extension waits for a successor arm to report started or attached; default 35000 on Windows to stay above the MSYS confirm budget FM_WATCH_ARM_RETIRE_TIMEOUT_MS=1000 # milliseconds Pi/OpenCode wait for an unready successor arm to exit before abandoning retries diff --git a/docs/watcher-continuity.md b/docs/watcher-continuity.md index cf458e2640..38a546e970 100644 --- a/docs/watcher-continuity.md +++ b/docs/watcher-continuity.md @@ -68,6 +68,10 @@ An attached arm follows verified identity-matched successors and resolves the sa Before releasing its singleton lock after printing an actionable reason, the watcher records that reason with its PID and process identity in `state/.watch-deliveries.log`. A matching PID and identity lets an attached arm report the delivered reason and exit zero even after its durable wake was handled and acknowledged, while an unrelated queue producer or a recycled PID cannot satisfy the match. Only a cycle with no matching delivery record emits `watcher: FAILED - cycle ended without an actionable reason` and exits nonzero. +After initial readiness, an arm that forked the watcher keeps applying the same identity-bound beacon predicate instead of treating a live PID as permanent health. +When that owned watcher reaches the shared stale-beacon grace, the arm sends TERM and then KILL to the watcher's isolated process group within `FM_WATCH_STALL_RETIRE_TIMEOUT`, publishes the existing watcher-down recovery episode, and exits with a typed failure. +The stale lock is removed only once the child is dead and still matches the expected PID, so a child that survives the bound keeps its lock and the ledger records the refused release. +Persistent adapters therefore lose their owned-child no-op when the child is no longer healthy and can run their existing bounded retry without restarting the primary session. The arm layer appends one tab-separated record per observed cycle to `state/.watch-cycle-exits.log`. Each record includes arm and watcher PIDs, start and end timestamps, exit code and signal, classified reason, beacon age, lock identity before and after close, and successor disposition. @@ -83,7 +87,7 @@ Only the watcher process touches `state/.last-watcher-beat`; no helper process c The same suite covers ordinary same-process session replacement for `/new`, `/resume`, and `/fork`, same-instance shutdown-plus-start, stale prior-generation callbacks, repeated transitions with exactly one live cycle, disappearance of the shutting-down refusal after a valid replacement activates, and terminal quit still refusing late rearm. `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-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-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 real-process SIGSTOP counterfactual that proves the bounded retirement contract on both platforms: on Linux the retirement's KILL kills the stopped watcher immediately and the reaped child yields the released-lock shape, and on other hosts the same relinquished lock followed by same-session re-arm surfacing the accepted downtime episode and a healthy successor whose beacon keeps advancing. The refusal shape that keeps a live holder's lock is not exercised by this counterfactual and is described at the arm-layer contract above. `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, and exit-2 translation. It also covers abandoned single-flight claims: a claim the ledger shows already finished, and one whose recorded pid-identity no longer matches its live pid while the ledger still reads arming or is absent entirely, are both reclaimed so a lapsed home re-arms, while an identity-matched claim still arming, one the ledger does not name, and the guard's own terminal check keep the gate closed ([`turnend-guard.md`](turnend-guard.md) owns that boundary). diff --git a/tests/fm-watch-arm.test.sh b/tests/fm-watch-arm.test.sh index 2a3a5173c4..f93092aecd 100755 --- a/tests/fm-watch-arm.test.sh +++ b/tests/fm-watch-arm.test.sh @@ -141,7 +141,8 @@ drain_ack_pair() { # start_rearm_arm() { # [predecessor-arm-pid] local home=$1 state=$2 fakebin=$3 armout=$4 predecessor=${5:-} i PATH="$fakebin:$PATH" FM_HOME="$home" FM_STATE_OVERRIDE="$state" \ - FM_POLL=1 FM_SIGNAL_GRACE=0 FM_CHECK_INTERVAL=999999 FM_HEARTBEAT=999999 \ + FM_ARM_CONFIRM_TIMEOUT=60 FM_POLL=1 FM_SIGNAL_GRACE=0 \ + FM_CHECK_INTERVAL=999999 FM_HEARTBEAT=999999 \ FM_WATCH_PREDECESSOR_ARM_PID="$predecessor" \ "$WATCH_ARM" --restart > "$armout" & ARM_PID=$! @@ -287,16 +288,14 @@ test_rearm_resurfaces_durable_queue_and_remote_open_decision() { append_wake "$state" check startup-network 'check: startup-network' start_rearm_arm "$home" "$state" "$fakebin" "$armout" - sleep 0.25 - if is_live_non_zombie "$ARM_PID"; then - # End the fixture through an ordinary actionable status transition so this - # failing pre-fix path leaves no child behind. - printf 'done: fixture cleanup\n' > "$state/cleanup.status" - wait_for_exit "$ARM_PID" 80 || true + # The arm reaps its liveness watchdog before returning the watcher reason, so + # assert the recovery's bounded outcome instead of a scheduler-sensitive + # quarter-second process snapshot. + wait_for_exit "$ARM_PID" 100 + status=$? + if [ "$status" -eq 124 ]; then fail "re-arm stayed live instead of surfacing durable wakes and the still-open remote decision" fi - wait "$ARM_PID" - status=$? expect_code 0 "$status" "re-arm re-surface wake must close successfully" grep -F 'check: rearm-resurface' "$armout" >/dev/null \ || fail "re-arm did not report the durable recovery wake: $(cat "$armout")" @@ -779,6 +778,67 @@ test_moved_generation_acknowledgement_is_self_healing() { pass "watch-arm: a moved recovery generation consumes handled rows and names its remedy" } +# A lost startup race (another watcher holds the singleton, so the arm's own +# forked child must stand down) must never block the arm forever on that child. +# Predicate: while the "other" watcher is healthy, the arm's own child is alive +# but genuinely stalled (hung before it can stand down). The arm retires the +# stalled child within a bounded wait and fails loudly instead of blocking on +# `wait $child` indefinitely - the exact regression this bounds the forever-wait +# against. +test_lost_race_child_stand_down_is_bounded() { + local dir home state fakebin armout holder_pid identity rc + dir=$(make_case lost-race-stalled-child) + home="$dir/home" + state="$dir/state" + fakebin="$dir/fakebin" + armout="$dir/arm.out" + mkdir -p "$home" + + # A live process P holds a well-formed singleton lock, so the arm's healthy + # predicate can name a healthy "other" watcher that is NOT the arm's child. + sleep 1000 & holder_pid=$! + identity=$(LC_ALL=C ps -p "$holder_pid" -o lstart= -o command= | sed 's/^ *//') + mkdir -p "$state/.watch.lock" + printf '%s\n' "$holder_pid" > "$state/.watch.lock/pid" + printf '%s\n' "$home" > "$state/.watch.lock/fm-home" + printf '%s\n' "$WATCH" > "$state/.watch.lock/watcher-path" + printf '%s\n' "$identity" > "$state/.watch.lock/pid-identity" + # The arm's own forked child is the real watcher. A FIFO in the state dir makes + # its pre-lock check migration block on an open-for-read with no writer, so the + # child stays alive and never stands down - the stalled-child shape the arm + # must retire instead of waiting on forever. + mkfifo "$state/stand-down.block.check.sh" + # No fresh beacon yet: the delegated holder is NOT healthy at arm start, so the + # arm forks a child rather than attaching. The beacon is published mid-window + # to flip the holder healthy while that child is still stalled. + PATH="$fakebin:$PATH" FM_HOME="$home" FM_STATE_OVERRIDE="$state" \ + FM_ARM_ATTACH_POLL=0.05 FM_ARM_CONFIRM_TIMEOUT=3 FM_POLL=999999 \ + FM_SIGNAL_GRACE=1 FM_CHECK_INTERVAL=999999 FM_HEARTBEAT=999999 \ + "$WATCH_ARM" > "$armout" 2>&1 & + ARM_PID=$! + sleep 0.6 + touch "$state/.last-watcher-beat" + + wait_for_exit "$ARM_PID" 150 + rc=$? + kill "$holder_pid" 2>/dev/null || true + wait "$holder_pid" 2>/dev/null || true + + # The arm must have exited on its own (bounded), not have been killed by the + # wait_for_exit deadline (124: a pre-fix arm blocks forever on the stalled + # child and only dies when the tester kills it). + [ "$rc" -ne 124 ] \ + || fail "the arm blocked forever on the stalled lost-race child instead of retiring it" + grep -qF 'stalled before standing down' "$armout" \ + || fail "the arm did not report the stalled stand-down child: $(cat "$armout")" + grep -q '^watcher: FAILED' "$armout" \ + || fail "a retired stalled child must fail the cycle loudly: $(cat "$armout")" + grep -q 'reason=child-stand-down-stalled' "$state/.watch-cycle-exits.log" \ + || fail "the stalled stand-down was not classified in the lifecycle ledger" + is_live_non_zombie "$ARM_PID" && fail "the arm is still alive after reporting" + pass "watch-arm: a lost-race child that stalls before standing down is retired instead of blocking the arm forever" +} + test_downtime_marker_does_not_follow_symlink() { local dir home state fakebin armout watcher_pid sentinel dir=$(make_case downtime-marker-symlink) @@ -817,4 +877,5 @@ test_restart_preserves_recovery_across_reused_pid_lock test_markerless_legacy_queue_is_recovered_on_arm test_handling_window_close_keeps_the_acknowledgement_valid test_moved_generation_acknowledgement_is_self_healing +test_lost_race_child_stand_down_is_bounded test_downtime_marker_does_not_follow_symlink diff --git a/tests/fm-watcher-lock.test.sh b/tests/fm-watcher-lock.test.sh index 482e425a9f..d7b3f6b206 100755 --- a/tests/fm-watcher-lock.test.sh +++ b/tests/fm-watcher-lock.test.sh @@ -198,24 +198,36 @@ test_guard_warnings() { } test_lock_single_winner_under_concurrency() { - local dir state lockdir marker i pids pid wins + local dir state lockdir marker contrib i pids pid wins total others dir=$(make_case lock-concurrency) state="$dir/state" lockdir="$state/.contend.lock" marker="$dir/wins" + contrib="$dir/contributions" : > "$marker" + : > "$contrib" + total=40 + others=$((total - 1)) pids= i=1 - while [ "$i" -le 40 ]; do + while [ "$i" -le "$total" ]; do FM_STATE_OVERRIDE="$state" bash -c ' . "$1" if fm_lock_try_acquire "$2"; then printf "%s\n" "$$" >> "$3" - # Stay alive so the held lock names a live pid for the whole window; - # otherwise a late contender could legitimately reclaim a dead-pid lock. - sleep 1 + # Hold the lock until every OTHER contender has had its chance. A + # fixed sleep is shorter than the contention window on a loaded or + # multi-core host, so an early "winner" once exited and had its lock + # legitimately reclaimed as a dead-pid lock - a second winner in the + # ledger. Staying alive until all losers have finished makes the + # single-winner invariant hold deterministically. + while [ "$(wc -l < "$4" 2>/dev/null || true)" -lt "$5" ]; do + sleep 0.01 + done + else + printf "%s\n" "$$" >> "$4" fi - ' _ "$LIB" "$lockdir" "$marker" & + ' _ "$LIB" "$lockdir" "$marker" "$contrib" "$others" & pids="$pids $!" i=$((i + 1)) done @@ -247,25 +259,37 @@ test_lock_steals_dead_pid_lock() { } test_lock_stale_steal_single_winner_under_concurrency() { - local dir state lockdir dead marker i pids pid wins + local dir state lockdir dead marker contrib i pids pid wins total others dir=$(make_case lock-stale-concurrency) state="$dir/state" lockdir="$state/.contend.lock" marker="$dir/wins" + contrib="$dir/contributions" dead=$(dead_pid) mkdir "$lockdir" printf '%s\n' "$dead" > "$lockdir/pid" : > "$marker" + : > "$contrib" + total=40 + others=$((total - 1)) pids= i=1 - while [ "$i" -le 40 ]; do + while [ "$i" -le "$total" ]; do FM_STATE_OVERRIDE="$state" bash -c ' . "$1" if fm_lock_try_acquire "$2"; then printf "%s\n" "${BASHPID:-$$}" >> "$3" - sleep 1 + # Hold the stolen lock until every OTHER contender has had its chance, + # mirroring test_lock_single_winner_under_concurrency: a fixed sleep is + # shorter than the contention window on a loaded host, so an early + # "winner" previously exited and had its dead-pid lock reclaimed again. + while [ "$(wc -l < "$4" 2>/dev/null || true)" -lt "$5" ]; do + sleep 0.01 + done + else + printf "%s\n" "$$" >> "$4" fi - ' _ "$LIB" "$lockdir" "$marker" & + ' _ "$LIB" "$lockdir" "$marker" "$contrib" "$others" & pids="$pids $!" i=$((i + 1)) done @@ -925,18 +949,27 @@ SH pass "cycle-exit ledger links a verified successor and remains size-capped" } -test_stopped_watcher_is_live_but_stale_then_exit_is_classified() { - local dir state fakebin armout armpid watcher_pid i status +test_stopped_watcher_is_retired_and_rearms_without_session_restart() { + local dir state fakebin armout recovery_out healthy_out armpid watcher_pid i status + local recovery_arm healthy_arm healthy_pid beat_before beat_after token wedge_limit dir=$(make_case stopped-watcher) state="$dir/state" fakebin="$dir/fakebin" armout="$dir/arm.out" + recovery_out="$dir/recovery.out" + healthy_out="$dir/healthy.out" mark_pr_check_migration_complete "$state" - PATH="$fakebin:$PATH" FM_HOME="$dir" FM_STATE_OVERRIDE="$state" FM_POLL=5 FM_SIGNAL_GRACE=1 FM_CHECK_INTERVAL=999999 FM_HEARTBEAT=999999 "$WATCH_ARM" > "$armout" & + PATH="$fakebin:$PATH" FM_HOME="$dir" FM_STATE_OVERRIDE="$state" \ + FM_GUARD_GRACE=30 FM_ARM_CONFIRM_TIMEOUT=60 FM_ARM_ATTACH_POLL=0.05 FM_POLL=0.1 FM_SIGNAL_GRACE=1 \ + FM_CHECK_INTERVAL=999999 FM_HEARTBEAT=999999 "$WATCH_ARM" > "$armout" & armpid=$! i=0 - while [ "$i" -lt 80 ]; do + # Under host contention the owned child's first fresh beacon can land after + # the arm's default 11s confirmation deadline, so poll through the widened + # confirm budget and fail fast on a loud typed failure instead. + while [ "$i" -lt 650 ]; do grep -qF 'watcher: started pid=' "$armout" 2>/dev/null && break + grep -qF 'watcher: FAILED' "$armout" 2>/dev/null && break sleep 0.1 i=$((i + 1)) done @@ -951,14 +984,108 @@ test_stopped_watcher_is_live_but_stale_then_exit_is_classified() { fail "SIGSTOP watcher with a stale beacon was classified healthy" fi - kill -CONT "$watcher_pid" 2>/dev/null || true - kill -TERM "$watcher_pid" 2>/dev/null || true - wait_for_exit "$armpid" 80 + i=0 + wedge_limit=80 + [ "$(uname)" != Linux ] || wedge_limit=160 + while [ "$i" -lt "$wedge_limit" ] && is_live_non_zombie "$armpid"; do + sleep 0.1 + i=$((i + 1)) + done + if is_live_non_zombie "$armpid"; then + # Pre-fix cleanup: a raw wait on the stopped child held this arm forever. + # Continue the watcher before terminating it so this failing regression + # never strands a stopped process in the test host. + kill -CONT "$watcher_pid" 2>/dev/null || true + kill -TERM "$watcher_pid" 2>/dev/null || true + wait "$armpid" 2>/dev/null || true + fail "arm stayed wedged behind a live watcher whose beacon was stale" + fi + wait "$armpid" + status=$? + [ "$status" -ne 0 ] || fail "stale-beacon retirement did not fail the owned arm loudly" + grep -F 'watcher: FAILED - watcher pid=' "$armout" >/dev/null \ + || fail "stale-beacon retirement omitted its typed watcher failure: $(cat "$armout")" + grep -F 'stopped advancing its beacon' "$armout" >/dev/null \ + || fail "stale-beacon retirement did not name the liveness failure: $(cat "$armout")" + token=$(cat "$state/.watcher-down" 2>/dev/null || true) + case "$token" in + pending:downtime:*) ;; + *) fail "stale-beacon retirement did not publish watcher-down recovery state: '$token'" ;; + esac + case "$(uname)" in + Linux) + # SIGKILL is never held pending for a stopped process on Linux: the + # retirement's KILL kills the stopped watcher immediately, the arm's + # wait reaps it before the expected-pid hardening runs, and the + # retirement takes the released-lock shape (stale-beacon-retired), not + # the release-failed shape. + ! is_live_non_zombie "$watcher_pid" \ + || fail "bounded retirement did not kill the stopped watcher on Linux" + [ ! -e "$state/.watch.lock" ] && [ ! -L "$state/.watch.lock" ] \ + || fail "stalled watcher retained singleton ownership after retirement on Linux" + grep -q 'reason=stale-beacon-retired' "$state/.watch-cycle-exits.log" \ + || fail "stale-beacon retirement was not classified in the lifecycle ledger on Linux" + kill -CONT "$watcher_pid" 2>/dev/null || true + wait_for_exit "$watcher_pid" 40 2>/dev/null || true + pass "owned arm bounds its retirement of a stopped watcher and fails loudly on Linux" + return 0 + ;; + esac + ! is_live_non_zombie "$watcher_pid" \ + || fail "stalled watcher remained alive after bounded retirement" + [ ! -e "$state/.watch.lock" ] && [ ! -L "$state/.watch.lock" ] \ + || fail "stalled watcher retained singleton ownership after retirement" + grep -q 'reason=stale-beacon-retired' "$state/.watch-cycle-exits.log" \ + || fail "stale-beacon retirement was not classified in the lifecycle ledger" + + # A fresh arm in the same primary session must take ownership and surface the + # accepted downtime episode. No Pi/Herdr process is involved in this fixture. + PATH="$fakebin:$PATH" FM_HOME="$dir" FM_STATE_OVERRIDE="$state" \ + FM_GUARD_GRACE=30 FM_ARM_CONFIRM_TIMEOUT=60 FM_ARM_ATTACH_POLL=0.05 FM_POLL=0.1 FM_SIGNAL_GRACE=1 \ + FM_CHECK_INTERVAL=999999 FM_HEARTBEAT=999999 "$WATCH_ARM" > "$recovery_out" & + recovery_arm=$! + wait_for_exit "$recovery_arm" 140 status=$? - [ "$status" -ne 0 ] && [ "$status" -ne 124 ] || fail "terminated stopped-watcher cycle did not surface nonzero (status $status)" - grep -Eq 'reason=(nonzero-exit|signal-exit)' "$state/.watch-cycle-exits.log" \ - || fail "terminated watcher exit was not classified in the lifecycle ledger" - pass "SIGSTOP distinguishes live PID from stale beacon and termination records the exit class" + expect_code 0 "$status" "same-session recovery arm must surface accepted watcher downtime" + grep -F 'check: rearm-resurface' "$recovery_out" >/dev/null \ + || fail "same-session recovery arm did not surface the watcher-down episode: $(cat "$recovery_out")" + drain_and_ack "$state" || fail "same-session watcher recovery acknowledgement failed" + + # Once the recovery episode is acknowledged, the next healthy cycle stays + # live and keeps advancing its real watcher-owned beacon. The real watcher + # advances the beacon once per main-loop iteration, which takes ~2-3.5s on a + # loaded host, so this arm's grace must outrun that cadence: a grace tighter + # than the cadence makes the arm's own stale-beacon watchdog retire a + # healthy child. Poll through the grace with a liveness check per tick, then + # require the beacon to have advanced. + PATH="$fakebin:$PATH" FM_HOME="$dir" FM_STATE_OVERRIDE="$state" \ + FM_GUARD_GRACE=30 FM_ARM_CONFIRM_TIMEOUT=60 FM_ARM_ATTACH_POLL=0.05 FM_POLL=0.1 FM_SIGNAL_GRACE=1 \ + FM_CHECK_INTERVAL=999999 FM_HEARTBEAT=999999 "$WATCH_ARM" > "$healthy_out" & + healthy_arm=$! + i=0 + while [ "$i" -lt 650 ]; do + grep -qF 'watcher: started pid=' "$healthy_out" 2>/dev/null && break + grep -qF 'watcher: FAILED' "$healthy_out" 2>/dev/null && break + sleep 0.1 + i=$((i + 1)) + done + healthy_pid=$(cat "$state/.watch.lock/pid" 2>/dev/null || true) + grep -qF "watcher: started pid=$healthy_pid" "$healthy_out" \ + || fail "healthy recovery cycle did not establish: $(cat "$healthy_out")" + beat_before=$(FM_STATE_OVERRIDE="$state" bash -c '. "$1"; fm_path_mtime "$2"' _ "$LIB" "$state/.last-watcher-beat") + i=0 + while [ "$i" -lt 140 ]; do + sleep 0.1 + is_live_non_zombie "$healthy_arm" \ + || fail "healthy arm was retired by the stale-beacon watchdog" + i=$((i + 1)) + done + beat_after=$(FM_STATE_OVERRIDE="$state" bash -c '. "$1"; fm_path_mtime "$2"' _ "$LIB" "$state/.last-watcher-beat") + [ "$beat_after" -gt "$beat_before" ] \ + || fail "healthy watcher did not advance its beacon ($beat_before -> $beat_after)" + kill -HUP "$healthy_arm" 2>/dev/null || true + wait "$healthy_arm" 2>/dev/null || true + pass "owned arm retires a live stale watcher, releases recovery state, and preserves a healthy successor" } test_pid_identity_is_locale_invariant() { @@ -1149,4 +1276,4 @@ test_arm_propagates_immediate_wake_before_confirmation test_arm_waits_for_peer_beacon_after_child_stands_down test_arm_fails_loud_when_no_fresh_watcher_confirmable test_cycle_exit_ledger_links_successor_and_stays_bounded -test_stopped_watcher_is_live_but_stale_then_exit_is_classified +test_stopped_watcher_is_retired_and_rearms_without_session_restart