fix(orchestrator): Preserve claude/codex post-interrupt recovery state#4229
fix(orchestrator): Preserve claude/codex post-interrupt recovery state#4229mwolson 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 |
12dfe4e to
2b5cda2
Compare
2b5cda2 to
2557fda
Compare
2557fda to
5e0f0d5
Compare
207e6fa to
ae99f40
Compare
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit ae99f40. Configure here.
| threadId, | ||
| turnId: activeTurn.nativeTurnId, | ||
| }); | ||
| yield* Deferred.await(turnCompleted); |
There was a problem hiding this comment.
Interrupt awaits completion indefinitely
Medium Severity
interruptTurn now blocks on Deferred.await(turnCompleted) with no timeout after turn/interrupt. If Codex never emits turn/completed, the provider-turn.interrupt outbox effect never finishes, leaving interrupt bookkeeping and the worker fiber stuck even though the user already saw an interrupted projection.
Reviewed by Cursor Bugbot for commit ae99f40. Configure here.
cc5776d to
22db7bb
Compare
| const completedIndex = transcript.entries.findIndex( | ||
| (entry) => entry.type === "emit_inbound" && protocolMethod(entry.frame) === "turn/completed", | ||
| ); | ||
| const terminateIndex = transcript.entries.findIndex( | ||
| (entry) => | ||
| entry.type === "expect_outbound" && | ||
| protocolMethod(entry.frame) === "thread/backgroundTerminals/terminate", | ||
| ); | ||
| assert.isAtLeast(commandIndex, 0, "Codex interrupt fixture must record command execution start"); |
There was a problem hiding this comment.
🟡 Medium turn_interrupt_mid_tool/codex_output.ts:58
assert.isAbove(terminateIndex, completedIndex) passes even when completedIndex is -1, so this fixture can accept a thread/backgroundTerminals/terminate request that was sent without a corresponding turn/completed frame. That defeats the regression check — termination should be verified to occur only after completion is confirmed. Add an explicit nonnegative assertion for completedIndex before the comparison.
const completedIndex = transcript.entries.findIndex(
(entry) => entry.type === "emit_inbound" && protocolMethod(entry.frame) === "turn/completed",
);
+ assert.isAtLeast(
+ completedIndex,
+ 0,
+ "Codex interrupt fixture must record turn completion before termination",
+ );
const terminateIndex = transcript.entries.findIndex(
(entry) =>
entry.type === "expect_outbound" &&
protocolMethod(entry.frame) === "thread/backgroundTerminals/terminate",
);🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/server/src/orchestration-v2/testkit/fixtures/turn_interrupt_mid_tool/codex_output.ts around lines 58-66:
`assert.isAbove(terminateIndex, completedIndex)` passes even when `completedIndex` is `-1`, so this fixture can accept a `thread/backgroundTerminals/terminate` request that was sent without a corresponding `turn/completed` frame. That defeats the regression check — termination should be verified to occur only after completion is confirmed. Add an explicit nonnegative assertion for `completedIndex` before the comparison.
22db7bb to
8f2fb40
Compare


Summary
notification arrives during session resume.
the command completion event.
terminals against each owning native thread during root Stop.
provider turns and threads have reached durable terminal state.
native turn without hiding a sole empty response or distinct later answer.
child timeline items when a stopped root turn leaves them open.
Stack
This draft targets
t3code/codex-turn-mappingand is stacked on #4193, the samepost-merge fixture-fix base used by #4218. This branch contains one unique
commit above that stack base.
Problem and Fix
task_notificationresult between the first post-Stop prompt and its real response. The adapter treated that result as the active root result, completing the recovery run with no assistant output.item/completedfor an in-flight command, acceptturn/interruptwithout later sendingturn/completed, and keep descendant native turns running after the root stops. Its interrupt response also does not contain provider-owned background terminals. The projected run could look interrupted while a child shell survived Stop and later emitted a final notification. A root terminal arriving before its child provider terminal events also stopped run ingestion and stranded the durable child provider turn and thread as running.turn.terminal. Defer non-completed root terminal events until active descendant turns have terminalized, including timeout and interrupt-request failure paths. Verify ambiguous false results against the paginated terminal list.Validation
vp check: pass with pre-existing warnings onlyvp run typecheck: passowning-thread terminal containment, bounded interrupt fallback, late native
completion rejection, redundant empty and identical-final suppression,
overlapping streams, sole-empty preservation, unknown phases, zero-length
deltas, subagent result safety, and child provider terminal ordering after
native completion, timeout, interrupt-request failure, and a descendant that
starts after the initial Stop snapshot
turn_interrupt_mid_toolreplay and fixture-loading checks:pass
queued scheduler, projection, process ownership, and driver timeout gates
claude-interrupt-direct-stop: pass with one interrupted run and two distinctcompleted recovery runs
rendered as distinct assistant responses with no stuck Working state
during recovery 1; both recovery runs completed with their expected markers
in Claude and Codex and the stale Claude child root node
then reproduced two identical streamed final answers in one native turn
surviving root Stop and emitting a late final notification; the focused
descendant-turn replay now covers the exact native thread and process boundary
child native turn, terminated its tracked foreground process, kept the child
command and parent subagent interrupted, and completed one same-thread
recovery response. No forbidden child output appeared through the original
command deadline or the additional late-event polling window.
passed, but the root terminal reached the run stream before the child
provider terminal events. The durable child provider turn remained running
and its provider thread remained active, which the new ordering regressions
reproduce.
failed/cancelled mapping, supersede protection, linked-child cleanup,
linked-child timeline cleanup, unreferenced-child exclusion, suppressed
streaming exclusion, partial result preservation, and same-attempt and
cross-attempt late-event rejection
codex-interrupt-direct-stopon Codex 0.144.6: pass with run statuses[interrupted, completed, completed], no running command projection, and theforeground workload PID absent after Stop
Defensive Fixes
If Codex returns
terminated: false, query the paginated background-terminallist before deciding whether containment succeeded. Treat an absent process as
already contained, fail if it remains listed, attempt every tracked process ID,
and always clear interrupt bookkeeping.
If Codex never emits
turn/completedafter accepting an interrupt, locallyterminalize after the same 10-second ceiling used by sibling adapters. A shared
finalization permit prevents the timeout and a racing native completion from
publishing duplicate terminal events; late native events cannot reopen the
settled turn.
When root Stop has active native descendant turns, install completion waiters
for the full active lineage before sending interrupts. Treat a target that
settled during waiter setup as already complete. Run the first terminal sweep
concurrently with the single completion deadline, then sweep again after
settlement so commands observed during the interrupt race are also contained. If
a marked native target reports
completedduring that race, preserve theuser-requested
interruptedprojection.For non-completed Codex roots, retain the root terminal event while any native
descendant turn remains active. Descendant finalization publishes its provider
turn and provider-thread terminal updates first, then releases the root event.
Interrupt RPC failures locally terminalize remaining targets before release,
and the existing finalization permit serializes native completion, timeout, and
failure cleanup.
Before Stop returns, rescan the interrupted native lineage and interrupt plus
terminalize descendants that started after the initial snapshot. Native child
starts that arrive after an ancestor has terminalized non-completed are
interrupted without creating a new running provider projection.