Skip to content

fix: hide Pi watcher wakes and harden supervision state#973

Open
BazsaX254 wants to merge 5 commits into
kunchenguid:mainfrom
BazsaX254:fm/hide-pi-watcher-prompts
Open

fix: hide Pi watcher wakes and harden supervision state#973
BazsaX254 wants to merge 5 commits into
kunchenguid:mainfrom
BazsaX254:fm/hide-pi-watcher-prompts

Conversation

@BazsaX254

Copy link
Copy Markdown

Intent

Fix FirstMate's Pi watcher bridge so internal signal, stale, result/check, and monitoring notifications wake the model through trusted hidden transport without rendering raw watcher prompts, local paths, queue-drain commands, or supervision instructions in captain chat. Preserve durable queue-first handling, model-free monitoring, exactly-one successor arm, extension-owned continuity, turn-end safety, actionable failure wakes, and all supported harness/backend behavior. Treat a current run-step-backed no-mistakes validation decision as a verified parked wait rather than a stall; retain bounded cadence for declared external waits; continue escalating genuinely stopped, looping, confused, or unresponsive workers; and retire watcher tracking for torn-down worker identities. Require deterministic unit and strict-TypeScript coverage plus a live interactive Pi 0.81.1 regression proving hidden delivery is received exactly once without transcript leakage, with focused, broad, lint, documentation, PR, and CI validation.

What Changed

  • Deliver Pi watcher wakes through non-displayed custom messages, keeping internal prompts, paths, and queue instructions out of captain chat while preserving follow-up turns and extension-owned continuity.
  • Treat current run-step-backed parked validation decisions as verified waits while retaining bounded external-wait checks and actionable stale-worker escalation.
  • Revalidate queued signals and clear watcher deduplication state during teardown to prevent retired-worker races, empty-queue wakes, and suppressed events when task IDs are reused; expand unit, strict TypeScript, live Pi, and documentation coverage.

Risk Assessment

✅ Low: The append-count guard closes the empty-queue wake race, and the deterministic regression covers teardown between active filtering and enqueue while preserving exactly-once wake behavior after task-ID reuse.

Testing

Targeted commit inspection, deterministic Pi/OpenCode watcher tests, strict Pi 0.81.1 TypeScript checking, triage/queue/checkpoint/teardown tests, model-free daemon lifecycle checks, and a credentialed interactive Pi 0.81.1 run passed the requested behavior; the live test needed one retry after an unrelated provider-sentinel fluctuation. The relevant Calm operational-follow-up path passed, but its later rendered-export section could not run because Chrome/Chromium is absent, and the broader daemon script eventually hit an unrelated root-user chmod assumption after all relevant daemon cases passed. No screenshot was produced because the isolated terminal harness deletes its tmux lab on exit; its reviewer-visible assertion transcript is preserved as the live log. The worktree remained clean with no surviving test processes or transient lab directories.

Evidence: Live Pi 0.81.1 acceptance

ok - Pi 0.81.1 live E2E covered hidden exactly-once watcher transport, native Calm Working visibility, Ahoy first/later messages, legacy transcripts, near misses, and watcher continuity

