Skip to content

fix(chat): settle turns correctly across replay, resume, and observers - #2383

Merged
threepointone merged 3 commits into
mainfrom
fix/chat-review-findings
Sep 27, 2026
Merged

threepointone merged 3 commits into
mainfrom
fix/chat-review-findings

Conversation

@threepointone

@threepointone threepointone commented Sep 27, 2026 •

Copy link
Copy Markdown
Contributor

Follow-ups from a review of recently landed chat protocol PRs (#2378, #2365, #2361, #2348, #2344, #2334, #2023, #1838). Each fix has a regression test that fails on main.

Wire protocol (agents, ai-chat, think)

  • Replayed terminals carry outcome. A stream's close outcome is recorded in its metadata and echoed on replayed and bare-done terminals. Orphaned streams report aborted, and a close that schedules recovery reports recovering (ai-chat and Think). Outcomes and origin ids survive cutover/reclaim deleting the row (last 32 kept in memory).
  • No early done during the persist window. A resume ACK that lands while ai-chat holds a request's terminal frames replays the finished stream without a bare done; the real done follows the persisted transcript (regression of fix(ai-chat): send the done frame after the transcript is persisted #2334).
  • "No response was generated" done now reaches every connection, including the originating tab, with outcome: "completed". Previously the sending tab stayed submitted forever.

useAgentChat

Tests

  • agents: resumable-stream-settlement, resume-handshake, broadcast-state
  • ai-chat: terminal-frame-order, a new "useAgentChat terminal frames" block in use-agent-chat.test.tsx
  • think: origin-message-ids asserts the stored recovering outcome

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.

Likely conflicts with the stream-recovery review PR (ai-chat index.ts and Think think.ts recovery paths); whichever lands second needs a rebase.


Devin Review

threepointone and others added 2 commits September 27, 2026 16:40
Wire protocol (agents, ai-chat):
- Record a stream's close outcome in its metadata and echo it on replayed
  and bare-done terminals; orphaned streams report "aborted" and a
  recovery-scheduled close reports "recovering". Outcomes and origin ids
  survive cutover/reclaim deleting the row.
- A resume ACK that lands while ai-chat holds the terminal frames replays
  the finished stream without an early bare done.
- The "No response" done reaches every connection, including the origin,
  with an outcome.

useAgentChat:
- Observers settle on a done for a request other than the observed one.
- A recovering done keeps isRecovering and holds onToolCall until the
  successor turn.
- onTurnEnd for the tab's own request fires after status settles, so
  sendMessage works inside it.
- With resume:false, probe on reconnect so a held onToolCall is released.
- Live chunks skip the merge once divergence is detected.
- Cap remembered turn errors and clear them in clearHistory.

Co-authored-by: Cursor <cursoragent@cursor.com>
…play

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

changeset-bot Bot commented Sep 27, 2026 •

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: c86b5cf

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

This PR includes changesets to release 4 packages
Name Type
agents Patch
@cloudflare/ai-chat Patch
@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: 1 entry point grew

Entry point Exports Largest gzip change Size now
🟡 agents/chat 1 resized +257 B (+4.68%) 5.6 KiB
Changed exports (1)
Import Gzip change Size now
🟡 agents/chat#ResumableStream +257 B (+4.68%) 5.6 KiB
How this works

Each runtime export is bundled on its own, minified, and gzipped. Changes smaller than 100 B, or smaller than 1% and 1 KiB, are ignored. Growth over 10% or 5 KiB is marked 🔴. This report is informational and does not fail CI. The workflow artifact contains every measurement.

Compared dbf170cf → c86b5cf0 · workflow run · reported by agent-think[bot]

@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 4 potential issues.

Devin Review

Comment on lines +2560 to +2568
if (idleProbeNeeded) {
idleProbeNeeded = false;
idleProbeId = nanoid();
agentRef.current.send(
JSON.stringify({
type: MessageType.CF_AGENT_STREAM_RESUME_REQUEST,
probeId: idleProbeId
})
);

@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.

🔴 Active resume offers release tools early

When resume: false receives only a probe-correlated offer for an active stream, setUnresolvedObservedRequestId clears the old hold. The server excludes unacknowledged connections from live chunks, so onToolCall runs before that stream ends.

Learn more

A reconnect with resume: false sends an idle probe when a prior turn's terminal was missed. An active stream can answer that probe with STREAM_RESUMING carrying a probeId; notifyStreamResuming then puts this connection in pendingResumeConnections, excluding it from live frames until an ACK. The client intentionally does not ACK with resume: false. This branch holds only offers without probeId, while the preceding state update clears the previous hold. An assistant tool part in the current transcript can therefore be treated as client-owned before the server finishes it.

Example: Tab A misses request A's done, then reconnects while request B runs a server-side tool. Its proactive offer is missed; its probe receives {id: "B", probeId: "probe"}. Request A's hold clears, B gets no hold, and onToolCall fires for B's unfinished tool before B's done arrives.

Recommended fix: Distinguish active stream offers from terminal replay offers in the handshake, or arrange for resume: false to settle a probe offer without exposing tool calls until the offered turn is known to have ended. Cover the probe-correlated active-offer path with a test.

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.

Partly valid — fixed in c86b5cf. Both hosts already enroll every reconnecting socket as pending-resume in onConnect, and the list is normally cleared before done, so the probe itself doesn't strand it. Two real bugs underneath: Think's chat() RPC path never cleared pending-resume connections (now cleared on finish/complete/error, like ai-chat), and with resume: false an offer for a newer request never released a held older one (now released). Didn't ACK the offer, since resume: false is documented as no automatic resumption.

Comment thread packages/agents/src/chat/react.tsx Outdated
Comment thread packages/agents/src/chat/resume-handshake.ts Outdated
Comment thread packages/agents/src/chat/react.tsx
…olCall gates

- ResumableStream replays the request's last closed stream (finished,
  recovering, or errored) to a resume ACK that lands while ai-chat holds
  the terminal frames, instead of only the pending-cutover stream.
- useAgentChat tracks the recovering request id: replays and the
  recovering request's own frames no longer release the hold; live frames
  for a later request, recovering:false, or an idle reconnect probe with
  no recovery reported on connect do.
- With resume:false the reconnect probe also runs for a recovering hold,
  and an offer for another request releases a stale held turn; a
  proactive offer holds tool calls until that stream's terminal frame.
- Think clears pending resume connections whenever a chat stream ends, so
  a connection that never ACKs still gets the terminal frame on chat()
  RPC turns.

Co-authored-by: Cursor <cursoragent@cursor.com>
@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@2383

@cloudflare/ai-chat

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

@cloudflare/codemode

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

hono-agents

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

@cloudflare/shell

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

@cloudflare/think

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

@cloudflare/voice

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

@cloudflare/worker-bundler

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

commit: c86b5cf

@threepointone
threepointone merged commit efbb005 into main Sep 27, 2026
21 of 23 checks passed
@threepointone
threepointone deleted the fix/chat-review-findings branch September 27, 2026 17:33
@github-actions github-actions Bot mentioned this pull request Sep 27, 2026
tombeckenham added a commit to tombeckenham/agents-tom that referenced this pull request Oct 7, 2026
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.
tombeckenham added a commit to tombeckenham/agents-tom that referenced this pull request Oct 7, 2026
…ge ids and per-event seq

Ports the server half of upstream cloudflare#2334, cloudflare#2365, cloudflare#2378, cloudflare#2383 and cloudflare#2348 into
AGUIChatAgent. An in-band RUN_ERROR now marks its stream errored when the body
ends (a settled stream cannot be re-marked, which had broken cloudflare#1575's errored
replay) and rides an error frame the client transport still delivers as an event.
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