Repository navigation
fix(agents): preserve optimistic send dropped on reconnect (#1983) - #2039
Conversation
🦋 Changeset detectedLatest commit: d220c61 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
@cloudflare/ai-chat
@cloudflare/codemode
hono-agents
@cloudflare/shell
@cloudflare/think
@cloudflare/voice
@cloudflare/worker-bundler
commit: |
| --- | ||
| "agents": patch | ||
| --- | ||
|
|
There was a problem hiding this comment.
Are you sure that @cloudflare/ai-chat is actually affected by this bug? I’m not sure the server behanves the same as Think in this respect.
There was a problem hiding this comment.
You're right: AIChatAgent never sends a transcript on connect, so it doesn't have this bug. In d220c61 the rescue is gated on a new connect: true marker that only Think's connect transcript sets, so ai-chat keeps main's behaviour. The changeset now names agents (hook) and @cloudflare/think (marker) only.
|
|
||
| const sendMessageWithStreamingProtection: typeof sendMessage = useCallback( | ||
| async (message, options) => { | ||
| // Non-OPEN socket => PartySocket buffers this send; flag it for the |
There was a problem hiding this comment.
I’m not sure inspecting the state of the socket is necessarily safe, because the socket could disconnect later (but still before the messsage is actually sent) in which case the messgae would buffer but we wouldn’t know about it. I think that PartySocket’s send() returns wheter the message was sent immediately, so maybe you can use that result instead of checking readyState?
| function preserveTrailingLocalUserSends<ChatMessage extends UIMessage>( | ||
| snapshot: ChatMessage[], | ||
| local: readonly ChatMessage[] | ||
| ): ChatMessage[] { |
There was a problem hiding this comment.
Is there any chance that messagesRef.current might contain messages that the server deliberately omitted (rejected?). If so, we’d have to track the actual message IDs that were buffered and only restore those, rather than assuming that everything that’s missing from the snapshot should be restored.
| cleanup(); | ||
| }); | ||
|
|
||
| it("preserves a buffered optimistic send when the reconnect snapshot omits it", async () => { |
There was a problem hiding this comment.
In line with my unsafe comment earlier, could we extend coverage to test socket changes during prepareSendMessagesRequest?
- start OPEN, close before the actual send()
- start CLOSED, reopen before the actual send()
There was a problem hiding this comment.
Both are covered: "preserves a send that buffers because the socket drops during request prep" (OPEN at call time, closed before send()) and "does NOT preserve a send delivered after the socket reopens during request prep" (CLOSED at call time, reopened before send()). Detection is now at the real send() site in the transport, so both pass.
|
+1 for the optimistic-send fix, with an additional queued-send case to cover. Source recheck on 2026-09-15: latest released agents 0.23.0 / ai-chat 0.12.0 / Think 0.18.0 and main Our UI presents each Send immediately, before a durable submission is promoted into the transcript. A full sync snapshot arriving in that interval can omit the sender's own local user message; it disappears until the promotion broadcast. We currently bridge these attempts by user-message identity outside Please cover both the reconnect-buffered case in this PR and a send that has reached the server but is still awaiting durable queue promotion when a sync snapshot arrives. The latter may require a separate follow-up rather than extending this PR's disconnected-only preservation rule. Preserve genuine server rollback/drop/clear semantics: do not unconditionally merge every stale local user message. An acknowledgement/sync contract exposing request-to-user-message identity would also let callers reconcile deterministically. This is current source-path validation and our integration use case, not a fresh end-to-end reproduction against the unmerged PR. |
Co-authored-by: Cursor <cursoragent@cursor.com> # Conflicts: # packages/agents/src/chat/react.tsx
|
✅ agents import sizes: no significant changes ( |
| // One-shot, consumed outside the updater so a re-invoked updater | ||
| // sees the same ids. | ||
| const bufferedSendIds = new Set(pendingBufferedSendIdsRef.current); | ||
| pendingBufferedSendIdsRef.current.clear(); |
There was a problem hiding this comment.
🟡 Later snapshots erase pending sends
If another transcript arrives before a buffered submit is persisted, pendingBufferedSendIdsRef has already lost its ID. That snapshot erases the optimistic message again, despite the submit still being in flight.
Learn more
The hook consumes every buffered message ID on the first CF_AGENT_CHAT_MESSAGES frame, regardless of whether the server has accepted that request. A server can send further transcript snapshots while the request is pending, such as when another tab updates the conversation. The next snapshot can still omit the buffered message. With its ID gone, the replacement drops the restored optimistic message.
Example: A disconnected tab buffers user message u2. On reconnect, the first snapshot omits u2, so the hook restores it. Before the server persists u2, another tab's update produces a second snapshot omitting u2; the hook now replaces the transcript and loses u2 again.
Recommended fix: Keep pending IDs until the associated request is acknowledged, rejected, or canceled, rather than consuming them on the first snapshot. Correlate them with request IDs so later snapshots can distinguish legitimate rollbacks from still-pending sends.
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
Leaving as is. After Think's connect transcript, the next frame it handles on that connection is the buffered submit, and Think saves an accepted submit before it enters the turn queue, so later snapshots include it. A snapshot that omits it would need another event processed in between. That window is no wider than for any send made while connected, and keeping the ids longer would reopen the rollback case above.
…ranscript (cloudflare#1983) Co-authored-by: Cursor <cursoragent@cursor.com>
| function restoreBufferedSends<ChatMessage extends UIMessage>( | ||
| snapshot: ChatMessage[], | ||
| local: readonly ChatMessage[], | ||
| bufferedIds: ReadonlySet<string> | ||
| ): ChatMessage[] { | ||
| if (bufferedIds.size === 0) { | ||
| return snapshot; | ||
| } | ||
|
|
||
| const snapshotIds = new Set(snapshot.map((message) => message.id)); | ||
| const restored = local.filter( | ||
| (message) => bufferedIds.has(message.id) && !snapshotIds.has(message.id) | ||
| ); | ||
|
|
||
| return restored.length === 0 ? snapshot : [...snapshot, ...restored]; |
There was a problem hiding this comment.
There was a problem hiding this comment.
Intended. The only IDs tracked are the user message of a buffered submit-message (onRequestBuffered passes the last user message's id), so in practice only user sends are restored. They're appended in local order after the connect transcript, which is where they belong because the server hasn't seen them yet. The PR description no longer says "trailing".
…port - appliedChunks ledger: a continuation replayed after a reconnect skips the frames this client already applied, decided as each frame arrives and applied to the projected chunks so the projector still sees the whole run (cloudflare#1951, upstream cloudflare#2348) - a socket close before the terminal frame marks the stream interrupted, and the AI SDK transport ends it with an error chunk behind the unread chunks (cloudflare#2013, upstream cloudflare#2019) - activeServerTurnId getter for the hook's held-turn tracking (cloudflare#2361) - onBuffered / onRequestBuffered when the request frame was buffered, and a close while the body is still being prepared no longer drops the request (cloudflare#1983, upstream cloudflare#2039) - the chunk stream keeps pulling past events that render nothing, which stalled a waiting reader on RUN_STARTED followed by STEP_STARTED The two stack tests that pinned a clean close mid-stream now expect the error chunk, as upstream changed its own in cloudflare#2019.
Brings react-agui.tsx level with the legacy hook in agents/chat/react, which received these through the upstream merge: - cloudflare#2344 the server snapshot heals a diverged observed assistant - cloudflare#2348 an observer skips replayed continuation frames it already applied - cloudflare#2361 onToolCall fires only after the stream ends - cloudflare#2378 onTurnEnd fires once per ended request - cloudflare#2383 turns settle across replay, resume and observers (held and recovering requests, the resume:false idle probe) - cloudflare#2496 streaming protection is released when the socket closes - cloudflare#2394 follow the socket when the agent name changes - cloudflare#2403 the tool-result cleanup no longer dispatches per chunk - cloudflare#2039 keep an optimistic send the reconnect transcript omits The last three had no failing test: their upstream tests are ported into react-tests (the address-change one against a fake of useAgent's pending socket, since this package has no test worker) and failed before the fix. restoreBufferedSends, MAX_REMEMBERED_ENDED_TURNS and isSocketAddressPending are exported from agents/chat/react so the two hooks share them.
Fixes #1983:
useAgentChatdropped an in-flight optimistic send on reconnect. When a message was sent while the socket was down, PartySocket buffered the frame, but the transcript Think sends on connect (cf_agent_chat_messages) didn't include it yet, so the whole-arraysetMessageserased it from the UI until the turn completed.How it works:
send()returnsfalse(the socket wasn't OPEN at the real send site, including when it drops during async request preparation).connect: true, a new optional field oncf_agent_chat_messages.connect: true, the hook re-appends the buffered user messages it omits. Any other snapshot consumes the buffered IDs without restoring them, so a server rollback (for examplemessageConcurrency: "drop", including after a mid-stream reconnect) still wins.AIChatAgentsends no transcript on connect, so it wasn't affected and its behaviour is unchanged. An older Think server without the marker also gets main's behaviour.