fix(core): preserve prompt cache across deferred tool discovery - #8276
fix(core): preserve prompt cache across deferred tool discovery#8276DragonnZhang wants to merge 92 commits into
Conversation
E2E test reportEnvironment
Live acceptance results
Provider prompt-cache evidence
The stable 24,259-token base was reused across fresh and resumed processes. Within a running prompt/tool loop, cache reuse rose to 94.8%–99.7% after reveal rather than dropping to zero. The only zero-cache event in that pass was the full-compression summary request under its separate An earlier approximately 13-minute idle gap also produced a first-request cache miss, followed immediately by 99% cache reuse. That remains consistent with provider cache expiry/eviction rather than a deferred declaration change. Startup deferred catalog evidence
Raw serialized request hashesA bundled local build was run with
All three requests contained the same 22 provider declarations. None contained As an additional prefix check, the first user-message envelope had a first-request-only The isolated logging run reported cache reads of Persisted-session evidence
Automated verification
Dynamic MCP add/remove/reconnect, schema-fingerprint replacement, permission tightening, truncation, and batch-budget behavior were covered by focused tests. The live pass intentionally did not mutate the developer's active MCP configuration to simulate add/remove events. Remaining acceptance work
|
…-stable-cache # Conflicts: # packages/cli/src/nonInteractiveCli.ts # packages/core/src/core/nonInteractiveToolExecutor.test.ts # packages/core/src/core/nonInteractiveToolExecutor.ts
…' into dragon/deferred-tools-stable-cache
|
Addressed the review findings in 39bc677 (with latest main integrated in 4aa15f2):
Verification: 474 focused Core tests, 491 ACP session tests, 104 headless tests; Core/CLI typechecks; affected package builds; staged Prettier/ESLint. CI is running. |
🖼️ web-shell visual previewRendered against a mock daemon (no real backend): the PR base vs this PR head Screenshots · before / afterFull-resolution recordings (.webm) are attached to the workflow run. — Qwen Code · web-shell visuals |
|
Re-run at @wenshao's request — gate refreshed against the same head ( Template: complete ✓ Problem: real and observed. Both linked issues (#6721, #4777) are still open and document the same failure with root-cause analysis: revealing a deferred tool rewrites the provider Direction: aligned. Prompt-cache prefix stability is squarely in qwen-code's core mission — the reference product's CHANGELOG shows sustained work in exactly this area — and the approach matches the fix #4777 itself proposes: move the dynamic catalog out of the cached prefix into tail reminders. Size: still the flag, recomputed at this head: 2,257 production-logic lines across 32 source files, plus an 896-line design doc and 6,401 test lines over 25 test files — 9,554 churned lines across 58 files total. That clears the 500-line maintainer-awareness bar and the 1,000-line large-PR advisory by a wide margin, so the Stage-0 maintainer escalation stands regardless of how clean the diff reads. The growth over the cycle has been review-driven hardening, not scope creep; a first PR of this shape would still have been easier to land in slices. Approach: sound, and it's what I'd have proposed independently: one permanently stable Risk: elevated, unchanged. Two changed files sit on revert-history high-risk paths: Moving on to code review. 🔍 中文说明应 @wenshao 的要求重跑——gate 基于同一 head( 模板: 完整 ✓ 问题: 真实且已被观测到。两个关联 issue(#6721、#4777)仍然 open,且都用根因分析记录了同一故障:reveal deferred tool 会改写面向 provider 的 方向: 对齐。Prompt cache 前缀稳定性完全属于 qwen-code 的核心使命——参考产品的 CHANGELOG 在同一方向有持续工作——方案也与 #4777 自身提出的修复一致:把动态 catalog 从被缓存的前缀移到对话尾部的 reminder。 规模: 仍是要点出的问题,按当前 head 重新计算:2,257 行生产逻辑、32 个源文件,外加 896 行设计文档和 25 个测试文件中的 6,401 行测试——58 个文件共 9,554 行变更。远超 500 行维护者关注线和 1,000 行大 PR 建议线,因此无论 diff 读起来多干净,Stage 0 维护者转交维持不变。本轮周期的增长来自评审驱动的加固而非范围蔓延;只是这种形态的首个 PR 如果更小会更容易落地。 方案: 合理,也是我独立会提出的方案:一个永久稳定的 风险: 升级,不变。两个改动文件落在 revert 历史高风险路径上: 进入代码审查。🔍 — Qwen Code · qwen3.8-max Reviewed at |
Code review (head
|
| File | What changed |
|---|---|
docs/design/prompt-cache/deferred-tool-call-stable-schema.md |
Design doc covering the bridge, fingerprints, and lifecycle invariants |
packages/core/src/tools/deferred-tool-call.ts |
New stable bridge tool whose execute is a defensive error path |
packages/core/src/core/deferred-tool-call-normalization.ts |
New fail-closed normalization boundary plus identity helpers |
packages/core/src/tools/tool-search.ts |
Schemas as result text, presentation staging, reworked oversized-budget fallback |
packages/core/src/tools/tool-registry.ts |
Schema fingerprints, presentation state, reserved-name guards, pair check |
packages/core/src/core/coreToolScheduler.ts |
Normalization wiring, provider-name responses, presentation drop on payload change, commit gate |
packages/core/src/core/client.ts |
Resume restoration with interruption-aware reminder placement, compression restore, paired clears |
packages/cli/src/acp-integration/session/Session.ts |
ACP normalization, symbol-staged commit-on-delivery, COMPRESSED clear |
packages/cli/src/ui/hooks/useGeminiStream.ts |
Interactive acceptance-gated commit via acknowledged stream |
packages/cli/src/nonInteractiveCli.ts |
Headless carry and commit chain, unwrapped concurrency classification |
packages/core/src/config/config.ts |
Paired registration of tool_search and the bridge, subagent exclusion |
packages/core/src/core/geminiChat.ts |
Repair no longer merges into system reminders, pop-guard comment updated |
packages/core/src/tools/function-schema-rendering.ts |
Shared schema-block rendering with angle-bracket escaping |
packages/core/src/tools/tools.ts |
DeferredToolPresentation type and ToolResult field |
packages/core/src/core/turn.ts |
providerName on requests, presentation metadata on responses |
packages/core/src/services/loopDetectionService.ts |
Loop heuristics reason about the unwrapped target |
packages/core/src/services/memoryPressureMonitor.ts |
Idle compact honors the paired-clear invariant |
packages/core/src/agents/runtime/agent-core.ts |
Bridge excluded from subagents and teammates |
packages/core/src/core/nonInteractiveToolExecutor.ts |
Presentation commit deferred to the caller batch |
packages/core/src/tools/enterPlanMode.ts |
Drops the plan-mode declaration resync, no longer needed |
packages/core/src/tools/tool-names.ts |
Registers the deferred_tool_call name and display name |
packages/core/src/tools/mcp-tool.ts |
MCP tools participate in proxy eligibility |
packages/core/src/telemetry/types.ts |
Provider-name telemetry field |
packages/core/src/utils/environmentContext.ts |
System-reminder content helpers for the placement logic |
packages/core/src/index.ts |
Exports the new public type |
…and 32 more files |
25 test files (6,401 lines), i18n locale strings, web-shell label, integration tests |
Test evidence (this PR's own CI, read via the API — I did not run any PR code)
I re-fetched the check-runs for the reviewed commit this pass. One red check, and I re-read its log before classifying it: Test (ubuntu-latest, Node 22.x) failed — but not on a test assertion. The @qwen-code/web-shell workspace's vitest run --coverage worker died with FATAL ERROR: Ineffective mark-compacts near heap limit Allocation failed - JavaScript heap out of memory (~3 GB heap, ~193 s in), and the job exited on the resulting ERR_IPC_CHANNEL_CLOSED. Every suite that reported a result shows only passes. Three reasons I'm calling this environmental rather than PR-caused: the exact same OOM signature failed the same job on the previous head 79d98dc; the same check was green one head earlier (ebb7504) with nearly identical core changes; and this PR's entire web-shell footprint is two one-line additions (a locale label and a formatting helper string), which cannot move a coverage run's heap profile. The maintainer's independent read of the same red leg agrees — environmental, worth a re-run rather than a code change. The fork-gated lanes (Integration Tests, macOS/Windows Node-22 tests) remain skipped pending maintainer authorization — expected for a cross-repository PR.
| Check | Conclusion |
|---|---|
Test (ubuntu-latest, Node 22.x) |
❌ failure (environmental web-shell vitest OOM — see note above; needs a green re-run) |
Capture web-shell visuals (ubuntu-latest, Node 22.x) |
✅ success |
Classify PR |
✅ success |
Desktop Shell (ubuntu-22.04) |
✅ success |
Desktop Shell (windows-2022) |
✅ success |
Real daemon E2E / Java 11 |
✅ success |
macos-latest / Java 21 |
✅ success |
precheck-pr / precheck |
✅ success |
review-pr |
✅ success |
ubuntu-latest / Java 11 |
✅ success |
ubuntu-latest / Java 17 |
✅ success |
ubuntu-latest / Java 21 |
✅ success |
web-shell E2E Smoke (ubuntu-latest, Node 22.x) |
✅ success |
windows-latest / Java 21 |
✅ success |
One row per check name (latest run); skipped checks omitted; failures sort first. / 每个检查名一行(取最新一次运行),省略 skipped,失败项排在最前。
The behavioural gap my previous pass flagged is now closed by an independent maintainer run — this is @wenshao's evidence, not mine, and I did not re-run it: two full CLI bundles (base 86a474b vs head 14e9310) driven against a real stdio MCP server with a side-effect ledger and a recording OpenAI-compatible provider, plus the real TUI in tmux. On the wire, the base arm's tools digest changes across a reveal while the head arm stays on one byte-identical digest with the schema appearing only in the tool result; the fail-closed matrix (no prior search, self-recursion, directly visible target, targeting tool_search, invalid target arguments) all rejected with the target's own schema validation and an empty ledger; a hostile MCP server claiming the reserved name gets namespaced without collision; the subagent surface declares its tools directly with no bridge; the oversized-schema escape hatch and a forced real auto-compaction both behave as designed. The maintainer's verdict: "the central claim holds, and every fail-closed path I could reach behaved correctly. I did not find a merge blocker," with three non-blocking observations (a one-request window after compaction where registry eligibility precedes the restored reminder into the payload; rejected proxy calls rendering the raw envelope before normalization; and tool_search's unbounded maxOutputChars making the truncation-clear guard defense-in-depth rather than a live path). Because the load-bearing claim is now substantiated by that independent run, I'm not re-emitting a sponsored-verification trigger this pass.
中文说明
代码审查(head 14e9310,与上次审查相比无变化)
自上次全量 diff 审查以来没有新 commit 落地;这次我没有直接沿用旧结论,而是重新核对了安全关键区域:normalization 边界、client.ts 的 resume/压缩恢复、registry 的 fingerprint 状态、scheduler 的提交门控、tool_search 的 presentation 暂存,以及三条送达路径。这些安全属性在当前 head 上仍然全部成立:
- Fail-closed 归一化。
normalizeDeferredToolCallRequest拒绝所有不应到达真实工具的路径——发现/代理对未注册、目标名缺失或为空、参数非对象、自引用、懒加载失败、工具不存在、归一化期间工具变更、目标不适格、schema 未送达——每条都有独立的错误类型,且面向 provider 的响应始终经providerToolName与桥接名和原始 call ID 配对。 - 第 8 轮 Critical 保持已修复。 resume 路径用
detectTurnInterruption计算恢复 schema reminder 的插入位置——interrupted_turn把 reminder 插到悬空尾段之前,interrupted_prompt向前跳过尾部连续的普通 user 条目——复用既有工具,并由两个startChat回归测试钉住。 - 授权由 fingerprint 门控,且在我追查到的每条历史变更路径上撤销:
setHistory、truncateHistory、microcompaction、idle-compact、自动压缩、手动tryCompress、被剥离的tool_search响应,以及 ACP 的COMPRESSED流事件——ACP 包装层写明了为何在那里只做 registry 清空是不够的。 - 三条路径的送达后提交都是真实的。 交互:
acknowledgedStream只把 provider 产生的事件当作接受证据,接受前出现ChatCompressed会抑制提交。无头:carry/commit 链使用同一白名单门控,send 被 hook 拦截或未送达时丢弃 presentations。ACP:presentations 经 symbol 暂存在消息上,只在送达或显式历史保留之后提交;三条路径在 finalization 时都会在 payload 变化时丢弃 metadata。 - 保留名防护与 subagent 排除成立:名为
deferred_tool_call的 MCP 工具被命名空间化为限定名,命令发现的同名工具被显式警告后丢弃,registerFactory对保留名抛错,且桥接工具在forSubAgent时从不注册——subagent 保持直接声明。
没有可从该 diff 证明的正确性 blocker、安全漏洞或约定违反。评审轮次遗留的是 Suggestion 级问题(第 9 轮共 20 项——测试钉住形态、subagent 上下文中一条超大 fallback 消息未覆盖、每次资格检查重算 fingerprint、两处文档不一致);均不阻塞合并。
测试证据(该 PR 自己的 CI,经 API 读取——我没有运行任何 PR 代码)
本轮重新拉取了被审 commit 的 check-runs。一个红色检查,我重读其日志后才归类:Test (ubuntu-latest, Node 22.x) 失败——但不是测试断言失败。 @qwen-code/web-shell 的 vitest run --coverage worker 因 FATAL ERROR: Ineffective mark-compacts near heap limit(约 3 GB 堆、约 193 秒)崩溃,job 因随后的 ERR_IPC_CHANNEL_CLOSED 退出。所有有结果的套件全部通过。归为环境性而非 PR 导致的三个理由:同样的 OOM 签名在上一 head 79d98dc 的同一 job 上也出现过;再往前一个 head(ebb7504)核心改动几乎相同却是绿的;本 PR 在 web-shell 的全部改动只有两处单行新增,不可能移动 coverage 运行的堆曲线。维护者对同一红色检查的独立判断一致——环境性问题,重跑即可。fork 受限通道(Integration Tests、macOS/Windows Node-22 测试)仍为 skipped——跨仓库 PR 的预期状态。
上一轮我指出的行为性缺口,现已由维护者的独立运行闭合——这是 @wenshao 的证据,不是我的,我也没有重跑:两个完整 CLI bundle(base 86a474b vs head 14e9310)对着一个带副作用台账的真实 stdio MCP server 和一个录制式 OpenAI 兼容 provider 运行,并在 tmux 中驱动了真实 TUI。线上证据:base 侧的 tools digest 在 reveal 前后变化,head 侧保持单一逐字节一致的 digest、schema 只出现在工具结果里;fail-closed 矩阵(未先搜索、自引用、直接可见目标、指向 tool_search 本身、非法目标参数)全部被拒绝,且报错来自目标工具自身的 schema 校验、台账为空;恶意 MCP server 抢占保留名会被命名空间化而无冲突;subagent 侧直接声明工具、没有桥接;超大 schema 逃生通道与强制真实 auto-compaction 行为均符合设计。维护者的结论:"核心主张成立,我能触及的每条 fail-closed 路径行为都正确,没有发现合并 blocker",另有三条不阻塞的观察项。由于核心主张已被该独立运行证实,本轮不再重复发出赞助验证触发语。
— Qwen Code · qwen3.8-max
Reviewed at 14e93105f8bdedab712d90e7685762660f1c4bab · re-run with @qwen-code /triage
|
Confidence: 3/5 — clean review, no blocker found; the 3 is the policy cap for a Stage-0-escalated core change of this scale, and the maintainer sign-off that cap exists to obtain is already on record. Stepping back: my independent proposal for this problem is essentially what shipped, and a fresh verification pass over the safety-critical areas at this head — fail-closed normalization, fingerprint-gated authorization with paired clears on every history mutation, acceptance-proof commits in all three delivery lanes, reserved-name protection, subagent exclusion — turned up nothing new to object to. The round-8 Critical remains fixed with the right tool and regression coverage. The author's evidence has been unusually rigorous for a fork PR throughout, and the two items my last defer asked a human to settle are now both settled by the maintainer personally: @wenshao approved this exact commit and independently verified the load-bearing behaviour on a real stack — byte-stable declarations on the wire, a fail-closed matrix proven by a side-effect ledger, subagent boundary, oversized-schema escape hatch, and compaction — concluding there is no merge blocker. I have nothing to add against the code itself. What keeps this a defer rather than an approval from me:
The follow-up comment carries the @-mention. 中文说明置信度:3/5 —— 审查干净,未发现 blocker;这个 3 分是 Stage 0 规模转交对这一量级核心改动的政策性上限,而该上限所要换取的维护者把关,目前已有记录。 退一步看:我对这个问题独立提出的方案基本就是最终落地的方案;本轮对该 head 上安全关键区域的重新核对——fail-closed 归一化、fingerprint 门控授权与每条历史变更上的成对清空、三条送达路径上以接受为证的提交、保留名防护、subagent 排除——没有发现新的可反对之处。第 8 轮 Critical 保持用正确的方式修复并有回归覆盖。作者的证据在整个周期里对 fork PR 来说都严谨得少见,而我上次 defer 请人类定夺的两项,现已由维护者本人定夺:@wenshao 批准了该 commit 本身,并在真实栈上独立验证了关键行为——线上声明字节稳定、以副作用台账证明的 fail-closed 矩阵、subagent 边界、超大 schema 逃生通道、压缩——结论是没有合并 blocker。对代码本身我没有补充意见。 让我选择 defer 而非由我 approve 的原因:
@ 提醒见后续评论。 — Qwen Code · qwen3.8-max Reviewed at |
|
⏸️ Deferring to @wenshao — re-run on Reviewed at |
Review:
|
| Suite | Result |
|---|---|
core: deferred-tool-call, deferred-tool-call-normalization, tool-search, tool-registry, function-schema-rendering |
138 passed |
core: coreToolScheduler, client, mcp-tool, enterPlanMode, nonInteractiveToolExecutor, agent-core, environmentContext, loggers, agent |
1192 passed |
core: config |
490 passed |
cli: useToolScheduler, useGeminiStream |
198 passed |
cli: Session (ACP), nonInteractiveCli |
597 passed, 1 skipped |
typecheck (core; cli after building core) |
clean for this PR's files |
Findings
1. ACP/daemon retry & continue bypass the presentation clear — fail-open (should fix)
GeminiClient.stripOrphanedUserEntriesFromHistory() gained the new "a stripped tool_search response must revoke authorization" guard, but ACP never goes through it:
// packages/cli/src/acp-integration/session/Session.ts:2889
strippedOrphanEntries =
this.#getCurrentChat().stripOrphanedUserEntriesFromHistory() ?? null;
// packages/cli/src/acp-integration/session/Session.ts:2903
this.#getCurrentChat().stripOrphanedUserEntriesFromHistory();Both call the chat directly, so clearProxySchemaPresentationsAfterHistoryMutation never runs. Concretely: tool_search result delivered → commitDeferredToolPresentationsForDeliveredMessage commits → the send fails → the retry path strips that orphaned user entry → the registry still authorizes deferred_tool_call for a schema that is no longer in the model's context. This is exactly the fail-open case the PR closes for the interactive path.
The PR already converted the two neighbouring calls (chat.truncateHistory → geminiClient.truncateHistory, chat.setHistory → geminiClient.setHistory); these two should get the same treatment.
2. tool_search output is now unbounded on the paths that produce no presentations (should fix)
Three size controls were removed or bypassed at once:
ToolSearchTool.maxOutputChars→Number.POSITIVE_INFINITY(opts out of per-tool char truncation)ToolNames.TOOL_SEARCHadded toGATE_EXEMPT_TOOLS(opts out ofmaybePersistLargeToolResultdisk spill)- the only remaining bound,
revealOversizedSchemasDirectly, returns early whenpresentations.length === 0
presentations is empty whenever isSubagentLikeExecutionContext() is true, and also for select: on already-visible / alwaysLoad tools. So inside a subagent, a broad keyword search over a large MCP catalog can now inject arbitrarily large schema text with no truncation and no spill — previously that content was gated. Suggest keeping a byte bound on the no-presentation path (or moving the budget check ahead of the presentations.length === 0 early return and truncating instead of returning undefined).
3. Steer input is now accepted at the first stream event, and is no longer restored on a mid-stream error (should confirm)
onContextAccepted: () => {
drainedSteer?.accept();
commitDeferredToolPresentations();
...
},resolveDrainedSteerMessages latches settled, so once accept() fires, the later restore() from reportDeliveryFailure is a no-op. Previously onDelivered: drainedSteer?.accept ran only after a clean stream, and any errored prompt (lastPromptErroredRef.current) restored the queued text.
New behaviour: Content event arrives → accept → stream errors → the user's steered message is not re-queued. If the user then retries, AppContainer.tsx:2634 strips the orphaned user entry containing those parts, so the typed text is silently lost. Arguably the old restore was also wrong (it could duplicate text already in history), but the two paths now disagree and one of them drops user input. Worth decoupling the presentation-commit signal from the steer-accept signal, or restoring only when the failed turn is actually stripped.
4. Oversized fallback is batch-wide and threshold-fuzzy
if (presentations.length === 0 || !Number.isFinite(budget) || budget <= 0 ||
llmContent.length <= budget) return undefined;
...
const names = [...new Set(presentations.map(({ name }) => name))];The PR description says "a single schema too large to fit atomically," but the implementation trips on the combined llmContent, then reveals every presented tool directly. Two consequences: (a) N individually-small schemas invalidate the tool list for all of them, and (b) llmContent at that point already includes the missing / blocked / truncated footers, so error text counts toward the budget. Selecting only the schemas that genuinely don't fit — and leaving the rest inline — would keep the cache-stability win in the common case.
5. convertToFunctionResponse change affects every MCP tool result, not just proxied calls
- // It's a functionResponse that we should pass through as is.
- return [contentToProcess];
+ return [{ functionResponse: { ...contentToProcess.functionResponse, id: callId, name: toolName } }];wrapMcpCallToolResultAsParts (mcp-tool.ts:753) emits functionResponse with no id. After this change every MCP tool result carries id: callId. That's plausibly a latent bug fix, but it changes the wire shape for all MCP calls on all providers in every session — well outside "deferred tool discovery." Please call it out in the Risk & Scope section (and ideally note which providers were exercised).
6. Excluding deferred_tool_call silently disables tool_search and all deferral
if (!deferredToolCallRegistered) {
registry.unregisterFactory(ToolNames.TOOL_SEARCH);
}A deny rule or --exclude-tools deferred_tool_call therefore also removes tool_search, and resolveDeferredToolsForReminder then eagerly reveals every deferred tool into the declaration list. That is a large, surprising blast radius from denying one tool name, with no log line. A debugLogger.warn (or better, a startup warning) explaining the coupling would save a lot of debugging.
7. The scheduler now holds isFinalizingToolCalls across a model round trip
onAllToolCallsComplete returns the acceptance promise, which settles on the first event of the next model request. _schedule's finally — which resets isFinalizingToolCalls and drains requestQueue — is therefore delayed by the model's TTFT (plus any pre-send compression/hook work). All settle paths look covered (onContextAccepted, onDeliveryFailed, and both .then arms), so no deadlock, but it is a new liveness dependency on a network round trip inside a scheduler lock. Worth a comment at minimum, and worth checking that queued client-initiated tool calls don't visibly stall.
8. Compression ordering differs between CLI and ACP
client.ts clears presentations on ChatCompressed before yield event, so the CLI's wrapper commits after the clear (net: kept). ACP calls commitDeferredToolPresentationsForDeliveredMessage before iterating, and its wrapper clears on StreamEventType.COMPRESSED (net: dropped). Same scenario, opposite outcomes. ACP's direction is the safe one, so this is not a correctness bug, but the asymmetry is undocumented and will confuse the next reader — one of the two should be aligned or the divergence commented.
Nits
deferred_tool_callisalwaysLoad: true— it is declared in every session, including ones with zero deferred tools. Understandable (conditional declaration would itself mutate the tool list when an MCP server connects), but it's a permanent unconditional cost; worth a sentence in the design doc.- Fingerprints are key-order sensitive.
getFunctionSchemaFingerprintandmcp-tool.ts'sschemaSnapshotboth hashJSON.stringify(schema). An MCP server that reorders JSON keys without changing semantics invalidates the presentation / aborts the reconnect retry. Harmless (fail-closed) but noisy; canonical-key serialization would be more robust. - Reserved-name handling is asymmetric.
registerToolwarns and skips fordeferred_tool_call;registerFactorythrows. AlsoregisterLazynow pre-checksthis.getDisabledTools().has(toolName)whileregisterFactoryusesisToolDisabled(name, aliases)(which additionally normalizesmcp__names). They agree for built-in names today, but two gates that must stay in sync will drift — havingregisterFactoryreturn a boolean would remove the duplication. - Fail-closed message can loop.
Schema for deferred tool "X" has not been fetched in the active context. Use tool_search first, then call deferred_tool_call on a later turn.tells the model to do precisely what it just did. If the commit was skipped (early-returnedsubmitQuery, stream rejection),collectCandidateswill re-surface the tool and the model can loop search → proxy → search.recordBatchRetryableToolError/RETRY_LOOP_STOP_DIRECTIVEdoes bound it, but the message could hint at the actual failure ("the previous tool_search result was not delivered"). - No user-facing docs. Only
docs/design/prompt-cache/deferred-tool-call-stable-schema.md(which is excellent). A new model-visible tool that changes how every MCP tool is invoked probably deserves a line in the user tool reference. enterPlanMode.ts: removing theexit_plan_modereveal is correct dead code removal (ExitPlanModeToolisalwaysLoad: true) — no behavior change. 👍Session.tstry/finally→try/catch+ explicit call aroundrefreshMemoryIfNeeded()is semantically equivalent for both return and throw. 👍
Verdict
The core mechanism, the fingerprint binding, the fail-closed normalization, and the lifecycle coverage are well built, and the test suite backs it up. I'd hold on merging until (1) the ACP orphan-strip bypass and (2) the unbounded tool_search output on the no-presentation path are addressed, and (3) the steer-accept semantics change is confirmed intentional. The rest are cleanups and documentation.
|
@qwen-code /resolve |
|
Qwen Code did not run conflict resolution for this request. PR #8276 does not currently have merge conflicts with main. |
|
Thanks for the detailed review. I followed up on findings 1-3 against the current head.
Validation for the fix: ACP Session 494/494, CLI typecheck, changed-file lint, pre-commit hooks, and the full repository build all pass. |
Deferred tools prompt-cache A/B benchmarkGenerated: 2026-08-01T16:48:27.349Z Environment and method
Result
Paired mean improvement: 97.00 percentage points (95% CI 95.97 to 98.04); mean uncached-token change: -23597.2 tokens/run. All main-agent requests
Across all 45 main-agent requests per arm, the PR improved the weighted hit rate by 32.27 percentage points and reduced uncached input tokens by 352,359 (44.97%). Request phase breakdown
The post-target aggregate includes zoom_image, whose inline image made that phase a cache miss in both arms. Across the other four tasks, post-target hit rates were 99.49% for baseline and 99.42% for PR. By task (post-reveal weighted hit rate)
By repeat (post-reveal weighted hit rate)
Protocol checks
Scope limits
Excluded preflight
|
# Conflicts: # packages/cli/src/acp-integration/session/Session.ts
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed.
7 Suggestion-level finding(s) this review confirmed are already reported on this PR and are not repeated:
- D25-8 strip retain branch has no pinning test — already reported (comment 3824717371, geminiChat.ts:4606)
- D25-12 aggregate-overflow refusal lacks error status — already reported (comment 3759608875, tool-search.ts:696)
- D25-13 delegating wrapper mock vacates sibling strip assertion — already reported (comment 3756164538, Session.test.ts:677)
- D25-19 pre-push compression suppresses onContextAccepted for accepted sends — already reported (comment 3764717070, useGeminiStream.ts:3933)
- D25-24 daemon turn-loop guard misses the unwrap — already reported (comment 3782025721, loopDetectionService.ts:282)
- D25-33 canonicalToolName hardening has no direct test — already reported (comment 3792016739, tool-names.ts:155)
- D25-36 setTools-failure refusal drops directlyDeclared diagnostic — already reported (comment 3736780357, tool-search.test.ts:1308)
Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.
Not explored to full depth (tool budget reached): chunk 5: run the six new ledger-rollback tests under vitest (shared review worktree has no node_modules; monorepo install/build skipped to keep the shared tree read-only….
Not reviewed: reverse audit — stopped before round 3 by the review time budget.
Deferred under the convergence posture (round 25, not a blocker) — recorded, not requested in this round:
docs/design/prompt-cache/deferred-tool-call-stable-schema.md:71 — [review] design doc and PR body claim the ledger is removed; shipped code adds it and gates execution on itpackages/cli/src/acp-integration/session/Session.test.ts:30488 — [review] compression restore test cannot distinguish refused from applied (empty snapshot)packages/cli/src/acp-integration/session/Session.ts:6320 — [review] #runStopContinuation catch rolls back marks after preservation already landed them in historypackages/cli/src/nonInteractiveCli.ts:1983 — [review] gate-rejected calls emitted with target name but wrapper-envelope args in JSON outputpackages/cli/src/ui/hooks/useGeminiStream.ts:4229 — [review] flush commit gate === true stricter than settlement decode !== false (addHistory-backed exits lose presentations)packages/core/src/core/client.ts:373 — [review] dangling references to deleted/renamed reminder symbols (comment + preload-threshold design doc)packages/core/src/utils/environmentContext.ts:105 — [review] stale security comment cites deleted formatDeferredToolLinepackages/cli/src/ui/hooks/useGeminiStream.ts:5430 — [review] steer restore on onAdmissionFailed has no paired testpackages/core/src/core/client.test.ts:2654 — [review] deleted test was the only coverage of the strip wrapper's no-strip branchpackages/cli/src/acp-integration/session/Session.test.ts:26973 — [review] post-execution cancellation x ledger-commit intersection untestedpackages/cli/src/acp-integration/session/Session.test.ts:30116 — [review] rollback tests restore only empty snapshots; content fidelity untestedpackages/cli/src/acp-integration/session/Session.test.ts:30150 — [review] push-count rollback predicate never modeled (constant 0 <= 0)packages/cli/src/nonInteractiveCli.test.ts:2955 — [review] R23-30 release guard's fingerprint-mismatch (do-not-delete) branch untestedpackages/cli/src/nonInteractiveCli.test.ts:3197 — [review] arm-time generation wiring unpinned; no intervening-clear test on the headless surfacepackages/core/src/core/client.test.ts:111 — [review] two-tool AND gate unpinned: &&-to-|| mutant keeps all reveal tests green (probe-verified)packages/core/src/core/client.ts:1288 — [review] preload skip guard checks only TOOL_SEARCH while eager reveal requires both control toolspackages/core/src/core/coreToolScheduler.ts:2409 — [review] retry-loop detector defeated by its own instructed tool_search remediation (probe-verified)packages/core/src/core/coreToolScheduler.ts:2569 — [review] wrapper deny gate runs after normalization; target loaded first, denial preemptedpackages/core/src/tools/mcp-tool.ts:391 — [review] reconnect guard byte-compares the full declaration; key-order/description changes reject valid replayspackages/core/src/tools/tool-search.ts:385 — [review] subagent surface equates not-hidden with declared (probe-verified fail-open for not-hidden undeclared)- …and 9 more (see the run report)
中文说明
仅完成部分审查,审查缺口已披露。
本轮确认的 7 条建议级发现已在 PR 上报告过,不再重复发布(列表见上方英文部分)。
未审查:build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。
未探索到全部深度(达到工具调用预算):chunk 5:run the six new ledger-rollback tests under vitest (shared review worktree has no node_modules; monorepo install/build skipped to keep the shared tree read-only…。
未审查:反向审计——评审时间预算不足,未能开始第 3 轮。
收敛姿态下延后(第 25 轮,非阻断)——已记录,本轮不要求修改:共 29 条(原文未翻译,列表见上方英文部分)。
— qwen3.8-max via Qwen Code /review (v0.21.15)
The declaredToolNames set prepareTools() recorded was scoped to the AsyncLocalStorage frame alive during preparation and died with it. Every round woken later enters a fresh frame shallow-copied from the delivery caller's ambient store: an idle AgentInteractive woken by enqueueMessage() (top-level delivery -> undefined; delivery inside another agent's tool body -> the SENDER's set, since runInContext restores only teammate identity), background-agent continuation turns (fresh per-turn frame while AgentHeadless caches toolsList and skips prepareTools), resumed background-agent turns, and deferred-approval restorations. tool_search's select: gate then misread the agent's surface in every round after the first — blocking registry-hidden deferred tools the agent had declared and could call directly, or failing open off the sender's set (qwen-code-ci-bot R25-1). Persist the recorded set on AgentCore and re-record it onto the live frame in runInAgentFrames() — the single entry every reasoning loop (AgentInteractive and AgentHeadless alike) and every deferred-approval continuation passes through. No-op until the core's first prepareTools() completes, so frames inheriting a shallow copy keep it. Also corrects the stale prepareTools comment: forks prepare an explicit parent-name list rather than skipping prepareTools. Tests: woken-frame re-recording after start-frame unwind, sender-set overwrite (sender frame left intact), full runReasoningLoop path, restored deferred-approval frame, never-prepared no-op invariant.
# Conflicts: # packages/core/src/core/coreToolScheduler.ts
# Conflicts: # packages/cli/src/acp-integration/session/Session.ts
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed.
2 Suggestion-level finding(s) this review confirmed are already reported on this PR and are not repeated:
- R4-13 design doc contradicts the shipped presentation-ledger code — already reported (comment 3736780308)
- SDK E2E tests have no pre-merge gate — already reported (comment 3747401940)
Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.
Not explored to full depth (tool budget reached): "agent reverse-audit (round 1)": verifying which pre-PR presentation/reminder mechanism the design doc's "The old schema-presentation ledger is removed" sentence refers to — merge-base tool-re…; "agent invariant-a (packages/core/src/tools/tool-search.ts)": none — all checks in my slice completed (~13 tool calls)..
Not reviewed: reverse audit — stopped before round 3 by the review time budget.
Deferred under the convergence posture (round 26, not a blocker) — recorded, not requested in this round:
packages/cli/src/acp-integration/session/Session.test.ts:30830 (+1 locations) — [review] The mid-send-compression test starts from an empty ledger,…packages/cli/src/acp-integration/session/Session.test.ts:30857 (+1 locations) — [review] All six added ledger-rollback tests run against a mockChat…packages/cli/src/acp-integration/session/Session.ts:5306 (+2 locations) — [review] Empty catch {} around restoreProxySchemaPresentationSnapsho…packages/cli/src/acp-integration/session/Session.ts:6364 (+1 locations) — [review] In #runStopContinuation's catch, the pre-existing preserve…packages/cli/src/nonInteractiveCli.ts:1993 (+1 locations) — [review] The new headless batch gate emits wrapper-rejection…packages/cli/src/ui/hooks/useGeminiStream.test.tsx:2894 (+1 locations) — [review] The R23-46 test never exercises or pins the 'accepted…packages/cli/src/ui/hooks/useGeminiStream.ts:5341 (+1 locations) — [review] The capacity-exhaustion exit returns false (discard…packages/core/src/core/client.test.ts:2266 (+1 locations) — [review] The diff's two-control-tool proxy gate silently vacates…packages/core/src/core/client.test.ts:2696 (+1 locations) — [review] This diff deletes the only test pinning the…packages/core/src/core/coreToolScheduler.ts:2515 (+1 locations) — [review] The new wrapper permission gate runs AFTER normalization,…packages/core/src/core/coreToolScheduler.ts:5638 (+1 locations) — [review] The producer-boundary mutation detector…packages/core/src/core/geminiChat.ts:4338 (+1 locations) — [review] The #6721 ledger-clearing added to setHistory/truncateHisto…packages/core/src/tools/tool-registry.ts:293 (+1 locations) — [review] The new reserved-name defense reserves only tool_call; its…packages/core/src/tools/tool-registry.ts:840 (+1 locations) — [review] The added isProxyEligibleDeferredTool is a verbatim…packages/core/src/tools/tool-registry.ts:966 (+1 locations) — [review] markProxySchemaPresented is a second, single-item write…packages/core/src/tools/tool-search.test.ts:1000 (+1 locations) — [review] Rewriting this test deleted the suite's only pin of a…packages/core/src/tools/tool-search.ts:385 (+1 locations) — [review] The subagent block gate predicates on registry-level…packages/core/src/tools/tool-search.ts:435 (+1 locations) — [review] schemaFingerprint hashes the entire declaration…packages/core/src/tools/tool-search.ts:591 (+1 locations) — [review] In the no-deferred-names oversized-budget refusal, the…packages/core/src/tools/tool-search.ts:654 (+1 locations) — [review] The setTools-failure refusal in revealOversizedSchemasDirec…- …and 2 more (see the run report)
Convergence: round 26 posted 1 inline comment(s), 1 of them reported for the first time; the previous round posted 1 (1 new). The rate of new findings is not falling. Batching the remaining fixes and verifying them before the next push keeps the loop from re-deriving the same set; this PR's reviews already resolve to a critical posting floor. (Observation only — nothing was withheld from this review because of this observation.)
中文说明
仅完成部分审查,审查缺口已披露。
本轮确认的 2 条建议级发现已在 PR 上报告过,不再重复发布(列表见上方英文部分)。
未审查:build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。
未探索到全部深度(达到工具调用预算):"agent reverse-audit (round 1)":verifying which pre-PR presentation/reminder mechanism the design doc's "The old schema-presentation ledger is removed" sentence refers to — merge-base tool-re…;"agent invariant-a (packages/core/src/tools/tool-search.ts)":none — all checks in my slice completed (~13 tool calls).。
未审查:反向审计——评审时间预算不足,未能开始第 3 轮。
收敛姿态下延后(第 26 轮,非阻断)——已记录,本轮不要求修改:共 22 条(原文未翻译,列表见上方英文部分)。
收敛情况:第 26 轮发布了 1 条行内评论,其中 1 条是首次提出;上一轮发布了 1 条(其中 1 条首次提出)。新发现的产出速度没有下降。把剩余修复攒成一批、验证后再推送,可以避免循环反复推导同一组发现;本 PR 的评审已解析为 critical 发布下限。(仅为观察——本轮评审未因此扣留任何内容。)
— qwen3.8-max via Qwen Code /review (v0.22.0)
R26-1 alleged that R24-1's exclusion of error responses from the admission gate (getHistoryToolCallFingerprints) leaves the sibling getHistoryFunctionResponseIds — consumed by the TUI handleCompletedTools late-result dedup — still counting the gate-rejection response, so the admitted retry's completed result would be silently dropped as "already responded" and a synthetic error placeholder planted in its place. The premise conflates the reused provider tool-call id with the internal callId the dedup actually keys on. The rejection response ships under the wrapper call's internal callId (createErrorResponse uses request.callId), while the instructed re-issue is a fresh call that the production stream path (processStreamResponse -> normalizeModelToolCallIds) suffixes to a brand-new internal id. Gate B and the handleCompletedTools dedup predicate (historyCallIdsWithResponse.has(tc.request.callId)) both key on the internal callId, so the re-issue's result never collides with the rejection response and is delivered, not dropped. Pin the contract with two regression tests: - the rejection id stays in gate B (Race-A requires matching error responses) yet is not marked handled in gate A (R24-1); - the instructed re-issue normalizes to a suffixed internal id that gate B does not contain, so the dedup predicate is false and the result ships.
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
No blocking issues. LGTM! ✅
Deferred under the convergence posture (round 27, not a blocker) — recorded, not requested in this round:
packages/core/src/core/geminiChat.test.ts:16444 — [review] registry-with-ledger mock written three times in this diff with already-divergent casts (two inline copies + one helper)packages/core/src/core/geminiChat.test.ts:16474 — [probe] stripThoughtsFromHistory negative ledger test never seeds history — vacuous for the conditional-clear regression it pins
中文说明
无阻断问题。LGTM!✅
收敛姿态下延后(第 27 轮,非阻断)——已记录,本轮不要求修改:共 2 条(原文未翻译,列表见上方英文部分)。
— qwen3.8-max via Qwen Code /review (v0.22.0)
# Conflicts: # packages/core/src/tools/mcp-tool.ts
Same latent main-side violation as QwenLM#9682/QwenLM#9683-era fixes: QwenLM#9587 grew the workflow without a baseline update; record the new size as the check message directs (precedent QwenLM#9747).
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed.
Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.
Not explored to full depth (tool budget reached): chunk 2: executing the new/updated tests in packages/cli ( npx vitest run src/acp-integration/session/Session.test.ts ) — worktree has no node_modules or built works…; chunk 11: none — no check was cut short.; chunk 9: execute the three new nonInteractiveCli.test.ts cases to confirm green — the review worktree has no node_modules or built package dists, and a full npm ci + b….
Not reviewed: reverse audit — stopped before round 3 by the review time budget.
Deferred under the convergence posture (round 28, not a blocker) — recorded, not requested in this round:
packages/core/src/utils/environmentContext.ts:105 (+3 locations) — [review] Stale comments/doc references to deleted deferred/MCP reminder machinerypackages/cli/src/acp-integration/session/Session.test.ts:27555 — [review] Partial DaemonToolLoopState objects smuggled past the type via bespoke castspackages/cli/src/acp-integration/session/Session.test.ts:30824 — [review] Compression-clear test cannot observe the generation guardpackages/cli/src/acp-integration/session/Session.ts:4948 — [review] Send-failure rollback sequence hand-copied into four loops with drifting namespackages/cli/src/ui/hooks/useGeminiStream.ts:3896 — [review] Pre-send compression forfeits delivered presentationspackages/core/src/core/client.test.ts:2266 — [review] History re-reveal test no longer isolates the scan (mock not migrated to two-tool check)packages/core/src/core/client.test.ts:2694 — [review] Deleted the strip wrapper's negative-branch testpackages/core/src/tools/tool-registry.ts:966 — [review] markProxySchemaPresented: dead public writer into the #6721 ledger + misleading {@link}packages/core/src/tools/tool-search.ts:654 — [review] setTools-failure refusal omits the 'Already declared and directly callable' sectionpackages/core/src/core/geminiChat.ts:3271 — [review] Producer-side payloadRebuilt flag untestedpackages/core/src/core/geminiChat.ts:4593 — [review] isCompletedToolResult preservation branch unpinnedpackages/core/src/services/loopDetectionService.ts:376 — [probe] Loop-detection unwrap covered only by a mutation-green test (paired interleaves evade)packages/cli/src/acp-integration/session/Session.ts:9874 — [review] ACP release of the replay record untested (resolve together with the dead-store finding)integration-tests/sdk-typescript/sdk-mcp-server.test.ts:85 — [test] E2E deferred-bridge coverage runs in no pre-merge CI jobpackages/cli/src/acp-integration/session/Session.test.ts:30491 — [review] Rollback tests degenerate the push-count guard (mock lacks getUserContentPushCount)packages/core/src/core/coreToolScheduler.test.ts:2700 — [review] No-consumer settlement branch unpinned; dead disableCompletionCallback switchpackages/core/src/core/coreToolScheduler.test.ts:4218 — [review] Atomic tool_search test never exercises the batch-budget exemptionpackages/core/src/core/deferred-tool-call-normalization.test.ts:415 — [review] Snapshot fingerprint-mismatch branch unpinnedpackages/cli/src/acp-integration/session/Session.ts:847 — [review] Daemon loop-detection mirror lacks the unwrap (core/daemon key drift)packages/core/src/tools/tools.ts:504 — [review] Presentation fingerprint hashes volatile description prose (month boundary)- …and 13 more (see the run report)
中文说明
仅完成部分审查,审查缺口已披露。
未审查:build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。
未探索到全部深度(达到工具调用预算):chunk 2:executing the new/updated tests in packages/cli ( npx vitest run src/acp-integration/session/Session.test.ts ) — worktree has no node_modules or built works…;chunk 11:none — no check was cut short.;chunk 9:execute the three new nonInteractiveCli.test.ts cases to confirm green — the review worktree has no node_modules or built package dists, and a full npm ci + b…。
未审查:反向审计——评审时间预算不足,未能开始第 3 轮。
收敛姿态下延后(第 28 轮,非阻断)——已记录,本轮不要求修改:共 33 条(原文未翻译,列表见上方英文部分)。
— qwen3.8-max via Qwen Code /review (v0.22.0)
# Conflicts: # packages/core/src/agents/background-agent-resume.ts # packages/core/src/core/coreToolScheduler.ts # packages/core/src/tools/tool-registry.ts
…ed retries
R28-1: getHistoryToolCallFingerprints excludes error responses (R24-1)
and keys successes by their normalized internal id, so a retry that
succeeded after a gate rejection landed only under the suffixed id
(`tool_call_X__qwen_dup_N`) while the raw provider id stayed unmarked.
Every surface re-seeds this map from history (TUI per-batch after its
per-submit ref clear, daemon per runToolCalls, headless per run, agent
runtime per batch) and keys the replay check on the RAW provider id —
which restarts per response on `{name}_{index}` providers — so a
history-only re-seed admitted an identical re-issue under the restarted
raw id and re-executed the side-effecting tool.
Stamp every suffixed success under its base raw provider id as well
(first occurrence wins, mirroring recordHandledToolCall's surface-side
semantics). Centralized in the accessor, so all four re-seed surfaces
are covered. R24-1 semantics are preserved: error-only histories still
reseed empty, so the gate's instructed retry is admitted.
Tests: error→suffixed-success history reseed suppresses the raw-id
re-issue (fails pre-fix); error-only retry chain stamps nothing; base's
own success fingerprint wins over a later stamp; second R24-1 test
updated to assert the base key. geminiChat 343/343, agent-core 42/42,
nonInteractiveCli 138+1 skip, Session 707/708 (+1 known load-flake
passing alone).
… set in tool_search R28-2: in subagent-like contexts the keyword-candidate branch and the `select:` gate equated "not isDeferredAndHidden" with "declared for this agent". That is false for explicit-tool-list subagents: a deferred tool made visible via settings.tools.visible (which propagates through Object.create(base) config inheritance) or revealed earlier is not hidden, yet prepareTools declared only the names the agent lists (getFunctionDeclarationsFiltered) — so the delivered bare schema invited a direct call the provider rejects as an unknown function, the exact failure the R24-3 gate's own comment says it prevents. Gate every deferred tool on the recorded declaration set when one exists — block when getCurrentAgentDeclaredToolNames() is present and omits the name, fail closed on undefined — via a new ToolRegistry.isDeferredTool predicate; align collectCandidates' subagent branch and the blocked-message wording with the same criterion. Wildcard agents and forks declaring the tool are unaffected (they record the declared names); plan-required teammates keep exit_plan_mode because TeamManager injects it into their tool list and prepareTools records it. Tests: explicit-list frames omitting a visible/revealed deferred tool are blocked for both select: and keyword search (fail pre-fix); declared frames keep access; oversized-batch and plan-teammate tests updated to record production-realistic declaration sets. tool-search 77/77.
…scheduler R28-3: a flushed batch taking the terminatesGoalTurn exit plants its results into history via addHistory and returns undefined — documented as "accepted at settlement … presentations ARE backed by history" — but the flush gate committed only on `flushedAccepted === true`, so those history-backed presentations were silently discarded. The direct path decodes undefined as accepted (the scheduler settles with `deliveryAccepted !== false`), so the two paths disagreed: the schema sat in the model context yet uncommitted in the QwenLM#6721 ledger, the model's subsequent direct tool_call was refused, and a redundant re-search was burned. Decode the flush result exactly like the scheduler's settlement (`!== false`). Every discard exit returns an explicit `false` (R20-4), so the looser check cannot admit one; onDeliveredCallIds fires before the goal exit, so the delivered-set filter stays intact. Test: deferred batch mixing a schema-carrying tool_search result with a terminateTurn goal-tool result commits its presentations on flush (commit count was 0 pre-fix). useGeminiStream 249/249.
Resolve conflicts from the utils/ -> core/ + tools/ module relocation: - environmentContext(.test).ts: keep this PR's trimmed import set (ToolNames/DeferredToolSummary/SendMessageTool removed with the deferred catalog move to tool search; 332a0a5), adopt upstream's new ../utils/ relative paths. - loopDetectionService.ts: keep unwrapDeferredToolCallShape import, follow tool-call-repeat-key to ../tools/ (still a leaf module: only node:crypto + tool-names.js, no new import cycle). - tool-response-finalizer.ts: keep the ToolNames import added for the TOOL_SEARCH resolved-slot check (1ce8cfe), adopt upstream's ./tools.js path after the move into tools/. - agent-core.test.ts: dedupe CoreToolScheduler import added independently by both sides.
# Conflicts: # packages/core/src/config/config.ts
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed.
Not reviewed: build-and-test — "Integration Tests (CLI, No Sandbox)" was skipped in CI and its suite did not run locally.
Not explored to full depth (tool budget reached): chunk 32: executing the new tests with vitest (worktree has no node_modules and no packages/core/dist ; a full npm ci + build exceeded the remaining tool budget) — a…; chunk 28: executing deferred-tool-call-normalization.test.ts via vitest — the review worktree has no installed node_modules (vitest unresolvable); install/build not attem….
Not reviewed: reverse audit — stopped before round 2 by the review time budget.
Deferred under the convergence posture (round 29, not a blocker) — recorded, not requested in this round:
docs/design/prompt-cache/deferred-tool-call-stable-schema.md:71 — [review] design doc claims the presentation ledger is removed; the ledger is load-bearingpackages/cli/src/acp-integration/session/Session.ts:6410 — [review] continuation catch rolls the ledger back after preserving the carrying messagepackages/core/src/core/environmentContext.ts:105 — [review] stale comment citing deleted formatDeferredToolLinepackages/core/src/tools/tool-registry.ts:858 — [review] isProxyEligibleDeferredTool duplicates isDeferredAndHiddenpackages/core/src/tools/tool-registry.ts:1007 — [review] markProxySchemaPresented is a dead ledger write path the gate tests usepackages/core/src/tools/tool-search.ts:547 — [probe] oversized refusal can exceed the budget it enforcespackages/core/src/tools/tool-search.ts:707 — [probe] aggregate-overflow refusal returns without error (executionStatus 'success')packages/core/src/tools/tool-search.ts:594 — [probe] oversized refusal misattributes when zero schemas loadedpackages/core/src/core/geminiChat.ts:3590 — [review] payloadRebuilt producer contract untestedpackages/cli/src/acp-integration/session/Session.ts:9922 — [review] daemon R23-30 replay-record release untestedpackages/cli/src/ui/hooks/useGeminiStream.ts:1012 — [review] TUI normalization-rejected release callback untestedpackages/cli/src/acp-integration/session/Session.test.ts:30983 — [probe] generation-rollback test cannot distinguish refused vs accepted restorepackages/cli/src/nonInteractiveCli.test.ts:3102 — [probe] R23-30 test does not pin the fingerprint guard on the releasepackages/cli/src/ui/hooks/useGeminiStream.ts:3901 — [review] pre-send compression suppresses onContextAccepted (R17-2 residual)packages/cli/src/ui/hooks/useGeminiStream.ts:5370 — [review] capacity-exhausted exit plants history but returns falsepackages/core/src/core/coreToolScheduler.ts:5638 — [probe] producerContentEqual name mismatch flags every proxied call mutatedpackages/core/src/services/loopDetectionService.ts:336 — [probe] daemon loop guard keys raw wrapper, missing mixed direct+proxied repeatspackages/core/src/tools/tool-response-finalizer.ts:123 — [probe] per-slot tool_search exemption unbounds the batch budgetpackages/core/src/tools/tools.ts:562 — [review] doc {@link} names the dead ledger write pathintegration-tests/sdk-typescript/sdk-mcp-server.test.ts:86 — [test] bridge E2E not collected by any workspace test suite- …and 1 more (see the run report)
中文说明
仅完成部分审查,审查缺口已披露。
未审查:build-and-test — "Integration Tests (CLI, No Sandbox)" was skipped in CI and its suite did not run locally。
未探索到全部深度(达到工具调用预算):chunk 32:executing the new tests with vitest (worktree has no node_modules and no packages/core/dist ; a full npm ci + build exceeded the remaining tool budget) — a…;chunk 28:executing deferred-tool-call-normalization.test.ts via vitest — the review worktree has no installed node_modules (vitest unresolvable); install/build not attem…。
未审查:反向审计——评审时间预算不足,未能开始第 2 轮。
收敛姿态下延后(第 29 轮,非阻断)——已记录,本轮不要求修改:共 21 条(原文未翻译,列表见上方英文部分)。
— qwen3.8-max via Qwen Code /review (v0.22.0)
# Conflicts: # packages/core/src/core/coreToolScheduler.test.ts # packages/core/src/core/coreToolScheduler.ts
…etry test The merge with main left getSessionDisplayName and getCurrentCustomTitle declared twice in the same object literals (TS1117). Keep one of each — the vi.fn() variants where spied.
The design doc claimed the schema-presentation ledger was removed and that execution no longer depends on whether a schema was presented. Neither is true at this head. `proxySchemaPresentations` is alive in the registry, and `tool_call` still refuses a target whose mark is missing or whose schema fingerprint has moved — issue QwenLM#6721's fail-closed gate against routing guessed or stale arguments. What this branch removed is narrower: the *reminder re-injection* paths that kept re-announcing the catalog into conversation history (`buildDeferredToolsReminder`, the added/changed MCP tool reminders, and the `client.ts` bookkeeping behind them). The catalog now lives in `tool_search`'s own description, rebuilt from the live registry on every declaration read, so nothing has to be restored after compression or resume. The ledger is a different mechanism with a different job, and conflating the two read as if the safety gate had been deleted. Correct the doc, and document the ledger's lifecycle explicitly: commit only after the carrying `tool_search` result enters active history, roll back a failed send, gate a sequential batch against a pre-batch snapshot so a sibling `tool_search` cannot self-authorize a `tool_call`, and clear on every history mutation that can evict a tool result. Also refresh descriptions the reminder removal left stale: comments in four files still enumerated "mid-history MCP added-tool reminders" and "resume-restored deferred schemas" as live history entry kinds, three references still named the renamed `resolveDeferredToolsForReminder`, the subagent test's header miscounted the reminder parts it guards, and the profiler and fork-resume docs still listed a deferred reminder stage that no longer exists. Docs and comments only; no behavior change.
…lowlist `tool_call` was added to `ToolNames`/`ToolDisplayNames` without a rule-parser alias, which is exactly the QwenLM#9827 drift the exhaustiveness test guards: a rule naming the bridge parsed as an unknown tool and matched nothing. The allowlist exemption was the load-bearing half. `tool_search` is exempt so that a narrow allowlist cannot collapse deferred discovery into an eager reveal of every deferred schema — but config.ts registers the two as a pair and unregisters `tool_search` whenever `tool_call` is unavailable. Gating the bridge therefore reached that same bloat by another route and silently undid the exemption above it, so an allowlist user lost the prefix-cache benefit this branch exists to deliver. Exempting the bridge grants no execution. `normalizeDeferredToolCallRequest` rewrites the request to the real target before scheduling, so the target's own allow and deny rules still gate the call, and an explicit `tool_call` deny still removes the bridge.
Resolve conflicts: - Session.ts: keep the presentation-ledger snapshot tracking (PR) and the new repeated-tool-failure stop early return (upstream QwenLM#9992) in the main prompt loop; snapshot is tracked first, matching the sibling loop the branch already integrated, since the stop path preserves history inside #buildNextMessageAfterToolRun. - Session.test.ts: union of mock additions (history-wrapper mocks from the PR + managed auto-memory / memory-manager mocks from upstream QwenLM#9992).
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed.
Unresolved, please confirm:
- [Critical] packages/cli/src/acp-integration/session/Session.test.ts — ACP same-batch self-authorization blessing (comment 3817109623) — cannot tell: this round's time budget expired before the per-blocker mechanism re-check at the reviewed commit
- [Critical] packages/cli/src/acp-integration/session/Session.ts — COMPRESSED wrapper clears only registry presentations (comment 3747401872) — cannot tell: this round's time budget expired before the per-blocker mechanism re-check at the reviewed commi…
- [Critical] packages/cli/src/acp-integration/session/Session.ts:5426 — R22-1 send-failure rollback resurrection (comment 3828625658) — cannot tell: this round's time budget expired before the per-blocker mechanism re-check at the reviewed commit
- [Critical] packages/cli/src/acp-integration/session/Session.ts:8355 — R23-27 top-of-lap abort drops carrying message (comment 3832640919) — cannot tell: this round's time budget expired before the per-blocker mechanism re-check at the reviewed commit
- [Critical] packages/cli/src/acp-integration/session/Session.ts:9692 — daemon commit-before-send with no rollback (comment 3824717380) — cannot tell: this round's time budget expired before the per-blocker mechanism re-check at the reviewed commit
- [Critical] packages/cli/src/acp-integration/session/Session.ts:9692 — R21-2 rollback missing on two daemon surfaces (comment 3828625673) — cannot tell: this round's time budget expired before the per-blocker mechanism re-check at the reviewed commit
- [Critical] packages/cli/src/acp-integration/session/Session.ts:9744 — daemon finalizeRunToolResult schema-truncation hole (comment 3817109658) — cannot tell: this round's time budget expired before the per-blocker mechanism re-check at the reviewed co…
- [Critical] packages/cli/src/acp-integration/session/Session.ts:10680 — R18-1 daemon wrapper deny rules never enforced (comment 3817109635) — cannot tell: this round's time budget expired before the per-blocker mechanism re-check at the reviewed commit
- [Critical] packages/cli/src/acp-integration/session/Session.ts:12123 — R4-27 fourth response-part site missed (comment 3736780228) — cannot tell: this round's time budget expired before the per-blocker mechanism re-check at the reviewed commit
- [Critical] packages/cli/src/nonInteractiveCli.test.ts — key-presence assertion pattern (comment 3759608952) — cannot tell: this round's time budget expired before the per-blocker mechanism re-check at the reviewed commit
- [Critical] packages/cli/src/nonInteractiveCli.ts — headless commits presentations before carrying history (comment 3747401864) — cannot tell: this round's time budget expired before the per-blocker mechanism re-check at the reviewed commit
- [Critical] packages/cli/src/nonInteractiveCli.ts:490 — headless same-batch self-authorization (comment 3817109642) — cannot tell: this round's time budget expired before the per-blocker mechanism re-check at the reviewed commit
- [Critical] packages/cli/src/nonInteractiveCli.ts:1040 — R24-2 settlePendingPresentationLedger wrong-send attribution (comment 3835699738) — cannot tell: this round's time budget expired before the per-blocker mechanism re-check at the reviewed commit
- [Critical] packages/cli/src/nonInteractiveCli.ts:1998 — R23-30 gate rejects after admission record (comment 3832640926) — cannot tell: this round's time budget expired before the per-blocker mechanism re-check at the reviewed commit
- [Critical] packages/cli/src/ui/hooks/useGeminiStream.ts — R20-5 dedup-dropped presentations still settled (comment 3828625666) — cannot tell: this round's time budget expired before the per-blocker mechanism re-check at the reviewed commit
- [Critical] packages/cli/src/ui/hooks/useGeminiStream.ts — R23-1 flush commits against shared mutable ref (comment 3832640914) — cannot tell: this round's time budget expired before the per-blocker mechanism re-check at the reviewed commit
- [Critical] packages/cli/src/ui/hooks/useGeminiStream.ts — R28-3 flush decodes stricter than producer (comment 3840455264) — cannot tell: this round's time budget expired before the per-blocker mechanism re-check at the reviewed commit
- [Critical] packages/cli/src/ui/hooks/useGeminiStream.ts — R4-1 acceptance heuristic treats local-limit events as acceptance (comment 3736780220) — cannot tell: this round's time budget expired before the per-blocker mechanism re-check at the reviewed …
- [Critical] packages/cli/src/ui/hooks/useGeminiStream.ts — deferred flush commits dedup-dropped presentations (comment 3822016763) — cannot tell: this round's time budget expired before the per-blocker mechanism re-check at the reviewed commit
- [Critical] packages/cli/src/ui/hooks/useGeminiStream.ts:1024 — R24-1 replay-record release defeated by fingerprint pairing (comment 3835699734) — cannot tell: this round's time budget expired before the per-blocker mechanism re-check at the reviewed c…
- [Critical] packages/cli/src/ui/hooks/useGeminiStream.ts:3920 — ChatCompressed/Retry ordering heuristic misreports delivery (comment 3822016748) — cannot tell: this round's time budget expired before the per-blocker mechanism re-check at the reviewed c…
- [Critical] packages/cli/src/ui/hooks/useGeminiStream.ts:3927 — R17-2 fail-closed branch conflates pre-send compression (comment 3810967007) — cannot tell: this round's time budget expired before the per-blocker mechanism re-check at the reviewed commi…
- [Critical] packages/cli/src/ui/hooks/useGeminiStream.ts:3937 — reactive compression strips schema, gate still commits (comment 3750387881) — cannot tell: this round's time budget expired before the per-blocker mechanism re-check at the reviewed commit
- [Critical] packages/cli/src/ui/hooks/useGeminiStream.ts:4501 — R23-46 secondary-interaction drop keeps presentations (comment 3832640905) — cannot tell: this round's time budget expired before the per-blocker mechanism re-check at the reviewed commit
- [Critical] packages/cli/src/ui/hooks/useGeminiStream.ts:4988 — R20-4 goal-context fail-closed exits return undefined (comment 3832640901) — cannot tell: this round's time budget expired before the per-blocker mechanism re-check at the reviewed commit
- [Critical] packages/core/src/agents/runtime/agent-context.ts:124 — R25-1 declaredToolNames scoped to start() frame (comment 3837017536) — cannot tell: this round's time budget expired before the per-blocker mechanism re-check at the reviewed commit
- [Critical] packages/core/src/core/client.ts — R2-1 memory-pressure microcompaction bypasses presentation clear (comment 3699434328) — cannot tell: this round's time budget expired before the per-blocker mechanism re-check at the reviewed commit
- [Critical] packages/core/src/core/client.ts — R7-1 resume reminder merged with orphan-repair response (comment 3756164482) — cannot tell: this round's time budget expired before the per-blocker mechanism re-check at the reviewed commit
- [Critical] packages/core/src/core/client.ts — resume reminder masks dangling model turn (comment 3759608857) — cannot tell: this round's time budget expired before the per-blocker mechanism re-check at the reviewed commit
- [Critical] packages/core/src/core/client.ts — R13-2 resume reminder becomes final history entry (comment 3783625652) — cannot tell: this round's time budget expired before the per-blocker mechanism re-check at the reviewed commit
- [Critical] packages/core/src/core/coreToolScheduler.ts:2518 — wrapper deny rules never enforced per-call (comment 3813685411) — cannot tell: this round's time budget expired before the per-blocker mechanism re-check at the reviewed commit
- [Critical] packages/core/src/core/coreToolScheduler.ts:6365 — R23-33 headless commits before history, no rollback (comment 3832640930) — cannot tell: this round's time budget expired before the per-blocker mechanism re-check at the reviewed commit
- [Critical] packages/core/src/core/coreToolScheduler.ts:6370 — presentations commit at execution, not delivery (comment 3817109617) — cannot tell: this round's time budget expired before the per-blocker mechanism re-check at the reviewed commit
- [Critical] packages/core/src/core/coreToolScheduler.ts:6499 — delivered schema blocks truncated after delivery (comment 3817109648) — cannot tell: this round's time budget expired before the per-blocker mechanism re-check at the reviewed commit
- [Critical] packages/core/src/core/deferred-tool-call-normalization.ts:205 — R17-1 fingerprint gate fail-open (comment 3810967001) — cannot tell: this round's time budget expired before the per-blocker mechanism re-check at the reviewed commit
- [Critical] packages/core/src/core/deferred-tool-call-normalization.ts:205 — R17-1 fingerprint gate re-check (comment 3813685396) — cannot tell: this round's time budget expired before the per-blocker mechanism re-check at the reviewed commit
- [Critical] packages/core/src/core/geminiChat.ts:4742 — R26-1 gate-B keeps rejection id gate-A excludes (comment 3838352372) — cannot tell: this round's time budget expired before the per-blocker mechanism re-check at the reviewed commit
- [Critical] packages/core/src/core/geminiChat.ts:4742 — R28-1 replay guard defeated after error answer (comment 3840455253) — cannot tell: this round's time budget expired before the per-blocker mechanism re-check at the reviewed commit
- [Critical] packages/core/src/core/geminiChat.ts:5054 — stripOrphanedUserEntries evicts schema-carrying mixed entry (comment 3824717371) — cannot tell: this round's time budget expired before the per-blocker mechanism re-check at the reviewed commit
- [Critical] packages/core/src/tools/tool-registry.ts:954 — proxy eligibility survives every history mutation (comment 3817109630) — cannot tell: this round's time budget expired before the per-blocker mechanism re-check at the reviewed commit
- [Critical] packages/core/src/tools/tool-search.ts — R28-2 keyword/select gate equates not-hidden with declared (comment 3840455258) — cannot tell: this round's time budget expired before the per-blocker mechanism re-check at the reviewed commit
- [Critical] packages/core/src/tools/tool-search.ts — presentation marked before delivery decision (comment 3817109613) — cannot tell: this round's time budget expired before the per-blocker mechanism re-check at the reviewed commit
- [Critical] packages/core/src/tools/tool-search.ts:125 — R16-1 fork/subagent catalog advertises uncallable tools (comment 3808458113) — cannot tell: this round's time budget expired before the per-blocker mechanism re-check at the reviewed commit
- [Critical] packages/core/src/tools/tool-search.ts:125 — R16-1 re-check (comment 3810967000) — cannot tell: this round's time budget expired before the per-blocker mechanism re-check at the reviewed commit
- [Critical] packages/core/src/tools/tool-search.ts:125 — R16-1 re-check (comment 3813685407) — cannot tell: this round's time budget expired before the per-blocker mechanism re-check at the reviewed commit
- [Critical] packages/core/src/tools/tool-search.ts:403 — R24-3 subagent select: block keys on parent predicate (comment 3835699741) — cannot tell: this round's time budget expired before the per-blocker mechanism re-check at the reviewed commit
- [Critical] packages/core/src/tools/tool-search.ts:516 — R13-1 oversized fallback discards blocked diagnostics (comment 3783625647) — cannot tell: this round's time budget expired before the per-blocker mechanism re-check at the reviewed commit
- [Critical] packages/core/src/tools/tool-search.ts:574 — oversized guard early-returns in subagent contexts (comment 3747401879) — cannot tell: this round's time budget expired before the per-blocker mechanism re-check at the reviewed commit
- [Critical] packages/core/src/tools/tool-search.ts:655 — R7-2 oversized escape hatch triggers MCP-removed reminder (comment 3756164486) — cannot tell: this round's time budget expired before the per-blocker mechanism re-check at the reviewed commit
- [Critical] packages/core/src/tools/tool-search.ts:732 — Closes #4777 claim leaves cache-bust intact (comment 3822016723) — cannot tell: this round's time budget expired before the per-blocker mechanism re-check at the reviewed commit
- [Critical] 2 entries — cannot tell: this round's time budget expired before the per-blocker mechanism re-check at the reviewed commit:
- @wenshao issue-level review (comment 5154754478)
- @qwen-code-dev-bot issue-level round summary (comment 5162086617)
Not explored to full depth (tool budget reached): "agent invariant-b (packages/core/src/tools/tool-search.ts)": none — no check was cut short..
Not reviewed: reverse audit — stopped before round 3 by the review time budget.
Deferred under the convergence posture (round 30, not a blocker) — recorded, not requested in this round:
integration-tests/sdk-typescript/sdk-mcp-server.test.ts:86 — [test] The PR's rewrite of this integration test is unreachable…packages/cli/src/acp-integration/session/Session.test.ts:31595 — [review] None of the six new ledger-rollback tests can observe the…packages/cli/src/acp-integration/session/Session.ts:6528 — [review] In #runStopContinuation's catch, the pre-existing preserve…packages/cli/src/acp-integration/session/Session.ts:10086 — [review] releaseRejectedCallReplayRecord deletes from…packages/cli/src/nonInteractiveCli.test.ts:3263 — [review] All five new headless ledger-rollback tests mock…packages/cli/src/nonInteractiveCli.ts:2961 — [review] The drain-item loop's ledger arm/settle path added by this…packages/cli/src/ui/hooks/useGeminiStream.test.tsx:495 — [review] The new acceptance-proof event set (provesAcceptance) has…packages/cli/src/ui/hooks/useGeminiStream.test.tsx:583 — [review] Pre-send auto-compression permanently suppresses…packages/cli/src/ui/hooks/useGeminiStream.test.tsx:3423 — [review] The two deferred-flush commit tests pin only the…packages/cli/src/ui/hooks/useGeminiStream.ts:5365 — [review] The backgroundLaunchExhaustedCapacity exit plants the…packages/core/src/core/client.test.ts:1455 — [review] The PR changes startChat's deferral gate to 'both control…packages/core/src/core/client.test.ts:2331 — [review] The deleted tests were the only end-to-end tests of the…packages/core/src/core/client.test.ts:2678 — [review] The diff deletes the only test pinning the negative branch…packages/core/src/core/client.ts:436 — [review] The diff deletes rememberAnnouncedDeferredTools but the…packages/core/src/core/client.ts:1733 — [review] The diff deletes queueAddedMcpToolsReminder and its…packages/core/src/core/coreToolScheduler.test.ts:2587 — [review] The wrapper-deny gate is two-branched (permission-manager…packages/core/src/core/coreToolScheduler.test.ts:2770 — [review] The settlement describe never exercises a…packages/core/src/core/coreToolScheduler.test.ts:2775 — [review] Dead option in the new settlement-test helper:…packages/core/src/core/coreToolScheduler.test.ts:2840 — [review] The settlement describe pins three consumer outcomes…packages/core/src/core/coreToolScheduler.test.ts:4295 — [review] The 'atomic tool_search schema block' test never exercises…- …and 15 more (see the run report)
中文说明
仅完成部分审查,审查缺口已披露。
未决,请确认:共 52 条(原文未翻译,列表见上方英文部分)。
未探索到全部深度(达到工具调用预算):"agent invariant-b (packages/core/src/tools/tool-search.ts)":none — no check was cut short.。
未审查:反向审计——评审时间预算不足,未能开始第 3 轮。
收敛姿态下延后(第 30 轮,非阻断)——已记录,本轮不要求修改:共 35 条(原文未翻译,列表见上方英文部分)。
— qwen3.8-max via Qwen Code /review (v0.22.0)
What this PR does
This PR moves the deferred-tool catalog out of startup and lifecycle
<system-reminder>messages and into the livetool_searchfunction description. The description is rebuilt from the current registry when provider-facing declarations are read, with stable grouping and ordering for bundled and MCP tools.tool_searchstill returns the selected full schemas in its result. Main-session calls then use the stabletool_callbridge, which resolves the live target and enters the normal target execution pipeline. Validation, permissions, confirmation, hooks, cancellation, truncation, telemetry, and UI identity remain owned by the real target, while provider request and response pairing remains undertool_calland the original call ID.What this removes is the reminder re-injection machinery: the startup and lifecycle catalog reminders, plus the client-side bookkeeping that replayed them after resume, rewind, MCP change, and compression. Reaching the catalog therefore no longer depends on reminder text surviving in conversation history. Execution still requires that the target schema was genuinely presented to the model: the registry keeps a session-scoped presentation ledger, and
tool_callrejects a target whose mark is missing or whose schema fingerprint no longer matches the live schema — #6721's fail-closed gate against routing guessed or stale arguments. Subagents and teammates keep direct declarations, and sessions without the complete discovery bridge fall back to declaring deferred tools directly.Why it is needed
A catalog stored in conversation reminders competes for attention as the context grows and can disappear during compression. Restoring it requires more synthetic history and lifecycle bookkeeping. Keeping the catalog in
tool_search.descriptionmakes it part of the current tool surface on every request, so compression does not require a reminder replay and a long conversation does not separate discovery from the tool declaration.Reviewer Test Plan
How to verify
tool_search.descriptionand confirm the live catalog is grouped, sorted, quoted, and updated after registry changes.tool_call. Confirm the real target receives the nested arguments and runs its normal validation, permission, confirmation, hook, telemetry, and execution paths.tool_searchdisabled. Confirm both retain direct-declaration behavior and do not receive the main-session bridge.Evidence (Before & After)
Before: the catalog and restored schemas were carried in synthetic user-history reminders, so every lifecycle event that evicted them needed its own replay path.
After: the catalog lives in the current
tool_searchdescription and the reminder replay machinery is gone, whiletool_callroutes through the live target under the presentation ledger this PR introduces. There is no visual TUI change, so screenshots are N/A.Tested on
Environment (optional)
Validated locally on macOS with Node.js 24.14.0 and no sandbox. Focused Core, headless, TUI-hook, ACP, and SDK MCP tests passed. Core, CLI, and Web Shell typechecks passed; affected builds, the CLI bundle, changed-file lint, formatting, and diff checks also passed.
Risk & Scope
tool_searchdeclaration and changes when the live deferred registry changes. This is intentional capability metadata rather than conversation history. Concretely, a late MCP discovery mutates the provider-facing tools block (thetool_searchdescription) on the next request, which invalidates the prefix cache for the remainder of the session — the same harm Deferred-tools listing in the system prompt busts prompt cache on every MCP discovery / tool reveal #4777 reports, relocated from the system instruction to the tools prefix. Making the tools prefix byte-stable across registry changes (e.g. a deterministic tail-of-conversation snapshot as discussed in the issue thread) is a follow-up pending maintainer ruling; see the open review thread.tool_searchresult (compression,/clear, resume reload, rewind) deterministically costs one extra search before the affected tool can be routed again. This is the fail-closed direction by design.tool_call; subagents, teammates, disabled discovery sessions, and resumed direct-call compatibility paths retain direct declarations where required.Linked Issues
Closes #6721
Partially addresses #4777 — removes the catalog from system reminders and lifecycle bookkeeping, but late MCP discovery still mutates the tools prefix (cache-bust tradeoff documented above, pending maintainer ruling on the stable-prefix follow-up).
中文说明
这个 PR 把 deferred tool catalog 从启动、MCP 变化、恢复和压缩后的 system reminder 中移到动态的
tool_search.description。每次读取 provider tool declaration 时,catalog 都从当前 registry 构建,因此上下文压缩后不需要重新注入 reminder,长上下文也不会把 catalog 和工具声明分开。tool_search继续返回选中工具的完整 schema;执行统一通过稳定的tool_callbridge。bridge 只负责解析并保留当前真实目标,后续参数校验、权限、确认、hooks、取消、截断、telemetry 和执行仍由真实工具负责。被删掉的是 reminder 重新注入机制:启动与生命周期的 catalog reminder,以及在 resume、rewind、MCP 重连和 compression 之后把它们重放回历史的 client 侧记账逻辑。因此“能不能看到 catalog”不再取决于 reminder 文本是否还留在历史里。但执行仍然要求目标 schema 确实被展示过:registry 维护一份会话级 presentation ledger,
tool_call会拒绝没有标记、或 schema fingerprint 与当前实际 schema 不一致的目标——这正是 #6721 要求的 fail-closed 闸门,避免用猜测或过期的参数去调用工具。测试覆盖动态 catalog、同批 search/call、失败 search 后调用、非法与过期目标、真实工具权限和响应配对、压缩与恢复、subagent 直连、超大 schema fallback,以及 SDK MCP 端到端路径。