Skip to content

fix(core): ignore duplicate provider tool-call ids - #5038

Merged
wenshao merged 15 commits into
QwenLM:mainfrom
YingchaoX:fix/duplicate-tool-call-id
Jun 18, 2026
Merged

fix(core): ignore duplicate provider tool-call ids#5038
wenshao merged 15 commits into
QwenLM:mainfrom
YingchaoX:fix/duplicate-tool-call-id

Conversation

@YingchaoX

Copy link
Copy Markdown
Contributor

What this PR does

This PR makes provider-supplied tool-call IDs idempotent within a single agentic loop. When the model/provider returns the same tool-call ID more than once, the first call executes normally and later duplicates are answered with a synthetic tool result explaining that the duplicate was ignored, without running the tool again.

The fallback/generated local call ID is kept separate from the provider ID, so calls without a provider-supplied ID continue through the existing path and are not deduplicated across rounds by a generated value.

Why it's needed

Issue #5014 shows a model repeatedly returning the same provider tool-call ID across tool-result rounds, causing side-effect tools such as shell commands to execute over and over. This can amplify one model mistake into many repeated writes or shell executions.

This fix addresses only the exact duplicate provider-ID case. Semantically similar calls with different IDs, such as the related loop-breaker cases discussed in #5015 / #4695, remain out of scope for this PR.

Reviewer Test Plan

How to verify

Run the focused regression tests for provider tool-call ID preservation and duplicate suppression:

cd packages/core && npx vitest run src/core/turn.test.ts src/agents/runtime/agent-headless.test.ts
cd packages/cli && npx vitest run src/nonInteractiveCli.test.ts

Run build/typecheck for the touched packages and local bundle path:

npm run build && npm run typecheck
npm run build && npm run bundle

For an end-to-end verification, point the locally built CLI at a mock OpenAI-compatible server that returns the same tool-call ID (shell_1) in two consecutive rounds. Expected behavior: the first shell call executes, the second duplicate receives an error-shaped synthetic tool result, the final answer succeeds, and the side-effect file contains exactly one write. I also verified the same scenario with --max-tool-calls 1; the run still succeeds and reports stats.tools.totalCalls=1, confirming the ignored duplicate does not consume the tool-call budget.

Evidence (Before & After)

Before: the original #5014 reproduction on macOS Docker Desktop repeatedly executed the same shell_1 provider tool call, with the side-effect write occurring hundreds of times.

After: deterministic mock-provider E2E against local dist/cli.js returned shell_1 twice, exited successfully, produced exactly one side-effect write, and included this duplicate response in the transcript: Duplicate provider tool call id "shell_1" was already handled. The duplicate tool call was ignored and not executed again.

Tested on

OS Status
🍏 macOS ✅ tested
🪟 Windows ⚠️ not tested
🐧 Linux ⚠️ not tested

Environment (optional)

Local verification used the built bundle (node dist/cli.js) with a mock OpenAI-compatible chat completions server, --approval-mode yolo, isolated QWEN_RUNTIME_DIR, and no sandbox.

Risk & Scope

  • Main risk or tradeoff: repeated provider IDs now produce a synthetic error-shaped tool result instead of re-running the tool, so models that intentionally reuse a provider call ID for a new action will be told the duplicate was ignored.
  • Not validated / out of scope: semantic loop breaking for repeated tool calls with different provider IDs is not handled here.
  • Breaking changes / migration notes: none expected.

Linked Issues

Fixes #5014

中文说明

What this PR does

这个 PR 在单次 agentic loop 内把 provider 原始返回的 tool-call ID 作为幂等键处理。模型或 provider 返回同一个 tool-call ID 多次时,第一次正常执行,后续重复调用会收到一个 synthetic tool result,说明该重复调用已被忽略,并且不会再次执行工具。

本地 fallback/generated call ID 与 provider ID 分开保留,所以没有 provider 原始 ID 的调用仍然走现有路径,不会因为生成出来的 ID 被跨 round 去重。

Why it's needed

#5014 显示模型会跨 tool-result round 反复返回同一个 provider tool-call ID,导致 shell 等有副作用的工具被重复执行。这样会把一次模型错误放大成大量重复写入或 shell 执行。

这个修复只处理 provider ID 完全相同的重复调用。#5015 / #4695 中提到的“语义相同但 ID 不同”的 loop breaker 问题不包含在本 PR 范围内。

Reviewer Test Plan

How to verify

运行 provider tool-call ID 保留和重复调用抑制的聚焦回归测试:

cd packages/core && npx vitest run src/core/turn.test.ts src/agents/runtime/agent-headless.test.ts
cd packages/cli && npx vitest run src/nonInteractiveCli.test.ts

运行 build/typecheck,并验证本地 bundle 路径:

npm run build && npm run typecheck
npm run build && npm run bundle

端到端验证可以把本地构建出来的 CLI 指向一个 mock OpenAI-compatible server,让 mock provider 连续两轮返回同一个 tool-call ID(shell_1)。预期行为是:第一次 shell 调用正常执行,第二次重复调用收到 error-shaped synthetic tool result,最终回答成功,并且副作用文件里只有一次写入。我也用 --max-tool-calls 1 验证了同一个场景;运行仍然成功,并报告 stats.tools.totalCalls=1,说明被忽略的 duplicate 不会消耗工具调用预算。

Evidence (Before & After)

Before:#5014 原始复现在 macOS Docker Desktop 上会重复执行同一个 shell_1 provider tool call,副作用写入发生数百次。

After:使用本地 dist/cli.js 和确定性的 mock provider 做 E2E,mock 连续两轮返回 shell_1,CLI 成功退出,副作用写入只有一次,并且 transcript 中包含 duplicate response:Duplicate provider tool call id "shell_1" was already handled. The duplicate tool call was ignored and not executed again.

Tested on

OS Status
🍏 macOS ✅ tested
🪟 Windows ⚠️ not tested
🐧 Linux ⚠️ not tested

Environment (optional)

本地验证使用构建后的 bundle(node dist/cli.js)、mock OpenAI-compatible chat completions server、--approval-mode yolo、隔离的 QWEN_RUNTIME_DIR,未启用 sandbox。

Risk & Scope

  • Main risk or tradeoff: provider ID 重复时现在会返回 synthetic error-shaped tool result,而不是再次执行工具;如果某个模型有意复用同一个 provider call ID 表示新动作,它会被告知该 duplicate 已被忽略。
  • Not validated / out of scope: 本 PR 不处理 provider ID 不同但语义重复的工具调用 loop breaker。
  • Breaking changes / migration notes: 预期没有破坏性变更。

