fix(core): make permissions.allow restrict the tool schemas sent to the model

[yiliang114]

What this PR does

Makes permissions.allow act as the registry-level allowlist the settings docs promise: when at least one allow rule is configured in settings.permissions.allow, built-in tools not covered by any allow rule are no longer registered — so they disappear from /tools and, more importantly, their schemas are never included in the tools array sent to the model. Previously permissions.allow only auto-approved matching calls; registration was never gated, so the full built-in tool set was always sent regardless of the allowlist.

Also completes the permission-rule alias map so rules written with the display names shown by /tools (SendMessage, UpdateGoal, LoopWakeup, ...) actually match — before this change such rules silently never matched anything, which is why the reporter's permissions.deny attempts had no effect while --exclude-tools (wire names) worked.

Compatibility decisions, deliberately conservative:

• Only settings.permissions.allow activates the allowlist. --allowed-tools, the SDK allowedTools param, and the legacy tools.allowed key keep their documented pure auto-approval semantics, and rules granted mid-session ("Always allow", skill allowedTools grants) extend allowlist membership but can never activate it — so approving one tool mid-session can never permission-error the rest of the running session.
• MCP tools and the synthetic structured_output contract (--json-schema terminal contract) are exempt from the allowlist, matching the existing --core-tools exemptions.
• Specifier allow rules keep their tool registered (Bash(npm test) registers the shell tool); meta-category semantics are identical to runtime rule matching (Read covers grep/glob/..., Bash covers monitor).

Why it's needed

Fixes #9827. The settings docs migration table promises tools.core → permissions.allow with "unlisted tools are disabled at registry level", but the implementation never did that. For reporters using constrained-decoding backends (llama.cpp / Docker Model Runner), which compile every tool schema into one GBNF grammar, the always-full tools array is fatal: built-in schemas with large maxLength params (send_message.message → 65536, loop_wakeup.prompt → 10000, update_goal.reason → 8000, read_mcp_resource.uri → 4096) exceed llama.cpp's repetition threshold and grammar compilation fails, breaking all tool calling for the session. There was no settings-level way to avoid sending those schemas (--exclude-tools worked, permissions.allow/permissions.deny with display names did not).

Reviewer Test Plan

How to verify

Reproduced end-to-end by pointing the CLI at a recording OpenAI-compatible mock server and capturing the outgoing tools array, with the reporter's exact settings (permissions.allow: ["ReadFile", "WriteFile", "Edit", "Grep", "Glob", "ListFiles", "Shell", "WebFetch"]):

