feat(core): let plan mode vouch for extra read-only shell roots

[TianYuan1024]

What this PR does

Adds a setting that lets you tell Plan Mode which extra root commands are read-only, so a project-specific CLI stops triggering an approval prompt on every single read.

{
  "permissions": {
    "planMode": {
      "extraReadOnlyCommands": ["ib"],
    },
  },
}

A listed root joins the classifier's built-in read-only set — nothing else about the analysis changes. The entry is consulted only at the very end of the classification chain, after every root the classifier already understands has been matched, so a listed entry can only ever add to the read-only set and can never turn a known write into a read. Listing rm, git, or tee leaves rm -rf build, git push, and tee out.txt classified exactly as before. Redirections, command substitution, environment-assignment prefixes, and pipes into unknown commands are likewise untouched: with ib listed, ib list runs silently while ib list > out.txt is still blocked as state-modifying and ib list $(whoami) still prompts.

Two categories of entry are rejected during normalisation. Anything that is not a bare command name — a path, a command with arguments, a string with shell metacharacters — is dropped, because the classifier matches on the lowercased root token and such an entry could never match anyway. Shell interpreters and generic command wrappers (bash, sh, env, sudo, xargs, nohup, timeout, exec, eval, and similar) are also dropped: each exists to run some other command, so accepting one would let a single settings line bypass the analysis entirely.

The setting is scoped to Plan Mode. It is read through one accessor that returns an empty set in every other approval mode, so vouching for a CLI while planning never widens auto-approval in default, auto-edit, auto, or yolo mode — permissions.allow remains the mechanism there. Entries merge as a union across the user, project, and system scopes, and are dropped in --bare and safe mode, matching how permissions.autoMode behaves.

An entry vouches for the entire binary. Qwen Code cannot see inside a custom CLI, so if it has mutating sub-commands, listing it silences the prompt for those too. That tradeoff is called out in the docs.

Why it's needed

Plan Mode decides whether a shell command is read-only by matching its root against a hardcoded set. A binary outside that set cannot be judged, so it classifies as unknown and triggers the "could not determine whether this shell command is read-only" prompt. Plan-mode shell confirmations deliberately hide "Always allow" and accept a one-time approval only, so that prompt reappears for every invocation, forever.

For a team whose Plan Mode sessions run through a project-specific read-only CLI, every single read needs a manual click, while the built-in equivalents (cat, grep, git status) pass silently. There is no way out today: Plan Mode intentionally overrides permissions.allow for shell, so an allow rule does not help, and PreToolUse hooks run after the permission decision and can only deny or ask, so they cannot help either.

This gives users the same kind of explicit, narrow vouch they already make with permissions.allow in every other mode, without relaxing any of Plan Mode's syntactic guarantees.

Reviewer Test Plan

How to verify

Create a scratch workspace with a fake read-only CLI on PATH (printf '#!/bin/sh\necho ok\n' > ib && chmod +x ib), add permissions.planMode.extraReadOnlyCommands: ["ib"] to .qwen/settings.json, and enter Plan Mode with /plan. The full scripted plan is committed at .qwen/e2e-tests/2026-08-22-plan-mode-extra-read-only-commands.md.

Ask the model to run ib domain list: it should run with no confirmation prompt. Remove the settings key and repeat: the "could not determine whether this shell command is read-only" prompt appears, and appears again on every identical invocation.

Confirm the guardrails still hold while the key is in place. ib domain list > out.txt must be rejected as state-modifying, not prompted. ib domain list $(whoami) and IB_TOKEN=x ib domain list must still show the one-time unknown prompt. ib domain list | badcmd must still prompt, while ib domain list | wc -l runs silently.

Confirm the safety net cannot be switched off from settings. Add "bash", "rm", and "git" to the list and restart: bash -c 'echo hi' must still prompt, and rm -rf tmp and git push origin main must still be blocked as state-modifying.

Confirm the scope. Leave Plan Mode with /approval-mode default and run ib domain list — the normal shell confirmation must appear. Switch back with /plan and it stops prompting again, with no restart.

Finally, confirm invalid entries are ignored rather than fatal: set the list to ["", " ", "ib list", "/usr/local/bin/ib", "ib;rm", "IB"] and restart. The CLI should start normally, and ib domain list should run without a prompt from the "IB" entry alone.

Evidence (Before & After)

N/A — no TUI change. The user-visible difference is the absence of a confirmation prompt, covered by the steps above and by unit tests.

Unit tests, run from packages/core:

npx vitest run src/utils/shellAstParser.test.ts src/config/config.test.ts src/core/plan-mode-shell-policy.test.ts src/tools/shell.test.ts src/tools/monitor.test.ts src/permissions/permission-manager.test.ts

 Test Files  6 passed (6)
      Tests  1843 passed (1843)

New coverage asserts the vouched root classifies read-only, that redirects, substitutions, env prefixes and unknown pipe targets are unaffected, that listing rm / git / tee / mv / dd cannot override their built-in write classification, that normalisation drops malformed and wrapper entries, and that the accessor returns an empty set outside Plan Mode including across a runtime mode switch.

Tested on

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

Environment (optional)

Unit tests via vitest, plus the settings JSON Schema regenerated with npm run generate:settings-schema.

Risk & Scope

• Main risk or tradeoff: a listed root vouches for the whole binary, including any mutating sub-commands it may have — the classifier cannot see inside a custom CLI. This is documented explicitly. The change is inert unless a user opts in; with no setting configured, every classification is byte-for-byte identical to today.
• Not validated / out of scope: honouring permissions.allow for unknown-classified shell commands in Plan Mode — that changes Plan Mode's trust model and needs its own design discussion. Sub-command scoping (a per-CLI analogue of the git sub-command table) is also out of scope, since custom CLIs place their verb at varying argument positions. The deprecated regex checker used as a fallback when the tree-sitter parser is unavailable is deliberately left alone: it ignores the setting and keeps prompting, which fails closed. The synchronous concurrency-batching check, the speculation gate, and memory-scoped agent config are likewise untouched.
• Breaking changes / migration notes: none. The setting is new and optional.

Linked Issues

Closes #9694

中文说明

这个 PR 做了什么

新增一个配置项，让你可以告诉 Plan 模式哪些额外的根命令是只读的，这样项目自定义 CLI 就不会在每一次读操作时都弹出确认框。

{
  "permissions": {
    "planMode": {
      "extraReadOnlyCommands": ["ib"],
    },
  },
}

列出的根命令会加入分类器内建的只读集合——分析逻辑的其它部分完全不变。该条目只在分类链条的最末尾被查询，此时分类器已经理解的所有根命令都已匹配完毕，因此列出的条目只可能扩大只读集合，绝不可能把已知的写操作变成读操作。把 rm、git、tee 写进配置，rm -rf build、git push、tee out.txt 的判定与之前完全一致。重定向、命令替换、环境变量前缀、管道到未知命令同样不受影响：列出 ib 后，ib list 静默执行，而 ib list > out.txt 仍会被判为写操作并拦截，ib list $(whoami) 仍会弹窗。

归一化阶段会拒绝两类条目。任何不是纯命令名的内容——路径、带参数的命令、含 shell 元字符的字符串——都会被丢弃，因为分类器匹配的是小写后的根命令 token，这类条目本来也匹配不上。shell 解释器和通用命令包装器（bash、sh、env、sudo、xargs、nohup、timeout、exec、eval 等）同样会被丢弃：它们存在的意义就是执行其它命令，接受其中任何一个都会让一行配置彻底绕过整个分析。

该配置的作用域限定在 Plan 模式。它通过单一访问器读取，在其它所有审批模式下返回空集，因此为规划而给某个 CLI 的担保绝不会放宽 default、auto-edit、auto、yolo 模式下的自动批准——那些模式仍然使用 permissions.allow。条目在用户、项目、系统三个作用域之间以并集合并，并在 --bare 与 safe 模式下被丢弃，与 permissions.autoMode 的行为一致。

一个条目是对整个二进制的担保。Qwen Code 无法看透自定义 CLI 的内部，因此如果它有写操作子命令，列出该命令也会一并免除那些子命令的确认。这个取舍在文档中已明确说明。

为什么需要

Plan 模式通过把根命令与一个硬编码集合做匹配来判断 shell 命令是否只读。集合之外的二进制无法判定，于是被归类为 unknown 并触发「无法确定该 shell 命令是否只读」的确认框。Plan 模式的 shell 确认有意隐藏「始终允许」且只接受一次性批准，因此该弹窗会在每一次调用时重新出现，永远如此。

对于 Plan 模式会话依赖项目自定义只读 CLI 的团队来说，每一次读都需要手动点击，而内建的等价命令（cat、grep、git status）却静默通过。目前没有任何出路：Plan 模式有意覆盖 shell 的 permissions.allow，所以 allow 规则没用；PreToolUse 钩子在权限决策之后才运行且只能拒绝或询问，所以钩子也没用。

本改动让用户能够做出与其它模式中 permissions.allow 同样明确、同样窄的担保，同时不放松 Plan 模式的任何语法级保证。

审阅者测试计划

如何验证

创建一个临时工作区，把一个假的只读 CLI 放进 PATH（printf '#!/bin/sh\necho ok\n' > ib && chmod +x ib），在 .qwen/settings.json 中加入 permissions.planMode.extraReadOnlyCommands: ["ib"]，然后用 /plan 进入 Plan 模式。完整的脚本化计划已提交在 .qwen/e2e-tests/2026-08-22-plan-mode-extra-read-only-commands.md。

让模型执行 ib domain list：应当无确认弹窗直接运行。移除该配置项后重复：会出现「无法确定该 shell 命令是否只读」的弹窗，且每一次相同调用都会再次出现。

在配置项存在的情况下确认护栏依然有效。ib domain list > out.txt 必须被判为写操作直接拒绝，而不是弹窗。ib domain list $(whoami) 和 IB_TOKEN=x ib domain list 必须仍然显示一次性 unknown 弹窗。ib domain list | badcmd 必须仍然弹窗，而 ib domain list | wc -l 静默执行。

确认配置无法关掉安全网。把 "bash"、"rm"、"git" 加进列表并重启：bash -c 'echo hi' 必须仍然弹窗，rm -rf tmp 与 git push origin main 必须仍被判为写操作并拦截。

确认作用域。用 /approval-mode default 退出 Plan 模式后执行 ib domain list——必须出现正常的 shell 确认框。再用 /plan 切回，无需重启即可恢复静默执行。