Linked Issues

Fixes #5014

Comment thread packages/cli/src/nonInteractiveCli.ts Outdated
Comment thread packages/core/src/agents/runtime/agent-core.ts
YingchaoX and others added 3 commits June 12, 2026 19:44
Co-authored-by: Shaojin Wen <shaojin.wensj@alibaba-inc.com>
Co-authored-by: Shaojin Wen <shaojin.wensj@alibaba-inc.com>

@wenshao wenshao left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Scope note: The interactive TUI path (packages/cli/src/ui/hooks/useGeminiStream.ts) has no providerCallId or pre-execution duplicate detection logic. If a model re-emits the same provider tool-call ID in the TUI, the tool still executes with full side effects before the post-execution dedup drops the result from the conversation. Consider adding analogous pre-execution dedup to the TUI path, or documenting that it is intentionally out of scope.

Comment thread packages/cli/src/nonInteractiveCli.ts
const toolName = String(fc.name);

if (providerCallId) {
if (handledProviderToolCallIds.has(providerCallId)) {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Suggestion] Zero debug logging when a duplicate provider tool call is detected and suppressed. If a model repeatedly emits the same tool-call ID, the only observability is via emitted events — no log trail exists in --debug output or log files. This makes production debugging at 3 AM difficult.

Consider adding a debug log at the detection point:

this.runtimeContext.getDebugLogger()?.debug(
  `[processFunctionCalls] Suppressing duplicate provider tool-call id: ${providerCallId} (tool: ${toolName}, round: ${currentRound})`,
);

Same applies to the CLI path in nonInteractiveCli.ts (around line 763) using debugLogger.debug(...).

— qwen3.7-max via Qwen Code /review

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.

Addressed in 0972eeaed: duplicate suppression now logs debug messages in AgentCore and in the non-interactive path. I also added the same lightweight debug log to the new TUI pre-schedule suppression path so all three execution entrypoints leave a trace when a provider id is ignored.

Comment thread packages/core/src/agents/runtime/agent-core.ts
Comment thread packages/core/src/agents/runtime/agent-core.ts
Comment thread packages/core/src/agents/runtime/agent-core.ts
Comment thread packages/cli/src/nonInteractiveCli.test.ts

@qqqys qqqys left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Critical] The duplicate-provider-id guard does not cover the interactive TUI path.

This PR preserves providerCallId in Turn and suppresses duplicates in non-interactive/headless flows before execution, but useGeminiStream.ts still collects every ToolCallRequest and calls scheduleToolCalls(toolCallRequests, signal) with no provider-id guard. The existing history/functionResponse dedup in handleCompletedTools only runs after the scheduler has already executed the tool, so an interactive session can still execute the same provider tool-call ID twice and only drop or repair the duplicate result afterward.

Impact: the main TUI path can repeat side-effecting tools such as shell, edit/write, or hook-triggering calls when a provider re-emits the same tool-call ID across rounds. That is the core failure mode this PR is meant to prevent, and it is user-visible/session-corrupting for normal interactive usage.

Please add the same pre-execution handledProviderToolCallIds suppression to the TUI stream path, or commonize the duplicate filtering before any path reaches CoreToolScheduler.schedule, so duplicate provider calls emit a synthetic tool response without scheduling local side effects.

@YingchaoX

Copy link
Copy Markdown
Contributor Author

Addressed the interactive TUI scope gap in 0972eeaed: useGeminiStream now suppresses duplicate provider tool-call ids before scheduleToolCalls, so duplicate provider calls do not enter CoreToolScheduler and cannot re-run side-effecting tools in the TUI. Duplicate-only batches submit a synthetic ToolResult continuation immediately; mixed batches merge the synthetic duplicate response with the real tool responses when the scheduled tools complete. Added TUI regressions for same-stream duplicates, history-paired duplicate ids, and no-provider-id calls that must still schedule normally.

Local verification:

  • cd packages/core && npx vitest run src/agents/runtime/agent-headless.test.ts
  • cd packages/cli && npx vitest run src/nonInteractiveCli.test.ts src/ui/hooks/useGeminiStream.test.tsx
  • npm run build && npm run typecheck

Comment thread packages/core/src/agents/runtime/agent-core.ts
Co-authored-by: Shaojin Wen <shaojin.wensj@alibaba-inc.com>

@qqqys qqqys left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Critical blocker: the latest head does not typecheck in packages/core/src/agents/runtime/agent-core.ts. The new emitSyntheticToolError parameter type declares errorMessage twice and no longer declares responseParts, but the implementation still reads params.responseParts and both call sites pass responseParts. That leaves the core package with hard TypeScript errors on the main agent/subagent runtime path, so CI/build cannot pass until the helper parameter shape is restored (for example errorMessage: string plus responseParts: Part[]).

@wenshao

wenshao commented Jun 14, 2026

Copy link
Copy Markdown
Collaborator

@YingchaoX heads up — this PR currently has merge conflicts with main and can't be merged as-is. Could you merge main in (or rebase) and resolve them when you get a chance?

Conflicting files:

  • packages/cli/src/nonInteractiveCli.ts
  • packages/cli/src/ui/hooks/useGeminiStream.ts
  • packages/core/src/agents/runtime/agent-core.ts
  • packages/core/src/agents/runtime/agent-headless.test.ts

The rest merges cleanly. Thanks!

中文

@YingchaoX 提个醒 —— 这个 PR 目前和 main 有合并冲突,暂时没法直接合入。方便的时候麻烦把最新的 main merge 进来(或 rebase)解决一下冲突。

冲突文件:

  • packages/cli/src/nonInteractiveCli.ts
  • packages/cli/src/ui/hooks/useGeminiStream.ts
  • packages/core/src/agents/runtime/agent-core.ts
  • packages/core/src/agents/runtime/agent-headless.test.ts

其余文件可以自动合并。谢谢!

qqqys
qqqys previously approved these changes Jun 15, 2026

@qqqys qqqys left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Prior critical issues are resolved in the latest head: the interactive TUI path now suppresses duplicate provider tool-call IDs before scheduling, duplicate-only TUI batches submit a synthetic tool response, and the AgentCore synthetic error parameter shape/typecheck blocker has been fixed. CI is green, and I did not find any remaining critical issue in this pass.

@wenshao

wenshao commented Jun 15, 2026

Copy link
Copy Markdown
Collaborator

✅ Verification report — deterministic mock-provider E2E + completeness audit

