fix: harden watcher supervision during slow reply polls - #3
Merged
Conversation
|
Bugbot is not enabled for your account, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
jokim1
force-pushed
the
fm/fm-pending-reply-leak-slow-watch
branch
from
August 16, 2026 14:11
4278da9 to
e28718c
Compare
Answered pending-reply records were left on disk forever, so the watcher tick walked a growing archive every poll and could starve the liveness beacon past grace while still healthy (false WATCHER DOWN). Retire only resolved records after closing any open escalation, touch the beacon at bounded intervals during a large walk, and attach re-arms to a live identity-matched holder even when its beacon is temporarily stale mid poll so Stop auto-arm does not thrash replacement cycles.
…ot walk Resolved records that cannot close their escalation (blank parent_status, unwritable status append) no longer remain in state/pending-replies/ forever. Quarantine them under pending-replies-stuck/ with a durable receipt so open status-fold decisions stay intact and polls cannot re-accumulate answered files. Fail closed if the quarantine move cannot complete; regressions use chmod 444 (readable) for the unwritable-status fixture.
…and-down Close BREAK 1: resolved+escalated records that cannot close leave the hot pending-replies walk via quarantine to pending-replies-stuck/ with a durable receipt (chmod-444 unwritable fixture; fail closed on quarantine move failure). Restore peer-startup race: arm entry still requires a fresh beacon to attach without starting a child, and stand-down diagnostics stay on disk during live-holder attach so the child "already running" line remains visible.
jokim1
force-pushed
the
fm/fm-pending-reply-leak-slow-watch
branch
from
August 17, 2026 00:18
a80adf9 to
dd19280
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
Fix two coupled watcher-supervision reliability defects: promptly retire only resolved or terminal answered pending-reply records while never expiring genuinely unanswered records; keep the beacon fresh during bounded slow polls only while the watcher retains identity-matched singleton lock ownership; and attach Stop-hook re-arms to a live identity-matched stale-beacon holder without starting a competing cycle, while newly started watchers still require a fresh beacon. Preserve singleton lock, self-eviction, unresolved-record durability, and guard fail-open/fail-closed behavior. Include the large resolved-population regression and deliver PR 3 green. Also fix the observed CI blockers without weakening behavior: suppress duplicate away-mode injection when a capacity-freeing status and same-window refill share a batch, bound daemon watcher teardown, make normal-injection waits event-driven, and launch the remote-worker test supervisor in the production process-group shape.
What Changed
Risk Assessment
✅ Low: The shared health predicate now requires generation-bound beacon proof, while the arm-only stale-holder path remains distinct and avoids competing watcher startup.
Testing
After reviewing the prior-round failures and base-to-target diff, I reran the watcher, pending-reply, Stop-hook, AFK injection, remote-worker, lock, guard, and full remote-retirement workflows; all passed and produced CLI evidence, while PR inspection showed target b875caa remains unpublished and PR 3 is still red. No screenshot applies because this is shell/CLI supervision behavior with no rendered UI.
Evidence: Core watcher end-to-end transcript
Evidence: CI-blocker behavior transcript
Evidence: Remote lifecycle end-to-end transcript
Evidence: PR 3 publication and CI status
Source: PR 3 publication and CI status
Pipeline
Updates from git push no-mistakes
✅ **intent** - passed
✅ No issues found.
✅ **Rebase** - passed
✅ No issues found.
🔧 **Review** - 1 issue found → auto-fixed (3) ✅
bin/fm-watch-arm.sh:477- Intent requires “attach Stop-hook re-arms to a live identity-matched stale-beacon holder without starting a competing cycle,” but this hunk explicitly starts the usual watcher child for that case. The child can exit nonzero on fm-watch.sh's stale-beacon lock check; lines 638-643 then report failure before the live-holder attach at line 650, recreating the false watcher-down race. Attach to the verified stale holder before spawning while retaining fresh-beacon confirmation for genuinely new watchers.🔧 Fix: Attach stale-beacon watcher holders before spawning
1 error still open:
bin/fm-watch-arm.sh:481- Intent requires stale-holder attachment while “newly started watchers still require a fresh beacon,” butlive_watcher_holderverifies only PID/lock identity and this gate attaches even when.last-watcher-beatis absent. A peer watcher can publish its lock identity before its first beacon (fm-watch.sh:798-803versus:856), then be reported as attached and followed indefinitely if startup wedges. Require proof that the current lock generation previously emitted a beacon before using the stale-holder path; otherwise retain bounded fresh-beacon confirmation.🔧 Fix: Bind watcher attachment to generation beacon proof
1 error still open:
bin/fm-watch-arm.sh:258- Intent requires “newly started watchers still require a fresh beacon” and preserved guard fail-closed behavior, but generation proof is enforced only by this local wrapper.fm-claude-stop-autoarm.sh:191andfm-turnend-guard.sh:149,244,312,346,355still callfm_watcher_healthy, which accepts a new live lock with a fresh leftover global beat without checkingbeacon-identity. An unbeaconed startup can therefore be suppressed as healthy/clean. Enforce generation proof in the shared health predicate or update every guard consumer while keeping stale-holder attachment distinct.🔧 Fix: Enforce generation-bound health across watcher guards
✅ Re-checked - no issues remain.
tests/fm-turnend-guard.test.sh:213- The new guard regression aborts underset -u:record_watcher_lockexpands$identityin the samelocalstatement that assigns it. Split the declarations soidentityis assigned first.tests/fm-remote-job.test.sh:479- The remote-worker suite failed twice at different points: an orphaned command mutated after crash recovery, then a rerun reported queue delay consuming the next job's execution timeout. The production-shape fixture is not reliably green.tests/fm-watch-arm.test.sh:373- The watcher-arm suite hung for over four minutes in the rearm-resurface fixture after intentional teardown, leaving the arm wrapper and watcher child alive until the test session was interrupted.git diff --stat 5b9e8629923a0b7623c82d5e151490e8185f59f7..fcc80cb17171c46af999bd1558ef76bb2f6d3b64tests/fm-pending-reply.test.shtests/fm-watch-arm.test.sh- interrupted after the rearm-resurface hangtests/fm-daemon.test.shtests/fm-afk-inject-e2e.test.shtests/fm-remote-job.test.sh- run twicetests/fm-turnend-guard.test.shtests/fm-claude-stop-autoarm.test.shgh-axi pr checks 3gh-axi run view 31934901693 --log-failedgit status --shortand process checks for transient workspace artifacts🔧 Fix: Bound watcher teardown and stabilize supervision tests
2 issues (1 error, 1 warning) still open:
bin/fm-wake-lib.sh:762- Safe remote retirement reproducibly fails becausefm_lock_try_acquirerecursively acquires$lockdir.steal. When that mutex is stale, each recursion appends another.stealuntil paths exceed the filesystem limit. Acquire or reclaim the steal mutex without recursively callingfm_lock_try_acquire, while preserving refusal for live steal owners.git status --short --branchandgit rev-parse HEADgit diff --stat 5b9e8629923a0b7623c82d5e151490e8185f59f7..6f23745211432df7495561a8e01fdbd6d961b590/opt/homebrew/bin/gh-axi pr view 3and/opt/homebrew/bin/gh-axi pr checks 3/opt/homebrew/bin/gh-axi run list --branch fm/fm-pending-reply-leak-slow-watch --limit 10 --fields headSha,number,updatedAt,url/opt/homebrew/bin/gh-axi run view 31934901693 --conclusion failure --log-failedbin/fm-test-run.sh tests/fm-pending-reply.test.sh tests/fm-watch-arm.test.sh tests/fm-claude-stop-autoarm.test.sh tests/fm-turnend-guard.test.sh tests/fm-watcher-lock.test.sh tests/fm-daemon.test.sh tests/fm-afk-inject-e2e.test.sh tests/fm-pi-watch-extension.test.sh tests/fm-remote-job.test.shbin/fm-test-run.sh tests/fm-afk-inject-herdr-e2e.test.sh tests/fm-remote-secondmate-lifecycle-e2e.test.shFM_SNAPSHOT_SECONDMATE_TIMEOUT=30 bin/fm-test-run.sh tests/fm-remote-secondmate-lifecycle-e2e.test.shFinalgit status --shortand transient-directory check🔧 Fix: Fix stale lock reclamation and remote teardown completion
1 warning still open:
git diff --stat 5b9e8629923a0b7623c82d5e151490e8185f59f7..b875caafa99da9db3b5965d2c61c9a99adc2e359and intent-linked source/test inspectionbin/fm-test-run.sh --json .../core-watcher-tests.json tests/fm-pending-reply.test.sh tests/fm-watch-arm.test.sh tests/fm-claude-stop-autoarm.test.shbin/fm-test-run.sh --json .../ci-blocker-tests.json tests/fm-daemon.test.sh tests/fm-afk-inject-e2e.test.sh tests/fm-remote-job.test.sh tests/fm-watcher-lock.test.sh tests/fm-turnend-guard.test.shbin/fm-test-run.sh --json .../remote-lifecycle-test.json tests/fm-remote-secondmate-lifecycle-e2e.test.shgh-axi pr checks 3gh-axi run list --branch fm/fm-pending-reply-leak-slow-watchgit ls-remote origin refs/heads/fm/fm-pending-reply-leak-slow-watchgit status --shortand transient-output inspection✅ **Document** - passed
✅ No issues found.
✅ **Lint** - passed
✅ No issues found.
✅ **Push** - passed
✅ No issues found.