Skip to content

🤖 fix: resume turns stranded by a withdrawn queued tool-end message - #4065

Open
ibetitsmike wants to merge 30 commits into
mainfrom
mike/resume-stranded-tool-turn
Open

🤖 fix: resume turns stranded by a withdrawn queued tool-end message#4065
ibetitsmike wants to merge 30 commits into
mainfrom
mike/resume-stranded-tool-turn

Conversation

@ibetitsmike

@ibetitsmike ibetitsmike commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

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's hasQueuedMessages("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-nested task_await, so the reconciler withdrew the wake (aborted its cancel signal) before the queued entry was dispatched. sendQueuedMessages() dequeued a canceled entry, cancelBeforeAcceptance returned 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.createStopWhenCondition reports a stop made purely for a queued message through a new onQueuedMessageStop request 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. The report carries the steps the stream had left under its ceiling, and the resumed stream runs under exactly that budget (stepBudget on the request; the provider-tool soft stop reports the same figure in its abort metadata). A cut spends at least one step, so the cut stream plus every synthetic resume share the one MAX_STREAM_STEPS ceiling an uninterrupted turn has; at zero no continuation is registered. Auto-retry carries the budget too (a failed attempt reports what it left of the ceiling through its turn completion, so the retry runs under that remainder and is abandoned at zero), and the in-stream loop restarts (fallback swap, empty-output retry, previous_response_id retry) each start an SDK loop that counts from zero, so they inherit the stream's remainder (restartStepBudget) and none runs once it is spent; no same-turn restart refills it. The report also carries the stream's fallback chain state (ModelFallbackProgress: requested model, refusals so far, and the chain), as does the soft stop's abort metadata; a resumed stream keeps that chain instead of resolving one from the model it resumes on and seeds its refusals from it, so a refusal on the fallback continues down the original chain rather than the fallback's own, and the resumed message records the fallback. A failed attempt hands its retry the model it reached and the chain state (auto-retry and the in-session retries alike), and a turn interrupted for mid-stream compaction hands its follow-up its remaining budget and chain state through the persisted CompactionFollowUpRequest, so neither a retry nor the compaction handoff resets the logical turn's ceiling or replays a refused hop. The handoff also carries the turn's admission revalidation, and a follow-up whose turn spent its last step is dropped instead of dispatched. A follow-up continuing a delegated turn is admitted like a stranded resume (workspace state and the owner's handle, with the probe carried to the launch boundary), and a dropped follow-up settles the delegated turn it continued before the durable follow-up is cleared (a failed settlement leaves it pending for the next startup). A goal, workspace, or handle refusal at the launch boundary drops the follow-up the same way. An on-send-compacted wake carries its delegated turn beside its own metadata, and its retries revalidate against that turn. The in-session context_exceeded retries treat a launch refused at the boundary as a retry that did not start, so the episode settles terminal instead of reporting a started retry. A follow-up refused by its own launch probe leaves no durable row: the send rolls its rows back, emits their deletion, and re-derives goal state from the tail (the one rollback past goal sync). A follow-up whose persisted step budget is malformed is dropped and settled rather than run under the default ceiling. Every drop (spent budget, malformed persisted attribution, goal or admission refusal, a refusal at the launch boundary) settles it through one path, its retries revalidate, and persisted correlations are validated before use.
  • AgentSession records 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 hard system stop, or a terminal stream error withdraws it, and so does any send whose durable row is not a continuation of the cut turn (checked as the pre-turn batch crosses the rollback horizon, so a cutter that fails after its row is durable supersedes the turn instead of being resumed over). A launch refused or withdrawn after streamWithHistory appended its [CONTINUE] sentinel removes that row again, so no orphaned sentinel reaches a later turn's provider request. 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 hard stops (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; because a completed stream gives the delegated turn's owner no stream event, the forfeit settles the turn the owner deferred on (a no-op when the stop already did). A user clearing the queue keeps them.
  • One idempotent sweep, 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 calls resumeStream (which streams from history and appends the [CONTINUE] sentinel). The resume claims the turn synchronously, revalidates goal state, workspace rename/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 (refuseStreamStart on TurnExecutionOptions). It also runs under an abort signal that Stop, disposal, and pre-stream hard stops cancel. Resumes that fail before their stream starts (pricing gate, admission reads) stay owed and are swept again at once, since an idle session gets no later poke; attempts are bounded at 3, and at the cap the continuation is forfeited and the delegated owner settled (successful resumes are bounded by the step budget above, not by a count). A resume admitted under this revalidation that later hits a retryable stream error revalidates again on each auto-retry and abandons the retry when the goal or delegated turn no longer admits it; the in-session context_exceeded retries (compaction, post-compaction) revalidate the same way, carry the launch-boundary probe, and run under what the failed attempt left of the budget. 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, through AgentTaskIntegration.settleWorkspaceTurnContinuationFailure. A Resuming turn stranded by a withdrawn queued message log line is emitted for forensics.
  • The queued-message soft stop for provider-executed tools uses its own "queued-message" abort reason, so a hard "system" stop landing while it is pending is recognized as a hard stop. Its abort also reports whether the cut step had already completed a required tool (toolPolicy); the turn was over then, so no continuation is registered, as in the loop's own queued-message stop. The cut's remainder is also stamped on the partial the abort commits (stepsRemaining), and startup auto-retry runs that row under it after a crash; a present but malformed remainder abandons that retry instead of reading as absent.
  • The delegated turn owner's DEFER-or-SETTLE decision at a correlated tool-calls cut 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); a withdrawn entry does not count for anything (MessageQueue's next-entry readers look past entries whose cancel signal fired, as the dispatch-mode reader already did, so a canceled wake ahead of an unrelated live entry does not make the owner defer on that entry, and the all-predecessors scan behind hasQueuedOrDispatchingEntry skips them too, so a draining canceled wake does not strip a same-turn report of its correlation); a queued entry continuing the owed turn is that turn's terminal path if it is cleared or removed (its onCanceled settles the handle), so the continuation is forfeited before the cancellation runs and every queue removal sweeps only after the cancellation callbacks settle, as the dequeue path already did; 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. A cut whose continuation already started stays claimable once (consumedContinuationCuts, bounded), so an owner processing that stream-end after the successor ran defers instead of settling the turn as failed.