I validated this PR by building dist from the branch and driving the real CLI against a mock OpenAI server that returns the same provider tool-call id (shell_1) across two rounds (over tmux), plus an independent read of the dedup logic. TL;DR: the fix is correct and well-behaved on the paths it touches — but it is incomplete: the qwen serve / ACP daemon path still reproduces #5014. Details below; recommend covering the daemon path (or scoping it explicitly) before treating #5014 as fully fixed.

How it was validated

Area Method Result
Focused unit tests vitest (the files the PR lists) green — core turn/agent-headless/toolCallIdUtils 63 pass (2 pre-existing skips); cli nonInteractiveCli 51 (1 skip), useGeminiStream 110
E2E — duplicate suppression built dist/cli.js + mock returns shell_1 twice; --approval-mode yolo first executes, second suppressed with synthetic error, exactly 1 side-effect write, tools.totalCalls=1, exit 0
E2E — budget --max-tool-calls 1 + duplicate run succeeds, totalCalls=1, 1 write (ignored duplicate consumes no budget)
E2E — precision control mock returns shell_1 then shell_2 (different ids) both execute, 2 writes, totalCalls=2, no false suppression
E2E — no provider id mock omits id both rounds both execute (unique generated ids), 2 writes — not deduped, as claimed
Completeness audit qwen serve daemon vs same mock ⚠️ 2 writes — #5014 reproduces on the daemon/ACP path (Finding 1)

E2E evidence — qwen -p against the duplicate mock (the titular fix)

Real --output-format json transcript (abridged):

// round 1: shell_1 executes
{"role":"assistant","content":[{"type":"tool_use","id":"shell_1","name":"run_shell_command", ...}]}
{"role":"user","content":[{"type":"tool_result","tool_use_id":"shell_1","is_error":false,"content":"...Exit Code: 0..."}]}
// round 2: provider re-sends shell_1 → normalized id suffixed, provider id preserved → SUPPRESSED
{"role":"assistant","content":[{"type":"tool_use","id":"shell_1__qwen_dup_2","name":"run_shell_command", ...}]}
{"role":"user","content":[{"type":"tool_result","tool_use_id":"shell_1__qwen_dup_2","is_error":true,
  "content":"Duplicate provider tool call id \"shell_1\" was already handled. The duplicate tool call was ignored and not executed again."}]}
// round 3: success
{"type":"result","is_error":false,"result":"All done. ...","stats":{"tools":{"totalCalls":1,...}}}

Side-effect file contained exactly one HIT. The mechanism is sound: normalizeModelToolCallIds suffixes the cross-round duplicate's callId (shell_1__qwen_dup_2) — the very thing that caused re-execution before — while stashing the raw provider id on a non-enumerable Symbol; agent-core / nonInteractiveCli / useGeminiStream seed handledProviderToolCallIds from getHistoryFunctionResponseIds() and suppress on the provider id. Clean, and the synthetic result is error-shaped exactly as the PR describes.

🟠 Finding 1 — the qwen serve / ACP daemon path is not covered; #5014 still reproduces there · Medium

The fix is wired into three agentic loops (agent-core.ts:664, nonInteractiveCli.ts:741, useGeminiStream.ts:1687) but not into the ACP session loop. Session.runToolCalls (packages/cli/src/acp-integration/session/Session.ts:2794) still only does within-batch dedupeToolCallsById(functionCalls) (:2801) — no getProviderToolCallId, no handledProviderToolCallIds, no getHistoryFunctionResponseIds seeding. (The PR's only change to that file is a prettier reflow of Session.test.ts.)

I confirmed this end-to-end: I ran the same duplicate-shell_1 mock through qwen serve (POST /session → POST /session/:id/prompt). The mock served both shell rounds and both executed → 2 side-effect writes, whereas qwen -p produced 1. So a user who hits #5014 through the web-shell or an ACP/IDE client (Zed, etc.) is still exposed. Recommend extending the same provider-id suppression into Session.runToolCalls, or explicitly scoping the daemon path as a follow-up in the PR description (it currently reads "Fixes #5014" unqualified).

🟡 Finding 2 — undocumented scope creep in the fix commit · Low–Med

The fix commit (4fedaa3ba) also changes packages/core/src/utils/filesearch/crawler.ts (+61: a new collectDirectoryRows that makes directories appear in file-search/@-completion results) and the matching useAtCompletion.test.ts, plus pure-prettier reflows in truncation.ts / toolResultCleanup.ts. None of this is mentioned in the PR (titled/scoped solely to duplicate tool-call ids). It is additive (it does not revert the recent #4596 submodule-recursion work, and filesearch tests pass), so it's not a regression — but it broadens the review/risk surface of a bug-fix PR. Recommend splitting the crawler/at-completion change out, or documenting it.

⚪ Finding 3 — dedup logic is duplicated across loops · Info

The same provider-id suppression is hand-written in three places (and missing from the fourth, per Finding 1). Factoring it into one shared helper would prevent both the daemon-path miss and future drift between the copies.

Recommendation

The fix itself is correct and I'd approve it for the CLI surfaces it covers — the deterministic E2E proves duplicate suppression, budget-neutrality, and precision (no over-suppression). Before calling #5014 fully fixed, please either extend the suppression to Session.runToolCalls (daemon/ACP) or re-scope the PR; and consider splitting out the unrelated crawler change.

Validated at 665014e93 (delta = commits 4fedaa3ba + 665014e93). Built core+cli dist fresh from PR source; E2E via node packages/cli + the repo's .qwen/skills/e2e-testing/scripts/mock-openai-server.js (duplicate-shell_1 scenario) over tmux; daemon path via qwen serve HTTP API. The daemon log's craft/drainMidTurnQueue "method not found" is a pre-existing unrelated ACP warning.

🇨🇳 中文版验证报告(点击展开)

✅ 验证报告 —— 确定性 mock provider 端到端 + 完整性审计

我从分支构建 dist,并把真实 CLI 指向一个 mock OpenAI server(连续两轮返回同一个 provider tool-call id shell_1,经 tmux 驱动),同时独立通读了去重逻辑。结论:修复在它改动到的路径上是正确、行为良好的 —— 但不完整:qwen serve / ACP daemon 路径仍会复现 #5014 详见下文;建议在认定 #5014 已彻底修复前,覆盖 daemon 路径(或显式限定范围)。

验证方式

范围 方法 结果
聚焦单测 vitest(PR 列出的文件) 通过 —— core turn/agent-headless/toolCallIdUtils 63 过(2 个既有 skip);cli nonInteractiveCli 51(1 skip)、useGeminiStream 110
E2E — 重复抑制 构建 dist/cli.js + mock 两次返回 shell_1--approval-mode yolo 第一次执行,第二次被抑制(synthetic error),副作用恰好写入 1 次tools.totalCalls=1,exit 0
E2E — 预算 --max-tool-calls 1 + 重复 运行成功totalCalls=1,写入 1 次(被忽略的 duplicate 不消耗预算)
E2E — 精确性对照 mock 返回 shell_1 再返回 shell_2(不同 id) 都执行,写入 2 次,totalCalls=2,无误抑制
E2E — 无 provider id mock 两轮都不带 id 都执行(生成的本地 id 唯一),写入 2 次 —— 不去重,符合声明
完整性审计 qwen serve daemon 跑同一个 mock ⚠️ 写入 2 次 —— #5014 在 daemon/ACP 路径复现(发现 1)

E2E 证据 —— qwen -p 跑重复 mock(本 PR 的主修复)

真实 --output-format json transcript(节选):

// 第 1 轮:shell_1 执行
{"role":"assistant","content":[{"type":"tool_use","id":"shell_1","name":"run_shell_command", ...}]}
{"role":"user","content":[{"type":"tool_result","tool_use_id":"shell_1","is_error":false,"content":"...Exit Code: 0..."}]}
// 第 2 轮:provider 再次发 shell_1 → 规范化 id 被加后缀、provider id 被保留 → 抑制
{"role":"assistant","content":[{"type":"tool_use","id":"shell_1__qwen_dup_2","name":"run_shell_command", ...}]}
{"role":"user","content":[{"type":"tool_result","tool_use_id":"shell_1__qwen_dup_2","is_error":true,
  "content":"Duplicate provider tool call id \"shell_1\" was already handled. ..."}]}
// 第 3 轮:成功
{"type":"result","is_error":false,"result":"All done. ...","stats":{"tools":{"totalCalls":1,...}}}

副作用文件恰好只有一个 HIT。机制是可靠的:normalizeModelToolCallIds 给跨轮重复的 callId 加后缀(shell_1__qwen_dup_2,正是从前导致重复执行的根源),同时把原始 provider id 存到一个不可枚举的 Symbol 上;agent-core / nonInteractiveCli / useGeminiStreamgetHistoryFunctionResponseIds() 播种 handledProviderToolCallIds,按 provider id 抑制。干净利落,synthetic 结果也正是 PR 描述的 error 形状。

🟠 发现 1 —— qwen serve / ACP daemon 路径未覆盖;#5014 在那里仍复现 ·

修复接入了三个 agentic loop(agent-core.ts:664nonInteractiveCli.ts:741useGeminiStream.ts:1687),但没有接入 ACP session loop。Session.runToolCallspackages/cli/src/acp-integration/session/Session.ts:2794)仍然只做批内 dedupeToolCallsById(functionCalls):2801)—— 没有 getProviderToolCallId、没有 handledProviderToolCallIds、没有 getHistoryFunctionResponseIds 播种。(PR 对该文件的唯一改动是 Session.test.ts 的 prettier 重排。)

