Persist settled tool results durably so recovery never re-runs completed tool calls#1621
Merged
Merged
Conversation
🦋 Changeset detectedLatest commit: 7ed073d The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
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 |
…uns them
Stream chunks are buffered in memory and batch-flushed to SQLite every ~10
chunks. The WebSocket chat path did not force a flush on settled tool results,
so an isolate eviction (deploy) before the next batch flush lost them: recovery
rebuilt the partial assistant message from SQLite without those tool calls and
the model re-ran the already-completed (non-idempotent) tool call — duplicate
INSERTs, a whole message's tool calls rolling back, not just the interrupted
step.
The sub-agent RPC streaming path (`_streamResultToRpcCallback`) already flushed
recoverable content; the main WebSocket `_streamResult` (used by chat turns and
`continueLastTurn` recoveries) did not. This factors a shared `_storeChunkDurably`
helper used by both paths and flushes **immediately** on `tool-output-available`
/ `tool-output-error` (frequent text/reasoning/tool-input streaming stays
throttled to avoid write amplification). Net: recovery loses at most the single
in-flight step, even when multiple evictions hit one turn — matching the
tool-call-level guarantee agent-tools already had.
Test: think-session.test.ts "flushes a settled tool result to durable storage
immediately" — a settled tool result forces a flush even when throttled text is
still buffered; verified to fail without the tool-output special-case. Full
think unit suite: 433.
Note: the think kill/restart e2e currently boot-times-out on this machine
("Wrangler did not start in time") identically on clean main — environmental,
not from this change; CI runs it cleanly.
Co-authored-by: Cursor <cursoragent@cursor.com>
threepointone
force-pushed
the
fix/persist-settled-tool-results
branch
from
May 30, 2026 07:59
190ed08 to
12869ef
Compare
…_disabled skips Closes two remaining "frozen turn" hydration gaps from the terminal-status work (#1619), folded in alongside the tool-result durability fix: - The outer catch in `_handleChatRequest` broadcasts an error for failures that happen before the stream starts (e.g. message reconciliation/persist), but never recorded the terminal status. A client disconnected at that moment missed the transient broadcast and stayed frozen on reconnect. It now records terminal "error" before broadcasting, so `_buildIdleConnectMessages` replays it. - A recovery skip from `onChatRecovery` returning `{ continue: false }` only marked the submission interrupted (which does not touch the WS transcript), so a WS chat client got no durable terminal signal at all. Unlike a benign `conversation_changed` skip (a newer turn already owns the UI), disabling recovery abandons the turn with no superseding turn, so it now records terminal status and broadcasts an error like exhaustion does. `conversation_changed` / `not_recoverable` skips remain silent. Tests: a pre-stream failure and a `{ continue: false }` skip both surface a terminal error to a reconnecting client via the idle-connect replay. Co-authored-by: Cursor <cursoragent@cursor.com>
agents
@cloudflare/ai-chat
@cloudflare/codemode
hono-agents
@cloudflare/shell
@cloudflare/think
@cloudflare/voice
@cloudflare/worker-bundler
commit: |
…ination failures
The e2e suite's assertions pass, but `nx run @cloudflare/think:test:e2e`
intermittently exited non-zero with "Timeout terminating forks worker for
test files .../assistant-e2e.test.ts" / "Worker exited unexpectedly". The
vitest forks-pool worker occasionally could not terminate within the teardown
window after a file that spawned `wrangler dev`.
Root cause: leftover open handles keeping the worker's event loop alive past
the pool's termination timeout under load. `assistant-e2e`'s `killProcess` left
an uncleared `setTimeout(resolve, 3000)` fallback (the sibling chat-recovery
file already cleared it), and it keeps a single wrangler alive across all tests
so undici keep-alive sockets linger until the final afterAll kill.
Fixes (harness only — no published-package behavior change):
- Clear the fallback timer in `assistant-e2e`'s `killProcess` once the child
exits, matching `chat-recovery`.
- Raise the e2e `teardownTimeout` to 60s so a slow workerd/socket teardown
under load doesn't fail an otherwise-green run.
- Raise the `waitForReady` boot budget to 60s in both files to absorb slow
wrangler boots under load ("Wrangler did not start in time").
Verified: full suite runs green repeatedly after the change.
Co-authored-by: Cursor <cursoragent@cursor.com>
Contributor
Author
|
Folded in an e2e harness stability fix ( Symptom: Diagnosis: not a logic failure and not introduced by this PR's code — the e2e harness (untouched since #1611/#1601) leaves open handles that keep the vitest forks-pool worker alive past the teardown window under load. Reproduced ~1/4 full-suite runs on this branch; passed in isolation and on a pre-#1619 commit. The error always named Fix (harness only, no published-package change):
Verified green on repeated full-suite runs after the change. |
…esses Running the SDK e2e suites surfaced a second class of flaky failure beyond the think teardown timeout: an unhandled `setTypeOfService EINVAL` from undici while a probe fetch/WebSocket connects to a server that is mid-SIGKILL/restart. It comes from happy-eyeballs (autoSelectFamily) dual-stack racing — the abandoned socket throws a connect-time option error that surfaces as an unhandled error and fails an otherwise-green chaos run (all assertions pass). It reproduced reliably in ai-chat (2/2 runs). Fixes (harness only — no published-package change): - Disable happy-eyeballs via `setDefaultAutoSelectFamily(false)` in the think, ai-chat, and agents recovery harnesses. Verified: ai-chat now green over 3 consecutive runs (was failing every run). - ai-chat: drain the probe response body (`res.body?.cancel()`) like the think and agents harnesses already do, so probe fetches don't leak sockets across kill/restart churn; raise the boot budget to 60s. - Add `teardownTimeout: 60_000` + `fileParallelism: false` to the ai-chat and agents e2e configs (matching think) so a slow workerd/socket teardown under load doesn't fail a green run. Verified green locally: think (9/9), ai-chat (2/2 ×3), agents (6/6). Out of scope (environmental, not code): the codemode and agents-as-tools playwright e2e fail at webServer startup because the account's `/workers/subdomain/edge-preview` remote-proxy API call fails — required by codemode's `worker_loaders` and agents-as-tools' vite-dev remote AI. The vitest-based SDK suites only use the remote AI binding and are unaffected. Co-authored-by: Cursor <cursoragent@cursor.com>
Merged
threepointone
added a commit
that referenced
this pull request
May 31, 2026
…mpaction under deploy churn (#1623) * fix(think,ai-chat): stop recovery falsely erroring a turn under repeated mid-turn deploys Under repeated real `wrangler deploy`s mid-turn, chat recovery runs a chain of continuations. Three bugs combined to mark a turn's durable submission `error` even when it actually completed every step (validated end-to-end with the deploy-churn harness + a recovery trace): 1. Lost ownership: the submission link (`recoveredRequestId`) was derived from each continuation's own fresh requestId, so chained continuations dropped it and the continuation that finally completed the turn could not mark the submission `completed`. Now keyed off the stable recovery root and threaded through the whole chain. 2. Stale-continuation clobber: a superseded continuation tripped the `conversation_changed` guard because the leaf had advanced via recovery's own forward progress (a new assistant message), not a new user turn, and overwrote the still-running submission to `error`. Now a superseded continuation skips benignly; only a genuinely newer user turn marks the submission `skipped` (never `error`). 3. Premature stable_timeout: a timeout while waiting for the isolate to settle (common while a deploy is in flight) failed the turn terminally at attempt 1. Now it reschedules within the `maxAttempts` budget. `@cloudflare/ai-chat` shares the recovery machinery but has no durable-submission layer, so it receives only the stable_timeout reschedule fix (mirrored in both `_chatRecoveryContinue` and `_chatRecoveryRetry`). Tests: 7 deterministic unit tests (5 in think, 2 in ai-chat) covering chained ownership, benign superseded skip, newer-user-turn -> skipped, stable_timeout reschedule within budget, and exhaustion. think 441 / ai-chat 475 green. Co-authored-by: Cursor <cursoragent@cursor.com> * test(deploy-churn): add tool-result rollback harness for real-deploy recovery testing Extends the deploy-churn example to drive a long, tool-using session via HTTP (no browser) against a REAL model (Workers AI or Anthropic) while firing real `wrangler deploy`s mid-turn, and measures whether completed tool calls re-run or the durable submission is wrongly errored. - `recordStep` tool: one ledger row per execution, so a re-run of a completed step shows up as a duplicate index (the "rollback" signal). - provider switch (workers-ai | anthropic) stored in a SQL config table so getModel()/getTools() observe it on a fresh post-deploy isolate. - `/drive/start|status|reset` HTTP routes driving `submitMessages` + ledger. - `scripts/deploy-rollback.ts` orchestrator: real deploys during the session, then a CLEAN / MINIMAL / ROLLBACK verdict plus submission status. This reproduced and validated both the #1621 tool-result durability fix and the recovery submission-status fix in the preceding commit. Co-authored-by: Cursor <cursoragent@cursor.com> * fix(think,ai-chat): reschedule stable-timeout recovery on a fresh schedule row The stable-timeout retry added in the previous commit used `schedule(..., { idempotent: true })` from INSIDE the currently-executing one-shot `_chatRecoveryContinue`/`_chatRecoveryRetry` schedule row. Because `alarm()` deletes that one-shot row only AFTER the callback returns, the idempotent reschedule deduped onto the still-present executing row and was then deleted with it — so the retry silently never fired and the turn STALLED (incident frozen at `stable_timeout_retry`, submission stuck `running`). A real-deploy repro with a 12s tool reproduced this: the turn stalled at 4/8. With the reschedule switched to a fresh (non-idempotent) delayed row it now completes 8/8 under the same churn. The unit tests previously passed only because they drive the callback directly (no executing row to dedup against). They now pre-insert a matching schedule row to simulate the executing one-shot, and assert the reschedule creates a NEW row. Co-authored-by: Cursor <cursoragent@cursor.com> * test(think,deploy-churn): rollback-depth + task-amplification repros under churn Adds rigorous reproductions used to characterize recovery behavior under deploy churn (all show the framework is BOUNDED — re-runs at most the in-flight step, no deep rollback, no task amplification): - think e2e `tool-rollback.test.ts` + `ThinkToolRollbackE2EAgent`: a long deterministic tool loop with a non-idempotent ledger, rapid SIGKILL/restart; measures rollback DEPTH (re-runs vs evictions). - think e2e `task-amplification.test.ts` + `ThinkTaskParentE2EAgent`: a parent `runTask` tool driving a child agent; verifies an eviction mid-task does NOT re-run the whole child turn. - deploy-churn: configurable per-tool delay (`--delay-ms` / `delayMs`) so a real ~33s `wrangler deploy` lands DURING a tool execution (code-update reset mid-tool). This repro surfaced the stable-timeout reschedule stall fixed in the previous commit. Co-authored-by: Cursor <cursoragent@cursor.com> * test(think): assert re-reconstructing an interrupted stream is idempotent Pins the property that protects against the "disappearing/duplicated completed tool calls" failure mode under churn: re-running recovery on the same interrupted stream (e.g. a second eviction during the persist window) replaces the reconstructed assistant message by its stable id (taken from the stream's `start` chunk) rather than appending a duplicate or losing it. Verified the content is preserved across two recovery passes. Co-authored-by: Cursor <cursoragent@cursor.com> * fix(think): preserve interrupted tool calls as errored instead of deleting them `_repairToolTranscriptParts` deleted any tool call with no recorded output before the next turn. For a tool interrupted mid-execution (deploy/eviction) or an `ask_user` answered by the next message, that: - removed the call from the durable + broadcast transcript (it visibly "disappeared" — the exact symptom in the customer's deploy-churn video), and - let the model silently re-run it, duplicating non-idempotent side effects. Now the orphan is flipped to `state: "output-error"` with an explanatory message: the record is preserved, the model is told the tool errored (so it doesn't blindly re-run it), and conversion still gets a valid tool-result so the provider doesn't 400 with AI_MissingToolResultsError. Stringified `input`s are normalized in the same pass. Blast radius was a single test (which asserted the old deletion); it now asserts preservation. Full think suite green (442). Co-authored-by: Cursor <cursoragent@cursor.com> * fix(think): ignoreIncompleteToolCalls at convert as a last-line backstop After `_repairTranscriptForProvider` heals orphan tool calls (preserving them as errored results), pass `ignoreIncompleteToolCalls: true` to `convertToModelMessages` so any incomplete tool call that still slips through (compaction edges, addToolOutput races, unrecognized part shapes) is dropped at conversion instead of throwing AI_MissingToolResultsError and wedging the turn. No-op in the common path (the repair runs first); verified no test churn. Co-authored-by: Cursor <cursoragent@cursor.com> * fix(agents): flow Session tokenCounter into compaction boundary logic (#1593) A tokenCounter set on Session.compactAfter() only drove the fire/no-fire trigger; createCompactFunction's tail-budget boundary still used the chars/4 heuristic. On tool-heavy histories that under-counts ~4-5x, so the tail budget covered the whole history and compaction fired every turn but returned null — never shortening history (worse than not configuring it). The Session now passes its counter to the compaction function via a new CompactContext argument; createCompactFunction uses it for the tail walk when no explicit CompactOptions.tokenCounter was given. One counter on compactAfter() now drives both "should we compact?" and "what should we compact?". If the trigger fires but compaction still returns null, the Session logs a one-time warning instead of looping silently. CompactFunction gains an optional second context?: CompactContext arg (backward compatible). Session suite green (74). Co-authored-by: Cursor <cursoragent@cursor.com> * compaction: re-arm no-op warning on success + document per-message counter caveat - Reset the one-time auto-compaction no-op warning when compaction succeeds, so a later regression is surfaced again instead of staying silent. - Document that the Session counter flowed into createCompactFunction is invoked per-message: usage-only counters degrade the tail budget to minTailMessages, and the counter runs O(n) per compaction. Recommend an explicit per-message CompactOptions.tokenCounter for precise budgeting. Co-authored-by: Cursor <cursoragent@cursor.com> * chore: format changeset files with oxfmt * fix(think): give getScheduledChatRecoveryPayloadForTest a serializable return type A `Record<string, unknown>` return collapses to `never` across the Durable Object RPC stub boundary (Workers RPC drops `unknown`-valued records as non-serializable), so the chained-continuation test saw `payload` as `never` and failed typecheck. Return the concrete recovery-link fields instead. Co-authored-by: Cursor <cursoragent@cursor.com> * fix(think): default a missing tool input to {} during transcript repair Transcript repair already parsed stringified-JSON tool inputs, but a tool call with a missing or null `input` was left unrepaired — Anthropic rejects a `tool_use` block whose `input` is absent, so the turn 400s forever. A new `_normalizeToolInput` helper now also defaults a missing/null input to `{}` on both the orphan-healing and settled-part paths. Co-authored-by: Cursor <cursoragent@cursor.com> * feat(agents): structured retryable failure envelope for agentTool() agentTool() collapsed every non-completed sub-agent run to an opaque { ok: false, error: string }, so a parent agent could not tell a transient interruption (child reset/superseded by a deploy or parent recovery) apart from a terminal failure or an intentional cancellation — and would often parrot the interruption text to the user as final. Failures now return AgentToolFailure { ok: false, status, error, retryable }: interrupted -> retryable: true (and surfaces the interruption reason), while aborted and error -> retryable: false. Backward compatible for consumers reading ok/error; AgentToolFailure is exported from `agents`. Co-authored-by: Cursor <cursoragent@cursor.com> * feat(think): opt-in inactivity watchdog for the streaming read loop A model stream that parks without ever throwing (no chunk, no error, no `done`) left the chat read loop waiting forever — an infinite spinner with no terminal state. There was no detection for a silently hung turn. Add `chatStreamStallTimeoutMs` (default 0 = off): if no UI-message-stream chunk arrives within the window, the watchdog aborts the turn so the loop exits with a terminal stream error (routed through onChatError stage "stream") and emits a new `chat:stream:stalled` observability event. Applies to both the WebSocket turn loop and the chat()/sub-agent callback loop. The watchdog aborts the turn's signal (not a reader cancel) so the AI SDK pipeline tears down without writing to an already-cancelled readable; the abandoned read's rejection is pre-caught to avoid an unhandled rejection. It measures inter-chunk inactivity (which includes tool execution), so it must be set above the slowest expected model TTFT and tool latency. Co-authored-by: Cursor <cursoragent@cursor.com> * refactor(think): address PR #1623 review — dedup reschedule, retry-path test, sharper diagnostics - Extract the stable-timeout reschedule into a shared `_rescheduleRecoveryAfterStableTimeout` helper (mirrors @cloudflare/ai-chat), removing the two inlined near-identical copies in `_chatRecoveryRetry` / `_chatRecoveryContinue` so a future fix can't diverge between them. - Add a `_chatRecoveryRetry` stable-timeout reschedule test (the path previously only covered for `_chatRecoveryContinue`). - Surface when an incomplete tool call survives transcript repair and is about to be dropped by `ignoreIncompleteToolCalls` (warns + emits), so the backstop can't silently mask a repair gap. - Make the compaction no-op warning distinguish a per-message vs whole-prompt (usage) tokenCounter, since "configure a tokenCounter" was misleading when one was already configured. - Document the single-field `_activeChatRecoveryRootRequestId` serialization invariant (safe only because turns are serialized by the turn queue). Co-authored-by: Cursor <cursoragent@cursor.com> * docs: align with PR #1623 behavior changes - chat-agents: transcript repair now heals orphaned tool calls (preserved as errored results) and normalizes malformed/missing inputs — was "removing". - observability: add the new chat:stream:stalled event (agents:chat channel) and clarify chat:transcript:repaired counts (preserved-as-errored + backstop). - think README: document the opt-in chatStreamStallTimeoutMs inactivity watchdog. - agent-tools: document the AgentToolFailure shape and retryable semantics. - sessions: note the compactAfter tokenCounter now also drives the boundary walk (CompactContext), with the per-message/usage-counter caveat. Co-authored-by: Cursor <cursoragent@cursor.com> * docs(think): add chatStreamStallTimeoutMs to the docs-site config reference Mirrors the package README so the developers.cloudflare.com Think config table (the target of the observability chat:stream:stalled link) documents the new inactivity watchdog. Co-authored-by: Cursor <cursoragent@cursor.com> * fix(think): cancel the source stream when the stall-watchdog wrapper exits early The inactivity-watchdog generator wraps the model stream, but on an early consumer exit (a `break` on an in-band stream error, where the abort signal is NOT set) it never forwarded `.return()` to the source — leaking the wrapped ReadableStream that the old direct `for await` would have cancelled. Add a top-level finally that cancels the source on early termination, skipped after a watchdog stall (which already aborted the upstream, where a late cancel would make the AI SDK write to an already-cancelled readable). Tests: watchdog does not false-fire on a slow-but-steady stream (timer resets per chunk), and an in-band error under an armed watchdog terminates cleanly with no unhandled rejection. Co-authored-by: Cursor <cursoragent@cursor.com> * fix(think): key submission abandon-paths off the recovery root, not the continuation id _markRecoveredSubmissionInterrupted was called with the per-continuation `requestId` in both terminal abandon paths — recovery exhaustion (`_exhaustChatRecovery`) and `{ continue: false }`. Under chained continuations (recoveryRootRequestId !== requestId) the durable submission row still carries the root id, so the `WHERE request_id = ?` lookup missed it and left the submission stuck `running` forever instead of `error`. Thread the recovery root through both paths (storing it on the incident record for the exhaustion path). Regression test drives a disabled-recovery chained continuation and asserts the root submission flips to `error` (verified to fail without the fix). Co-authored-by: Cursor <cursoragent@cursor.com> * fix(think): treat output-error as settled in transcript repair _repairToolTranscriptParts' hasOutput check omitted `state === "output-error"`, so a tool part already healed to output-error (no `output` field) — or a tool that legitimately errored — re-entered the heal branch on every turn. That clobbered a real errorText with the generic "interrupted" message and emitted a spurious chat:transcript:repaired event + updateMessage write + broadcast each turn for the life of the conversation. Treat output-error as a settled terminal state (matching _incompleteToolCallIds). Regression test asserts a real errorText survives a follow-up turn (verified to fail without the fix). Co-authored-by: Cursor <cursoragent@cursor.com> * fix(think): treat output-denied as a settled tool state; centralize the enumeration Sweep for the recurring "incomplete terminal-state enumeration" bug class found one more instance: `output-denied` (a user-denied tool approval) is a settled state the AI SDK converts into a denial tool-result, but transcript repair, the backstop detector, and the immediate-flush check all omitted it. Repair therefore flipped a denial into a generic "interrupted" error (losing the denial), and the denied result wasn't durably flushed. - Centralize the terminal-state check into `_toolPartHasSettledResult` (output-available | output-error | output-denied, plus legacy output/result), shared by `_repairToolTranscriptParts` and `_incompleteToolCallIds` so the two can no longer drift. - Flush `tool-output-denied` chunks immediately, like other settled results. - Regression test: an output-denied part survives a follow-up turn (verified to fail without the fix). Reviewed and confirmed complete (no change needed): _isTerminalSubmissionStatus, streamIsTerminal, _messageHasPendingInteraction, shouldMarkSkippedAfterGenerationChange. Co-authored-by: Cursor <cursoragent@cursor.com> * fix(agents): reconcile protects errored/denied tool results from stale-client clobber AIChatAgent sweep for the terminal-state-enumeration bug class: ai-chat's own tool-state handling already covers output-denied (it's the HITL home), but the shared reconciler did not. reconcileMessages (run at persist by both Think and AIChatAgent) only carried over the server's `output-available` result into a stale client part — so a client that persisted a stale `input-available` for a tool the server had already resolved to `output-error`/`output-denied` clobbered the resolved result, losing the error or the user's denial. Index all three terminal states and overlay the matching result field (output / errorText / approval). Tests assert a server output-error and output-denied survive a stale client input-available (verified to fail before). Co-authored-by: Cursor <cursoragent@cursor.com> * refactor: address PR #1623 deep-review-2 hardening items - think: remove the now-dead message-deletion branch in _repairTranscriptForProvider (repair preserves every message, never deletes). - think: _normalizeToolInput now also parses a stringified-ARRAY input (`[...]`), not just objects. + test. - agents(reconciler): make the server-state overlay state-driven so only the field matching the terminal state is carried (a stray `output` on an output-error part can't ride along). + test. - ai-chat: document why _chatRecoveryContinue's conversation_changed skip does NOT split assistant-leaf vs user-leaf like Think (no submission layer to protect) — guards against a future regression if submissions are added. Co-authored-by: Cursor <cursoragent@cursor.com> --------- Co-authored-by: Cursor <cursoragent@cursor.com> Co-authored-by: ask-bonk[bot] <ask-bonk[bot]@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Under deploy churn, recovery could re-run already-completed tool calls (the customer observed byte-identical duplicate, non-idempotent INSERTs — a whole message's tool calls rolling back, not just the interrupted step).
Root cause: resumable-stream chunks are buffered in memory and batch-flushed to SQLite every ~10 chunks (
CHUNK_BUFFER_SIZE). The main WebSocket_streamResult(used by chat turns andcontinueLastTurnrecoveries) did not force a flush on settled tool results, so an isolate eviction before the next batch flush lost them. On recovery,_persistOrphanedStreamrebuilds the partial assistant message from SQLite — without those settled tool calls — so the continuation re-anchors without them and the model re-runs the completed tool call.The sub-agent RPC streaming path (
_streamResultToRpcCallback) already flushed recoverable content on settle (the tool-call-level durability referenced by #1615/#1617). This PR brings the WebSocket path to parity.Fix
A shared
_storeChunkDurablyhelper (used by both the WebSocket and RPC stream loops) stores each chunk and flushes the buffer immediately on a settled tool result (tool-output-available/tool-output-error). Frequent recoverable content (text / reasoning / tool-input streaming) stays throttled (first chunk + every ~10) to avoid write amplification._shouldFlushRpcStreamChunkis generalized to_shouldFlushRecoverableChunk.Net effect: a settled tool result is durable the moment it settles, so
_persistOrphanedStreamalways reconstructs every completed tool call, and recovery loses at most the single in-flight step — even when multiple evictions hit one turn.Also: two terminal-status hydration gaps (#1619 follow-up)
Review of #1619 surfaced two remaining "frozen turn" cases where a terminal outcome was broadcast (or silent) but never persisted, so a client disconnected at that moment stayed frozen on reconnect. Both are folded in here:
_handleChatRequest. The outer catch broadcastsMSG_CHAT_RESPONSE { error, done }for failures before the stream starts (e.g. message reconciliation/persist), but never called_recordTerminalChatStatus. Fix silent/frozen interrupted turns: replay terminal status on reconnect #1619 only hooked the stream-level_fireResponseHookand recovery exhaustion. It now records terminal"error"before broadcasting, so_buildIdleConnectMessagesreplays it on connect.continue_disabledrecovery skip. WhenonChatRecoveryreturns{ continue: false }, the turn was only marked interrupted via_markRecoveredSubmissionInterrupted(which updates the programmatic submission row — it does not touch the WS transcript), so a WS chat client got no durable terminal signal at all. Unlike a benignconversation_changedskip (a newer turn already owns the UI), disabling recovery abandons the turn with no superseding turn, so it now records terminal status and broadcasts an error like exhaustion does.conversation_changed/not_recoverableskips remain silent.Test
think-session.test.ts:expected 1 to be greater than 1).beforeTurnthrow (standing in for a reconciliation failure) drives_handleChatRequest's outer catch; assert the idle-connect replay surfaces the terminal error.onChatRecoveryreturns{ continue: false }; assert the idle-connect replay surfaces the terminal error.Test plan
npm run test -w @cloudflare/think—think-session.test.ts130 pass (incl. 3 new regression tests)tsc --noEmit -p packages/think/tsconfig.json)npm run test:e2e -w @cloudflare/think— confirmed green by the author locallyRelated
Completes the deploy-churn recovery hardening series: #1615, #1617, #1618, #1619.
Made with Cursor