Skip to content

fix(orchestrator/grok): Prevent spurious wake run after in-turn monitors#4218

Open
mwolson wants to merge 1 commit into
pingdotgg:t3code/codex-turn-mappingfrom
mwolson:fix/grok-v2.1
Open

fix(orchestrator/grok): Prevent spurious wake run after in-turn monitors#4218
mwolson wants to merge 1 commit into
pingdotgg:t3code/codex-turn-mappingfrom
mwolson:fix/grok-v2.1

Conversation

@mwolson

@mwolson mwolson commented Jul 21, 2026

Copy link
Copy Markdown
Contributor

Base

Single commit on codex-turn-mapping. #4193 (fix/ctm-post-merge-ci) has
landed, so the earlier stacked-commit note no longer applies.

Summary

  • Fixes a multiturn bug where the Grok adapter queued a spurious synthetic
    "Background task completed." continuation run while the original turn was
    still streaming, after the agent had already reported the monitor result
    in-turn.
  • Ensures injected <monitor-event> acknowledgement chatter for work already
    reported in-turn is never retained as wake evidence for later turns.
  • Adds four adapter tests covering the multiturn repro, mid-turn completion
    deferral, the legitimate unhandled-completion wake, and the
    settle-without-report hold interaction.

Problem and Fix

Problem and Why it Happened Fix
A x.ai/task_completed ext notification landing mid-turn reached applyLateBackgroundMutation, which had no notion of an active un-finalized root turn. With the running set empty and stale frames in wakeBuffer, it called offerContinuationRun and dispatched a "Background task completed." run (queue_after_active) while the original turn was still streaming its own report. Continuation offers are never made while a root turn is active and un-finalized. Completions that register mid-turn but are not handled in-turn are recorded in a new midTurnUnreportedCompletedTaskIds set, and finalizeTurn offers exactly one continuation after finalize when legitimate evidence remains (settled status completed, 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.
After a turn that reported its monitor in-turn, the CLI's injected <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). Late monitor-event mutations no longer erase in-turn handled marks, the handled-chatter guard runs before wake buffering, and stale wakeBuffer frames 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

Problem and Why it Happened Fix
In the late-mutation path, suppressPostSettleMonitorPrompt was 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. The suppress logic is rewritten as mutually exclusive running/terminal branches.
A stopped run's quarantine cleared the wake buffer and running set but would have left mid-turn unreported completions armed. quarantineStoppedRun also clears midTurnUnreportedCompletedTaskIds.
A completion armed pre-settle into midTurnUnreportedCompletedTaskIds survived the settle-hold: when the CLI's injected report streamed into the held turn, only pendingInjectedReport was cleared, so finalizeTurn still saw the armed id and offered a duplicate continuation right after the report projected (Bugbot round 1). When the injected report chunk streams, the delivered task ids are also removed from midTurnUnreportedCompletedTaskIds. If no report ever streams, the id survives and finalize offers exactly one wake, as before.
A midTurn-only continuation offer is made with an empty 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). An empty-drain continuation now uses the same 3s deferred-finalize quiet window as a non-empty drain (when the flavor defers finalize for background work), letting late frames attach; flavors without the defer flag keep the immediate finalize so the turn cannot wedge.
This PR adds the only wakeBuffer clear at the start of a non-continuation turn. An orchestrator-injected wake is not a user turn: #4499 (fix/delegated-task-parent-wake) dispatches app-owned delegated-child wakes as creationSource: "server", which is non-continuation, so once both land such a wake would discard this session's pending native wake frames and lose real agent output. acpIsAppOwnedWakeTurn exempts orchestrator-injected wakes from the clear, matching what ClaudeAdapterV2 already does on its own buffer. The exemption is inert on this branch alone, since nothing currently dispatches creationSource: "server"; it only takes effect once #4499 lands, and it is included here because this PR introduces the clear it guards.
finalizeTurn cleared midTurnUnreportedCompletedTaskIds unconditionally, 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). The marks are kept across finalize when the turn settled completed and 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)
  • Focused suites (AcpAdapterV2.test.ts, GrokAdapterV2.test.ts): 98/98,
    including 9 new tests
  • Live desktop retest on a rebuilt AppImage containing this commit: the
    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 continuation timing, wake-buffer clearing, and finalize behavior in orchestration-v2 ACP; regressions could miss legitimate wakes or wedge turns on empty continuations.

Overview
Fixes spurious "Background task completed." wake runs in the Grok/ACP adapter when monitors were already handled in-turn, while preserving a single legitimate continuation when background work finishes unreported.

