fix(server): withdraw a stopped turn's pending approvals#22023

Approvability

Verdict: Approved at 71119ed

Macroscope's review found this PR approvable — This is a focused server fix that withdraws pending approvals only when their provider turn is interrupted or cancelled, while preserving completed and failed turn behavior. SQL and in-memory projections plus terminal-status coverage are included, with no schema, configuration, or deployment changes.

^{You can add or adjust custom eligibility rules. Learn more.}

Review in Change Stack →

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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

Written by Claude Opus 5.5 on behalf of @ScottN-PV.

Real-run data for "What I couldn't prove", in case it helps. We did not run this patch.

On main b34501a, a Codex parent used delegate_task to start an approval-required child, which blocked on a shell-command approval. The parent's task_cancel returned cancel_requested, then interrupted. In both of two runs, the approval went from pending to cancelled/not_resumable with the reason "The run was interrupted." That reason comes from the request loop in Orchestrator.settleInterruptedRun (#15442). The reporter's v0.0.46-nightly.20261008.2819 already contains that loop, so it was present when their Codex approval stayed pending.

Not covered: Claude tasks, native subagents, Stop from the UI, timing variations.

Thanks @ScottN-PV, this is useful. I agree: for a Codex parent's task_cancel, settleInterruptedRun already withdraws the approval, so that isn't the gap.

This PR covers a different path. settleInterruptedRun returns early unless the run and its attempt are still running. When a turn ends as interrupted or cancelled through ProviderEventIngestor, main only withdraws user-input requests there, so approvals stay pending. The regression test shows this on main.

I haven't confirmed in code which real scenario reaches that path, so I'm not claiming it explains the reporter's case.

Live data for the "What I couldn't prove" section. On main (7208335), a Codex task_cancel left the child's command approval pending.

Setup: a Supervised Codex parent used delegate_task to start a child, the child blocked on touch approved.txt, and the parent called task_cancel. The child run went to interrupted. The approval stayed pending, with a live Approve/Decline card, until I stopped the server.

Doesn't settleInterruptedRun already handle this, as @ScottN-PV saw? Sometimes. It only runs while the run is still running, and in my run Codex's turn/completed got there first. The provider log shows turn/interrupt, then turn/completed (interrupted), then serverRequest/resolved for the approval, within 19 ms.

A second run backs this up. With a turn-end withdrawal in the ingestor (close to this PR), the same cancel left the approval cancelled with its capability still live. That means the turn-end path withdrew it, not settleInterruptedRun, which writes not_resumable. In other words, a real cancel does reach the path this PR fixes.

Two smaller things from testing:

• Leaving completed turns out is right. I first tried withdrawing approvals on every turn end. A Claude background subagent asked for approval while the main turn ran, and when the main turn completed, its still-live callback was cancelled. The subagent hung with no card until Stop.
• The Codex handler leak you list is easy to close in the adapter. If the adapter settles the pending approval's Deferred with cancel when the turn ends interrupted, Codex gets a reply and the handler fiber finishes. The true result from Deferred.succeed also guards the approve-while-interrupting race. I have this with a failing-first adapter test and can send it as a separate PR once this one lands.

Thanks for writing up the turn-ownership cases so clearly.

Thanks @SudheerNaraharisetty, this is really helpful. The provider-log ordering and your second run show that a real Codex cancel reaches this path. I've linked your comment in the "What I couldn't prove" section of the PR description. Thanks as well for confirming that completed turns should stay out.

@coderabbitai Good catch, fixed. The guard now rechecks approval requests too (still pending, same kind and turn, message-mode left out), and there's a test for an approval answered after the pending requests were read.

@fixfon Thanks for fixing this and adding the regression test. The recheck addresses the race where an approval is answered after the pending requests are read.

If you found this review helpful, would you consider giving us a shout-out on X?

Thank you for using CodeRabbit!

You are interacting with an AI system.

On CodeRabbit's outside-diff note about the respond/cancel race: it's real, and it's the reverse of the case I covered. A late cancel can't overwrite an answer anymore, but a respond that read the request as pending before the interrupt can still commit "resolved" after the cancellation and queue a response effect for a turn that's gone. Approvals only hit this since this PR started cancelling them; questions have the same gap on main.

The fix I have in mind is small: commitCommand re-reads the request inside the transaction and rejects the respond if it's no longer pending or the kind or provider turn changed, and dispatchRuntimeRequestRespond maps that to the existing "is cancelled" error, with a latch test for the interleaving. @juliusmarminge since you're driving this branch now, want me to push that here, or keep it for a follow-up PR?