ok - Pi 0.81.1 live E2E covered hidden exactly-once watcher transport, native Calm Working visibility, Ahoy first/later messages, legacy transcripts, near misses, and watcher continuity
Evidence: Deterministic watcher extension
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 exposes repair-only metadata and returns automatic-continuation guidance
ok - Pi redundant tool call returns ownership guidance and spawns no second child
ok - Pi scheduled retry remains extension-owned after another tool call
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 triage and parked-wait behavior
ok - signal_reason_is_actionable: benign absorbed, captain verbs and coalesced batches surfaced
ok - stale_is_terminal: terminal status surfaces, non-terminal and no-status are benign
ok - scan_captain_relevant_statuses lists only current captain-relevant worker statuses
ok - classifier primitives: keyed decisions and activity phases, captain relevance, window-to-task, and overrides
ok - crew_is_provably_working: active work is provable and only run-step parked state becomes a verified wait
ok - status_is_paused: only the leading paused verb matches, and paused is not captain-relevant
ok - crew_absorb_class: working/waiting/paused/none from one read; crew_is_paused and crew_is_provably_working agree
ok - signal_crew_provably_working: benign only when every referenced crew is provably working
ok - a no-verb signal whose crew is provably working is absorbed (no exit, no queue, suppressor advanced, beacon present)
ok - a bare turn-end whose crew is provably working (busy pane) is absorbed
ok - a bare turn-end whose crew is not provably working is surfaced (the swallowed-finish fix)
ok - a no-verb working: note whose crew is idle with no running pipeline is surfaced
ok - captain-relevant signal is surfaced (queue + exit) and marked surfaced
ok - a stale pane sitting on a terminal status is surfaced (queue + exit)
ok - a stale terminal-looking status is overridden and absorbed while a run is actively working, then wedge-escalated
ok - provably-working non-terminal stale is absorbed on first sight, then wedge-escalated past the threshold
ok - consecutive wedge escalations on the same pane accumulate and demand deep inspection at the threshold
ok - a pane becoming active again resets the consecutive wedge-escalation counter
ok - a not-provably-working non-terminal stale is surfaced immediately (never left to wait out the timer)
ok - a declared pause is absorbed on first sight, then re-surfaced as a recheck past the threshold, never wedge-escalated
ok - verified no-mistakes decisions remain absorbed without stale or wedge escalation
ok - exited declared-pause and captain-held panes use bounded pause cadence while a live decision gate still surfaces once
ok - a declared paused secondmate re-surfaces on the bounded normal-mode cadence
ok - a non-paused secondmate retains normal stale suppression
ok - a resumed secondmate clears pause and stale tracking before stale exemption
ok - unchanged stale hashes reclassify when a crew enters or leaves pause
ok - a declared pause is periodically rechecked against authoritative active-run state
ok - a paused status overridden by authoritative working preserves its wedge timer and escalates
ok - matching non-terminal stale suppressors repair missing or corrupt stale-since timers
ok - triage log capping handles wc byte counts with leading spaces
ok - a heartbeat with no captain-relevant change is absorbed and backs off the cadence
ok - heartbeat backstop fail-safe surfaces a captain-relevant status the per-wake path missed
ok - the liveness beacon stays fresh while the watcher absorbs benign wakes (fm-guard never false-alarms)
ok - with .afk present the watcher reverts to one-shot so the daemon owns triage (no double-triage)
ok - AFK changed paused panes hand off plain stale identities for daemon-owned pause triage
Evidence: Durable wake queue
ok - concurrent append plus drain preserves queue records
ok - signal written while no watcher runs is caught on next run
ok - stale wake is queued before suppressor state is advanced
ok - a not-provably-working stale wake is queued before its suppressor is advanced
ok - registered custom check output is queued before cadence suppression
ok - two atomic drains cannot consume the same records twice
ok - drain collapses obvious duplicate heartbeat and signal records
ok - drain asserts watcher liveness: warns on a lapse, stays silent right after a fire
ok - structural signal enrichment is separate, deduped, home-local, and tier-zero for other wakes
ok - bounded reads and per-item/global caps fail open with explicit truncation and omission markers
ok - slow annotation releases the append lock and a deleted status file fails open
ok - interruptions restore before commitment and never replay after raw commitment
Evidence: Model-free daemon lifecycle
ok - lifecycle: routine self-handles, terminal survives a watcher restart, buffers once, no dup, injects once
ok - lifecycle: stale pane transient self-handles, persistent escalates once and clears, resumed clears quietly
Evidence: Worker identity teardown
ok - local-only worktree with HEAD on a fork remote is torn down (fix holds)
ok - teardown prompts tasks-axi backlog refresh when compatible
ok - teardown honors config/backlog-backend=manual even when tasks-axi is compatible
ok - local-only worktree with truly unpushed work is refused (safety preserved)
ok - local-only worktree with work merged into local main is torn down (no regression)
ok - no-mistakes worktree with HEAD on origin is torn down (no regression)
ok - no-mistakes worktree with genuinely unlanded work is refused (safety preserved)
ok - local-only worktree with unpushed work is torn down under --force (escape hatch)
ok - teardown drops collected retired signals before the task ID is reused
ok - teardown after signal filtering cannot wake without a durable queue record
ok - herdr projection teardown retires its journal only after confirming the exact recorded pane is gone
ok - herdr projection teardown retains the stale journal and attempts no workspace cleanup when exact-pane close is unconfirmed
ok - squash-merged + deleted-branch worktree (PR merged) is torn down (the fix)
ok - squash-merged PR accepts a local HEAD that is an ancestor of the final PR head
ok - teardown discovers a merged PR by branch name and tears down when no pr= was ever recorded
ok - squash-merged PR accepts replayed unpushed local patches contained in the PR head
ok - merged PR does not allow teardown after a later local commit
ok - fm-pr-check does not refresh PR head after HEAD moves
ok - fm-pr-check records the remote PR head when the local worktree lags
ok - worktree whose content already landed in the default branch is torn down (content fallback)
ok - content fallback refreshes origin default before comparing trees
ok - dirty worktree is refused even when its committed work has landed (dirty always wins)
ok - gh lookup error with content not in default refuses (fail-safe)
ok - provably-stale worktree index.lock (old, no live holder) is cleared and teardown succeeds
ok - live-held worktree index.lock is never removed and teardown refuses
ok - lsof errors leave worktree index.lock in place and refuse teardown
ok - stale lock cleanup rechecks and refuses dirty worktree before return
ok - normal repo index.lock is resolved from the worktree and cleared when stale
ok - lock mtime read failures leave worktree index.lock in place and refuse teardown
ok - transient index.lock cleared after first failed return is retried successfully without force-remove
ok - persistent index.lock exhausts retries and refuses without force-removing the lock
ok - empty retry wait overrides use the default without aborting teardown
ok - fractional legacy retry wait remains supported without arithmetic
Evidence: Strict TypeScript contract

ok - tracked Pi extensions pass strict no-emit typecheck against Pi 0.81.1

ok - tracked Pi extensions pass strict no-emit typecheck against Pi 0.81.1
- Outcome: ⚠️ 1 warning across 1 run (12m13s)

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-teardown.sh:1214 - The required criterion “retire watcher tracking for torn-down worker identities” is incomplete here: this hunk deletes pane-keyed state but leaves task-keyed .seen-$ID_status, .seen-$ID_turn-ended, .hb-surfaced-$ID, and .subsuper-seen-status-$ID markers. Because task IDs can be reused after teardown, an identical new status can match these old suppressors and be treated as already surfaced, losing a genuine wake. Clear all task-keyed watcher/daemon dedup state during teardown and cover ID reuse.

🔧 Fix: Retire task dedup state during worker teardown
1 error still open:

  • 🚨 bin/fm-teardown.sh:1216 - The required “retire watcher tracking for torn-down worker identities” guarantee remains racy: this rm can finish while fm-watch.sh is sleeping on an already-collected signal, after which the watcher writes the old signature back to .seen-* without rechecking metadata (lines 779–824). Reusing that task ID with the same signature can again suppress its actionable status. Decide whether retirement must cover concurrent watcher scans; recommended smallest fix is to revalidate each pending row after the grace period and ensure any marker written after teardown is removed when its task metadata is absent, with a synchronized teardown-during-grace regression. Declining leaves the accepted cleanup reliable only when teardown does not overlap signal processing.

🔧 Fix: Drop retired signals across watcher teardown races
1 error still open:

  • 🚨 bin/fm-watch.sh:845 - The required “Preserve durable queue-first handling” invariant is violated by the new second revalidation: if teardown removes every task after active_pending is built but before lines 823–825, all rows are skipped, yet line 845 still emits the signal wake with no durable queue record. The smallest fix is to count successful fm_wake_append calls and continue monitoring instead of calling wake when none were appended; add a regression paused between active filtering and enqueue. Without it, the model can receive a wake whose required queue drain is empty.

🔧 Fix: Prevent signal wakes without durable queue records
✅ Re-checked - no issues remain.

⚠️ **Test** - 1 warning
  • ⚠️ tests/fm-pi-primary-live-e2e.test.sh:240 - The credentialed live regression is provider-sensitive: its first run stopped before watcher delivery because the model omitted the requested PRIOR_BOUNDARY_ACK sentinel and answered “Nothing happened after your previous message.” A bounded retry passed the complete Pi 0.81.1 hidden-delivery regression.
  • git status --short --branch and commit-range diff/name inspection against 10ee7797e50c88c9865d8fb382cdfee5c2b8bcd1
  • tests/fm-pi-watch-extension.test.sh
  • PATH="/root/.hermes/lsp/node_modules/typescript/bin:$PATH" tests/fm-pi-primary-types.test.sh
  • tests/fm-watch-triage.test.sh
  • tests/fm-teardown.test.sh
  • PATH="/root/.hermes/node/bin:$PATH" tests/fm-calm-pi-extension.test.sh
  • PATH="/root/.hermes/node/bin:$PATH" FM_PI_LIVE_E2E=1 tests/fm-pi-primary-live-e2e.test.sh (two attempts; retry passed)
  • tests/fm-wake-queue.test.sh
  • tests/fm-watch-checkpoint.test.sh
  • tests/fm-wake-daemon-lifecycle-e2e.test.sh
  • tests/fm-daemon.test.sh
  • Final git status --short, transient lab search, and live-process check
✅ **Document** - passed

✅ No issues found.

⚠️ **Lint** - 1 warning
  • ⚠️ linter found issues (exit code 127)
✅ **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