AcpAdapterV2 defers continuation offers while a root turn is still active: mid-turn terminal mutations arm midTurnUnreportedCompletedTaskIds instead of calling offerContinuationRun, and finalizeTurn (or post-finalize late mutations) offers once when the running set is empty and wake evidence or those marks remain. In-turn hydration/reporting clears matching marks; streaming an injected report into a held turn clears them too so finalize does not double-wake.

Wake buffering no longer retains agent/thought chatter after in-turn-handled background work (guard runs before buffer). Late monitor-event mutations stop removing handledBackgroundTaskIdsInActiveTurn. User turns clear stale wakeBuffer and mid-turn marks at start; acpIsAppOwnedWakeTurn (agent + creationSource: "server") skips those clears so delegated-child wakes do not drop native pending frames.

Continuation attach with an empty drained buffer uses the deferred-finalize quiet window when the flavor defers for background work, instead of finalizing immediately. Quarantine and interrupt paths clear mid-turn marks appropriately; staggered multi-monitor finalize keeps marks until the last task ends.

Adds extensive AcpAdapterV2.test.ts coverage for settle-hold, pre-settle arms, multiturn ack chatter, empty-drain quiet window, and related edge cases.

Reviewed by Cursor Bugbot for commit c2a9e0f. Bugbot is set up for automated code reviews on this repo. Configure here.

Note

Fix spurious wake runs triggered by in-turn monitor completions in AcpAdapterV2

  • Introduces midTurnUnreportedCompletedTaskIds to track background tasks that reach a terminal state mid-turn without being reported in-turn, deferring any continuation offer until after finalizeTurn.
  • Residual agent_message_chunk/agent_thought_chunk chatter from monitors after in-turn-handled background work no longer fills wakeBuffer or triggers synthetic continuation runs.
  • Late monitor-event mutations no longer clear in-turn handled marks, preventing duplicate "Background task completed." wake runs.
  • Adds acpIsAppOwnedWakeTurn predicate to detect orchestrator-injected agent/server wake turns; these turns no longer clear pending wake evidence or mid-turn marks.
  • Empty-drain continuation turns now wait the quiet window (when deferFinalizeForBackgroundWork is true) instead of finalizing immediately, allowing late frames to attach.
  • Risk: significant changes to continuation scheduling logic across multiple code paths in AcpAdapterV2.ts.

Macroscope summarized c2a9e0f.

@coderabbitai

coderabbitai Bot commented Jul 21, 2026

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 913c16d1-807c-41b2-8333-e8667d52246f

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@github-actions github-actions Bot added vouch:unvouched PR author is not yet trusted in the VOUCHED list. size:L 100-499 changed lines (additions + deletions). labels Jul 21, 2026
@mwolson
mwolson marked this pull request as ready for review July 21, 2026 01:51
Comment thread apps/server/src/orchestration-v2/Adapters/AcpAdapterV2.ts
Comment thread apps/server/src/orchestration-v2/Adapters/AcpAdapterV2.ts
Comment thread apps/server/src/orchestration-v2/Adapters/AcpAdapterV2.ts
@macroscopeapp

macroscopeapp Bot commented Jul 21, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Needs human review

This PR modifies complex orchestration state machine logic controlling when wake/continuation runs are offered after background task completion. While well-tested, the changes affect runtime behavior in the orchestration layer and involve intricate state transitions that warrant human review.

You can customize Macroscope's approvability policy. Learn more.

Comment thread apps/server/src/orchestration-v2/Adapters/AcpAdapterV2.ts
Comment thread apps/server/src/orchestration-v2/Adapters/AcpAdapterV2.ts Outdated
Comment thread apps/server/src/orchestration-v2/Adapters/AcpAdapterV2.ts
Comment thread apps/server/src/orchestration-v2/Adapters/AcpAdapterV2.ts
Comment thread apps/server/src/orchestration-v2/Adapters/AcpAdapterV2.ts
Comment thread apps/server/src/orchestration-v2/Adapters/AcpAdapterV2.ts
@mwolson mwolson changed the title fix(grok): Prevent spurious wake run after in-turn monitors fix(orchestrator/grok): Prevent spurious wake run after in-turn monitors Jul 22, 2026
@juliusmarminge
juliusmarminge force-pushed the t3code/codex-turn-mapping branch 2 times, most recently from 1e58e65 to a286c60 Compare July 24, 2026 13:37
@mwolson
mwolson force-pushed the fix/grok-v2.1 branch 4 times, most recently from 3a467b5 to f62b5d4 Compare July 25, 2026 04:04

@cursor cursor Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit f62b5d4. Configure here.

Comment thread apps/server/src/orchestration-v2/Adapters/AcpAdapterV2.ts
- 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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:L 100-499 changed lines (additions + deletions). vouch:unvouched PR author is not yet trusted in the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant