Skip to content

feat: add hybrid code mode - #11854

Open
DragonnZhang wants to merge 3 commits into
mainfrom
dragon/add-codemode
Open

DragonnZhang wants to merge 3 commits into
mainfrom
dragon/add-codemode

Conversation

@DragonnZhang

Copy link
Copy Markdown
Collaborator

What this PR does

Adds a Codex-aligned tools.mode enum with direct, code_mode, and code_mode_only. code_mode keeps ordinary tools directly callable while also exposing the isolated exec JavaScript tool; each visible ordinary tool carries its nested JavaScript declaration, while exec retains compact ALL_TOOLS metadata. code_mode_only preserves the strict exec-only ordinary-tool surface, and direct remains 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.codeModeOnly boolean 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 exec orchestration for multi-tool workflows. A three-value enum matches Codex's ToolMode naming and gives users one explicit setting for all supported exposure modes.

Reviewer Test Plan

How to verify

  1. Leave tools.mode unset or set it to direct, restart Qwen Code, and confirm ordinary tools remain available while exec is absent.
  2. Set tools.mode to code_mode, restart, and confirm both ordinary tools and exec are available. Confirm ordinary tool descriptions include their tools.<name>(args) declaration and deferred tools remain discoverable through tool_search.
  3. Set tools.mode to code_mode_only, restart, and confirm ordinary tools are available through exec rather than as top-level calls, while direct control tools remain available.
  4. Open Settings and confirm Tool Mode cycles among Default, Code Mode, and Code Mode Only. Confirm tools.codeModeOnly is no longer present in the settings schema.
  5. Start with --safe-mode or --bare while either code mode is configured and confirm the effective mode is direct.

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 with false / 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

OS Status
🍏 macOS
🪟 Windows ⚠️
🐧 Linux ⚠️

Environment (optional)

macOS, Node.js v24.18.0; local source build and package-level Vitest suites.

Risk & Scope

  • Main risk or tradeoff: Hybrid mode adds a short nested-call declaration to each ordinary tool description, increasing prompt size slightly; complete schemas are not duplicated in the exec description.
  • Not validated / out of scope: Live-provider interactive runs and Windows/Linux local execution were not tested; CI covers supported platforms.
  • Breaking changes / migration notes: tools.codeModeOnly is removed. Replace { "tools": { "codeModeOnly": true } } with { "tools": { "mode": "code_mode_only" } }.

Linked Issues

Related to #10377.

中文说明

本 PR 做了什么

新增与 Codex 对齐的 tools.mode 枚举,支持 directcode_modecode_mode_onlycode_mode 在保留普通工具直接调用能力的同时提供隔离的 exec JavaScript 工具;每个可见普通工具会携带自己的嵌套 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 测试计划

如何验证

  1. 不设置 tools.mode 或将其设为 direct,重启 Qwen Code,确认普通工具仍可用且不存在 exec
  2. tools.mode 设为 code_mode 并重启,确认普通工具和 exec 同时可用;确认普通工具描述包含各自的 tools.<name>(args) 声明,延迟工具仍可通过 tool_search 发现。
  3. tools.mode 设为 code_mode_only 并重启,确认普通工具通过 exec 而不是顶层调用使用,同时直接控制工具仍然可用。
  4. 打开设置界面,确认 Tool Mode 可在 Default、Code Mode 和 Code Mode Only 之间切换,并确认 settings schema 中不再存在 tools.codeModeOnly
  5. 配置任一 code mode 后用 --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 覆盖默认显示。

测试平台

OS 状态
🍏 macOS
🪟 Windows ⚠️
🐧 Linux ⚠️

环境(可选)

macOS、Node.js v24.18.0;本地源码构建和 package 级 Vitest 测试套件。

风险与范围

  • 主要风险或取舍:混合模式会在每个普通工具描述中增加一段简短的嵌套调用声明,使 prompt 略微增大;完整 schema 不会在 exec 描述中重复。
  • 未验证 / 范围外:未进行真实 provider 的交互式运行,也未在 Windows/Linux 本地执行;支持平台由 CI 覆盖。
  • 破坏性变更 / 迁移说明:tools.codeModeOnly 已移除。请将 { "tools": { "codeModeOnly": true } } 替换为 { "tools": { "mode": "code_mode_only" } }

关联 Issue

关联 #10377

@DragonnZhang

Copy link
Copy Markdown
Collaborator Author

E2E test report

  • Baseline CLI: globally installed qwen reports 0.23.3-nightly.20260913.faa395885e.
  • Branch CLI: the built CLI reports 0.23.3.
  • Automated behavior coverage passed for all three enum values, safe/bare fallback to direct, direct/hybrid/only declaration surfaces, deferred-tool discovery, filtered subagent allowlists, and the Settings enum/default rendering.
  • Focused Core: 121 tests passed.
  • Focused CLI: 585 tests passed; the 60-test settings-schema suite was rerun after the final removal assertion and passed.
  • Repository checks: npm run build, npm run typecheck, and npm run lint passed. The first build attempt was interrupted by a transient SIGABRT in the Telegram workspace; that workspace passed independently and the complete build passed on retry.
  • Live-provider interactive execution was not run locally. Reviewer verification steps for direct, code_mode, and code_mode_only are included in the PR body.

@qwen-code-ci-bot

qwen-code-ci-bot commented Sep 14, 2026

Copy link
Copy Markdown
Collaborator

Qwen Triage finishedview run. See the stage comments in this thread for the result.

Qwen Triage 已完成 —— 查看运行。结果见本线程中的各阶段评论。

@qwen-code-ci-bot

Copy link
Copy Markdown
Collaborator

Thanks for the PR!

Template looks good ✓

Problem: real and already scoped by the project, not theoretical hardening. #10377 explicitly deferred this half of the work — "本期在 Hybrid CodeMode 与 CodeModeOnly 之间选择 CodeModeOnly 先行;Hybrid CodeMode 作为后续模式" — and #10607 closed that issue having delivered only the CodeModeOnly mode. So this is the planned follow-up, and the gap it fills is documented. Worth noting for the record: the body says "Related to #10377" without a closing keyword, so GitHub reports no closing reference and the duplicate/already-fixed check has nothing to fire on — the correct outcome here, since #10607 does not subsume this diff. The underlying limitation is also observable from the code rather than argued: a boolean tools.codeModeOnly cannot express three states, so adding a third mode required replacing it.

Direction: aligned. The repo already ships two of Codex's three ToolMode values; this adds the missing middle one and collapses the boolean into the enum Codex uses, so the setting name and values match the thing being modelled. Leaving direct as the default and forcing it under --safe-mode / --bare keeps the rollback story identical to the one the CodeModeOnly design doc already promises. I checked the reference CHANGELOG for a matching signal and found no direct mention of code mode or programmatic tool calling there — the stronger signal is the project's own roadmap issue, which names this mode explicitly.

