Skip to content

fix(bin): suppress empty Grok watcher turns - #2250

Open
4mb1t10n wants to merge 15 commits into
kunchenguid:mainfrom
4mb1t10n:fm/fm-2227
Open

fix(bin): suppress empty Grok watcher turns#2250
4mb1t10n wants to merge 15 commits into
kunchenguid:mainfrom
4mb1t10n:fm/fm-2227

Conversation

@4mb1t10n

@4mb1t10n 4mb1t10n commented Aug 12, 2026

Copy link
Copy Markdown

Intent

Implement #2227 exactly as specified by the full issue. Eliminate billable empty Grok watcher-cycle model turns while preserving every actionable wake, OPEN DECISIONS entry, watcher failure, durable-queue/lifecycle semantic, session-lock ownership boundary, away-mode supervision transfer, and supervision safety. Focus only on eliminating billable empty Grok watcher-cycle turns; exclude unrelated harness and quota work and exclude the upstream Grok automatic session recap behavior. Target main. Reproduce the billing-triggering control flow and add focused behavioral regression evidence. For the unchanged remote-decision watcher regression, inspect its exact expected contract and compare the owning implementation and fixture against main before changing implementation; if unrelated or pre-existing, record equivalent focused evidence, and if caused by this branch, fix it causally rather than repeating the same test-only attempt. No database or browser behavior is involved; do not start a stack and never run Cypress locally. Complete No Mistakes, obtain exact-head authoritative CI and mergeability, never merge, and ensure the final PR body contains a standalone Closes #2227.

What Changed

  • Add a persistent Grok watcher owner that internally re-arms empty closes without completing a billable background model turn.
  • Preserve actionable wakes, open decisions, recovery episodes, failures, session-lock boundaries, and away-mode supervision transfers.
  • Update Grok supervision guidance, command policy, and focused regression coverage for the persistent owner.

Closes #2227

Decisions

  • Chose Fix under ship-cycle authority: after every watcher child closes and immediately before actionable completion, recheck session-lock ownership and .afk. If ownership transferred or away mode took supervision, keep the former Grok task dormant and leave durable work or failure evidence for the current owner. Executable coverage exercises actionable-close, classification-failure, and arm-failure transfer sequences.

Risk Assessment

✅ Low: The change now consistently enforces the ownership and away-mode boundary across all completion paths, preserves transferred failures through the durable queue, and performs bounded identity- and membership-checked child retirement without a substantiated remaining defect.

Testing

Targeted shell tests exercised the end-user Grok tracked-task flow and rendered launch/policy surfaces: empty closes re-armed without completing the tracked task, while durable wakes, OPEN DECISIONS, recovery states, and failures completed visibly; ownership transfer, away mode, lifecycle, bounded-output, and cleanup safety also passed. Equivalent open-decision coverage passed in the new suite, and the unchanged remote-decision contract matched base. No visual artifact was attempted because the authoritative intent explicitly excludes browser behavior.

Evidence: Grok persistent watcher behavioral transcript
ok - empty close re-arms internally while a durable actionable wake completes the tracked Grok task
ok - arm failure completes the Grok task loudly
ok - bounded output overflow retires the child and fails once
ok - transferred bounded overflow queues once without completing
ok - no-newline output is bounded and retires the child
ok - TERM-resistant overflow forcibly retires its exact process tree
ok - retirement skips changed identities and preserves unrelated processes
ok - retirement skips unchanged identities outside the current tree
ok - transferred no-newline overflow stays bounded and queues once
ok - an OPEN DECISIONS entry completes the Grok task even without a queue row
ok - a pending recovery episode completes the Grok task even without a queue row
ok - malformed recovery state completes the Grok task with a loud failure
ok - an empty close with no supervision need stays dormant until work returns
ok - away mode keeps the Grok owner dormant until normal supervision returns
ok - a Grok task whose session lost the fleet lock stays dormant and does not re-arm
ok - an actionable close stays dormant when away mode takes supervision
ok - an actionable close stays dormant after session-lock ownership transfers
ok - away transfer during actionable classification keeps the old owner dormant
ok - session transfer during actionable classification keeps the old owner dormant
ok - away transfer during failed classification keeps the old owner dormant
ok - session transfer during failed classification keeps the old owner dormant
ok - away transfer during arm failure classification keeps the old owner dormant
ok - session transfer during arm failure classification keeps the old owner dormant
ok - terminating the tracked Grok owner retires its arm child

