ag-ui-protocol / ag-ui-protocol/ag-ui

[Bug]: TOOL_CALL_START handler in defaultApplyEvents is not idempotent — duplicates toolCalls when connect() replays historic events over pre-populated message state`

Đang mở
#1,524 3 bình luận 1 reaction 0 người được giao Xem trên GitHub
bug
Ngôn ngữ chính
Python
Star
15.9k
Fork
1.4k
Merge trung bình
1 ngày 17 giờ
Pull request đã merge (30 ngày)
163

Mô tả

### Pre-flight Checklist

- [x] I have searched [existing issues](https://github.com/ag-ui-protocol/ag-ui/issues) and this hasn't been reported yet.
- [x] I am using the **latest** version AG-UI.

### Describe the Bug

In `defaultApplyEvents` (`sdks/typescript/packages/client/src/apply/default.ts`), the `TOOL_CALL_START` case unconditionally pushes a new entry into `targetMessage.toolCalls` with no duplicate guard:

```ts
targetMessage.toolCalls ??= [];
targetMessage.toolCalls.push({
// ← no idempotency check
id: toolCallId,
type: "function",
function: { name: toolCallName, arguments: "" },
});
```

This is safe for a fresh run, but breaks when `connectAgent()` is called on an agent whose `messages` state already contains a completed assistant message with tool calls (e.g. after a prior run). The `connect()` endpoint replays compacted historic events — including `TOOL_CALL_START` for tool calls that are already present in the initial message state. Because `defaultApplyEvents` does not check for an existing entry before pushing, the same tool call ID is inserted twice:

```ts
message.toolCalls = [
{ id: "call_abc123", ... }, // ← already in initial state
{ id: "call_abc123", ... }, // ← pushed again by replayed TOOL_CALL_START
]
```

Consumers that use `toolCall.id` as a React key (e.g. CopilotKit's `CopilotChatToolCallsView`) then produce a React duplicate-key warning, and React may render the tool call twice or omit it.

### Steps to Reproduce

1. Use `@ag-ui/client` with a runtime that supports `connect()` (e.g. CopilotKit's `InMemoryAgentRunner`)
2. Register a frontend tool via `useFrontendTool` (or any tool the agent can call)
3. Send a message that triggers a tool call and let the run complete
4. Trigger a reconnect — e.g. reload the page, or call `connectAgent()` on an agent whose `messages` already contain the completed assistant message
5. Observe the browser console

The sequence that triggers the bug:

```ts
// Initial agent message state (from prior run):
// messages = [{ id: "msg_1", role: "assistant", toolCalls: [{ id: "call_abc" }] }]

// connect() replays compacted historic events:
TOOL_CALL_START { toolCallId: "call_abc", parentMessageId: "msg_1" } ← pushed again
TOOL_CALL_ARGS { toolCallId: "call_abc", delta: '{"location":"NYC"}' }
TOOL_CALL_END { toolCallId: "call_abc" }

// Result:
// msg_1.toolCalls = [{ id: "call_abc" }, { id: "call_abc" }] ← duplicate
```

Note: `InMemoryAgentRunner.connect()` does filter already-seen events for text messages (via a `messageId`-keyed set), but this guard only checks for the `messageId` property — `TOOL_CALL_START` carries `toolCallId`, so it is not filtered and always passes through.

### Expected Behavior

Processing a `TOOL_CALL_START` event for a `toolCallId` that already exists in `targetMessage.toolCalls` should be a no-op. `defaultApplyEvents` must be idempotent with respect to tool call IDs, since `connect()` legitimately replays the same historic events over an agent that already has them in its message state.

**Proposed fix** — a one-liner guard before the push:

```ts
targetMessage.toolCalls ??= [];
if (!targetMessage.toolCalls.some((tc) => tc.id === toolCallId)) {
targetMessage.toolCalls.push({
id: toolCallId,
type: "function",
function: { name: toolCallName, arguments: "" },
});
}
```

### Environment

```text
- AG-UI package(s) & version(s): `@ag-ui/client@0.0.52`, `@ag-ui/core@0.0.52`
- Runtime: Node.js v24.14.1
- Consumer: `@copilotkit/react-core@1.55.3` with `InMemoryAgentRunner`
```

### Screenshots

_No response_

### Logs & Errors

```shell
Encountered two children with the same key, `call_uUF5OZtugHE6tyoC9PPtpAs6`.
Keys should be unique so that components maintain their identity across updates.
Non-unique keys may cause children to be duplicated and/or omitted —
the behavior is unsupported and could change in a future version.
at CopilotChatToolCallsView
at MemoizedSlotWrapper
at CopilotChatAssistantMessage
...

