fix(bin): keep watcher supervision continuous across child cycles#693
Open
kunchenguid wants to merge 19 commits into
Open
fix(bin): keep watcher supervision continuous across child cycles#693kunchenguid wants to merge 19 commits into
kunchenguid wants to merge 19 commits into
Conversation
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 watcher supervision continuity so Pi and OpenCode adapters re-arm event-driven after actionable child closes with single-flight, session-lock verification, bounded backoff, independent wake delivery, loud unexplained clean exits, and a bounded arm-owned cycle ledger; add only the tightly scoped Claude in-flight/no-live-watcher fleet-command gate while preserving drain/arm access and leaving the existing turn-end guard unchanged; preserve the 300-second grace and watcher-only beacon ownership; validate credentialed real E2Es for Claude, Codex, OpenCode, Pi via shared openai-codex auth, and Grok with sanitized evidence; create a green PR but do not merge it because captain review is the merge gate.
What Changed
Risk Assessment
Testing
The supplied full-suite baseline was green; I reran the Pi/OpenCode lifecycle suite in isolation and twice concurrently, exercised lock/peer-attach/ledger and Claude gate regressions, and completed isolated credentialed Claude, Codex, OpenCode, Pi, and Grok E2Es with sanitized CLI-transcript evidence. No graphical UI applies to these terminal adapters, so the reviewer-visible evidence is captured terminal output.
Evidence: Pi credentialed TUI continuity transcript
Evidence: OpenCode credentialed TUI continuity transcript
Evidence: Claude credentialed continuity-gate transcript
Evidence: Codex credentialed checkpoint transcript
Evidence: Grok credentialed continuity transcript
Evidence: Pi/OpenCode extension suite under concurrent load
Evidence: Watcher lock, peer-attach, and lifecycle-ledger evidence
Pipeline
Updates from git push no-mistakes
✅ **intent** - passed
✅ No issues found.
✅ **Rebase** - passed
✅ No issues found.
🔧 **Review** - 3 issues found → auto-fixed (5) ✅
.opencode/plugins/fm-primary-watch-arm.js:169- A cycle that emitsstarted/attachedand then immediately exits unexpectedly resetsretryFailuresbefore its close handler schedules the retry. Every retry is therefore attempt 1, so the configured retry limit is never reached and this can respawn arm children indefinitely at the base delay. Reset the counter only after a cycle has completed successfully, not when readiness is first observed..pi/extensions/fm-primary-pi-watch.ts:219- As in OpenCode, observing a ready line resetsretryFailuresbefore an unexpected close. A watcher that starts and then repeatedly exits will bypassretryLimitforever, producing an unbounded Pi restart loop instead of surfacing the promised restoration failure.bin/fm-watch-arm.sh:341- The attach path begins tracking an observed cycle and immediately entersattach_and_wait, but signal traps are installed only later in the child-start path. TERM/HUP/INT therefore terminates an attached arm without a lifecycle record or signal classification, contradicting the ledger's per-observed-cycle diagnostics. Install the signal handling before this branch or add an attach-specific trap.🔧 Fix: Bound watcher retries and log attached signals
2 errors still open:
.pi/extensions/fm-primary-pi-watch.ts:256- The required criterion says “independent wake delivery,” but the actionable-close path waits forsuccessorReadybefore callingsendWake. If the replacement arm hangs before emittingstartedorattached, that promise never settles and the actionable wake is never delivered. Decide whether delivery should be dispatched immediately afterstartArmis invoked (while the successor continues establishing) rather than gated on readiness..opencode/plugins/fm-primary-watch-arm.js:272- The required criterion says “independent wake delivery,” but OpenCode waits forensureArmto resolve before it callspromptAsync. A successor that does not become ready delays the actionable wake for the arm-ready timeout, coupling delivery to restoration rather than merely starting the successor first. Decide whether to dispatch the wake after initiating the successor and handle restoration failures separately.🔧 Fix: Add bounded successor-recovery wake fallbacks
2 errors still open:
.pi/extensions/fm-primary-pi-watch.ts:180- The captain-required fallback must “remain single-flight,” but timeout abandonment clears the tracked child and sends TERM without waiting for that arm to exit. The next retry can launch while the timed-out arm is still alive or ignoring TERM, leaving concurrent arm processes and allowing the old process to publish a watcher later. Bound termination as well: only retry after close confirmation, or stop retrying and deliver the typed fallback if the old arm cannot be retired..opencode/plugins/fm-primary-watch-arm.js:221- The captain-required fallback must “remain single-flight,” but this clearschildimmediately after TERM without awaiting the prior arm’s close. A retry can therefore overlap an unresponsive predecessor arm, defeating single-flight and leaving a late old arm able to interfere with the new cycle. Wait for bounded retirement before retrying, then fall back to the typed wake if retirement does not complete.🔧 Fix: Prevent overlapping successor-arm retries
2 errors still open:
.pi/extensions/fm-primary-pi-watch.ts:312- The required event-driven re-arm path is lost after a retirement timeout. The arm remains tracked until it eventually closes, but it is permanently marked abandoned, so this handler discards that later close - including an actionable wake - rather than delivering it or resuming recovery. The fallback prompt covers the original wake only; decide how a late unretired arm should be supervised after fallback, while still avoiding overlap..opencode/plugins/fm-primary-watch-arm.js:325- After a timed-out arm fails retirement, its latercloseevent is permanently ignored because it remains inabandonedChildren. If that arm eventually establishes and exits with an actionable reason, the queue receives a wake but OpenCode never delivers it or starts the next cycle. The fallback only reports the earlier wake; decide how late closes should re-enter supervised handling without violating single-flight.🔧 Fix: Resume supervision after late arm closes
1 error still open:
.opencode/plugins/fm-primary-watch-arm.js:247- When a successor closes with an actionable reason before becoming ready, its close handler synchronously starts a new restoration successor and resolves this restoration's readiness waiter. On resume, mutablechildnow refers to that new successor, so this call sends TERM to it rather than the already-closed arm. Track the arm associated with each readiness attempt and retire that specific child to preserve continuous single-flight recovery.🔧 Fix: Bind OpenCode recovery to attempted arm
✅ Re-checked - no issues remain.
🔧 **Test** - 1 issue found → auto-fixed (2) ✅
command -v tmux >/dev/null || { echo "tmux is required for e2e tests" >&2; exit 1; }; tmux -V; rc=0; for t in tests/*.test.sh; do echo "== $t =="; bash "$t" || rc=1; done; exit "$rc"🔧 Fix: Synchronize peer beacon regression fixture
2 warnings still open:
tests/fm-pi-watch-extension.test.sh:444-fm-pi-watch-extension.test.shfailed once in its late-close fixture at a 20 ms readiness/retirement boundary while other focused tests were still active. The identical isolated scenario and a complete clean rerun passed, indicating a load-sensitive test flake rather than a persistent adapter failure. Consider synchronizing or widening this fixture before relying on it as a deterministic gate.fm/fm-watcher-continuity-fix(gh-axi pr list --state open --head fm/fm-watcher-continuity-fixreturned zero results), so the required green-PR acceptance criterion cannot be demonstrated. Creating a PR is an external state change outside this testing-only request.command -v tmux >/dev/null || { echo "tmux is required for e2e tests" >&2; exit 1; }; tmux -V; rc=0; for t in tests/*.test.sh; do echo "== $t =="; bash "$t" || rc=1; done; exit "$rc"Baseline already completed:command -v tmux >/dev/null || { echo "tmux is required for e2e tests" >&2; exit 1; }; tmux -V; rc=0; for t in tests/*.test.sh; do echo "== $t =="; bash "$t" || rc=1; done; exit "$rc"bash tests/fm-watcher-lock.test.shbash tests/fm-continuity-pretool-check.test.shbash tests/fm-pi-watch-extension.test.sh- clean rerun after isolated late-close reproductionFM_CLAUDE_LIVE_E2E=1 bash tests/fm-claude-continuity-live-e2e.test.shFM_CODEX_LIVE_E2E=1 bash tests/fm-codex-continuity-live-e2e.test.shFM_OPENCODE_LIVE_E2E=1 bash tests/fm-opencode-primary-live-e2e.test.shFM_PI_LIVE_E2E=1 bash tests/fm-pi-primary-live-e2e.test.shFM_GROK_LIVE_E2E=1 bash tests/fm-grok-continuity-live-e2e.test.shgit status --shortand transient-live-lab scan🔧 Fix: Synchronize Pi and OpenCode late-close lifecycle fixtures
✅ Re-checked - no issues remain.
command -v tmux >/dev/null || { echo "tmux is required for e2e tests" >&2; exit 1; }; tmux -V; rc=0; for t in tests/*.test.sh; do echo "== $t =="; bash "$t" || rc=1; done; exit "$rc"Baseline already completed successfully:command -v tmux >/dev/null || { echo "tmux is required for e2e tests" >&2; exit 1; }; tmux -V; rc=0; for t in tests/*.test.sh; do echo "== $t =="; bash "$t" || rc=1; done; exit "$rc"bash tests/fm-pi-watch-extension.test.shin clean isolationTwo concurrentbash tests/fm-pi-watch-extension.test.shrunsbash tests/fm-watcher-lock.test.shbash tests/fm-continuity-pretool-check.test.shFM_CLAUDE_LIVE_E2E=1 bash tests/fm-claude-continuity-live-e2e.test.shFM_CODEX_LIVE_E2E=1 bash tests/fm-codex-continuity-live-e2e.test.shFM_OPENCODE_LIVE_E2E=1 bash tests/fm-opencode-primary-live-e2e.test.shFM_PI_LIVE_E2E=1 bash tests/fm-pi-primary-live-e2e.test.shFM_GROK_LIVE_E2E=1 bash tests/fm-grok-continuity-live-e2e.test.shVerified no live-E2E temporary directories or worktree changes remained after cleanup.✅ **Document** - passed
✅ No issues found.
✅ **Lint** - passed
✅ No issues found.
✅ **Push** - passed
✅ No issues found.