最后确认非法条目会被忽略而非导致崩溃：把列表设为 ["", " ", "ib list", "/usr/local/bin/ib", "ib;rm", "IB"] 并重启。CLI 应当正常启动，且仅凭 "IB" 这一条，ib domain list 就应无弹窗运行。

证据（改动前后）

N/A —— 无 TUI 变更。用户可见的差异是确认弹窗的消失，已由上述步骤和单元测试覆盖。

单元测试，在 packages/core 目录下运行：

npx vitest run src/utils/shellAstParser.test.ts src/config/config.test.ts src/core/plan-mode-shell-policy.test.ts src/tools/shell.test.ts src/tools/monitor.test.ts src/permissions/permission-manager.test.ts

 Test Files  6 passed (6)
      Tests  1843 passed (1843)

新增覆盖断言了：被担保的根命令判定为只读；重定向、替换、环境变量前缀、未知管道目标不受影响；列出 rm / git / tee / mv / dd 无法覆盖它们内建的写判定；归一化会丢弃格式错误的条目与包装器条目；访问器在 Plan 模式之外返回空集，包括运行时模式切换的情形。

测试平台

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

环境（可选）

通过 vitest 运行单元测试，并用 npm run generate:settings-schema 重新生成了 settings JSON Schema。

风险与范围

• 主要风险或取舍：列出的根命令是对整个二进制的担保，包括它可能拥有的写操作子命令——分类器无法看透自定义 CLI 的内部。这一点已在文档中明确说明。该改动在用户主动开启前是完全惰性的；未配置时，所有分类结果与今天逐字节一致。
• 未验证 / 超出范围：在 Plan 模式下对 unknown 分类的 shell 命令尊重 permissions.allow——那会改变 Plan 模式的信任模型，需要单独的设计讨论。子命令级限定（针对每个 CLI 的、类似 git 子命令表的机制）同样超出范围，因为自定义 CLI 的动词位置各不相同。在 tree-sitter 解析器不可用时作为回退的已废弃正则检查器被有意保留原样：它会忽略该配置并继续弹窗，方向是 fail-closed。同步的并发批处理检查、推测执行闸门、memory-scoped agent config 同样未改动。
• 破坏性变更 / 迁移说明：无。该配置项是全新且可选的。

关联 Issue

Closes #9694

[qwen-code-ci-bot]

⚠️ Deferred approval withheld — 1 PR CI workflow run(s) on 229aff5 did not finish green; see the updated table in the Stage 2 comment. Re-run @qwen-code /triage after fixes. finalize run

⚠️ 延迟审批已搁置 —— 229aff5 有 1 个 PR CI workflow 未以绿色完成，详见 Stage 2 评论中已更新的表格。修复后可重新运行 @qwen-code /triage。查看 finalize 运行

[qwen-code-ci-bot]

Thanks for the PR!

Template looks good ✓

Problem: real and observed — this implements #9694, where a user's project-specific CLI (ib) triggers the one-time plan-mode approval prompt on every read invocation. The issue triage already verified all three underlying claims against main (hardcoded read-only root set, plan mode overriding permissions.allow for shell, hooks unable to grant): the gap exists and there is no existing escape hatch.

Direction: aligned. The linked issue was accepted for exploration with exactly this design direction — extend the known-safe root set while keeping every syntactic gate on top — and it sits on the roadmap/configuration track with scope/shell / scope/settings labels.

