cloudflare / cloudflare/agents

useAgentChat: the clientToolResults cleanup effect dispatches setState once per streamed chunk, accumulating nestedUpdateCount to the React #185 throw

Open
#2,217 1 comment 0 reactions 0 assignees View on GitHub
Dominant language
TypeScript
Stars
5.6k
Forks
711
Avg merge
1d 20h
Merged PRs (30d)
53

Description

## Summary

`useAgentChat`'s `clientToolResults` stale-entry cleanup effect calls `setClientToolResults` **unconditionally** on every `chatMessages` change. Because the dispatch is not actually free, it consumes one unit of React's nested-update budget per streamed chunk, and any sufficiently long answer ends in `Maximum update depth exceeded` (React #185).

This is a distinct dispatch source from the ones covered by #1361 / #1732 / #1913 (per-frame `setMessages`, replay merging). It also explains why `experimental_throttle` — the guidance given on #1732 — reduced the failure rate for people without closing it.

`packages/agents/src/chat/react.tsx`, unchanged on `main` @ `ec93caf6` and in the published `agents@0.22.0`:

```ts
useEffect(() => {
const currentToolCallIds = new Set();
// ...collect ids from chatMessages...

// Use functional update to check and clean stale entries atomically
setClientToolResults((prev) => {
if (prev.size === 0) return prev;
// ...
if (!hasStaleEntries) return prev; // <-- believed to be a bailout
// ...
});
}, [chatMessages]);
```

## Why returning `prev` is not a bailout

React's `useState` dispatch only takes the eager-bailout path when the fiber has **no pending work**. During a streamed turn the next chunk has usually already scheduled an update, so the eager path is skipped, the update is queued, and the component re-renders even though the map is identical.

That alone would only be wasteful. The failure comes from *which lane* it lands on:

- This is a **passive** effect, and a SyncLane commit flushes passive effects **inside the commit itself** (`0 !== (pendingEffectsLanes & 3) && flushPendingEffects()`).
- So the dispatch schedules a DefaultLane update while `root.pendingLanes` is still non-empty.
- That is exactly the condition under which React **increments** `nestedUpdateCount` instead of resetting it. The reset only happens on a commit that ends with no `SyncLane | InputContinuousLane | DefaultLane` pending.

`nestedUpdateCount` is a **monotonic accumulator, not a loop detector**. There is no render loop here — each streamed chunk contributes exactly one count, the counter never resets while the turn is live, and at 51 React throws from `getRootForUpdatedFiber`. The throw therefore lands on whatever update happens to come next, which in an AI SDK v6 setup is the `ReactChatState` store notification inside `Chat.makeRequest`'s `try`/`catch` — so it is swallowed and reassigned to chat `error`, with no error boundary, no component stack, and nothing in server logs. The server-side turn completes normally.

## Measurement

Reproduced against `agents@0.21.0` + `@ai-sdk/react@3.0.204` + React 19.2.6, with an instrumented `react-dom-client.development.js` logging the owner fiber, lane, and stack in `scheduleUpdateOnFiber` whenever `isFlushingPassiveEffects`, plus every `nestedUpdateCount++`:

- **353** dispatches from this effect in a single answer
- all on **lane 32** (DefaultLane), owner fiber = the transcript component
- `nestedUpdateCount` climbing monotonically, `remaining=32` on every increment
- reproduced on long answers, never on short ones — consistent with an accumulator rather than a loop

## Fix

Compute staleness from a ref mirror of the map **before** dispatching, so a turn that prunes nothing schedules no update at all. The updater still recomputes from `prev`, so a concurrent write between the check and the update is not clobbered.

Branch and commit on my fork, since PR creation against this repo 404s for external accounts (same as #1363, #2181, #2182):

- https://github.com/Konan69/agents/commit/ce283f35
- https://github.com/Konan69/agents/tree/fix/agentchat-tool-result-prune-update-depth

`pnpm --filter agents typecheck`, `oxlint`, and `oxfmt --check` pass on the change. Happy to add a regression test in `packages/agents/src/react-tests/` if you'd like one in the same shape as `default-throttle.test.tsx` — it needs a counted-dispatch assertion rather than a render count, so I'd rather match whatever you prefer there.

## Verified downstream

Carried as a `pnpm` patch in our app against `0.21.0`. Long streamed answers that reliably produced the banner before now complete cleanly (3/3), with no change in tool-result rendering behaviour.

One gotcha worth recording for anyone reproducing: with Vite, `agents/chat/react` gets inlined into the pre-bundled dep chunk, so rewriting `node_modules` does **not** invalidate it. Clear the dep cache and restart, or you will silently test the unpatched code.

Contributor guide

Open the contributing guide

Research direction

Start in packages/agents/src/chat/react.tsx at the clientToolResults cleanup effect, then review the related tests under packages/agents/src/react-tests/, especially default-throttle.test.tsx. Add a regression test that counts dispatches during streamed chunks and run pnpm --filter agents typecheck, oxlint, and oxfmt --check; done means long streamed answers no longer trigger the update-depth error without changing tool-result rendering.

Written by the indexing model from the issue text.

Assessment

Tech stack
react, typescript
Domain
frontend
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Active
Clarity
Clearly specified
Newbie friendliness
45/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.