Skip to content

fix(think): submission and turn lifecycle edge cases - #2386

Merged
threepointone merged 2 commits into
mainfrom
fix/think-submissions-review
Sep 27, 2026
Merged

threepointone merged 2 commits into
mainfrom
fix/think-submissions-review

Conversation

@threepointone

@threepointone threepointone commented Sep 27, 2026 •

Copy link
Copy Markdown
Contributor

Follow-ups from a review of recently landed Think submission/turn PRs (#2353, #2352, #2351, #2350, #2341, #2331, #2330, #2327, #2326).

  • waitForSubmission can't deadlock. It throws a clear error when called from inside the active turn (e.g. a tool that did runTurn({ mode: "submit" }) then waited, the pattern the in-turn error message recommended) or from onSubmissionStatus for a submission that can't settle until the hook returns. Docs and the in-turn error message are updated.
  • Cancelling a running submission emits its terminal status once. Only the path that actually moved the row out of running emits; the abort controller is registered before the running hook fires.
  • After resetTurnState, waiters on skipped submissions resolve only after their hook ran.
  • activeChannel is gated like activeTurn, so leftover tool work from one turn no longer routes notices to a later turn's channel.
  • Reject with autoContinue: false mid-stream now drops the stale pending-state text once the parking turn finishes.
  • dropGenerationAfterToolCall drops only the paused tool's own step and the step reacting to it, keeping later steps' text.
  • _lastTurnChannel is set only when a turn starts inference and is cleared on reset.
  • Tests added for feat/think reject without continue #2353 gaps (open WebSocket, mixed batch) and a Think port of the fix/2185 approval batch rearm #2352 ai-chat regression test (passes; no code change needed).

Not fixed: messagesApplied fallback misreporting when a submission reuses an existing message id; the stored data can't reliably distinguish a reused id from a fresh write, and a wrong answer would affect recovery replay.

Verified on a combined branch with the other review follow-ups: pnpm run check, agents chat/workers/react, ai-chat, and Think suites all pass.


Devin Review

- waitForSubmission throws in-turn / from onSubmissionStatus instead of deadlocking
- cancelled running submissions emit terminal status once; controller registered before running hook
- reset-skipped submissions visible to waiters only after their hook
- activeChannel gated to the admitted turn
- _lastTurnChannel set at inference start, cleared on reset
- autoContinue:false mid-stream reject flushes deferred drop at finalize
- dropGenerationAfterToolCall keeps later steps' text
- regression tests, including ai-chat #2352 port and #2353 gaps

Co-authored-by: Cursor <cursoragent@cursor.com>
@changeset-bot

changeset-bot Bot commented Sep 27, 2026 •

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: 255089f

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 2 packages
Name Type
@cloudflare/think Patch
@cloudflare/agent-think Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@agent-think

agent-think Bot commented Sep 27, 2026 •

Copy link
Copy Markdown
Contributor

✅ agents import sizes: no significant changes (dbf170cf → 255089f0, workflow run)

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Devin Review found 1 potential issue.

Devin Review

Comment thread packages/think/src/think.ts Outdated
Comment on lines +7008 to +7009
const active = this._activeAdmittedTurn();
if (active) this._lastTurnChannel = { channel: active.channel };

@devin-ai-integration devin-ai-integration Bot Sep 27, 2026 •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Turn reset resurrects a cancelled channel

When resetTurnState() runs during asynchronous preparation, _runInferenceLoop restores the cancelled turn's channel afterward. Automatic continuations can then route to that channel instead of the post-reset conversation.

Learn more

A turn may pause while assembling context or awaiting beforeTurn. Meanwhile, resetTurnState() advances the turn queue's generation and clears _lastTurnChannel there. The in-flight turn is not skipped by TurnQueue; it can finish preparation after the reset. Since active was captured before that pause, recording its channel reverses the reset, and _channelForAutoContinuation uses the stale value on a later continuation.

Example: A voice turn waits in beforeTurn. A chat-clear resets the agent, then the hook finishes. The old voice turn records voice again; a later automatic continuation can inherit voice instead of resolving from the cleared or newly started conversation.

Recommended fix: Check the turn queue generation and active request again after preparation before updating _lastTurnChannel. Keep it cleared if the turn was invalidated by a reset, including when an asynchronous preparation finishes after its abort signal fires.

Devin Review


Was this helpful? React with 👍 or 👎 to provide feedback.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Valid — fixed in 255089f. _lastTurnChannel is now recorded only after _prepareInferenceInvocation succeeds and the stream starts, so a turn whose beforeTurn throws keeps the previous channel. Regression test in ws-web-channel.test.ts.

@pkg-pr-new

pkg-pr-new Bot commented Sep 27, 2026 •

Copy link
Copy Markdown

Open in StackBlitz

agents

npm i https://pkg.pr.new/agents@2386

@cloudflare/ai-chat

npm i https://pkg.pr.new/@cloudflare/ai-chat@2386

@cloudflare/codemode

npm i https://pkg.pr.new/@cloudflare/codemode@2386

hono-agents

npm i https://pkg.pr.new/hono-agents@2386

@cloudflare/shell

npm i https://pkg.pr.new/@cloudflare/shell@2386

@cloudflare/think

npm i https://pkg.pr.new/@cloudflare/think@2386

@cloudflare/voice

npm i https://pkg.pr.new/@cloudflare/voice@2386

@cloudflare/worker-bundler

npm i https://pkg.pr.new/@cloudflare/worker-bundler@2386

commit: 255089f

Record _lastTurnChannel only after inference preparation (beforeTurn)
succeeds and the stream starts, so a channel-less continuation does not
route to a turn that never ran.

Co-authored-by: Cursor <cursoragent@cursor.com>
@threepointone
threepointone merged commit 3a3e9eb into main Sep 27, 2026
9 checks passed
@threepointone
threepointone deleted the fix/think-submissions-review branch September 27, 2026 17:33
@github-actions github-actions Bot mentioned this pull request Sep 27, 2026
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