Skip to content

Fix chat recovery being permanently abandoned under repeated deploys#1615

Merged
threepointone merged 3 commits into
mainfrom
fix/chat-recovery-survives-deploys
May 29, 2026
Merged

Fix chat recovery being permanently abandoned under repeated deploys#1615
threepointone merged 3 commits into
mainfrom
fix/chat-recovery-survives-deploys

Conversation

@threepointone

@threepointone threepointone commented May 29, 2026

Copy link
Copy Markdown
Contributor

Summary

Under repeated deploys, durable chat recovery could permanently abandon an in-flight turn — even though the Durable Object itself stays healthy. A mid-turn deploy resets the DO (Durable Object reset because its code was updated); the interrupted continuation is re-detected on the next wake, and every such interruption consumed one of the bounded recovery attempts. So a deploy every few minutes drove the incident to max_attempts_exceeded and terminalized a turn that was actually making progress. The turn then never retried until the user sent a new message.

This was reproduced empirically against real deploys (see the new harness below): a 10-deploy churn over a single turn exhausted recovery at deploy 5 (attempt 8/6), the orphaned fiber row never cleared, and the turn was abandoned — while a fresh message afterward worked fine, proving only the turn was orphaned, not the DO.

Root cause

_beginChatRecoveryIncident (duplicated in @cloudflare/think and @cloudflare/ai-chat) incremented attempt on every eviction-detection and terminalized at attempt > maxAttempts, with no notion of whether recovery was advancing. A deploy-driven interruption was indistinguishable from a genuinely failing turn, so deploy churn burned the whole budget.

Notably, the reset is a hard isolate kill — not a catchable JS throw — so onChatError/onError never fire for it; the only server-side symptom is the orphaned fiber + the attempt counter, which is exactly what this logic governs.

The fix

  • Progress-aware budget reset. When recovery has made forward progress since the last attempt (more persisted assistant messages — _persistOrphanedStream persists each interrupted partial and does not prune it, so the count is monotonic within an incident), reset the attempt counter. An environmental interruption that followed progress no longer counts against the budget; a turn that never advances still exhausts at maxAttempts.
  • Wall-clock ceiling. A 15-minute per-incident window (CHAT_RECOVERY_MAX_WINDOW_MS) bounds the worst case so a continuously churning environment cannot retry forever.

Applied identically to both packages (they share this recovery foundation).

Edge cases considered

  • Poison / no-progress turn still exhausts at maxAttempts (marker stays flat) — preserved, and confirmed by the existing exhaustion tests still passing.
  • Backward compat: pre-upgrade incidents lack progress → defaults to 0 → at most one lenient reset on first detection (errs toward recovering).
  • High-water mark (progress: max(prev, current)) so message deletion / clearHistory cannot lower the bar or trigger a spurious reset.
  • Exhausted stays effectively terminal: recovery returns before running a continuation once exhausted, so no new progress is generated to revive it.

Validation

  • New unit tests in both packages cover the progress-reset and wall-clock branches (the previous exhaustion tests only exercised the no-progress path):
    • resets the attempt budget when recovery makes forward progress
    • exhausts via the wall-clock window even while making progress
  • All suites green: agents 1662, ai-chat 543, think 429, plus kill/restart e2e recovery suites (think 9, ai-chat 2).
  • Verified against real deploys: the same 10-deploy churn that previously exhausted at deploy 5 now keeps attempt at 1–2 throughout, never exhausts, and self-recovers (continuation completes) once churn settles — SELF-RECOVERED · DO HEALTHY.

Also included: examples/deploy-churn

A reliability harness that drives a durable chat turn against a deployed Worker while firing repeated real wrangler deploys mid-turn. It records a millisecond timeline (deploys + script versions, WS reconnects, recovery-incident transitions), captures errors via onChatError/onError, and prints a verdict distinguishing self-recovery vs. exhaustion vs. a wedged DO. This is the tool used to find and verify the fix; it also serves as a repro others can run. Flags include --deploys, --duration, --no-monitor (proves the failure is deploy-driven, not the client reconnect storm), and --client partysocket.

Follow-ups (not in this PR)

  • Make the recovery window configurable via ChatRecoveryConfig (maxRecoveryWindowMs).
  • Consider hoisting the duplicated recovery-incident logic into agents/chat so think + ai-chat can't drift.
  • Optional small jittered backoff on the continuation schedule to further reduce deploy re-collision.

Test plan

  • npm run test -w @cloudflare/think
  • npm run test -w @cloudflare/ai-chat
  • npx nx run-many -t test --projects=agents
  • npm run test:e2e -w @cloudflare/think
  • npm run test:e2e -w @cloudflare/ai-chat
  • Real-deploy churn via examples/deploy-churn (before: exhausts; after: self-recovers)
  • CI green

Made with Cursor


Open in Devin Review

