Repository navigation
fix(agents): reclaim resumable-stream buffers from an alarm (#1706) - #1712
Merged
Merged
Conversation
Resumable-stream chunk buffers (cf_ai_chat_stream_*) were only swept
lazily when a *subsequent* stream completed. A chat that received a
single turn and then went idle never triggered that sweep, so its
buffers lingered in the Durable Object's SQLite for the lifetime of the
DO — an unbounded leak for one-off / low-traffic chats.
Alarm-driven cleanup
--------------------
AIChatAgent and Think now arm a scheduled cleanup alarm whenever a
stream starts and whenever it finishes (completes or errors):
- Arming on start is the safety net for the non-durable path
(chatRecovery: false, the AIChatAgent default). Those turns don't
run inside runFiber, so there's no leftover keepAlive alarm and no
fiber-recovery scan; if the client never reconnects, nothing else
wakes the DO. Arming on start guarantees a stream whose DO is
evicted mid-flight still gets a future sweep instead of leaking.
- Arming on finish covers the common completed/errored case.
- Durable runFiber turns already self-heal (keepAlive survives
eviction -> recovery finalizes -> arms cleanup), so arming on start
is belt-and-suspenders there.
The alarm sweeps aged buffers and re-arms only while reclaimable rows
remain, so a fully-swept DO stops waking itself. Arming is idempotent so
high-turn-count chats never accumulate cleanup schedules. The re-arm
inside the fired callback is deliberately NON-idempotent: alarm()
deletes the fired one-shot row after the callback returns, so an
idempotent reschedule would dedup onto that doomed row and vanish with
it, leaking any buffer that survived the sweep.
Two short, purpose-specific retention windows
---------------------------------------------
Replaces the single 24h threshold (which would have kept the leak alive
for a day) with windows matched to what each buffer is actually for:
- COMPLETED_RETENTION_MS = 10 min, measured from completion. The
assistant message is persisted separately (cf_ai_chat_agent_messages),
so a finished buffer is only a brief reconnect-and-replay grace:
long enough to cover a client that dropped at the completion
boundary, and to deliver a pending terminal error frame (#1645) on a
resumed stream. After the window a late reconnect degrades
gracefully to a clean done frame plus persisted history.
- ABANDONED_STREAM_RETENTION_MS = 1 h, measured from LAST chunk
activity. Generous so an interrupted turn has ample time to be
resumed by a reconnecting client or healed by fiber recovery before
its buffer is presumed dead. Keyed off last activity (not start
time) so a long-running stream still emitting chunks is never swept
mid-flight. Server-side reconstruction (recovery, resume-ACK via
_persistOrphanedStream) only ever reads ACTIVE streaming rows, which
live in this 1h window — never the 10min completed grace — so
recovering a turn interrupted >10 min ago still works.
No new public configuration: correctness never depends on the window
(the durable message is the source of truth), so these are sane internal
defaults rather than another knob to misconfigure.
API
---
ResumableStream gains cleanup(now?) (force a sweep, bypassing the lazy
interval gate) and hasReclaimableStreams() to support alarm-driven
cleanup.
Tests
-----
New stream-cleanup coverage in both ai-chat and think:
- arms a single alarm on finish, deduping repeats
- reclaims aged buffers when the alarm fires (no completeStream)
- re-arms only while reclaimable buffers remain
- survives the real alarm fire and re-arms when a younger buffer
remains (guards the non-idempotent-reschedule footgun)
- stops re-arming after the last buffer is swept
- arms cleanup when a stream starts
- window boundaries: completed grace (10m) vs abandoned stale (1h)
- locks the arming delay at 10 min (regression guard against
re-lengthening toward the old leak window)
- keeps an in-flight buffer's chunks reconstructable past the
completion grace (proves recovery reads the 1h streaming window)
Docs: updated docs/resumable-streaming.md to describe the two windows
(needs manual port to cloudflare/cloudflare-docs).
🦋 Changeset detectedLatest commit: 41b1fc2 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 |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
agents
@cloudflare/ai-chat
@cloudflare/codemode
create-think
hono-agents
@cloudflare/shell
@cloudflare/think
@cloudflare/voice
@cloudflare/worker-bundler
commit: |
_streamResultToRpcCallback called this._resumableStream.start() raw, bypassing the _startResumableStream wrapper that arms the cleanup alarm on stream start. Its WebSocket sibling _streamResult already uses the wrapper; this RPC path (the sub-agent chat() entry point) was missed — an incomplete transformation. Normal finishes were already covered (every terminal path goes through _completeResumableStream / _errorResumableStream, which arm). The gap was the mid-flight-eviction case the start-time arm exists for: a sub-agent whose isolate resets mid RPC stream with chatRecovery: false has no fiber-recovery scan to re-arm on wake, so without start-time arming its buffer would never get a sweep — the exact #1706 leak. Now both streaming entry points share the same arm-on-start invariant.
Merged
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
Resumable-stream chunk buffers (
cf_ai_chat_stream_*) were only swept lazily when a subsequent stream completed. A chat that received a single turn and then went idle never triggered that sweep, so its buffers lingered in the Durable Object's SQLite for the lifetime of the DO — an unbounded storage leak for one-off / low-traffic chats.This PR drives buffer cleanup from a scheduled alarm and replaces the single 24h retention threshold with two short, purpose-specific windows.
Fixes #1706.
Alarm-driven cleanup
AIChatAgentandThinknow arm a scheduled cleanup alarm whenever a stream starts and whenever it finishes (completes or errors):chatRecovery: false, theAIChatAgentdefault). Those turns don't run insiderunFiber, so there's no leftoverkeepAlivealarm and no fiber-recovery scan; if the client never reconnects, nothing else wakes the DO. Arming on start guarantees a stream whose DO is evicted mid-flight still gets a future sweep instead of leaking.runFiberturns already self-heal (keepAlive survives eviction → recovery finalizes → arms cleanup), so arming on start is belt-and-suspenders there.The alarm sweeps aged buffers and re-arms only while reclaimable rows remain, so a fully-swept DO stops waking itself. Arming is idempotent so high-turn-count chats never accumulate cleanup schedules.
Note
The re-arm inside the fired callback is deliberately non-idempotent:
alarm()deletes the fired one-shot row after the callback returns, so an idempotent reschedule would dedup onto that doomed row and vanish with it — leaking any buffer that survived the sweep. There's a dedicated regression test for this footgun.Two retention windows
Replaces the single 24h threshold (which would have kept the leak alive for a day) with windows matched to what each buffer is actually for:
COMPLETED_RETENTION_MScf_ai_chat_agent_messages), so a finished buffer is only a brief reconnect-and-replay grace: enough to cover a client that dropped at the completion boundary, and to deliver a pending terminal error frame (#1645) on a resumed stream.ABANDONED_STREAM_RETENTION_MSWhy two windows matter for recovery: server-side reconstruction (fiber recovery and resume-ACK, via
_persistOrphanedStream) only ever reads activestreamingrows, which live in the 1h window — never the 10min completed grace. So recovering a turn interrupted more than 10 minutes ago still works. A regression test pins this.No new public configuration. Correctness never depends on the window (the durable message is the source of truth), so these are sane internal defaults rather than another knob to misconfigure.
Behavior change to be aware of
The chunk-replay window for completed streams shrinks from 24h to ~10 min. A client reconnecting after that still gets the full conversation via persisted history (state sync) — it just won't get token-by-token replay of an old finished stream, which degrades gracefully to a clean
doneframe. No data is lost (the message is always persisted independently).API
ResumableStreamgains:cleanup(now?)— force a sweep, bypassing the lazy interval gate (alarm entrypoint).hasReclaimableStreams()— whether any rows remain, to decide re-arming.Tests
New
stream-cleanupcoverage in bothai-chatandthink:completeStream)streamingwindow)Verification:
pnpm run check— passes (sherif + export checks + oxfmt + oxlint + typecheck, 102 projects)examples/playground: normal streaming, resume-after-reconnect, and late reconnect all behave correctlyDocs
Updated
docs/resumable-streaming.mdto describe the two windows.Important
Per
docs/AGENTS.md, this doc change must be manually ported tocloudflare/cloudflare-docs(src/content/docs/agents/) — there is no automated sync.