Skip to content

added resumable streaming with minimal setup#673

Merged
whoiskatrin merged 12 commits into
mainfrom
resumable-stream-minimal
Nov 25, 2025
Merged

added resumable streaming with minimal setup#673
whoiskatrin merged 12 commits into
mainfrom
resumable-stream-minimal

Conversation

@whoiskatrin

@whoiskatrin whoiskatrin commented Nov 23, 2025

Copy link
Copy Markdown
Contributor

@changeset-bot

changeset-bot Bot commented Nov 23, 2025

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: 7859d25

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

This PR includes changesets to release 1 package
Name Type
agents 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

@pkg-pr-new

pkg-pr-new Bot commented Nov 23, 2025

Copy link
Copy Markdown

Open in StackBlitz

npm i https://pkg.pr.new/cloudflare/agents@673

commit: 7859d25

@claude

claude Bot commented Nov 23, 2025

Copy link
Copy Markdown

Claude Code Review

Overview: Adds resumable streaming capability to AIChatAgent with automatic stream restoration on reconnect.

Issues Found

1. Critical: Hook wrapping breaks subclass overrides (ai-chat-agent.ts:153-174)
The constructor wraps onConnect and onMessage methods, which will override any subclass implementations:

const _onConnect = this.onConnect.bind(this);
this.onConnect = async (connection: Connection, ctx: ConnectionContext) => {
  if (this._activeStreamId) {
    this._notifyStreamResuming(connection);
  }
  return _onConnect(connection, ctx);
};

Subclasses that override these methods will have their implementations replaced. Consider using a hook/event system or template method pattern instead.

2. Unbounded chunk buffer memory leak (ai-chat-agent.ts:360-377)
CHUNK_BUFFER_MAX_SIZE is checked but never enforced - chunks can accumulate indefinitely:

if (this._chunkBuffer.length >= CHUNK_BUFFER_SIZE) {
  await this._flushChunkBuffer();
} else if (this._chunkBuffer.length >= CHUNK_BUFFER_MAX_SIZE) {
  console.warn("Chunk buffer exceeded max size");
}

After warning, chunks keep accumulating. Should force flush or drop chunks.

3. Race condition in flush lock (ai-chat-agent.ts:390-399)
Non-atomic check-and-set of _isFlushingChunks:

if (this._isFlushingChunks) return;
this._isFlushingChunks = true;
// ... async operations

Multiple concurrent calls can pass the check before the flag is set, causing duplicate flushes and potential data corruption.

4. SQL injection via template literals (ai-chat-agent.ts:403)
Using template string instead of parameterized query:

this.sql`insert into cf_ai_chat_stream_chunks (id, stream_id, body, chunk_index, created_at)
  values ${values}`;

Should use proper parameterized queries with ? placeholders.

5. Stale stream threshold too aggressive (ai-chat-agent.ts:31)
5 minutes may be too short for legitimate long-running streams, especially if the LLM response is slow. Consider 15-30 minutes or make it configurable.

6. No error handling in resume flow (ai-react.tsx:236-270)
Resume logic has no try-catch around chunk parsing/processing. Corrupted chunks will crash the client with no recovery mechanism.

7. Test worker implementation is incomplete (tests/worker.ts)
The test worker stub doesn't implement the full resumable streaming flow end-to-end. Tests only verify individual methods, not the actual WebSocket message protocol.

Minor Issues

  • Type safety: StreamChunk and StreamMetadata types should be exported for testing/extensions
  • Docs: Missing error scenarios in resumable-streaming.md (what happens on error streams?)
  • Performance: Index on request_id would help _restoreActiveStream query (ai-chat-agent.ts:440)
  • Logging: No structured logging for debugging resume failures in production

Testing Coverage

✅ Good test coverage for basic flow
✅ Tests verify stale stream cleanup
❌ Missing integration tests for actual reconnection scenario
❌ No tests for error handling/recovery paths
❌ No tests for concurrent flush scenarios

Verdict

NEEDS WORK - The hook wrapping pattern (#1) and race condition (#3) need fixes before merge. The other issues should be addressed but are less critical.

@whoiskatrin
whoiskatrin marked this pull request as ready for review November 24, 2025 11:51

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

changeset

Comment thread examples/resumable-stream-chat/wrangler.jsonc
Comment thread examples/resumable-stream-chat/wrangler.jsonc

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

Looks good, only had a few thoughts. This needs a test though.

};

