feat(core): preserve prompt cache for deferred tools - #10410
feat(core): preserve prompt cache for deferred tools#10410DragonnZhang wants to merge 20 commits into
Conversation
E2E Test ReportStatus: passed locally with a live stdio MCP server and the development CLI. Environment:
Observed live flow:
Additional verification:
The raw provider request body was not captured in this smoke run, so byte-for-byte declaration equality remains covered by automated tests rather than a live payload trace. |
…call-bridge # Conflicts: # packages/core/src/skills/bundled/review/SKILL.md
|
Thanks for the PR — gate review below.
Also noting: the merge conflict with Moving on to code review. 🔍 @chiga0 flagging for awareness: core change at ~940 production lines — final call will be deferred to a maintainer. 中文说明门控审查结果如下。
另注：与 进入代码审查 🔍 @chiga0 提请关注：约 940 行生产代码的核心改动，最终结论将交由维护者定夺。 — Qwen Code · qwen3.8-max Reviewed at |
Code reviewMy independent take before reading the diff: the only design that keeps the prefix stable while preserving deferral's token savings is a frozen declaration list plus two fixed bridge tools — reveal schemas as tool output, wrap invocation in an executor tool, unwrap at the scheduler under the target identity. That is exactly what this PR does, and I did not find a simpler path it missed. No critical blockers in the ~940 production lines. The fail-closed rails check out:
Non-blocking notes:
sequenceDiagram
participant P1 as Model
participant P2 as tool_search
participant P3 as CoreToolScheduler
participant P4 as ToolRegistry
participant P5 as PermissionManager
participant P6 as target deferred tool
P1->>P2: query or select name (review schema)
P2->>P4: ensureTool (load only, no reveal)
P2-->>P1: schema in functions block, declaration list unchanged
P1->>P3: tool_call envelope with name and arguments
P3->>P5: is tool_call enabled
P3->>P4: resolve target, reject recursion visible or unknown
P3->>P5: permissions, hooks, approvals under target identity
P3->>P6: execute
P6-->>P3: result
P3-->>P1: function response keeps tool_call name and original call id
Files changed (30 of 58 shown)
Testing — the PR's own CI, read via the APIUnattended run — PR code is never built or executed here. Evidence below is the PR's own CI on the reviewed commit, fetched once via the API; pending checks are reported as pending, not polled. The author's self-reported local results (dev-CLI smoke test with a stdio MCP server, 2,281 + 1,229 unit tests on macOS) are their claim, not independently re-run. The unit suite Final CI results for
One row per check name (latest run); skipped checks omitted; failures sort first. / 每个检查名一行（取最新一次运行），省略 skipped，失败项排在最前。 Sandboxed verification would settle the central claim: Real-scenario (tmux) testing: not attempted — unattended CI run; the live-behavior signal comes from the sandboxed lane above. 中文说明代码审查：我在读 diff 前的独立方案就是"冻结声明列表 + 两个固定桥工具"，与本 PR 一致，且没找到被它遗漏的更简路径。约 940 行生产代码中未发现阻塞性问题：解析器对各种非法目标（递归、可见工具、未知工具、工厂失败）均失败关闭；权限、hooks、审批、并发、遥测在解包后全部使用目标工具身份（测试钉住了 PreToolUse 收到目标名与目标参数）；模型侧响应（成功/错误/取消/超时）在调度器、ACP、headless 三条路径上都保留 非阻塞提醒：DeepSeek 模型不再自动禁用 测试：无人值守运行，不构建/执行 PR 代码。以上证据来自所审提交的 PR 自身 CI（API 一次性读取，不轮询）。单测套件仍在运行中；macOS/Windows 矩阵腿 skipped 为仓库常态（对照已合并的 #10402 相同）。作者自报的本地验证（macOS 冒烟测试与单测数字）是其声明，未独立复现。 沙箱验证可以落定核心主张： — Qwen Code · qwen3.8-max Reviewed at |
|
Confidence: 3/5 — clean review on substance, but this is a ~940-production-line core change from a fork with a model-facing contract shift, and policy puts that final call in a maintainer's hands rather than mine. Stepping back: this is the design I would have written, and the execution matches it. The frozen-declaration-list approach is the only one that keeps both the prompt-cache prefix and deferral's token savings; the unwrap-at-the-scheduler split (target identity for policy and execution, bridge identity for the model) is carried consistently through all three frontends; the failure direction is right everywhere I probed (recursion, visible targets, deny rules, missing bridge halves, aborts mid-resolution). Test coverage is genuine — it pins the semantics (PreToolUse sees the target name, responses keep the bridge name, deny wins over the exemption) rather than the implementation. The author also handled the mid-review merge with What keeps this at 3/5 is not any defect I found — it's what triage cannot settle:
⏸️ Deferring to @chiga0 — core infrastructure change at this scale needs a human decision on direction and maintainability before merge. Nothing here is a request for changes; if the two questions above are answered, this looks ready. CI on the reviewed commit is still running (unit suite pending); the finalize job will update the test table in the review comment when it lands. 中文说明总体判断：这是一次实质干净的评审——方案正是我会选的设计（冻结声明列表、调度层按目标身份解包），且在三个执行前端贯彻一致；失败方向处处正确（递归、可见目标、deny 规则、桥缺失、解包中被中止）；测试钉住的是语义而非实现。作者还在评审过程中干净地处理了与 main 的合并。 保持 3/5 的不是缺陷，而是三件事：（1）核心主张（发现过程中声明列表字节级稳定、缓存持续命中）是请求体层面的性质，目前唯一的活体证据是作者自测， ⏸️ 转交 @chiga0 ——此规模的核心基础设施改动需要人类在方向与可维护性上做出决定后再合并。本评论不是要求修改；上述两点有答案后，此 PR 看起来即可就绪。所审提交的 CI 仍在运行（单测待定），finalize 任务会在结束后原地更新评审评论中的测试表格。 — Qwen Code · qwen3.8-max Reviewed at |
🖼️ web-shell visual previewRendered against a mock daemon (no real backend): the PR base vs this PR head Screenshots · before / afterℹ️ No screenshot changed against the PR base — but this PR edits 1 render-shaping file:
Either the change has no visual effect (logic, plumbing, a state the scenarios never reach), or no scenario renders this UI — in which case the preview cannot see it, and an empty result is a coverage gap rather than a clean bill of health. To make it visible, add a scenario to Full-resolution recordings (.webm) are attached to the workflow run. — Qwen Code · web-shell visuals |
|
@qwen-code /verify |
|
Sandboxed verification: ✅ passed — merge-ready (agent verdict) - workflow run Ran the PR in an isolated, token-free container: A/B against the base build, mock-free harness assertions, targeted gates. Advisory evidence for human reviewers — not a review, an approval, or a CI check. Scripted assertions: 42 passed · 0 failed · 42 total Flakiness gate: 中文 — 判定：✅ 通过 · 可合入（agent 判定）沙箱验证在隔离、无凭证的容器中执行了该 PR 的代码（与 base 构建 A/B 对照、无 mock harness 断言、定向门禁）。仅作为评审证据，不构成评审、批准或 CI 检查。 脚本断言：42 通过 · 0 失败 · 42 总计 抖动门： Verification reportPR #10410 Deep Verification —
|
| cell | build | scripted scenario | observable oracle | result |
|---|---|---|---|---|
head-bridge |
head | tool_search → tool_call → final |
tools block byte-identical across requests 1/2/3; history append-only; result tied to the tool_call id; exactly 1 MCP invocation |
9/9 |
base-reveal |
base | tool_search → direct call → final |
CONTROL (expected mutation): request-2 tools block differs; revealed schema present; direct call used |
7/7 |
head-fallback |
head | direct call, toolSearch.enabled: false |
echo declared eagerly in request 1; no bridge tools declared | 5/5 |
head-routing-recursive |
head | tool_call targeting tool_call |
rejected: tool_call cannot invoke bridge tool "tool_call"; no MCP call |
4/4 |
head-routing-visible |
head | tool_call targeting eager read_file |
rejected: already visible to the model or is not deferred |
4/4 |
head-routing-deny |
head | tool_search + tool_call on a denied target |
denied at permission time citing Matching deny rule: "mcp__echo-server__echo" — i.e. evaluated against the underlying target identity; never invoked |
5/5 |
Witness: 01-ab-declaration-stability-head-vs-base.png (cell table) and 02-wire-assertions-six-cells.png (all 34 assertions as printed). The flip is exactly the load-bearing shape: head 3/3 requests byte-stable, base mutates at discovery.
Reviewer Test Plan, per step: (1) deferred flow with no schema added — proven by head-bridge (H1–H3, H5–H7). (2) byte-stable declarations — H2/H3 exact JSON equality of the tools array across all three requests. (3) permissions — deny wins (D1–D3, at the underlying identity), allowed target executes (H7), recursive/visible targets rejected (R1–R3, V1–V3); lazy-load-failure path covered by unit tests only (see Not covered). (4) headless + ACP identity split — the E2E harness itself is the headless path (runNonInteractive), model-facing identity proven on the wire (H5/H6); the Gemini-side functionResponse.name = tool_call and ACP/concurrency/telemetry target-identity are pinned by the PR's unit tests, which this round mutation-proved non-vacuous (M2/M4/M5 below). (5) fallback eager declaration — F1–F4. No step was unexecutable.
Mutation matrix (vacuity of the new tests)
All runs in a scratch worktree at the merge commit; unmutated control green first (447/447 across the three core suites); each mutation restored afterward (final git status clean). Witness: 03-mutation-matrix-five-mutants.png; full log at logs/mutation-matrix.txt.
| # | mutation (file) | suite | outcome | failure mode observed |
|---|---|---|---|---|
| — | none (control) | tool-call + tool-search + coreToolScheduler | 447/447 green | — |
| M1 | re-add revealDeferredTool() in returnSchemas (tool-search.ts) — resurrects base behavior |
tool-search.test.ts | KILLED 11/50 | select: mode reviews a named tool without revealing it → expected true to be false; keeps function declarations stable… → declaration-list deep-equal mismatch |
| M2 | resolveToolCallBridgeRequest returns request unchanged (coreToolScheduler.ts) |
coreToolScheduler.test.ts | KILLED 3 red | routing-identity spy never called; visible-target reject expected 'success' to be 'error'; timeout identity likewise |
| M3 | recursion guard disabled (tool-call.ts) — positive control | tool-call.test.ts | KILLED 1/2 | tool_search variant red; the tool_call variant survived via the sibling visibility check with the same errorType — see Findings |
| M4 | getHeadlessExecutionRequest disabled (nonInteractiveCli.ts) |
nonInteractiveCli.test.ts | KILLED | uses deferred target identity for headless bridge concurrency… times out (batch deadlocks when classified under the bridge name) |
| M5 | ACP bridge branch disabled (Session.ts) | Session.test.ts (-t "routes tool_call…") |
KILLED | expected "spy" to be called once, but got 0 times — underlying tool never invoked |
No mutant regressed a pinned behavior; every load-bearing guard the PR introduces is pinned by a test that fails with the behavioral mismatch it exists to catch. M4's timeout is the behavioral mismatch itself: the test's gate only opens when both bridge calls run concurrently under the target's Read-kind identity.
Targeted gates
- core (15 changed test files, incl. tool-call/tool-search/tool-registry/coreToolScheduler/client/environmentContext/agent-core/config/permission-manager/autoMode/builtin-agents/askUserQuestion/read-mcp-resource/syntheticOutput/fileUtils): 2318/2318 pass. Run twice; identical. Witness:
04-core-gate-15-changed-files.png. Gate liveness is established by the mutation matrix — the same vitest invocations went red under M1/M2/M3. - cli (nonInteractiveCli, Session, config, contextCommand): 1229 pass + 1 skipped — byte-identical to the PR's own reported numbers. junit artifacts preserved under
logs/.
Findings
S1 — Suggestion (test precision): the recursion test's tool_call variant is pinned by a sibling guard, not the one it names. Deleting the recursion check in resolveDeferredToolCall (M3) turned only the tool_search variant of rejects recursive bridge target red; the tool_call variant stayed green because the envelope then falls through to isDeferredAndHidden('tool_call') === false, which returns the same INVALID_TOOL_PARAMS errorType the test asserts. Production is still doubly protected (recursion check, then visibility check — both reject), so this is a completeness item, not a defect: assert on the error message (cannot invoke bridge tool) so the named guard is actually pinned. Classification per the matrix taxonomy: redundant defence — the sibling hunk closes the same hazard, so nothing can observe this guard alone through the current assertion.
S2 — Informational: removal of the DeepSeek auto-disable is a user-visible default change the description undersells. Base auto-added tool_search to the deny list for deepseek-(v3|v4|chat) models; head keeps the bridge enabled for them and only an explicit tools.toolSearch.enabled: false disables both halves. The change is deliberate and test-pinned (should keep the stable bridge enabled for deepseek-v4 models, etc.) and is justified by the PR's own mechanism — the bridge keeps the prefix stable, which is what the auto-disable was compensating for. But "Breaking changes / migration notes: No public breaking change" should mention that DeepSeek users' default flips from eager declarations to the bridge.
S3 — Observation (pre-existing, not introduced by this PR): a denied MCP tool's schema is still handed out by tool_search; deny wins at call time. In the deny cell the tool remained registered and reviewable, and the bridge invocation was rejected with Qwen Code requires permission to use "mcp__echo-server__echo" … Matching deny rule — the correct claimed behavior ("explicit deny rules must win"), evaluated at the underlying identity. Registration gating for MCP deny rules is unchanged here (the tool-registry.ts diff is comments plus one debug-log string's wording), so the one wasted round-trip when a model tries a denied tool predates this PR. Reported for awareness only.
Not covered
- Per-commit attribution: depth-2 checkout —
git rev-list HEAD^1..HEAD^2yields only the merge commit; the feat commit90a2e34ais not locally reachable (the snapshot lists 2 commits). The aggregateHEAD^1..HEADdiff is what was verified. - Real prompt-cache hits: the wire oracle proves the necessary condition (byte-stable
toolsblock + append-only history = stable serialized prefix). Actual cache-hit accounting lives provider-side and was not measured; the PR's "prompt-cache reads on continuation turns" smoke claim could not be replayed without a caching provider. This reproduces the wire shape that preserves caching, not a provider's cache behavior. - Live ACP / hooks / approvals / telemetry E2E: covered by the PR's unit tests, which this round proved non-vacuous (M2/M4/M5), but no live ACP session was driven.
- Lazy-load failure path (
ensureToolthrows → tool error): unit tests only (tool-call.test.ts), no E2E cell. - Repo-wide test/lint/typecheck gates (only changed files were run); Windows behavior; docs accuracy, i18n entries, and web-shell display-name additions were diff-reviewed only.
- The PR's "12 core files, 2,281 tests" number was not reproduced verbatim — this round ran a slightly different (superset) selection of 15 files → 2,318; the CLI figures matched exactly.
Methodology
Environment: the CI verify container (node:22-bookworm, Node v22.23.2), repo at merge commit ee904f951b with npm ci/npm run build pre-run. The wire harness (harness/) drives the real compiled bundles: a zero-dependency scripted mock OpenAI server (specialized from the e2e-testing skill template) records every request body to JSONL, a zero-dependency stdio MCP server logs each tools/call at the destination, and harness/assert.mjs turns both into 34 scripted pass/fail checks per run (scratch git project per cell, isolated QWEN_RUNTIME_DIR). The base arm was rebuilt from a HEAD^1 worktree (npm ci + npm run build + npm run bundle, ~6 min, logs in logs/base-*.log); the lockfile is untouched by the PR, and the base control was validated by asserting the workspace-link realpath and the absence of tool_call from the base build. Mutation runs used a second scratch worktree with QWEN_VITEST_GUARD_ROOT pointed at the main tree and package-local node_modules symlinked (identical lockfile). Raw per-cell artifacts (captured request bodies, CLI stdout/stderr, MCP call logs, meta) live under run/<cell>/; mutation log at logs/mutation-matrix.txt; junit from both gates under logs/. Evidence images were produced with scripts/verify-capture.mjs.
Flakiness gate log
rounds=5 files=19 skipped=0
file packages/cli/src/acp-integration/session/Session.test.ts: (cd packages/cli) npx --no-install vitest run ./src/acp-integration/session/Session.test.ts
file packages/cli/src/config/config.test.ts: (cd packages/cli) npx --no-install vitest run ./src/config/config.test.ts
file packages/cli/src/nonInteractiveCli.test.ts: (cd packages/cli) npx --no-install vitest run ./src/nonInteractiveCli.test.ts
file packages/cli/src/ui/commands/contextCommand.test.ts: (cd packages/cli) npx --no-install vitest run ./src/ui/commands/contextCommand.test.ts
file packages/core/src/agents/runtime/agent-core.test.ts: (cd packages/core) npx --no-install vitest run ./src/agents/runtime/agent-core.test.ts
file packages/core/src/config/config.test.ts: (cd packages/core) npx --no-install vitest run ./src/config/config.test.ts
file packages/core/src/core/client.test.ts: (cd packages/core) npx --no-install vitest run ./src/core/client.test.ts
file packages/core/src/core/coreToolScheduler.test.ts: (cd packages/core) npx --no-install vitest run ./src/core/coreToolScheduler.test.ts
file packages/core/src/core/environmentContext.test.ts: (cd packages/core) npx --no-install vitest run ./src/core/environmentContext.test.ts
file packages/core/src/permissions/autoMode.test.ts: (cd packages/core) npx --no-install vitest run ./src/permissions/autoMode.test.ts
file packages/core/src/permissions/permission-manager.test.ts: (cd packages/core) npx --no-install vitest run ./src/permissions/permission-manager.test.ts
file packages/core/src/subagents/builtin-agents.test.ts: (cd packages/core) npx --no-install vitest run ./src/subagents/builtin-agents.test.ts
file packages/core/src/tools/askUserQuestion.test.ts: (cd packages/core) npx --no-install vitest run ./src/tools/askUserQuestion.test.ts
file packages/core/src/tools/read-mcp-resource.test.ts: (cd packages/core) npx --no-install vitest run ./src/tools/read-mcp-resource.test.ts
file packages/core/src/tools/syntheticOutput.test.ts: (cd packages/core) npx --no-install vitest run ./src/tools/syntheticOutput.test.ts
file packages/core/src/tools/tool-call.test.ts: (cd packages/core) npx --no-install vitest run ./src/tools/tool-call.test.ts
file packages/core/src/tools/tool-registry.test.ts: (cd packages/core) npx --no-install vitest run ./src/tools/tool-registry.test.ts
file packages/core/src/tools/tool-search.test.ts: (cd packages/core) npx --no-install vitest run ./src/tools/tool-search.test.ts
file packages/core/src/utils/fileUtils.test.ts: (cd packages/core) npx --no-install vitest run ./src/utils/fileUtils.test.ts
per-file results (P=pass F=fail I=infra-exit, one letter per run):
packages/cli/src/acp-integration/session/Session.test.ts: PPP
packages/cli/src/config/config.test.ts: PPP
packages/cli/src/nonInteractiveCli.test.ts: PPP
packages/cli/src/ui/commands/contextCommand.test.ts: PP
packages/core/src/agents/runtime/agent-core.test.ts: PP
packages/core/src/config/config.test.ts: PP
packages/core/src/core/client.test.ts: PP
packages/core/src/core/coreToolScheduler.test.ts: PP
packages/core/src/core/environmentContext.test.ts: PP
packages/core/src/permissions/autoMode.test.ts: PP
packages/core/src/permissions/permission-manager.test.ts: PP
packages/core/src/subagents/builtin-agents.test.ts: PP
packages/core/src/tools/askUserQuestion.test.ts: PP
packages/core/src/tools/read-mcp-resource.test.ts: PP
packages/core/src/tools/syntheticOutput.test.ts: PP
packages/core/src/tools/tool-call.test.ts: PP
packages/core/src/tools/tool-registry.test.ts: PP
packages/core/src/tools/tool-search.test.ts: PP
packages/core/src/utils/fileUtils.test.ts: PP
verdict: timeout
summary: only 2 of 5 rounds fit the 15-minute budget; the completed rounds agreed
--- per-invocation detail (full copy in the artifact) ---
round 1 · packages/cli/src/acp-integration/session/Session.test.ts: P (exit 0)
round 1 · packages/cli/src/config/config.test.ts: P (exit 0)
round 1 · packages/cli/src/nonInteractiveCli.test.ts: P (exit 0)
round 1 · packages/cli/src/ui/commands/contextCommand.test.ts: P (exit 0)
round 1 · packages/core/src/agents/runtime/agent-core.test.ts: P (exit 0)
round 1 · packages/core/src/config/config.test.ts: P (exit 0)
round 1 · packages/core/src/core/client.test.ts: P (exit 0)
round 1 · packages/core/src/core/coreToolScheduler.test.ts: P (exit 0)
round 1 · packages/core/src/core/environmentContext.test.ts: P (exit 0)
round 1 · packages/core/src/permissions/autoMode.test.ts: P (exit 0)
round 1 · packages/core/src/permissions/permission-manager.test.ts: P (exit 0)
round 1 · packages/core/src/subagents/builtin-agents.test.ts: P (exit 0)
round 1 · packages/core/src/tools/askUserQuestion.test.ts: P (exit 0)
round 1 · packages/core/src/tools/read-mcp-resource.test.ts: P (exit 0)
round 1 · packages/core/src/tools/syntheticOutput.test.ts: P (exit 0)
round 1 · packages/core/src/tools/tool-call.test.ts: P (exit 0)
round 1 · packages/core/src/tools/tool-registry.test.ts: P (exit 0)
round 1 · packages/core/src/tools/tool-search.test.ts: P (exit 0)
round 1 · packages/core/src/utils/fileUtils.test.ts: P (exit 0)
round 2 · packages/cli/src/acp-integration/session/Session.test.ts: P (exit 0)
round 2 · packages/cli/src/config/config.test.ts: P (exit 0)
round 2 · packages/cli/src/nonInteractiveCli.test.ts: P (exit 0)
round 2 · packages/cli/src/ui/commands/contextCommand.test.ts: P (exit 0)
round 2 · packages/core/src/agents/runtime/agent-core.test.ts: P (exit 0)
round 2 · packages/core/src/config/config.test.ts: P (exit 0)
round 2 · packages/core/src/core/client.test.ts: P (exit 0)
round 2 · packages/core/src/core/coreToolScheduler.test.ts: P (exit 0)
round 2 · packages/core/src/core/environmentContext.test.ts: P (exit 0)
round 2 · packages/core/src/permissions/autoMode.test.ts: P (exit 0)
round 2 · packages/core/src/permissions/permission-manager.test.ts: P (exit 0)
round 2 · packages/core/src/subagents/builtin-agents.test.ts: P (exit 0)
round 2 · packages/core/src/tools/askUserQuestion.test.ts: P (exit 0)
round 2 · packages/core/src/tools/read-mcp-resource.test.ts: P (exit 0)
round 2 · packages/core/src/tools/syntheticOutput.test.ts: P (exit 0)
round 2 · packages/core/src/tools/tool-call.test.ts: P (exit 0)
round 2 · packages/core/src/tools/tool-registry.test.ts: P (exit 0)
round 2 · packages/core/src/tools/tool-search.test.ts: P (exit 0)
round 2 · packages/core/src/utils/fileUtils.test.ts: P (exit 0)
round 3 · packages/cli/src/acp-integration/session/Session.test.ts: P (exit 0)
round 3 · packages/cli/src/config/config.test.ts: P (exit 0)
round 3 · packages/cli/src/nonInteractiveCli.test.ts: P (exit 0)
Evidence images
Harness scripts and raw logs are in the workflow run artifacts (7-day retention).
— Qwen Code · sandboxed verification
…call-bridge # Conflicts: # docs/developers/sdk-typescript.md # docs/users/configuration/settings.md # packages/cli/src/config/settingsSchema.ts # packages/core/src/agents/runtime/agent-core.ts # packages/core/src/config/config.ts # packages/core/src/core/client.test.ts # packages/core/src/core/client.ts # packages/core/src/core/coreToolScheduler.test.ts # packages/core/src/permissions/permission-manager.test.ts # packages/core/src/permissions/permission-manager.ts # packages/core/src/tools/tool-registry.test.ts # packages/core/src/tools/tool-registry.ts # packages/sdk-typescript/README.md # packages/sdk-typescript/src/types/types.ts # packages/vscode-ide-companion/schemas/settings.schema.json
|
CI note: Investigation so far (this is NOT a merge-conflict regression):
Working hypothesis: the SDK E2E asserts eager MCP declarations, while under the bridge redesign those tools are (or should be) reached through the deferred bridge — so either the E2E needs adapting to the bridge flow, or there is a declaration/preload gap for MCP tools in the bridge path. Left to the review round / author judgment; no automatic code change applied. 中文说明CI 说明： 已排查（这不是合并冲突回归）：本分支 工作假设：SDK E2E 断言 MCP 工具即时声明，而桥接重设计下这些工具经由延迟桥接触达——因此要么 E2E 需要适配桥接流程，要么桥接路径上存在 MCP 工具声明/预加载缺口。交由评审轮次/作者判断，未自动改码。 |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Not explored to full depth (tool budget reached): chunk 11: executing tool-call.test.ts to confirm green — the review worktree has no node_modules , and a full monorepo install + workspace build to satisfy vitest's gl…; chunk 8: execute coreToolScheduler.test.ts bridge tests (no node_modules; install+build exceeds budget).
中文说明
未探索到全部深度（达到工具调用预算）：chunk 11：executing tool-call.test.ts to confirm green — the review worktree has no node_modules , and a full monorepo install + workspace build to satisfy vitest's gl…；chunk 8：execute coreToolScheduler.test.ts bridge tests (no node_modules; install+build exceeds budget)。
— qwen3.8-max via Qwen Code /review (v0.22.2)
…wenLM#10410) Security/correctness: - Re-check the execution allowlist against the resolved tool_call target in the scheduler (fork allowlist previously only saw the wrapper name). - Skip the pending bridge TOOL_CALL start on abort so a cancelled call never emits a start event after its synthetic TOOL_RESULT. - Route INVALID_TOOL_PARAMS bridge resolution errors through the validation-retry loop so repeated bad envelopes get the stop directive. - Reject a hidden deferred target when tool_search is unregistered, and make the TOOL_NOT_REGISTERED remedy conditional on tool_search existing. - Seed announcedMcpToolNames for eagerly revealed MCP tools so a later disconnect is still announced when the bridge is incomplete. - buildDeferredToolsReminder returns null when the bridge is incomplete. Docs (bridge-registered caveats, two-half reachability, DeepSeek removal, schema regeneration, table cells, wording): settings.md, settingsSchema.ts + regenerated settings.schema.json, sdk types.ts/README/sdk-typescript.md, design doc, skills/types.ts, web-shell i18n. Tests: allowlist re-check, abort ordering, retry directive, leader-only and tool_search-absent rejections, revealed-tool exclusion, bridge availability both directions, PM bridge gate, cancel-after-resolution rename, incomplete- bridge reminder, MCP disconnect announcement, zoom hint wording.
…call-bridge # Conflicts: # packages/core/src/tools/tool-search.ts
|
Qwen Code review timed out. Qwen review timed out after 21600 seconds (of the 360-minute budget). This run already used the maximum 360 minute timeout. See workflow logs. |
…ge contract (QwenLM#10410) Under the ToolSearch + ToolCall bridge, tools fetched via tool_search select: stay hidden from the model-facing declaration list so the prompt-cache prefix remains stable (they are invoked through tool_call; direct invocation by name still executes, as the resume test already exercises). Flip the three fresh-session advertisement assertions from 'tool is advertised after select:' to 'tool remains hidden', matching the contract documented in tool-search.ts. The resume-restoration test is unchanged and already passes at the merge commit.
|
CI note update (follow-up to the 2026-08-28 note above): the Evidence that it is not environmental: in the same CI window where this PR failed the suite, other branches' runs passed it — Root cause. The E2E asserted the pre-bridge contract: after Fix. Test-only change (no production code): flip the three fresh-session assertions from "tool is advertised after 中文说明CI 说明更新（承接上方 2026-08-28 的说明）： 非环境问题的证据： 在本 PR 该套件失败的同一 CI 时间窗内，其他分支的运行通过了它—— 根因。 该 E2E 断言的是桥接之前的契约： 修复。 仅测试变更（不动产品代码）：把三处新会话断言从"select: 之后工具被声明"翻转为"工具保持隐藏"，并加注释说明桥接契约。第四个测试（ |
|
CI note: the red checks on this PR in the last ~6h are runner-fleet degradation, not PR content. Evidence:
Response is mechanical rerun until the fleet recovers; no code change is indicated. If a maintainer sees this note going stale (fleet recovered), disregard. 中文说明CI 说明：本 PR 最近约 6 小时的红灯是 runner 集群降级所致，不是 PR 内容问题。 证据：
对策是机械性重跑直至集群恢复，无需任何代码改动。若维护者看到本帖时集群已恢复，请忽略。 |
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.
Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.
Deferred under the convergence posture (round 2, not a blocker) — recorded, not requested in this round:
packages/cli/src/acp-integration/session/Session.ts:10607 — [review] ACP errorResponse model-facing rename unpinned (success-path-only test)packages/core/src/permissions/permission-manager.ts:774 — [probe] tool_call eager-allowlist exemption has no test (mutant flips probe registered->deferred)packages/cli/src/acp-integration/session/Session.ts:10784 (+2 locations) — [review] bridge gates' isToolEnabled failure paths untested (ACP + scheduler)packages/core/src/agents/runtime/agent-core.ts:1838 — [probe] preToolUse identity for bridged calls unobserved by tests (mutant: 49 passed with hook deleted)packages/core/src/agents/runtime/agent-core.ts:1988 — [probe] one-shot pendingToolCallStarts.delete semantics unpinned (mutant delete->has: 49 passed)packages/core/src/core/coreToolScheduler.test.ts:1034 — [probe] PM denial of resolved bridge target unpinned (asked-but-not-enforced; probe mutant executes denied target, suite green)packages/cli/src/acp-integration/session/Session.ts:10804 — [probe] bridged TOOL_NOT_REGISTERED escapes the ACP invalid-params stagnation detector (probe flip)packages/core/src/agents/runtime/agent-core.ts:2188 — [probe] abort fallback naming arm (?? req.name) unpinned (mutant matrix)packages/core/src/core/coreToolScheduler.test.ts:1035 — [probe] bridge approval flow untested (all ten tests under YOLO; probe: awaiting_approval reachable)
中文说明
仅完成部分审查，审查缺口已披露。
未审查：build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。
收敛姿态下延后（第 2 轮，非阻断）——已记录，本轮不要求修改：共 9 条（原文未翻译，列表见上方英文部分）。
— qwen3.8-max via Qwen Code /review (v0.22.3)
… fallback (QwenLM#10410) The no-PermissionManager bridge deny gate matched deny entries exactly (canonicalToolName is alias-resolution only — no lowercase/trim), but the _schedule legacy-deny fallback it delegates to compares excludedTool.toLowerCase().trim() === normalizedToolName. Config stores permissions.deny entries verbatim, so an entry like 'Tool_Call' or ' tool_call ' slipped past the gate's exact compare: the envelope was unwrapped and the fallback then checked the deny list only against the resolved TARGET name — executing a call the same config denies when it arrives as a direct tool_call (pre-diff behavior). Mirror the loop's normalization on the gate so both paths deny identically (R2-1). Adds three parameterized cases ('Tool_Call', ' tool_call ', 'TOOL_CALL') extending the legacy-deny pin; removing the normalization turns all three red while the exact-entry case stays green (verified). Full coreToolScheduler.test.ts 395/395.
…racking, and prune pinning (QwenLM#10410) Three fix-induced findings from the bridge rounds: R1-15: the unreachable-tools warning's remedy clause now names tools.disabled alongside deny rules and --exclude-tools, matching the cause enumeration the PR documents in settingsSchema.ts; the warning test pins the mention. R1-28: an eager-reveal of an MCP tool (incomplete bridge) now also records the name in announcedMcpToolNames. The seed alone only reaches that set via rememberAnnouncedDeferredTools, which runs exclusively in startChat — so a server registering after the initial startChat used to disconnect silently and the model kept calling a dead server's tools. New test covers the mid-session reveal, the disconnect announcement, and a reconnect/disconnect flap announcing twice; removing the add turns it red (verified). R1-18: new interleaved test pinning the batch-start prune as the only mechanism that clears a tool_call-keyed retry counter across a successful bridged execution (recording keys on the model-facing name, prune keys on post-resolution names); disabling the prune fires RETRY LOOP DETECTED prematurely and turns the test red (verified). client.test.ts + coreToolScheduler.test.ts: 770/770 green.
…M#10410) Four fix-induced doc findings, applied together across all copies: R1-12 (9 locations): 'the demoted tools are out of reach for that session' is now qualified with the two escape hatches the code has — tools also listed in tools.visible are declared upfront, and resumed sessions re-declare demoted tools referenced by direct calls in the transcript (consistent with client.ts's own 'would be false for this session' comment). R1-25 (settings.md tools.core/discoveryCommand/migration rows): 'stay reachable through tool_search + tool_call' now carries the 'while both bridge tools are registered' condition. R1-9 (threshold, 3 copies): 'Set 0 to always keep deferred tools behind the bridge' overstated — two reveal paths ignore the threshold; replaced with the qualified wording naming resumed-history replay, tools.visible, and the incomplete-bridge fallback. R2-2 (4 locations): the unaffected/bypass lists now name tool_call next to tool_search, matching isExemptFromEagerAllowList which exempts both identically.
…10410) R2-9: the flipped not.toContain declaration assertions now carry positive controls on the same snapshot — the post-select list must contain both alwaysLoad bridge tools (tool_search, tool_call) — so an emptied or index-shifted declaration list can no longer pass them vacuously. R2-15: the first test now scripts the fake model to invoke the selected tools through tool_call envelopes (the documented post-select path), exercising resolveDeferredToolCall end-to-end through query(); results are paired by the model-facing name ('tool_call') with content checks, since the scheduler keeps modelFacingName on the wire. Reverting the script to direct-by-name calls would remove the suite's only end-to-end signal for a broken invocation half. typecheck -p integration-tests clean.
Maintainer verification — real local stack, PR head
|
| request 0 | after tool_search |
after invoke | distinct blocks | |
|---|---|---|---|---|
main @168a88c02e |
14 tools / 40 837 B / 6e50202b3646 |
15 / 41 276 / adc146a7c3f8 |
15 / 41 276 / adc146a7c3f8 |
2 |
| PR #10410 @cd12dd31ac | 15 tools / 41 441 B / f79c8b8eb8e1 |
15 / 41 441 / f79c8b8eb8e1 |
15 / 41 441 / f79c8b8eb8e1 |
1 |
On main, tool_search splices mcp__probe__weather_lookup (+439 B) into the declared list, so the entire 40 837-byte tools block changes at exactly the turn the deferred tool is discovered. On this PR the block is the same 41 441 bytes with the same SHA-256 in all three requests, and messages[] is append-only in both builds — so on the PR side the full logical prefix (system + tools + prior turns) stays reusable, and on main it does not.
Measured cost of the fix, same captures: +613 B constant for the always-declared tool_call envelope, against −439 B per deferred tool that would otherwise be spliced in mid-session. For reference, the incomplete-bridge fallback declaring all 9 MCP tools eagerly costs 70 578 B (+70 %).
2 · The two-step bridge works end to end, with a real MCP round trip
tool_search(select:mcp__probe__weather_lookup) returns the schema inside <functions>…</functions> as tool output; tool_call then reaches the MCP server, whose own log shows exactly one CALL weather_lookup city=Hangzhou units=metric, and PROBE_MCP_RESULT city=Hangzhou units=metric temp=21C comes back to the model under tool_call_id=call_bridge_1 — the original call ID, model-facing name still tool_call. Keyword mode ("weather city lookup") reaches the same tool, and two tool_call envelopes in one assistant turn both resolve and execute, each keeping its own call ID.
The identity split is observable from three different vantage points in the same runs:
- model — sees
tool_search→tool_call; the declared list never changes. - hooks — a
PreToolUsehook fires once, withtool_name = mcp__probe__weather_lookupand the target's arguments. A hook matcher ontool_call, configured in the same run, did not fire. - ACP client —
tool_callarrivespendingwith_meta.toolName=tool_call, then flips toin_progresswith_meta.toolName=run_shell_command,title="Shell: echo ACP_BRIDGE_OK",kind=execute, and completes withrawOutput="ACP_BRIDGE_OK"(shell exit 0).
3 · Policy, refusals and fallbacks
Every refusal path in the description reproduces, and in each the MCP server records zero invocations: recursive tool_call→tool_call, tool_call→tool_search, a visible non-deferred target (read_file), and an unregistered target.
Two points worth calling out, because the round-2 review deferred both as untested:
permissions.denyon the resolved target wins. Withpermissions.deny = ["mcp__probe__weather_lookup"], the bridged call returns "…requires permission to use"mcp__probe__weather_lookup"… Matching deny rule:"mcp__probe__weather_lookup"" and the MCP server is never called. The deny is evaluated against the target, not the wrapper.- Approval is evaluated under the target's identity. Under
approvalMode = defaultin headless, the bridged call is refused with the same wordingmainemits for the equivalent direct deferred call — the target name, nottool_call. (The unit-test gap the reviewer flagged is real; the production behaviour is correct.)
Fallbacks all keep deferred tools reachable — no configuration stranded one:
| configuration | declared | tool_search |
tool_call |
MCP schemas | direct call |
|---|---|---|---|---|---|
| bridge complete (baseline) | 15 | yes | yes | hidden | n/a |
tools.toolSearch.enabled = false |
36 | no | no | eager | works |
permissions.deny = ["tool_search"] |
37 | no | yes | eager | works |
permissions.deny = ["tool_call"] |
37 | yes | no | eager | n/a (bridge denied) |
The startup reminder on the wire matches the new contract: "The following tools are reachable through tool_search and tool_call. Review a schema with select:<name> or a keyword query, then invoke it with tool_call."
4 · Mutation probe on the load-bearing guards
Five mutants against src/tools/tool-call.test.ts + src/tools/tool-search.test.ts + src/core/coreToolScheduler.test.ts (459 tests, green at baseline):
| mutant | outcome |
|---|---|
drop the isDeferredAndHidden target guard |
killed |
drop the scheduler's isToolExecutionAllowed re-check on the resolved target |
killed |
| model-facing response name → resolved target name | killed (3 tests) |
| drop the incomplete-bridge unreachability gate | killed |
| drop the recursive-bridge guard | survived — see note 2 |
5 · Suites, typecheck, lint
packages/core: 22 597 passed / 22 628, 1 failure —exit-worktree.test.tstimed out under parallel load and passes 16/16 in isolation on both this branch and the base. Environmental.packages/cli(run from the package dir): 26 298 passed / 26 375, 7 failures acrossacpAgent.test.ts,AuthDialog.test.tsx,systemController.test.ts— all three files pass in isolation (535/535, 25/25, 25/25 with an isolatedQWEN_HOME). Known host-pollution / parallel-load flakes, not this PR.npm run typecheck— pass.npm run lint:ci— pass.
Non-blocking notes
packages/core/src/tools/tool-search.ts:326is not Prettier-formatted —const schemaBlocks = reviewed.map( (tool) =>should wrap. It slips through CI because the Prettier step runsprettier --write .rather than--check, so it reformats silently and exits 0.npm run formatfixes it.- The recursive-bridge guard is not pinned by its test.
rejects recursive bridge target %sasserts onlyerrorType: INVALID_TOOL_PARAMS, which the downstreamisDeferredAndHiddenrejection also returns — so deleting the dedicated guard leaves the suite green. Behaviour is still correct either way (I confirmed both distinct messages live), but asserting on the message would make the guard load-bearing.
What I did not cover
- MCP tools under ACP. MCP servers from user-scope settings did not register in my ACP sessions (gating in my rig, not a PR behaviour), so the ACP bridge path was exercised with a
tools.eager-demoted built-in (run_shell_command) instead of an MCP tool. SameresolveDeferredToolCallcall site inSession.ts. - Provider-side cache counters. The evidence here is the byte-level precondition (identical tools block + append-only messages), not a measured cache hit-rate against a live provider.
- Subagent execution allowlist — pinned by mutation (M2 above) and unit tests, not driven through a live subagent run.
- macOS only (Node v24.18.1). Windows and Linux untested, matching the PR's own table.
中文说明
维护者验证 — 本地真实环境，PR head cd12dd31ac 对比 merge-base 168a88c02e
我为这个 PR 搭了一套真实链路的验证台，跑了 21 次完整 CLI 会话。重点是 PR 描述里自己列为未验证的那一条："没有抓取原始 provider request body 来进行实时的工具声明字节级对比"。这个缺口现在用链路证据补上了，并且与 main 的 A/B 直接展示了这个 PR 消除的回归。
结论：描述中的行为在真实链路上成立，建议合入。 文末两条非阻断意见。
验证台
CLI 边界以下没有任何 mock：
- CLI — 打包后的
node <worktree>/dist/cli.js，两侧各自npm run build && npm run bundle，分别跑 headless-p和 ACP--acp。 - Provider — OpenAI 兼容的 HTTP 服务，在响应前把每一个 request body 原样落盘，按轮次脚本化返回 tool call。
- MCP — 基于
@modelcontextprotocol/sdk的一次性 stdio server，暴露 9 个工具，声明在 user scope 的$QWEN_HOME/settings.json，服务端自己记录每一次调用。 - defer — 用
tools.toolSearch.threshold = 0强制探针工具真的留在 bridge 后面。
两侧同一个 prompt、同一个 MCP server、同一份 settings，只有构建不同。
1 · 已声明工具块在整个会话中逐字节相同（main 上则不然）
| request 0 | tool_search 之后 |
调用之后 | 不同的块数 | |
|---|---|---|---|---|
main @168a88c02e |
14 个 / 40 837 B / 6e50202b3646 |
15 / 41 276 / adc146a7c3f8 |
15 / 41 276 / adc146a7c3f8 |
2 |
| PR #10410 @cd12dd31ac | 15 个 / 41 441 B / f79c8b8eb8e1 |
15 / 41 441 / f79c8b8eb8e1 |
15 / 41 441 / f79c8b8eb8e1 |
1 |
在 main 上，tool_search 把 mcp__probe__weather_lookup（+439 B）插进已声明列表，于是整个 40 837 字节的 tools 块恰好在发现 deferred tool 的那一轮发生变化。这个 PR 上三次请求都是同样的 41 441 字节、同样的 SHA-256；两侧的 messages[] 都是只追加的——所以 PR 这一侧完整的逻辑前缀（system + tools + 之前的轮次）保持可复用，main 那一侧则不行。
同一批抓包量到的代价：固定 +613 B（始终声明的 tool_call 信封），换取每个 deferred tool −439 B 的中途插入。作为参照，bridge 不完整时的回退把 9 个 MCP 工具全部提前声明，代价是 70 578 B（+70%）。
2 · 两阶段桥接端到端可用，并且真的打到了 MCP
tool_search(select:mcp__probe__weather_lookup) 把 schema 包在 <functions>…</functions> 里作为工具输出返回；随后 tool_call 打到 MCP server，服务端日志里恰好一条 CALL weather_lookup city=Hangzhou units=metric，PROBE_MCP_RESULT city=Hangzhou units=metric temp=21C 以 tool_call_id=call_bridge_1（原始 call ID）回到模型，模型侧工具名仍然是 tool_call。关键词模式（"weather city lookup"）同样能找到该工具；一轮里发两个 tool_call 信封也都能各自解析并执行，各自保留自己的 call ID。
身份分离在同一批运行里可以从三个不同视角观察到：
- 模型侧 — 看到的是
tool_search→tool_call，已声明列表始终不变。 - hooks —
PreToolUse只触发一次，tool_name = mcp__probe__weather_lookup，参数是目标工具的参数。同一次运行里配置的tool_callmatcher 没有触发。 - ACP 客户端 —
tool_call先以pending到达，_meta.toolName=tool_call；随后翻成in_progress，_meta.toolName=run_shell_command、title="Shell: echo ACP_BRIDGE_OK"、kind=execute；最后completed，rawOutput="ACP_BRIDGE_OK"（shell exit 0）。
3 · 权限、拒绝与回退
描述里列的每条拒绝路径都复现了，并且每一条 MCP server 都记录到零次调用：递归的 tool_call→tool_call、tool_call→tool_search、可见的非 deferred 目标（read_file）、未注册的目标。
有两点值得单独指出，因为第 2 轮评审把它们都记为未测：
permissions.deny对解析后的目标生效。 配置permissions.deny = ["mcp__probe__weather_lookup"]时，桥接调用返回 "…requires permission to use"mcp__probe__weather_lookup"… Matching deny rule:"mcp__probe__weather_lookup""，MCP server 完全没被调用。deny 判定的是目标，不是外层信封。- 审批按目标身份评估。 headless 下
approvalMode = default时，桥接调用被拒的措辞与main上等价的直接 deferred 调用完全一致——出现的是目标名而不是tool_call。（评审指出的单测缺口确实存在；生产行为是对的。）
回退路径都让 deferred tools 保持可达，没有任何一种配置把工具变成孤儿：
| 配置 | 已声明 | tool_search |
tool_call |
MCP schema | 直接调用 |
|---|---|---|---|---|---|
| bridge 完整（基线） | 15 | 有 | 有 | 隐藏 | 不适用 |
tools.toolSearch.enabled = false |
36 | 无 | 无 | 提前声明 | 可用 |
permissions.deny = ["tool_search"] |
37 | 无 | 有 | 提前声明 | 可用 |
permissions.deny = ["tool_call"] |
37 | 有 | 无 | 提前声明 | 不适用（桥接被拒） |
链路上的启动提醒也与新契约一致："The following tools are reachable through tool_search and tool_call. Review a schema with select:<name> or a keyword query, then invoke it with tool_call."
4 · 对承重防线做变异验证
针对 src/tools/tool-call.test.ts + src/tools/tool-search.test.ts + src/core/coreToolScheduler.test.ts（基线 459 个用例全绿）做了 5 个变异体：
| 变异体 | 结果 |
|---|---|
去掉 isDeferredAndHidden 目标校验 |
被杀 |
去掉 scheduler 对解析后目标的 isToolExecutionAllowed 复检 |
被杀 |
| 模型侧响应名改成解析后的目标名 | 被杀（3 个用例） |
| 去掉 bridge 不完整时的不可达判定 | 被杀 |
| 去掉递归桥接校验 | 存活 — 见第 2 条意见 |
5 · 测试套件、typecheck、lint
packages/core：22 597 通过 / 22 628，1 个失败 ——exit-worktree.test.ts在并行负载下超时，单独跑在本分支和 base 上都是 16/16 通过。属于环境抖动。packages/cli（在包目录里跑）：26 298 通过 / 26 375，7 个失败分布在acpAgent.test.ts、AuthDialog.test.tsx、systemController.test.ts—— 三个文件单独跑都通过（535/535、25/25、隔离QWEN_HOME后 25/25）。是已知的宿主污染 / 并行负载抖动，与本 PR 无关。npm run typecheck通过；npm run lint:ci通过。
非阻断意见
packages/core/src/tools/tool-search.ts:326没有过 Prettier ——const schemaBlocks = reviewed.map( (tool) =>应该换行。之所以能溜过 CI，是因为 Prettier 那一步跑的是prettier --write .而不是--check，会静默重写并以 0 退出。npm run format即可修复。- 递归桥接校验没有被它的测试钉住。
rejects recursive bridge target %s只断言了errorType: INVALID_TOOL_PARAMS，而下游isDeferredAndHidden的拒绝路径返回的是同一个 errorType —— 所以删掉这个专门的 guard，套件依然全绿。两条路径的行为都正确（两种不同措辞我都在真实链路上确认过），但断言到 message 才能让这个 guard 真正承重。
未覆盖的部分
- ACP 下的 MCP 工具。 user scope settings 里的 MCP server 在我的 ACP 会话中没有注册成功（是我这套台子的 gating 问题，不是 PR 行为），因此 ACP 桥接路径改用被
tools.eager降级的内置工具（run_shell_command）验证。走的是Session.ts里同一个resolveDeferredToolCall调用点。 - Provider 侧的缓存计数。 这里的证据是字节级前提（tools 块完全一致 + messages 只追加），不是对真实 provider 测出的缓存命中率。
- Subagent 执行白名单 —— 由上面的变异体和单测钉住，没有通过真实 subagent 运行来驱动。
- 仅 macOS（Node v24.18.1）。Windows / Linux 未测，与 PR 自己的表格一致。
…#10410) 1. tool-search.ts:326 — Prettier-format the schemaBlocks map (slipped through CI because the format step runs --write, not --check). 2. tool-call.test.ts — pin the recursive-bridge guard by its message ('cannot invoke bridge tool'), not just errorType: the downstream isDeferredAndHidden rejection returns the same INVALID_TOOL_PARAMS, so the old errorType-only assertion stayed green with the guard deleted. Mutation check confirms the strengthened test now kills that mutant (2 failed with the guard removed, 12/12 intact).
|
Thank you for the thorough real-stack verification and the byte-level declaration evidence — that closes the one gap the description flagged as unvalidated. Both non-blocking notes are addressed in 839bd8b:
中文说明感谢这份真实链路的验证与字节级声明证据——它补上了描述中唯一标记为未验证的缺口。两条非阻断意见已在 839bd8b 处理：
|
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.
3 Suggestion-level finding(s) this review confirmed are already reported on this PR and are not repeated:
- ACP bridge gates' isToolEnabled failure paths untested (deny / resolution-error / abort) — already recorded in the round-2 review body's deferred list (review 5060349583) as
Session.ts:10784 (+2 locations) - Bridge E2E suite outside every npm workspace — already posted in round 2 as R2-19 (comment 3888891678, integration-tests/sdk-typescript/sdk-mcp-server.test.ts); the author's CI-gate claim was verified this round (test:integration:no-ak:sand…
- Bridged TOOL_NOT_REGISTERED escapes the ACP invalid-params stagnation detector — already recorded in the round-2 review body's deferred list (review 5060349583) as
Session.ts:10804
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 10: running the new tests in this worktree ( cd packages/core && npx vitest run src/core/coreToolScheduler.test.ts ) — the tree has no node_modules or built dist…; chunk 6: run the three new agent-core.test.ts bridge tests and the background-agent-resume reminder test (no node_modules in the review worktree; install+build not attem….
Deferred under the convergence posture (round 3, not a blocker) — recorded, not requested in this round:
packages/core/src/core/coreToolScheduler.test.ts:847 — [review] dead switch: includeToolSearch option never set by any testpackages/cli/src/nonInteractiveCli.ts:2288 (+2 locations) — [probe] headless bridge identity map unwitnessed in both directionspackages/cli/src/acp-integration/session/Session.ts:10980 — [probe] daemon stagnation detector collapses all bridge failures into one bucketpackages/core/src/tools/tool-call.test.ts:67 — [probe] envelope-validation catch untested through the resolution pathpackages/core/src/tools/tool-call.test.ts:201 — [review] bridge-half-unavailable branches have zero test pinspackages/core/src/tools/tool-call.ts:77 — [probe] select: is case-insensitive but tool_call is exact-matchpackages/core/src/core/client.ts:1837 — [probe] queue-time eager-reveal seed delete is unwitnessed and redundant — delete itpackages/core/src/core/coreToolScheduler.test.ts:1214 — [probe] bridge gate's isToolEnabled-throw branch unwitnessedpackages/core/src/core/coreToolScheduler.ts:2591 — [probe] truncation-corrupted bridge envelopes miss the recovery guidancepackages/cli/src/nonInteractiveCli.ts:2053 — [probe] cancelled bridge calls recorded under target identity flip skillsModifiedInSessionpackages/core/src/skills/types.ts:47 (+4 locations) — [review] old-model 'tool_search loads tools' wording left standing in three placespackages/core/src/core/environmentContext.ts:227 — [probe] fork-resume reminder advertises the bridge beyond the fork's allowlist
中文说明
仅完成部分审查，审查缺口已披露。
本轮确认的 3 条建议级发现已在 PR 上报告过，不再重复发布（列表见上方英文部分）。
未审查：build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。
未探索到全部深度（达到工具调用预算）：chunk 10：running the new tests in this worktree ( cd packages/core && npx vitest run src/core/coreToolScheduler.test.ts ) — the tree has no node_modules or built dist…；chunk 6：run the three new agent-core.test.ts bridge tests and the background-agent-resume reminder test (no node_modules in the review worktree; install+build not attem…。
收敛姿态下延后（第 3 轮，非阻断）——已记录，本轮不要求修改：共 12 条（原文未翻译，列表见上方英文部分）。
— qwen3.8-max via Qwen Code /review (v0.22.3)
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.
10 Suggestion-level finding(s) this review confirmed are already reported on this PR and are not repeated:
- includeToolSearch dead switch in the scheduler-test helper — already recorded in the round-3 review body's deferred list (review 5061525570) as coreToolScheduler.test.ts:847
- truncation-corrupted bridge envelopes miss TRUNCATION_PARAM_GUIDANCE — already recorded in the round-3 review body's deferred list (review 5061525570) as coreToolScheduler.ts:2591
- tool_call eager-allowlist exemption has no test pin — already recorded in the round-2 review body's deferred list (review 5060349583) as permission-manager.ts:774
- ACP bridge unwrap failure paths (enablement gate / resolution-error mapping / abort) untested — already recorded in the round-2 review body's deferred list (review 5060349583) as Session.ts:10784 (+2 locations) and disclosed as duplicate in…
- headless bridge identity map unwitnessed in the output-finalization direction — already recorded in the round-3 review body's deferred list (review 5061525570) as nonInteractiveCli.ts:2288 (+2 locations)
- E2E bridge suite outside every npm workspace — already posted in round 2 as R2-19 (comment 3888891678, integration-tests/sdk-typescript/sdk-mcp-server.test.ts); the author's CI-gate claim (test:integration:no-ak:sandbox:none) was verified i…
- bridge gate isToolEnabled-throw branch unwitnessed — already recorded in the round-3 review body's deferred list (review 5061525570) as coreToolScheduler.test.ts:1214
- PM/deny re-check of the resolved bridge target unpinned — already recorded in the round-2 review body's deferred list (review 5060349583) as coreToolScheduler.test.ts:1034
- bridged TOOL_NOT_REGISTERED escapes the ACP invalid-params stagnation detector — already recorded in the round-2 review body's deferred list (review 5060349583) as Session.ts:10804 and confirmed as a duplicate in the round-3 body
- bridge approval flow untested (awaiting_approval reachable, all bridge tests under YOLO) — already recorded in the round-2 review body's deferred list (review 5060349583) as coreToolScheduler.test.ts:1035
Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally; packages/cli, sdk-typescript, vscode-ide-companion and web-shell suites also did not run locally (Agent 7's whole-call budget expired in the build phase). packages/core was re-run locally: 22582 passed; 36 residual failures all in files this PR does not touch (hook process-spawn, fs-lease, timing, session-env shapes), 71 of the original 107 disappeared when this review session's QWEN_HOME env leak was excluded, and CI's ubuntu Test matrix is green at this exact commit — classified environmental.
Not explored to full depth (tool budget reached): "agent 1c": none — no check was cut short.; "agent reverse-audit (round 2)": none — the full chunk (diff lines 2952-3346) was read untruncated and every check above was completed..
Not reviewed: reverse audit — stopped before round 4 by the review time budget.
Deferred under the convergence posture (round 4, not a blocker) — recorded, not requested in this round:
integration-tests/sdk-typescript/sdk-mcp-server.test.ts:208 — [probe] post-select E2E assertions are membership-only; declaration-list equality (the prompt-cache stability property) unpinnedpackages/core/src/core/client.ts:1783 — [probe] rememberAnnouncedDeferredTools re-seed liveness guard and dead-seed drop have no test witnesspackages/core/src/core/environmentContext.test.ts:518 — [probe] incomplete-bridge reminder gate's TOOL_SEARCH term unwitnessedpackages/core/src/core/client.ts:2622 — [probe] resumed sessions seed recentCompletedToolNames with bridge names instead of resolved targets, degrading memory-recall tool hintspackages/core/src/core/client.test.ts:2870 — [probe] missingHalves warning enumeration half-unwitnessed (two mutants pass the committed suite)packages/core/src/core/coreToolScheduler.test.ts:1035 — [probe] PostToolUse resolved-name pin missing for bridged calls
中文说明
仅完成部分审查，审查缺口已披露。
本轮确认的 10 条建议级发现已在 PR 上报告过，不再重复发布（列表见上方英文部分）。
未审查：build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally; packages/cli, sdk-typescript, vscode-ide-companion and web-shell suites also did not run locally (Agent 7's whole-call budget expired in the build phase). packages/core was re-run locally: 22582 passed; 36 residual failures all in files this PR does not touch (hook process-spawn, fs-lease, timing, session-env shapes), 71 of the original 107 disappeared when this review session's QWEN_HOME env leak was excluded, and CI's ubuntu Test matrix is green at this exact commit — classified environmental。
未探索到全部深度（达到工具调用预算）："agent 1c"：none — no check was cut short.；"agent reverse-audit (round 2)"：none — the full chunk (diff lines 2952-3346) was read untruncated and every check above was completed.。
未审查：反向审计——评审时间预算不足，未能开始第 4 轮。
收敛姿态下延后（第 4 轮，非阻断）——已记录，本轮不要求修改：共 6 条（原文未翻译，列表见上方英文部分）。
— qwen3.8-max via Qwen Code /review (v0.22.3)
…ol_call bridge (QwenLM#10410) R4-1 (review round 4): the tool_call bridge bypassed the subagent/teammate tool-exclusion set. EXCLUDED_TOOLS_FOR_SUBAGENTS / EXCLUDED_TOOLS_FOR_TEAMMATES were enforced only at declaration level (prepareTools isExcluded), but the bridge makes invocation independent of declaration without carrying the exclusion set over — resolveDeferredToolCall enforced only the plan-lifecycle and leader-only policies, tool_search had no exclusion filter, and the scheduler's post-unwrap re-checks never consulted the set. A wildcard/ general-purpose subagent (or teammate) could therefore discover (tool_search) and execute (tool_call) control-plane tools it must not reach (team_delete, cron_*, workflow, send_message, ...); the probe executed team_delete end-to-end through the bridge from a subagent context. The merge base rejected the same trigger (undeclared names fail the declaredToolNames gate), so the bridge introduced the path. Fix: enforce the same context-aware exclusion set in resolveDeferredToolCall, after the plan-lifecycle/leader-only checks (so those keep their specific messages). To avoid a circular import (agent-core imports coreToolScheduler, which imports tool-call), the exclusion sets and getExcludedToolsForCurrentContext moved from agent-core.ts to subagent-plan-tool-policy.ts (already imported by tool-call.ts); agent-core.ts re-exports them for existing consumers. The check is guarded by isSubagentLikeExecutionContext so the leader context is not gated. Adds a mutation-checked test (rejects an exclusion-set target team_delete / workflow bridged from a subagent context): removing the exclusion check turns both red. tool-call.test.ts 14/14; tool-search + coreToolScheduler 447; subagent-plan-tool-policy 6; agent-core + background-agent-resume 99; core tsc --noEmit clean.
…re (TS6133) The exclusion sets that spread SUBAGENT_PLAN_LIFECYCLE_TOOLS moved to subagent-plan-tool-policy.ts so the tool_call bridge can enforce them without a circular import; the import in agent-core.ts was left behind and tsc --build flags it as unused (TS6133).
Dismissing stale review: the sole finding R4-1 (bridge bypasses the subagent/teammate tool-exclusion set) was addressed after this review's commit (839bd8b):
- bad5b1e: resolveDeferredToolCall now enforces the same context-aware exclusion set as prepareTools (EXECUTION_DENIED), checked after the plan-lifecycle/leader-only checks so those keep their specific messages; exclusion sets moved to subagent-plan-tool-policy.ts to avoid a circular import and re-exported from agent-core.ts for existing consumers.
- Mutation-checked: new it.each cases in tool-call.test.ts fail when the check is removed.
- 192ffba: follow-up fix for the TS6133 unused-import the refactor left behind.
The review was submitted 2026-08-31T11:52 against pre-fix commit 839bd8b; no open findings remain against current head.
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.
1 Suggestion-level finding(s) this review confirmed are already reported on this PR and are not repeated:
- envelope-validation catch untested through the resolution path (tool-call.test.ts:67) — already recorded in the round-3 review body's deferred list (review 5061525570) as tool-call.test.ts:67 — [probe]
Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally; this round's harness test phase also did not run (whole-call budget exhausted by install + 16 workspace builds — packages/core build verified green, and the new tool-call.test.ts passes 14/14 when run directly).
Not reviewed: reverse audit — did not converge within the reverse-audit round cap of 5.
Deferred under the convergence posture (round 5, not a blocker) — recorded, not requested in this round:
packages/core/src/tools/tool-call.ts:90 — [probe] case-sensitive tool_call resolution contradicts case-insensitive tool_search discovery (select:Read_File resolves, tool_call Read_File → TOOL_NOT_REGISTERED)packages/core/src/tools/tool-call.test.ts:106 — [probe] unknown-target present-side remedy ('Run tool_search again') unpinned; absent-side-constant mutant survives the suite
Convergence: round 5 posted 7 inline comment(s), 7 of them reported for the first time; the previous round posted 1 (1 new). Findings keep coming back to the same files: packages/core/src/tools/tool-call.ts (findings in round 4; 3 more now). The rate of new findings is not falling. A cluster that keeps producing siblings usually means the fixes are treating instances of a shared root cause — triaging that cause before the next round, or splitting an independent cluster into its own pull request, tends to end the loop faster than fixing them one at a time. Batching the remaining fixes and verifying them before the next push, or dropping this PR's reviews to --severity-floor critical, keeps the loop from re-deriving the same set. (Observation only — nothing was withheld from this review because of this observation.)
中文说明
仅完成部分审查，审查缺口已披露。
本轮确认的 1 条建议级发现已在 PR 上报告过，不再重复发布（列表见上方英文部分）。
未审查：build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally; this round's harness test phase also did not run (whole-call budget exhausted by install + 16 workspace builds — packages/core build verified green, and the new tool-call.test.ts passes 14/14 when run directly)。
未审查：反向审计——在 5 轮的反审轮数上限内未收敛。
收敛姿态下延后（第 5 轮，非阻断）——已记录，本轮不要求修改：共 2 条（原文未翻译，列表见上方英文部分）。
收敛情况：第 5 轮发布了 7 条行内评论，其中 7 条是首次提出；上一轮发布了 1 条（其中 1 条首次提出）。发现反复回到同一批文件：packages/core/src/tools/tool-call.ts（第 4 轮已出过发现，本轮又有 3 条）。新发现的产出速度没有下降。一个不断再生兄弟发现的簇，通常意味着逐条修复只在处理同一根因的实例——先定位并处理该根因，或把独立的簇拆成单独的 PR，通常比逐条修复更快结束循环。把剩余修复攒成一批、验证后再推送，或将本 PR 的评审降到 --severity-floor critical，可以避免循环反复推导同一组发现。（仅为观察——本轮评审未因此扣留任何内容。）
— qwen3.8-max via Qwen Code /review (v0.22.3)
…wenLM#10410) R4-1 follow-up (Critical): the flat exclusion denied AgentTool through the bridge even while maxSubagentDepth permitted the spawn — prepareTools depth-gates AgentTool (re-admits it while spawnBlockReason === null) but the bridge consumed the raw set. Add the shared predicate isToolExcludedForCurrentContext (subagent-plan-tool-policy.ts) that carries the depth-gated re-admission, consumed by BOTH prepareTools and resolveDeferredToolCall so the layers cannot drift; the scheduler threads the real configured maxSubagentDepth through. Without the configured depth the bridge fails closed on AgentTool (the raw-set floor). The predicate is deliberately ungated so prepareTools keeps its frameless fail-closed contract; the bridge and tool_search apply their own isSubagentLikeExecution context gate, leaving the leader session untouched. R5-1: mirror the exclusion on the discovery side — collectCandidates and the select: blocked predicate now drop context-excluded tools, so subagents are no longer advertised the schemas of tools they are forbidden to invoke (schema leak + wasted turns). Leader re-inspection stays unrestricted. R5-2: move the plan-lifecycle / leader-only / exclusion checks ahead of the isDeferredAndHidden gate (relative order kept). Real excluded tools are frequently not deferred (workflow/team_delete/todo_write shouldDefer=false, enter_plan_mode constructed shouldDefer=false), so the old gate order misrouted them into the factually-wrong 'already visible — call it directly' INVALID_TOOL_PARAMS denial and the scheduler's malformed-envelope retry counting. R5-3..R5-6 test pins: SEND_MESSAGE subagent denial + teammate resolve/deny cases discriminate the context-aware selector; the plan-tool denial pins the dedicated message; allow-side cases pin the context gate (leader and in-frame non-excluded resolution); the legacy alias case pins canonical-name membership keying. Fixtures now use the real non-deferred shape. Mutation-verified: nine mutants (check removed, selector swapped both ways, plan check removed, envelope-name keying, flat AgentTool, bridge gate removed, blanket in-frame denial, tool-search filter removed) all turn the new tests red; suites green (tool-call 22, tool-search 56, policy 6, agent-core 49, coreToolScheduler 396).
|
Round-5 review batch addressed in 074b4ad (all seven requested findings; verified before pushing, per the convergence note): R4-1 follow-up (Critical) — the flat exclusion no longer denies R5-1 — discovery side closed in this PR: R5-2 — plan-lifecycle / leader-only / exclusion checks moved ahead of the R5-3..R5-6 — SEND_MESSAGE + teammate resolve/deny cases discriminate the selector both ways; the plan-tool denial pins its message; allow-side cases pin the context gate (in-frame non-excluded + leader); the legacy-alias case pins canonical-name membership keying. Mutation-verified: nine mutants (check removed; selector swapped both ways; plan check removed; envelope-name keying; flat AgentTool; bridge gate removed; blanket in-frame denial; tool-search filter removed) all turn the new tests red. Green suites: tool-call 22, tool-search 56, policy 6, agent-core 49, coreToolScheduler 396. Recorded, intentionally not in this batch (deferred list from the round-5 body): the case-sensitive 第 5 轮评审要求的 7 项发现已在 074b4ad 全部处理并推送前验证完毕：R4-1 后续（新增共享谓词，prepareTools 与桥接共用深度门控的 AgentTool 重新准入，调度器透传真实配置深度，深度未知 fail-closed，leader 不受影响）；R5-1 发现侧过滤关闭（候选与 select: 阻断均按上下文门控，leader 复查不受限）；R5-2 三个策略检查前移至 deferred 门之前（保持相对顺序，fixture 钉真实形状）；R5-3..R5-6 测试补齐（选择器双向区分、plan 消息钉、放行侧两用例、遗留别名钉规范名键控）。九个变异体全部变红；各套件全绿。评审体中列出的两项延迟项（大小写不一致、未知目标补救钉）按轮次纪律留给下一轮/跟进。 |
Conflicts resolved in favor of the bridge-aware documentation and the tool_search + tool_call wording from this PR, while carrying forward main's new content: the automatic DeepSeek opt-out condition and the structured_output exempt-list entry are folded into the bridge pairing descriptions (settingsSchema, vscode schema, settings.md, SDK types/README/docs tables); main's new visibleTools registry test is kept alongside this PR's renamed reveal test; main's expanded settings table is kept with the six bridge-touched rows merged.
Case-insensitive resolution: tool_search's select: resolves requested names
case-insensitively, but the tool_call half resolved them exactly, so a schema
reviewed as e.g. Read_File was not callable through the bridge. The
invocation half now falls back to a case-insensitive match against the
registered names (exact-case names keep the fast path), and the fallback feeds
the recursive bridge guard, so casing cannot dodge it either.
Remedy pin: 'rejects an unknown deferred target' now pins the present-side
remedy ('Run tool_search again...'); the absent-side constant was already
pinned by the sibling case, so both remedy branches are mutation-checked.
Mutation-verified: removing the fallback turns 2 tests red; dropping the
remedy suffix turns 1 red; suite green (24).
3 participants
What this PR does
This PR replaces deferred-tool schema revelation with a stable two-step bridge.
tool_searchlets the model review a deferred tool's schema without changing the declared tool list, andtool_callvalidates and invokes that deferred tool through the existing execution pipeline.The bridge works consistently across interactive scheduling, headless mode, ACP sessions, permissions, approvals, hooks, concurrency, telemetry, subagent restrictions, and lazy tool loading. Model-facing requests and responses retain the stable bridge identity while policy and execution use the underlying target identity. If tool search is disabled or either bridge is unavailable, deferred tools are declared eagerly as a safe fallback.
Configuration, SDK types, generated settings, UI labels, tests, and design documentation are updated to describe the new behavior.
Why it's needed
The previous deferred-tool flow revealed schemas by mutating the model's declared tool list after
tool_search. That changes the prompt prefix and breaks prompt-cache reuse precisely when deferred tools are discovered, increasing latency and token cost and creating inconsistent behavior across execution paths.This is a clean, independent reimplementation of the bridge idea, inspired by Hermes Agent's
tool_searchfollowed bytool_callflow. It supersedes the earlier implementation in #8276 without reusing its code.Reviewer Test Plan
How to verify
tool_search, invokes it throughtool_call, and never adds the deferred schema to later model requests.tool_searchandtool_calldeclarations remain byte-stable and that the rest of the declared tool list does not change.tool_callwith the original call ID.Local verification completed: a live development-CLI smoke test used a disposable stdio MCP server with the tool forced behind the bridge and observed
tool_search→tool_call→ one underlying MCP invocation, the exact expected result, and prompt-cache reads on both continuation turns. In addition, 12 core test files passed with 2,281 tests; 4 CLI/ACP/headless test files passed with 1,229 tests and 1 skipped;npm run build,npm run typecheck,npm run lint, andgit diff --checkall passed.Evidence (Before & After)
N/A — this changes tool routing and model request structure rather than user-visible TUI output.
Tested on
Environment (optional)
Local workspace on macOS with Node.js 22+, a live development-CLI run against a disposable stdio MCP server, package-level Vitest suites, and monorepo build, typecheck, and lint checks.
Risk & Scope
tools.toolSearch.enabledset tofalse, both bridge tools are disabled and deferred tool schemas are declared eagerly.Linked Issues
Supersedes #8276.
中文说明
这个 PR 做了什么
这个 PR 用一个稳定的两阶段桥接流程替换了 deferred tool 的动态 schema reveal。
tool_search让模型查看 deferred tool 的 schema，但不会改变已声明的工具列表；tool_call则负责校验目标，并通过现有执行链路调用该 deferred tool。这套桥接在交互式调度、headless 模式、ACP session、权限、审批、hooks、并发、遥测、subagent 限制和 lazy tool 加载中保持一致。模型侧请求和响应保留稳定的桥接工具身份，而策略判断和实际执行使用底层目标工具身份。如果 tool search 被禁用，或者任一桥接工具不可用，deferred tools 会被直接声明，作为安全回退。
配置、SDK 类型、生成的 settings、UI 文案、测试和设计文档也一并更新，以说明新的行为。
为什么需要它
之前的 deferred-tool 流程会在
tool_search之后通过修改模型已声明的工具列表来 reveal schema。这样会改变提示词前缀，恰好在发现 deferred tools 时破坏提示词缓存复用，从而增加延迟和 token 成本，并让不同执行路径的行为不一致。这是对该桥接思路的一次干净、独立的重新实现，流程参考了 Hermes Agent 的
tool_search后接tool_call设计。它取代了 #8276 中较早的实现，但没有复用其中的代码。Reviewer 测试计划
如何验证
tool_search查看 schema，再通过tool_call调用，并且后续模型请求中不会新增该 deferred schema。tool_search和tool_call的声明保持字节级稳定，其他已声明工具列表也不发生变化。tool_call，并保留原始 call ID。本地验证已完成：使用一次性 stdio MCP server 运行了真实开发版 CLI 冒烟测试，将目标工具强制置于 bridge 后，实际观察到
tool_search→tool_call→ 一次底层 MCP 调用、完全一致的预期结果，以及两个 continuation turn 上的提示词缓存读取。此外，12 个 core 测试文件通过，共 2,281 个测试；4 个 CLI/ACP/headless 测试文件通过，共 1,229 个通过、1 个跳过；npm run build、npm run typecheck、npm run lint和git diff --check均通过。证据（Before & After）
不适用——这个改动影响的是工具路由和模型请求结构，不是用户可见的 TUI 输出。
测试平台
环境（可选）
macOS 本地 workspace，Node.js 22+，针对一次性 stdio MCP server 运行了真实开发版 CLI，并运行了 package 级 Vitest suites，以及 monorepo build、typecheck 和 lint 检查。
风险与范围
tools.toolSearch.enabled设为false时，两个桥接工具都会被禁用，deferred tool schemas 会被直接声明。关联 Issue
取代 #8276。