Repository navigation
fix(think): deliver the recovered answer to the messenger thread (#2106) - #2347
Conversation
🦋 Changeset detectedLatest commit: e84e6b0 The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 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]. |
| await host._cf_deliverRecoveredMessengerReply(input); | ||
| } else { | ||
| await this._cf_deliverRecoveredMessengerReply(input); | ||
| } | ||
| await this.ctx.storage.delete(key); |
There was a problem hiding this comment.
Yes, at-least-once is the intended tradeoff and is documented.
| A reply can also be interrupted without a restart, for example by the stream | ||
| stall watchdog or by a model error that `classifyChatError` marks as | ||
| `"transient"` or `"rate_limit"`. Think then continues the turn through chat | ||
| recovery and posts the rest of the answer to the thread as a new message once | ||
| the continuation finishes. This works whether the conversation target is the | ||
| root agent or a per-thread sub-agent. If recovery gives up, Think posts the | ||
| configured interruption message instead. A reset after recovery finishes but | ||
| before the post lands can post the recovered text twice. |
There was a problem hiding this comment.
Noted; the cloudflare-docs port happens when the stack lands.
agents
@cloudflare/ai-chat
@cloudflare/codemode
hono-agents
@cloudflare/shell
@cloudflare/think
@cloudflare/voice
@cloudflare/worker-bundler
commit: |
2c7ee65 to
8947afa
Compare
8947afa to
422f2a2
Compare
422f2a2 to
eec566b
Compare
eec566b to
92b4992
Compare
| ): void { | ||
| const pinnedText = this._recoveredReplyText.get(incidentId); | ||
| this._recoveredReplyText.delete(incidentId); | ||
| void this.keepAliveWhile(async () => { |
There was a problem hiding this comment.
🟡 Reset strands recovered messenger delivery
A terminal event starts _settleMessengerRecovery without awaiting its outcome write. A reset can leave an outcome-less record that startup skips forever.
Learn more
The recovery engine persists the terminal transition before emitting the event; completed incidents are deleted in updateIncident. _settleMessengerRecovery then starts a detached task whose first durable action happens later. A reset in that gap cannot recreate the terminal event because the incident is already terminal or gone. Startup finds the pending messenger record, but _replayMessengerRecoveryDeliveries ignores records without outcome.
Example: Recovery completes and deletes its incident. The object resets before the detached task writes outcome: "completed". On every later start, the pending record remains without an outcome, so the thread never receives the recovered text.
Recommended fix: Make terminal recovery settlement await the durable outcome write before completing the incident transition, or persist enough terminal delivery state atomically before the incident becomes unrecoverable. Delivery itself can remain asynchronous and at-least-once after that write.
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
Valid, fixed. The recovery callbacks now durably write the messenger reply outcome (and the recovered text) before the incident transition that completes or ends it. Settlement then delivers a record that already has an outcome instead of skipping it, so a reset after the incident settles leaves a deliverable record for startup replay. As a backstop for transitions that settle without that staging, startup replay also gives an outcome-less record whose incident is gone, skipped or exhausted the interrupted apology. A failed incident is left alone, since the platform may still re-run it. Covered by "stores the reply outcome before recovery reports completion" and "apologizes on start for a pending reply whose incident is gone".
| const stub = (await this.host.subAgent( | ||
| agentClass, | ||
| target.name | ||
| )) as unknown as MessengerDeliveryTarget; | ||
| )) as unknown as Required<MessengerThinkTarget>; | ||
| // A live thread cannot cross the sub-agent RPC boundary, so the stub's | ||
| // `bindActiveDeliverySurface` must not be offered to delivery. | ||
| return { | ||
| cancelChat: (requestId, reason) => stub.cancelChat(requestId, reason), | ||
| chat: (userMessage, callback) => stub.chat(userMessage, callback), | ||
| chatWithMessengerContext: (userMessage, callback, context) => | ||
| stub.chatWithMessengerContext(userMessage, callback, context) | ||
| }; |
There was a problem hiding this comment.
Thanks. Yes, that is intended: the live ThreadImpl stays on the host side, and the sub-agent only receives the chat, cancellation and messenger-context calls.
92b4992 to
4008b23
Compare
Co-authored-by: Cursor <cursoragent@cursor.com>
…ptions and the visible limit Co-authored-by: Cursor <cursoragent@cursor.com>
…se text for a blank recovered reply Co-authored-by: Cursor <cursoragent@cursor.com>
… recovery Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
…ident settles Co-authored-by: Cursor <cursoragent@cursor.com>
…ed key Co-authored-by: Cursor <cursoragent@cursor.com>
4dc50f8 to
e84e6b0
Compare
Fixes #2106.
When a messenger turn stalls or hits a transient stream error and bounded chat recovery takes over, the messenger posted the interruption apology and dropped the reply. Recovery then finished the answer in the Think transcript, but nothing sent it to the thread.
Now, before recovery is scheduled for a messenger turn, Think writes a pending-delivery record with the messenger id, thread id and the partial text already posted. The streaming callback is told (
onInterrupted({ deliversRecoveredReply: true })) so the messenger skips its apology. When recovery settles:The agent that owns the messenger runtime delivers the post. That is the host, reached from a sub-agent through
parentAgent(...)._cf_deliverRecoveredMessengerReply. The record is deleted after delivery and replayed fromonStart, so delivery is at-least-once.Deviation from the agreed design. The record lives on the agent that runs recovery (the sub-agent for thread conversations), not on the host. The sub-agent writes it before scheduling recovery, so a fast recovery cannot settle before the host has registered anything.
Limitation. This covers interruptions inside the isolate on the RPC path. If the Durable Object restarts mid-turn, host fiber recovery still posts the apology as before.
Also fixed:
resolveTargetnow wraps the sub-agent stub and exposes only the methods the messenger calls. Before,bindActiveDeliverySurfacesent a chat-sdkThreadImplover RPC and threw aDataCloneError.messengers.test.tscovers self, thread and exhausted recovery end to end, plus a delivery unit test. The self and thread tests fail without the fix.Delivery edge cases. Repeated interruptions rely on #2343, which leaves the incident to the next scheduled attempt instead of emitting
chat:recovery:failedearly. If the stream post itself was rejected, the whole streamed text is posted before the recovered reply. A recovered reply with no text posts the empty-response text, unless a partial was already posted. Text past the messenger'svisibleSoftLimitthat was buffered at the interruption is now posted before the recovered reply.