fix(grok): Prevent spurious wake run after in-turn monitors#4218
fix(grok): Prevent spurious wake run after in-turn monitors#4218mwolson wants to merge 2 commits into
Conversation
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
ApprovabilityVerdict: Needs human review This PR introduces new state tracking ( You can customize Macroscope's approvability policy. Learn more. |
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 3 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit fe55e2a. Configure here.
- Defer continuation offers for background completions that land while a root turn is active and un-finalized; finalize offers exactly one wake only when unhandled completed work remains - Stop late monitor-event mutations from erasing in-turn handled marks, so injected-turn ack chatter is never retained as wake evidence - Clear stale wake buffer frames at non-continuation user-turn start - Make the late-mutation suppress logic's running/terminal branches mutually exclusive

Base
This branch is stacked on #4193 (
fix/ctm-post-merge-ci) so CI's Test jobruns against aligned post-#3578 fixtures. The first commit
(
test(orchestrator): Align post-merge CTM fixtures) belongs to #4193;please review only the second commit here. Once #4193 lands in
codex-turn-mapping, this branch rebases back to a single commit above thestack base.
Summary
"Background task completed." continuation run while the original turn was
still streaming, after the agent had already reported the monitor result
in-turn.
<monitor-event>acknowledgement chatter for work alreadyreported in-turn is never retained as wake evidence for later turns.
deferral, the legitimate unhandled-completion wake, and the
settle-without-report hold interaction.
Problem and Fix
x.ai/task_completedext notification landing mid-turn reachedapplyLateBackgroundMutation, which had no notion of an active un-finalized root turn. With the running set empty and stale frames inwakeBuffer, it calledofferContinuationRunand dispatched a "Background task completed." run (queue_after_active) while the original turn was still streaming its own report.midTurnUnreportedCompletedTaskIdsset, andfinalizeTurnoffers exactly one continuation after finalize when legitimate evidence remains (settled statuscompleted, no running tasks). The arming also requires the prompt not to have settled, so the settled-held window still uses the pre-existing injected-report path.<monitor-event>turn hit the late-mutation path, which deleted the task id from the in-turn handled set. The subsequent injected ack chatter then failed the handled-chatter guard and was buffered as wake evidence, surviving into later turns (the buffer was intentionally not cleared at turn start).wakeBufferframes are cleared at non-continuation user-turn start. Frames cleared there cannot be legitimate: a genuinely unhandled completion is tracked by the mid-turn set and offered at finalize instead.Defensive Fixes
suppressPostSettleMonitorPromptwas set true and then unconditionally set false two lines later for terminal mutations on already-ended tasks, making the intent unreadable and the true branch dead.quarantineStoppedRunalso clearsmidTurnUnreportedCompletedTaskIds.midTurnUnreportedCompletedTaskIdssurvived the settle-hold: when the CLI's injected report streamed into the held turn, onlypendingInjectedReportwas cleared, sofinalizeTurnstill saw the armed id and offered a duplicate continuation right after the report projected (Bugbot round 1).midTurnUnreportedCompletedTaskIds. If no report ever streams, the id survives and finalize offers exactly one wake, as before.wakeBuffer(the buffer cannot grow while the root turn is active), so if the CLI's late frames had not arrived by the wake run's start, the empty drain finalized a blank continuation run immediately (Bugbot round 1).finalizeTurnclearedmidTurnUnreportedCompletedTaskIdsunconditionally, but the offer gate requires the running set to be empty. A task armed pre-settle lost its mark when the turn finalized while a second task was still running, and the second task's bare end-notice frame is excluded from wake evidence, so no continuation was ever offered for either completion (Bugbot round 2).completedand background work is still running, so the post-finalize gate offers exactly once when the last task ends. Interrupted and failed turns still clear unconditionally, and non-continuation turn start clears too, so kept marks cannot wake after an interrupt or arm a later user turn.Validation
vp check: pass (0 errors, 63 pre-existing warnings)vp run typecheck: pass (all packages)AcpAdapterV2.test.ts,GrokAdapterV2.test.ts): 98/98,including 9 new tests
original two-turn repro produced no spurious wake and no replayed acks;
legitimate post-settle monitor and detached-command scenarios each produced
exactly one continuation; interrupt/steer/queue scenarios stayed clean
Note
Medium Risk
Changes orchestration timing for Grok background monitors and continuation dispatch; behavior is heavily tested but regressions could affect wake timing or legitimate post-settle continuations.
Overview
Fixes Grok/ACP post-settle continuation logic so in-turn monitor work does not spawn extra "Background task completed." runs or carry stale wake evidence across turns.
AcpAdapterV2defers continuation offers while a root turn is still active: unhandled mid-turn completions are tracked inmidTurnUnreportedCompletedTaskIdsand a single offer is made fromfinalizeTurn(or post-finalize late mutations) when appropriate. Late monitor mutations no longer clear in-turn handled marks; agent/thought chatter for handled tasks is blocked before wake buffering; user turns clearwakeBuffer; streaming an injected report clears matching mid-turn arms; empty-drain continuation runs use the deferred quiet window when configured; quarantine clears the new set.Adds extensive
AcpAdapterV2.test.tscoverage for settle-hold, staggered monitors, multiturn ack chatter, mid-turn deferral, and empty-drain continuations.Also updates
config.test.tsexpectations touserdata-v2and refreshes two Claude replay fixtures’ expectedquery.opentool lists.Reviewed by Cursor Bugbot for commit 71671fa. Bugbot is set up for automated code reviews on this repo. Configure here.
Note
Fix spurious wake runs after in-turn monitor completions in
AcpAdapterV2midTurnUnreportedCompletedTaskIdsto track background tasks that reach a terminal state mid-turn without being reported in-turn, deferring wake offers until after turn finalization.Background task completed.continuations.wakeBuffer, preventing spurious post-settle wake runs for already-handled background work.midTurnUnreportedCompletedTaskIdson interrupted/quarantined turns and when injected reports stream, preventing stale deferred wakes.Macroscope summarized 71671fa.