我做了端到端确认:把同一个重复 shell_1 的 mock 通过 qwen serve 跑(POST /session → POST /session/:id/prompt)。mock 提供了两轮 shell,两轮都执行 → 副作用写入 2 次,而 qwen -p 只写 1 次。也就是说,通过 web-shell 或 ACP/IDE 客户端(如 Zed)触发 #5014 的用户仍然暴露在该问题下。建议把同样的 provider-id 抑制扩展进 Session.runToolCalls,或在 PR 描述里显式把 daemon 路径列为后续项(目前是无限定的 "Fixes #5014")。

🟡 发现 2 —— 修复 commit 里有未说明的范围蔓延 · 低–中

修复 commit(4fedaa3ba)还改了 packages/core/src/utils/filesearch/crawler.ts(+61:新增 collectDirectoryRows,让目录出现在文件搜索/@ 补全结果里)以及配套的 useAtCompletion.test.ts,外加 truncation.ts / toolResultCleanup.ts 的纯 prettier 重排。这些在 PR(标题/范围仅限重复 tool-call id)里只字未提。它是增量的(没有回退近期 #4596 的子模块递归工作,filesearch 测试通过),所以不是回归 —— 但它扩大了一个 bugfix PR 的评审/风险面。建议把 crawler/@ 补全改动拆出,或加以说明。

⚪ 发现 3 —— 去重逻辑在多个 loop 中重复 · 提示

同样的 provider-id 抑制被手写了三处(按发现 1,第四处缺失)。抽成一个共享 helper 能同时避免 daemon 路径漏改和将来各副本之间的漂移。

建议

修复本身是正确的,就其覆盖的 CLI 路径而言我会赞成合并 —— 确定性 E2E 证明了重复抑制、预算不消耗、以及精确性(无过度抑制)。在认定 #5014 彻底修复前,请把抑制扩展到 Session.runToolCalls(daemon/ACP),或重新限定 PR 范围;并考虑把无关的 crawler 改动拆出。

基于 665014e93 验证(增量 = commit 4fedaa3ba + 665014e93)。core+cli dist 从 PR 源码全新构建;E2E 用 node packages/cli + 仓库 .qwen/skills/e2e-testing/scripts/mock-openai-server.js(重复 shell_1 场景)经 tmux;daemon 路径走 qwen serve HTTP API。daemon 日志里的 craft/drainMidTurnQueue"method not found" 是既有的、无关的 ACP 警告。

Comment thread packages/core/src/utils/filesearch/crawler.ts Outdated
Comment thread packages/core/src/core/turn.ts
Co-authored-by: Shaojin Wen <shaojin.wensj@alibaba-inc.com>
wenshao
wenshao previously approved these changes Jun 15, 2026

@wenshao wenshao left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No issues found. LGTM! ✅ — qwen3.7-max via Qwen Code /review

DragonnZhang
DragonnZhang previously approved these changes Jun 15, 2026

@DragonnZhang DragonnZhang left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well-designed fix for duplicate provider tool-call IDs. Uses a non-enumerable Symbol (PROVIDER_TOOL_CALL_ID) to preserve the original provider-emitted id alongside the internally-generated normalized id. ToolCallRequestInfo gains providerCallId for dedup at execution time. createDuplicateProviderToolCallResponse returns a clean error response without executing the duplicate. Comprehensive test coverage in toolCallIdUtils.test.ts and turn.test.ts. Also includes doc formatting fixes and filesearch/crawler.ts cleanup. CI green. LGTM ✅ — claude-opus-4-6 via Qwen Code /review

@wenshao

wenshao commented Jun 15, 2026

Copy link
Copy Markdown
Collaborator

🧪 Local runtime verification (real CLI + mock provider, tmux)

I reproduced the PR's described E2E against the locally-built bundle (node dist/cli.js, built from this PR's core) talking to a mock OpenAI-compatible server in tmux, with --approval-mode yolo and an isolated HOME. The mock returns the same provider tool-call id shell_1 in two consecutive rounds, each running a side-effecting shell command (echo WRITE >> sidefile). Built on PR head bfa002816.

