[orchestrator-v2] fix(orchestrator): Harden Grok v2 settlement, background task lifecycle, steer visibility, and images#3578
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 |
github-actions
Bot
added
size:L
![macroscopeapp[bot]](https://avatars.githubusercontent.com/in/900172?s=60&v=4){kind=small}
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
ApprovabilityVerdict: Needs human review 4 blocking correctness issues found. Diff is too large for automated approval analysis. A human reviewer should evaluate this PR. You can customize Macroscope's approvability policy. Learn more. |
4c4d0dd to
8f6ec37
Compare
github-actions
Bot
added
size:XL
| if (typeof exitCode === "number") { | ||
| if (exitCode === 0 && (xAiToolOutputText(withTitle)?.trim().length ?? 0) === 0) { | ||
| return { ...withTitle, status: "inProgress" }; | ||
| } | ||
| return { | ||
| ...withTitle, | ||
| status: exitCode === 0 ? "completed" : "failed", | ||
| }; | ||
| } |
There was a problem hiding this comment.
🟠 High acp/XAiAcpExtension.ts:359
normalizeXAiAcpToolCallState forces every successful Bash result with empty output to inProgress, so legitimately completed silent commands like true, test, or sleep (which produce { type: "Bash", exit_code: 0 } with no text) stay spinning forever instead of becoming completed. The comment says this is only for provisional permission ACKs, but the code has no way to distinguish those from real empty-output completions. When non-terminal tool items hold finalization, this also keeps the run open indefinitely. Consider restricting the inProgress override to a narrower signal (e.g. only when the original status was not already completed) or removing it entirely so empty-output exit-0 Bash results settle as completed.
if (typeof exitCode === "number") {
- if (exitCode === 0 && (xAiToolOutputText(withTitle)?.trim().length ?? 0) === 0) {
- return { ...withTitle, status: "inProgress" };
- }
return {
...withTitle,
status: exitCode === 0 ? "completed" : "failed",
};🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/server/src/provider/acp/XAiAcpExtension.ts around lines 359-367:
`normalizeXAiAcpToolCallState` forces every successful Bash result with empty output to `inProgress`, so legitimately completed silent commands like `true`, `test`, or `sleep` (which produce `{ type: "Bash", exit_code: 0 }` with no text) stay spinning forever instead of becoming `completed`. The comment says this is only for provisional permission ACKs, but the code has no way to distinguish those from real empty-output completions. When non-terminal tool items hold finalization, this also keeps the run open indefinitely. Consider restricting the `inProgress` override to a narrower signal (e.g. only when the original status was not already `completed`) or removing it entirely so empty-output exit-0 Bash results settle as `completed`.
There was a problem hiding this comment.
Leaving this as-is for now (not taking the suggested removal).
The empty exit-0 Bash → inProgress override is intentional and covers more than provisional permission ACKs:
- Provisional / permission-style ACKs — Grok often emits a completed Bash frame with empty output before real stdout streams. Completing the row too early is wrong for that shape (covered by the unit test keeps empty successful Bash updates running until native output or turn settlement).
- Post-settle monitor re-reports — finished monitors can reappear as Bash frames with empty/minimal
rawInputand a generic title, so monitor detection cannot match. Forcing empty exit-0 Bash tocompletedwould re-open the “row spins or wake mis-classifies” class of bugs this heuristic was added to contain (see the comment onnormalizeXAiAcpToolCallState).
Silent real commands (true, finished sleep with no stdout) can stay non-terminal until turn settlement; that is the known trade-off. A safer future fix is a narrower gate (e.g. only when the tool looks like a provisional “Ran command” ACK, or is already a tracked background/monitor task), not “always complete on empty exit 0.”
Happy to revisit if we have a concrete stuck-row repro that outweighs those regressions.
There was a problem hiding this comment.
Sorry, I'm unable to act on this request because you do not have permissions within this repository.
There was a problem hiding this comment.
Effect service conventions review found one convention deviation in the ACP session-runtime service failure introduced by this PR. Details inline.
Posted via Macroscope — Effect Service Conventions
ddfedee to
e84f666
Compare
ba852af to
0583e10
Compare
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 2 potential issues.
There are 6 total unresolved issues (including 4 from previous reviews).
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 838d38d. Configure here.
| hasUnpairedRunInterruptRequest: () => | ||
| projectionStore.getThreadProjection(projection.thread.id).pipe( | ||
| Effect.map((current) => { | ||
| const requestId = idAllocator.derive.runSignalTurnItem({ | ||
| runId: run.id, | ||
| signal: "interrupt-request", | ||
| }); | ||
| const resultId = idAllocator.derive.runSignalTurnItem({ | ||
| runId: run.id, | ||
| signal: "interrupt-result", | ||
| }); | ||
| const hasRequest = current.turnItems.some((item) => item.id === requestId); | ||
| const hasResult = current.turnItems.some((item) => item.id === resultId); | ||
| return hasRequest && !hasResult; | ||
| }), | ||
| Effect.catchCause(() => Effect.succeed(false)), | ||
| ), |
There was a problem hiding this comment.
🟡 Medium orchestration-v2/ProviderTurnStartService.ts:442
hasUnpairedRunInterruptRequest catches every getThreadProjection failure and returns false, so a transient projection read error makes an outstanding interrupt-request appear paired when it is not. RunExecutionService then skips writing the run_interrupt_result, and because the error was swallowed there is no retry — the Stop request stays permanently unpaired in the persisted state. Consider propagating the read failure instead of catching it, so the enclosing operation can retry rather than silently treating a failed lookup as a negative result.
| hasUnpairedRunInterruptRequest: () => | |
| projectionStore.getThreadProjection(projection.thread.id).pipe( | |
| Effect.map((current) => { | |
| const requestId = idAllocator.derive.runSignalTurnItem({ | |
| runId: run.id, | |
| signal: "interrupt-request", | |
| }); | |
| const resultId = idAllocator.derive.runSignalTurnItem({ | |
| runId: run.id, | |
| signal: "interrupt-result", | |
| }); | |
| const hasRequest = current.turnItems.some((item) => item.id === requestId); | |
| const hasResult = current.turnItems.some((item) => item.id === resultId); | |
| return hasRequest && !hasResult; | |
| }), | |
| Effect.catchCause(() => Effect.succeed(false)), | |
| ), | |
| hasUnpairedRunInterruptRequest: () => | |
| projectionStore.getThreadProjection(projection.thread.id).pipe( | |
| Effect.map((current) => { | |
| const requestId = idAllocator.derive.runSignalTurnItem({ | |
| runId: run.id, | |
| signal: "interrupt-request", | |
| }); | |
| const resultId = idAllocator.derive.runSignalTurnItem({ | |
| runId: run.id, | |
| signal: "interrupt-result", | |
| }); | |
| const hasRequest = current.turnItems.some((item) => item.id === requestId); | |
| const hasResult = current.turnItems.some((item) => item.id === resultId); | |
| return hasRequest && !hasResult; | |
| }), | |
| ), |
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/server/src/orchestration-v2/ProviderTurnStartService.ts around lines 442-458:
`hasUnpairedRunInterruptRequest` catches every `getThreadProjection` failure and returns `false`, so a transient projection read error makes an outstanding interrupt-request appear paired when it is not. `RunExecutionService` then skips writing the `run_interrupt_result`, and because the error was swallowed there is no retry — the Stop request stays permanently unpaired in the persisted state. Consider propagating the read failure instead of catching it, so the enclosing operation can retry rather than silently treating a failed lookup as a negative result.
There was a problem hiding this comment.
🟡 Medium acp/XAiAcpExtension.ts:280
isXAiPersistentMonitor misclassifies a persistent monitor as non-persistent when the structured rawOutput has type: "Monitor" but omits the persistent field. In that case the function returns false immediately at line 281 without checking the rawInput.persistent fallback, so a monitor that is actually persistent (per rawInput) is treated as non-persistent and can hold root-turn finalization indefinitely. The structured branch should only short-circuit when rawOutput.persistent is actually a boolean; otherwise it should fall through to the rawInput check.
if (rawOutput !== undefined && nonEmptyString(rawOutput.type)?.toLowerCase() === "monitor") {
- return rawOutput.persistent === true;
+ if (typeof rawOutput.persistent === "boolean") {
+ return rawOutput.persistent;
+ }
}🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/server/src/provider/acp/XAiAcpExtension.ts around lines 280-282445546767:
`isXAiPersistentMonitor` misclassifies a persistent monitor as non-persistent when the structured `rawOutput` has `type: "Monitor"` but omits the `persistent` field. In that case the function returns `false` immediately at line 281 without checking the `rawInput.persistent` fallback, so a monitor that is actually persistent (per `rawInput`) is treated as non-persistent and can hold root-turn finalization indefinitely. The structured branch should only short-circuit when `rawOutput.persistent` is actually a boolean; otherwise it should fall through to the `rawInput` check.
Settle root turns from provider signals and preserve post-settle continuations. Track asynchronous subagent and monitor work, allow image prompts, and keep steered messages visible. Separate soft steering from hard Stop while containing and reaping native process trees.
2 participants
Fixes #3580
Summary
Harden Grok orchestration v2 so root turns complete correctly, steered user
messages stay visible, Stop then continue recovers from a poisoned ACP child,
async
spawn_subagent/monitorproject real lifecycle (not start ACKs), latepost-settle Grok traffic can land as a continuation run instead of being
dropped, screenshot / image attachments actually reach Grok instead of failing
turn start, and interrupt cleanup actually contains and reaps native Grok work
(cgroup kill / owned process ledger) instead of leaving zombie shells.
Follow-up commits soften non-Stop interrupts: steer / restart_active now cancels
and re-prompts the same live session instead of killing the CLI, a steer on a
settled held turn preserves the runtime (running subagents survive), and user
Stop keeps the unchanged hard kill + respawn.
Primary commits on
fix/grok-v2abovet3code/codex-turn-mapping(thefeature story reviewers should start with):
Follow-up commits may land on top of those two. Treat them as reviewable
delta fixes, not a rewrite of the primary story. Integration snapshots should
still merge the whole branch, not tip-only cherry-pick.
The shared provider-continuation plumbing (C0) already landed in the base branch
alongside the Claude and Codex consumers.
What you'll notice
Steered messages stay visible
When you steer an in-flight Grok run, your message no longer flashes away
between server events. The timeline keeps the optimistic user row until the
committed
user_messageturn item lands.Steering also works while a run is held open for background work: pre-fix,
steering a settled turn that deferred finalize was holding (a subagent still
running) froze the thread at Working, the steer text never reached Grok, and
later messages queued forever.
Grok multi-tool turns finish with tools and final text
Grok often emits a short assistant preamble, then tools, then more text.
Speculative idle settlement used to complete the turn after a short quiet and
call
session/cancel, which froze T3 projection while the CLI kept working.This PR disables Grok idle settle and terminalizes from real provider signals.
Stop then continue recovers instead of
task_already_runningAfter you Stop a Grok run mid-turn, the next message restarts the Grok ACP child
before the recovery prompt, clearing zombie shell state.
Interrupt cleanup contains native work
Stop is not only an ACP cancel. On Linux with cgroup v2, delegated Grok launches
join a unique child cgroup and interrupt uses
cgroup.killas the authoritativecleanup. When cgroup containment is unavailable, the owned-process ledger is the
fallback (Darwin process groups / Windows
taskkillretained). ACP cancel isgeneration-aware; unexpected transport death, double-fork descendants, and
teardown races are covered by focused tests.
Steers no longer kill the Grok process
The interrupt matrix after the follow-up commits:
session/loadon the next turnsession/cancel+ same-session re-prompt; native context survives without asession/loadreplayGrok's
session/cancelis detach-and-continue: the CLI reaps the foregroundshell but re-runs the interrupted command as a background task, and only the
model can kill it (via
kill_command_or_subagent). Product call: the steeredmodel, not the client, decides whether that detached work dies, matching the
CLI's own behavior. The adapter tracks
_x.ai/task_backgrounded/task_completedinto background-task mutations and tombstones model-killedtasks so wake machinery stays truthful, and a direct Stop quarantines any late
lifecycle noise from the stopped run.
spawn_subagent and monitor project real work
Async
spawn_subagentno longer completes on the background ACK alone. T3 bindsthe child id, keeps the subagent running, shows durable
subagent.resultwhenavailable, and hydrates from TaskOutput /
get_command_or_subagent_outputwhenneeded.
monitorstays running through the start ACK, streams as commandexecution, and completes from Bash exit codes or monitor-end text.
Late “will report when finished” can wake again
After the root turn settles (including when a subagent card already has a
result), Grok may still stream a late root follow-up (tool fetch + confirmation)
without a new
session/promptfrom T3. That traffic is buffered and attached toa provider continuation run so the UI can go idle, then show Working again
with the follow-up, instead of dropping frames after finalize.
Long-running monitors initially made this path spam: Grok narrates every
monitor event (~3s apart, wider than the ~2s finalize quiet window), and each
burst opened its own continuation run with its own synthetic "Background task
completed." user row (4+ observed live). Continuation offers are now fully
suppressed while a tracked background task is still running: per-event agent
commentary AND tool re-reports buffer (Grok re-reports a running monitor as
Bash frames that already carry
exit_code: 0mid-stream, so a terminal-lookingtool status is not evidence the task ended), and exactly one continuation run
opens once the monitor actually ends, draining the buffered commentary.
In-turn monitors do not open a redundant wake
When Grok waits on a monitor and reports the listing in the same root turn
(no post-settle wait), late CLI re-reports and residual agent chatter must not
open synthetic "Background task completed." continuation runs. The follow-up
commit marks those tools handled mid-turn and suppresses residual agent-message
offers once handled work is done and nothing is still running.
Settled turns still get the injected monitor report
When Grok settles first ("I'll report when the monitor ends") and the CLI later
injects a monitor-event turn, the report could vanish: injected turns emit no
turn_completed, and the deferred-finalize debounce could fire while Grok wasstill generating the report, dropping the chunk into an already finalized turn
(thread
a8e8b0a9run 5). The run now holds until the report streams (or a 25ssafety elapses), so the listing lands as a normal assistant message.
Screenshots and image attachments work on Grok
Attaching a screenshot to a Grok turn no longer fails immediately with a generic
provider turn start error. Grok’s ACP
initializestill reportspromptCapabilities.image: false, but the agent accepts and vision-processesimage content blocks. The Grok flavor now opts in via
supportsImagePromptssoattachments are sent as ACP image blocks instead of being rejected before
session/prompt.Problem and Fix
user_messageturn item landssession/cancelfor Grok; terminalize only on real provider signalssession/promptstayed open without a reliable complete pathturn_completed(and legacy prompt-complete) for the same prompt idtask_already_runningspawn_subagentshowed completed in ~1ms on the background ACK; child work never boundmonitorstayed spinning forever or never streamed outputpersistent: truemonitors from deferred-finalize holds; they continue post-settleimage: falserestartRuntimeOnEveryInterrupt), discarding native session context and any running subagents even when the user was only redirectingsession/cancel+ same-session re-prompt; a steer on a settled held turn skips the cancel entirely and preserves the runtimesession/canceldetaches and re-runs the interrupted command as a background task; nothing tracked that work, so wake machinery could misread pending state_x.ai/task_backgrounded/task_completedinto background-task mutations; tombstone tasks the model kills viakill_command_or_subagent(notask_completedis emitted for kills)activeSessionIdstill points at the stopped session until the next turn respawns)stoppedRunQuarantineis set, and ignore child-session mutations that must not gate root wake statesession/cancel, which Grok answers by killing detached background workpromptWireSettleddeferred succeeds the moment the prompt RPC resolves; settled classification also accepts wire settlement and skips the canceltask.result) and fell back to showing the spawn promptassistantTextinto the terminal result with the same rule as live emissionrun_interrupt_resultwhose parentrun_interrupt_requestonly hard Stop emits, leaving a dangling parent link and letting timelines fold a continuing steered run as interruptedexit_code: 0re-report, reading as a clean success exitDefensive Fixes
provider-turn.restartcould swallow a failed replacement startcontinuationRequestedafter a failed or skipped dispatch blocked further offers and pinned idlestartTurnactiveTurnwas null and in-turn-handled task ids remained<monitor-event>chunks dropped later progress / end notices<monitor>block, and end notice in the chunktask_idsTaskOutput hydrated only the first idExit Code: -1read as successrunningcleared the hydration hold earlyEffect.sleep, whichit.effectTestClock freezes, hanging the CI Test job with no outputt3-acp-<pid>-*cgroup lease directories accumulated when a server process died before cleanuppopulated 0) before creating a new leaseproviderTurnslookup whose null fallback was actually the child's thread idhasRunInterruptRequestcallback threaded likeshouldFinalizeRun)run_interrupt_resultonly when unpaired (norun_interrupt_requestfor the same run); legacy plain-steer results in existing DBs stay hiddenhasUnpairedRunInterruptRequest: emits only when the request item exists and the derived result item is absent_x.ai/session/prompt_completesettled the first pending prompt for the session, which is the wrong one whenever more than one is pending<subagent_meta>/<subagent_result>blocks, stripping left nothing and the code fell back to the raw string, leaking machine tags into subagent cards and monitor outputresultFromGetOutputToolreturns null when the cleaned output is emptyuser_messageturn items, which hydrate later than the projection, so an existing thread briefly unlocked env/branch overrides on load and right after a run settledactiveMessageCounttakes the max of committed turn-item ids andserverProjection.messages.length, which hydrates atomically with the projectionV2EventTimelineRowearly-returns lifecycle items (including subagents) toV2LifecycleRow, leaving an unreachable subagent presentation branch that review bots kept flaggingv2EventPresentationsubagent branch and its unreachable styling conditions; the live V2LifecycleRow path is untouchedRoot-matched terminal signal (live evidence)
Current Grok CLI emits completion as:
{ "method": "_x.ai/session/update", "params": { "sessionId": "<root session>", "update": { "sessionUpdate": "turn_completed", "prompt_id": "t3-xai-prompt-N", "stop_reason": "end_turn" } } }Matching rules:
sessionIdequals the active ACP sessionprompt_idequals the pending T3-injected id (t3-xai-prompt-N)task-completed-*prompt ids (background tools)_x.ai/session/prompt_completefor older buildsDo not treat idle silence or subagent completions as root terminalization.
Server mechanics
settleRootTurnWhenIdle: false); speculative idle complete stays disabledsession/promptRPC result or root-matchedturn_completed/ legacyprompt_completestartTurnspawns a fresh Grok process. Non-Stop interrupts no longer restart the runtime (restartRuntimeOnEveryInterruptdropped from the Grok flavor)running+ binds child; TaskOutput / get_command hydrates; durablesubagent.resultwhen availableactiveTurnis null; offer only on completion-like evidence (terminal tool status after normalization, or agent text with no tracked background task still running); monitor-ended reminders andTaskOutputcompletion frames are the genuine end signals that clear tracked task ids (ended ids are tombstoned against straggler re-adds); attach mode skipssession/promptand drains the buffersupportsImagePrompts: trueoverrides ACPpromptCapabilities.image:falseso screenshots are sent as image content blocks_x.ai/task_backgrounded/task_completedbecome background-task mutations;kill_command_or_subagenttombstones the killed task; mutations are dropped while the Stop quarantine is active or when they belong to a child sessionDesign decisions (interrupt complexity; safe to simplify later)
Two choices below are deliberate and reviewable. If maintainers want less
surface area, each has a known downgrade path and a concrete fidelity trade-off.
1. Linux cgroup join wrapper (production spawn path)
Current landing: before
execof the real Grok/ACP binary on Linux, spawnis rewritten through
process.execPath+ a small inline-escript(
wrapCommandForLinuxCgroup). The shim writes its PID into the leasedcgroup.procs, verifies/proc/self/cgroup, strips wrapper env, thenprocess.execves into the real command so membership sticks to the same PID.This is only used when cgroup v2 containment is available; otherwise spawn is
unwrapped and interrupt falls back to the owned-process ledger (reduced
guarantee). Mac/Windows never use this path (process group /
taskkill).Why not PATH
node: the wrapper uses the running Electron/Node binary(
process.execPath+ELECTRON_RUN_AS_NODEunder desktop), so packagedAppImages do not require a separate system Node install.
Preferred cleanup if inline
-eis too opaque: replace the string with asmall checked-in
acp-cgroup-enter.mjsnext to the server (sameexecPathinvocation, no temp files, no C toolchain for production). Same semantics;
easier to review and diff.
Heavier alternative: checked-in C helper binary shipped in the desktop
artifact. Clearer argv contract and no
execvedependency on the ElectronNode build, but adds a real packaging/build matrix. Not chosen for this PR.
Complexity you can drop: if cgroup pre-exec join is too much, ship only the
ledger / process-group fallback on Linux too. Trade-off: double-fork /
reparented descendants can escape kill (the portable ledger comment already
calls this out). Headless interrupt packs would get flakier on adversarial
shell shapes.
2. Pthread spawn test helper (
.c, test-only)Current landing: keep
apps/server/scripts/acp-thread-spawn-helper.c. OneLinux live test compiles it with
cc -pthreadto fork a child from anon-leader pthread, then asserts
childrenOf(parent)still finds thatchild. That exercises the production scan of every
/proc/<pid>/task/<tid>/childrenentry (main-thread-only scans miss this).Local: if
ccis missing, the test soft-skips (suite stays green).CI: Blacksmith Ubuntu runners mirror GitHub’s Ubuntu image and often
already have
gcc, but that is not a repo contract. The Test job thereforeinstalls
build-essentialsoccis present and the fixture runs insteadof silently no-oping:
Simpler alternative (drop complexity): replace the helper with a plain
Node/
.mjsspawn("sleep", …)and rename the test to “finds a spawned child.”No compiler, always runs. Fidelity loss: a regression that only walks the
main task’s children file (not every thread) can still pass a normal spawn
fixture and only fail the pthread case. Other double-fork / cgroup-kill tests
remain; only this multi-thread discovery edge case goes soft.
Not a good substitute without rethinking the assertion: pure JS cannot
faithfully create “non-leader pthread then fork” without native code.
Validation
vp check: format clean on changed files; monorepo lint warnings only (knownVite+ stdout panic can flake the full wrapper after lint)
vp run typecheck: cleanvp test apps/web/src/components/ChatView.logic.test.ts: passvp test apps/server/src/orchestration-v2/Adapters/GrokAdapterV2.test.ts:pass (includes image capability override)
vp test apps/server/src/orchestration-v2/Adapters/AcpAdapterV2.test.ts: pass(wake-evidence filters, continuation offer gating, the monitor start ACK
normalization case, steer-during-hold interrupt finalize, subagent lineage
carry-over across a steering restart, injected-report hold, interrupt
cleanup fixtures, soft mid-prompt cancel + same-session re-prompt, direct
Stop dropping late background task mutations, and the production Grok flag
set still hard-killing and respawning on user Stop)
vp test apps/server/src/provider/acp/AcpSessionRuntime.processTree.test.ts:pass (cgroup / process-tree interrupt evidence; pthread fixture needs
cc,provided in CI via
build-essentialon the Test job; soft-skips locally ifccis absent)vp test apps/server/src/orchestration-v2/ProviderSessionManager.test.ts:pass (idle pin with C0)
vp test apps/server/src/provider/acp/XAiAcpExtension.test.ts: pass (asyncspawn, TaskOutput, Monitor variant cases)
live-scenarios, private serve against thisbranch / restacked AppImage):
grok-interrupt-fast: direct Stop +restart_active(the restart_activepack rewritten for soft semantics: process retained, one initialize, no
session/load, cancel + re-prompt, codeword carry-forward provingsame-session context; detach handling recorded as evidence, with a
branch-conditional
-detachvariant)grok-interrupt-soak: held subagent, mid-prompt steer during a subagenthold (new pack sampling the cancel-kills-subagent edge), duplicate Stop,
leader exit, queued follow-up (scheduler gate); re-run green on the C4 tip
Grok thread titled from scenario 1 "Live-test direct Stop…", 2026-07-17 on
orchestrator-v2 AppImage @ current snapshot; projection checked in userdata
DB for thread
6c9994b7-bb04-4fb7-a9a8-e95688a6b7d7):INTERRUPTED_OK: passRESTART_ACTIVE_OK: passQUEUE_CONTRAST_OK: pass(one late-queue practice attempt finished the sleep without a queue; retry
with queue while Working passed)
DUPLICATE_STOP_OK: passwhile queued; headless
grok-interrupt-queued-followupremains theautomated contract)
hides Stop while the card is open)
HOLD_FOLLOW_OK: pass (subagentinterrupted, no
HOLD_SUB_DONE/ forbidden shell marker)scenario numbering; 4–5 were blocked on desktop Stop UI as above.
16092557-…, 2026-07-17, scenarios 1 and 7 only):INTERRUPTED_OK: pass (sleep interrupted; noSHOULD_NOT_FINISH_CMDoutput)HOLD_FOLLOW_OK: pass (subagentinterrupted; no
HOLD_SUB_DONE/ forbidden shell marker)trees/orchestrator-v2):steer-during-hold, screenshot attach as previously reported on this PR
production Grok flag set (now also covering carryover subagent
terminalization, streamed-text preservation, and the parent thread id pin),
settled-soft wire-race test, cgroup lease sweep unit test; full server suite
(1369 tests) and typecheck green locally, and the CI Test job green on the
branch tip after each round
items while hard Stop keeps the linked pair; AcpAdapterV2 unit + it.live
tests pin no
exitCodeon interrupted commands (mock agent now re-reportsGrok's mid-stream
exit_code: 0) and real exit codes on completed ones;message_steering grok/cursor fixtures and the selection-restart integration
test assert zero
run_interrupt_*on supersede; full server suite (1373tests) and typecheck green locally
(seeded request + interrupted terminal emits the paired result with the
request as parent); existing supersede/hard-stop pins unchanged; full
server suite (1374 tests) and typecheck green locally
unpaired-superseded hidden / terminal visible; new RunExecutionService
test pins the already-paired supersede emitting nothing; full server
suite (1375 tests),
packages/sharedandpackages/client-runtimesuites, and typecheck green locally across all three packages
prompt_complete no-op (two pending prompts, neither settles) and the
meta-only get_output yielding null result / empty appendOutput; full
server suite (1377 tests), server + web typecheck, and
vp checkgreen locally
thread
b6eb3501-…): steer-while-held, direct Stop, steer mid-command,subagent-hold Stop, and steer-then-Stop orphan containment all pass;
projection DB audit confirms exactly one linked request/result pair per
hard Stop, zero interrupt items on the three steers, zero interrupted
commands with an
exitCode, and the R3 streamed-text + parent-pin behaviorintact
Out of Scope
Client presentation and desktop chrome that are not part of this Grok
settlement / interrupt PR. Called out so maintainers can track separate UI work
if desired; projection and run status still settle correctly.
+N previous tool calls(interrupted tools, Ask, etc.). Mobile keeps neutral tool rows in the work group; desktop filters neutrals out. Presentation difference only.run_interrupt_request/run_interrupt_resultfor Stops from desktop (verified in userdata). On mobile, those Interrupted rows are not shown for desktop-initiated Stops—not merely collapsed under+N previous tool calls(expanding the fold still does not surface them the way a live mobile Stop does). Live mobile Stops can show anX Interruptedwork-log row and aYou stopped after …fold. Incomplete multi-client / mobile lifecycle parity; not a missing server interrupt.--- Run interrupted ---dividerrun_interrupt_resultas a full-width system divider and leaves interrupted turns unfolded. Mobile never uses that divider chrome.workEntryPreviewprefersdetailover file_change filenamespromptId/ low-risk Grok session edge casesinProgressuntil native outputKnown limitations
After a real terminal signal, the hung
session/promptfiber may still logInterruptwhen cancelled locally; runs complete with full content (lognoise).
Builds that emit neither prompt RPC completion nor
turn_completed/prompt_completecan still under-settle (no silent idle cancel fallback).After Stop, a tool in the old shell may finish briefly before process kill
(
interruptPromptOnCancel: false); recovery still succeeds on the freshchild.
Subagent child-session tool calls (tools inside the child) are not fully
projected as nested tool items; assistant text + final hydration / result card
are covered.
Subagent wall time is provider-variable. A trivial “sleep ~5s then print a
token” general-purpose spawn is often ~10–20s end-to-end, but the child agent
can occasionally take much longer (minute-scale outliers observed). A long
Working state usually reflects child runtime, not the ~2s deferred-finalize
quiet window.
The root model may poll
get_command_or_subagent_outputwith longtimeouts, so the root run’s Working state can cover the entire child wait even
when the subagent card is the durable success signal.
Non-persistent background work still holds Working. Deferred finalize
keeps the root run open while a spawned subagent or a non-persistent monitor
tool row is still running. Persistent monitors (
persistent: true) nolonger hold that gate; they settle the root and continue via post-settle
continuation. Claude-style “go idle then reopen when background finishes” for
non-persistent work is still future work. Content that arrives during a hold
(CLI-injected monitor report) is delivered by the injected-report hold, so
this is about run status staying Working, not about lost output.
Continuation dispatch is by
threadId(the request also carriesdriverand
providerThreadIddetail for future scoping).Buffered per-event commentary drains into the continuation run as several
small assistant messages (one per burst). That is faithful to what the
provider emitted; coalescing the display would be UI-side follow-up work.
Soft-interrupt detach tracking is best-effort. Grok's
session/canceldetaches and re-runs the interrupted foreground command to full duration as
a background task; no client-side ACP method kills it (only the model can,
via
kill_command_or_subagent)._x.ai/task_backgroundedcan arrive late(~16s observed) or not at all (the model's poll-and-report shape emits
none), so lifecycle tracking begins only when a notification arrives.
Residual frames in that gap are accepted best-effort noise per the product
call above; live packs record detach handling as evidence rather than
pinning it.
Fast steers can trigger a model-side no-op flood. Steering within a
couple of seconds of a reply while background work is still held (send a
short message right after the settle reply with a subagent still running)
can push Grok into emitting no-op
run_terminal_command(true) callsabout once per second until interrupted; the same loop shows up without
steering after monitor-event injections (154 no-ops observed in one live
thread, 207 in another). The steer text and background completions still
land once the loop breaks or the run is stopped. This is xAI model
behavior, not orchestrator state; recovery is Stop.
Desktop UI: no Stop while a message is queued or an approval/question card
is open. See Out of Scope (mobile can still run those paths; headless
covers queue-then-Stop).
Mobile vs desktop interrupt presentation. Trailing work-log rows after a
mobile Stop, missing
--- Run interrupted ---dividers, and no Interruptedlifecycle chrome on mobile for desktop-initiated Stops (not just collapsed)
are client parity gaps only; server still projects interrupt items. See
Out of Scope.
Grok exact-reply EOS token leakage (model, not T3). On short
“reply with exactly TOKEN, no tools” recovery prompts, Grok sometimes
returns
TOKENplus a literal end-of-sequence marker. Observed 2026-07-16on desktop AppImage after subagent-hold Stop:
grok-4.5.general-purposesubagent that runssleep 30 && echo HOLD_SHOULD_NOT_MATTERand should returnHOLD_SUB_DONE; root waits for the subagent.Reply with exactly HOLD_FOLLOW_OK. No tools.HOLD_FOLLOW_OK<|eos|>(ASCII<|eos|>appended). Projection stored that string verbatim; subagent was
correctly
interruptedwith no forbidden shell marker.Other exact-reply markers in the same session (
INTERRUPTED_OK,RESTART_ACTIVE_OK,DUPLICATE_STOP_OK) were clean. Treat as xAI/Grokmodel leakage suitable for upstream Grok team follow-up if desired; not a
T3 interrupt/projection bug.
Stuck Working after hard interrupt on a poisoned session is reduced, not
fully gone. This PR hardens the common Stop races (idempotent interrupt,
finalize-before-kill, missing-session restart, sticky continuation flag,
process-group reap). Residual risk remains if a held turn keeps a
background/lineage entry that can no longer receive a terminal signal after
restart (no overall force-finalize cap on that path). Fresh threads and the
headless steer pack are the reliable recovery; candidate follow-ups are a
bounded hold cap and reconciling carried subagent / background-tool state on
interrupt.
Note
Harden Grok v2 prompt settlement, process containment, background task lifecycle, steer visibility, and image prompt support
makeXAiPromptCompletionRuntimein XAiAcpExtension.ts to race both_x.ai/session/prompt_completeand_x.ai/session/update(turn_completed) notifications for prompt settlement, interrupting in-flight prompt RPC fibers on external completion.AcpSessionRuntime, with platform-specific termination (cgroup kill on Linux, process-group on others); Grok runtime enables detached process-group on all platforms and descendant tracking on Linux.AcpAdapterV2with soft/hard interrupt semantics (soft preserves runtime on settled turns; hard tears down with process-group termination on Stop), post-settle wake buffering for continuation runs, native transport request correlation with admission/acknowledgement sequencing, and deferred finalization for background tool/subagent activity.isOrchestrationV2SupersededInterruptin orchestrationV2Timeline.ts to only hiderun_interrupt_resultitems when unpaired (no matchingrun_interrupt_request), correcting steer visibility in superseded attempts.acpSupportsImagePromptsto allow Grok flavor to enable image prompt blocks regardless of capability negotiation.acpRootTurnIsIdlealways returns false, unconditionally disabling speculative idle settlement; callers that relied on local idle settle will not trigger completion from that path.Macroscope summarized 542c87b.
Note
Low Risk
Changes are limited to CI workflow and test/mock scripts; no production orchestration or runtime paths are modified in this diff.
Overview
Adds CI test infrastructure so ACP process-tree live tests always compile their pthread fixture, and expands the mock ACP agent plus a small C spawn helper to exercise Grok-style interrupt, monitor, and post-settle behavior in automated tests.
The Test job now installs
build-essentialand verifiesccis onPATH, soAcpSessionRuntime.processTreetests that compileacp-thread-spawn-helper.crun in CI instead of soft-skipping when no compiler is present.acp-mock-agent.tsgains many env-flag scenarios: cancel-as-detach (task_backgrounded/task_completed), residual session/permission/elicitation callbacks after teardown, long-running terminal tools with optional realbash/sleepchildren (PID file, TERM-ignoring, detached session), duplicate empty successful Bash updates, post-settle and in-turn monitor + TaskOutput flows (including late duplicate updates), form/URL elicitation, and hang/exit variants around permission and cancel.acp-thread-spawn-helper.cis a test-only program that forkssleepfrom a worker pthread and writestid child_pidto a path, so process-tree discovery can be asserted for children not visible from the main thread alone.Reviewed by Cursor Bugbot for commit 542c87b. Bugbot is set up for automated code reviews on this repo. Configure here.