Size: core paths are touched (packages/core/src/utils/shellAstParser.ts, core/plan-mode-shell-policy.ts, permissions/permission-manager.ts, tools/shell.ts, tools/monitor.ts, config/config.ts, plus packages/cli/src/config/*). Production logic is ~254 lines (additions + deletions), ~312 lines of tests, ~13 lines of JSON schema, ~319 lines of docs and the committed e2e test plan. Below the 500-line awareness threshold; no size escalation needed.

Approach: the scope feels right. It implements exactly the setting shape proposed in the issue, and the PR description makes the honest scope call: the issue's alternative form (honoring permissions.allow for unknown-classified commands in plan mode) is deliberately left out because it would change plan mode's trust model — that belongs in its own discussion. One thing the code review will look at closely: entries merge as a union across user/project/system scopes, which means a project's .qwen/settings.json can vouch for a root too — worth confirming the trust assumptions there match how permissions.autoMode is treated.

Risk: elevated — packages/core/src/tools/shell.ts is one of the paths correlated with post-merge reverts in this repo's history. That doesn't block anything, but it means no enrichment gets skipped: full code review plus the PR's own CI evidence before any approval.

Moving on to code review. 🔍

中文说明

感谢贡献！

模板完整 ✓

问题：真实且已观测——本 PR 实现 #9694，用户的项目自定义 CLI（ib）在 Plan 模式下每次读操作都触发一次性确认弹窗。issue 分流已对照 main 验证了全部三个底层事实（只读根命令集合是硬编码的、Plan 模式有意覆盖 shell 的 permissions.allow、钩子无法授权）：缺口确实存在，且目前没有任何逃生通道。

方向：对齐。关联 issue 已按此设计方向接受探索——在保留全部语法级门禁的前提下扩展已知安全根命令集合——且该需求在 roadmap/configuration 路线上，带 scope/shell / scope/settings 标签。

规模：触及核心路径（packages/core/src/utils/shellAstParser.ts、core/plan-mode-shell-policy.ts、permissions/permission-manager.ts、tools/shell.ts、tools/monitor.ts、config/config.ts，以及 packages/cli/src/config/*）。生产逻辑约 254 行（增+删），测试约 312 行，JSON schema 约 13 行，文档与提交的 e2e 测试计划约 319 行。低于 500 行关注阈值，无需因规模升级。

方案：范围合理。实现了 issue 中提议的配置形态，并做了诚实的范围取舍：issue 中的另一种形式（在 Plan 模式下对 unknown 分类的命令尊重 permissions.allow）被有意排除，因为它会改变 Plan 模式的信任模型，应另行讨论。代码审查会重点关注一点：条目在用户/项目/系统三个作用域之间取并集，意味着项目的 .qwen/settings.json 也能为根命令担保——需要确认这里的信任假设与 permissions.autoMode 的处理方式一致。

风险：升级——packages/core/src/tools/shell.ts 是本仓库历史上与合并后回滚相关的文件之一。这不阻断流程，但意味着不跳过任何审查增强项：完整代码审查加上 PR 自身 CI 证据，之后才谈批准。

进入代码审查 🔍

— Qwen Code · qwen3.8-max

_{Reviewed at 229aff5ae298b341b5951ea4a79ea17a1a42b672 · re-run with @qwen-code /triage}

[qwen-code-ci-bot]

Code review

I formed an independent proposal before reading the diff (new permissions.planMode.* setting, normalized entries, consulted only at the end of the classifier chain, mode-scoped through one accessor), and this PR lands essentially on it, executed carefully. What I verified against the code, not just the description:

• The vouch can only add, never override. The extra-root lookup sits in the terminal else branch of evaluateCommandSafety, after every root the classifier treats specially (git, find, sed, awk, sort, tree, uniq, tee, dd, kill family, the WRITE_ROOT_COMMANDS regex, and the printf -v / less / more / rg unknown cases). So a listed root can only turn an unknown into read-only; rm -rf build, git push, tee out.txt keep their built-in classification even if listed. Confirmed by reading the current classifier source and by the new tests.
• Syntactic gates still dominate. The extra set is threaded through statement, substitution, and redirection evaluation, but those results merge after the fallback and win: ib list > out.txt → write, ib list $(whoami) → unknown, IB_TOKEN=x ib list → unknown, ib list | badcmd → unknown. The pre-existing rawRoot !== root guard also survives, so an uppercase invocation (IB list) still prompts even with ib vouched — conservative, correct.
• Mode scoping lives in one place. Config.getPlanModeReadOnlyRoots() returns the empty set outside PLAN, so the shell tool, monitor tool, permission manager, and plan-mode policy all consume it unconditionally without widening default/auto-edit/auto/yolo. Tests cover all four other modes plus a runtime /approval-mode switch. --bare and safe mode drop the setting exactly like permissions.autoMode.
• Untouched consumers fail closed. The deprecated regex checker (fallback when tree-sitter is unavailable), the speculation gate, and memory-scoped agent config all keep calling the classifier without extra roots — they keep prompting on vouched commands, which is the right direction.
• Conventions check: cross-scope merge uses the existing MergeStrategy.UNION (same as the other permission arrays), the schema entry and regenerated VS Code schema follow the autoMode shape, kebab/lowercase naming and test colocation match house style. No duplication of existing utilities that I could find — the normalization is the one genuinely new piece, and it belongs where it lives.

One non-blocking observation for the maintainer: NON_VOUCHABLE_READ_ONLY_ROOTS is inherently a denylist and cannot be exhaustive — e.g. busybox, watch, su, runuser, setsid, pkexec also exist to run some other command but are not listed. The documented "an entry vouches for the entire binary" tradeoff covers the residual risk (any vouched CLI could have mutating sub-commands), so I'm treating this as a suggestion, not a blocker.

No critical issues found.

Files changed (19 of 19 shown)

File	What changed
packages/core/src/utils/shellAstParser.ts	The core change: an options bag carrying the vouched roots is threaded through statement, substitution and redirection evaluation, consulted only in the terminal fallback branch
packages/core/src/utils/shellAstParser.test.ts	Classifier coverage: vouched root reads, redirects/substitutions/env-prefixes/pipes unaffected, built-in writes cannot be overridden, compound statements and subshells
packages/core/src/config/config.ts	PlanModeSettings type, the wrapper denylist, the normalizer that drops malformed and non-vouchable entries, and the PLAN-gated accessor
packages/core/src/config/config.test.ts	Normalizer cases (trim, lowercase, metacharacters, paths, wrappers) and accessor behavior across every mode plus a runtime mode switch
packages/core/src/core/plan-mode-shell-policy.ts	Passes the roots into both classifier entry points used by the plan-mode policy
packages/core/src/core/plan-mode-shell-policy.test.ts	Policy-level coverage including the monitor tool name
packages/core/src/tools/shell.ts	Shell tool default-permission path consumes the roots for whole commands and sub-commands
packages/core/src/tools/shell.test.ts	Shell tool coverage: vouch flips ask to allow, redirects and substitutions still ask
packages/core/src/tools/monitor.ts	Same wiring for the monitor tool
packages/core/src/tools/monitor.test.ts	Monitor tool coverage
packages/core/src/permissions/permission-manager.ts	The L3 default-branch read-only check receives the roots; the accessor already returns empty outside PLAN
packages/core/src/permissions/permission-manager.test.ts	Resolution tests for vouched roots and redirect fallback
packages/cli/src/config/config.ts	Two-line passthrough of the new setting, dropped in bare and safe mode like autoMode
packages/cli/src/config/settingsSchema.ts	Schema entry with UNION merge across scopes
packages/vscode-ide-companion/schemas/settings.schema.json	Regenerated JSON schema
docs/users/features/approval-mode.md	User-facing section with an accurate behavior table and the whole-binary tradeoff spelled out
docs/users/configuration/settings.md	Permissions settings table gains the new key
docs/design/2026-08-22-plan-mode-extra-read-only-commands.md	Committed design doc
.qwen/e2e-tests/2026-08-22-plan-mode-extra-read-only-commands.md	Committed scripted E2E plan

Test evidence

This is an unattended CI run — I did not build or execute any PR code. The evidence below is the PR's own CI on the reviewed commit, fetched via the API.

The macOS/Windows unit jobs and the integration job show skipped by design: per ci.yml they only run in the merge queue; Ubuntu is the fast PR signal. So this commit's machine coverage is the Ubuntu suite plus the desktop-shell and security checks. The author reports testing on macOS only (their claim, not independently re-run here) — see the sandboxed-lane note below for closing that gap.

At fetch time the main unit suite is still running; the Qwen Triage Finalize job will rewrite the table below once CI settles on this commit.

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

Check	Conclusion
Test (ubuntu-latest, Node 22.x)	❌ failure
Classify PR	✅ success
Dependency CVE audit	✅ success
Desktop Shell (ubuntu-22.04)	✅ success
Desktop Shell (windows-2022)	✅ success
Secret scan (TruffleHog)	✅ 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，失败项排在最前。}

Not verified: live TUI behavior (that the prompt actually disappears in a Plan Mode session and reappears after leaving it) — unit and policy tests pin the classification change, but nothing in CI exercises the end-to-end prompt path, and the author's manual testing was macOS-only.

Sandboxed verification would settle this: @qwen-code /verify — that vouching a root suppresses the plan-mode prompt end-to-end while ib list > out.txt stays blocked is a behavioural claim CI's static checks cannot prove, and the author (fork, read-only access) tested on one platform. The author lacks write access, so /tmux is unavailable and this would be a sponsored run: a maintainer's @qwen-code /verify comment approves the head it was written against, with a pre-execution risk screen and full workspace wipe — read the resulting report with the same skepticism as the fork's own CI logs.

中文说明

代码审查

在读 diff 之前我先独立给出了自己的方案（新增 permissions.planMode.* 配置、条目归一化、仅在分类链末端查询、通过单一访问器限定模式），这个 PR 的最终实现与我的方案基本一致，且执行得很细致。以下是我对照代码（而非仅凭描述）验证过的内容：

• 担保只能做加法，不能覆盖已有判定。 额外根命令查询位于 evaluateCommandSafety 的末尾 else 分支，排在分类器所有特殊处理（git、find、sed、awk、sort、tree、uniq、tee、dd、kill 族、WRITE_ROOT_COMMANDS 正则、printf -v / less / more / rg 的 unknown 分支）之后。因此列出的根命令只能把 unknown 变成 read-only；即使列出 rm、git、tee，rm -rf build、git push、tee out.txt 也保持内建判定。已对照现有分类器源码确认，并有新测试覆盖。
• 语法级门禁仍然优先。 extra 集合贯穿语句、替换、重定向的求值，但这些结果在回退分支之后合并且优先级更高：ib list > out.txt → write，ib list $(whoami) → unknown，IB_TOKEN=x ib list → unknown，ib list | badcmd → unknown。既有的 rawRoot !== root 守卫也仍然生效，因此即使担保了小写 ib，大写调用（IB list）依旧弹窗——保守且正确。
• 模式限定集中在一处。 Config.getPlanModeReadOnlyRoots() 在 PLAN 之外返回空集，因此 shell 工具、monitor 工具、权限管理器、plan-mode 策略都可以无条件消费它，而不会放宽 default/auto-edit/auto/yolo。测试覆盖其余全部四种模式及运行时 /approval-mode 切换。--bare 与 safe 模式下丢弃该配置，与 permissions.autoMode 一致。
• 未改动的消费方全部 fail-closed。 已废弃的正则检查器（tree-sitter 不可用时的回退）、推测执行闸门、memory-scoped agent config 都继续不带额外根命令调用分类器——对被担保的命令继续弹窗，方向正确。
• 规范检查：跨作用域合并使用既有的 MergeStrategy.UNION（与其它 permissions 数组一致），schema 条目与重新生成的 VS Code schema 沿用 autoMode 形态，命名与测试同目录放置符合仓库惯例。未发现对既有工具的重复实现——归一化函数是唯一真正的新增逻辑，位置也恰当。

一个非阻断的观察，留给维护者：NON_VOUCHABLE_READ_ONLY_ROOTS 本质上是否决名单，不可能穷尽——例如 busybox、watch、su、runuser、setsid、pkexec 同样以执行其它命令为存在意义，但不在名单中。文档中"条目即对整个二进制的担保"这一取舍已覆盖残余风险（任何被担保的 CLI 都可能有写操作子命令），因此视为建议而非阻断项。

未发现严重问题。

测试证据

本次为无人值守的 CI 运行——我没有构建或执行任何 PR 代码。以上证据来自 PR 自身在被审 commit 上的 CI，通过 API 获取。macOS/Windows 单元测试与集成测试显示 skipped 是 ci.yml 的设计（仅在 merge queue 运行），Ubuntu 是 PR 阶段的快速信号。抓取时主单元测试仍在运行，收尾任务会在 CI 结束后更新上方表格。作者自述仅在 macOS 上手动测试（其自述，未在此独立复跑）。端到端行为（Plan 模式会话中弹窗确实消失）未被 CI 覆盖，可用维护者赞助的 @qwen-code /verify 运行来补齐；作者无写权限，/tmux 不可用，该验证为赞助运行并带执行前风险筛查与工作区清理，报告应像对待 fork 的 CI 日志一样保持怀疑态度阅读。

— Qwen Code · qwen3.8-max

_{Reviewed at 229aff5ae298b341b5951ea4a79ea17a1a42b672 · re-run with @qwen-code /triage}

[qwen-code-ci-bot]

Confidence: 4/5 — clean review of a well-executed, narrowly scoped feature; only non-blocking nits, and CI is still running.

Stepping back: this is what a good fork contribution looks like. It answers a real request from a different user (#9694), implements exactly the setting shape that issue proposed, and makes the conservative call on the scope question the issue left open (honoring permissions.allow in plan mode is deliberately not done — that trust-model change would need its own discussion).

The part that usually goes wrong in this kind of feature is the guardrail boundary, and that's where I spent the review. The design gets it right the way I would have wanted: the vouch is consulted only at the very end of the classification chain, write classifications are decided before it and cannot be overridden, syntactic gates (redirects, substitutions, env prefixes, unknown pipe targets) merge afterwards and dominate, the interpreter/wrapper denylist closes the obvious "vouch for bash and bypass everything" hole, and mode scoping lives in a single accessor so nothing leaks into default/auto/yolo. Every consumer I could name was checked; the ones left untouched fail closed. If I'm maintaining this in six months, the code reads clearly and the comments say why, not what.

The honest reservations, none of them blocking:

• The wrapper denylist can't be exhaustive (busybox, watch, su, and friends are not listed) — the documented whole-binary tradeoff covers it, but a maintainer may want to widen the list.
• Nothing end-to-end has exercised the prompt actually disappearing in a live Plan Mode session; the classification itself is pinned by the new tests, and the maintainer can close the rest with the sandboxed lane named in my review comment.
• The unit suite hadn't finished when this was written — hence the deferred approval below rather than an immediate one.

Every edit in the diff serves the goal; the docs and committed test plan follow this repo's conventions rather than padding the change. Approval is deferred until CI lands green on 229aff5ae298b341b5951ea4a79ea17a1a42b672; if it does, the approval posts automatically against that exact commit.

中文说明

退一步看：这是一次高质量的外部贡献。它回应的是另一位用户提出的真实需求（#9694），实现了该 issue 提议的配置形态，并在 issue 留下的范围问题上做了保守取舍（有意不在 Plan 模式下尊重 permissions.allow——那属于信任模型变更，需要另行讨论）。

这类功能最容易出错的是护栏边界，审查也集中在这里，且设计是正确的：担保只在分类链的最末端被查询；写判定在其之前做出、不可被覆盖；语法级门禁（重定向、命令替换、环境变量前缀、未知管道目标）在其后合并且优先级更高；解释器/包装器否决名单堵住了"担保 bash 即可绕过一切"的明显漏洞；模式限定集中在单一访问器，不会泄漏到 default/auto/yolo。所有能点名的消费方都已核查，未改动的消费方全部 fail-closed。半年后维护这段代码也不会吃力——代码清晰，注释解释的是"为什么"。

诚实的保留意见（均非阻断）：否决名单无法穷尽（busybox、watch、su 等不在列），由文档中"对整个二进制担保"的取舍兜底，维护者可考虑扩充名单；端到端层面尚无真实 Plan 模式会话验证弹窗确实消失——分类本身已由新测试钉死，维护者可用审查评论中点名的沙箱验证通道补齐；撰写本评论时单元测试套件尚未跑完，因此采用延迟批准而非立即批准。

diff 中每一处改动都服务于目标；文档与提交的测试计划遵循本仓库惯例而非凑数。批准将延迟到 229aff5ae298b341b5951ea4a79ea17a1a42b672 上 CI 全绿后自动提交，且精确绑定该 commit。

— Qwen Code · qwen3.8-max

_{Reviewed at 229aff5ae298b341b5951ea4a79ea17a1a42b672 · re-run with @qwen-code /triage}

[TianYuan1024]

@qwen-code /triage

Two commits since the reviewed head:

• 13c2f12f88 fixes the CI failure. The 29 plan-shell routing failures across coreToolScheduler.test.ts and Session.test.ts were Config test doubles built by casting object literals, so they lacked the accessor this PR adds; the plan-mode policy reads it before classifying and threw. Production code is unchanged. Both package suites now run in full locally rather than only the files this PR touches.
• a47196ca04 acts on the review note about NON_VOUCHABLE_READ_ONLY_ROOTS. Added the multi-call binaries (busybox, toybox), the privilege launchers (su, runuser, pkexec), and the process/namespace launchers (watch, setsid, chroot, unshare, nsenter, flock) — watch and busybox in particular were real holes. The list is still a denylist and still cannot be exhaustive, so that is now stated where it is defined and in the user docs, with the documented whole-binary scope of a vouch named as the backstop.

Still tested on macOS only.

[qwen-code-ci-bot]

Test Plan (not a blocker): src/utils/shellAstParser.test.ts — no such file or directory; src/config/config.test.ts — no such file or directory; src/core/plan-mode-shell-policy.test.ts — no such file or directory; src/tools/shell.test.ts — no such file or directory; src/tools/monitor.test.ts — no such file or directory; and 2 more.

中文说明

Test Plan（非阻断）：src/utils/shellAstParser.test.ts — no such file or directory; src/config/config.test.ts — no such file or directory; src/core/plan-mode-shell-policy.test.ts — no such file or directory; src/tools/shell.test.ts — no such file or directory; src/tools/monitor.test.ts — no such file or directory; and 2 more。

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

[qwen-code-ci-bot]

Partially reviewed — gaps disclosed.

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

Test Plan (not a blocker): src/utils/shellAstParser.test.ts — no such file or directory; src/config/config.test.ts — no such file or directory; src/core/plan-mode-shell-policy.test.ts — no such file or directory; src/tools/shell.test.ts — no such file or directory; src/tools/monitor.test.ts — no such file or directory; and 2 more.

中文说明

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

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

Test Plan（非阻断）：src/utils/shellAstParser.test.ts — no such file or directory; src/config/config.test.ts — no such file or directory; src/core/plan-mode-shell-policy.test.ts — no such file or directory; src/tools/shell.test.ts — no such file or directory; src/tools/monitor.test.ts — no such file or directory; and 2 more。

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

[TianYuan1024]

@qwen-code /triage

All three Criticals and the five Suggestions are addressed in eed50f9d39. Each Critical reproduced first; the probes now return unknown.

R2-1 (launchers). Agreed that a name list does not converge — so it is no longer the only defence. The list moved into the classifier, where it gates the terminal branch directly, so no caller can vouch a launcher back in by supplying its own set; it also gained all 16 demonstrated names. On top of that, a vouched root is refused as soon as one of its arguments names a command the classifier knows, matched on the basename. That closes the demonstrated shape for launchers nobody has enumerated: obscurelauncher rm -rf build and obscurelauncher /bin/rm -rf build both classify unknown without anyone having to know obscurelauncher is a launcher. The cost is an occasional extra prompt when a CLI sub-command shares a name with a real command; that is the side to err on, and it is documented. Residual, also documented: a vouched launcher wrapping something the classifier does not recognise (time ./script.sh) — though time itself is now refused.

R2-2 (expansion pattern word). Fixed as suggested. evaluateSubstitutions now treats a $( or backtick surviving in an expansion — after the substitution walk collected nothing — as exactly that hidden channel. This also closes the pre-existing built-in-root exposure you noted: echo ${HOME%%$(rm -rf build)} classified read-only before this PR and classifies unknown now. All four operators are pinned, quoted and unquoted, plus backtick form.

R2-6 / ledger R2-3 (state planters). Took the deeper fix rather than the name fix: these are refused at the classifier, not filtered out of the caller-supplied set. hash -p /tmp/evil/git git && git status and the alias variant both classify unknown. enable, trap, builtin and the rest of the family are included.

R1-2. Array.isArray guard added — a string is no longer walked per character, and a number, object, or boolean no longer throws out of the Config constructor during startup.

R1-3. Answered structurally rather than by enumeration: the refused-roots test is it.each([...NEVER_READ_ONLY_ROOT_COMMANDS]), driven off the exported set, so no future edit can leave an entry untested.

R1-4. Safe-mode and --bare tests added, both asserting the set stays empty after --approval-mode plan and after a later setApprovalMode(PLAN), plus a positive control without the flags.

R1-5. getConfirmationDetails tests added for both tools, asserting the vouched sub-command drops out of the confirmation and the unvouched one stays.

R2-7 (trailing commas). Confirmed against the repos own dependency — JSON.parse(stripJsonComments(...))` rejects all three snippets. Fixed in the user doc, the design doc, and the e2e plan; all three now parse to the intended object.

R2-4 / R2-5 (e2e plan). Baseline expectations reworded — the state-modifying cases are blocked on baseline, not prompted, and only cases 1, 5b, 7, 8, 9 depend on the setting. Launch instructions corrected: the plan now says to launch from the scratch workspace via node <repo>/scripts/dev.js or the bundle, and says why npm run dev cannot work. Case 6 gained the launcher and state-planter probes, and a new case 6b covers the unrecognised-launcher guard.

Verification: full packages/core (20,954) and packages/cli (23,100) suites, not just the touched files. Remaining local failures are pre-existing on a clean tree and green in CI. Still tested on macOS only.

[TianYuan1024]

Correction on my previous comment: I said all three trailing-comma snippets were fixed. Only the e2e plan was. 38bd5d44f9 fixes the other two, and the reason the first attempt did not stick is worth recording.

Removing the commas is not enough. This repo's prettier config sets trailingComma: "all", and prettier's jsonc parser honours it, so the formatter puts them straight back — my earlier fix was undone by the pre-commit hook and I did not re-check the file afterwards. That is also why all 51 ```jsonc blocks under docs/ carry this defect, not just mine. The json parser accepts `//` comments and never adds trailing commas, so changing the fence language is the fix that survives formatting. All three snippets now round-trip through `JSON.parse(stripJsonComments(...))` and `prettier --check` is clean.

Out of scope, flagging rather than fixing: the two pre-existing permissions.autoMode snippets on the same page have the identical defect and still fail that parse.

State of the other findings at 38bd5d44f9, re-verified against the tree rather than from memory:

Finding	State
R2-1 launchers	NEVER_READ_ONLY_ROOT_COMMANDS in the classifier, all 16 demonstrated names present, plus the structural vouchedRootIsSafe refusal
R2-2 expansion pattern word	HIDDEN_SUBSTITUTION check in evaluateSubstitutions
R2-6 / ledger R2-3 state planters	refused at the classifier; alias, bind, builtin, compgen, complete, enable, hash, let, set, shopt, trap, unalias
R1-2	Array.isArray guard in normalizePlanModeReadOnlyRoots
R1-3	it.each([...NEVER_READ_ONLY_ROOT_COMMANDS]), driven off the exported set
R1-4	safe-mode and --bare tests, plus a positive control
R1-5	getConfirmationDetails vouch tests for both shell and monitor
R2-4 / R2-5	e2e baseline expectations and launch instructions corrected, case 6b added

@qwen-code /triage

[qwen-code-ci-bot]

Partially reviewed — gaps disclosed.

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

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

Test Plan (not a blocker): src/utils/shellAstParser.test.ts — no such file or directory; src/config/config.test.ts — no such file or directory; src/core/plan-mode-shell-policy.test.ts — no such file or directory; src/tools/shell.test.ts — no such file or directory; src/tools/monitor.test.ts — no such file or directory; and 2 more.

Convergence: round 3 posted 14 inline comment(s), 13 of them reported for the first time; the previous round posted 12 (8 new). Findings keep coming back to the same files: packages/core/src/utils/shellAstParser.ts (findings in round 2; 5 more now); .qwen/e2e-tests/2026-08-22-plan-mode-extra-read-only-commands.md (findings in round 2; 3 more now); docs/users/features/approval-mode.md (findings in round 2; 2 more now), and 1 more file(s). 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.)

中文说明

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

未审查：reverse audit — did not converge within the reverse-audit round cap of 5。

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

Test Plan（非阻断）：src/utils/shellAstParser.test.ts — no such file or directory; src/config/config.test.ts — no such file or directory; src/core/plan-mode-shell-policy.test.ts — no such file or directory; src/tools/shell.test.ts — no such file or directory; src/tools/monitor.test.ts — no such file or directory; and 2 more。

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

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

[TianYuan1024]

All 14 round-3 findings are addressed in 5c7f5718a2. Taking the convergence observation first, because it is the finding that matters.

The root cause. Every launcher finding — 16 names in round 2, language interpreters and exec wrappers in round 3 — is the same defect. A vouch is a claim about a binary; the classifier was applying it to arbitrary arguments, and both halves of the round-2 defence matched text against surfaces that have no edge. Enumerating spellings of rm terminates no sooner than enumerating launchers, so the rule is inverted rather than extended.

A vouched root now classifies read-only only for an invocation the classifier can actually read:

• every argument is a plain literal word — whitelisted by character, cross-checked with the existing hasShellExpansion. All seven demonstrated evasions (r\m, r'm', r"m", "r"m, $cmd, ${cmd}, *) plus {rm,ls} classify unknown;
• no argument names a command the classifier knows — unchanged, but it now runs on literal words only, so the text it inspects is the text that executes;
• the root is not a known command under another spelling — git.exe matched no dispatch arm and reached the vouch branch. .exe is stripped for the known-command test in both places. Deliberately not stripped before the dispatch chain: that could only widen classifications to read-only, and this must not. All five .exe rows flip back to unknown.

On the interpreters (R2-1, third round). This one has no structural answer and I want to be straight about it: python3 evil.py names a file, not a command, so no argument inspection can catch it. For that family the list is the answer. Added: python, python3, node, deno, bun, perl, ruby, php, lua, tclsh, wish, osascript, ssh, rsh, sudoedit, coproc, fc, history, and . (R3-12 — the POSIX spelling of source). The docstring and both docs now say the lists are the floor under the shape rule rather than the defence itself, which is what R3-13 was pointing at.

R3-4 / R3-5 (hidden substitutions). Both pre-existing, neither needs a vouch. <(…)/>(…) in a pattern word run exactly as $(…) does; a heredoc body with an unquoted delimiter is expanded before it reaches stdin, so cat <<EOF with a backtick payload classified read-only. Handled in the branch that already existed for this. A quoted delimiter still classifies read-only, because there the body genuinely is inert.

R3-2. Correct, and the shape rule refuses ib domain watch by design. Case 8 now monitors ib domain list, with a companion assertion that ib domain watch prompts deliberately and must not be "fixed". The monitor unit test used the same command with a mocked classifier — changed too, so the test and the plan agree with the code.

Remaining suggestions: R3-6 dist/cli.js (confirmed — esbuild outdir: 'dist', nothing produces bundle/qwen.js); R3-7 mixed-array element guard; R3-8 the dd/kill/killall/pkill/tee arm of namesAKnownCommand, which no test pinned; R3-9 cases relabelled 5a/5b; R3-10 residual example no longer uses time, a launcher the list covers; R3-11 the strict-JSON fence no longer carries // comments (the fence stays json — restoring jsonc reopens R2-8, since prettier's trailingComma: "all" puts the commas back).

Verification: full packages/core suite, 20,988 passed. The one failure is extensionManager.test.ts, pre-existing on a clean tree and green in CI. Every witness row from all five Criticals re-probed against this commit, with controls (ib domain list, ib show 1, ib get a/b.txt --format=json, cat <<'EOF' with a backtick payload) confirming the rule did not over-refuse. Still macOS only.

@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 and its suite did not run locally.

Test Plan (not a blocker): src/utils/shellAstParser.test.ts — no such file or directory; src/config/config.test.ts — no such file or directory; src/core/plan-mode-shell-policy.test.ts — no such file or directory; src/tools/shell.test.ts — no such file or directory; src/tools/monitor.test.ts — no such file or directory; and 1 more.

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

• docs/design/2026-08-22-plan-mode-extra-read-only-commands.md:55 — [review] Design doc's quoted behavioural hook omits the vouchedRootIsSafe gate
• packages/core/src/utils/shellAstParser.test.ts:1114 — [probe] it.each([...NEVER_READ_ONLY_ROOT_COMMANDS]) iterates the very constant it guards; a deletion is unobservable

Convergence: round 4 posted 3 inline comment(s), 2 of them reported for the first time; the previous round posted 14 (13 new). Findings keep coming back to the same files: packages/core/src/utils/shellAstParser.ts (findings in rounds 2, 3; 2 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.)

中文说明

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

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

Test Plan（非阻断）：src/utils/shellAstParser.test.ts — no such file or directory; src/config/config.test.ts — no such file or directory; src/core/plan-mode-shell-policy.test.ts — no such file or directory; src/tools/shell.test.ts — no such file or directory; src/tools/monitor.test.ts — no such file or directory; and 1 more。

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

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

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

[TianYuan1024]

Round 4 is in 9435437405. R2-1 has now come back four times, so this round I stopped adding names and went after why the enumeration was load-bearing.

It was load-bearing because the setting merged from workspace scope. A cloned repository could put "extraReadOnlyCommands": ["make"] in its own .qwen/settings.json. That turns "the user accepted this binary" into "the repository asserted something about a binary" — and defending that means enumerating every binary an attacker might pick, which is exactly the list that keeps regenerating. Four rounds, four batches: launchers, interpreters, versioned interpreter spellings, build tools. There is no end to it, because any binary can execute something its command line does not mention.

So the setting now follows security.allowPrivateNetworkHooks: stripped from workspace scope in stripWorkspaceSecurityBypasses, with a startup warning when a workspace file sets it, and the schema carries the same "Only honored from User, System, and SystemDefaults" wording. An entry can only come from someone who typed it into their own settings file, so a vouch for make means what the docs already said it means — the user accepted that binary.

What that changes about the lists: they are now a floor under foreseeable mistakes, not a boundary against an adversary who chooses the entry, and the docs and the set docstring say which of the two they are. What stays adversarial is the invocation — the model picks the arguments and repository content can influence the model — and that is the bounded problem vouchedRootIsSafe addresses.

This narrows the feature and I want that on the record: a team can no longer ship the vouch in the repo, only in each developer's settings. If you would rather keep breadth and accept repository-supplied vouches, deleting the permissions.planMode branch in stripWorkspaceSecurityBypasses restores the old behaviour — but then the enumeration is load-bearing again, and I do not think it can be made to converge. Your call; I picked the bounded side.

Also fixed from this round:

• Variable planters (read, mapfile, readarray, getopts) plant PATH for the next statement with no VAR=VALUE word, so the variable_assignment guard never saw them. Enumerable family, now listed.
• Versioned spellings matched by shape (VERSIONED_INTERPRETER), not by name — listing every release of every interpreter is not a finite job.
• =-embedded names: namesAKnownCommand now splits on = as well as the path separators. This one is a genuine bypass of the shape rule under the invocation threat model, and the most important of the four sub-findings.
• R4-1 ${v@P}: correct, and no vouch needed — echo ${x%%${v@P}} auto-ran. Prompt expansion runs a substitution held in the variable's value, and in a pattern word it is a leaf, so neither the adjacency check nor the opener scan saw it. All five operators pinned.
• R4-2 <<\EOF: taken as suggested. The backslash quotes the delimiter, the body is inert, and it no longer prompts.

Both deferred items are done too: the design doc's quoted hook was stale (predates the vouchedRootIsSafe gate), and the set-driven it.each iterates the constant it guards — so a deletion would delete its own test. There is now a spelled-out containment test for the security-critical names alongside it.

The e2e plan needed reworking for the scope change: the vouch moves to $QWEN_HOME/settings.json (scratch home, so it cannot touch a real one), with a new case 6c asserting a workspace vouch is ignored and warned about.

Verification: full packages/core (21,019 passed) and packages/cli (23,292 passed) suites. Remaining failures are the pre-existing extensionManager and ink-cursor/AuthDialog sets, all reproducing on a clean tree and green in CI. Every witness row from this round re-probed, with controls confirming no over-refusal. Still macOS only.

@qwen-code /triage

[qwen-code-ci-bot]

Test Plan (not a blocker): src/utils/shellAstParser.test.ts — no such file or directory; src/config/config.test.ts — no such file or directory; src/core/plan-mode-shell-policy.test.ts — no such file or directory; src/tools/shell.test.ts — no such file or directory; src/tools/monitor.test.ts — no such file or directory; and 2 more.

Convergence: round 5 posted 15 inline comment(s), 15 of them reported for the first time; the previous round posted 3 (2 new). Findings keep coming back to the same files: packages/core/src/utils/shellAstParser.ts (findings in rounds 2, 4; 6 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.)

中文说明

Test Plan（非阻断）：src/utils/shellAstParser.test.ts — no such file or directory; src/config/config.test.ts — no such file or directory; src/core/plan-mode-shell-policy.test.ts — no such file or directory; src/tools/shell.test.ts — no such file or directory; src/tools/monitor.test.ts — no such file or directory; and 2 more。

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

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

[TianYuan1024]

All 15 round-5 findings are in 16b4a1782a. No new laundering class this round — the scope change closed that one — but four findings are regressions I introduced last round tightening the refusal rules, and those matter most, because they broke the reads this feature exists to allow.

R5-2, path arguments. Scanning every /-separated segment made ./report.json name ., the POSIX spelling of source, so a vouched CLI prompted on every relative path — and docs/history/x.md went the same way through history. Back to the basename of each =-separated part, as suggested. /bin/rm, --exec=rm and --exec=/bin/rm are still refused; ib get ./report.json classifies read-only again.

R5-3, non-ASCII. \w is ASCII-only, so 报告.md and café.txt refused the vouch. Every shell metacharacter is ASCII, so a bare non-ASCII word is literal by construction — the whitelist now matches letters, digits and marks by Unicode property.

R5-5, Windows spellings. The refusal side normalized .exe, the acceptance side did an exact lookup, so vouching mytool never matched mytool.exe. Both sides agree now; a known command under an .exe spelling is still refused first, so this cannot re-open one. Case is documented rather than changed — MyTool and mytool really are different binaries on Linux.

R5-7, process substitution in heredoc bodies. You are right and I had this wrong: a heredoc body expands as if double-quoted, so <(…) never runs there. I reused the pattern-word opener set. The body scan has its own set now, and the test that asserted the wrong behaviour asserts the right one, with the comment corrected.

R5-1 (Critical) is the other half of that same scan: PROMPT_EXPANSION was consulted everywhere except there, and a <<-EOF body always parses as one raw leaf, so a tab-indented ${v@P} classified read-only while bash ran the substitution held in the variable. Fixed as suggested, both heredoc forms pinned.

R5-6 was the sharpest test finding of the review so far. I reproduced the mutant: deleting VERSIONED_INTERPRETER left the it.each at 9 passed, because ./verify.py was already refused by path-segment handling. With the argument changed to verify.py, the same mutant fails it 10/10.

Remaining: R5-4 adds crontab, systemd-run, cc, go, expect, luajit, bunx, pnpx and the python3.13t suffix — plus docker/podman, which you flagged as borderline; I included them, since a payload living inside an image is the same "never in argv" case the list is for. R5-13: verified before writing it down — shell defines no requiresUserInteraction, so a PermissionRequest hook's allow decision does reach validatePlanModeShellApproval; the design doc now says so instead of claiming nothing existed. R5-14, R5-8, R5-9, R5-10, R5-11, R5-12: doc caption points at ~/.qwen/settings.json, the scope list names SystemDefaults, and the e2e plan gains the login step the scratch QWEN_HOME makes necessary, the restart case 6c's restore needs, git in case 6 so that bullet tests what it claims, and an honest precondition for 6b.

Verification: full packages/core, 21,034 passed, one pre-existing extensionManager failure that reproduces on a clean tree and is green in CI. Every round-5 witness re-probed, plus every refusal from rounds 1–4 re-probed as a regression check — 30 hostile rows still closed. No CLI sources changed this round. Still macOS only.

@qwen-code /triage

[TianYuan1024]

Merged main (10f5d0c465) — the branch was conflicting on packages/cli/src/config/settings.ts, and the resolution is worth a note because it deletes code rather than reconciling it.

main (#9098, #9737) generalised the workspace-scope strip this PR had hooked into: WORKSPACE_RESTRICTED_SETTINGS in settingsUtils.ts is now the one list driving the strip, the "your workspace value was ignored" warning, and the settings dialog scope filter, so the three surfaces cannot drift apart.

This PR had hand-rolled the same thing for permissions.planMode — a second branch inside stripWorkspaceSecurityBypasses, plus a third copy of the warning string. Both are gone in favour of upstream's version, and the setting is registered as a single entry in the list. config/settings.ts no longer appears in this PR's diff at all; the whole scope restriction is now three lines in settingsUtils.ts. Behaviour is unchanged and the four scope tests added here pass unmodified against the generic mechanism.

Verification after the merge: full packages/core (21,229 passed) and packages/cli (23,502 passed). Failures are the known pre-existing set — extensionManager, ink-cursor-rendering, AuthDialog — plus two serve/server.test.ts cases that pass in isolation and fail only under full-suite load, like the transport case noted earlier. Settings schema regenerated.

[qwen-code-ci-bot]

Partially reviewed — gaps disclosed.

Not reviewed: reverse audit — did not converge within the reverse-audit round cap of 5 (rounds 3, 4, 5 each reported findings).

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

Not explored to full depth (tool budget reached): chunk 5: could not execute the new vitest suites (permission-manager/monitor/shell) against the real WASM classifier — the review worktree has no node_modules/dist and ….

Test Plan (not a blocker): src/utils/shellAstParser.test.ts — no such file or directory; src/config/config.test.ts — no such file or directory; src/core/plan-mode-shell-policy.test.ts — no such file or directory; src/tools/shell.test.ts — no such file or directory; src/tools/monitor.test.ts — no such file or directory; and 2 more.

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

• docs/design/2026-08-22-plan-mode-extra-read-only-commands.md:113 — [review] Design doc cites nonexistent stripWorkspaceSecurityBypasses ; actual function is stripWorkspaceRestrictedSettings
• docs/design/2026-08-22-plan-mode-extra-read-only-commands.md:66 — [probe] Design doc's dispatch snippet omits the .exe disjunct, which IS a widening
• docs/users/features/approval-mode.md:196 — [probe] Doc promises .exe ignored on both sides; only the invocation side strips it
• .qwen/e2e-tests/2026-08-22-plan-mode-extra-read-only-commands.md:91 — [review] Cases 3, 4, 5a call the re-firing prompt 'one-time' — contradicting case 1 and the implementation
• packages/core/src/utils/shellAstParser.test.ts:517 — [review] Pattern-word substitution tests omit the ^ / , operators and the ${var/…/…} pattern part
• packages/core/src/utils/shellAstParser.test.ts:570 — [probe] No test pins substitutions in the replacement part of ${var/pat/rep}
• packages/core/src/utils/shellAstParser.test.ts:588 — [probe] $(…) inside always-leaf <<- heredoc bodies is caught only by the regex's $( branch, and no test pins it
• packages/core/src/utils/shellAstParser.test.ts:1275 — [review] The versioned-interpreter pin list covers 8 of 9 regex families — no wish spelling
• packages/core/src/utils/shellAstParser.ts:298 — [probe] A bare . argument of a vouched root is refused — . / .. can never name an executable

Convergence: round 6 posted 2 inline comment(s), 2 of them reported for the first time; the previous round posted 15 (15 new). Findings keep coming back to the same files: packages/core/src/utils/shellAstParser.ts (findings in round 5; 2 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.)

中文说明

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

未审查：reverse audit — did not converge within the reverse-audit round cap of 5 (rounds 3, 4, 5 each reported findings)。

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

未探索到全部深度（达到工具调用预算）：chunk 5：could not execute the new vitest suites (permission-manager/monitor/shell) against the real WASM classifier — the review worktree has no node_modules/dist and …。

Test Plan（非阻断）：src/utils/shellAstParser.test.ts — no such file or directory; src/config/config.test.ts — no such file or directory; src/core/plan-mode-shell-policy.test.ts — no such file or directory; src/tools/shell.test.ts — no such file or directory; src/tools/monitor.test.ts — no such file or directory; and 2 more。

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

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

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

[TianYuan1024]

Both round-6 Criticals are in 74a4bbbb7c, but they are not the same kind of finding and I want to be straight about which one I think is closed.

R6-2 is the real bug of this round, and it was mine. localGitConfigMakesCommandUnsafe is a defence against a repository planting diff.external / core.fsmonitor in its own .git/config, and it was keyed to the literal string git. This PR's whole premise is that a binary can be a read-only frontend under some other name — so the feature introduced aliases for exactly the name that defence was pinned to. gitw status in a hostile checkout auto-executed the planted hook while git status two lines away prompted.

I did not take the suggested minimum ("at least when its first argument is diff/status"): a wrapper spells its verb wherever it likes, and the reporting issue's own CLI puts it in the second argument, so filtering on the sub-command would have moved the gap rather than closed it. A vouched root now gets the gate unfiltered. That costs nothing in ordinary repositories — getLocalGitConfigRisk returns no risk and the command stays read-only — which the new test pins in both directions, clean repo and planted repo.

R6-1 I have fixed as asked, and I do not claim it is closed. The 22 names and the regex rewrite are in: VERSIONED_INTERPRETER now matches a family name followed by a version rather than one release shape, so gcc-13, luajit-2.1.0-beta3, expect5.45 and python3.7m fall out of the rule instead of needing entries. All 27 witnesses flip to unknown; all controls hold. Nine sibling pairs are pinned as tests, at the family edges where every round has found its gap.

But you are right that this does not converge, and I think it is worth naming why, because I do not want a seventh batch to look like progress. The threat model changed under the list. Through round 4 the adversary picked the entry — a cloned repo could vouch make — and that was unbounded by construction, which is what the workspace-scope restriction fixed. R6-1's adversary picks the argument, and the vouch comes from the user's own settings. There, uv is a user error of precisely the same kind as bash: the user asserted "this binary only reads" about a binary that runs whatever it is handed. The list catches the assertions people are most likely to get wrong. It cannot catch the assertion being wrong in general, because uv run evil.py and the reporter's ib get ./report.json are structurally identical — same arity, same literal shapes, same everything the classifier can see.

I considered the argument-side rule that would be the structural answer — refuse a vouched invocation whose arguments name a script or a loadable object — and rejected it. It does not close the class: 6 of your 20 witnesses (poetry run evil, gradle build, bare ninja, run0 ./payload, newgrp wheel, mvn exec:java) have no payload extension to match. And it would break the feature's core case, a read-only analyser over source files: ib lint src/main.py would start prompting. Partial mitigation, real cost. I have left ib lint src/main.py in the control set so that trade is visible in the tests rather than only here.

So: the list is a floor under foreseeable mistakes, and the boundary is that only the user can set it. The design doc says that; if you would rather the setting refuse to load unknown roots entirely, that is a product decision I am happy to take, but it is a different feature and I would rather not smuggle it in as a fix.

Deferred items, all done since they were cheap: the design doc's function name (stripWorkspaceRestrictedSettings) and its dispatch snippet's missing .exe disjunct; the doc claim that .exe is stripped on both sides — it is stripped from the invocation only, so listing mytool.exe covers only mytool.exe, now stated that way; the e2e plan's "one-time" wording for a prompt that re-fires by design; ^/, case operators and both halves of ${var/pat/rep} in the pattern-word battery (the $(…) spellings classify write, not unknown — stronger, and now pinned apart so a spelling changing category fails); $(…) in an always-leaf <<- body; and a wish spelling in the versioned list.

One of the deferred probes was a real over-refusal, so I took it as a finding: a bare . argument was refused because . is the POSIX spelling of source, which made ib list . prompt for nothing. . and .. as whole words now contribute nothing to the known-command check; . as a root is still refused by the dispatch chain, which reaches the refusal list before any vouch is consulted.

Verification: full packages/core, 21,293 passed. The one failure is the pre-existing extensionManager case. shellAstParser.test.ts is at 758.

[qwen-code-ci-bot]

Partially reviewed — gaps disclosed.

Not reviewed: reverse audit — did not converge within the reverse-audit round cap of 5 (rounds 3, 4, 5 each reported findings).

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

Test Plan (not a blocker): src/utils/shellAstParser.test.ts — no such file or directory; src/config/config.test.ts — no such file or directory; src/core/plan-mode-shell-policy.test.ts — no such file or directory; src/tools/shell.test.ts — no such file or directory; src/tools/monitor.test.ts — no such file or directory; and 2 more.

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

• packages/cli/src/config/settingsUtils.ts:273 — [review] PR description's scope-union and byte-for-byte-identical claims contradict shipped behavior
• .qwen/e2e-tests/2026-08-22-plan-mode-extra-read-only-commands.md:54 — [review] Baseline-divergence list omits case 6d
• packages/core/src/utils/shellAstParser.test.ts:552 — [probe] it.each titles mangled by printf %s; regression output ungreppable
• packages/core/src/utils/shellAstParser.test.ts:1233 — [probe] Refusal-floor test iterates its own constant; 70 of 125 entries un-pinned against deletion
• packages/core/src/utils/shellAstParser.ts:1498 — [probe] changedDirectory branch for vouched git frontends is pinned by no test
• packages/core/src/utils/shellAstParser.test.ts:1486 — [probe] .exe vouch entries never match the bare invocation; the Windows-natural entry spelling is silently inert
• .qwen/e2e-tests/2026-08-22-plan-mode-extra-read-only-commands.md:186 — [review] Case 6c restart lacks a launch directory; the moved vouch never loads
• .qwen/e2e-tests/2026-08-22-plan-mode-extra-read-only-commands.md:174 — [probe] Case 6d step 2 promises 'no prompt' but every in-session execution form prompts
• .qwen/e2e-tests/2026-08-22-plan-mode-extra-read-only-commands.md:104 — [review] All five instructed restarts silently drop Plan mode; post-restart cases exercise AUTO
• packages/core/src/utils/shellAstParser.test.ts:659 — [probe] Loose .not.toBe('read-only') hides an existing unknown/write category drift in heredoc pins
• packages/core/src/utils/shellAstParser.test.ts:550 — [probe] Default-value operator family (:- := :? …) unpinned; bash executes its payloads
• packages/core/src/utils/shellAstParser.test.ts:1356 — [probe] Versioned-interpreter pins miss the g++/cc/clang++ families; dropping them passes silently

Convergence: round 7 posted 7 inline comment(s), 6 of them reported for the first time; the previous round posted 2 (2 new). Findings keep coming back to the same files: packages/core/src/utils/shellAstParser.ts (findings in round 6; 6 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 keeps the loop from re-deriving the same set; this PR's reviews already resolve to a critical posting floor. (Observation only — nothing was withheld from this review because of this observation.)

Mechanism health: this round did not close cleanly, so it withholds the incremental anchor — and the round it recovered had none either, so the next review re-reads the whole diff and will keep doing so until a round's marker carries an anchor again. (Stated, not acted on — this changes nothing about what the round posts.)

中文说明

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

未审查：reverse audit — did not converge within the reverse-audit round cap of 5 (rounds 3, 4, 5 each reported findings)。

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

Test Plan（非阻断）：src/utils/shellAstParser.test.ts — no such file or directory; src/config/config.test.ts — no such file or directory; src/core/plan-mode-shell-policy.test.ts — no such file or directory; src/tools/shell.test.ts — no such file or directory; src/tools/monitor.test.ts — no such file or directory; and 2 more。

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

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

机制健康：本轮未能干净收尾，因而扣留了增量锚点，而它恢复到的那一轮也没有锚点，因此下一次评审将重读整个 diff——并会一直如此，直到某一轮的标记重新带上锚点。（仅陈述，不据此行动——这不改变本轮发布的任何内容。）

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

[TianYuan1024]

Merged main (64e9fd855a, clean — no conflicts) and fixed round 7 in a085ee7af8.

On the merge: main had already recorded cd-cua-driver.yml's shipped size in .size-baseline with the same 42519 value, so my earlier commit for it is now redundant but harmless. #9796 moved ApprovalMode to core/config/approval-mode.js; the PLAN gate on getPlanModeReadOnlyRoots() is unchanged and all four decision points still thread the vouch. Settings schema regenerated with no drift.

R7-1/2/3 — git's global options. This is the same mistake as R6-2 one level down: I gave a vouched wrapper git's planted-config gate, and then left the options that walk past the gate unscreened. -C and --git-dir point the probe at the wrong repository; -c core.fsmonitor=./evil.sh needs no planted config at all, because the value arrives through argv; --exec-path=./evil redirects where git resolves every non-builtin sub-command, and a clone preserves the executable bit, so the payload ships in the repository under review.

I did not take the broad reading of the suggestion ("treat any leading-dash argument on a vouched root as unknown, matching evaluateGitSafety"). Most CLIs take flags; refusing all of them retracts the feature. What I took instead is the "minimally refuse" list, because it is the rare case where enumeration is actually the right tool — git's global options are a finite, documented, versioned set, unlike the open universe of binaries that R6-1 chases. ib --json list and ib list --format=json stay read-only and are pinned as tests; the cost is a prompt for a CLI that spells its own config flag -c or -C, and that is in the docs.

The screen went into vouchedRootIsSafe rather than the config gate on purpose: R7-2's witness is a clean repo, so there is no repository signal to key on, and the gate is only consulted on the InDirectory entry points. Argument shape is the only thing that closes it everywhere.

R7-5. Flag half fixed with the same screen — GIT_EXTERNAL_HELPER_OPTION already named --textconv/--filters/--show-signature/--ext-diff for literal git, so the wrapper path now reuses it. I did not extend getLocalGitConfigRisk's key set, and I want to flag that as a deliberate omission rather than an oversight. Adding filter.*.clean/smudge would downgrade git diff in every repository where git lfs install --local has run, which is a large number of real checkouts — a broad regression to literal git's behaviour that this PR has no mandate to make. As you note, that key-set gap pre-exists for literal git; it wants its own PR with the git-lfs question settled. diff.*.textconv and gpg.program do not have that problem and could go in that PR cleanly.

R7-4 is a real AST bug and the best find of this round. I had not registered that a pipeline written after a heredoc opener parses inside heredoc_redirect, so the redirected_statement arm — which filters every *_redirect child out — was dropping a whole write segment. Fixed by evaluating the outermost statement-shaped children of a redirect; deeper nodes stay owned by the substitution walk. cat <<EOF | rm -rf build is pinned too, since the shape predates the vouch for built-in roots.

R7-6. Finite, as you say: two families were in the refusal list but missing from the companion regex. Both added, go1.22/go-1.22/go1.22.0/nodejs18 pinned.

R6-1. Seven demonstrated names added, plus their task-runner and environment-manager siblings; the floor is at 155. My position is unchanged and I am not restating it — the maintainer decision you are waiting on is the right thing to wait on.

One thing I did change is how the floor is tested, because your deferred probe was right that it could not detect its own erosion: the it.each iterated the constant it guards, so deleting an entry deleted its own test. Every name is now spelled out with the size asserted both ways — a deletion fails containment, an undeclared addition fails the count. Verified with a mutant: dropping one name fails the suite.

Other deferred items taken: default/assign/error/alternate expansion operators pinned (all already refused — coverage gap, not a hole), substring and subscript positions, the changedDirectory branch for vouched frontends, the g++/cc/clang++ versioned families; four heredoc assertions tightened from not.toBe('read-only') to exact categories, which surfaced that only the tab-indented <<- spelling is the leaf case; three it.each titles un-mangled. The e2e plan gains the /plan step every restart silently needed — you were right that the post-restart cases were exercising the default mode — plus launch directories for 6c and 6d, and 6d's second step now says relaunch rather than pretending cwd can change mid-session.

Verification: full packages/core 21,701 passed, the one failure the pre-existing extensionManager case. shellAstParser.test.ts at 823. Typecheck and lint 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.

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

Test Plan (not a blocker): src/utils/shellAstParser.test.ts — no such file or directory; src/config/config.test.ts — no such file or directory; src/core/plan-mode-shell-policy.test.ts — no such file or directory; src/tools/shell.test.ts — no such file or directory; src/tools/monitor.test.ts — no such file or directory; and 2 more.

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

• packages/core/src/utils/shellAstParser.test.ts:590 — [review] False bash-semantics claim about <(...) in pattern words, in three places this PR adds
• packages/core/src/utils/shellAstParser.ts:1439 — [review] No test pins that an uppercase-spelled invocation of a vouched root still prompts
• packages/core/src/utils/shellAstParser.test.ts:702 — [review] The <<- always-one-raw-leaf coverage comment is false in both directions
• .qwen/e2e-tests/2026-08-22-plan-mode-extra-read-only-commands.md:61 — [review] Restart warning misstates approval-mode persistence and the post-restart landing mode
• packages/core/src/utils/shellAstParser.ts:435 — [review] Quoted literal arguments refuse the vouch — whitelist tests raw text, not the stripped form
• .qwen/e2e-tests/2026-08-22-plan-mode-extra-read-only-commands.md:178 — [review] The plan's session-cwd immutability premise is false — /cd and the tool directory parameter move the gate's directory
• packages/core/src/utils/shellAstParser.test.ts:689 — [review] Arithmetic expansion $((…)) is unpinned in the hidden-channel test blocks

Convergence: round 8 posted 4 inline comment(s), 3 of them reported for the first time; the previous round posted 7 (6 new). Findings keep coming back to the same files: packages/core/src/utils/shellAstParser.ts (findings in rounds 6, 7; 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] R8-2: The confirmation-scope filter in getConfirmationDetails (packages/core/src/tools/shell.ts:2129-2132; same shape packages/core/src/tools/monitor.ts:226-229) classifies each sub-command alone against the original cwd and has no model of state planted by a preceding sub-command, so a vouched root whose danger depends on that state is excluded from the displayed confirmation scope. Two demonstrated vectors: (1) cd /hostile && gitw status && curl example.com — after the cd, gitw status is probed at the clean original cwd, passes the planted-config gate and is dropped, so the dialog asks only about curl; (2) export GIT_DIR=/x/.git GIT_WORK_TREE=/x && gitw status — the export stays confirmable while gitw status is dropped the same way (a cd-tracking fix does not close this vector). Approval then executes gitw status inside the hostile repo, whose planted core.fsmonitor executes attacker code — the exact attack the planted-config gate this PR extends exists to prevent; the changed-directory fail-closed branch this PR added fires only when cd and the vouched root stay inside one classified string. Witness (probed end-to-end at this commit): SCOPE VOUCHED: defaultPermission=ask rootCommand="export" permissionRules=[] (gitw dropped) vs SCOPE UNVOUCHED: rootCommand="export, gitw"; FSMONITOR EXECUTED: marker file created by git status via env-planted GIT_DIR. Pre-PR, gitw status was always unknown and always in scope; the identical hiding pre-exists for built-in read-only commands, but the diff line is what makes the wrapper form newly reachable. Fix: mirror PermissionManager.evaluateCompoundCommand — if any sub matches the existing cd/pushd test or is an environment/state planter (export, declare, readonly, typeset, local), keep all later subs confirmable. (Inline placement at shell.ts:2131 was dropped as a location overlap with the existing R1-5 comments 3836659886/3836948926 — those report missing tests at this call site; this is the defect such tests would catch.)

[Critical] R6-1 (still stands — re-posted under its original id): the vouch refusal surface remains an unbounded enumeration of payload-executing programs. The round-7 minimum fix landed — all seven names demonstrated in round 7 (just, rake, tox, dotnet, pipx, bazel, task) are now in the 155-entry NEVER_READ_ONLY_ROOT_COMMANDS, verified at this commit — but the class mechanism still fires: this round demonstrated 28 fresh same-family names absent from both refusal mechanisms — ash, mksh, osh, yash, posh (shell families beside the listed bash/sh/dash/ksh/zsh), jshell, scala, groovy, tsx, ts-node, swift, racket (interpreters/runners), dart, julia, kotlin, elixir, escript, rscript, ocaml, guile, crystal, nim, clojure, ghc, runghc, tcc, zig, dmd (language toolchains). Witness (sweep at HEAD): all 28 report inNEVERset=false and vouched <name> ./evil.<ext> = read-only; controls bash/python3/ruby/make report inNEVERset=true -> unknown; VERSIONED_INTERPRETER covers none of the 28 families. Trigger: a Node project adds tsx (a natural entry) so tsx --version stops prompting; the agent then runs tsx ./build.sh from the untrusted repo under investigation — read-only, unattended, arbitrary writes. Six review rounds have now each produced a fresh batch of missed names; the author's recorded position is that the list is a floor and whether to refuse unknown roots entirely is a maintainer product decision — that decision is still outstanding, and until it is made or the surface is structurally closed this finding stands. Fix: close the class structurally — generate the refusal list from an authoritative enumeration with a per-entry regression battery, or refuse any root whose read-only-ness cannot be established from structure (the way env/sudo are unwrapped); minimum if deferred: add the 28 demonstrated names and extend the family-enumeration pins (REFUSAL_FLOOR forces the pairing). (Inline placement at shellAstParser.ts:162 was dropped as a location overlap with comments 3837407390/3838200967 — the round-3/4 postings of this same class under its then-id R2-1.)

[Critical] R8-5: A planted [alias] x = "!cmd" turns an argv-clean vouched wrapper verb into unattended code execution. The planted-config gate this PR extends to vouched wrappers (shellAstParser.ts:1579-1581) models only diff.external/core.fsmonitor; a hostile checkout's [alias] pwn = "!./evil.sh" is invisible to the probe, git executes !-aliases through the shell, and vouchedRootIsSafe passes pwn (a literal word naming no known command) — so gitw pwn classifies read-only and Plan Mode runs it unattended. Literal git never reaches this channel: evaluateGitSafety returns unknown for any sub-command outside READ_ONLY_GIT_SUBCOMMANDS — the filter the wrapper surface drops. Git refuses aliases that shadow built-ins, so the exposure is confined to non-builtin alias verbs — exactly the filter-less wrapper surface this PR adds; pre-PR gitw pwn was unknown and prompted. Witness (probed end-to-end at this commit with real classifier + real git): vouched gitw pwn in planted repo -> read-only; unvouched -> unknown; literal git pwn -> unknown; executing gitw pwn: status=0, marker-exists=true (alias payload executed); the candidate fix flips the vouched classification to unknown. Bounded sibling of the R7-5 family, enumerated separately. Fix: widen the risk probe to ^alias\. (and ^gpg\.program$) and report risk when any local/worktree-scoped alias value begins with ! — or refuse the vouch whenever the repo's local config contains any ! shell alias. (Inline placement at shellAstParser.ts:1580 was dropped as a location overlap with comment 3841589716 — R7-1, a different finding.)

中文说明

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

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

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

Test Plan（非阻断）：src/utils/shellAstParser.test.ts — no such file or directory; src/config/config.test.ts — no such file or directory; src/core/plan-mode-shell-policy.test.ts — no such file or directory; src/tools/shell.test.ts — no such file or directory; src/tools/monitor.test.ts — no such file or directory; and 2 more。

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

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

[Critical] R8-2: The confirmation-scope filter in getConfirmationDetails (packages/core/src/tools/shell.ts:2129-2132; same shape packages/core/src/tools/monitor.ts:226-229) classifies each sub-command alone against the original cwd and has no model of state planted by a preceding sub-command, so a vouched root whose danger depends on that state is excluded from the displayed confirmation scope. Two demonstrated vectors: (1) cd /hostile && gitw status && curl example.com — after the cd, gitw status is probed at the clean original cwd, passes the planted-config gate and is dropped, so the dialog asks only about curl; (2) export GIT_DIR=/x/.git GIT_WORK_TREE=/x && gitw status — the export stays confirmable while gitw status is dropped the same way (a cd-tracking fix does not close this vector). Approval then executes gitw status inside the hostile repo, whose planted core.fsmonitor executes attacker code — the exact attack the planted-config gate this PR extends exists to prevent; the changed-directory fail-closed branch this PR added fires only when cd and the vouched root stay inside one classified string. Witness (probed end-to-end at this commit): SCOPE VOUCHED: defaultPermission=ask rootCommand="export" permissionRules=[] (gitw dropped) vs SCOPE UNVOUCHED: rootCommand="export, gitw"; FSMONITOR EXECUTED: marker file created by git status via env-planted GIT_DIR. Pre-PR, gitw status was always unknown and always in scope; the identical hiding pre-exists for built-in read-only commands, but the diff line is what makes the wrapper form newly reachable. Fix: mirror PermissionManager.evaluateCompoundCommand — if any sub matches the existing cd/pushd test or is an environment/state planter (export, declare, readonly, typeset, local), keep all later subs confirmable. (Inline placement at shell.ts:2131 was dropped as a location overlap with the existing R1-5 comments 3836659886/3836948926 — those report missing tests at this call site; this is the defect such tests would catch.)

[Critical] R6-1 (still stands — re-posted under its original id): the vouch refusal surface remains an unbounded enumeration of payload-executing programs. The round-7 minimum fix landed — all seven names demonstrated in round 7 (just, rake, tox, dotnet, pipx, bazel, task) are now in the 155-entry NEVER_READ_ONLY_ROOT_COMMANDS, verified at this commit — but the class mechanism still fires: this round demonstrated 28 fresh same-family names absent from both refusal mechanisms — ash, mksh, osh, yash, posh (shell families beside the listed bash/sh/dash/ksh/zsh), jshell, scala, groovy, tsx, ts-node, swift, racket (interpreters/runners), dart, julia, kotlin, elixir, escript, rscript, ocaml, guile, crystal, nim, clojure, ghc, runghc, tcc, zig, dmd (language toolchains). Witness (sweep at HEAD): all 28 report inNEVERset=false and vouched <name> ./evil.<ext> = read-only; controls bash/python3/ruby/make report inNEVERset=true -> unknown; VERSIONED_INTERPRETER covers none of the 28 families. Trigger: a Node project adds tsx (a natural entry) so tsx --version stops prompting; the agent then runs tsx ./build.sh from the untrusted repo under investigation — read-only, unattended, arbitrary writes. Six review rounds have now each produced a fresh batch of missed names; the author's recorded position is that the list is a floor and whether to refuse unknown roots entirely is a maintainer product decision — that decision is still outstanding, and until it is made or the surface is structurally closed this finding stands. Fix: close the class structurally — generate the refusal list from an authoritative enumeration with a per-entry regression battery, or refuse any root whose read-only-ness cannot be established from structure (the way env/sudo are unwrapped); minimum if deferred: add the 28 demonstrated names and extend the family-enumeration pins (REFUSAL_FLOOR forces the pairing). (Inline placement at shellAstParser.ts:162 was dropped as a location overlap with comments 3837407390/3838200967 — the round-3/4 postings of this same class under its then-id R2-1.)

[Critical] R8-5: A planted [alias] x = "!cmd" turns an argv-clean vouched wrapper verb into unattended code execution. The planted-config gate this PR extends to vouched wrappers (shellAstParser.ts:1579-1581) models only diff.external/core.fsmonitor; a hostile checkout's [alias] pwn = "!./evil.sh" is invisible to the probe, git executes !-aliases through the shell, and vouchedRootIsSafe passes pwn (a literal word naming no known command) — so gitw pwn classifies read-only and Plan Mode runs it unattended. Literal git never reaches this channel: evaluateGitSafety returns unknown for any sub-command outside READ_ONLY_GIT_SUBCOMMANDS — the filter the wrapper surface drops. Git refuses aliases that shadow built-ins, so the exposure is confined to non-builtin alias verbs — exactly the filter-less wrapper surface this PR adds; pre-PR gitw pwn was unknown and prompted. Witness (probed end-to-end at this commit with real classifier + real git): vouched gitw pwn in planted repo -> read-only; unvouched -> unknown; literal git pwn -> unknown; executing gitw pwn: status=0, marker-exists=true (alias payload executed); the candidate fix flips the vouched classification to unknown. Bounded sibling of the R7-5 family, enumerated separately. Fix: widen the risk probe to ^alias\. (and ^gpg\.program$) and report risk when any local/worktree-scoped alias value begins with ! — or refuse the vouch whenever the repo's local config contains any ! shell alias. (Inline placement at shellAstParser.ts:1580 was dropped as a location overlap with comment 3841589716 — R7-1, a different finding.)

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

[TianYuan1024]

Round 8 fixed in 09a3a53c99, with main merged on top (8bcfc5275d).

R8-3/R8-4 are the finding I should have anticipated two rounds ago, and the diagnosis matters more than the patch. In round 6 I decided a vouched root is a possible git frontend and gave it git's planted-config gate. In round 7 I bolted two option regexes onto that. What I never did was ask the obvious follow-up: if this thing is a git frontend, why is it not going through evaluateGitSafety? Every finding in the R7-1/2/3 → R7-5 → R8-3 → R8-4 chain is the same omission surfacing through a different argument. That is the shared root cause your convergence note has been pointing at for this cluster.

So the fix is not another regex. When the first non-flag argument is a git verb, the whole invocation now goes through git's own evaluator and has to come back read-only — write verbs, the branch -D flag arm, --output, the %G… signature formats, all of it, for free and already tested. Screening every vouched root that way would refuse every CLI whose verbs are not git's, which is why the screen is keyed to the git-shaped case. gitw status, gitw diff, gitw log --oneline stay read-only and are pinned; the cost is a prompt when a vouched CLI's own verb collides with git's (ib add, ib tag), and that is in the docs.

One row of your witness list I did not flip: gitw diff -o f stays read-only, because literal git diff -o f does too — the short form is missing from evaluateGitSafety's --output arm, which predates this PR. Matching literal git is the whole point of the fix, so I did not special-case the wrapper to be stricter than the thing it wraps. Flagging it as a real pre-existing gap rather than leaving it silent.

R8-5 and R7-5's config-key half. You were right and my round-7 reasoning for declining was too broad. I refused the whole key-set extension because git lfs install --local writes filter.lfs.clean, and keying literal git diff to it would downgrade a large share of real checkouts. But that objection only applies to literal git. The wrapper path is new in this PR and has no sub-command filter, so it is exactly where the extra keys bite. getLocalGitConfigRisk now reports textconv drivers, clean/smudge filters, gpg.program and !-aliases as a third flag, consumed on the vouched path only. Literal git keeps its two original checks, and the git-lfs case is pinned as a test so this cannot quietly drift into it later. The literal-git half still wants its own PR with the lfs question settled.

R8-1 is the same lesson as R8-3 in miniature: my round-7 fix enumerated the shapes your witness list happened to contain, and you found two more that need no vouch at all. Inverted to a skip-list of inert redirect leaves — everything else goes to evaluateStatementSafety, whose default arm floors an unknown type at unknown. An unanticipated shape now prompts instead of vanishing. (vtool <<EOF; rm -rf build lands on unknown rather than write; still a refusal, pinned at its real category.)

R8-2 — good catch on the second vector. A cd-tracking fix really would not have closed export GIT_DIR=…, so both call sites now stop dropping sub-commands once any earlier one has planted state, which is what evaluateCompoundCommand already does. monitor.test.ts mocks the parser module wholesale, so the new predicate is mocked with the real regex rather than a stub — a stub returning false would hide the defect the test exists to catch.

R6-1. 28 names added, floor at 183, REFUSAL_FLOOR regenerated. Position unchanged and still not claimed closed.

Verification: full packages/core 21,761 passed before the merge, the one failure the pre-existing extensionManager case; the six affected suites re-run green after it (2,194). shellAstParser.test.ts at 881. Typecheck and lint clean.

[TianYuan1024]

Superseded by #9948 — same change, squashed into one commit, with a description rewritten to match what actually ships rather than the first draft.

Closing this one because eight review rounds have made the thread unreadable, not because anything here is unresolved. Carrying forward:

• All 45 findings from rounds 1–6, all 7 from round 7, and all 5 from round 8 are in the new branch. I re-ran every witness against the squashed tree before opening it.
• R6-1 remains open by its own terms, and the new PR description says so in the body rather than burying it in a comment thread: the refusal floor (183 entries) is documented as a floor under user error, not a boundary, with the reasoning for why enumeration cannot close it. The maintainer decision the last two rounds have been waiting on — whether the setting should refuse to load roots it cannot verify — is stated as an open product question I am happy to implement either way.
• The two fixes that are not about this setting (statements nested in heredoc redirects; confirmation scope after a state planter) are called out as their own section, since both affect built-in roots today.
• Round 7's deferred note that this description contradicted shipped behaviour is fixed: the scope-union and byte-for-byte-inert claims were both stale and are gone.

Round 8's remaining deferred items are recorded and not lost; I will take them on the new PR.