TL;DR

The fix works end-to-end and is correctly scoped to exact provider-id duplicates. Decisive A/B below. One non-blocking note: this PR also bundles an unrelated filesearch/crawler.ts change (a new collectDirectoryRows, wired into results at line 1330) plus a prettier-only truncation.ts reflow and doc edits — none related to tool-call ids; consider splitting them out.

Evidence 1 — Real-CLI A/B (side-effect writes are the ground truth)

Scenario provider ids across 2 rounds shell executions (writes) outcome
duplicate (the fix) shell_1, shell_1 1 succeeds; duplicate answered with synthetic result
duplicate, dedup neutered (≈ pre-PR) shell_1, shell_1 2 the #5014 bug — side effect runs twice
distinct ids (no false-dedup) shell_1, shell_2 2 both execute correctly
duplicate + --max-tool-calls 1 shell_1, shell_1 1 succeeds — duplicate does not consume the budget

The neutered row is the clincher: I surgically forced the suppression off and rebuilt — the same duplicate then executed the shell twice, exactly reproducing #5014; restoring the guard returned it to one write.

On the wire, the second round's tool result sent back to the model is the synthetic error (note the suffixed callId vs. the raw provider id as the idempotency key):

tool_call_id: shell_1            -> real shell result (Exit Code: 0)
tool_call_id: shell_1__qwen_dup_2 -> "Duplicate provider tool call id \"shell_1\" was already
                                      handled. The duplicate tool call was ignored and not
                                      executed again."

Evidence 2 — Design is sound (no false dedup)

The raw provider id is stashed on the function call via a non-enumerable Symbol and surfaced as ToolCallRequestInfo.providerCallId; the suffixed callId (…__qwen_dup_2) keeps the tool_use/tool_result pairing valid. Generated/fallback ids are always chosen fresh against history, so distinct calls are never collapsed — confirmed by the shell_1+shell_2 run (2 writes). The cross-round key store is getHistoryFunctionResponseIds(), so a reused id is caught even across separate tool-result rounds.

Evidence 3 — All three execution paths + test teeth

The same createDuplicateProviderToolCallResponse + getHistoryFunctionResponseIds() guard is applied in non-interactive (nonInteractiveCli.ts), interactive (useGeminiStream.ts), and agent/subagent (agent-core.ts). I verified non-interactive E2E directly; the other two are covered by the PR's unit tests:

  • turn.test.ts + agent-headless.test.ts + toolCallIdUtils.test.ts: 63 pass (2 skipped)
  • nonInteractiveCli.test.ts + useGeminiStream.test.tsx: 161 pass (1 skipped)
  • Mutation check: neutering getProviderToolCallId() (drop the provider id) fails exactly the 3 dedup tests — preserve raw provider ids, ignore duplicates across rounds, ignore duplicates already in history — so the suite genuinely pins the behavior.

Scope note (not a correctness issue)

git diff shows changes outside the stated fix: packages/core/src/utils/filesearch/crawler.ts (+59/−2) adds a real collectDirectoryRows that's wired into file-search results, truncation.ts is a prettier-only reflow, and there are two docs/ edits and some test-stabilization churn (useAtCompletion.test.ts, etc.). They look harmless but are unrelated to "ignore duplicate provider tool-call ids" — easier to review/revert if split into their own PR.

Verdict

Approve the fix. It correctly makes exact-duplicate provider tool-call ids idempotent within a loop, stops the #5014 repeated-side-effect amplification (verified: 2 → 1 write), doesn't over-dedup distinct calls, and doesn't waste the tool-call budget. Suggest splitting the unrelated crawler.ts/truncation.ts/docs changes out before merge.


🇨🇳 中文版(点击展开)

🧪 本地运行验证(真实 CLI + mock provider,tmux)

我用本地构建的 bundle(node dist/cli.js,基于本 PR 的 core)复现了 PR 描述的 E2E:在 tmux 里连一个 mock OpenAI 兼容 server,--approval-mode yolo、隔离 HOME。mock 连续两轮返回同一个 provider tool-call id shell_1,每轮都执行一个有副作用的 shell 命令(echo WRITE >> sidefile)。基于 PR head bfa002816

结论速览

修复端到端有效,且正确地只针对“provider id 完全相同”的重复。 决定性 A/B 见下。一个不阻塞的提醒: 本 PR 还夹带了一个无关filesearch/crawler.ts 改动(新增 collectDirectoryRows,在第 1330 行接入结果)、一个纯 prettier 的 truncation.ts 重排、以及文档改动——都和 tool-call id 无关,建议拆分。

证据 1 —— 真实 CLI A/B(副作用写入次数是 ground truth)

场景 两轮的 provider id shell 执行次数(写入) 结果
重复(修复后) shell_1, shell_1 1 成功;重复调用收到 synthetic result
重复,关掉去重(≈ 修复前) shell_1, shell_1 2 #5014 的 bug——副作用执行两次
不同 id(不应被误去重) shell_1, shell_2 2 两个都正常执行
重复 + --max-tool-calls 1 shell_1, shell_1 1 成功——重复消耗预算

“关掉去重”那一行是关键:我手术式把抑制关掉再重建,同一个重复就把 shell 执行了两次,精确复现 #5014;恢复后又回到一次写入。

线上看,第二轮回传给模型的 tool result 是 synthetic error(注意后缀化的 callId 与作为幂等键的 raw provider id):

tool_call_id: shell_1            -> 真实 shell 结果 (Exit Code: 0)
tool_call_id: shell_1__qwen_dup_2 -> "Duplicate provider tool call id \"shell_1\" was already
                                      handled. The duplicate tool call was ignored and not
                                      executed again."

证据 2 —— 设计正确(不会误去重)

raw provider id 通过一个不可枚举的 Symbol 挂在 function call 上,并以 ToolCallRequestInfo.providerCallId 暴露;后缀化callId…__qwen_dup_2)保证 tool_use/tool_result 配对仍然合法。生成/fallback 的 id 总是相对 history 取新的,所以不同的调用不会被合并——shell_1+shell_2 那次(2 写入)确认了这一点。跨轮的键库是 getHistoryFunctionResponseIds(),所以即使跨不同的 tool-result round,复用的 id 也能被抓到。

证据 3 —— 三条执行路径 + 测试牙齿

同一套 createDuplicateProviderToolCallResponse + getHistoryFunctionResponseIds() 守卫被用在 non-interactivenonInteractiveCli.ts)、interactiveuseGeminiStream.ts)、agent/subagentagent-core.ts)。我直接 E2E 验证了 non-interactive;另两条由 PR 的单测覆盖:

  • turn.test.ts + agent-headless.test.ts + toolCallIdUtils.test.ts63 通过(2 skip)
  • nonInteractiveCli.test.ts + useGeminiStream.test.tsx161 通过(1 skip)
  • 变异测试:getProviderToolCallId() 改成丢弃 provider id,恰好让 3 个去重测试失败——保留 raw provider id跨轮忽略重复忽略 history 里已有的重复——说明这套测试是真的钉住了行为。

范围提醒(非正确性问题)

git diff 里有超出本修复范围的改动:packages/core/src/utils/filesearch/crawler.ts(+59/−2)新增了一个真正接入文件搜索结果的 collectDirectoryRowstruncation.ts 是纯 prettier 重排,还有两处 docs/ 改动和一些测试稳定化的 churn(useAtCompletion.test.ts 等)。它们看起来无害,但和“ignore duplicate provider tool-call ids”无关——拆成独立 PR 更好审查/回退。

结论

同意合并这个修复。 它正确地让“完全相同的 provider tool-call id”在一个 loop 内幂等,止住了 #5014 的副作用放大(已验证:2 → 1 次写入),不会误去重不同的调用,也不会浪费 tool-call 预算。建议合并前把无关的 crawler.ts/truncation.ts/docs 改动拆出去。

Method: locally-built dist/cli.js (this PR's core) + a mock OpenAI server returning a repeated provider tool-call id across two rounds, in tmux; side-effect write count as ground truth; surgical dedup-neuter counterfactual + distinct-id + --max-tool-calls 1 controls; PR's vitest suites + a stub-mutation teeth check. PR head bfa002816.

@YingchaoX
YingchaoX dismissed stale reviews from wenshao and DragonnZhang via cdbeb48 June 17, 2026 10:15
@wenshao

wenshao commented Jun 17, 2026

Copy link
Copy Markdown
Collaborator

@copilot resolve the merge conflicts in this pull request

2 similar comments
@wenshao

wenshao commented Jun 18, 2026

Copy link
Copy Markdown
Collaborator

@copilot resolve the merge conflicts in this pull request

@wenshao

wenshao commented Jun 18, 2026

Copy link
Copy Markdown
Collaborator

@copilot resolve the merge conflicts in this pull request

@wenshao

wenshao commented Jun 18, 2026

Copy link
Copy Markdown
Collaborator

@qwen-code /triage

@qqqys qqqys left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Prior critical blockers remain resolved on the latest head. The duplicate provider tool-call guard now covers the TUI, non-interactive/headless, AgentCore, and ACP session paths before scheduling local side effects; the earlier typecheck blocker is also gone, and current CI is green. I did not find any new critical issue in this pass.

@qwen-code-ci-bot

Copy link
Copy Markdown
Collaborator

Thanks for the PR!

Template looks good ✓ — all required sections filled in with bilingual detail.

On direction: this is a clear, real bug fix. #5014 shows models returning the same provider tool-call ID across rounds, amplifying one mistake into hundreds of repeated shell executions. That's a safety-relevant issue for any user running with --approval-mode yolo. Well within qwen-code's core mission of safe agentic execution.

On approach: the Symbol-based provider ID preservation + handledProviderToolCallIds set is the right design. I like that it keeps the normalized callId separate from the raw provider ID — the conversation history stays valid while the dedup key is tracked independently. Coverage across all four execution paths (agent-core, nonInteractiveCli, useGeminiStream, Session/daemon) addresses the completeness gap that earlier reviewers flagged.

One concern: the diff carries an unrelated crawler.ts change (collectDirectoryRows adding directories to file-search/@-completion results) plus useAtCompletion.test.ts changes. These have nothing to do with duplicate tool-call IDs and broaden the review/risk surface of a focused bugfix PR. Consider splitting them into a separate PR for cleaner review and revert history.

Moving on to code review and testing. 🔍

中文说明

感谢贡献!

模板完整 ✓ — 所有必填章节都有双语详细说明。

方向:这是一个明确的、真实的 bug 修复。#5014 显示模型跨轮次返回相同的 provider tool-call ID,把一次错误放大为数百次重复 shell 执行。对于使用 --approval-mode yolo 的用户来说,这是一个安全相关的问题,完全符合 qwen-code 安全代理执行的核心使命。

方案:基于 Symbol 的 provider ID 保留 + handledProviderToolCallIds 集合是正确的设计。将规范化的 callId 与原始 provider ID 分离是个好思路——对话历史保持有效,同时独立跟踪去重键。覆盖了所有四条执行路径(agent-core、nonInteractiveCli、useGeminiStream、Session/daemon),解决了早期评审者指出的完整性缺口。

一个顾虑:diff 中包含无关的 crawler.ts 改动collectDirectoryRows 在文件搜索/@ 补全结果中添加目录)以及 useAtCompletion.test.ts 改动。这些和重复 tool-call ID 毫无关系,扩大了一个聚焦 bugfix PR 的审查/风险面。建议拆成独立 PR,便于审查和回退。

进入代码审查和测试阶段 🔍

Qwen Code · qwen3.7-max

@qwen-code-ci-bot

Copy link
Copy Markdown
Collaborator

Code Review

Independent proposal before reading the diff: I'd track provider tool-call IDs via a non-enumerable Symbol on the FunctionCall object, add providerCallId to ToolCallRequestInfo, create a shared helper for the synthetic duplicate response, and wire the check into each of the four agentic loops — seeding from getHistoryFunctionResponseIds() to catch cross-session duplicates.

The PR's approach matches this exactly. The Symbol-based storage in toolCallIdUtils.ts is clean — PROVIDER_TOOL_CALL_ID as a non-enumerable property means it survives object spreads without polluting JSON serialization or conversation history. The getProviderToolCallId() accessor is the single read path. createDuplicateProviderToolCallResponse() centralizes the synthetic error shape, keeping it consistent across all four loops.