Validation

  • Red-green in agentSession.queueDispatch.test.ts, streamManager.test.ts, and taskService.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 with a budget that runs down per cut; the step budget replaces the default ceiling and a spent ceiling or a budget-less abort owes nothing; a wake canceled after a provider-tool soft stop still drains and the turn resumes, which pins the reconciliation with 🤖 fix: drop withdrawn queued wakes from pending work #4067's pending-only hasQueuedMessages; a fallback hop inherits the refused stream's remaining steps and a spent budget buys no hop; an auto-retry of a stranded goal resume asks the goal again and starts nothing when refused; a withdrawn wake ahead of an unrelated entry is not claimed as the delegated turn's continuation; a task hard stop on a stranded delegated turn settles it; a rename refuses the resume at read and at the launch boundary; clearing the queued continuation of a delegated turn settles it before any sweep; an auto-retry runs under what the failed resumed attempt left and is abandoned when that is zero; a launch refused after the sentinel landed removes the sentinel; the post-compaction retry of a stranded goal resume asks the goal again and runs under the failed attempt's remainder).
  • Remote dogfood UAT on dev.coder.com (Coder Agents, headless mux server driven through the browser UI with a live Anthropic model) at d35f042, 3 rounds: the stranded path was exercised 7 times (deterministic ordering: spawn a monitored background bash, sleep past the match, then task_await in 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 test a 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 bounded by the cut turn's remaining step budget; a resume that cannot start is retried at most 3 times.


Generated with xum • Model: anthropic:claude-fable-5-1 • Thinking: xhigh • Cost: $431.84

…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.
@chatgpt-codex-connector

This comment has been minimized.

Copy link
Copy Markdown
Contributor Author

@codex review

@chatgpt-codex-connector

This comment has been minimized.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 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".

Comment thread src/node/services/agentSession.ts Outdated
Comment thread src/node/services/agentSession.ts Outdated
Comment thread src/node/services/agentSession.ts Outdated
Comment thread src/node/services/agentSession.ts Outdated
Comment thread src/node/services/agentSession.ts Outdated
Comment thread src/node/services/streamManager.ts Outdated
…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.

Copy link
Copy Markdown
Contributor Author

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 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".

Comment thread src/node/services/agentSession.ts Outdated
Comment thread src/node/services/agentSession.ts
Comment thread src/node/services/agentSession.ts Outdated
Comment thread src/node/services/agentSession.ts
Comment thread src/node/services/agentSession.ts Outdated

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 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.

Comment thread src/node/services/agentSession.ts
…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.

Copy link
Copy Markdown
Contributor Author

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 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".

Comment thread src/node/services/agentSession.ts
Comment thread src/node/services/agentSession.ts Outdated
Comment thread src/node/services/agentSession.ts Outdated
Comment thread src/node/services/agentSession.ts
Comment thread src/node/services/streamManager.ts
Comment thread src/node/services/agentSession.ts

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 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.

Comment thread src/node/services/agentSession.ts
…, 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.

Copy link
Copy Markdown
Contributor Author

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 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".

Comment thread src/node/services/agentSession.ts Outdated
Comment thread src/node/services/agentSession.ts
Comment thread src/node/services/agentSession.ts
Comment thread src/node/services/agentSession.ts Outdated
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.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 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".

Comment thread src/node/services/agentSession.ts
Comment thread src/node/services/agentSession.ts
Comment thread src/node/services/agentSession.ts Outdated
Comment thread src/node/services/agentSession.ts
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).

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 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".

