Repository navigation
feat: add hybrid code mode - #11854
DragonnZhang wants to merge 32 commits into
Conversation
E2E test report
|
doudouOUC
left a comment
doudouOUC
left a comment
There was a problem hiding this comment.
Read the full diff of the ten production files and audited every non-test getToolMode/getCodeModeOnly/isCodeModeEnabled read site at this head. No new production defect found — and, per the review rules, I re-verified the four open Criticals against the code as it stands rather than against their threads. All four still stand; I am not re-filing them, listing them here only so the record shows they were measured at this commit.
- R1-1 — still stands.
packages/cli/src/config/config.ts:2261-2264passessettings.tools?.modestraight through (? (settings.tools?.mode ?? ToolMode.Direct)), whilepackages/core/src/config/config.ts:10399gates registration with a denylist (if (this.getToolMode() === ToolMode.Direct) return;). An out-of-union value therefore registersexecwhile the other read sites treat the session asdirect. I confirmed there is no value validation on the settings-load path. - R1-22 — still stands.
agent-core.ts:1720-1724:isCodeModeEnabled(...) && this.executionAllowedExactTools?.has(ToolNames.EXEC) && getToolExposure(toolName) === 'code-mode-callable'returns true, i.e. an allowlist containing onlyexecauthorises direct calls to every code-mode-callable tool. In hybrid there is no second wall (the scheduler gate isCodeModeOnly-only, which I verified is correct for hybrid). - R1-2 and R1-3 I did not re-derive line by line; the first needs only that
ToolModeis a value import evaluated at module load insettingsSchema.ts(it is), and the second only thatagent-core.tsnarrowsdeclarationNamesto{exec} ∪ configuredNameswhile the binding plan keeps the widened set (it does).
Verified correct for hybrid, so no change is needed there: the twelve unconverted === CodeModeOnly checks are each semantically right for code_mode (scheduler/ACP direct-call gates, deferred reveal and preload, getDeferredToolSummary, the zoom hint); planCodeModeBindings builds a fresh plan per call, so there is no cache-staleness path; tool_search stays registered and top-level-visible in hybrid with revealed deferred tools receiving augmented declarations through the normal refresh; and no codeModeOnly stragglers remain in any production file, with the regenerated settings.schema.json enum matching the schema source.
|
@qwen-code /takeover from 5 |
qwen-code-dev-bot
added
the
autofix/takeover
|
🤝 Takeover engaged: the autofix loop now manages this PR — it will address new review feedback and resolve base conflicts until the label is removed or the round cap is reached. This window's round counter starts at 5 (the rounds this PR spent in review before takeover), so the Critical-only brake engages after 0 more change-producing round(s) instead of a full fresh 5. Remove the 中文说明🤝 已接管：autofix 循环现在管理此 PR —— 将持续处理新的评审反馈与 base 冲突，直到移除标签或达到轮次上限。本窗口轮次计数从 5 起算（即本 PR 托管前已进行的评审轮数），因此再经过 0 个产生改动的轮次即进入 Critical-only，而非重新计满 5 轮。移除 |
|
🔄 AutoFix is working on this PR — round 11/100. Watch live progress; this round posts its report here when it finishes. 中文说明🔄 AutoFix 正在处理此 PR —— 第 11/100 轮。查看实时进度；本轮结束后会在此发布报告。 |
- web-shell: call the exported isSettingVisible in the code-mode alias test and sync the mirrored alias table to tools.mode, fixing the ReferenceError and stale-alias failures in the Test job - coreToolScheduler: render only the skill invocation surfaces the session actually has — no exec surface in direct mode where exec is never registered, no top-level Skill surface in code_mode_only where direct skill calls are rejected - agent-core: let MCP tool names fall through to the exact-name and pattern checks before the nested exec gateway admits them, so a server-level executionAllowedTools pattern is honored for nested bindings - docs/settings: sync the threshold bridge-mode wording, the container execution fallback, and the zh-CN withheld-tool translation with the shipped behavior
# Conflicts: # packages/cli/src/config/settingsSchema.ts # packages/core/src/config/config.ts # packages/vscode-ide-companion/schemas/settings.schema.json
|
🤖 Addressed the latest review feedback (round 6/100). What changed, and what I pushed back on: · 已处理最新评审反馈（第 6/100 轮）。改动内容与我反驳保留之处如下： Autofix round summary — PR #11854Round mode: Critical-only (window seeded at round 5). Scope this round: the failed Commits: Resolved findingsR4-1 (rc:4066711873) — Critical — web-shell test calls nonexistent
|
The execution-allowlist branch already restricted MCP nested bindings to matching exact names or server patterns once the list mentioned MCP, but the configured-tools branch — the only allowlist SubagentConfig exposes — applied the exec carve-out to every code-mode-callable name, so tools: ['exec', 'mcp__github__read_*'] admitted every connected server's tools as nested exec bindings. Route MCP names through the shared raw-identity matcher in that branch too, and pin the bridge-route zoom hint case for narrowed agents. Co-authored-by: Qwen-Coder <[email protected]>
|
🤖 Addressed the latest review feedback (round 8/100). What changed, and what I pushed back on: · 已处理最新评审反馈（第 8/100 轮）。改动内容与我反驳保留之处如下： Address-review round — PR #11854This round merged Merge with origin/main (
|
|
🤖 Reviewed the latest feedback — no changes needed. Why, point by point: · 已审阅最新反馈——无需改动。逐点说明原因如下： Autofix round — no changeNo code change was made in this round. One item was triaged — the CANCELLED Actionable review feedbackNone. The round ran in Critical-only mode and the actionable sections (Reviews / Inline comments / Issue-level comments) were empty — every open non-Critical item is deferred by the workflow for human follow-up and was not touched (no code, thread resolutions, or replies), as required. Failed checks triage:
|
Under tools.mode "code_mode_only", a configured agent tools list granting exec expands to every code-mode-callable registry name — including every connected MCP tool — so the includes() disjunct in isToolExecutionAllowed short-circuited before the MCP raw-identity narrowing could fire: tools: ['exec', 'mcp__github__read_*'] admitted mcp__payments__charge as a nested exec binding. Stop the expansion from adding MCP names once the configured list mentions MCP, letting the existing narrowing decide, and pin the CodeModeOnly case mirroring the hybrid one. Co-authored-by: Qwen-Coder <[email protected]>
|
🤖 Addressed the latest review feedback (round 9/100). What changed, and what I pushed back on: · 已处理最新评审反馈（第 9/100 轮）。改动内容与我反驳保留之处如下： Address review round — PR #11854Critical-only mode is active this round (window seeded at round 5 by takeover). The actionable surface held one Critical finding; the deferred non-Critical list and the advisory sections (diff-growth, residual-risk) are the workflow's own records and were not worked. Feedback addressed
Changes
No merge was performed ( Verification
中文说明本轮评审处理 — PR #11854本轮处于仅处理 Critical 的模式（窗口由 takeover 从第 5 轮起算）。可执行区域只有一条 Critical 发现；延后处理的非 Critical 清单与各项提示段（diff 增长、残余风险）是工作流自身的记录，本轮未处理。 已处理的反馈
改动
本轮未做合并（ 验证
Deferred non-Critical feedbackCritical-only mode is active: the round counter reached 5 (this window was seeded at round 5 by 中文说明已进入仅处理 Critical 的模式：轮次计数已达 5（本窗口由 Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 🧵 Resolved all 1 selected review thread(s). · 已关闭全部选中的 1 条评审线程。 Re-review when you have a moment. After round 100 this bot stops and leaves the PR for a human. · 有空请复审；第 100 轮后本 bot 停止并将 PR 交给人工。 🧠 Handled by Qwen Code · model/模型 |
|
🔀 Base updated: red check(s) [Lint & Static (ubuntu-latest, Node 22.x)] pass on current main — merged current main via update-branch; CI will re-run. 中文说明🔀 已更新 base：红色检查 [Lint & Static (ubuntu-latest, Node 22.x)] 在当前 main 上通过 —— 已通过 update-branch 合入当前 main，CI 将重新运行。 |
|
⏸️ AutoFix paused by a review convergence signal: 3 consecutive automated review round(s) signalled machine-readable non-convergence (codes observed since the last maintainer response, or the window start if none: batch-fixes, root-cause-triage) — the reviewer keeps finding new defects at a rate that is not falling while the loop keeps widening the diff, so another automatic round is unlikely to converge this PR. The loop resumes once a maintainer responds on this PR (a review or comment counts, and steers the next rounds), and pauses again if the signal persists for 3 more round(s). Alternatives: split the recurring cluster into its own PR, batch the remaining fixes into one push, comment 中文说明⏸️ AutoFix 已因评审收敛信号暂停：3 轮自动评审连续发出机器可读的不收敛信号（自上次维护者响应以来观察到的信号码；若无响应则自窗口开始：batch-fixes, root-cause-triage）——评审仍在以不降的速率发现新缺陷，而循环在继续扩大 diff，再跑一轮自动修复难以收敛本 PR。维护者在本 PR 上作出回应后循环自动恢复（评论或评审均可，并将作为后续轮次的指引）；若信号再持续 3 轮会再次暂停。可选做法：把反复出问题的簇拆成独立 PR、把剩余修复攒成一批一次推送、评论 |
🩺 serve daemon A/BBuilt the PR base vs this PR head
|
| field | PR base (before) | this PR (after) |
|---|---|---|
activeWorkStaleMs |
6 |
9 |
— Qwen Code · serve A/B
🖼️ web-shell visual previewRendered against a mock daemon (no real backend): the PR base vs this PR head Screenshots · before / after✅ No screenshot changes against the PR base. Full-resolution recordings (.webm) are attached to the workflow run. — Qwen Code · web-shell visuals |
|
🔀 Base updated: red check(s) [delay-automatic-review] pass on current main — merged current main via update-branch; CI will re-run. 中文说明🔀 已更新 base：红色检查 [delay-automatic-review] 在当前 main 上通过 —— 已通过 update-branch 合入当前 main，CI 将重新运行。 |
qwen-code-review-bot
left a comment
qwen-code-review-bot
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed.
Unresolved, please confirm:
- [Critical] Prior-round Critical R1-2 (asserted in @doudouOUC's review 5217747523, packages/cli/src/config/settingsSchema.ts:36) — the original inline comment was deleted, so only the reviewer's premise survives and the claimed mechanism could not be t…
Not reviewed: build-and-test — 'Integration Tests (CLI, No Sandbox)' was skipped in CI and its suite did not run locally (Agent 7 ran per-package vitest suites only).
Not explored to full depth (tool budget reached): "agent reverse-audit (round 1)": none — the chunk's remaining items were walked clear: agent-types.ts 's new field and all five read sites, the subagent-plan-tool-policy.test.ts title-only c…; "agent reverse-audit (round 1)": mutation-run confirming that deleting the hybrid declarationNames snapshot clause leaves agent-core.skill-gate.test.ts and agent-core.fork-policy.test.ts …; "agent reverse-audit (round 1)": whether getToolExposure(ToolNames.EXEC) is 'code-mode-callable' — inferred from the passing codeModeAllowedToolNames assertions ( exec never appears in a…; "agent reverse-audit (round 2)": did not execute mutation probes (plain registerTool in place of registerPermissionDeferredFactory ; a hybrid search_memory allowlist through getFunctionDe…; "agent reverse-audit (round 2)": did not run npx vitest run src/code-mode/code-mode.test.ts to confirm the suite is green at this commit, so my claim that F1's assertions hold today is derive…, and 2 more.
Not reviewed: reverse audit — stopped before round 3 by the review time budget.
中文说明
仅完成部分审查，审查缺口已披露。
未决，请确认：共 1 条（原文未翻译，列表见上方英文部分）。
未审查（原文为英文）：build-and-test — 'Integration Tests (CLI, No Sandbox)' was skipped in CI and its suite did not run locally (Agent 7 ran per-package vitest suites only).
未探索到全部深度（达到工具调用预算）："agent reverse-audit (round 1)"：none — the chunk's remaining items were walked clear: agent-types.ts 's new field and all five read sites, the subagent-plan-tool-policy.test.ts title-only c…；"agent reverse-audit (round 1)"：mutation-run confirming that deleting the hybrid declarationNames snapshot clause leaves agent-core.skill-gate.test.ts and agent-core.fork-policy.test.ts …；"agent reverse-audit (round 1)"：whether getToolExposure(ToolNames.EXEC) is 'code-mode-callable' — inferred from the passing codeModeAllowedToolNames assertions ( exec never appears in a…；"agent reverse-audit (round 2)"：did not execute mutation probes (plain registerTool in place of registerPermissionDeferredFactory ; a hybrid search_memory allowlist through getFunctionDe…；"agent reverse-audit (round 2)"：did not run npx vitest run src/code-mode/code-mode.test.ts to confirm the suite is green at this commit, so my claim that F1's assertions hold today is derive…，另有 2 条。
未审查：反向审计——评审时间预算不足，未能开始第 3 轮。
— qwen3.8-max via Qwen Code /review (v0.25.0)
| (this.runtimeContext.getToolMode?.() === ToolMode.CodeModeOnly || | ||
| !isHiddenByEagerAllowList(name)) && |
There was a problem hiding this comment.
[Critical] R1-1: N03: [certifies-falsely] [new-surface] The new Hybrid-only eager exclusion removes a demoted skill from both the declarations and the exec binding set, but the skill-listing predicate that this same diff rewired (willHaveSkillTool() → toolConfigAllowsSkill(this.toolConfig, hasAgentSkillExecBinding(...)), agent-core.ts:660-664) still answers true through its two non-exec clauses, so in tools.mode: "code_mode" the listing and canInvokeSkill() disagree — the exact #12424 divergence the shared predicate exists to prevent, and the invariant the author's own matrix asserts (expect(gate(core, declared)).toBe(willHaveSkill), agent-core.skill-gate.test.ts:402).
Trigger: tools.mode: "code_mode" (a value this diff adds — ToolMode.CodeMode is new at packages/core/src/tools/code-mode.ts:23) plus an active settings.tools.eager allowlist that omits skill. registerLazyTool turns that omission into status === 'deferred' → registry.registerPermissionDeferredFactory(ToolNames.SKILL, …) (config.ts:12251-12252, skill registered lazily at config.ts:12704-12708), getVisibleTools() is sourced only from settings.tools.visible (config.ts:8322-8331) so it does not contain skill, and getToolExposure('skill') is 'code-mode-callable' (code-mode.ts:39-58 — skill is in neither HIDDEN_TOOLS nor DIRECT_ONLY_TOOLS). Any subagent reaches this with the default config, because SubagentManager builds tools: configuredToolConfig?.tools ?? ['*'] (subagent-manager.ts:1150-1151). Then, traced line by line at this commit: prepareTools() sets isHiddenByEagerAllowList('skill') true (agent-core.ts:709-712 → isPermissionDeferred ∧ isDeferredAndHidden, tool-registry.ts:589 + 1352-1360, alwaysLoad defaults false — SkillTool's super() passes only 7 args, skill.ts:157-166), so the anchored clause drops skill from allowedNames; Hybrid declarationNames ⊆ allowedNames (agent-core.ts:764-773) so skill is not declared, and codeModeAllowedToolNames (agent-core.ts:759-762) excludes it. canInvokeSkill(declared) is therefore false on both routes: declaredToolNames.has(SKILL) false, and the nested route fails its codeModeAllowedToolNames?.includes(SKILL) === true term (agent-core.ts:1854-1862). willHaveSkillTool() is nevertheless true: toolConfigAllowsSkill returns inheritsRegistry || names.includes(SKILL) || reachesThroughExec (subagent-plan-tool-policy.ts:160-164), and inheritsRegistry (names.includes('*')) or names.includes('skill') short-circuits before the exec term that hasAgentSkillExecBinding correctly answers false for. Wrong outcomes at all three consumers of that answer: createChat injec
Witness:
`Source: [probe]` — permission-deferred, unrevealed, non-visible `skill`; hybrid vs Only × four `tools` lists: ``` N03 {"mode":"code_mode","tools":["*"], "willHaveSkillTool":true, "canInvokeSkill":false,"declaredHasSkill":false,"codeModeAllowedHasSkill":false} N03 {"mode":"code_mode","tools":["skill"], "willHaveSkillTool":true, "canInvokeSkill":false,"declaredHasSkill":false} N03 {"mode":"code_mode","tools":["skill","exec"],"willHaveSkillTool":true,"canInvokeSkill":false} N03 {"mode":"code_mode","tools":["exec"], "willHaveSkillTool":false,"canInvokeSkill":false} ← agrees N03 {"mode":"code_mode
Suggested fix: Give the shared predicate the same eager-scope fact prepareTools() uses, so it vetoes every route in Hybrid, not just the exec route: thread a skillEagerHidden (or mode-aware) input into toolConfigAllowsSkill computed as mode === ToolMode.CodeMode && registry.isPermissionDeferred?.(SKILL) === true && registry.isDeferredAndHidden?.(SKILL) === true, return false when it is set, and pass it from all three call sites (agent-core.ts:660-664, subagent-manager.ts:1187-1189, background-agent-resume.ts:157) — fixing only hasAgentSkillExecBinding (as R1-17 proposes) leaves the wildcard and explicit-skill clauses returning true.
The fix must not violate this existing fact: agent-core.skill-gate.test.ts:398-400 — expect(willHaveSkill).toBe(mode === ToolMode.CodeModeOnly || visibility === 'visible');. The fix must keep CodeModeOnly answering true for visibility: 'hidden' (its prepareTools() bypasses the eager filter at agent-core.ts:741, so skill really is bound there) and keep Hybrid answering true for 'visible' and 'revealed', both of which make isDeferredAndHidden false (`!this.revealedDeferred.has(name) && !this.config.getVisibleTools().has(n
Acceptance criterion: packages/core/src/agents/runtime/agent-core.skill-gate.test.ts:341-404 — the it.each([ToolMode.CodeMode, ToolMode.CodeModeOnly] × warm × visibility) matrix already builds a permission-deferred skill and asserts expect(gate(core, declared)).toBe(willHaveSkill), but instantiates only { tools: [ToolNames.EXEC] }. Add { tools: ['*'] } and { tools: [ToolNames.SKILL] } rows to the same matrix: without the fix, mode: ToolMode.CodeMode + visibility: 'hidden' (both warm values) goes R Please prove it by removing the fix and confirming that test goes red.
中文说明
新增的 Hybrid 专用 eager 排除会把被降级的 skill 同时从声明列表和 exec 绑定集合中去掉，但本 PR 改接的 skill 列表判定（willHaveSkillTool() → toolConfigAllowsSkill）仍会通过它的两个非 exec 分支返回 true。因此在 tools.mode: "code_mode" 下，列表判定与执行闸门 canInvokeSkill() 结论相反——这正是该共享判定本应防止的 #12424 分歧。触发条件：hybrid 模式 + 生效的 tools.eager 白名单未包含 skill；子智能体默认 tools: ['*'] 即可命中。后果：createChat 注入 includeAvailableSkillsReminder: true，缓存的 prompt 前缀里出现 <available_skills>，列出该智能体其实没有的工具，模型调用后得到 Tool "skill" not found；SubagentManager 保留 skillsAvailable = true，把 bundled skill 引用作为子智能体无法跟随的指针下发；后台恢复路径重复同样的错误 true。探针实测三行 hybrid 组合分歧、四行 CodeModeOnly 全部一致；打上候选修复后分歧消失且 77 个作者测试仍全绿。另需注意：Direct 模式在未被本 PR 触碰的代码上已有同样分歧，只修 hybrid 会留下一半。
— qwen3.8-max via Qwen Code /review (v0.25.0)
There was a problem hiding this comment.
[Suggestion] R1-1: hasAgentSkillExecBinding() credits the exec gateway from mode + session registry only, so in hybrid code_mode — which this diff newly credits (the call site used to pass this.runtimeContext.getToolMode?.() === ToolMode.CodeModeOnly, agent-core.ts diff hunk @@ -646,7 +660,7 @@) — toolConfigAllowsSkill() answers true for a ToolConfig that merely names exec in tools, ignoring the same config's executionAllowedTools and the nestedExecutionAllowedTools field this PR adds. AgentCore.prepareTools()/canInvokeSkill() do honour those bounds, so the shared predicate can now disagree with the gate it exists to match.
Hybrid code_mode, registry holds exec + skill (not permission-deferred) → hasAgentSkillExecBinding() returns true. Give AgentCore the config { tools: [EXEC, READ_FILE], executionAllowedTools: [READ_FILE] } — the shape the fork path produces when fork_tools omits exec while tools: parentToolNames still contains it (buildForkExecutionAllowlist returns {'read_file', …bridges} with no exec, fork-subagent.ts:109-125; resolveForkExecutionAllowedTools passes it through, fork-subagent.ts:69-86). isToolExecutionAllowed(EXEC) is false (executionAllowedTools is defined and omits exec, so the executionAllowedTools === undefined carve-out at agent-core.ts:1972-1981 does not apply), so exec is never declared and codeModeAllowedToolNames never contains skill → canInvokeSkill(declared) is false (agent-core.ts:1852-1863). Yet toolConfigAllowsSkill returns true via reachesThroughExec, because names.includes(ToolNames.EXEC) reads the configured list, not the executable one. Consequences at the two other consumers: SubagentManager.createAgentHeadless sets skillsAvailable = true (subagent-manager.ts:1187-1195) → the subagent's Config keeps a SkillManager (agentManager = skillsAvailable ? sessionManager : null, subagent-manager.ts:1372) → a bundled reference is delivered as a pointer the agent cannot follow; and background-agent-resume.ts:1019 sets includeAvailableSkillsReminder: true → an <available_skills> block advertising skills the agent cannot load. That is the #12424 disagreement this file's own header says the single predicate prevents, and it is the invariant the author's matrix test asserts (expect(gate(core, declared)).toBe(willHaveSkill), agent-core.skill-gate.test.ts:398-400). The same blindness applies to the new nestedExecutionAllowedTools bound ({ tools: [EXEC, READ_FILE, WRITE_FILE], executionAllowedTools: [], nestedExecutionAllowedTools: [READ_FILE] }, the shape asserted at agent-core.skill-gate
Witness:
`A … willHaveSkillTool: true / canInvokeSkill: false / DISAGREES: true` (unpatched) → `A … toolConfigAllowsSkill: false / canInvokeSkill: false / DISAGREES: false` (patched); positive control `D,E,H … DISAGREES: false` on both arms, so the comparator can report agreement. Fix-constraint arm: `77 passed` with the patch applied, i.e. the pinned `{tools:[EXEC], executionAllowedTools:[EXEC]}` case (agent-core.skill-gate.test.ts:331-340) survives. **Three corrections to the finding's trace** (the verdict stands, the reasoning and two of the three consequences do not): 1. *"`isToolExecutionAllowed(E
Suggested fix: Keep the predicate's inputs to what it can actually see in the ToolConfig: in toolConfigAllowsSkill, drop reachesThroughExec when the config's own bounds exclude the route — e.g. const execExecutable = toolConfig.executionAllowedTools === undefined || toolConfig.executionAllowedTools.includes(ToolNames.EXEC); const nestedAdmitsSkill = toolConfig.nestedExecutionAllowedTools === undefined || toolConfig.nestedExecutionAllowedTools.includes(ToolNames.SKILL); and require both alongside execBindingsAvailable && names.includes(ToolNames.EXEC). (Preserve the documented "answers true where it cannot tell" bias for registry facts; this is a config fact it is handed.)
The fix must not violate this existing fact: A fix keyed on executionAllowedTools must not break the nested-binding exec carve-out that keeps skill reachable when exec itself is executable — forNestedBinding && isCodeModeEnabled(...) && configuredAllowlist.includes(ToolNames.EXEC) && getToolExposure(toolName) === 'code-mode-callable' (packages/core/src/agents/runtime/agent-core.ts:1996-2005), pinned by it.each([…, { tools: [ToolNames.EXEC], executionAllowedTools: [ToolNames.EXEC] }])('opens for an executable nested skill: %j') → `exp
Acceptance criterion: Add the two shapes above to the agent-core.skill-gate.test.ts matrix that asserts expect(gate(core, declared)).toBe(willHaveSkill) (the it.each([ToolMode.CodeMode, ToolMode.CodeModeOnly])…'matches the Skill route in $mode…' block, lines 338-401, or a sibling case): { tools: [EXEC, READ_FILE], executionAllowedTools: [READ_FILE] } under ToolMode.CodeMode must yield willHaveSkillTool() === false === gate(...). Both assertions go red without the fix (today willHaveSkillTool() is true Please prove it by removing the fix and confirming that test goes red.
中文说明
hasAgentSkillExecBinding() 只依据模式 + 会话注册表判定 exec 通道，因此在 hybrid code_mode（本 diff 新增支持的取值）下，toolConfigAllowsSkill() 会对一个仅仅在 tools 里写了 exec 的 ToolConfig 返回 true，而忽略同一个 config 上的 executionAllowedTools 和本 PR 新增的 nestedExecutionAllowedTools。探针实测：{tools:[exec,read_file], executionAllowedTools:[read_file]} 等四种组合下列表判定为 true 而闸门为 false；打上候选修复后四种中有四种转为一致，作者 77 个测试仍全绿（{tools:['*'], executionAllowedTools:[read_file]} 一种仍分歧，说明修复比问题窄）。实际影响限于 prompt 前缀多出一段不可用的 <available_skills> 以及一次被拒绝的调用，执行闸门本身是正确的，且文件头已把这一方向的误差记为有意选择，故定为 Suggestion。
— qwen3.8-max via Qwen Code /review (v0.25.0)
There was a problem hiding this comment.
[Critical] R1-1: [certifies-falsely] [new-surface] Still standing at head f669bd76 — re-confirmed this round. The branch's own code is unchanged since round 1 (the only new commit is a merge of main), so this is a re-report under its original id, not a new finding.
In hybrid tools.mode: "code_mode", this clause drops an eager-demoted skill from both the declarations and the nested binding set, but the skill-listing predicate this same diff rewired (willHaveSkillTool() -> toolConfigAllowsSkill(this.toolConfig, hasAgentSkillExecBinding(...)), agent-core.ts:660-664) still answers true through its two non-exec clauses, so the model-visible listing and the canInvokeSkill() gate disagree.
Failure scenario: tools.mode: "code_mode" plus an active settings.tools.eager allowlist that omits skill, and any subagent — including the default one, because SubagentManager builds tools: configuredToolConfig?.tools ?? ['*'] (subagent-manager.ts:1151). toolConfigAllowsSkill returns inheritsRegistry || names.includes(SKILL) || reachesThroughExec (subagent-plan-tool-policy.ts:159-164), and names.includes('*') short-circuits before the exec term that hasAgentSkillExecBinding correctly answers false for. Both consumers of that true then act on it: createChat gets includeAvailableSkillsReminder: true (agent-core.ts:589-594), and environmentContext.ts:589-596 builds an <available_skills> block into the stable cached prefix listing skills the agent has no route to — despite that file's own comment that announcing uninvokable skills wastes tokens. SubagentManager also keeps skillsAvailable = true (subagent-manager.ts:1187-1189). The author's own invariant expect(gate(core, declared)).toBe(willHaveSkill) (agent-core.skill-gate.test.ts:402) is violated; the matrix misses it because it instantiates only { tools: [ToolNames.EXEC] } (:391).
Witness:
Probe (round 1, re-confirmed round 2 at HEAD f669bd76): {"mode":"code_mode","tools":["*"],"willHaveSkillTool":true,"canInvokeSkill":false,"declaredHasSkill":false} {"mode":"code_mode","tools":["skill"],"willHaveSkillTool":true,"canInvokeSkill":false} {"mode":"code_mode","tools":["exec"],"willHaveSkillTool":false,"canInvokeSkill":false} <- agrees
Suggested fix: Give the shared predicate the same eager-scope fact prepareTools() uses, so it vetoes every route rather than only the exec route: thread a skillEagerHidden input into toolConfigAllowsSkill, computed at each call site as registry.isPermissionDeferred?.(SKILL) === true && registry.isDeferredAndHidden?.(SKILL) === true, and return false when it is set — passing it from all three call sites (agent-core.ts:660-664, subagent-manager.ts:1187-1189, background-agent-resume.ts:1019-1021). Guard it on mode so CodeModeOnly is untouched, since its prepareTools() bypasses the eager filter at the anchored line and skill really is bound there. Fixing only hasAgentSkillExecBinding (as ledger R1-28 proposes) leaves the wildcard and explicit-skill clauses returning true.
The fix must not violate this existing fact: agent-core.skill-gate.test.ts:398-400 — expect(willHaveSkill).toBe(mode === ToolMode.CodeModeOnly || visibility === 'visible');. CodeModeOnly must keep answering true for visibility: 'hidden' (its prepareTools() bypasses the eager filter at agent-core.ts:742, so skill really is bound there), and Hybrid must keep answering true for 'visible' and 'revealed', both of which make isDeferredAndHidden false (tool-registry.ts:1352-1361).
Acceptance criterion: agent-core.skill-gate.test.ts:341-403 — the it.each([ToolMode.CodeMode, ToolMode.CodeModeOnly] x warm x visibility) matrix already builds a permission-deferred skill and asserts expect(gate(core, declared)).toBe(willHaveSkill) at :402, but instantiates only { tools: [ToolNames.EXEC] } at :391. Add { tools: ['*'] } and { tools: [ToolNames.SKILL] } rows beside it. Please prove it by removing the fix and confirming that mode: ToolMode.CodeMode + visibility: 'hidden' goes red on both warm values, and that all rows agree once the fix is in place.
中文说明
在混合模式 tools.mode: "code_mode" 下，本行新增的 eager 降级过滤会把被降级的 skill 同时从声明列表和嵌套绑定集合中移除；但本 PR 同时改接的 skill 列表判定 willHaveSkillTool()（改为 toolConfigAllowsSkill(this.toolConfig, hasAgentSkillExecBinding(...))，agent-core.ts:660-664）仍会经由它的两个非 exec 分支返回 true。于是「模型可见的列表」与「实际调用闸门 canInvokeSkill()」结论相反。
触发条件：tools.mode: "code_mode" 且生效的 settings.tools.eager 白名单未包含 skill；任意子智能体即可命中，默认子智能体也会，因为 SubagentManager 构造 tools: configuredToolConfig?.tools ?? ['*']（subagent-manager.ts:1151）。names.includes('*') 会在 exec 分支之前短路，而 exec 分支正是 hasAgentSkillExecBinding 正确返回 false 的地方。
后果：createChat 得到 includeAvailableSkillsReminder: true；environmentContext.ts:589-596 会把 <available_skills> 写入稳定缓存前缀，列出该智能体其实无法调用的技能（该文件自己的注释就写着「宣告模型无法调用的技能是浪费 token」）；SubagentManager 同时保留 skillsAvailable = true。作者自己的不变式 expect(gate(core, declared)).toBe(willHaveSkill)（agent-core.skill-gate.test.ts:402）被违反，而该矩阵没有发现，是因为它只构造了 { tools: [ToolNames.EXEC] }（:391）。
修复约束：不得违反 agent-core.skill-gate.test.ts:398-400 的既有断言 —— CodeModeOnly 在 visibility: 'hidden' 时必须仍返回 true（它的 prepareTools() 在 agent-core.ts:742 绕过 eager 过滤，skill 确实已绑定），Hybrid 在 'visible' / 'revealed' 时也必须仍返回 true。
验收标准：在 agent-core.skill-gate.test.ts:391 的矩阵中补上 { tools: ['*'] } 与 { tools: [ToolNames.SKILL] } 两行；去掉修复后，mode: ToolMode.CodeMode + visibility: 'hidden' 应在两个 warm 取值下都变红，打上修复后全部一致。
— qwen3.8-max via Qwen Code /review (v0.25.0)
| | `exec` | CodeMode and CodeModeOnly | No | | ||
| | Direct control | Yes | No | | ||
| | Ordinary registered tool | CodeMode only | Yes | | ||
| | Hidden bridge (`tool_search`, `tool_call`) | Existing behavior outside strict mode | No | |
There was a problem hiding this comment.
[Suggestion] R1-2: N06: The rewritten exposure row conditions the table's only statement about bridge exposure on "strict mode" — a term that exists nowhere in the mode vocabulary, the code, or any other English document — and it groups two tools the code exposes differently.
ToolMode has exactly three values, direct / code_mode / code_mode_only (packages/core/src/tools/code-mode.ts:22-26); no setting, code path, or EN doc defines a "strict mode", and a repo-wide grep over docs/ finds the phrase only on this line (the ZH twin's 严格模式 is glossed only in a different file, docs/design/code-mode.zh-CN.md:104: "tools.mode: "code_mode_only" 选择严格模式"). A user on tools.mode: "code_mode" therefore cannot tell from this table whether hybrid counts as strict, i.e. whether tool_search/tool_call are declared for their session — and the shipped rule splits the pair the row keeps together: tool_search is a top-level direct control under CodeModeOnly (DIRECT_ONLY_TOOLS, code-mode.ts:39-51, admitted by the exposure === 'direct-only' filter in getCodeModeFunctionDeclarations, tool-registry.ts:1126-1131), while tool_call is hidden in every mode (const HIDDEN_TOOLS = new Set<string>(['tool_call']);, code-mode.ts:38). The same file's unchanged Status paragraph states that split correctly ("tool_search is now a top-level direct control … tool_call stays hidden", code-mode-only.md:12-16), so the table and the Status paragraph disagree about whether the two bridge tools share a rule. Every other row in this table names modes explicitly ("CodeMode and CodeModeOnly", "CodeMode only"), making this the one cell a reader cannot resolve to a tools.mode value.
Witness:
`Source: [probe]` sweep — `grep -rn 'strict mode|strict \`|严格模式' **/*.md` over the whole worktree: as a **tool-mode** term the phrase occurs exactly **once** in EN docs (this row) and once in its ZH twin; every other hit is an unrelated domain (TypeScript strict mode, Ajv strict, screen-reader strict, `parseLastEventId`). `ToolMode` has three values (code-mode.ts:22-26) and no setting or code path names one "strict". Mitigation I found and the finder did not weigh: the sibling EN doc this same PR adds glosses it adjectivally — `code-mode.md:22` "the strict `CodeModeOnly` exposure policy" — and
Suggested fix: Replace the cell with mode names and split the pair, e.g. two rows — | tool_search | CodeModeOnly top level; existing Direct/CodeMode behavior | No | and | tool_call | Hidden in every mode | No | — and mirror the change in docs/design/code-mode-only.zh-CN.md:54.
The fix must not violate this existing fact: const HIDDEN_TOOLS = new Set<string>(['tool_call']); and const DIRECT_ONLY_TOOLS = new Set<string>([ToolNames.TOOL_SEARCH, …]) — packages/core/src/tools/code-mode.ts:38-51; the rewritten rows must keep tool_call hidden in every mode and tool_search top-level under CodeModeOnly, matching docs/design/code-mode-only.md:12-16.
Acceptance criterion: N/A (documentation prose; no guard, branch or behaviour to pin). Please prove it by removing the fix and confirming that test goes red.
中文说明
改写后的 exposure 表格行把该表唯一一句关于 bridge 暴露的说明限定在 "strict mode" 下——而这个词在模式词表、代码和其余文档中都不存在（真实枚举是 direct / code_mode / code_mode_only）。读者无法把它对应到任何可配置取值。
— qwen3.8-max via Qwen Code /review (v0.25.0)
There was a problem hiding this comment.
[Suggestion] R1-2: The new Chinese translation states the opposite of the English original (docs/design/code-mode-only.md:144-148) on Code Mode shell concurrency: EN says "Code Mode Bash calls bypass the read-only command classifier, with the model responsible for keeping dependent calls sequential. Other tools retain their existing concurrency classification"; ZH says the batch is "由现有的只读并发分类器处理" and drops the Bash carve-out entirely.
A reader working from the ZH design concludes that shell calls submitted together inside one exec (await Promise.all([...])) are serialized unless the read-only command checker classifies them as read-only. The shipped behavior is the reverse: packages/core/src/core/coreToolScheduler.ts:1557-1563 returns true for source === 'code_mode' + ToolNames.SHELL + Kind.Execute before isShellCommandReadOnly is ever consulted, so two dependent mutating Bash calls in one batch run in parallel. Someone reasoning about ordering safety, or reviewing a nested-call bug from the ZH doc, starts from a false premise; the ZH doc also contradicts docs/design/code-mode-concurrency.zh-CN.md:20-21 ("Code Mode 中的 Bash 调用跳过只读命令判定"), so the two Chinese designs disagree with each other.
Witness:
driving the real `isToolCallConcurrencySafe` (coreToolScheduler.ts:1546-1564) with the same args, varying only `source`: ``` mutating shell (npm install && git push origin main), source=code_mode -> true mutating shell (npm install && git push origin main), source=model -> false read-only shell (git log --oneline -5), source=code_mode -> true read-only shell (git log --oneline -5), source=model -> true ``` i.e. code-mode Bash is classified concurrency-safe *before* `isShellCommandReadOnly` is consulted — the EN sentence is right, the new ZH sentence is the inverse. ---
Suggested fix: Translate the two missing EN sentences into the ZH paragraph, e.g. replace "…Promise.all 调用会进入同一个 batch，由现有的只读并发分类器处理。" with "…Promise.all 调用会进入同一个 batch。Code Mode 中的 Bash 调用跳过只读命令判定，由模型负责让存在依赖的调用保持串行；其他工具沿用现有的并发分类。"
The fix must not violate this existing fact: packages/core/src/core/coreToolScheduler.ts:1557 — // Code Mode lets the model batch independent shell calls explicitly. guarding if (source === 'code_mode' && canonicalName === ToolNames.SHELL && kind === Kind.Execute) { return true; }; the corrected wording must not reassert read-only classification for code-mode Bash, which docs/design/code-mode-concurrency.zh-CN.md:20 also forbids.
Acceptance criterion: N/A (documentation text; no guard, branch, or behavior to pin). Please prove it by removing the fix and confirming that test goes red.
中文说明
新增中文翻译在 Code Mode shell 并发这一点上与英文原文相反：英文说 "Code Mode Bash calls bypass the read-only command classifier"，中文写成"由现有的只读并发分类器处理"，恰好把豁免说成了适用。同仓库的 code-mode-concurrency.zh-CN.md:21 与英文一致，可证这是翻译错误。实测 isToolCallConcurrencySafe 对同样的 mutating shell 命令在 source=code_mode 时返回 true、source=model 时返回 false。
— qwen3.8-max via Qwen Code /review (v0.25.0)
| property, an exact canonical match wins over rewritten names. If neither is an | ||
| exact match, the lexicographically first name wins. The description names the |
There was a problem hiding this comment.
[Suggestion] R1-3: N01: The sentence this diff rewrote (pre-diff: "one warning names the omitted collision") claims the exec description names the dropped binding, but in CodeModeOnly with tool_search registered — the shipped default this same diff's new blockquote declares ("Only discovers schemas through top-level tool_search") — buildExecDescription suppresses the collision block entirely. The pre-diff wording was the accurate one; the new one is false for the mode the document is about.
A registry holding mcp__srv__get-data and mcp__srv__get_data normalizes both to mcp__srv__get_data, so planCodeModeBindings keeps the exact match and records { jsName, kept: 'mcp__srv__get_data', omitted: 'mcp__srv__get-data' } (packages/core/src/tools/code-mode.ts:111-124). In a tools.mode: "code_mode_only" session, getCodeModeFunctionDeclarations computes searchAvailable = !!this.getTool(TOOL_SEARCH) && (!allowedNames || allowedNames.has(TOOL_SEARCH)) → true (packages/core/src/tools/tool-registry.ts:1121-1123, passed at :1135), and buildExecDescription sets const searchAvailable = codeModeOnly && (options.searchAvailable ?? false) → true (code-mode.ts:253), so collisionText = (searchAvailable ? [] : plan.collisions) → [] (code-mode.ts:264) and the description ends with no Name collisions: block (code-mode.ts:325). The repo's own test pins that suppression: packages/core/src/code-mode/code-mode.test.ts:723-739 registers exec + tool_search under code_mode_only with hidden-tool/hidden_tool and asserts expect(description).not.toContain('hidden-tool') and not.toContain('hidden_tool'). The only surface that names the drop is debugLogger.warn inside warnCodeModeCollisions (tool-registry.ts:1158-1166) — debug-gated, not model-facing. Concrete cost: the model is never told the binding was dropped in the default Only configuration, and the maintainer triaging "tool X is unreachable through exec / exec called the wrong MCP tool" reads this document as the normative collision contract, goes looking for a missing line in the description generator, and finds code that is behaving exactly as written. The same false sentence ships in the new Chinese twin ("描述会指出被省略的冲突项。", code-mode-only.zh-CN.md:73-74).
Witness:
`Source: [probe]` — CodeModeOnly registry with `exec` + `tool_search` + colliding `get-data`/`get_data` (non-deferred, so deferral filtering cannot explain the absence): `N01-only {"collisions":[{"jsName":"get_data","kept":"get_data","omitted":"get-data"}],"descriptionHasCollisionsBlock":false,"descriptionNamesOmittedTool":false,"declaredNames":["exec","tool_search"]}` Positive controls (the probe *can* see the block): `N01-only-nosearch {"descriptionHasCollisionsBlock":true,"descriptionNamesOmittedTool":true}` · `N01-hybrid {"descriptionHasCollisionsBlock":true,"descriptionNamesOmittedTool":t
Suggested fix: Scope the claim to the configuration where it holds and name the other surface, e.g. "When search is unavailable in the current scope, the description names the omitted collision; when tool_search is available the collision block is left out of the description and the drop is logged once through the debug logger." Apply the identical correction to docs/design/code-mode-only.zh-CN.md:73-74.
The fix must not violate this existing fact: const collisionText = (searchAvailable ? [] : plan.collisions) — packages/core/src/tools/code-mode.ts:264, with const searchAvailable = codeModeOnly && (options.searchAvailable ?? false); at :253. The corrected sentence must keep the hybrid case true: decorateCodeModeDeclarations passes codeModeOnly: false (packages/core/src/tools/tool-registry.ts:1087-1092), so hybrid descriptions always emit the block.
Acceptance criterion: N/A (documentation prose). The behaviour the corrected sentence must match is already pinned from both sides: packages/core/src/code-mode/code-mode.test.ts:723-739 (CodeModeOnly + tool_search → neither collision name appears) and :563-591 'describes normalized-name collisions on the hybrid surface' (hybrid description contains '- read-file is omitted because it collides with read_file as tools.read_file.'). Please prove it by removing the fix and confirming that test goes red.
中文说明
本次改写后的句子声称 exec 的 description 会点出被丢弃的 binding，但在 CodeModeOnly 下被丢弃的 binding 恰恰不会进入 description；改写前的措辞（"one warning names the omitted collision"）才是准确的——warnCodeModeCollisions 在所有模式下都会触发。中英文两份设计文档同句同错。
— qwen3.8-max via Qwen Code /review (v0.25.0)
There was a problem hiding this comment.
[Suggestion] R1-3: The new ZH version's 状态 section omits the English Status paragraph at docs/design/code-mode-only.md:12-16 ("Partly superseded by Lazy Code Mode: tool_search is now a top-level direct control, and exec omits deferred tool signatures while search is available. The exposure table and the deferred-schema paragraph below describe this MVP; tool_call stays hidden."), and the ZH Sandbox paragraph at line 103 likewise drops EN's clause "whose own scheduler/ACP timeouts remain authoritative" (docs/design/code-mode-only.md:119-121).
The ZH reader never learns that tool_call remains hidden under current behavior, nor that the 暴露策略 table and the deferred-schema paragraph below it describe a superseded MVP rather than shipped behavior — the top blockquote only supersedes "隐藏 tool_search / 始终完整 schema", not the table as a whole. Concretely the ZH table row "隐藏 bridge（tool_search、tool_call）| 严格模式之外沿用既有行为" plus ZH line 76 "CodeModeOnly 会隐藏 tool_search" read as current policy, which the lazy-loading design replaced. At ZH:103 the dropped clause leaves the reader with no statement of which timeout governs while the guest CPU budget and watchdog are paused on a host tool. This violates the repo's own rule in docs/design/README.md: "neither version omits decisions, limitations, acceptance criteria, or follow-up work" and "Do not leave one version with an earlier requirement … that the other version has already answered."
Witness:
`grep -n "Partly superseded|stays hidden|scheduler/ACP timeouts remain authoritative" code-mode-only.md code-mode-only.zh-CN.md` → **3 hits, all in the EN file** (`code-mode-only.md:12`, `:15`, `:120`), zero in the ZH file; and `sed -n '7,11p' code-mode-only.zh-CN.md` → ``` ## 状态 已为 [#10377](…) 实现。 该功能为可选功能，默认关闭。 ``` So the ZH 状态 carries neither the "Partly superseded by Lazy Code Mode … The exposure table and the deferred-schema paragraph below describe this MVP; `tool_call` stays hidden" scope note (EN 12-15) nor, at ZH:103-104, EN's "whose own scheduler/ACP timeouts remain authoritative" (E
Suggested fix: Translate the EN Status paragraph into 状态 (adding the tool_call 仍隐藏 statement and the scope note that the exposure table and deferred-schema paragraph describe the MVP), and add "已注册 host 工具自身的 scheduler/ACP timeout 仍然生效" to the ZH watchdog sentence at line 103.
The fix must not violate this existing fact: docs/design/code-mode-only.md:12-16 is the source text the translation must carry, and docs/design/README.md ("Keep section order and heading levels aligned") requires it land in the existing 状态 section rather than as a new heading.
Acceptance criterion: N/A (documentation text; no guard, branch, or behavior to pin). Please prove it by removing the fix and confirming that test goes red.
中文说明
新增中文版的「状态」小节漏掉了英文 Status 段落（docs/design/code-mode-only.md:12-16）中"Partly superseded by Lazy Code Mode"这一段，导致中英两版结构不同步，违反 AGENTS.md 对双语文档"完整且同步"的要求。
— qwen3.8-max via Qwen Code /review (v0.25.0)
| 已为 [#10377](https://github.com/QwenLM/qwen-code/issues/10377) 实现。 | ||
| 该功能为可选功能，默认关闭。 |
There was a problem hiding this comment.
[Suggestion] R1-3: The new ZH version's 状态 section omits the English Status paragraph at docs/design/code-mode-only.md:12-16 ("Partly superseded by Lazy Code Mode: tool_search is now a top-level direct control, and exec omits deferred tool signatures while search is available. The exposure table and the deferred-schema paragraph below describe this MVP; tool_call stays hidden."), and the ZH Sandbox paragraph at line 103 likewise drops EN's clause "whose own scheduler/ACP timeouts remain authoritative" (docs/design/code-mode-only.md:119-121).
The ZH reader never learns that tool_call remains hidden under current behavior, nor that the 暴露策略 table and the deferred-schema paragraph below it describe a superseded MVP rather than shipped behavior — the top blockquote only supersedes "隐藏 tool_search / 始终完整 schema", not the table as a whole. Concretely the ZH table row "隐藏 bridge（tool_search、tool_call）| 严格模式之外沿用既有行为" plus ZH line 76 "CodeModeOnly 会隐藏 tool_search" read as current policy, which the lazy-loading design replaced. At ZH:103 the dropped clause leaves the reader with no statement of which timeout governs while the guest CPU budget and watchdog are paused on a host tool. This violates the repo's own rule in docs/design/README.md: "neither version omits decisions, limitations, acceptance criteria, or follow-up work" and "Do not leave one version with an earlier requirement … that the other version has already answered."
Witness:
`grep -n "Partly superseded|stays hidden|scheduler/ACP timeouts remain authoritative" code-mode-only.md code-mode-only.zh-CN.md` → **3 hits, all in the EN file** (`code-mode-only.md:12`, `:15`, `:120`), zero in the ZH file; and `sed -n '7,11p' code-mode-only.zh-CN.md` → ``` ## 状态 已为 [#10377](…) 实现。 该功能为可选功能，默认关闭。 ``` So the ZH 状态 carries neither the "Partly superseded by Lazy Code Mode … The exposure table and the deferred-schema paragraph below describe this MVP; `tool_call` stays hidden" scope note (EN 12-15) nor, at ZH:103-104, EN's "whose own scheduler/ACP timeouts remain authoritative" (E
Suggested fix: Translate the EN Status paragraph into 状态 (adding the tool_call 仍隐藏 statement and the scope note that the exposure table and deferred-schema paragraph describe the MVP), and add "已注册 host 工具自身的 scheduler/ACP timeout 仍然生效" to the ZH watchdog sentence at line 103.
The fix must not violate this existing fact: docs/design/code-mode-only.md:12-16 is the source text the translation must carry, and docs/design/README.md ("Keep section order and heading levels aligned") requires it land in the existing 状态 section rather than as a new heading.
Acceptance criterion: N/A (documentation text; no guard, branch, or behavior to pin). Please prove it by removing the fix and confirming that test goes red.
中文说明
新增中文版的「状态」小节漏掉了英文 Status 段落（docs/design/code-mode-only.md:12-16）中"Partly superseded by Lazy Code Mode"这一段，导致中英两版结构不同步，违反 AGENTS.md 对双语文档"完整且同步"的要求。
— qwen3.8-max via Qwen Code /review (v0.25.0)
| 并发链路。guest 的连续 await 会生成连续 batch；`Promise.all` 调用会进入同一个 | ||
| batch，由现有的只读并发分类器处理。嵌套 request id 包含父 id，并携带 |
There was a problem hiding this comment.
[Suggestion] R1-2: The new Chinese translation states the opposite of the English original (docs/design/code-mode-only.md:144-148) on Code Mode shell concurrency: EN says "Code Mode Bash calls bypass the read-only command classifier, with the model responsible for keeping dependent calls sequential. Other tools retain their existing concurrency classification"; ZH says the batch is "由现有的只读并发分类器处理" and drops the Bash carve-out entirely.
A reader working from the ZH design concludes that shell calls submitted together inside one exec (await Promise.all([...])) are serialized unless the read-only command checker classifies them as read-only. The shipped behavior is the reverse: packages/core/src/core/coreToolScheduler.ts:1557-1563 returns true for source === 'code_mode' + ToolNames.SHELL + Kind.Execute before isShellCommandReadOnly is ever consulted, so two dependent mutating Bash calls in one batch run in parallel. Someone reasoning about ordering safety, or reviewing a nested-call bug from the ZH doc, starts from a false premise; the ZH doc also contradicts docs/design/code-mode-concurrency.zh-CN.md:20-21 ("Code Mode 中的 Bash 调用跳过只读命令判定"), so the two Chinese designs disagree with each other.
Witness:
driving the real `isToolCallConcurrencySafe` (coreToolScheduler.ts:1546-1564) with the same args, varying only `source`: ``` mutating shell (npm install && git push origin main), source=code_mode -> true mutating shell (npm install && git push origin main), source=model -> false read-only shell (git log --oneline -5), source=code_mode -> true read-only shell (git log --oneline -5), source=model -> true ``` i.e. code-mode Bash is classified concurrency-safe *before* `isShellCommandReadOnly` is consulted — the EN sentence is right, the new ZH sentence is the inverse. ---
Suggested fix: Translate the two missing EN sentences into the ZH paragraph, e.g. replace "…Promise.all 调用会进入同一个 batch，由现有的只读并发分类器处理。" with "…Promise.all 调用会进入同一个 batch。Code Mode 中的 Bash 调用跳过只读命令判定，由模型负责让存在依赖的调用保持串行；其他工具沿用现有的并发分类。"
The fix must not violate this existing fact: packages/core/src/core/coreToolScheduler.ts:1557 — // Code Mode lets the model batch independent shell calls explicitly. guarding if (source === 'code_mode' && canonicalName === ToolNames.SHELL && kind === Kind.Execute) { return true; }; the corrected wording must not reassert read-only classification for code-mode Bash, which docs/design/code-mode-concurrency.zh-CN.md:20 also forbids.
Acceptance criterion: N/A (documentation text; no guard, branch, or behavior to pin). Please prove it by removing the fix and confirming that test goes red.
中文说明
新增中文翻译在 Code Mode shell 并发这一点上与英文原文相反：英文说 "Code Mode Bash calls bypass the read-only command classifier"，中文写成"由现有的只读并发分类器处理"，恰好把豁免说成了适用。同仓库的 code-mode-concurrency.zh-CN.md:21 与英文一致，可证这是翻译错误。实测 isToolCallConcurrencySafe 对同样的 mutating shell 命令在 source=code_mode 时返回 true、source=model 时返回 false。
— qwen3.8-max via Qwen Code /review (v0.25.0)
| ), | ||
| ), | ||
| ])( | ||
| 'uses only exposed tools for image guidance: $declared, code mode $codeModeOnly', |
There was a problem hiding this comment.
[Suggestion] R1-30: The it.each title still interpolates only $declared and $codeModeOnly, so the 16 cases this diff adds — whose discriminators are toolMode and allowedNames — collapse onto titles that are already taken by the pre-existing rows.
Five rows now share the title uses only exposed tools for image guidance: read_file,exec, code mode false: the added hybrid-code-mode row that expects ' If details are too small, call tools.zoom_image …', the narrowed-agent row (allowedNames: ['read_file']) that expects '', and the three flatMap rows over declared: ['read_file','exec'] × [[], ['read_file'], ['read_file','zoom_image']] that also expect ''. Four more collide on read_file,exec,tool_search,tool_call, code mode false (one expects the tool_search/tool_call bridge hint, three expect ''). When one of these fails, the reporter prints an ambiguous name — you cannot tell whether the regression is in the reachability computation or in the new ambient-allowlist gate without counting array positions, which is exactly the distinction this diff turns on.
Witness:
not run — probe/mutation via `review scratch-tree` and `base-tree`: unavailable in this environment (repo-local git includeIf resolves to a missing credentials file, so scratch-tree refuses to create a tree; the base build timed out), so the verdict rests on a line-by-line source trace plus repo-wide greps
Suggested fix: Add the new discriminators to the title, e.g. 'uses only exposed tools for image guidance: $declared, code mode $codeModeOnly, tool mode $toolMode, allowed $allowedNames'.
The fix must not violate this existing fact: toolMode?: ToolMode; (fileUtils.test.ts:1187) and allowedNames?: string[]; (fileUtils.test.ts:1191) are optional in the it.each<{…}> row type, and the five pre-existing rows starting at fileUtils.test.ts:1194 supply neither — interpolating $toolMode/$allowedNames renders those rows with no value unless the fix also gives every row both keys.
Acceptance criterion: N/A — a title-only change pins no behaviour; the existing it.each body (asserting text: `Image overview: 20x10; oriented source: 20x10.${hint}` at fileUtils.test.ts:1332-1335) already covers the behaviour. Please prove it by removing the fix and confirming that test goes red.
中文说明
it.each 的标题仍然只插值 $declared 和 $codeModeOnly，因此本 diff 新增的 16 个用例（其区分维度是 toolMode 和 allowedNames）会塌缩到与既有用例相同的标题上，失败时无法从名字定位是哪一例。
— qwen3.8-max via Qwen Code /review (v0.25.0)
| const ambientAllowedNames = getCurrentCodeModeAllowedNames(); | ||
| const zoomAvailable = | ||
| ambientAllowedNames === undefined && |
There was a problem hiding this comment.
[Suggestion] R1-31: The new guard treats any defined ambient allowlist as "route unknown", but the ambient store is produced by AgentCore.prepareTools() as exactly the agent's exec binding set, so the nested route is the one route it can answer — and it is answered with silence.
Producer/consumer trace. AgentCore.prepareTools() sets this.codeModeAllowedToolNames to allowedNames.filter(name => getToolExposure(name) === 'code-mode-callable') (agent-core.ts:759-762); processFunctionCalls now stamps it on every request, not just exec/tool_search (agent-core.ts:2684-2691, changed by this diff); CoreToolScheduler enters the ALS with it around every execution (coreToolScheduler.ts:5868, 5914), and runWithCodeModeAllowedNames(undefined, …) deliberately does not clear the store, so a nested read_file dispatched from inside exec inherits the parent agent's binding set. Result: a hybrid or CodeModeOnly subagent whose codeModeAllowedToolNames contains zoom_image reads an image, ambientAllowedNames !== undefined, zoomAvailable is false, and zoomHint is '' — the model is never told tools.zoom_image exists even though the very value in scope proves this agent's exec binds it. Pre-diff, a CodeModeOnly agent got that hint (from the session plan); the PR trades a sometimes-wrong hint for no hint at all. The stated reason in the added comment — "An agent's target allowlist does not identify its declared direct, bridge, or exec routes" — is true of the direct and bridge routes (declaredTools comes from the registry's session-wide getFunctionDeclarations(), not the agent's filtered surface) but false of the exec route.
Witness:
not run — probe/mutation via `review scratch-tree` and `base-tree`: unavailable in this environment (repo-local git includeIf resolves to a missing credentials file, so scratch-tree refuses to create a tree; the base build timed out), so the verdict rests on a line-by-line source trace plus repo-wide greps
Suggested fix: Recover the nested route instead of blanket-suppressing: when ambientAllowedNames is defined, set zoomAvailable from useNestedZoom && ambientAllowedNames.includes('zoom_image') (leave the direct and bridge routes suppressed, since declaredTools is a session fact inside an agent).
The fix must not violate this existing fact: The sibling rows must keep passing unchanged: allowedNames: ['read_file'] with declared: ['read_file','exec'] asserts hint: '' (fileUtils.test.ts:1261-1270, "the calling agent's narrowed plan does not [bind it]"), and declared: ['read_file','tool_search','tool_call'] with allowedNames: ['read_file'] asserts hint: '' (fileUtils.test.ts:1271-1283). The fix must therefore key on membership of zoom_image in the ambient set, not merely on the set being defined, and must not resurrec
Acceptance criterion: packages/core/src/utils/fileUtils.test.ts:1285-1298 — the flatMap row declared: ['read_file','exec'] × allowedNames: ['read_file','zoom_image'] currently asserts hint: ''; it must be flipped to ' If details are too small, call tools.zoom_image with coordinates normalized from 0 to 1000.'. Removing the membership check returns it to '' and the row goes red. Please prove it by removing the fix and confirming that test goes red.
中文说明
新增的这道判断把任何已定义的 ambient 白名单都当作"路由未知"，但该 ambient store 恰恰是 AgentCore.prepareTools() 产出的、该智能体的 exec 绑定集合，也就是唯一能回答的那条路由——而它被回答以沉默。结果是 ambient 白名单里明明含有 zoom_image 的 hybrid/CodeModeOnly 子智能体读取图片后拿不到任何提示；改动前 CodeModeOnly 智能体是能拿到该提示的。
— qwen3.8-max via Qwen Code /review (v0.25.0)
There was a problem hiding this comment.
[Suggestion] R1-31: The new guard treats any defined ambient allowlist as "route unknown", but the ambient store is produced by AgentCore.prepareTools() as exactly the agent's exec binding set, so the nested route is the one route it can answer — and it is answered with silence.
Producer/consumer trace. AgentCore.prepareTools() sets this.codeModeAllowedToolNames to allowedNames.filter(name => getToolExposure(name) === 'code-mode-callable') (agent-core.ts:759-762); processFunctionCalls now stamps it on every request, not just exec/tool_search (agent-core.ts:2684-2691, changed by this diff); CoreToolScheduler enters the ALS with it around every execution (coreToolScheduler.ts:5868, 5914), and runWithCodeModeAllowedNames(undefined, …) deliberately does not clear the store, so a nested read_file dispatched from inside exec inherits the parent agent's binding set. Result: a hybrid or CodeModeOnly subagent whose codeModeAllowedToolNames contains zoom_image reads an image, ambientAllowedNames !== undefined, zoomAvailable is false, and zoomHint is '' — the model is never told tools.zoom_image exists even though the very value in scope proves this agent's exec binds it. Pre-diff, a CodeModeOnly agent got that hint (from the session plan); the PR trades a sometimes-wrong hint for no hint at all. The stated reason in the added comment — "An agent's target allowlist does not identify its declared direct, bridge, or exec routes" — is true of the direct and bridge routes (declaredTools comes from the registry's session-wide getFunctionDeclarations(), not the agent's filtered surface) but false of the exec route.
Witness:
not run — probe/mutation via `review scratch-tree` and `base-tree`: unavailable in this environment (repo-local git includeIf resolves to a missing credentials file, so scratch-tree refuses to create a tree; the base build timed out), so the verdict rests on a line-by-line source trace plus repo-wide greps
Suggested fix: Recover the nested route instead of blanket-suppressing: when ambientAllowedNames is defined, set zoomAvailable from useNestedZoom && ambientAllowedNames.includes('zoom_image') (leave the direct and bridge routes suppressed, since declaredTools is a session fact inside an agent).
The fix must not violate this existing fact: The sibling rows must keep passing unchanged: allowedNames: ['read_file'] with declared: ['read_file','exec'] asserts hint: '' (fileUtils.test.ts:1261-1270, "the calling agent's narrowed plan does not [bind it]"), and declared: ['read_file','tool_search','tool_call'] with allowedNames: ['read_file'] asserts hint: '' (fileUtils.test.ts:1271-1283). The fix must therefore key on membership of zoom_image in the ambient set, not merely on the set being defined, and must not resurrec
Acceptance criterion: packages/core/src/utils/fileUtils.test.ts:1285-1298 — the flatMap row declared: ['read_file','exec'] × allowedNames: ['read_file','zoom_image'] currently asserts hint: ''; it must be flipped to ' If details are too small, call tools.zoom_image with coordinates normalized from 0 to 1000.'. Removing the membership check returns it to '' and the row goes red. Please prove it by removing the fix and confirming that test goes red.
中文说明
新增的这道判断把任何已定义的 ambient 白名单都当作"路由未知"，但该 ambient store 恰恰是 AgentCore.prepareTools() 产出的、该智能体的 exec 绑定集合，也就是唯一能回答的那条路由——而它被回答以沉默。结果是 ambient 白名单里明明含有 zoom_image 的 hybrid/CodeModeOnly 子智能体读取图片后拿不到任何提示；改动前 CodeModeOnly 智能体是能拿到该提示的。
— qwen3.8-max via Qwen Code /review (v0.25.0)
| // An agent's target allowlist does not identify its declared | ||
| // direct, bridge, or exec routes. Do not advertise session routes. |
There was a problem hiding this comment.
[Suggestion] R1-47: N32: The reachability rule this hunk states is enforced only on the image-overview branch; the omni-delivery branch that returns 48 lines earlier inside the same case 'image' still advertises zoom_image unconditionally, so on an omni-policy session the new gate never executes for images at all.
tools.mode: "code_mode_only" (or "code_mode") plus a subagent whose surface withholds zoom_image, on a model where modalities.image is true and config.isOmniEnabled?.() / omni.isOmniDeliveryActive(config) hold — all model/policy facts, independent of tool mode (fileUtils.ts:1389-1410). processSingleFileContent takes if (omniModule) { return await omniModule.readMediaViaOmniDelivery({…}) } at fileUtils.ts:1638-1650 and never reaches the changed code. packages/core/src/omni/index.ts:1189-1196 then pushes Image <name>: full resolution WxH px. Use zoom_image for a closer look at details. (or the degradation variant Use zoom_image to inspect details — it reads the original file.) with no registry, tool-mode, or ambient-allowlist check. The agent emits a zoom_image call and AgentCore.processFunctionCalls rejects it with Tool "zoom_image" not found. Tools must use the exact names provided. (agent-core.ts:2201-2206) — one wasted turn, which is exactly the cost the deleted #12271 comment and this new gate exist to prevent. The identical agent on a non-omni model gets '' and does not waste the turn, so after this diff whether the invariant holds depends on the session's model policy rather than on the agent's surface.
Witness:
not run — probe/mutation via `review scratch-tree` and `base-tree`: unavailable in this environment (repo-local git includeIf resolves to a missing credentials file, so scratch-tree refuses to create a tree; the base build timed out), so the verdict rests on a line-by-line source trace plus repo-wide greps
Suggested fix: Hoist the route resolution (registry facts + getCurrentCodeModeAllowedNames()) into one helper that returns the hint string, call it before the if (omniModule) early return, and pass the result into readMediaViaOmniDelivery so its two text variants name zoom_image / tools.zoom_image / nothing under the same rule the overview branch now uses.
The fix must not violate this existing fact: packages/core/src/omni/index.ts:1183-1187 — "calling them 'full resolution' would contradict the disclosure pushed right below and steer the model away from zoom_image, the exact remedy for degradation-stripped detail (it reads the original from disk)". Suppression must key on the route being genuinely absent for the calling agent, not on the degradation/disclosure path, or the disclosure and the hint start contradicting each other again.
Acceptance criterion: packages/core/src/omni/index.test.ts:410-464 (adds a resolution + zoom_image hint part for images) currently pins the unconditional text. Add a sibling case that drives the same delivery with the route absent — e.g. wrap in runWithCodeModeAllowedNames(['read_file'], …), or a registry double whose getFunctionDeclarations() omits zoom_image — and assert the pushed part's text does not contain zoom_image. It is red while the omni branch ignores reachability. Please prove it by removing the fix and confirming that test goes red.
中文说明
这段 hunk 所陈述的可达性规则只在 image-overview 分支上被执行；同一个 case 'image' 内提前 48 行 return 的 omni-delivery 分支并不受它约束，形成同族分支的不对称。
— qwen3.8-max via Qwen Code /review (v0.25.0)
| | `abortController` | `AbortController` | - | Controller to cancel the query session. Call `abortController.abort()` to terminate the session and cleanup resources. | | ||
| | `debug` | `boolean` | `false` | Enable debug mode for verbose logging from the CLI process. | | ||
| | `maxSessionTurns` | `number` | `-1` (unlimited) | Maximum number of conversation turns before the session automatically terminates. Must be an integer. A turn consists of a user message and an assistant response. | | ||
| | `coreTools` | `string[]` | - | Uses the legacy `coreTools` / CLI `--core-tools` allowlist semantics. If specified, only matching core tools are registered for the session. This is the only allowlist-style option that restricts built-in tool registration; a whole-tool `permissions.deny` / `excludeTools` rule (and `tools.disabled` in settings.json) also removes a tool from the registry. `permissions.allow` in settings.json is pure auto-approval and never removes, demotes, or hides a tool (#10075). To keep a tool's schema out of the initial model request, use `tools.eager` in settings.json (requires restart, #9827) — `tool_search`, `tool_call`, `structured_output`, plan-mode lifecycle tools, `task_stop`, `mcp__*` and `computer_use__*` tools are exempt from that allowlist and keep their normal loading; tools demoted this way stay registered and reachable through `tool_search` + `tool_call` while both bridge tools are registered — when either is unregistered (`tools.toolSearch.enabled: false` denies both; a `tool_search` or `tool_call` deny rule, or a `tools.disabled` entry removes one) the demoted tools that remain hidden are absent from top-level declarations and cannot be reached through the bridge for that session, and a warning is written to the CLI process's stderr (SDK forwards it only with piped stderr and effective `debug` logging; an explicit `logLevel` always wins). These bridge and warning rules apply to direct and hybrid code modes. On the session surface in hybrid mode, while `exec` itself is registered (container and SSH execution warn and fall back to direct tools without it), exec retains callable nested bindings; their schemas are included in exec when either bridge tool is unavailable. CodeModeOnly discovers deferred schemas through top-level tool_search and invokes them through exec. It skips deferred preload and startup catalogs; tools.eager reduces the initial exec description. When search is unavailable in the current scope, exec includes all allowed signatures. In Hybrid mode, AgentCore excludes tools still hidden by tools.eager from nested bindings. In both code modes, agent allowlists that do not grant `exec` narrow nested bindings. Inheriting or explicitly granting `exec` keeps all otherwise admitted ordinary code-mode-callable bindings. An execution allowlist that mentions any MCP tool additionally restricts MCP bindings to matching exact names or server patterns. In direct and hybrid modes they stay registered, so a direct call by their own name is still evaluated and approved normally — except tools also listed in `tools.visible`, which are declared upfront, and sessions whose live history contains a direct call to a still-hidden demoted tool, which any tool-set refresh (resume, MCP discovery, the first plan-mode entry in a session, a subagent definition change) re-declares.; to remove a tool entirely, use a whole-tool `excludeTools` / `permissions.deny` rule — a rule with a specifier (such as `'Bash(rm *)'`) only denies matching invocations at runtime. MCP tools are exempt from deny-based removal: hide them with the per-server `excludeTools` / `tools.disabled` filters instead (deny still blocks their calls at runtime). Example: `['read_file', 'edit', 'run_shell_command']`. | |
There was a problem hiding this comment.
[Suggestion] R1-48: N33: The rewritten coreTools contract is stored twice, byte-identical, in two files this diff edits — nothing generates one from the other and nothing asserts they agree.
I extracted the last cell of the coreTools row from both files at HEAD and compared: len == 3226 for each and a == b is True. This diff had to apply the same ~1.5 KB rewrite to both (docs/developers/sdk-typescript.md hunk @@ -53,27 +53,27 @@ carries the identical added sentence "On the session surface in hybrid mode…", and the identical removed sentence "These bridge and warning rules apply to direct tool mode."). The next semantic correction — e.g. the hybrid/exec condition in Finding 1, or the canSearchDeferredSchemas = false hardcode on the filtered path at tool-registry.ts:1538 that makes "when either bridge tool is unavailable" wrong for subagents — has to be found and made in both cells or the two published SDK references contradict each other, and an integrator who reads the stale one builds a client against a contract the shipped CLI no longer honours. Nine independently falsifiable statements were added to each copy in this diff and no test pins any of them in either file.
Witness:
``` ARM 1 intact tree (both real files): PASS: coreTools description cells identical (3226 chars) ARM 2 one sentence edited in a /tmp copy (positive control): FAIL: coreTools description cells diverged … first difference at char 1411: A: …always wins). These bridge and warning rules apply to direct and hybrid code modes. On the session surface… B: …always wins). These bridge and warning rules apply to direct code mode only. On the session surface… ARM 3 pre-PR cells replayed from the diff's own '-' lines: FAIL: old-docs-cell.md: 2154 chars / old-readme-cell.md: 2451 chars … first difference at
Suggested fix: Keep one authoritative copy and derive the other (a small generate step, or a <!-- prettier-ignore -->-style include), or — cheapest — add a test that reads both files, extracts the coreTools row's description cell, and asserts the two strings are equal, so drift fails CI instead of shipping.
Acceptance criterion: A new test (e.g. packages/sdk-typescript/src/readme-doc-sync.test.ts) that fails when one cell is edited without the other; it is red the moment either file's coreTools description diverges. Please prove it by removing the fix and confirming that test goes red.
中文说明
改写后的 coreTools 契约在本 diff 编辑的两个文件里逐字节重复存放，既没有由一方生成另一方，也没有任何测试断言二者一致。实测：两文件 22 个共有选项行的 description 全部逐字节相同（coreTools 行为 3226 字符）；在 merge base 上二者曾分别为 2154 与 2451 字符、互相矛盾，正是本 PR 手工同步修好的。同一句新增文案在 HEAD 上散布于 6 个文件、11 处手工维护位置（另有 3 处由 CI 固定），而本 diff 手工改了其中 5 个文件。
— qwen3.8-max via Qwen Code /review (v0.25.0)
| 'settings.label.tools.codeModeOnly': '仅代码模式（实验性）', | ||
| 'settings.description.tools.codeModeOnly': | ||
| '普通工具只通过隔离的 exec JavaScript 工具暴露给模型。直接控制类工具仍然可用。在 safe 和 bare 模式下忽略。', | ||
| 'settings.label.tools.mode': '工具模式（实验性）', |
There was a problem hiding this comment.
[Suggestion] R1-32: The row became an enum but the ZH dictionary only translates its label and description — the three option labels have no settings.option.tools.mode.* entries, so a zh-CN user gets the raw English schema labels.
With language zh-CN, open Settings → Tools → 工具模式（实验性）. SettingsMessage.tsx:745-757 builds the picker from the daemon descriptor and calls formatSettingOption, which looks up settings.option.tools.mode.direct|code_mode|code_mode_only; none exist (repo-wide grep for settings.option.tools.mode returns zero source hits), so translateSettingText falls back to the served labels — { value: ToolMode.Direct, label: 'Default' } etc. (packages/cli/src/config/settingsSchema.ts:2917-2919, forwarded verbatim by buildSettingsResponse at packages/cli/src/serve/routes/workspace-settings.ts:233). The Chinese panel therefore shows "Default / Code Mode / Code Mode Only" under a fully Chinese label and description, while every other enum in this same ZH map is translated — settings.option.ui.chatWidth.* (messages.ts:192-193), settings.option.review.effort.* (234-237), settings.option.model.reasoningEffort.* (307-311), settings.option.tools.workflowSizeGuideline.* (362-366), settings.option.policy.permissionStrategy.* (377-380). Before this diff the row was a boolean rendered through the already-translated settings.value.on/off, so the untranslated option list is new. "Default" is also the least self-describing of the three for a Chinese reader, since the description calls that mode "Direct".
Witness:
not run — probe/mutation via `review scratch-tree` and `base-tree`: unavailable in this environment (repo-local git includeIf resolves to a missing credentials file, so scratch-tree refuses to create a tree; the base build timed out), so the verdict rests on a line-by-line source trace plus repo-wide greps
Suggested fix: Add the three option keys next to the new label/description, e.g. 'settings.option.tools.mode.direct': '直接（默认）', 'settings.option.tools.mode.code_mode': 'Code Mode', 'settings.option.tools.mode.code_mode_only': 'Code Mode Only' (keeping the two proper-noun mode names as-is if that is the intended house rendering).
The fix must not violate this existing fact: Keys must be spelled exactly settings.option.${setting.key}.${String(value)} (packages/web-shell/client/components/messages/SettingsMessage.tsx:186) with the values the schema ships — { value: ToolMode.Direct, label: 'Default' }, { value: ToolMode.CodeMode, ... }, { value: ToolMode.CodeModeOnly, ... } (packages/cli/src/config/settingsSchema.ts:2917-2919), i.e. direct / code_mode / code_mode_only, not the label text.
Acceptance criterion: A case in packages/web-shell/client/components/messages/SettingsMessage.dom.test.tsx (or a new collocated settings/messages.test.ts) that renders/derives the option list for a tools.mode enum descriptor under the zh-CN dictionary and asserts the Direct option is not the raw schema label 'Default' — equivalently, asserts SETTINGS_MESSAGES_ZH['settings.option.tools.mode.direct'] is defined. Delete the added keys and that assertion must go red; no existing test covers it (`SettingsMessage Please prove it by removing the fix and confirming that test goes red.
中文说明
该行变成了枚举，但中文字典只翻译了它的 label 和 description——三个选项标签没有对应的 settings.option.tools.mode.* 条目，因此 zh-CN 用户在完全中文的标签与描述下会看到原始的英文选项 "Default / Code Mode / Code Mode Only"。同一 diff 已为 CLI 翻译了这三个标签并加了 must-translate 测试，可见 web-shell 这一处属于遗漏而非有意。
— qwen3.8-max via Qwen Code /review (v0.25.0)
There was a problem hiding this comment.
[Suggestion] R1-32: The row became an enum but the ZH dictionary only translates its label and description — the three option labels have no settings.option.tools.mode.* entries, so a zh-CN user gets the raw English schema labels.
With language zh-CN, open Settings → Tools → 工具模式（实验性）. SettingsMessage.tsx:745-757 builds the picker from the daemon descriptor and calls formatSettingOption, which looks up settings.option.tools.mode.direct|code_mode|code_mode_only; none exist (repo-wide grep for settings.option.tools.mode returns zero source hits), so translateSettingText falls back to the served labels — { value: ToolMode.Direct, label: 'Default' } etc. (packages/cli/src/config/settingsSchema.ts:2917-2919, forwarded verbatim by buildSettingsResponse at packages/cli/src/serve/routes/workspace-settings.ts:233). The Chinese panel therefore shows "Default / Code Mode / Code Mode Only" under a fully Chinese label and description, while every other enum in this same ZH map is translated — settings.option.ui.chatWidth.* (messages.ts:192-193), settings.option.review.effort.* (234-237), settings.option.model.reasoningEffort.* (307-311), settings.option.tools.workflowSizeGuideline.* (362-366), settings.option.policy.permissionStrategy.* (377-380). Before this diff the row was a boolean rendered through the already-translated settings.value.on/off, so the untranslated option list is new. "Default" is also the least self-describing of the three for a Chinese reader, since the description calls that mode "Direct".
Witness:
not run — probe/mutation via `review scratch-tree` and `base-tree`: unavailable in this environment (repo-local git includeIf resolves to a missing credentials file, so scratch-tree refuses to create a tree; the base build timed out), so the verdict rests on a line-by-line source trace plus repo-wide greps
Suggested fix: Add the three option keys next to the new label/description, e.g. 'settings.option.tools.mode.direct': '直接（默认）', 'settings.option.tools.mode.code_mode': 'Code Mode', 'settings.option.tools.mode.code_mode_only': 'Code Mode Only' (keeping the two proper-noun mode names as-is if that is the intended house rendering).
The fix must not violate this existing fact: Keys must be spelled exactly settings.option.${setting.key}.${String(value)} (packages/web-shell/client/components/messages/SettingsMessage.tsx:186) with the values the schema ships — { value: ToolMode.Direct, label: 'Default' }, { value: ToolMode.CodeMode, ... }, { value: ToolMode.CodeModeOnly, ... } (packages/cli/src/config/settingsSchema.ts:2917-2919), i.e. direct / code_mode / code_mode_only, not the label text.
Acceptance criterion: A case in packages/web-shell/client/components/messages/SettingsMessage.dom.test.tsx (or a new collocated settings/messages.test.ts) that renders/derives the option list for a tools.mode enum descriptor under the zh-CN dictionary and asserts the Direct option is not the raw schema label 'Default' — equivalently, asserts SETTINGS_MESSAGES_ZH['settings.option.tools.mode.direct'] is defined. Delete the added keys and that assertion must go red; no existing test covers it (`SettingsMessage Please prove it by removing the fix and confirming that test goes red.
中文说明
该行变成了枚举，但中文字典只翻译了它的 label 和 description——三个选项标签没有对应的 settings.option.tools.mode.* 条目，因此 zh-CN 用户在完全中文的标签与描述下会看到原始的英文选项 "Default / Code Mode / Code Mode Only"。同一 diff 已为 CLI 翻译了这三个标签并加了 must-translate 测试，可见 web-shell 这一处属于遗漏而非有意。
— qwen3.8-max via Qwen Code /review (v0.25.0)
qwen-code-review-bot
left a comment
qwen-code-review-bot
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed.
Unresolved, please confirm:
- [Critical] Prior-round Critical R1-2 (asserted in @doudouOUC's review 5217747523, packages/cli/src/config/settingsSchema.ts:36) — the original inline comment was deleted, so only the reviewer's premise survives and the claimed mechanism could not be t…
Not reviewed: build-and-test — 'Integration Tests (CLI, No Sandbox)' was skipped in CI and its suite did not run locally (Agent 7 ran per-package vitest suites only).
Not explored to full depth (tool budget reached): "agent reverse-audit (round 1)": none — the chunk's remaining items were walked clear: agent-types.ts 's new field and all five read sites, the subagent-plan-tool-policy.test.ts title-only c…; "agent reverse-audit (round 1)": mutation-run confirming that deleting the hybrid declarationNames snapshot clause leaves agent-core.skill-gate.test.ts and agent-core.fork-policy.test.ts …; "agent reverse-audit (round 1)": whether getToolExposure(ToolNames.EXEC) is 'code-mode-callable' — inferred from the passing codeModeAllowedToolNames assertions ( exec never appears in a…; "agent reverse-audit (round 2)": did not execute mutation probes (plain registerTool in place of registerPermissionDeferredFactory ; a hybrid search_memory allowlist through getFunctionDe…; "agent reverse-audit (round 2)": did not run npx vitest run src/code-mode/code-mode.test.ts to confirm the suite is green at this commit, so my claim that F1's assertions hold today is derive…, and 2 more.
Not reviewed: reverse audit — stopped before round 3 by the review time budget.
中文说明
仅完成部分审查，审查缺口已披露。
未决，请确认：共 1 条（原文未翻译，列表见上方英文部分）。
未审查（原文为英文）：build-and-test — 'Integration Tests (CLI, No Sandbox)' was skipped in CI and its suite did not run locally (Agent 7 ran per-package vitest suites only).
未探索到全部深度（达到工具调用预算）："agent reverse-audit (round 1)"：none — the chunk's remaining items were walked clear: agent-types.ts 's new field and all five read sites, the subagent-plan-tool-policy.test.ts title-only c…；"agent reverse-audit (round 1)"：mutation-run confirming that deleting the hybrid declarationNames snapshot clause leaves agent-core.skill-gate.test.ts and agent-core.fork-policy.test.ts …；"agent reverse-audit (round 1)"：whether getToolExposure(ToolNames.EXEC) is 'code-mode-callable' — inferred from the passing codeModeAllowedToolNames assertions ( exec never appears in a…；"agent reverse-audit (round 2)"：did not execute mutation probes (plain registerTool in place of registerPermissionDeferredFactory ; a hybrid search_memory allowlist through getFunctionDe…；"agent reverse-audit (round 2)"：did not run npx vitest run src/code-mode/code-mode.test.ts to confirm the suite is green at this commit, so my claim that F1's assertions hold today is derive…，另有 2 条。
未审查：反向审计——评审时间预算不足，未能开始第 3 轮。
— qwen3.8-max via Qwen Code /review (v0.25.0)
| (this.runtimeContext.getToolMode?.() === ToolMode.CodeModeOnly || | ||
| !isHiddenByEagerAllowList(name)) && |
There was a problem hiding this comment.
[Critical] R1-1: N03: [certifies-falsely] [new-surface] The new Hybrid-only eager exclusion removes a demoted skill from both the declarations and the exec binding set, but the skill-listing predicate that this same diff rewired (willHaveSkillTool() → toolConfigAllowsSkill(this.toolConfig, hasAgentSkillExecBinding(...)), agent-core.ts:660-664) still answers true through its two non-exec clauses, so in tools.mode: "code_mode" the listing and canInvokeSkill() disagree — the exact #12424 divergence the shared predicate exists to prevent, and the invariant the author's own matrix asserts (expect(gate(core, declared)).toBe(willHaveSkill), agent-core.skill-gate.test.ts:402).
Trigger: tools.mode: "code_mode" (a value this diff adds — ToolMode.CodeMode is new at packages/core/src/tools/code-mode.ts:23) plus an active settings.tools.eager allowlist that omits skill. registerLazyTool turns that omission into status === 'deferred' → registry.registerPermissionDeferredFactory(ToolNames.SKILL, …) (config.ts:12251-12252, skill registered lazily at config.ts:12704-12708), getVisibleTools() is sourced only from settings.tools.visible (config.ts:8322-8331) so it does not contain skill, and getToolExposure('skill') is 'code-mode-callable' (code-mode.ts:39-58 — skill is in neither HIDDEN_TOOLS nor DIRECT_ONLY_TOOLS). Any subagent reaches this with the default config, because SubagentManager builds tools: configuredToolConfig?.tools ?? ['*'] (subagent-manager.ts:1150-1151). Then, traced line by line at this commit: prepareTools() sets isHiddenByEagerAllowList('skill') true (agent-core.ts:709-712 → isPermissionDeferred ∧ isDeferredAndHidden, tool-registry.ts:589 + 1352-1360, alwaysLoad defaults false — SkillTool's super() passes only 7 args, skill.ts:157-166), so the anchored clause drops skill from allowedNames; Hybrid declarationNames ⊆ allowedNames (agent-core.ts:764-773) so skill is not declared, and codeModeAllowedToolNames (agent-core.ts:759-762) excludes it. canInvokeSkill(declared) is therefore false on both routes: declaredToolNames.has(SKILL) false, and the nested route fails its codeModeAllowedToolNames?.includes(SKILL) === true term (agent-core.ts:1854-1862). willHaveSkillTool() is nevertheless true: toolConfigAllowsSkill returns inheritsRegistry || names.includes(SKILL) || reachesThroughExec (subagent-plan-tool-policy.ts:160-164), and inheritsRegistry (names.includes('*')) or names.includes('skill') short-circuits before the exec term that hasAgentSkillExecBinding correctly answers false for. Wrong outcomes at all three consumers of that answer: createChat injec
Witness:
`Source: [probe]` — permission-deferred, unrevealed, non-visible `skill`; hybrid vs Only × four `tools` lists: ``` N03 {"mode":"code_mode","tools":["*"], "willHaveSkillTool":true, "canInvokeSkill":false,"declaredHasSkill":false,"codeModeAllowedHasSkill":false} N03 {"mode":"code_mode","tools":["skill"], "willHaveSkillTool":true, "canInvokeSkill":false,"declaredHasSkill":false} N03 {"mode":"code_mode","tools":["skill","exec"],"willHaveSkillTool":true,"canInvokeSkill":false} N03 {"mode":"code_mode","tools":["exec"], "willHaveSkillTool":false,"canInvokeSkill":false} ← agrees N03 {"mode":"code_mode
Suggested fix: Give the shared predicate the same eager-scope fact prepareTools() uses, so it vetoes every route in Hybrid, not just the exec route: thread a skillEagerHidden (or mode-aware) input into toolConfigAllowsSkill computed as mode === ToolMode.CodeMode && registry.isPermissionDeferred?.(SKILL) === true && registry.isDeferredAndHidden?.(SKILL) === true, return false when it is set, and pass it from all three call sites (agent-core.ts:660-664, subagent-manager.ts:1187-1189, background-agent-resume.ts:157) — fixing only hasAgentSkillExecBinding (as R1-17 proposes) leaves the wildcard and explicit-skill clauses returning true.
The fix must not violate this existing fact: agent-core.skill-gate.test.ts:398-400 — expect(willHaveSkill).toBe(mode === ToolMode.CodeModeOnly || visibility === 'visible');. The fix must keep CodeModeOnly answering true for visibility: 'hidden' (its prepareTools() bypasses the eager filter at agent-core.ts:741, so skill really is bound there) and keep Hybrid answering true for 'visible' and 'revealed', both of which make isDeferredAndHidden false (`!this.revealedDeferred.has(name) && !this.config.getVisibleTools().has(n
Acceptance criterion: packages/core/src/agents/runtime/agent-core.skill-gate.test.ts:341-404 — the it.each([ToolMode.CodeMode, ToolMode.CodeModeOnly] × warm × visibility) matrix already builds a permission-deferred skill and asserts expect(gate(core, declared)).toBe(willHaveSkill), but instantiates only { tools: [ToolNames.EXEC] }. Add { tools: ['*'] } and { tools: [ToolNames.SKILL] } rows to the same matrix: without the fix, mode: ToolMode.CodeMode + visibility: 'hidden' (both warm values) goes R Please prove it by removing the fix and confirming that test goes red.
中文说明
新增的 Hybrid 专用 eager 排除会把被降级的 skill 同时从声明列表和 exec 绑定集合中去掉，但本 PR 改接的 skill 列表判定（willHaveSkillTool() → toolConfigAllowsSkill）仍会通过它的两个非 exec 分支返回 true。因此在 tools.mode: "code_mode" 下，列表判定与执行闸门 canInvokeSkill() 结论相反——这正是该共享判定本应防止的 #12424 分歧。触发条件：hybrid 模式 + 生效的 tools.eager 白名单未包含 skill；子智能体默认 tools: ['*'] 即可命中。后果：createChat 注入 includeAvailableSkillsReminder: true，缓存的 prompt 前缀里出现 <available_skills>，列出该智能体其实没有的工具，模型调用后得到 Tool "skill" not found；SubagentManager 保留 skillsAvailable = true，把 bundled skill 引用作为子智能体无法跟随的指针下发；后台恢复路径重复同样的错误 true。探针实测三行 hybrid 组合分歧、四行 CodeModeOnly 全部一致；打上候选修复后分歧消失且 77 个作者测试仍全绿。另需注意：Direct 模式在未被本 PR 触碰的代码上已有同样分歧，只修 hybrid 会留下一半。
— qwen3.8-max via Qwen Code /review (v0.25.0)
| | `exec` | CodeMode and CodeModeOnly | No | | ||
| | Direct control | Yes | No | | ||
| | Ordinary registered tool | CodeMode only | Yes | | ||
| | Hidden bridge (`tool_search`, `tool_call`) | Existing behavior outside strict mode | No | |
There was a problem hiding this comment.
[Suggestion] R1-2: N06: The rewritten exposure row conditions the table's only statement about bridge exposure on "strict mode" — a term that exists nowhere in the mode vocabulary, the code, or any other English document — and it groups two tools the code exposes differently.
ToolMode has exactly three values, direct / code_mode / code_mode_only (packages/core/src/tools/code-mode.ts:22-26); no setting, code path, or EN doc defines a "strict mode", and a repo-wide grep over docs/ finds the phrase only on this line (the ZH twin's 严格模式 is glossed only in a different file, docs/design/code-mode.zh-CN.md:104: "tools.mode: "code_mode_only" 选择严格模式"). A user on tools.mode: "code_mode" therefore cannot tell from this table whether hybrid counts as strict, i.e. whether tool_search/tool_call are declared for their session — and the shipped rule splits the pair the row keeps together: tool_search is a top-level direct control under CodeModeOnly (DIRECT_ONLY_TOOLS, code-mode.ts:39-51, admitted by the exposure === 'direct-only' filter in getCodeModeFunctionDeclarations, tool-registry.ts:1126-1131), while tool_call is hidden in every mode (const HIDDEN_TOOLS = new Set<string>(['tool_call']);, code-mode.ts:38). The same file's unchanged Status paragraph states that split correctly ("tool_search is now a top-level direct control … tool_call stays hidden", code-mode-only.md:12-16), so the table and the Status paragraph disagree about whether the two bridge tools share a rule. Every other row in this table names modes explicitly ("CodeMode and CodeModeOnly", "CodeMode only"), making this the one cell a reader cannot resolve to a tools.mode value.
Witness:
`Source: [probe]` sweep — `grep -rn 'strict mode|strict \`|严格模式' **/*.md` over the whole worktree: as a **tool-mode** term the phrase occurs exactly **once** in EN docs (this row) and once in its ZH twin; every other hit is an unrelated domain (TypeScript strict mode, Ajv strict, screen-reader strict, `parseLastEventId`). `ToolMode` has three values (code-mode.ts:22-26) and no setting or code path names one "strict". Mitigation I found and the finder did not weigh: the sibling EN doc this same PR adds glosses it adjectivally — `code-mode.md:22` "the strict `CodeModeOnly` exposure policy" — and
Suggested fix: Replace the cell with mode names and split the pair, e.g. two rows — | tool_search | CodeModeOnly top level; existing Direct/CodeMode behavior | No | and | tool_call | Hidden in every mode | No | — and mirror the change in docs/design/code-mode-only.zh-CN.md:54.
The fix must not violate this existing fact: const HIDDEN_TOOLS = new Set<string>(['tool_call']); and const DIRECT_ONLY_TOOLS = new Set<string>([ToolNames.TOOL_SEARCH, …]) — packages/core/src/tools/code-mode.ts:38-51; the rewritten rows must keep tool_call hidden in every mode and tool_search top-level under CodeModeOnly, matching docs/design/code-mode-only.md:12-16.
Acceptance criterion: N/A (documentation prose; no guard, branch or behaviour to pin). Please prove it by removing the fix and confirming that test goes red.
中文说明
改写后的 exposure 表格行把该表唯一一句关于 bridge 暴露的说明限定在 "strict mode" 下——而这个词在模式词表、代码和其余文档中都不存在（真实枚举是 direct / code_mode / code_mode_only）。读者无法把它对应到任何可配置取值。
— qwen3.8-max via Qwen Code /review (v0.25.0)
| property, an exact canonical match wins over rewritten names. If neither is an | ||
| exact match, the lexicographically first name wins. The description names the |
There was a problem hiding this comment.
[Suggestion] R1-3: N01: The sentence this diff rewrote (pre-diff: "one warning names the omitted collision") claims the exec description names the dropped binding, but in CodeModeOnly with tool_search registered — the shipped default this same diff's new blockquote declares ("Only discovers schemas through top-level tool_search") — buildExecDescription suppresses the collision block entirely. The pre-diff wording was the accurate one; the new one is false for the mode the document is about.
A registry holding mcp__srv__get-data and mcp__srv__get_data normalizes both to mcp__srv__get_data, so planCodeModeBindings keeps the exact match and records { jsName, kept: 'mcp__srv__get_data', omitted: 'mcp__srv__get-data' } (packages/core/src/tools/code-mode.ts:111-124). In a tools.mode: "code_mode_only" session, getCodeModeFunctionDeclarations computes searchAvailable = !!this.getTool(TOOL_SEARCH) && (!allowedNames || allowedNames.has(TOOL_SEARCH)) → true (packages/core/src/tools/tool-registry.ts:1121-1123, passed at :1135), and buildExecDescription sets const searchAvailable = codeModeOnly && (options.searchAvailable ?? false) → true (code-mode.ts:253), so collisionText = (searchAvailable ? [] : plan.collisions) → [] (code-mode.ts:264) and the description ends with no Name collisions: block (code-mode.ts:325). The repo's own test pins that suppression: packages/core/src/code-mode/code-mode.test.ts:723-739 registers exec + tool_search under code_mode_only with hidden-tool/hidden_tool and asserts expect(description).not.toContain('hidden-tool') and not.toContain('hidden_tool'). The only surface that names the drop is debugLogger.warn inside warnCodeModeCollisions (tool-registry.ts:1158-1166) — debug-gated, not model-facing. Concrete cost: the model is never told the binding was dropped in the default Only configuration, and the maintainer triaging "tool X is unreachable through exec / exec called the wrong MCP tool" reads this document as the normative collision contract, goes looking for a missing line in the description generator, and finds code that is behaving exactly as written. The same false sentence ships in the new Chinese twin ("描述会指出被省略的冲突项。", code-mode-only.zh-CN.md:73-74).
Witness:
`Source: [probe]` — CodeModeOnly registry with `exec` + `tool_search` + colliding `get-data`/`get_data` (non-deferred, so deferral filtering cannot explain the absence): `N01-only {"collisions":[{"jsName":"get_data","kept":"get_data","omitted":"get-data"}],"descriptionHasCollisionsBlock":false,"descriptionNamesOmittedTool":false,"declaredNames":["exec","tool_search"]}` Positive controls (the probe *can* see the block): `N01-only-nosearch {"descriptionHasCollisionsBlock":true,"descriptionNamesOmittedTool":true}` · `N01-hybrid {"descriptionHasCollisionsBlock":true,"descriptionNamesOmittedTool":t
Suggested fix: Scope the claim to the configuration where it holds and name the other surface, e.g. "When search is unavailable in the current scope, the description names the omitted collision; when tool_search is available the collision block is left out of the description and the drop is logged once through the debug logger." Apply the identical correction to docs/design/code-mode-only.zh-CN.md:73-74.
The fix must not violate this existing fact: const collisionText = (searchAvailable ? [] : plan.collisions) — packages/core/src/tools/code-mode.ts:264, with const searchAvailable = codeModeOnly && (options.searchAvailable ?? false); at :253. The corrected sentence must keep the hybrid case true: decorateCodeModeDeclarations passes codeModeOnly: false (packages/core/src/tools/tool-registry.ts:1087-1092), so hybrid descriptions always emit the block.
Acceptance criterion: N/A (documentation prose). The behaviour the corrected sentence must match is already pinned from both sides: packages/core/src/code-mode/code-mode.test.ts:723-739 (CodeModeOnly + tool_search → neither collision name appears) and :563-591 'describes normalized-name collisions on the hybrid surface' (hybrid description contains '- read-file is omitted because it collides with read_file as tools.read_file.'). Please prove it by removing the fix and confirming that test goes red.
中文说明
本次改写后的句子声称 exec 的 description 会点出被丢弃的 binding，但在 CodeModeOnly 下被丢弃的 binding 恰恰不会进入 description；改写前的措辞（"one warning names the omitted collision"）才是准确的——warnCodeModeCollisions 在所有模式下都会触发。中英文两份设计文档同句同错。
— qwen3.8-max via Qwen Code /review (v0.25.0)
| arguments as JavaScript, with ordinary runtime validation still enforced. | ||
|
|
||
| This change makes the existing experimental `tools.codeModeOnly` mode load | ||
| This change makes the existing experimental `tools.mode: "code_mode_only"` mode load |
There was a problem hiding this comment.
[Suggestion] R1-10: N07: This diff renames the setting in the two sentences it touches but leaves "Code Mode" standing as a bare mode name at :43 ("Direct-only and collided tools are excluded from Code Mode discovery.") and :57 ("In Code Mode, an exact-name miss explains that select: requires the registered name…") — a name this same PR now assigns to a different mode (tools.mode: "code_mode", settings label 'Code Mode' at packages/cli/src/config/settingsSchema.ts:2918, and the new docs/design/code-mode.md titled "# Code Mode", which cites this file as the authority that superseded its MVP prose). Both standing sentences describe behavior the code gates on ToolMode.CodeModeOnly alone, so under the naming this PR introduces they are false for the mode they now appear to name.
ToolSearchTool.execute() computes const codeMode = this.config.getToolMode?.() === ToolMode.CodeModeOnly; and const bindings = codeMode ? new Map(…getCodeModeBindingPlan(…).bindings…) : undefined (packages/core/src/tools/tool-search.ts:172-183). In hybrid code_mode, bindings is undefined, so both exec-guidance tails are skipped — '\n\nCall these tools through exec using tools.<jsName>(args) and the required parameters above.' (:498-501) and '\nselect: requires the registered name, including mcp__<server>__<tool> for MCP tools. Search with keywords without select: …, then use the returned jsName in exec.' (:506-508) — and discovery is not restricted to the callable binding plan, so direct-only and collision-omitted tools are not excluded. Concrete cost: a maintainer debugging a hybrid session where select:bogus returns only Not found: bogus reads :57 as a statement about "Code Mode" (= hybrid, per this PR's own naming), concludes discovery guidance is broken, and fixes against the wrong gate; :43 likewise reads as a hybrid permission-boundary claim while hybrid applies no such exclusion. The paragraph's own contrast clause — "Direct mode retains its current response" — leaves hybrid no slot at all, yet hybrid is exactly the case that gets the Direct response. The ZH twins carry the same collision (Code Mode 搜索排除只能直接调用的工具…, Code Mode 的精确名称查找未命中时…).
Witness:
`Source: [probe]` — same registry, same query, two modes: `N07 {"mode":"code_mode_only","query":"select:bogus","hasSelectHintTail":true,"contentHead":"Not found: bogus\nselect: requires the registered name, including mcp__<server>__<tool> for MCP tools. … then use the returned jsName in exec."}` `N07 {"mode":"code_mode","query":"select:bogus","hasSelectHintTail":false,"contentHead":"Not found: bogus"}` So the behaviour :57 attributes to "Code Mode" is Only-only, and hybrid — which this PR names "Code Mode" (`settingsSchema.ts:2918-2921`, option `{ value: ToolMode.CodeMode, label: 'Code Mode' }
Suggested fix: Scope the standing sentences to the mode that implements them — ":43 → Direct-only and collided tools are excluded from CodeModeOnly discovery." and ":57 → In CodeModeOnly, an exact-name miss explains that \select:` requires the registered name…" — and mirror both in lazy-code-mode.zh-CN.md:21/:29 (CodeModeOnly 搜索排除…, CodeModeOnly 的精确名称查找未命中时…). Where the family is genuinely meant, say "both code modes" only for behavior gated on isCodeModeEnabled(…)`.
The fix must not violate this existing fact: const codeMode = this.config.getToolMode?.() === ToolMode.CodeModeOnly; — packages/core/src/tools/tool-search.ts:172. The correction must not widen the sentences to "both code modes": the binding-plan restriction and both exec tails at :498-508 read bindings, which stays undefined in hybrid, and the scheduler's own code-mode call-surface gate is likewise Only-scoped (`this.config.getToolMode?.() === ToolMode.CodeModeOnly && … !isCodeModeToolCallAllowed(canonicalName, reqInfo.source ?? 'mod
Acceptance criterion: N/A (documentation prose; the fix adds no guard, branch, or behavior a test can pin). Please prove it by removing the fix and confirming that test goes red.
中文说明
本 diff 重命名了它改到的两句里的设置项，却在第 43 行（及 :57、中文孪生行）把 "Code Mode" 留作一个裸模式名，而该标签现在已被本 PR 指派给另一个取值 code_mode。
— qwen3.8-max via Qwen Code /review (v0.25.0)
| | `abortController` | `AbortController` | - | Controller to cancel the query session. Call `abortController.abort()` to terminate the session and cleanup resources. | | ||
| | `debug` | `boolean` | `false` | Enable debug mode for verbose logging from the CLI process. | | ||
| | `maxSessionTurns` | `number` | `-1` (unlimited) | Maximum number of conversation turns before the session automatically terminates. Must be an integer. A turn consists of a user message and an assistant response. | | ||
| | `coreTools` | `string[]` | - | Uses the legacy `coreTools` / CLI `--core-tools` allowlist semantics. If specified, only matching core tools are registered for the session. This is the only allowlist-style option that restricts built-in tool registration; a whole-tool `permissions.deny` / `excludeTools` rule (and `tools.disabled` in settings.json) also removes a tool from the registry. `permissions.allow` in settings.json is pure auto-approval and never removes, demotes, or hides a tool (#10075). To keep a tool's schema out of the initial model request, use `tools.eager` in settings.json (requires restart, #9827) — `tool_search`, `tool_call`, `structured_output`, plan-mode lifecycle tools, `task_stop`, `mcp__*` and `computer_use__*` tools are exempt from that allowlist and keep their normal loading; tools demoted this way stay registered and reachable through `tool_search` + `tool_call` while both bridge tools are registered — when either is unregistered (`tools.toolSearch.enabled: false` denies both; a `tool_search` or `tool_call` deny rule, or a `tools.disabled` entry removes one) the demoted tools that remain hidden are absent from top-level declarations and cannot be reached through the bridge for that session, and a warning is written to the CLI process's stderr (SDK forwards it only with piped stderr and effective `debug` logging; an explicit `logLevel` always wins). These bridge and warning rules apply to direct and hybrid code modes. On the session surface in hybrid mode, while `exec` itself is registered (container and SSH execution warn and fall back to direct tools without it), exec retains callable nested bindings; their schemas are included in exec when either bridge tool is unavailable. CodeModeOnly discovers deferred schemas through top-level tool_search and invokes them through exec. It skips deferred preload and startup catalogs; tools.eager reduces the initial exec description. When search is unavailable in the current scope, exec includes all allowed signatures. In Hybrid mode, AgentCore excludes tools still hidden by tools.eager from nested bindings. In both code modes, agent allowlists that do not grant `exec` narrow nested bindings. Inheriting or explicitly granting `exec` keeps all otherwise admitted ordinary code-mode-callable bindings. An execution allowlist that mentions any MCP tool additionally restricts MCP bindings to matching exact names or server patterns. In direct and hybrid modes they stay registered, so a direct call by their own name is still evaluated and approved normally — except tools also listed in `tools.visible`, which are declared upfront, and sessions whose live history contains a direct call to a still-hidden demoted tool, which any tool-set refresh (resume, MCP discovery, the first plan-mode entry in a session, a subagent definition change) re-declares.; to remove a tool entirely, use a whole-tool `excludeTools` / `permissions.deny` rule — a rule with a specifier (such as `'Bash(rm *)'`) only denies matching invocations at runtime. MCP tools are exempt from deny-based removal: hide them with the per-server `excludeTools` / `tools.disabled` filters instead (deny still blocks their calls at runtime). Example: `['read_file', 'edit', 'run_shell_command']`. | |
There was a problem hiding this comment.
[Suggestion] R1-11: N08: The newly documented CodeModeOnly guarantee "It skips deferred preload" rests on a single untested early-return — packages/core/src/core/client.ts:2129 if (this.config.getToolMode?.() === ToolMode.CodeModeOnly) return; — while all three sibling guards in the same method are pinned (client.test.ts:1931 threshold 0, :1937 non-finite threshold, :1954 bridge unavailable). The other half of the same sentence is pinned (tool-registry.test.ts:1152 "getDeferredToolSummary is empty in CodeModeOnly"), so only the preload half is unprotected.
A refactor that consolidates or drops the mode guard in preloadDeferredToolsWithinBudget() (nothing turns red — no test sets getToolMode to ToolMode.CodeModeOnly anywhere in client.test.ts; the only getToolMode mocks are at :3419/:3455/:3490, all ToolMode.CodeMode) lets the budget preload call revealDeferredTool() for every ordinary deferred tool at CodeModeOnly session start. isDeferredAndHidden() then returns false for them (tool-registry.ts:1353-1361 consults revealedDeferred), so planCodeModeBindings stamps deferred: false (code-mode.ts:127), and visibleBindings = plan.bindings.filter((binding) => !searchAvailable || !binding.deferred) (code-mode.ts:252-254) stops filtering them out even though tool_search is available. Their full tools.<jsName>(args: …) signatures and descriptions land in the initial exec declaration — exactly the eager-everything behavior the sentence promises CodeModeOnly skips, and the exec-declaration rewrite / prompt-cache bust that tool-registry.ts:1389-1392 says the empty summary exists to prevent. An SDK integrator who sized their tools.eager allowlist against this documented promise gets the deferred schemas in the first request anyway.
Witness:
two runs. Sweep (coverage): `getToolMode` is mocked three times in `client.test.ts` (:3419, :3455, :3490) and **all three return `ToolMode.CodeMode`**; `ToolMode.CodeModeOnly`/`'code_mode_only'` appears **0** times in that file (the only nearby mocks are a different accessor, `getCodeModeOnly` at :889 and :12634). The sibling guards in the same method *are* pinned — `'skips deferred preload when the threshold is 0'` (:1931), `'…not finite'` (:1937), `'clamps a threshold above 100%'` (:1944), `'…when the bridge is unavailable'` (:1954) — and no other test file touches the client-level guard (`p
Suggested fix: Add one case to the preloadDeferredToolsWithinBudget block in packages/core/src/core/client.test.ts, beside 'skips deferred preload when the threshold is 0' (:1931) and 'skips deferred preload when the bridge is unavailable' (:1954): set mockConfig.getToolMode = vi.fn().mockReturnValue(ToolMode.CodeModeOnly), give the registry both bridge tools and a positive getToolSearchThreshold(), run startChat, and assert the registry preload spy was never called.
The fix must not violate this existing fact: The assertion must target the registry spy, not a reveal side effect: the shared registry stub is preloadDeferredToolsWithinBudget: vi.fn().mockReturnValue(0) (packages/core/src/core/client.test.ts:860) and does not record revealDeferredTool calls for this path. The test must also drive the mode through getToolMode, which is what the guard reads (packages/core/src/core/client.ts:2129), and not through the separately-stubbed getCodeModeOnly: vi.fn().mockReturnValue(false) (`packages/c
Acceptance criterion: That new test — expect(reg.preloadDeferredToolsWithinBudget).not.toHaveBeenCalled() under ToolMode.CodeModeOnly with a positive threshold and both bridge tools registered — goes red if the client.ts:2129 guard is deleted, because the threshold and bridge guards that satisfy the existing three tests are both satisfied in this configuration. Please prove it by removing the fix and confirming that test goes red.
中文说明
新增的 CodeModeOnly 保证"It skips deferred preload"依赖一个没有测试覆盖的提前返回（client.ts:2129），该分支一旦回归，文档承诺就会静默失效。
— qwen3.8-max via Qwen Code /review (v0.25.0)
| You may call exec and tools matched by this direct-call allowlist: ${JSON.stringify(executionAllowedTools ?? [])}. | ||
| Inside exec, only these exact nested tool names are permitted: ${JSON.stringify(nestedExecutionAllowedTools)}. |
There was a problem hiding this comment.
[Suggestion] R1-37: N28: The new sentence asserts a direct-call allowlist unconditionally, but the nested branch is entered for both code modes (isCodeModeEnabled), and under code_mode_only no ordinary tool is declared to the fork and a direct model call to one is hard-denied — so the sentence enumerates tools the child cannot call.
tools.mode: "code_mode_only", a subagent defined with tools: ['exec','read_file','write_file'], launching a plain fork. nestedExecutionAllowedTools is defined (mode is code-mode-enabled and the allowlist frame is set), which also makes requestedExecutionAllowedTools defined for a plain fork for the first time (agent.ts:1922-1928: requestedTools === undefined && nestedExecutionAllowedTools === undefined ? undefined : …) — pre-diff this shape rendered no restriction text at all. The child's declarations come from getFunctionDeclarationsFiltered, which routes CodeModeOnly to getCodeModeFunctionDeclarations(new Set(toolNames)) (tool-registry.ts:1520-1522), keeping only exposure === 'exec' and 'direct-only' tools (tool-registry.ts:1119-1131) — read_file is never declared. If the child nonetheless acts on "You may call exec and tools matched by this direct-call allowlist: ["read_file","write_file"]", coreToolScheduler.ts:3145-3162 rejects it: getToolMode() === ToolMode.CodeModeOnly and isCodeModeToolCallAllowed('read_file','model') is false (exposure code-mode-callable is not exec/direct-only), producing Tool "read_file" is unavailable on this CodeModeOnly call surface. with ToolErrorType.EXECUTION_DENIED and status: 'error'. Cost: a denied call and a wasted turn inside a fork capped at FORK_DEFAULT_MAX_TURNS = 200 (fork-subagent.ts:41), repeated once per file the directive needs, with the authoritative restriction block — not stale boilerplate — as the source of the false belief.
Witness:
probe/grep output recorded by the verifier: — **confirmed (high confidence)** — Suggestion witness: [probe] CodeModeOnly, subagent configured tools: ['exec','read_file','write_file'], plain fork — the real exported builders give defaultExecutionToolNames: ["read_file","write_file"] → executionAllowedTools (direct): ["read_file","write_file"], and the rendered block reads: You may call exec and tools matched by this direct-call allowlist: ["read_file","write_fi
Suggested fix: Make the sentence mode-aware: thread the tool mode into buildChildMessage (the caller already has agentConfig.getToolMode?.()) and, when the mode is code_mode_only, render "You may call exec; ordinary tools are reachable only through it." while keeping the nested line, instead of naming a direct-call allowlist that declares nothing.
The fix must not violate this existing fact: The same sentence must stay true in hybrid — isToolExecutionAllowed returns true for EXEC whenever the mode is code-mode-enabled and executionAllowedTools is defined (packages/core/src/agents/runtime/agent-core.ts:2012-2019: if ((toolName === ToolNames.EXEC && isCodeModeEnabled(this.runtimeContext.getToolMode?.())) || …) { return true; }), and ordinary allowlist names are declared there (tool-registry.ts:1523-1538), so a mode-aware fix must not strip the enumeration for `code_mod
Acceptance criterion: A buildChildMessage case in packages/core/src/tools/agent/agent.test.ts for the CodeModeOnly rendering asserting the message contains Inside exec, only these exact nested tool names are permitted: ["read_file"] and does not contain direct-call allowlist: ["read_file"]; goes red while the sentence is unconditional. Please prove it by removing the fix and confirming that test goes red.
中文说明
新增的这句话无条件地断言存在一个直接调用白名单，但进入该嵌套分支的条件是 isCodeModeEnabled（两种 code 模式都会进），而在 code_mode_only 下并不存在可直接调用的普通工具，提示语因此会误导子智能体。
— qwen3.8-max via Qwen Code /review (v0.25.0)
| ? `\n\nTOOL EXECUTION RESTRICTION: | ||
| You may call exec and tools matched by this direct-call allowlist: ${JSON.stringify(executionAllowedTools ?? [])}. | ||
| Inside exec, only these exact nested tool names are permitted: ${JSON.stringify(nestedExecutionAllowedTools)}. | ||
| Nested permission does not permit direct calls to those tools.` |
There was a problem hiding this comment.
[Suggestion] R1-39: N29: In the new nested branch, sentence 3 denies direct calls to the whole nested list while sentence 1 grants direct calls to the direct list — and the two lists overlap by construction, so for any overlapping name the restriction block grants and forbids the same call in consecutive lines.
The PR's own new fixture produces the overlap: agent.test.ts:3605-3621 ('inherit bounded default', a plain fork — no fork_tools) runs inside runWithAgentConfiguredToolAllowlist(['read_file']) + runWithCodeModeAllowedNames(['read_file','write_file']) and asserts executionAllowedTools: ['read_file'] with nestedExecutionAllowedTools: ['read_file','write_file']. buildChildMessage therefore renders: "You may call exec and tools matched by this direct-call allowlist: ["read_file"]. / Inside exec, only these exact nested tool names are permitted: ["read_file","write_file"]. / Nested permission does not permit direct calls to those tools." — "those tools" resolves to ["read_file","write_file"], so the third line withdraws the read_file grant the first line just made. Full coincidence is the common plain-fork case: with requestedTools === undefined, isRequestedByFork returns true for every name (agent.ts:1850-1851), so the direct list (buildParentBoundExecutionAllowlist(defaultExecutionToolNames)) and the nested list (buildParentBoundExecutionAllowlist(getCurrentCodeModeAllowedNames() ?? defaultExecutionToolNames, true)) run the same filters over the same base and come out equal — e.g. both ["read_file","write_file"]. The child resolves the conflict conservatively: it stops calling read_file directly even though isToolExecutionAllowed('read_file') is true and the declaration is present (hybrid), routing every read through an extra exec runtime round trip, or skipping the tool and reporting the directive blocked. That is the same "the child obeys and the grant is discarded" outcome this branch was added to prevent.
Witness:
probe/grep output recorded by the verifier: — **confirmed (high confidence)** — Suggestion witness: [probe] rendered from the built product with the PR's own fixture values (agent.test.ts:3596-3621 asserts direct ['read_file'], nested ['read_file','write_file']): You may call exec and tools matched by this direct-call allowlist: ["read_file"]. Inside exec, only these exact nested tool names are permitted: ["read_file","write_file"]. Nested permission does not
Suggested fix: Scope sentence 3 to the nested-only remainder instead of the whole nested list — e.g. const nestedOnly = nestedExecutionAllowedTools.filter((name) => !(executionAllowedTools ?? []).includes(name)); and emit the denial only when nestedOnly.length > 0, naming that subset; or reword so it cannot conflict: "A nested listing alone does not grant a direct call; direct calls are governed only by the allowlist above."
The fix must not violate this existing fact: packages/core/src/tools/agent/fork-subagent.ts:421-426 — the nested branch is tested before the executionAllowedTools === undefined and length === 0 arms, so a reworded branch must still win for executionAllowedTools: [] (the shape agent.test.ts's 'bounds and persists an explicit exec fork' produces), or the deny-all text at line 430 returns and the nested grant is discarded.
Acceptance criterion: A buildChildMessage case in packages/core/src/tools/agent/agent.test.ts beside the existing shapes at 3089-3104: with executionAllowedTools: ['read_file'] and nestedExecutionAllowedTools: ['read_file','write_file'], assert the rendered restriction does not deny direct calls to read_file while still naming write_file as nested-only; and with executionAllowedTools: ['read_file'], nestedExecutionAllowedTools: ['read_file'], assert no denial sentence is emitted at all. Both go re Please prove it by removing the fix and confirming that test goes red.
中文说明
在新的嵌套分支里，第三句否认对整个嵌套列表的直接调用，而第一句又授予对直接列表的直接调用——两个列表存在重叠时，同一段提示语会自相矛盾。
— qwen3.8-max via Qwen Code /review (v0.25.0)
| const execTool = this.tools.get(ToolNames.EXEC); | ||
| const codeModeBindings = | ||
| this.config.getToolMode?.() === ToolMode.CodeMode && | ||
| execTool !== undefined && | ||
| this.isToolAvailable(execTool.name) && | ||
| this.isToolDeclared(execTool.name) |
There was a problem hiding this comment.
[Suggestion] R1-42: N30: The budget preload's new decoration gate tests whether exec is registered, available and declared, but the decoration it is estimating for (decorateCodeModeDeclarations, tool-registry.ts:1069) only happens when exec is among the declarations actually returned — so when exec is permission-deferred by a tools.eager allowlist, the estimate charges every candidate for an exec-signature block that will never be sent, and the all-or-nothing preload can refuse reveals the operator's budget would have allowed.
Hybrid session (tools.mode: "code_mode") with tools.eager: ["read_file", "write_file"] — an allowlist that omits exec. exec is not exempt (PermissionManager.isExemptFromEagerAllowList, permission-manager.ts:885-895, exempts structured_output, plan-lifecycle tools, task_stop, tool_call, tool_search, mcp__*, computer_use__* — not exec), so getToolRegistrationStatus returns 'deferred', it is registered through registerPermissionDeferredFactory (config.ts:12252) and lands in permissionDeferred. tool_search/tool_call stay declared (exempt), so the preload runs rather than early-returning. Now isToolDeclared('exec') and isToolAvailable('exec') are both true, so codeModeBindings is built and each deferred candidate is measured as JSON.stringify(augmentDeclarationForCodeMode(tool.schema, binding)) — raw schema plus `\n\nexec tool declaration:\n```ts\ndeclare const tools: { <jsName>(args: <full rendered parameter type>): Promise<CodeModeToolResult>; };\n``` ` (code-mode.ts:219-231 + bindingSignature), which for an MCP tool with a real schema is often as large as the schema itself. But getFunctionDeclarations() filters exec out at tool-registry.ts:1048-1054 (tool.alwaysLoad || !this.isDeferredAndHidden(tool.name)), so decorateCodeModeDeclarations hits its !tools.some(name === EXEC) early return and reveals the raw schemas. Result: with a budget sized to the raw deferred schemas, estimatedTokens > budgetTokens, the preload returns 0, and every deferred tool stays behind the bridge — extra tool_search round trips on a session whose declarations never grew by the amount that was charged. This is the opposite direction from R1-29 (which is an undercount of the exec-side ALL_TOOLS growth when the bridge is present); both live in the same estimate, and a fix for one must not cancel the other.
Witness:
probe/grep output recorded by the verifier: — **confirmed (high confidence)** — Suggestion witness: [probe] hybrid registry, deferred (a shouldDefer MockTool) plus exec registered through registerPermissionDeferredFactory + warmAll(), budget = the raw schema cost: ARM A (exec permission-deferred): exec present in getFunctionDeclarations(): false raw schema cost 37 tokens | charged (decorated) 70 | measured reveal threshold 70 ACTUAL declaration growth on revea
Suggested fix: Make the gate match the condition the decoration actually uses, i.e. that exec will be among the returned declarations: ts const execDeclaredEagerly = execTool !== undefined && this.isToolAvailable(execTool.name) && this.isToolDeclared(execTool.name) && (execTool.alwaysLoad || !this.isDeferredAndHidden(execTool.name)); const codeModeBindings = this.config.getToolMode?.() === ToolMode.CodeMode && execDeclaredEagerly ? new Map(/* unchanged */) : undefined;
The fix must not violate this existing fact: if (!tools.some((tool) => tool.name === ToolNames.EXEC)) { return tools.map((tool) => tool.schema); } — packages/core/src/tools/tool-registry.ts:1069-1071, reached because getFunctionDeclarations()'s filter is tool.alwaysLoad || !this.isDeferredAndHidden(tool.name) (tool-registry.ts:1052-1053). The gate must key on exec's own declared/eager state only — not on the bridge: tool_search and tool_call are exempt from tools.eager (`canonicalName === ToolNames.TOOL_CALL || canonicalName
Acceptance criterion: packages/core/src/tools/tool-registry.test.ts, beside the added counts CodeMode declaration decoration toward the budget (lines 1060-1095): same fixture (toolMode: 'code_mode', a shouldDefer MockTool named deferred, rawBudget = tokensFor(directTool)), but register exec through registry.registerPermissionDeferredFactory('exec', async () => new MockTool({ name: 'exec' })) and await registry.warmAll() (the pattern already used at tool-registry.test.ts:213 and :1340). Assert `codeMod Please prove it by removing the fix and confirming that test goes red.
中文说明
预算预加载新增的 decoration 门控检测的是 exec 是否已注册、可用且已声明，但它要估算的那次 decoration 用的是另一个条件，二者并不等价，估算因此可能在门控通过时并不适用。
— qwen3.8-max via Qwen Code /review (v0.25.0)
| // An agent's target allowlist does not identify its declared | ||
| // direct, bridge, or exec routes. Do not advertise session routes. |
There was a problem hiding this comment.
[Suggestion] R1-47: N32: The reachability rule this hunk states is enforced only on the image-overview branch; the omni-delivery branch that returns 48 lines earlier inside the same case 'image' still advertises zoom_image unconditionally, so on an omni-policy session the new gate never executes for images at all.
tools.mode: "code_mode_only" (or "code_mode") plus a subagent whose surface withholds zoom_image, on a model where modalities.image is true and config.isOmniEnabled?.() / omni.isOmniDeliveryActive(config) hold — all model/policy facts, independent of tool mode (fileUtils.ts:1389-1410). processSingleFileContent takes if (omniModule) { return await omniModule.readMediaViaOmniDelivery({…}) } at fileUtils.ts:1638-1650 and never reaches the changed code. packages/core/src/omni/index.ts:1189-1196 then pushes Image <name>: full resolution WxH px. Use zoom_image for a closer look at details. (or the degradation variant Use zoom_image to inspect details — it reads the original file.) with no registry, tool-mode, or ambient-allowlist check. The agent emits a zoom_image call and AgentCore.processFunctionCalls rejects it with Tool "zoom_image" not found. Tools must use the exact names provided. (agent-core.ts:2201-2206) — one wasted turn, which is exactly the cost the deleted #12271 comment and this new gate exist to prevent. The identical agent on a non-omni model gets '' and does not waste the turn, so after this diff whether the invariant holds depends on the session's model policy rather than on the agent's surface.
Witness:
not run — probe/mutation via `review scratch-tree` and `base-tree`: unavailable in this environment (repo-local git includeIf resolves to a missing credentials file, so scratch-tree refuses to create a tree; the base build timed out), so the verdict rests on a line-by-line source trace plus repo-wide greps
Suggested fix: Hoist the route resolution (registry facts + getCurrentCodeModeAllowedNames()) into one helper that returns the hint string, call it before the if (omniModule) early return, and pass the result into readMediaViaOmniDelivery so its two text variants name zoom_image / tools.zoom_image / nothing under the same rule the overview branch now uses.
The fix must not violate this existing fact: packages/core/src/omni/index.ts:1183-1187 — "calling them 'full resolution' would contradict the disclosure pushed right below and steer the model away from zoom_image, the exact remedy for degradation-stripped detail (it reads the original from disk)". Suppression must key on the route being genuinely absent for the calling agent, not on the degradation/disclosure path, or the disclosure and the hint start contradicting each other again.
Acceptance criterion: packages/core/src/omni/index.test.ts:410-464 (adds a resolution + zoom_image hint part for images) currently pins the unconditional text. Add a sibling case that drives the same delivery with the route absent — e.g. wrap in runWithCodeModeAllowedNames(['read_file'], …), or a registry double whose getFunctionDeclarations() omits zoom_image — and assert the pushed part's text does not contain zoom_image. It is red while the omni branch ignores reachability. Please prove it by removing the fix and confirming that test goes red.
中文说明
这段 hunk 所陈述的可达性规则只在 image-overview 分支上被执行；同一个 case 'image' 内提前 48 行 return 的 omni-delivery 分支并不受它约束，形成同族分支的不对称。
— qwen3.8-max via Qwen Code /review (v0.25.0)
| | `abortController` | `AbortController` | - | Controller to cancel the query session. Call `abortController.abort()` to terminate the session and cleanup resources. | | ||
| | `debug` | `boolean` | `false` | Enable debug mode for verbose logging from the CLI process. | | ||
| | `maxSessionTurns` | `number` | `-1` (unlimited) | Maximum number of conversation turns before the session automatically terminates. Must be an integer. A turn consists of a user message and an assistant response. | | ||
| | `coreTools` | `string[]` | - | Uses the legacy `coreTools` / CLI `--core-tools` allowlist semantics. If specified, only matching core tools are registered for the session. This is the only allowlist-style option that restricts built-in tool registration; a whole-tool `permissions.deny` / `excludeTools` rule (and `tools.disabled` in settings.json) also removes a tool from the registry. `permissions.allow` in settings.json is pure auto-approval and never removes, demotes, or hides a tool (#10075). To keep a tool's schema out of the initial model request, use `tools.eager` in settings.json (requires restart, #9827) — `tool_search`, `tool_call`, `structured_output`, plan-mode lifecycle tools, `task_stop`, `mcp__*` and `computer_use__*` tools are exempt from that allowlist and keep their normal loading; tools demoted this way stay registered and reachable through `tool_search` + `tool_call` while both bridge tools are registered — when either is unregistered (`tools.toolSearch.enabled: false` denies both; a `tool_search` or `tool_call` deny rule, or a `tools.disabled` entry removes one) the demoted tools that remain hidden are absent from top-level declarations and cannot be reached through the bridge for that session, and a warning is written to the CLI process's stderr (SDK forwards it only with piped stderr and effective `debug` logging; an explicit `logLevel` always wins). These bridge and warning rules apply to direct and hybrid code modes. On the session surface in hybrid mode, while `exec` itself is registered (container and SSH execution warn and fall back to direct tools without it), exec retains callable nested bindings; their schemas are included in exec when either bridge tool is unavailable. CodeModeOnly discovers deferred schemas through top-level tool_search and invokes them through exec. It skips deferred preload and startup catalogs; tools.eager reduces the initial exec description. When search is unavailable in the current scope, exec includes all allowed signatures. In Hybrid mode, AgentCore excludes tools still hidden by tools.eager from nested bindings. In both code modes, agent allowlists that do not grant `exec` narrow nested bindings. Inheriting or explicitly granting `exec` keeps all otherwise admitted ordinary code-mode-callable bindings. An execution allowlist that mentions any MCP tool additionally restricts MCP bindings to matching exact names or server patterns. In direct and hybrid modes they stay registered, so a direct call by their own name is still evaluated and approved normally — except tools also listed in `tools.visible`, which are declared upfront, and sessions whose live history contains a direct call to a still-hidden demoted tool, which any tool-set refresh (resume, MCP discovery, the first plan-mode entry in a session, a subagent definition change) re-declares.; to remove a tool entirely, use a whole-tool `excludeTools` / `permissions.deny` rule — a rule with a specifier (such as `'Bash(rm *)'`) only denies matching invocations at runtime. MCP tools are exempt from deny-based removal: hide them with the per-server `excludeTools` / `tools.disabled` filters instead (deny still blocks their calls at runtime). Example: `['read_file', 'edit', 'run_shell_command']`. | |
There was a problem hiding this comment.
[Suggestion] R1-48: N33: The rewritten coreTools contract is stored twice, byte-identical, in two files this diff edits — nothing generates one from the other and nothing asserts they agree.
I extracted the last cell of the coreTools row from both files at HEAD and compared: len == 3226 for each and a == b is True. This diff had to apply the same ~1.5 KB rewrite to both (docs/developers/sdk-typescript.md hunk @@ -53,27 +53,27 @@ carries the identical added sentence "On the session surface in hybrid mode…", and the identical removed sentence "These bridge and warning rules apply to direct tool mode."). The next semantic correction — e.g. the hybrid/exec condition in Finding 1, or the canSearchDeferredSchemas = false hardcode on the filtered path at tool-registry.ts:1538 that makes "when either bridge tool is unavailable" wrong for subagents — has to be found and made in both cells or the two published SDK references contradict each other, and an integrator who reads the stale one builds a client against a contract the shipped CLI no longer honours. Nine independently falsifiable statements were added to each copy in this diff and no test pins any of them in either file.
Witness:
``` ARM 1 intact tree (both real files): PASS: coreTools description cells identical (3226 chars) ARM 2 one sentence edited in a /tmp copy (positive control): FAIL: coreTools description cells diverged … first difference at char 1411: A: …always wins). These bridge and warning rules apply to direct and hybrid code modes. On the session surface… B: …always wins). These bridge and warning rules apply to direct code mode only. On the session surface… ARM 3 pre-PR cells replayed from the diff's own '-' lines: FAIL: old-docs-cell.md: 2154 chars / old-readme-cell.md: 2451 chars … first difference at
Suggested fix: Keep one authoritative copy and derive the other (a small generate step, or a <!-- prettier-ignore -->-style include), or — cheapest — add a test that reads both files, extracts the coreTools row's description cell, and asserts the two strings are equal, so drift fails CI instead of shipping.
Acceptance criterion: A new test (e.g. packages/sdk-typescript/src/readme-doc-sync.test.ts) that fails when one cell is edited without the other; it is red the moment either file's coreTools description diverges. Please prove it by removing the fix and confirming that test goes red.
中文说明
改写后的 coreTools 契约在本 diff 编辑的两个文件里逐字节重复存放，既没有由一方生成另一方，也没有任何测试断言二者一致。实测：两文件 22 个共有选项行的 description 全部逐字节相同（coreTools 行为 3226 字符）；在 merge base 上二者曾分别为 2154 与 2451 字符、互相矛盾，正是本 PR 手工同步修好的。同一句新增文案在 HEAD 上散布于 6 个文件、11 处手工维护位置（另有 3 处由 CI 固定），而本 diff 手工改了其中 5 个文件。
— qwen3.8-max via Qwen Code /review (v0.25.0)
|
🤖 AutoFix updated a stale base — the fix did not pass verification, but this PR was behind What I found before stopping: See the Qwen Autofix agent step logs for model/tool output. 中文说明🤖 AutoFix 更新了一个过期的 base —— 修复未通过验证，但本 PR 落后于 Run log: https://github.com/QwenLM/qwen-code/actions/runs/37692965975 🧠 Handled by Qwen Code · model/模型 |
qwen-code-review-bot
left a comment
qwen-code-review-bot
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed.
17 Suggestion-level finding(s) this review confirmed are already reported on this PR and are not repeated:
- R1-2 ?:64 — already reported on this PR (round 1 inline thread)
- R1-3 ?:87 — already reported on this PR (round 1 inline thread)
- R1-4 ?:9 — already reported on this PR (round 1 inline thread)
- R1-5 ?:123 — already reported on this PR (round 1 inline thread)
- R1-6 ?:3 — already reported on this PR (round 1 inline thread)
- R1-13 ?:581 — already reported on this PR (round 1 inline thread)
- R1-19 ?:654 — already reported on this PR (round 1 inline thread)
- R1-20 ?:1089 — already reported on this PR (round 1 inline thread)
- R1-23 ?:352 — already reported on this PR (round 1 inline thread)
- R1-25 ?:657 — already reported on this PR (round 1 inline thread)
- R1-26 ?:950 — already reported on this PR (round 1 inline thread)
- R1-27 ?:2686 — already reported on this PR (round 1 inline thread)
- R1-29 ?:468 — already reported on this PR (round 1 inline thread)
- R1-31 ?:13478 — already reported on this PR (round 1 inline thread)
- R1-33 ?:1470 — already reported on this PR (round 1 inline thread)
- R1-37 ?:424 — already reported on this PR (round 1 inline thread)
- R1-41 ?:1347 — already reported on this PR (round 1 inline thread)
Not reviewed: build-and-test — 'Integration Tests (CLI, No Sandbox)' was skipped in CI and its suite did not run locally (Agent 7 ran per-package vitest suites only).
Not explored to full depth (tool budget reached): "agent reverse-audit (round 1)": I did not open buildSubagentContextOverride to check whether the child registry AgentCore.willHaveSkillTool() probes (agent-core.ts:663, this.runtimeContex…; "agent reverse-audit (round 1)": I did not verify whether registerLazy (config.ts:12486-12492, registerExecIfEnabled ) honours --exclude-tools / coreTools for exec ; if it does, the uncon…; "agent reverse-audit (round 2)": whether tools.eager can demote exec itself in hybrid mode — the row's exempt list ( tool_search , tool_call , structured_output , plan-mode lifecycle tool…; "agent reverse-audit (round 1)": the new visible description claim in packages/vscode-ide-companion/schemas/settings.schema.json:1584 ("In Code Mode Only, listed tools have signatures in th…; chunk 5: verifying the tools.visible half of the CodeModeOnly "initial documented bindings" claim in context-cost.md against the exec prompt-assembly code., and 6 more.
Not reviewed: reverse audit — stopped before round 3 by the review time budget.
Deferred under the convergence posture (round 2, not a blocker) — recorded, not requested in this round:
?:103 — [review] The new Chinese version drops the English clause whose…?:39 — [review] The new effective-mode table claims an omitted…?:418 — [review] The new tools.mode row states the hybrid nested-binding…?:56 — [review] The rewritten bullet still states the incomplete-bridge…?:4471 — [review] The only legacy-setting test sets codeModeOnly alone, so…?:186 — [review] The rename swept settings.md , context-cost.md ,…?:47 — [review] This diff makes one ~800-character normative paragraph…?:626 — [review] The forced-write clause is unconditional, so tools.mode …?:448 — [review] Billing the detail rows from the declarations themselves…?:1243 — [review] Inserting the two new Hybrid rows immediately above the…?:161 — [review] The mcp__offline__read_* arm — the fixture whose only…?:735 — [review] The mcp__payments__charge half of this assertion pair…?:972 — [review] None of the nine new hybrid ( ToolMode.CodeMode ) tests…?:2001 — [review] The third disjunct of the forNestedBinding arm re-runs…?:2677 — [review] The behaviour this hunk actually adds — the allowlist…?:122 — [review] The paragraph this diff adds contradicts the two…?:160 — [review] toolConfigAllowsSkill grew a registry-aware second…?:208 — [review] The new "invalid mode" case in registers exec in both…?:351 — [review] These two new assertions bless an exec description that…?:557 — [review] The new caught-vs-uncaught failure guidance added to…- …and 16 more (see the run report)
中文说明
仅完成部分审查，审查缺口已披露。
本轮确认的 17 条建议级发现已在 PR 上报告过，不再重复发布（列表见上方英文部分）。
未审查（原文为英文）：build-and-test — 'Integration Tests (CLI, No Sandbox)' was skipped in CI and its suite did not run locally (Agent 7 ran per-package vitest suites only).
未探索到全部深度（达到工具调用预算）："agent reverse-audit (round 1)"：I did not open buildSubagentContextOverride to check whether the child registry AgentCore.willHaveSkillTool() probes (agent-core.ts:663, this.runtimeContex…；"agent reverse-audit (round 1)"：I did not verify whether registerLazy (config.ts:12486-12492, registerExecIfEnabled ) honours --exclude-tools / coreTools for exec ; if it does, the uncon…；"agent reverse-audit (round 2)"：whether tools.eager can demote exec itself in hybrid mode — the row's exempt list ( tool_search , tool_call , structured_output , plan-mode lifecycle tool…；"agent reverse-audit (round 1)"：the new visible description claim in packages/vscode-ide-companion/schemas/settings.schema.json:1584 ("In Code Mode Only, listed tools have signatures in th…；chunk 5：verifying the tools.visible half of the CodeModeOnly "initial documented bindings" claim in context-cost.md against the exec prompt-assembly code.，另有 6 条。
未审查：反向审计——评审时间预算不足，未能开始第 3 轮。
收敛姿态下延后（第 2 轮，非阻断）——已记录，本轮不要求修改：共 36 条（原文未翻译，列表见上方英文部分）。
— qwen3.8-max via Qwen Code /review (v0.25.0)
This branch has not been deployed
6 participants
What this PR does
Adds a Codex-aligned
tools.modeenum withdirect,code_mode, andcode_mode_only.code_modekeeps ordinary tools directly callable while also exposing the isolatedexecJavaScript tool; each visible ordinary tool carries its nested JavaScript declaration, whileexecretains compactALL_TOOLSmetadata.code_mode_onlypreserves the strict exec-only ordinary-tool surface, anddirectremains the default.Updates filtered subagent tool surfaces so direct declarations and nested exec bindings honor the same allowlists, exposes the enum in Settings as Default / Code Mode / Code Mode Only, regenerates the settings JSON schema, and removes the former
tools.codeModeOnlyboolean setting entirely.Adds synchronized English and Chinese design documents, and brings the existing CodeModeOnly design up to the repository's bilingual documentation standard (English, Chinese).
Why it's needed
Qwen Code previously offered only the default direct tool surface or the strict CodeModeOnly surface. Models could not combine direct calls for simple operations with
execorchestration for multi-tool workflows. A three-value enum matches Codex'sToolModenaming and gives users one explicit setting for all supported exposure modes.Reviewer Test Plan
How to verify
tools.modeunset or set it todirect, restart Qwen Code, and confirm ordinary tools remain available whileexecis absent.tools.modetocode_mode, restart, and confirm both ordinary tools andexecare available. Confirm ordinary tool descriptions include theirtools.<name>(args)declaration and deferred tools remain discoverable throughtool_search.tools.modetocode_mode_only, restart, and confirm ordinary tools are available throughexecrather than as top-level calls, while direct control tools remain available.tools.codeModeOnlyis no longer present in the settings schema.--safe-modeor--barewhile either code mode is configured and confirm the effective mode isdirect.Automated verification completed: focused Core tests (121 tests), focused CLI tests (585 tests), a repeated settings-schema regression run (60 tests), full build, full typecheck, full lint, and both installed and branch-built CLI version smoke checks.
Evidence (Before & After)
Before: Settings exposed
Code Mode Only (Experimental)as a boolean withfalse/true.After: Settings exposes
Tool Mode (Experimental)as an enum displaying Default / Code Mode / Code Mode Only. Updated TUI snapshots cover the default display.Tested on
Environment (optional)
macOS, Node.js v24.18.0; local source build and package-level Vitest suites.
Risk & Scope
execdescription.tools.codeModeOnlyis removed. Replace{ "tools": { "codeModeOnly": true } }with{ "tools": { "mode": "code_mode_only" } }.Linked Issues
Related to #10377.
中文说明
本 PR 做了什么
新增与 Codex 对齐的
tools.mode枚举，支持direct、code_mode和code_mode_only。code_mode在保留普通工具直接调用能力的同时提供隔离的execJavaScript 工具；每个可见普通工具会携带自己的嵌套 JavaScript 声明，而exec保留精简的ALL_TOOLS元数据。code_mode_only保持严格的普通工具仅经exec调用模式，direct仍为默认值。更新经过过滤的子智能体工具面，使直接声明和
exec嵌套 binding 遵守同一组 allowlist；在设置界面中将该枚举显示为 Default / Code Mode / Code Mode Only；重新生成 settings JSON schema；并彻底移除原来的tools.codeModeOnly布尔设置。新增同步的英文和中文设计文档，并将现有 CodeModeOnly 设计补齐为仓库要求的双语文档（英文、中文）。
为什么需要
Qwen Code 此前只提供默认的直接工具面或严格的 CodeModeOnly 工具面。模型无法在简单操作中使用直接调用，同时在多工具工作流中使用
exec编排。三值枚举与 Codex 的ToolMode命名一致，并为所有支持的暴露模式提供一个明确的统一设置项。Reviewer 测试计划
如何验证
tools.mode或将其设为direct，重启 Qwen Code，确认普通工具仍可用且不存在exec。tools.mode设为code_mode并重启，确认普通工具和exec同时可用；确认普通工具描述包含各自的tools.<name>(args)声明，延迟工具仍可通过tool_search发现。tools.mode设为code_mode_only并重启，确认普通工具通过exec而不是顶层调用使用，同时直接控制工具仍然可用。tools.codeModeOnly。--safe-mode或--bare启动，确认有效模式为direct。已完成自动化验证：Core 相关测试（121 个）、CLI 相关测试（585 个）、再次运行的 settings schema 回归测试（60 个）、完整 build、完整 typecheck、完整 lint，以及已安装版本和分支构建版本的 CLI 版本 smoke 检查。
前后对比证据
之前：设置界面以
false/true布尔值展示Code Mode Only (Experimental)。之后：设置界面以枚举展示
Tool Mode (Experimental)，可选 Default / Code Mode / Code Mode Only。更新后的 TUI snapshot 覆盖默认显示。测试平台
环境（可选）
macOS、Node.js v24.18.0；本地源码构建和 package 级 Vitest 测试套件。
风险与范围
exec描述中重复。tools.codeModeOnly已移除。请将{ "tools": { "codeModeOnly": true } }替换为{ "tools": { "mode": "code_mode_only" } }。关联 Issue
关联 #10377。