The Session.ts daemon path is now properly covered (earlier reviewers flagged this gap — it's been closed). The ExecutableBatch/DuplicateBatch discriminated union is a clean way to thread duplicate handling through the existing batch execution flow without major refactoring.

No correctness bugs or security issues found. Tests are thorough — 63 core tests, 158 Session tests, 123 useGeminiStream tests, 51 nonInteractiveCli tests, all passing. Build and typecheck clean.

One note on the crawler.ts change: it adds real new functionality (collectDirectoryRows) that's orthogonal to the tool-call ID fix. Not a correctness concern, but it should be in its own PR.

E2E Test Results

Real CLI against a mock OpenAI server returning the same provider tool-call ID (shell_1) in two consecutive rounds, each calling run_shell_command with echo HIT >> /tmp/triage-sidefile.

Before (installed qwen v0.18.3, main branch)

Round 1: id=shell_1              → executed (HIT #1)
Round 2: id=shell_1__qwen_dup_2  → executed (HIT #2)  ← BUG: re-executed
Round 3: "All done."
Stats: totalCalls=2, totalSuccess=2
Side-effect file: 2 HITs

Full transcript key excerpt:

{"type":"tool_use","id":"shell_1","name":"run_shell_command",...}
{"type":"tool_result","tool_use_id":"shell_1","is_error":false,...}
{"type":"tool_use","id":"shell_1__qwen_dup_2","name":"run_shell_command",...}
{"type":"tool_result","tool_use_id":"shell_1__qwen_dup_2","is_error":false,...}

Both tool calls executed. The ID was renamed (__qwen_dup_2) but the shell command still ran twice.

After (PR #5038 via npm run dev from worktree)

Round 1: id=shell_1              → executed (HIT #1)
Round 2: id=shell_1__qwen_dup_2  → SUPPRESSED (is_error=true)  ← FIX
Round 3: "All done."
Stats: totalCalls=1, totalSuccess=1
Side-effect file: 1 HIT (only from this run)

Full transcript key excerpt:

{"type":"tool_use","id":"shell_1","name":"run_shell_command",...}
{"type":"tool_result","tool_use_id":"shell_1","is_error":false,...}
{"type":"tool_use","id":"shell_1__qwen_dup_2","name":"run_shell_command",...}
{"type":"tool_result","tool_use_id":"shell_1__qwen_dup_2","is_error":true,
 "content":"Duplicate provider tool call id \"shell_1\" was already handled.
           The duplicate tool call was ignored and not executed again."}

The fix works exactly as described: first call executes, duplicate gets a synthetic error response, totalCalls=1 confirms the ignored duplicate doesn't consume the tool-call budget.

中文说明

代码审查

在看 diff 之前的独立方案:我会通过 FunctionCall 对象上的不可枚举 Symbol 来跟踪 provider tool-call ID,在 ToolCallRequestInfo 中添加 providerCallId,创建一个共享的 synthetic duplicate response helper,并在四个 agentic loop 中接入检查——从 getHistoryFunctionResponseIds() 播种以捕获跨会话重复。

PR 的方案与此完全一致。toolCallIdUtils.ts 中基于 Symbol 的存储很干净——PROVIDER_TOOL_CALL_ID 作为不可枚举属性,在对象展开时存活,但不会污染 JSON 序列化或对话历史。getProviderToolCallId() 是唯一的读取路径。createDuplicateProviderToolCallResponse() 集中了 synthetic error 的形状,在所有四个 loop 中保持一致。

Session.ts 的 daemon 路径现在已正确覆盖(早期评审者标记了这个缺口——已经修复)。ExecutableBatch/DuplicateBatch 可辨识联合是在现有批执行流程中线程化重复处理的干净方式,不需要大规模重构。

未发现正确性 bug 或安全问题。测试覆盖充分——63 个 core 测试、158 个 Session 测试、123 个 useGeminiStream 测试、51 个 nonInteractiveCli 测试,全部通过。Build 和 typecheck 干净。

关于 crawler.ts 改动的一点说明:它添加了真正的新功能(collectDirectoryRows),与 tool-call ID 修复正交。不是正确性问题,但应该放在自己的 PR 里。

E2E 测试结果

真实 CLI 对 mock OpenAI 服务器,连续两轮返回相同的 provider tool-call ID(shell_1),每轮调用 run_shell_command 执行 echo HIT >> /tmp/triage-sidefile

Before(已安装的 qwen v0.18.3,main 分支)

第 1 轮:id=shell_1              → 执行(HIT #1)
第 2 轮:id=shell_1__qwen_dup_2  → 执行(HIT #2)  ← BUG:重复执行
第 3 轮:"All done."
统计:totalCalls=2, totalSuccess=2
副作用文件:2 HITs

两个工具调用都执行了。ID 被重命名(__qwen_dup_2)但 shell 命令仍然运行了两次。

After(PR #5038 通过 worktree 的 npm run dev

第 1 轮:id=shell_1              → 执行(HIT #1)
第 2 轮:id=shell_1__qwen_dup_2  → 已抑制(is_error=true)  ← 修复
第 3 轮:"All done."
统计:totalCalls=1, totalSuccess=1
副作用文件:1 HIT(仅来自本次运行)

修复完全如描述:第一次调用执行,重复调用收到 synthetic error 响应,totalCalls=1 确认被忽略的重复不消耗工具调用预算。

Qwen Code · qwen3.7-max

@qwen-code-ci-bot

Copy link
Copy Markdown
Collaborator

Reflection

Stepping back: this PR solves a real, safety-relevant bug with a clean mechanism. The Symbol-based provider ID preservation is the right abstraction — it's invisible to JSON serialization, survives object spreads, and has a single read accessor. The suppression logic is consistent across all four execution paths, and the earlier reviewers' concern about the daemon/ACP path has been addressed.

The E2E evidence is decisive: before = 2 shell executions / totalCalls=2, after = 1 shell execution / totalCalls=1 with the duplicate getting a clean synthetic error response. The --max-tool-calls budget neutrality is a nice touch — the ignored duplicate doesn't consume the user's tool-call limit.

My independent proposal matched the PR's approach almost exactly, which gives me confidence the design is sound. The test suite has real teeth — neutering getProviderToolCallId() fails exactly the dedup tests, proving the behavior is pinned.

The one reservation is the crawler.ts / collectDirectoryRows change bundled into a bugfix PR. It's additive and harmless, but it's a real feature that deserves its own review cycle. If a maintainer is comfortable accepting it as part of this PR, that's fine — but it should at least be called out explicitly rather than silently riding along.

Overall: the fix is correct, well-tested, and ships a meaningful safety improvement. The scope creep is minor and non-blocking.

Verdict: Approve ✅ — with a request to split the crawler.ts change into a separate PR if possible.

中文说明

反思

退一步看:这个 PR 用干净的机制解决了一个真实的、安全相关的 bug。基于 Symbol 的 provider ID 保留是正确的抽象——对 JSON 序列化不可见,在对象展开时存活,并且有单一的读取访问器。抑制逻辑在所有四条执行路径上一致,早期评审者关于 daemon/ACP 路径的顾虑也已解决。

E2E 证据是决定性的:before = 2 次 shell 执行 / totalCalls=2,after = 1 次 shell 执行 / totalCalls=1,重复调用收到干净的 synthetic error 响应。--max-tool-calls 预算中性是一个不错的细节——被忽略的重复不消耗用户的工具调用限额。

我的独立方案与 PR 的方案几乎完全一致,这给了我对设计正确性的信心。测试套件有真正的牙齿——禁用 getProviderToolCallId() 恰好让去重测试失败,证明行为被牢牢钉住。

唯一的保留意见是 crawler.ts / collectDirectoryRows 改动被打包进了一个 bugfix PR。它是增量的且无害的,但它是一个真正的功能,值得有自己的审查周期。如果维护者愿意接受它作为这个 PR 的一部分,那没问题——但至少应该明确指出来,而不是默默地搭便车。

总结:修复正确、测试充分、带来了有意义的安全改进。范围蔓延很小,不阻塞合并。

结论:批准 ✅ — 建议尽可能把 crawler.ts 改动拆到独立 PR。

Qwen Code · qwen3.7-max

@qwen-code-ci-bot qwen-code-ci-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, looks ready to ship. ✅ The fix correctly suppresses duplicate provider tool-call IDs across all four execution paths (verified E2E: 2→1 execution, totalCalls=1). Suggest splitting the unrelated crawler.ts change into a separate PR when convenient.

@wenshao
wenshao merged commit d0a3cd6 into QwenLM:main Jun 18, 2026
24 checks passed
doudouOUC added a commit that referenced this pull request Aug 19, 2026
…matching args

The duplicate provider tool-call guard (#5038/#5657) keyed on the id
alone, so models whose ids are only unique within a single response —
e.g. Kimi emits {name}_{index} and the index can restart at 0 on any
round — had fresh calls misclassified as replays: the second collision
got a synthetic duplicate error and the third tripped the circuit
breaker, killing every turn by round three.

A handled id now maps to a (name, canonical args) fingerprint — the
same sha256 repeat key the loop guards use, moved to a leaf module so
toolCallIdUtils can share it without an import cycle. An incoming call
is a replay only when its fingerprint matches the call that first
executed under that provider id; id collisions with different args
execute normally under the unique suffixed id that normalization
already assigns. Exact same-args replays keep the unchanged #5014
suppression and #5657 breaker behavior at all four entry points
(AgentCore, TUI stream, non-interactive CLI, ACP daemon session).

The synthetic duplicate message now tells the model to re-issue with a
fresh tool-call id when a new invocation was intended, giving
id-emitting models a recovery path.
samuelhsin pushed a commit to samuelhsin/qwen-code that referenced this pull request Aug 19, 2026
…n arguments match (QwenLM#9436)

* fix(core): treat duplicate provider tool-call ids as replays only on matching args

The duplicate provider tool-call guard (QwenLM#5038/QwenLM#5657) keyed on the id
alone, so models whose ids are only unique within a single response —
e.g. Kimi emits {name}_{index} and the index can restart at 0 on any
round — had fresh calls misclassified as replays: the second collision
got a synthetic duplicate error and the third tripped the circuit
breaker, killing every turn by round three.

A handled id now maps to a (name, canonical args) fingerprint — the
same sha256 repeat key the loop guards use, moved to a leaf module so
toolCallIdUtils can share it without an import cycle. An incoming call
is a replay only when its fingerprint matches the call that first
executed under that provider id; id collisions with different args
execute normally under the unique suffixed id that normalization
already assigns. Exact same-args replays keep the unchanged QwenLM#5014
suppression and QwenLM#5657 breaker behavior at all four entry points
(AgentCore, TUI stream, non-interactive CLI, ACP daemon session).

The synthetic duplicate message now tells the model to re-issue with a
fresh tool-call id when a new invocation was intended, giving
id-emitting models a recovery path.

* qwen: address PR review feedback (QwenLM#9436)

- Fingerprint each incoming call once per carrier object: the WeakMap
  cache now keys on any stable carrier (FunctionCall part or
  ToolCallRequestInfo), and the replay predicate / recording helpers
  take the precomputed fingerprint instead of rehashing (name, args)
  on every breaker scan, admission pass, and record.
- Move the getToolCallRepeatKey tests next to the extracted leaf
  module instead of exercising it through the loop detection service's
  compatibility re-export.
- Restore the assertion pinning that runToolCalls never mutates the
  history accessor's returned fingerprint map, now that the defensive
  copy is load-bearing.

* qwen: address PR review feedback (QwenLM#9436)

Criticals:
- Canonicalize onto a null-prototype object so a literal __proto__ own
  key (preserved by JSON.parse) stays a data property instead of
  vanishing through the inherited setter — two calls differing only in
  __proto__ no longer collide on one repeat key, which the replay
  oracle would have turned into a wrongly suppressed execution.
- Clone request args at scheduler intake: callers pass args that can
  alias the model-emitted functionCall part stored in chat history, and
  the executor rewrites PATH_ARG_KEYS on request.args in place (a
  persistence the post-'ask' bounce re-execution relies on). Without
  the clone those rewrites leak into history and skew the replay
  fingerprints derived from it, letting genuine replays of
  path-carrying calls re-execute in multi-round agent runtimes.

Suggestions:
- Complete the duplicate message with the different-arguments recovery
  path required by design decision D3, for models whose provider
  assigns ids.
- Fix the copy-convention comments (the accessor returns a fresh map
  per call; copies are future-proofing) and align all four entry
  points on copying the accessor result.
- Pin the untested branches: history first-occurrence-wins for reused
  ids and orphan response-id exclusion, the fingerprint cache-hit path,
  the __proto__ distinction, the caller-args no-mutation invariant, and
  a cross-round runtime test that a replay of the original call stays
  suppressed after an id-colliding execution.
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.

Qwen Code executes duplicate tool calls

5 participants