cloudflare / cloudflare/agents
applyChunkToParts/isReplayChunk can persist a needsApproval tool part without its input (tool-input-delta field mismatch + tool-approval-request ordering)
- Dominant language
- TypeScript
- Stars
- 5.6k
- Forks
- 711
- Avg merge
- 1d 20h
- Merged PRs (30d)
- 53
Description
### Summary
Two unit-level defects in `applyChunkToParts` / `isReplayChunk` (`src/chat/message-builder.ts`) can leave a `needsApproval` tool part persisted **without its `input`** under specific chunk orderings. If that happens, the post-approval continuation reconstructs the tool call from the persisted part and runs `execute` with empty input — while the client UI (which accumulates input deltas correctly) shows the full arguments in the approval card. For a human-in-the-loop feature that's the bad case: the action the user approved is not the action that executes, and for tolerant tools it's a *silent* no-op/wrong mutation.
**Scoping note (honesty about impact):** we found this while chasing a production failure that ultimately turned out to be an unrelated application-side bug on our end, so we have **not** confirmed a live stream ordering that triggers these paths. Filing anyway because the code paths are provably wrong (pure-function repro below, no model or DO needed) and the failure mode, if an ordering ever produces it, is severe and very hard to attribute.
### Versions
- `agents` 0.16.2 (`AIChatAgent`, WS transport, server-held history)
- `@cloudflare/ai-chat` 0.8.6, `ai` 6.0.208
### The defects
`applyChunkToParts` / `isReplayChunk` are what the DO uses (via `_streamSSEReply`) to rebuild the assistant message from stream chunks for persistence:
1. **`tool-input-delta` never accumulates input text.** The handler does:
```js
if (toolPart && toolPart.state === "input-streaming") toolPart.input = chunk.input;
```
but the v6 UI stream protocol carries the delta in `inputTextDelta`; `chunk.input` is always `undefined`. Deltas contribute nothing, so the part's `input` stays `undefined` until `tool-input-available` arrives — and there is no recovery if it doesn't, or if it's discarded (below).
2. **A `tool-input-available` that arrives after `tool-approval-request` is discarded.** In `ai@6` `streamText`, the parsed tool-call part (which becomes `tool-input-available`) is enqueued on the main transform stream while the `tool-approval-request` is enqueued on the tool-results stream; the two are merged, and we did not find a documented ordering guarantee across the merge. If the approval request lands first, the part flips to `approval-requested` and the late `tool-input-available` is dropped twice over:
- `isReplayChunk` classifies it as a replay (`existing.state !== "input-streaming"`) so `_streamSSEReply` skips it entirely, and
- even if it reached `applyChunkToParts`, the `tool-input-available` handler only assigns input `if (p.state === "input-streaming")`.
If either path leaves the persisted part with `input: undefined`, the continuation after approval converts it via `convertToModelMessages` (`input: part.input`) and `executeToolCall` runs `execute(undefined)`.
### Repro (pure, no model needed)
Feed the chunk sequences through the same `isReplayChunk` → `applyChunkToParts` pipeline `_streamSSEReply` uses:
```js
import { applyChunkToParts, isReplayChunk } from "agents/chat";
const INPUT = { id: "res-1", data: { approval_flow: "..." } };
const INPUT_JSON = JSON.stringify(INPUT);
function applyStream(parts, chunks) {
for (const chunk of chunks) {
if (isReplayChunk(parts, chunk)) continue;
applyChunkToParts(parts, chunk);
}
return parts;
}
const start = { type: "tool-input-start", toolCallId: "tc1", toolName: "update_thing" };
const deltas = [
{ type: "tool-input-delta", toolCallId: "tc1", inputTextDelta: INPUT_JSON.slice(0, 20) },
{ type: "tool-input-delta", toolCallId: "tc1", inputTextDelta: INPUT_JSON.slice(20) },
];
const inputAvailable = { type: "tool-input-available", toolCallId: "tc1", toolName: "update_thing", input: INPUT };
const approvalRequest = { type: "tool-approval-request", toolCallId: "tc1", approvalId: "ap1" };
// approval request beats tool-input-available in the merged stream:
let [part] = applyStream([], [start, approvalRequest, inputAvailable]);
console.log(part.input); // undefined — expected INPUT
// tool-input-available missing entirely (deltas only):
[part] = applyStream([], [start, ...deltas, approvalRequest]);
console.log(part.input); // undefined — expected INPUT (deltas were never accumulated)
```
### Suggested fix
We run this as a pnpm patch (defensively); all changes are in `applyChunkToParts` / `isReplayChunk`:
1. `tool-input-delta`: accumulate `inputTextDelta` onto the part (e.g. `rawInput` string) instead of assigning the nonexistent `chunk.input`.
2. `tool-approval-request`: if the part has no `input` yet, recover it from the accumulated raw text (parse via `normalizeToolInput`) **before** the approval-time persistence snapshot is taken.
3. Late `tool-input-available`: when the existing part's `input` is still `undefined`, adopt the chunk's input even though the state already advanced — and exempt that case in `isReplayChunk` so the chunk isn't skipped as a replay.
```diff
case "tool-input-delta": {
const toolPart = findToolPartByCallId(parts, chunk.toolCallId);
- if (toolPart && toolPart.state === "input-streaming") toolPart.input = chunk.input;
+ if (toolPart && toolPart.state === "input-streaming") {
+ if (typeof chunk.inputTextDelta === "string") toolPart.rawInput = (typeof toolPart.rawInput === "string" ? toolPart.rawInput : "") + chunk.inputTextDelta;
+ if (chunk.input !== void 0) toolPart.input = chunk.input;
+ }
return true;
}
```
```diff
if (p.state === "input-streaming") {
p.state = "input-available";
p.input = normalizeToolInput(chunk.input).input;
...
- }
+ } else if (p.input === void 0) p.input = normalizeToolInput(chunk.input).input;
return true;
```
```diff
if (toolPart) {
const p = toolPart;
p.state = "approval-requested";
p.approval = { id: chunk.approvalId };
+ if (p.input === void 0 && typeof p.rawInput === "string") p.input = normalizeToolInput(p.rawInput).input;
}
```
```diff
if (chunk.type === "tool-input-start") return true;
+ if (chunk.type === "tool-input-available" && existing.input === void 0) return false;
return existing.state !== "input-streaming";
```
Happy to turn this into a PR if useful. Possibly related history: #918, #1627 (other `needsApproval` persistence-state issues).
Contributor guide
Assessment
This issue has not been assessed yet.