From 3d0876da9f5013973b24f47e5726957588e39c82 Mon Sep 17 00:00:00 2001 From: Joseph Kim Date: Fri, 14 Aug 2026 23:28:50 -0700 Subject: [PATCH 01/19] fix(supervision): retire resolved pending-replies and keep beacon alive Answered pending-reply records were left on disk forever, so the watcher tick walked a growing archive every poll and could starve the liveness beacon past grace while still healthy (false WATCHER DOWN). Retire only resolved records after closing any open escalation, touch the beacon at bounded intervals during a large walk, and attach re-arms to a live identity-matched holder even when its beacon is temporarily stale mid poll so Stop auto-arm does not thrash replacement cycles. --- bin/fm-pending-reply-lib.sh | 87 ++++++++++++-- bin/fm-watch-arm.sh | 174 ++++++++++++++++++++++----- bin/fm-watch.sh | 6 +- tests/fm-claude-stop-autoarm.test.sh | 36 ++++++ tests/fm-pending-reply.test.sh | 143 ++++++++++++++++++++-- tests/fm-watch-arm.test.sh | 58 +++++++++ 6 files changed, 455 insertions(+), 49 deletions(-) diff --git a/bin/fm-pending-reply-lib.sh b/bin/fm-pending-reply-lib.sh index a57113dc0f..7d7b1c3a96 100755 --- a/bin/fm-pending-reply-lib.sh +++ b/bin/fm-pending-reply-lib.sh @@ -15,7 +15,10 @@ # and escalate once if the recovery turn also completes without a correlated # report. Never loop, never repeatedly inject, never silently expire unresolved # records, and never treat wrong-home or structured-home heuristics as -# acknowledgement. +# acknowledgement. Resolved records (and only resolved records) are retired from +# state/pending-replies/ once their escalation lifecycle is closed, so the +# watcher tick cannot accumulate answered files until a single poll starves the +# liveness beacon. # # Record location (parent FM_HOME): # state/pending-replies/ @@ -75,6 +78,8 @@ # FM_PENDING_REPLY_SEND_HOOK optional command template for recovery delivery # (tests); receives task_id and full message as args # FM_PENDING_REPLY_NOW optional fixed epoch for deterministic tests +# FM_PENDING_REPLY_BEAT_EVERY records between optional mid-tick beacon touches +# (default 25; 0 disables mid-tick touches) # shellcheck source=bin/fm-marker-lib.sh _FM_PENDING_REPLY_LIB_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd 2>/dev/null)" || _FM_PENDING_REPLY_LIB_DIR="." @@ -422,6 +427,48 @@ fm_pending_reply_discard_undelivered() { # rm -f "$rec" } +# Retire a resolved pending-reply record after its escalation lifecycle is closed +# (or never opened). Unresolved records always refuse and stay on disk. +# Returns 0 when the record is gone (already or newly), 1 when it must remain. +fm_pending_reply_retire_resolved() { # + local state=$1 corr=$2 rec phase escalated closed marker + rec=$(fm_pending_reply_path "$state" "$corr") + [ -f "$rec" ] || return 0 + phase=$(fm_pending_reply_get "$rec" phase) + [ "$phase" = resolved ] || return 1 + escalated=$(fm_pending_reply_get "$rec" escalated_epoch) + if [ -n "$escalated" ]; then + closed=$(fm_pending_reply_get "$rec" escalation_closed_epoch) + if [ -z "$closed" ]; then + fm_pending_reply_close_escalation "$state" "$corr" || return 1 + # Re-read after close: a transient write failure must keep the record. + [ -f "$rec" ] || return 0 + closed=$(fm_pending_reply_get "$rec" escalation_closed_epoch) + [ -n "$closed" ] || return 1 + fi + fi + marker=$(fm_pending_reply_delivery_confirmation_path "$state" "$corr") + rm -f "$marker" 2>/dev/null || true + rm -f "$rec" + return 0 +} + +# Optional mid-tick liveness touch so a healthy watcher cannot look dead while +# walking a large pending-replies population. beat_path empty is a no-op. +fm_pending_reply_maybe_beat() { # + local beat_path=$1 n=$2 every + [ -n "$beat_path" ] || return 0 + every=${FM_PENDING_REPLY_BEAT_EVERY:-25} + case "$every" in + ''|*[!0-9]*) every=25 ;; + esac + [ "$every" -gt 0 ] || return 0 + # Touch on the first record and every Nth thereafter. + if [ "$n" -eq 1 ] || [ $((n % every)) -eq 0 ]; then + touch "$beat_path" 2>/dev/null || true + fi +} + # 0 if a status line is a correlated acknowledgement for . # Accepts short status replies and status lines that point at a document. # Unrelated verbs without the token never match. Stale/wrong corr never match. @@ -553,6 +600,9 @@ _fm_pending_reply_try_resolve_locked() { # [status-file-o fm_pending_reply_set "$rec" resolved_via "$via" || return 1 # The record is resolved either way; a failed close stays retryable from the # watcher tick rather than turning a settled request back into a failure. + # Retirement of answered files is owned by fm_pending_reply_tick so direct + # resolvers can still inspect the durable resolved record, and so a failed + # escalation close remains on disk for the next poll. _fm_pending_reply_close_escalation_locked "$state" "$corr" || true return 0 } @@ -1120,9 +1170,11 @@ fm_pending_reply_tick_one() { # [secondmate- # Scan every pending record for this parent state. Safe to call every poll. # Never scrapes secondmate conversation; uses only parent status, backend busy # state, and optional secondmate-home wrong-home path checks. -fm_pending_reply_tick() { # - local state=$1 dir rec corr task_id phase delivered meta backend target label busy sm_home harness remote_host - local observation observation_task found i +# Optional second argument is a liveness-beacon path the watcher may touch at +# bounded intervals during a large walk so a healthy poll cannot starve grace. +fm_pending_reply_tick() { # [beat-path] + local state=$1 beat_path=${2-} dir rec corr task_id phase delivered meta backend target label busy sm_home harness remote_host + local observation observation_task found i seen=0 local -a observation_tasks=() observation_values=() dir=$(fm_pending_reply_dir "$state") [ -d "$dir" ] || return 0 @@ -1131,14 +1183,16 @@ fm_pending_reply_tick() { # case "$(basename "$rec")" in .*) continue ;; esac + seen=$((seen + 1)) + fm_pending_reply_maybe_beat "$beat_path" "$seen" corr=$(fm_pending_reply_get "$rec" corr_id) [ -n "$corr" ] || corr=$(basename "$rec") task_id=$(fm_pending_reply_get "$rec" task_id) phase=$(fm_pending_reply_get "$rec" phase) if [ "$phase" = resolved ]; then - # Cheap no-op unless an escalation for this record is still open; this is - # the retry that makes the close converge after a transient write failure. - fm_pending_reply_close_escalation "$state" "$corr" || true + # Close any open escalation, then remove the answered record so later + # polls do not walk a growing archive of settled expectations. + fm_pending_reply_retire_resolved "$state" "$corr" || true continue fi fm_pending_reply_reconcile_delivery "$state" "$corr" || true @@ -1148,6 +1202,11 @@ fm_pending_reply_tick() { # case "$phase" in delivery_unknown|escalated) fm_pending_reply_tick_one "$state" "$corr" unknown "" || true + # tick_one may resolve a late correlated report without a prior + # delivered_epoch; retire so answered files do not accumulate. + if [ -f "$rec" ] && [ "$(fm_pending_reply_get "$rec" phase)" = resolved ]; then + fm_pending_reply_retire_resolved "$state" "$corr" || true + fi ;; esac continue @@ -1163,6 +1222,7 @@ fm_pending_reply_tick() { # meta="$state/${task_id}.meta" if [ "$phase" = escalated ]; then if fm_pending_reply_try_resolve "$state" "$corr"; then + fm_pending_reply_retire_resolved "$state" "$corr" || true continue fi if [ -f "$meta" ]; then @@ -1176,6 +1236,11 @@ fm_pending_reply_tick() { # case "$phase" in recovery_failed|recovery_unknown) fm_pending_reply_tick_one "$state" "$corr" unknown "" || true + # tick_one may have resolved; retire so the next poll stays cheap. + phase=$(fm_pending_reply_get "$rec" phase 2>/dev/null || true) + if [ "$phase" = resolved ]; then + fm_pending_reply_retire_resolved "$state" "$corr" || true + fi continue ;; esac @@ -1224,7 +1289,15 @@ fm_pending_reply_tick() { # fi fi fm_pending_reply_tick_one "$state" "$corr" "$busy" "$sm_home" || true + # Retire if this open record just became answered mid-tick. + if [ -f "$rec" ] && [ "$(fm_pending_reply_get "$rec" phase)" = resolved ]; then + fm_pending_reply_retire_resolved "$state" "$corr" || true + fi done + # Final beat so a large walk that ended between intervals still looks alive. + if [ -n "$beat_path" ] && [ "$seen" -gt 0 ]; then + touch "$beat_path" 2>/dev/null || true + fi return 0 } diff --git a/bin/fm-watch-arm.sh b/bin/fm-watch-arm.sh index 0e1dd3bc1a..46ce084aef 100755 --- a/bin/fm-watch-arm.sh +++ b/bin/fm-watch-arm.sh @@ -21,28 +21,33 @@ # pre-execution seatbelt, not a substitute for the verification here. # # This script forks the watcher as a tracked child, then VERIFIES the outcome -# before it settles in. It confirms a watcher process is genuinely alive AND the -# liveness beacon (state/.last-watcher-beat) is fresh within the shared -# poll-derived grace (fm_guard_grace_seconds / FM_GUARD_GRACE override), and prints +# before it settles in. It confirms a watcher process is genuinely alive, prefers +# a fresh liveness beacon (state/.last-watcher-beat within the shared +# poll-derived grace from fm_guard_grace_seconds / FM_GUARD_GRACE), and prints # exactly one unambiguous status line: # watcher: started pid= (beacon fresh) - it launched one and confirmed it -# watcher: attached pid= (beacon s) - a live+fresh successor holds the lock; -# this arm attaches and follows it +# watcher: attached pid= (beacon s) - a live identity-matched successor +# holds the lock; this arm attaches +# and follows it (age may briefly +# exceed grace while a poll is mid- +# iteration; the holder is still live) # watcher: FAILED - no live watcher with a fresh beacon - could not confirm one # watcher: FAILED - cycle ended without an actionable reason # - a clean cycle ended with no wake and no # verified healthy successor -# 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 -# 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 -# failure. Neither is ever a clean empty completion. On FAILED it exits non-zero -# so the failure is loud. A live cycle already present means re-arm attaches - do -# not start a second watcher. +# It NEVER reports started/attached/healthy off a dead or identity-mismatched pid. +# A live identity-matched holder is attached even when its beacon is temporarily +# stale mid-iteration, so a long poll cannot be mistaken for a finished cycle +# and replaced with a second watcher. A dead-pid holder 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 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 failure. Neither is ever a clean empty completion. On +# FAILED it exits non-zero so the failure is loud. A live cycle already present +# means re-arm attaches - do not start a second watcher. # # Every observed watcher cycle appends one tab-separated lifecycle record to # state/.watch-cycle-exits.log. The arm layer owns that bounded ledger; it records @@ -236,8 +241,9 @@ clear_stale_recorded_watcher_lock() { # A watcher is "healthy" iff the lock names a live process that is genuinely THIS # home's watcher (the identity match guards against a recycled/reused pid) AND the # liveness beacon is fresh within GRACE. Sets HEALTHY_PID on success. This is the -# single honesty gate: a dead pid, a reused pid, or a stale beacon all fail it, so -# this script can never report a watcher that is not really there. +# single honesty gate for started confirmation and benign cycle ends: a dead pid, +# a reused pid, or a stale beacon all fail it, so this script can never report a +# started/healthy watcher that is not really there. HEALTHY_PID= HEALTHY_IDENTITY= healthy_watcher() { @@ -248,6 +254,23 @@ healthy_watcher() { HEALTHY_IDENTITY=$FM_WATCHER_HEALTHY_IDENTITY } +# Live identity-matched holder of this home's lock, ignoring beacon age. A long +# mid-poll iteration can starve the beacon without ending the cycle; attach paths +# must follow that holder rather than treating it as cycle-end or starting a +# second watcher. Sets HEALTHY_PID/HEALTHY_IDENTITY on success. +live_watcher_holder() { + local pid identity + HEALTHY_PID= + HEALTHY_IDENTITY= + pid=$(cat "$WATCH_LOCK/pid" 2>/dev/null || true) + fm_pid_alive "$pid" || return 1 + fm_watcher_lock_matches_pid "$STATE" "$WATCH" "$pid" "$FM_HOME" || return 1 + identity=$FM_WATCHER_MATCHED_IDENTITY + HEALTHY_PID=$pid + HEALTHY_IDENTITY=$identity + return 0 +} + report_attached() { local age age=$(fm_path_age "$BEAT") @@ -305,10 +328,12 @@ close_unobserved_cycle() { return 1 } -# Stay alive across identity-matched healthy holders. If one cycle ends, attach -# to a verified successor. With no successor, report the wake that cycle durably -# delivered, or fail loudly - never a clean empty completion that an adapter could -# mistake for a no-op. +# Stay alive across identity-matched holders. Prefer a fresh-beacon healthy +# holder; if the beacon is only temporarily stale while the same live process +# still holds the lock, keep waiting (a long poll iteration, not cycle-end). +# When the holder is gone, attach to a verified successor. With no successor, +# report the wake that cycle durably delivered, or fail loudly - never a clean +# empty completion that an adapter could mistake for a no-op. attach_and_wait() { local attached_pid=$1 while :; do @@ -322,6 +347,12 @@ attach_and_wait() { sleep "$ATTACH_POLL" continue fi + # Same live identity-matched holder with a starved beacon is still mid-cycle. + if fm_pid_alive "$attached_pid" \ + && fm_watcher_lock_matches_pid "$STATE" "$WATCH" "$attached_pid" "$FM_HOME"; then + sleep "$ATTACH_POLL" + continue + fi if wait_for_healthy_successor; then cycle_log_append unknown unknown attached-cycle-ended "attached:$HEALTHY_PID" attached_pid=$HEALTHY_PID @@ -329,6 +360,15 @@ attach_and_wait() { report_attached continue fi + # A different live holder may also be mid-poll with a stale beacon; follow it + # rather than failing while the singleton is still legitimately held. + if live_watcher_holder; then + cycle_log_append unknown unknown lock-replaced "attached:$HEALTHY_PID" + attached_pid=$HEALTHY_PID + cycle_begin "$attached_pid" attached "$HEALTHY_IDENTITY" + report_attached + continue + fi if close_unobserved_cycle; then cycle_log_append unknown unknown attached-delivered-wake none return 0 @@ -431,16 +471,20 @@ if [ "$mode" = restart ]; then fi fi -# If a genuinely live+fresh watcher already holds the lock, do not start a second -# one - attach to that cycle and wait until it ends so the harness notify fires -# then, not as an immediate empty wake. (--restart skips this: it just stopped -# this home's watcher and wants a fresh one.) -if [ "$mode" = arm ] && healthy_watcher; then - cycle_mark_predecessor_successor "attached:$HEALTHY_PID" - cycle_begin "$HEALTHY_PID" attached "$HEALTHY_IDENTITY" - report_attached - attach_and_wait "$HEALTHY_PID" - exit $? +# If a genuinely live identity-matched watcher already holds the lock, do not +# start a second one - attach to that cycle and wait until it ends so the +# harness notify fires then, not as an immediate empty wake. Prefer a fresh +# beacon, but still attach when the holder is live with a temporarily stale +# beacon (mid long poll). (--restart skips this: it just stopped this home's +# watcher and wants a fresh one.) +if [ "$mode" = arm ]; then + if healthy_watcher || live_watcher_holder; then + cycle_mark_predecessor_successor "attached:$HEALTHY_PID" + cycle_begin "$HEALTHY_PID" attached "$HEALTHY_IDENTITY" + report_attached + attach_and_wait "$HEALTHY_PID" + exit $? + fi fi # Start a watcher as a tracked child and confirm it before settling in. The child @@ -514,6 +558,20 @@ owned_child_finished() { attach_and_wait "$HEALTHY_PID" return $? fi + # Child stood down because a live identity-matched holder still owns the + # lock (possibly mid long poll with a starved beacon). Follow that holder. + if live_watcher_holder; then + cycle_log_append "$rc" "$signal" unexpected-clean-exit "attached:$HEALTHY_PID" + print_watch_output "$child_out" + rm -f "$child_out" 2>/dev/null || true + child= + child_out= + cycle_mark_predecessor_successor "attached:$HEALTHY_PID" + report_attached + cycle_begin "$HEALTHY_PID" attached "$HEALTHY_IDENTITY" + attach_and_wait "$HEALTHY_PID" + return $? + fi print_watch_output "$child_out" rm -f "$child_out" 2>/dev/null || true child= @@ -582,11 +640,63 @@ while :; do owned_child_finished "$rc" exit $? fi + # Another live identity-matched holder (not our child) may own the lock mid + # long poll with a starved beacon. Follow that holder instead of waiting for + # a fresh-beacon confirmation that never arrives from our stood-down child. + # Do not treat our own starting child as that case: it still needs a fresh + # beacon before we report started. + if live_watcher_holder && [ "$HEALTHY_PID" != "$child" ]; then + wait "$child" 2>/dev/null || true + child= + print_watch_output "$child_out" + rm -f "$child_out" 2>/dev/null || true + child_out= + cycle_mark_predecessor_successor "attached:$HEALTHY_PID" + report_attached + cycle_begin "$HEALTHY_PID" attached "$HEALTHY_IDENTITY" + attach_and_wait "$HEALTHY_PID" + exit $? + fi [ "$(date +%s)" -ge "$deadline" ] && break sleep 0.2 done trap - HUP TERM INT +# Confirmation budget exhausted. Prefer attaching to a different live holder +# with a starved beacon over a false FAILED, and if our own child still holds +# the lock mid-poll treat it as a started cycle rather than killing it. +if live_watcher_holder; then + if [ "$HEALTHY_PID" = "$child" ]; then + 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 + fi + cycle_mark_predecessor_successor "started:$child" + if [ -n "$handling_generation" ]; then + echo "watcher: started pid=$child (beacon live) recovery-generation=$handling_generation" + else + echo "watcher: started pid=$child (beacon live)" + fi + wait "$child" + rc=$? + owned_child_finished "$rc" + exit $? + fi + print_watch_output "$child_out" + cleanup_child + wait "$child" 2>/dev/null || true + child= + child_out= + cycle_mark_predecessor_successor "attached:$HEALTHY_PID" + report_attached + cycle_begin "$HEALTHY_PID" attached "$HEALTHY_IDENTITY" + attach_and_wait "$HEALTHY_PID" + exit $? +fi print_watch_output "$child_out" cleanup_child wait "$child" 2>/dev/null diff --git a/bin/fm-watch.sh b/bin/fm-watch.sh index 4f1150df56..557060e8a2 100755 --- a/bin/fm-watch.sh +++ b/bin/fm-watch.sh @@ -852,9 +852,11 @@ while :; do # Parent-owned secondmate pending-reply reconciliation: resolve correlated # parent reports, observe backend busy/idle turn completion, send one recovery - # repost after grace, and escalate once if the recovery turn is also missed. + # repost after grace, escalate once if the recovery turn is also missed, and + # retire answered records so the poll cannot accumulate settled files. + # Pass the liveness beacon so a large walk cannot starve grace mid-iteration. # No conversation scraping; unresolved records are never silently expired. - fm_pending_reply_tick "$STATE" || true + fm_pending_reply_tick "$STATE" "$STATE/.last-watcher-beat" || true # Process-to-event liveness repair. This never discovers a result by polling: # each registered source has its own child blocking on that source, and this diff --git a/tests/fm-claude-stop-autoarm.test.sh b/tests/fm-claude-stop-autoarm.test.sh index 2557f12b32..0a21d451c4 100755 --- a/tests/fm-claude-stop-autoarm.test.sh +++ b/tests/fm-claude-stop-autoarm.test.sh @@ -487,6 +487,41 @@ test_benign_cycle_end_with_live_watcher_is_silent() { pass "auto-arm: benign cycle end with a live watcher and fresh beacon stays silent across the next cycle" } +# Defect 2 evidence shape: arm returns FAILED (could not confirm fresh beacon) +# while a live identity-matched holder still owns the lock. With a fresh beacon +# the existing benign path treats this as clean. This case documents that a +# starved beacon is still a hard failure for the auto-arm HEALTHY gate - the +# arm layer (not auto-arm) is responsible for attaching to mid-poll holders so +# the arm never returns FAILED while a live cycle continues. When the arm +# fixture returns FAILED and the beacon is stale, auto-arm correctly exhausts +# and alarms rather than falsely claiming clean supervision. +test_failed_arm_with_stale_beacon_live_holder_fails_closed() { + local dir out status pid identity + dir=$(make_primary_dir "$TMP_ROOT/failed-stale-live") + : > "$dir/state/task.meta" + write_arm_fixture "$dir" failed + sleep 60 & + pid=$! + identity=$(watcher_identity "$dir" "$pid") || fail "could not identify live holder for stale-beacon failure" + record_watcher_lock "$dir" "$pid" "$identity" + if [ "$(uname -s 2>/dev/null)" = Darwin ]; then + touch -t 202001010000 "$dir/state/.last-watcher-beat" 2>/dev/null \ + || fail "could not age the beacon on Darwin" + else + touch -d '2020-01-01 00:00:00' "$dir/state/.last-watcher-beat" 2>/dev/null \ + || fail "could not age the beacon" + fi + out=$(FM_GUARD_GRACE=1 run_autoarm "$dir" 2>/dev/null); status=$? + kill "$pid" 2>/dev/null || true + wait "$pid" 2>/dev/null || true + expect_code 2 "$status" "a FAILED arm with only a stale-beacon live holder must fail closed" + assert_contains "$out" "automatic supervision mechanism is broken" \ + "stale-beacon live holder must not be mistaken for a healthy cycle" + [ "$(epoch_outcome "$dir")" = failed ] \ + || fail "epoch must record outcome=failed when only a stale-beacon holder exists, got: $(epoch_outcome "$dir")" + pass "auto-arm: FAILED arm with live-but-stale holder remains fail-closed (arm attach owns mid-poll)" +} + test_positive_recovery_budget_contention_preserves_episode() { local dir out status pid identity holder dir=$(make_primary_dir "$TMP_ROOT/recovery-budget-contention") @@ -613,6 +648,7 @@ test_failed_cycles_notify_once_and_keep_retrying test_unverified_clean_close_exhausts_retries test_post_alarm_actionable_close_is_suppressed test_benign_cycle_end_with_live_watcher_is_silent +test_failed_arm_with_stale_beacon_live_holder_fails_closed test_positive_recovery_budget_contention_preserves_episode test_arms_for_x_mode_poll_need_without_inflight test_single_flight_admits_exactly_one_owner diff --git a/tests/fm-pending-reply.test.sh b/tests/fm-pending-reply.test.sh index 5df64f8bbc..d816450edc 100755 --- a/tests/fm-pending-reply.test.sh +++ b/tests/fm-pending-reply.test.sh @@ -582,15 +582,17 @@ test_delivery_confirmation_fallback_reconciles() { || fail "delivery-unknown escalation should publish once, got $escalations" printf 'done [corr=%s]: late report proves delivery\n' "$prepared_corr" >> "$state/hibit.status" fm_pending_reply_tick "$state" || fail "watcher should accept a late delivery report" - [ "$(phase_of "$state" "$prepared_corr")" = resolved ] \ - || fail "late report should resolve escalated delivery-unknown" - [ "$(fm_pending_reply_get "$prepared_rec" delivered_epoch)" = 5760 ] \ - || fail "late report should provide delivery evidence" + # Full tick retires answered records after closing any open escalation. + [ ! -f "$prepared_rec" ] \ + || fail "late report should resolve and retire the escalated delivery-unknown record" + grep -Fq "resolved [key=pending-reply-$prepared_corr]: pending-reply-resolved:" \ + "$state/hibit.status" \ + || fail "late report should close the keyed delivery-unknown escalation" escalations=$(grep -Fc "blocked [key=pending-reply-$prepared_corr]:" "$state/hibit.status") [ "$escalations" = 1 ] || fail "late report must not re-escalate delivery-unknown" - fm_pending_reply_tick "$state" || fail "resolved late report should remain idempotent" - [ "$(phase_of "$state" "$prepared_corr")" = resolved ] \ - || fail "late report resolution should remain durable" + fm_pending_reply_tick "$state" || fail "retired late report should remain idempotent" + [ ! -f "$prepared_rec" ] \ + || fail "late report retirement must remain durable across a second tick" export FM_PENDING_REPLY_NOW=5800 reported_corr=$(fm_pending_reply_create "$home" "$state" hibit "reported delivery") reported_rec=$(fm_pending_reply_path "$state" "$reported_corr") @@ -934,6 +936,8 @@ test_tick_skips_terminal_and_reuses_target_observation() { rec=$(fm_pending_reply_path "$state" "$open2") [ "$(fm_pending_reply_get "$rec" turn_seen_busy)" = 1 ] \ || fail "cached observation should update the second open record" + [ ! -f "$(fm_pending_reply_path "$state" "$resolved")" ] \ + || fail "resolved records must be retired from the pending-replies directory" rec=$(fm_pending_reply_path "$state" "$escalated") snapshot=$(fm_pending_reply_get "$rec" wrong_home_scan_signature) [ -n "$snapshot" ] || fail "wrong-home scan should persist its file-set signature" @@ -943,8 +947,10 @@ test_tick_skips_terminal_and_reuses_target_observation() { || fail "unchanged records should scan two open and one escalated status only once, got $scans" [ "$(fm_pending_reply_get "$rec" wrong_home_scan_signature)" = "$snapshot" ] \ || fail "unchanged wrong-home logs should retain their scan signature" + [ -f "$(fm_pending_reply_path "$state" "$escalated")" ] \ + || fail "unresolved escalated records must never be silently retired" ) || fail "terminal-skip and observation-cache regression failed" - pass "tick skips terminal records and reuses target observations" + pass "tick retires resolved records and reuses target observations" } test_correlations_reuse_only_for_matching_open_task() { @@ -1039,6 +1045,125 @@ test_failed_send_discards_undelivered_expectation() { pass "failed transport discards undelivered expectation only" } +# Regression for the 2026-08-14 watcher-down episode: hundreds of already-answered +# pending-reply records remained on disk, so each poll walked them all, the +# liveness beacon went stale mid-iteration, and guards raised false WATCHER DOWN +# while the watcher was still alive. Resolved records must retire; unresolved +# must stay; a large walk must keep a passed-in beacon fresh. +test_large_resolved_population_retires_and_keeps_beacon_fresh() { + local home state dir beat open_corr open_rec i corr rec mtime now after open_left resolved_left + home=$(setup_parent large-retire) + state="$home/state" + dir=$(fm_pending_reply_dir "$state") + beat="$state/.last-watcher-beat" + export FM_PENDING_REPLY_NOW=11000 + export FM_PENDING_REPLY_BEAT_EVERY=10 + + # Seed a backlog shaped like the live home: many resolved, one still open. + mkdir -p "$dir" || fail "could not create pending-replies fixture dir" + chmod 700 "$dir" 2>/dev/null || true + for i in $(seq 1 120); do + corr=$(printf 'a%015x' "$i") + rec="$dir/$corr" + cat > "$rec" < "$beat" + # Age the beacon so a non-touching tick would leave it older than a tight grace. + if [ "$(uname -s 2>/dev/null)" = Darwin ]; then + touch -t 202001010000 "$beat" 2>/dev/null \ + || fail "fixture could not age the beacon on Darwin" + else + touch -d '2020-01-01 00:00:00' "$beat" 2>/dev/null \ + || fail "fixture could not age the beacon" + fi + + fm_pending_reply_tick "$state" "$beat" || fail "large-population tick failed" + + resolved_left=0 + for rec in "$dir"/*; do + [ -f "$rec" ] || continue + case "$(basename "$rec")" in .*) continue ;; esac + if [ "$(fm_pending_reply_get "$rec" phase)" = resolved ]; then + resolved_left=$((resolved_left + 1)) + fi + done + [ "$resolved_left" -eq 0 ] \ + || fail "resolved backlog must be retired, left $resolved_left" + [ -f "$open_rec" ] || fail "open unresolved record must survive the large tick" + [ "$(fm_pending_reply_get "$open_rec" phase)" = awaiting_report ] \ + || fail "open record phase must remain awaiting_report" + if [ "$(uname -s 2>/dev/null)" = Darwin ]; then + mtime=$(stat -f %m "$beat") + else + mtime=$(stat -c %Y "$beat") + fi + now=$(date +%s) + after=$((now - mtime)) + [ "$after" -lt 30 ] \ + || fail "large pending-reply tick must refresh the beacon (age=${after}s)" + open_left=0 + for rec in "$dir"/*; do + [ -f "$rec" ] || continue + case "$(basename "$rec")" in .*) continue ;; esac + open_left=$((open_left + 1)) + done + [ "$open_left" -eq 1 ] || fail "exactly one open record should remain, got $open_left" + pass "large resolved population retires and keeps the beacon fresh" +} + +test_tick_retires_record_that_resolves_mid_poll() { + local home state corr + home=$(setup_parent mid-poll-retire) + state="$home/state" + export FM_PENDING_REPLY_NOW=12000 + corr=$(fm_pending_reply_create "$home" "$state" "hibit" "lands during tick") + fm_pending_reply_mark_delivered "$state" "$corr" + printf 'done [corr=%s]: answered mid-poll\n' "$corr" > "$state/hibit.status" + fm_pending_reply_tick "$state" || fail "tick should resolve and retire" + [ ! -f "$(fm_pending_reply_path "$state" "$corr")" ] \ + || fail "record resolved during the tick must leave pending-replies" + pass "tick retires a record that resolves mid-poll" +} + # --- run -------------------------------------------------------------------- test_normal_correlated_reply_resolves_once @@ -1069,5 +1194,7 @@ test_tick_skips_terminal_and_reuses_target_observation test_correlations_reuse_only_for_matching_open_task test_tick_end_to_end_missed_then_escalate test_failed_send_discards_undelivered_expectation +test_large_resolved_population_retires_and_keeps_beacon_fresh +test_tick_retires_record_that_resolves_mid_poll printf 'ok - all pending-reply tests passed\n' diff --git a/tests/fm-watch-arm.test.sh b/tests/fm-watch-arm.test.sh index 5ecf095af9..89e0f38dc8 100755 --- a/tests/fm-watch-arm.test.sh +++ b/tests/fm-watch-arm.test.sh @@ -186,6 +186,63 @@ test_attached_arm_reports_the_delivered_wake() { pass "watch-arm: an attached arm reports the wake its cycle delivered instead of a false failure" } +# Regression for the 2026-08-13/14 auto-arm episode: a live identity-matched +# watcher mid long poll can present a beacon older than grace. Re-arm must +# attach to that holder rather than failing or starting a second cycle. +test_arm_attaches_to_live_holder_with_stale_beacon() { + local dir state fakebin out armout i + dir=$(make_case attach-stale-beacon) + state="$dir/state" + fakebin="$dir/fakebin" + out="$dir/watch.out" + armout="$dir/arm.out" + export FM_FAKE_CREW_STATE='state: working · source: run-step · validating (running)' + # Tiny grace so a deliberately aged beacon fails the healthy check. + start_seed_watcher "$state" "$fakebin" "$out" + if [ "$(uname -s 2>/dev/null)" = Darwin ]; then + touch -t 202001010000 "$state/.last-watcher-beat" 2>/dev/null \ + || fail "could not age the seed watcher beacon on Darwin" + else + touch -d '2020-01-01 00:00:00' "$state/.last-watcher-beat" 2>/dev/null \ + || fail "could not age the seed watcher beacon" + fi + # Confirm the seed still holds the lock while the aged beacon fails grace. + [ "$(cat "$state/.watch.lock/pid" 2>/dev/null || true)" = "$SEED_PID" ] \ + || fail "seed watcher lost the lock before the stale-beacon arm" + is_live_non_zombie "$SEED_PID" || fail "seed watcher died before the stale-beacon arm" + + PATH="$fakebin:$PATH" FM_STATE_OVERRIDE="$state" FM_CREW_STATE_BIN="$fakebin/fm-crew-state.sh" \ + FM_ARM_ATTACH_POLL=0.1 FM_ARM_CONFIRM_TIMEOUT=2 FM_GUARD_GRACE=1 \ + FM_POLL=5 FM_SIGNAL_GRACE=1 FM_CHECK_INTERVAL=999999 FM_HEARTBEAT=999999 \ + "$WATCH_ARM" > "$armout" & + ARM_PID=$! + i=0 + while [ "$i" -lt 80 ]; do + grep -qF "watcher: attached pid=$SEED_PID" "$armout" 2>/dev/null && break + sleep 0.1 + i=$((i + 1)) + done + grep -qF "watcher: attached pid=$SEED_PID" "$armout" \ + || fail "arm did not attach to the live stale-beacon watcher: $(cat "$armout")" + ! grep -qF 'watcher: FAILED' "$armout" \ + || fail "stale-beacon live holder must not be reported as FAILED: $(cat "$armout")" + ! grep -qF 'watcher: started' "$armout" \ + || fail "stale-beacon live holder must not start a second watcher: $(cat "$armout")" + # Still the original singleton; a competing start would replace the lock pid. + [ "$(cat "$state/.watch.lock/pid" 2>/dev/null || true)" = "$SEED_PID" ] \ + || fail "arm replaced the live stale-beacon holder instead of attaching" + + printf 'resolved [key=fixture]: capacity freed\n' > "$state/demo.status" + wait_for_exit "$SEED_PID" 120 + unset FM_FAKE_CREW_STATE + wait_for_exit "$ARM_PID" 120 + ! grep -qF 'watcher: FAILED' "$armout" \ + || fail "attached stale-beacon arm failed after the cycle delivered: $(cat "$armout")" + grep -qE '^(refill:|signal:)' "$armout" \ + || fail "attached stale-beacon arm did not surface the delivered wake: $(cat "$armout")" + pass "watch-arm: attaches to a live identity-matched holder even when the beacon is stale" +} + test_attached_arm_reports_the_delivered_wake_after_drain() { local dir state fakebin out armout status dir=$(make_case attached-drained-wake) @@ -798,6 +855,7 @@ test_downtime_marker_does_not_follow_symlink() { } test_attached_arm_reports_the_delivered_wake +test_arm_attaches_to_live_holder_with_stale_beacon test_attached_arm_reports_the_delivered_wake_after_drain test_attached_arm_still_fails_on_a_wake_it_did_not_deliver test_rearm_resurfaces_durable_queue_and_remote_open_decision From 7c734bdaaa0b5e81ff1943c49f075a9ed865f532 Mon Sep 17 00:00:00 2001 From: Joseph Kim Date: Sat, 15 Aug 2026 09:12:22 -0700 Subject: [PATCH 02/19] no-mistakes(review): Keep watcher beacons fresh and require confirmed starts --- bin/fm-pending-reply-lib.sh | 49 +++++++++++++++++++--------------- bin/fm-watch-arm.sh | 26 +++--------------- bin/fm-watch.sh | 7 ++++- tests/fm-pending-reply.test.sh | 37 ++++++++++++++++++++++++- tests/fm-watch-arm.test.sh | 32 ++++++++++++++++++++++ 5 files changed, 104 insertions(+), 47 deletions(-) diff --git a/bin/fm-pending-reply-lib.sh b/bin/fm-pending-reply-lib.sh index 7d7b1c3a96..106947f5aa 100755 --- a/bin/fm-pending-reply-lib.sh +++ b/bin/fm-pending-reply-lib.sh @@ -78,8 +78,8 @@ # FM_PENDING_REPLY_SEND_HOOK optional command template for recovery delivery # (tests); receives task_id and full message as args # FM_PENDING_REPLY_NOW optional fixed epoch for deterministic tests -# FM_PENDING_REPLY_BEAT_EVERY records between optional mid-tick beacon touches -# (default 25; 0 disables mid-tick touches) +# FM_PENDING_REPLY_BEAT_INTERVAL seconds between mid-tick beacon touches +# (default 30) # shellcheck source=bin/fm-marker-lib.sh _FM_PENDING_REPLY_LIB_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd 2>/dev/null)" || _FM_PENDING_REPLY_LIB_DIR="." @@ -453,20 +453,17 @@ fm_pending_reply_retire_resolved() { # return 0 } -# Optional mid-tick liveness touch so a healthy watcher cannot look dead while -# walking a large pending-replies population. beat_path empty is a no-op. -fm_pending_reply_maybe_beat() { # - local beat_path=$1 n=$2 every - [ -n "$beat_path" ] || return 0 - every=${FM_PENDING_REPLY_BEAT_EVERY:-25} - case "$every" in - ''|*[!0-9]*) every=25 ;; - esac - [ "$every" -gt 0 ] || return 0 - # Touch on the first record and every Nth thereafter. - if [ "$n" -eq 1 ] || [ $((n % every)) -eq 0 ]; then +fm_pending_reply_beat_loop() { # + local beat_path=$1 interval=$2 owner_pid=$3 sleeper= + trap '[ -z "$sleeper" ] || kill "$sleeper" 2>/dev/null; exit 0' TERM INT + while kill -0 "$owner_pid" 2>/dev/null; do + sleep "$interval" & + sleeper=$! + wait "$sleeper" || return 0 + sleeper= + kill -0 "$owner_pid" 2>/dev/null || return 0 touch "$beat_path" 2>/dev/null || true - fi + done } # 0 if a status line is a correlated acknowledgement for . @@ -1172,19 +1169,26 @@ fm_pending_reply_tick_one() { # [secondmate- # state, and optional secondmate-home wrong-home path checks. # Optional second argument is a liveness-beacon path the watcher may touch at # bounded intervals during a large walk so a healthy poll cannot starve grace. -fm_pending_reply_tick() { # [beat-path] - local state=$1 beat_path=${2-} dir rec corr task_id phase delivered meta backend target label busy sm_home harness remote_host - local observation observation_task found i seen=0 +fm_pending_reply_tick() { # [beat-path] [beat-interval] + local state=$1 beat_path=${2-} beat_interval=${3:-${FM_PENDING_REPLY_BEAT_INTERVAL:-30}} + local dir rec corr task_id phase delivered meta backend target label busy sm_home harness remote_host + local observation observation_task found i beat_pid= local -a observation_tasks=() observation_values=() dir=$(fm_pending_reply_dir "$state") [ -d "$dir" ] || return 0 + if [ -n "$beat_path" ]; then + if ! awk -v interval="$beat_interval" 'BEGIN { exit !((interval + 0) > 0) }'; then + beat_interval=30 + fi + touch "$beat_path" 2>/dev/null || true + fm_pending_reply_beat_loop "$beat_path" "$beat_interval" "${BASHPID:-$$}" & + beat_pid=$! + fi for rec in "$dir"/*; do [ -f "$rec" ] || continue case "$(basename "$rec")" in .*) continue ;; esac - seen=$((seen + 1)) - fm_pending_reply_maybe_beat "$beat_path" "$seen" corr=$(fm_pending_reply_get "$rec" corr_id) [ -n "$corr" ] || corr=$(basename "$rec") task_id=$(fm_pending_reply_get "$rec" task_id) @@ -1294,8 +1298,9 @@ fm_pending_reply_tick() { # [beat-path] fm_pending_reply_retire_resolved "$state" "$corr" || true fi done - # Final beat so a large walk that ended between intervals still looks alive. - if [ -n "$beat_path" ] && [ "$seen" -gt 0 ]; then + if [ -n "$beat_pid" ]; then + kill "$beat_pid" 2>/dev/null || true + wait "$beat_pid" 2>/dev/null || true touch "$beat_path" 2>/dev/null || true fi return 0 diff --git a/bin/fm-watch-arm.sh b/bin/fm-watch-arm.sh index 46ce084aef..a9c600f6c7 100755 --- a/bin/fm-watch-arm.sh +++ b/bin/fm-watch-arm.sh @@ -663,29 +663,9 @@ done trap - HUP TERM INT # Confirmation budget exhausted. Prefer attaching to a different live holder -# with a starved beacon over a false FAILED, and if our own child still holds -# the lock mid-poll treat it as a started cycle rather than killing it. -if live_watcher_holder; then - if [ "$HEALTHY_PID" = "$child" ]; then - 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 - fi - cycle_mark_predecessor_successor "started:$child" - if [ -n "$handling_generation" ]; then - echo "watcher: started pid=$child (beacon live) recovery-generation=$handling_generation" - else - echo "watcher: started pid=$child (beacon live)" - fi - wait "$child" - rc=$? - owned_child_finished "$rc" - exit $? - fi +# with a starved beacon over a false FAILED. Our own child still requires the +# fresh beacon checked above before it can be reported as started. +if live_watcher_holder && [ "$HEALTHY_PID" != "$child" ]; then print_watch_output "$child_out" cleanup_child wait "$child" 2>/dev/null || true diff --git a/bin/fm-watch.sh b/bin/fm-watch.sh index 557060e8a2..56d34cd71f 100755 --- a/bin/fm-watch.sh +++ b/bin/fm-watch.sh @@ -102,6 +102,11 @@ WATCHER_DOWNTIME_MARKER="$STATE/.watcher-down" # Defaults to the shared poll-derived grace (fm_guard_grace_seconds) so re-arm # stale detection stays calibrated with the guard/beacon contract. WATCHER_STALE_GRACE=${FM_WATCHER_STALE_GRACE:-$(fm_guard_grace_seconds)} +PENDING_REPLY_BEAT_INTERVAL=$(awk -v grace="$WATCHER_STALE_GRACE" 'BEGIN { + interval = grace / 3 + if (interval < 0.1) interval = 0.1 + print interval +}') # The singleton-lock acquisition, EXIT trap, and the blocking supervision loop # all live below the source guard at the very bottom of this file (see "Main # entry"). Sourcing this file for unit tests therefore loads the functions - @@ -856,7 +861,7 @@ while :; do # retire answered records so the poll cannot accumulate settled files. # Pass the liveness beacon so a large walk cannot starve grace mid-iteration. # No conversation scraping; unresolved records are never silently expired. - fm_pending_reply_tick "$STATE" "$STATE/.last-watcher-beat" || true + fm_pending_reply_tick "$STATE" "$STATE/.last-watcher-beat" "$PENDING_REPLY_BEAT_INTERVAL" || true # Process-to-event liveness repair. This never discovers a result by polling: # each registered source has its own child blocking on that source, and this diff --git a/tests/fm-pending-reply.test.sh b/tests/fm-pending-reply.test.sh index d816450edc..136fa6cb34 100755 --- a/tests/fm-pending-reply.test.sh +++ b/tests/fm-pending-reply.test.sh @@ -1057,7 +1057,6 @@ test_large_resolved_population_retires_and_keeps_beacon_fresh() { dir=$(fm_pending_reply_dir "$state") beat="$state/.last-watcher-beat" export FM_PENDING_REPLY_NOW=11000 - export FM_PENDING_REPLY_BEAT_EVERY=10 # Seed a backlog shaped like the live home: many resolved, one still open. mkdir -p "$dir" || fail "could not create pending-replies fixture dir" @@ -1150,6 +1149,41 @@ EOF pass "large resolved population retires and keeps the beacon fresh" } +test_single_slow_observation_keeps_beacon_fresh_mid_poll() { + ( + local home state beat checkpoint probe corr tick_pid i + home=$(setup_parent slow-observation-beat) + state="$home/state" + beat="$state/.last-watcher-beat" + checkpoint="$home/beat-checkpoint" + probe="$home/observation-started" + export FM_PENDING_REPLY_NOW=11500 + corr=$(fm_pending_reply_create "$home" "$state" hibit "slow backend observation") + fm_pending_reply_mark_delivered "$state" "$corr" + fm_write_secondmate_meta "$state/hibit.meta" "$home/hibit" "sess:fm-hibit" + fm_backend_busy_state() { + : > "$probe" + sleep 3 + printf 'busy' + } + fm_pending_reply_tick "$state" "$beat" 0.2 & + tick_pid=$! + i=0 + while [ "$i" -lt 40 ] && [ ! -f "$probe" ]; do + sleep 0.05 + i=$((i + 1)) + done + [ -f "$probe" ] || fail "slow observation did not start" + touch "$checkpoint" + sleep 1 + kill -0 "$tick_pid" 2>/dev/null || fail "tick ended before the slow observation completed" + [ "$beat" -nt "$checkpoint" ] \ + || fail "beacon was not refreshed while one record observation was still blocked" + wait "$tick_pid" || fail "slow-observation tick failed" + ) || fail "single slow observation beacon regression failed" + pass "single slow observation refreshes the beacon mid-poll" +} + test_tick_retires_record_that_resolves_mid_poll() { local home state corr home=$(setup_parent mid-poll-retire) @@ -1195,6 +1229,7 @@ test_correlations_reuse_only_for_matching_open_task test_tick_end_to_end_missed_then_escalate test_failed_send_discards_undelivered_expectation test_large_resolved_population_retires_and_keeps_beacon_fresh +test_single_slow_observation_keeps_beacon_fresh_mid_poll test_tick_retires_record_that_resolves_mid_poll printf 'ok - all pending-reply tests passed\n' diff --git a/tests/fm-watch-arm.test.sh b/tests/fm-watch-arm.test.sh index 89e0f38dc8..4ae5156324 100755 --- a/tests/fm-watch-arm.test.sh +++ b/tests/fm-watch-arm.test.sh @@ -243,6 +243,37 @@ test_arm_attaches_to_live_holder_with_stale_beacon() { pass "watch-arm: attaches to a live identity-matched holder even when the beacon is stale" } +test_new_child_without_fresh_beacon_fails_confirmation() { + local dir state fakebin armout arm_pid status + dir=$(make_case new-child-no-beacon) + state="$dir/state" + fakebin="$dir/fakebin" + armout="$dir/arm.out" + cat > "$fakebin/touch" <<'SH' +#!/usr/bin/env bash +for arg in "$@"; do + case "$arg" in + */.last-watcher-beat) exit 0 ;; + esac +done +exec /usr/bin/touch "$@" +SH + chmod +x "$fakebin/touch" + PATH="$fakebin:$PATH" FM_STATE_OVERRIDE="$state" FM_CREW_STATE_BIN="$fakebin/fm-crew-state.sh" \ + FM_ARM_CONFIRM_TIMEOUT=1 FM_GUARD_GRACE=1 FM_POLL=5 FM_SIGNAL_GRACE=1 \ + FM_CHECK_INTERVAL=999999 FM_HEARTBEAT=999999 "$WATCH_ARM" > "$armout" & + arm_pid=$! + wait_for_exit "$arm_pid" 100 + status=$? + [ "$status" -ne 0 ] && [ "$status" -ne 124 ] \ + || fail "arm did not fail after its child withheld the first beacon (status $status)" + grep -qF 'watcher: FAILED - no live watcher with a fresh beacon' "$armout" \ + || fail "arm did not report failed fresh-beacon confirmation: $(cat "$armout")" + ! grep -qF 'watcher: started' "$armout" \ + || fail "arm reported its unconfirmed child as started: $(cat "$armout")" + pass "watch-arm: a newly started child requires a fresh beacon" +} + test_attached_arm_reports_the_delivered_wake_after_drain() { local dir state fakebin out armout status dir=$(make_case attached-drained-wake) @@ -856,6 +887,7 @@ test_downtime_marker_does_not_follow_symlink() { test_attached_arm_reports_the_delivered_wake test_arm_attaches_to_live_holder_with_stale_beacon +test_new_child_without_fresh_beacon_fails_confirmation test_attached_arm_reports_the_delivered_wake_after_drain test_attached_arm_still_fails_on_a_wake_it_did_not_deliver test_rearm_resurfaces_durable_queue_and_remote_open_decision From 79806dbc26be9e497cfe4948d615dc4295446711 Mon Sep 17 00:00:00 2001 From: Joseph Kim Date: Sat, 15 Aug 2026 09:21:51 -0700 Subject: [PATCH 03/19] no-mistakes(review): Bind pending-reply beacons to watcher lock ownership --- bin/fm-pending-reply-lib.sh | 37 ++++++++++++++++++++++++--------- bin/fm-watch.sh | 3 ++- tests/fm-pending-reply.test.sh | 38 +++++++++++++++++++++++++++++----- 3 files changed, 62 insertions(+), 16 deletions(-) diff --git a/bin/fm-pending-reply-lib.sh b/bin/fm-pending-reply-lib.sh index 106947f5aa..11686f769e 100755 --- a/bin/fm-pending-reply-lib.sh +++ b/bin/fm-pending-reply-lib.sh @@ -453,16 +453,31 @@ fm_pending_reply_retire_resolved() { # return 0 } -fm_pending_reply_beat_loop() { # - local beat_path=$1 interval=$2 owner_pid=$3 sleeper= +fm_pending_reply_touch_if_owner() { # + ( + local beat_path=$1 owner_pid=$2 state=$3 watch_path=$4 owner_home=$5 + local FM_HOME=$owner_home FM_STATE_OVERRIDE=$state STATE=$state + # shellcheck source=bin/fm-wake-lib.sh + . "$_FM_PENDING_REPLY_LIB_DIR/fm-wake-lib.sh" + [ "$(cat "$state/.watch.lock/pid" 2>/dev/null || true)" = "$owner_pid" ] \ + && fm_watcher_lock_matches_pid "$state" "$watch_path" "$owner_pid" "$owner_home" \ + && touch "$beat_path" 2>/dev/null + ) +} + +fm_pending_reply_beat_loop() { # + local beat_path=$1 interval=$2 owner_pid=$3 state=$4 watch_path=$5 owner_home=$6 sleeper= + local FM_HOME=$owner_home FM_STATE_OVERRIDE=$state STATE=$state trap '[ -z "$sleeper" ] || kill "$sleeper" 2>/dev/null; exit 0' TERM INT - while kill -0 "$owner_pid" 2>/dev/null; do + # shellcheck source=bin/fm-wake-lib.sh + . "$_FM_PENDING_REPLY_LIB_DIR/fm-wake-lib.sh" + while [ "$(cat "$state/.watch.lock/pid" 2>/dev/null || true)" = "$owner_pid" ] \ + && fm_watcher_lock_matches_pid "$state" "$watch_path" "$owner_pid" "$owner_home"; do + touch "$beat_path" 2>/dev/null || true sleep "$interval" & sleeper=$! wait "$sleeper" || return 0 sleeper= - kill -0 "$owner_pid" 2>/dev/null || return 0 - touch "$beat_path" 2>/dev/null || true done } @@ -1169,19 +1184,22 @@ fm_pending_reply_tick_one() { # [secondmate- # state, and optional secondmate-home wrong-home path checks. # Optional second argument is a liveness-beacon path the watcher may touch at # bounded intervals during a large walk so a healthy poll cannot starve grace. -fm_pending_reply_tick() { # [beat-path] [beat-interval] +fm_pending_reply_tick() { # [beat-path] [beat-interval] [owner-pid] [watch-path] [owner-home] local state=$1 beat_path=${2-} beat_interval=${3:-${FM_PENDING_REPLY_BEAT_INTERVAL:-30}} + local owner_pid=${4-} watch_path=${5-} owner_home=${6-} local dir rec corr task_id phase delivered meta backend target label busy sm_home harness remote_host local observation observation_task found i beat_pid= local -a observation_tasks=() observation_values=() dir=$(fm_pending_reply_dir "$state") [ -d "$dir" ] || return 0 - if [ -n "$beat_path" ]; then + if [ -n "$beat_path" ] && [ -n "$owner_pid" ] && [ -n "$watch_path" ] && [ -n "$owner_home" ]; then if ! awk -v interval="$beat_interval" 'BEGIN { exit !((interval + 0) > 0) }'; then beat_interval=30 fi - touch "$beat_path" 2>/dev/null || true - fm_pending_reply_beat_loop "$beat_path" "$beat_interval" "${BASHPID:-$$}" & + fm_pending_reply_touch_if_owner "$beat_path" "$owner_pid" \ + "$state" "$watch_path" "$owner_home" || true + fm_pending_reply_beat_loop "$beat_path" "$beat_interval" "$owner_pid" \ + "$state" "$watch_path" "$owner_home" & beat_pid=$! fi for rec in "$dir"/*; do @@ -1301,7 +1319,6 @@ fm_pending_reply_tick() { # [beat-path] [beat-interval] if [ -n "$beat_pid" ]; then kill "$beat_pid" 2>/dev/null || true wait "$beat_pid" 2>/dev/null || true - touch "$beat_path" 2>/dev/null || true fi return 0 } diff --git a/bin/fm-watch.sh b/bin/fm-watch.sh index 56d34cd71f..f8477488e9 100755 --- a/bin/fm-watch.sh +++ b/bin/fm-watch.sh @@ -861,7 +861,8 @@ while :; do # retire answered records so the poll cannot accumulate settled files. # Pass the liveness beacon so a large walk cannot starve grace mid-iteration. # No conversation scraping; unresolved records are never silently expired. - fm_pending_reply_tick "$STATE" "$STATE/.last-watcher-beat" "$PENDING_REPLY_BEAT_INTERVAL" || true + fm_pending_reply_tick "$STATE" "$STATE/.last-watcher-beat" "$PENDING_REPLY_BEAT_INTERVAL" \ + "$WATCHER_PID" "$WATCH_PATH" "$FM_HOME" || true # Process-to-event liveness repair. This never discovers a result by polling: # each registered source has its own child blocking on that source, and this diff --git a/tests/fm-pending-reply.test.sh b/tests/fm-pending-reply.test.sh index 136fa6cb34..728574b168 100755 --- a/tests/fm-pending-reply.test.sh +++ b/tests/fm-pending-reply.test.sh @@ -81,6 +81,16 @@ setup_parent() { # -> home printf '%s\n' "$home" } +setup_beat_owner() { # + local state=$1 home=$2 watch_path=$3 pid=$4 lock="$1/.watch.lock" identity + identity=$(fm_test_pid_identity "$pid") || fail "could not identify beacon owner" + mkdir -p "$lock" + printf '%s\n' "$pid" > "$lock/pid" + printf '%s\n' "$home" > "$lock/fm-home" + printf '%s\n' "$watch_path" > "$lock/watcher-path" + printf '%s\n' "$identity" > "$lock/pid-identity" +} + run_send() { local fb=$1 home=$2 log=$3; shift 3 : > "$log" @@ -1051,12 +1061,15 @@ test_failed_send_discards_undelivered_expectation() { # while the watcher was still alive. Resolved records must retire; unresolved # must stay; a large walk must keep a passed-in beacon fresh. test_large_resolved_population_retires_and_keeps_beacon_fresh() { - local home state dir beat open_corr open_rec i corr rec mtime now after open_left resolved_left + local home state dir beat watch_path owner_pid open_corr open_rec i corr rec mtime now after open_left resolved_left home=$(setup_parent large-retire) state="$home/state" dir=$(fm_pending_reply_dir "$state") beat="$state/.last-watcher-beat" + watch_path="$ROOT/bin/fm-watch.sh" + owner_pid=${BASHPID:-$$} export FM_PENDING_REPLY_NOW=11000 + setup_beat_owner "$state" "$home" "$watch_path" "$owner_pid" # Seed a backlog shaped like the live home: many resolved, one still open. mkdir -p "$dir" || fail "could not create pending-replies fixture dir" @@ -1115,7 +1128,8 @@ EOF || fail "fixture could not age the beacon" fi - fm_pending_reply_tick "$state" "$beat" || fail "large-population tick failed" + fm_pending_reply_tick "$state" "$beat" 0.2 "$owner_pid" "$watch_path" "$home" \ + || fail "large-population tick failed" resolved_left=0 for rec in "$dir"/*; do @@ -1151,13 +1165,16 @@ EOF test_single_slow_observation_keeps_beacon_fresh_mid_poll() { ( - local home state beat checkpoint probe corr tick_pid i + local home state beat checkpoint probe watch_path owner_pid replacement corr tick_pid i home=$(setup_parent slow-observation-beat) state="$home/state" beat="$state/.last-watcher-beat" checkpoint="$home/beat-checkpoint" probe="$home/observation-started" + watch_path="$ROOT/bin/fm-watch.sh" + owner_pid=${BASHPID:-$$} export FM_PENDING_REPLY_NOW=11500 + setup_beat_owner "$state" "$home" "$watch_path" "$owner_pid" corr=$(fm_pending_reply_create "$home" "$state" hibit "slow backend observation") fm_pending_reply_mark_delivered "$state" "$corr" fm_write_secondmate_meta "$state/hibit.meta" "$home/hibit" "sess:fm-hibit" @@ -1166,7 +1183,7 @@ test_single_slow_observation_keeps_beacon_fresh_mid_poll() { sleep 3 printf 'busy' } - fm_pending_reply_tick "$state" "$beat" 0.2 & + fm_pending_reply_tick "$state" "$beat" 0.2 "$owner_pid" "$watch_path" "$home" & tick_pid=$! i=0 while [ "$i" -lt 40 ] && [ ! -f "$probe" ]; do @@ -1179,9 +1196,20 @@ test_single_slow_observation_keeps_beacon_fresh_mid_poll() { kill -0 "$tick_pid" 2>/dev/null || fail "tick ended before the slow observation completed" [ "$beat" -nt "$checkpoint" ] \ || fail "beacon was not refreshed while one record observation was still blocked" + sleep 5 & + replacement=$! + setup_beat_owner "$state" "$home" "$watch_path" "$replacement" + sleep 0.5 + touch "$checkpoint" + sleep 0.8 + kill -0 "$tick_pid" 2>/dev/null || fail "tick ended before lock-loss behavior was observed" + [ ! "$beat" -nt "$checkpoint" ] \ + || fail "former owner refreshed the beacon after losing its watcher lock" wait "$tick_pid" || fail "slow-observation tick failed" + kill "$replacement" 2>/dev/null || true + wait "$replacement" 2>/dev/null || true ) || fail "single slow observation beacon regression failed" - pass "single slow observation refreshes the beacon mid-poll" + pass "single slow observation beats only while its watcher owns the lock" } test_tick_retires_record_that_resolves_mid_poll() { From b2b3acd0f72520c8ffb14db8787a3dedb9a476e8 Mon Sep 17 00:00:00 2001 From: Joseph Kim Date: Sat, 15 Aug 2026 09:35:06 -0700 Subject: [PATCH 04/19] no-mistakes(document): Update watcher continuity documentation --- docs/architecture.md | 2 +- docs/configuration.md | 4 ++-- docs/watcher-continuity.md | 11 +++++++---- 3 files changed, 10 insertions(+), 7 deletions(-) diff --git a/docs/architecture.md b/docs/architecture.md index b2bf0faaf9..49f8dc8a29 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -63,7 +63,7 @@ Optional Relay integrates with the watcher only after explicit opt-in; [configur At session start, `bin/fm-session-start.sh` emits exactly one primary-harness supervision block rendered by `bin/fm-supervision-instructions.sh` from `docs/supervision-protocols/`. That block owns the live wait shape for the running primary harness: Claude's Stop `asyncRewake` hook owns tokenless re-arm cycles, Grok uses background-notify cycles, Codex uses bounded foreground checkpoints, Pi and pi-signed use the same two tracked primary extensions, and OpenCode uses its TUI plugin. -`bin/fm-watch-arm.sh` remains the verified arm wrapper for protocols that call it; it forks the watcher as a tracked child, verifies it is genuinely alive with a fresh liveness beacon, and prints an honest `started`, `attached`, or nonzero `FAILED` status. +`bin/fm-watch-arm.sh` remains the verified arm wrapper for protocols that call it; it coordinates tracked-child startup and singleton attachment, then prints an honest `started`, `attached`, or nonzero `FAILED` status. [`watcher-continuity.md`](watcher-continuity.md#arm-layer-cycle-contract) owns the arm layer's successor, terminal-delivery, re-arm recovery, and typed clean-close failure contract. The arm layer records one bounded lifecycle row per observed cycle in `state/.watch-cycle-exits.log`; `state/.watch-triage.log` remains exclusively the absorbed-wake debug log. Pi and OpenCode verify session-lock ownership and launch one singleton successor from their child-close handlers before delivering an actionable wake prompt, with bounded exponential retry for failed restoration. diff --git a/docs/configuration.md b/docs/configuration.md index cce75eda49..6449ddfee4 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -577,7 +577,7 @@ 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 an existing live identity-matched watcher cycle 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 @@ -586,7 +586,7 @@ FM_WATCH_REARM_RETRY_MAX_MS=4000 # Pi/OpenCode adapter cap for exponential con FM_WATCH_REARM_RETRY_LIMIT=5 # Pi/OpenCode adapter launch-failure retries before surfacing restoration failure FM_WATCH_CYCLE_LOG_MAX_BYTES=262144 # size cap for the arm-owned watcher lifecycle ledger FM_WATCH_CYCLE_LOG_KEEP_LINES=1000 # newest complete lifecycle rows considered when the ledger is capped -FM_WATCHER_STALE_GRACE= # optional; defaults to the same poll-derived grace as FM_GUARD_GRACE; seconds a live watcher lock may have a stale beacon before re-arm errors +FM_WATCHER_STALE_GRACE= # optional; defaults to the same poll-derived grace as FM_GUARD_GRACE; seconds a direct duplicate watcher tolerates a stale live-holder beacon before erroring, and the basis for its pending-reply mid-poll beacon interval (one third of this value) FM_SIGNAL_GRACE=30 # seconds to coalesce nearby status and turn-end signals into one wake FM_CAPTAIN_RE='done:|needs-decision:|blocked:|failed:|PR ready|checks green|ready in branch|merged' # captain-relevant status regex; nonterminal progress verbs remain excluded even when their prose matches FM_CLASSIFY_PAUSED_VERB=paused # leading status verb for a declared external wait; excluded from FM_CAPTAIN_RE and distinct from blocked diff --git a/docs/watcher-continuity.md b/docs/watcher-continuity.md index 030c7b13c1..bebb5b05f8 100644 --- a/docs/watcher-continuity.md +++ b/docs/watcher-continuity.md @@ -58,8 +58,9 @@ An acknowledged episode does not freeze the generation, because the next downtim `bin/fm-watch-arm.sh` never returns a clean empty success. An actionable child output returns that reason normally. -A zero/empty child return rechecks the home lock and beacon, attaches to a verified healthy successor when one exists, or resolves the close against the watcher's bounded terminal-delivery ledger. -An attached arm follows verified identity-matched successors and resolves the same way when that chain ends without one, because it holds no handle on the watcher's stdout and cannot read the reason line itself. +A zero/empty child return rechecks the home lock and beacon, attaches to a live identity-matched successor even when its beacon is temporarily stale mid-poll, or resolves the close against the watcher's bounded terminal-delivery ledger. +A newly launched child must still publish a fresh beacon before the arm reports `started`. +An attached arm follows live identity-matched holders through temporary beacon starvation and resolves the same way when that chain ends without one, because it holds no handle on the watcher's stdout and cannot read the reason line itself. 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. @@ -71,13 +72,15 @@ The file is size-capped through `FM_WATCH_CYCLE_LOG_MAX_BYTES` and `FM_WATCH_CYC Beacon stale grace defaults to `min(floor(60 * poll), FM_GUARD_GRACE_MAX)` via `fm_guard_grace_seconds` / `fm_poll_seconds` in `bin/fm-wake-lib.sh` (900s at the default 15s poll; 3600s ceiling by default). The watcher assigns the same normalized poll to its sleep loop so malformed, zero, negative, and fractional `FM_POLL` values never diverge between sleep and grace; explicit `FM_GUARD_GRACE` still overrides and is not capped. -Only the watcher process touches `state/.last-watcher-beat`; no helper process can make a wedged watcher appear healthy. +During pending-reply reconciliation, the watcher starts a bounded beacon helper so one slow observation or a large record walk cannot make a healthy cycle appear down. +That helper touches `state/.last-watcher-beat` only while the same PID and process identity still own this home's watcher lock, and stops after the tick or ownership loss. ## Regression coverage `tests/fm-pi-watch-extension.test.sh` checks Pi's first-cycle-or-explicit-repair tool metadata and ownership-based redundant-call no-ops, then simulates actionable and empty child closes against the actual Pi and OpenCode close handlers, blocks prompt delivery to prove the successor launches first, verifies single-flight behavior, changes the session lock before close to prove ownership is rechecked, and hangs each successor arm to prove bounded fallback delivery includes the typed restoration failure. 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-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, stale-beacon attachment to a live identity-matched holder, fresh-beacon confirmation for a newly launched child, 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-pending-reply.test.sh` covers retirement of a large resolved population while an unresolved record remains durable, plus beacon freshness during a single slow observation and lock-loss cutoff for the helper. `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, and exit-2 translation. From c24e57f0dd76cc4056bf349e108bd4a7426c1e8b Mon Sep 17 00:00:00 2001 From: Joseph Kim Date: Sat, 15 Aug 2026 09:40:39 -0700 Subject: [PATCH 05/19] no-mistakes(lint): Suppress intentional ShellCheck fixture warnings --- tests/fm-pending-reply.test.sh | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/tests/fm-pending-reply.test.sh b/tests/fm-pending-reply.test.sh index 728574b168..1091ce842d 100755 --- a/tests/fm-pending-reply.test.sh +++ b/tests/fm-pending-reply.test.sh @@ -1173,11 +1173,15 @@ test_single_slow_observation_keeps_beacon_fresh_mid_poll() { probe="$home/observation-started" watch_path="$ROOT/bin/fm-watch.sh" owner_pid=${BASHPID:-$$} + # This fixture clock is intentionally scoped to the isolated subshell. + # shellcheck disable=SC2030,SC2031 export FM_PENDING_REPLY_NOW=11500 setup_beat_owner "$state" "$home" "$watch_path" "$owner_pid" corr=$(fm_pending_reply_create "$home" "$state" hibit "slow backend observation") fm_pending_reply_mark_delivered "$state" "$corr" fm_write_secondmate_meta "$state/hibit.meta" "$home/hibit" "sess:fm-hibit" + # Invoked indirectly through the pending-reply tick. + # shellcheck disable=SC2329 fm_backend_busy_state() { : > "$probe" sleep 3 @@ -1216,6 +1220,8 @@ test_tick_retires_record_that_resolves_mid_poll() { local home state corr home=$(setup_parent mid-poll-retire) state="$home/state" + # Reset the fixture clock after the isolated subshell test. + # shellcheck disable=SC2031 export FM_PENDING_REPLY_NOW=12000 corr=$(fm_pending_reply_create "$home" "$state" "hibit" "lands during tick") fm_pending_reply_mark_delivered "$state" "$corr" From 0fabf3456cf94e061fbad70128de4a9f2f2ff2b7 Mon Sep 17 00:00:00 2001 From: Joseph Kim Date: Sun, 16 Aug 2026 00:02:49 -0700 Subject: [PATCH 06/19] fix(supervision): quarantine stuck-resolved pending-replies off the hot walk Resolved records that cannot close their escalation (blank parent_status, unwritable status append) no longer remain in state/pending-replies/ forever. Quarantine them under pending-replies-stuck/ with a durable receipt so open status-fold decisions stay intact and polls cannot re-accumulate answered files. Fail closed if the quarantine move cannot complete; regressions use chmod 444 (readable) for the unwritable-status fixture. --- bin/fm-pending-reply-lib.sh | 109 ++++++++++++++++++--- tests/fm-pending-reply.test.sh | 169 +++++++++++++++++++++++++++++++++ 2 files changed, 267 insertions(+), 11 deletions(-) diff --git a/bin/fm-pending-reply-lib.sh b/bin/fm-pending-reply-lib.sh index 11686f769e..e218a77f14 100755 --- a/bin/fm-pending-reply-lib.sh +++ b/bin/fm-pending-reply-lib.sh @@ -15,13 +15,19 @@ # and escalate once if the recovery turn also completes without a correlated # report. Never loop, never repeatedly inject, never silently expire unresolved # records, and never treat wrong-home or structured-home heuristics as -# acknowledgement. Resolved records (and only resolved records) are retired from -# state/pending-replies/ once their escalation lifecycle is closed, so the -# watcher tick cannot accumulate answered files until a single poll starves the -# liveness beacon. +# acknowledgement. Resolved records leave the hot state/pending-replies/ walk +# once their escalation lifecycle is closed, or - when close cannot complete +# (blank parent_status, unwritable status, permanent close failure) - via +# quarantine to state/pending-replies-stuck/ with a durable receipt. That keeps +# open status-fold decisions intact while the hot poll cannot re-accumulate +# answered files until a single iteration starves the liveness beacon. +# Unresolved records are never silently expired. # # Record location (parent FM_HOME): # state/pending-replies/ +# Stuck-resolved quarantine (not walked by the tick): +# state/pending-replies-stuck/ +# state/pending-replies-stuck.log # Each record is a key=value file owned by this library. Schema: # schema=fm-pending-reply.v1 # corr_id= privacy-safe correlation token @@ -126,6 +132,21 @@ fm_pending_reply_path() { # printf '%s/%s' "$(fm_pending_reply_dir "$1")" "$2" } +# Cold storage for resolved records whose escalation close cannot complete. +# Not scanned by fm_pending_reply_tick - preserves open status-fold decisions +# while keeping the hot pending-replies walk bounded. +fm_pending_reply_stuck_dir() { # + printf '%s/pending-replies-stuck' "$1" +} + +fm_pending_reply_stuck_path() { # + printf '%s/%s' "$(fm_pending_reply_stuck_dir "$1")" "$2" +} + +fm_pending_reply_stuck_log() { # + printf '%s/pending-replies-stuck.log' "$1" +} + # Privacy-safe correlation id: 16 lowercase hex chars (64 bits of entropy). fm_pending_reply_new_id() { local raw hex @@ -427,11 +448,73 @@ fm_pending_reply_discard_undelivered() { # rm -f "$rec" } -# Retire a resolved pending-reply record after its escalation lifecycle is closed -# (or never opened). Unresolved records always refuse and stay on disk. -# Returns 0 when the record is gone (already or newly), 1 when it must remain. +# Classify why close_escalation failed for a resolved+escalated record. +# Used only for the stuck-receipt trail; never invents a status close line. +fm_pending_reply_close_failure_reason() { # + local state=$1 corr=$2 rec parent_status + rec=$(fm_pending_reply_path "$state" "$corr") + parent_status=$(fm_pending_reply_get "$rec" parent_status) + if [ -z "$parent_status" ]; then + printf 'blank-parent-status' + return 0 + fi + if [ ! -e "$parent_status" ]; then + printf 'missing-parent-status' + return 0 + fi + if [ ! -w "$parent_status" ]; then + printf 'status-unwritable' + return 0 + fi + printf 'close-failed' +} + +# Move a resolved hot record into pending-replies-stuck/ and append one receipt. +# Does not close open status-fold decisions. Returns 0 when the hot path is +# clear (moved or already gone); 1 when the hot record must remain (mv failed). +fm_pending_reply_quarantine_resolved() { # + local state=$1 corr=$2 reason=$3 + local rec stuck_dir stuck dest marker log now task_id + rec=$(fm_pending_reply_path "$state" "$corr") + [ -f "$rec" ] || return 0 + [ "$(fm_pending_reply_get "$rec" phase)" = resolved ] || return 1 + stuck_dir=$(fm_pending_reply_stuck_dir "$state") + mkdir -p "$stuck_dir" || return 1 + chmod 700 "$stuck_dir" 2>/dev/null || true + dest=$(fm_pending_reply_stuck_path "$state" "$corr") + # Explicit destination/move failure: leave the hot record so we never claim + # success while the tick would still walk it, and never drop the only durable + # copy without a successful quarantine landing. + if [ -e "$dest" ]; then + if [ ! -f "$dest" ]; then + return 1 + fi + rm -f "$dest" 2>/dev/null || return 1 + fi + if ! mv "$rec" "$dest" 2>/dev/null; then + return 1 + fi + if [ ! -f "$dest" ]; then + return 1 + fi + marker=$(fm_pending_reply_delivery_confirmation_path "$state" "$corr") + rm -f "$marker" 2>/dev/null || true + now=$(fm_pending_reply_now) + task_id=$(fm_pending_reply_get "$dest" task_id) + log=$(fm_pending_reply_stuck_log "$state") + printf 'epoch=%s corr=%s task=%s reason=%s phase=resolved\n' \ + "$now" "$corr" "${task_id:-}" "${reason:-close-failed}" >> "$log" 2>/dev/null || true + return 0 +} + +# Retire a resolved pending-reply record from the hot pending-replies/ walk. +# Happy path: escalation closed (or never opened), then delete. +# Terminal close failure: quarantine out of the hot walk with a durable receipt +# so open status-fold decisions stay intact and polls cannot re-accumulate +# stuck-resolved files. Unresolved records always refuse and stay on disk. +# Returns 0 when the hot path no longer has the record, 1 when it must remain. fm_pending_reply_retire_resolved() { # - local state=$1 corr=$2 rec phase escalated closed marker + local state=$1 corr=$2 rec phase escalated closed marker reason rec=$(fm_pending_reply_path "$state" "$corr") [ -f "$rec" ] || return 0 phase=$(fm_pending_reply_get "$rec" phase) @@ -440,11 +523,15 @@ fm_pending_reply_retire_resolved() { # if [ -n "$escalated" ]; then closed=$(fm_pending_reply_get "$rec" escalation_closed_epoch) if [ -z "$closed" ]; then - fm_pending_reply_close_escalation "$state" "$corr" || return 1 - # Re-read after close: a transient write failure must keep the record. + fm_pending_reply_close_escalation "$state" "$corr" || true + # Re-read after close: a concurrent retire may have removed the file. [ -f "$rec" ] || return 0 closed=$(fm_pending_reply_get "$rec" escalation_closed_epoch) - [ -n "$closed" ] || return 1 + if [ -z "$closed" ]; then + reason=$(fm_pending_reply_close_failure_reason "$state" "$corr") + fm_pending_reply_quarantine_resolved "$state" "$corr" "$reason" || return 1 + return 0 + fi fi fi marker=$(fm_pending_reply_delivery_confirmation_path "$state" "$corr") diff --git a/tests/fm-pending-reply.test.sh b/tests/fm-pending-reply.test.sh index 1091ce842d..0102469996 100755 --- a/tests/fm-pending-reply.test.sh +++ b/tests/fm-pending-reply.test.sh @@ -1232,6 +1232,170 @@ test_tick_retires_record_that_resolves_mid_poll() { pass "tick retires a record that resolves mid-poll" } +# Panel BREAK 1: resolved+escalated records that cannot close must leave the hot +# walk via quarantine without losing open status-fold decisions. +seed_stuck_resolved_escalated() { # + local state=$1 home=$2 corr=$3 parent_status=$4 summary=$5 rec dir + dir=$(fm_pending_reply_dir "$state") + mkdir -p "$dir" || return 1 + rec=$(fm_pending_reply_path "$state" "$corr") + cat > "$rec" < "$status" + chmod 444 "$status" || fail "could not make parent status read-only" + seed_stuck_resolved_escalated "$state" "$home" "$corr" "$status" "unwritable status" + fm_pending_reply_retire_resolved "$state" "$corr" \ + || fail "unwritable status resolved+escalated must leave the hot path" + [ ! -f "$(fm_pending_reply_path "$state" "$corr")" ] \ + || fail "hot path must not retain unwritable-status stuck records" + dest=$(fm_pending_reply_stuck_path "$state" "$corr") + [ -f "$dest" ] || fail "unwritable-status record must quarantine" + log=$(fm_pending_reply_stuck_log "$state") + grep -Fq "corr=$corr" "$log" || fail "stuck log must name the unwritable corr" + grep -Eq 'reason=(status-unwritable|close-failed)' "$log" \ + || fail "stuck log must classify unwritable/close-failed, got: $(cat "$log")" + open=$(status_open_decisions "$status") + assert_contains "$open" "pending-reply-$corr" \ + "open escalation must remain open after quarantine (no false close)" + chmod 644 "$status" 2>/dev/null || true + pass "unwritable status quarantines without closing the open fold decision" +} + +test_two_hundred_stuck_empty_parent_status_leave_hot_path() { + local home state dir i corr hot_left stuck_left + home=$(setup_parent stuck-scale) + state="$home/state" + export FM_PENDING_REPLY_NOW=13200 + dir=$(fm_pending_reply_dir "$state") + mkdir -p "$dir" || fail "could not create hot pending-replies dir" + for i in $(seq 1 200); do + corr=$(printf 'c%015x' "$i") + seed_stuck_resolved_escalated "$state" "$home" "$corr" "" "stuck scale $i" \ + || fail "could not seed stuck record $i" + done + fm_pending_reply_tick "$state" || fail "tick over stuck population failed" + hot_left=0 + for corr in "$dir"/*; do + [ -f "$corr" ] || continue + case "$(basename "$corr")" in .*) continue ;; esac + hot_left=$((hot_left + 1)) + done + [ "$hot_left" -eq 0 ] \ + || fail "hot pending-replies must be empty after stuck quarantine, left $hot_left" + stuck_left=0 + for corr in "$(fm_pending_reply_stuck_dir "$state")"/*; do + [ -f "$corr" ] || continue + case "$(basename "$corr")" in .*) continue ;; esac + stuck_left=$((stuck_left + 1)) + done + [ "$stuck_left" -eq 200 ] \ + || fail "all 200 stuck records must quarantine, got $stuck_left" + pass "200 stuck empty-parent_status records leave the hot walk in one tick" +} + +test_unresolved_escalated_never_quarantines() { + local home state corr rec + home=$(setup_parent never-quarantine-open) + state="$home/state" + export FM_PENDING_REPLY_NOW=13300 + corr=$(fm_pending_reply_create "$home" "$state" "hibit" "still open escalate") + fm_pending_reply_mark_delivered "$state" "$corr" + rec=$(fm_pending_reply_path "$state" "$corr") + fm_pending_reply_set "$rec" phase escalated + fm_pending_reply_set "$rec" escalated_epoch 13290 + if fm_pending_reply_retire_resolved "$state" "$corr" 2>/dev/null; then + fail "unresolved escalated must refuse retire" + fi + [ -f "$rec" ] || fail "unresolved escalated must remain on the hot path" + [ ! -f "$(fm_pending_reply_stuck_path "$state" "$corr")" ] \ + || fail "unresolved escalated must never quarantine" + pass "unresolved escalated never quarantines" +} + +test_quarantine_mv_failure_keeps_hot_record() { + local home state corr stuck_dir dest rec + home=$(setup_parent stuck-mv-fail) + state="$home/state" + export FM_PENDING_REPLY_NOW=13400 + corr=$(printf 'b%015x' 3) + seed_stuck_resolved_escalated "$state" "$home" "$corr" "" "mv failure" + rec=$(fm_pending_reply_path "$state" "$corr") + stuck_dir=$(fm_pending_reply_stuck_dir "$state") + dest=$(fm_pending_reply_stuck_path "$state" "$corr") + # Non-file destination cannot be replaced by mv of the hot record. + mkdir -p "$dest" || fail "could not create blocking stuck destination" + if fm_pending_reply_quarantine_resolved "$state" "$corr" blank-parent-status 2>/dev/null; then + fail "quarantine must fail closed when the stuck destination cannot accept the move" + fi + [ -f "$rec" ] || fail "hot record must remain when quarantine mv cannot complete" + [ ! -f "$dest" ] || fail "blocking destination must not become the quarantined file" + rm -rf "$stuck_dir" + pass "quarantine mv failure keeps the hot record" +} + # --- run -------------------------------------------------------------------- test_normal_correlated_reply_resolves_once @@ -1265,5 +1429,10 @@ test_failed_send_discards_undelivered_expectation test_large_resolved_population_retires_and_keeps_beacon_fresh test_single_slow_observation_keeps_beacon_fresh_mid_poll test_tick_retires_record_that_resolves_mid_poll +test_blank_parent_status_resolved_escalated_quarantines +test_unwritable_status_resolved_escalated_quarantines_preserves_open +test_two_hundred_stuck_empty_parent_status_leave_hot_path +test_unresolved_escalated_never_quarantines +test_quarantine_mv_failure_keeps_hot_record printf 'ok - all pending-reply tests passed\n' From b7b92e89a5965e75f095f5fb03d47c09a21d9c97 Mon Sep 17 00:00:00 2001 From: Joseph Kim Date: Sun, 16 Aug 2026 00:03:02 -0700 Subject: [PATCH 07/19] fix(supervision): drop unused local in pending-reply quarantine --- bin/fm-pending-reply-lib.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bin/fm-pending-reply-lib.sh b/bin/fm-pending-reply-lib.sh index e218a77f14..44a55456d5 100755 --- a/bin/fm-pending-reply-lib.sh +++ b/bin/fm-pending-reply-lib.sh @@ -474,7 +474,7 @@ fm_pending_reply_close_failure_reason() { # # clear (moved or already gone); 1 when the hot record must remain (mv failed). fm_pending_reply_quarantine_resolved() { # local state=$1 corr=$2 reason=$3 - local rec stuck_dir stuck dest marker log now task_id + local rec stuck_dir dest marker log now task_id rec=$(fm_pending_reply_path "$state" "$corr") [ -f "$rec" ] || return 0 [ "$(fm_pending_reply_get "$rec" phase)" = resolved ] || return 1 From 456585181ab853e2f3603ad61e60e94ec8715295 Mon Sep 17 00:00:00 2001 From: Joseph Kim Date: Sun, 16 Aug 2026 00:19:30 -0700 Subject: [PATCH 08/19] fix(tests): drop unused locals in stuck-quarantine fixtures --- tests/fm-pending-reply.test.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/fm-pending-reply.test.sh b/tests/fm-pending-reply.test.sh index 0102469996..47a5e085d5 100755 --- a/tests/fm-pending-reply.test.sh +++ b/tests/fm-pending-reply.test.sh @@ -1272,7 +1272,7 @@ EOF } test_blank_parent_status_resolved_escalated_quarantines() { - local home state corr stuck dest log open + local home state corr dest log home=$(setup_parent stuck-blank-ps) state="$home/state" export FM_PENDING_REPLY_NOW=13000 From 7851dee50f737a6b274603297d55ac2ef4836350 Mon Sep 17 00:00:00 2001 From: Joseph Kim Date: Sun, 16 Aug 2026 00:50:06 -0700 Subject: [PATCH 09/19] fix(supervision): quarantine stuck-resolved records; preserve peer stand-down Close BREAK 1: resolved+escalated records that cannot close leave the hot pending-replies walk via quarantine to pending-replies-stuck/ with a durable receipt (chmod-444 unwritable fixture; fail closed on quarantine move failure). Restore peer-startup race: arm entry still requires a fresh beacon to attach without starting a child, and stand-down diagnostics stay on disk during live-holder attach so the child "already running" line remains visible. --- bin/fm-watch-arm.sh | 53 ++++++++++++++++++++++++++------------------- 1 file changed, 31 insertions(+), 22 deletions(-) diff --git a/bin/fm-watch-arm.sh b/bin/fm-watch-arm.sh index a9c600f6c7..f5dfb3908e 100755 --- a/bin/fm-watch-arm.sh +++ b/bin/fm-watch-arm.sh @@ -471,20 +471,20 @@ if [ "$mode" = restart ]; then fi fi -# If a genuinely live identity-matched watcher already holds the lock, do not -# start a second one - attach to that cycle and wait until it ends so the -# harness notify fires then, not as an immediate empty wake. Prefer a fresh -# beacon, but still attach when the holder is live with a temporarily stale -# beacon (mid long poll). (--restart skips this: it just stopped this home's +# If a genuinely live+fresh watcher already holds the lock, do not start a +# second one - attach to that cycle and wait until it ends so the harness +# notify fires then, not as an immediate empty wake. A live holder with a +# temporarily stale beacon is still mid-poll: start the usual tracked child so +# it can self-evict/stand down behind the peer, then attach via the +# confirmation and owned-child paths that accept live_watcher_holder without +# requiring a fresh beacon. (--restart skips this: it just stopped this home's # watcher and wants a fresh one.) -if [ "$mode" = arm ]; then - if healthy_watcher || live_watcher_holder; then - cycle_mark_predecessor_successor "attached:$HEALTHY_PID" - cycle_begin "$HEALTHY_PID" attached "$HEALTHY_IDENTITY" - report_attached - attach_and_wait "$HEALTHY_PID" - exit $? - fi +if [ "$mode" = arm ] && healthy_watcher; then + cycle_mark_predecessor_successor "attached:$HEALTHY_PID" + cycle_begin "$HEALTHY_PID" attached "$HEALTHY_IDENTITY" + report_attached + attach_and_wait "$HEALTHY_PID" + exit $? fi # Start a watcher as a tracked child and confirm it before settling in. The child @@ -563,14 +563,16 @@ owned_child_finished() { if live_watcher_holder; then cycle_log_append "$rc" "$signal" unexpected-clean-exit "attached:$HEALTHY_PID" print_watch_output "$child_out" - rm -f "$child_out" 2>/dev/null || true child= - child_out= + # Keep child_out until after attach so stand-down diagnostics stay visible. cycle_mark_predecessor_successor "attached:$HEALTHY_PID" report_attached cycle_begin "$HEALTHY_PID" attached "$HEALTHY_IDENTITY" attach_and_wait "$HEALTHY_PID" - return $? + rc=$? + rm -f "$child_out" 2>/dev/null || true + child_out= + return "$rc" fi print_watch_output "$child_out" rm -f "$child_out" 2>/dev/null || true @@ -649,13 +651,16 @@ while :; do wait "$child" 2>/dev/null || true child= print_watch_output "$child_out" - rm -f "$child_out" 2>/dev/null || true - child_out= + # Leave child_out on disk until EXIT cleanup so peer-stand-down diagnostics + # (e.g. "watcher: already running pid …") remain readable during attach. cycle_mark_predecessor_successor "attached:$HEALTHY_PID" report_attached cycle_begin "$HEALTHY_PID" attached "$HEALTHY_IDENTITY" attach_and_wait "$HEALTHY_PID" - exit $? + rc=$? + rm -f "$child_out" 2>/dev/null || true + child_out= + exit "$rc" fi [ "$(date +%s)" -ge "$deadline" ] && break sleep 0.2 @@ -667,15 +672,19 @@ trap - HUP TERM INT # fresh beacon checked above before it can be reported as started. if live_watcher_holder && [ "$HEALTHY_PID" != "$child" ]; then print_watch_output "$child_out" - cleanup_child + if [ -n "$child" ] && fm_pid_alive "$child"; then + kill -TERM "$child" 2>/dev/null || true + fi wait "$child" 2>/dev/null || true child= - child_out= cycle_mark_predecessor_successor "attached:$HEALTHY_PID" report_attached cycle_begin "$HEALTHY_PID" attached "$HEALTHY_IDENTITY" attach_and_wait "$HEALTHY_PID" - exit $? + rc=$? + rm -f "$child_out" 2>/dev/null || true + child_out= + exit "$rc" fi print_watch_output "$child_out" cleanup_child From ae296c622300f71f851e46efdabecea8c84430ff Mon Sep 17 00:00:00 2001 From: Joseph Kim Date: Sun, 16 Aug 2026 01:25:25 -0700 Subject: [PATCH 10/19] no-mistakes(review): Attach stale-beacon watcher holders before spawning --- bin/fm-watch-arm.sh | 26 +++++++++++++------------- tests/fm-watch-arm.test.sh | 4 +++- 2 files changed, 16 insertions(+), 14 deletions(-) diff --git a/bin/fm-watch-arm.sh b/bin/fm-watch-arm.sh index f5dfb3908e..c38a8a8c4c 100755 --- a/bin/fm-watch-arm.sh +++ b/bin/fm-watch-arm.sh @@ -471,20 +471,20 @@ if [ "$mode" = restart ]; then fi fi -# If a genuinely live+fresh watcher already holds the lock, do not start a -# second one - attach to that cycle and wait until it ends so the harness -# notify fires then, not as an immediate empty wake. A live holder with a -# temporarily stale beacon is still mid-poll: start the usual tracked child so -# it can self-evict/stand down behind the peer, then attach via the -# confirmation and owned-child paths that accept live_watcher_holder without -# requiring a fresh beacon. (--restart skips this: it just stopped this home's +# If a genuinely live identity-matched watcher already holds the lock, do not +# start a second one - attach to that cycle and wait until it ends so the +# harness notify fires then, not as an immediate empty wake. Prefer a fresh +# beacon, but still attach when the holder is live with a temporarily stale +# beacon (mid long poll). (--restart skips this: it just stopped this home's # watcher and wants a fresh one.) -if [ "$mode" = arm ] && healthy_watcher; then - cycle_mark_predecessor_successor "attached:$HEALTHY_PID" - cycle_begin "$HEALTHY_PID" attached "$HEALTHY_IDENTITY" - report_attached - attach_and_wait "$HEALTHY_PID" - exit $? +if [ "$mode" = arm ]; then + if healthy_watcher || live_watcher_holder; then + cycle_mark_predecessor_successor "attached:$HEALTHY_PID" + cycle_begin "$HEALTHY_PID" attached "$HEALTHY_IDENTITY" + report_attached + attach_and_wait "$HEALTHY_PID" + exit $? + fi fi # Start a watcher as a tracked child and confirm it before settling in. The child diff --git a/tests/fm-watch-arm.test.sh b/tests/fm-watch-arm.test.sh index 4ae5156324..ec5e0eb58f 100755 --- a/tests/fm-watch-arm.test.sh +++ b/tests/fm-watch-arm.test.sh @@ -214,7 +214,7 @@ test_arm_attaches_to_live_holder_with_stale_beacon() { PATH="$fakebin:$PATH" FM_STATE_OVERRIDE="$state" FM_CREW_STATE_BIN="$fakebin/fm-crew-state.sh" \ FM_ARM_ATTACH_POLL=0.1 FM_ARM_CONFIRM_TIMEOUT=2 FM_GUARD_GRACE=1 \ FM_POLL=5 FM_SIGNAL_GRACE=1 FM_CHECK_INTERVAL=999999 FM_HEARTBEAT=999999 \ - "$WATCH_ARM" > "$armout" & + "$WATCH_ARM" > "$armout" 2> "$dir/arm.err" & ARM_PID=$! i=0 while [ "$i" -lt 80 ]; do @@ -228,6 +228,8 @@ test_arm_attaches_to_live_holder_with_stale_beacon() { || fail "stale-beacon live holder must not be reported as FAILED: $(cat "$armout")" ! grep -qF 'watcher: started' "$armout" \ || fail "stale-beacon live holder must not start a second watcher: $(cat "$armout")" + [ ! -s "$dir/arm.err" ] \ + || fail "stale-beacon arm launched a competing watcher: $(cat "$dir/arm.err")" # Still the original singleton; a competing start would replace the lock pid. [ "$(cat "$state/.watch.lock/pid" 2>/dev/null || true)" = "$SEED_PID" ] \ || fail "arm replaced the live stale-beacon holder instead of attaching" From 29ae58568ce1cd6ff648acfcf3da4cd3bdec8b1a Mon Sep 17 00:00:00 2001 From: Joseph Kim Date: Sun, 16 Aug 2026 01:31:13 -0700 Subject: [PATCH 11/19] no-mistakes(review): Bind watcher attachment to generation beacon proof --- bin/fm-wake-lib.sh | 1 + bin/fm-watch-arm.sh | 7 +++++++ bin/fm-watch.sh | 12 ++++++++++-- tests/fm-watch-arm.test.sh | 39 ++++++++++++++++++++++++++++++++++++++ 4 files changed, 57 insertions(+), 2 deletions(-) diff --git a/bin/fm-wake-lib.sh b/bin/fm-wake-lib.sh index 6c25b4637f..68e6acfe99 100755 --- a/bin/fm-wake-lib.sh +++ b/bin/fm-wake-lib.sh @@ -298,6 +298,7 @@ fm_lock_clean_known_files() { rm -f \ "$lockdir/pid" \ "$lockdir/fm-home" \ + "$lockdir/beacon-identity" \ "$lockdir/pid-identity" \ "$lockdir/role" \ "$lockdir/watcher-path" \ diff --git a/bin/fm-watch-arm.sh b/bin/fm-watch-arm.sh index c38a8a8c4c..b0c765622d 100755 --- a/bin/fm-watch-arm.sh +++ b/bin/fm-watch-arm.sh @@ -246,10 +246,16 @@ clear_stale_recorded_watcher_lock() { # started/healthy watcher that is not really there. HEALTHY_PID= HEALTHY_IDENTITY= +watcher_generation_beaconed() { + local identity=$1 + [ "$(cat "$WATCH_LOCK/beacon-identity" 2>/dev/null || true)" = "$identity" ] +} + healthy_watcher() { HEALTHY_PID= HEALTHY_IDENTITY= fm_watcher_healthy "$STATE" "$WATCH" "$GRACE" "$FM_HOME" || return 1 + watcher_generation_beaconed "$FM_WATCHER_HEALTHY_IDENTITY" || return 1 HEALTHY_PID=$FM_WATCHER_HEALTHY_PID HEALTHY_IDENTITY=$FM_WATCHER_HEALTHY_IDENTITY } @@ -266,6 +272,7 @@ live_watcher_holder() { fm_pid_alive "$pid" || return 1 fm_watcher_lock_matches_pid "$STATE" "$WATCH" "$pid" "$FM_HOME" || return 1 identity=$FM_WATCHER_MATCHED_IDENTITY + watcher_generation_beaconed "$identity" || return 1 HEALTHY_PID=$pid HEALTHY_IDENTITY=$identity return 0 diff --git a/bin/fm-watch.sh b/bin/fm-watch.sh index f8477488e9..f9798b2baa 100755 --- a/bin/fm-watch.sh +++ b/bin/fm-watch.sh @@ -757,6 +757,7 @@ if ! fm_lock_try_acquire "$WATCH_LOCK"; then fi exit 0 fi +WATCH_LOCK_OWNER=$FM_LOCK_OWNER_DIR WATCHER_RECOVERY_PENDING=0 if [ -n "${FM_LOCK_RECOVERED_PID:-}" ]; then WATCHER_RECOVERY_PENDING=1 @@ -802,6 +803,13 @@ FM_WATCH_DELIVERY_PID=$WATCHER_PID FM_WATCH_DELIVERY_IDENTITY=$(fm_pid_identity "$WATCHER_PID" 2>/dev/null || true) printf '%s\n' "$FM_WATCH_DELIVERY_IDENTITY" > "$WATCH_LOCK/pid-identity" 2>/dev/null || true +watcher_beat() { + touch "$STATE/.last-watcher-beat" || return 1 + if [ "$(cat "$WATCH_LOCK_OWNER/beacon-identity" 2>/dev/null || true)" != "$FM_WATCH_DELIVERY_IDENTITY" ]; then + printf '%s\n' "$FM_WATCH_DELIVERY_IDENTITY" > "$WATCH_LOCK_OWNER/beacon-identity" 2>/dev/null + fi +} + [ -e "$STATE/.last-heartbeat" ] || touch "$STATE/.last-heartbeat" # A merged poll may have queued its terminal wake and then lost the process @@ -826,7 +834,7 @@ resurface_after_downtime() { } if [ "${FM_WATCH_HANDLING_SUCCESSOR:-0}" = 1 ]; then - touch "$STATE/.last-watcher-beat" + watcher_beat || true handling_wait=0 while [ "$handling_wait" -lt 600 ]; do fm_recovery_marker_snapshot "$WATCHER_DOWNTIME_MARKER" || true @@ -853,7 +861,7 @@ while :; do # Liveness beacon for fm-guard.sh: a fresh mtime here means a watcher is # alive. Supervision scripts warn when this goes stale with tasks in flight. - touch "$STATE/.last-watcher-beat" + watcher_beat || true # Parent-owned secondmate pending-reply reconciliation: resolve correlated # parent reports, observe backend busy/idle turn completion, send one recovery diff --git a/tests/fm-watch-arm.test.sh b/tests/fm-watch-arm.test.sh index ec5e0eb58f..3d7320b50f 100755 --- a/tests/fm-watch-arm.test.sh +++ b/tests/fm-watch-arm.test.sh @@ -210,6 +210,8 @@ test_arm_attaches_to_live_holder_with_stale_beacon() { [ "$(cat "$state/.watch.lock/pid" 2>/dev/null || true)" = "$SEED_PID" ] \ || fail "seed watcher lost the lock before the stale-beacon arm" is_live_non_zombie "$SEED_PID" || fail "seed watcher died before the stale-beacon arm" + [ "$(cat "$state/.watch.lock/beacon-identity" 2>/dev/null || true)" = "$(fm_test_pid_identity "$SEED_PID")" ] \ + || fail "seed watcher did not publish generation-bound beacon proof" PATH="$fakebin:$PATH" FM_STATE_OVERRIDE="$state" FM_CREW_STATE_BIN="$fakebin/fm-crew-state.sh" \ FM_ARM_ATTACH_POLL=0.1 FM_ARM_CONFIRM_TIMEOUT=2 FM_GUARD_GRACE=1 \ @@ -245,6 +247,42 @@ test_arm_attaches_to_live_holder_with_stale_beacon() { pass "watch-arm: attaches to a live identity-matched holder even when the beacon is stale" } +test_arm_does_not_attach_before_peer_first_beacon() { + local dir home state fakebin armout holder owner identity arm_pid status + dir=$(make_case peer-before-first-beacon) + home="$dir/home" + state="$dir/state" + fakebin="$dir/fakebin" + armout="$dir/arm.out" + owner="$state/.watch.lock.owner.fixture" + mkdir -p "$home/data" "$owner" + + sleep 300 & + holder=$! + identity=$(fm_test_pid_identity "$holder") || fail "could not identify peer watcher fixture" + printf '%s\n' "$holder" > "$owner/pid" + printf '%s\n' "$home" > "$owner/fm-home" + printf '%s\n' "$WATCH" > "$owner/watcher-path" + printf '%s\n' "$identity" > "$owner/pid-identity" + ln -s "$owner" "$state/.watch.lock" + + PATH="$fakebin:$PATH" FM_HOME="$home" FM_STATE_OVERRIDE="$state" \ + FM_ARM_CONFIRM_TIMEOUT=1 FM_GUARD_GRACE=1 FM_POLL=5 FM_SIGNAL_GRACE=1 \ + FM_CHECK_INTERVAL=999999 FM_HEARTBEAT=999999 "$WATCH_ARM" > "$armout" & + arm_pid=$! + wait_for_exit "$arm_pid" 100 + status=$? + [ "$status" -ne 0 ] && [ "$status" -ne 124 ] \ + || fail "arm did not fail bounded confirmation for an unbeaconed peer (status $status)" + ! grep -qF "watcher: attached pid=$holder" "$armout" \ + || fail "arm attached before the peer emitted its first beacon: $(cat "$armout")" + grep -qF 'watcher: FAILED - no live watcher with a fresh beacon' "$armout" \ + || fail "arm did not fail closed for an unbeaconed peer: $(cat "$armout")" + kill "$holder" 2>/dev/null || true + wait "$holder" 2>/dev/null || true + pass "watch-arm: a peer must emit its first beacon before attachment" +} + test_new_child_without_fresh_beacon_fails_confirmation() { local dir state fakebin armout arm_pid status dir=$(make_case new-child-no-beacon) @@ -889,6 +927,7 @@ test_downtime_marker_does_not_follow_symlink() { test_attached_arm_reports_the_delivered_wake test_arm_attaches_to_live_holder_with_stale_beacon +test_arm_does_not_attach_before_peer_first_beacon test_new_child_without_fresh_beacon_fails_confirmation test_attached_arm_reports_the_delivered_wake_after_drain test_attached_arm_still_fails_on_a_wake_it_did_not_deliver From 245e59425ee74cc2ac024859d926f923f779476c Mon Sep 17 00:00:00 2001 From: Joseph Kim Date: Sun, 16 Aug 2026 01:36:37 -0700 Subject: [PATCH 12/19] no-mistakes(review): Enforce generation-bound health across watcher guards --- bin/fm-wake-lib.sh | 6 +++++ bin/fm-watch-arm.sh | 8 +----- tests/fm-claude-stop-autoarm.test.sh | 1 + tests/fm-guard-stale-banner.test.sh | 1 + tests/fm-pr-check-security.test.sh | 1 + ...fm-remote-secondmate-lifecycle-e2e.test.sh | 1 + tests/fm-secondmate-harness.test.sh | 1 + tests/fm-turnend-guard.test.sh | 25 ++++++++++++++++++- tests/fm-wake-queue.test.sh | 1 + tests/fm-watcher-lock.test.sh | 3 +++ 10 files changed, 40 insertions(+), 8 deletions(-) diff --git a/bin/fm-wake-lib.sh b/bin/fm-wake-lib.sh index 68e6acfe99..9095e7046d 100755 --- a/bin/fm-wake-lib.sh +++ b/bin/fm-wake-lib.sh @@ -196,6 +196,11 @@ fm_guard_grace_seconds() { FM_WATCHER_HEALTHY_PID= FM_WATCHER_HEALTHY_IDENTITY= +fm_watcher_generation_beaconed() { + local state=$1 identity=$2 + [ "$(cat "$state/.watch.lock/beacon-identity" 2>/dev/null || true)" = "$identity" ] +} + fm_watcher_healthy() { local state=$1 watch_path=$2 grace=${3:-} home=${4:-$FM_HOME} lockdir beat pid identity age [ -n "$grace" ] || grace=$(fm_guard_grace_seconds) @@ -207,6 +212,7 @@ fm_watcher_healthy() { fm_pid_alive "$pid" || return 1 fm_watcher_lock_matches_pid "$state" "$watch_path" "$pid" "$home" || return 1 identity=$FM_WATCHER_MATCHED_IDENTITY + fm_watcher_generation_beaconed "$state" "$identity" || return 1 age=$(fm_path_age "$beat") [ "$age" -lt "$grace" ] || return 1 # shellcheck disable=SC2034 # Read by callers after fm_watcher_healthy returns. diff --git a/bin/fm-watch-arm.sh b/bin/fm-watch-arm.sh index b0c765622d..5ae722d411 100755 --- a/bin/fm-watch-arm.sh +++ b/bin/fm-watch-arm.sh @@ -246,16 +246,10 @@ clear_stale_recorded_watcher_lock() { # started/healthy watcher that is not really there. HEALTHY_PID= HEALTHY_IDENTITY= -watcher_generation_beaconed() { - local identity=$1 - [ "$(cat "$WATCH_LOCK/beacon-identity" 2>/dev/null || true)" = "$identity" ] -} - healthy_watcher() { HEALTHY_PID= HEALTHY_IDENTITY= fm_watcher_healthy "$STATE" "$WATCH" "$GRACE" "$FM_HOME" || return 1 - watcher_generation_beaconed "$FM_WATCHER_HEALTHY_IDENTITY" || return 1 HEALTHY_PID=$FM_WATCHER_HEALTHY_PID HEALTHY_IDENTITY=$FM_WATCHER_HEALTHY_IDENTITY } @@ -272,7 +266,7 @@ live_watcher_holder() { fm_pid_alive "$pid" || return 1 fm_watcher_lock_matches_pid "$STATE" "$WATCH" "$pid" "$FM_HOME" || return 1 identity=$FM_WATCHER_MATCHED_IDENTITY - watcher_generation_beaconed "$identity" || return 1 + fm_watcher_generation_beaconed "$STATE" "$identity" || return 1 HEALTHY_PID=$pid HEALTHY_IDENTITY=$identity return 0 diff --git a/tests/fm-claude-stop-autoarm.test.sh b/tests/fm-claude-stop-autoarm.test.sh index 0a21d451c4..4fa3ec0d79 100755 --- a/tests/fm-claude-stop-autoarm.test.sh +++ b/tests/fm-claude-stop-autoarm.test.sh @@ -179,6 +179,7 @@ record_watcher_lock() { printf '%s\n' "$root" > "$dir/state/.watch.lock/fm-home" printf '%s\n' "$bin_dir/fm-watch.sh" > "$dir/state/.watch.lock/watcher-path" printf '%s\n' "$identity" > "$dir/state/.watch.lock/pid-identity" + printf '%s\n' "$identity" > "$dir/state/.watch.lock/beacon-identity" } # --- registration contract ---------------------------------------------------- diff --git a/tests/fm-guard-stale-banner.test.sh b/tests/fm-guard-stale-banner.test.sh index 939c027d17..fd1791dcc4 100755 --- a/tests/fm-guard-stale-banner.test.sh +++ b/tests/fm-guard-stale-banner.test.sh @@ -39,6 +39,7 @@ record_live_watcher() { printf '%s\n' "$home" > "$home/state/.watch.lock/fm-home" printf '%s\n' "$ROOT/bin/fm-watch.sh" > "$home/state/.watch.lock/watcher-path" printf '%s\n' "$identity" > "$home/state/.watch.lock/pid-identity" + printf '%s\n' "$identity" > "$home/state/.watch.lock/beacon-identity" } # These cases exercise the persistent-watcher model (a live pid is the real diff --git a/tests/fm-pr-check-security.test.sh b/tests/fm-pr-check-security.test.sh index 03c6ce688e..d50e46d12d 100755 --- a/tests/fm-pr-check-security.test.sh +++ b/tests/fm-pr-check-security.test.sh @@ -177,6 +177,7 @@ write_watcher_lock() { 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" + printf '%s\n' "$identity" > "$state/.watch.lock/beacon-identity" } assert_valid_migration_marker() { diff --git a/tests/fm-remote-secondmate-lifecycle-e2e.test.sh b/tests/fm-remote-secondmate-lifecycle-e2e.test.sh index a095831dff..00768d691d 100755 --- a/tests/fm-remote-secondmate-lifecycle-e2e.test.sh +++ b/tests/fm-remote-secondmate-lifecycle-e2e.test.sh @@ -252,6 +252,7 @@ publish_healthy_watcher_identity() { # printf '%s\n' "$identity" > "$state/.watch.lock/pid-identity" 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/beacon-identity" touch "$state/.last-watcher-beat" } diff --git a/tests/fm-secondmate-harness.test.sh b/tests/fm-secondmate-harness.test.sh index cd9d7dd06c..0207636e76 100755 --- a/tests/fm-secondmate-harness.test.sh +++ b/tests/fm-secondmate-harness.test.sh @@ -961,6 +961,7 @@ record_live_watcher_fixture() { printf '%s\n' "$home" > "$home/state/.watch.lock/fm-home" printf '%s\n' "$ROOT/bin/fm-watch.sh" > "$home/state/.watch.lock/watcher-path" printf '%s\n' "$identity" > "$home/state/.watch.lock/pid-identity" + printf '%s\n' "$identity" > "$home/state/.watch.lock/beacon-identity" touch "$home/state/.last-watcher-beat" } diff --git a/tests/fm-turnend-guard.test.sh b/tests/fm-turnend-guard.test.sh index 73da54b75a..4ed8b0062a 100755 --- a/tests/fm-turnend-guard.test.sh +++ b/tests/fm-turnend-guard.test.sh @@ -210,7 +210,7 @@ watcher_identity() { } record_watcher_lock() { - local dir=$1 pid=$2 identity=$3 root bin_dir + local dir=$1 pid=$2 identity=$3 beacon_identity=${4-$identity} root bin_dir root=$(cd "$dir" && pwd) bin_dir=$(cd "$dir/bin" && pwd) mkdir -p "$dir/state/.watch.lock" @@ -218,6 +218,7 @@ record_watcher_lock() { printf '%s\n' "$root" > "$dir/state/.watch.lock/fm-home" printf '%s\n' "$bin_dir/fm-watch.sh" > "$dir/state/.watch.lock/watcher-path" printf '%s\n' "$identity" > "$dir/state/.watch.lock/pid-identity" + [ -z "$beacon_identity" ] || printf '%s\n' "$beacon_identity" > "$dir/state/.watch.lock/beacon-identity" } test_hook_silent_when_no_work_in_flight() { @@ -296,6 +297,27 @@ test_hook_silent_with_live_lock_and_fresh_beacon() { pass "fm-turnend-guard: silent no-op with a live watcher lock and fresh beacon" } +test_hook_blocks_unbeaconed_live_lock_with_fresh_leftover() { + local dir pid identity out status + dir=$(make_primary_dir "$TMP_ROOT/hook-live-lock-unbeaconed") + : > "$dir/state/task1.meta" + sleep 60 & + pid=$! + identity=$(watcher_identity "$dir" "$pid") || { + kill "$pid" 2>/dev/null || true + wait "$pid" 2>/dev/null || true + fail "could not identify unbeaconed watcher holder" + } + record_watcher_lock "$dir" "$pid" "$identity" '' + touch "$dir/state/.last-watcher-beat" + out=$(run_hook "$dir" false); status=$? + kill "$pid" 2>/dev/null || true + wait "$pid" 2>/dev/null || true + expect_code 2 "$status" "hook must block when the current watcher generation has not beaconed" + assert_contains "$out" "$REQUIRED_REASON" "unbeaconed generation block must contain the required instruction" + pass "fm-turnend-guard: fresh leftover beacon cannot validate a new lock generation" +} + test_hook_non_claude_health_ignores_claude_budget_contention() { local dir home pid identity holder harness payload out status dir=$(make_primary_dir "$TMP_ROOT/hook-non-claude-budget-contention") @@ -1638,6 +1660,7 @@ test_hook_blocks_source_only_home test_hook_blocks_queue_only_home_with_queue_reason test_hook_blocks_when_dead_lock_has_fresh_beacon test_hook_silent_with_live_lock_and_fresh_beacon +test_hook_blocks_unbeaconed_live_lock_with_fresh_leftover test_hook_non_claude_health_ignores_claude_budget_contention test_hook_blocks_with_live_lock_and_stale_beacon test_hook_blocks_when_unhealthy_in_primary diff --git a/tests/fm-wake-queue.test.sh b/tests/fm-wake-queue.test.sh index f2ca20b15e..a7e07fecf5 100755 --- a/tests/fm-wake-queue.test.sh +++ b/tests/fm-wake-queue.test.sh @@ -320,6 +320,7 @@ test_drain_asserts_watcher_liveness() { printf '%s\n' "$dir" > "$state/.watch.lock/fm-home" printf '%s\n' "$WATCH" > "$state/.watch.lock/watcher-path" printf '%s\n' "$identity" > "$state/.watch.lock/pid-identity" + printf '%s\n' "$identity" > "$state/.watch.lock/beacon-identity" touch "$state/.last-watcher-beat" FM_HOME="$dir" FM_STATE_OVERRIDE="$state" FM_GUARD_GRACE=300 "$DRAIN" >/dev/null 2> "$err" \ || fail "drain failed with a live watcher and fresh beacon" diff --git a/tests/fm-watcher-lock.test.sh b/tests/fm-watcher-lock.test.sh index d3b693a9db..198e937f2e 100755 --- a/tests/fm-watcher-lock.test.sh +++ b/tests/fm-watcher-lock.test.sh @@ -180,6 +180,7 @@ test_guard_warnings() { printf '%s\n' "$dir" > "$state/.watch.lock/fm-home" printf '%s\n' "$WATCH" > "$state/.watch.lock/watcher-path" printf '%s\n' "$identity" > "$state/.watch.lock/pid-identity" + printf '%s\n' "$identity" > "$state/.watch.lock/beacon-identity" touch "$state/.last-watcher-beat" # Non-git FM_ROOT keeps the worktree-tangle check inert so "fresh watcher -> # total silence" stays a pure assertion about watcher state. @@ -500,6 +501,7 @@ test_watch_restart_attaches_to_healthy_peer() { printf '%s\n' "$dir" > "$state/.watch.lock/fm-home" printf '%s\n' "$WATCH" > "$state/.watch.lock/watcher-path" printf '%s\n' "$identity" > "$state/.watch.lock/pid-identity" + printf '%s\n' "$identity" > "$state/.watch.lock/beacon-identity" touch "$state/.last-watcher-beat" PATH="$fakebin:$PATH" FM_HOME="$dir" FM_POLL=5 FM_SIGNAL_GRACE=1 FM_CHECK_INTERVAL=999999 FM_HEARTBEAT=999999 FM_ARM_ATTACH_POLL=0.1 FM_ARM_CONFIRM_TIMEOUT=1 "$WATCH_ARM" --restart > "$out" & armpid=$! @@ -803,6 +805,7 @@ test_arm_waits_for_peer_beacon_after_child_stands_down() { grep -qF "watcher: already running pid $peer" "$state"/.watch-arm-output.* 2>/dev/null \ || fail "arm child did not stand down behind the peer watcher" touch "$state/.last-watcher-beat" + printf '%s\n' "$identity" > "$state/.watch.lock/beacon-identity" i=0 while [ "$i" -lt 80 ]; do grep -qF "watcher: attached pid=$peer" "$armout" 2>/dev/null && break From 08c2e9eec28724611dfb7db68c0b4d85c5de361d Mon Sep 17 00:00:00 2001 From: Joseph Kim Date: Sun, 16 Aug 2026 02:51:23 -0700 Subject: [PATCH 13/19] no-mistakes(test): Bound watcher teardown and stabilize supervision tests --- bin/fm-watch-arm.sh | 35 ++++++++++++++++++---------------- tests/fm-turnend-guard.test.sh | 3 ++- tests/fm-watch-arm.test.sh | 18 ++++++++--------- tests/fm-watcher-lock.test.sh | 31 ++++++++++++++---------------- tests/wake-helpers.sh | 6 ++++++ 5 files changed, 50 insertions(+), 43 deletions(-) diff --git a/bin/fm-watch-arm.sh b/bin/fm-watch-arm.sh index 5ae722d411..afee34c988 100755 --- a/bin/fm-watch-arm.sh +++ b/bin/fm-watch-arm.sh @@ -424,6 +424,21 @@ handling_successor_generation() { esac } +stop_pid_bounded() { # [tenths-of-a-second] + local pid=$1 limit=${2:-50} i=0 + [ -n "$pid" ] || return 0 + fm_pid_alive "$pid" || return 0 + kill -TERM "$pid" 2>/dev/null || true + while [ "$i" -lt "$limit" ] && fm_pid_alive "$pid"; do + sleep 0.1 + i=$((i + 1)) + done + if fm_pid_alive "$pid"; then + kill -KILL "$pid" 2>/dev/null || true + fi + wait "$pid" 2>/dev/null || true +} + mode=arm handling_generation= handling_watcher_pid= @@ -454,15 +469,7 @@ if [ "$mode" = restart ]; then lock_pid=$(cat "$WATCH_LOCK/pid" 2>/dev/null || true) if fm_pid_alive "$lock_pid"; then if fm_watcher_lock_matches_pid "$STATE" "$WATCH" "$lock_pid" "$FM_HOME"; then - kill -TERM "$lock_pid" 2>/dev/null || true - # Wait for it to actually exit before relaunching, so the fresh watcher - # either takes a released lock or reclaims a now-dead-pid stale lock instead - # of seeing the dying one as a live holder and no-opping. - i=0 - while [ "$i" -lt 50 ] && fm_pid_alive "$lock_pid"; do - sleep 0.1 - i=$((i + 1)) - done + stop_pid_bounded "$lock_pid" else if ! clear_stale_recorded_watcher_lock; then echo "watcher: FAILED - stale watcher recovery state could not be persisted" >&2 @@ -495,9 +502,7 @@ fi child= child_out= cleanup_child() { - if [ -n "$child" ] && fm_pid_alive "$child"; then - kill -TERM "$child" 2>/dev/null || true - fi + [ -z "$child" ] || stop_pid_bounded "$child" if [ -n "$child_out" ]; then rm -f "$child_out" 2>/dev/null || true fi @@ -507,10 +512,8 @@ 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 + [ -z "$child" ] || stop_pid_bounded "$child" + clear_stale_recorded_watcher_lock || true cycle_log_append "$rc" "$signal" arm-interrupted none cleanup_child exit "$rc" diff --git a/tests/fm-turnend-guard.test.sh b/tests/fm-turnend-guard.test.sh index 4ed8b0062a..1a54672268 100755 --- a/tests/fm-turnend-guard.test.sh +++ b/tests/fm-turnend-guard.test.sh @@ -210,7 +210,8 @@ watcher_identity() { } record_watcher_lock() { - local dir=$1 pid=$2 identity=$3 beacon_identity=${4-$identity} root bin_dir + local dir=$1 pid=$2 identity=$3 root bin_dir + local beacon_identity=${4-$identity} root=$(cd "$dir" && pwd) bin_dir=$(cd "$dir/bin" && pwd) mkdir -p "$dir/state/.watch.lock" diff --git a/tests/fm-watch-arm.test.sh b/tests/fm-watch-arm.test.sh index 3d7320b50f..496e9b8f5f 100755 --- a/tests/fm-watch-arm.test.sh +++ b/tests/fm-watch-arm.test.sh @@ -405,7 +405,7 @@ test_rearm_resurfaces_durable_queue_and_remote_open_decision() { is_live_non_zombie "$ARM_PID" || fail "pre-outage watcher did not stay live" watcher_pid=$(cat "$state/.watch.lock/pid" 2>/dev/null || true) kill -KILL "$watcher_pid" 2>/dev/null || fail "could not abruptly stop pre-outage watcher" - wait "$ARM_PID" 2>/dev/null || true + wait_for_exit "$ARM_PID" 80 || true [ ! -e "$state/.watcher-down" ] || fail "abrupt watcher exit unexpectedly ran cleanup" rm -f "$state/.pr-check-migration-v1" "$state/.pr-check-migration-scan-v1" @@ -452,7 +452,7 @@ test_rearm_resurfaces_durable_queue_and_remote_open_decision() { # A later down interval can have no new queue rows at all. The unchanged # remote decision must still trigger a recovery wake and be folded again. kill "$ARM_PID" 2>/dev/null || true - wait "$ARM_PID" 2>/dev/null || true + wait_for_exit "$ARM_PID" 80 || true start_rearm_arm "$home" "$state" "$fakebin" "$dir/decision-only-arm.out" wait_for_exit "$ARM_PID" 80 || fail "decision-only re-arm did not surface the open decision" decision_recovery_arm=$ARM_PID @@ -474,7 +474,7 @@ test_rearm_resurfaces_durable_queue_and_remote_open_decision() { || fail "decision-only handling successor emitted recursive recovery" kill -TERM "$decision_successor" 2>/dev/null || fail "could not interrupt decision handling successor" - wait "$decision_successor" 2>/dev/null || true + wait_for_exit "$decision_successor" 80 || true start_rearm_arm "$home" "$state" "$fakebin" "$dir/interrupted-decision-arm.out" wait_for_exit "$ARM_PID" 80 || fail "interrupted decision handling was not recovered on successor re-arm" grep -F 'check: rearm-resurface' "$dir/interrupted-decision-arm.out" >/dev/null \ @@ -493,7 +493,7 @@ test_rearm_resurfaces_durable_queue_and_remote_open_decision() { start_rearm_arm "$home" "$state" "$fakebin" "$dir/decision-successor-arm.out" is_live_non_zombie "$ARM_PID" || fail "acknowledged decision recovery did not leave a live successor" kill "$ARM_PID" 2>/dev/null || true - wait "$ARM_PID" 2>/dev/null || true + wait_for_exit "$ARM_PID" 80 || true pass "watch-arm: re-arm surfaces every queued wake and an open remote decision after downtime" } @@ -511,7 +511,7 @@ test_marker_publish_failure_retains_recovery_evidence() { watcher_pid=$(cat "$state/.watch.lock/pid" 2>/dev/null || true) mkdir "$state/.watcher-down" kill -TERM "$watcher_pid" 2>/dev/null || fail "could not stop marker-failure fixture watcher" - wait "$first_arm" 2>/dev/null || true + wait_for_exit "$first_arm" 80 || true [ "$(cat "$state/.watch.lock/pid" 2>/dev/null || true)" = "$watcher_pid" ] \ || fail "marker publication failure discarded stale-lock recovery evidence" @@ -562,7 +562,7 @@ test_delivery_gap_wake_is_recovered_once() { start_rearm_arm "$home" "$state" "$fakebin" "$dir/stable-successor.out" is_live_non_zombie "$ARM_PID" || fail "successor looped after the delivery gap was drained" kill "$ARM_PID" 2>/dev/null || true - wait "$ARM_PID" 2>/dev/null || true + wait_for_exit "$ARM_PID" 80 || true pass "watch-arm: a wake queued after handling drain is recovered once at successor arm" } @@ -627,7 +627,7 @@ test_interrupted_handling_is_redrained_on_rearm() { is_live_non_zombie "$ARM_PID" || fail "handling drain stopped its live successor" kill -TERM "$ARM_PID" 2>/dev/null || fail "could not interrupt the handling successor" - wait "$ARM_PID" 2>/dev/null || true + wait_for_exit "$ARM_PID" 80 || true case "$(cat "$state/.watcher-down" 2>/dev/null || true)" in pending:downtime:*) ;; *) fail "interrupted pre-handling successor did not persist downtime recovery" ;; @@ -674,7 +674,7 @@ test_malformed_marker_is_quarantined_once() { start_rearm_arm "$home" "$state" "$fakebin" "$dir/stable-successor.out" is_live_non_zombie "$ARM_PID" || fail "malformed marker caused a persistent recovery loop" kill "$ARM_PID" 2>/dev/null || true - wait "$ARM_PID" 2>/dev/null || true + wait_for_exit "$ARM_PID" 80 || true pass "watch-arm: malformed recovery state is quarantined without a successor loop" } @@ -916,7 +916,7 @@ test_downtime_marker_does_not_follow_symlink() { printf 'must remain intact\n' > "$sentinel" ln -s "$sentinel" "$state/.watcher-down" kill -TERM "$watcher_pid" 2>/dev/null || fail "could not stop symlink fixture watcher" - wait "$ARM_PID" 2>/dev/null || true + wait_for_exit "$ARM_PID" 80 || true [ "$(cat "$sentinel")" = "must remain intact" ] \ || fail "downtime marker publication followed and truncated a symlink" diff --git a/tests/fm-watcher-lock.test.sh b/tests/fm-watcher-lock.test.sh index 198e937f2e..605d5c7c57 100755 --- a/tests/fm-watcher-lock.test.sh +++ b/tests/fm-watcher-lock.test.sh @@ -475,8 +475,8 @@ test_watch_restart_rejects_reused_pid() { pass "watch restart preserves recovery without signaling a reused pid" } -test_watch_restart_attaches_to_healthy_peer() { - local dir state fakebin out peer_ready peer identity armpid status i +test_watch_restart_force_stops_term_resistant_holder() { + local dir state fakebin out peer_ready peer identity armpid i dir=$(make_case restart-healthy-peer) state="$dir/state" fakebin="$dir/fakebin" @@ -503,24 +503,21 @@ test_watch_restart_attaches_to_healthy_peer() { printf '%s\n' "$identity" > "$state/.watch.lock/pid-identity" printf '%s\n' "$identity" > "$state/.watch.lock/beacon-identity" touch "$state/.last-watcher-beat" - PATH="$fakebin:$PATH" FM_HOME="$dir" FM_POLL=5 FM_SIGNAL_GRACE=1 FM_CHECK_INTERVAL=999999 FM_HEARTBEAT=999999 FM_ARM_ATTACH_POLL=0.1 FM_ARM_CONFIRM_TIMEOUT=1 "$WATCH_ARM" --restart > "$out" & + PATH="$fakebin:$PATH" FM_HOME="$dir" FM_POLL=5 FM_SIGNAL_GRACE=1 FM_CHECK_INTERVAL=999999 FM_HEARTBEAT=999999 FM_ARM_ATTACH_POLL=0.1 FM_ARM_CONFIRM_TIMEOUT=10 "$WATCH_ARM" --restart > "$out" & armpid=$! i=0 - while [ "$i" -lt 80 ]; do - grep -qF "watcher: attached pid=$peer" "$out" 2>/dev/null && break + while [ "$i" -lt 200 ]; do + grep -qF 'watcher: started pid=' "$out" 2>/dev/null && break sleep 0.1 i=$((i + 1)) done - grep -qF "watcher: attached pid=$peer" "$out" || fail "restart did not attach to the verified healthy peer: $(cat "$out")" - is_live_non_zombie "$armpid" || fail "restart arm exited instead of following the healthy peer" - is_live_non_zombie "$peer" || fail "restart killed a TERM-resistant peer unexpectedly" - kill -KILL "$peer" 2>/dev/null || true + grep -qF 'watcher: started pid=' "$out" || fail "restart did not establish a fresh watcher after bounded teardown: $(cat "$out")" + ! is_live_non_zombie "$peer" || fail "restart left the TERM-resistant recorded holder alive" wait "$peer" 2>/dev/null || true - wait_for_exit "$armpid" 80 - status=$? - [ "$status" -ne 0 ] && [ "$status" -ne 124 ] || fail "restart arm did not fail after its attached peer ended without a successor (status $status)" - grep -qF 'watcher: FAILED - cycle ended without an actionable reason' "$out" || fail "restart arm did not surface the attached cycle end" - pass "watch restart attaches to a verified healthy peer and later surfaces a successor gap" + is_live_non_zombie "$armpid" || fail "restart arm exited instead of following its fresh watcher" + kill -TERM "$armpid" 2>/dev/null || true + wait_for_exit "$armpid" 80 || true + pass "watch restart force-stops a TERM-resistant holder before starting fresh" } test_watcher_self_evicts_on_lock_takeover() { @@ -560,10 +557,10 @@ test_arm_self_eviction_is_loud_without_successor() { fakebin="$dir/fakebin" armout="$dir/arm.out" mark_pr_check_migration_complete "$state" - PATH="$fakebin:$PATH" FM_STATE_OVERRIDE="$state" FM_POLL=0.2 FM_SIGNAL_GRACE=1 FM_CHECK_INTERVAL=999999 FM_HEARTBEAT=999999 FM_ARM_CONFIRM_TIMEOUT=1 "$WATCH_ARM" > "$armout" & + PATH="$fakebin:$PATH" FM_STATE_OVERRIDE="$state" FM_POLL=0.2 FM_SIGNAL_GRACE=1 FM_CHECK_INTERVAL=999999 FM_HEARTBEAT=999999 FM_ARM_CONFIRM_TIMEOUT=10 "$WATCH_ARM" > "$armout" & armpid=$! i=0 - while [ "$i" -lt 80 ]; do + while [ "$i" -lt 150 ]; do grep -qF 'watcher: started pid=' "$armout" 2>/dev/null && break sleep 0.1 i=$((i + 1)) @@ -1141,7 +1138,7 @@ test_lock_empty_pid_uses_minimum_grace test_lock_late_claim_loses_after_recreate test_lock_paused_mid_acquire_claim_fails_during_steal test_watch_restart_rejects_reused_pid -test_watch_restart_attaches_to_healthy_peer +test_watch_restart_force_stops_term_resistant_holder test_watcher_self_evicts_on_lock_takeover test_arm_self_eviction_is_loud_without_successor test_arm_attaches_and_waits_for_live_fresh_watcher diff --git a/tests/wake-helpers.sh b/tests/wake-helpers.sh index 5964598c76..d386ef4d90 100644 --- a/tests/wake-helpers.sh +++ b/tests/wake-helpers.sh @@ -262,6 +262,12 @@ wait_for_exit() { i=$((i + 1)) done kill "$pid" 2>/dev/null || true + i=0 + while [ "$i" -lt 50 ] && is_live_non_zombie "$pid"; do + sleep 0.1 + i=$((i + 1)) + done + is_live_non_zombie "$pid" && kill -KILL "$pid" 2>/dev/null || true wait "$pid" 2>/dev/null || true return 124 } From b684387ee4a31bfdbc762db91ee995f3e0819bf8 Mon Sep 17 00:00:00 2001 From: Joseph Kim Date: Sun, 16 Aug 2026 06:12:38 -0700 Subject: [PATCH 14/19] no-mistakes(test): Fix stale lock reclamation and remote teardown completion --- bin/fm-teardown.sh | 8 +++-- bin/fm-wake-lib.sh | 61 +++++++++++++++++------------------ tests/fm-watcher-lock.test.sh | 41 ++++++++++++++--------- 3 files changed, 60 insertions(+), 50 deletions(-) diff --git a/bin/fm-teardown.sh b/bin/fm-teardown.sh index 64c68db5f9..da99d42973 100755 --- a/bin/fm-teardown.sh +++ b/bin/fm-teardown.sh @@ -2993,6 +2993,10 @@ fi echo "teardown $ID complete (window $T, worktree $WT)" # Capacity free: advisory refill so firstmate re-evaluates ready work. # Multiple teardowns before drain collapse to one refill record (dedupe by kind). -fm_wake_enqueue_refill || \ - echo "warning: could not enqueue fleet refill after teardown of $ID" >&2 +# A host-local remote-secondmate teardown removes the state directory it was +# launched against. Its parent teardown owns the refill after remote success. +if [ -d "$STATE" ]; then + fm_wake_enqueue_refill || \ + echo "warning: could not enqueue fleet refill after teardown of $ID" >&2 +fi backlog_refresh_reminder diff --git a/bin/fm-wake-lib.sh b/bin/fm-wake-lib.sh index 9095e7046d..e226699fcb 100755 --- a/bin/fm-wake-lib.sh +++ b/bin/fm-wake-lib.sh @@ -393,7 +393,7 @@ fm_lock_claim_blocked_by_steal() { } fm_lock_claim() { - local lockdir=$1 ownerdir=$2 allowed_steal_owner=${3:-} mypid back + local lockdir=$1 ownerdir=$2 allowed_steal_owner=${3:-} ignore_nested_steal=${4:-false} mypid back mypid=${BASHPID:-$$} if ! { printf '%s\n' "$mypid" > "$ownerdir/pid"; } 2>/dev/null; then fm_lock_discard_owner "$ownerdir" @@ -408,7 +408,8 @@ fm_lock_claim() { fm_lock_discard_owner "$ownerdir" return 1 fi - if fm_lock_claim_blocked_by_steal "$lockdir" "$allowed_steal_owner"; then + if [ "$ignore_nested_steal" != true ] \ + && fm_lock_claim_blocked_by_steal "$lockdir" "$allowed_steal_owner"; then if fm_lock_points_to_owner "$lockdir" "$ownerdir"; then rm -f "$lockdir" 2>/dev/null || true fi @@ -419,7 +420,7 @@ fm_lock_claim() { } fm_lock_try_create() { - local lockdir=$1 allowed_steal_owner=${2:-} ownerdir + local lockdir=$1 allowed_steal_owner=${2:-} ignore_nested_steal=${3:-false} ownerdir FM_LOCK_OWNER_DIR= ownerdir=$(fm_lock_owner_dir "$lockdir") || return 1 if [ -e "$lockdir" ] || [ -L "$lockdir" ]; then @@ -431,7 +432,7 @@ fm_lock_try_create() { return 1 fi if ln -s "$ownerdir" "$lockdir" 2>/dev/null && fm_lock_points_to_owner "$lockdir" "$ownerdir"; then - if fm_lock_claim "$lockdir" "$ownerdir" "$allowed_steal_owner"; then + if fm_lock_claim "$lockdir" "$ownerdir" "$allowed_steal_owner" "$ignore_nested_steal"; then FM_LOCK_OWNER_DIR=$ownerdir return 0 fi @@ -738,8 +739,27 @@ fm_recovery_marker_arm_check() { fm_recovery_transition "$1" arm-check } +fm_lock_try_acquire_steal() { + local lockdir=$1 pid owner= + + if fm_lock_try_create "$lockdir"; then + return 0 + fi + + pid=$(cat "$lockdir/pid" 2>/dev/null || true) + if fm_pid_alive "$pid" || fm_lock_mid_acquire_is_fresh "$lockdir" "$pid"; then + return 1 + fi + if [ -L "$lockdir" ]; then + owner=$(fm_lock_link_owner "$lockdir" 2>/dev/null || true) + fi + fm_lock_recheck_stale_owner "$lockdir" "$owner" "$pid" || return 1 + fm_lock_remove_path "$lockdir" || true + fm_lock_try_create "$lockdir" '' true +} + fm_lock_try_acquire() { - local lockdir=$1 pid steal cur rc steal_pid steal_owner primary_owner + local lockdir=$1 pid steal cur rc steal_owner primary_owner FM_LOCK_HELD_PID= FM_LOCK_OWNER_DIR= FM_LOCK_RECOVERED_PID= @@ -758,34 +778,11 @@ fm_lock_try_acquire() { return 1 fi - # A steal mutex is the terminal serialization layer. Never recurse into a - # .steal.steal chain when a caller encounters a stale mutex directly. - case "$lockdir" in - *.steal) - FM_LOCK_HELD_PID=$pid - return 1 - ;; - esac - steal="$lockdir.steal" - if ! fm_lock_try_create "$steal"; then - steal_pid=$(cat "$steal/pid" 2>/dev/null || true) - if fm_pid_alive "$steal_pid" || fm_lock_mid_acquire_is_fresh "$steal" "$steal_pid"; then - FM_LOCK_HELD_PID=$(cat "$lockdir/pid" 2>/dev/null || true) - FM_LOCK_OWNER_DIR= - return 1 - fi - steal_owner= - if [ -L "$steal" ]; then - steal_owner=$(fm_lock_link_owner "$steal" 2>/dev/null || true) - fi - if ! fm_lock_recheck_stale_owner "$steal" "$steal_owner" "$steal_pid" \ - || ! fm_lock_remove_path "$steal" \ - || ! fm_lock_try_create "$steal"; then - FM_LOCK_HELD_PID=$(cat "$lockdir/pid" 2>/dev/null || true) - FM_LOCK_OWNER_DIR= - return 1 - fi + if ! fm_lock_try_acquire_steal "$steal"; then + FM_LOCK_HELD_PID=$(cat "$lockdir/pid" 2>/dev/null || true) + FM_LOCK_OWNER_DIR= + return 1 fi steal_owner=${FM_LOCK_OWNER_DIR:-} diff --git a/tests/fm-watcher-lock.test.sh b/tests/fm-watcher-lock.test.sh index 605d5c7c57..a52cea2a07 100755 --- a/tests/fm-watcher-lock.test.sh +++ b/tests/fm-watcher-lock.test.sh @@ -311,26 +311,35 @@ test_lock_live_steal_mutex_is_not_reclaimed() { pass "live steal mutex is not reclaimed" } -test_lock_dead_steal_mutex_is_reclaimed_without_recursion() { - local dir state lockdir dead rc newpid - dir=$(make_case lock-dead-stealer) +test_lock_stale_steal_mutex_is_reclaimed_without_recursion() { + local dir state lockdir dead out stale i + dir=$(make_case lock-stale-stealer) state="$dir/state" lockdir="$state/.contend.lock" dead=$(dead_pid) - mkdir "$lockdir" "$lockdir.steal" + mkdir "$lockdir" printf '%s\n' "$dead" > "$lockdir/pid" - printf '%s\n' "$dead" > "$lockdir.steal/pid" - rc=0 - newpid=$(FM_LOCK_STALE_AFTER=0 FM_STATE_OVERRIDE="$state" bash -c ' + touch -t 200001010000 "$lockdir" + stale=$lockdir + i=0 + while [ "$i" -lt 40 ]; do + stale="$stale.steal" + mkdir "$stale" + printf '%s\n' "$dead" > "$stale/pid" + touch -t 200001010000 "$stale" + i=$((i + 1)) + done + + out=$(FM_LOCK_STALE_AFTER=0 FM_STATE_OVERRIDE="$state" bash -c ' . "$1" - if fm_lock_try_acquire "$2"; then cat "$2/pid"; else exit 7; fi - ' _ "$LIB" "$lockdir") || rc=$? - [ "$rc" -eq 0 ] || fail "acquirer failed to reclaim a dead steal mutex (rc=$rc)" - [ -n "$newpid" ] && [ "$newpid" != "$dead" ] \ - || fail "reclaimed lock did not record a new owner" - [ ! -e "$lockdir.steal.steal" ] && [ ! -L "$lockdir.steal.steal" ] \ - || fail "dead steal mutex reclamation created a recursive steal chain" - pass "dead steal mutex is reclaimed without recursive steal chains" + fm_lock_try_acquire "$2" || exit 7 + printf "lockpid=%s steal_exists=%s\n" "$(cat "$2/pid")" "$([ -e "$2.steal" ] || [ -L "$2.steal" ] && echo yes || echo no)" + ' _ "$LIB" "$lockdir") || fail "stale steal mutex prevented stale primary lock recovery" + case "$out" in + *"lockpid="*" steal_exists=no"*) ;; + *) fail "stale steal mutex recovery left invalid lock state: $out" ;; + esac + pass "stale steal mutex is reclaimed without recursive lock paths" } test_lock_does_not_steal_live_lock() { @@ -1132,7 +1141,7 @@ test_lock_single_winner_under_concurrency test_lock_steals_dead_pid_lock test_lock_stale_steal_single_winner_under_concurrency test_lock_live_steal_mutex_is_not_reclaimed -test_lock_dead_steal_mutex_is_reclaimed_without_recursion +test_lock_stale_steal_mutex_is_reclaimed_without_recursion test_lock_does_not_steal_live_lock test_lock_empty_pid_uses_minimum_grace test_lock_late_claim_loses_after_recreate From b3272370ae539be4c0e9d2f16e813043de74b7c6 Mon Sep 17 00:00:00 2001 From: Joseph Kim Date: Sun, 16 Aug 2026 07:04:45 -0700 Subject: [PATCH 15/19] no-mistakes(document): Document generation-bound watcher health --- docs/configuration.md | 2 +- docs/turnend-guard.md | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/configuration.md b/docs/configuration.md index 6449ddfee4..5d38fff7af 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -576,7 +576,7 @@ FM_CLAUDE_AUTOARM_ATTEMPTS=2 # bounded Stop-owned arm attempts per Claude auto FM_CLAUDE_AUTOARM_SYNC_WAIT_MS=800 # milliseconds the --claude turn-end guard waits for watcher health, a role-verified Stop auto-arm claim, or a fresh epoch before deciding recovery ownership or failure progression 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_CONFIRM_TIMEOUT=10 # seconds fm-watch-arm waits to confirm a newly launched watcher's fresh generation-bound beacon or attach to another live identity-matched holder before reporting FAILED; default 30 on Git Bash/MSYS FM_ARM_ATTACH_POLL=0.5 # seconds between checks while fm-watch-arm follows an existing live identity-matched watcher cycle 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 diff --git a/docs/turnend-guard.md b/docs/turnend-guard.md index 26a6333e45..24f927da39 100644 --- a/docs/turnend-guard.md +++ b/docs/turnend-guard.md @@ -31,7 +31,7 @@ Registered `state/procevent/*.source` records also require supervision even thou An unread record in `state/.wake-queue` also requires supervision even when no task, process-event source, or Relay poll remains. The default cross-harness mode exits silently with no supervision need. Every mode treats `state/x-watch.check.sh` as supervision need, so Relay polling remains guarded without an in-flight task. -Otherwise it calls `fm_watcher_healthy [grace-seconds] [home]` from `bin/fm-wake-lib.sh`, the same PID-strict identity-matched lock and fresh-beacon check used by `bin/fm-watch-arm.sh`: a stale beacon blocks even when a watcher pid is live, and a fresh leftover beacon blocks when the lock is missing, dead, or identity-mismatched. +Otherwise it calls `fm_watcher_healthy [grace-seconds] [home]` from `bin/fm-wake-lib.sh`, the same PID-strict identity-matched lock and generation-bound fresh-beacon check used to confirm a newly launched watcher in `bin/fm-watch-arm.sh`: a stale beacon blocks even when a watcher pid is live, and a fresh leftover beacon blocks when it was not published by the current lock holder or when the lock is missing, dead, or identity-mismatched. The turn-end guard needs that strict check because it fires at the turn boundary, where the auto-arm is bringing a fresh watcher up for the upcoming idle period, and it cooperates with that arm rather than trusting a beacon left by the cycle that just ended. `bin/fm-guard.sh`, the pull warning, instead uses the model-aware `fm_watcher_supervision_verdict` from the same library, because it fires mid-turn when the auto-arm model runs no watcher at all. Under the Claude Stop auto-arm model a beacon fresh within grace is healthy even with no live watcher process, and only a beacon stale beyond grace (or absent) alarms. From 06fd118f2019cbe2284106a3832fffd00124c970 Mon Sep 17 00:00:00 2001 From: Joseph Kim Date: Sun, 16 Aug 2026 08:02:08 -0700 Subject: [PATCH 16/19] no-mistakes: apply CI fixes --- bin/fm-watch-arm.sh | 72 ++++++++++++++++++++++++++++++----- tests/fm-watcher-lock.test.sh | 23 +++++------ 2 files changed, 74 insertions(+), 21 deletions(-) diff --git a/bin/fm-watch-arm.sh b/bin/fm-watch-arm.sh index afee34c988..4d7a0bf404 100755 --- a/bin/fm-watch-arm.sh +++ b/bin/fm-watch-arm.sh @@ -228,14 +228,19 @@ cycle_mark_predecessor_successor() { } clear_stale_recorded_watcher_lock() { - local lock_home lock_path lock_identity + local disposition=${1:-recover} expected_identity=${2:-} lock_home lock_path lock_identity 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) [ "$lock_home" = "$FM_HOME" ] || return 0 [ "$lock_path" = "$WATCH" ] || return 0 [ -n "$lock_identity" ] || return 0 - fm_recovery_transition "$STATE/.watcher-down" clear-stale-lock "$WATCH_LOCK" downtime + [ -z "$expected_identity" ] || [ "$lock_identity" = "$expected_identity" ] || return 0 + case "$disposition" in + recover) fm_recovery_transition "$STATE/.watcher-down" clear-stale-lock "$WATCH_LOCK" downtime ;; + restart) fm_lock_remove_path "$WATCH_LOCK" ;; + *) return 2 ;; + esac } # A watcher is "healthy" iff the lock names a live process that is genuinely THIS @@ -258,7 +263,7 @@ healthy_watcher() { # mid-poll iteration can starve the beacon without ending the cycle; attach paths # must follow that holder rather than treating it as cycle-end or starting a # second watcher. Sets HEALTHY_PID/HEALTHY_IDENTITY on success. -live_watcher_holder() { +identity_matched_watcher_holder() { local pid identity HEALTHY_PID= HEALTHY_IDENTITY= @@ -266,12 +271,16 @@ live_watcher_holder() { fm_pid_alive "$pid" || return 1 fm_watcher_lock_matches_pid "$STATE" "$WATCH" "$pid" "$FM_HOME" || return 1 identity=$FM_WATCHER_MATCHED_IDENTITY - fm_watcher_generation_beaconed "$STATE" "$identity" || return 1 HEALTHY_PID=$pid HEALTHY_IDENTITY=$identity return 0 } +live_watcher_holder() { + identity_matched_watcher_holder || return 1 + fm_watcher_generation_beaconed "$STATE" "$HEALTHY_IDENTITY" +} + report_attached() { local age age=$(fm_path_age "$BEAT") @@ -424,19 +433,27 @@ handling_successor_generation() { esac } -stop_pid_bounded() { # [tenths-of-a-second] - local pid=$1 limit=${2:-50} i=0 +stop_pid_bounded() { # [tenths-of-a-second] [expected-identity] + local pid=$1 limit=${2:-50} expected_identity=${3:-} current_identity i=0 [ -n "$pid" ] || return 0 - fm_pid_alive "$pid" || return 0 + current_identity=$(fm_pid_identity "$pid" 2>/dev/null || true) + [ -n "$current_identity" ] || return 0 + [ -z "$expected_identity" ] && expected_identity=$current_identity + [ "$current_identity" = "$expected_identity" ] || return 0 kill -TERM "$pid" 2>/dev/null || true - while [ "$i" -lt "$limit" ] && fm_pid_alive "$pid"; do + while [ "$i" -lt "$limit" ]; do + current_identity=$(fm_pid_identity "$pid" 2>/dev/null || true) + [ "$current_identity" = "$expected_identity" ] || return 0 sleep 0.1 i=$((i + 1)) done - if fm_pid_alive "$pid"; then + current_identity=$(fm_pid_identity "$pid" 2>/dev/null || true) + if [ "$current_identity" = "$expected_identity" ]; then kill -KILL "$pid" 2>/dev/null || true fi wait "$pid" 2>/dev/null || true + current_identity=$(fm_pid_identity "$pid" 2>/dev/null || true) + [ "$current_identity" != "$expected_identity" ] } mode=arm @@ -469,7 +486,18 @@ if [ "$mode" = restart ]; then lock_pid=$(cat "$WATCH_LOCK/pid" 2>/dev/null || true) if fm_pid_alive "$lock_pid"; then if fm_watcher_lock_matches_pid "$STATE" "$WATCH" "$lock_pid" "$FM_HOME"; then - stop_pid_bounded "$lock_pid" + lock_identity=$FM_WATCHER_MATCHED_IDENTITY + if ! stop_pid_bounded "$lock_pid" 50 "$lock_identity"; then + echo "watcher: FAILED - recorded watcher did not stop within the bounded teardown" >&2 + exit 1 + fi + # A TERM-resistant or externally supplied holder cannot run the watcher's + # EXIT cleanup. Remove only the exact generation we just stopped. This is + # an attended restart, so the replacement arm itself closes the gap. + if ! clear_stale_recorded_watcher_lock restart "$lock_identity"; then + echo "watcher: FAILED - stopped watcher lock could not be cleared" >&2 + exit 1 + fi else if ! clear_stale_recorded_watcher_lock; then echo "watcher: FAILED - stale watcher recovery state could not be persisted" >&2 @@ -493,6 +521,30 @@ if [ "$mode" = arm ]; then attach_and_wait "$HEALTHY_PID" exit $? fi + # A peer can publish its identity-bound lock just before its first beacon. + # Do not launch a competing child into that startup window. Wait directly for + # this generation's proof, and fail closed if the same holder never emits it. + if identity_matched_watcher_holder; then + peer_pid=$HEALTHY_PID + peer_identity=$HEALTHY_IDENTITY + deadline=$(( $(date +%s) + CONFIRM_TIMEOUT + 1 )) + while identity_matched_watcher_holder \ + && [ "$HEALTHY_PID" = "$peer_pid" ] \ + && [ "$HEALTHY_IDENTITY" = "$peer_identity" ]; do + if fm_watcher_generation_beaconed "$STATE" "$peer_identity"; then + cycle_mark_predecessor_successor "attached:$peer_pid" + cycle_begin "$peer_pid" attached "$peer_identity" + report_attached + attach_and_wait "$peer_pid" + exit $? + fi + if [ "$(date +%s)" -ge "$deadline" ]; then + echo "watcher: FAILED - no live watcher with a fresh beacon" + exit 1 + fi + sleep 0.2 + done + fi fi # Start a watcher as a tracked child and confirm it before settling in. The child diff --git a/tests/fm-watcher-lock.test.sh b/tests/fm-watcher-lock.test.sh index a52cea2a07..09757c9147 100755 --- a/tests/fm-watcher-lock.test.sh +++ b/tests/fm-watcher-lock.test.sh @@ -781,7 +781,7 @@ SH pass "arm propagates an immediate watcher wake before confirmation" } -test_arm_waits_for_peer_beacon_after_child_stands_down() { +test_arm_waits_for_peer_beacon_without_starting_child() { local dir state fakebin armout peer identity armpid status i dir=$(make_case arm-peer-startup-race) state="$dir/state" @@ -798,18 +798,17 @@ test_arm_waits_for_peer_beacon_after_child_stands_down() { printf '%s\n' "$identity" > "$state/.watch.lock/pid-identity" PATH="$fakebin:$PATH" FM_HOME="$dir" FM_POLL=5 FM_SIGNAL_GRACE=1 FM_CHECK_INTERVAL=999999 FM_HEARTBEAT=999999 FM_ARM_CONFIRM_TIMEOUT=1 FM_ARM_ATTACH_POLL=0.1 "$WATCH_ARM" > "$armout" & armpid=$! - # Synchronize on the owned child declining the live peer lock before making - # the peer healthy. Sleeping for the same one-second budget as the arm made - # this regression fixture race the confirmation deadline under full-suite - # load, rather than testing the intended successor-handshake boundary. + # Give the arm time to observe the identity-matched peer before publishing + # that generation's first beacon. It must wait directly on the peer instead + # of starting a child that competes for the already-held singleton. i=0 - while [ "$i" -lt 80 ]; do - grep -qF "watcher: already running pid $peer" "$state"/.watch-arm-output.* 2>/dev/null && break + while [ "$i" -lt 5 ]; do + is_live_non_zombie "$armpid" || fail "arm exited before the peer published its first beacon" sleep 0.1 i=$((i + 1)) done - grep -qF "watcher: already running pid $peer" "$state"/.watch-arm-output.* 2>/dev/null \ - || fail "arm child did not stand down behind the peer watcher" + ! ls "$state"/.watch-arm-output.* >/dev/null 2>&1 \ + || fail "arm started a competing child while an identity-matched peer held the singleton" touch "$state/.last-watcher-beat" printf '%s\n' "$identity" > "$state/.watch.lock/beacon-identity" i=0 @@ -819,6 +818,8 @@ test_arm_waits_for_peer_beacon_after_child_stands_down() { i=$((i + 1)) done grep -qF "watcher: attached pid=$peer" "$armout" || fail "arm did not wait for and attach to the peer watcher: $(cat "$armout")" + ! ls "$state"/.watch-arm-output.* >/dev/null 2>&1 \ + || fail "peer attachment left evidence of a competing child watcher" ! grep -qF 'watcher: FAILED' "$armout" || fail "arm falsely reported FAILED during peer startup race" is_live_non_zombie "$armpid" || fail "arm exited while the peer was still healthy" # After the peer dies without a successor, the attached arm must fail loudly. @@ -828,7 +829,7 @@ test_arm_waits_for_peer_beacon_after_child_stands_down() { status=$? [ "$status" -ne 0 ] && [ "$status" -ne 124 ] || fail "attached arm did not fail after peer died (status $status): $(cat "$armout")" grep -qF 'watcher: FAILED - cycle ended without an actionable reason' "$armout" || fail "peer-attached arm did not emit the typed cycle-end failure" - pass "arm attaches to a peer watcher after child stands down and surfaces a missing successor" + pass "arm waits for a peer's first beacon without starting a child and surfaces a missing successor" } test_arm_fails_loud_when_no_fresh_watcher_confirmable() { @@ -1155,7 +1156,7 @@ test_attached_arm_signal_is_recorded_in_cycle_ledger test_arm_starts_and_self_heals test_arm_hup_cleans_child_and_temp_output test_arm_propagates_immediate_wake_before_confirmation -test_arm_waits_for_peer_beacon_after_child_stands_down +test_arm_waits_for_peer_beacon_without_starting_child 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 From 528dc3c77bfbddc28ada5c210001113151f59fb7 Mon Sep 17 00:00:00 2001 From: Joseph Kim Date: Sun, 16 Aug 2026 08:36:58 -0700 Subject: [PATCH 17/19] no-mistakes: apply CI fixes --- tests/fm-watcher-lock.test.sh | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/tests/fm-watcher-lock.test.sh b/tests/fm-watcher-lock.test.sh index 09757c9147..c0a4b65872 100755 --- a/tests/fm-watcher-lock.test.sh +++ b/tests/fm-watcher-lock.test.sh @@ -566,7 +566,11 @@ test_arm_self_eviction_is_loud_without_successor() { fakebin="$dir/fakebin" armout="$dir/arm.out" mark_pr_check_migration_complete "$state" - PATH="$fakebin:$PATH" FM_STATE_OVERRIDE="$state" FM_POLL=0.2 FM_SIGNAL_GRACE=1 FM_CHECK_INTERVAL=999999 FM_HEARTBEAT=999999 FM_ARM_CONFIRM_TIMEOUT=10 "$WATCH_ARM" > "$armout" & + # This case has deliberately installed an identity-mismatched lock holder, so + # only a short successor-confirmation window is needed. Keep it below the + # wait_for_exit budget so Linux and macOS test the typed failure rather than + # racing the fixture's own timeout. + PATH="$fakebin:$PATH" FM_STATE_OVERRIDE="$state" FM_POLL=0.2 FM_SIGNAL_GRACE=1 FM_CHECK_INTERVAL=999999 FM_HEARTBEAT=999999 FM_ARM_CONFIRM_TIMEOUT=1 "$WATCH_ARM" > "$armout" & armpid=$! i=0 while [ "$i" -lt 150 ]; do From dd1928059a27c36799fd8825c8d7d26a089a8c8e Mon Sep 17 00:00:00 2001 From: Joseph Kim Date: Sun, 16 Aug 2026 17:18:13 -0700 Subject: [PATCH 18/19] fix(supervision): reap immediate watcher wakes --- bin/fm-watch-arm.sh | 10 ++++++++++ tests/fm-watcher-lock.test.sh | 15 +++++++-------- 2 files changed, 17 insertions(+), 8 deletions(-) diff --git a/bin/fm-watch-arm.sh b/bin/fm-watch-arm.sh index 4d7a0bf404..267445f0c4 100755 --- a/bin/fm-watch-arm.sh +++ b/bin/fm-watch-arm.sh @@ -664,6 +664,16 @@ owned_child_finished() { # collapsing when startup begins just before the next second boundary. deadline=$(( $(date +%s) + CONFIRM_TIMEOUT + 1 )) while :; do + # An owned child can publish an actionable wake and exit before its first + # beacon. Reap it from that durable output event instead of relying on + # kill -0, which still reports an unreaped exited child as present. + if watch_output_has_wake "$child_out"; then + wait "$child" + rc=$? + child_done=1 + owned_child_finished "$rc" + exit $? + fi if healthy_watcher; then if [ "$HEALTHY_PID" = "$child" ]; then cycle_refresh_lock_before diff --git a/tests/fm-watcher-lock.test.sh b/tests/fm-watcher-lock.test.sh index c0a4b65872..7c8c72050d 100755 --- a/tests/fm-watcher-lock.test.sh +++ b/tests/fm-watcher-lock.test.sh @@ -566,11 +566,10 @@ test_arm_self_eviction_is_loud_without_successor() { fakebin="$dir/fakebin" armout="$dir/arm.out" mark_pr_check_migration_complete "$state" - # This case has deliberately installed an identity-mismatched lock holder, so - # only a short successor-confirmation window is needed. Keep it below the - # wait_for_exit budget so Linux and macOS test the typed failure rather than - # racing the fixture's own timeout. - PATH="$fakebin:$PATH" FM_STATE_OVERRIDE="$state" FM_POLL=0.2 FM_SIGNAL_GRACE=1 FM_CHECK_INTERVAL=999999 FM_HEARTBEAT=999999 FM_ARM_CONFIRM_TIMEOUT=1 "$WATCH_ARM" > "$armout" & + # Leave enough startup budget for the owned child to publish its identity- + # bound beacon under CI load. The outer wait remains bounded beyond the + # successor-confirmation window so this case observes the typed failure. + PATH="$fakebin:$PATH" FM_STATE_OVERRIDE="$state" FM_POLL=0.2 FM_SIGNAL_GRACE=1 FM_CHECK_INTERVAL=999999 FM_HEARTBEAT=999999 FM_ARM_CONFIRM_TIMEOUT=10 "$WATCH_ARM" > "$armout" & armpid=$! i=0 while [ "$i" -lt 150 ]; do @@ -585,7 +584,7 @@ test_arm_self_eviction_is_loud_without_successor() { # self-evict normally. With no verified successor, the arm must turn that # otherwise clean empty close into the typed nonzero failure. printf '%s\n' "$$" > "$state/.watch.lock/pid" - wait_for_exit "$armpid" 80 + wait_for_exit "$armpid" 200 status=$? [ "$status" -ne 0 ] && [ "$status" -ne 124 ] || fail "self-evicted arm did not fail nonzero (status $status)" grep -qF 'watcher: FAILED - cycle ended without an actionable reason' "$armout" || fail "self-evicted arm omitted the typed cycle-end failure" @@ -894,7 +893,7 @@ SH PATH="$fakebin:$PATH" FM_STATE_OVERRIDE="$state" FM_WATCH_PREDECESSOR_ARM_PID="$first_arm" FM_POLL=5 FM_SIGNAL_GRACE=1 FM_CHECK_INTERVAL=999999 FM_HEARTBEAT=999999 "$WATCH_ARM" > "$armout" & successor_arm=$! i=0 - while [ "$i" -lt 80 ]; do + while [ "$i" -lt 200 ]; do grep -qF 'watcher: started pid=' "$armout" 2>/dev/null && break sleep 0.1 i=$((i + 1)) @@ -919,7 +918,7 @@ SH PATH="$fakebin:$PATH" FM_STATE_OVERRIDE="$state" FM_WATCH_CYCLE_LOG_MAX_BYTES=1400 FM_WATCH_CYCLE_LOG_KEEP_LINES=2 FM_POLL=5 FM_SIGNAL_GRACE=1 FM_CHECK_INTERVAL=999999 FM_HEARTBEAT=999999 "$WATCH_ARM" > "$armout" & successor_arm=$! i=0 - while [ "$i" -lt 80 ]; do + while [ "$i" -lt 200 ]; do grep -qF 'watcher: started pid=' "$armout" 2>/dev/null && break sleep 0.1 i=$((i + 1)) From 0a5bd2e1d5ebd296b3ea6f127f7f302641008eaa Mon Sep 17 00:00:00 2001 From: Joseph Kim Date: Sun, 16 Aug 2026 17:49:05 -0700 Subject: [PATCH 19/19] fix(ci): bound watcher lifecycle teardown --- bin/fm-watch-arm.sh | 10 ++++++++++ tests/fm-watcher-lock.test.sh | 10 +++++----- 2 files changed, 15 insertions(+), 5 deletions(-) diff --git a/bin/fm-watch-arm.sh b/bin/fm-watch-arm.sh index 267445f0c4..a6d1901666 100755 --- a/bin/fm-watch-arm.sh +++ b/bin/fm-watch-arm.sh @@ -732,6 +732,16 @@ while :; do sleep 0.2 done +# Close the deadline-boundary race where the child publishes its wake after the +# loop's last observation but before timeout cleanup begins. +if watch_output_has_wake "$child_out"; then + wait "$child" + rc=$? + child_done=1 + owned_child_finished "$rc" + exit $? +fi + trap - HUP TERM INT # Confirmation budget exhausted. Prefer attaching to a different live holder # with a starved beacon over a false FAILED. Our own child still requires the diff --git a/tests/fm-watcher-lock.test.sh b/tests/fm-watcher-lock.test.sh index 7c8c72050d..5938fdb286 100755 --- a/tests/fm-watcher-lock.test.sh +++ b/tests/fm-watcher-lock.test.sh @@ -626,7 +626,7 @@ test_arm_attaches_and_waits_for_live_fresh_watcher() { is_live_non_zombie "$armpid" || fail "arm exited while the seed watcher was still healthy" # After the seed dies without a successor, the attached arm must fail loudly. kill "$wpid" 2>/dev/null || true - wait "$wpid" 2>/dev/null || true + wait_for_exit "$wpid" 80 || true wait_for_exit "$armpid" 80 status=$? [ "$status" -ne 0 ] && [ "$status" -ne 124 ] || fail "attached arm did not fail after seed died (status $status)" @@ -667,7 +667,7 @@ test_attached_arm_signal_is_recorded_in_cycle_ledger() { || fail "attached arm signal was not recorded in the lifecycle ledger" is_live_non_zombie "$wpid" || fail "signaling an attached arm terminated the peer watcher" kill "$wpid" 2>/dev/null || true - wait "$wpid" 2>/dev/null || true + wait_for_exit "$wpid" 80 || true pass "attached arm signals record a classified lifecycle entry" } @@ -743,7 +743,7 @@ test_arm_hup_cleans_child_and_temp_output() { grep -qF 'watcher: started pid=' "$armout" || fail "arm did not start before HUP cleanup check" lock_pid=$(cat "$state/.watch.lock/pid" 2>/dev/null || true) kill -HUP "$armpid" 2>/dev/null || fail "could not send HUP to arm" - wait_for_exit "$armpid" 80 + wait_for_exit "$armpid" 200 status=$? [ "$status" -eq 129 ] || fail "arm did not exit with HUP status (got $status)" i=0 @@ -903,7 +903,7 @@ SH grep -q "arm_pid=$first_arm.*successor=started:$successor_pid" "$state/.watch-cycle-exits.log" \ || fail "predecessor ledger record was not linked to its verified successor" kill -HUP "$successor_arm" 2>/dev/null || true - wait "$successor_arm" 2>/dev/null || true + wait_for_exit "$successor_arm" 80 || true # The forced interruption is a watcher-down interval. Consume the prior # delivered wake before beginning independent ledger cycles, just as the # recovery handling turn does, so this fixture does not intentionally carry a @@ -925,7 +925,7 @@ SH done grep -qF 'watcher: started pid=' "$armout" || fail "bounded ledger cycle $iteration did not start" kill -HUP "$successor_arm" 2>/dev/null || true - wait "$successor_arm" 2>/dev/null || true + wait_for_exit "$successor_arm" 80 || true drain_and_ack "$state" \ || fail "recovery drain after bounded ledger cycle $iteration failed" iteration=$((iteration + 1))