A mid-turn deploy resets the Durable Object ("Durable Object reset because
its code was updated"); the interrupted chat continuation is re-detected on
the next wake. Previously every such interruption consumed one of the bounded
recovery attempts, so a deploy every few minutes drove the incident to
`max_attempts_exceeded` and terminally abandoned a turn that was actually
making progress — even though each fresh isolate was healthy and served new
messages fine.

Root cause: `_beginChatRecoveryIncident` incremented `attempt` on every
eviction-detection and terminalized at `attempt > maxAttempts`, with no notion
of whether recovery was advancing. An environmental interruption (deploy) was
indistinguishable from a genuinely failing turn.

Fix (in both @cloudflare/think and @cloudflare/ai-chat, which share this
logic): reset the attempt budget when recovery has made forward progress since
the last attempt (count of persisted assistant messages — `_persistOrphanedStream`
persists each interrupted partial and does not prune it, so the count is
monotonic within an incident). A turn that never advances still exhausts at
`maxAttempts`. A 15-minute wall-clock ceiling per incident bounds the worst
case so a continuously churning environment cannot retry forever.

Validation:
- New unit tests in both packages cover the progress-reset and wall-clock
  branches (the previous exhaustion tests only covered the no-progress path).
- All suites pass: agents (1662), ai-chat (543), think (429), plus the
  kill/restart e2e recovery suites (think 9, ai-chat 2).
- Reproduced and verified against real deploys with the new
  examples/deploy-churn harness: the same 10-deploy churn that previously
  exhausted recovery at deploy 5 now self-recovers and stays healthy.

Also adds examples/deploy-churn: a harness that drives a durable chat turn
against a deployed Worker while firing repeated real `wrangler deploy`s
mid-turn, records a millisecond timeline (deploys, script versions, WS
reconnects, recovery-incident transitions), and captures errors via
`onChatError`/`onError`. Used to find and verify this fix.

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

changeset-bot Bot commented May 29, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: 67941df

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

This PR includes changesets to release 2 packages
Name Type
@cloudflare/think Patch
@cloudflare/ai-chat 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

@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: No Issues Found

Devin Review analyzed this PR and found no potential bugs to report.

View in Devin Review to see 5 additional findings.

Open in Devin Review

threepointone and others added 2 commits May 29, 2026 21:47
Fixes a typecheck error: the test helper was added to the agent worker but
not to the typed RPC stub interface used by the durable-chat-recovery tests.

Co-authored-by: Cursor <cursoragent@cursor.com>
Format README table for consistent column alignment and switch a word to italic emphasis. Reorder dependencies in examples/deploy-churn/package.json (moved `@cloudflare/think` below `@cloudflare/kumo`) — purely stylistic change with no functional impact.
@pkg-pr-new

pkg-pr-new Bot commented May 29, 2026

Copy link
Copy Markdown

Open in StackBlitz

agents

npm i https://pkg.pr.new/agents@1615

@cloudflare/ai-chat

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

@cloudflare/codemode

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

hono-agents

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

@cloudflare/shell

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

@cloudflare/think

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

@cloudflare/voice

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

@cloudflare/worker-bundler

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

commit: 67941df

@threepointone
threepointone merged commit 51a771f into main May 29, 2026
4 checks passed
@threepointone
threepointone deleted the fix/chat-recovery-survives-deploys branch May 29, 2026 20:58
@github-actions github-actions Bot mentioned this pull request May 29, 2026
threepointone added a commit that referenced this pull request May 29, 2026
…s on a code-update DO reset (#1617)

* test(ai-chat): reproduce recovery abandonment on a superseded-isolate alarm

Adds a deterministic reproduction (currently RED) for a second deploy-churn
abandonment path that #1615 cannot reach. After an interrupted turn is
detected, recovery schedules `_chatRecoveryContinue` and deletes the orphaned
`cf_agents_runs` row — so the turn rides solely on the one-shot schedule row.
If that alarm then fires on a SUPERSEDED isolate, the first storage op throws
the catchable `Durable Object reset because its code was updated.` for the
whole invocation; `_executeScheduleCallback` burns its in-process retries,
swallows the error, and `alarm()` deletes the one-shot row. Both recovery
vehicles are now gone and `_beginChatRecoveryIncident` never runs again — the
turn is permanently abandoned.

The test drives the REAL dispatch path (`alarm()` ->
`_executeScheduleCallback` -> swallow -> one-shot DELETE); the only injected
element is the (production-observed) reset error itself, thrown from the
recovery callback via a test-only `setSimulateSupersededIsolateForTest` flag.
It asserts the desired post-fix behavior (a recovery vehicle survives), so it
fails on current main. The fix follows in the next commit.

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

* fix(agents): defer one-shot recovery callbacks on a code-update DO reset

When a scheduled one-shot callback fires on an isolate a deploy has just
superseded, the first `ctx.storage` op throws
`Durable Object reset because its code was updated.` for the whole invocation.
`_executeScheduleCallback` previously burned its in-process retries (all
doomed — code never reloads mid-invocation), swallowed the error, and let
`alarm()` delete the one-shot row — permanently abandoning the work. For chat
recovery this is a second deploy-churn abandonment path, upstream of and
unreachable by the progress-aware budget added in #1615: the orphaned fiber
row is deleted as soon as recovery is "handled", so the turn rides solely on
the `_chatRecoveryContinue` / `_chatRecoveryRetry` schedule row, and deleting
it leaves nothing for the boot scan to re-detect.

Fix: for a one-shot row failing with this transient, skip the doomed
in-process retries (`shouldRetry`) and re-throw instead of swallowing, so
`alarm()` rejects, the one-shot row survives, and the platform re-runs the
alarm on a fresh isolate (= new code) under the at-least-once guarantee. The
work auto-resumes once the deploy settles. Every other callback and error
class keeps the existing swallow-and-exhaust behavior.

Turns the reproduction test added in the previous commit green; verified it
fails without this change. All suites pass (agents 1662, ai-chat 544, think
431, e2e think 9 + ai-chat 2).

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

---------

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