Full Stack Trace

installHook.js:1 Encountered two children with the same key, `call_6L4zojIwnOpVsBXCwFmVvCMp`. Keys should be unique so that components maintain their identity across updates. Non-unique keys may cause children to be duplicated and/or omitted — the behavior is unsupported and could change in a future version.

overrideMethod @ installHook.js:1
(anonymous) @ react-dom-client.development.js:6604
runWithFiberInDEV @ react-dom-client.development.js:871
warnOnInvalidKey @ react-dom-client.development.js:6603
reconcileChildrenArray @ react-dom-client.development.js:6672
reconcileChildFibersImpl @ react-dom-client.development.js:6993
(anonymous) @ react-dom-client.development.js:7098
reconcileChildren @ react-dom-client.development.js:9701
updateFunctionComponent @ react-dom-client.development.js:10182
beginWork @ react-dom-client.development.js:11778
runWithFiberInDEV @ react-dom-client.development.js:871
performUnitOfWork @ react-dom-client.development.js:17641
workLoopSync @ react-dom-client.development.js:17469
renderRootSync @ react-dom-client.development.js:17450
performWorkOnRoot @ react-dom-client.development.js:16504
performWorkOnRootViaSchedulerTask @ react-dom-client.development.js:18957
performWorkUntilDeadline @ scheduler.development.js:45
<>
(anonymous) @ react-jsx-runtime.development.js:335
(anonymous) @ copilotkit-Cd-NrDyp.mjs:4795
CopilotChatToolCallsView @ copilotkit-Cd-NrDyp.mjs:4793
react_stack_bottom_frame @ react-dom-client.development.js:25904
renderWithHooks @ react-dom-client.development.js:7662
updateFunctionComponent @ react-dom-client.development.js:10166
beginWork @ react-dom-client.development.js:11778
runWithFiberInDEV @ react-dom-client.development.js:871
performUnitOfWork @ react-dom-client.development.js:17641
workLoopSync @ react-dom-client.development.js:17469
renderRootSync @ react-dom-client.development.js:17450
performWorkOnRoot @ react-dom-client.development.js:16504
performWorkOnRootViaSchedulerTask @ react-dom-client.development.js:18957
performWorkUntilDeadline @ scheduler.development.js:45

(anonymous) @ react.development.js:1054
renderSlotElement @ copilotkit-Cd-NrDyp.mjs:88
MemoizedSlotWrapper @ copilotkit-Cd-NrDyp.mjs:96
react_stack_bottom_frame @ react-dom-client.development.js:25904
renderWithHooks @ react-dom-client.development.js:7662
updateForwardRef @ react-dom-client.development.js:9724
beginWork @ react-dom-client.development.js:12117
runWithFiberInDEV @ react-dom-client.development.js:874
performUnitOfWork @ react-dom-client.development.js:17641
workLoopSync @ react-dom-client.development.js:17469
renderRootSync @ react-dom-client.development.js:17450
performWorkOnRoot @ react-dom-client.development.js:16504
performWorkOnRootViaSchedulerTask @ react-dom-client.development.js:18957
performWorkUntilDeadline @ scheduler.development.js:45
postMessage
(anonymous) @ scheduler.development.js:225
(anonymous) @ scheduler.development.js:344
scheduleTaskForRootDuringMicrotask @ react-dom-client.development.js:18924
processRootScheduleInMicrotask @ react-dom-client.development.js:18840
(anonymous) @ react-dom-client.development.js:18991

