Skip to content

fix(bin): keep watcher supervision continuous across child cycles#693

Open
kunchenguid wants to merge 19 commits into
mainfrom
fm/fm-watcher-continuity-fix
Open

fix(bin): keep watcher supervision continuous across child cycles#693
kunchenguid wants to merge 19 commits into
mainfrom
fm/fm-watcher-continuity-fix

Conversation

@kunchenguid

Copy link
Copy Markdown
Owner

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

  • Keep watcher arms alive across verified successor cycles, emit typed failures for unexplained clean exits, and record bounded per-cycle lifecycle diagnostics.
  • Have Pi and OpenCode re-arm watcher successors with lock checks, single-flight bounded retries, and restoration-failure wake fallbacks after child closes.
  • Add Claude's narrow continuity gate, allowing wake-drain and arm recovery while blocking other fleet scripts only when in-flight work lacks a live watcher.

Risk Assessment

⚠️ Medium: The implementation is source-clean on review, but it changes a broad, timing-sensitive supervision path across multiple adapters.

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
ok - Pi 0.80.10 live E2E used shared Codex auth, auto-started one successor before turn end, and cleaned up
Evidence: OpenCode credentialed TUI continuity transcript
ok - OpenCode 1.17.18 live E2E auto-started one successor before prompt handling without a model re-arm
Evidence: Claude credentialed continuity-gate transcript
ok - Claude 2.1.214 (Claude Code) live E2E refused only the post-completion fleet command with exact re-arm guidance
Evidence: Codex credentialed checkpoint transcript
ok - codex-cli 0.144.5 live E2E preserved the one-second foreground checkpoint path
Evidence: Grok credentialed continuity transcript
ok - grok 0.2.103 (89c3d36fb6f1) [stable] live E2E preserved tracked background completion and shared ledger classification
Evidence: Pi/OpenCode extension suite under concurrent load
ok - Pi primary watcher extension is tracked, self-hashing, and self-locating
ok - Pi secondmate launch wiring includes both tracked primary extensions
ok - Pi extension reports external healthy watcher output
ok - Pi custom tool returns text content and structured details
ok - Pi actionable close starts one successor before wake delivery settles
ok - Pi hung successor falls back to one typed actionable wake
ok - Pi unretired successor falls back without an overlapping retry
ok - Pi late unretired closes resume classified supervision
ok - Pi clean empty close triggers a bounded continuity retry
ok - Pi established clean closes stop at the configured retry limit
ok - Pi close handler verifies session-lock ownership before successor launch
ok - Pi watcher arm distinguishes all session lock ownership states
ok - Pi process-exit cleanup listener has a bounded lifecycle
ok - Pi process-exit cleanup stops the attached arm child
ok - OpenCode primary watcher plugin has the verified TUI wake wiring
ok - OpenCode plugins have an explicit ESM boundary even under a typeless parent package
ok - OpenCode watcher plugin uses the effective FM_HOME state
ok - OpenCode watcher plugin sources the effective config
ok - OpenCode watcher plugin requires session lock ownership
ok - OpenCode watcher coordinator respects primary scope
ok - OpenCode watcher plugin starts one successor before wake prompt delivery settles
ok - OpenCode pre-ready actionable close preserves its successor
ok - OpenCode hung successor falls back to one typed actionable wake
ok - OpenCode unretired successor falls back without an overlapping retry
ok - OpenCode late unretired closes resume classified supervision
ok - OpenCode clean empty close triggers a bounded continuity retry
ok - OpenCode established clean closes stop at the configured retry limit
ok - OpenCode close handler verifies session-lock ownership before successor launch
ok - OpenCode watcher plugin coordinates with the turn-end guard
ok - OpenCode healthy arm output does not suppress the turn-end guard
Evidence: Watcher lock, peer-attach, and lifecycle-ledger evidence
ok - simultaneous watcher starts leave exactly one live process
ok - fm_pid_identity is locale-invariant across LC_ALL/LC_TIME
ok - killed watcher stale lock is reclaimed
ok - live watcher lock with stale heartbeat is actionable
ok - guard banner leads when down with pending wakes (re-arm-after-drain) and stays silent when fresh
ok - concurrent fm_lock_try_acquire yields exactly one winner
ok - dead-pid stale lock is reclaimed by a single acquirer
ok - concurrent stale-lock steal yields exactly one winner
ok - live steal mutex is not reclaimed
ok - live-held lock is not stolen
ok - empty mid-acquire lock keeps a minimum grace
ok - late original claimant cannot claim a recreated lock
ok - paused mid-acquire claimant backs off to active stealer
ok - watch restart refuses to signal a reused pid
ok - watch restart attaches to a verified healthy peer and later surfaces a successor gap
ok - watcher self-evicts when the lock pid no longer names it
ok - arm turns clean self-eviction without a successor into a typed failure
ok - arm attaches to a live fresh watcher and fails loudly when that cycle has no successor
ok - attached arm signals record a classified lifecycle entry
ok - arm starts+confirms a fresh watcher on a clean lock and self-heals a dead-pid lock (never healthy off a dead pid)
ok - arm cleans child watcher and temp output on HUP
ok - arm propagates an immediate watcher wake before confirmation
ok - arm attaches to a peer watcher after child stands down and surfaces a missing successor
watcher: lock held by live pid 84446 but heartbeat is stale for 837652020s (>300s); inspect or stop that watcher before re-arming.
ok - arm reports FAILED and exits non-zero when no fresh watcher can be confirmed
ok - cycle-exit ledger links a verified successor and remains size-capped
ok - SIGSTOP distinguishes live PID from stale beacon and termination records the exit class
- Outcome: 🔧 1 issue found → auto-fixed (2) ✅ across 3 runs (1h47m25s)

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 emits started/attached and then immediately exits unexpectedly resets retryFailures before 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 resets retryFailures before an unexpected close. A watcher that starts and then repeatedly exits will bypass retryLimit forever, 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 enters attach_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 for successorReady before calling sendWake. If the replacement arm hangs before emitting started or attached, that promise never settles and the actionable wake is never delivered. Decide whether delivery should be dispatched immediately after startArm is 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 for ensureArm to resolve before it calls promptAsync. 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 clears child immediately 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 later close event is permanently ignored because it remains in abandonedChildren. 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, mutable child now 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) ✅
  • 🚨 tests failed with exit code 1
  • 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.sh failed 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.
  • ⚠️ No open PR exists for fm/fm-watcher-continuity-fix (gh-axi pr list --state open --head fm/fm-watcher-continuity-fix returned 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.sh
  • bash tests/fm-continuity-pretool-check.test.sh
  • bash tests/fm-pi-watch-extension.test.sh - clean rerun after isolated late-close reproduction
  • FM_CLAUDE_LIVE_E2E=1 bash tests/fm-claude-continuity-live-e2e.test.sh
  • FM_CODEX_LIVE_E2E=1 bash tests/fm-codex-continuity-live-e2e.test.sh
  • FM_OPENCODE_LIVE_E2E=1 bash tests/fm-opencode-primary-live-e2e.test.sh
  • FM_PI_LIVE_E2E=1 bash tests/fm-pi-primary-live-e2e.test.sh
  • FM_GROK_LIVE_E2E=1 bash tests/fm-grok-continuity-live-e2e.test.sh
  • git status --short and 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.sh in clean isolation
  • Two concurrent bash tests/fm-pi-watch-extension.test.sh runs
  • bash tests/fm-watcher-lock.test.sh
  • bash tests/fm-continuity-pretool-check.test.sh
  • FM_CLAUDE_LIVE_E2E=1 bash tests/fm-claude-continuity-live-e2e.test.sh
  • FM_CODEX_LIVE_E2E=1 bash tests/fm-codex-continuity-live-e2e.test.sh
  • FM_OPENCODE_LIVE_E2E=1 bash tests/fm-opencode-primary-live-e2e.test.sh
  • FM_PI_LIVE_E2E=1 bash tests/fm-pi-primary-live-e2e.test.sh
  • FM_GROK_LIVE_E2E=1 bash tests/fm-grok-continuity-live-e2e.test.sh
  • Verified 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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant