From 51532884be3c6e9ad6a20628d905d638aae9c29d Mon Sep 17 00:00:00 2001 From: Marcus Moreira Date: Wed, 23 Sep 2026 11:19:19 -0300 Subject: [PATCH 01/19] fix(bin): bound recovery-episode reopen so a stuck watcher can stay up An announced-but-unacked recovery episode (state/.watcher-down) reopens into a brand new generation on every plain restart with no session or re-arm loop to run the printed acknowledgement, so each restart resurfaces "check: rearm-resurface" and exits again - the watcher never holds the lock or lives past its first beacon. Bound consecutive reopens of one stuck episode with FM_RECOVERY_REOPEN_LIMIT (default 3): past the limit, settle the episode to acked directly instead of minting another generation, so the next restart proceeds into its normal poll loop and stays armed. A real acknowledgement or a fresh downtime episode both clear the counter, so this never shortens the once-per-genuine-generation resurface a live session still relies on. --- .../skills/operational-home-layout/SKILL.md | 1 + bin/fm-wake-lib.sh | 52 ++++++++++++++++--- docs/watcher-continuity.md | 9 ++++ tests/fm-watch-arm.test.sh | 49 +++++++++++++++++ 4 files changed, 105 insertions(+), 6 deletions(-) diff --git a/.agents/skills/operational-home-layout/SKILL.md b/.agents/skills/operational-home-layout/SKILL.md index 8e45221e53d..2597641868f 100644 --- a/.agents/skills/operational-home-layout/SKILL.md +++ b/.agents/skills/operational-home-layout/SKILL.md @@ -102,6 +102,7 @@ state/ runtime records and signals; gitignored .startup-network.* status, report, per-step elapsed timings, inline-print claim, and lock for the deferred startup stage that runs network checks and the inactive-outcome scan off the digest's blocking path; bin/fm-startup-network.sh .wake-queue durable queued wakes retained until post-handling acknowledgement: epochseqkindkeypayload .watcher-down private generation-bound recovery state coupling watcher downtime, durable wake presentation, and post-handling acknowledgement; never touch + .watcher-down.reopen-count private per-episode reopen counter bounding how many unacknowledged plain restarts reopen the same stuck episode before it settles (docs/watcher-continuity.md "Recovery episode acknowledgement"); never touch ..open-decisions-cursor per-task byte cursor and folded open-decision set bounding the OPEN DECISIONS scan's cost to new status-log appends; written only by fm-classify-lib.sh's status_open_decisions_incremental, removed by teardown, safe to delete (forces one full re-fold) ..home-appends per-task ledger of byte ranges this home itself appended as bookkeeping closes, so a wake scan can tell its own growth from a foreign write instead of waking on it; presentation is unaffected, so both the signal annotation and UNREAD STATUS still print those lines; written only by fm-classify-lib.sh's status_home_appends_record; its sibling ..home-appends.lock serializes that ledger's read-merge-write; both removed by teardown, safe to delete .status-presentation-cursor .status-presentation-lock fleet-wide per-task status identity plus independent annotation and outcome-backstop byte offsets, with a serialization lock preventing already-presented lines from replaying while preserving delayed signal annotations; owned by fm-classify-lib.sh, with each task's row retired by teardown diff --git a/bin/fm-wake-lib.sh b/bin/fm-wake-lib.sh index 60a9d289090..7dffa605a78 100755 --- a/bin/fm-wake-lib.sh +++ b/bin/fm-wake-lib.sh @@ -865,6 +865,10 @@ _fm_recovery_marker_ack() { fm_lock_release "$lock" return 1 fi + # A genuine acknowledgement proves this episode was actually seen, so a + # later down stretch's reopen starts with a fresh FM_RECOVERY_REOPEN_LIMIT + # budget rather than inheriting this one's count. + rm -f -- "${marker}.reopen-count" 2>/dev/null || true fm_lock_release "$lock" } @@ -944,9 +948,24 @@ _fm_recovery_marker_arm_check() { # Apply the owner-documented announced-episode arm transition atomically with # the queue read. Handling successors must not call this transition. +# +# Nothing else ever retires that generation when no live session runs the +# printed acknowledgement, so a plain restart with no re-arm loop and no +# session reopens the SAME stuck episode into a fresh generation every single +# time, forever: each generation is used for exactly one resurface and then +# abandoned still announced, and the very next restart reopens it again. Bound +# that: past FM_RECOVERY_REOPEN_LIMIT consecutive reopens of one episode with +# no intervening explicit acknowledgement, settle it to acked directly instead +# of minting yet another generation nobody is watching, so a watcher can start +# and stay up. A real acknowledgement (_fm_recovery_marker_ack) or a fresh +# downtime episode both clear the counter, so this bound never shortens the +# once-per-genuine-generation resurface a live, attentive session relies on. +FM_RECOVERY_REOPEN_LIMIT=${FM_RECOVERY_REOPEN_LIMIT:-3} + _fm_recovery_marker_reopen_announced() { - local marker=$1 lock + local marker=$1 lock counter_file count generation lock="${marker}.lock" + counter_file="${marker}.reopen-count" fm_lock_acquire_wait "$FM_WAKE_QUEUE_LOCK" || return 1 if ! fm_lock_acquire_wait "$lock"; then fm_lock_release "$FM_WAKE_QUEUE_LOCK" @@ -959,11 +978,32 @@ _fm_recovery_marker_reopen_announced() { fi case "$FM_RECOVERY_MARKER_TOKEN" in announced:*) - if [ -s "$FM_WAKE_QUEUE" ] \ - && ! _fm_recovery_marker_write_locked "$marker" downtime ""; then - fm_lock_release "$lock" - fm_lock_release "$FM_WAKE_QUEUE_LOCK" - return 1 + if [ -s "$FM_WAKE_QUEUE" ]; then + count=$(cat "$counter_file" 2>/dev/null || true) + case "$count" in ''|*[!0-9]*) count=0 ;; esac + count=$((count + 1)) + if [ "$count" -gt "$FM_RECOVERY_REOPEN_LIMIT" ]; then + generation=${FM_RECOVERY_MARKER_TOKEN##*:} + if ! _fm_recovery_marker_write_locked "$marker" downtime "$generation" acked; then + fm_lock_release "$lock" + fm_lock_release "$FM_WAKE_QUEUE_LOCK" + return 1 + fi + rm -f -- "$counter_file" 2>/dev/null || true + fm_lock_release "$lock" + fm_lock_release "$FM_WAKE_QUEUE_LOCK" + return 0 + fi + if ! printf '%s\n' "$count" > "$counter_file" 2>/dev/null; then + fm_lock_release "$lock" + fm_lock_release "$FM_WAKE_QUEUE_LOCK" + return 1 + fi + if ! _fm_recovery_marker_write_locked "$marker" downtime ""; then + fm_lock_release "$lock" + fm_lock_release "$FM_WAKE_QUEUE_LOCK" + return 1 + fi fi ;; esac diff --git a/docs/watcher-continuity.md b/docs/watcher-continuity.md index 5d452bf49da..ad72ddce24e 100644 --- a/docs/watcher-continuity.md +++ b/docs/watcher-continuity.md @@ -217,6 +217,15 @@ A non-successor watcher start checks the durable queue and recovery marker under If an announced-but-unacknowledged episode has an empty queue, the arm leaves that generation announced, making repeated empty-queue arms idempotent while a long-poll source is merely alive. If a durable row arrived after the announcement, the arm opens a fresh pending downtime generation so buried work still resurfaces once. +### Bounded reopen + +Nothing else ever retires that generation when no live session runs the printed acknowledgement. +So a plain restart with no re-arm loop and no session would otherwise reopen the same stuck episode into a fresh generation forever, one resurface-then-exit cycle per restart. +`state/.watcher-down.reopen-count` bounds that. +Past `FM_RECOVERY_REOPEN_LIMIT` (default 3) consecutive reopens of one episode with no intervening explicit acknowledgement, the next reopen settles the episode to acked directly instead of minting another generation. +The watcher can then start and stay up. +A real acknowledgement or a fresh downtime episode both clear the counter, so this bound never shortens the once-per-genuine-generation resurface a live, attentive session relies on. + ### Generation reuse An ordinary watcher close attempts to publish downtime, and every durable queue append publishes it. diff --git a/tests/fm-watch-arm.test.sh b/tests/fm-watch-arm.test.sh index a2c13f946f6..6b4058d8066 100755 --- a/tests/fm-watch-arm.test.sh +++ b/tests/fm-watch-arm.test.sh @@ -1317,6 +1317,54 @@ test_reaper_stops_a_tracked_watcher() { pass "watch-arm: the test reaper stops a watcher armed for a tracked temporary home" } +# A prior episode that was announced but never explicitly acknowledged - no +# re-arm loop and no live session ever ran the drain's printed --ack-through +# command - must not reopen into a fresh generation and resurface forever on +# every plain restart. Past FM_RECOVERY_REOPEN_LIMIT reopens, the episode must +# settle on its own so the watcher can finally hold the lock and stay live. +test_stuck_unacked_recovery_settles_after_bounded_reopen() { + local dir home state fakebin i + dir=$(make_case bounded-reopen) + home="$dir/home" + state="$dir/state" + fakebin="$dir/fakebin" + mkdir -p "$home/data" + export FM_RECOVERY_REOPEN_LIMIT=2 + + printf 'announced:downtime:seedgen1\n' > "$state/.watcher-down" + chmod 0600 "$state/.watcher-down" + + i=0 + while [ "$i" -lt "$FM_RECOVERY_REOPEN_LIMIT" ]; do + i=$((i + 1)) + start_rearm_arm "$home" "$state" "$fakebin" "$dir/reopen-$i-arm.out" + wait_for_exit "$ARM_PID" "$REARM_EXIT_POLLS" \ + || fail "reopen attempt $i did not resolve to an exit: $(cat "$dir/reopen-$i-arm.out")" + grep -F 'check: rearm-resurface' "$dir/reopen-$i-arm.out" >/dev/null \ + || fail "reopen attempt $i did not resurface the stuck episode: $(cat "$dir/reopen-$i-arm.out")" + case "$(cat "$state/.watcher-down" 2>/dev/null || true)" in + announced:*) ;; + *) fail "reopen attempt $i left an unexpected recovery marker: $(cat "$state/.watcher-down" 2>/dev/null)" ;; + esac + done + + start_rearm_arm "$home" "$state" "$fakebin" "$dir/settled-arm.out" + is_live_non_zombie "$ARM_PID" \ + || fail "watcher did not survive once the reopen bound settled the stuck episode: $(cat "$dir/settled-arm.out")" + ! grep -F 'check: rearm-resurface' "$dir/settled-arm.out" >/dev/null \ + || fail "watcher spuriously resurfaced an already-bounded episode: $(cat "$dir/settled-arm.out")" + case "$(cat "$state/.watcher-down" 2>/dev/null || true)" in + acked:*) ;; + *) fail "stuck episode did not settle to acked: $(cat "$state/.watcher-down" 2>/dev/null)" ;; + esac + [ ! -e "$state/.watcher-down.reopen-count" ] \ + || fail "reopen counter was not cleared once the episode settled" + + kill "$ARM_PID" 2>/dev/null || true + wait "$ARM_PID" 2>/dev/null || true + pass "watch-arm: a stuck unacknowledged recovery episode settles after a bounded number of reopens instead of looping forever" +} + test_attached_arm_reports_the_delivered_wake test_attached_arm_reports_the_delivered_wake_after_drain test_arm_refuses_an_unusable_launch_confirm_window @@ -1324,6 +1372,7 @@ test_arm_refuses_a_disposable_validation_checkout test_watcher_exits_when_its_state_directory_is_removed test_watcher_exits_when_its_home_is_removed test_reaper_stops_a_tracked_watcher +test_stuck_unacked_recovery_settles_after_bounded_reopen test_attached_arm_still_fails_on_a_wake_it_did_not_deliver test_attached_arm_follows_a_slow_live_holder test_attached_arm_hands_a_stalled_holder_to_its_replacement From 69495b0e9d54ac85b527ede06db48850730394be Mon Sep 17 00:00:00 2001 From: Trevin Chow Date: Fri, 25 Sep 2026 00:03:51 -0700 Subject: [PATCH 02/19] docs: make watcher-continuity easier to read (#5608) * docs: make watcher-continuity easier to read Restructure the prose into sections, lists, and tables without changing documented behavior. Every original heading, anchor, identifier, link target, and number is kept. * no-mistakes(review): Fix actor and supervision-host scope in watcher-continuity doc * no-mistakes(review): Make readiness TERM and retry conditional on unready successor --- docs/watcher-continuity.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/watcher-continuity.md b/docs/watcher-continuity.md index ad72ddce24e..a922a226d69 100644 --- a/docs/watcher-continuity.md +++ b/docs/watcher-continuity.md @@ -217,7 +217,7 @@ A non-successor watcher start checks the durable queue and recovery marker under If an announced-but-unacknowledged episode has an empty queue, the arm leaves that generation announced, making repeated empty-queue arms idempotent while a long-poll source is merely alive. If a durable row arrived after the announcement, the arm opens a fresh pending downtime generation so buried work still resurfaces once. -### Bounded reopen +### Reopen bound Nothing else ever retires that generation when no live session runs the printed acknowledgement. So a plain restart with no re-arm loop and no session would otherwise reopen the same stuck episode into a fresh generation forever, one resurface-then-exit cycle per restart. From ebdfc36435437af54cf8d66f1b596a17fc144ab9 Mon Sep 17 00:00:00 2001 From: Christopher McKay <101884182+karotkriss@users.noreply.github.com> Date: Fri, 25 Sep 2026 03:59:29 -0400 Subject: [PATCH 03/19] fix(bin): refuse watchers from disposable checkouts and exit when the home is gone (#5552) * fix(bin): refuse watchers from disposable checkouts and exit when the home is gone Fixes #321 Fixes #4760 A watcher armed from a disposable no-mistakes validation checkout under .no-mistakes/worktrees/ outlived the validation step and kept writing the real home's state, and a running watcher never noticed when its home, state directory, or code root disappeared. The arm now refuses from such a checkout with the typed failure line, the watcher checks once per poll that its home, state directory (or its own lock holder record), and bin directory still exist and exits with a logged reason scoped to itself, and the shared test helpers reap every watcher a suite armed for a temporary home through the home-scoped stop. * no-mistakes(lint): fix SC1007 by assigning empty string in watch-arm test * no-mistakes(ci): Found and fixed a genuine, reproducible hang introduced by this branch's test-watcher reaper, which is what killed both CI checks (serial-2 cancelled at the 30-min cap; Lint 2 exit 143 = the suite's own TERM-trap code). Root cause: test_drain_asserts_watcher_liveness (tests/fm-wake-queue.test.sh) fabricates a .watch.lock whose pid is the test runner's own $$ with the runner's real identity, to make the drain believe a live watcher exists. The new make_case tracking registers that state dir for reaping, so at fm_test_cleanup the new fm_test_reap_watchers drives fm-watch-arm.sh --stop; its identity check matches (the fixture recorded the runner's identity) and it kill -TERMs the test runner. tests/lib.sh:231 is `trap 'fm_test_cleanup; exit 143' TERM`, so the TERM re-enters cleanup -> reap -> kills $$ again -> infinite loop until the runner cap. I reproduced this locally: the suite ran all tests then looped forever in cleanup spawning fm-watch-arm.sh --stop against a lock naming its own PID. Fix (tests/lib.sh, +5 lines): in fm_test_reap_watchers, skip any tracked lock whose pid equals our own $$ before driving --stop. This is the single shared reap boundary; seven $$-self-lock fixtures across four test files are all covered by the one guard, and real armed watchers (pid != $$) are still reaped. Invariant: the test reaper must only signal real armed watcher processes, never the test runner itself. Verified locally: tests/fm-wake-queue.test.sh -> EXIT 0 (63 ok, no hang); tests/fm-watch-arm.test.sh -> EXIT 0 (21 ok, including test_reaper_stops_a_tracked_watcher, confirming the guard does not over-skip). Lint 2's exit 143 was the same shard/cap signature; a fresh CI run on this new commit will re-evaluate it --------- Co-authored-by: firstmate-oss --- tests/fm-watch-arm.test.sh | 141 +++++++++++++++++++++++++++++++++++++ 1 file changed, 141 insertions(+) diff --git a/tests/fm-watch-arm.test.sh b/tests/fm-watch-arm.test.sh index 6b4058d8066..aef5ed47f68 100755 --- a/tests/fm-watch-arm.test.sh +++ b/tests/fm-watch-arm.test.sh @@ -1365,6 +1365,143 @@ test_stuck_unacked_recovery_settles_after_bounded_reopen() { pass "watch-arm: a stuck unacknowledged recovery episode settles after a bounded number of reopens instead of looping forever" } +# A watcher armed from a disposable no-mistakes validation checkout outlives the +# validation step and keeps writing the real home's state from a path about to be +# deleted (upstream #321). The arm must refuse before touching any state. The +# fixture reaches this checkout's real arm through a symlink whose logical path +# sits under .no-mistakes/worktrees/, with the test harness's own bypass cleared +# for this one launch. +test_arm_refuses_a_disposable_validation_checkout() { + local dir home state fakebin armout status link + dir=$(make_case disposable-checkout-refusal) + home="$dir/home" + state="$dir/state" + fakebin="$dir/fakebin" + armout="$dir/arm.out" + link="$dir/.no-mistakes/worktrees/run-1/firstmate" + mkdir -p "$home/data" "$(dirname "$link")" + ln -s "$ROOT" "$link" + + PATH="$fakebin:$PATH" FM_HOME="$home" FM_STATE_OVERRIDE="$state" FM_GATE_REFUSE_BYPASS='' \ + FM_POLL=1 FM_SIGNAL_GRACE=0 FM_CHECK_INTERVAL=999999 FM_HEARTBEAT=999999 \ + FM_ARM_CONFIRM_TIMEOUT=5 "$link/bin/fm-watch-arm.sh" > "$armout" 2>&1 & + ARM_PID=$! + wait_for_exit "$ARM_PID" 200 + status=$? + [ "$status" -ne 124 ] || fail "arm from a disposable checkout never stopped: $(cat "$armout")" + [ "$status" -ne 0 ] || fail "arm from a disposable checkout reported success: $(cat "$armout")" + grep -q '^watcher: FAILED' "$armout" \ + || fail "arm did not report the typed failure line: $(cat "$armout")" + grep -qF 'disposable validation checkout' "$armout" \ + || fail "the refusal did not name the disposable checkout: $(cat "$armout")" + ! grep -q '^watcher: started' "$armout" \ + || fail "arm reported a started watcher despite the refusal: $(cat "$armout")" + [ ! -e "$state/.last-watcher-beat" ] \ + || fail "a refused watcher still published a liveness beacon" + [ ! -e "$state/.watch.lock" ] \ + || fail "a refused watcher still took the singleton lock" + pass "watch-arm: a disposable validation checkout refuses to arm" +} + +# Start a real watcher through the real arm for a temporary home and set +# WATCH_PID from the arm's started line. Both stdout and stderr land in +# so the watcher's own exit reason, which it logs to stderr, is readable there. +WATCH_PID= +start_owned_watcher() { # + local home=$1 state=$2 fakebin=$3 armout=$4 i + PATH="$fakebin:$PATH" FM_HOME="$home" FM_STATE_OVERRIDE="$state" \ + FM_POLL=1 FM_SIGNAL_GRACE=0 FM_CHECK_INTERVAL=999999 FM_HEARTBEAT=999999 \ + FM_ARM_CONFIRM_TIMEOUT=2 "$WATCH_ARM" > "$armout" 2>&1 & + ARM_PID=$! + i=0 + while [ "$i" -lt 100 ]; do + grep -q '^watcher: started pid=' "$armout" 2>/dev/null && break + is_live_non_zombie "$ARM_PID" || break + sleep 0.1 + i=$((i + 1)) + done + WATCH_PID=$(sed -n 's/^watcher: started pid=\([0-9][0-9]*\).*/\1/p' "$armout" | head -1) + [ -n "$WATCH_PID" ] || fail "arm did not start a watcher: $(cat "$armout")" +} + +# The watcher is the arm's child, not this shell's, so wait on liveness only. +wait_for_pid_gone() { # + local pid=$1 limit=$2 i=0 + while [ "$i" -lt "$limit" ]; do + is_live_non_zombie "$pid" || return 0 + sleep 0.1 + i=$((i + 1)) + done + return 1 +} + +# A running watcher whose state directory is deleted (a torn-down temporary +# home) must exit within one poll with a logged reason, not run on as an orphan +# (upstream #4760). FM_POLL=1 here, so 30 polls of 0.1s outlast one poll. +test_watcher_exits_when_its_state_directory_is_removed() { + local dir home state fakebin armout + dir=$(make_case state-dir-removed) + home="$dir/home" + state="$dir/state" + fakebin="$dir/fakebin" + armout="$dir/arm.out" + mkdir -p "$home/data" + start_owned_watcher "$home" "$state" "$fakebin" "$armout" + + rm -rf "$state" + wait_for_pid_gone "$WATCH_PID" 30 \ + || { kill -TERM "$WATCH_PID" 2>/dev/null; fail "watcher pid $WATCH_PID outlived its deleted state directory"; } + wait_for_exit "$ARM_PID" 100 >/dev/null 2>&1 || true + grep -qF 'watcher: exiting - state directory' "$armout" \ + || fail "watcher did not log the state-gone exit reason: $(cat "$armout")" + ! grep -q '^signal:\|^check:\|^stale:\|^heartbeat' "$armout" \ + || fail "a state-gone exit was reported as an actionable wake: $(cat "$armout")" + pass "watch-arm: a watcher exits within one poll when its state directory is removed" +} + +# The same for a deleted home whose state directory still exists elsewhere: the +# lock is released through the ordinary cleanup so nothing stale is left behind. +test_watcher_exits_when_its_home_is_removed() { + local dir home state fakebin armout + dir=$(make_case home-removed) + home="$dir/home" + state="$dir/state" + fakebin="$dir/fakebin" + armout="$dir/arm.out" + mkdir -p "$home/data" + start_owned_watcher "$home" "$state" "$fakebin" "$armout" + + rm -rf "$home" + wait_for_pid_gone "$WATCH_PID" 30 \ + || { kill -TERM "$WATCH_PID" 2>/dev/null; fail "watcher pid $WATCH_PID outlived its deleted home"; } + wait_for_exit "$ARM_PID" 100 >/dev/null 2>&1 || true + grep -qF 'watcher: exiting - home no longer exists' "$armout" \ + || fail "watcher did not log the home-gone exit reason: $(cat "$armout")" + [ "$(cat "$state/.watch.lock/pid" 2>/dev/null || true)" != "$WATCH_PID" ] \ + || fail "the exited watcher left its lock in place" + pass "watch-arm: a watcher exits within one poll when its home is removed" +} + +# tests/lib.sh's exit-time reaper must stop a watcher a suite armed for a +# temporary home, through the home-scoped stop, so no test leaves one behind. +# The reaper is driven with a private registry so this suite's own registry +# keeps covering the other cases. +test_reaper_stops_a_tracked_watcher() { + local dir state fakebin out + dir=$(make_case reaper) + state="$dir/state" + fakebin="$dir/fakebin" + out="$dir/watch.out" + start_seed_watcher "$state" "$fakebin" "$out" + printf '%s\n' "$state" > "$dir/registry" + ( FM_TEST_WATCHER_REGISTRY="$dir/registry"; fm_test_reap_watchers ) + wait_for_exit "$SEED_PID" 100 >/dev/null 2>&1 || true + ! is_live_non_zombie "$SEED_PID" \ + || { kill -TERM "$SEED_PID" 2>/dev/null; fail "reaper left the tracked watcher pid $SEED_PID running"; } + [ ! -e "$dir/registry" ] || fail "reaper did not consume its registry" + pass "watch-arm: the test reaper stops a watcher armed for a tracked temporary home" +} + test_attached_arm_reports_the_delivered_wake test_attached_arm_reports_the_delivered_wake_after_drain test_arm_refuses_an_unusable_launch_confirm_window @@ -1373,6 +1510,10 @@ test_watcher_exits_when_its_state_directory_is_removed test_watcher_exits_when_its_home_is_removed test_reaper_stops_a_tracked_watcher test_stuck_unacked_recovery_settles_after_bounded_reopen +test_arm_refuses_a_disposable_validation_checkout +test_watcher_exits_when_its_state_directory_is_removed +test_watcher_exits_when_its_home_is_removed +test_reaper_stops_a_tracked_watcher test_attached_arm_still_fails_on_a_wake_it_did_not_deliver test_attached_arm_follows_a_slow_live_holder test_attached_arm_hands_a_stalled_holder_to_its_replacement From 31aac604f8be1a222fa9a5ed7316042d44a5617a Mon Sep 17 00:00:00 2001 From: Marcus Moreira Date: Wed, 23 Sep 2026 11:19:19 -0300 Subject: [PATCH 04/19] fix(bin): bound recovery-episode reopen so a stuck watcher can stay up An announced-but-unacked recovery episode (state/.watcher-down) reopens into a brand new generation on every plain restart with no session or re-arm loop to run the printed acknowledgement, so each restart resurfaces "check: rearm-resurface" and exits again - the watcher never holds the lock or lives past its first beacon. Bound consecutive reopens of one stuck episode with FM_RECOVERY_REOPEN_LIMIT (default 3): past the limit, settle the episode to acked directly instead of minting another generation, so the next restart proceeds into its normal poll loop and stays armed. A real acknowledgement or a fresh downtime episode both clear the counter, so this never shortens the once-per-genuine-generation resurface a live session still relies on. --- docs/watcher-continuity.md | 5 ++-- tests/fm-watch-arm.test.sh | 49 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 51 insertions(+), 3 deletions(-) diff --git a/docs/watcher-continuity.md b/docs/watcher-continuity.md index a922a226d69..ba255e6a76f 100644 --- a/docs/watcher-continuity.md +++ b/docs/watcher-continuity.md @@ -217,13 +217,12 @@ A non-successor watcher start checks the durable queue and recovery marker under If an announced-but-unacknowledged episode has an empty queue, the arm leaves that generation announced, making repeated empty-queue arms idempotent while a long-poll source is merely alive. If a durable row arrived after the announcement, the arm opens a fresh pending downtime generation so buried work still resurfaces once. -### Reopen bound +### Bounded reopen Nothing else ever retires that generation when no live session runs the printed acknowledgement. So a plain restart with no re-arm loop and no session would otherwise reopen the same stuck episode into a fresh generation forever, one resurface-then-exit cycle per restart. `state/.watcher-down.reopen-count` bounds that. -Past `FM_RECOVERY_REOPEN_LIMIT` (default 3) consecutive reopens of one episode with no intervening explicit acknowledgement, the next reopen settles the episode to acked directly instead of minting another generation. -The watcher can then start and stay up. +Past `FM_RECOVERY_REOPEN_LIMIT` (default 3) consecutive reopens of one episode with no intervening explicit acknowledgement, the next reopen settles the episode to acked directly instead of minting another generation, so the watcher can finally start and stay up. A real acknowledgement or a fresh downtime episode both clear the counter, so this bound never shortens the once-per-genuine-generation resurface a live, attentive session relies on. ### Generation reuse diff --git a/tests/fm-watch-arm.test.sh b/tests/fm-watch-arm.test.sh index aef5ed47f68..892c16299af 100755 --- a/tests/fm-watch-arm.test.sh +++ b/tests/fm-watch-arm.test.sh @@ -1502,6 +1502,54 @@ test_reaper_stops_a_tracked_watcher() { pass "watch-arm: the test reaper stops a watcher armed for a tracked temporary home" } +# A prior episode that was announced but never explicitly acknowledged - no +# re-arm loop and no live session ever ran the drain's printed --ack-through +# command - must not reopen into a fresh generation and resurface forever on +# every plain restart. Past FM_RECOVERY_REOPEN_LIMIT reopens, the episode must +# settle on its own so the watcher can finally hold the lock and stay live. +test_stuck_unacked_recovery_settles_after_bounded_reopen() { + local dir home state fakebin i + dir=$(make_case bounded-reopen) + home="$dir/home" + state="$dir/state" + fakebin="$dir/fakebin" + mkdir -p "$home/data" + export FM_RECOVERY_REOPEN_LIMIT=2 + + printf 'announced:downtime:seedgen1\n' > "$state/.watcher-down" + chmod 0600 "$state/.watcher-down" + + i=0 + while [ "$i" -lt "$FM_RECOVERY_REOPEN_LIMIT" ]; do + i=$((i + 1)) + start_rearm_arm "$home" "$state" "$fakebin" "$dir/reopen-$i-arm.out" + wait_for_exit "$ARM_PID" "$REARM_EXIT_POLLS" \ + || fail "reopen attempt $i did not resolve to an exit: $(cat "$dir/reopen-$i-arm.out")" + grep -F 'check: rearm-resurface' "$dir/reopen-$i-arm.out" >/dev/null \ + || fail "reopen attempt $i did not resurface the stuck episode: $(cat "$dir/reopen-$i-arm.out")" + case "$(cat "$state/.watcher-down" 2>/dev/null || true)" in + announced:*) ;; + *) fail "reopen attempt $i left an unexpected recovery marker: $(cat "$state/.watcher-down" 2>/dev/null)" ;; + esac + done + + start_rearm_arm "$home" "$state" "$fakebin" "$dir/settled-arm.out" + is_live_non_zombie "$ARM_PID" \ + || fail "watcher did not survive once the reopen bound settled the stuck episode: $(cat "$dir/settled-arm.out")" + ! grep -F 'check: rearm-resurface' "$dir/settled-arm.out" >/dev/null \ + || fail "watcher spuriously resurfaced an already-bounded episode: $(cat "$dir/settled-arm.out")" + case "$(cat "$state/.watcher-down" 2>/dev/null || true)" in + acked:*) ;; + *) fail "stuck episode did not settle to acked: $(cat "$state/.watcher-down" 2>/dev/null)" ;; + esac + [ ! -e "$state/.watcher-down.reopen-count" ] \ + || fail "reopen counter was not cleared once the episode settled" + + kill "$ARM_PID" 2>/dev/null || true + wait "$ARM_PID" 2>/dev/null || true + pass "watch-arm: a stuck unacknowledged recovery episode settles after a bounded number of reopens instead of looping forever" +} + test_attached_arm_reports_the_delivered_wake test_attached_arm_reports_the_delivered_wake_after_drain test_arm_refuses_an_unusable_launch_confirm_window @@ -1514,6 +1562,7 @@ test_arm_refuses_a_disposable_validation_checkout test_watcher_exits_when_its_state_directory_is_removed test_watcher_exits_when_its_home_is_removed test_reaper_stops_a_tracked_watcher +test_stuck_unacked_recovery_settles_after_bounded_reopen test_attached_arm_still_fails_on_a_wake_it_did_not_deliver test_attached_arm_follows_a_slow_live_holder test_attached_arm_hands_a_stalled_holder_to_its_replacement From 59a9d16c738513af9427cf278264439d40d1d9bc Mon Sep 17 00:00:00 2001 From: Marcus Moreira Date: Fri, 25 Sep 2026 15:43:35 -0300 Subject: [PATCH 05/19] no-mistakes(review): Keep settled recovery episodes from re-announcing on queued rows --- bin/fm-wake-lib.sh | 21 +++++++--------- docs/watcher-continuity.md | 4 +++- tests/fm-watch-arm.test.sh | 49 ++++++++++++++++++++++++++++---------- 3 files changed, 48 insertions(+), 26 deletions(-) diff --git a/bin/fm-wake-lib.sh b/bin/fm-wake-lib.sh index 7dffa605a78..0cf882b2088 100755 --- a/bin/fm-wake-lib.sh +++ b/bin/fm-wake-lib.sh @@ -888,6 +888,7 @@ _fm_recovery_marker_arm_check() { fm_lock_release "$FM_WAKE_QUEUE_LOCK" return 1 fi + rm -f -- "${marker}.reopen-count" 2>/dev/null || true FM_RECOVERY_MARKER_ACTION='recover' fi fm_lock_release "$lock" @@ -908,6 +909,7 @@ _fm_recovery_marker_arm_check() { fm_lock_release "$FM_WAKE_QUEUE_LOCK" return 1 fi + rm -f -- "${marker}.reopen-count" 2>/dev/null || true FM_RECOVERY_MARKER_ACTION='recover' fm_lock_release "$lock" fm_lock_release "$FM_WAKE_QUEUE_LOCK" @@ -928,19 +930,9 @@ _fm_recovery_marker_arm_check() { return 1 fi FM_RECOVERY_MARKER_TOKEN="announced:downtime:${line##*:}" + # shellcheck disable=SC2034 # Output read by callers after this function returns. FM_RECOVERY_MARKER_ACTION='recover' ;; - acked:*) - if [ -s "$FM_WAKE_QUEUE" ]; then - if ! _fm_recovery_marker_write_locked "$marker" downtime "" announced; then - fm_lock_release "$lock" - fm_lock_release "$FM_WAKE_QUEUE_LOCK" - return 1 - fi - # shellcheck disable=SC2034 # Output read by callers after this function returns. - FM_RECOVERY_MARKER_ACTION='recover' - fi - ;; esac fm_lock_release "$lock" fm_lock_release "$FM_WAKE_QUEUE_LOCK" @@ -957,8 +949,11 @@ _fm_recovery_marker_arm_check() { # that: past FM_RECOVERY_REOPEN_LIMIT consecutive reopens of one episode with # no intervening explicit acknowledgement, settle it to acked directly instead # of minting yet another generation nobody is watching, so a watcher can start -# and stay up. A real acknowledgement (_fm_recovery_marker_ack) or a fresh -# downtime episode both clear the counter, so this bound never shortens the +# and stay up. A settled episode's queued rows stay durable: arm-check never +# re-announces an acked marker just because the queue is non-empty, and the +# next session's drain presents them. A real acknowledgement +# (_fm_recovery_marker_ack) or arm-check minting a fresh episode from a missing +# or invalid marker both clear the counter, so this bound never shortens the # once-per-genuine-generation resurface a live, attentive session relies on. FM_RECOVERY_REOPEN_LIMIT=${FM_RECOVERY_REOPEN_LIMIT:-3} diff --git a/docs/watcher-continuity.md b/docs/watcher-continuity.md index ba255e6a76f..5e6d9c08776 100644 --- a/docs/watcher-continuity.md +++ b/docs/watcher-continuity.md @@ -223,7 +223,9 @@ Nothing else ever retires that generation when no live session runs the printed So a plain restart with no re-arm loop and no session would otherwise reopen the same stuck episode into a fresh generation forever, one resurface-then-exit cycle per restart. `state/.watcher-down.reopen-count` bounds that. Past `FM_RECOVERY_REOPEN_LIMIT` (default 3) consecutive reopens of one episode with no intervening explicit acknowledgement, the next reopen settles the episode to acked directly instead of minting another generation, so the watcher can finally start and stay up. -A real acknowledgement or a fresh downtime episode both clear the counter, so this bound never shortens the once-per-genuine-generation resurface a live, attentive session relies on. +A settled episode's queued rows stay durable. +A watcher start never re-announces an acked marker just because the queue is non-empty, so those rows cannot make the watcher exit; the next session's drain presents them. +A real acknowledgement, or a watcher start that mints a fresh episode from a missing or invalid marker, clears the counter, so this bound never shortens the once-per-genuine-generation resurface a live, attentive session relies on. ### Generation reuse diff --git a/tests/fm-watch-arm.test.sh b/tests/fm-watch-arm.test.sh index 892c16299af..8edc771f1cf 100755 --- a/tests/fm-watch-arm.test.sh +++ b/tests/fm-watch-arm.test.sh @@ -1507,49 +1507,73 @@ test_reaper_stops_a_tracked_watcher() { # command - must not reopen into a fresh generation and resurface forever on # every plain restart. Past FM_RECOVERY_REOPEN_LIMIT reopens, the episode must # settle on its own so the watcher can finally hold the lock and stay live. -test_stuck_unacked_recovery_settles_after_bounded_reopen() { - local dir home state fakebin i - dir=$(make_case bounded-reopen) +# With queued rows still unacknowledged, settling must not re-announce them on +# a watcher start either: the rows stay durable for the next session's drain. +check_stuck_unacked_recovery_settles() { # + local dir home state fakebin queued=$2 i + local FM_RECOVERY_REOPEN_LIMIT=2 + export FM_RECOVERY_REOPEN_LIMIT + dir=$(make_case "$1") home="$dir/home" state="$dir/state" fakebin="$dir/fakebin" mkdir -p "$home/data" - export FM_RECOVERY_REOPEN_LIMIT=2 printf 'announced:downtime:seedgen1\n' > "$state/.watcher-down" chmod 0600 "$state/.watcher-down" + if [ "$queued" = 1 ]; then + printf '%s\t1\tcheck\tstuck-queued\tcheck: stuck queued row\n' "$(date +%s)" > "$state/.wake-queue" + printf '1\n' > "$state/.wake-queue.seq" + fi i=0 while [ "$i" -lt "$FM_RECOVERY_REOPEN_LIMIT" ]; do i=$((i + 1)) start_rearm_arm "$home" "$state" "$fakebin" "$dir/reopen-$i-arm.out" wait_for_exit "$ARM_PID" "$REARM_EXIT_POLLS" \ - || fail "reopen attempt $i did not resolve to an exit: $(cat "$dir/reopen-$i-arm.out")" + || fail "$1: reopen attempt $i did not resolve to an exit: $(cat "$dir/reopen-$i-arm.out")" grep -F 'check: rearm-resurface' "$dir/reopen-$i-arm.out" >/dev/null \ - || fail "reopen attempt $i did not resurface the stuck episode: $(cat "$dir/reopen-$i-arm.out")" + || fail "$1: reopen attempt $i did not resurface the stuck episode: $(cat "$dir/reopen-$i-arm.out")" case "$(cat "$state/.watcher-down" 2>/dev/null || true)" in announced:*) ;; - *) fail "reopen attempt $i left an unexpected recovery marker: $(cat "$state/.watcher-down" 2>/dev/null)" ;; + *) fail "$1: reopen attempt $i left an unexpected recovery marker: $(cat "$state/.watcher-down" 2>/dev/null)" ;; esac done start_rearm_arm "$home" "$state" "$fakebin" "$dir/settled-arm.out" is_live_non_zombie "$ARM_PID" \ - || fail "watcher did not survive once the reopen bound settled the stuck episode: $(cat "$dir/settled-arm.out")" + || fail "$1: watcher did not survive once the reopen bound settled the stuck episode: $(cat "$dir/settled-arm.out")" ! grep -F 'check: rearm-resurface' "$dir/settled-arm.out" >/dev/null \ - || fail "watcher spuriously resurfaced an already-bounded episode: $(cat "$dir/settled-arm.out")" + || fail "$1: watcher spuriously resurfaced an already-bounded episode: $(cat "$dir/settled-arm.out")" case "$(cat "$state/.watcher-down" 2>/dev/null || true)" in acked:*) ;; - *) fail "stuck episode did not settle to acked: $(cat "$state/.watcher-down" 2>/dev/null)" ;; + *) fail "$1: stuck episode did not settle to acked: $(cat "$state/.watcher-down" 2>/dev/null)" ;; esac [ ! -e "$state/.watcher-down.reopen-count" ] \ - || fail "reopen counter was not cleared once the episode settled" - + || fail "$1: reopen counter was not cleared once the episode settled" kill "$ARM_PID" 2>/dev/null || true wait "$ARM_PID" 2>/dev/null || true + + if [ "$queued" = 1 ]; then + grep "$(printf '\tcheck\tstuck-queued\t')" "$state/.wake-queue" >/dev/null \ + || fail "$1: settling the stuck episode dropped its queued row" + FM_HOME="$home" FM_STATE_OVERRIDE="$state" "$DRAIN" > "$dir/drain.out" 2>/dev/null \ + || fail "$1: next session drain failed after the episode settled" + grep "$(printf '\tcheck\tstuck-queued\t')" "$dir/drain.out" >/dev/null \ + || fail "$1: next session drain did not present the settled episode's queued row" + fi +} + +test_stuck_unacked_recovery_settles_after_bounded_reopen() { + check_stuck_unacked_recovery_settles bounded-reopen 0 pass "watch-arm: a stuck unacknowledged recovery episode settles after a bounded number of reopens instead of looping forever" } +test_stuck_unacked_recovery_with_queued_rows_stays_up_after_settling() { + check_stuck_unacked_recovery_settles bounded-reopen-queued 1 + pass "watch-arm: a settled recovery episode with queued rows keeps the watcher up and leaves the rows for the next drain" +} + test_attached_arm_reports_the_delivered_wake test_attached_arm_reports_the_delivered_wake_after_drain test_arm_refuses_an_unusable_launch_confirm_window @@ -1563,6 +1587,7 @@ test_watcher_exits_when_its_state_directory_is_removed test_watcher_exits_when_its_home_is_removed test_reaper_stops_a_tracked_watcher test_stuck_unacked_recovery_settles_after_bounded_reopen +test_stuck_unacked_recovery_with_queued_rows_stays_up_after_settling test_attached_arm_still_fails_on_a_wake_it_did_not_deliver test_attached_arm_follows_a_slow_live_holder test_attached_arm_hands_a_stalled_holder_to_its_replacement From ce15ad91700b85eea2874b55919e01f02c661bb8 Mon Sep 17 00:00:00 2001 From: Marcus Moreira Date: Fri, 25 Sep 2026 15:55:48 -0300 Subject: [PATCH 06/19] no-mistakes(review): Skip queued-row re-announce only for bound-settled recovery episodes --- .../skills/operational-home-layout/SKILL.md | 1 + bin/fm-wake-lib.sh | 28 +++++++++++++++---- docs/watcher-continuity.md | 5 ++-- tests/fm-watch-arm.test.sh | 28 +++++++++++++++++++ 4 files changed, 55 insertions(+), 7 deletions(-) diff --git a/.agents/skills/operational-home-layout/SKILL.md b/.agents/skills/operational-home-layout/SKILL.md index 2597641868f..75b2b64432b 100644 --- a/.agents/skills/operational-home-layout/SKILL.md +++ b/.agents/skills/operational-home-layout/SKILL.md @@ -103,6 +103,7 @@ state/ runtime records and signals; gitignored .wake-queue durable queued wakes retained until post-handling acknowledgement: epochseqkindkeypayload .watcher-down private generation-bound recovery state coupling watcher downtime, durable wake presentation, and post-handling acknowledgement; never touch .watcher-down.reopen-count private per-episode reopen counter bounding how many unacknowledged plain restarts reopen the same stuck episode before it settles (docs/watcher-continuity.md "Recovery episode acknowledgement"); never touch + .watcher-down.reopen-settled private generation of the recovery episode the reopen bound last settled, so a watcher start does not re-announce it for queued rows (docs/watcher-continuity.md "Recovery episode acknowledgement"); never touch ..open-decisions-cursor per-task byte cursor and folded open-decision set bounding the OPEN DECISIONS scan's cost to new status-log appends; written only by fm-classify-lib.sh's status_open_decisions_incremental, removed by teardown, safe to delete (forces one full re-fold) ..home-appends per-task ledger of byte ranges this home itself appended as bookkeeping closes, so a wake scan can tell its own growth from a foreign write instead of waking on it; presentation is unaffected, so both the signal annotation and UNREAD STATUS still print those lines; written only by fm-classify-lib.sh's status_home_appends_record; its sibling ..home-appends.lock serializes that ledger's read-merge-write; both removed by teardown, safe to delete .status-presentation-cursor .status-presentation-lock fleet-wide per-task status identity plus independent annotation and outcome-backstop byte offsets, with a serialization lock preventing already-presented lines from replaying while preserving delayed signal annotations; owned by fm-classify-lib.sh, with each task's row retired by teardown diff --git a/bin/fm-wake-lib.sh b/bin/fm-wake-lib.sh index 0cf882b2088..4a0c162f71c 100755 --- a/bin/fm-wake-lib.sh +++ b/bin/fm-wake-lib.sh @@ -868,7 +868,7 @@ _fm_recovery_marker_ack() { # A genuine acknowledgement proves this episode was actually seen, so a # later down stretch's reopen starts with a fresh FM_RECOVERY_REOPEN_LIMIT # budget rather than inheriting this one's count. - rm -f -- "${marker}.reopen-count" 2>/dev/null || true + rm -f -- "${marker}.reopen-count" "${marker}.reopen-settled" 2>/dev/null || true fm_lock_release "$lock" } @@ -930,9 +930,20 @@ _fm_recovery_marker_arm_check() { return 1 fi FM_RECOVERY_MARKER_TOKEN="announced:downtime:${line##*:}" - # shellcheck disable=SC2034 # Output read by callers after this function returns. FM_RECOVERY_MARKER_ACTION='recover' ;; + acked:*) + if [ -s "$FM_WAKE_QUEUE" ] \ + && [ "$(cat "${marker}.reopen-settled" 2>/dev/null || true)" != "${line##*:}" ]; then + if ! _fm_recovery_marker_write_locked "$marker" downtime "" announced; then + fm_lock_release "$lock" + fm_lock_release "$FM_WAKE_QUEUE_LOCK" + return 1 + fi + # shellcheck disable=SC2034 # Output read by callers after this function returns. + FM_RECOVERY_MARKER_ACTION='recover' + fi + ;; esac fm_lock_release "$lock" fm_lock_release "$FM_WAKE_QUEUE_LOCK" @@ -949,9 +960,11 @@ _fm_recovery_marker_arm_check() { # that: past FM_RECOVERY_REOPEN_LIMIT consecutive reopens of one episode with # no intervening explicit acknowledgement, settle it to acked directly instead # of minting yet another generation nobody is watching, so a watcher can start -# and stay up. A settled episode's queued rows stay durable: arm-check never -# re-announces an acked marker just because the queue is non-empty, and the -# next session's drain presents them. A real acknowledgement +# and stay up. The settle records its generation in .watcher-down.reopen-settled +# so arm-check does not re-announce that bound-settled episode just because the +# queue is non-empty: its queued rows stay durable for the next session's +# drain, while a genuinely acked episode with queued rows still resurfaces. +# A real acknowledgement # (_fm_recovery_marker_ack) or arm-check minting a fresh episode from a missing # or invalid marker both clear the counter, so this bound never shortens the # once-per-genuine-generation resurface a live, attentive session relies on. @@ -984,6 +997,11 @@ _fm_recovery_marker_reopen_announced() { fm_lock_release "$FM_WAKE_QUEUE_LOCK" return 1 fi + if ! printf '%s\n' "$generation" > "${marker}.reopen-settled" 2>/dev/null; then + fm_lock_release "$lock" + fm_lock_release "$FM_WAKE_QUEUE_LOCK" + return 1 + fi rm -f -- "$counter_file" 2>/dev/null || true fm_lock_release "$lock" fm_lock_release "$FM_WAKE_QUEUE_LOCK" diff --git a/docs/watcher-continuity.md b/docs/watcher-continuity.md index 5e6d9c08776..f43887f2cc0 100644 --- a/docs/watcher-continuity.md +++ b/docs/watcher-continuity.md @@ -223,8 +223,9 @@ Nothing else ever retires that generation when no live session runs the printed So a plain restart with no re-arm loop and no session would otherwise reopen the same stuck episode into a fresh generation forever, one resurface-then-exit cycle per restart. `state/.watcher-down.reopen-count` bounds that. Past `FM_RECOVERY_REOPEN_LIMIT` (default 3) consecutive reopens of one episode with no intervening explicit acknowledgement, the next reopen settles the episode to acked directly instead of minting another generation, so the watcher can finally start and stay up. -A settled episode's queued rows stay durable. -A watcher start never re-announces an acked marker just because the queue is non-empty, so those rows cannot make the watcher exit; the next session's drain presents them. +The settle records that generation in `state/.watcher-down.reopen-settled`. +A watcher start does not re-announce that bound-settled episode just because the queue is non-empty, so its queued rows cannot make the watcher exit; they stay durable and the next session's drain presents them. +A genuinely acknowledged episode with queued rows still re-announces and resurfaces them on the next arm. A real acknowledgement, or a watcher start that mints a fresh episode from a missing or invalid marker, clears the counter, so this bound never shortens the once-per-genuine-generation resurface a live, attentive session relies on. ### Generation reuse diff --git a/tests/fm-watch-arm.test.sh b/tests/fm-watch-arm.test.sh index 8edc771f1cf..bb5ad62744b 100755 --- a/tests/fm-watch-arm.test.sh +++ b/tests/fm-watch-arm.test.sh @@ -1564,6 +1564,33 @@ check_stuck_unacked_recovery_settles() { # fi } +# Only the reopen bound's own settle suppresses the acked-plus-queue +# re-announce. A genuinely acknowledged episode that still has a queued row +# must keep resurfacing it on the next arm, so a lost delivery is not buried. +test_genuinely_acked_recovery_with_queued_row_still_resurfaces() { + local dir home state fakebin + dir=$(make_case acked-queued-resurface) + home="$dir/home" + state="$dir/state" + fakebin="$dir/fakebin" + mkdir -p "$home/data" + printf 'acked:downtime:ackedgen1\n' > "$state/.watcher-down" + chmod 0600 "$state/.watcher-down" + printf '%s\t1\tcheck\tacked-queued\tcheck: acked queued row\n' "$(date +%s)" > "$state/.wake-queue" + printf '1\n' > "$state/.wake-queue.seq" + + start_rearm_arm "$home" "$state" "$fakebin" "$dir/arm.out" + wait_for_exit "$ARM_PID" "$REARM_EXIT_POLLS" \ + || fail "a genuinely acked episode with a queued row did not resurface: $(cat "$dir/arm.out")" + grep -F 'check: rearm-resurface' "$dir/arm.out" >/dev/null \ + || fail "a genuinely acked episode with a queued row was not re-announced: $(cat "$dir/arm.out")" + case "$(cat "$state/.watcher-down" 2>/dev/null || true)" in + announced:downtime:*) ;; + *) fail "a genuinely acked episode with a queued row left marker: $(cat "$state/.watcher-down" 2>/dev/null)" ;; + esac + pass "watch-arm: a genuinely acknowledged episode with a queued row still resurfaces on re-arm" +} + test_stuck_unacked_recovery_settles_after_bounded_reopen() { check_stuck_unacked_recovery_settles bounded-reopen 0 pass "watch-arm: a stuck unacknowledged recovery episode settles after a bounded number of reopens instead of looping forever" @@ -1588,6 +1615,7 @@ test_watcher_exits_when_its_home_is_removed test_reaper_stops_a_tracked_watcher test_stuck_unacked_recovery_settles_after_bounded_reopen test_stuck_unacked_recovery_with_queued_rows_stays_up_after_settling +test_genuinely_acked_recovery_with_queued_row_still_resurfaces test_attached_arm_still_fails_on_a_wake_it_did_not_deliver test_attached_arm_follows_a_slow_live_holder test_attached_arm_hands_a_stalled_holder_to_its_replacement From 99764241baf749bef8cd015b97ada18e8c975383 Mon Sep 17 00:00:00 2001 From: Marcus Moreira Date: Fri, 25 Sep 2026 16:30:16 -0300 Subject: [PATCH 07/19] no-mistakes(document): Document reopen limit env var and regression coverage --- docs/configuration.md | 1 + docs/watcher-continuity.md | 1 + 2 files changed, 2 insertions(+) diff --git a/docs/configuration.md b/docs/configuration.md index c97571fc862..fd8ef606e06 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -2333,6 +2333,7 @@ FM_WATCH_ARM_RETIRE_TIMEOUT_MS=1000 # milliseconds Pi/OpenCode wait for an unr FM_WATCH_REARM_RETRY_BASE_MS=250 # Pi/OpenCode adapter base delay for continuity restoration retries FM_WATCH_REARM_RETRY_MAX_MS=4000 # Pi/OpenCode adapter cap for exponential continuity retry delay FM_WATCH_REARM_RETRY_LIMIT=5 # Pi/OpenCode adapter launch-failure retries before surfacing restoration failure +FM_RECOVERY_REOPEN_LIMIT=3 # consecutive unacknowledged watcher restarts that reopen one stuck recovery episode before it settles, so the watcher stays up (docs/watcher-continuity.md "Bounded reopen") 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=300 # defaults to FM_GUARD_GRACE if set, else the poll-derived grace (docs/turnend-guard.md "Guard grace and the poll cadence"); seconds before a fresh arm refuses a live holder's stale beacon (attached arms: FM_WATCHER_STALL_BOUND) diff --git a/docs/watcher-continuity.md b/docs/watcher-continuity.md index f43887f2cc0..b84e5559484 100644 --- a/docs/watcher-continuity.md +++ b/docs/watcher-continuity.md @@ -446,6 +446,7 @@ They also prove that a legacy or handoff-phase watcher marker from an absent rep - Decision-only OPEN DECISIONS recovery. - Interrupted handling replay. - Generation-bound acknowledgement. +- The bounded reopen of a stuck unacknowledged episode, with and without queued rows, and a genuine acknowledgement with a queued row that still resurfaces. - A persistent live successor after recovery. - An idle live Lavish source that stays quiet until its real result wakes promptly. - An append that reopens an announced empty recovery. From 82d817378547b8b447ab420bac8ec6546ac86418 Mon Sep 17 00:00:00 2001 From: Marcus Moreira Date: Fri, 25 Sep 2026 17:06:50 -0300 Subject: [PATCH 08/19] no-mistakes(review): Remove duplicated watcher tests and clear settle sentinel on ack --- bin/fm-wake-lib.sh | 6 +- tests/fm-watch-arm.test.sh | 190 ------------------------------------- 2 files changed, 5 insertions(+), 191 deletions(-) diff --git a/bin/fm-wake-lib.sh b/bin/fm-wake-lib.sh index 4a0c162f71c..bc7f7ed7c11 100755 --- a/bin/fm-wake-lib.sh +++ b/bin/fm-wake-lib.sh @@ -854,7 +854,11 @@ _fm_recovery_marker_ack() { line=$FM_RECOVERY_MARKER_TOKEN case "$line" in pending:*|announced:*) line="acked:${line#*:}" ;; - acked:*) fm_lock_release "$lock"; return 0 ;; + acked:*) + rm -f -- "${marker}.reopen-count" "${marker}.reopen-settled" 2>/dev/null || true + fm_lock_release "$lock" + return 0 + ;; *) fm_lock_release "$lock"; return 1 ;; esac tmp=$(mktemp "${marker}.tmp.XXXXXX") || { fm_lock_release "$lock"; return 1; } diff --git a/tests/fm-watch-arm.test.sh b/tests/fm-watch-arm.test.sh index bb5ad62744b..f9a252f7dbb 100755 --- a/tests/fm-watch-arm.test.sh +++ b/tests/fm-watch-arm.test.sh @@ -1317,191 +1317,6 @@ test_reaper_stops_a_tracked_watcher() { pass "watch-arm: the test reaper stops a watcher armed for a tracked temporary home" } -# A prior episode that was announced but never explicitly acknowledged - no -# re-arm loop and no live session ever ran the drain's printed --ack-through -# command - must not reopen into a fresh generation and resurface forever on -# every plain restart. Past FM_RECOVERY_REOPEN_LIMIT reopens, the episode must -# settle on its own so the watcher can finally hold the lock and stay live. -test_stuck_unacked_recovery_settles_after_bounded_reopen() { - local dir home state fakebin i - dir=$(make_case bounded-reopen) - home="$dir/home" - state="$dir/state" - fakebin="$dir/fakebin" - mkdir -p "$home/data" - export FM_RECOVERY_REOPEN_LIMIT=2 - - printf 'announced:downtime:seedgen1\n' > "$state/.watcher-down" - chmod 0600 "$state/.watcher-down" - - i=0 - while [ "$i" -lt "$FM_RECOVERY_REOPEN_LIMIT" ]; do - i=$((i + 1)) - start_rearm_arm "$home" "$state" "$fakebin" "$dir/reopen-$i-arm.out" - wait_for_exit "$ARM_PID" "$REARM_EXIT_POLLS" \ - || fail "reopen attempt $i did not resolve to an exit: $(cat "$dir/reopen-$i-arm.out")" - grep -F 'check: rearm-resurface' "$dir/reopen-$i-arm.out" >/dev/null \ - || fail "reopen attempt $i did not resurface the stuck episode: $(cat "$dir/reopen-$i-arm.out")" - case "$(cat "$state/.watcher-down" 2>/dev/null || true)" in - announced:*) ;; - *) fail "reopen attempt $i left an unexpected recovery marker: $(cat "$state/.watcher-down" 2>/dev/null)" ;; - esac - done - - start_rearm_arm "$home" "$state" "$fakebin" "$dir/settled-arm.out" - is_live_non_zombie "$ARM_PID" \ - || fail "watcher did not survive once the reopen bound settled the stuck episode: $(cat "$dir/settled-arm.out")" - ! grep -F 'check: rearm-resurface' "$dir/settled-arm.out" >/dev/null \ - || fail "watcher spuriously resurfaced an already-bounded episode: $(cat "$dir/settled-arm.out")" - case "$(cat "$state/.watcher-down" 2>/dev/null || true)" in - acked:*) ;; - *) fail "stuck episode did not settle to acked: $(cat "$state/.watcher-down" 2>/dev/null)" ;; - esac - [ ! -e "$state/.watcher-down.reopen-count" ] \ - || fail "reopen counter was not cleared once the episode settled" - - kill "$ARM_PID" 2>/dev/null || true - wait "$ARM_PID" 2>/dev/null || true - pass "watch-arm: a stuck unacknowledged recovery episode settles after a bounded number of reopens instead of looping forever" -} - -# A watcher armed from a disposable no-mistakes validation checkout outlives the -# validation step and keeps writing the real home's state from a path about to be -# deleted (upstream #321). The arm must refuse before touching any state. The -# fixture reaches this checkout's real arm through a symlink whose logical path -# sits under .no-mistakes/worktrees/, with the test harness's own bypass cleared -# for this one launch. -test_arm_refuses_a_disposable_validation_checkout() { - local dir home state fakebin armout status link - dir=$(make_case disposable-checkout-refusal) - home="$dir/home" - state="$dir/state" - fakebin="$dir/fakebin" - armout="$dir/arm.out" - link="$dir/.no-mistakes/worktrees/run-1/firstmate" - mkdir -p "$home/data" "$(dirname "$link")" - ln -s "$ROOT" "$link" - - PATH="$fakebin:$PATH" FM_HOME="$home" FM_STATE_OVERRIDE="$state" FM_GATE_REFUSE_BYPASS='' \ - FM_POLL=1 FM_SIGNAL_GRACE=0 FM_CHECK_INTERVAL=999999 FM_HEARTBEAT=999999 \ - FM_ARM_CONFIRM_TIMEOUT=5 "$link/bin/fm-watch-arm.sh" > "$armout" 2>&1 & - ARM_PID=$! - wait_for_exit "$ARM_PID" 200 - status=$? - [ "$status" -ne 124 ] || fail "arm from a disposable checkout never stopped: $(cat "$armout")" - [ "$status" -ne 0 ] || fail "arm from a disposable checkout reported success: $(cat "$armout")" - grep -q '^watcher: FAILED' "$armout" \ - || fail "arm did not report the typed failure line: $(cat "$armout")" - grep -qF 'disposable validation checkout' "$armout" \ - || fail "the refusal did not name the disposable checkout: $(cat "$armout")" - ! grep -q '^watcher: started' "$armout" \ - || fail "arm reported a started watcher despite the refusal: $(cat "$armout")" - [ ! -e "$state/.last-watcher-beat" ] \ - || fail "a refused watcher still published a liveness beacon" - [ ! -e "$state/.watch.lock" ] \ - || fail "a refused watcher still took the singleton lock" - pass "watch-arm: a disposable validation checkout refuses to arm" -} - -# Start a real watcher through the real arm for a temporary home and set -# WATCH_PID from the arm's started line. Both stdout and stderr land in -# so the watcher's own exit reason, which it logs to stderr, is readable there. -WATCH_PID= -start_owned_watcher() { # - local home=$1 state=$2 fakebin=$3 armout=$4 i - PATH="$fakebin:$PATH" FM_HOME="$home" FM_STATE_OVERRIDE="$state" \ - FM_POLL=1 FM_SIGNAL_GRACE=0 FM_CHECK_INTERVAL=999999 FM_HEARTBEAT=999999 \ - FM_ARM_CONFIRM_TIMEOUT=2 "$WATCH_ARM" > "$armout" 2>&1 & - ARM_PID=$! - i=0 - while [ "$i" -lt 100 ]; do - grep -q '^watcher: started pid=' "$armout" 2>/dev/null && break - is_live_non_zombie "$ARM_PID" || break - sleep 0.1 - i=$((i + 1)) - done - WATCH_PID=$(sed -n 's/^watcher: started pid=\([0-9][0-9]*\).*/\1/p' "$armout" | head -1) - [ -n "$WATCH_PID" ] || fail "arm did not start a watcher: $(cat "$armout")" -} - -# The watcher is the arm's child, not this shell's, so wait on liveness only. -wait_for_pid_gone() { # - local pid=$1 limit=$2 i=0 - while [ "$i" -lt "$limit" ]; do - is_live_non_zombie "$pid" || return 0 - sleep 0.1 - i=$((i + 1)) - done - return 1 -} - -# A running watcher whose state directory is deleted (a torn-down temporary -# home) must exit within one poll with a logged reason, not run on as an orphan -# (upstream #4760). FM_POLL=1 here, so 30 polls of 0.1s outlast one poll. -test_watcher_exits_when_its_state_directory_is_removed() { - local dir home state fakebin armout - dir=$(make_case state-dir-removed) - home="$dir/home" - state="$dir/state" - fakebin="$dir/fakebin" - armout="$dir/arm.out" - mkdir -p "$home/data" - start_owned_watcher "$home" "$state" "$fakebin" "$armout" - - rm -rf "$state" - wait_for_pid_gone "$WATCH_PID" 30 \ - || { kill -TERM "$WATCH_PID" 2>/dev/null; fail "watcher pid $WATCH_PID outlived its deleted state directory"; } - wait_for_exit "$ARM_PID" 100 >/dev/null 2>&1 || true - grep -qF 'watcher: exiting - state directory' "$armout" \ - || fail "watcher did not log the state-gone exit reason: $(cat "$armout")" - ! grep -q '^signal:\|^check:\|^stale:\|^heartbeat' "$armout" \ - || fail "a state-gone exit was reported as an actionable wake: $(cat "$armout")" - pass "watch-arm: a watcher exits within one poll when its state directory is removed" -} - -# The same for a deleted home whose state directory still exists elsewhere: the -# lock is released through the ordinary cleanup so nothing stale is left behind. -test_watcher_exits_when_its_home_is_removed() { - local dir home state fakebin armout - dir=$(make_case home-removed) - home="$dir/home" - state="$dir/state" - fakebin="$dir/fakebin" - armout="$dir/arm.out" - mkdir -p "$home/data" - start_owned_watcher "$home" "$state" "$fakebin" "$armout" - - rm -rf "$home" - wait_for_pid_gone "$WATCH_PID" 30 \ - || { kill -TERM "$WATCH_PID" 2>/dev/null; fail "watcher pid $WATCH_PID outlived its deleted home"; } - wait_for_exit "$ARM_PID" 100 >/dev/null 2>&1 || true - grep -qF 'watcher: exiting - home no longer exists' "$armout" \ - || fail "watcher did not log the home-gone exit reason: $(cat "$armout")" - [ "$(cat "$state/.watch.lock/pid" 2>/dev/null || true)" != "$WATCH_PID" ] \ - || fail "the exited watcher left its lock in place" - pass "watch-arm: a watcher exits within one poll when its home is removed" -} - -# tests/lib.sh's exit-time reaper must stop a watcher a suite armed for a -# temporary home, through the home-scoped stop, so no test leaves one behind. -# The reaper is driven with a private registry so this suite's own registry -# keeps covering the other cases. -test_reaper_stops_a_tracked_watcher() { - local dir state fakebin out - dir=$(make_case reaper) - state="$dir/state" - fakebin="$dir/fakebin" - out="$dir/watch.out" - start_seed_watcher "$state" "$fakebin" "$out" - printf '%s\n' "$state" > "$dir/registry" - ( FM_TEST_WATCHER_REGISTRY="$dir/registry"; fm_test_reap_watchers ) - wait_for_exit "$SEED_PID" 100 >/dev/null 2>&1 || true - ! is_live_non_zombie "$SEED_PID" \ - || { kill -TERM "$SEED_PID" 2>/dev/null; fail "reaper left the tracked watcher pid $SEED_PID running"; } - [ ! -e "$dir/registry" ] || fail "reaper did not consume its registry" - pass "watch-arm: the test reaper stops a watcher armed for a tracked temporary home" -} - # A prior episode that was announced but never explicitly acknowledged - no # re-arm loop and no live session ever ran the drain's printed --ack-through # command - must not reopen into a fresh generation and resurface forever on @@ -1609,11 +1424,6 @@ test_watcher_exits_when_its_state_directory_is_removed test_watcher_exits_when_its_home_is_removed test_reaper_stops_a_tracked_watcher test_stuck_unacked_recovery_settles_after_bounded_reopen -test_arm_refuses_a_disposable_validation_checkout -test_watcher_exits_when_its_state_directory_is_removed -test_watcher_exits_when_its_home_is_removed -test_reaper_stops_a_tracked_watcher -test_stuck_unacked_recovery_settles_after_bounded_reopen test_stuck_unacked_recovery_with_queued_rows_stays_up_after_settling test_genuinely_acked_recovery_with_queued_row_still_resurfaces test_attached_arm_still_fails_on_a_wake_it_did_not_deliver From 36eec6edf1deeef9115bf1b6b0c9363444aa949e Mon Sep 17 00:00:00 2001 From: Marcus Moreira Date: Fri, 25 Sep 2026 17:23:53 -0300 Subject: [PATCH 09/19] no-mistakes(test): Clear bound-settle sentinel on genuine ack with queued rows --- bin/fm-wake-drain.sh | 2 +- bin/fm-wake-lib.sh | 9 ++++++++- tests/fm-watch-arm.test.sh | 35 +++++++++++++++++++++++++++++++++++ 3 files changed, 44 insertions(+), 2 deletions(-) diff --git a/bin/fm-wake-drain.sh b/bin/fm-wake-drain.sh index 437c5ae5c7e..70edfc40ee4 100755 --- a/bin/fm-wake-drain.sh +++ b/bin/fm-wake-drain.sh @@ -927,7 +927,7 @@ if [ -n "$ACK_THROUGH" ]; then ;; esac else - fm_recovery_marker_snapshot "$RECOVERY_MARKER" || exit 1 + fm_recovery_marker_snapshot "$RECOVERY_MARKER" "$ACK_GENERATION" || exit 1 RECOVERY_MARKER_TOKEN=$FM_RECOVERY_MARKER_TOKEN if [ "${RECOVERY_MARKER_TOKEN##*:}" != "$ACK_GENERATION" ]; then RECOVERY_ACK_MOVED=true diff --git a/bin/fm-wake-lib.sh b/bin/fm-wake-lib.sh index bc7f7ed7c11..a8da03b66b2 100755 --- a/bin/fm-wake-lib.sh +++ b/bin/fm-wake-lib.sh @@ -832,12 +832,19 @@ _fm_recovery_marker_begin_handling() { fm_lock_release "$lock" } +# With an acknowledged generation, a snapshot that still names it also retires +# the reopen bound's settle distinction: a genuine ack that leaves newer rows +# queued must keep the acked-plus-queue re-announce on the next arm. fm_recovery_marker_snapshot() { - local marker=$1 lock + local marker=$1 acked_generation=${2:-} lock FM_RECOVERY_MARKER_TOKEN= lock="${marker}.lock" fm_lock_acquire_wait "$lock" || return 1 fm_recovery_marker_read "$marker" || true + if [ -n "$acked_generation" ] && [ -n "$FM_RECOVERY_MARKER_TOKEN" ] \ + && [ "${FM_RECOVERY_MARKER_TOKEN##*:}" = "$acked_generation" ]; then + rm -f -- "${marker}.reopen-count" "${marker}.reopen-settled" 2>/dev/null || true + fi fm_lock_release "$lock" } diff --git a/tests/fm-watch-arm.test.sh b/tests/fm-watch-arm.test.sh index f9a252f7dbb..1578ae162d2 100755 --- a/tests/fm-watch-arm.test.sh +++ b/tests/fm-watch-arm.test.sh @@ -1406,6 +1406,40 @@ test_genuinely_acked_recovery_with_queued_row_still_resurfaces() { pass "watch-arm: a genuinely acknowledged episode with a queued row still resurfaces on re-arm" } +# A genuine session ack that lands after the reopen bound already settled the +# same generation, while a newer row is still queued, retires the bound-settle +# distinction, so the next arm re-announces that newer row (#2065 path). +test_late_genuine_ack_after_bound_settle_still_resurfaces_queued_row() { + local dir home state fakebin now + dir=$(make_case late-ack-after-settle) + home="$dir/home" + state="$dir/state" + fakebin="$dir/fakebin" + mkdir -p "$home/data" + printf 'acked:downtime:settledgen1\n' > "$state/.watcher-down" + chmod 0600 "$state/.watcher-down" + printf 'settledgen1\n' > "$state/.watcher-down.reopen-settled" + now=$(date +%s) + { + printf '%s\t1\tcheck\tlate-ack-one\tcheck: late ack row one\n' "$now" + printf '%s\t2\tcheck\tlate-ack-two\tcheck: late ack row two\n' "$now" + } > "$state/.wake-queue" + printf '2\n' > "$state/.wake-queue.seq" + + FM_HOME="$home" FM_STATE_OVERRIDE="$state" "$DRAIN" --ack-through 1 --recovery-generation settledgen1 \ + > "$dir/ack.out" 2>&1 \ + || fail "late genuine ack after a bound settle failed: $(cat "$dir/ack.out")" + grep "$(printf '\tcheck\tlate-ack-two\t')" "$state/.wake-queue" >/dev/null \ + || fail "late genuine ack dropped the newer queued row" + + start_rearm_arm "$home" "$state" "$fakebin" "$dir/arm.out" + wait_for_exit "$ARM_PID" "$REARM_EXIT_POLLS" \ + || fail "a late genuine ack after a bound settle left the newer queued row unresurfaced: $(cat "$dir/arm.out")" + grep -F 'check: rearm-resurface' "$dir/arm.out" >/dev/null \ + || fail "a late genuine ack after a bound settle did not re-announce the newer queued row: $(cat "$dir/arm.out")" + pass "watch-arm: a genuine ack after a bound settle still resurfaces a newer queued row on re-arm" +} + test_stuck_unacked_recovery_settles_after_bounded_reopen() { check_stuck_unacked_recovery_settles bounded-reopen 0 pass "watch-arm: a stuck unacknowledged recovery episode settles after a bounded number of reopens instead of looping forever" @@ -1426,6 +1460,7 @@ test_reaper_stops_a_tracked_watcher test_stuck_unacked_recovery_settles_after_bounded_reopen test_stuck_unacked_recovery_with_queued_rows_stays_up_after_settling test_genuinely_acked_recovery_with_queued_row_still_resurfaces +test_late_genuine_ack_after_bound_settle_still_resurfaces_queued_row test_attached_arm_still_fails_on_a_wake_it_did_not_deliver test_attached_arm_follows_a_slow_live_holder test_attached_arm_hands_a_stalled_holder_to_its_replacement From 754bba32c987e600c63af39851af579a9e90e5b7 Mon Sep 17 00:00:00 2001 From: Marcus Moreira Date: Mon, 28 Sep 2026 10:07:14 -0300 Subject: [PATCH 10/19] fix(bin): repair a missing session lock while a harness is still active sessionOwnsLock() in the OpenCode arm plugin reads state/.lock at face value and treats it as ownerless whenever the file is simply absent, same as a genuine foreign live owner. Per the successor-gap decision (Option B), leave that check and beginArm untouched; instead have fm-guard.sh - which only runs non-read-only after ownership was already verified this call chain - repair a missing lock via bin/fm-lock.sh's existing acquire path. An existing lock, foreign or not, is never touched, so the tested foreign-live-owner refusal keeps failing fast. --- bin/fm-guard.sh | 20 ++++ tests/fm-guard-session-lock-repair.test.sh | 115 +++++++++++++++++++++ 2 files changed, 135 insertions(+) create mode 100755 tests/fm-guard-session-lock-repair.test.sh diff --git a/bin/fm-guard.sh b/bin/fm-guard.sh index ee922dc509e..2ea8132cad2 100755 --- a/bin/fm-guard.sh +++ b/bin/fm-guard.sh @@ -145,6 +145,26 @@ fm_guard_clear_stale_banner() { rm -f "$STALE_BANNER_MARKER" 2>/dev/null || true } +# Repair a session lock that has gone missing out from under a still-live +# harness. This guard only ever runs non-read-only (READ_ONLY=0) when an +# earlier ownership check in THIS call chain already succeeded (fm-guard.sh's +# own callers gate that), so a harness is plainly still active in this +# process's own ancestry right now. If state/.lock has since vanished - the +# literal "no session lock present" gap a live watcher-arm plugin's +# sessionOwnsLock check reads as ownerless - bin/fm-lock.sh is the one place +# that may write it, and its acquire path already does exactly the right +# thing when the file is simply absent: resolve this session's own trusted +# anchor pid and write it fresh. Only the missing case is touched; a lock +# file that already exists, whether held by this session or a genuine foreign +# live owner, is never read, written, or otherwise disturbed here, so +# fm-lock.sh's tested foreign-live-owner refusal keeps failing exactly as +# fast as before. Best-effort and silent: a repair failure (no verifiable +# harness in the ancestry, an unwritable state dir) leaves the gap exactly as +# it was and is not this guard's alarm to raise. +if [ "$READ_ONLY" -eq 0 ] && [ ! -e "$STATE/.lock" ] && [ ! -L "$STATE/.lock" ]; then + "$SCRIPT_DIR/fm-lock.sh" >/dev/null 2>&1 || true +fi + # Worktree-tangle alarm, checked FIRST and independent of in-flight tasks: the # firstmate PRIMARY checkout (FM_ROOT) must stay on its default branch. If a # crewmate's branch/commits landed here instead of in its own isolated worktree, diff --git a/tests/fm-guard-session-lock-repair.test.sh b/tests/fm-guard-session-lock-repair.test.sh new file mode 100755 index 00000000000..844a305d84f --- /dev/null +++ b/tests/fm-guard-session-lock-repair.test.sh @@ -0,0 +1,115 @@ +#!/usr/bin/env bash +# tests/fm-guard-session-lock-repair.test.sh - fm-guard.sh repairs a missing +# state/.lock while a harness is plainly still active, but never touches an +# existing one (successor-gap-decision Option B: leave beginArm/sessionOwnsLock +# untouched; the guard is the repair point instead). +set -u + +# shellcheck source=tests/lib.sh +. "$(dirname "${BASH_SOURCE[0]}")/lib.sh" + +TMP_ROOT=$(fm_test_tmproot fm-guard-session-lock-repair) + +# A `ps` stub that answers every comm=/args= query as a harness process +# ("opencode"), so fm-lock.sh's ancestry walk - run from inside a real +# fm-guard.sh subprocess whose pid cannot be known ahead of time - finds a +# match on its very first hop and stops there (non-Claude harnesses never +# extend past their own match). ppid= is never consulted after that first +# match, so its value is immaterial. +make_harness_ps_fakebin() { + local dir=$1 fakebin + fakebin=$(fm_fakebin "$dir") + cat > "$fakebin/ps" <<'SH' +#!/usr/bin/env bash +set -u +field= +while [ "$#" -gt 0 ]; do + case "$1" in + -o) field=$2; shift 2 ;; + -p) shift 2 ;; + *) shift ;; + esac +done +case "$field" in + comm=) printf '%s\n' opencode ;; + args=) printf '%s\n' opencode ;; + ppid=) printf '%s\n' 1 ;; +esac +SH + chmod +x "$fakebin/ps" + printf '%s\n' "$fakebin" +} + +test_guard_repairs_missing_session_lock() { + local dir home root fakebin + dir="$TMP_ROOT/repair" + home="$dir/home" + root="$dir/root" + mkdir -p "$home/state" "$home/config" "$root" + fakebin=$(make_harness_ps_fakebin "$dir") + + [ ! -e "$home/state/.lock" ] || fail "setup: state/.lock must start absent" + + PATH="$fakebin:$PATH" \ + FM_ROOT_OVERRIDE="$root" \ + FM_HOME="$home" \ + FM_GUARD_GRACE=999 \ + "$ROOT/bin/fm-guard.sh" >/dev/null 2>&1 + + [ -f "$home/state/.lock" ] || fail "a missing state/.lock was not repaired by fm-guard.sh" + [ ! -L "$home/state/.lock" ] || fail "the repaired state/.lock must be a regular file, not a symlink" + grep -qE '^[0-9]+$' "$home/state/.lock" || fail "the repaired state/.lock must hold a plain pid, got: $(cat "$home/state/.lock" 2>/dev/null)" + pass "fm-guard: repairs a missing state/.lock while a harness is plainly still active" +} + +test_guard_read_only_never_repairs_missing_session_lock() { + local dir home root fakebin + dir="$TMP_ROOT/read-only" + home="$dir/home" + root="$dir/root" + mkdir -p "$home/state" "$home/config" "$root" + fakebin=$(make_harness_ps_fakebin "$dir") + + PATH="$fakebin:$PATH" \ + FM_ROOT_OVERRIDE="$root" \ + FM_HOME="$home" \ + FM_GUARD_GRACE=999 \ + FM_GUARD_READ_ONLY=1 \ + "$ROOT/bin/fm-guard.sh" >/dev/null 2>&1 + + [ ! -e "$home/state/.lock" ] || fail "a read-only guard call must never repair state/.lock (ownership was never verified this call)" + pass "fm-guard: a read-only call leaves a missing state/.lock alone" +} + +test_guard_never_touches_an_existing_foreign_lock() { + local dir home root fakebin before after + dir="$TMP_ROOT/foreign-owner" + home="$dir/home" + root="$dir/root" + mkdir -p "$home/state" "$home/config" "$root" + fakebin=$(make_harness_ps_fakebin "$dir") + + # A lock recorded by some other, unrelated live session. sleep is a real, + # live process this test controls, so fm_harness_pid_alive-style liveness + # checks elsewhere in the tree cannot mistake it for gone. + sleep 300 & + local foreign_pid=$! + trap 'kill "$foreign_pid" 2>/dev/null || true' RETURN + printf '%s\n' "$foreign_pid" > "$home/state/.lock" + before=$(cat "$home/state/.lock") + + PATH="$fakebin:$PATH" \ + FM_ROOT_OVERRIDE="$root" \ + FM_HOME="$home" \ + FM_GUARD_GRACE=999 \ + "$ROOT/bin/fm-guard.sh" >/dev/null 2>&1 + + after=$(cat "$home/state/.lock" 2>/dev/null || true) + [ "$before" = "$after" ] || fail "fm-guard.sh must never rewrite an existing state/.lock, foreign or not; was '$before', now '$after'" + kill "$foreign_pid" 2>/dev/null || true + pass "fm-guard: an existing lock (foreign live owner) is left completely alone, so its refusal keeps failing exactly as fast as before" +} + +test_guard_repairs_missing_session_lock +test_guard_read_only_never_repairs_missing_session_lock +test_guard_never_touches_an_existing_foreign_lock From 713aaf9d5f6cb2f193d7ae49a78543df69269010 Mon Sep 17 00:00:00 2001 From: Marcus Moreira Date: Mon, 28 Sep 2026 11:10:53 -0300 Subject: [PATCH 11/19] no-mistakes(review): Limit guard session-lock repair to the main actor --- bin/fm-guard.sh | 32 +++++++++++----------- tests/fm-guard-session-lock-repair.test.sh | 20 ++++++++++++++ 2 files changed, 36 insertions(+), 16 deletions(-) diff --git a/bin/fm-guard.sh b/bin/fm-guard.sh index 2ea8132cad2..2956efda926 100755 --- a/bin/fm-guard.sh +++ b/bin/fm-guard.sh @@ -146,22 +146,22 @@ fm_guard_clear_stale_banner() { } # Repair a session lock that has gone missing out from under a still-live -# harness. This guard only ever runs non-read-only (READ_ONLY=0) when an -# earlier ownership check in THIS call chain already succeeded (fm-guard.sh's -# own callers gate that), so a harness is plainly still active in this -# process's own ancestry right now. If state/.lock has since vanished - the -# literal "no session lock present" gap a live watcher-arm plugin's -# sessionOwnsLock check reads as ownerless - bin/fm-lock.sh is the one place -# that may write it, and its acquire path already does exactly the right -# thing when the file is simply absent: resolve this session's own trusted -# anchor pid and write it fresh. Only the missing case is touched; a lock -# file that already exists, whether held by this session or a genuine foreign -# live owner, is never read, written, or otherwise disturbed here, so -# fm-lock.sh's tested foreign-live-owner refusal keeps failing exactly as -# fast as before. Best-effort and silent: a repair failure (no verifiable -# harness in the ancestry, an unwritable state dir) leaves the gap exactly as -# it was and is not this guard's alarm to raise. -if [ "$READ_ONLY" -eq 0 ] && [ ! -e "$STATE/.lock" ] && [ ! -L "$STATE/.lock" ]; then +# main-session harness. Callers do not verify ownership before invoking this +# guard, so the repair is limited to the main actor: a supervision-branch +# actor runs in its own harness and must never record itself as the home's +# session owner. If state/.lock has vanished - the literal "no session lock +# present" gap a live watcher-arm plugin's sessionOwnsLock check reads as +# ownerless - bin/fm-lock.sh is the one place that may write it, and its +# acquire path resolves this session's own trusted anchor pid and writes it +# fresh. Only the missing case is touched; a lock file that already exists, +# whether held by this session or a genuine foreign live owner, is never +# read, written, or otherwise disturbed here, so fm-lock.sh's +# foreign-live-owner refusal keeps failing exactly as fast as before. +# Best-effort and silent: a repair failure (no verifiable harness in the +# ancestry, an unwritable state dir) leaves the gap exactly as it was and is +# not this guard's alarm to raise. +if [ "$READ_ONLY" -eq 0 ] && [ "$GUARD_ACTOR" = main ] \ + && [ ! -e "$STATE/.lock" ] && [ ! -L "$STATE/.lock" ]; then "$SCRIPT_DIR/fm-lock.sh" >/dev/null 2>&1 || true fi diff --git a/tests/fm-guard-session-lock-repair.test.sh b/tests/fm-guard-session-lock-repair.test.sh index 844a305d84f..ddd85e61fe9 100755 --- a/tests/fm-guard-session-lock-repair.test.sh +++ b/tests/fm-guard-session-lock-repair.test.sh @@ -110,6 +110,26 @@ test_guard_never_touches_an_existing_foreign_lock() { pass "fm-guard: an existing lock (foreign live owner) is left completely alone, so its refusal keeps failing exactly as fast as before" } +test_guard_branch_actor_never_repairs_missing_session_lock() { + local dir home root fakebin + dir="$TMP_ROOT/branch-actor" + home="$dir/home" + root="$dir/root" + mkdir -p "$home/state" "$home/config" "$root" + fakebin=$(make_harness_ps_fakebin "$dir") + + PATH="$fakebin:$PATH" \ + FM_ROOT_OVERRIDE="$root" \ + FM_HOME="$home" \ + FM_GUARD_GRACE=999 \ + FM_SUPERVISION_ACTOR=branch \ + "$ROOT/bin/fm-guard.sh" >/dev/null 2>&1 + + [ ! -e "$home/state/.lock" ] || fail "a supervision-branch guard call must never write itself into state/.lock" + pass "fm-guard: a supervision-branch call leaves a missing state/.lock alone" +} + test_guard_repairs_missing_session_lock test_guard_read_only_never_repairs_missing_session_lock +test_guard_branch_actor_never_repairs_missing_session_lock test_guard_never_touches_an_existing_foreign_lock From 3bf81b7d9e591ef24bd4531430f4e111a66b5b24 Mon Sep 17 00:00:00 2001 From: Marcus Moreira Date: Mon, 28 Sep 2026 11:19:49 -0300 Subject: [PATCH 12/19] no-mistakes(review): Move missing-lock repair out of guard to owner path --- bin/fm-guard.sh | 20 ---- tests/fm-guard-session-lock-repair.test.sh | 123 +++++++++------------ 2 files changed, 52 insertions(+), 91 deletions(-) diff --git a/bin/fm-guard.sh b/bin/fm-guard.sh index 2956efda926..ee922dc509e 100755 --- a/bin/fm-guard.sh +++ b/bin/fm-guard.sh @@ -145,26 +145,6 @@ fm_guard_clear_stale_banner() { rm -f "$STALE_BANNER_MARKER" 2>/dev/null || true } -# Repair a session lock that has gone missing out from under a still-live -# main-session harness. Callers do not verify ownership before invoking this -# guard, so the repair is limited to the main actor: a supervision-branch -# actor runs in its own harness and must never record itself as the home's -# session owner. If state/.lock has vanished - the literal "no session lock -# present" gap a live watcher-arm plugin's sessionOwnsLock check reads as -# ownerless - bin/fm-lock.sh is the one place that may write it, and its -# acquire path resolves this session's own trusted anchor pid and writes it -# fresh. Only the missing case is touched; a lock file that already exists, -# whether held by this session or a genuine foreign live owner, is never -# read, written, or otherwise disturbed here, so fm-lock.sh's -# foreign-live-owner refusal keeps failing exactly as fast as before. -# Best-effort and silent: a repair failure (no verifiable harness in the -# ancestry, an unwritable state dir) leaves the gap exactly as it was and is -# not this guard's alarm to raise. -if [ "$READ_ONLY" -eq 0 ] && [ "$GUARD_ACTOR" = main ] \ - && [ ! -e "$STATE/.lock" ] && [ ! -L "$STATE/.lock" ]; then - "$SCRIPT_DIR/fm-lock.sh" >/dev/null 2>&1 || true -fi - # Worktree-tangle alarm, checked FIRST and independent of in-flight tasks: the # firstmate PRIMARY checkout (FM_ROOT) must stay on its default branch. If a # crewmate's branch/commits landed here instead of in its own isolated worktree, diff --git a/tests/fm-guard-session-lock-repair.test.sh b/tests/fm-guard-session-lock-repair.test.sh index ddd85e61fe9..e1228d6760b 100755 --- a/tests/fm-guard-session-lock-repair.test.sh +++ b/tests/fm-guard-session-lock-repair.test.sh @@ -1,8 +1,8 @@ #!/usr/bin/env bash -# tests/fm-guard-session-lock-repair.test.sh - fm-guard.sh repairs a missing -# state/.lock while a harness is plainly still active, but never touches an -# existing one (successor-gap-decision Option B: leave beginArm/sessionOwnsLock -# untouched; the guard is the repair point instead). +# tests/fm-guard-session-lock-repair.test.sh - a missing state/.lock is +# repaired only on the verified-owner path (bin/fm-lock.sh, the session-start +# LOCK step), never by fm-guard.sh, which runs from guarded commands that never +# checked ownership; an existing foreign live lock is never rewritten. set -u # shellcheck source=tests/lib.sh @@ -12,7 +12,7 @@ TMP_ROOT=$(fm_test_tmproot fm-guard-session-lock-repair) # A `ps` stub that answers every comm=/args= query as a harness process # ("opencode"), so fm-lock.sh's ancestry walk - run from inside a real -# fm-guard.sh subprocess whose pid cannot be known ahead of time - finds a +# subprocess whose pid cannot be known ahead of time - finds a # match on its very first hop and stops there (non-Claude harnesses never # extend past their own match). ppid= is never consulted after that first # match, so its value is immaterial. @@ -40,96 +40,77 @@ SH printf '%s\n' "$fakebin" } -test_guard_repairs_missing_session_lock() { - local dir home root fakebin - dir="$TMP_ROOT/repair" - home="$dir/home" - root="$dir/root" - mkdir -p "$home/state" "$home/config" "$root" - fakebin=$(make_harness_ps_fakebin "$dir") - - [ ! -e "$home/state/.lock" ] || fail "setup: state/.lock must start absent" - - PATH="$fakebin:$PATH" \ +run_guard() { + local home=$1 root=$2 fakebin=$3 + shift 3 + env PATH="$fakebin:$PATH" \ FM_ROOT_OVERRIDE="$root" \ FM_HOME="$home" \ FM_GUARD_GRACE=999 \ - "$ROOT/bin/fm-guard.sh" >/dev/null 2>&1 + "$@" "$ROOT/bin/fm-guard.sh" >/dev/null 2>&1 +} - [ -f "$home/state/.lock" ] || fail "a missing state/.lock was not repaired by fm-guard.sh" - [ ! -L "$home/state/.lock" ] || fail "the repaired state/.lock must be a regular file, not a symlink" - grep -qE '^[0-9]+$' "$home/state/.lock" || fail "the repaired state/.lock must hold a plain pid, got: $(cat "$home/state/.lock" 2>/dev/null)" - pass "fm-guard: repairs a missing state/.lock while a harness is plainly still active" +setup_home() { + local dir=$1 + mkdir -p "$dir/home/state" "$dir/home/config" "$dir/root" } -test_guard_read_only_never_repairs_missing_session_lock() { - local dir home root fakebin - dir="$TMP_ROOT/read-only" +test_guard_never_repairs_missing_session_lock() { + local dir fakebin mode + for mode in main read-only branch; do + dir="$TMP_ROOT/guard-$mode" + setup_home "$dir" + fakebin=$(make_harness_ps_fakebin "$dir") + case "$mode" in + main) run_guard "$dir/home" "$dir/root" "$fakebin" ;; + read-only) run_guard "$dir/home" "$dir/root" "$fakebin" FM_GUARD_READ_ONLY=1 ;; + branch) run_guard "$dir/home" "$dir/root" "$fakebin" FM_SUPERVISION_ACTOR=branch ;; + esac + [ ! -e "$dir/home/state/.lock" ] || fail "fm-guard.sh ($mode) must never write state/.lock; guarded callers never verified ownership" + done + pass "fm-guard: never repairs a missing state/.lock (main, read-only, or supervision-branch caller)" +} + +test_owner_path_repairs_missing_session_lock() { + local dir home fakebin + dir="$TMP_ROOT/owner-repair" + setup_home "$dir" home="$dir/home" - root="$dir/root" - mkdir -p "$home/state" "$home/config" "$root" fakebin=$(make_harness_ps_fakebin "$dir") - PATH="$fakebin:$PATH" \ - FM_ROOT_OVERRIDE="$root" \ - FM_HOME="$home" \ - FM_GUARD_GRACE=999 \ - FM_GUARD_READ_ONLY=1 \ - "$ROOT/bin/fm-guard.sh" >/dev/null 2>&1 + PATH="$fakebin:$PATH" FM_HOME="$home" "$ROOT/bin/fm-lock.sh" >/dev/null 2>&1 \ + || fail "fm-lock.sh must acquire an absent state/.lock for a live harness" - [ ! -e "$home/state/.lock" ] || fail "a read-only guard call must never repair state/.lock (ownership was never verified this call)" - pass "fm-guard: a read-only call leaves a missing state/.lock alone" + [ -f "$home/state/.lock" ] || fail "a missing state/.lock was not repaired by the owner path" + [ ! -L "$home/state/.lock" ] || fail "the repaired state/.lock must be a regular file, not a symlink" + grep -qE '^[0-9]+$' "$home/state/.lock" || fail "the repaired state/.lock must hold a plain pid, got: $(cat "$home/state/.lock" 2>/dev/null)" + pass "fm-lock: the verified-owner path repairs a missing state/.lock" } -test_guard_never_touches_an_existing_foreign_lock() { - local dir home root fakebin before after +test_owner_path_refuses_an_existing_foreign_lock() { + local dir home fakebin before after rc dir="$TMP_ROOT/foreign-owner" + setup_home "$dir" home="$dir/home" - root="$dir/root" - mkdir -p "$home/state" "$home/config" "$root" fakebin=$(make_harness_ps_fakebin "$dir") - # A lock recorded by some other, unrelated live session. sleep is a real, - # live process this test controls, so fm_harness_pid_alive-style liveness - # checks elsewhere in the tree cannot mistake it for gone. sleep 300 & local foreign_pid=$! trap 'kill "$foreign_pid" 2>/dev/null || true' RETURN printf '%s\n' "$foreign_pid" > "$home/state/.lock" before=$(cat "$home/state/.lock") - PATH="$fakebin:$PATH" \ - FM_ROOT_OVERRIDE="$root" \ - FM_HOME="$home" \ - FM_GUARD_GRACE=999 \ - "$ROOT/bin/fm-guard.sh" >/dev/null 2>&1 + PATH="$fakebin:$PATH" FM_HOME="$home" "$ROOT/bin/fm-lock.sh" >/dev/null 2>&1 + rc=$? + run_guard "$home" "$dir/root" "$fakebin" after=$(cat "$home/state/.lock" 2>/dev/null || true) - [ "$before" = "$after" ] || fail "fm-guard.sh must never rewrite an existing state/.lock, foreign or not; was '$before', now '$after'" + [ "$rc" -ne 0 ] || fail "fm-lock.sh must refuse a lock held by another live session" + [ "$before" = "$after" ] || fail "an existing foreign state/.lock must never be rewritten; was '$before', now '$after'" kill "$foreign_pid" 2>/dev/null || true - pass "fm-guard: an existing lock (foreign live owner) is left completely alone, so its refusal keeps failing exactly as fast as before" -} - -test_guard_branch_actor_never_repairs_missing_session_lock() { - local dir home root fakebin - dir="$TMP_ROOT/branch-actor" - home="$dir/home" - root="$dir/root" - mkdir -p "$home/state" "$home/config" "$root" - fakebin=$(make_harness_ps_fakebin "$dir") - - PATH="$fakebin:$PATH" \ - FM_ROOT_OVERRIDE="$root" \ - FM_HOME="$home" \ - FM_GUARD_GRACE=999 \ - FM_SUPERVISION_ACTOR=branch \ - "$ROOT/bin/fm-guard.sh" >/dev/null 2>&1 - - [ ! -e "$home/state/.lock" ] || fail "a supervision-branch guard call must never write itself into state/.lock" - pass "fm-guard: a supervision-branch call leaves a missing state/.lock alone" + pass "fm-lock/fm-guard: an existing foreign live lock is refused and left untouched" } -test_guard_repairs_missing_session_lock -test_guard_read_only_never_repairs_missing_session_lock -test_guard_branch_actor_never_repairs_missing_session_lock -test_guard_never_touches_an_existing_foreign_lock +test_guard_never_repairs_missing_session_lock +test_owner_path_repairs_missing_session_lock +test_owner_path_refuses_an_existing_foreign_lock From 7298de81efb3d685100865c515fda5b0939ad580 Mon Sep 17 00:00:00 2001 From: Marcus Moreira Date: Mon, 28 Sep 2026 11:40:02 -0300 Subject: [PATCH 13/19] no-mistakes(test): Fix empty-queue bounded-reopen test to expect watcher stays up --- tests/fm-watch-arm.test.sh | 32 ++++++++++++++++++++++++++++++-- 1 file changed, 30 insertions(+), 2 deletions(-) diff --git a/tests/fm-watch-arm.test.sh b/tests/fm-watch-arm.test.sh index 1578ae162d2..8b6934685eb 100755 --- a/tests/fm-watch-arm.test.sh +++ b/tests/fm-watch-arm.test.sh @@ -1440,9 +1440,37 @@ test_late_genuine_ack_after_bound_settle_still_resurfaces_queued_row() { pass "watch-arm: a genuine ack after a bound settle still resurfaces a newer queued row on re-arm" } +# With an empty queue an announced-but-unacknowledged episode is never +# reopened: every plain restart leaves that generation announced, starts no +# resurface, and keeps the watcher up without spending the reopen bound. test_stuck_unacked_recovery_settles_after_bounded_reopen() { - check_stuck_unacked_recovery_settles bounded-reopen 0 - pass "watch-arm: a stuck unacknowledged recovery episode settles after a bounded number of reopens instead of looping forever" + local dir home state fakebin i + local FM_RECOVERY_REOPEN_LIMIT=2 + export FM_RECOVERY_REOPEN_LIMIT + dir=$(make_case bounded-reopen) + home="$dir/home" + state="$dir/state" + fakebin="$dir/fakebin" + mkdir -p "$home/data" + printf 'announced:downtime:seedgen1\n' > "$state/.watcher-down" + chmod 0600 "$state/.watcher-down" + + i=0 + while [ "$i" -le "$FM_RECOVERY_REOPEN_LIMIT" ]; do + i=$((i + 1)) + start_rearm_arm "$home" "$state" "$fakebin" "$dir/arm-$i.out" + is_live_non_zombie "$ARM_PID" \ + || fail "empty-queue restart $i did not keep the watcher up: $(cat "$dir/arm-$i.out")" + ! grep -F 'check: rearm-resurface' "$dir/arm-$i.out" >/dev/null \ + || fail "empty-queue restart $i resurfaced an episode with nothing queued: $(cat "$dir/arm-$i.out")" + [ "$(cat "$state/.watcher-down" 2>/dev/null || true)" = 'announced:downtime:seedgen1' ] \ + || fail "empty-queue restart $i changed the announced generation: $(cat "$state/.watcher-down" 2>/dev/null)" + [ ! -e "$state/.watcher-down.reopen-count" ] \ + || fail "empty-queue restart $i spent the reopen bound" + kill "$ARM_PID" 2>/dev/null || true + wait "$ARM_PID" 2>/dev/null || true + done + pass "watch-arm: a stuck unacknowledged recovery episode with an empty queue keeps the watcher up on every restart" } test_stuck_unacked_recovery_with_queued_rows_stays_up_after_settling() { From e7f88afe1152c4a39f131a3f5beb98565c6968f6 Mon Sep 17 00:00:00 2001 From: Marcus Moreira Date: Mon, 28 Sep 2026 11:49:59 -0300 Subject: [PATCH 14/19] no-mistakes(test): Keep bound-settled recovery episode across watcher restart close --- bin/fm-wake-lib.sh | 21 +++++++++++++++------ docs/watcher-continuity.md | 1 + tests/fm-watch-arm.test.sh | 18 +++++++++++++++--- 3 files changed, 31 insertions(+), 9 deletions(-) diff --git a/bin/fm-wake-lib.sh b/bin/fm-wake-lib.sh index a8da03b66b2..4fe30cd0d53 100755 --- a/bin/fm-wake-lib.sh +++ b/bin/fm-wake-lib.sh @@ -709,12 +709,14 @@ _fm_recovery_marker_write_locked() { } # Apply the downtime republication states owned by docs/watcher-continuity.md -# while preserving an outstanding generation-bound acknowledgement. +# while preserving an outstanding generation-bound acknowledgement. A watcher +# lock ending (source close) keeps an episode the reopen bound settled, so the +# next restart does not reopen that same unwatched stretch all over again. _fm_recovery_marker_publish() { local marker=$1 kind=${2:-downtime} bound=${3:-} source=${4:-watcher} local lock saved_token generation='' status=pending previous_append_token='' case "$kind" in handling|downtime) ;; *) return 1 ;; esac - case "$source" in watcher|append) ;; *) return 1 ;; esac + case "$source" in watcher|close|append) ;; *) return 1 ;; esac if [ "$source" = append ]; then FM_WAKE_APPEND_RECOVERY_PREVIOUS_TOKEN= FM_WAKE_APPEND_RECOVERY_PUBLISHED_TOKEN= @@ -748,11 +750,18 @@ _fm_recovery_marker_publish() { status=pending ;; announced:downtime:*) - if [ "$source" = watcher ]; then + if [ "$source" != append ]; then generation=${FM_RECOVERY_MARKER_TOKEN##*:} status=announced fi ;; + acked:downtime:*) + if [ "$source" = close ] \ + && [ "$(cat "${marker}.reopen-settled" 2>/dev/null || true)" = "${FM_RECOVERY_MARKER_TOKEN##*:}" ]; then + generation=${FM_RECOVERY_MARKER_TOKEN##*:} + status=acked + fi + ;; esac fi FM_RECOVERY_MARKER_TOKEN=$saved_token @@ -1052,7 +1061,7 @@ fm_recovery_transition() { ;; release-lock) [ -n "$target" ] || return 1 - _fm_recovery_marker_publish "$marker" "${value:-downtime}" "$bound" || return 1 + _fm_recovery_marker_publish "$marker" "${value:-downtime}" "$bound" close || return 1 fm_lock_release "$target" ;; release-lock-existing) @@ -1072,7 +1081,7 @@ fm_recovery_transition() { ;; clear-stale-lock) [ -n "$target" ] || return 1 - _fm_recovery_marker_publish "$marker" "${value:-downtime}" "$bound" || return 1 + _fm_recovery_marker_publish "$marker" "${value:-downtime}" "$bound" close || return 1 fm_lock_remove_path "$target" ;; *) return 2 ;; @@ -1233,7 +1242,7 @@ fm_lock_try_acquire() { fi if [ "$lockdir" = "$STATE/.watch.lock" ] \ - && ! _fm_recovery_marker_publish "$STATE/.watcher-down" downtime; then + && ! _fm_recovery_marker_publish "$STATE/.watcher-down" downtime "" close; then fm_lock_release "$steal" FM_LOCK_HELD_PID=$cur FM_LOCK_OWNER_DIR= diff --git a/docs/watcher-continuity.md b/docs/watcher-continuity.md index b84e5559484..1ce3f76a8c0 100644 --- a/docs/watcher-continuity.md +++ b/docs/watcher-continuity.md @@ -225,6 +225,7 @@ So a plain restart with no re-arm loop and no session would otherwise reopen the Past `FM_RECOVERY_REOPEN_LIMIT` (default 3) consecutive reopens of one episode with no intervening explicit acknowledgement, the next reopen settles the episode to acked directly instead of minting another generation, so the watcher can finally start and stay up. The settle records that generation in `state/.watcher-down.reopen-settled`. A watcher start does not re-announce that bound-settled episode just because the queue is non-empty, so its queued rows cannot make the watcher exit; they stay durable and the next session's drain presents them. +When a watcher's lock later ends (a `--restart` retiring it, or a stale lock cleared), that close keeps the bound-settled episode instead of publishing a fresh downtime generation, so the next restart stays up too. A genuinely acknowledged episode with queued rows still re-announces and resurfaces them on the next arm. A real acknowledgement, or a watcher start that mints a fresh episode from a missing or invalid marker, clears the counter, so this bound never shortens the once-per-genuine-generation resurface a live, attentive session relies on. diff --git a/tests/fm-watch-arm.test.sh b/tests/fm-watch-arm.test.sh index 8b6934685eb..396a6a3a8ec 100755 --- a/tests/fm-watch-arm.test.sh +++ b/tests/fm-watch-arm.test.sh @@ -1325,7 +1325,7 @@ test_reaper_stops_a_tracked_watcher() { # With queued rows still unacknowledged, settling must not re-announce them on # a watcher start either: the rows stay durable for the next session's drain. check_stuck_unacked_recovery_settles() { # - local dir home state fakebin queued=$2 i + local dir home state fakebin queued=$2 i settled_pid settled_token local FM_RECOVERY_REOPEN_LIMIT=2 export FM_RECOVERY_REOPEN_LIMIT dir=$(make_case "$1") @@ -1366,8 +1366,20 @@ check_stuck_unacked_recovery_settles() { # esac [ ! -e "$state/.watcher-down.reopen-count" ] \ || fail "$1: reopen counter was not cleared once the episode settled" - kill "$ARM_PID" 2>/dev/null || true - wait "$ARM_PID" 2>/dev/null || true + settled_pid=$ARM_PID + settled_token=$(cat "$state/.watcher-down" 2>/dev/null || true) + + # Restarting the healthy settled watcher retires it; that close must keep the + # settled episode rather than reopen it into another resurface-and-exit cycle. + start_rearm_arm "$home" "$state" "$fakebin" "$dir/restart-after-settle-arm.out" + is_live_non_zombie "$ARM_PID" \ + || fail "$1: restarting a bound-settled watcher did not keep supervision up: $(cat "$dir/restart-after-settle-arm.out")" + ! grep -F 'check: rearm-resurface' "$dir/restart-after-settle-arm.out" >/dev/null \ + || fail "$1: restarting a bound-settled watcher resurfaced the settled episode: $(cat "$dir/restart-after-settle-arm.out")" + [ "$(cat "$state/.watcher-down" 2>/dev/null || true)" = "$settled_token" ] \ + || fail "$1: restarting a bound-settled watcher changed its settled marker: $(cat "$state/.watcher-down" 2>/dev/null)" + kill "$ARM_PID" "$settled_pid" 2>/dev/null || true + wait "$ARM_PID" "$settled_pid" 2>/dev/null || true if [ "$queued" = 1 ]; then grep "$(printf '\tcheck\tstuck-queued\t')" "$state/.wake-queue" >/dev/null \ From 7d0d91529a612ea3223ef91b1c4bd6a302505042 Mon Sep 17 00:00:00 2001 From: Marcus Moreira Date: Mon, 28 Sep 2026 12:46:27 -0300 Subject: [PATCH 15/19] no-mistakes(ci): I fixed the four Greptile findings. All the tests I ran pass. I ran the two new watcher tests against the old code, and both failed there. - **ci-3 (lock repair reachable from other callers):** No code change was needed. `bin/fm-guard.sh` is the same as on base, so the guard never calls `fm-lock.sh`. `bin/fm-remote-secondmate-control.sh` does not call `fm-lock.sh` either. Only `bin/fm-lock.sh`'s own acquire, run from the LOCK step in `bin/fm-session-start.sh`, can write a missing `state/.lock`. The test `tests/fm-guard-session-lock-repair.test.sh` checks that the guard never writes it (main, read-only and branch callers). - **ci-4 (retried empty ack):** In `bin/fm-wake-drain.sh`, the snapshot gets the ack generation only when the ack removed at least one row (`ACK_REMOVED > 0`). So a retried stale ack that removes nothing keeps `.watcher-down.reopen-settled`. The ack path that empties the queue (`fm_recovery_marker_ack`) is unchanged, as the earlier R3-2 fix requires. New test: `test_stale_zero_row_ack_keeps_bound_settle`. It checks that the sentinel stays and that the next watcher stays up without a resurface. - **ci-5 (non-numeric limit):** In `bin/fm-wake-lib.sh`, a non-numeric `FM_RECOVERY_REOPEN_LIMIT` now falls back to the default of 3. New test: `test_invalid_reopen_limit_falls_back_to_default`. It sets the limit to `three` and checks that the episode settles and the watcher stays up. - **ci-6 (repair test accepted a dead pid):** The `ps` stub now calls a pid a harness only if that pid is in a list of live harness pids. It sends every other query to the real `ps`. `fm-lock.sh` runs as a child of a live harness process. The test checks that `state/.lock` holds that harness pid and that the pid is alive. The foreign-owner test runs the same way. Checks: - shellcheck is clean on the four changed files. - `tests/fm-guard-session-lock-repair.test.sh`: 3 of 3 pass. - The 6 recovery tests in `tests/fm-watch-arm.test.sh`: all pass. I ran them alone, because on this host the full file stops early on a test that also fails on base (T1). - All four `tests/fm-wake-drain*.test.sh` files pass --- bin/fm-wake-drain.sh | 6 +- bin/fm-wake-lib.sh | 1 + tests/fm-guard-session-lock-repair.test.sh | 94 +++++++++++++++------- tests/fm-watch-arm.test.sh | 61 ++++++++++++++ 4 files changed, 132 insertions(+), 30 deletions(-) diff --git a/bin/fm-wake-drain.sh b/bin/fm-wake-drain.sh index 70edfc40ee4..c22bf0217f4 100755 --- a/bin/fm-wake-drain.sh +++ b/bin/fm-wake-drain.sh @@ -927,7 +927,11 @@ if [ -n "$ACK_THROUGH" ]; then ;; esac else - fm_recovery_marker_snapshot "$RECOVERY_MARKER" "$ACK_GENERATION" || exit 1 + # Only an ack that consumed rows retires the reopen bound's settle + # distinction; a retried stale ack that consumed nothing must not. + SNAPSHOT_ACK_GENERATION= + [ "$ACK_REMOVED" -gt 0 ] && SNAPSHOT_ACK_GENERATION=$ACK_GENERATION + fm_recovery_marker_snapshot "$RECOVERY_MARKER" "$SNAPSHOT_ACK_GENERATION" || exit 1 RECOVERY_MARKER_TOKEN=$FM_RECOVERY_MARKER_TOKEN if [ "${RECOVERY_MARKER_TOKEN##*:}" != "$ACK_GENERATION" ]; then RECOVERY_ACK_MOVED=true diff --git a/bin/fm-wake-lib.sh b/bin/fm-wake-lib.sh index 4fe30cd0d53..bfab8e3ddc7 100755 --- a/bin/fm-wake-lib.sh +++ b/bin/fm-wake-lib.sh @@ -989,6 +989,7 @@ _fm_recovery_marker_arm_check() { # or invalid marker both clear the counter, so this bound never shortens the # once-per-genuine-generation resurface a live, attentive session relies on. FM_RECOVERY_REOPEN_LIMIT=${FM_RECOVERY_REOPEN_LIMIT:-3} +case "$FM_RECOVERY_REOPEN_LIMIT" in ''|*[!0-9]*) FM_RECOVERY_REOPEN_LIMIT=3 ;; esac _fm_recovery_marker_reopen_announced() { local marker=$1 lock counter_file count generation diff --git a/tests/fm-guard-session-lock-repair.test.sh b/tests/fm-guard-session-lock-repair.test.sh index e1228d6760b..95a4e088af7 100755 --- a/tests/fm-guard-session-lock-repair.test.sh +++ b/tests/fm-guard-session-lock-repair.test.sh @@ -10,36 +10,64 @@ set -u TMP_ROOT=$(fm_test_tmproot fm-guard-session-lock-repair) -# A `ps` stub that answers every comm=/args= query as a harness process -# ("opencode"), so fm-lock.sh's ancestry walk - run from inside a real -# subprocess whose pid cannot be known ahead of time - finds a -# match on its very first hop and stops there (non-Claude harnesses never -# extend past their own match). ppid= is never consulted after that first -# match, so its value is immaterial. +# A `ps` stub that names only the pids listed in $FM_TEST_HARNESS_PIDS as a +# harness ("opencode") and defers every other query to the real ps, so +# fm-lock.sh's ancestry walk must climb its real process tree to a live harness +# ancestor rather than match its own short-lived shell. make_harness_ps_fakebin() { - local dir=$1 fakebin + local dir=$1 fakebin real_ps fakebin=$(fm_fakebin "$dir") - cat > "$fakebin/ps" <<'SH' + real_ps=$(command -v ps) + cat > "$fakebin/ps" </dev/null; then + printf '%s\\n' opencode + exit 0 + fi + ;; esac +exec "$real_ps" "\${args[@]}" SH chmod +x "$fakebin/ps" printf '%s\n' "$fakebin" } +# Run bin/fm-lock.sh as a child of a live harness process that outlives it. +# Sets HARNESS_PID and LOCK_RC. The harness waits until its pid is registered +# in the harness list before running fm-lock.sh, then stays alive. +run_lock_under_harness() { + local home=$1 fakebin=$2 pids=$3 dir=$4 i + rm -f "$dir/lock.rc" + # shellcheck disable=SC2016 # expanded by the harness shell, not here + env PATH="$fakebin:$PATH" FM_HOME="$home" FM_TEST_HARNESS_PIDS="$pids" \ + LOCK="$ROOT/bin/fm-lock.sh" RC="$dir/lock.rc" bash -c ' + while ! grep -qx "$$" "$FM_TEST_HARNESS_PIDS" 2>/dev/null; do sleep 0.05; done + "$LOCK" >/dev/null 2>&1 + printf "%s\n" "$?" > "$RC" + exec sleep 300 + ' & + HARNESS_PID=$! + printf '%s\n' "$HARNESS_PID" >> "$pids" + i=0 + while [ ! -s "$dir/lock.rc" ] && [ "$i" -lt 400 ]; do + sleep 0.05 + i=$((i + 1)) + done + LOCK_RC=$(cat "$dir/lock.rc" 2>/dev/null || echo timeout) +} + run_guard() { local home=$1 root=$2 fakebin=$3 shift 3 @@ -72,42 +100,50 @@ test_guard_never_repairs_missing_session_lock() { } test_owner_path_repairs_missing_session_lock() { - local dir home fakebin + local dir home fakebin pids lock_pid dir="$TMP_ROOT/owner-repair" setup_home "$dir" home="$dir/home" fakebin=$(make_harness_ps_fakebin "$dir") + pids="$dir/harness-pids" + : > "$pids" - PATH="$fakebin:$PATH" FM_HOME="$home" "$ROOT/bin/fm-lock.sh" >/dev/null 2>&1 \ - || fail "fm-lock.sh must acquire an absent state/.lock for a live harness" + run_lock_under_harness "$home" "$fakebin" "$pids" "$dir" + trap 'kill "$HARNESS_PID" 2>/dev/null || true' RETURN + [ "$LOCK_RC" = 0 ] || fail "fm-lock.sh must acquire an absent state/.lock for a live harness (rc=$LOCK_RC)" [ -f "$home/state/.lock" ] || fail "a missing state/.lock was not repaired by the owner path" [ ! -L "$home/state/.lock" ] || fail "the repaired state/.lock must be a regular file, not a symlink" - grep -qE '^[0-9]+$' "$home/state/.lock" || fail "the repaired state/.lock must hold a plain pid, got: $(cat "$home/state/.lock" 2>/dev/null)" - pass "fm-lock: the verified-owner path repairs a missing state/.lock" + lock_pid=$(head -n 1 "$home/state/.lock" 2>/dev/null || true) + [ "$lock_pid" = "$HARNESS_PID" ] || fail "the repaired state/.lock must record the live harness pid $HARNESS_PID, got: $lock_pid" + kill -0 "$lock_pid" 2>/dev/null || fail "the repaired state/.lock records pid $lock_pid, which is not alive" + kill "$HARNESS_PID" 2>/dev/null || true + pass "fm-lock: the verified-owner path repairs a missing state/.lock with the live harness pid" } test_owner_path_refuses_an_existing_foreign_lock() { - local dir home fakebin before after rc + local dir home fakebin pids before after dir="$TMP_ROOT/foreign-owner" setup_home "$dir" home="$dir/home" fakebin=$(make_harness_ps_fakebin "$dir") + pids="$dir/harness-pids" sleep 300 & local foreign_pid=$! - trap 'kill "$foreign_pid" 2>/dev/null || true' RETURN + trap 'kill "$foreign_pid" "${HARNESS_PID:-}" 2>/dev/null || true' RETURN + printf '%s\n' "$foreign_pid" > "$pids" printf '%s\n' "$foreign_pid" > "$home/state/.lock" before=$(cat "$home/state/.lock") - PATH="$fakebin:$PATH" FM_HOME="$home" "$ROOT/bin/fm-lock.sh" >/dev/null 2>&1 - rc=$? - run_guard "$home" "$dir/root" "$fakebin" + run_lock_under_harness "$home" "$fakebin" "$pids" "$dir" + FM_TEST_HARNESS_PIDS="$pids" run_guard "$home" "$dir/root" "$fakebin" after=$(cat "$home/state/.lock" 2>/dev/null || true) - [ "$rc" -ne 0 ] || fail "fm-lock.sh must refuse a lock held by another live session" + [ "$LOCK_RC" != timeout ] || fail "fm-lock.sh under the harness never finished" + [ "$LOCK_RC" -ne 0 ] || fail "fm-lock.sh must refuse a lock held by another live session" [ "$before" = "$after" ] || fail "an existing foreign state/.lock must never be rewritten; was '$before', now '$after'" - kill "$foreign_pid" 2>/dev/null || true + kill "$foreign_pid" "$HARNESS_PID" 2>/dev/null || true pass "fm-lock/fm-guard: an existing foreign live lock is refused and left untouched" } diff --git a/tests/fm-watch-arm.test.sh b/tests/fm-watch-arm.test.sh index 396a6a3a8ec..b052197d052 100755 --- a/tests/fm-watch-arm.test.sh +++ b/tests/fm-watch-arm.test.sh @@ -1452,6 +1452,65 @@ test_late_genuine_ack_after_bound_settle_still_resurfaces_queued_row() { pass "watch-arm: a genuine ack after a bound settle still resurfaces a newer queued row on re-arm" } +# A retried stale ack for a bound-settled generation consumes no row, so it is +# not a genuine acknowledgement: the settle distinction must survive and the +# next arm must keep the watcher up instead of re-announcing the newer row. +test_stale_zero_row_ack_keeps_bound_settle() { + local dir home state fakebin + dir=$(make_case stale-ack-after-settle) + home="$dir/home" + state="$dir/state" + fakebin="$dir/fakebin" + mkdir -p "$home/data" + printf 'acked:downtime:settledgen1\n' > "$state/.watcher-down" + chmod 0600 "$state/.watcher-down" + printf 'settledgen1\n' > "$state/.watcher-down.reopen-settled" + printf '%s\t2\tcheck\tstale-ack-two\tcheck: stale ack row two\n' "$(date +%s)" > "$state/.wake-queue" + printf '2\n' > "$state/.wake-queue.seq" + + FM_HOME="$home" FM_STATE_OVERRIDE="$state" "$DRAIN" --ack-through 1 --recovery-generation settledgen1 \ + > "$dir/ack.out" 2>&1 \ + || fail "stale zero-row ack after a bound settle failed: $(cat "$dir/ack.out")" + [ "$(cat "$state/.watcher-down.reopen-settled" 2>/dev/null || true)" = settledgen1 ] \ + || fail "a stale zero-row ack cleared the bound-settle distinction" + + start_rearm_arm "$home" "$state" "$fakebin" "$dir/arm.out" + is_live_non_zombie "$ARM_PID" \ + || fail "a stale zero-row ack after a bound settle made the next watcher exit: $(cat "$dir/arm.out")" + ! grep -F 'check: rearm-resurface' "$dir/arm.out" >/dev/null \ + || fail "a stale zero-row ack after a bound settle re-announced the settled episode: $(cat "$dir/arm.out")" + kill "$ARM_PID" 2>/dev/null || true + wait "$ARM_PID" 2>/dev/null || true + pass "watch-arm: a stale zero-row ack keeps a bound-settled episode settled and the watcher up" +} + +# A non-numeric FM_RECOVERY_REOPEN_LIMIT falls back to the default bound, so a +# configuration typo cannot make a stuck episode reopen on every restart. +test_invalid_reopen_limit_falls_back_to_default() { + local dir home state fakebin + local FM_RECOVERY_REOPEN_LIMIT=three + export FM_RECOVERY_REOPEN_LIMIT + dir=$(make_case invalid-reopen-limit) + home="$dir/home" + state="$dir/state" + fakebin="$dir/fakebin" + mkdir -p "$home/data" + printf 'announced:downtime:seedgen1\n' > "$state/.watcher-down" + chmod 0600 "$state/.watcher-down" + printf '3\n' > "$state/.watcher-down.reopen-count" + printf '%s\t1\tcheck\tinvalid-limit\tcheck: invalid limit row\n' "$(date +%s)" > "$state/.wake-queue" + printf '1\n' > "$state/.wake-queue.seq" + + start_rearm_arm "$home" "$state" "$fakebin" "$dir/arm.out" + is_live_non_zombie "$ARM_PID" \ + || fail "a non-numeric reopen limit defeated the bound; the watcher exited: $(cat "$dir/arm.out")" + [ "$(cat "$state/.watcher-down" 2>/dev/null || true)" = 'acked:downtime:seedgen1' ] \ + || fail "a non-numeric reopen limit did not settle past the default bound: $(cat "$state/.watcher-down" 2>/dev/null)" + kill "$ARM_PID" 2>/dev/null || true + wait "$ARM_PID" 2>/dev/null || true + pass "watch-arm: a non-numeric FM_RECOVERY_REOPEN_LIMIT falls back to the default bound" +} + # With an empty queue an announced-but-unacknowledged episode is never # reopened: every plain restart leaves that generation announced, starts no # resurface, and keeps the watcher up without spending the reopen bound. @@ -1501,6 +1560,8 @@ test_stuck_unacked_recovery_settles_after_bounded_reopen test_stuck_unacked_recovery_with_queued_rows_stays_up_after_settling test_genuinely_acked_recovery_with_queued_row_still_resurfaces test_late_genuine_ack_after_bound_settle_still_resurfaces_queued_row +test_stale_zero_row_ack_keeps_bound_settle +test_invalid_reopen_limit_falls_back_to_default test_attached_arm_still_fails_on_a_wake_it_did_not_deliver test_attached_arm_follows_a_slow_live_holder test_attached_arm_hands_a_stalled_holder_to_its_replacement From 82e22fbc5c0070c88828d8e535cd4fb0b1e98e34 Mon Sep 17 00:00:00 2001 From: Marcus Moreira Date: Tue, 29 Sep 2026 23:42:52 -0300 Subject: [PATCH 16/19] no-mistakes(review): Lower recovery reopen limit default from 3 to 1 --- bin/fm-wake-lib.sh | 4 ++-- docs/configuration.md | 2 +- docs/watcher-continuity.md | 2 +- tests/fm-watch-arm.test.sh | 2 +- 4 files changed, 5 insertions(+), 5 deletions(-) diff --git a/bin/fm-wake-lib.sh b/bin/fm-wake-lib.sh index bfab8e3ddc7..890378d9797 100755 --- a/bin/fm-wake-lib.sh +++ b/bin/fm-wake-lib.sh @@ -988,8 +988,8 @@ _fm_recovery_marker_arm_check() { # (_fm_recovery_marker_ack) or arm-check minting a fresh episode from a missing # or invalid marker both clear the counter, so this bound never shortens the # once-per-genuine-generation resurface a live, attentive session relies on. -FM_RECOVERY_REOPEN_LIMIT=${FM_RECOVERY_REOPEN_LIMIT:-3} -case "$FM_RECOVERY_REOPEN_LIMIT" in ''|*[!0-9]*) FM_RECOVERY_REOPEN_LIMIT=3 ;; esac +FM_RECOVERY_REOPEN_LIMIT=${FM_RECOVERY_REOPEN_LIMIT:-1} +case "$FM_RECOVERY_REOPEN_LIMIT" in ''|*[!0-9]*) FM_RECOVERY_REOPEN_LIMIT=1 ;; esac _fm_recovery_marker_reopen_announced() { local marker=$1 lock counter_file count generation diff --git a/docs/configuration.md b/docs/configuration.md index fd8ef606e06..2e9cce7a7ff 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -2333,7 +2333,7 @@ FM_WATCH_ARM_RETIRE_TIMEOUT_MS=1000 # milliseconds Pi/OpenCode wait for an unr FM_WATCH_REARM_RETRY_BASE_MS=250 # Pi/OpenCode adapter base delay for continuity restoration retries FM_WATCH_REARM_RETRY_MAX_MS=4000 # Pi/OpenCode adapter cap for exponential continuity retry delay FM_WATCH_REARM_RETRY_LIMIT=5 # Pi/OpenCode adapter launch-failure retries before surfacing restoration failure -FM_RECOVERY_REOPEN_LIMIT=3 # consecutive unacknowledged watcher restarts that reopen one stuck recovery episode before it settles, so the watcher stays up (docs/watcher-continuity.md "Bounded reopen") +FM_RECOVERY_REOPEN_LIMIT=1 # consecutive unacknowledged watcher restarts that reopen one stuck recovery episode before it settles, so the watcher stays up (docs/watcher-continuity.md "Bounded reopen") 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=300 # defaults to FM_GUARD_GRACE if set, else the poll-derived grace (docs/turnend-guard.md "Guard grace and the poll cadence"); seconds before a fresh arm refuses a live holder's stale beacon (attached arms: FM_WATCHER_STALL_BOUND) diff --git a/docs/watcher-continuity.md b/docs/watcher-continuity.md index 1ce3f76a8c0..24521addc61 100644 --- a/docs/watcher-continuity.md +++ b/docs/watcher-continuity.md @@ -222,7 +222,7 @@ If a durable row arrived after the announcement, the arm opens a fresh pending d Nothing else ever retires that generation when no live session runs the printed acknowledgement. So a plain restart with no re-arm loop and no session would otherwise reopen the same stuck episode into a fresh generation forever, one resurface-then-exit cycle per restart. `state/.watcher-down.reopen-count` bounds that. -Past `FM_RECOVERY_REOPEN_LIMIT` (default 3) consecutive reopens of one episode with no intervening explicit acknowledgement, the next reopen settles the episode to acked directly instead of minting another generation, so the watcher can finally start and stay up. +Past `FM_RECOVERY_REOPEN_LIMIT` (default 1) consecutive reopens of one episode with no intervening explicit acknowledgement, the next reopen settles the episode to acked directly instead of minting another generation, so the watcher can finally start and stay up. The settle records that generation in `state/.watcher-down.reopen-settled`. A watcher start does not re-announce that bound-settled episode just because the queue is non-empty, so its queued rows cannot make the watcher exit; they stay durable and the next session's drain presents them. When a watcher's lock later ends (a `--restart` retiring it, or a stale lock cleared), that close keeps the bound-settled episode instead of publishing a fresh downtime generation, so the next restart stays up too. diff --git a/tests/fm-watch-arm.test.sh b/tests/fm-watch-arm.test.sh index b052197d052..d70dd6327de 100755 --- a/tests/fm-watch-arm.test.sh +++ b/tests/fm-watch-arm.test.sh @@ -1497,7 +1497,7 @@ test_invalid_reopen_limit_falls_back_to_default() { mkdir -p "$home/data" printf 'announced:downtime:seedgen1\n' > "$state/.watcher-down" chmod 0600 "$state/.watcher-down" - printf '3\n' > "$state/.watcher-down.reopen-count" + printf '1\n' > "$state/.watcher-down.reopen-count" printf '%s\t1\tcheck\tinvalid-limit\tcheck: invalid limit row\n' "$(date +%s)" > "$state/.wake-queue" printf '1\n' > "$state/.wake-queue.seq" From 0475f89a0d56186d8353863ec155d86a74908ff0 Mon Sep 17 00:00:00 2001 From: Marcus Moreira Date: Wed, 30 Sep 2026 00:08:53 -0300 Subject: [PATCH 17/19] no-mistakes(review): Reset reopen budget when append mints fresh episode --- bin/fm-wake-lib.sh | 12 +++++++++--- tests/fm-watch-arm.test.sh | 36 ++++++++++++++++++++++++++++++++++++ 2 files changed, 45 insertions(+), 3 deletions(-) diff --git a/bin/fm-wake-lib.sh b/bin/fm-wake-lib.sh index 890378d9797..488087336fc 100755 --- a/bin/fm-wake-lib.sh +++ b/bin/fm-wake-lib.sh @@ -714,7 +714,7 @@ _fm_recovery_marker_write_locked() { # next restart does not reopen that same unwatched stretch all over again. _fm_recovery_marker_publish() { local marker=$1 kind=${2:-downtime} bound=${3:-} source=${4:-watcher} - local lock saved_token generation='' status=pending previous_append_token='' + local lock saved_token generation='' status=pending previous_append_token='' fresh_episode=0 case "$kind" in handling|downtime) ;; *) return 1 ;; esac case "$source" in watcher|close|append) ;; *) return 1 ;; esac if [ "$source" = append ]; then @@ -753,6 +753,8 @@ _fm_recovery_marker_publish() { if [ "$source" != append ]; then generation=${FM_RECOVERY_MARKER_TOKEN##*:} status=announced + else + fresh_episode=1 fi ;; acked:downtime:*) @@ -770,6 +772,9 @@ _fm_recovery_marker_publish() { fm_lock_release "$lock" return 1 fi + if [ "$fresh_episode" = 1 ]; then + rm -f -- "${marker}.reopen-count" 2>/dev/null || true + fi if [ -n "$previous_append_token" ] \ && [ "$previous_append_token" != "$FM_RECOVERY_MARKER_WRITTEN_TOKEN" ]; then FM_WAKE_APPEND_RECOVERY_PREVIOUS_TOKEN=$previous_append_token @@ -985,8 +990,9 @@ _fm_recovery_marker_arm_check() { # queue is non-empty: its queued rows stay durable for the next session's # drain, while a genuinely acked episode with queued rows still resurfaces. # A real acknowledgement -# (_fm_recovery_marker_ack) or arm-check minting a fresh episode from a missing -# or invalid marker both clear the counter, so this bound never shortens the +# (_fm_recovery_marker_ack), arm-check minting a fresh episode from a missing +# or invalid marker, and a durable append minting a fresh episode from an +# announced one all clear the counter, so this bound never shortens the # once-per-genuine-generation resurface a live, attentive session relies on. FM_RECOVERY_REOPEN_LIMIT=${FM_RECOVERY_REOPEN_LIMIT:-1} case "$FM_RECOVERY_REOPEN_LIMIT" in ''|*[!0-9]*) FM_RECOVERY_REOPEN_LIMIT=1 ;; esac diff --git a/tests/fm-watch-arm.test.sh b/tests/fm-watch-arm.test.sh index d70dd6327de..5db9ec8ba19 100755 --- a/tests/fm-watch-arm.test.sh +++ b/tests/fm-watch-arm.test.sh @@ -1549,6 +1549,41 @@ test_stuck_unacked_recovery_with_queued_rows_stays_up_after_settling() { pass "watch-arm: a settled recovery episode with queued rows keeps the watcher up and leaves the rows for the next drain" } +# A durable append that mints a fresh episode from an announced one starts that +# episode with the full reopen budget instead of inheriting the old count. +test_append_fresh_episode_resets_reopen_budget() { + local dir home state fakebin + local FM_RECOVERY_REOPEN_LIMIT=1 + export FM_RECOVERY_REOPEN_LIMIT + dir=$(make_case append-resets-reopen-budget) + home="$dir/home" + state="$dir/state" + fakebin="$dir/fakebin" + mkdir -p "$home/data" + printf 'announced:downtime:seedgen1\n' > "$state/.watcher-down" + chmod 0600 "$state/.watcher-down" + printf '1\n' > "$state/.watcher-down.reopen-count" + + append_wake "$state" check fresh-episode 'check: fresh episode row' \ + || fail "the producer could not append its wake" + [ ! -e "$state/.watcher-down.reopen-count" ] \ + || fail "an append that minted a fresh episode kept the old reopen count" + + start_rearm_arm "$home" "$state" "$fakebin" "$dir/announce-arm.out" + wait_for_exit "$ARM_PID" "$REARM_EXIT_POLLS" \ + || fail "the fresh episode was not announced: $(cat "$dir/announce-arm.out")" + start_rearm_arm "$home" "$state" "$fakebin" "$dir/reopen-arm.out" + wait_for_exit "$ARM_PID" "$REARM_EXIT_POLLS" \ + || fail "the fresh episode settled without its own reopen: $(cat "$dir/reopen-arm.out")" + grep -F 'check: rearm-resurface' "$dir/reopen-arm.out" >/dev/null \ + || fail "the fresh episode's first reopen did not resurface: $(cat "$dir/reopen-arm.out")" + case "$(cat "$state/.watcher-down" 2>/dev/null || true)" in + announced:*) ;; + *) fail "the fresh episode's first reopen left an unexpected marker: $(cat "$state/.watcher-down" 2>/dev/null)" ;; + esac + pass "watch-arm: an append that mints a fresh episode resets the reopen budget" +} + test_attached_arm_reports_the_delivered_wake test_attached_arm_reports_the_delivered_wake_after_drain test_arm_refuses_an_unusable_launch_confirm_window @@ -1562,6 +1597,7 @@ test_genuinely_acked_recovery_with_queued_row_still_resurfaces test_late_genuine_ack_after_bound_settle_still_resurfaces_queued_row test_stale_zero_row_ack_keeps_bound_settle test_invalid_reopen_limit_falls_back_to_default +test_append_fresh_episode_resets_reopen_budget test_attached_arm_still_fails_on_a_wake_it_did_not_deliver test_attached_arm_follows_a_slow_live_holder test_attached_arm_hands_a_stalled_holder_to_its_replacement From 52271b18f1740840c9ee7635e008c0283bc0e3fe Mon Sep 17 00:00:00 2001 From: Marcus Moreira Date: Wed, 30 Sep 2026 00:15:34 -0300 Subject: [PATCH 18/19] no-mistakes(review): Clear reopen count only after append succeeds --- bin/fm-wake-lib.sh | 10 ++++------ docs/watcher-continuity.md | 2 +- tests/fm-watch-arm.test.sh | 21 +++++++++++++++++++++ 3 files changed, 26 insertions(+), 7 deletions(-) diff --git a/bin/fm-wake-lib.sh b/bin/fm-wake-lib.sh index 488087336fc..a83e5cabe03 100755 --- a/bin/fm-wake-lib.sh +++ b/bin/fm-wake-lib.sh @@ -714,7 +714,7 @@ _fm_recovery_marker_write_locked() { # next restart does not reopen that same unwatched stretch all over again. _fm_recovery_marker_publish() { local marker=$1 kind=${2:-downtime} bound=${3:-} source=${4:-watcher} - local lock saved_token generation='' status=pending previous_append_token='' fresh_episode=0 + local lock saved_token generation='' status=pending previous_append_token='' case "$kind" in handling|downtime) ;; *) return 1 ;; esac case "$source" in watcher|close|append) ;; *) return 1 ;; esac if [ "$source" = append ]; then @@ -753,8 +753,6 @@ _fm_recovery_marker_publish() { if [ "$source" != append ]; then generation=${FM_RECOVERY_MARKER_TOKEN##*:} status=announced - else - fresh_episode=1 fi ;; acked:downtime:*) @@ -772,9 +770,6 @@ _fm_recovery_marker_publish() { fm_lock_release "$lock" return 1 fi - if [ "$fresh_episode" = 1 ]; then - rm -f -- "${marker}.reopen-count" 2>/dev/null || true - fi if [ -n "$previous_append_token" ] \ && [ "$previous_append_token" != "$FM_RECOVERY_MARKER_WRITTEN_TOKEN" ]; then FM_WAKE_APPEND_RECOVERY_PREVIOUS_TOKEN=$previous_append_token @@ -2118,6 +2113,9 @@ fm_wake_append_locked() { if [ "$status" -ne 0 ]; then _fm_wake_append_recovery_restore_locked || true else + case "$FM_WAKE_APPEND_RECOVERY_PREVIOUS_TOKEN" in + announced:downtime:*) rm -f -- "${recovery_marker}.reopen-count" 2>/dev/null || true ;; + esac FM_WAKE_APPEND_RECOVERY_PREVIOUS_TOKEN= FM_WAKE_APPEND_RECOVERY_PUBLISHED_TOKEN= fi diff --git a/docs/watcher-continuity.md b/docs/watcher-continuity.md index 24521addc61..38788cd2dba 100644 --- a/docs/watcher-continuity.md +++ b/docs/watcher-continuity.md @@ -227,7 +227,7 @@ The settle records that generation in `state/.watcher-down.reopen-settled`. A watcher start does not re-announce that bound-settled episode just because the queue is non-empty, so its queued rows cannot make the watcher exit; they stay durable and the next session's drain presents them. When a watcher's lock later ends (a `--restart` retiring it, or a stale lock cleared), that close keeps the bound-settled episode instead of publishing a fresh downtime generation, so the next restart stays up too. A genuinely acknowledged episode with queued rows still re-announces and resurfaces them on the next arm. -A real acknowledgement, or a watcher start that mints a fresh episode from a missing or invalid marker, clears the counter, so this bound never shortens the once-per-genuine-generation resurface a live, attentive session relies on. +A real acknowledgement, a watcher start that mints a fresh episode from a missing or invalid marker, or a successful durable append that mints a fresh episode from an announced one clears the counter, so this bound never shortens the once-per-genuine-generation resurface a live, attentive session relies on. ### Generation reuse diff --git a/tests/fm-watch-arm.test.sh b/tests/fm-watch-arm.test.sh index 5db9ec8ba19..f077176c795 100755 --- a/tests/fm-watch-arm.test.sh +++ b/tests/fm-watch-arm.test.sh @@ -1584,6 +1584,26 @@ test_append_fresh_episode_resets_reopen_budget() { pass "watch-arm: an append that mints a fresh episode resets the reopen budget" } +# A failed append restores the announced episode, so it must keep that +# episode's reopen count rather than grant it a fresh budget. +test_failed_append_keeps_reopen_count() { + local dir state + dir=$(make_case failed-append-keeps-reopen-count) + state="$dir/state" + printf 'announced:downtime:seedgen1\n' > "$state/.watcher-down" + chmod 0600 "$state/.watcher-down" + printf '1\n' > "$state/.watcher-down.reopen-count" + mkdir -p "$state/.wake-queue.seq" + + ! append_wake "$state" check failed-append 'check: failed append row' 2>/dev/null \ + || fail "an append whose sequence write failed reported success" + [ "$(cat "$state/.watcher-down" 2>/dev/null || true)" = 'announced:downtime:seedgen1' ] \ + || fail "a failed append did not restore the announced episode: $(cat "$state/.watcher-down" 2>/dev/null)" + [ "$(cat "$state/.watcher-down.reopen-count" 2>/dev/null || true)" = 1 ] \ + || fail "a failed append cleared the restored episode's reopen count" + pass "watch-arm: a failed append keeps the restored episode's reopen count" +} + test_attached_arm_reports_the_delivered_wake test_attached_arm_reports_the_delivered_wake_after_drain test_arm_refuses_an_unusable_launch_confirm_window @@ -1598,6 +1618,7 @@ test_late_genuine_ack_after_bound_settle_still_resurfaces_queued_row test_stale_zero_row_ack_keeps_bound_settle test_invalid_reopen_limit_falls_back_to_default test_append_fresh_episode_resets_reopen_budget +test_failed_append_keeps_reopen_count test_attached_arm_still_fails_on_a_wake_it_did_not_deliver test_attached_arm_follows_a_slow_live_holder test_attached_arm_hands_a_stalled_holder_to_its_replacement From 67802098c4659a211fde9441f6f010ff6e130a17 Mon Sep 17 00:00:00 2001 From: Marcus Moreira Date: Wed, 30 Sep 2026 00:35:50 -0300 Subject: [PATCH 19/19] no-mistakes(document): Note bounded reopen also settles recovery episodes --- docs/watcher-continuity.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/watcher-continuity.md b/docs/watcher-continuity.md index 38788cd2dba..5bee5a76cc0 100644 --- a/docs/watcher-continuity.md +++ b/docs/watcher-continuity.md @@ -206,7 +206,7 @@ In its `--claude` mode it cooperates with the auto-arm. ## Recovery episode acknowledgement A recovery episode is one generation of the `state/.watcher-down` marker. -It is retired only by the generation-bound acknowledgement the drain prints as `WAKE_ACK_REQUIRED`. +It is retired by the generation-bound acknowledgement the drain prints as `WAKE_ACK_REQUIRED`, or settled by the bounded reopen below when nobody runs that acknowledgement. The away return brief treats a still-open handling episode as a wake in progress, not watcher downtime; an open downtime episode remains a gap. ### Announcement