fix(core): treat duplicate provider tool-call ids as replays only when arguments match - #9436
Conversation
E2E test reportMock OpenAI-compatible SSE provider returning tool-call id Scenario 1 — same id, different args each round (the false positive)Before (released After (this branch Scenario 2 — same id, identical args each round (a true replay)After (this branch #5014 suppression and the #5657 circuit breaker are unchanged for true replays; only the different-args misclassification is gone. Unit suites
Tested on macOS; Windows/Linux via CI. |
|
Re-run after the deferred sequence completed — the gate passes again, and this time the evidence exists. Template: complete ✓ (including the bilingual section). Problem: unchanged from the prior pass — an observed bug with concrete evidence, not theory. Live daemon sessions showed the Direction: aligned. Direct continuation of the duplicate-provider-id guard chain already in the CHANGELOG, implementing decisions D2/D3 of the committed design doc. The stacked base #9435 has merged, so this PR now stands on Size: core paths touched; 430 production lines across 11 files vs 948 test lines across 10 files (no generated/schema changes). Under the 500-line maintainer-awareness threshold. The growth since the prior pass (was 383 production lines) is entirely review-feedback artifacts — the scheduler-intake clone fixing one of the two Criticals, the fingerprint precomputation addressing the recomputation Suggestions, and their tests — not scope creep. Approach: scope remains right, and I verified the rebase claim independently rather than taking it on faith: fetching the previously reviewed head Risk: Stage 1e still matches two high-risk paths — Moving on to code review. 🔍 中文说明延期序列完成后的重新运行——门禁再次通过，且这次证据已经存在。 模板：完整 ✓（含中文部分）。 问题：与上一轮结论一致——已观测到的 bug，有具体证据而非理论推演。线上 daemon 会话呈现 方向：对齐。是 CHANGELOG 中既有 duplicate-provider-id 防护链的直接延续，实现已提交设计文档的 D2/D3 决策。堆叠基础 #9435 已合并，本 PR 现在独立立足于 规模：触及核心路径；11 个文件 430 行生产代码 vs 10 个文件 948 行测试代码（无生成/schema 改动）。低于 500 行维护者关注阈值。相比上一轮（383 行生产代码）的增量全部来自 review 反馈产物——修复两个 Critical 之一的调度器入口克隆、处理重算 Suggestion 的指纹预计算，以及相应测试——不是范围蔓延。 方案：范围仍然合理，且我独立核实了 rebase 论断而非照单接受：取回此前审查过的 head 风险：Stage 1e 仍命中两个高风险路径—— 进入代码审查 🔍 — Qwen Code · qwen3.8-max Reviewed at |
Code reviewThe design still matches the proposal I'd write before reading the diff — replay identity as (provider id, canonical name+args fingerprint), reusing the loop guards' own repeat key, applied uniformly at all four entry points — so this pass focused on what moved since the last review: the rebase and the two review-feedback rounds. Both hold up under verification. The rebase is clean. I fetched the previously reviewed head Both Criticals from the review rounds are genuinely fixed, not patched around:
The 13 Suggestion threads are all verifiably addressed — I checked each against the diff: the redundant-copy comments now state the truth and all four entry points share one copy convention; the deleted no-mutation pin in One standing note for the maintainer, unchanged from the prior pass: Files changed (21 of 21 shown)
TestingWhat this comment carries: the PR's own CI, fetched once via the API at the reviewed SHA — and it is green. The prior pass deferred for exactly one reason: no CI could exist while the base was #9435's branch. That condition is gone — the branch was retargeted to One precision note on the trigger comment's "three-OS test matrix": per
The one lane that produced nothing: the earlier 中文说明代码审查设计仍与我在读 diff 前会写出的方案一致——重放身份 = (provider id, 规范化 name+args 指纹)，复用 loop 守卫自己的 repeat key，四个入口统一应用——因此本轮聚焦上次审查之后变动的部分：rebase 与两轮 review 反馈。两者都经得起核实。 **rebase 干净。**我取回此前审查过的 head 两轮 review 的两个 Critical 是真修复，不是绕过：
13 条 Suggestion 线程均已可核实地处理——逐条对照 diff 检查过：冗余拷贝注释改为如实描述且四入口统一拷贝约定； 一条留给维护者的既有提醒（与上轮一致）： 测试**本评论携带的是：经 API 在被审 SHA 上一次性读取的 PR 自身 CI——绿了。**上一轮延期的唯一原因正是：base 为 #9435 分支时不可能有任何 CI。该条件已消失——分支重定 base 到 关于触发评论中"三 OS 测试矩阵"的一点精确化说明：按 曾有一条通道没有产出：此前的 — Qwen Code · qwen3.8-max Reviewed at |
|
Confidence: 4/5 — everything a gate can check is now green: both review rounds' findings verified fixed at this head, full CI green on the exact SHA reviewed, rebase patch-proven identical to the previously reviewed commit. The missing fifth point is a maintainer judgment item (the public-export surface note), not doubt about the code. Stepping back: the prior pass deferred for exactly one reason — the stacked base meant no CI existed or could exist for the commit, and the behavioural claim rested on the author's report alone. The sequence that deferral prescribed is now complete, and I checked each link rather than trusting the summary: #9435 is in Three items for the maintainer, none blocking:
Verdict: approve — pinned to the reviewed commit. ✅ 中文说明置信度：4/5 —— 门禁能查的一切现已全绿：两轮 review 的全部发现已核实修复于当前 head、完整 CI 在被审 SHA 上绿、rebase 经补丁对比证明与此前审查过的 commit 一致。缺的一点是维护者判断题（公共导出面的提醒），而非对代码的疑虑。 退一步看：上一轮延期的唯一原因——堆叠 base 导致该 commit 没有也不可能有 CI，行为论断仅依赖作者报告。该延期规定的序列现已完成，且我逐一核实而非照单接受：#9435 已在 三项留给维护者，均不阻断：
结论：批准——钉住被审 commit。✅ — Qwen Code · qwen3.8-max Reviewed at |
|
✅ Resolved by the 2026-08-19 re-run — the sequence this deferral prescribed is complete: #9435 merged, branch rebased onto ⏸️ Deferring to @ytahdn — the review itself is clean, but three things need a human call before this can ship:
cc @doudouOUC — nothing to fix in the code; this is an evidence/process gate, not a rejection. |
ytahdn
left a comment
ytahdn
left a comment
There was a problem hiding this comment.
总体结论：✅ 批准 (Approve)
中文
🎉 精准的根因修复。问题背景清晰：部分模型的 tool-call id 只在单次响应内唯一（如 run_shell_command_{index} 每轮从 0 重新计数），旧的仅凭 id 判重逻辑把每轮的新调用误判为重放，合成重复错误随后触发熔断器，导致会话在第 3 轮即被杀掉。本 PR 将判据升级为 重放 = 相同 provider id 且相同 (name, args) 规范指纹；id 碰撞但参数不同视为新调用，以 __qwen_dup_ 后缀 id 正常执行。
🎉 四个入口接线一致：subagent runtime（agent-core.ts）、TUI stream（useGeminiStream.ts）、非交互 CLI、ACP Session 全部使用同一套 isReplayOfHandledToolCall / recordHandledToolCall 原语与"首次出现为准"的 id 语义。交叉验证确认旧的 Set 访问器（getHistoryFunctionResponseIds）仅保留在 REPAIR 去重路径——那里匹配的是本地 callId 空间，保留 id-only 是正确的，无遗漏调用点。
🎉 工程质量扎实：sha256 指纹基于规范化工具名（含 legacy 别名解析）+ 键排序的参数哈希（数组顺序保留）；getToolCallRepeatKey 抽到叶子模块规避 import cycle 并保留 re-export 兼容；WeakMap 按 FunctionCall 对象缓存指纹（历史遍历只读不改对象，缓存安全），避免长会话中每批重复哈希大参数；合成重复响应消息新增"如需重跑请用新 id 重新发起"的引导，利于模型自我恢复。
🎉 测试覆盖充分：每个入口均有"id 碰撞但参数不同 → 正常执行"的新用例；工具层单测覆盖键序无关、首现保留；turn 层新增关键用例"合成响应已发过但指纹不匹配时不触发熔断"，正是本次修复的核心语义边界。
💬 少量非阻塞观察：
🟢 Session.ts / useGeminiStream.ts 中 new Map(getHistoryToolCallFingerprints()) 的防御性拷贝实际是冗余的——该访问器每次调用都新建 Map。拷贝无害，但注释"避免改动 accessor 持有的 map"与实现不完全相符，可在后续顺手修正注释。
💡 已知设计权衡（PR 描述已声明，此处确认）：碰撞调用（以后缀 id 执行的那次）若被模型原样重放，会逃过 id 层检测再次执行——因为 map 中该 provider id 记录的是首次调用的指纹。该缺口由与 id 无关的循环守卫（repeat-key 同名同参检测）兜底。"宁可放行重放、不误杀新调用"的取舍是正确的：误杀正是本 PR 要修复的原始 bug。
📚 语义要点：首现语义意味着一个 id 永久命名第一个以它执行的调用；配合按 prompt/submit 作用域清零的 duplicateProviderToolCallResponseIds，跨轮行为是确定的。
English
Verdict: ✅ Approve
🎉 Precise root-cause fix. Some models emit tool-call ids that are only unique per response (run_shell_command_{index} restarting at 0 each round), so the old id-only replay check misclassified every fresh call as a replay, and the circuit breaker killed sessions by round 3. This PR upgrades the predicate to replay = same provider id AND same canonical (name, args) fingerprint; an id collision with different args is a fresh call and executes under a __qwen_dup_-suffixed id.
🎉 Consistent wiring across all four entry points (agent-core.ts, useGeminiStream.ts, nonInteractiveCli.ts, ACP Session), all sharing the same isReplayOfHandledToolCall / recordHandledToolCall primitives and first-occurrence id semantics. Cross-verified that the old Set accessor survives only in the REPAIR dedup path, where matching against the local callId space makes id-only correct — no missed call sites.
🎉 Solid engineering: sha256 fingerprints over canonical tool name (legacy alias resolution included) + key-sorted args (array order preserved); getToolCallRepeatKey extracted to a leaf module to avoid the import cycle while keeping a re-export for compatibility; WeakMap fingerprint cache keyed on FunctionCall objects (safe since history walks are read-only) avoids re-hashing large args per batch; the synthetic duplicate message now tells the model to re-issue with a new id — a genuine recovery improvement.
🎉 Thorough tests: every entry point gains an "id collision with different args executes" case; unit tests cover key-order invariance and first-occurrence retention; turn.ts gains the critical "synthetic response already sent but fingerprint mismatch does not trip the breaker" case — exactly this PR's semantic boundary.
Non-blocking notes:
🟢 The new Map(getHistoryToolCallFingerprints()) defensive copies in Session.ts / useGeminiStream.ts are redundant — the accessor builds a fresh Map per call. Harmless, but the "accessor-owned map" comment doesn't quite match the implementation; worth a follow-up tweak.
💡 Known design trade-off (acknowledged in the PR description, confirming here): an exact replay of a colliding call escapes the id layer and executes again, because the map stores the first call's fingerprint under that provider id. This gap is bounded by the id-independent loop guard (repeat-key detection on name+args). Preferring false negatives over false positives is the right call — false positives were the original bug.
📚 Semantic note: first-occurrence semantics mean an id permanently names the call that first executed under it; combined with duplicateProviderToolCallResponseIds being cleared per prompt/submit, cross-turn behavior is deterministic.
ytahdn
left a comment
ytahdn
left a comment
There was a problem hiding this comment.
审查结论：💬 评论 —— 无阻塞项
这是经由独立审查管线的复审结果（纯静态、按模块分批交叉验证），与先前的 APPROVE 结论一致：未发现 Critical/Important 问题，仅两条维护性 Nit，供后续顺手处理。
中文
🎉 做得好的地方
- 重放判据与循环守卫复用同一个 repeat key：指纹函数抽到叶子模块规避 import 环、原位 re-export 保持兼容，"重放"与"循环"两处定义完全一致，没有第二套指纹逻辑。
- 四个入口（subagent 运行时、TUI 流、非交互 CLI、ACP Session）接线一致：熔断调用点、replay 分支、record 位置、空 id 守卫全部对齐；旧 Set 的
.has/.add语义在 Map 版本中等价重建。 - 删除行为审计通过：合成 duplicate 消息追加恢复引导后，全库 20+ 处远端匹配均用前缀
toContain，前缀保留，无形状失配。 - WeakMap 指纹缓存安全：历史遍历只读不改 part 对象，压缩/redact 构造新对象触发缓存重算而非 stale；大参数每个调用只哈希一次。
- 测试从两个方向钉住新契约：每个入口都有"碰撞但参数不同→执行"的新用例，既有"同参重放→压制/熔断"用例改为指纹 mock 后语义不变；turn 层新增"合成响应已发过但指纹不匹配→不触发熔断"用例，正中本次修复的语义边界。
🟢 Nit 1：防御性拷贝冗余、注释与实现不符、四端不对称
Session.ts 与 useGeminiStream.ts 中 new Map(getHistoryToolCallFingerprints()) 的注释称"避免改动 accessor 持有的 map"，但该 accessor 每次调用都新建 Map，不存在共享 map；而 agent-core.ts 与 nonInteractiveCli.ts 直接使用返回值。无行为问题，但注释前提为假；若未来有人把 accessor 改为缓存复用，真正需要拷贝的反而是没拷贝的两端。建议统一去掉拷贝，或把注释改为如实描述。
🟢 Nit 2：replay 谓词闭包四端重复，值得提取
"取 provider id → 空值短路 → isReplayOfHandledToolCall"的闭包在四处独立内联（fc 形态两处逐字相同）。非交互 CLI 一端的取 id 带 structured-output 特判，其余三端没有——目前各端内部自洽无错位，但未来调整 replay 语义需四处同步，漏改即四端行为分叉。建议在 toolCallIdUtils.ts 增加泛型工厂或 fc 形态的现成 helper。
📚 语义收窄说明（非问题）：历史指纹 map 从"所有 functionResponse id"收窄为"call/response 成对的 id"——孤儿 response id 不再视为 handled。异常历史形状下失败方向是"多执行一次"（安全方向），与 PR 主旨一致。
结论：无阻塞项；两条 Nit 不阻塞合入。提醒合并顺序：本 PR 堆叠于基础 PR（9435）之上，需先合入基础 PR。
English
Verdict: 💬 Comment — no blockers
Follow-up review via an independent review pipeline (static-only, module-batched with cross-verification). Consistent with the earlier APPROVE: no Critical/Important issues, two maintenance nits for a future pass.
🎉 What's done well
- The replay predicate reuses the exact repeat key the loop guards use — the fingerprint function was extracted to a leaf module to avoid the import cycle while keeping a re-export for compatibility, so "replay" and "loop" share one definition with no second fingerprinting logic.
- All four entry points (subagent runtime, TUI stream, non-interactive CLI, ACP Session) are wired consistently: breaker call sites, replay branches, record positions, and empty-id guards all align; the old Set
.has/.addsemantics are faithfully rebuilt in the Map version. - Removed-behavior audit passes: after the recovery guidance was appended to the synthetic duplicate message, all 20+ remote matches across the repo use prefix
toContain, and the prefix is preserved — no shape mismatch. - The WeakMap fingerprint cache is safe: history walks never mutate part objects, compression/redaction construct new objects (cache recompute, not stale), and large args are hashed once per call.
- Tests pin the new contract from both directions: every entry point gains an "id collision with different args executes" case alongside the existing "exact replay suppressed/breaker" cases (now fingerprint-mocked with unchanged semantics); the new turn-level case "synthetic response already sent but fingerprint mismatch does not trip the breaker" lands exactly on this PR's semantic boundary.
🟢 Nit 1: redundant defensive copies, inaccurate comment, four-way asymmetry. Session.ts and useGeminiStream.ts copy with new Map(getHistoryToolCallFingerprints()), commenting "avoid mutating the accessor-owned map" — but the accessor builds a fresh Map on every call, so there is no owned map; agent-core.ts and nonInteractiveCli.ts use the return value directly. No behavioral issue, but the comment's premise is false, and if someone memoizes the accessor later, the two non-copying ends would be the ones needing copies. Suggest dropping the copies uniformly, or fixing the comment.
🟢 Nit 2: the replay predicate closure is duplicated across all four entry points. The "resolve provider id → short-circuit empty → isReplayOfHandledToolCall" closure is inlined four times (two fc-shaped copies are verbatim identical). The non-interactive CLI's id getter carries a structured-output special case the other three lack — each end is internally consistent today, but future replay-semantics changes must sync four places, and a miss forks behavior across TUI/headless/ACP/subagent. Suggest a generic factory or an fc-shaped helper in toolCallIdUtils.ts.
📚 Semantic narrowing note (not an issue): the history map narrows from "all functionResponse ids" to "call/response-paired ids" — orphan response ids are no longer treated as handled. Under abnormal history shapes the failure direction is "execute one more time" (the safe direction), consistent with the PR's intent.
Conclusion: no blockers; the two nits do not block merge. Merge-order reminder: this PR is stacked on the base PR (9435), which needs to land first.
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed. Suggestions are inline.
Not reviewed: build-and-test — no build/test job ran in CI at this commit (all skipped; stacked-PR base); this review built all workspaces and ran the changed packages' unit suites locally, but the skipped tmux-testing and verify E2E suites did not run.
中文说明
仅完成部分审查，审查缺口已披露。 建议见行内评论。
未审查：build-and-test — no build/test job ran in CI at this commit (all skipped; stacked-PR base); this review built all workspaces and ran the changed packages' unit suites locally, but the skipped tmux-testing and verify E2E suites did not run。
— qwen3.8-max via Qwen Code /review (v0.21.13)
wenshao
left a comment
wenshao
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed.
Not reviewed: reverse audit — did not converge within the reverse-audit round cap of 5.
中文说明
仅完成部分审查，审查缺口已披露。
未审查：反向审计——在 5 轮的反审轮数上限内未收敛。
— qwen3.8-max via Qwen Code /review (v0.21.11)
…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.
8ed358b to
78ee248
Compare
|
Please do not rebase or force-push to an active PR as it invalidates existing review comments. Note for future reference, the bots always squash all changes into a single commit automatically as part of the integration. 中文请勿对活跃的 PR 执行 rebase 或 force-push，因为这会使已有的评审评论失效。另外，供日后参考：作为集成流程的一部分，机器人始终会自动将所有改动压缩（squash）为单个提交。 |
Code Coverage Summary
CLI Package - Full Text ReportCore Package - Full Text ReportFor detailed HTML reports, please see the 'coverage-reports-22.x-ubuntu-latest' artifact from the main CI run. |
yiliang114
left a comment
yiliang114
left a comment
There was a problem hiding this comment.
Independent review of the core fingerprint logic (tool-call-repeat-key.ts, toolCallIdUtils.ts, turn.ts, Session.ts, nonInteractiveCli.ts).
Verdict: not ready — 2 blocking Criticals. The design (id + name/args fingerprint instead of id-only) is sound, but two correctness bugs make the replay oracle fail-silent or fail-open.
-
canonicalizeForHashdrops__proto__own keys (confirmed in source).sortedis a plain{}andsorted['__proto__'] = …invokes the inherited setter — it sets the prototype instead of creating an own key. So(id X, {a:1})and(id X, {a:1, __proto__:{…}})hash identically and the second (a genuinely different call that must execute under a suffixed id) is suppressed as a replay.JSON.parsepreserves__proto__as an own enumerable key, so this is reachable from a model that emits a literal__proto__arg. Fix: buildsortedwithObject.create(null), or key the loop withObject.defineProperty/aMap. -
Args in-place mutation makes admission-time and history-seeded fingerprints diverge.
normalizeModelToolCallIdsbuilds the part with a shallow copy, soargsaliases the history entry;_executeToolCallBodythen rewrites PATH_ARG_KEYS in place after admission. A multi-round agent runtime that re-derives the map post-execution hashes the mutated args and WeakMap-caches the wrong fingerprint, so an already-answered call re-executes. Fix: fingerprint a stable snapshot (or canonicalize/copy the args before execution mutates them).
The 13 Suggestion threads (fingerprint recomputation perf, WeakMap cache-test gaps, comment accuracy, module/test colocation) are non-blocking but worth folding in with the Critical fixes.
This is doudouOUC's PR (not mine), so I'm flagging rather than fixing.
- 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.
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.
|
@qwen-code /triage The deferral's prescribed sequence is now complete: #9435 merged, this branch was rebased onto |
|
Sandboxed verification: The verification job did not complete (checkout, runner, or setup error) and produced no report. See the workflow run for details. 中文 — 判定：
|
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
LGTM, looks ready to ship. Both review rounds are verified addressed at this commit and full CI is green. ✅
chiga0
left a comment
chiga0
left a comment
There was a problem hiding this comment.
Code Review Overview (AI Generated)
PR: #9436 fix(core): treat duplicate provider tool-call ids as replays only when arguments match
Type: Bug Fix
Change size: +1231/-147 across 21 files
Reviewed at HEAD: b69bc55
Findings Summary
- Critical/Major: 0 items (all prior critical findings resolved)
- Minor: 1 item
- Nit: 1 item
Key Observations
This is a precise, well-scoped fix for a real production problem: LLM models such as Kimi that generate tool-call IDs only unique within a single response turn (e.g. run_shell_command_0 resetting on every round) caused false-positive replay detection, killing every session at round 3. The solution — requiring both provider ID match AND a SHA256 content fingerprint match to classify a call as a replay — is architecturally sound. The fingerprint reuses the same getToolCallRepeatKey function already used by the loop detection service, ensuring consistent "same call" semantics across all guards. All four entry points (AgentCore, TUI stream, non-interactive CLI, ACP Session) are updated consistently. The structuredClone at CoreToolScheduler intake correctly prevents the PATH_ARG_KEYS in-place rewrite from leaking into history-stored args and skewing replay fingerprints.
Cross-Validation
Phase 2 cross-validation against findings from prior reviewers wenshao, ytahdn, qwen-code-ci-bot, and yiliang114:
| Finding | Other Reviewer | Status at HEAD | My Assessment |
|---|---|---|---|
C1: canonicalizeForHash drops __proto__ own keys via inherited setter — two calls differing only in __proto__ collide on same fingerprint |
wenshao, yiliang114 |
✓ Fixed: Object.create(null) at line 28 of tool-call-repeat-key.ts |
Confirmed fixed; regression test added |
| C2: In-place PATH_ARG_KEYS rewrite in executor leaks into history-aliased args, skewing replay fingerprints in multi-round agents | wenshao |
✓ Fixed: structuredClone(item.args) at scheduler intake in coreToolScheduler.ts:2327 |
Confirmed fixed; mutation-isolation test added |
| S1: Duplicate message missing D3 args-based recovery path | wenshao |
✓ Fixed: message now includes "or explicitly different arguments" | Confirmed fixed |
| S2: First-occurrence-wins semantics not tested across turns | wenshao, qwen-code-ci-bot |
✓ Fixed: agent-headless.test.ts 3-round cross-turn test added |
Confirmed; test covers original-replay suppression after id-colliding execution |
| S3: WeakMap cache-hit path not tested (mutant survived) | wenshao, qwen-code-ci-bot |
✓ Fixed: toolCallIdUtils.test.ts pins cache-hit by mutating args between two lookups |
Confirmed |
| S4: Fingerprint recomputed 3x per item per batch (counting pass, find pass, admission loop) | wenshao, qwen-code-ci-bot |
✓ Fixed: fingerprint now precomputed once via getFunctionCallFingerprint(fc) / getCachedToolCallFingerprint(request, ...) and passed to helpers |
Confirmed via WeakMap cache |
S5: getToolCallRepeatKey tests not collocated with source module |
wenshao |
✓ Fixed: moved to tool-call-repeat-key.test.ts |
Confirmed |
| S6: Defensive copy comment misstated accessor ownership; entry points diverged on copy convention | qwen-code-ci-bot |
✓ Fixed: all 4 entry points now copy with consistent "fresh map per call" wording | Confirmed |
| S7: No mutation-pin assertion in Session.test.ts | wenshao, qwen-code-ci-bot |
✓ Fixed: restored assertion verifying seeded map is unchanged after runToolCalls |
Confirmed |
S8: getHistoryToolCallFingerprints first-occurrence-wins and orphan-exclusion branches untested |
qwen-code-ci-bot |
✓ Fixed: geminiChat.test.ts adds case with reused ID across turns and orphan functionResponse |
Confirmed |
Additional Audit Coverage
Areas I independently checked beyond the above findings:
- Public API surface break:
findRepeatedDuplicateProviderToolCallis re-exported from the package index viaexport * from './core/turn.js'(index.ts:97). Its third-parameter type changed fromReadonlySet<string>to a predicate(item: T) => boolean. All in-repo callers are migrated. External consumers using this specialized internal function could break on upgrade — worth noting in a release note or changelog, though the function's purpose makes external use unlikely. getHistoryFunctionResponseIdsbackward compatibility: Still exported fromGeminiChat(line 4142) andGeminiClient. Any REPAIR dedup paths that use id-only matching are unaffected.structuredClonesafety: Args from LLM responses are JSON-serializable;structuredClone(undefined)returnsundefinedin Node.js 17+. No throw risk.handledToolCallFingerprintsdead entries:getHistoryToolCallFingerprints()includes entries for suffixed IDs (e.g.,run_shell_command_0__qwen_dup_2 -> fingerprint2) which are never looked up via raw provider IDs. These are benign dead-weight entries with no correctness impact.- Null-prototype
JSON.stringifycorrectness:JSON.stringify(Object.create(null))returns"{}"— own properties on null-prototype objects serialize correctly, andObject.keysenumerates them including a data-property__proto__. Verified correct. nonInteractiveCli.tsvs other entry points:getProviderResponseIdis a local helper wrappingrequest.providerCallId ??...— functionally equivalent to directrequest.providerCallIdaccess in other entry points.
Minor Finding
[Minor] findRepeatedDuplicateProviderToolCall is a public package export with a breaking signature change.
packages/core/src/index.ts:97 does export * from './core/turn.js', making findRepeatedDuplicateProviderToolCall part of the public @qwen-code/qwen-code-core surface. The third parameter changed from ReadonlySet<string> to (item: T) => boolean. All in-repo callers are migrated correctly. For a semver-compliant release, this should be documented as a breaking change. Recommend adding a note to the release changelog or CHANGELOG.md.
Nit
[Nit] packages/core/src/utils/tool-call-repeat-key.ts license header says Copyright 2025 Qwen but the PR design doc is dated 2026-08-19. Other files added in 2026 may also carry 2025 — worth aligning with the actual creation year if the project's convention is to use the creation year.
Final Verdict
APPROVE. Both blocking criticals (__proto__ key dropping and args-mutation fingerprint skew) have been correctly resolved in the final commit. The fix design is sound, the four entry points are consistently wired, all eight suggestions from prior reviewers are addressed with evidence, and the new tests are thorough. The one minor concern (public API surface break) is low-risk given the function's internal nature and should be tracked in the changelog rather than held as a blocking issue.
This review was generated by QoderWork AI
yiliang114
left a comment
yiliang114
left a comment
There was a problem hiding this comment.
Re-reviewed the updated head b69bc55. The two Criticals I flagged earlier are now addressed:
-
__proto__own keys —canonicalizeForHashnow buildssortedviaObject.create(null)(null prototype), so a literal__proto__arg key becomes an own key instead of setting the prototype, and the replay oracle no longer conflates{a:1}with{a:1, __proto__:{...}}. Verified in source. -
Args in-place mutation divergence — fingerprints are now computed through
getCachedToolCallFingerprint(WeakMap keyed by the carrier part/request object), andgetHistoryToolCallFingerprintsroutes through the same cache, so admission-time and history-seeded walks pin the same pre-mutation fingerprint.
0 unresolved threads, 0 open Criticals. Approve.
wenshao
left a comment
wenshao
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed.
Not reviewed: reverse audit — stopped at the round cap of 5 without certifying convergence: rounds 2-5 were substantively dry (the pair's two findings verified into low-confidence Suggestions), but the builder's mechanical receipt check never certified the dry rounds.
Not reviewed: build-and-test — the Windows and macOS CI matrix jobs were skipped at this commit and no Windows runner ran locally; the Windows-only expectation R2-1 reports was verified via a win32-simulation probe instead.
Not explored to full depth (tool budget reached): "agent reverse-audit (round 2)": verifying reachability of the speculation path — whether it is flag-gated/enabled in fingerprint-admission flows and whether a committed speculated tool call is….
中文说明
仅完成部分审查，审查缺口已披露。
未审查：reverse audit — stopped at the round cap of 5 without certifying convergence: rounds 2-5 were substantively dry (the pair's two findings verified into low-confidence Suggestions), but the builder's mechanical receipt check never certified the dry rounds。
未审查：build-and-test — the Windows and macOS CI matrix jobs were skipped at this commit and no Windows runner ran locally; the Windows-only expectation R2-1 reports was verified via a win32-simulation probe instead。
未探索到全部深度（达到工具调用预算）："agent reverse-audit (round 2)"：verifying reachability of the speculation path — whether it is flag-gated/enabled in fingerprint-admission flows and whether a committed speculated tool call is…。
— qwen3.8-max via Qwen Code /review (v0.21.11)
| expect(completedCalls[0].request.args['file_path']).toBe( | ||
| '/tmp/my docs/a.txt', | ||
| ); |
There was a problem hiding this comment.
[Critical] The new regression test hardcodes the POSIX unescape result, but unescapePath is a documented no-op on win32 (early return in utils/paths.ts), so this assertion fails on the Windows test job — exactly the platform this PR's description defers to CI ("Windows/Linux local runs (CI covers)"). — Failure scenario: once this lands on main, the merge-queue Windows job (Test (windows-latest, Node 22.x), merge_group-only) runs the packages/core suite: on win32 _executeToolCallBody's PATH_ARG_KEYS rewrite leaves /tmp/my\ docs/a.txt unchanged, while the assertion expects /tmp/my docs/a.txt → a required platform gate is red on a test this PR adds.
Witness (probe, both arms, then reverted):
BASE(POSIX): Tests 1 passed
PR + win32-sim (vi.mock node:os platform→win32): AssertionError: expected '/tmp/my\ docs/a.txt' to be '/tmp/my docs/a.txt'
PR + fix + win32-sim: Tests 1 passed
fix + POSIX: Tests 1 passed
| expect(completedCalls[0].request.args['file_path']).toBe( | |
| '/tmp/my docs/a.txt', | |
| ); | |
| expect(completedCalls[0].request.args['file_path']).toBe( | |
| unescapePath('/tmp/my\\ docs/a.txt'), | |
| ); |
unescapePath is already imported in this file; this mirrors the sibling test does not double-unescape path args across an ask bounce. The second assertion (caller args unchanged) is platform-independent and keeps the leak invariant pinned on every platform.
中文说明
[严重] 新增的回归测试把 POSIX 平台的 unescape 结果写死了，但 unescapePath 在 win32 上是明确声明的空操作（utils/paths.ts 中提前返回），因此该断言在 Windows 测试任务上会失败——而本 PR 的描述恰好把该平台交给 CI 验证（"Windows/Linux 本地运行（CI 覆盖）"）。——失败场景：本 PR 合入 main 后，合并队列的 Windows 任务（Test (windows-latest, Node 22.x)，仅 merge_group 触发）会运行 packages/core 套件：win32 上 _executeToolCallBody 的 PATH_ARG_KEYS 改写保持 /tmp/my\ docs/a.txt 不变，而断言期望 /tmp/my docs/a.txt → 本 PR 新增的测试让一个必选平台门禁变红。
见证（探针，双臂验证后已还原）：见英文部分代码块。
建议修复：改用 unescapePath('/tmp/my\\ docs/a.txt') 作为期望值（上方 suggestion）。该文件已导入 unescapePath，写法与姊妹测试 does not double-unescape path args across an ask bounce 一致；第二条断言（调用方参数不变）平台无关，在任何平台都继续钉住泄漏不变量。
— qwen3.8-max via Qwen Code /review (v0.21.11)
| export function getToolCallFingerprint( | ||
| name: string | undefined, | ||
| args: unknown, | ||
| ): string { | ||
| return getToolCallRepeatKey(name ?? '', args ?? {}); |
There was a problem hiding this comment.
[Suggestion] Missing tool names are canonicalized differently by the two halves of replay detection: history seeding fingerprints a stored nameless functionCall via getFunctionCallFingerprint → name ?? '', while the TUI/headless admission side fingerprints ToolCallRequestInfo.name, which Turn.handlePendingFunctionCall coerces to 'undefined_tool_name' (fnCall.name || 'undefined_tool_name', turn.ts:740). canonicalToolName is identity for both strings, so after the per-submit ref clear forces a history re-seed, an exact replay of a nameless call mismatches the stored fingerprint and is re-admitted instead of suppressed. (agent-core and ACP Session pass the raw fc.name on both sides and are self-consistent.) — Failure scenario: a provider emits a functionCall with a provider id but no name (answered with a TOOL_NOT_REGISTERED error); in a later submit the stuck provider replays the identical call under the same id → the re-seeded map holds F('') while the incoming request fingerprints as F('undefined_tool_name') → isReplayOfHandledToolCall returns false and the call is re-admitted for another error response instead of the synthetic duplicate. Impact is bounded — the replayed call cannot resolve to a real tool (no side-effect path), the trigger is degenerate, and the consecutive-identical guard halts bursts — hence Suggestion, not Critical.
Witness (probe against the real GeminiChat + fingerprint functions, 4/4 assertions):
getToolCallFingerprint(undefined, args) !== getToolCallFingerprint('undefined_tool_name', args)
isReplayOfHandledToolCall(historyMap, 'call_1', F('undefined_tool_name')) === false // replay re-admitted
recomputed with the unified coercion → true // flip
| export function getToolCallFingerprint( | |
| name: string | undefined, | |
| args: unknown, | |
| ): string { | |
| return getToolCallRepeatKey(name ?? '', args ?? {}); | |
| export function getToolCallFingerprint( | |
| name: string | undefined, | |
| args: unknown, | |
| ): string { | |
| const canonicalName = | |
| name === undefined || name === 'undefined_tool_name' ? '' : name; | |
| return getToolCallRepeatKey(canonicalName, args ?? {}); | |
| } |
(Alternatively, carry the raw provider name through ToolCallRequestInfo so both halves hash the provider's actual value.)
中文说明
[建议] 重放检测的两半对"缺失工具名"的规范化不一致：历史播种侧经 getFunctionCallFingerprint → name ?? '' 为无名 functionCall 计算指纹，而 TUI/headless 准入侧指纹化的 ToolCallRequestInfo.name 是 Turn.handlePendingFunctionCall 强制转换出的 'undefined_tool_name'（turn.ts:740）。canonicalToolName 对这两个字符串都原样返回，因此在每次 submit 清空 ref、被迫从历史重新播种之后，对无名调用的精确重放会与存储指纹不匹配，被当作全新调用重新准入而非压制。（agent-core 与 ACP Session 两侧都传原始 fc.name，自洽。）——失败场景：provider 发出带 provider id 但无名的 functionCall（以 TOOL_NOT_REGISTERED 错误应答）；之后的 submit 中卡住的 provider 以同一 id 重放完全相同的调用 → 重新播种的 map 存 F('')，而入站请求指纹为 F('undefined_tool_name') → isReplayOfHandledToolCall 返回 false，该调用被再次准入并再得一个错误响应，而非收到合成 duplicate。影响有限——重放调用无法解析到真实工具（无副作用路径）、触发条件退化、连续相同守卫会终止连发——因此是建议而非严重。
见证（对真实 GeminiChat + 指纹函数的探针，4/4 断言）：见英文部分代码块。
建议修复：在指纹边界统一缺失名的规范化（上方 suggestion 为一种写法），或让 ToolCallRequestInfo 携带 provider 的原始名字，使两半哈希同一实际值。
— qwen3.8-max via Qwen Code /review (v0.21.11)
| `The duplicate tool call was ignored and not executed again. If you ` + | ||
| `intended to run this tool again, re-issue the call with a new unique ` + | ||
| `tool-call id (or explicitly different arguments).` |
There was a problem hiding this comment.
[Suggestion] The extended duplicate-response message — the model-facing recovery guidance design decision D3 requires and this PR adds — has no test asserting the appended clause: all 29 assertions on this message across the four suites (Session.test.ts, nonInteractiveCli.test.ts, useGeminiStream.test.tsx, agent-headless.test.ts) match only the unchanged 'Duplicate provider tool call id "..."' prefix via toContain/stringContaining. — Concrete cost: a regression that truncates or garbles the appended guidance removes the model's only documented recovery path from the suppressed-replay dead end, and every existing assertion stays green.
Witness (sweep at the reviewed commit): re-issue the call / explicitly different arguments appear only in turn.ts and the design doc — zero test files; 0 of the 29 message assertions cover the appended clause.
Suggested fix: extend one existing assertion (e.g. the agent-headless round-3 check) with toContain('re-issue the call with a new unique tool-call id') — a substring assertion keeps it refactor-safe.
中文说明
[建议] 扩展后的 duplicate 响应消息——设计决策 D3 要求、本 PR 新增的面向模型的恢复指引——没有任何测试断言新增的子句：四个套件中全部 29 处对该消息的断言都只经 toContain/stringContaining 匹配未改变的 'Duplicate provider tool call id "..."' 前缀。——具体代价：任何截断或破坏新增指引的回归都会移除模型在被压制重放死胡同里唯一有文档记载的恢复路径，而所有现有断言依旧全绿。
见证（在受审提交上的穷举扫描）：re-issue the call / explicitly different arguments 仅出现在 turn.ts 与设计文档中——测试文件中为零；29 处消息断言中 0 处覆盖新增子句。
建议修复：扩展现有断言之一（如 agent-headless 第三轮检查），追加 toContain('re-issue the call with a new unique tool-call id')——子串断言对重构安全。
— qwen3.8-max via Qwen Code /review (v0.21.11)
| for (const [ | ||
| providerCallId, | ||
| fingerprint, | ||
| ] of handledToolCallFingerprintsRef.current) { | ||
| if (!handledToolCallFingerprints.has(providerCallId)) { | ||
| handledToolCallFingerprints.set(providerCallId, fingerprint); | ||
| } |
There was a problem hiding this comment.
[Suggestion] The merge-precedence rule "the history fingerprint for an id wins over in-flight entries from this submit" (the !has guard) is untested: no test seeds the same provider id into both history and the in-flight ref with different fingerprints. — Failure scenario: if the guard were flipped (in-flight overwrites history), an already-answered replay whose id also carries an in-flight colliding entry would re-execute, and the just-admitted colliding call would be suppressed as a "replay" — while every existing test stays green: the only three tests overriding getHistoryToolCallFingerprints (useGeminiStream.test.tsx:4936/4990/5038) keep the ref empty at merge time (single replay batch; single collision batch that stops at scheduling; breaker-dropped batch where the suppressed replay never records).
Witness (enumeration + flip trace at the reviewed commit): exactly 3 tests override the accessor; 0 exercise precedence; flipping the guard changes no existing test outcome.
Suggested fix: add a test that seeds getHistoryToolCallFingerprints with X→fp(A), drives one admitted X/B round so the ref is populated, then asserts a follow-up X/A request still receives the synthetic duplicate response (history wins).
中文说明
[建议] 合并优先级规则"同一 id 以历史指纹优先于本次 submit 的进行中条目"（即该 !has 守卫）没有测试覆盖：没有测试把同一 provider id 以不同指纹同时播种进历史与进行中 ref。——失败场景：若守卫被翻转（进行中覆盖历史），一个已应答的重放（其 id 同时带有进行中的碰撞条目）会再次执行，而刚准入的碰撞调用反被当作"重放"压制——所有现有测试依旧全绿：仅有的三个覆写 getHistoryToolCallFingerprints 的测试（useGeminiStream.test.tsx:4936/4990/5038）在合并时 ref 均为空（单重放批次；在调度处即停止的单碰撞批次；被熔断丢弃、压制重放从不记录的批次）。
见证（在受审提交上的穷举 + 翻转推演）：恰好 3 个测试覆写该 accessor；0 个覆盖优先级；翻转守卫不改变任何现有测试结果。
建议修复：新增测试——以 X→fp(A) 播种 getHistoryToolCallFingerprints，驱动一轮准入 X/B 使 ref 被填充，然后断言随后的 X/A 请求仍收到合成 duplicate 响应（历史优先）。
— qwen3.8-max via Qwen Code /review (v0.21.11)
|
Released in v0.21.15. |
6 participants
What this PR does
The duplicate provider tool-call guard now identifies a replay by the provider id and a canonical fingerprint of the tool name and arguments — the same repeat key the loop guards already use. An id collision whose arguments differ from the call that originally executed under that id is treated as a fresh call and executes normally under the unique suffixed id that normalization already assigns. Exact same-arguments replays keep the existing protection unchanged at all four entry points (subagent runtime, TUI stream, non-interactive CLI, ACP daemon session): the first replay is answered with a synthetic duplicate error and a recurrence trips the circuit breaker. The synthetic duplicate message additionally tells the model to re-issue the call with a fresh tool-call id when a new invocation was intended, giving models that generate their own id tokens a recovery path. The fingerprint key lives in a leaf utility module (moved out of the loop detection service, which re-exports it unchanged) so the id-normalization layer can share it without an import cycle; the history-derived fingerprint map is walk-only with per-part caching so large arguments are hashed once, not once per dedup pass.
Why it's needed
Some models emit tool-call ids that are only unique within a single response — e.g. an id shaped like
{name}_{index}whose index restarts at 0 on any round. With id-only matching, a fresh call that happened to reuse an old id was misclassified as a replay: the second collision received the synthetic duplicate error and the third killed the whole turn via the circuit breaker, so once a model entered this mode every turn died by round three. Live daemon sessions showed exactly this signature: the model emittedrun_shell_command_0with a different command each round, and every prompt terminated after two failed rounds (silently before the stacked base PR). The original #5014 replay bug — and the regression fixtures guarding it — are same id and same arguments, so argument-aware matching preserves that protection at full strength while eliminating the false positive. Pathological repetition that slips past the id layer remains bounded by the id-independent guards (consecutive-identical threshold, global same-call threshold, per-turn cap).Stacked on #9435 (visible daemon stop for this breaker); both implement the committed design doc.
Reviewer Test Plan
How to verify
With a deterministic mock OpenAI-compatible provider that returns the same tool-call id (
run_shell_command_0) on every round:global_tool_call_duplicate.global_tool_call_duplicatehalt — unchanged protection.Focused suites:
Each entry point now carries the suppression case and the new different-args execution case side by side, so the fingerprint consultation is pinned from both directions.
Evidence (Before & After)
Verified end-to-end on the bundled CLI against the mock provider above (isolated HOME, headless
--approval-mode yolo):Before (qwen 0.21.13, the released build): different-args scenario — round 2's fresh command was answered with
Duplicate provider tool call id "run_shell_command_0" was already handled..., round 3 halted the run withLoop detection halted the run (global_tool_call_duplicate: ...); the round-2/3 commands never executed.After (this branch's
dist/cli.js): different-args scenario — all three rounds executed (each round's tool result is real shell output) and the run finished with the model's final text. Same-args scenario — round 2 suppressed with the duplicate error, round 3 halted visibly withglobal_tool_call_duplicate; round 4 was never reached.Tested on
Environment (optional)
Local bundle (
node dist/cli.js) against a local mock OpenAI-compatible SSE server with a fake API key; unit suites via vitest.Risk & Scope
Linked Issues
Design doc committed under
docs/design/2026-08-19-duplicate-provider-toolcall-id-guard.md(this PR implements decisions D2 and D3). Related history: #5014, #5038, #5657.中文说明
本 PR 做了什么
重复 provider tool-call 防护现在以 provider id 加 工具名与参数的规范化指纹（与 loop 守卫使用的同一 repeat key）识别重放。同 id 但参数与该 id 首次执行不同的碰撞视为全新调用，在归一化已分配的唯一后缀 id 下正常执行。同 id 同参数的精确重放在四个入口（subagent 运行时、TUI 流、非交互 CLI、ACP daemon 会话）保持既有保护不变：首次重放回注合成 duplicate 错误，再次出现触发熔断。合成 duplicate 消息追加提示：若确为新调用，请以新的 tool-call id 重新发起——给自产 id token 的模型恢复路径。指纹函数移至叶子工具模块（loop 检测服务原位转发导出），使 id 归一化层可共享而不引入 import 环；历史指纹映射为免克隆遍历并带按部件缓存，大参数只哈希一次而非每次去重都算。
为什么需要
某些模型产出的 tool-call id 仅在单次响应内唯一——例如形如
{name}_{index}、index 随时归零的 id。仅按 id 匹配时，恰好复用旧 id 的全新调用被误判为重放：第二次碰撞收到合成 duplicate 错误，第三次经熔断杀死整个 turn，模型一旦进入该模式，每个 turn 都在第三轮死亡。本地 daemon 会话呈现的正是该特征：模型每轮都发run_shell_command_0但命令各不相同，每个 prompt 两轮失败后终止（在堆叠的基础 PR 之前还是静默终止）。最初的 #5014 重放 bug 及其回归用例都是同 id 且同参数，因此参数感知匹配在完整保留该保护的同时消除误判。绕过 id 层的病态重复仍由与 id 无关的守卫兜底（连续相同阈值、全局同调用阈值、每 turn 上限）。堆叠于 #9435（该熔断的 daemon 可见终止）；两者共同实现已提交的设计文档。
Reviewer 测试计划
如何验证
使用每轮都返回同一 tool-call id（
run_shell_command_0）的确定性 mock OpenAI 兼容 provider：global_tool_call_duplicate终止。global_tool_call_duplicate停止——保护不变。回归套件见英文部分命令。每个入口现在同时具备"压制"与新的"异参数执行"成对用例，从两个方向钉住指纹判定。
证据（Before & After）
已在打包产物上对 mock provider 端到端验证（隔离 HOME、headless
--approval-mode yolo）：Before（已发布的 qwen 0.21.13）：异参数场景——第 2 轮的全新命令被回以
Duplicate provider tool call id "run_shell_command_0" was already handled...，第 3 轮以Loop detection halted the run (global_tool_call_duplicate: ...)终止；第 2/3 轮命令从未执行。After（本分支
dist/cli.js）：异参数场景——三轮全部执行（每轮工具结果均为真实 shell 输出），运行以模型最终文本收尾。同参数场景——第 2 轮以 duplicate 错误压制，第 3 轮以global_tool_call_duplicate可见终止；从未到达第 4 轮。测试平台
macOS ✅；Windows / Linux⚠️ 未本地验证（依赖 CI）。
风险与范围
关联 Issue
设计文档见
docs/design/2026-08-19-duplicate-provider-toolcall-id-guard.md（本 PR 实现决策 D2 与 D3）。相关历史：#5014、#5038、#5657。