installHook.js:1 Encountered two children with the same key, `call_6L4zojIwnOpVsBXCwFmVvCMp`. Keys should be unique so that components maintain their identity across updates. Non-unique keys may cause children to be duplicated and/or omitted — the behavior is unsupported and could change in a future version.
overrideMethod @ installHook.js:1
(anonymous) @ react-dom-client.development.js:6604
runWithFiberInDEV @ react-dom-client.development.js:871
warnOnInvalidKey @ react-dom-client.development.js:6603
reconcileChildrenArray @ react-dom-client.development.js:6672
reconcileChildFibersImpl @ react-dom-client.development.js:6993
(anonymous) @ react-dom-client.development.js:7098
reconcileChildren @ react-dom-client.development.js:9701
updateFunctionComponent @ react-dom-client.development.js:10182
beginWork @ react-dom-client.development.js:11778
runWithFiberInDEV @ react-dom-client.development.js:871
performUnitOfWork @ react-dom-client.development.js:17641
workLoopSync @ react-dom-client.development.js:17469
renderRootSync @ react-dom-client.development.js:17450
performWorkOnRoot @ react-dom-client.development.js:16504
performSyncWorkOnRoot @ react-dom-client.development.js:18972
flushSyncWorkAcrossRoots_impl @ react-dom-client.development.js:18814
flushSpawnedWork @ react-dom-client.development.js:18334
commitRoot @ react-dom-client.development.js:17955
commitRootWhenReady @ react-dom-client.development.js:16824
performWorkOnRoot @ react-dom-client.development.js:16722
performWorkOnRootViaSchedulerTask @ react-dom-client.development.js:18957
performWorkUntilDeadline @ scheduler.development.js:45
<>
(anonymous) @ react-jsx-runtime.development.js:335
(anonymous) @ copilotkit-Cd-NrDyp.mjs:4795
CopilotChatToolCallsView @ copilotkit-Cd-NrDyp.mjs:4793
react_stack_bottom_frame @ react-dom-client.development.js:25904
renderWithHooks @ react-dom-client.development.js:7662
updateFunctionComponent @ react-dom-client.development.js:10166
beginWork @ react-dom-client.development.js:11778
runWithFiberInDEV @ react-dom-client.development.js:871
performUnitOfWork @ react-dom-client.development.js:17641
workLoopSync @ react-dom-client.development.js:17469
renderRootSync @ react-dom-client.development.js:17450
performWorkOnRoot @ react-dom-client.development.js:16504
performSyncWorkOnRoot @ react-dom-client.development.js:18972
flushSyncWorkAcrossRoots_impl @ react-dom-client.development.js:18814
flushSpawnedWork @ react-dom-client.development.js:18334
commitRoot @ react-dom-client.development.js:17955
commitRootWhenReady @ react-dom-client.development.js:16824
performWorkOnRoot @ react-dom-client.development.js:16722
performWorkOnRootViaSchedulerTask @ react-dom-client.development.js:18957
performWorkUntilDeadline @ scheduler.development.js:45

(anonymous) @ react.development.js:1054
renderSlotElement @ copilotkit-Cd-NrDyp.mjs:88
MemoizedSlotWrapper @ copilotkit-Cd-NrDyp.mjs:96
react_stack_bottom_frame @ react-dom-client.development.js:25904
renderWithHooks @ react-dom-client.development.js:7662
updateForwardRef @ react-dom-client.development.js:9724
beginWork @ react-dom-client.development.js:12117
runWithFiberInDEV @ react-dom-client.development.js:874
performUnitOfWork @ react-dom-client.development.js:17641
workLoopSync @ react-dom-client.development.js:17469
renderRootSync @ react-dom-client.development.js:17450
performWorkOnRoot @ react-dom-client.development.js:16504
performSyncWorkOnRoot @ react-dom-client.development.js:18972
flushSyncWorkAcrossRoots_impl @ react-dom-client.development.js:18814
flushSpawnedWork @ react-dom-client.development.js:18334
commitRoot @ react-dom-client.development.js:17955
commitRootWhenReady @ react-dom-client.development.js:16824
performWorkOnRoot @ react-dom-client.development.js:16722
performWorkOnRootViaSchedulerTask @ react-dom-client.development.js:18957
performWorkUntilDeadline @ scheduler.development.js:45
postMessage
(anonymous) @ scheduler.development.js:225
(anonymous) @ scheduler.development.js:344
scheduleTaskForRootDuringMicrotask @ react-dom-client.development.js:18924
processRootScheduleInMicrotask @ react-dom-client.development.js:18840
(anonymous) @ react-dom-client.development.js:18991

```

### Additional Context

The exact location of the bug: [`sdks/typescript/packages/client/src/apply/default.ts` lines 302–313](https://github.com/ag-ui-protocol/ag-ui/blob/8daae85f63b1880ddc403970fd94f661c51d71c1/sdks/typescript/packages/client/src/apply/default.ts#L302-L313)

A related guard already exists in `middleware-sse-parser.ts` (CopilotKit) in the post-loop fallback for `TOOL_CALL_CHUNK` flows:

```ts
const alreadyAttached = parent.toolCalls?.some((t) => t.id === tc.id);
if (!alreadyAttached) {
parent.toolCalls.push(tc);
}
```

The same pattern just needs to be applied in `defaultApplyEvents`.

Note: `InMemoryAgentRunner.connect()` does have a de-duplication mechanism, but it only skips events with a `messageId` property (used for text messages). `TOOL_CALL_START` carries `toolCallId` instead, so it is not caught by that guard and always replays.

Hướng dẫn đóng góp

Mở hướng dẫn đóng góp

Đánh giá

Issue này chưa được đánh giá.

Nhận issue mới trong hộp thư của bạn

Bản tóm tắt ngắn những issue GitHub phù hợp với người mới.