Skip to content

fix(server): Grok monitor updates stay running until Grok says they finished - #13557

Merged
juliusmarminge merged 1 commit into
t3code/codex-turn-mappingfrom
v2/grok-monitor-status
Sep 25, 2026
Merged

juliusmarminge merged 1 commit into
t3code/codex-turn-mappingfrom
v2/grok-monitor-status

Conversation

@juliusmarminge

@juliusmarminge juliusmarminge commented Sep 25, 2026 •

Copy link
Copy Markdown
Member

Live Grok 1.0.41 recordings (#13537) showed that a Grok Monitor row completes at its first tick. The root run then settles while the monitored command is still running.

Cause

Grok streams every monitor tick as a tool_call_update with status: "in_progress", and its rawOutput is shaped like a finished shell result ({ type: "Bash", exit_code: 0, output_for_prompt: "tick 1\n" }). normalizeXAiAcpToolCallState inferred "completed" from any Bash rawOutput with exit_code: 0 and some text, ignoring Grok's own status. Recorded sequence for for i in 1 2 3; do sleep 8; echo tick $i; done:

  • tool_call monitor → tool_call_update variant: "Monitor" → start ACK { type: "Monitor", taskId, persistent: false }, plus _x.ai/task_backgrounded
  • one in_progress Bash-shaped update per tick, with a new _x.ai/monitor_event {task_id, event_text} for each
  • the end: _x.ai/task_completed {task_snapshot: {task_id, output, exit_code, kind: "monitor"}}. Grok never sends a completed status for the monitor tool itself, and the old <monitor-event …> and Monitor … ended text forms never appear as user_message_chunks.

Fix

  • An explicit in_progress or pending status from Grok now wins. The exit_code inference applies only when Grok sends no status.
  • Once that inference is gone, nothing ever finished the monitor row. _x.ai/task_completed is now authoritative: the lifecycle mutation carries the snapshot's final output and exit code (non-zero means failed). When a settled root turn is held open by deferred finalize for that task, the registered row finishes with that output and the turn can settle. While the prompt is still open, the existing TaskOutput hydration and mid-turn continuation paths are unchanged.

Live check through the real adapter after the fix: the monitor row stays running through tick 1 and tick 2, completes with tick 1\ntick 2\ntick 3\n when task_completed arrives, and the run completes after the 3 s quiet window. Before the fix it completed at tick 1.

Tests

  • New XAiAcpExtension tests, fed the exact frames recorded from Grok 1.0.41 through the adapter's parse path: the start update merged with the tick-1 in_progress update stays inProgress, and the recorded task_completed snapshot maps to a completed mutation with the final output (a non-zero exit code maps to failed). Without the fix the tick test fails with expected 'completed' to be 'inProgress'.
  • The old "completes wake re-reports…" test asserted the bug: its frame said inProgress and expected completed. It now covers the case the inference is still for, a Bash result with no status.
  • No replay fixture yet. Replaying this scenario needs ACP replay gates, which don't exist: the deferred-finalize debounce runs on the replay TestClock, and Grok's own wake-turn frames arrive right after task_completed, so replay has to hold them until the held turn settles. The fixture is planned for the gates PR that stacks on this one.

Verification

  • vp test run src/provider/acp/XAiAcpExtension.test.ts src/orchestration-v2/Adapters/AcpAdapterV2.test.ts src/orchestration-v2/testkit/OrchestratorReplayFixtures.integration.test.ts: 249 passed.
  • Reverting only the normalizer change: the recorded tick test fails as above.
  • vp exec tsc --noEmit -p . in apps/server: 0 error TS / warning TS. vp run knip:check: clean. vp lint on the touched files: no new warnings (two existing unused-symbol warnings in these files are on the base).
  • Not run: repo-wide suites.

Stacked on #13537.

Model: Claude Opus 5.5 (Claude Code)

🤖 Generated with Claude Code


Devin Review

@github-actions github-actions Bot added vouch:trusted PR author is trusted by repo permissions or the VOUCHED list. size:M 30-99 changed lines (additions + deletions). labels Sep 25, 2026
readonly output?: string;
}) {
const context = yield* Ref.get(activeTurn);
if (context === null || context.finalized || !context.promptSettled) return;

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.

🟠 High Adapters/AcpAdapterV2.ts:3596

When task_completed arrives before promptSettled is set, the registered tool row remains inProgress, so deferred finalization stays blocked and the run never completes. finishRegisteredBackgroundTool returns on !context.promptSettled, while applyLateBackgroundMutation has already cleared the running-task set; allow this terminal mutation to finish the row before the completion callback settles the prompt.

🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @apps/server/src/orchestration-v2/Adapters/AcpAdapterV2.ts around line 3596:

When `task_completed` arrives before `promptSettled` is set, the registered tool row remains `inProgress`, so deferred finalization stays blocked and the run never completes. `finishRegisteredBackgroundTool` returns on `!context.promptSettled`, while `applyLateBackgroundMutation` has already cleared the running-task set; allow this terminal mutation to finish the row before the completion callback settles the prompt.

const rawOutput = unknownRecord(withTitle.data.rawOutput);
const outputType = nonEmptyString(rawOutput?.type)?.toLowerCase();
if (outputType === "bash") {
const reportedRunning = withTitle.status === "inProgress" || withTitle.status === "pending";

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.

🟠 High acp/XAiAcpExtension.ts:510

A status-less terminal Bash re-report after an earlier inProgress update remains inProgress, so its exit_code is never applied and deferred finalization leaves the turn hanging. mergeToolCallState preserves the previous status via next.status ?? previous.status, causing reportedRunning to misclassify the current update; preserve whether status was present in the incoming update and terminalize status-less Bash results from their exit_code.

🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @apps/server/src/provider/acp/XAiAcpExtension.ts around line 510:

A status-less terminal Bash re-report after an earlier `inProgress` update remains `inProgress`, so its `exit_code` is never applied and deferred finalization leaves the turn hanging. `mergeToolCallState` preserves the previous status via `next.status ?? previous.status`, causing `reportedRunning` to misclassify the current update; preserve whether `status` was present in the incoming update and terminalize status-less Bash results from their `exit_code`.

@github-actions

github-actions Bot commented Sep 25, 2026 •

Copy link
Copy Markdown
Contributor

Thread transfer impact

✅ Thread transfer remains within every enforced ceiling.

ℹ️ No successful main baseline artifact is available yet. This run establishes the initial measurement.

Provider Metric Main baseline This PR Impact PR ceiling
Codex Total thread wire — 4.9 KiB — 6.8 KiB ✅
Codex Thread snapshot wire — 3.7 KiB — 4.9 KiB ✅
Codex Live turn WebSocket wire — 1.1 KiB — 2.0 KiB ✅
Codex Live turn WebSocket decoded — 20.4 KiB — 29.3 KiB ✅
Codex Live turn messages — 1 — 8 ✅
Claude Total thread wire — 4.9 KiB — 6.8 KiB ✅
Claude Thread snapshot wire — 3.7 KiB — 4.9 KiB ✅
Claude Live turn WebSocket wire — 1.2 KiB — 2.0 KiB ✅
Claude Live turn WebSocket decoded — 20.8 KiB — 29.3 KiB ✅
Claude Live turn messages — 2 — 8 ✅

Baseline: unavailable · PR result: 7fd0e1d · Source CI: success

Scenario and decoded snapshot size

10 historical turns, 5 command tools per turn, 878.9 KiB retained MCP result per historical turn, and a 1.05 MiB retained result in the measured turn.

  • Codex decoded thread snapshot: 106.1 KiB
  • Claude decoded thread snapshot: 106.4 KiB

Updated in place by a trusted workflow. PR artifacts are strictly validated and never executed.

if (status !== "pending" && status !== "running") return;
context.awaitingBackgroundHydration.delete(mutation.taskId);
const finished = { ...tool, status: mutation.status };
yield* emitTool(

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.

This changes how a settled, deferred turn finishes its registered monitor tool, but the tests do not assert that a task_completed mutation terminalizes that row with the snapshot output and lets the turn settle without a TaskOutput update. Could you add a focused adapter regression test for that path (including the failed exit-code case)?

Posted via Macroscope — Effect Service Conventions

@macroscopeapp

macroscopeapp Bot commented Sep 25, 2026 •

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Would Approve

Macroscope's review found this PR approvable — This is a focused Grok lifecycle bug fix that keeps monitor rows running through intermediate ticks and terminalizes them from the structured completion snapshot, including failure status and final output. The adapter-level deferred-finalization edge cases and missing integration regression coverage remain notable risks to verify.

Not approved because:

  • 2 blocking correctness issues found at or above your repo's Minimum Blocking Severity

Adjust the Minimum Blocking Severity for this repo — including turning it Off — in Settings. You can add or adjust custom eligibility rules. Learn more.

Base automatically changed from v2/grok-recorder to t3code/codex-turn-mapping September 25, 2026 05:26
…inished

Grok 1.0.41 streams every monitor tick as a tool_call_update with
status "in_progress" and a Bash-shaped rawOutput whose exit_code is
already 0. normalizeXAiAcpToolCallState read that exit_code as a
finished command, so the monitor row completed at its first tick and
the held root turn settled while the monitor was still running.

An explicit in_progress or pending status from Grok now wins; the
exit_code inference only applies when Grok sends no status. Grok never
sends a completed status for a monitor. Its end is the structured
`_x.ai/task_completed`, which now carries the snapshot's final output
and exit code and finishes the registered monitor row while a settled
turn is held open for it.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@juliusmarminge
juliusmarminge merged this pull request into t3code/codex-turn-mapping Sep 25, 2026
24 checks passed
@juliusmarminge
juliusmarminge deleted the v2/grok-monitor-status branch September 25, 2026 05:39
juliusmarminge added a commit that referenced this pull request Sep 25, 2026
…inished (#13557)

Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:M 30-99 changed lines (additions + deletions). vouch:trusted PR author is trusted by repo permissions or the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant