Repository navigation
fix(bin): keep acknowledged downtime retired when a take-over stop lands late - #6670
Open
Vela3202019 wants to merge 5 commits into
Open
Vela3202019 wants to merge 5 commits into
Vela3202019 wants to merge 5 commits into
Conversation
|
Vela3202019
force-pushed
the
fm/firstmate-resurface-loop
branch
from
October 7, 2026 06:31
38dd791 to
960390a
Compare
…s retired A watcher TERM'd mid-poll while blocked inside a pane-capture command substitution defers its exit until the substitution completes, which can outlive the take-over's old fixed five-second stop window. The abandoned stop never ran the handover restore, so the dying watcher's downtime publication minted a fresh pending generation from the acknowledged marker and every later park re-announced 'check: rearm-resurface' to main. The stop window now covers the slowest legitimate poll block: the watcher's check timeout plus margin (FM_ARM_TAKE_OVER_STOP_BOUND), so the stop is confirmed, the restore runs, and the fresh cycle stays quiet. Regression: a slow-capture take-over keeps the acknowledged episode retired and owns a fresh cycle (fails without the fix with the attached cycle ending without an actionable reason).
…the stop window A pane capture can run past the take-over stop window (FM_POLL + 25). The stop is already in flight and the death is guaranteed once the capture completes, so the taking arm now keeps waiting for the watcher to exit up to the stall bound instead of abandoning the stop: the handover restore still runs and the acknowledged downtime episode stays retired instead of being re-announced on the next park. Regression: a 30s capture against the 26s window derived from FM_POLL=1 fails without the extended wait (attached cycle ending without an actionable reason) and stays quiet with it.
Vela3202019
force-pushed
the
fm/firstmate-resurface-loop
branch
from
October 12, 2026 02:39
960390a to
137568d
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Intent
Stop the supervision host from re-announcing 'check: rearm-resurface' on every turn end while attended.
Seen 2026-10-04 10:15-10:17 PDT on main at 241d461, attended, supervision host on, wake queue empty (.wake-queue 0 bytes, seq 76). Four consecutive empty 'check: rearm-resurface' wakes arrived about 28s apart, each handled with a drain plus --ack-through 0. state/.supervision-host.log repeats per turn: 'start gen=host-N' -> 'pass-through attended main-only check: rearm-resurface' -> (main turn ends) 'start gen=host-M' + 'take-over arm=' -> about 19s later 'pass-through a close without a wake' -> 'start' -> resurface again. state/.watch-cycle-exits.log shows the taken-over cycle ending with a signal-exit reason, an attached arm ending attached-cycle-ended, then a started cycle exiting actionable-check at once. The marker state/.watcher-down read acked:handling:, so the dying cycle's pending downtime was announced to main instead of being restored by fm_recovery_marker_handover_restore (bin/fm-wake-lib.sh; bin/fm-watch-arm.sh around lines 559-590). Suspect the host's restart after 'a close without a wake' re-arms with recovery pending, or the handover snapshot is taken before main's acknowledgement.
Find why an acknowledged, empty-queue home re-announces downtime on every take-over, fix it, and add a regression test. Cost today: one main turn per loop while attended.
The PR must also cover Greptile's P1 'Slow captures outlive takeover' (bin/fm-watch-arm.sh:149), so Greptile passes. Make the take-over path restore the acknowledged downtime marker even when the watcher's pane capture runs past the stop bound - bound that capture (or otherwise make take_over_cycle not give up before the restore), plus a regression test with a capture slower than the bound. Out of scope: bounding the other herdr calls in the watcher cycle body; that stays a separate task.
What Changed
take_over_cycleinbin/fm-watch-arm.shused to give up after a fixed 5-second wait for the TERM'd watcher to exit. It now waits up to the stall bound (STALL_BOUND). A watcher stuck in a slow pane capture, poll sleep, or event wait now still dies inside the take-over. The handover restore then runs, so an acknowledged, empty-queue episode is no longer re-announced ascheck: rearm-resurfaceon every take-over.tests/fm-watch-arm.test.sh. Both use a fake tmuxcapture-panethat blocks mid-capture: one takes 12s, and the other takes 30s againstFM_POLL=1, so it runs well past the poll interval. Each test checks that the take-over waits for the watcher to die, restores the acknowledged marker, and keeps the fresh cycle quiet.docs/watcher-continuity.mdand the--take-overheader comment to describe the stall-bounded wait for the watcher's death. They also note that the short ledger-row wait still follows it. The slow-capture take-over is added to the arm test coverage list.Risk Assessment
✅ Low: The fix round turns take_over_cycle's two waits into one wait bounded by STALL_BOUND, as the user decided, and removes WATCH_POLL, TAKE_OVER_STOP_BOUND and the "stop window" wording; behavior matches the previous combined wait, the restore still runs after a capture slower than the poll interval, and a watcher that outlives the stall bound still falls back to the plain attach.
Testing
I ran the five take-over tests in tests/fm-watch-arm.test.sh on HEAD, and all passed. The two new regression tests fail when the base fm-watch-arm.sh is swapped in, so they reproduce the bug. I then drove the real fm-watch-arm.sh and fm-watch.sh in disposable lab homes against a real tmux pane on a private socket, with a tmux shim that only slows
capture-pane. The base code reproduced the incident end to end: the acknowledged marker became pending downtime,check: rearm-resurfaceprinted again, and the ledger showed the same three exit reasons. On HEAD the take-over waited out both a 12s capture and a 30s capture with a 1s poll (the Greptile P1 case), kept the markeracked:handling, started a quiet fresh cycle, and linked it in the ledger. A watcher that outlived the stall bound still made the take-over give up on time and attach as a plain arm. The guard scenarios (queued work, self-exit downtime, an unowned cycle) were checked only by the repository's tests, not in a live lab run, so that scenario is marked untested. Every lab was torn down.Evidence: Live run on base 99da16d: take-over gives up, marker reopens, re-arm prints check: rearm-resurface
Source: Live run on base 99da16d9: take-over gives up, marker reopens, re-arm prints check: rearm-resurface
Evidence: Live run on HEAD, 12s capture: marker stays acked, fresh quiet cycle, ledger taken-over successor=started
Source: Live run on HEAD, 12s capture: marker stays acked, fresh quiet cycle, ledger taken-over successor=started
Evidence: Live run on HEAD, FM_POLL=1 with a 30s capture (Greptile P1): restore still runs
Source: Live run on HEAD, FM_POLL=1 with a 30s capture (Greptile P1): restore still runs
Evidence: Live adversarial run: watcher outlives the stall bound, take-over gives up at the bound and plain-attaches
Source: Live adversarial run: watcher outlives the stall bound, take-over gives up at the bound and plain-attaches
Evidence: Live scenario driver script (lab home + private tmux socket)
Source: Live scenario driver script (lab home + private tmux socket)
Evidence: Take-over tests on HEAD (all ok)
Source: Take-over tests on HEAD (all ok)
Evidence: New regression tests against base fm-watch-arm.sh (both fail)
Source: New regression tests against base fm-watch-arm.sh (both fail)
Pipeline
Updates from git push no-mistakes
✅ **intent** - passed
✅ No issues found.
✅ **Rebase** - passed
✅ No issues found.
🔧 **Review** - 1 issue found → auto-fixed ✅
bin/fm-watch-arm.sh:588- take_over_cycle now runs two wait loops back to back with the same condition (fm_pid_alive "$pid"). The first is bounded by TAKE_OVER_STOP_BOUND (FM_POLL+25). The second is bounded by STALL_BOUND, which is at least 900s by default (3 × max(300, POLL+60), fm-wake-lib.sh:136-154). Together they behave as one wait of POLL+25+STALL_BOUND. So the poll-derived stop window has no observable effect: no path acts when the first deadline passes. The leftover parts are WATCH_POLL with its validation and TAKE_OVER_STOP_BOUND (bin/fm-watch-arm.sh:151-153), the first loop (:579-582), and the comments and docs that describe a 'stop window' as a separate stage (fm-watch-arm.sh:79-82, :142-150, :571, :583-587; docs/watcher-continuity.md:236-237; the test comments at tests/fm-watch-arm.test.sh around the 'window = FM_POLL + 25' note). A single loop bounded by STALL_BOUND would behave the same and drop that code. The earlier round-1 decision asked for a poll-derived bound, and this commit makes that bound redundant, so removing it needs the user's sign-off. Recommended remedy: keep one wait bounded by STALL_BOUND and delete WATCH_POLL, TAKE_OVER_STOP_BOUND, and the 'stop window' wording. The other option is to keep the code and only fix the docs so they describe the effective bound.🔧 Fix applied.
✅ Re-checked - no issues remain.
✅ **Test** - passed
✅ No issues found.
bash <runner sourcing tests/fm-watch-arm.test.sh> test_take_over_waits_out_a_slow_stop_and_keeps_the_episode_acknowledged test_take_over_restores_after_a_capture_that_outlives_the_poll_interval test_take_over_owns_a_fresh_cycle_and_keeps_queued_work_surfacing test_take_over_preserves_downtime_from_watcher_self_exit test_take_over_attaches_to_a_cycle_the_named_arm_does_not_ownon HEAD: all 5 okSame two new regression tests with basegit show 99da16d9:bin/fm-watch-arm.shswapped in temporarily (restored afterwards): both fail ('--take-over did not own a fresh cycle ... attached pid=... cycle ended without an actionable reason')Live:live-takeover-scenario.sh <base bin> 12in a disposable lab FM_HOME (fm-lab-home.sh create) with a real tmux crew pane on a private fm-lab socket, a 12s capture delay, then main's drain/ack, thenfm-watch-arm.sh --take-over <owner>, then the host's follow-up re-armLive: same scenario on HEAD with a 12s captureLive: same scenario on HEAD with FM_POLL=1 and a 30s capture (Greptile P1)Live adversarial: HEAD with FM_WATCHER_STALL_BOUND=6 and a 25s capture (the watcher outlives the bound)✅ **Document** - passed
✅ No issues found.
✅ **Lint** - passed
✅ No issues found.
✅ **Push** - passed
✅ No issues found.