async onStart() {
await super.onStart();

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.

What is this for?

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.

not needed anymore with Sunil's suggestion

Comment thread examples/resumable-stream-chat/env.d.ts Outdated
@@ -0,0 +1 @@
/// <reference types="vite/client" />

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.

🧐

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.

tbh, we don't really need it, removed

Comment thread packages/agents/src/ai-chat-agent.ts Outdated
// Check if stream is stale (started too long ago)
if (streamAge > STREAM_STALE_THRESHOLD_MS) {
// Mark stale stream as error and don't restore
this.sql`

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.

Just a thought, do we rather delete the entry here instead? As a user I'm not sure I want to keep taking storage with older stream metadata

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.

fixed

Comment thread packages/agents/src/ai-chat-agent.ts Outdated
* Stream chunks are sent only after the client acknowledges with CF_AGENT_STREAM_RESUME_ACK.
*/
override async onConnect(connection: Connection, ctx: ConnectionContext) {
await super.onConnect(connection, ctx);

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.

Is the super.onConnect needed?

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.

fixed

Comment thread packages/agents/src/ai-chat-agent.ts Outdated
* Override onConnect to notify clients about active streams that can be resumed.
* Stream chunks are sent only after the client acknowledges with CF_AGENT_STREAM_RESUME_ACK.
*/
override async onConnect(connection: Connection, ctx: ConnectionContext) {

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.

instead of overriding directly here, can we do it like we dp in the base agent class? so that consumers won't have to call super.onConnect

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.

good call, yes, we totally should do that

@whoiskatrin
whoiskatrin force-pushed the resumable-stream-minimal branch from 1c2884a to e4b095f Compare November 25, 2025 14:31
Added support for resumable streaming with minimal setup.
agents-git-bot Bot pushed a commit to cloudflare/cloudflare-docs that referenced this pull request Nov 25, 2025
Documents the new resumable streaming feature added to AIChatAgent class
in PR cloudflare/agents#673. This feature provides automatic stream
resumption when clients disconnect and reconnect during AI model responses,
which is especially useful for long-running reasoning models.

Key additions:
- New resumable-streaming.mdx page in api-reference section
- Server implementation examples using AIChatAgent
- Client implementation examples using useAgentChat hook
- Technical details on how resumable streaming works internally
- Configuration guidance for SQLite migrations

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
agents-git-bot Bot pushed a commit to cloudflare/cloudflare-docs that referenced this pull request Nov 25, 2025
This documentation covers the new automatic resumable streaming feature in AIChatAgent and useAgentChat, which allows AI chat responses to automatically resume when clients reconnect after disconnection.

Key features documented:
- Automatic stream persistence to SQLite
- Client reconnection handling
- Server-side and client-side implementation examples
- Configuration options for disabling resume

Based on PR cloudflare/agents#673

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
agents-git-bot Bot pushed a commit to cloudflare/cloudflare-docs that referenced this pull request Nov 25, 2025
Synced from cloudflare/agents PR #673: added resumable streaming with minimal setup

This documentation covers the automatic resumable streaming feature in AIChatAgent that allows chat streams to seamlessly resume after disconnections.

Source PR: cloudflare/agents#673

🤖 Generated with Claude Code

Co-Authored-By: Claude <noreply@anthropic.com>
@whoiskatrin
whoiskatrin merged commit 603b825 into main Nov 25, 2025
6 checks passed
@whoiskatrin
whoiskatrin deleted the resumable-stream-minimal branch November 25, 2025 17:02
@github-actions github-actions Bot mentioned this pull request Nov 25, 2025
threepointone pushed a commit to cloudflare/cloudflare-docs that referenced this pull request Nov 27, 2025
* Add resumable streaming documentation

Synced from cloudflare/agents PR #673: added resumable streaming with minimal setup

This documentation covers the automatic resumable streaming feature in AIChatAgent that allows chat streams to seamlessly resume after disconnections.

Source PR: cloudflare/agents#673

🤖 Generated with Claude Code

Co-Authored-By: Claude <noreply@anthropic.com>

* adding resumable streaming to the docs

---------

Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Co-authored-by: Claude <noreply@anthropic.com>
Co-authored-by: katereznykova <kreznykova@cloudflare.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.

3 participants