Repository navigation
fix(think): replay onChatResponse owed across a reset; checkpoint before messenger apology (#2266, #1842) - #2340
Conversation
🦋 Changeset detectedLatest commit: 89d1448 The changes in this PR will be included in the next version bump. This PR includes changesets to release 3 packages
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 |
⚪ agents import sizesMeasured 344 runtime imports as minified bundles. The primary size is gzip; raw minified size is included for diagnosis. An existing import growing by more than 10% is marked red. This report is informational.
Compared No import sizes changed. All 344 current runtime imports
Reported by agent-think[bot]. |
agents
@cloudflare/ai-chat
@cloudflare/codemode
hono-agents
@cloudflare/shell
@cloudflare/think
@cloudflare/voice
@cloudflare/worker-bundler
commit: |
| options.snapshotThread | ||
| ) | ||
| ); | ||
| await options.surface |
There was a problem hiding this comment.
🟡 Terminal messenger replies can disappear
A reset after checkpoint but before surface.post leaves recovery seeing a completed delivery. Interrupted and failed turns then lose their terminal reply permanently.
(Refers to this code)
Learn more
The completed snapshot now commits before the external messenger side effect. Recovery treats that snapshot as proof that delivery finished, but the process can reset before surface.post starts or completes. The same ordering appears for both interrupted apologies and error responses. Avoiding duplicates by advancing the checkpoint first changes delivery from at-least-once to at-most-once and introduces silent loss.
Example: Slack delivery reaches the interrupted branch. Think stores stage: "completed", then the isolate resets before posting the apology. Recovery sees the completed stage and performs no post, leaving the user with only a truncated stream or no response.
Recommended fix: Use a durable outbox or provider idempotency key so the terminal post can be retried without duplication. Mark the snapshot completed only after the idempotent delivery succeeds.
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
Keeping this ordering on purpose. #1842 asked for it: the terminal post is a courtesy apology or error line, and posting it twice was the reported bug. The posts already swallow failures, so they were never guaranteed delivery; chat-sdk has no provider idempotency key to build an outbox on. The streamed partial stays visible either way.
| If the Durable Object resets after the assistant message is persisted but before the hook runs, Think fires the hook when the agent wakes, with `recovered: true` and the stored message. A reset while the hook itself is running can fire it again on wake, so make side effects in the hook idempotent (for example, keyed by `requestId`). | ||
|
|
||
| ```typescript | ||
| onChatResponse(result: ChatResponseResult): void | Promise<void> | ||
| ``` | ||
|
|
||
| ### ChatResponseResult | ||
|
|
||
| | Field | Type | Description | | ||
| | -------------- | ------------------------------------- | ------------------------------------------ | | ||
| | `message` | `UIMessage` | The persisted assistant message | | ||
| | `requestId` | `string` | Unique ID for this turn | | ||
| | `continuation` | `boolean` | Whether this was a continuation turn | | ||
| | `status` | `"completed" \| "error" \| "aborted"` | How the turn ended | | ||
| | `error` | `string?` | Error message (when `status` is `"error"`) | | ||
| | Field | Type | Description | | ||
| | -------------- | ------------------------------------- | ----------------------------------------------------------------------- | | ||
| | `message` | `UIMessage` | The persisted assistant message | | ||
| | `requestId` | `string` | Unique ID for this turn | | ||
| | `continuation` | `boolean` | Whether this was a continuation turn | | ||
| | `status` | `"completed" \| "error" \| "aborted"` | How the turn ended | | ||
| | `error` | `string?` | Error message (when `status` is `"error"`) | | ||
| | `recovered` | `boolean?` | `true` when the hook is fired on wake for a turn interrupted by a reset | |
There was a problem hiding this comment.
Noted; the cloudflare-docs port happens when the stack lands.
1b9f5c9 to
97b1780
Compare
…C finalizer Co-authored-by: Cursor <cursoragent@cursor.com>
97b1780 to
89d1448
Compare
Fixes #2266. Fixes #1842.
#2266. If the Durable Object reset between persisting the assistant message and running
onChatResponse, the hook never ran, and fiber recovery re-ran a turn that had already completed. Think now writes a small marker before the cutover and clears it after the hook. On recovery (fiber path and startup), a marker whose message is persisted replays the hook withrecovered: trueand settles the submission without re-running the model. Replay is at-least-once if the reset lands inside the hook itself; the docs say so.#1842. Messenger delivery posted the apology or error text before checkpointing the turn as completed, so a reset between the two re-posted it. The checkpoint now comes first.
ChatResponseResult.recovered?: boolean.run-turn-recovery.test.ts, three messenger cases (all fail without the fix).