Size: core paths are touched (packages/core/src/**, packages/*/src/config/**, packages/*/src/tools/**), so the breakdown: 168 production logic lines, 325 test lines, 12 generated/schema lines (settings.schema.json), 20 snapshot lines, 359 doc lines. Production is well under the 500-line escalation threshold and the 1000-line large-PR advisory, so neither applies. The author has write access and this is a same-repo branch rather than a fork, so the external two-tier core gate does not apply either — I'm reporting the numbers for visibility, not gating on them.

Approach: the scope feels right and the implementation is genuinely minimal for what it does. It reuses the existing machinery rather than growing a parallel path — planCodeModeBindings, buildExecDeclaration, and getToolExposure all carry over unchanged, and the one new exported helper (isCodeModeEnabled) replaces five separate inline === CodeModeOnly comparisons instead of adding a sixth. The new tools.mode schema entry mirrors the existing tools.approvalMode enum shape exactly, which is the right thing to copy.

Two scope questions, neither a blocker:

  • Docs are 359 of the 884 changed lines, and most of that is not this feature — docs/design/code-mode-only.md gets rewritten for the new setting name and gains a brand-new 168-line Chinese translation. Bringing that doc up to the bilingual standard is legitimate work the repo asks for, but it is a separate concern from adding hybrid mode and would have been easy to split. Fine to keep if you'd rather not churn the branch.
  • In code_mode, an agent whose tools allowlist does not name exec still receives it. I can see the reasoning — it mirrors CodeModeOnly, where exec is the audited gateway and the nested allowlist does the real narrowing — and the nested bindings are correctly narrowed to that agent's own set, so there is no capability escalation. But in hybrid mode the direct surface is still usable, so unlike CodeModeOnly this is a choice rather than a necessity. Is granting exec to an agent that asked for ['read_file'] intended, or should the allowlist be able to omit it?

Risk: no elevated risk signals — none of the changed files match the revert-correlated high-risk paths.

Moving on to code review. 🔍

中文说明

感谢贡献!

模板完整 ✓

问题: 真实存在,而且是项目自己已经规划好的范围,不属于理论性加固。#10377 明确把这一半工作推后——"本期在 Hybrid CodeMode 与 CodeModeOnly 之间选择 CodeModeOnly 先行;Hybrid CodeMode 作为后续模式"——而 #10607 关闭该 issue 时只交付了 CodeModeOnly。所以本 PR 正是计划中的后续项,缺口有据可查。补充说明:正文写的是 "Related to #10377",没有关闭关键字,因此 GitHub 解析不出 closing reference,重复/已修复检查无从触发——这里的结果是对的,因为 #10607 并没有覆盖本 PR 的改动。这个限制也能从代码本身看出来:布尔值 tools.codeModeOnly 无法表达三种状态,要新增第三种模式就必须替换它。

方向: 对齐。仓库已经实现了 Codex 三种 ToolMode 中的两种,本 PR 补上缺失的中间一种,并把布尔值收敛为 Codex 使用的枚举,设置名和取值都与被建模的对象一致。默认仍为 direct,且 --safe-mode / --bare 强制 direct,回滚路径与 CodeModeOnly 设计文档已有的承诺一致。我查了参考 CHANGELOG,没有找到 code mode 或程序化工具调用的直接记录——更强的信号是项目自己的 roadmap issue,其中明确列出了这个模式。

规模: 触及核心路径(packages/core/src/**packages/*/src/config/**packages/*/src/tools/**),明细为:168 行生产代码、325 行测试、12 行生成/schema(settings.schema.json)、20 行 snapshot、359 行文档。生产代码远低于 500 行升级阈值和 1000 行大 PR 提示阈值,两者均不适用。作者具有 write 权限且为同仓库分支(非 fork),因此外部两级核心门禁同样不适用——列出数字只为透明,不作为门禁依据。

方案: 范围合理,实现相对于目标确实做到了最小化。它复用了既有机制而不是新长出一条平行路径——planCodeModeBindingsbuildExecDeclarationgetToolExposure 全部原样沿用,唯一新增的导出辅助函数 isCodeModeEnabled 替换了五处内联的 === CodeModeOnly 比较,而不是再加第六处。新的 tools.mode schema 条目完全对齐既有 tools.approvalMode 枚举写法,这是正确的参照对象。

两个范围问题,都不是阻塞项:

  • 884 行改动里有 359 行是文档,其中大部分并不属于本特性——docs/design/code-mode-only.md 因设置改名而被重写,并新增了 168 行中文翻译。把该文档补齐到双语标准是仓库要求的正当工作,但它与"新增混合模式"是两件不同的事,本来很容易拆开。如果你不想让这个分支反复变动,保留现状也可以。
  • code_mode 下,tools allowlist 中没有 exec 的 agent 仍然会拿到 exec。我能理解理由——这与 CodeModeOnly 一致,exec 是经审计的 gateway,真正做收窄的是嵌套 allowlist——而且嵌套 binding 确实被正确收窄到该 agent 自己的集合,所以不存在能力提升。但在混合模式下直接调用面仍然可用,因此与 CodeModeOnly 不同,这里是一种选择而非必需。把 exec 交给只申请了 ['read_file'] 的 agent,是有意为之,还是应该允许 allowlist 把它排除掉?

风险: 无升级风险信号——改动文件均未命中与回滚相关的高风险路径。

进入代码审查 🔍

Qwen Code · qwen3.8-max-2026-09-02

Reviewed at 6ab42db9b41170014c3a3d7ac6168af41894dd8b · re-run with @qwen-code /triage

@qwen-code-ci-bot

qwen-code-ci-bot commented Sep 14, 2026

Copy link
Copy Markdown
Collaborator

Code review

The part worth a maintainer's time first. This diff converts five === ToolMode.CodeModeOnly comparisons in agent-core.ts to isCodeModeEnabled(...) and deliberately leaves twelve others alone. I checked all twelve against the code, and all twelve are correctly left as CodeModeOnly-only, because hybrid mode preserves the direct surface:

  • prompts.ts (codeModeOnlycodeModeToolCallExamples), reached via client.ts:411 and ArenaManager.ts:1115 — correct to leave. Those examples tell the model exec is its only ordinary surface, which is false in code_mode; the generated exec description carries the protocol instead.
  • coreToolScheduler.ts:2737 and acp-integration/session/Session.ts:12448 model-source fail-closed guards — correct to leave. They exist to reject direct calls to code-mode-callable tools, which code_mode must permit.
  • client.ts:1183 / client.ts:2267 history reveal, client.ts:1747 preloadDeferredToolsWithinBudget, tool-registry.ts:1009 getDeferredToolSummary — correct to leave. All are suppressed in CodeModeOnly only because tool_search is hidden there; code_mode keeps deferred discovery, and the new preserves deferred discovery in CodeMode test pins that.
  • fileUtils.ts:1572 zoom hint — correct to leave; zoom_image stays directly callable.
  • tool-registry.ts:852 / 1113 — the two entry points the diff does extend.

That enumeration is the load-bearing part of this review. Getting it wrong in either direction breaks something: converting any of the first four groups would break the direct surface, and missing one that should have been converted would break code mode. A reviewer who skips it has to redo it from scratch.

The nested-binding enforcement also carries over correctly. dispatchCodeModeTool (coreToolScheduler.ts:1697) reads parent.codeModeAllowedToolNames and is mode-agnostic, and agent-core.ts now populates it for both code modes, so a narrowed subagent's exec really is restricted to its own allowlist at dispatch time rather than only in the description it was shown.

Finding 1 — an unrecognized tools.mode value fails open and silently enables exec (blocking)

registerExecIfEnabled in packages/core/src/config/config.ts changed from an allowlist to a denylist:

- if (this.getToolMode() !== ToolMode.CodeModeOnly) return;
+ if (this.getToolMode() === ToolMode.Direct) return;

Nothing validates the value on the way in. loadCliConfig passes settings.tools?.mode ?? ToolMode.Direct straight through, Config stores params.toolMode ?? ToolMode.Direct, and loadSettings only warns about unknown keys (settings.ts:276) — it never checks a value against the schema's options. getSettingsSchema() has three consumers (settingsSchema.ts itself, key-existence in settings.ts, dialog ordering in settingsUtils.ts) and none of them validates. The regenerated settings.schema.json enum is an editor artifact produced by scripts/generate-settings-schema.ts, not a runtime validator.

So "tools": { "mode": "code-mode" } — a hyphen where the value wants an underscore — produces this:

  1. registerExecIfEnabled sees a value that is not direct, so exec is registered.
  2. getFunctionDeclarations() matches neither the CodeModeOnly branch nor the new CodeMode branch, so it returns raw schemas. exec is declared with ExecTool's own one-line description — 'Execute JavaScript in an isolated runtime.' (exec.ts:220) — carrying none of the generated tools.* / text() / ALL_TOOLS protocol, and no ordinary tool receives its nested declaration.
  3. If the model calls it anyway, dispatchCodeModeTool runs with parent.codeModeAllowedToolNames undefined (only agent-core sets it), so isCodeModeToolCallAllowed(name, 'code_mode', undefined) admits every code-mode-callable tool.

No error, no warning: the user asked for a mode and silently got an undocumented one. To be fair about blast radius, this is not a privilege escalation — every one of those tools is directly callable in that state anyway, so exec adds no reach. The defect is the inverted fail-closed posture and the silent, half-configured surface.

The repo already has the convention for this exact case, in the same file: parseApprovalModeValue() (config.ts:151) normalizes an enum read from settings and, per its own doc comment, "Throws for values boot would reject." tools.mode has no equivalent. Narrowing params.toolMode in Config so an unrecognized value resolves to direct would fix it; keeping registerExecIfEnabled an allowlist (=== CodeMode || === CodeModeOnly) is worth doing regardless, so the registration gate fails closed independently of however the value was parsed.

No test covers an unrecognized value. Since the untested path is the defect here, a test belongs with the fix rather than after it.

Finding 2 — tools.eager silently disables all of code_mode (suggestion)

exec registers through registerLazyTool (config.ts:1040010200), which asks the PermissionManager. isExemptFromEagerAllowList covers structured_output, the plan-lifecycle tools, task_stop, tool_search, mcp__*, and computer_use__* — but not exec. So an active tools.eager allowlist that omits exec demotes it through registerPermissionDeferredFactory, which makes isDeferredAndHidden('exec') true, which drops it from the filter chain in getFunctionDeclarations(), which makes decorateCodeModeDeclarations hit its !tools.some((tool) => tool.name === ToolNames.EXEC) bail and return plain schemas.

Net effect: no exec declaration and no nested declaration on any tool — code_mode degrades to direct wholesale, silently. code_mode_only is immune because getCodeModeFunctionDeclarations includes exposure === 'exec' unconditionally.

Two reasons this is a suggestion and not a blocker: it is recoverable, since a tool_search reveal of exec flips isDeferredAndHidden and the decoration comes back; and the bail itself is correct for the subagent case where exec genuinely is not allowed. The problem is specifically the eager-allowlist interaction. Either exempting exec there, or logging when a code mode is configured but exec ended up deferred, would close it. Note this is also the combination a user enabling code_mode is most likely to hit, since both settings exist to control prompt size.

Nit

isCodeModeEnabled is added to the core barrel (packages/core/src/index.ts) but nothing outside packages/core imports it — agent-core.ts uses the relative ../../tools/code-mode.js. Harmless, and symmetric with the neighbouring getToolExposure / isCodeModeToolCallAllowed exports, so keeping it is fine; flagging only because an added export should have its read sites named.

Test coverage

Good, and aimed at the right things. code-mode.test.ts adds the hybrid declaration surface (including that a direct-only tool like agent is not augmented, and that exec keeps ALL_TOOLS without duplicating schemas), the filtered-surface scoping, the exec-unavailable bail, and deferred discovery. agent-core.skill-gate.test.ts adds both halves of the two-set path: a narrowed agent whose direct surface is [exec, read_file] with bindings restricted to read_file, and an exec-only agent whose direct surface is [exec] alone while its inherited bindings still cover read_file and write_file. keeps Direct declarations unchanged was strengthened from name-list equality to full schema equality, which is the right guard for the "byte-for-byte unchanged in direct" constraint the design doc states.

Gaps: nothing covers an unrecognized tools.mode (Finding 1) or tools.eager combined with a code mode (Finding 2).

The bilingual design docs meet docs/design/README.md — matching section order, matching tables, matching acceptance criteria, reciprocal language links in both directions. The new code-mode-only.zh-CN.md is a faithful full translation rather than a summary, including the non-goals list. One small thing for later: that non-goals list still names "Hybrid exposure of direct and code calls," which code-mode.md now implements. It is accurate as a statement of the CodeModeOnly MVP's scope, so nothing is wrong, but a cross-link between the two docs would save the next reader the double take.

CI test evidence

This is an unattended CI run, so per the gate rules I did not build, run, or execute anything from this PR. The evidence below is the PR's own CI, read from the check-run API for the reviewed commit.

At fetch time Qwen Code CI is still in progress: Test (ubuntu-latest, Node 22.x) and Lint & Static (ubuntu-latest, Node 22.x) have not concluded, so the unit suite and lint results do not exist yet and I am not guessing at them. No check is red.

Two things about the current picture that matter more than the green rows. First, Test (macos-latest, Node 22.x) and Test (windows-latest, Node 22.x) are skipped, not passed — so platform coverage rests entirely on the ubuntu job that is still running, while the PR's own Tested-on table records macOS ✅ with Windows and Linux ⚠️. Second, verify and tmux-testing are both skipped, so no sandboxed lane has exercised this change.

The author's E2E report in this thread is their own account of a local run — 121 focused Core tests, 585 focused CLI tests, and build/typecheck/lint passing on macOS. I am not adopting it as evidence and did not re-run any of it; I quote it only so the record shows what was and was not covered, and the author states plainly there that live-provider interactive execution was not run locally.

Final CI results for 6ab42db (auto-updated by the triage finalize job after CI completed):

Check Conclusion
Test (ubuntu-latest, Node 22.x) ❌ failure
Classify PR ✅ success
Desktop Shell (ubuntu-22.04) ✅ success
Desktop Shell (windows-2022) ✅ success
Integration Tests (no-AK, No Sandbox) ✅ success
Lint & Static (ubuntu-latest, Node 22.x) ✅ success
OpenTUI no-flicker gate ✅ success
TUI parity snapshots (ink vs opentui) ✅ success
web-shell E2E Smoke (ubuntu-latest, Node 22.x) ✅ success

One row per check name (latest run); skipped checks omitted; failures sort first. / 每个检查名一行(取最新一次运行),省略 skipped,失败项排在最前。

Sandboxed verification would settle what the unit suite cannot. @qwen-code /tmux — that Settings really cycles Default → Code Mode → Code Mode Only, and that a code_mode session actually presents both ordinary tools and a usable exec to the model. This is the PR's central behavioural claim and it is currently unsubstantiated end to end: the snapshot test pins only the default display, the declaration tests stop at the registry boundary, and by the author's own account no live-provider run happened. @qwen-code /verify would additionally settle the A/B constraint the design doc states — that direct declarations are byte-for-byte unchanged against the base build — which a schema-equality unit test asserts but does not compare against base.

中文说明

代码审查

先说最值得维护者关注的部分。 本 diff 把 agent-core.ts 中五处 === ToolMode.CodeModeOnly 比较改为 isCodeModeEnabled(...),并有意保留其余十二处不动。我逐一对照代码检查了这十二处,结论是全部保留正确,因为混合模式保留了直接调用面:

  • prompts.tscodeModeOnlycodeModeToolCallExamples),经 client.ts:411ArenaManager.ts:1115 到达——保留正确。这些示例告诉模型 exec 是它唯一的普通工具面,而这在 code_mode 下不成立;协议内容由生成的 exec 描述承载。
  • coreToolScheduler.ts:2737acp-integration/session/Session.ts:12448 的 model-source fail-closed 门禁——保留正确。它们的作用是拒绝对 code-mode-callable 工具的直接调用,而 code_mode 必须允许这类调用。
  • client.ts:1183 / client.ts:2267 的历史 reveal、client.ts:1747preloadDeferredToolsWithinBudgettool-registry.ts:1009getDeferredToolSummary——保留正确。它们在 CodeModeOnly 下被屏蔽,仅仅是因为那里隐藏了 tool_searchcode_mode 保留延迟发现,新增的 preserves deferred discovery in CodeMode 测试固定了这一点。
  • fileUtils.ts:1572 的 zoom 提示——保留正确,zoom_image 仍可直接调用。
  • tool-registry.ts:852 / 1113——正是 diff 扩展的两个入口。

这段枚举是本次审查真正承重的部分。两个方向出错都会造成破坏:把前四组中任何一处也改掉会破坏直接调用面;漏改本应转换的一处则会破坏 code mode。跳过它的审阅者必须自己从头重做一遍。

嵌套 binding 的强制校验也正确延续了下来。dispatchCodeModeToolcoreToolScheduler.ts:1697)读取 parent.codeModeAllowedToolNames 且与模式无关,而 agent-core.ts 现在对两种 code mode 都会填充它,因此被收窄的子智能体的 exec 是在分派时真正受限,而不只是在展示给它的描述里受限。

发现 1 —— 无法识别的 tools.mode 取值会 fail open 并静默启用 exec(阻塞)

packages/core/src/config/config.ts 中的 registerExecIfEnabled 从 allowlist 变成了 denylist:原本是不等于 CodeModeOnly 就返回,现在是等于 Direct 才返回。

取值在整条链路上都没有被校验。loadCliConfig 直接把 settings.tools?.mode ?? ToolMode.Direct 传下去,Config 存下 params.toolMode ?? ToolMode.Direct,而 loadSettings 只对未知的告警(settings.ts:276),从不按 schema 的 options 校验取值。getSettingsSchema() 只有三个消费者(settingsSchema.ts 自身、settings.ts 的键存在性检查、settingsUtils.ts 的对话框排序),没有一个做值校验。重新生成的 settings.schema.json 枚举是 scripts/generate-settings-schema.ts 产出的编辑器产物,不是运行时校验器。

于是 "tools": { "mode": "code-mode" }——该用下划线的地方写成了连字符——会导致:

  1. registerExecIfEnabled 看到取值不是 direct,于是注册 exec
  2. getFunctionDeclarations() 既不匹配 CodeModeOnly 分支也不匹配新的 CodeMode 分支,因此返回原始 schema。exec 带着 ExecTool 自己那一行描述被声明出来——'Execute JavaScript in an isolated runtime.'exec.ts:220)——完全不含生成的 tools.* / text() / ALL_TOOLS 协议,任何普通工具也拿不到嵌套声明。
  3. 如果模型仍然调用它,dispatchCodeModeTool 运行时 parent.codeModeAllowedToolNames 为 undefined(只有 agent-core 会设置它),因此 isCodeModeToolCallAllowed(name, 'code_mode', undefined) 会放行每一个 code-mode-callable 工具。

没有报错也没有告警:用户申请了一个模式,却静默得到了另一个未被记录的模式。就影响范围而言需要公允说明:这不是权限提升——在该状态下这些工具本来都可以直接调用,exec 没有增加任何触达能力。缺陷在于 fail-closed 姿态被反转,以及这个静默的、半配置的调用面。

仓库对这种情形已有现成约定,而且就在同一个文件里:parseApprovalModeValue()config.ts:151)会规范化从 settings 读取的枚举,并按其自身注释所述"对 boot 会拒绝的取值抛错"。tools.mode 没有对应的处理。在 Config 中收窄 params.toolMode,让无法识别的取值落到 direct,即可修复;无论如何也建议把 registerExecIfEnabled 保持为 allowlist(=== CodeMode || === CodeModeOnly),让注册门禁独立于取值如何解析而自行 fail closed。

没有测试覆盖无法识别的取值。由于这条未测试路径本身就是缺陷,测试应当与修复一起提交,而不是事后补。

发现 2 —— tools.eager 会静默关掉整个 code_mode(建议)

exec 通过 registerLazyTool 注册(config.ts:1040010200),该函数会询问 PermissionManager。isExemptFromEagerAllowList 覆盖 structured_output、plan 生命周期工具、task_stoptool_searchmcp__*computer_use__*——但不含 exec。因此一个生效中的 tools.eager allowlist 如果没列出 exec,就会经 registerPermissionDeferredFactory 把它降级,使 isDeferredAndHidden('exec') 为真,从而被 getFunctionDeclarations() 的过滤链丢弃,进而让 decorateCodeModeDeclarations 命中它的 !tools.some((tool) => tool.name === ToolNames.EXEC) 提前返回,输出原始 schema。

最终效果:既没有 exec 声明,任何工具也没有嵌套声明——code_mode 整体退化为 direct,且毫无提示。code_mode_only 不受影响,因为 getCodeModeFunctionDeclarations 无条件纳入 exposure === 'exec'

两条理由使它属于建议而非阻塞:它是可恢复的,tool_search 一旦 reveal execisDeferredAndHidden 翻转,装饰随即恢复;而这个提前返回对"确实未被授予 exec"的子智能体场景是正确的。问题专属于 eager allowlist 这个交互。要么在那里豁免 exec,要么在配置了 code mode 但 exec 最终被降级时打一条日志,都能收口。另外请注意,这恰恰是启用 code_mode 的用户最可能撞上的组合,因为这两个设置都是为了控制 prompt 体积而存在的。

小问题

isCodeModeEnabled 被加入 core barrel(packages/core/src/index.ts),但 packages/core 之外没有任何地方导入它——agent-core.ts 用的是相对路径 ../../tools/code-mode.js。无害,且与相邻的 getToolExposure / isCodeModeToolCallAllowed 导出对称,保留也没问题;提出来只是因为新增导出应当点名其读取点。

测试覆盖

质量不错,而且打在了该打的地方。code-mode.test.ts 新增了混合声明面(包括 agent 这类 direct-only 工具被装饰,以及 exec 保留 ALL_TOOLS 而不重复 schema)、过滤后调用面的范围、exec 不可用时的提前返回、以及延迟发现。agent-core.skill-gate.test.ts 补齐了两套集合路径的两半:一个是直接面为 [exec, read_file] 且 binding 限制在 read_file 的收窄 agent,另一个是直接面只有 [exec]、而继承的 binding 仍覆盖 read_filewrite_file 的 exec-only agent。keeps Direct declarations unchanged 从名称列表相等升级为完整 schema 相等,对设计文档所述"direct 下逐字节不变"这一约束来说正是合适的守卫。

缺口:没有覆盖无法识别的 tools.mode(发现 1),也没有覆盖 tools.eager 与 code mode 的组合(发现 2)。

双语设计文档符合 docs/design/README.md——章节顺序一致、表格一致、验收标准一致、双向语言链接齐备。新增的 code-mode-only.zh-CN.md 是完整的忠实翻译而非摘要,包含非目标清单。一点后续小事:该非目标清单仍写着"混合暴露直接调用和代码调用",而这正是 code-mode.md 现在实现的内容。作为对 CodeModeOnly MVP 范围的陈述它是准确的,因此没有错误,但两份文档之间加一个交叉链接可以省掉下一位读者的回头确认。

CI 测试证据

本次为无人值守 CI 运行,按门禁规则我没有构建、运行或执行本 PR 的任何代码。下面的证据来自 PR 自身的 CI,通过 check-run API 针对被审查的 commit 读取。

抓取时 Qwen Code CI 仍在进行:Test (ubuntu-latest, Node 22.x)Lint & Static (ubuntu-latest, Node 22.x) 尚未得出结论,因此单测套件和 lint 结果目前还不存在,我不会去猜测。没有检查项为红。

当前状况中有两点比绿色的行更重要。第一,Test (macos-latest, Node 22.x)Test (windows-latest, Node 22.x)skipped,不是通过——因此平台覆盖完全依赖仍在运行的 ubuntu 任务,而 PR 自己的 Tested-on 表格记录的是 macOS ✅、Windows 与 Linux ⚠️。第二,verifytmux-testing 均为 skipped,所以没有任何沙箱通道实际执行过这个改动。

作者在线程中的 E2E 报告是其本地运行的自述——121 个 Core 相关测试、585 个 CLI 相关测试,以及 macOS 上通过的 build/typecheck/lint。我不把它当作证据采纳,也没有重跑其中任何一项;引用它只是为了让记录显示覆盖了什么、没覆盖什么,而且作者在其中明确说明本地没有进行真实 provider 的交互式运行。

沙箱验证可以了结单测无法了结的部分。@qwen-code /tmux——用于确认设置界面确实能在 Default → Code Mode → Code Mode Only 之间循环,以及 code_mode 会话确实同时向模型呈现普通工具和可用的 exec。这是本 PR 的核心行为主张,而目前端到端尚未被证实:snapshot 测试只固定了默认显示,声明类测试止步于 registry 边界,且按作者自述没有做过真实 provider 运行。@qwen-code /verify 还能了结设计文档所述的 A/B 约束——direct 声明相对 base 构建逐字节不变——这一点 schema 相等的单测只是断言,并未与 base 做比较。

Qwen Code · qwen3.8-max-2026-09-02

Reviewed at 6ab42db9b41170014c3a3d7ac6168af41894dd8b · re-run with @qwen-code /triage

@qwen-code-ci-bot

Copy link
Copy Markdown
Collaborator

Confidence: 2/5 — the mode-gating work is genuinely careful and I could not find a way to break the direct surface, but the registration gate now fails open on an unrecognized setting value and that should be fixed before this merges.

Stepping back: this is a good PR that has one real defect in it, and I want to be clear about the ratio, because the defect is small and the surrounding work is not.

I wrote down what I'd do before reading the diff — replace the boolean with a three-value enum, reuse the existing declaration formatter rather than writing a second one, make subagent allowlists govern the direct and nested surfaces identically, force direct under safe/bare. The PR does all of that, and then does two things my proposal had not accounted for. It splits the subagent surface into a narrow direct set and a wider nested binding set, which is what makes an exec-inheriting fork behave sensibly instead of either losing its bindings or leaking every tool onto its top level. And it correctly reasons through twelve separate === CodeModeOnly checks scattered across the scheduler, the ACP session, the prompt builder, the client's deferred-tool machinery, and a zoom hint — leaving every one of them alone for the right reason. I went looking for a site that should have been converted and was not, and for one that was converted and should not have been. Neither exists. That is the hard part of this change and it is right.

So I am not requesting changes because I doubt the design. I am requesting changes because of one line. registerExecIfEnabled went from "return unless this is exactly CodeModeOnly" to "return only if this is exactly Direct," and no layer between settings.json and that comparison validates the value. An allowlist became a denylist in the gate that decides whether a JavaScript execution tool gets registered, in a subsystem whose own design documents say "fail closed" about six times. A hyphen instead of an underscore in a config file now registers exec, declares it with a one-line description that documents none of its protocol, and leaves nested dispatch with no allowlist — silently, with nothing logged.

I want to be honest about how small that is. It needs a typo to trigger. It grants no capability the model did not already have, because in that state every ordinary tool is directly callable anyway. It is recoverable and it is not a security hole. If the repo had no convention for it I might have filed it as a follow-up and let this merge. But parseApprovalModeValue sits in the same file and throws on exactly this shape of bad input, so the project has already decided how an enum read from settings should behave, and this setting does not do that. The fix is a few lines and a test, and it removes the only thing standing between this PR and a clean approval.

Finding 2 is behind it in importance but shares a root cause worth naming: code_mode degrades to direct silently in two different situations now — an unrecognized value, and an active tools.eager allowlist that omits exec. Neither is unsafe, and both are the kind of thing a user reports as "the setting does nothing" six weeks later with no log line to go on. Whichever way you fix Finding 1, making the degraded state observable would pay for itself.

On the questions I could not settle from the diff: whether granting exec to a subagent that asked only for ['read_file'] is intended (Stage 1), and whether the 359 doc lines belong in this PR rather than their own. Neither blocks anything, and I would not hold the PR for either.

CI had not concluded when I wrote this — the ubuntu unit suite and lint were still running, macOS and Windows unit jobs were skipped rather than passed, and neither sandboxed lane had run. So nothing here is an approval held back only by CI; the verdict is the finding.

中文说明

Confidence: 2/5 —— 模式门禁这部分做得确实细致,我没能找到破坏直接调用面的办法;但注册门禁现在会在无法识别的设置取值上 fail open,这一点应当先修掉再合。

退一步看:这是一个好 PR,里面有一个真实缺陷,我想把比例说清楚,因为缺陷很小,而周边的工作不小。

在读 diff 之前我先写下了自己的做法——把布尔值换成三值枚举、复用既有的声明格式化逻辑而不是再写一份、让子智能体 allowlist 对直接面和嵌套面一视同仁、safe/bare 强制 direct。这个 PR 全都做到了,而且还做了两件我的方案没考虑到的事。它把子智能体调用面拆成"收窄的直接集合"和"更宽的嵌套 binding 集合",这正是让继承 exec 的 fork 表现合理的关键,否则它要么丢掉 binding,要么把每个工具都泄漏到自己的顶层。它也正确推演了散落在 scheduler、ACP session、prompt 构建、client 延迟工具机制和一处 zoom 提示中的十二处独立 === CodeModeOnly 判断——每一处都出于正确的理由保留原样。我去找过"本该转换却没转换"的地方,也去找过"转换了但不该转换"的地方,两者都不存在。这是本次改动最难的部分,而它是对的。

所以我请求修改,不是因为怀疑设计,而是因为一行代码。registerExecIfEnabled 从"除非恰好是 CodeModeOnly 否则返回"变成了"只有恰好是 Direct 才返回",而 settings.json 到这个比较之间没有任何一层校验取值。在决定是否注册一个 JavaScript 执行工具的门禁里,allowlist 变成了 denylist,而这个子系统自己的设计文档里"fail closed"出现了六次。配置文件里一个连字符代替下划线,现在就会注册 exec、用一行完全不描述其协议的描述把它声明出去、并让嵌套分派没有 allowlist——全程静默,什么日志都没有。

关于它有多小,我想说得诚实。它需要一次拼写错误才会触发。它没有授予模型原本不具备的能力,因为在该状态下每个普通工具本来就可以直接调用。它可恢复,也不是安全漏洞。如果仓库对此没有既有约定,我可能会把它记为后续项然后让这个 PR 合进去。但 parseApprovalModeValue 就在同一个文件里,对完全相同形态的坏输入会抛错——也就是说项目早已决定"从 settings 读到的枚举该如何表现",而这个设置没有照做。修复只是几行代码加一个测试,而它是横在本 PR 与一次干净批准之间的唯一障碍。

发现 2 重要性次之,但根因值得点明:code_mode 现在会在两种不同情形下静默退化为 direct——取值无法识别,以及生效中的 tools.eager allowlist 没列出 exec。两者都不危险,但都属于六周后用户会以"这个设置没用"来上报、而日志里没有任何线索可查的那类问题。无论发现 1 怎么修,让这个退化状态可观测都会值回成本。

关于我无法从 diff 判定的问题:把 exec 授予只申请了 ['read_file'] 的子智能体是否有意为之(Stage 1),以及 359 行文档是否该独立成 PR。两者都不阻塞,我也不会为此压住这个 PR。

我写这段时 CI 尚未结束——ubuntu 单测套件和 lint 仍在运行,macOS 与 Windows 单测任务是 skipped 而非通过,两条沙箱通道都没跑。所以这里不存在"仅因 CI 而暂缓的批准";结论来自上述发现。

Qwen Code · qwen3.8-max-2026-09-02

Reviewed at 6ab42db9b41170014c3a3d7ac6168af41894dd8b · re-run with @qwen-code /triage

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

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

@DragonnZhang Requesting changes on one specific point — the rest of this is strong and I have set out why in the review notes above.

registerExecIfEnabled in packages/core/src/config/config.ts changed from an allowlist check to a denylist check, and nothing between settings.json and that comparison validates the value. loadSettings warns about unknown keys but never checks a value against the schema's options, and the regenerated settings.schema.json is an editor artifact rather than a runtime validator. So an unrecognized tools.mode — a hyphen instead of an underscore, say — registers exec, declares it with ExecTool's own one-line description that documents none of the tools.* / text() / ALL_TOOLS protocol, leaves every ordinary tool without its nested declaration, and leaves dispatchCodeModeTool with no allowlist. Silently, with nothing logged.

The project already has the convention for this, in the same file: parseApprovalModeValue() normalizes an enum read from settings and throws for values boot would reject. Narrowing params.toolMode in Config so an unrecognized value resolves to direct would fix it, and keeping the registration gate an allowlist is worth doing regardless so it fails closed independently of how the value was parsed. A test for the unrecognized value belongs with the fix, since that untested path is the defect.

To be fair about scale: this needs a typo to trigger and grants no capability the model does not already have, so it is not a security hole — it is an inverted fail-closed posture in the gate that decides whether a JavaScript execution tool gets registered. Finding 2 in the notes (tools.eager omitting exec silently degrading code_mode to direct) is the same root cause and is a suggestion rather than a blocker, but making the degraded state observable would pay for itself.

Nothing here questions the design. The five converted mode checks and the twelve deliberately unconverted ones are all correct — I checked each against the code, and the subagent split into a narrow direct surface and a wider nested binding set is the part I would not have thought to do. CI had also not concluded when I reviewed, so the unit suite and lint results are still outstanding on this commit.

中文说明

@DragonnZhang 就一个具体问题请求修改——其余部分都很扎实,理由已写在上面的审查记录里。

packages/core/src/config/config.ts 中的 registerExecIfEnabled 从 allowlist 判断改成了 denylist 判断,而 settings.json 到这个比较之间没有任何一层校验取值。loadSettings 只对未知的键告警,从不按 schema 的 options 校验取值;重新生成的 settings.schema.json 是编辑器产物,不是运行时校验器。因此一个无法识别的 tools.mode——比如把下划线写成连字符——会注册 exec,用 ExecTool 自己那一行完全不描述 tools.* / text() / ALL_TOOLS 协议的描述把它声明出去,让每个普通工具都拿不到嵌套声明,并让 dispatchCodeModeTool 没有 allowlist。全程静默,没有任何日志。

项目对此已有约定,而且就在同一个文件里:parseApprovalModeValue() 会规范化从 settings 读到的枚举,并对 boot 会拒绝的取值抛错。在 Config 中收窄 params.toolMode、让无法识别的取值落到 direct 即可修复;无论如何也建议把注册门禁保持为 allowlist,让它独立于取值如何解析而自行 fail closed。针对无法识别取值的测试应与修复一起提交,因为这条未测试路径本身就是缺陷。

公允地说明量级:它需要一次拼写错误才会触发,也没有授予模型原本不具备的能力,所以不是安全漏洞——它是在"决定是否注册一个 JavaScript 执行工具"的门禁里把 fail-closed 姿态反转了。记录中的发现 2(tools.eager 未列出 exec 会让 code_mode 静默退化为 direct)根因相同,属于建议而非阻塞,但让这个退化状态可观测会值回成本。

以上没有一处质疑设计。五处被转换的模式判断和十二处被有意保留的判断全部正确——我逐一对照代码检查过,而把子智能体拆成收窄的直接面与更宽的嵌套 binding 集合,正是我自己不会想到去做的那部分。另外,我审查时 CI 尚未结束,因此该 commit 的单测套件与 lint 结果仍未出来。

Qwen Code · qwen3.8-max-2026-09-02

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

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Partially reviewed — gaps disclosed.

Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.

Not reviewed: reverse audit — stopped before round 7 by the review time budget.

中文说明

仅完成部分审查,审查缺口已披露。

未审查(原文为英文):build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.

未审查:反向审计——评审时间预算不足,未能开始第 7 轮。

— qwen3.8-max via Qwen Code /review (v0.23.3)

Comment thread packages/core/src/config/config.ts Outdated
Comment thread packages/cli/src/config/settingsSchema.ts Outdated
Comment thread packages/core/src/tools/code-mode.ts Outdated
Comment thread packages/core/src/agents/runtime/agent-core.ts
Comment thread packages/cli/src/config/settingsSchema.ts
Comment thread packages/core/src/code-mode/code-mode.test.ts Outdated
Comment thread packages/cli/src/config/settingsSchema.ts
Comment thread packages/core/src/tools/code-mode.ts Outdated
Comment thread docs/design/code-mode.md Outdated
Comment thread packages/core/src/tools/tool-registry.ts

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

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Partially reviewed — gaps disclosed.

2 Suggestion-level finding(s) this review confirmed are already reported on this PR and are not repeated:

  • R1-6 / R1-7 hybrid exec-description duplication — still standing from round 1; the author declined both as intentional, so neither is re-posted (comments 4008183313, 4008183322)
  • hybrid exec-description duplication re-derived this round — already reported as R1-6 and R1-7 (comments 4008183313, 4008183322)

Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.

Not reviewed: build-and-test — Test (macos-latest, Node 22.x) and Test (windows-latest, Node 22.x) were skipped in CI; the local suite ran on Linux only.

Not explored to full depth (tool budget reached): "agent reverse-audit (round 3)": no runtime probe of the hybrid + active tools.eager + ToolSearch-off combination (would have needed a built dist/ and a PermissionManager -backed registry)…; chunk 7: I did not trace whether any production caller constructs a ToolConfig with inline-only FunctionDeclaration entries — a repo-wide grep over non-test package…; "agent reverse-audit (round 4)": no executed probe or mutation run — the finding and every mutation claim above rest on source reads of agent-core.ts , tool-registry.ts , code-mode.ts , cor…; "agent reverse-audit (round 4)": renderAvailableSkillsBlock ( skill-utils.ts:247 ) body not read — I verified the enclosing reminder sentence that names the Skill tool, but not whether the re…; chunk 6: I did not execute npx vitest run for mustTranslateKeys.test.ts , SettingsDialog.test.tsx , dialogs-settings.test.ts or agent-core.skill-gate.test.ts , no….

Not reviewed: reverse audit — did not converge within the reverse-audit round cap of 5.

中文说明

仅完成部分审查,审查缺口已披露。

本轮确认的 2 条建议级发现已在 PR 上报告过,不再重复发布(列表见上方英文部分)。

未审查(原文为英文):build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.

未审查(原文为英文):build-and-test — Test (macos-latest, Node 22.x) and Test (windows-latest, Node 22.x) were skipped in CI; the local suite ran on Linux only.

未探索到全部深度(达到工具调用预算):"agent reverse-audit (round 3)"no runtime probe of the hybrid + active tools.eager + ToolSearch-off combination (would have needed a built dist/ and a PermissionManager -backed registry)…;chunk 7:I did not trace whether any production caller constructs a ToolConfig with inline-only FunctionDeclaration entries — a repo-wide grep over non-test package…"agent reverse-audit (round 4)"no executed probe or mutation run — the finding and every mutation claim above rest on source reads of agent-core.ts , tool-registry.ts , code-mode.ts , cor…"agent reverse-audit (round 4)"renderAvailableSkillsBlock ( skill-utils.ts:247 ) body not read — I verified the enclosing reminder sentence that names the Skill tool, but not whether the re…;chunk 6:I did not execute npx vitest run for mustTranslateKeys.test.ts , SettingsDialog.test.tsx , dialogs-settings.test.ts or agent-core.skill-gate.test.ts , no…

未审查:反向审计——在 5 轮的反审轮数上限内未收敛。

— qwen3.8-max via Qwen Code /review (v0.23.3)

| `agents.builtin.exploreModel` | string | Model selector for the built-in Explore subagent. Use `inherit` for the main session model, `fast` for `fastModel`, a model ID, or an `authType:model-id` selector. A custom same-name Explore agent keeps its own model configuration. Requires restart. | `inherit` |
| `agents.modelGrades` | object | Maps semantic grade names exposed to the Agent tool to model selectors. Requires restart. | `undefined` |
| `agents.allowedGrades` | array of strings | Optional whitelist of configured model grades the Agent tool may use. Requires restart. | `undefined` |
| `agents.crossSessionMessaging` | boolean | Experimental. Let Qwen Code sessions on this machine send each other messages over a per-session local socket. Turning it on opens this session to peer messages, makes it discoverable to others, and lets its model address them from `send_message`. Requires restart. A workspace may set this to `false` only; a workspace `true` is ignored, with a warning only when it would loosen the operator-set or default value. | `false` |

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

[Critical] R2-1: [certifies-falsely] [regression] The merge commit resolved this file to the branch's older copy and silently reverted three documentation changes main had already landed — one of which now documents a security-relevant default backwards.

git merge-base main HEAD is 3fc1133d44, which is main, so HEAD^2 and main are the same tree. A padding-normalized semantic diff of HEAD^2 against HEAD returns exactly three reverts, and git diff --stat shows the merge took 1 insertion from main and 41/36 from the branch side — contrary to its own message, "chore: merge main into code mode branch".

  1. :432 agents.crossSessionMessaging. The row is restored to its pre-feat(settings): turn cross-session messaging on by default #11840 text describing the setting as "Experimental" with the default documented as false, while packages/cli/src/config/settingsSchema.ts:3561 ships default: true and settingsSchema.test.ts:389 asserts it. The revert also drops main's caveat naming the two senders delivered without review, and the "Set to false to keep this session invisible and unreachable." sentence. An operator auditing peer reachability reads "off unless opted in", never sets false, and the session is in fact discoverable and reachable.
  2. :805. The "Loader-affecting variables are always rejected" paragraph loses main's feat(cli): add bwrap kernel sandbox backend for Linux #11614 clause enumerating the sandbox confinement-decision variables (QWEN_SANDBOX, QWEN_SANDBOX_IMAGE, QWEN_SANDBOX_NET, QWEN_SANDBOX_PROXY_COMMAND) and the writable-root derivation variables (XDG_CACHE_HOME, TMPDIR, TMP, TEMP). All eight are still rejected by packages/cli/src/config/shared-env-keys.ts:173-190, so the security rationale for eight live denylist entries is now documented nowhere in the user docs. git grep "writable-root derivation variables" HEAD returns nothing.
  3. :200 model.maxToolCallsPerTurn. The row loses main's docs(goal): retire the Stop-hook era prose after the implementation was removed #11922 "runtime-scheduled Goal turns" clause and resurrects "blocking Stop-hook continuations such as /goal iterations start a fresh budget", which main retired because the Stop-hook implementation was removed. client.ts:2925-2931 includes SendMessageType.Goal in startsInteraction and resets the loop detector at :3282/:3613.

The commit is also internally inconsistent with itself: settingsSchema.ts:1818 and the generated settings.schema.json both still carry main's post-#11922 wording for the same setting this file reverted.

Witness:

git diff --stat HEAD^1 HEAD  ->  1 insertion      (what the merge took from main)
git diff --stat HEAD^2 HEAD  ->  41 insertions, 36 deletions   (what it kept from the branch)
semantic diff HEAD^2 (== main) vs HEAD -> 3 reverts, at :200, :432, :805
git grep "On by default: this session is discoverable" HEAD -- docs  ->  not found
git grep "writable-root derivation variables" HEAD                   ->  not found anywhere
settingsSchema.ts:3561  default: true,      settingsSchema.test.ts:389  expect(...).toBe(true)
shared-env-keys.ts:173-176, :187-190  ->  the eight keys the reverted paragraph used to name
No CI check or test reads docs/users/configuration/settings.md.

Re-resolve the file against main rather than patching the three passages by hand — git checkout origin/main -- docs/users/configuration/settings.md, then re-apply this PR's own tools.mode, tools.eager and deprecation-block additions on top, and diff against main to confirm only the intended deltas remain.

The restored text must match the code it describes: settingsSchema.ts:3561 ships default: true for crossSessionMessaging and settingsSchema.test.ts:389 pins it, and shared-env-keys.ts:173-176/:187-190 hold the eight project-env exclusions the :805 paragraph must name.

中文说明

R2-1(Critical):合并提交把本文件解析成了分支侧的旧版本,静默回退了 main 已经合入的三处文档改动,其中一处现在把与安全相关的默认值写反了。

git merge-base main HEAD3fc1133d44,也就是 main 本身,因此 HEAD^2main 是同一棵树。对 HEAD^2HEAD 做忽略对齐空白的语义比对,恰好得到三处回退;git diff --stat 显示这次合并只从 main 取了 1 行插入,而从分支侧保留了 41/36 行——与提交信息「chore: merge main into code mode branch」相反。

  1. :432 agents.crossSessionMessaging 该行被恢复成 feat(settings): turn cross-session messaging on by default #11840 之前的文本,把此设置描述为「Experimental」且默认值写作 false,而 packages/cli/src/config/settingsSchema.ts:3561 实际是 default: truesettingsSchema.test.ts:389 也如此断言。回退同时删掉了 main 中说明两类「无需审查即可投递」发送方的注意事项,以及「设为 false 可让本会话不可见、不可达」这句。运维者据此认为「默认关闭、需显式开启」,于是从不设置 false,而会话实际上是可被发现、可被访问的。
  2. :805「Loader-affecting variables are always rejected」一段丢失了 main 在 feat(cli): add bwrap kernel sandbox backend for Linux #11614 中加入的子句,即枚举沙箱收敛决策变量(QWEN_SANDBOXQWEN_SANDBOX_IMAGEQWEN_SANDBOX_NETQWEN_SANDBOX_PROXY_COMMAND)与可写根目录推导变量(XDG_CACHE_HOMETMPDIRTMPTEMP)。这八个变量至今仍被 packages/cli/src/config/shared-env-keys.ts:173-190 拒绝,因此这八条生效中的拒绝项背后的安全理由,在用户文档里已无任何记载。
  3. :200 model.maxToolCallsPerTurn 该行丢失了 main 在 docs(goal): retire the Stop-hook era prose after the implementation was removed #11922 中加入的「runtime-scheduled Goal turns」子句,并复活了「blocking Stop-hook continuations such as /goal iterations start a fresh budget」——main 之所以删除它,是因为 Stop-hook 实现已被移除。client.ts:2925-2931SendMessageType.Goal 纳入 startsInteraction,并在 :3282/:3613 重置循环检测器。

该提交还与自身不一致:settingsSchema.ts:1818 与生成的 settings.schema.json 仍保留 main 在 #11922 之后的措辞,而本文件把同一设置回退了。

修复方式:不要把这三处逐条手改,而应重新按 main 解析该文件——git checkout origin/main -- docs/users/configuration/settings.md,然后把本 PR 自己新增的 tools.modetools.eager 与弃用说明块重新叠加上去,最后与 main 比对,确认只剩下本 PR 有意做出的差异。

约束:恢复后的文本必须与代码一致——settingsSchema.ts:3561crossSessionMessagingdefault: truesettingsSchema.test.ts:389 固定了这一点;shared-env-keys.ts:173-176/:187-190 列有 :805 段落必须点名的八个 project-env 排除项。

— qwen3.8-max via Qwen Code /review (v0.23.3)

this.runtimeContext.getToolMode?.() === ToolMode.CodeModeOnly &&
forNestedBinding &&
isCodeModeEnabled(this.runtimeContext.getToolMode?.()) &&
this.executionAllowedExactTools?.has(ToolNames.EXEC) &&

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

[Critical] R2-2: [certifies-falsely] The widened nested exec-gateway branch returns true before executionAllowedMcpPatterns is ever consulted, so a literal exec entry in an agent's execution allowlist admits every code-mode-callable tool — including MCP tools the operator's pattern was meant to exclude.

A hybrid (tools.mode: "code_mode") fork configured with tools: ['exec', 'mcp__github__read_*'] and executionAllowedTools: ['exec', 'mcp__github__read_*'] is a shape fork_tools explicitly accepts (isValidForkToolWildcard, tools/agent/fork-subagent.ts:120-138) and buildForkExecutionAllowlist passes through verbatim; operator-authored .qwen/fork-profiles/<name>.md frontmatter reaches it through the same validator. In prepareTools, isToolExecutionAllowed('mcp__github__create_issue', true) enters this branch — forNestedBinding is true, code mode is enabled, executionAllowedExactTools.has('exec') is true, and getToolExposure returns 'code-mode-callable' for every mcp__* name — and returns true here, before the exact-name check and long before the pattern matcher at :1768. The name lands in codeModeAllowedToolNames, rides the exec request at :2312, and dispatchCodeModeTool accepts it (coreToolScheduler.ts:1697-1704), which reads only that field; nothing downstream re-checks the agent allowlist. So an agent the operator narrowed to "exec plus read-only GitHub" can call tools.mcp__github__create_issue(...) — and every other server's write-capable MCP tool, plus built-ins such as write_file — from inside exec, while the same allowlist correctly refuses both on the direct path. One allowlist, two opposite answers depending on whether the call is direct or nested.

This contradicts the field's own contract at agents/runtime/agent-types.ts:99-105 ("calls outside this list are rejected before scheduling or approval. Supports exact tool names and MCP server-level patterns") and the fail-closed intent of resolveForkExecutionAllowedTools (fork-subagent.ts:75-79, "fail closed to deny-all … rather than hand the fork unrestricted execution"). Neither design doc mentions MCP patterns. Nested calls still traverse the normal permission and confirmation chain, so this is an allowlist bypass rather than an approval bypass — but under auto/yolo there is no prompt.

Note the distinction from the exact-name half of this branch, which is deliberate and pinned by the test this PR adds (tools:[EXEC], executionAllowedTools:[EXEC]codeModeAllowedToolNames equals [READ_FILE, WRITE_FILE]) and documented in code-mode.md. The claim here is specifically the MCP-pattern half: a pattern is a narrowing this branch cannot honour, and the direct path does honour it.

Witness:

Probe, unmodified PR source. Hybrid AgentCore; registry = exec + read_file + write_file
+ mcp__github__read_issue + mcp__github__create_issue;
toolConfig = { tools: ['exec','mcp__github__read_*'],
               executionAllowedTools: ['exec','mcp__github__read_*'] }

{"mode":"code_mode",
 "nestedAllowlist":["read_file","write_file",
                    "mcp__github__read_issue","mcp__github__create_issue"],
 "direct_write_file":false, "nested_write_file":true,
 "direct_mcp_create":false, "nested_mcp_create":true,
 "schedulerWouldDispatchCreateIssue":true,
 "execDescriptionAdvertisesCreateIssue":true}

Reachability, real functions:
  validateForkToolList(['exec','mcp__github__read_*']) -> undefined (valid)
  buildForkExecutionAllowlist(...)                     -> ["exec","mcp__github__read_*"]
  resolveForkExecutionAllowedTools(...)                -> same, verbatim

Flip, with the one-line narrowing below applied:
  nestedAllowlist drops mcp__github__create_issue;
  schedulerWouldDispatchCreateIssue false; execDescriptionAdvertisesCreateIssue false;
  pattern-matched tool and the pinned exact-name widening untouched;
  3 files / 110 tests pass.
Suggested change
this.executionAllowedExactTools?.has(ToolNames.EXEC) &&
(!toolName.startsWith('mcp__') ||
(this.executionAllowedMcpPatterns?.length ?? 0) === 0) &&
this.executionAllowedExactTools?.has(ToolNames.EXEC) &&

A pattern-free allowlist must keep inheriting the full registry: agent-core.skill-gate.test.ts:214-223 asserts gate(core, declared) is true for { tools: ['*'] }, { tools: [SKILL] } and { tools: [EXEC], executionAllowedTools: [EXEC] }, and the test this diff adds pins codeModeAllowedToolNames equal to [READ_FILE, WRITE_FILE] for the last of those — so the narrowing may only take effect when executionAllowedMcpPatterns is non-empty.

Please add a case to packages/core/src/agents/runtime/agent-core.skill-gate.test.tstoolMode: ToolMode.CodeMode, a registry holding exec plus MockTools named mcp__github__read_file and mcp__github__create_issue, and toolConfig = { tools: ['exec','mcp__github__read_*'], executionAllowedTools: ['exec','mcp__github__read_*'] } — asserting codeModeAllowedToolNames contains mcp__github__read_file and not mcp__github__create_issue; removing the added clause above must turn it red.

If instead inheriting the whole registry through exec is meant to be unconditional, the alternative fix is documentation: ToolConfig.executionAllowedTools's comment and buildChildMessage's "You may execute only tools matched by this allowlist" (fork-subagent.ts:411) must say that naming exec voids the rest of the list.

中文说明

R2-2(Critical):放宽后的嵌套 exec 网关分支在根本没有查询 executionAllowedMcpPatterns 之前就返回 true,于是执行白名单里只要有一个字面量 exec,就会放行所有 code-mode-callable 工具——包括运维方本想用模式排除掉的 MCP 工具。

一个混合模式(tools.mode: "code_mode")的 fork,若配置 tools: ['exec', 'mcp__github__read_*']executionAllowedTools: ['exec', 'mcp__github__read_*'],正是 fork_tools 明确接受的形状(isValidForkToolWildcardtools/agent/fork-subagent.ts:120-138),buildForkExecutionAllowlist 会原样透传;运维方编写的 .qwen/fork-profiles/<name>.md frontmatter 经同一校验器到达这里。在 prepareTools 中,isToolExecutionAllowed('mcp__github__create_issue', true) 进入本分支——forNestedBinding 为真、code mode 已启用、executionAllowedExactTools.has('exec') 为真,且 getToolExposure 对任何 mcp__* 名称都返回 'code-mode-callable'——于是就在这里返回 true,早于精确名匹配,更远早于 :1768 的模式匹配器。该名称随后进入 codeModeAllowedToolNames,随 exec 请求在 :2312 下发,dispatchCodeModeTool 予以接受(coreToolScheduler.ts:1697-1704),而它只读取该字段;下游没有任何环节重新校验 agent 白名单。因此一个被运维方收窄为「exec 加只读 GitHub」的 agent,可以在 exec 内部调用 tools.mcp__github__create_issue(...)——以及其他任何服务器的写能力 MCP 工具和 write_file 等内建工具——而同一份白名单在直接调用路径上会正确拒绝这两者。同一份白名单,因调用是直接还是嵌套而给出相反答案。

这与该字段自身的契约相矛盾:agents/runtime/agent-types.ts:99-105「calls outside this list are rejected before scheduling or approval. Supports exact tool names and MCP server-level patterns」,也与 resolveForkExecutionAllowedTools 的 fail-closed 意图相矛盾(fork-subagent.ts:75-79,「fail closed to deny-all … rather than hand the fork unrestricted execution」)。两份设计文档都没有提到 MCP 模式。嵌套调用仍会经过正常的权限与确认链,所以这是白名单绕过而非审批绕过——但在 auto/yolo 下不存在任何提示。

请注意与本分支「精确名」那一半的区别:那一半是有意为之,并被本 PR 新增的测试固定(tools:[EXEC], executionAllowedTools:[EXEC]codeModeAllowedToolNames 等于 [READ_FILE, WRITE_FILE]),也写在 code-mode.md 里。本条只针对 MCP 模式那一半:模式是一种本分支无法兑现的收窄,而直接路径是能兑现的。

修复方式:见上方的 suggestion 代码块——当白名单携带本分支无法兑现的收窄时,让它落回逐工具匹配器,即为该分支加上「名称不是 mcp__*,或 executionAllowedMcpPatterns 为空」这一条件,使带模式的白名单像直接路径一样逐个解析 MCP 名称。若「通过 exec 继承整个 registry」本就应当是无条件的,则替代修复是文档:ToolConfig.executionAllowedTools 的注释与 buildChildMessage 中的「You may execute only tools matched by this allowlist」(fork-subagent.ts:411)必须说明,一旦列入 exec,白名单其余部分即失效。

约束:不带模式的白名单必须继续继承整个 registry——agent-core.skill-gate.test.ts:214-223{ tools: ['*'] }{ tools: [SKILL] }{ tools: [EXEC], executionAllowedTools: [EXEC] } 都断言 gate(core, declared)true,且本 diff 新增的测试固定了最后一种形状的 codeModeAllowedToolNames 等于 [READ_FILE, WRITE_FILE],因此收窄只能在 executionAllowedMcpPatterns 非空时生效。

验证:请在 packages/core/src/agents/runtime/agent-core.skill-gate.test.ts 中新增用例——toolMode: ToolMode.CodeMode,registry 含 exec 以及名为 mcp__github__read_filemcp__github__create_issue 的 MockTool,toolConfig = { tools: ['exec','mcp__github__read_*'], executionAllowedTools: ['exec','mcp__github__read_*'] }——断言 codeModeAllowedToolNamesmcp__github__read_file不含 mcp__github__create_issue;移除上面新增的条件必须使其变红。

— qwen3.8-max via Qwen Code /review (v0.23.3)

if (withheld.length > 0 && !this.warnedAboutUnreachableEagerTools) {
this.warnedAboutUnreachableEagerTools = true;
const hybridCodeMode =
this.config.getToolMode?.() === ToolMode.CodeMode;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

[Suggestion] R2-3: The new hybrid tools.eager warning derives its claim from the tool mode alone, so it tells the operator the withheld tools "remain callable through exec" in configurations where exec was never registered.

With permissions.deny: ["exec"] — or --exclude-tools exec — plus an active tools.eager allowlist, PermissionManager.getToolRegistrationStatus('exec') returns 'disabled' ("Deny rules win over everything", permission-manager.ts:867-869) and config.ts:10413-10417 takes neither registration branch, so exec is genuinely absent. But getToolMode() still reports code_mode and deferredSummary stays non-empty from the other demoted tools, so this branch fires and prints "they remain callable through exec". An operator who denied exec and set a narrow tools.eager reads an operator-facing console.warn asserting the withheld tools are still reachable, and leaves a configuration in which they are not; nothing in the output says exec itself is the thing that is missing.

The test this PR adds asserts the false claim in the exact state that makes it false: client.test.ts:3069 sets reg.getTool.mockReturnValue(null) — so every getTool, including exec's, returns null — and then expects the text 'they remain callable through exec'.

Witness:

Probe, real PermissionManager, unmodified source.
permissions.deny ["exec"] + tools.eager ["read_file"]:
  {"OBSERVED_denyExec_eagerReadFile_exec":"disabled",
   "OBSERVED_denyExec_eagerReadFile_grep":"deferred",
   "OBSERVED_denyExec_eagerReadFile_read_file":"registered"}
  -> exec never registered; withheld non-empty; warning fires with the false clause.
control, no deny:
  {"OBSERVED_noDeny_eagerReadFile_exec":"deferred"}

Consult the registry rather than the mode when choosing the wording — e.g. const execReachable = !!toolRegistry.getTool(ToolNames.EXEC); and branch on hybridCodeMode && execReachable — or add a third arm for the exec-absent case saying the withheld tools are unreachable until the allowlist or deny rule changes.

getDeferredToolSummary() is hardcoded empty in CodeModeOnly (packages/core/src/tools/tool-registry.ts:1051, if (this.config.getToolMode?.() === ToolMode.CodeModeOnly) { return []; }), so this warning is reachable only in Direct and hybrid CodeMode; and isCodeModeEnabled (code-mode.ts:33-35) also gates registerExecIfEnabled (config.ts:10579), so it must not be substituted into this branch where only the hybrid mode is meant.

Please extend packages/core/src/core/client.test.ts:3069 — the existing case already builds the exec-absent state via reg.getTool.mockReturnValue(null); change its expectation to the corrected wording, so restoring the mode-only condition turns it red.

中文说明

R2-3(Suggestion):新增的混合模式 tools.eager 警告只根据工具模式来断言,于是在 exec 根本没有注册的配置下,它仍然告诉运维方这些被扣住的工具「remain callable through exec」。

当同时存在 permissions.deny: ["exec"](或 --exclude-tools exec)与生效的 tools.eager 白名单时,PermissionManager.getToolRegistrationStatus('exec') 返回 'disabled'(「Deny rules win over everything」,permission-manager.ts:867-869),config.ts:10413-10417 两个注册分支都不会进入,因此 exec 确实不存在。但 getToolMode() 仍报告 code_mode,且其他被降级工具使 deferredSummary 非空,于是本分支照常触发并输出「they remain callable through exec」。一个既禁用了 exec 又设置了较窄 tools.eager 的运维者,会读到一条面向运维的 console.warn 声称这些工具仍可调用,从而保留了一个它们其实不可调用的配置;输出中也没有任何一处说明缺失的正是 exec 本身。

本 PR 新增的测试恰好在那种使其为假的状态下断言了这句假话:client.test.ts:3069 设置了 reg.getTool.mockReturnValue(null)——于是包括 exec 在内的每个 getTool 都返回 null——随后却断言文本 'they remain callable through exec'

修复方式:在选择措辞时查询 registry 而非模式,例如 const execReachable = !!toolRegistry.getTool(ToolNames.EXEC);,并以 hybridCodeMode && execReachable 分支;或为 exec 缺失的情况增加第三种文案,说明这些工具在allowlist 或 deny 规则改变前不可达。

约束:getDeferredToolSummary() 在 CodeModeOnly 下被硬编码为空(packages/core/src/tools/tool-registry.ts:1051if (this.config.getToolMode?.() === ToolMode.CodeModeOnly) { return []; }),因此该警告只在 Direct 与混合 CodeMode 下可达;另外 isCodeModeEnabledcode-mode.ts:33-35)同时把守 registerExecIfEnabledconfig.ts:10579),所以在这个只指混合模式的分支里不能用它替换。

验证:请扩展 packages/core/src/core/client.test.ts:3069——该用例已通过 reg.getTool.mockReturnValue(null) 构造出 exec 缺失的状态;把它的断言改为修正后的措辞,这样一旦恢复「只看模式」的条件就会变红。

— qwen3.8-max via Qwen Code /review (v0.23.3)

return (
(declaredToolNames.has(ToolNames.SKILL) ||
(this.runtimeContext.getToolMode?.() === ToolMode.CodeModeOnly &&
(isCodeModeEnabled(this.runtimeContext.getToolMode?.()) &&

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

[Suggestion] R2-4: Widening canInvokeSkill to hybrid makes CoreToolScheduler.hasSkillTool() answer "skill is invocable" where its consumer's contract is "skill was DECLARED to the model", so the activated-skills reminder instructs a direct call that the hybrid direct surface rejects.

With tools.mode: "code_mode" and a subagent or fork profile declaring tools: ['exec', 'read_file'], skill is code-mode-callable and is not in EXCLUDED_TOOLS_FOR_SUBAGENTS, so inheritsCodeModeBindings puts it in codeModeAllowedToolNames and this gate returns true. But the hybrid declarationNames filter at :768-777 requires configuredNames.has(name), so skill is not declared, and willHaveSkillTool() (:663-678, mode-unaware) injects no <available_skills> snapshot either. When any tool call activates a skill, coreToolScheduler.ts:5776-5779 reads hasSkillToolOverride() — the gate this diff widened — and :5811-5814 appends "The following skill(s) became available via the Skill tool … invoke a skill by passing its name to the Skill tool". That is the model's only notification of the skill and it names a direct call, which hits :1890 and returns Tool "skill" not found. Tools must use the exact names provided. — a burned turn. renderAvailableSkillsBlock (skill-utils.ts:247-278) emits only <skill><name>/<description>/<location>, no invocation form, so nothing corrects it. The consumer's own comment states the contract this breaks: coreToolScheduler.ts:5774-5775, "Gate on whether SkillTool was DECLARED to the model — the registry cannot answer that."

Hybrid is materially different from CodeModeOnly here, and not only in framing: client.ts:415 passes config.getCodeModeOnly() (false in hybrid, config.ts:7144) into the prompt builder, so hybrid receives the Direct guidance section and never gets prompts.ts:313's "Ordinary tools exist only inside exec" framing that would let a CodeModeOnly model translate "the Skill tool" into tools.skill(...).

Witness:

Probe, HEAD source. Hybrid fork tools=["exec","read_file"]:
  declaredNames ["exec","read_file"]
  codeModeAllowedToolNames ["read_file","write_file","skill"]
  canInvokeSkill true        willHaveSkillTool false
Same shape, merge-base gate re-applied at runtime
(text verbatim from git show 3fc1133d44:…agent-core.ts:1684-1695):
  canInvokeSkill false       <- the flip isolates the + line at :1706
Control, code_mode_only fork tools=["exec"]:
  canInvokeSkill true under BOTH gates   <- pre-existing there, new in hybrid
Correction to the filed cost: execDescriptionHasSkillBinding true, so the nested
form tools.skill(args: …) IS advertised; the harm is a reminder pointing at a
rejected invocation form and a likely-abandoned skill, not a lost capability.

Have the gate report the surface rather than only reachability — return 'declared' | 'nested-only' | false from canInvokeSkill (or add a sibling isSkillNestedOnly(declaredToolNames)), thread it through CoreToolSchedulerOptions.hasSkillTool, and render the nested-only case as "invoke a skill with await tools.skill({ name: '<name>' }) inside exec" instead of the direct-call sentence at coreToolScheduler.ts:5811. Widen willHaveSkillTool() in the same change so the snapshot is injected whenever the gate can be true.

:1890 rejects any name absent from declaredToolNames with Tool "${toolName}" not found. Tools must use the exact names provided., so a nested-only reminder must not name a tool the model can call directly; and code-mode.ts:60-68 makes tools.skill(...) a legal nested target, since skill is code-mode-callable.

Please add a hybrid case to packages/core/src/agents/runtime/agent-core.skill-gate.test.ts with tools: [ToolNames.EXEC] asserting the gate reports nested-only rather than declared — it must go red both if the distinction is collapsed back to a boolean and if this branch reverts to === ToolMode.CodeModeOnly — paired with a coreToolScheduler test asserting the reminder for that state contains tools.skill( and not passing its name to the Skill tool.

中文说明

R2-4(Suggestion):把 canInvokeSkill 放宽到混合模式,使 CoreToolScheduler.hasSkillTool() 回答的是「skill 可被调用」,而其消费方的契约是「skill 已被声明给模型」,于是激活技能的提醒会让模型去发起一个混合模式直接调用面会拒绝的调用。

tools.mode: "code_mode" 且子智能体或 fork 配置声明 tools: ['exec', 'read_file'] 时,skill 属于 code-mode-callable 且不在 EXCLUDED_TOOLS_FOR_SUBAGENTS 中,因此 inheritsCodeModeBindings 会把它放进 codeModeAllowedToolNames,本判断返回真。但混合模式的 declarationNames 过滤(:768-777)要求 configuredNames.has(name),所以 skill被声明;willHaveSkillTool():663-678,与模式无关)也不会注入 <available_skills> 快照。当任何工具调用激活了某个技能时,coreToolScheduler.ts:5776-5779 读取 hasSkillToolOverride()——也就是本 diff 放宽的那个判断——随后 :5811-5814 追加「The following skill(s) became available via the Skill tool … invoke a skill by passing its name to the Skill tool」。这是模型关于该技能的唯一通知,而它指名的是直接调用;该调用会撞上 :1890,返回 Tool "skill" not found. Tools must use the exact names provided.,白白消耗一个回合。renderAvailableSkillsBlockskill-utils.ts:247-278)只输出 <skill><name>/<description>/<location>,不含任何调用形式,因此没有东西能纠正它。消费方自己的注释写明了被破坏的契约:coreToolScheduler.ts:5774-5775,「Gate on whether SkillTool was DECLARED to the model — the registry cannot answer that.」

在这一点上混合模式与 CodeModeOnly 有实质差别,且不只是措辞:client.ts:415config.getCodeModeOnly()(混合模式下为假,config.ts:7144)传给提示词构造器,因此混合模式拿到的是 Direct 指引段落,永远得不到 prompts.ts:313 中「Ordinary tools exist only inside exec」那段框架——正是那段框架能让 CodeModeOnly 的模型把「the Skill tool」翻译成 tools.skill(...)

修复方式:让该判断报告「调用面」而不仅是「可达性」——从 canInvokeSkill 返回 'declared' | 'nested-only' | false(或新增一个 isSkillNestedOnly(declaredToolNames) 兄弟方法),把它串到 CoreToolSchedulerOptions.hasSkillTool,并在 nested-only 情况下渲染为「invoke a skill with await tools.skill({ name: '<name>' }) inside exec」,取代 coreToolScheduler.ts:5811 的直接调用句。同一改动中放宽 willHaveSkillTool(),使只要该判断可能为真就注入快照。

约束::1890 会以 Tool "${toolName}" not found. Tools must use the exact names provided. 拒绝任何不在 declaredToolNames 中的名称,因此 nested-only 的提醒绝不能指名一个模型无法直接调用的工具;而 code-mode.ts:60-68 使 tools.skill(...) 成为合法的嵌套目标,因为 skillcode-mode-callable

验证:请在 packages/core/src/agents/runtime/agent-core.skill-gate.test.ts 中新增一个 tools: [ToolNames.EXEC] 的混合模式用例,断言该判断报告 nested-only 而非 declared——无论是把这个区分退回布尔值,还是把本分支改回 === ToolMode.CodeModeOnly,都必须使其变红;并配一个 coreToolScheduler 测试,断言该状态下的提醒含 tools.skill( 而不含 passing its name to the Skill tool

— qwen3.8-max via Qwen Code /review (v0.23.3)

(declaredToolNames.has(ToolNames.SKILL) ||
(this.runtimeContext.getToolMode?.() === ToolMode.CodeModeOnly &&
(isCodeModeEnabled(this.runtimeContext.getToolMode?.()) &&
declaredToolNames.has(ToolNames.EXEC) &&

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

[Suggestion] R2-5: No test exercises the widened canInvokeSkill gate under hybrid code_mode, so narrowing it back to === ToolMode.CodeModeOnly leaves the whole suite green.

makeCodeModeCore (agent-core.skill-gate.test.ts:195) hardcodes makeFakeConfig({ toolMode: 'code_mode_only' }), and the four new hybrid tests assert only on declarations, executable() and codeModeAllowedToolNames — never on gate(). Reverting this line keeps all 29 tests passing while a hybrid subagent declared { tools: [EXEC], executionAllowedTools: [EXEC] } silently stops announcing activated skills, even though skill is in codeModeAllowedToolNames and callable as await tools.skill(...). The gate's own doc comment already warns that checking the inputs separately "stays green when the gate stops combining them, which is how the first version of these tests missed both mutations" — the hybrid widening is a third instance of exactly the failure mode that comment names.

This is a coverage gap, not a claim that the widened answer is wrong; the separate finding on line 1706 argues the answer itself violates the consumer's contract, and one fix could close both.

Witness:

Mutation (isolated copy): narrow :1706 from
  isCodeModeEnabled(this.runtimeContext.getToolMode?.())
to
  this.runtimeContext.getToolMode?.() === ToolMode.CodeModeOnly
INTACT   agent-core.skill-gate.test.ts  29 passed (29)
MUTATED  agent-core.skill-gate.test.ts  29 passed (29)   <- survives
Paired probe (hybrid code_mode subagent, { tools: [exec], executionAllowedTools: [exec] },
skill registered, asserted through gate() = the private canInvokeSkill):
INTACT   {"OBSERVED_toolMode":"code_mode","OBSERVED_declared":["exec"],
          "OBSERVED_codeModeAllowedToolNames":["read_file","skill"],
          "OBSERVED_skillRegistered":true,"OBSERVED_gate_canInvokeSkill":true}
MUTATED  identical inputs, "OBSERVED_gate_canInvokeSkill":false
Harm at the consumer: canInvokeSkill feeds hasSkillTool (agent-core.ts:2071),
which gates the tail-reminder announcement at coreToolScheduler.ts:5766-5773.
Correction: the forNestedBinding `true` at :1712 IS pinned - dropping it reddens
`opens for an executable nested skill` (1 failed | 28 passed); only the mode
widening is unpinned.

Add a hybrid gate() case beside the existing CodeModeOnly ones: makeFakeConfig({ toolMode: ToolMode.CodeMode }), a registry holding exec and skill, toolConfig = { tools: [EXEC], executionAllowedTools: [EXEC] }, asserting gate(core, declared) is true.

The existing executable() helper (agent-core.skill-gate.test.ts:74-79) calls isToolExecutionAllowed with one argument, so forNestedBinding defaults false — a new test must assert through gate() (the private canInvokeSkill), not through executable(core, ToolNames.SKILL), or it will not see the nested semantics.

Please add that case to packages/core/src/agents/runtime/agent-core.skill-gate.test.ts; narrowing this line back to === ToolMode.CodeModeOnly must turn it red.

中文说明

R2-5(Suggestion):没有任何测试在混合 code_mode 下走通放宽后的 canInvokeSkill 判断,因此把它改回 === ToolMode.CodeModeOnly,整个测试套件仍然全绿。

makeCodeModeCoreagent-core.skill-gate.test.ts:195)硬编码了 makeFakeConfig({ toolMode: 'code_mode_only' }),而四个新增的混合模式测试只断言 declarationsexecutable()codeModeAllowedToolNames,从不断言 gate()。把这一行改回去,29 个测试仍全部通过,而一个声明为 { tools: [EXEC], executionAllowedTools: [EXEC] } 的混合模式子智能体会静默地不再播报已激活的技能——尽管 skill 就在 codeModeAllowedToolNames 中,且可以通过 await tools.skill(...) 调用。该判断自己的文档注释已经警告过:分别检查各个输入「会在判断不再把它们组合起来时依然全绿,第一版测试正是这样漏掉了两处变异」——混合模式的放宽正是该注释所点名的失效模式的第三个实例。

这是一处覆盖缺口,并非主张放宽后的答案是错的;针对 1706 行的另一条发现主张答案本身违反了消费方契约,一个修复可以同时关闭两者。

修复方式:在现有 CodeModeOnly 用例旁新增一个混合模式的 gate() 用例——makeFakeConfig({ toolMode: ToolMode.CodeMode }),registry 含 execskilltoolConfig = { tools: [EXEC], executionAllowedTools: [EXEC] },断言 gate(core, declared) 为真。

约束:现有的 executable() 辅助函数(agent-core.skill-gate.test.ts:74-79)以一个参数调用 isToolExecutionAllowed,因此 forNestedBinding 默认为假——新测试必须通过 gate()(即私有的 canInvokeSkill)断言,而不是通过 executable(core, ToolNames.SKILL),否则看不到嵌套语义。

验证:请把该用例加入 packages/core/src/agents/runtime/agent-core.skill-gate.test.ts;把这一行改回 === ToolMode.CodeModeOnly 必须使其变红。

— qwen3.8-max via Qwen Code /review (v0.23.3)

const execDescription = declarations.find(
(item) => item.name === 'exec',
)?.description;
expect(execDescription).not.toContain('tools.read_file(args:');

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

[Suggestion] R2-14: The hybrid exec description's Name collisions: disclosure has zero coverage — every buildExecDescription test call passes one argument, so codeModeOnly defaults true — yet hybrid is the only mode where the omitted tool stays visible as a top-level declaration.

In a hybrid session where two registered tools normalize to the same JS name (an extension or MCP tool read-file beside core read_file; normalizeCodeModeToolName maps -_, code-mode.ts:88-92), planCodeModeBindings' code-unit sort keeps read-file and puts read_file in plan.collisions (:104-112). decorateCodeModeDeclarations then augments the kept tool's own top-level declaration with declare const tools: { read_file(args: …) } — a jsName belonging to the other tool — while the omitted read_file keeps a raw top-level declaration carrying no note. The model's only disambiguator is the - read_file is omitted because it collides with read-file as tools.read_file. line appended at code-mode.ts:297; without it the model calls tools.read_file(args) and silently executes read-file, with arguments validated against the wrong schema. In CodeModeOnly neither tool is declared top-level, so this cross-reference hazard is new to hybrid.

The shipped behaviour is correct — the disclosure is emitted in hybrid — so this is a missing test for changed behaviour rather than a defect in the code.

Witness:

Sweep (external authority: grep over the repo):
  all 9 buildExecDescription( test call sites pass ONE argument
    (code-mode.test.ts:454,457,460,461,462,465,468,491,517) -> codeModeOnly defaults true
  "is omitted because" appears only at code-mode.ts:248; no test asserts it
  "Name collisions" appears in no test
  no hybrid registry test registers two tools whose normalized names collide
Mutation (isolated copy): delete the ${collisionText ? …} interpolation at code-mode.ts:297
  INTACT   code-mode.test.ts + tool-registry.test.ts   147 passed (147)
           control probe: ["Name collisions:",
             "- read_file is omitted because it collides with read-file as tools.read_file."]
  MUTATED  1 failed | 147 passed  - the ONLY failure is that control
Sort direction confirmed, not inverted: '-' (0x2D) < '_' (0x5F), so read-file is KEPT
and read_file OMITTED, matching the pinned { jsName:'z_tool', kept:'z-tool',
omitted:'z_tool' } at :451-453.

Add one hybrid registry case next to "keeps ordinary tools direct and adds nested declarations in CodeMode": register read-file (with params) and read_file plus exec under ToolMode.CodeMode, then assert (a) exec's description contains - read_file is omitted because it collides with read-file as tools.read_file., (b) the read-file declaration contains declare const tools: { read_file(args:, and (c) the read_file declaration does not contain declare const tools:.

First-wins is decided by the code-unit sort at packages/core/src/tools/code-mode.ts:104-106const sorted = [...tools].sort((a, b) => a.name < b.name ? -1 : a.name > b.name ? 1 : 0); — so with - (0x2D) < _ (0x5F) the kept tool is read-file and the omitted one is read_file. A test written the other way round would assert the opposite of the shipped rule.

Please add those assertions to packages/core/src/code-mode/code-mode.test.ts; removing the ${collisionText ? …} interpolation at code-mode.ts:297 must redden (a), and removing the bindings.get(tool.name) lookup in decorateCodeModeDeclarations must redden (b) and (c).

中文说明

R2-14(Suggestion):混合模式 exec 描述中的 Name collisions: 披露完全没有测试覆盖——所有 buildExecDescription 的测试调用都只传一个参数,因此 codeModeOnly 默认为真——而混合模式恰恰是唯一一个「被省略的工具仍以顶层声明可见」的模式。

在混合模式会话中,若两个已注册工具归一化后得到相同的 JS 名称(例如扩展或 MCP 工具 read-file 与核心 read_file 并存;normalizeCodeModeToolName- 映射为 _code-mode.ts:88-92),planCodeModeBindings 的码元排序会保留 read-file,并把 read_file 放进 plan.collisions:104-112)。随后 decorateCodeModeDeclarations 会为被保留那个工具自己的顶层声明追加 declare const tools: { read_file(args: …) }——一个属于另一个工具的 jsName——而被省略的 read_file 则保留一份不带任何说明的原始顶层声明。模型唯一的消歧依据,是 code-mode.ts:297 追加的那行 - read_file is omitted because it collides with read-file as tools.read_file.;没有它,模型会调用 tools.read_file(args),从而静默执行 read-file,并按错误的 schema 校验参数。在 CodeModeOnly 下两个工具都不会有顶层声明,因此这种交叉引用的隐患是混合模式新引入的。

已发布的行为是正确的——混合模式下确实会输出该披露——所以这是「变更行为缺少测试」,而非代码缺陷。

修复方式:在「keeps ordinary tools direct and adds nested declarations in CodeMode」旁边新增一个混合模式 registry 用例:在 ToolMode.CodeMode 下注册 read-file(带 params)、read_fileexec,然后断言 (a) exec 描述包含 - read_file is omitted because it collides with read-file as tools.read_file.;(b) read-file 的声明包含 declare const tools: { read_file(args:;(c) read_file 的声明不包含 declare const tools:

约束:谁先胜出由 packages/core/src/tools/code-mode.ts:104-106 的码元排序决定——const sorted = [...tools].sort((a, b) => a.name < b.name ? -1 : a.name > b.name ? 1 : 0);——由于 -(0x2D)< _(0x5F),被保留的是 read-file,被省略的是 read_file。若测试反过来写,就会断言出与已发布规则相反的结论。

验证:请把这些断言加入 packages/core/src/code-mode/code-mode.test.ts;移除 code-mode.ts:297${collisionText ? …} 插值必须使 (a) 变红,移除 decorateCodeModeDeclarations 中的 bindings.get(tool.name) 查找必须使 (b) 与 (c) 变红。

— qwen3.8-max via Qwen Code /review (v0.23.3)


> [!note]
>
> **Migrating from `tools.core` / `tools.exclude` / `tools.allowed`:** These legacy settings are **deprecated** but are not automatically migrated; they continue to work at startup. Migrate `tools.allowed` and `tools.exclude` manually to `permissions.allow` and `permissions.deny`. `tools.core` has no exact replacement; see the table below.
>
> The nightly-only legacy setting `tools.codeModeOnly: true` is still honored

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

[Suggestion] R2-15: This deprecation note calls tools.codeModeOnly nightly-only, but the key shipped dialog-editable in the stable v0.23.4 release, so the note narrows its own audience to the wrong users — and it is the only deprecated-but-honoured setting in this file with no row in the reference table.

git show v0.23.4:packages/cli/src/config/settingsSchema.ts lines 2717-2726 carry codeModeOnly: { type: 'boolean', label: 'Code Mode Only (Experimental)', …, showInDialog: true }; the tag is fdcd7c2761 chore(release): v0.23.4 (2026-09-14) and gh release view v0.23.4 reports isPrerelease: false, published 2026-09-14, with feat(core): add CodeModeOnly tool execution (#10607) in its notes. The same day's nightly is a separate prerelease tag. grep -rn "nightly-only" docs/ returns exactly this one line, so it is not an established project term either.

A stable user who ticked "Code Mode Only (Experimental)" in the settings dialog reads "nightly-only", concludes the note cannot apply to their install, and skips the migration. When the compatibility path is later removed, tools.codeModeOnly becomes an unrecognised nested key: getSettingsFileKeyWarnings walks only top-level keys against the schema (packages/cli/src/config/settings.ts:262-277), so nothing is surfaced at all and the session silently reverts to direct.

Separately, the key is the only deprecated-but-still-honoured setting here with no row in the reference table, while its three siblings each have one — tools.exclude at :381 ("Deprecated. Use permissions.deny instead. … Not automatically migrated; the legacy setting remains honoured at startup."), and likewise tools.core at :380 and tools.allowed at :385. Its paragraph also sits inside a blockquote whose bolded lead-in at :404 names those three keys and not this one, so a user grepping the table for codeModeOnly gets no hit and concludes the key is inert.

Witness:

git show v0.23.4:packages/cli/src/config/settingsSchema.ts (lines 2717-2726)
  codeModeOnly: { type:'boolean', label:'Code Mode Only (Experimental)', …,
                  showInDialog: true }
git log --oneline -1 v0.23.4          -> fdcd7c2761 chore(release): v0.23.4  (2026-09-14)
gh release view v0.23.4 --json isDraft,isPrerelease,publishedAt
  -> {"isDraft":false,"isPrerelease":false,"publishedAt":"2026-09-14T15:20:21Z"}
  (notes include: feat(core): add CodeModeOnly tool execution (#10607))
same-day nightly tag                  -> isPrerelease: true   (a separate release)
grep -rn "nightly-only" docs/         -> 1 hit, this line only
Table sweep: whole-file grep for codeModeOnly in settings.md -> 1 hit, :406
Correction to one filed cost: `tools` sets no additionalProperties in
settings.schema.json, so an editor does NOT grey the key out as unknown.

Rewrite :404-408 in one edit: drop "nightly-only", and give the key either its own lead-in or a row beside tools.exclude — e.g. | `tools.codeModeOnly` | boolean | **Deprecated.** Use `tools.mode: "code_mode_only"` instead. Removed from the settings schema; still honoured at startup only while `tools.mode` is unset. Not shown in the settings dialog. | `undefined` | Requires restart. | — then reduce the note paragraph to a pointer at it. One rewrite closes both halves.

The precedence wording must match resolveToolModeSetting, which reads the legacy key only when tools.mode is absent (packages/cli/src/config/config.ts:142-166) and returns for any defined tools.mode including an unrecognized one — so the note must not imply the legacy key can rescue an invalid tools.mode. It must also not claim dialog support: tools.mode has showInDialog: true (settingsSchema.ts:2738) while codeModeOnly is no longer in the schema at all.

中文说明

R2-15(Suggestion):这段弃用说明把 tools.codeModeOnly 称为 nightly-only,但该键其实是在稳定版 v0.23.4 中随设置对话框一起发布的,于是这段说明把自己的受众限定错了;而且它是本文件中唯一一个「已弃用但仍生效」却没有出现在设置参考表格里的设置项。

git show v0.23.4:packages/cli/src/config/settingsSchema.ts 的 2717-2726 行含有 codeModeOnly: { type: 'boolean', label: 'Code Mode Only (Experimental)', …, showInDialog: true };该 tag 是 fdcd7c2761 chore(release): v0.23.4(2026-09-14),gh release view v0.23.4 报告 isPrerelease: false、发布于 2026-09-14,发布说明中含 feat(core): add CodeModeOnly tool execution (#10607)。同日的 nightly 是另一个 prerelease tag。grep -rn "nightly-only" docs/ 只返回这一行,因此它也不是项目中既有的术语。

一个在设置对话框里勾选过「Code Mode Only (Experimental)」的稳定版用户,读到「nightly-only」后会认为这段说明与自己的安装无关,从而跳过迁移。等到兼容路径日后被移除时,tools.codeModeOnly 就变成一个无法识别的嵌套键:getSettingsFileKeyWarnings 只按 schema 检查顶层键(packages/cli/src/config/settings.ts:262-277),因此不会有任何提示,会话会静默退回 direct

另外,该键是这里唯一一个「已弃用但仍生效」却没有参考表格行的设置项,而它的三个兄弟各有一行——tools.exclude:381(「Deprecated. Use permissions.deny instead. … Not automatically migrated; the legacy setting remains honoured at startup.」),tools.core:380tools.allowed:385 同理。它的段落还位于一个引用块内,而该引用块在 :404 的加粗引导句只点名了那三个键,没有点名它,因此在表格中搜索 codeModeOnly 的用户一无所获,会以为该键已失效。

修复方式:一次性重写 :404-408——去掉「nightly-only」,并为该键单独给出引导句,或在 tools.exclude 旁边加一行,例如 | `tools.codeModeOnly` | boolean | **Deprecated.** Use `tools.mode: "code_mode_only"` instead. Removed from the settings schema; still honoured at startup only while `tools.mode` is unset. Not shown in the settings dialog. | `undefined` | Requires restart. |,然后把说明段落精简为指向该行。一次重写即可同时解决两个方面。

约束:优先级的措辞必须与 resolveToolModeSetting 一致——它只在 tools.mode 缺失时读取旧键(packages/cli/src/config/config.ts:142-166),且对任何已定义的 tools.mode(包括无法识别的值)都会直接返回,因此说明不得暗示旧键能挽救一个非法的 tools.mode。也不得声称支持对话框:tools.modeshowInDialog: truesettingsSchema.ts:2738),而 codeModeOnly 已完全不在 schema 中。

— qwen3.8-max via Qwen Code /review (v0.23.3)

- 包含规范名称和 JavaScript 名称的 `ALL_TOOLS`;
- `text(value)`、`image(value)`、`audio(value)` 和 `exit()`;
- 从 JSON Schema 确定性生成的类 TypeScript 签名;
- 不提供 Node.js、import、网络 API、timer 和持久状态。

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

[Suggestion] R2-16: This new Chinese design doc asserts twice that the exec guest has no timers and lists only text/image/audio/exit, while the shipped sandbox installs setTimeout, clearTimeout and generatedImage as guest globals and the generated exec description advertises all three to the model.

packages/core/src/code-mode/host.ts:405-441 builds the __setTimeout/__clearTimeout host helpers and :477-478,486-488 re-exposes them to the guest as setTimeout/clearTimeout alongside generatedImage; packages/core/src/tools/code-mode.ts:283-285 tells the model "setTimeout(callback, delayMs?): schedule a callback to run later and return a timeout id. Pending timeouts do not keep exec alive by themselves". The shipped description's own absence-list at code-mode.ts:291 deliberately omits "timers" — "There is no Node.js, process, require, filesystem, network, import, console, WebAssembly, Atomics, or persistent state" — so the design doc is the only artifact still claiming their absence, and this PR propagates that stale claim into a second language. generatedImage is missing from the doc's list too, which breaks the documented image_gen round-trip the suite pins.

This is the only place in the docs tree that enumerates the guest capability boundary, and this PR adds it as the authoritative Chinese counterpart. An implementer hardening the sandbox, or writing the E2E probe the doc's own 验证 section calls for, takes 不提供 … timer as policy and strips or never exercises helpers the model is told it has.

To be precise about scope: the EN twins at code-mode-only.md:89-90 and :105 are pre-existing and unchanged by this diff, so correcting them is a legitimate follow-up — the ask here is that the new zh copy not be minted with a statement the code contradicts.

Witness:

DOC (added by this diff - code-mode-only.zh-CN.md is `new file mode 100644`, 166 lines):
  :78   - `text(value)`、`image(value)`、`audio(value)` 和 `exit()`;
  :80   - 不提供 Node.js、import、网络 API、timer 和持久状态。
  :93-94  guest 不提供 Node 全局变量、`require`、`process`、文件系统、socket、
          模块加载器、`console`、timer、`Atomics`、`SharedArrayBuffer` 或 `WebAssembly`。
CODE:
  host.ts:477-478  const setTimeout = (callback, delayMs) => hostSetTimeout(callback, delayMs);
                   const clearTimeout = (timeoutId) => hostClearTimeout(timeoutId);
  host.ts:486-488  generatedImage / setTimeout / clearTimeout installed as guest globals
  code-mode.ts:283-285  all three advertised to the model
  code-mode.ts:291  shipped absence-list omits "timers"
  code-mode.test.ts:463  pins the setTimeout description line

In both language files, list the advertised globals — text, image, audio, generatedImage, setTimeout, clearTimeout, exit — and scope the absence claim to what is genuinely absent (Node.js, process, require, filesystem, network, import, console, WebAssembly, Atomics, SharedArrayBuffer, persistent state), matching the description text in code-mode.ts.

packages/core/src/tools/code-mode.ts:284 — "Pending timeouts do not keep exec alive by themselves; await an explicit promise if you need to wait for one." The corrected doc must not imply pending timers keep the guest alive, and must leave the teardown sentence alone (code-mode-only.md:117 / zh :101-102, "no child process, timer, promise handle, or guest global survives the call") — that one is true, since host.ts:553-558 clears every timer in finally. The correction is about timers being unavailable (false), not about pending timers surviving (true); do not conflate the two.

中文说明

R2-16(Suggestion):这份新增的中文设计文档两次声称 exec guest 没有 timer,并且只列出 text/image/audio/exit,而实际发布的沙箱会把 setTimeoutclearTimeoutgeneratedImage 安装为 guest 全局量,生成的 exec 描述也把这三者都告知了模型。

packages/core/src/code-mode/host.ts:405-441 构造了 __setTimeout/__clearTimeout 宿主辅助函数,:477-478,486-488 又把它们以 setTimeout/clearTimeout 的形式连同 generatedImage 一起重新暴露给 guest;packages/core/src/tools/code-mode.ts:283-285 告诉模型「setTimeout(callback, delayMs?): schedule a callback to run later and return a timeout id. Pending timeouts do not keep exec alive by themselves」。而发布版本描述自身的「不存在清单」在 code-mode.ts:291 刻意省略了「timers」——「There is no Node.js, process, require, filesystem, network, import, console, WebAssembly, Atomics, or persistent state」——因此设计文档是唯一仍声称它们不存在的产物,而本 PR 把这一过时说法又传播到了第二种语言。文档列表中也缺少 generatedImage,这与测试套件所固定的 image_gen 往返流程相冲突。

这是文档树中唯一一处枚举 guest 能力边界的地方,而本 PR 把它作为权威的中文对照版本加入。一个正在加固沙箱的实现者,或正在编写该文档「验证」一节所要求的 E2E 探针的人,会把「不提供 … timer」当作策略,从而移除或从不去验证模型被告知拥有的那些辅助函数。

关于范围需说明清楚:英文对照版本 code-mode-only.md:89-90:105 是既有内容、本 diff 未改动,因此修正它们可以作为合理的后续工作——本条的要求是,新增的中文副本不应带着一个与代码相矛盾的陈述被创建出来。

修复方式:在两种语言的文档中列出实际对外声明的全局量——textimageaudiogeneratedImagesetTimeoutclearTimeoutexit——并把「不存在」的断言限定在确实不存在的内容上(Node.js、processrequire、文件系统、网络、importconsoleWebAssemblyAtomicsSharedArrayBuffer、持久状态),与 code-mode.ts 中的描述文本保持一致。

约束:packages/core/src/tools/code-mode.ts:284——「Pending timeouts do not keep exec alive by themselves; await an explicit promise if you need to wait for one.」修正后的文档不得暗示待处理的 timer 会让 guest 保持存活,也不要改动清理(teardown)那句(code-mode-only.md:117 / 中文 :101-102,「no child process, timer, promise handle, or guest global survives the call」)——那一句是真的,因为 host.ts:553-558 会在 finally 中清除每个 timer。本次修正针对的是「timer 不可用」(假),而非「待处理 timer 会存活」(真);两者不可混为一谈。

— qwen3.8-max via Qwen Code /review (v0.23.3)

Comment thread docs/design/code-mode.md
| Ordinary eager tools | Direct | Direct and nested | Nested only |
| Deferred tools | `tool_search` | `tool_search` and nested | Nested only |
| Direct-control tools | Direct | Direct only | Direct only |
| `exec` | Hidden | Direct | Direct |

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

[Suggestion] R2-17: This new exposure table labels exec in direct mode as "Hidden" — the term this feature's own code reserves for registered-but-undeclared tools, which the very next row uses for tool_search — while exec under direct is not registered at all.

The two states differ observably: a hidden tool is in ToolRegistry.tools and answers getTool(), /tools list and diagnostics, while an unregistered tool answers undefined and appears in no listing. getToolExposure() never returns 'hidden' for exec — it returns the distinct 'exec' category in every mode (code-mode.ts:55) — so this cell has no code counterpart. config.ts:10579 reads if (!isCodeModeEnabled(this.getToolMode())) return; inside registerExecIfEnabled, so exec is never added to the registry in direct.

A maintainer treating this table as the normative spec for the new mode family assumes exec exists in the registry under direct and writes a lookup, an --exclude-tools exec check or a diagnostic against it. The sibling doc added by this same PR states the accurate rule and therefore contradicts the table: code-mode-only.md:43-45, "exec is only registered when a code mode is enabled, so selecting direct also removes it from diagnostics and registry listings" (zh mirror code-mode-only.zh-CN.md:40). Nothing in the diff documents a deliberate loose use of "Hidden", so the deliberate-design reading does not apply. The zh twin at code-mode.zh-CN.md:48 (隐藏) needs the same correction.

Witness:

Probe driving the real registry built from this head:
=== tools.mode = direct
 exec:        inRegistry(getAllToolNames)=false  getTool()=undefined
              declaredTopLevel=false  getToolExposure()='exec'
 tool_search: inRegistry=true  getTool()=instance
              declaredTopLevel=true   getToolExposure()='hidden'
=== tools.mode = code_mode_only
 exec:        inRegistry=true  getTool()=instance
              declaredTopLevel=true   getToolExposure()='exec'
 tool_search: inRegistry=true  getTool()=instance
              declaredTopLevel=false  getToolExposure()='hidden'  <- row 51's "Hidden"

In both language files, replace the direct cell of the exec row with a term that means "not registered" — Not registered / 未注册 — and leave "Hidden" on row 51, where it matches HIDDEN_TOOLS.

packages/core/src/config/config.ts:10579 (if (!isCodeModeEnabled(this.getToolMode())) return;) means ToolNames.EXEC is never added to the registry in direct, and packages/core/src/tools/code-mode.ts:37,53-57 has getToolExposure() return 'exec' for ToolNames.EXEC and 'hidden' only for HIDDEN_TOOLS (tool_search, tool_call) — so the replacement wording must not claim exec is classified hidden anywhere. Per docs/design/README.md the correction lands in both language files in the same change.

中文说明

R2-17(Suggestion):这张新增的暴露表格把 direct 模式下的 exec 标为「Hidden」——而这个术语在本特性的代码中专门指「已注册但未声明」的工具,紧接着的下一行正是用它来描述 tool_search——但 direct 模式下的 exec 根本就没有被注册。

这两种状态在可观测层面上不同:hidden 的工具存在于 ToolRegistry.tools 中,getTool()/tools list 与诊断信息都能返回它;而未注册的工具返回 undefined,在任何列表中都不出现。getToolExposure()exec 从不返回 'hidden'——在任何模式下它都返回独立的 'exec' 类别(code-mode.ts:55)——因此这个单元格在代码中没有对应物。config.ts:10579registerExecIfEnabled 内部是 if (!isCodeModeEnabled(this.getToolMode())) return;,所以 direct 下 exec 永远不会被加入 registry。

一个把这张表当作新模式族规范性说明的维护者,会以为 direct 下 registry 中存在 exec,进而针对它编写查找逻辑、--exclude-tools exec 检查或诊断代码。本 PR 同时新增的姊妹文档陈述了准确的规则,因此与该表相矛盾:code-mode-only.md:43-45,「exec is only registered when a code mode is enabled, so selecting direct also removes it from diagnostics and registry listings」(中文对照 code-mode-only.zh-CN.md:40)。diff 中没有任何地方记载这是有意宽松使用「Hidden」,因此「刻意设计」这一解读不成立。code-mode.zh-CN.md:48 的中文对照(隐藏)需要同样的修正。

修复方式:在两种语言的文档中,把 exec 行的 direct 单元格换成表示「未注册」的词——Not registered / 未注册——并把「Hidden」留在第 51 行,那里它与 HIDDEN_TOOLS 相符。

约束:packages/core/src/config/config.ts:10579if (!isCodeModeEnabled(this.getToolMode())) return;)意味着 directToolNames.EXEC 永远不会被加入 registry;packages/core/src/tools/code-mode.ts:37,53-57getToolExposure()ToolNames.EXEC 返回 'exec',只对 HIDDEN_TOOLStool_searchtool_call)返回 'hidden'——因此替换后的措辞不得声称 exec 在任何地方被归类为 hidden。依据 docs/design/README.md,该修正须在同一次改动中同时落到两种语言的文档。

— qwen3.8-max via Qwen Code /review (v0.23.3)

由于后续无法暴露顶层声明,所有嵌套声明都集中在 `exec` 描述中。

经过过滤的子智能体声明沿用相同模式。智能体的 `tools` 列表会收窄直接调用面。
显式列出的普通工具会收窄嵌套集合;继承或显式允许的 `exec` 则携带所有仍可用的

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

[Suggestion] R2-18: This new Chinese design doc drops the English original's execution-entry qualifier, so it documents one narrowing mechanism where the code and code-mode.md:61 have two — and a Chinese reader cannot learn that executionAllowedTools narrows the nested set at all.

EN code-mode.md:60-63 reads "The agent's tools list narrows its direct surface. Explicit ordinary-tool execution entries narrow the nested set, while an inherited or explicitly allowed exec carries all surviving code-mode-callable bindings." Those are two distinct inputs: toolConfig.toolsconfiguredNames (agent-core.ts:745-747), and toolConfig.executionAllowedToolsisToolExecutionAllowed(name, true) inside the allowedNames filter that feeds codeModeAllowedToolNames (agent-core.ts:759-766, gate at :1714-1736). The Chinese renders the second clause as 显式列出的普通工具 immediately after a sentence whose subject is 智能体的 tools 列表, so the natural reading attributes the narrowing to tools.

A reader shipping a fork profile with executionAllowedTools: ['read_file'] and no tools key then believes the third clause applies — "an inherited … exec carries all surviving code-mode-callable bindings" — and expects every code-mode-callable tool to be nested-reachable, while the guest script's tools.<name>(…) calls fail for everything but read_file. The sibling pair is consistent (code-mode-only.md:159 "Explicit ordinary-tool entries" ↔ code-mode-only.zh-CN.md:148 「显式列出的普通工具」), which is what makes this doc's dropped word a divergence rather than house style.

Witness:

Probe against HEAD source:
  {"tools":[],"executionAllowedTools":["read_file"]}
      -> codeModeAllowedToolNames ["read_file"]              <- the execution allowlist narrows
  {"tools":["exec","read_file"]}
      -> codeModeAllowedToolNames ["read_file","write_file","skill"]
  tools=['*']
      -> codeModeAllowedToolNames ["read_file","write_file","skill"]   <- `tools` does NOT
Both docs are new in this diff (`new file mode 100644`).
Rule quoted verbatim, docs/design/README.md:26-28:
  "Preserve technical identifiers, file paths, commands, configuration keys, protocol
   fields, example values, and numeric limits. Translate explanatory prose and captions
   without changing the behavior they describe."
Correction to the filing: neither language version names the key `executionAllowedTools`,
so "the only sentence naming a configuration key" is inaccurate - the operative clause
is the second one.

Restore the execution-entry subject in the Chinese, e.g. 显式列出的普通工具执行项(`executionAllowedTools`)会收窄嵌套集合;继承或显式允许的 `exec` 则携带所有仍可用的 code-mode-callable binding。, keeping the tools-list sentence at :56 as the direct-surface rule so the two mechanisms stay distinguishable as they are in English.

Do not fix the Chinese by mirroring the English verbatim — code-mode.md:60-61 ("The agent's tools list narrows its direct surface") is itself contradicted by agent-core.ts:772's unconditional name === ToolNames.EXEC ||, which is the subject of a separate finding. Per docs/design/README.md, "Create or update both versions in the same change and submit them together in the same PR", so the two edits must land together.

中文说明

R2-18(Suggestion):这份新增的中文设计文档丢掉了英文原文中「执行项」这一限定语,因此在代码与 code-mode.md:61 存在两种收窄机制的地方,它只记载了一种——中文读者完全无从得知 executionAllowedTools 也会收窄嵌套集合。

英文 code-mode.md:60-63 写的是「The agent's tools list narrows its direct surface. Explicit ordinary-tool execution entries narrow the nested set, while an inherited or explicitly allowed exec carries all surviving code-mode-callable bindings.」这是两个不同的输入:toolConfig.toolsconfiguredNamesagent-core.ts:745-747),以及 toolConfig.executionAllowedTools → 在喂给 codeModeAllowedToolNamesallowedNames 过滤中调用 isToolExecutionAllowed(name, true)agent-core.ts:759-766,判断在 :1714-1736)。中文把第二个分句译作「显式列出的普通工具」,且紧接在一个主语为「智能体的 tools 列表」的句子之后,因此自然的读法会把收窄归因于 tools

于是一个提交了 executionAllowedTools: ['read_file'] 且不带 tools 键的 fork 配置的读者,会认为适用第三个分句——「继承或显式允许的 exec 则携带所有仍可用的 code-mode-callable binding」——并期待每个 code-mode-callable 工具都能通过嵌套调用到达,而 guest 脚本中的 tools.<name>(…) 调用除 read_file 外全部失败。姊妹文档对是一致的(code-mode-only.md:159 「Explicit ordinary-tool entries」↔ code-mode-only.zh-CN.md:148「显式列出的普通工具」),这正说明本文档丢掉的那个词是偏离,而非项目既有风格。

修复方式:在中文里恢复「执行项」这一主语,例如改为「显式列出的普通工具执行项(executionAllowedTools)会收窄嵌套集合;继承或显式允许的 exec 则携带所有仍可用的 code-mode-callable binding。」,同时保留 :56 那句关于 tools 列表的直接调用面规则,使两种机制像英文那样可以区分。

约束:不要通过逐字照搬英文来修中文——code-mode.md:60-61(「The agent's tools list narrows its direct surface」)本身与 agent-core.ts:772 中无条件的 name === ToolNames.EXEC || 相矛盾,那是另一条发现的主题。依据 docs/design/README.md,「Create or update both versions in the same change and submit them together in the same PR」,因此两处修改必须一起落地。

— qwen3.8-max via Qwen Code /review (v0.23.3)

@doudouOUC doudouOUC left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

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-2264 passes settings.tools?.mode straight through (? (settings.tools?.mode ?? ToolMode.Direct)), while packages/core/src/config/config.ts:10399 gates registration with a denylist (if (this.getToolMode() === ToolMode.Direct) return;). An out-of-union value therefore registers exec while the other read sites treat the session as direct. 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 only exec authorises direct calls to every code-mode-callable tool. In hybrid there is no second wall (the scheduler gate is CodeModeOnly-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 ToolMode is a value import evaluated at module load in settingsSchema.ts (it is), and the second only that agent-core.ts narrows declarationNames to {exec} ∪ configuredNames while 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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants