Repository navigation
fix(agents): preserve optimistic send dropped on reconnect (#1983) #2039
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
5934de3
1ff94b8
7aca398
d220c61
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| --- | ||
| "agents": patch | ||
| --- | ||
|
|
||
| Fix `useAgentChat` dropping an in-flight optimistic send on reconnect (#1983). A message buffered by PartySocket while the socket was down was missing from the transcript the server sends on connect, so the whole-array `setMessages` erased it from the UI until the turn completed. The hook now keeps such buffered sends when a snapshot is marked `connect: true` (the new optional field on `cf_agent_chat_messages`). Every other snapshot stays authoritative, so a server rollback of a rejected send (for example under `messageConcurrency: "drop"`) still removes it. | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| --- | ||
| "@cloudflare/think": patch | ||
| --- | ||
|
|
||
| Mark the transcript Think sends to a newly connected client with `connect: true`, so `useAgentChat` keeps a message the user sent while disconnected instead of erasing it on reconnect (#1983). |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -651,6 +651,25 @@ function prependMissingHydratedMessages<ChatMessage extends UIMessage>( | |
| return [...missingHydratedMessages, ...currentMessages]; | ||
| } | ||
|
|
||
| // Re-append the specific buffered sends a connect transcript omits, restoring | ||
| // only the tracked ids (in local order). | ||
| 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]; | ||
|
devin-ai-integration[bot] marked this conversation as resolved.
Comment on lines
+656
to
+670
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Intended. The only IDs tracked are the user message of a buffered |
||
| } | ||
|
|
||
| /** | ||
| * React hook for building AI chat interfaces using an Agent | ||
| * @param options Chat options including the agent connection | ||
|
|
@@ -1047,6 +1066,10 @@ export function useAgentChat< | |
| * Used by onAgentMessage to skip messages already handled by the transport. | ||
| */ | ||
| const localRequestIdsRef = useRef<Set<string>>(new Set()); | ||
| // Ids of sends the transport buffered (socket not OPEN when `send()` ran, so | ||
| // the server hasn't seen them). Consumed by the next transcript snapshot to | ||
| // rescue the optimistic messages from the reconnect replay. | ||
| const pendingBufferedSendIdsRef = useRef<Set<string>>(new Set()); | ||
| const pendingReplayResumeRequestIdsRef = useRef<Set<string>>(new Set()); | ||
| const replayHydratedAssistantMessageIdsRef = useRef<Set<string>>(new Set()); | ||
| /** | ||
|
|
@@ -1079,6 +1102,11 @@ export function useAgentChat< | |
| agent: agentRef.current, | ||
| activeRequestIds: localRequestIdsRef.current, | ||
| cancelOnClientAbort, | ||
| onRequestBuffered: (messageId) => { | ||
| if (messageId) { | ||
| pendingBufferedSendIdsRef.current.add(messageId); | ||
| } | ||
| }, | ||
| prepareBody: async ({ messages: msgs, trigger, messageId }) => { | ||
| // Start with the top-level body option (static or dynamic) | ||
| let extraBody: Record<string, unknown> = {}; | ||
|
|
@@ -1624,6 +1652,7 @@ export function useAgentChat< | |
| fallbackAckedResumeRequestIdsRef.current.clear(); | ||
| replayHydratedAssistantMessageIdsRef.current.clear(); | ||
| protectedStreamingAssistantRef.current = null; | ||
| pendingBufferedSendIdsRef.current.clear(); | ||
| customTransport.appliedChunks.clear(); | ||
| turnErrorsRef.current.clear(); | ||
| pendingTurnEndsRef.current = []; | ||
|
|
@@ -1636,6 +1665,9 @@ export function useAgentChat< | |
|
|
||
| const sendMessageWithStreamingProtection: typeof sendMessage = useCallback( | ||
| async (message, options) => { | ||
| // Whether this send was buffered is detected at the real send site in the | ||
| // transport (via `onRequestBuffered`), not here — the socket can drop | ||
| // during the async request preparation between now and the actual send(). | ||
| const request = sendMessage(message, options); | ||
|
|
||
| if ( | ||
|
|
@@ -2126,6 +2158,14 @@ export function useAgentChat< | |
| break; | ||
|
|
||
| case MessageType.CF_AGENT_CHAT_MESSAGES: { | ||
| // One-shot, consumed outside the updater so a re-invoked updater | ||
| // sees the same ids. Only a connect transcript predates the buffered | ||
| // sends; any other snapshot (e.g. a `drop` rollback after a | ||
| // mid-stream reconnect) has already seen them and wins. | ||
| const bufferedSendIds = data.connect | ||
| ? new Set(pendingBufferedSendIdsRef.current) | ||
| : new Set<string>(); | ||
| pendingBufferedSendIdsRef.current.clear(); | ||
|
Comment on lines
+2161
to
+2168
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟡 Later snapshots erase pending sends If another transcript arrives before a buffered submit is persisted, Learn moreThe hook consumes every buffered message ID on the first Example: A disconnected tab buffers user message 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.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. |
||
| setMessages((currentMessages: ChatMessage[]) => { | ||
| let next = preserveProtectedStreamingAssistant( | ||
| data.messages, | ||
|
|
@@ -2165,7 +2205,9 @@ export function useAgentChat< | |
| next = observed.accumulator.mergeInto(next) as ChatMessage[]; | ||
| } | ||
| } | ||
| return next; | ||
| // Rescue sends the transport buffered while the socket was down | ||
| // that the reconnect replay would otherwise drop. | ||
| return restoreBufferedSends(next, currentMessages, bufferedSendIds); | ||
| }); | ||
| break; | ||
| } | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You're right:
AIChatAgentnever sends a transcript on connect, so it doesn't have this bug. In d220c61 the rescue is gated on a newconnect: truemarker that only Think's connect transcript sets, so ai-chat keeps main's behaviour. The changeset now namesagents(hook) and@cloudflare/think(marker) only.