• Before (original main): first chat request carries 63 tool schemas — send_message, update_goal, get_goal, loop_wakeup, read_mcp_resource, agent, skill, todo_write, all computer_use__*, etc. — despite the allowlist. Bug reproduced.
• After (this PR): the same configuration sends 10 schemas — only tools covered by the allow rules (read_file, write_file, edit, notebook_edit, grep_search, glob, zoom_image, run_shell_command, web_fetch, plus monitor, which Shell rules cover on purpose so the shell can't be bypassed). The reporter's grammar-breaking tools are all gone.
• Regression guards: with no permissions configured the request still carries all 63 tools; --allowed-tools ReadFile alone also keeps all 63 (auto-approve only); --exclude-tools send_message,update_goal still removes exactly those (61 left).

Unit tests: the PR replaces the old "permissionsAllow is not a whitelist" test with the new documented semantic and adds a dedicated suite (permissions.allow registry allowlist (#9827) in permission-manager.test.ts: activation, membership, specifier/meta-category coverage, deny precedence, MCP + structured_output exemptions, session-rule safety, AUTO-mode-stripped rules, coreTools combination, --allowed-tools non-activation, malformed-rule safety), registry-level registration tests in core config.test.ts, and CLI wiring tests asserting only settings.permissions.allow becomes the registry allowlist. On the original sources 11 of the 15 new permission-manager tests fail (red), with the fix all pass.

Evidence (Before & After)

Captured request payloads (mock server log):

# Before (original main, permissions.allow configured):
POST /v1/chat/completions tools=63   ← full tool set incl. send_message/update_goal/loop_wakeup/read_mcp_resource

# After (this PR, same settings):
POST /v1/chat/completions tools=10   ← edit, glob, grep_search, monitor, notebook_edit, read_file,
                                        run_shell_command, web_fetch, write_file, zoom_image

Tested on

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

Environment (optional)

Node v24.19.0, built CLI (node scripts/build.js --cli-only) run headless against a local recording mock OpenAI endpoint; targeted vitest suites in packages/core and packages/cli; tsc --noEmit for core/cli/sdk-typescript; eslint + prettier clean.

Risk & Scope

• Main risk or tradeoff: sessions that configured settings.permissions.allow purely for auto-approval will, after restart, see unlisted built-in tools disappear from the model's toolset. That is the behavior the docs migration table promises (and what this issue asks for), but it is a behavior change; release notes should call it out. Activation is restart-scoped and requires a settings-sourced allow rule, so mid-session approvals and --allowed-tools/SDK users are unaffected.
• Not validated / out of scope: MCP tool filtering (intentionally exempt — per-server includeTools/excludeTools and tools.disabled cover it); the dead migrateLegacyPermissions() code in packages/cli/src/config/settings.ts; interactive TUI verification (request-layer change, no UI surface beyond /tools, which reads the same registry).
• Breaking changes / migration notes: none for users without settings.permissions.allow; for users with it, the documented allowlist semantic now applies (restart required). settings.tools.disabled remains the knob for hiding individual tools without an allowlist.

Linked Issues

Fixes #9827

Related: #982 (original tools.core allowlist request)

中文说明

这个 PR 做了什么

让 permissions.allow 真正成为 settings 文档承诺的注册表级白名单：只要 settings.permissions.allow 配置了至少一条规则，未被任何 allow 规则覆盖的内置工具就不再注册——既不会出现在 /tools，其 schema 也不会进入发给模型的 tools 数组。此前 permissions.allow 只做自动批准，从不约束注册，所以无论白名单怎么配，请求里始终带上全部内置工具。

同时补全了权限规则的别名表：用 /tools 显示的展示名（SendMessage、UpdateGoal、LoopWakeup 等）写的规则现在能真正匹配——改动前这类规则会静默失配，这正是报告者 permissions.deny 无效、而 --exclude-tools（wire 名）有效的原因。

兼容性上刻意保守：

• 只有 settings.permissions.allow 会激活白名单。--allowed-tools、SDK 的 allowedTools 参数、遗留 tools.allowed 键保持文档承诺的纯自动批准语义；会话中临时授予的规则（"Always allow"、skill 的 allowedTools）只扩展白名单成员、永远不会激活白名单——避免会话中途批准一个工具就把其余工具全部权限报错。
• MCP 工具和 --json-schema 的合成 structured_output 契约工具豁免，与现有 --core-tools 的豁免一致。
• 带参数的 allow 规则保留其工具注册（Bash(npm test) 会注册 shell 工具）；元类别语义与运行时规则匹配一致（Read 覆盖 grep/glob/…，Bash 覆盖 monitor）。

为什么需要

修复 #9827。settings 文档迁移表承诺 tools.core → permissions.allow 且"未列出的工具在注册表级禁用"，但实现从未如此。对使用受限解码后端（llama.cpp / Docker Model Runner，把所有工具 schema 编成单一 GBNF grammar）的用户，全量 tools 数组是致命的：send_message.message（65536）、loop_wakeup.prompt（10000）、update_goal.reason（8000）、read_mcp_resource.uri（4096）等大 maxLength 参数超出 llama.cpp 重复上限，grammar 编译直接失败，整个会话的工具调用全挂。此前没有任何 settings 层面的办法避免发送这些 schema（--exclude-tools 有效，permissions.allow/permissions.deny 用展示名则无效）。

测试方式

• 端到端复现：用本地录像式 mock OpenAI 端点抓取出站 tools 数组，配置与报告者完全一致。修复前首个请求携带 63 个工具（含全部 grammar 破坏者）；修复后同样配置只剩 10 个（仅 allow 规则覆盖的工具；monitor 因 Shell 元类别有意保留，防止绕过 shell）。

• 回归保护：不配置任何 permissions 时仍发送全部 63 个工具；仅 --allowed-tools ReadFile 也保持 63 个（纯自动批准）；--exclude-tools send_message,update_goal 仍精确排除（61 个）。

• 单测：新增权限管理器白名单专项套件、core 注册层测试、CLI 接线测试。在原始代码上 15 个新测试中 11 个失败（红），应用修复后全部通过；相关目标套件 1680+ 用例全绿；core/cli/sdk typecheck、eslint、prettier 均通过。

• 主要风险：此前把 settings.permissions.allow 纯当自动批准用的用户，重启后会发现未列出的内置工具从模型工具集中消失。这是文档承诺的行为（也是本 issue 的诉求），但属于行为变更，建议 release notes 提示。激活仅限重启后生效、且仅 settings 来源的 allow 规则触发，会话中途批准与 --allowed-tools/SDK 用户不受影响。

• 未覆盖：MCP 工具过滤（有意豁免，走各 server 的 includeTools/excludeTools 与 tools.disabled）；settings.ts 中的死代码 migrateLegacyPermissions()；TUI 交互验证（本改动在请求层，/tools 与请求读同一注册表，天然一致）。

• 无迁移负担：未配置 settings.permissions.allow 的用户行为完全不变。

[qwen-code-ci-bot]

✅ Qwen Triage finished — view run. See the stage comments in this thread for the result.

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

[qwen-code-ci-bot]

Re-run gate pass at the current head.

• Template: still complete ✓ — all required sections present, bilingual summary included.
• Problem: unchanged from round 1 — an observed bug with solid evidence, not theoretical hardening. permissions.allow (and legacy tools.core/tools.exclude) do not restrict the tool schemas sent to the model — full tool set still included in API request #9827 is triaged (priority/P2, type/bug) with exact reproduction steps, and the docs migration table (tools.core → permissions.allow, "unlisted tools are disabled at registry level") already promises what this PR implements.
• Direction: unchanged — aligned. It makes the implementation honor a documented contract, and the compatibility calls (settings-only activation; --allowed-tools / SDK allowedTools / legacy tools.allowed stay pure auto-approval; mid-session grants extend membership but never activate) remain the right ones.
• Size: the diff grew through eight review rounds. At this head: ~612 production lines (permission-manager.ts 288, rule-parser.ts 98, core config.ts 104, coreToolScheduler.ts 34, tool-registry.ts 22, cli config.ts 10, sdk types.ts 25, shims/docs comments 31) vs ~1658 test lines vs ~96 docs lines. The 500+ production-line threshold is crossed, which normally calls for maintainer awareness — noted here for the record; the author is a repo maintainer (admin) and the change sits exactly where it should (permissions/ + the registry chokepoints), so this is informational, not an escalation. Under the 1000-line advisory either way.
• Approach: scope still matches the stated goal. The final three commits (since the maintainer's wire A/B verification) are tests and docs only — verified via the compare API: no production delta, no drive-by changes.
• Risk: no Stage 1e high-risk path matches (no streamingToolCallParser / geminiChat / acpConnection / shell / mcp / sandbox surfaces touched).

Moving on to code review. 🔍

中文说明

在当前 head 上重跑门禁。

• 模板：依然完整 ✓ —— 必需章节齐全，含中文说明。
• 问题：与第一轮一致 —— 已观测到的 bug、证据充分，不是理论性加固。permissions.allow (and legacy tools.core/tools.exclude) do not restrict the tool schemas sent to the model — full tool set still included in API request #9827 已分级（priority/P2、type/bug），有精确复现步骤；文档迁移表（tools.core → permissions.allow，"未列出的工具在注册表级禁用"）早已承诺了本 PR 实现的行为。
• 方向：不变 —— 对齐。让实现兑现文档承诺；兼容性决策（仅 settings 激活；--allowed-tools / SDK allowedTools / 遗留 tools.allowed 保持纯自动批准；会话中授予只扩展成员、永不激活）依然正确。
• 规模：经过八轮 review 后 diff 有所增长。当前：约 612 行生产代码（permission-manager.ts 288、rule-parser.ts 98、core config.ts 104、coreToolScheduler.ts 34、tool-registry.ts 22、cli config.ts 10、sdk types.ts 25、shim/文档注释 31），约 1658 行测试，约 96 行文档。超过 500 行生产代码阈值，按惯例需提醒维护者关注 —— 在此记录；作者是仓库维护者（admin），改动也恰好落在该在的位置（permissions/ + 注册表咽喉点），故仅作信息说明、不构成升级。同时也低于 1000 行大 PR 建议线。
• 方案：范围仍与目标匹配。最后三个提交（维护者 wire A/B 验证之后）只有测试与文档 —— 已用 compare API 核实：无生产代码增量、无顺手改动。
• 风险：未命中 Stage 1e 高风险路径（未触及 streamingToolCallParser / geminiChat / acpConnection / shell / mcp / sandbox 等面）。

进入代码审查。🔍

— Qwen Code · qwen3.8-max

_{Reviewed at 5e63280089932c6b323d3b83df98884c72b70969 · re-run with @qwen-code /triage}

[qwen-code-ci-bot]

Code review (re-run at this head)

Both round-1 holds are resolved — I re-verified at this head, not taken on trust:

• The SDK docs now match the implementation. types.ts JSDoc (and settings.md, sdk-typescript.md, the SDK README) now state the real predicate: only permissions.allow in settings.json activates the registry allowlist (≥1 valid rule; malformed entries don't count), the SDK allowedTools param maps to --allowed-tools and cannot activate it alone — but while the allowlist is active those rules merge into the effective allow set and count toward coverage. That was the round-1 finding; it's correctly reworded on all four surfaces.
• CI signal now exists — see the table below; the round-1 workflow-size-baseline blocker no longer applies and the unit suite ran green.

The round-1 architecture read still holds (gate at isToolEnabled, the single chokepoint consulted by registry construction, the runtime scheduler, and the ACP session; activation snapshotted at initialize(); monotonic membership). On top of that, this pass checked:

• The discovered-tools path is gated too (tool-registry.ts): a command-discovered tool not covered by an active allowlist is skipped at registration instead of advertised-then-rejected. Both sides of the gate agree.
• The scheduler's error message distinguishes gates: an allowlist miss (uncovered tool) gets the "add a rule and restart" advice; a deny-rule or legacy coreTools rejection keeps the pre-permissions.allow (and legacy tools.core/tools.exclude) do not restrict the tool schemas sent to the model — full tool set still included in API request #9827 message, so the advice is never a no-op. The scoped PermissionManager shims (memory-scoped agent, skill-review planner) delegate isPermissionsAllowListActive instead of throwing.
• isLsToolEnabled mirrors the gate exactly — same activation predicate (including the non-string/empty-entry guards) and same coverage semantics (allow ∪ ask, meta-categories via toolMatchesRuleToolName), so the opt-in list_directory can't drift from isToolEnabled.
• The delta since the maintainer's wire A/B is tests + docs only (compare c9b670e...5e63280: two test files, four doc surfaces). The production code covered by that 34/34 verification is bit-identical to what's under review.

One standing item — disclosed, not treated as blocking this run: the review lane's R2-2. It still reproduces statically at this head: ProceedAlways* → persistPermissionOutcome → onPersistPermissionRule writes an interactive "Always allow" grant into settings permissions.allow (cli config.ts ~2216), and this PR's wiring (~2205) consumes exactly that key unfiltered as the allowlist activator. So a single "Always allow" click — with no hand-authored allowlist in place — silently collapses the built-in toolset to that one granted family on the next restart. The PR's "mid-session grants can never activate" invariant holds within a session; it does not hold across a restart, because the grant persists. The author confirmed it real and escalated it for a maintainer settings-format decision; no code fix has landed and no follow-up issue exists yet. The maintainer approved this exact head after that review — I read the approval as the product decision (ship now, fix forward). Please file the follow-up issue (persist interactive grants under a non-activating key, or gate activation on hand-authored intent) so it isn't lost.

CI test evidence

This run is unattended CI — I do not build or run PR code; the evidence below is the PR's own CI fetched via the API. All pull_request-event workflow runs on this head are completed; the unit suite is green. The skipped test jobs are structural, not failures: Integration Tests (CLI, No Sandbox) is merge-queue-only by design (if: github.event_name == 'merge_group' in ci.yml), and the macOS/Windows matrix legs are skipped for this fork push. A sandboxed /verify run was triggered and is still in flight (run 32828262972) — it will post its report in its own comment; note it exercises the same production code the maintainer already A/B-verified locally (see the 34/34 wire-oracle verification comment above).

Check	Conclusion
Test (ubuntu-latest, Node 22.x)	✅ success
Desktop Shell (ubuntu-22.04)	✅ success
Desktop Shell (windows-2022)	✅ success
web-shell E2E Smoke (ubuntu-latest, Node 22.x)	✅ success
Secret scan (TruffleHog)	✅ success
Dependency CVE audit	✅ success
Classify PR / label / precheck	✅ success
Integration Tests (CLI, No Sandbox)	⏭️ skipped (merge-queue-only by design)
Test (macos-latest / windows-latest, Node 22.x)	⏭️ skipped

中文说明

代码审查（在此 head 上重跑）

第一轮的两个保留项都已解决 —— 我在当前 head 上重新核实，不是照单全收：

• SDK 文档已与实现一致。 types.ts JSDoc（以及 settings.md、sdk-typescript.md、SDK README）现在写的是真实判定条件：只有 settings.json 里的 permissions.allow 会激活注册表白名单（至少一条有效规则；畸形条目不计入）；SDK 的 allowedTools 参数映射到 --allowed-tools，自身不能激活白名单 —— 但白名单激活期间，这些规则会并入有效 allow 集合并计入覆盖。这是第一轮的发现，四处表述都已正确修正。
• CI 信号现已存在 —— 见下表；第一轮的工作流体积基线阻塞已不再适用，单测套件已跑绿。

第一轮的架构结论依然成立（在 isToolEnabled 设门 —— 注册表构建、运行时调度器、ACP 会话共同咨询的唯一咽喉点；激活在 initialize() 快照；成员单调）。本轮另外核实：

• 命令发现的工具路径也设了门（tool-registry.ts）：未被激活白名单覆盖的发现工具在注册时即被跳过，而不是"先广告、后拒绝"。门两侧一致。
• 调度器错误信息区分了不同的门：白名单未覆盖的 miss 给出"加规则并重启"的指引；deny 规则或遗留 coreTools 拒绝保留 permissions.allow (and legacy tools.core/tools.exclude) do not restrict the tool schemas sent to the model — full tool set still included in API request #9827 之前的措辞，指引永远不会是空操作。作用域 PermissionManager shim（memory-scoped agent、skill-review planner）改为委托 isPermissionsAllowListActive，不再抛异常。
• isLsToolEnabled 与门完全镜像 —— 激活判定一致（含非字符串/空条目守卫）、覆盖语义一致（allow ∪ ask、经 toolMatchesRuleToolName 支持元类别），opt-in 的 list_directory 不会与 isToolEnabled 漂移。
• 维护者 wire A/B 之后的增量只有测试与文档（比较 c9b670e...5e63280：两个测试文件、四处文档面）。那次 34/34 验证覆盖的生产代码与本次审查对象逐位一致。

一个遗留项 —— 公开披露，但本轮不作为阻断：review 通道的 R2-2。 在此 head 上静态仍可复现：ProceedAlways* → persistPermissionOutcome → onPersistPermissionRule 会把交互式 "Always allow" 授权写进 settings 的 permissions.allow（cli config.ts ~2216），而本 PR 的接线（~2205）恰好无条件消费同一个键作为白名单激活源。因此在没有手写白名单的情况下，一次 "Always allow" 点击就会让下次重启时内置工具集静默坍缩到那一个被授权的工具族。PR 的"会话中授予永不激活"不变式在会话内成立；跨重启不成立 —— 因为授权会持久化。作者已确认属实并升级给维护者做 settings 数据格式决策；代码修复尚未落地，也还没有跟进 issue。维护者在那次 review 之后批准了这个 head —— 我把该批准理解为产品决策（先合入、后续修复）。请建一个跟进 issue（把交互式授权持久化到非激活键，或把激活限定为手写意图），以免遗失。

CI 测试证据

本轮是无人值守 CI —— 我不构建、不运行 PR 代码；以上证据全部来自 PR 自身 CI 的 API 读取。此 head 上所有 pull_request 事件的工作流运行均已完成，单测套件绿色。被跳过的测试作业是结构性的、不是失败：Integration Tests (CLI, No Sandbox) 按设计只在 merge queue 触发（ci.yml 中 if: github.event_name == 'merge_group'）；macOS/Windows 矩阵腿在此 fork 推送中被跳过。沙箱 /verify 运行已被触发、仍在进行中（见上方链接），报告将发布在它自己的评论里；它验证的生产代码与维护者已在本地 A/B 验证过的完全相同（见上方 34/34 wire-oracle 验证评论）。

— Qwen Code · qwen3.8-max

_{Reviewed at 5e63280089932c6b323d3b83df98884c72b70969 · re-run with @qwen-code /triage}

[qwen-code-ci-bot]

Confidence: 4/5 — both round-1 holds are verifiably resolved at this head, and the one known footgun is disclosed, maintainer-accepted, and tracked below rather than fixed here.

Stepping back: this is the fix #9827 needed, built to the design I'd have chosen independently — gate at the single isToolEnabled chokepoint, settings-only activation snapshotted at startup, monotonic membership, MCP / structured_output / plan-lifecycle / deferred-family exemptions that each serve the schema-shrink goal instead of fighting it. Eight review rounds have visibly worked: the alias map, the discovered-tools gate, the scheduler message arms, the shim delegations, and the doc wording are all places where earlier drafts were wrong and this head is right. The test-to-production ratio (~2.7:1) and the maintainer's independent wire A/B (34/34, with red controls on base — the suite genuinely pins the change) are what a load-bearing fix looks like. If I'm maintaining this in six months I'll thank the author.

What keeps this at 4 rather than 5: the R2-2 interaction ("Always allow" persisting into the very key that now activates the allowlist, collapsing the toolset on next restart) is real and still unfixed — see Stage 2 for the mechanism. It is disclosed, the maintainer approved this exact head with the review lane's finding on record, and I'm treating that as the product decision; a follow-up issue is the one thing I'd like to see filed before merge so the fix-forward is tracked. The skipped integration/matrix legs are by design (merge-queue-only) or fork-lane structural, not evidence against.

The bot's earlier CHANGES_REQUESTED reviews on this PR belong to the review lane's rounds; their standing item was escalated to the maintainer and resolved by the maintainer's approval of this exact head, so this run's verdict supersedes them. Approving, pinned to the reviewed commit. ✅

中文说明

置信度：4/5 —— 第一轮的两个保留项在此 head 上均已可核实地解决；唯一已知的坑已公开披露、经维护者接受，并在下文登记，而不是在本 PR 内修复。

退一步看：这就是 #9827 需要的修复，也与我独立会选的设计一致 —— 在 isToolEnabled 这个唯一咽喉点设门、仅 settings 激活并在启动时快照、成员单调、MCP / structured_output / 计划模式生命周期 / 延迟工具族的豁免都服务于缩减 schema 的目标而不是与之相悖。八轮 review 的效果有目共睹：别名表、发现工具的门、调度器的错误信息分支、shim 委托、文档措辞，都是早期版本有错、当前 head 已正确的地方。约 2.7:1 的测试/生产行数比，加上维护者独立的 wire A/B（34/34、且在 base 上有红色对照 —— 测试套件确实钉住了改动），就是一个负载性修复该有的样子。六个月后维护这段代码，我会感谢作者。

没给到 5 的原因：R2-2 交互（"Always allow" 恰好持久化到现在会激活白名单的那个键上，导致下次重启工具集坍缩）真实存在且尚未修复 —— 机制见 Stage 2。它已被披露，维护者是在 review 通道的发现仍在记录的情况下批准了这个 head，我把这视为产品决策；我希望合入前能建一个跟进 issue，让后续修复有据可查。被跳过的集成/矩阵作业是设计使然（仅 merge queue）或 fork 通道的结构性原因，不是不利证据。

本 bot 早先在这个 PR 上的 CHANGES_REQUESTED review 属于 review 通道的各轮；其遗留项已升级给维护者、并由维护者对这个 head 的批准所裁决，因此本轮结论取代它们。批准，钉在所审提交上。✅

— Qwen Code · qwen3.8-max

_{Reviewed at 5e63280089932c6b323d3b83df98884c72b70969 · re-run with @qwen-code /triage}

[qwen-code-ci-bot]

Partially reviewed — gaps disclosed.

Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI (downstream of the pre-existing workflow-size guard failure on main) and its suite did not run locally.

中文说明

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

未审查：build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI (downstream of the pre-existing workflow-size guard failure on main) and its suite did not run locally。

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

[qwen-code-ci-bot]

[Suggestion] R1-8: Any syntactically-valid rule activates the allowlist, even one that cannot cover any built-in. parseRule marks only unbalanced-parenthesis rules invalid and passes unknown names through, so: (a) a typo — permissions.allow: ["ReadFle"] — activates, covers nothing, and deregisters the entire built-in toolset with no diagnostic (pre-PR the same typo was an inert auto-approve rule); (b) an MCP-only rule (["mcp__github__create_issue"]) activates even though MCP tools are exempt from the gate, so the only rule present cannot express membership for any tool it gates; (c) neither activation nor the registration skips log anything (the rule warning sits behind QWEN_DEBUG_LOG_FILE), so "all my tools disappeared" has no signal to grep for. The comment above promises to keep "a malformed entry" from gating the toolset, but typos are the commonest malformed-entry class and slip through; the only malformed-rule test is syntactic ('Bash(git commit').

Witness (probe): parseRule('ReadFle').invalid === undefined; a manager with ['ReadFle'] reports active and disables read_file/send_message/run_shell_command; an MCP-only rule activates the same way.

If this semantic is intended, document it in the settings.md row and emit a startup warning naming allow rules that cover no known tool; otherwise require at least one rule resolving to a known tool for activation. Either way, pin the chosen behavior with a test for the lone unknown-name rule.

中文说明

任何语法有效的规则都会激活白名单，即使它覆盖不了任何内置工具。parseRule 只把括号不闭合的规则标记为无效，未知名称原样透传，于是：(a) 拼写错误——permissions.allow: ["ReadFle"]——会激活、什么都不覆盖、并在无任何诊断的情况下注销整个内置工具集（PR 之前同样的拼写错误只是一条无效的自动批准规则）；(b) 仅 MCP 的规则（["mcp__github__create_issue"]）也会激活，尽管 MCP 工具本身豁免于该门——唯一存在的规则无法为它所门控的任何工具表达成员身份；(c) 激活与注册跳过都不打日志（规则警告藏在 QWEN_DEBUG_LOG_FILE 后面），"我的工具全没了"没有任何可 grep 的信号。上方注释承诺防止"畸形条目"门控整个工具集，但拼写错误是最常见的畸形条目类型却能穿过；唯一的畸形规则测试是语法层的（'Bash(git commit'）。

见证（探针）：parseRule('ReadFle').invalid === undefined；['ReadFle'] 的管理器报告激活且禁用 read_file/send_message/run_shell_command；仅 MCP 规则同样激活。

若这是有意语义，请在 settings.md 对应行写明，并在启动时输出警告点名未覆盖任何已知工具的 allow 规则；否则要求至少一条能解析到已知工具的规则才可激活。无论哪种，用"单独一条未知名称规则"的测试钉住所选行为。

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

[yiliang114]

Behavior re-confirmed at the new head (c9b670e): parseRule marks only unbalanced-parenthesis rules invalid, so 'ReadFle' / MCP-only rules parse as valid rules covering nothing, and initialize activates on any non-invalid rule — one typo activates the allowlist and deregisters every uncovered built-in with no user-visible diagnostic. The harm is real (this PR raised a typo's cost from no-op to "toolset gone"), but both remediations are behavior-policy decisions rather than mechanical fixes: requiring ≥1 rule that resolves to a known gated tool changes activation semantics (a deliberately narrow config referencing extension/MCP tools unknown to core would silently stop activating — a fail-open contract change), and warning-only needs a user-visible channel decision (the PM logger sits behind QWEN_DEBUG_LOG_FILE). This is the same root question as the R2-1 thread — what expresses intent to activate the allowlist — so it is consolidated under that maintainer decision instead of being picked unilaterally here. Leaving this thread open to track it.

[qwen-code-ci-bot]

Partially reviewed — gaps disclosed.

Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI (downstream of the pre-existing workflow-size guard failure on main) and its suite did not run locally.

Not explored to full depth (tool budget reached): "agent 4": none — no check was cut short..

Deferred under the convergence posture (round 2, not a blocker) — recorded, not requested in this round:

• docs/users/configuration/settings.md:420 — [review] permissions.allow schema description left stale while settings.md gains the new allowlist semantics (settingsSchema.ts + published VS Code schema JSON)
• packages/core/src/permissions/rule-parser.ts:172 — [review] of the ~17 newly-aliased tool families only SendMessage/UpdateGoal are exercised by tests
• packages/core/src/permissions/permission-manager.ts:726 — [review] strippedAllowRules.session bucket in getEffectiveAllowRules() untested; mutant survives all 347 tests (runtime behavior verified correct)

中文说明

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

未审查：build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI (downstream of the pre-existing workflow-size guard failure on main) and its suite did not run locally。

未探索到全部深度（达到工具调用预算）："agent 4"：none — no check was cut short.。

收敛姿态下延后（第 2 轮，非阻断）——已记录，本轮不要求修改：共 3 条（原文未翻译，列表见上方英文部分）。

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

[qwen-code-ci-bot]

[Suggestion] R1-8: Still stands — the code is unchanged since round 1. Any syntactically-valid rule activates the allowlist, even one that cannot cover any built-in. parseRule marks only unbalanced-parenthesis rules invalid and passes unknown names through, so a typo — permissions.allow: ["Readfile"] or ["bash"] — parses as a VALID rule covering no tool: permissionsAllowListActive becomes true and every built-in tool silently vanishes from the registry and the model request, with no startup warning. Pre-PR the same typo was harmless (it merely failed to auto-approve); this change raises its cost from no-op to "toolset gone". The added guard ("A typo must not gate the whole toolset") covers only syntactic malformation. Consider emitting a user-visible startup warning for entries that resolve to no known built-in or mcp__* pattern, or requiring at least one rule that resolves to a known tool for activation — and logging when the registry allowlist activates and how many tools were gated.

中文说明

仍然成立——代码自上一轮审查以来未变。任何语法合法的规则都会激活白名单，即使它不可能覆盖任何内置工具。parseRule 只把括号不平衡的规则标为 invalid，未知名称原样放行，所以笔误——permissions.allow: ["Readfile"] 或 ["bash"]——会解析为一条不覆盖任何工具的合法规则：permissionsAllowListActive 变为 true，全部内置工具从注册表和模型请求中静默消失，且无任何启动警告。PR 之前同样的笔误是无害的（只是无法自动批准）；此改动把它的代价从"无效"提升为"工具集消失"。新增的防护（"笔误不能门控整个工具集"）只覆盖了语法畸形。建议对解析不到任何已知内置工具或 mcp__* 模式的条目输出用户可见的启动警告，或要求至少一条规则能解析到已知工具才激活——并在白名单激活时记录日志、说明有多少工具被门控。

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

[yiliang114]

Behavior re-confirmed at the new head (c9b670e): parseRule marks only unbalanced-parenthesis rules invalid, so 'ReadFle' / MCP-only rules parse as valid rules covering nothing, and initialize activates on any non-invalid rule — one typo activates the allowlist and deregisters every uncovered built-in with no user-visible diagnostic. The harm is real (this PR raised a typo's cost from no-op to "toolset gone"), but both remediations are behavior-policy decisions rather than mechanical fixes: requiring ≥1 rule that resolves to a known gated tool changes activation semantics (a deliberately narrow config referencing extension/MCP tools unknown to core would silently stop activating — a fail-open contract change), and warning-only needs a user-visible channel decision (the PM logger sits behind QWEN_DEBUG_LOG_FILE). This is the same root question as the R2-1 thread — what expresses intent to activate the allowlist — so it is consolidated under that maintainer decision instead of being picked unilaterally here. Leaving this thread open to track it.

[qwen-code-ci-bot]

[Critical] R2-1: A single "Always allow" confirmation choice persists into settings.permissions.allow — the exact key this PR makes the registry-allowlist activator — so the next restart collapses the entire built-in toolset to the one always-allowed tool family, violating the invariant the PR itself states for the mid-session path. Chain verified at HEAD: the shell confirmation dialog offers "Always allow in this project/user" → shell.ts builds the Bash(npm test) permission rule → coreToolScheduler._handleConfirmationResponseInner → persistPermissionOutcome (permission-helpers.ts:192) → the CLI's onPersistPermissionRule (config.ts:2227-2239) writes permissions.allow into the workspace or user-scope settings file → the next launch feeds that array into registryAllowList here (no filtering of interactive-granted rules) → permissionsAllowListActive becomes true → every uncovered built-in is dropped from the registry. The permissionsAllowListActive JSDoc names the exact harm ("approving one tool would suddenly permission-error every tool not on the list") and guards the in-session state — but the same click, persisted, delivers the identical outcome one restart later, silently, for every subsequent session; "Always allow for this user" writes the user-scope file, so every project opened afterwards boots with the collapsed toolset. Witness (round-2 probe, scratch tree, unmodified PR code — both arms deterministic): fresh project → persist('project','allow','Bash(npm test)') → WORKSPACE_SETTINGS_FILE: {"permissions":{"allow":["Bash(npm test)"]}}; simulated restart → REGISTRY_ALLOW_LIST_AFTER_RESTART: ["Bash(npm test)"], ACTIVE_WITH_ONE_PERSISTED_RULE: true, ENABLED[read_file/edit/write_file/grep_search/glob/list_directory/agent/todo_write/web_fetch]: false, ENABLED[run_shell_command/monitor]: true; flip arm: no persisted rules → ACTIVE_WITH_NO_RULES: false, ENABLED[read_file]: true. Don't let interactive grants silently activate the allowlist: persist always-allow rules under a key that stays a pure auto-approval grant (mirroring how this PR keeps --allowed-tools / tools.allowed out of registryAllowList), or gate activation behind explicit hand-authored intent. At minimum, emit a visible startup notice naming the tools hidden by an active allowlist and warn in the confirmation dialog that persisting the rule restricts the toolset after restart.

中文说明

一次"始终允许"确认选择会持久化到 settings.permissions.allow——正是本 PR 设为注册表白名单激活键的那个键——于是下次重启时整个内置工具集塌缩为那一个被始终允许的工具家族，违反了 PR 自己为会话中路径声明的不变量。已在 HEAD 验证完整链路：shell 确认弹窗提供"在此项目/此用户始终允许" → shell.ts 构造 Bash(npm test) 权限规则 → coreToolScheduler._handleConfirmationResponseInner → persistPermissionOutcome（permission-helpers.ts:192）→ CLI 的 onPersistPermissionRule（config.ts:2227-2239）把 permissions.allow 写入工作区或用户级设置文件 → 下次启动把该数组原样喂给此处的 registryAllowList（交互式授予的规则没有任何过滤）→ permissionsAllowListActive 变为 true → 每个未被覆盖的内置工具被逐出注册表。permissionsAllowListActive 的 JSDoc 明确写出了这一危害（"批准一个工具就会让列表外的所有工具突然权限报错"）并防护了会话内状态——但同一次点击持久化之后，下一次启动就会静默产生完全相同的结果，且影响之后每个会话；"此用户始终允许"写的是用户级设置文件，此后打开的每个项目都以塌缩的工具集启动。见证（本轮探针，隔离树，未改动的 PR 代码——两臂均确定性）：新项目 → 持久化 'Bash(npm test)' → 设置文件写入该规则；模拟重启 → 白名单激活、read_file/edit/write_file/grep_search/glob/list_directory/agent/todo_write/web_fetch 全部 false，仅 run_shell_command/monitor 为 true；翻转臂：无持久化规则 → 未激活、read_file 为 true。建议不要让交互式授予静默激活白名单：把"始终允许"规则持久化到一个保持纯自动批准语义的键（参照本 PR 刻意把 --allowed-tools / tools.allowed 排除在 registryAllowList 之外的做法），或把激活限定为显式手写意图。至少应在启动时输出可见提示、列出被激活白名单隐藏的工具，并在确认弹窗中警告：持久化该规则会在重启后收缩工具集。

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

[yiliang114]

Confirmed real at HEAD — the full chain reproduces (persist Bash(npm test) via "Always allow" -> next start registryAllowList=["Bash(npm test)"], allowlist active, read_file/edit/grep_search/glob/... all isToolEnabled=false). This directly violates the mid-session invariant the PR documents for itself, one restart later and silently.

I'm escalating rather than fixing because the only correct fixes are a data-semantics decision that shouldn't be made unilaterally inside this PR. Interactively-persisted rules and hand-authored rules are indistinguishable once both live in settings.permissions.allow, so there is no small, correct way to keep an interactive grant from activating the allowlist without choosing one of:

1. Persist "Always allow" grants under a separate key that stays a pure auto-approval grant (mirrors how this PR keeps --allowed-tools / tools.allowed out of registryAllowList). Cost: new settings key + load-path merge into the auto-approval path + migration for rules users already accumulated under permissions.allow.
2. Record provenance metadata on persisted rules and gate activation on hand-authored intent. Cost: settings-schema change.
3. Mitigation only (startup notice naming the hidden tools + confirmation-dialog warning). Cheap, but does not stop the toolset collapse.

My lean is (1) since it matches the grant-vs-allowlist split this PR already establishes, but it changes the on-disk settings format and the public permissions contract, so it needs a maintainer sign-off. Which direction do you want? Leaving this thread open until then.

[yiliang114]

Already escalated in the round-2 thread — see reply 3840449009: the chain is confirmed real at HEAD (persisting an "Always allow" rule writes settings.permissions.allow, which activates the allowlist on next restart and collapses the toolset to that one family). The remediation requires a maintainer settings data-format decision — persist interactive grants under a separate non-activating key, or gate allowlist activation behind explicit hand-authored intent — so it is not being fixed in this review round. Leaving this thread open to track that decision.

[yiliang114]

Still maintainer-gated this round — no code change on this surface (the round-6 R6-1 fix in c9b670e only touches the tool_search exemption). Note: round 6's R1-8 finding reduces to the same root question — what activates the permissions.allow registry allowlist — so both threads are consolidated under the decision laid out in reply 3840449009 (separate non-activating grant key vs. provenance-gated activation vs. mitigation-only). Leaving this thread open for maintainer sign-off.

[yiliang114]

Closeout for 105e5f0af9 (cap-4 round on the round-1/round-2 findings):

Fixed (3 findings, 8 threads):

• R1-1 (477c36f39e, docs-only): corrected the false JSDoc/doc claim that allowedTools activates the registry allowlist — it maps to --allowed-tools and stays a pure auto-approval grant; only settings.permissions.allow (restart-required) activates the allowlist. Reworded both types.ts JSDoc blocks + sdk-typescript.md + SDK README.
• R1-7 (a554edb433): isLsToolEnabled() now also consults getRegistryAllowList() via toolMatchesRuleToolName, so an allowlisted list_directory actually registers — the documented tools.core → permissions.allow migration equivalence now holds. 5 new config tests (alias/canonical/specifier/Read meta-category + negative pin); config.test.ts 557 passed.
• R1-3 (b8ba258c40 + settings note 105e5f0af9): plan-mode lifecycle tools (exit_plan_mode/enter_plan_mode/ask_user_question) exempted from the allowlist gate alongside structured_output (new PLAN_LIFECYCLE_TOOLS set; deny rules still win) — the exact permissions.allow (and legacy tools.core/tools.exclude) do not restrict the tool schemas sent to the model — full tool set still included in API request #9827 config no longer breaks plan mode. 2 new tests; permission-manager.test.ts 349 passed. Both suites 906 passed at pushed head; core typecheck + eslint/prettier clean.

Escalated (1, deliberately unresolved):

• R2-1 is real and reproduced (one persisted "Always allow" choice activates the registry allowlist on next restart, collapsing the built-in toolset). The correct fixes require a settings data-format decision (separate persistence key vs provenance metadata) vs mitigation-only — options and design question posted in-thread; needs a maintainer call.

Untouched this round (cap-4): Criticals R1-2/R1-4/R1-5/R1-6 and Suggestions R1-8..R1-12 (18 threads) — queued for a follow-up round.

One non-force push 6af0884806..105e5f0af9 to the fork branch.

[yiliang114]

Cap-4 round 2 on the four standing Criticals — all verified real at head 105e5f0af9 before fixing, all fixed (no escalations):

• R1-2 (allowlist silently deregisters the whole computer_use__* family) — fixed in 4d30e18: the family is exempted from the gate beside MCP (reviewer's option 1), documented; deny rules still win. 2 new tests, mutation-checked.
• R1-6 (command-discovered tools register bypassing the gate, then get rejected at invocation) — fixed in c68f473: registration-time gate permissionManager.isToolEnabled() in discoverAndRegisterToolsFromCommand (reviewer's option 2) — uncovered tools are hidden, not advertised-then-rejected. 1 new test, mutation-checked.
• R1-4 (membership predicate re-reads mutable rule state live while registration is startup-snapshotted) — fixed in 4f1ee39: monotonic membership — startupAllowRules frozen in initialize() (post AUTO-strip), unioned with live rules; rule removals revoke auto-approval within the session, deregistration happens at restart (the documented contract). 2 new tests, mutation-checked.
• R1-5 (session grants can never restore a tool the startup allowlist skipped) — addressed in a9031c1 via the reviewer's "narrow it" option: making grants re-register tools needs a new PermissionManager→registry mechanism (out of scope here), so the contract is narrowed instead — three docstrings now state restart-scoped registration honestly, addSessionAllowRule emits a once-per-session restart caveat, and the overclaiming test is rewritten (+2 caveat tests). Disclosed residual: an in-session grant of an unregistered tool still can't activate it until restart — now documented, warned, and no longer promised otherwise. 4 files (one over the 3-file guideline: the contract spans three doc surfaces).

Verification at pushed head: permission-manager 355, tool-registry 52, skill-utils 9 (416/416 green); npm run typecheck (core) + eslint clean on all changed files; every fix mutation-checked (red on revert). One non-force push 105e5f0af9..a9031c10cf; all 8 threads (both generations of each finding) replied with SHA evidence and resolved.

Still open: R2-1 (Always-allow persistence activating the allowlist — settings data-format design call, escalated with evidence on record) + 5 Suggestions (R1-8/9/10/11/12, queued for a future cap round).

[yiliang114]

Closeout — cap round on the standing round-1 Suggestions (push a9031c10cf..df5fada45c, one non-force push to the fork):

• R1-12 FIXED (e9afe40d7d + docs 1494aad0e1): permissions.ask rules now count toward registry-allowlist membership — isCoveredByAllowOrAskRule unions live + frozen startup ask rules (startupAskRules, frozen in initialize() for the same monotonic contract as startupAllowRules). 2 new tests, mutation-checked.
• R1-11 FIXED (de9a8e39af): --bare mirror test pins getRegistryAllowList() = []; dropping the bareMode clause fails exactly the new test.
• R1-10 FIXED (c965468075): allowlist-miss rejections now surface a permissions.allow-attributed message instead of the phantom "permission was declined"; 2 new scheduler tests, mutation-checked.
• R1-9 FIXED (df5fada45c): exhaustiveness test walks every ToolNames/ToolDisplayNames pair through resolveToolName + asserts alias registration (47 assertions).
• R1-8 DEFERRED: activation-time remediation for syntactically-valid-but-unmatchable rules is a design call entangled with the escalated R2-1 settings-format decision — left for a future round.
• R2-1: stays escalated (maintainer settings-format decision; reply 3840449009 on record), re-post thread answered and left unresolved.

Verification: permission-manager 404/404, cli wiring 5/5, core+cli typecheck clean, eslint clean on all touched files. Remaining unresolved: R1-8 ×2 + R2-1.

[qwen-code-ci-bot]

Partially reviewed — gaps disclosed.

Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI (downstream of the pre-existing workflow-size guard failure on main) and its suite did not run locally.

Not explored to full depth (tool budget reached): chunk 7: running packages/core unit tests to execute the new test live — the review worktree has no node_modules / dist , and a full npm ci + build exceeds this rev….

Convergence: round 3 posted 16 inline comment(s), 15 of them reported for the first time; the previous round posted 14 (2 new). Findings keep coming back to the same files: packages/sdk-typescript/src/types/types.ts (findings in rounds 1, 2; 2 more now). The rate of new findings is not falling. A cluster that keeps producing siblings usually means the fixes are treating instances of a shared root cause — triaging that cause before the next round, or splitting an independent cluster into its own pull request, tends to end the loop faster than fixing them one at a time. Batching the remaining fixes and verifying them before the next push, or dropping this PR's reviews to --severity-floor critical, keeps the loop from re-deriving the same set. (Observation only — nothing was withheld from this review because of this observation.)

[Critical] R2-2 Still stands — packages/cli/src/config/config.ts:2221: a single "Always allow" confirmation choice persists into settings.permissions.allow — the exact key this PR makes the registry-allowlist activator — so the next restart collapses the entire built-in toolset to the one always-allowed tool family ("Always allow for this user" writes the user-scope file, so every project opened afterwards boots collapsed). No commit since the round-2 anchor touched the file; mechanism re-read at HEAD: onPersistPermissionRule (config.ts:2227-2239) appends the interactive grant to permissions.allow, and the registryAllowList wiring consumes that key unfiltered — no startup notice, no dialog warning. Author confirmed real at HEAD and escalated for a maintainer settings data-format decision; no fix landed this round (original thread: comment 3840142514). Not anchorable in this round's incremental diff because the file is unchanged.

中文说明

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

未审查：build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI (downstream of the pre-existing workflow-size guard failure on main) and its suite did not run locally。

未探索到全部深度（达到工具调用预算）：chunk 7：running packages/core unit tests to execute the new test live — the review worktree has no node_modules / dist , and a full npm ci + build exceeds this rev…。

收敛情况：第 3 轮发布了 16 条行内评论，其中 15 条是首次提出；上一轮发布了 14 条（其中 2 条首次提出）。发现反复回到同一批文件：packages/sdk-typescript/src/types/types.ts（第 1、2 轮已出过发现，本轮又有 2 条）。新发现的产出速度没有下降。一个不断再生兄弟发现的簇，通常意味着逐条修复只在处理同一根因的实例——先定位并处理该根因，或把独立的簇拆成单独的 PR，通常比逐条修复更快结束循环。把剩余修复攒成一批、验证后再推送，或将本 PR 的评审降到 --severity-floor critical，可以避免循环反复推导同一组发现。（仅为观察——本轮评审未因此扣留任何内容。）

[Critical] R2-2 Still stands — packages/cli/src/config/config.ts:2221: a single "Always allow" confirmation choice persists into settings.permissions.allow — the exact key this PR makes the registry-allowlist activator — so the next restart collapses the entire built-in toolset to the one always-allowed tool family ("Always allow for this user" writes the user-scope file, so every project opened afterwards boots collapsed). No commit since the round-2 anchor touched the file; mechanism re-read at HEAD: onPersistPermissionRule (config.ts:2227-2239) appends the interactive grant to permissions.allow, and the registryAllowList wiring consumes that key unfiltered — no startup notice, no dialog warning. Author confirmed real at HEAD and escalated for a maintainer settings data-format decision; no fix landed this round (original thread: comment 3840142514). Not anchorable in this round's incremental diff because the file is unchanged.

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

[qwen-code-ci-bot]

[Suggestion] R1-8: Still stands — any syntactically-valid rule activates the allowlist, even one that cannot cover any built-in. parseRule marks only unbalanced-parenthesis rules invalid and passes unknown tool names through, so a tool-name typo in settings — permissions.allow: ["ReadFiel"], a more common typo class than a missing parenthesis — activates the allowlist while covering nothing: every non-exempt built-in disappears from the registry, the exact "whole toolset gated" outcome the comment above this check ("Requiring at least one VALID rule keeps a malformed entry from gating the entire toolset") claims to prevent. The malformed-rule test added this round pins only the unbalanced-parenthesis class. Either filter rules that resolve to no known tool out of activation (e.g. require a resolveToolName hit in initialize()), or narrow the comment to "a syntactically malformed rule" so the next maintainer doesn't read a broader guarantee than exists.

中文说明

仍然成立——任何语法合法的规则都会激活白名单，即使它不可能覆盖任何内置工具。parseRule 仅把括号不配对的规则标为 invalid，未知工具名原样通过；因此 settings 中的工具名笔误——permissions.allow: ["ReadFiel"]（比缺括号更常见的笔误类型）——会激活白名单却什么都不覆盖：所有未豁免的内置工具从注册表消失，恰是此检查上方注释（"要求至少一条合法规则可防止单个格式错误条目门控整个工具集"）声称要防止的结果。本轮新增的格式错误规则测试只固定了括号不配对这一类。建议在激活条件中过滤无法解析到已知工具的规则（例如在 initialize() 中要求 resolveToolName 命中），或把注释收窄为"语法格式错误的规则"，避免后续维护者读到比实现更宽的保证。

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

[yiliang114]

Behavior re-confirmed at the new head (c9b670e): parseRule marks only unbalanced-parenthesis rules invalid, so 'ReadFle' / MCP-only rules parse as valid rules covering nothing, and initialize activates on any non-invalid rule — one typo activates the allowlist and deregisters every uncovered built-in with no user-visible diagnostic. The harm is real (this PR raised a typo's cost from no-op to "toolset gone"), but both remediations are behavior-policy decisions rather than mechanical fixes: requiring ≥1 rule that resolves to a known gated tool changes activation semantics (a deliberately narrow config referencing extension/MCP tools unknown to core would silently stop activating — a fail-open contract change), and warning-only needs a user-visible channel decision (the PM logger sits behind QWEN_DEBUG_LOG_FILE). This is the same root question as the R2-1 thread — what expresses intent to activate the allowlist — so it is consolidated under that maintainer decision instead of being picked unilaterally here. Leaving this thread open to track it.

[yiliang114]

Closeout pass (worker 2): fixed 4 findings, pushed to the fork branch in one non-force push 5437a89f0..6c2694bf2.

Handled (4):

• [Critical] R5-1 (config.ts) — a4ee647: the list_directory gate's activation check and coverage scan now mirror parseRules' guard (typeof raw === 'string' + non-empty), so non-string entries in permissions.allow/ask/legacy tools.allowed no longer crash createToolRegistry while PermissionManager.initialize tolerates them. Both arms pinned in config.test.ts.
• [Critical] R5-2 (permission-manager.ts) — 640ec59: task_stop added to the exemption set with the documented rationale (it is shouldDefer=true and advertised to the model by run_shell_command's schema copy). Pinned next to the plan-mode exemption tests, including that a whole-tool deny rule still wins.
• [Suggestion] R5-3 (coreToolScheduler.ts) — 63970a3: the isCoveredByAllowOrAskRule fallback flipped : true → : false, so shim rejections of covered tools keep the pre-permissions.allow (and legacy tools.core/tools.exclude) do not restrict the tool schemas sent to the model — full tool set still included in API request #9827 declined message as the comment documents; the shim test now asserts that exact message.
• [Suggestion] R3-8 (permission-manager.test.ts) — 6c2694b: new test pins that ask-only rules (permissionsAsk: ['Shell'], no allow) never activate the allowlist — activation boundary complementary to the existing membership pin.

Verification: full runs of the three directly-hit files — permission-manager.test.ts (408 passed), config.test.ts (573 passed), coreToolScheduler.test.ts (376 passed, 4 failed). The 4 failures ("Plan shell routing") fail identically at the pre-fix base commit 5437a89f0 in an isolated scratch worktree — pre-existing/environmental on this machine, unrelated to this PR's diff (the PR's scheduler hunk is confined to the denial-message branch, and those tests run with no permission manager configured). tsc --noEmit clean on packages/core.

Deferred (8, out of this worker's scope): R1-8 ×3 (PRRT_kwDOPB-92c6bi3hm / ...bjjqm / ...boZJz — escalated product decision), R2-1 (...bjjqw — design decision), R3-7 (...boZJl), R3-9 (...boZJu), R4-5 (...bxjv6), R3-15 (...bxjv9). Left unresolved, untouched.

[qwen-code-ci-bot]

Partially reviewed — gaps disclosed.

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

• untested deny-rule message branch of the scheduler's permission-error path — already reported (comment 3841988333, R3-7; author deferred to a follow-up round)

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

Deferred under the convergence posture (round 6, not a blocker) — recorded, not requested in this round:

• packages/core/src/permissions/permission-manager.test.ts:128 — [review] resolveToolName exhaustiveness guard does not walk the legacy-rename maps (ToolNamesMigration / ToolDisplayNamesMigration)
• packages/core/src/permissions/permission-manager.test.ts:3128 — [review] no test pins that a malformed rule alongside valid rules still activates the allowlist (the .some() complement)

Convergence: round 6 posted 1 inline comment(s), 1 of them reported for the first time; the previous round posted 3 (3 new). Findings keep coming back to the same files: packages/core/src/permissions/permission-manager.ts (findings in round 5; 1 more now). A cluster that keeps producing siblings usually means the fixes are treating instances of a shared root cause — triaging that cause before the next round, or splitting an independent cluster into its own pull request, tends to end the loop faster than fixing them one at a time. (Observation only — nothing was withheld from this review because of this observation.)

[Critical] R2-2 Still stands — packages/cli/src/config/config.ts: a single "Always allow" confirmation choice persists into settings.permissions.allow — the exact key this PR makes the registry-allowlist activator — so the next restart collapses the entire built-in toolset to the one always-allowed tool family ("Always allow for this user" writes the user-scope file, so every project opened afterwards boots collapsed). Mechanism re-read at HEAD 6c2694b: onPersistPermissionRule (packages/cli/src/config/config.ts:2216-2227) still appends the interactive grant to permissions.${ruleType}, and the registryAllowList wiring (2205-2210) consumes settings.permissions.allow unfiltered — no startup notice, no dialog warning; contradicts the PR's own "mid-session grants can never activate it" invariant (the grant is mid-session; the activation arrives one restart later, silently). Author confirmed real at HEAD and escalated for a maintainer settings data-format decision (persist interactive grants under a separate non-activating key, or gate activation behind explicit hand-authored intent); no fix landed (original thread: comment 3840142514). Not anchorable — the mechanism lines sit outside this round's incremental diff hunks.

中文说明

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

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

未审查：build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。

收敛姿态下延后（第 6 轮，非阻断）——已记录，本轮不要求修改：共 2 条（原文未翻译，列表见上方英文部分）。

收敛情况：第 6 轮发布了 1 条行内评论，其中 1 条是首次提出；上一轮发布了 3 条（其中 3 条首次提出）。发现反复回到同一批文件：packages/core/src/permissions/permission-manager.ts（第 5 轮已出过发现，本轮又有 1 条）。一个不断再生兄弟发现的簇，通常意味着逐条修复只在处理同一根因的实例——先定位并处理该根因，或把独立的簇拆成单独的 PR，通常比逐条修复更快结束循环。（仅为观察——本轮评审未因此扣留任何内容。）

[Critical] R2-2 Still stands — packages/cli/src/config/config.ts: a single "Always allow" confirmation choice persists into settings.permissions.allow — the exact key this PR makes the registry-allowlist activator — so the next restart collapses the entire built-in toolset to the one always-allowed tool family ("Always allow for this user" writes the user-scope file, so every project opened afterwards boots collapsed). Mechanism re-read at HEAD 6c2694b: onPersistPermissionRule (packages/cli/src/config/config.ts:2216-2227) still appends the interactive grant to permissions.${ruleType}, and the registryAllowList wiring (2205-2210) consumes settings.permissions.allow unfiltered — no startup notice, no dialog warning; contradicts the PR's own "mid-session grants can never activate it" invariant (the grant is mid-session; the activation arrives one restart later, silently). Author confirmed real at HEAD and escalated for a maintainer settings data-format decision (persist interactive grants under a separate non-activating key, or gate activation behind explicit hand-authored intent); no fix landed (original thread: comment 3840142514). Not anchorable — the mechanism lines sit outside this round's incremental diff hunks.

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

[yiliang114]

Round 6 closeout — new head: c9b670e (fork branch).

• R6-1 (Critical, fixed): tool_search exempted from the allowlist gate — gating it tripped the ToolSearch-absent fallback that eagerly reveals every deferred mcp__*/computer_use__* schema, inverting schema-shrink into maximal bloat. Deny still wins; pinned with 2 tests + mutation check.
• R2-1 (Critical): unchanged — still blocked on the maintainer decision (persisted "Always allow" grants activating the allowlist; options in thread).
• R1-8 (×3 copies): confirmed real at head, but the fix is the same activation-intent decision as R2-1 — left for the maintainer, all 3 threads answered and kept open to track it.
• R3-7 / R3-9 / R3-15 / R4-5: still queued follow-ups from earlier rounds, untouched.
• Verification: permission-manager suite 410/410, packages/core typecheck clean.

[qwen-code-ci-bot]

Partially reviewed — gaps disclosed.

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

Deferred under the convergence posture (round 7, not a blocker) — recorded, not requested in this round:

• docs/users/configuration/settings.md:421 (+5 locations) — [review] exemption enumeration on five doc/JSDoc surfaces omits tool_search (settings.md, sdk-typescript.md, sdk README, types.ts, config.ts registryAllowList JSDoc)
• packages/core/src/permissions/permission-manager.test.ts:3023 — [review] new test comment cites renamed-away symbol isCoveredByAllowRule (actual predicate: isCoveredByAllowOrAskRule)
• packages/core/src/config/config.ts:7188 — [probe] added comment falsely claims PermissionManager.initialize tolerates non-string settings entries (parseRules throws; probe: parseRules([123]) → TypeError)

[Critical] R2-2 Still stands — packages/cli/src/config/config.ts: a single "Always allow" confirmation choice persists into settings.permissions.allow — the exact key this PR makes the registry-allowlist activator — so the next restart collapses the entire built-in toolset to the one always-allowed tool family ("Always allow for this user" writes the user-scope file, so every project opened afterwards boots collapsed). Mechanism re-read at HEAD c9b670e: onPersistPermissionRule (packages/cli/src/config/config.ts:2216-2227) still appends the interactive grant to permissions.${ruleType}, and the registryAllowList wiring (2205-2210) consumes settings.permissions.allow unfiltered — no startup notice, no dialog warning; contradicts the PR's own "mid-session grants can never activate it" invariant (the grant is mid-session; the activation arrives one restart later, silently). Author confirmed real at HEAD and escalated for a maintainer settings data-format decision (persist interactive grants under a separate non-activating key, or gate activation behind explicit hand-authored intent); no fix landed (original thread: comment 3840142514). Not anchorable — the mechanism lines sit outside this round's incremental diff hunks.

中文说明

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

未审查：build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。

收敛姿态下延后（第 7 轮，非阻断）——已记录，本轮不要求修改：共 3 条（原文未翻译，列表见上方英文部分）。

[Critical] R2-2 Still stands — packages/cli/src/config/config.ts: a single "Always allow" confirmation choice persists into settings.permissions.allow — the exact key this PR makes the registry-allowlist activator — so the next restart collapses the entire built-in toolset to the one always-allowed tool family ("Always allow for this user" writes the user-scope file, so every project opened afterwards boots collapsed). Mechanism re-read at HEAD c9b670e: onPersistPermissionRule (packages/cli/src/config/config.ts:2216-2227) still appends the interactive grant to permissions.${ruleType}, and the registryAllowList wiring (2205-2210) consumes settings.permissions.allow unfiltered — no startup notice, no dialog warning; contradicts the PR's own "mid-session grants can never activate it" invariant (the grant is mid-session; the activation arrives one restart later, silently). Author confirmed real at HEAD and escalated for a maintainer settings data-format decision (persist interactive grants under a separate non-activating key, or gate activation behind explicit hand-authored intent); no fix landed (original thread: comment 3840142514). Not anchorable — the mechanism lines sit outside this round's incremental diff hunks.

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

[wenshao]

Local maintainer verification — merge-ready ✅ (34/34 scripted assertions)

Ran an isolated local verification round (containerized node:22-bookworm, same image family as the CI verify lane; no credentials, host metadata only) against GitHub's merge ref 6708ed9 (head c9b670e, effective base 3f1b24e; the PR API's baseRefOid was stale — the effective diff is 21 files, +2026/−75, matching gh pr diff exactly).

Central claim — A/B on the wire

Oracle: the bundled CLI (dist/cli.js) run headless against a recording OpenAI-compatible loopback server; assertion target = the tools array of the first /chat/completions request. 13 cells, scripted set comparisons:

cell	config	tools on wire
base / none	—	27
head / none	—	27 — set identical to base
merged into live main / none	—	28 = + report_findings (#9794, new since merge base)
base + reporter's 8 permissions.allow rules	settings	27 — unchanged: bug reproduced (red control)
head + same rules	settings	13 — the fix
merged + same rules	settings	13 — report_findings correctly stripped
head + --allowed-tools ReadFile	flag	27 — flag never activates the allowlist
head + --exclude-tools send_message,update_goal	flag	25 = − exactly those two
head + ask-only rule	settings	27 — ask rules never activate
head + deny via display names ×3	settings	24 = − exactly those three
base + deny via display names ×3	settings	27 — unchanged: alias-map bug reproduced (red control)
base / head + tools.disabled:["read_file"]	settings	26 on both arms — symmetric positive control proving the settings file is read on both arms

Grammar breakers (the #9827 pain): with the reporter's allow rules, base sends all four of send_message, update_goal, loop_wakeup, read_mcp_resource; head sends none of them.

The 13 head sends = the PR body's promised 10 + task_stop + tool_search (exempted by the PR's own commits) + list_directory (opt-in tool activated by the ListFiles rule). See corrections below.

Corrections to the description (not code defects)

1. The body's "After … sends 10 schemas" under-counts: this environment sends 13, and the extra three are exactly the PR's own documented exemptions/opt-in. Mechanism behaves as specified; only the number in the Reviewer Test Plan is stale.
2. The "63 tool schemas" baseline is environment-dependent (27 built-ins in a minimal headless env — no computer_use__*, no interactive-only tools). The A/B holds on identical environments, so conclusions are unaffected.

Suite is load-bearing (vacuity check)

• Base's own permission-manager.test.ts on base sources: 332/332 green.
• Head's rewritten file (410 tests) on base sources: 52 red / 358 green — 37 behavioral AssertionError failures (e.g. deny via display name removes the tool from the registry → expected true to be false) + 15 missing-API failures (isPermissionsAllowListActive is not a function, expected when reverting wholesale). Behavioral, not import-breakage.

Targeted gates

• Core changed-file suites (6 files incl. permission-manager, config, tool-registry): 1472/1472 green.
• CLI config.test.ts: 355/355 green.
• Typecheck: zero errors under packages/. The 349 root-wide errors are all in integrations/external-context's client, root-caused by TS2307 @qwen-code/webui/daemon-react-sdk missing under this round's --cli-only build scope — a harness artifact, not the PR. Liveness control: a planted type error was reported by the gate and removed.

Interaction with concurrent main

git merge-tree into live main (c3d9279) is conflict-free; main's only file overlap (config.ts, adding report_findings) composes correctly on the measured merged tree — new tool joins the default set and is stripped under the allowlist.

Not covered

Interactive TUI / /tools rendering; a real llama.cpp backend (wire shape proven, not the backend's grammar compilation itself); MCP-server tool registration paths and the computer_use__* family (unit-suite only, absent from this env); per-commit attribution across the 31 commits (aggregate merge-ref, like CI); full-build root-wide typecheck.

Evidence

中文摘要

结论：merge-ready（34/34 断言全过，0 失败）。 在隔离容器（node:22-bookworm，与 CI verify lane 同源）对 base / PR 合并树 / 试合并 live main 三棵树做真实 A/B：录制式 OpenAI 兼容端点抓取 CLI 首个请求的 tools 数组。核心结论：base 上配置报告者的 permissions.allow 八条规则后 wire 仍是全量 27 个工具（bug 复现，红色对照）；PR 树上同样配置只剩 13 个 —— 四个 grammar 破坏者（send_message/update_goal/loop_wakeup/read_mcp_resource）全部消失。13 = PR 描述承诺的 10 + task_stop/tool_search（PR 自己的豁免提交）+ list_directory（ListFiles 规则激活的 opt-in 工具）—— 属于对 PR 描述计数的勘误，不是代码缺陷。回归保护全部成立：无 permissions 时两树集合完全一致；--allowed-tools 不激活白名单；--exclude-tools 精确排除；ask-only 规则不激活；display-name deny 在 PR 树精确移除 3 个工具、在 base 上完全无效（alias 表修复的红色对照）；tools.disabled 对称阳性对照两树都生效（证明 settings 确实被读取）。试合并 live main：新工具 report_findings 正常加入默认集（27→28），白名单下同样被剥离，无合并冲突。负载证明：head 的新 permission-manager 套件跑在 base 源码上 52 红 / 358 绿（37 个行为性断言失败 + 15 个新 API 缺失），base 自身套件 332/332 全绿。门禁：core 改动文件 1472/1472、cli config 355/355 全绿；typecheck 的 349 个错误全部位于 --cli-only 构建跳过的 webui 依赖 client（验证环境构建范围所致），packages/ 下零错误，planted-error 对照证明门是活的。未覆盖：TUI 交互、真实 llama.cpp 后端（wire 形状已证，非 grammar 编译本身）、MCP/computer_use 注册路径（仅单测覆盖）、逐提交归因、完整构建下的全仓 typecheck。

[yiliang114]

Closeout — cap round on the standing Suggestions (single non-force fork push c9b670e17..5e6328008):

• R3-7 3ca966c6f — scheduler deny-arm precedence pinned: two new three-way branch tests with findMatchingDenyRule returning a rule (one arms the allowlist branch to pin the if/else-if ordering, one pins deny over the generic fallback). Mutation-checked (deny-branch false → both fail).
• R3-9 + R3-15 7283ed12d — discovery-gate sibling semantics pinned: deny-covered config (denied tool also allow-covered → only the deny branch can reject it → deny removes despite an active allowlist) + ask-covered config (ask keeps the tool registered; uncovered control proves the gate is active). Both directions mutation-checked.
• R4-5 5e6328008 — activation wording corrected on all 4 surfaces (settings.md, sdk-typescript.md, sdk README, types.ts coreTools JSDoc) to the real predicate: ≥1 valid rule from settings.permissions.allow/getRegistryAllowList at PermissionManager.initialize; malformed entries never activate; --allowed-tools / SDK allowedTools / legacy tools.allowed never activate; task_stop + tool_search added to every exemption list.

Verification: tool-registry 56/56; coreToolScheduler 378 pass (4 pre-existing Plan-shell-routing failures reproduce identically at base — environmental); core typecheck clean. Threads: 4 cap threads replied with fix SHAs + resolved. Remaining 4 unresolved = the R1-8 ×3 product decision + R2-1 design decision ledger — escalated previously, left for maintainer direction. Fork lane: /triage structurally skipped (same-repo guard); push auto-triggered the fork review lane.

[qwen-code-ci-bot]

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: build-and-test — test-efficacy probe inconclusive (probe runner tripped vitest's missing-dist guard; 0 mutants and 0 hunks probed; direct suite runs of both changed test files are green).

Deferred under the convergence posture (round 8, not a blocker) — recorded, not requested in this round:

• packages/core/src/tools/tool-registry.test.ts:1037 — [review] three new discovery-gate tests triplicate the PermissionManager + spawn-mock boilerplate that the makeConfig factory in permission-manager.test.ts already provides
• packages/core/src/core/coreToolScheduler.test.ts:3780 — [probe] the isPermissionsAllowListActive() conjunct in the scheduler's allowlist-miss arm is unpinned — the deletion mutant survives all six new tests (probe flipped both ways)

[Critical] R2-2 Still stands — packages/cli/src/config/config.ts: a single "Always allow" confirmation choice persists into settings.permissions.allow — the exact key this PR makes the registry-allowlist activator — so the next restart collapses the entire built-in toolset to the one always-allowed tool family ("Always allow for this user" writes the user-scope file, so every project opened afterwards boots collapsed). Mechanism re-read at HEAD 5e63280: onPersistPermissionRule (packages/cli/src/config/config.ts:2216-2227) still appends the interactive grant to permissions.${ruleType}, and the registryAllowList wiring (2205-2210) consumes settings.permissions.allow unfiltered — no startup notice, no dialog warning; contradicts the PR's own "mid-session grants can never activate it" invariant (the grant is mid-session; the activation arrives one restart later, silently). Witness: round-2 chain verified at HEAD (persist Bash(npm test) via "Always allow" -> next start registryAllowList=["Bash(npm test)"], allowlist active, read_file/edit/grep_search/glob all isToolEnabled=false); this round's incremental delta does not touch the file. Author confirmed real and escalated for a maintainer settings data-format decision (persist interactive grants under a separate non-activating key, or gate allowlist activation behind explicit hand-authored intent); no fix landed (original thread: comment 3840142514). Not anchorable — the mechanism lines sit outside this round's incremental diff hunks.

中文说明

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

未审查：build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。

未审查：build-and-test — test-efficacy probe inconclusive (probe runner tripped vitest's missing-dist guard; 0 mutants and 0 hunks probed; direct suite runs of both changed test files are green)。

收敛姿态下延后（第 8 轮，非阻断）——已记录，本轮不要求修改：共 2 条（原文未翻译，列表见上方英文部分）。

[Critical] R2-2 Still stands — packages/cli/src/config/config.ts: a single "Always allow" confirmation choice persists into settings.permissions.allow — the exact key this PR makes the registry-allowlist activator — so the next restart collapses the entire built-in toolset to the one always-allowed tool family ("Always allow for this user" writes the user-scope file, so every project opened afterwards boots collapsed). Mechanism re-read at HEAD 5e63280: onPersistPermissionRule (packages/cli/src/config/config.ts:2216-2227) still appends the interactive grant to permissions.${ruleType}, and the registryAllowList wiring (2205-2210) consumes settings.permissions.allow unfiltered — no startup notice, no dialog warning; contradicts the PR's own "mid-session grants can never activate it" invariant (the grant is mid-session; the activation arrives one restart later, silently). Witness: round-2 chain verified at HEAD (persist Bash(npm test) via "Always allow" -> next start registryAllowList=["Bash(npm test)"], allowlist active, read_file/edit/grep_search/glob all isToolEnabled=false); this round's incremental delta does not touch the file. Author confirmed real and escalated for a maintainer settings data-format decision (persist interactive grants under a separate non-activating key, or gate allowlist activation behind explicit hand-authored intent); no fix landed (original thread: comment 3840142514). Not anchorable — the mechanism lines sit outside this round's incremental diff hunks.

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

[wenshao]

@qwen-code /triage

[qwen-code-ci-bot]

Sandboxed verification: ❌ not passed — findings reported (agent verdict) - workflow run

Ran the PR in an isolated, token-free container: A/B against the base build, mock-free harness assertions, targeted gates. Advisory evidence for human reviewers — not a review, an approval, or a CI check.

Scripted assertions: 18 passed · 1 failed · 19 total

Flakiness gate: ⚠️ consistent-fail — 1 of 7 changed test file(s) failed identically in every round — deterministic, so CI owns that signal

中文 — 判定：❌ 不通过 · 报告了发现（agent 判定）

沙箱验证在隔离、无凭证的容器中执行了该 PR 的代码（与 base 构建 A/B 对照、无 mock harness 断言、定向门禁）。仅作为评审证据，不构成评审、批准或 CI 检查。

脚本断言：18 通过 · 1 失败 · 19 总计

抖动门：⚠️ consistent-fail — 1 of 7 changed test file(s) failed identically in every round — deterministic, so CI owns that signal

Verification report

PR #9829 — deep verification

Verdict: findings — 18/19 scripted assertions passed; 1 failed. Verified head: 5e63280089932c6b323d3b83df98884c72b70969 (merge ref e33acbc7b7, base tip b449a9536a). The central claim is proven load-bearing by a wire-level A/B (28 → 13 tools on the reporter's config; base carries all four grammar-breaking schemas, head none). One concrete defect on the merged tree: the completed alias map misses report_findings/ReportFindings, which main added after this PR branched — the PR's own new exhaustiveness test fails 2/411 on the merge ref, and display-name rules for that tool silently never match. Fix is two alias lines, measured green.

中文摘要

• 结论：findings。核心声明经 A/B 线级证明成立：报告者配置下，base 首个模型请求携带全部 28 个工具 schema（含 4 个 grammar 破坏者 send_message/update_goal/loop_wakeup/read_mcp_resource），head 只发 13 个（被覆盖工具 + 有意豁免项），见下表与 01-ab-wire-oracle-base-vs-head.png。
• 发现 1（需处理）：别名表漏了 report_findings/ReportFindings。该工具在本 PR 分支后由 main 合入，因此 PR 自带的穷举测试在 merge ref 上红 2/411；用展示名写的 ReportFindings 规则会静默失配——正是本 PR 要修的那类 bug 的最后一个漏网者。两行别名即可修复，已实测全绿（411/411）。
• 发现 2（文档勘误）：PR 正文"10 个 schema"的证据早于后续提交；最终代码在本容器发 13 个（多出的 list_directory 来自 ListFiles 覆盖激活的 opt-in gate，task_stop/tool_search 为有意豁免）。行为是有意且已记录的，正文证据段需要刷新。
• 未覆盖：逐 commit 归因（shallow checkout 仅 2/33 commits 可达）；MCP 与 structured_output 豁免、plan-mode 生命周期豁免未在线级演练（沙箱无 MCP server / 未触发 plan mode），仅由单测钉住；TUI /tools 未单独验证（与请求读同一注册表）。

Central claim + A/B

Central claim: with settings.permissions.allow set to the reporter's eight display-name rules, built-in tools not covered by any allow/ask rule are no longer registered, so their schemas disappear from the first model request; without that key, nothing changes.

Wire oracle: each cell runs the built bundle headless (-p, --approval-mode yolo, --output-format json) against a recording mock OpenAI server (recording-server.mjs) with an isolated QWEN_HOME; the oracle is the tools[] array of the first captured POST /v1/chat/completions. Base arm = full npm ci (lockfile untouched by the PR, so a clean control) in a scratch worktree at b449a9536a; readlink -f node_modules/@qwen-code/qwen-code-core from inside it resolved to the base tree, quoted in methodology. This container registers 28 built-ins (the reporter's machine had 63 — computer_use__* etc. are absent here); the mechanism is config-driven, so the comparison is valid within one environment.

cell	config	tools	grammar-breakers
base-allow	reporter permissions.allow	28	ALL 4 PRESENT (bug)
head-allow	reporter permissions.allow	13	none
base-noperm / head-noperm	no permissions	28 / 28	identical sets
base / head --allowed-tools ReadFile	flag only	28 / 28	not activated
base / head --exclude-tools send_message,update_goal	flag only	26 / 26	symmetric
base deny: ["SendMessage"]	display-name deny	28	NO EFFECT (reporter's pain)
head deny: ["SendMessage"]	display-name deny	27	send_message removed

Witness: evidence/01-ab-wire-oracle-base-vs-head.png (cells + the 10 scripted assertions printed). Head-allow's exact set = edit, glob, grep_search, list_directory, monitor, notebook_edit, read_file, run_shell_command, task_stop, tool_search, web_fetch, write_file, zoom_image — the 10 the PR names, plus list_directory (the reporter's own ListFiles rule now activates the tool's opt-in gate), plus the two deliberate exemptions task_stop and tool_search. All four grammar-breaking schemas (send_message, update_goal, loop_wakeup, read_mcp_resource) are gone on head, present on base.

Corrections

The body's "10 schemas" evidence is stale, not wrong-in-spirit. The Evidence block predates three later commits in this PR (the list_directory opt-in gate, the task_stop exemption, the tool_search exemption). Final code sends 13 in this container and would send 13-equivalent on the reporter's machine; every extra member is an intentional, commit-documented decision, and the grammar-breaking tools are removed either way. The body's before/after block should be refreshed so a reader checking it against the merged code isn't confused.

Findings

F1 — alias map misses report_findings; the PR's own exhaustiveness test is red on the merge ref (2/411)

TOOL_NAME_ALIASES (rule-parser.ts) gained ~50 entries completing display-name coverage, but report_findings/ReportFindings are absent — report_findings entered ToolNames on main (base tip b449a9536a has it; PR head 5e63280089 does not), i.e. after this PR branched. On the merged tree the PR's new resolveToolName exhaustiveness (#9827) suite fails:

× covers 'REPORT_FINDINGS' ('ReportFindings' -> 'report_findings')
  → expected 'ReportFindings' to be 'report_findings'
× registers every canonical tool name in the alias map
  → expected undefined to be 'report_findings'

Reproduce: cd packages/core && npx vitest run src/permissions/permission-manager.test.ts → 2 failed | 409 passed (411). A census over the compiled dist (alias-census.mjs: every ToolNames entry must map canonical→canonical and display→canonical) shows this is the only gap of 48 entries (witness evidence/03-alias-census-report-findings-gap.png).

Behavioral consequence: a rule written with the /tools display name — permissions.allow: ["ReportFindings"] or deny: ["ReportFindings"] — silently never matches (the wire A/B proved exactly this failure mode for SendMessage on base: 28/28 tools). Under an active allowlist, "allowing" ReportFindings by display name would not keep the tool registered. Canonical-name rules (report_findings) still work, so the blast radius is the display-name form only.

Measured minimal fix (2 lines, applied in scratch copy, then reverted)

Add to TOOL_NAME_ALIASES in packages/core/src/permissions/rule-parser.ts:

  // Report findings tool
  report_findings: 'report_findings',
  ReportFindings: 'report_findings',

Measured with the patch: permission-manager.test.ts 411/411 green; census exits clean (0 gaps); all other suites unchanged (the fix only appends alias entries — zero collateral surface). The author should also rebase onto current main; the merge itself is conflict-free but semantically incomplete.

F2 — (nit) description evidence block understates the final tool count — see Corrections

Mutation matrix (new tests are load-bearing)

Ran by mutation-check.mjs (witness evidence/02-mutation-matrix.png); each mutation applied to the merge tree, measured, reverted; tree verified restored after.

mutation	expectation	result
none (baseline, with F1 scratch fix)	411/411 green	411 passed
none (as-shipped)	alias tests red	409 passed / 2 failed = F1
M1: delete allowlist branch in isToolEnabled	#9827 suite fails	14 failed / 16 passed / 381 skipped — behavioral (expected true to be false on isToolEnabled('send_message'))
M2: delete only tool_search exemption clause	its pinned test fails	exactly 1 failed / 410 skipped
M3: revert scheduler denial-message to pre-#9827	allowlist-miss test fails	1 failed / 5 passed (the other 5 pin behavior the old code also satisfied — deny-wins and legacy fallback)

No vacuous rows: every guard the PR introduces is killed by its own test when removed, and M2/M3 show the kills are fine-grained, not collateral.

Targeted gates

• packages/core (config, tool-registry, coreToolScheduler, memory-scoped-agent-config, skillReviewAgentPlanner): 1067/1067 passed (5 files).
• packages/cli src/config/config.test.ts (registry-allowlist wiring incl. bare-mode strip): 355/355 passed.
• npm run typecheck (all workspaces incl. integration-tests): exit 0.
• permission-manager.test.ts: 411/411 with the F1 scratch fix; 409/411 as-shipped (the 2 fails are F1).

Not covered

• Per-commit attribution: shallow checkout exposes only 2 of the 33 commits; the aggregate HEAD^1..HEAD diff was verified instead.
• MCP / structured_output / plan-lifecycle exemptions at wire level: no MCP server and no plan-mode trigger in this sandbox; these exemptions are pinned only by the unit suite (M1 kills them when the branch is removed).
• The reporter's 63-tool environment: this container registers 28 built-ins (computer_use__*, image_gen etc. absent). The mechanism is config-driven and identical; the absolute counts differ.
• TUI /tools rendering: not screenshotted; it reads the same registry the request layer reads.
• eslint/prettier: not re-run here (PR's CI covers them); typecheck was run.
• The PR's "11 of 15 new tests red on original sources" claim: not re-litigated; M1's revert of the final code kills 14/30 in the final suite, which supersedes it.

Methodology

Environment: CI verify container (node:22-bookworm, 64 cores), merge-ref checkout at depth 2; npm ci + npm run build pre-ran at HEAD. Base arm: scratch worktree at b449a9536a with a full npm ci (PR leaves package.json/lockfile untouched, so the dependency tree is a clean control); internal workspace links asserted via readlink -f node_modules/@qwen-code/qwen-code-core → base tree before trusting any base cell. Head arm: the prebuilt dist/cli.js at the merge ref. Wire oracle: recording mock OpenAI server storing each request body verbatim; assertions compare sorted tool-name sets (ab-assertions.mjs, 10/10 pass). Mutation and gate runs executed with vitest per package; raw logs, harness scripts, and per-cell request captures live in this artifact directory (logs/, *.mjs). All mutation edits were reverted; final tree is pristine (git status empty).

Flakiness gate log

rounds=5 files=7 skipped=0
file packages/cli/src/config/config.test.ts: (cd packages/cli) npx --no-install vitest run ./src/config/config.test.ts
file packages/core/src/config/config.test.ts: (cd packages/core) npx --no-install vitest run ./src/config/config.test.ts
file packages/core/src/core/coreToolScheduler.test.ts: (cd packages/core) npx --no-install vitest run ./src/core/coreToolScheduler.test.ts
file packages/core/src/memory/memory-scoped-agent-config.test.ts: (cd packages/core) npx --no-install vitest run ./src/memory/memory-scoped-agent-config.test.ts
file packages/core/src/memory/skillReviewAgentPlanner.test.ts: (cd packages/core) npx --no-install vitest run ./src/memory/skillReviewAgentPlanner.test.ts
file packages/core/src/permissions/permission-manager.test.ts: (cd packages/core) npx --no-install vitest run ./src/permissions/permission-manager.test.ts
file packages/core/src/tools/tool-registry.test.ts: (cd packages/core) npx --no-install vitest run ./src/tools/tool-registry.test.ts


per-file results (P=pass F=fail I=infra-exit, one letter per run):
  packages/cli/src/config/config.test.ts: PPPPP
  packages/core/src/config/config.test.ts: PPPPP
  packages/core/src/core/coreToolScheduler.test.ts: PPPPP
  packages/core/src/memory/memory-scoped-agent-config.test.ts: PPPPP
  packages/core/src/memory/skillReviewAgentPlanner.test.ts: PPPPP
  packages/core/src/permissions/permission-manager.test.ts: FFFFF
  packages/core/src/tools/tool-registry.test.ts: PPPPP

verdict: consistent-fail
summary: 1 of 7 changed test file(s) failed identically in every round — deterministic, so CI owns that signal

--- per-invocation detail (full copy in the artifact) ---
round 1 · packages/cli/src/config/config.test.ts: P (exit 0)
round 1 · packages/core/src/config/config.test.ts: P (exit 0)
round 1 · packages/core/src/core/coreToolScheduler.test.ts: P (exit 0)
round 1 · packages/core/src/memory/memory-scoped-agent-config.test.ts: P (exit 0)
round 1 · packages/core/src/memory/skillReviewAgentPlanner.test.ts: P (exit 0)
round 1 · packages/core/src/permissions/permission-manager.test.ts: F (exit 1)
--- output tail · round 1 · packages/core/src/permissions/permission-manager.test.ts ---
�[39m
   �[32m✓�[39m PermissionManager.findMatchingDenyRule�[2m > �[22mreturns the raw deny rule string when context matches�[32m 0�[2mms�[22m�[39m
   �[32m✓�[39m PermissionManager.findMatchingDenyRule�[2m > �[22mreturns undefined when no deny rule matches�[32m 0�[2mms�[22m�[39m
   �[32m✓�[39m PermissionManager.findMatchingDenyRule�[2m > �[22mmatches session deny rules�[32m 1�[2mms�[22m�[39m
   �[32m✓�[39m PermissionManager.findMatchingDenyRule�[2m > �[22mreturns undefined for non-denied tool�[32m 0�[2mms�[22m�[39m
   �[32m✓�[39m PermissionManager.findMatchingDenyRule�[2m > �[22mmatches bare tool deny rule�[32m 0�[2mms�[22m�[39m
   �[32m✓�[39m PermissionManager.findMatchingDenyRule�[2m > �[22mmatches a deny rule through a symlinked path�[32m 1�[2mms�[22m�[39m
   �[32m✓�[39m PermissionManager — strip/restore for AUTO mode�[2m > �[22mstrips Bash interpreter wildcards and stashes them�[32m 1�[2mms�[22m�[39m
   �[32m✓�[39m PermissionManager — strip/restore for AUTO mode�[2m > �[22mstrips bare tool-level Bash allow�[32m 1�[2mms�[22m�[39m
   �[32m✓�[39m PermissionManager — strip/restore for AUTO mode�[2m > �[22mstrips Agent / Skill any-allow rules�[32m 0�[2mms�[22m�[39m
   �[32m✓�[39m PermissionManager — strip/restore for AUTO mode�[2m > �[22mis idempotent — second strip returns the same stash without re-removal�[32m 0�[2mms�[22m�[39m
   �[32m✓�[39m PermissionManager — strip/restore for AUTO mode�[2m > �[22mrestoreDangerousRules reattaches stripped rules to their original scope�[32m 1�[2mms�[22m�[39m
   �[32m✓�[39m PermissionManager — strip/restore for AUTO mode�[2m > �[22mnever strips deny rules — user intent for deny is honored�[32m 0�[2mms�[22m�[39m
   �[32m✓�[39m PermissionManager — strip/restore for AUTO mode�[2m > �[22mauto-strips on initialize when approvalMode is "auto"�[32m 0�[2mms�[22m�[39m
   �[32m✓�[39m PermissionManager — strip/restore for AUTO mode�[2m > �[22mdoes NOT auto-strip when approvalMode is the default�[32m 0�[2mms�[22m�[39m
   �[32m✓�[39m PermissionManager — compound shell write attribution�[2m > �[22mdeny rule matches a write after `cd` into a subdir�[32m 1�[2mms�[22m�[39m
   �[32m✓�[39m PermissionManager — compound shell write attribution�[2m > �[22mdeny rule matches a write through a `bash -lc` wrapper after `cd`�[32m 1�[2mms�[22m�[39m
   �[32m✓�[39m PermissionManager — compound shell write attribution�[2m > �[22mask rule matches a write through nested shell wrappers�[32m 1�[2mms�[22m�[39m
   �[32m✓�[39m PermissionManager — compound shell write attribution�[2m > �[22mallow rule on the same shell command does NOT downgrade a virtual-op deny�[32m 1�[2mms�[22m�[39m
   �[32m✓�[39m PermissionManager — compound shell write attribution�[2m > �[22mordinary writes after `cd` into project subdirs stay unmatched by self-mod rules�[32m 1�[2mms�[22m�[39m
   �[32m✓�[39m PermissionManager — compound shell write attribution�[2m > �[22mdoes not treat canonical-only allow matches as relevant�[32m 1�[2mms�[22m�[39m
   �[32m✓�[39m PermissionManager — compound shell write attribution�[2m > �[22mhasRelevantRules sees protected writes after sibling shell-wrapper segments�[32m 1�[2mms�[22m�[39m
   �[32m✓�[39m PermissionManager — compound shell write attribution�[2m > �[22mhasRelevantRules sees protected writes after `cd` before compound recursion�[32m 1�[2mms�[22m�[39m
   �[32m✓�[39m PermissionManager — compound shell write attribution�[2m > �[22mhasMatchingAskRule sees writes after `cd` into a subdir�[32m 1�[2mms�[22m�[39m
   �[32m✓�[39m PermissionManager — compound shell write attribution�[2m > �[22mescalates dynamic-cd writes when path-specific deny rules may apply�[32m 2�[2mms�[22m�[39m
   �[32m✓�[39m PermissionManager — compound shell write attribution�[2m > �[22mpreserves wildcard deny rules for dynamic-cd writes�[32m 1�[2mms�[22m�[39m
   �[32m✓�[39m PermissionManager — toolParams end-to-end�[2m > �[22mevaluate respects allow rule with param matcher�[32m 0�[2mms�[22m�[39m
   �[32m✓�[39m PermissionManager — toolParams end-to-end�[2m > �[22mevaluate denies when param matcher does not match�[32m 0�[2mms�[22m�[39m
   �[32m✓�[39m PermissionManager — toolParams end-to-end�[2m > �[22mfindMatchingDenyRule matches deny rule with param matcher�[32m 0�[2mms�[22m�[39m
   �[32m✓�[39m PermissionManager — toolParams end-to-end�[2m > �[22mfindMatchingDenyRule returns undefined when param does not match�[32m 0�[2mms�[22m�[39m
   �[32m✓�[39m PermissionManager — toolParams end-to-end�[2m > �[22mhasRelevantRules returns true when param matcher rule exists�[32m 0�[2mms�[22m�[39m
   �[32m✓�[39m PermissionManager — toolParams end-to-end�[2m > �[22mhasMatchingAskRule returns true when param matcher ask rule matches�[32m 0�[2mms�[22m�[39m
   �[32m✓�[39m PermissionManager — toolParams end-to-end�[2m > �[22mcase-insensitive param matching: deny rule blocks different casing�[32m 0�[2mms�[22m�[39m
   �[32m✓�[39m matchesRule — param matcher type guards�[2m > �[22mrejects boolean param values�[32m 0�[2mms�[22m�[39m
   �[32m✓�[39m matchesRule — param matcher type guards�[2m > �[22mrejects null param values�[32m 0�[2mms�[22m�[39m
   �[32m✓�[39m matchesRule — param matcher type guards�[2m > �[22mrejects undefined param values�[32m 0�[2mms�[22m�[39m
   �[32m✓�[39m matchesRule — param matcher type guards�[2m > �[22mrejects object param values�[32m 0�[2mms�[22m�[39m
   �[32m✓�[39m matchesRule — param matcher type guards�[2m > �[22maccepts number param values via coercion�[32m 0�[2mms�[22m�[39m

�[31m⎯⎯⎯⎯⎯⎯⎯�[39m�[1m�[41m Failed Tests 2 �[49m�[22m�[31m⎯⎯⎯⎯⎯⎯⎯�[39m

�[41m�[1m FAIL �[22m�[49m src/permissions/permission-manager.test.ts�[2m > �[22mresolveToolName exhaustiveness (#9827)�[2m > �[22mcovers 'REPORT_FINDINGS' ('ReportFindings' -> 'report_findings')
�[31m�[1mAssertionError�[22m: expected 'ReportFindings' to be 'report_findings' // Object.is equality�[39m

Expected: �[32m"�[7mr�[27meport�[7m_f�[27mindings"�[39m
Received: �[31m"�[7mR�[27meport�[7mF�[27mindings"�[39m

�[36m �[2m❯�[22m src/permissions/permission-manager.test.ts:�[2m124:44�[22m�[39m
    �[90m122| �[39m      // The /tools display name — the spelling users copy into rules —
    �[90m123| �[39m      // must resolve to the canonical tool.
    �[90m124| �[39m      expect(resolveToolName(displayName)).toBe(canonicalName);
    �[90m   | �[39m                                           �[31m^�[39m
    �[90m125| �[39m    },
    �[90m126| �[39m  );

�[31m�[2m⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯[1/2]⎯�[22m�[39m

�[41m�[1m FAIL �[22m�[49m src/permissions/permission-manager.test.ts�[2m > �[22mresolveToolName exhaustiveness (#9827)�[2m > �[22mregisters every canonical tool name in the alias map
�[31m�[1mAssertionError�[22m: expected undefined to be 'report_findings' // Object.is equality�[39m

�[32m- Expected:�[39m 
"report_findings"

�[31m+ Received:�[39m 
undefined

�[36m �[2m❯�[22m src/permissions/permission-manager.test.ts:�[2m130:48�[22m�[39m
    �[90m128| �[39m  it('registers every canonical tool name in the alias map', () => {
    �[90m129| �[39m    for (const canonicalName of Object.values(ToolNames)) {
    �[90m130| �[39m      expect(TOOL_NAME_ALIASES[canonicalName]).toBe(canonicalName);
    �[90m   | �[39m                                               �[31m^�[39m
    �[90m131| �[39m    }
    �[90m132| �[39m  });

�[31m�[2m⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯[2/2]⎯�[22m�[39m


�[2m Test Files �[22m �[1m�[31m1 failed�[39m�[22m�[90m 

...truncated -- full content in the run artifacts.

Evidence images

Harness scripts and raw logs are in the workflow run artifacts (7-day retention).

— Qwen Code · sandboxed verification

[qwen-code-ci-bot]

LGTM, looks ready to ship. ✅

[qwen-code-ci-bot]

Released in v0.22.2.