Pipeline

Updates from git push no-mistakes

✅ **intent** - passed

✅ No issues found.

✅ **Rebase** - passed

✅ No issues found.

🔧 **Review** - 1 issue found → auto-fixed (12) ✅
  • 🚨 bin/fm-watch-arm-grok.sh:189 - The required “session-lock ownership boundary” and “away-mode supervision transfer” remain bypassable when either state changes while the child arm is running: after the child closes, durable_action_pending can return actionable and this branch immediately completes the old Grok task without rechecking ownership or .afk. That wakes a session which no longer owns supervision (or competes with away-mode supervision). Recheck these boundaries after every child close and before actionable completion; when ownership transferred, leave the durable work for its new owner and keep this old task dormant. The added tests cover transfer only after an empty close, so they do not exercise this reachable sequence.

🔧 Fix: Guard actionable Grok closes after supervision transfer
1 error still open:

  • 🚨 bin/fm-watch-arm-grok.sh:196 - The ownership/away check occurs before durable_action_pending, which can wait on the queue lock and scan other state. If .afk appears or .lock transfers during that classification, verdict 0 still completes the old Grok task. Recheck supervision ownership immediately before this exit; the new tests transfer only inside the child arm and therefore miss this remaining sequence.

🔧 Fix: Recheck Grok ownership after durable classification
1 error still open:

  • 🚨 bin/fm-watch-arm-grok.sh:212 - A supervision transfer during durable_action_pending is handled only when the verdict is actionable. If .afk appears or the session lock changes while classification waits and classification then fails (for example, the queue lock times out), this branch still completes the old Grok task with a failure, violating the required away-mode/session-ownership boundary and waking a session that no longer owns supervision. Recheck supervision ownership after classification for every completing verdict; if ownership transferred, leave the old task dormant and let the new owner surface durable state or failure.

🔧 Fix: Centralize post-classification Grok ownership guard
1 error still open:

  • 🚨 bin/fm-watch-arm-grok.sh:183 - The required “session-lock ownership boundary” and “away-mode supervision transfer” remain bypassable for child-arm failures. After the check at line 176, failure detection can inspect the captured output; if .afk appears or .lock transfers during that work, lines 183–190 still complete the old Grok task and wake its former session. Route child failure through the same centralized ownership guard immediately before any task completion, and add executable coverage for transfer during failure classification.

🔧 Fix: Centralize all Grok completion ownership checks
1 error still open:

  • 🚨 bin/fm-watch-arm-grok.sh:199 - The captain required transferred completion paths to “leave durable work/failure evidence for the new owner,” and the intent requires preserving every watcher failure. Here a classification failure exists only in completion_failure; if ownership or away mode changes, lines 205–209 delete the stream and continue without printing or durably recording that failure. The old task then remains dormant, so neither it nor the new supervisor surfaces the failed classification. Persist the failure at the centralized completion boundary before yielding to the new owner, using the earliest existing durable failure/recovery channel.

🔧 Fix: Persist transferred Grok failures through recovery
1 error still open:

  • 🚨 bin/fm-watch-arm-grok.sh:205 - Transferred failures are reduced to a generic downtime marker, so the required watcher failure evidence is lost; additionally, publishing downtime rewrites an existing pending:handling:<generation> episode to pending:downtime:<generation>, making an already-delivered recovery actionable again and risking duplicate resurfacing. Persist the actual failure through the existing durable wake channel without changing an active recovery episode. The new tests only verify a fresh generic marker, not failure visibility to the successor or coexistence with pending:handling.

🔧 Fix: Queue exact transferred failures without reopening recovery
1 error still open:

  • 🚨 bin/fm-watch-arm-grok.sh:168 - The accepted transfer contract says not to print or complete the old task, but tee streams child output before the final ownership/away-mode check. A child can print watcher: FAILED, then ownership transfers while failure classification is blocked; the failure is durably queued for the successor, yet it has already been printed through the former owner. Buffer child output until the centralized completion boundary, emitting it only when this owner may complete; on transfer, publish the captured failure solely through the durable queue. This also avoids printing ordinary child failures twice via tee and line 215.

🔧 Fix: Buffer Grok terminal output behind ownership checks
1 error still open:

  • 🚨 bin/fm-watch-arm-grok.sh:169 - The accepted fix explicitly requires a “bounded reader,” but this loop writes every child-output line to output and usually a second copy to terminal without any byte or line limit. A noisy or malfunctioning arm can therefore grow state files until disk exhaustion and make terminal replay arbitrarily long, undermining supervision safety and widening the final ownership-transfer window. Enforce a finite capture limit and convert overflow into a typed failure handled through the existing completion boundary.

🔧 Fix: Bound Grok output and handle overflow safely
1 error still open:

  • 🚨 bin/fm-watch-arm-grok.sh:184 - The required finite output cap is bypassable by a child that emits an unbounded stream without newline characters. Bash read buffers until a delimiter or EOF, so this loop never reaches the byte-count check, cannot retire the child, and can grow memory indefinitely. This contradicts the required supervision safety and bounded-reader behavior. Consume fixed-size byte chunks or otherwise enforce the cap before newline framing, then route overflow through the existing centralized completion boundary. Add executable coverage using a no-newline producer.

🔧 Fix: Enforce Grok cap before newline framing
1 error still open:

  • 🚨 bin/fm-watch-arm-grok.sh:192 - Output overflow sends only SIGTERM and the parent then waits indefinitely for the child. A malfunctioning/noisy arm that ignores or mishandles SIGTERM can therefore keep producing output, leave the tracked owner hung, and prevent the typed overflow failure from reaching either owner. The new cleanup tests use cooperative TERM handlers, so they miss this reachable failure. Add a bounded TERM grace period followed by forced retirement of the child/process tree before completing through the centralized ownership boundary.

🔧 Fix: Bound Grok overflow process-tree retirement
1 error still open:

  • 🚨 bin/fm-watch-arm-grok.sh:104 - pids is a cached list collected before the TERM grace, then every numeric PID is sent SIGKILL without revalidating that it still belongs to this child tree. A cooperative descendant can exit during the grace and its PID can be reused by an unrelated process, which this loop would then kill—violating the supervision-safety requirement and the requested exact-child identity boundary. Before KILL, re-resolve descendants/process-group membership and validate identity; never signal stale cached PIDs.

🔧 Fix: Validate Grok child identities before forced retirement
1 error still open:

  • 🚨 bin/fm-watch-arm-grok.sh:131 - Forced retirement still retains every pre-TERM identity record and sends KILL based only on unchanged PID identity. If the root exits during the grace period and a descendant is reparented, that process is no longer in the currently resolved child tree but lines 131–134 retain and kill it anyway, violating the required current-membership check. Build KILL candidates only from the freshly resolved tree intersected with recorded identities; treat escaped former descendants as incomplete retirement without signaling them. The new mismatch test covers changed identity, not unchanged identity after reparenting.

🔧 Fix: Require current membership before forced Grok retirement
✅ Re-checked - no issues remain.

✅ **Test** - passed

✅ No issues found.

  • gh-axi issue view 2227 --full
  • bash tests/fm-grok-watch-arm.test.sh
  • bash tests/fm-supervision-instructions.test.sh
  • bash tests/fm-arm-pretool-check.test.sh
  • bash tests/fm-watch-arm.test.sh (the unchanged remote-decision fixture reached three passing cases before its pre-existing Linux process-kill behavior terminated the invoking shell)
  • git diff b5d430d6fdcd961ce9b681bf196f365c1825c284..636b1654872a93aa35765a2be091f8b155aebf1b -- tests/fm-watch-arm.test.sh bin/fm-watch-arm.sh bin/fm-watch-once.sh bin/fm-wake-daemon.sh (confirmed the remote-decision fixture and owning implementation are unchanged from base)
✅ **Document** - passed

✅ No issues found.

🔧 **Lint** - 1 issue found → auto-fixed ✅
  • ⚠️ linter found issues (exit code 1)

🔧 Fix: Fix subshell-safe classification lock path
✅ Re-checked - no issues remain.

✅ **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.

grok: every watcher cycle close bills a full model turn (no zero-token re-arm equivalent to the Claude Stop hook)

1 participant