Comment thread src/node/services/agentSession.ts Outdated
Comment thread src/node/services/agentSession.ts
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.

Copy link
Copy Markdown
Contributor Author

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 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".

Comment thread src/node/services/agentSession.ts Outdated
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.

Copy link
Copy Markdown
Contributor Author

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 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".

Comment thread src/node/services/agentSession.ts Outdated
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.

Copy link
Copy Markdown
Contributor Author

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

this.sendQueuedMessages();

P1 Badge 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


this.messageQueue.hasNextWorkspaceTurnContinuation(

P1 Badge 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".

Comment thread src/node/services/agentSession.ts

Copy link
Copy Markdown
Contributor Author

@codex review

Round 19 fixes are on 3e52e26: the compaction handoff carries the interrupted turn's admission revalidation into the follow-up's send and retry state, and a follow-up whose turn has no step budget left is dropped instead of dispatched.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 3e52e26720

ℹ️ 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".

Comment thread src/node/services/agentSession.ts Outdated
Comment thread src/node/services/agentSession.ts Outdated

Copy link
Copy Markdown
Contributor Author

@codex review

Round 20 fixes are on c86e0ee: a compaction follow-up continuing a delegated turn is admitted through the workspace-turn gate before dispatch and at the launch boundary, and a follow-up that is dropped (spent budget, goal or admission refusal) settles the delegated turn it continued.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: c86e0ee10c

ℹ️ 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".

Comment thread src/node/services/agentSession.ts
Comment thread src/node/services/agentSession.ts Outdated
Comment thread src/node/services/agentSession.ts Outdated
Comment thread src/node/services/agentSession.ts Outdated
@chatgpt-codex-connector

Copy link
Copy Markdown

Security review completed. No security issues were found in this pull request.

Reviewed commit: c86e0ee10c

View security finding report

Only the user who started this review can view the report in Codex.

ℹ️ 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.

… rejected as malformed, revalidate its retries, validate persisted correlations

Copy link
Copy Markdown
Contributor Author

@codex review

Round 21 fixes are on 3ebfba3: a delegated turn's compaction follow-up refused at the launch boundary or rejected as malformed is settled and dropped through one path, its retries run under admission revalidation, and persisted correlations are validated before use.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

this.sendQueuedMessages();

P1 Badge Preserve correlation on provider-tool wake dispatch

When a live bash-monitor wake triggers the queued-message soft-stop path for a delegated stream, this dispatch sends the wake with only its bash-monitor-wake metadata. The abort's cut assistant is subsequently committed with partial: true, while inheritOpenWorkspaceTurnMetadata() explicitly accepts only non-partial tool-calls assistants, so the wake stream starts uncorrelated even though the owner was told a continuation existed; its eventual uncorrelated stream-end can fail the delegated handle instead of delivering the result. Fresh evidence beyond the earlier wake-continuation comments is that the new provider-tool abort path creates a partial cut that the inheritance predicate cannot consume; stamp the saved abortedStreamContext.workspaceTurnMetadata onto this continuation path.

ℹ️ 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".

Comment thread src/node/services/agentSession.ts Outdated
Comment thread src/node/services/agentSession.ts Outdated
Comment thread src/node/services/agentSession.ts
Comment thread src/node/services/agentSession.ts Outdated
Resolve the launch-abort predicate in streamWithHistory: main's isStreamStartAborted
(dispose/shutdown) also reads the branch's refuseStreamStart probe, and every
post-sentinel checkpoint keeps the branch's abortStartup rollback.
…ts delegated turn before clearing it, treat a refused in-session retry as not started, and revalidate a wake's retries against its delegated turn

- dispatchPendingFollowUp reads the combined launch probe after the send: a goal Pause or
  replacement during launch drops and settles the follow-up like a workspace or handle refusal.
- dropFollowUp awaits the owner's settlement before clearing the durable follow-up; a failed
  attempt leaves it pending for the next startup to re-drop and re-settle.
- The in-session context_exceeded retries launch through launchInSessionRetry, which treats an
  Ok with the turn still PREPARING under a tripped probe as no started retry, so the recovery
  decision settles terminal instead of publishing retry-started.
- AutoRetryResumeRequest, sendMessage, resumeStream, admitResumeLaunch, and the in-session
  retries carry workspaceTurnMetadata, the delegated turn a wake's own metadata does not name,
  so retry admission still reaches the owner's handle.

Copy link
Copy Markdown
Contributor Author

@codex review

@chatgpt-codex-connector

Copy link
Copy Markdown

Security review completed. No security issues were found in this pull request.

Reviewed commit: 19302d57cf

View security finding report

Only the user who started this review can view the report in Codex.

ℹ️ 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.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 19302d57cf

ℹ️ 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".

Comment thread src/node/services/agentSession.ts Outdated
…d a required tool

StreamManager records a successful required completion tool (toolPolicy) per step and
reports it on the queued-message abort (requiredToolSatisfied); the session then treats
the cut turn as complete instead of registering a stranded resume, matching the loop's own
queued-message stop condition.

Copy link
Copy Markdown
Contributor Author

@codex review

@chatgpt-codex-connector

Copy link
Copy Markdown

Security review completed. No security issues were found in this pull request.

Reviewed commit: 7f6de853d3

View security finding report

Only the user who started this review can view the report in Codex.

ℹ️ 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.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 7f6de853d3

ℹ️ 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".

Comment thread src/node/services/agentSession.ts
Comment thread src/node/services/agentSession.ts Outdated
Comment thread src/node/services/streamManager.ts
Comment thread src/node/services/messageQueue.ts
…ormed persisted step budget, persist a queued cut's remainder for startup retry, and skip withdrawn entries in the all-predecessors scan

- sendMessage: a synthetic continuation refused by its own launch probe at the boundary
  (Ok, turn still PREPARING) rolls its persisted rows back, emits their deletion, and
  re-derives goal state from the tail it leaves; the one rollback past goal sync.
- dispatchPendingFollowUp: a present but malformed stepBudget drops and settles the
  follow-up instead of reading as absent and running under the default ceiling.
- StreamManager stamps stepsRemaining on the partial committed by a queued-message abort;
  startup auto-retry runs the interrupted row under that remainder (abandoned at zero).
- MessageQueue.hasAllWorkspaceTurnContinuations ignores withdrawn entries, so a draining
  canceled wake no longer strips a same-turn report of its correlation.

Copy link
Copy Markdown
Contributor Author

@codex review

Copy link
Copy Markdown
Contributor Author

@codex review

@chatgpt-codex-connector

Copy link
Copy Markdown

Security review completed. No security issues were found in this pull request.

Reviewed commit: 25197940e3

View security finding report

Only the user who started this review can view the report in Codex.

ℹ️ 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.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 25197940e3

ℹ️ 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".

Comment thread src/node/services/agentSession.ts Outdated
Comment thread src/node/services/agentSession.ts Outdated
… persisted remainder

Codex round 25: a stranded resume that failed before its stream stayed owed with no later poke on an idle session, leaving a delegated owner running forever; the sweep now runs again after the failure, bounded by the consecutive cap whose forfeit settles the owner. Startup auto-retry validates the partial row's raw stepsRemaining as a nonnegative integer and abandons the retry otherwise instead of running under the default ceiling.

Copy link
Copy Markdown
Contributor Author

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 6a8263f324

ℹ️ 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".

Comment on lines +2549 to +2553
resumeOptions,
agentInitiated,
goalKind,
goalId,
persistedStepsRemaining

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Revalidate crash-recovered queued cuts

When the process exits after a queued-message partial is committed but before its in-memory resume, and its goal or delegated handle is stopped before restart, this startup path restores only the step budget. It does not set revalidateAdmission, and for a bash-monitor wake it also omits the correlation stored on interruptedAssistant (inheritOpenWorkspaceTurnMetadata rejects partial assistants), so retryActiveStream() can restart autonomous work uncorrelated to an already-terminal handle. Restore the parsed assistant correlation and mark queued-cut recovery for admission revalidation.

AGENTS.md reference: AGENTS.md:L112-L112

Useful? React with 👍 / 👎.

Comment on lines 1993 to +1996
...(streamInfo.toolModelUsages.length > 0
? { toolModelUsages: streamInfo.toolModelUsages.map(clonePersistedToolModelUsage) }
: {}),
...(stepsRemaining !== undefined ? { stepsRemaining } : {}),

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Persist fallback progress with crash-recoverable cuts

When A's fallback chain is [B, C], the stream is cut while running B, and the process exits before the in-memory resume, this partial persists only stepsRemaining; the original chain and refusal progress remain solely in the abort event. Startup recovery consequently selects B from the assistant metadata and resolves B's own chain, so a later B refusal cannot reliably continue to C. Fresh evidence beyond the earlier in-memory fallback finding is this crash-only persistence handoff; persist and restore modelFallbackProgress with the cut row.

AGENTS.md reference: AGENTS.md:L112-L112

Useful? React with 👍 / 👎.

Comment thread src/node/services/agentSession.ts
Comment thread src/node/services/agentSession.ts
@chatgpt-codex-connector

Copy link
Copy Markdown

Security review completed. No security issues were found in this pull request.

Reviewed commit: 6a8263f324

View security finding report

Only the user who started this review can view the report in Codex.

ℹ️ 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.

Codex round 26: the stream-abort accounting used the requested model from the active context; a configured fallback that ran the stream reports its usage under the effective model, which the abort event now carries. Mirror stream-end and price against payload.metadata.model when present.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 206328207c

ℹ️ 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".

Comment on lines +5777 to +5782
this.strandedTurnResume = buildStrandedTurnResume({
...this.activeStreamContext,
modelString: stop.modelString,
cutMessageId: this.activeStreamMessageId,
stepBudget: stop.stepsRemaining,
modelFallbackProgress: stop.modelFallbackProgress,

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Persist normal queue-cut continuation state

When a tool-end queued message triggers the normal AI SDK stopWhen path, this callback stores the continuation only in strandedTurnResume. If the process exits after the completed assistant is committed but before the queued entry dispatches or the idle sweep resumes it, the in-memory queue and continuation disappear; history then ends with a non-partial assistant, which scheduleStartupAutoRetryIfNeeded() does not recover, permanently stranding the original turn and potentially its delegated owner. Unlike the provider-tool soft-abort path, persist a recoverable cut marker—including its remaining budget, fallback state, and correlation—and restore it during startup.

AGENTS.md reference: AGENTS.md:L110-L112

Useful? React with 👍 / 👎.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant