🤖 fix: resume turns stranded by a withdrawn queued tool-end message - #4065
🤖 fix: resume turns stranded by a withdrawn queued tool-end message#4065ibetitsmike wants to merge 12 commits into
Conversation
…nd message StreamManager's stopWhen ends the model loop at a tool boundary whenever a tool-end message is queued. When that queued entry is then withdrawn before it starts a turn (a bash-monitor wake canceled by the reconciler after the model already consumed the output, a cleared queue, or a pre-stream failure), the stream-end drain finds nothing to dispatch and the session goes idle with a tool result the model never answered. Record why the loop stopped (onQueuedMessageStop, skipped when a required tool completed the turn) and have AgentSession owe a continuation for that stop. Any stream that actually starts consumes the mark; otherwise one idempotent sweep (resumeStrandedTurnIfIdle) resumes from history at every idle transition and queue removal. The provider-executed soft-stop path owes the same continuation. Consecutive stranded resumes are capped at 3.
This comment has been minimized.
This comment has been minimized.
|
@codex review |
This comment has been minimized.
This comment has been minimized.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d35f042459
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…on, keep it until a stream starts Codex round 1: record the owed continuation synchronously so delegated-turn settlement sees it, resume with the resolved workspace-turn correlation and goal attribution, yield to manual sends in preflight, keep the continuation owed when the resume fails pre-start, and do not report queue stops that coincide with the step cap.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 91ca0eecc8
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Security Review
Here are some automated security review suggestions for this pull request.
Reviewed commit: 91ca0eecc8
ℹ️ About Codex security reviews in GitHub
This is an experimental Codex feature. Security reviews are triggered when:
- You comment "@codex security review"
- A regular code review gets triggered (for example, "@codex review" or when a PR is opened), and you’re opted in so security review runs alongside code review
Once complete, Codex will leave suggestions, or a comment if no findings are found.
…al veto, and cap; claim the turn before resume gates - User Stop (interruptStream + user/system/startup aborts) withdraws the owed continuation so restoreQueueToInput's clearQueue sweep cannot restart the model. - resumeStream claims PREPARING before its async admission gates (pricing, goal), closing the idle window a manual send could slip through. - Stranded goal turns revalidate against buildGoalRedispatchAdmission once the turn is claimed; a paused or transitioned goal forfeits the continuation. - The resume options come from the startup-retry whitelist, dropping ACP-only fields (acpPromptId, delegatedToolNames) and other per-dispatch options. - Past the consecutive-resume cap the marker is forfeited on the first sweep and never advertised to hasPendingWorkspaceTurnContinuation; "consecutive" now resets whenever a non-resume stream starts.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2077d46d56
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Security Review
Here are some automated security review suggestions for this pull request.
Reviewed commit: 2077d46d56
ℹ️ About Codex security reviews in GitHub
This is an experimental Codex feature. Security reviews are triggered when:
- You comment "@codex security review"
- A regular code review gets triggered (for example, "@codex review" or when a PR is opened), and you’re opted in so security review runs alongside code review
Once complete, Codex will leave suggestions, or a comment if no findings are found.
…, keep goal and hard-stop vetoes honest - The sweep's resume carries an AbortController; withdrawStrandedTurnResume (user Stop, superseding send, edit, context discard, non-soft aborts) clears the marker and aborts it, and resumeStream threads the signal through its gates and streamWithHistory so a Stop during admission starts nothing. - resumeStream revalidates goal turns itself (revalidateGoal) and rechecks buildGoalRedispatchAdmission's staleness probe before launch; a refusal reports goalRefused so the sweep drops the marker. - A correlated marker is forfeited when sendQueuedMessages dequeues an entry that is not that turn's continuation (the owner settled the turn at the cut). - Messages queued behind a rejected resume drain from the sweep's settlement. - onQueuedMessageStop carries the request's modelString and stream-abort metadata carries the active model, so a resume continues on the fallback model that reached the cut; a mid-turn applied thinking level is kept. - The provider-tool soft stop uses a dedicated "queued-message" abort reason; only that reason (with the in-flight flag) rebuilds the marker, so a hard "system" stop from task_stop or an interrupt cascade cannot revive the turn.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3142786317
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Codex round 4 on #4065: - Thread the goal admission probe from resumeStream through streamWithHistory and aiService.streamMessage into TurnExecutionOptions.refuseStreamStart, so a Pause or goal replacement landing during the pre-stream history reads or request construction refuses the launch; StreamManager rechecks it right before the stream registers. - dispose() withdraws an in-flight stranded resume: past streamWithHistory's disposed check only its abort signal can stop it registering a stream after teardown. - A synthetic pre-stream abort (task_stop / interrupt cascade through aiService.stopStream with no registered stream) withdraws a preparing resume; only the queued-message soft stop keeps its obligation. - The resume snapshots a mid-turn thinking change still pending at the cut, not only an already applied one.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e5cf350e9e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Dogfood UAT: a prompt that awaits four monitored background processes in a row (each task_await consuming the wake) strands four times; the old cap counted every resume and dropped the fourth, leaving the turn on a text-less tool row with no answer, the original symptom. Every stranding follows a completed model step, so a resume that starts a stream is real progress and resets the counter; the cap now bounds only resume attempts that fail before their stream starts (pricing gate, history read, refused admission).
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4a784323f1
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Codex round 5 on #4065: - The soft-stop abort handler re-reads queuedProviderToolEndAbortInFlight before rebuilding the marker; a hard stop landing during its awaits reset it. - clearQueue gains a hardStop option. TaskService hard stops (task_stop, descendant cascade, workflow hard timeout) clear the queue and then stop the stream through StreamManager directly, which emits no abort when the stream has completed or has not registered, so the queue clear is the boundary that forfeits the owed continuation and a pending provider-tool soft stop. - Forfeiting a correlated marker without a successor stream (goal admission refused, retry cap) settles the delegated turn whose stream-end the owner deferred, through a new session hook wired to AgentTaskIntegration.settleWorkspaceTurnContinuationFailure. - handleStreamError withdraws the owed continuation; the error path owns what happens next.
pickStartupRetrySendOptions omits additionalSystemContext because it is not durable retry state, but the stranded resume is the same turn continued in memory: without it the resumed stream falls back to the persisted scratchpad, which can be stale or empty while the renderer's save is still in flight.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8a8e256085
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
A context-discarding mutation or session disposal drops the owed continuation while the session is idle, with no successor stream or terminal event to settle the delegated turn whose stream-end the owner deferred on it. Forfeit (withdraw plus settle) at both boundaries instead of withdrawing silently.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8a8494f84e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
The delegated turn owner decided DEFER vs SETTLE from two session reads (workspace-turn continuation, pending bash wake) while the session dropped a correlated marker at dequeue whenever the queue head was not a same-turn entry. The two views diverged both ways: a real bash-monitor wake at the head (no correlation on the entry) made the owner defer while the session dropped the marker, so a wake withdrawn after dequeue left the delegated turn stranded and the owner hanging; and an unrelated head removed before dequeue left the marker alive to resume a turn the owner had already settled. Replace both reads with one claimWorkspaceTurnContinuation(metadata, streamEndMessageId) that is the owner's decision: a pending wake counts as the turn's continuation, and a false answer voids the marker for that exact cut, so the marker cannot outlive the settlement regardless of how the superseding entry leaves the queue. The marker records the cut stream's message id so an owner still settling an older stream-end of the same turn defers instead of voiding a newer cut's continuation. The dequeue-time drop is removed.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
xum/src/node/services/agentSession.ts
Line 7162 in 55fa402
Forfeit correlated resumes on provider-tool supersession
When a workspace-turn-correlated stream is soft-stopped after a provider-executed tool, an unrelated queued entry is dispatched here without the binding claimWorkspaceTurnContinuation() check used by the normal stream-end path. If that entry is canceled after dequeue or fails before reaching STREAMING, sendQueuedMessages() falls through to resumeStrandedTurnIfIdle(), and the retained marker restarts the delegated turn instead of treating the unrelated cutter as superseding input; any persisted user row can consequently be processed under the old workspace-turn correlation. Fresh evidence beyond the earlier supersession finding is that provider-tool cuts take this abort-only branch, while finalizeWorkspaceTurnFromStreamAbort() ignores every non-user abort, so no later stream-end claim can forfeit the marker. Classify the queued cutter and withdraw a correlated marker before dispatching unrelated input.
AGENTS.md reference: AGENTS.md:L150-L150
xum/src/node/services/agentSession.ts
Line 6968 in 55fa402
Prioritize the dispatching cutter over queued continuations
When stream-end cleanup has already dequeued an unrelated entry and a same-turn report or bash-monitor wake remains at the queue head, this queue-head check (and the wake check below) returns true before the unrelated dispatchingQueuedEntryMuxMetadata can veto continuation. The owner consequently defers settlement based on work behind the superseding input; if the unrelated entry is then canceled or fails before STREAMING, the queued continuation starts under the old correlation even though an engaged unrelated cutter is supposed to settle that turn. Fresh evidence beyond the earlier queue-removal thread is this two-entry ordering: the continuation remains queued behind an already-dequeued unrelated cutter. Resolve PREPARING/dispatching attribution first and bind an unrelated engaged entry to false before inspecting the remaining queue.
AGENTS.md reference: AGENTS.md:L150-L150
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex review |
|
Codex Review: Didn't find any major issues. More of your lovely PRs please. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
Task hard stops (hard timeout, descendant terminate cascade) now clear the queue with { hardStop: true } so the session forfeits an owed stranded continuation; the two existing call-shape assertions still expected the bare call.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 23e9c8727a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Security Review
Here are some automated security review suggestions for this pull request.
Reviewed commit: 23e9c8727a
ℹ️ About Codex security reviews in GitHub
This is an experimental Codex feature. Security reviews are triggered when:
- You comment "@codex security review"
- A regular code review gets triggered (for example, "@codex review" or when a PR is opened), and you’re opted in so security review runs alongside code review
Once complete, Codex will leave suggestions, or a comment if no findings are found.
Retain and autonomously retry the delegated-turn settlement owed when a stranded continuation is forfeited, including after AgentSession disposal. Recheck pull-based admission after the durable turn-envelope write. Before an internal stranded resume starts, reapply workspace archive/removal admission and verify a correlated workspace-turn handle is still active, carrying the workspace stop epoch through StreamManager's pre-start probes. Add red-green coverage for settlement retry through disposal, stopped-turn refusal at read and after admission, post-envelope goal refusal, and the TaskService handle/epoch admission contract.
|
@codex review |
|
Codex Review: Something went wrong. Try again later by commenting “@codex review”. ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
|
@codex review |
|
Codex Review: Something went wrong. Try again later by commenting “@codex review”. ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
1 similar comment
|
Codex Review: Something went wrong. Try again later by commenting “@codex review”. ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
|
@codex review |
|
Codex Review: Didn't find any major issues. Delightful! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
There was a problem hiding this comment.
💡 Codex Security Review
Here are some automated security review suggestions for this pull request.
Reviewed commit: ba2a9da297
ℹ️ About Codex security reviews in GitHub
This is an experimental Codex feature. Security reviews are triggered when:
- You comment "@codex security review"
- A regular code review gets triggered (for example, "@codex review" or when a PR is opened), and you’re opted in so security review runs alongside code review
Once complete, Codex will leave suggestions, or a comment if no findings are found.
| // "Consecutive" counts only resume attempts that never got this far: a stream that starts | ||
| // (the resume's own included) consumed the marker, so a later stranding is new work. | ||
| this.consecutiveStrandedResumes = 0; |
There was a problem hiding this comment.
An untrusted model/provider (or prompt-injected repo) can repeatedly launch a monitored background task and consume its wake with task_await, causing each queued tool-end cut to be withdrawn. Every synthetic resume that reaches STREAMING resets the only counter, so the three-attempt cap covers only failed starts; the added test deliberately succeeds across four cuts, and the cycle has no bound. This bypasses the 100,000-step cap and can drive unbounded provider spend and repeated bash/file-tool execution until Stop. Preserve a logical-turn-wide budget. Fresh evidence beyond the prior cap thread is this reset after every successful synthetic stream.
Useful? React with 👍 / 👎.
1 participant
Summary
When the model loop is stopped at a tool boundary on behalf of a queued tool-end message, and that message is later withdrawn instead of starting a turn, the session now resumes the interrupted turn instead of leaving it idle on an unanswered tool result.
Background
Observed in a real workspace: three monitored background bash tasks finished while the agent was streaming. The bash-monitor wake was queued as a tool-end message, so
createStopWhenCondition'shasQueuedMessages("tool-end")ended the stream right after the next tool result (finishReason: "tool-calls"). The agent had already read the same output through a kernel-nestedtask_await, so the reconciler withdrew the wake (aborted its cancel signal) before the queued entry was dispatched.sendQueuedMessages()dequeued a canceled entry,cancelBeforeAcceptancereturned without a stream, and the session went IDLE. The tool result was never answered and the workspace sat idle until a human resumed it.The same shape applies to any tool-end entry that disappears between the stop decision and its dispatch (queue cleared, dedupe-key removal, pre-stream send failure) and to the provider-executed soft-stop path.
Implementation
StreamManager.createStopWhenConditionreports a stop made purely for a queued message through a newonQueuedMessageStoprequest callback, carrying the model that reached the cut (a configured fallback may differ from the requested one). A step that also carries a successful required tool result (agent_report,propose_plan) is a legitimate end of turn and does not report; neither does a step-cap stop.AgentSessionrecords that report synchronously as an owed continuation (strandedTurnResume), built from the retry-safe options whitelist (pickStartupRetrySendOptions) plus the live scratchpad snapshot, the workspace-turn correlation, the model at the cut, and a mid-turn thinking change (pending or applied). Any stream that actually starts (STREAMING transition) consumes it; a user Stop, a manual user message, a hard system stop, or a terminal stream error withdraws it. Context discard, session disposal, retry-cap exhaustion, and admission refusal forfeit it: the delegated-turn settlement is retained before the marker is removed and retries autonomously after task-store failure, including through session disposal. Marking keys on STREAMING rather than PREPARING because a dequeued entry can still be canceled before acceptance.task_stop, descendant interrupt cascade, workflow hard timeout) clear the queue and then stop the stream through StreamManager directly, which emits nothing when the stream has already completed or has not registered yet;clearQueue(..., { hardStop: true })is therefore the session-visible boundary that forfeits the owed continuation and a pending provider-tool soft stop. A user clearing the queue keeps them.resumeStrandedTurnIfIdle(), runs from the idle transitions and queue removals. When the session is idle with an empty queue, no manual send in preflight, and a continuation owed, it callsresumeStream(which streams from history and appends the[CONTINUE]sentinel). The resume claims the turn synchronously, revalidates goal state, workspace archive/removal state, and any delegated workspace-turn handle, and keeps the goal and workspace-stop probes live through pre-stream I/O, request construction, stream registration, and the awaited durable turn-envelope write (refuseStreamStartonTurnExecutionOptions). It also runs under an abort signal that Stop, disposal, and pre-stream hard stops cancel. Resumes that fail before their stream starts stay owed and retry, bounded at 3 attempts. When a correlated continuation is given up without a successor stream (goal no longer admits it, retry cap, context-discarding history mutation, session disposal), the session settles the delegated turn whose stream-end the owner deferred, throughAgentTaskIntegration.settleWorkspaceTurnContinuationFailure. AResuming turn stranded by a withdrawn queued messagelog line is emitted for forensics."queued-message"abort reason, so a hard"system"stop landing while it is pending is recognized as a hard stop.tool-callscut is one session call,claimWorkspaceTurnContinuation(correlation, streamEndMessageId), which replaces the two previous reads (workspace-turn continuation, pending bash wake). A pending wake counts as the turn's continuation (it inherits the correlation when it sends), and a false answer is binding: it voids the continuation owed to that exact cut, so a settled turn cannot resume as orphaned work no matter how the superseding entry later leaves the queue (dispatch, cancel before acceptance, clear). The marker records the cut stream's message id so an owner still settling an older stream-end of the same turn defers rather than voiding a newer cut's continuation.Validation
agentSession.queueDispatch.test.ts,streamManager.test.ts, andtaskService.test.ts: every mechanism above has a test that fails with that mechanism removed (withdrawn wake resumes, soft-stop resumes, Stop/dispose/system stop during the pre-stream window cancel the resume, a hard stop that finds a completed stream or lands during the soft-stop abort's cleanup withdraws it, a terminal stream error discards it, goal Pause before launch, before registration, and during the durable envelope write drops it; workspace removal/archive or a stopped delegated-turn handle refuses it at read or via the workspace stop epoch before launch; failed forfeiture settlement retries through disposal; correlation forfeit, fallback model, scratchpad snapshot and pending/applied thinking carry over, queue drains behind a rejected resume, failing resumes capped at 3 while a chain of legitimate strandings resumes every time).mux serverdriven through the browser UI with a live Anthropic model) atd35f042, 3 rounds: the stranded path was exercised 7 times (deterministic ordering: spawn a monitored background bash, sleep past the match, thentask_awaitin the same step) and resumed every time, 111 to 152 ms after the cut, with[CONTINUE]hidden in the UI and a normal final answer. Controls passed: a user message typed during the tool dispatched once with no duplicate, plain text turns produced no continuation, a hard Stop during a tool left the workspace idle with no resume, reload after a resume rendered correctly. The UAT found one defect at that SHA: four sequential monitored-bash-plus-await calls in one prompt strand four times and the fourth was dropped by the old "3 consecutive resumes" cap (text-less row, no answer). Fixed in this PR by counting only resume attempts that never start a stream (see the regression testa turn stranded after each of several awaited monitors resumes every time); the later review-hardening commits were not separately UAT'd.Risks
Touches the queue-dispatch path in
AgentSession, so the regression surface is turn continuation after tool calls. The new behavior only triggers when a queued-message stop was reported and no stream started afterwards. Internal resumes now repeat the same removal/archive and delegated-turn admission that external resume entry points enforce; ordinary queued dispatches, compaction follow-ups, goal continuations, and required-tool completions are unchanged. Worst case on a misfire is one extra agent-initiated continuation turn; a resume that cannot start is retried at most 3 times.Generated with
xum• Model:openai:gpt-5.6-sol• Thinking:xhigh• Cost:$183.81