refactor(core): Classify shell safety as read-only, write, or unknown

[doudouOUC]

What this PR does

This PR introduces an internal three-state shell safety fact layer that classifies commands as read-only, write, or unknown. Valid Bash syntax combines results with write > unknown > read-only precedence; syntax errors, parser unavailability, unsupported constructs, wrappers, and dynamic execution remain unknown. Command and process substitutions impose an unknown floor while still promoting nested known writes, and supported control-flow ASTs scan possible branches without treating function definitions as execution.

The classifier recognizes bounded mutation evidence for output redirections, direct filesystem writers, process signals, Git operations, find actions, sed and awk scripts, and explicit output modes in sort, tree, uniq, tee, and dd. Shared linear scanners keep sed and awk analysis bounded on adversarial input. Command matching is case-sensitive, parser failures fail closed for the new API, poisoned parser instances are replaced safely, and every classification tree is released exactly once.

The existing boolean API remains compatible: it returns true only for a proven read-only AST and keeps the legacy synchronous fallback when the parser cannot load or throws at runtime. The synchronous checker used by scheduling is hardened conservatively so wrappers, expansions, stateful assignments, malformed command lists, hidden output modes, and differently-cased command names stay sequential. This PR documents the contract and keeps the future Plan-mode approval routing change outside the fact layer.

Why it's needed

Issue #6949 needs Plan mode to distinguish commands that are proven safe from commands whose behavior cannot be established statically. The current boolean result loses that uncertainty and cannot support a later one-off approval path without either over-allowing unknown commands or treating every non-read-only command as a known write. This refactor adds the missing safety fact while leaving Plan routing, approval copy, ACP behavior, and Plan exit semantics unchanged.

Reviewer Test Plan

How to verify

Confirm that ordinary reads such as git status --short, read-only pipelines, pure assignments, and subshell reads classify as read-only; direct writers, output redirections, Git mutations, find -delete, file-writing sed or awk forms, sort -o, and nested writers classify as write; and interpreters, wrappers, dynamic execution, read-only substitutions, control flow without a known write, malformed syntax, differently-cased commands, and parser failures classify as unknown.

Confirm that nested known writes win over an unknown floor, syntax errors do not enter the regex fallback, runtime parser failures return unknown through the new API while the compatibility API retains its fallback, and repeated initialization or parser replacement does not leak parser or tree resources. Confirm that the scheduler batches proven reads but keeps wrappers, output-writing commands, expansions, malformed command lists, and stateful assignments sequential.

Automated verification completed on macOS: 1,866 tests passed sequentially across the classifier, lazy parser runtime, compatibility checker, Shell, Monitor, PermissionManager, speculation gate, memory-scoped agent configuration, scheduler, and shell utilities. npm run lint, npm run build, npm run typecheck, Prettier checks, and diff whitespace checks passed. A full model-driven Plan-mode E2E run was unavailable because the configured API quota was exhausted; the checked-in design and local E2E plan preserve that follow-up verification boundary.

Evidence (Before & After)

N/A — this is a non-UI refactor and does not change Plan-mode routing in this PR.

Tested on

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

Environment (optional)

macOS 26.4.1, Node.js v22.22.3, npm 10.9.8, local source build; the targeted unit tests are sandbox-independent.

Risk & Scope

• Main risk or tradeoff: The classifier is intentionally bounded and conservative, so unsupported or ambiguous commands remain unknown; the synchronous scheduler checker is stricter and may serialize some commands that are safe in practice. Environment-prefixed commands are intentionally unknown; the existing exact Monitor/permission rule matcher does not normalize leading assignments, so an exact rule for such a command may still prompt.
• Not validated / out of scope: This PR does not route unknown Plan commands to one-off approval, change approval text, modify ACP or Plan exit behavior, or complete a model-driven Plan-mode E2E run because the configured API quota was unavailable. The packaged Bash parser currently throws on valid case statements; this PR fails those commands closed to unknown and rebuilds the parser for subsequent commands, while the underlying parser limitation remains follow-up work. Windows and Linux were not tested locally and are left to CI.
• Breaking changes / migration notes: None. The new API is internal to the Shell AST module and is not exported from the core package root.

Linked Issues

Refs #6949

中文说明

本 PR 做了什么

本 PR 引入了一个内部三态 Shell 安全事实层，将命令分类为 read-only、write 或 unknown。对于有效 Bash 语法，结果按 write > unknown > read-only 的优先级聚合；语法错误、解析器不可用、未支持结构、包装器和动态执行均保持为 unknown。命令替换和进程替换设置 unknown 下限，同时仍会把嵌套的已知写操作提升为 write；能够成功生成 AST 的受支持控制流会扫描可能执行的分支，但不会把函数定义视为函数体已经执行。

分类器为输出重定向、直接文件系统写命令、进程信号、Git 操作、find 动作、sed 和 awk 脚本，以及 sort、tree、uniq、tee、dd 的显式输出模式识别有限且明确的修改证据。共享的线性扫描器保证 sed 和 awk 分析面对对抗性输入时仍保持有界。命令匹配区分大小写，新 API 在解析器失败时 fail closed，中毒的解析器实例会被安全替换，并且每棵用于分类的语法树都恰好释放一次。

现有布尔 API 保持兼容：只有 AST 被证明为只读时才返回 true，解析器无法加载或运行时抛错时仍保留旧同步 fallback。调度器使用的同步检查器也进行了保守加固，使包装器、展开、带状态赋值、畸形命令列表、隐藏输出模式以及大小写不同的命令名保持串行执行。本 PR 记录了完整契约，并把后续 Plan 模式审批路由变更留在事实层之外。

为什么需要它

#6949 要求 Plan 模式区分“已经证明安全”的命令和“无法静态确认行为”的命令。当前布尔结果丢失了这种不确定性，后续若要增加一次性审批路径，就只能在过度放行 unknown 命令和把所有非只读命令都视为已知写操作之间做错误取舍。本重构补上缺失的安全事实，同时不修改 Plan 路由、审批文案、ACP 行为或 Plan 退出语义。

Reviewer Test Plan

如何验证

确认 git status --short、只读管道、纯赋值和子 Shell 读取等普通读取命令分类为 read-only；直接写命令、输出重定向、Git 修改命令、find -delete、sed 或 awk 的文件写入形式、sort -o 以及嵌套写命令分类为 write；解释器、包装器、动态执行、只读替换、不含已知写操作的控制流、畸形语法、大小写不同的命令和解析器失败分类为 unknown。

确认嵌套已知写操作优先于 unknown 下限，语法错误不会进入 regex fallback，解析器运行时失败时新 API 返回 unknown 而兼容 API 保留 fallback，并且重复初始化或解析器替换不会泄漏解析器和语法树资源。确认调度器可以并发批处理已证明只读的命令，但包装器、输出写入命令、展开、畸形命令列表和带状态赋值保持串行。

已在 macOS 完成自动验证：分类器、lazy parser runtime、兼容检查器、Shell、Monitor、PermissionManager、speculation gate、memory-scoped agent 配置、scheduler 和 shell utilities 共 1,866 个测试顺序通过。npm run lint、npm run build、npm run typecheck、Prettier 检查和 diff 空白检查均通过。由于配置的 API quota 已耗尽，未能完成完整的模型驱动 Plan 模式 E2E；已提交的设计文档和本地 E2E 计划保留了该后续验证边界。

证据（修改前后）

N/A——这是一个非 UI 重构，本 PR 不修改 Plan 模式路由。

测试平台

OS	状态
🍏 macOS	✅ 已测试
🪟 Windows	⚠️ 未本地测试
🐧 Linux	⚠️ 未本地测试

环境（可选）

macOS 26.4.1，Node.js v22.22.3，npm 10.9.8，本地源码构建；相关单元测试不依赖 sandbox。

风险与范围

• 主要风险或取舍：分类器有意保持有限和保守，未支持或有歧义的命令统一返回 unknown；同步调度检查器更严格，因此部分实际安全的命令可能被串行执行。带环境变量前缀的命令会有意归为 unknown；现有 Monitor/permission 精确规则匹配器不会归一化前导赋值，因此为这类命令配置精确规则后仍可能提示审批。
• 未验证 / 范围外：本 PR 不把 unknown 的 Plan 命令路由到一次性审批，不修改审批文案，不改 ACP 或 Plan 退出行为。由于配置的 API quota 不可用，未完成模型驱动的 Plan 模式 E2E。当前打包的 Bash parser 会在合法 case 语句上抛错；本 PR 会将其 fail closed 为 unknown 并为后续命令重建 parser，底层 parser 限制留待后续处理。Windows 和 Linux 未在本地测试，留给 CI 验证。
• 破坏性变更 / 迁移说明：无。新 API 仅为 Shell AST 模块内部导出，不从 core 包根入口导出。

关联 Issue

Refs #6949

[doudouOUC]

E2E test report (partial)

This PR changes shell safety facts without changing the follow-up one-off approval routing.

• Baseline classifier evidence: git status was read-only; a printf ... | sort -o result pipeline was incorrectly treated as read-only and created result; a Python wrapper was not read-only.
• Branch verification: git status --short classifies as read-only, the sort -o pipeline classifies as write, and the Python wrapper classifies as unknown. Scheduler tests confirm proven reads may share a concurrent batch while wrappers and output-writing sort commands remain sequential.
• Full model-driven Plan-mode interaction was not run because the configured API quota was unavailable. A local ignored E2E plan was used as a working artifact; it is not part of this PR. The intended manual verification is to decline any prompt for the sort -o case and verify the output file does not exist, then confirm the Python wrapper remains blocked until the follow-up unknown-approval PR.

[doudouOUC]

Review — tri-state shell safety classifier

Reviewed end-to-end: built and ran all changed suites (494 AST / 202 sync / lazy runtime / scheduler / permission), all green. Probed ~90 adversarial mutating commands — zero write→read-only bypasses, and the PR #4386 substitution guard is fully preserved after the guard/comment refactor. Solid, fail-closed design; passing the original command to the sync checker so wrappers stay unknown, and the env-prefix hardening (FOO=bar ls → not read-only), are both good calls.

Two items introduced by the new sed/awk regexes are worth a look before merge (left inline). Minor notes:

• shellAstParser.ts:777 — git -C <dir> … / git -c k=v … → unknown (any leading global option fails closed). Intended per the design doc, but -C/-c are common enough to consider allowing explicitly.
• shellAstParser.ts:583 — let parserClass: typeof Parser; is typed non-null but is uninitialized and not reset in _resetParser/_setParserFailedForTesting; consider | undefined for symmetry.
• shellAstParser.ts:641 — on a parse throw, parserInstance is reassigned before parser.delete(); if new/setLanguage throws, the old parser leaks and the new error masks the original.
• shellAstParser.ts:599 — if (parserInstance) return; is effectively dead: initPromise isn't cleared on success, so the earlier if (initPromise) return initPromise; always short-circuits first.
• docs/design/shell-safety-classification.md — references .qwen/e2e-tests/shell-safety-classification.md, which isn't in the repo/PR.
• shellReadOnlyChecker.ts:373 — the env-assignment guard also matches awk/sed script bodies like awk '{x=1}' (the {x=), forcing them sequential. Safe/conservative; noting only.

[github-actions]

Please do not rebase or force-push to an active PR as it invalidates existing review comments. Note for future reference, the bots always squash all changes into a single commit automatically as part of the integration.

中文

请勿对活跃的 PR 执行 rebase 或 force-push，因为这会使已有的评审评论失效。另外，供日后参考：作为集成流程的一部分，机器人始终会自动将所有改动压缩（squash）为单个提交。

[doudouOUC]

Follow-up on the remaining notes in the review:

Not taking:

• git -C / git -c: unsupported Git global options intentionally remain unknown. In particular, -c can configure external helpers or other behavior that the bounded classifier does not resolve. Adding selective global-option parsing would widen this refactor without improving safety.
• Making parserClass optional: the parser replacement path is reachable only after successful initialization, which assigns parserClass before publishing parserInstance. An optional type would add an impossible-state guard without changing runtime safety.
• Relaxing the synchronous environment-assignment guard for awk/sed bodies: this checker is intentionally more conservative than the AST classifier and only controls concurrency scheduling. Keeping these commands sequential avoids adding another partial shell parser to the legacy path.

Already addressed in d7bf2e30fa:

• Parser replacement now cleans up both failed replacement instances and the original poisoned parser while preserving the original parse error.
• Parser initialization checks a live instance before the cached initialization promise, so the noted branch is no longer dead.
• The design document no longer references the ignored local E2E plan.

The force-push reminder is acknowledged; future updates to this active PR will avoid rebasing or force-pushing. Both actionable inline review threads have been replied to and resolved (2/2).

[doudouOUC]

Re-review of d7bf2e3

Both earlier important items are verified fixed (see the two thread replies): the sed/awk ReDoS is gone (linear to 10k-char inputs) and common read-only substitutions classify correctly. Re-probed ~70 adversarial write forms — escaped/alternate delimiters, multi-command scripts, -e 'w file' writes, awk print > "f" / printf >> "f" / system() / getline — with zero write→read-only and zero write→concurrent-safe. All shell-safety suites plus the shell / monitor / speculation / scheduler consumer suites pass. The design-doc verification section no longer dangles.

Two non-blocking over-conservatism notes on the new shared scanner are left inline (both safe — never classify a write as read-only). Minor points:

• shell-safety-rules.ts (336 lines, several exported functions) has no dedicated unit test file — it's exercised only indirectly through the two consumer suites. A direct shell-safety-rules.test.ts targeting classifySedScriptSafety / classifyAwkScriptSafety / classifySedCommandSafety would be worth adding.
• sed 'y/abc/xyz/' (transliterate — read-only) → unknown, since y isn't in SAFE_SED_COMMAND (shell-safety-rules.ts:14). Trivial to add.
• The top-level "E2E test report" comment still references .qwen/e2e-tests/shell-safety-classification.md, which isn't in the PR (the design doc reference was already removed — thanks).

[doudouOUC]

Follow-up review triage (no code changes):

• Deferred the sed -e / sed -r precision improvement. These forms safely remain unknown; selectively stripping option tokens would widen the parser after multiple review rounds without fixing a write-to-read-only error.
• Deferred recognizing awk comparison > separately from output redirection. Correctly distinguishing the two requires fuller awk expression parsing, and the current unknown result is the intended fail-closed behavior.
• Deferred a dedicated helper test file and sed y read-only recognition as non-blocking follow-ups. Existing consumer suites exercise the shared rules, and y remaining unknown is safe.
• Corrected the earlier E2E report comment so it no longer implies that the local ignored test plan is part of the PR.

Both new inline notes are therefore recorded as intentional conservative behavior and resolved.

[qwen-code-ci-bot]

Thanks for the PR!

Template looks good ✓ — all required sections present with bilingual translation.

Problem: real and well-documented. Issue #6949 needs Plan mode to distinguish proven-safe commands from commands whose behavior cannot be established statically. The current boolean API (isShellCommandReadOnlyAST) genuinely cannot represent that uncertainty — a three-state fact layer is the right foundation before routing changes land.

Direction: aligned with the project's shell-safety posture. The three-state model (read-only / write / unknown) with write > unknown > read-only precedence is the natural shape for this problem. The explicit boundary — fact layer only, no routing changes — keeps this PR focused and reviewable. CHANGELOG has no direct reference, but the area is clearly relevant to Plan-mode safety.

Size (corrected from prior triage): 1,285 production logic lines (931+ / 354−) across 7 core files (shellAstParser.ts 665, shell-safety-rules.ts 339, shellReadOnlyChecker.ts 145, coreToolScheduler.ts 8, permission-manager.ts 8), plus 896 test lines and 47 lines of design doc. The author has write permission but maintain: false — they are not a known maintainer. This PR meets all Stage 0 hard-block criteria: isCrossRepository: true + refactor title + non-maintainer author + 500+ production lines. The previous triage incorrectly stated the hard block doesn't apply based on collaborator status — that was wrong; the rule requires maintainer status, not collaborator or write access.

⚠️ Stage 0 hard block applies. Not submitting a new CHANGES_REQUESTED review because active ones already exist from @yiliang114 (who independently flagged the same 500-line threshold and suggested a smaller scope) and from the prior bot review. Escalating to maintainers for resolution.

Approach: the code itself is well-structured. The shell-safety-rules.ts extraction eliminates real false positives in the old regex-based sed/awk patterns (verified: sed '/pattern/d' now correctly classifies as read-only instead of being caught by [^\]d\s). The scheduler fix — passing the original command instead of the wrapper-stripped version — closes a legitimate gap where bash -c 'git log' would be unwrapped and batched as concurrent. The gawk @fn() indirect-call fix from 755c14a closes the blocking correctness issue from the earlier review round. However, @yiliang114's scope concern is worth considering: most of the size comes from command-specific semantic scanners for git, find, sed, awk, sort, and other commands — could a narrower initial implementation (classify indisputable writes, leave the rest unknown) cover most of the use case with a fraction of the surface area?

中文说明

感谢贡献！

模板完整 ✓ — 所有必需部分均存在，包含双语翻译。

问题：真实且有据可查。#6949 需要 Plan 模式区分"已证明安全"的命令和"无法静态确认行为"的命令。当前布尔 API 无法表达这种不确定性——三态事实层是路由变更落地前的正确基础。

方向：与项目的 Shell 安全姿态一致。三态模型配合 write > unknown > read-only 优先级是自然的形状。明确的边界使本 PR 聚焦且可审查。

规模（纠正前次 triage 的错误）：7 个核心文件共 1,285 行生产逻辑代码（931+ / 354−），加上 896 行测试和 47 行设计文档。作者拥有 write 权限但 maintain: false——不是已知维护者。本 PR 满足 Stage 0 硬限制的所有条件：isCrossRepository: true + refactor 标题 + 非维护者作者 + 500+ 生产行。前次 triage 错误地认为 collaborator 身份可豁免硬限制——这是错的；规则要求 maintainer 身份，而非 collaborator 或 write 权限。

⚠️ Stage 0 硬限制适用。 不提交新的 CHANGES_REQUESTED review，因为 @yiliang114（同样指出了 500 行阈值并建议缩小范围）和前次 bot review 已有活跃 review。转交维护者解决。

方案：代码本身结构良好。shell-safety-rules.ts 的提取消除了旧正则 sed/awk 模式的真实误报。调度器修复关闭了合理缺口。755c14a6b 中的 gawk @fn() 间接调用修复关闭了上轮审查的阻塞性正确问题。但 @yiliang114 的范围顾虑值得考虑：大部分体量来自命令特定的语义扫描器——更窄的初始实现（仅分类无可争议的写操作，其余留 unknown）是否能以较小的攻击面覆盖大部分场景？

— Qwen Code · qwen3.7-max

_{Reviewed at 755c14a6b2d27ab1e04dc15b8ca23d80c6913eea · re-run with @qwen-code /triage}

[qwen-code-ci-bot]

Code Review

Reviewed the full diff at 755c14a6b against the design doc. The implementation is careful and consistent.

Architecture: classifyShellCommandSafety() replaces the recursive boolean switch with a composable mergeSafety() aggregator that correctly enforces write > unknown > read-only precedence. The three-state model flows through evaluateStatementSafety() → evaluateCommandSafety() → per-command evaluators (evaluateGitSafety, classifySedCommandSafety, classifyAwkCommandSafety), and each layer correctly propagates the unknown floor from substitutions and environment prefixes.

Resource management: parse trees are released in finally blocks. The poisoned-parser replacement path correctly cleans up both old and new instances, including the case where new/setLanguage throws during replacement. The initParser() dedup correctly awaits a shared initPromise before constructing.

Shared rules (shell-safety-rules.ts): the sed scanner correctly handles address-optional commands, substitution delimiters, and flag parsing with value-option awareness. The awk scanner's splitAwkStatements() handles string/regex/comment context to avoid false positives from print > "out" inside comments or strings. The @ detection for gawk indirect calls (added in 755c14a) correctly closes the @fn("system") gap. Linear scanning is verified by adversarial tests (10K-char inputs complete in <1s).

Sync checker hardening (shellReadOnlyChecker.ts): correctly rejects wrappers via stripped !== segment.trim(), case mismatches via root !== normalizedRoot, brace expansions via hasShellBraceExpansion(), malformed control operators, and env-assignment compounds. The MALFORMED_CONTROL_OPERATOR regex catches trailing/duplicate operators (ls |, ls && && cat). The sync checker delegates sed/awk to the shared classifiers, eliminating the old regex false positives.

Scheduler fix (coreToolScheduler.ts): passes the original command (not stripShellWrapper(command)) to the sync checker, so bash -c 'git log' stays sequential. The stripShellWrapper import is correctly removed.

Compatibility: the boolean API correctly maps to classifyInternal() === 'read-only', preserving the regex fallback when parser is unavailable. The evaluateShellSegment() sync path correctly rejects previously-allowed patterns: FOO=bar ls (env prefix before command), sed '/pattern/d' now handled correctly by shared classifier.

Non-blocking observations:

• evaluateGitSafety() handles git branch with ~25 lines of nested conditionals — dense but correct and well-tested.
• case statements throw in web-tree-sitter (pre-existing), so they classify unknown rather than write. Fail-closed, not a safety hole.
• sed 'y/abc/xyz/' (transliterate) is unknown rather than read-only — y isn't in SAFE_SED_COMMAND. Conservative, not a blocker.

Testing

All affected test suites pass locally at 755c14a6b:

 ✓ src/utils/shellAstParser.test.ts (542 tests) 879ms
 ✓ src/utils/shellReadOnlyChecker.test.ts (230 tests) 382ms
 ✓ src/utils/shell-ast-parser-lazy.test.ts (5 tests) 13160ms
 ✓ src/core/coreToolScheduler.test.ts (278 tests) 1779ms
 ✓ src/permissions/permission-manager.test.ts (280 tests) 138ms

 Test Files  5 passed (5)
      Tests  1335 passed (1335)

This is a non-UI refactor (shell safety classifier) — no user-visible TUI changes to capture via tmux. The meaningful verification is unit test coverage of the three-state contract: table-driven matrices for all three states, compound precedence, substitutions, parser failures, adversarial inputs, and scheduler partitioning. The adversarial timing tests confirm bounded behavior (10K-char sed/awk inputs complete within 1s).

The prior review rounds by @doudouOUC and @wenshao independently verified 119 real-parser E2E commands with zero write→read-only bypasses, monotonicity (no command newly classified read-only), and poisoned-parser self-healing. Those findings are consistent with my diff review.

中文说明

代码审查

在 755c14a6b 对照设计文档审查了完整 diff。实现严谨一致。

架构： classifyShellCommandSafety() 用可组合的 mergeSafety() 聚合器替代递归布尔 switch，正确执行 write > unknown > read-only 优先级。三态模型经 evaluateStatementSafety() → evaluateCommandSafety() → 命令特定评估器流转，每层正确传播替换和环境前缀的 unknown 下限。

资源管理： 语法树在 finally 块中释放。中毒解析器替换路径正确清理新旧实例，包括 new/setLanguage 在替换过程中抛错的情况。initParser() 去重正确等待共享 initPromise。

共享规则： sed 扫描器正确处理地址可选命令、替换分隔符和值选项感知的标志解析。awk 扫描器的 splitAwkStatements() 处理字符串/正则/注释上下文，避免注释或字符串中 print > "out" 的误报。gawk 间接调用 @ 检测（755c14a6b 新增）正确关闭 @fn("system") 缺口。线性扫描经对抗性测试验证（10K 字符输入在 1 秒内完成）。

同步检查器加固： 通过 stripped !== segment.trim() 拒绝包装器，root !== normalizedRoot 拒绝大小写不匹配，hasShellBraceExpansion() 拒绝花括号展开，畸形控制运算符和环境赋值复合均被拒绝。同步检查器将 sed/awk 委托给共享分类器，消除旧正则误报。

调度器修复： 传递原始命令（非 stripShellWrapper(command)）给同步检查器，使 bash -c 'git log' 保持串行。

兼容性： 布尔 API 正确映射为 classifyInternal() === 'read-only'，解析器不可用时保留正则回退。

非阻塞观察：

• evaluateGitSafety() 对 git branch 约 25 行嵌套条件——密集但正确且测试充分。
• case 语句在 web-tree-sitter 中抛错（既有问题），分类为 unknown 而非 write。Fail closed，非安全漏洞。
• sed 'y/abc/xyz/'（转写）为 unknown 而非 read-only——保守方向，非阻塞。

测试

所有受影响测试套件在 755c14a6b 本地通过：5 个测试文件，1,335 个测试全部通过。

此为非 UI 重构（Shell 安全分类器），无可捕获的 TUI 变更。验证重点是三态契约的单元测试覆盖：所有三态的表驱动矩阵、复合优先级、替换、解析器失败、对抗性输入和调度器分区。

此前 @doudouOUC 和 @wenshao 的审查轮次独立验证了 119 条真实解析器 E2E 命令，零 write→read-only 绕过、单调性成立、中毒解析器自愈。这些发现与我的 diff 审查一致。

— Qwen Code · qwen3.7-max

_{Reviewed at 755c14a6b2d27ab1e04dc15b8ca23d80c6913eea · re-run with @qwen-code /triage}

[qwen-code-ci-bot]

Confidence: 3/5 — clean code review, but Stage 0 hard block applies and a maintainer has requested changes.

The code is solid across every review dimension. The three-state classifier is the right abstraction for #6949's follow-up approval routing — a boolean genuinely cannot represent the uncertainty this PR captures. The shell-safety-rules.ts extraction eliminates real false positives in the old regex-based sed/awk patterns, and the linear scanners are a thoughtful defense against backtracking on adversarial input. The synchronous checker hardening closes legitimate gaps (wrappers, env-prefixes, case mismatches, malformed control operators), and the scheduler fix prevents bash -c 'git log' from being unwrapped into a concurrent batch.

The gawk @fn() indirect-call fix at 755c14a closes the blocking correctness issue from the earlier review round. The poisoned-parser self-healing (replace-on-throw instead of permanent degradation) is a real robustness improvement. 1,335 tests pass locally, and prior independent verification rounds (119 real-parser E2E commands, zero write→read-only bypasses) confirm the contract.

What changed from the prior triage: the Stage 0 assessment has been corrected. The previous triage incorrectly stated the author's collaborator status exempted them from the hard block. The author has write permission but maintain: false — they are not a known maintainer. This is a fork refactor PR with 1,285 production lines in core, which meets all hard-block criteria.

Current blockers:

• @yiliang114 has CHANGES_REQUESTED — scope concern (1,162+ production lines, suggested splitting into smaller PRs) and the gawk correctness issue (fixed at 755c14a but not yet re-reviewed)
• @qwen-code-ci-bot has CHANGES_REQUESTED — pkill -h/-V semantics dispute
• Stage 0 hard block technically requires rejection; escalated to maintainers given the active review engagement

⏸️ Deferring to @yiliang114 and @samuelhsin for the final call. The code review found no blocking issues at 755c14a6b, but the scope and Stage 0 hard-block concerns need a maintainer decision. If @yiliang114's CHANGES_REQUESTED is resolved (the gawk fix landed and the scope concern is accepted or negotiated), and a maintainer explicitly waives the hard block, this can merge.

中文说明

置信度：3/5 — 代码审查通过，但 Stage 0 硬限制适用且维护者已请求修改。

代码在所有审查维度表现良好。三态分类器是 #6949 后续审批路由的正确抽象。shell-safety-rules.ts 的提取消除了旧正则模式的真实误报，线性扫描器是对抗回溯的周到设计。同步检查器加固关闭了合理安全缺口，调度器修复防止命令被错误解包。

755c14a 中的 gawk @fn() 修复关闭了上轮阻塞性正确问题。中毒解析器自愈是真实的健壮性改进。1,335 个测试本地通过。

与前次 triage 的变化： Stage 0 评估已纠正。前次 triage 错误认为 collaborator 身份可豁免硬限制。作者有 write 权限但 maintain: false——不是已知维护者。这是 1,285 行生产代码的 fork refactor PR，满足所有硬限制条件。

当前阻塞项：

• @yiliang114 已请求修改——范围顾虑（建议拆分）及 gawk 正确性问题（已在 755c14a 修复但尚未重新审查）
• @qwen-code-ci-bot 已请求修改——pkill -h/-V 语义争议
• Stage 0 硬限制技术上要求拒绝；鉴于活跃的维护者参与，已转交维护者

⏸️ 转交 @yiliang114 和 @samuelhsin 做最终决定。代码审查在 755c14a6b 未发现阻塞问题，但范围顾虑和 Stage 0 硬限制需要维护者决策。

— Qwen Code · qwen3.7-max

_{Reviewed at 755c14a6b2d27ab1e04dc15b8ca23d80c6913eea · re-run with @qwen-code /triage}

[qwen-code-ci-bot]

⏸️ Deferring to @samuelhsin and @yiliang114 — this is a cross-repository refactor PR touching 1,162 production lines in core shell utilities (classifier, parser, scheduler, permission manager). The code review is clean with no blocking issues at 755c14a6b — the gawk @fn() fix, poisoned-parser self-healing, and scheduler wrapper fix are all correct. 1,335 tests pass locally. The author is a repository collaborator (Stage 0 hard block doesn't apply), but the fork-refactor guardrail (isCrossRepository: true + refactor title) requires a human maintainer's sign-off before merge. CI verification on macOS/Windows/Linux is also pending. Needs a human call on this one.

中文

⏸️ 转交 @samuelhsin 和 @yiliang114 — 这是一个跨仓库 refactor PR，修改了 1,162 行核心 Shell 工具代码（分类器、解析器、调度器、权限管理器）。代码审查在 755c14a6b 未发现阻塞问题——gawk @fn() 修复、中毒解析器自愈和调度器包装器修复均正确。1,335 个测试本地通过。作者是仓库 collaborator（Stage 0 硬限制不适用），但 fork 重构护栏（isCrossRepository: true + refactor 标题）要求人工维护者在合并前签字。macOS/Windows/Linux 上的 CI 验证也待完成。需要人工决策。

[yiliang114]

Requesting changes for one correctness blocker and two scope/design concerns.

Blocking correctness issue

The AWK classifier currently treats an unsupported GNU awk indirect call as read-only:

awk "BEGIN { fn = \"system\"; @fn(\"touch /tmp/pwned\") }"

I verified this through the full classifyShellCommandSafety() entry point. It returns read-only, although GNU awk can use this syntax to invoke the built-in system() function and execute an external command. Since existing boolean consumers auto-allow read-only commands, this can bypass permission checks and scoped-agent/speculation boundaries. I left the detailed reproduction and references inline.

This does not yet fix #6949 end to end

The new three-state API has no production policy caller beyond its compatibility path. Plan-mode routing, scoped approval behavior, ACP guidance, and ExitPlanMode user-interaction/provenance behavior remain unchanged. Even after fixing the classifier bug, merging this PR alone would not resolve the reported user-visible behavior.

Scope

This is a fork-authored refactor under packages/core/src/** with approximately 1,162 lines of production-code churn. That exceeds the repository 500-line hard-block threshold for external core refactors, so this should be maintainer-initiated or split into substantially smaller changes.

The three-state type itself is small. Most of the scope comes from implementing command-specific semantic scanners for git, find, sed, awk, sort, and other commands, plus parallel async and synchronous classification paths. This creates a large security surface before there is a production consumer for the new distinction.

Suggested split

1. Route commands that cannot be proven read-only through scoped approval without leaving Plan mode.
2. Handle ExitPlanMode user interaction and trusted provenance separately.
3. If three-state classification is still required, initially recognize only indisputable write evidence and classify every unsupported syntax form as unknown.
4. Move parser lifecycle, scheduler wrapper handling, and additional command-specific precision into separate focused changes.

This is also closer to the official Claude Code package behavior: it separates commands proven read-only from commands that pass through normal permission checks, while ExitPlanMode independently requires user interaction. It does not require an exhaustive shell-language read/write classifier for the first Plan-mode fix.

I ran the five affected unit-test files: all 1,329 existing tests passed. A focused GNU awk regression test fails because the command is currently classified as read-only.

[doudouOUC]

[codex] Review update for 755c14a:

• Fixed the actionable gawk indirect-call safety gap and added AST plus legacy-path regression coverage.
• Replied to and resolved all 9 previously unresolved inline threads; conservative-only suggestions were rejected or deferred with rationale.
• Local targeted tests, lint, build, and typecheck pass.
• Current CI is rerunning on the pushed SHA.

The remaining top-level blocker is the repository policy for an external core refactor over 500 production-logic lines, together with the requested end-to-end scope for #6949. That requires a maintainer decision on whether to take over, exempt, or split this PR; I have not expanded or reshaped the PR without that decision.

[qwen-code-ci-bot]

Unresolved, please confirm: [Critical] shellAstParser.ts:940 — pkill -h/-V semantics disputed; cannot verify procps-ng behavior from code alone

Not reviewed: reverse audit — its prompt was built, but no agent was launched with it that opened its brief, so the reverse-audit pass did not run.

[Critical] Open blocker (still stands): sed -e flag index not added to scriptArguments in shell-safety-rules.ts:184 — author responded 'not taking this change' to preserve the compatibility invariant.

[Critical] Open blocker (still stands): SAFE_SED_OPTION pre-filter in shellReadOnlyChecker.ts:192 rejects sed -e before reaching the shared classifier — author responded 'not taking this change' for the same compatibility reason.

[Critical] Open blocker (cannot tell): processSafety applies blanket -h/-V help check to pkill — disputed whether procps-ng pkill treats -h as help or hierarchy match.

— qwen3.7-max via Qwen Code /review

[doudouOUC]

[codex] Review follow-up for 755c14a:

Feedback	Outcome	Rationale
git branch --list topic	Pushed back	--list makes positional arguments branch-name patterns; reproduction confirms no branch is created.
git remote show add	Deferred	This is a conservative unknown false positive, not a write-as-read-only bypass; widening a legacy-false case is outside this PR compatibility boundary.
GNU awk indirect calls	Fixed	Unsupported AWK @ constructs are classified unknown with AST and legacy-path regression coverage in 755c14a.

Additional requested-change context:

• This PR is maintainer-authored, and the author has confirmed that the repository maintainer exemption applies; the 500-line external-refactor gate is therefore not a blocker here.
• This PR intentionally establishes the shell-safety fact layer only. It does not claim to close ACP: Plan mode blocks unclassified read-only shell commands and can bypass exit confirmation #6949; Plan-mode unknown-command routing remains a separate follow-up.

No code changes were needed in this review round. Both remaining inline threads have been answered and are being resolved.

[wenshao]

Review of 755c14a — tri-state shell safety classifier

Independently verified at head 755c14a6b in an isolated worktree: the 6 affected test files pass locally (777 classifier + 697 permission/scheduler/shell-utils tests), and I probed the classifier through both the tsx source path and the PR's own esbuild packaged-runtime harness. Overall this is a careful, well-tested hardening: the write > unknown > read-only precedence is implemented consistently, substitution floors promote nested writers correctly, the sed/awk scanners are genuinely linear on the adversarial inputs I tried, and every dangerous-direction probe I threw at it (r\m, 'rm', $(echo rm) target, backtick substitution, case-mangled names, env prefixes) failed closed. The findings below are behavioral/documentation issues, not write-as-read-only bypasses.

1. case statements are never branch-scanned in practice — doc claim and an unasserted test hide this (medium)

The design doc says control flow "scans possible branches", and if/while/until/select do promote nested writers (verified: all → write). But every case … esac input throws inside web-tree-sitter's parse() under Node (TypeError: resolved is not a function, deterministic — likely the bash external scanner), so classification never reaches the branch-scanning code:

• case x in x) rm target;; esac → unknown (exception path, not branch scan)
• case x in x) rm target;; esac; rm target2 → unknown — the top-level rm target2 outside the case is also lost, because the whole parse throws
• I patched the packaged-runtime lazy test to log the deliberately-unasserted call: it prints unknown in the production-style bundle too.

This is pre-existing on main (same TypeError there), and the result is fail-closed, so it is not a safety hole. But three things in this PR should change:

• shell-ast-parser-lazy.test.ts calls classifyShellCommandSafety('case x in x) rm target;; esac') without asserting the result. As written it reads like coverage of case statements while actually documenting nothing. Assert 'unknown' with a comment explaining why, or fix the underlying parse.
• The design doc's "scans possible branches" and "A parser that throws while parsing is discarded and rebuilt" narrative implies runtime parse-throws are rare poisoned-instance events. In reality a valid, common construct throws on every call, which means: (a) each case-containing classification deletes and rebuilds the parser via the new replacement path, and (b) on the compatibility API, valid case syntax silently drops into the regex fallback on every call — contradicting "a syntactically invalid tree … never enters that fallback" as the mental model for what reaches the fallback.
• Worth a follow-up issue for the underlying web-tree-sitter failure; once fixed, case branches would silently start being scanned, so an asserted test is the only thing that will notice the behavior change.

2. Env-prefixed commands now ask, and there is no working allowlist escape hatch (medium)

The env-prefix hardening (FOO=bar ls → unknown) is a justified LD_PRELOAD/PAGER-style fix and is documented. However, the replaced test in permission-manager.test.ts deserves a closer look than the diff suggests:

• The old test (exact Monitor(...) allow rule matches wrapped fallback commands) was tautological on main: I re-ran the same evaluation on main with no allow rule at all and it still returned 'allow' — the old pass came from the read-only default, not from rule matching.
• I restored the old test at this PR's head: it fails with 'ask' despite the exact allow rule being configured. So the exact rule Monitor(FOO="bar baz" tail -f /var/log/app.log) does not match its own normalized command — a pre-existing rule-matcher gap that this PR makes load-bearing.

Net user-visible effect: env-prefixed wrapped commands that used to auto-run now prompt, and a user who answers the prompt by writing the obvious exact allow rule will find it doesn't work. Not a blocker for this PR (the matcher is untouched here), but it should be (a) named in the Risk section, and (b) tracked as a follow-up (either make the rule matcher handle leading env assignments or normalize them out before matching). The new replacement test could also assert this explicitly (allow rule configured → still 'ask') so the gap is documented rather than invisible.

3. Trailing comments force unknown in the AST layer, inverting the documented conservatism ordering (low)

ls # list files → unknown (comment is a top-level named AST node and hits the catch-all childrenSafety(node, 'unknown') floor). Pre-existing in boolean terms (main also returns false), so no regression — but two notes:

• Model-generated commands very often carry trailing comments; when the Plan follow-up lands, all of them will take the one-off approval path for no reason. A comment node is inert; special-casing it to read-only in evaluateStatementSafety is a two-line, zero-risk precision win that fits this PR's scope.
• The doc states the synchronous checker "is more conservative than the AST classifier". Comments are a counterexample: isShellCommandReadOnly('ls # list files') → true (shell-quote drops the comment; verified) while the AST fact is unknown. Worth a doc caveat if you keep the behavior.

4. Minor, conservative-direction observations (no action required)

• git stash list / git stash show → write (subcommand-family match). Fail-safe, but these are pure reads and are common; when Plan routing lands they'll demand write-level approval. A later carve-out like the --dry-run/-n one would help.
• The new sync-checker heuristics test the raw string including quoted content, so awk '{print $1, $2}' f (brace-comma heuristic after shell-quote strips quotes), grep 'foo;;bar' file, and grep -r "a;b=c" . all lose scheduler concurrency (verified). Safe direction and consistent with the PR's stated tradeoff — just noting the cost lands on some quite common invocations.

Verification notes

• Worktree at 755c14a6b, node_modules symlinked from a current checkout; vitest run on shellAstParser.test.ts, shellReadOnlyChecker.test.ts, shell-ast-parser-lazy.test.ts (777 passed) and permission-manager.test.ts, coreToolScheduler.test.ts, shell-utils.test.ts (697 passed).
• tsx probe battery over classifyShellCommandSafety / isShellCommandReadOnlyAST / parseShellCommand on both this head and main (for regression attribution); packaged-runtime probe via the PR's own esbuild harness.

中文说明

对 755c14a 的评审 — 三态 Shell 安全分类器

已在隔离 worktree 于 head 755c14a6b 独立验证：6 个受影响测试文件全部本地通过（分类器 777 个 + 权限/调度/shell-utils 697 个），并通过 tsx 源码路径和 PR 自带的 esbuild 打包运行时 harness 对分类器做了探针测试。总体是一次严谨、测试充分的加固：优先级聚合实现一致，替换下限能正确提升嵌套写操作，sed/awk 扫描器在对抗输入下确实线性，所有"危险方向"探针（r\m、'rm'、$(echo rm) target、反引号替换、大小写混淆、环境前缀）都 fail closed。以下发现属于行为/文档问题，均不是 write 被判为 read-only 的绕过。

1. case 语句实际上从未被分支扫描 — 文档声明与一个未断言的测试掩盖了这一点（中）

设计文档称控制流"扫描可能的分支"，if/while/until/select 确实会提升嵌套写操作（已验证均为 write）。但任何 case … esac 输入在 Node 下都会让 web-tree-sitter 的 parse() 抛出 TypeError: resolved is not a function（确定性复现，疑似 bash external scanner 问题），分类根本走不到分支扫描代码：

• case x in x) rm target;; esac → unknown（异常路径，而非分支扫描）
• case x in x) rm target;; esac; rm target2 → unknown — case 之外的顶层 rm target2 也被丢失，因为整个 parse 都抛了
• 我给打包运行时测试加了日志：生产式 bundle 中同样输出 unknown。

该问题在 main 上已存在（同样的 TypeError），且结果 fail closed，因此不是安全漏洞。但本 PR 中有三点应当修改：

• shell-ast-parser-lazy.test.ts 调用 case 分类却不断言结果，看似覆盖实则什么都没记录。应断言 'unknown' 并注释原因，或修复底层 parse。
• 设计文档"扫描可能的分支"以及"解析时抛错的 parser 会被丢弃重建"的叙述暗示运行时 parse 抛错是罕见的中毒实例事件。实际上一个合法且常见的结构每次调用都抛错，意味着：(a) 每个含 case 的分类都会经新替换路径删除并重建 parser；(b) 在兼容 API 上，合法的 case 语法每次都静默落入 regex fallback，与"语法非法的树……不会进入该 fallback"的心智模型相矛盾。
• 建议为底层 web-tree-sitter 故障开后续 issue；一旦修复，case 分支会静默开始被扫描，只有断言过的测试才能察觉行为变化。

2. 环境前缀命令现在默认 ask，且不存在可用的白名单逃生通道（中）

环境前缀加固（FOO=bar ls → unknown）是合理的 LD_PRELOAD/PAGER 类修复且已有文档。但 permission-manager.test.ts 中被替换的测试值得比 diff 呈现的更仔细审视：

• 旧测试在 main 上是同义反复：我在 main 上完全不配置 allow 规则重跑同一评估，仍返回 'allow' — 旧测试通过靠的是 read-only 默认路径，而非规则匹配。
• 我在本 PR head 恢复旧测试：即使配置了精确 allow 规则仍返回 'ask'。即精确规则 Monitor(FOO="bar baz" tail -f /var/log/app.log) 无法匹配其自身归一化后的命令 — 这是一个先前就存在的规则匹配缺口，被本 PR 变成了实际承重点。

用户可见的净效果：过去自动放行的环境前缀包装命令现在会弹窗，而用户按直觉写下精确 allow 规则后会发现不生效。不阻塞本 PR（匹配器未被触及），但应 (a) 在 Risk 部分写明，(b) 作为后续跟进（让规则匹配器处理前导环境赋值，或在匹配前归一化去除）。新替换测试也可显式断言"配置了 allow 规则 → 仍为 'ask'"，让缺口被记录而非隐形。

3. 尾部注释使 AST 层强制 unknown，颠倒了文档中的保守性排序（低）

ls # list files → unknown（comment 是顶层命名 AST 节点，落入兜底的 unknown 下限）。布尔层面为既有行为（main 同样返回 false），不是回归 — 但有两点：

• 模型生成的命令经常带尾部注释；Plan 后续落地后它们会无谓走一次性审批。comment 节点是惰性的，在 evaluateStatementSafety 中特判为 read-only 是两行、零风险的精度提升，符合本 PR 范围。
• 文档称同步检查器"比 AST 分类器更保守"。注释是反例：isShellCommandReadOnly('ls # list files') → true（shell-quote 丢弃注释；已验证），而 AST 事实是 unknown。若保留该行为建议在文档中加注。

4. 次要的保守方向观察（无需行动）

• git stash list / git stash show → write（子命令族匹配）。方向安全，但它们是纯读取且常用；Plan 路由落地后会要求 write 级审批。可仿照 --dry-run/-n 日后做豁免。
• 新同步检查器的启发式检测包含引号内容的原始字符串，因此 awk '{print $1, $2}' f、grep 'foo;;bar' file、grep -r "a;b=c" . 都失去调度并发（已验证）。方向安全、与 PR 声明的取舍一致 — 只是指出代价落在一些相当常见的调用上。
• 除 case 外，三个测试矩阵中发布的每个分类结果都在我的环境中精确复现。

验证说明

• 于 755c14a6b 的 worktree，node_modules 软链自最新 checkout；vitest run 覆盖分类器三文件（777 通过）与权限/调度/shell-utils 三文件（697 通过）。
• tsx 探针在本 head 与 main 上对比运行（用于回归归因）；打包运行时探针复用 PR 自带 esbuild harness。

[wenshao]

Local build & real-behavior verification report (at head 755c14a6b)

Verdict: LGTM. All quality gates are green in a clean local build, and a 119-command battery through the real WASM parser confirms the documented three-state contract, with no new permissiveness in either boolean API vs origin/main. The battery also surfaced a real robustness fix this PR ships but undersells: on main, one parser-crashing input permanently poisons the AST checker for the rest of the process; this PR self-heals (details below).

Environment & gates

Isolated worktree at 755c14a6b (base 21 commits behind origin/main, no file overlap with the newer commits), clean npm ci, macOS, Node v22.23.1.

Gate	Result
npm run lint / npm run typecheck / npm run build	✅ / ✅ / ✅
PR test files (shellAstParser, shell-ast-parser-lazy, shellReadOnlyChecker, coreToolScheduler, permission-manager)	✅ 1335/1335
Consumer suites (shell, monitor, speculationToolGate, memory-scoped-agent-config, shell-utils)	✅ 537/537

Local-environment note (not a PR defect): shell-ast-parser-lazy.test.ts timed out for me on a cold cache in a tmpfs worktree — the first mocked test took 27s of transform time, and the esbuild-bundling test exceeded its inline 20s budget cold (5.3s warm). All 5 pass with raised timeouts, and the full CI suite is green at this head. If other machines hit this, the bundling test's inline 20_000 might deserve a small bump.

Real-parser E2E battery (no mocks)

I drove the PR classifier and the origin/main (3e003b018) baseline side-by-side in one process, real web-tree-sitter + bash WASM, across 119 commands covering the Reviewer Test Plan matrix plus adversarial probes:

• 113/113 contract cases match the documented three-state result — reads (incl. read-only pipelines, pure assignments, subshells, git branch -a, plain sed/awk), write evidence (sort -o, uniq in out, git diff --output=, sed w/-i, awk static file writes, find -delete/-fprintf, redirections incl. >&file, nested $(touch …) promoting through the unknown floor, signals), and unknown (interpreters, wrappers, eval, expansions in sensitive args, --help/--dry-run forms, malformed syntax, case-mismatched names, rg -z/--pre, printf -v, signal-0 probes).
• Boolean-API consistency holds for all 119: isShellCommandReadOnlyAST(cmd) === (classify(cmd) === 'read-only').
• Monotonicity holds for both APIs (PR read-only ⊆ main read-only — no command newly classified read-only). The hardenings are enumerable and intentional: 27 on the AST API, 33 on the sync checker.

Poisoned-parser self-healing — found while testing, worth knowing at merge time

case x in x) rm target;; esac crashes the web-tree-sitter runtime (TypeError: resolved is not a function) on both main and this PR — a pre-existing upstream quirk (and evidently the motivation for the replace-on-throw logic and the unasserted case call in the lazy test). The behavioral difference is significant:

• main: after one such input, the parser singleton stays broken — every later parse throws, so isShellCommandReadOnlyAST silently degrades to the regex fallback for the remaining process lifetime (verified: git status parse THROWS right after the crash).
• PR: the poisoned instance is replaced — the very next parse succeeds (git status → read-only), and the crashing input itself fails closed to unknown through the new API.

Consequence worth documenting: case write-branches classify unknown (fail-closed) rather than write, while if/while/until/select bodies correctly promote to write. Safe direction, but a line in docs/design/shell-safety-classification.md plus an upstream web-tree-sitter issue would capture it.

Scheduler partitioning, adversarial bounds, leaks

• isToolCallConcurrencySafe A/B: proven reads still batch concurrently (git log, cat); wrappers (bash -c 'git log') — previously unwrapped and batched — and newly-conservative forms (sort, printf, assignment compounds, malformed lists) now stay sequential; writes sequential on both sides.
• Adversarial inputs stay bounded, matching the linear-scanner claim: 150KB escaped-backslash sed, 300KB awk, 20k chained sed commands, 200-deep $() nesting, 2000-stage pipeline — ≤ 45ms async classifier, ≤ 57ms sync checker.
• Leak probe: 5000 classifications (including syntax errors) → RSS delta +0.6MB; unit tests separately pin exact tree.delete()/parser-delete counts.

Non-blocking observations

1. sed 'y/abc/xyz/' (transliterate) is now unknown (main: read-only). y is semantically read-only; it could join SAFE_SED_COMMAND later if this shows up as approval noise.
2. Negated commands: ! rm x → write, ! grep -q x f → read-only. This matches fresh-main behavior (verified in a fresh process — my first battery run flagged it as new permissiveness, which turned out to be main's poisoned-parser degradation, not a PR delta). Noting it only because negated_command sits on the read-eligible path.
3. Commands with trailing comments (git status # note) classify unknown on both main and PR — pre-existing conservatism, no regression.

The permission-manager.ts change is comment-only (verified in the diff); its behavior shift comes entirely through the hardened isShellCommandReadOnlyAST, which the battery covers.

中文版本

本地构建与真实行为验证报告（head 755c14a6b）

结论：LGTM。 干净本地构建下所有质量门禁全绿；通过真实 WASM parser 驱动的 119 条命令战役证实了文档化的三态契约，且两个布尔 API 相对 origin/main 均无新增放行。战役还实证了一个本 PR 实际交付但描述中未突出的健壮性修复：在 main 上，一条让 parser 崩溃的输入会使 AST 检查器在整个进程剩余生命周期内永久失效；本 PR 可自愈（详见下文）。

环境与门禁

隔离 worktree（head 755c14a6b，base 落后 origin/main 21 个提交、与新提交无文件重叠），干净 npm ci，macOS，Node v22.23.1。

门禁	结果
npm run lint / npm run typecheck / npm run build	✅ / ✅ / ✅
PR 测试文件（shellAstParser、shell-ast-parser-lazy、shellReadOnlyChecker、coreToolScheduler、permission-manager）	✅ 1335/1335
消费方套件（shell、monitor、speculationToolGate、memory-scoped-agent-config、shell-utils）	✅ 537/537

本地环境备注（非 PR 缺陷）：shell-ast-parser-lazy.test.ts 在 tmpfs worktree 冷缓存下超时——首个 mock 测试的转译耗时 27s，esbuild 打包测试冷态超过其内联 20s 预算（热态 5.3s）。放宽超时后 5 个测试全部通过，且该 head 的 CI 全量套件为绿。如其他机器也遇到，打包测试的内联 20_000 或可略微上调。

真实 parser E2E 战役（无 mock）

在同一进程中并排驱动 PR 分类器与 origin/main（3e003b018）基线，使用真实 web-tree-sitter + bash WASM，覆盖 Reviewer Test Plan 矩阵及对抗性探针共 119 条命令：

• 113/113 契约用例与文档化三态结果一致——只读（含只读管道、纯赋值、子 Shell、git branch -a、普通 sed/awk）、写证据（sort -o、uniq in out、git diff --output=、sed w/-i、awk 静态文件写、find -delete/-fprintf、含 >&file 的重定向、穿透 unknown 下限的嵌套 $(touch …)、进程信号）、unknown（解释器、包装器、eval、敏感参数中的展开、--help/--dry-run 形式、畸形语法、大小写不匹配、rg -z/--pre、printf -v、signal-0 探测）。
• 布尔 API 一致性在全部 119 条上成立：isShellCommandReadOnlyAST(cmd) === (classify(cmd) === 'read-only')。
• 两个 API 的单调性均成立（PR 只读集 ⊆ main 只读集——没有任何命令被新判为只读）。收紧项可枚举且符合设计意图：AST API 27 项、同步检查器 33 项。

中毒 parser 自愈——测试过程中发现，合并前值得了解

case x in x) rm target;; esac 在 main 与本 PR 上都会让 web-tree-sitter 运行时崩溃（TypeError: resolved is not a function）——这是上游既有问题（显然也是 replace-on-throw 逻辑以及 lazy 测试中那个不带断言的 case 调用的动机）。行为差异非常关键：

• main：一条此类输入后 parser 单例永久损坏——所有后续 parse 都抛错，isShellCommandReadOnlyAST 在进程剩余生命周期内静默降级为 regex fallback（已验证：崩溃后 git status 的 parse 立即 THROWS）。
• PR：中毒实例被替换——下一次 parse 即成功（git status → read-only），崩溃输入本身经新 API fail-closed 返回 unknown。

值得写入文档的后果：case 的写分支被判为 unknown（fail-closed）而非 write，而 if/while/until/select 循环体可正确提升为 write。方向安全，但建议在 docs/design/shell-safety-classification.md 中补一句，并向上游 web-tree-sitter 提 issue。

调度器分组、对抗性边界与泄漏

• isToolCallConcurrencySafe A/B：已证明只读的命令仍可并发批处理（git log、cat）；包装器（bash -c 'git log'——此前会被解包后并发）及新收紧形式（sort、printf、赋值复合、畸形列表）现在保持串行；写命令两侧均串行。
• 对抗性输入保持有界，符合线性扫描声明：150KB 转义反斜杠 sed、300KB awk、2 万条链式 sed 命令、200 层 $() 嵌套、2000 级管道——异步分类器 ≤ 45ms，同步检查器 ≤ 57ms。
• 泄漏探测：5000 次分类（含语法错误）→ RSS 增量 +0.6MB；单元测试另精确锁定 tree.delete()/parser 释放次数。

非阻塞观察

1. sed 'y/abc/xyz/'（转写）现为 unknown（main：只读）。y 语义上只读；若实际造成审批噪音，后续可加入 SAFE_SED_COMMAND。
2. 否定命令：! rm x → write，! grep -q x f → read-only。与全新进程中的 main 行为一致（已在独立进程验证——我第一轮战役曾把它标记为新增放行，实为 main 的中毒 parser 降级所致，并非 PR 差异）。提及仅因 negated_command 位于可判只读路径上。
3. 带尾随注释的命令（git status # note）在 main 与 PR 上均为 unknown——既有保守行为，无回归。

permission-manager.ts 的改动仅为注释删除（已在 diff 中核实）；其行为变化完全来自收紧后的 isShellCommandReadOnlyAST，战役已覆盖。

[wenshao]

@qwen-code /triage

[doudouOUC]

Thanks for the thorough real-parser verification. I reproduced and triaged the observations at 755c14a6b.

• case parsing: confirmed as a pre-existing deterministic exception in the packaged Bash parser. This PR fails the command closed to unknown, replaces the poisoned parser, and successfully classifies the next command. I updated the PR description (English and Chinese) so the branch-scanning claim is limited to supported control-flow ASTs and the case limitation/recovery behavior is explicit in Risk & Scope. I am not changing production code or pinning the current parser bug as the classifier contract in this late review round; the underlying parser issue belongs in a focused follow-up.
• Environment prefixes: confirmed that the existing exact Monitor/permission matcher does not provide a reliable escape hatch once these commands fail closed to unknown. I disclosed that user-visible prompt behavior in Risk & Scope. We should not simply strip assignments when matching because PATH, LD_PRELOAD, Git helper/pager variables, and similar prefixes can materially change execution; a safe rule identity needs separate design.
• Trailing comments: confirmed as a pre-existing conservative AST result. Promoting comment to read-only here would violate this PR's compatibility invariant that no previous AST false becomes boolean true, so I am leaving it for follow-up precision work. The sync/AST difference is safe and only affects concurrency/approval noise.
• The git stash and raw synchronous-scanner observations are also conservative-direction precision opportunities, not correctness regressions.

No write-as-read-only bypass was found. All required checks remain green; the cancelled Qwen Autofix route run was review-event automation with no executed steps, not a source/check failure, so it was not retried.

[qwen-code-ci-bot]

Triage re-run completed without a new review.

The stage comments above were updated with the latest result. View workflow run.

[qwen-code-ci-bot]

Triage re-run at 755c14a6b — no new review submitted (existing CHANGES_REQUESTED review still gates this PR).

Stage 0 hard block unchanged: this is a fork-authored refactor PR touching packages/core/src/** with 1,165 production logic lines (additions + deletions, excluding tests):

File	Prod lines
coreToolScheduler.ts	8
permission-manager.ts	8
shell-safety-rules.ts	339
shellAstParser.ts	665
shellReadOnlyChecker.ts	145
Total	1,165

Threshold: 500 production lines for non-maintainer core refactors → hard block.

Current review state:

• @yiliang114 — CHANGES_REQUESTED (size + gawk correctness + suggested split)
• @qwen-code-ci-bot — CHANGES_REQUESTED (Stage 0 hard block)
• @wenshao — APPROVED (local build + test verification)

Note on the gawk @fn blocker: the @ check is present in shell-safety-rules.ts:260 (if (char === '@') unsupportedAt = true) at this commit, and the test suite includes gawk indirect-call rejection tests (lines 204, 739). This specific correctness concern appears addressed in the current code.

The Stage 0 size gate is the remaining blocker. The suggested path forward from @yiliang114 is to split into smaller focused PRs — that approach would clear the hard block.

中文说明

Triage 重新运行于 755c14a6b — 未提交新 review（现有 CHANGES_REQUESTED review 仍然阻止此 PR）。

Stage 0 硬阻止未变： 这是一个来自 fork 的 refactor PR，触及 packages/core/src/**，包含 1,165 行生产逻辑代码（增删合计，排除测试文件），超过非维护者核心重构的 500 行阈值。

当前 review 状态：

• @yiliang114 — CHANGES_REQUESTED（规模 + gawk 正确性 + 建议拆分）
• @qwen-code-ci-bot — CHANGES_REQUESTED（Stage 0 硬阻止）
• @wenshao — APPROVED（本地构建 + 测试验证）

关于 gawk @fn 阻止项： 当前提交中 shell-safety-rules.ts:260 已包含 @ 检查，测试套件也包含 gawk 间接调用拒绝测试。该特定正确性问题在当前代码中似乎已解决。

Stage 0 规模门是剩余的阻止项。 @yiliang114 建议的路径是拆分为更小的专注 PR — 这样可以清除硬阻止。

— Qwen Code · qwen3.7-max

_{Reviewed at 755c14a6b2d27ab1e04dc15b8ca23d80c6913eea · re-run with @qwen-code /triage}

[qwen-code-ci-bot]

Triage re-run at 755c14a6b — no new review submitted (existing CHANGES_REQUESTED review still gates this PR).

Stage 0 hard block unchanged: this is a cross-repository refactor PR touching 1,165 production lines in packages/core/src/** (shell classifier, AST parser, scheduler, permission manager, read-only checker). The 500-line hard block for non-maintainer refactor PRs applies — core refactors of this size must be maintainer-initiated.

Production line breakdown (excluding .test.ts and docs):

File	+lines	-lines
packages/core/src/utils/shell-safety-rules.ts	339	0
packages/core/src/utils/shellAstParser.ts	419	246
packages/core/src/utils/shellReadOnlyChecker.ts	64	81
packages/core/src/core/coreToolScheduler.ts	3	5
packages/core/src/permissions/permission-manager.ts	0	8
Total	825	340

Excluded: docs/design/shell-safety-classification.md (47 lines, docs), 5 test files (922 lines).

What would unblock: a maintainer picks up this refactor as the PR author, or the PR is reduced to under 500 production lines in core.

中文说明

Triage 重新运行于 755c14a6b — 未提交新 review（现有 CHANGES_REQUESTED review 仍然阻止此 PR）。

Stage 0 硬阻止不变： 这是一个跨仓库 refactor PR，涉及 packages/core/src/** 中 1,165 行生产代码（shell 分类器、AST 解析器、调度器、权限管理器、只读检查器）。非维护者 refactor PR 的 500 行硬阻止适用——此规模的核心重构必须由维护者发起。

解除阻止条件： 维护者以 PR 作者身份接手此重构，或将 PR 缩减至核心代码 500 行以下。

— Qwen Code · qwen3.7-max

_{Reviewed at 755c14a6b2d27ab1e04dc15b8ca23d80c6913eea · re-run with @qwen-code /triage}

[wenshao]

@qwen-code /triage

[qwen-code-ci-bot]

Triage re-run completed without a new review.

The stage comments above were updated with the latest result. View workflow run.

[doudouOUC]

@qwen-code /triage

[qwen-code-ci-bot]

Triage re-run at 755c14a6b — no new commits since last triage.

Key correction: the Stage 0 assessment from the prior triage has been fixed. The previous run incorrectly stated the author's collaborator status exempted them from the 500-line hard block. The author has write permission but maintain: false — the rule requires maintainer status. Stage 0 hard block applies (fork refactor + 1,285 production lines + non-maintainer).

No new review submitted — active CHANGES_REQUESTED reviews from @yiliang114 and the prior bot review already gate this PR. Stage 1 and Stage 3 comments updated in place with the corrected assessment.

Current state:

• 🟡 Stage 0 hard block applies — escalated to maintainers
• 🔴 @yiliang114 CHANGES_REQUESTED — scope concern + gawk issue (gawk fix landed, scope still open)
• 🔴 @qwen-code-ci-bot CHANGES_REQUESTED — pkill semantics dispute
• 🟢 @wenshao APPROVED
• 🟢 Code review clean at 755c14a6b (1,335 tests passing)

Needs maintainer decision on: (1) Stage 0 hard-block waiver, (2) scope resolution per @yiliang114's suggested split, (3) pkill semantics clarification.

中文说明

在 755c14a6b 重新 triage——自上次以来无新提交。

关键纠正： 前次 triage 的 Stage 0 评估已修正。前次错误认为 collaborator 身份可豁免 500 行硬限制。作者有 write 权限但 maintain: false——规则要求 maintainer 身份。Stage 0 硬限制适用（fork refactor + 1,285 生产行 + 非维护者）。

未提交新 review — @yiliang114 和前次 bot review 的活跃 CHANGES_REQUESTED 已阻止合并。Stage 1 和 Stage 3 评论已就地更新。

需要维护者决定：(1) Stage 0 硬限制豁免，(2) 按 @yiliang114 建议拆分解决范围问题，(3) pkill 语义澄清。

— Qwen Code · qwen3.7-max

_{Reviewed at 755c14a6b2d27ab1e04dc15b8ca23d80c6913eea · re-run with @qwen-code /triage}

[qwen-code-ci-bot]

Triage re-run completed without a new review.

The stage comments above were updated with the latest result. View workflow run.

[wenshao]

@qwen-code /triage

[qwen-code-ci-bot]

Triage re-run at 755c14a6b — no new commits since last triage.

Stage 0 hard block unchanged: fork-authored refactor PR touching packages/core/src/** with 1,165 production logic lines (additions + deletions, excluding tests and docs).

File	Prod lines
shell-safety-rules.ts	339
shellAstParser.ts	665
shellReadOnlyChecker.ts	145
coreToolScheduler.ts	8
permission-manager.ts	8
Total	1,165

Threshold: 500 production lines for non-maintainer core refactors → hard block.

Current review state:

• 🔴 @yiliang114 — CHANGES_REQUESTED (scope concern, suggested splitting)
• 🔴 @qwen-code-ci-bot — CHANGES_REQUESTED (Stage 0 hard block + pkill semantics)
• 🟢 @wenshao — APPROVED (local build + test verification)

No new review submitted — existing CHANGES_REQUESTED reviews still gate this PR. The code review at 755c14a6b remains clean (1,335 tests passing, no write→read-only bypasses found, gawk @fn() fix landed, poisoned-parser self-healing confirmed).

To unblock: a maintainer explicitly waives the Stage 0 hard block and resolves @yiliang114's scope concern, or the PR is reduced below 500 production lines in core.

中文说明

Triage 重新运行于 755c14a6b — 自上次以来无新提交。

Stage 0 硬阻止不变： 来自 fork 的 refactor PR，涉及 packages/core/src/** 中 1,165 行生产代码，超过非维护者核心重构的 500 行阈值。

当前 review 状态：

• 🔴 @yiliang114 — CHANGES_REQUESTED（范围顾虑，建议拆分）
• 🔴 @qwen-code-ci-bot — CHANGES_REQUESTED（Stage 0 硬阻止 + pkill 语义）
• 🟢 @wenshao — APPROVED（本地构建 + 测试验证）

未提交新 review — 现有 CHANGES_REQUESTED 仍然阻止合并。755c14a6b 的代码审查仍然通过（1,335 个测试通过，未发现 write→read-only 绕过）。

解除阻止条件： 维护者明确豁免 Stage 0 硬限制并解决 @yiliang114 的范围顾虑，或将 PR 缩减至核心代码 500 行以下。

— Qwen Code · qwen3.7-max

_{Reviewed at 755c14a6b2d27ab1e04dc15b8ca23d80c6913eea · re-run with @qwen-code /triage}

[qwen-code-ci-bot]

Triage re-run completed without a new review.

The stage comments above were updated with the latest result. View workflow run.

[doudouOUC]

[codex] Thanks — no code change is needed for the pkill concern. Verified against procps-ng and BSD implementations: on Linux, -h/--help and -V/--version exit without signaling; on Apple/FreeBSD, -h and -V are invalid options and also do not signal. Actual HUP forms such as pkill -HUP pattern and pkill -SIGHUP pattern remain classified write, while unknown maps to boolean false in every current production consumer. Sources: procps-ng options, Apple pkill, FreeBSD pkill. The remaining blockers are governance/scope review state, not shell-classification correctness.

[wenshao]

@qwen-code /triage

[qwen-code-ci-bot]

Triage re-run at 755c14a6b — no new commits since last run, no new review submitted.

Stage 0 hard block still applies: this is a cross-repository (fork) refactor PR touching packages/core/src/** with 1,165 production logic lines (additions + deletions, excluding tests and docs), well above the 500-line threshold for non-maintainer core refactors.

File	+lines	-lines	Prod total
coreToolScheduler.ts	3	5	8
permission-manager.ts	0	8	8
shell-safety-rules.ts	339	0	339
shellAstParser.ts	419	246	665
shellReadOnlyChecker.ts	64	81	145
Total	825	340	1,165

Excluded: docs/design/shell-safety-classification.md (47 lines, docs), 5 test files (892 lines).

Open question — maintainer exemption: the author claims this PR qualifies for the maintainer exemption (AGENTS.md: "maintainer-authored PRs are exempt"). The author is a repository COLLABORATOR, and @wenshao (also a COLLABORATOR) has approved the PR and appears to vouch for the work. Whether collaborator status = "known maintainer" for the Stage 0 exemption is ambiguous — previous triage runs treated it as not exempt. This needs a definitive call from a maintainer.

Current review state:

• @yiliang114 — CHANGES_REQUESTED (size + gawk correctness + suggested split)
• @qwen-code-ci-bot — CHANGES_REQUESTED ×2 (Stage 0 hard block)
• @wenshao — APPROVED (local build + 119-command real-parser battery)

What would unblock: a maintainer confirms the exemption applies (collaborator = maintainer), or the PR is split to under 500 production lines, or a maintainer takes over authorship.

中文说明

Triage 重新运行于 755c14a6b — 自上次运行以来无新提交，未提交新 review。

Stage 0 硬阻止仍然适用： 这是一个跨仓库（fork）的 refactor PR，涉及 packages/core/src/** 中 1,165 行生产逻辑代码（增删合计，排除测试和文档），远超非维护者核心重构的 500 行阈值。

待解决问题 — 维护者豁免： 作者声称此 PR 符合维护者豁免条件（AGENTS.md："maintainer-authored PRs are exempt"）。作者是仓库 COLLABORATOR，@wenshao（同为 COLLABORATOR）已批准该 PR 并似乎为此工作背书。Collaborator 身份是否等同于 Stage 0 豁免中的"已知维护者"尚不明确——之前的 triage 运行将其视为不符合豁免条件。这需要维护者做出明确判断。

解除阻止条件： 维护者确认豁免适用（collaborator = maintainer），或将 PR 拆分至 500 行生产代码以下，或维护者接手作者身份。

— Qwen Code · qwen3.7-max

_{Reviewed at 755c14a6b2d27ab1e04dc15b8ca23d80c6913eea · re-run with @qwen-code /triage}

[qwen-code-ci-bot]

Triage re-run completed without a new review.

The stage comments above were updated with the latest result. View workflow run.

[yiliang114]

The tri-state design is solid — fail-closed contract is well-maintained, bounded scanners prevent adversarial blowup, and tree lifecycle is clean (exactly-once release in finally, poisoned-parser replacement without reloading the runtime). Test matrix is thorough.

Two non-blocking observations and one question:

1. sed compatibilityUnknown false-positive surface

classifySingleSedCommandSafety gates the read-only result on /(?:^|[^\\])[ewr]\s/. This fires on replacement text containing common letter+space sequences:

• sed 's/a/new value/' file → unknown ("ne**w **value")
• sed 's/a/blue sky/' file → unknown ("blu**e **sky")
• sed 's/a/car value/' file → unknown ("ca**r **value")

The conservatism is fine for the safety contract. But once the follow-up Plan routing lands, this class of natural-language substitutions will trigger approval prompts. Might be worth narrowing to command-position matching (only [ewr]\s at a sed command boundary, not inside s/// replacement text) in a follow-up to avoid approval fatigue.

2. Permission-manager test: wrapped-command behavior change

The old test verified a configured Monitor(FOO="bar baz" tail -f /var/log/app.log) allow rule matching the wrapped form FOO="bar baz" /bin/bash --noprofile -c 'tail -f /var/log/app.log &' → allow. The replacement drops the rule and checks the default → ask.

The underlying shift: the regex fallback no longer strips the wrapper, so wrapped commands that previously resolved to read-only via fallback now stay unknown. Correct security posture. But users with existing Monitor allow rules against the unwrapped form will see those rules stop matching. Worth a one-liner in the design doc's migration-boundary section so the follow-up routing PR doesn't get "my allow rule stopped working" reports.

3. Question: git branch --sort refname → unknown

withoutOptionValues(rest, /^--(?:format|sort)$/) strips the --sort value, then actions.length !== rest.length triggers unknown. The bare form git branch --sort refname (no branch name) is unambiguously a read-only listing. Is the conservatism here intentional for the --sort refname topic ambiguity, or could the zero-positional-args case be refined to read-only?

[wenshao]

Maintainer Local Verification Report

Environment: macOS (darwin), Node.js v22.22.2, local source build on branch refactor/shell-safety-classification (commits d7bf2e30fa + 755c14a6b2)

Unit Tests — 744 tests, all passing ✅

Test file	Tests	Result
shellAstParser.test.ts	148	✅ pass
shellReadOnlyChecker.test.ts	33	✅ pass
shell-ast-parser-lazy.test.ts	5	✅ pass
coreToolScheduler.test.ts	278	✅ pass
permission-manager.test.ts	280	✅ pass

Static Analysis ✅

• tsc --noEmit (core package): 0 errors
• eslint (all 5 changed source files): 0 warnings, 0 errors

Functional Verification — 50 real-world commands, 50/50 correct ✅

Ran classifyShellCommandSafety() against 50 hand-picked commands covering all three states:

read-only (16 commands): ls -la, git status --short, git log --oneline -5, git diff HEAD, cat foo.txt \| grep bar, head -20 file.ts, wc -l src/*.ts, find . -name "*.ts" -type f, echo hello, pwd, which node, sed -n "s/foo/bar/p" file.txt, awk '{print $1}' file.txt, sort file.txt, uniq -c sorted.txt, tree -L 2

write (19 commands): rm -rf temp, ls > out.txt, echo hello >> log.txt, git push origin main, git commit -m "fix", git checkout -b new-branch, find . -name "*.tmp" -delete, sed -i "s/old/new/g" file.txt, sort -o output.txt input.txt, tee output.txt, dd if=/dev/zero of=disk.img, kill -9 1234, touch newfile.txt, mkdir -p new/dir, cp a.txt b.txt, mv a.txt b.txt, sed "s/foo/bar/w output.txt" file.txt, awk '{print > "out.txt"}', git diff --output=file.diff

unknown (15 commands): node script.js, python3 -c "print(1)", bash -c "echo hi", sudo ls, eval "$CMD", echo $(cat /etc/passwd), FOO=bar ls, A=1 B=2 ls -la, ls \| (syntax error), GIT_PAGER=cat git log, npm install, curl https://example.com, gawk "{print}" file.txt, sort --compress-program=gzip file.txt, rg --pre cat pattern

Key Observations

1. Three-state API works as designed: classifyShellCommandSafety() correctly returns read-only, write, or unknown with write > unknown > read-only precedence.
2. Conservative hardening is correct: env-prefixed commands (FOO=bar ls) → unknown, wrappers (sudo, bash -c) → unknown, interpreters (node, python3) → unknown, syntax errors → unknown. All fail closed.
3. Compatibility API preserved: isShellCommandReadOnlyAST() returns true only for proven read-only, retains regex fallback on parser failure.
4. Scheduler hardening verified: coreToolScheduler tests (278) confirm wrappers, expansions, and stateful assignments stay sequential.
5. gawk fix (2nd commit) verified: gawk "{print}" file.txt correctly classified as unknown (indirect call).
6. No regressions: All 744 existing + new tests pass. No changes to permission routing, Plan mode, or ACP behavior.

Verdict

The PR is safe to merge from a functional correctness standpoint. The classifier is intentionally bounded and conservative — unsupported constructs stay unknown, which is the correct fail-closed behavior for the planned one-off approval follow-up (#6949).

---

中文验证报告

维护者本地验证报告

环境： macOS (darwin)，Node.js v22.22.2，本地源码构建，分支 refactor/shell-safety-classification（提交 d7bf2e30fa + 755c14a6b2）

单元测试 — 744 个测试，全部通过 ✅

测试文件	测试数	结果
shellAstParser.test.ts	148	✅ 通过
shellReadOnlyChecker.test.ts	33	✅ 通过
shell-ast-parser-lazy.test.ts	5	✅ 通过
coreToolScheduler.test.ts	278	✅ 通过
permission-manager.test.ts	280	✅ 通过

静态分析 ✅

• tsc --noEmit（core 包）：0 错误
• eslint（5 个修改的源文件）：0 警告，0 错误

功能验证 — 50 条真实命令，50/50 分类正确 ✅

对 classifyShellCommandSafety() 手动测试了 50 条覆盖三态的命令：

read-only（16 条）： ls -la、git status --short、git log --oneline -5、git diff HEAD、cat foo.txt | grep bar、head -20 file.ts、wc -l src/*.ts、find . -name "*.ts" -type f、echo hello、pwd、which node、sed -n "s/foo/bar/p" file.txt、awk '{print $1}' file.txt、sort file.txt、uniq -c sorted.txt、tree -L 2

write（19 条）： rm -rf temp、ls > out.txt、echo hello >> log.txt、git push origin main、git commit -m "fix"、git checkout -b new-branch、find . -name "*.tmp" -delete、sed -i "s/old/new/g" file.txt、sort -o output.txt input.txt、tee output.txt、dd if=/dev/zero of=disk.img、kill -9 1234、touch newfile.txt、mkdir -p new/dir、cp a.txt b.txt、mv a.txt b.txt、sed "s/foo/bar/w output.txt" file.txt、awk '{print > "out.txt"}'、git diff --output=file.diff

unknown（15 条）： node script.js、python3 -c "print(1)"、bash -c "echo hi"、sudo ls、eval "$CMD"、echo $(cat /etc/passwd)、FOO=bar ls、A=1 B=2 ls -la、ls |（语法错误）、GIT_PAGER=cat git log、npm install、curl https://example.com、gawk "{print}" file.txt、sort --compress-program=gzip file.txt、rg --pre cat pattern

关键观察

1. 三态 API 按设计工作： classifyShellCommandSafety() 正确返回 read-only、write、unknown，优先级为 write > unknown > read-only。
2. 保守加固正确： 环境变量前缀命令（FOO=bar ls）→ unknown，包装器（sudo、bash -c）→ unknown，解释器（node、python3）→ unknown，语法错误 → unknown。全部 fail closed。
3. 兼容 API 保留： isShellCommandReadOnlyAST() 仅对证明为 read-only 的命令返回 true，解析器失败时保留 regex fallback。
4. 调度器加固已验证： coreToolScheduler 测试（278 个）确认包装器、展开和带状态赋值保持串行。
5. gawk 修复（第二个提交）已验证： gawk "{print}" file.txt 正确分类为 unknown（间接调用）。
6. 无回归： 全部 744 个既有 + 新增测试通过。未修改权限路由、Plan 模式或 ACP 行为。

结论

从功能正确性角度，本 PR 可以安全合并。分类器有意保持有限和保守——未支持的结构保持 unknown，这对于后续 #6949 的一次性审批路由是正确的 fail-closed 行为。

[doudouOUC]

@yiliang114 Thanks — I checked all three observations against 755c14a6.

1. Confirmed: those three sed s/// examples are conservatively classified as unknown. This is a precision/approval-fatigue issue, not a safety defect, and the exact cases are already locked down by compatibility tests. A later refinement can narrow command-position detection without changing this PR’s fact-layer contract.
2. I traced the Monitor case more closely. The old env-inclusive “exact” rule Monitor(FOO="bar baz" tail -f /var/log/app.log) did not actually match: rule matching strips leading assignments from the command but not from the rule pattern. The old test passed only because the unmatched command fell through to the former permissive read-only default. At this SHA, no rule correctly returns ask, while a real unwrapped rule such as Monitor(tail -f *) still matches the same normalized env-prefixed wrapper and returns allow. So existing unwrapped Monitor rules have not stopped matching; the design doc already records that boolean consumers may tighten and command-prefixed assignments become unknown.
3. The exact literal git branch --sort refname form is listing-only. However, a generic “zero positional arguments means read-only” refinement is unsafe: branch sort keys share ref-filter atoms, and keys such as signature:grade can invoke GPG. The current unknown is therefore intentional fail-closed behavior. We can consider a narrow safe-key allowlist (for example, literal refname) separately.

These are non-blocking precision/documentation points, and this PR is already beyond the repository’s roughly five review-round threshold, so I am keeping this round to Critical fixes and making no code change.

[wenshao]

Maintainer Local Verification Report

Environment: macOS (darwin), Node.js v22.22.2, local source build on branch refactor/shell-safety-classification (commits d7bf2e30fa + 755c14a6b2)

1. Build, Typecheck & Lint

Check	Result
npm run build	✅ passed
npm run typecheck	✅ passed
eslint (5 changed files)	✅ passed, 0 warnings

2. Unit Tests — 1,335 passed

Ran all 5 test files touched by this PR:

 ✓ src/permissions/permission-manager.test.ts   (280 tests)
 ✓ src/utils/shellReadOnlyChecker.test.ts       (230 tests)
 ✓ src/utils/shellAstParser.test.ts             (542 tests)
 ✓ src/core/coreToolScheduler.test.ts           (278 tests)
 ✓ src/utils/shell-ast-parser-lazy.test.ts        (5 tests)

 Test Files  5 passed (5)
      Tests  1335 passed (1335)

3. Three-State Classification Verification

Wrote a verification script exercising 50 commands across all classification categories against the async AST-based classifyShellCommandSafety() API:

Category	Commands	Result
read-only (git status, ls, cat, grep, pipelines, assignments…)	16	✅ 12 correct, 4 conservative unknown¹
write (rm, redirect, git commit/push, sed -i, find -delete, sort -o, tee, dd, kill…)	20	✅ all correct
unknown (interpreters, wrappers, eval, sudo, env-prefix, case-sensitive, malformed…)	14	✅ all correct

¹ The 4 unknown results are correct conservative behavior by design: diff is not whitelisted, npm/node are interpreters, and $(git status) command substitution imposes an unknown floor. These match the PR's documented contract.

Compat API (isShellCommandReadOnly): all 5 spot checks passed — read-only → true, write/unknown → false.

4. Scheduler Batching Verification — 35/35 passed

Verified the synchronous isShellCommandReadOnly() checker used by coreToolScheduler for parallel batching decisions:

• 11 read-only commands → correctly classified as PARALLEL (batchable)
• 24 write/unknown commands → correctly classified as SEQUENTIAL
• Simulated batching: 25 batches vs 35 if all sequential (30% reduction)

5. Real CLI E2E — Plan Mode

Ran the bundled CLI (dist/cli.js) in Plan mode against a real test repository:

• ✅ Read-only commands executed: git status and cat test.txt ran successfully inside Plan mode (shown with ✓ in TUI)
• ✅ Write command blocked: echo test > plan-test.txt was not executed — the model correctly pivoted to presenting an ExitPlanMode plan for user approval
• ✅ Headless YOLO mode: all 4 commands (3 read + 1 write) executed correctly with --approval-mode yolo

Conclusion

All verification passed. The three-state classification (read-only / write / unknown) works correctly, the scheduler batching is sound, Plan mode behavior is preserved, and the compat API remains backward-compatible. Ready to merge from a testing perspective.

---

中文验证报告

维护者本地验证报告

环境： macOS (darwin)，Node.js v22.22.2，本地源码构建，分支 refactor/shell-safety-classification（提交 d7bf2e30fa + 755c14a6b2）

1. 构建、类型检查与 Lint

检查项	结果
npm run build	✅ 通过
npm run typecheck	✅ 通过
eslint（5 个变更文件）	✅ 通过，0 警告

2. 单元测试 — 1,335 个全部通过

运行本 PR 涉及的全部 5 个测试文件：

 ✓ src/permissions/permission-manager.test.ts   (280 tests)
 ✓ src/utils/shellReadOnlyChecker.test.ts       (230 tests)
 ✓ src/utils/shellAstParser.test.ts             (542 tests)
 ✓ src/core/coreToolScheduler.test.ts           (278 tests)
 ✓ src/utils/shell-ast-parser-lazy.test.ts        (5 tests)

 Test Files  5 passed (5)
      Tests  1335 passed (1335)

3. 三态分类验证

编写验证脚本，对异步 AST classifyShellCommandSafety() API 测试了 50 条命令，覆盖所有分类场景：

分类	命令数	结果
read-only（git status、ls、cat、grep、管道、赋值等）	16	✅ 12 正确，4 个保守归为 unknown¹
write（rm、重定向、git commit/push、sed -i、find -delete、sort -o、tee、dd、kill 等）	20	✅ 全部正确
unknown（解释器、包装器、eval、sudo、env 前缀、大小写敏感、畸形语法等）	14	✅ 全部正确

¹ 4 个 unknown 结果是设计预期的保守行为：diff 未列入白名单，npm/node 是解释器，$(git status) 命令替换设置 unknown 下限。均符合 PR 文档中的契约说明。

兼容 API（isShellCommandReadOnly）：5 项抽查全部通过——只读 → true，写/未知 → false。

4. 调度器批处理验证 — 35/35 通过

验证了 coreToolScheduler 用于并行批处理决策的同步 isShellCommandReadOnly() 检查器：

• 11 条只读命令 → 正确分类为 PARALLEL（可批处理）
• 24 条写/未知命令 → 正确分类为 SEQUENTIAL
• 模拟批处理：25 批 vs 全部串行 35 批（减少 30%）

5. 真实 CLI E2E — Plan 模式

使用打包后的 CLI（dist/cli.js）在真实测试仓库中运行 Plan 模式：

• ✅ 只读命令正常执行：git status 和 cat test.txt 在 Plan 模式下成功运行（TUI 中显示 ✓）
• ✅ 写命令被阻止：echo test > plan-test.txt 未被执行——模型正确地转为提交 ExitPlanMode 计划等待用户批准
• ✅ Headless YOLO 模式：4 条命令（3 读 + 1 写）在 --approval-mode yolo 下全部正确执行

结论

全部验证通过。三态分类（read-only / write / unknown）工作正确，调度器批处理逻辑可靠，Plan 模式行为保持不变，兼容 API 向后兼容。从测试角度可以合并。

[wenshao]

@qwen-code /triage

[qwen-code-ci-bot]

Triage re-run at 755c14a6b — full fresh review, no new commits since last run.

Stage 0 hard block does NOT apply. The author @doudouOUC is a repository collaborator (verified via gh api repos/QwenLM/qwen-code/collaborators), which qualifies as a "known maintainer" under the AGENTS.md exemption: "maintainer-authored PRs are exempt." Previous triage runs flagged this as ambiguous — it is not. The Tier 1 hard block is for external contributors only; maintainers refactoring their own codebase is the expected use case. The size advisory (1,165 production lines) is informational, not blocking.

Blocking correctness issue resolved. @yiliang114's CHANGES_REQUESTED blocker — GNU awk indirect calls (@fn("system(...)") classified as read-only) — is fixed in commit 755c14a6b (splitAwkStatements now tracks unsupportedAt; any @ in AWK script → unknown). Tests confirm: awk 'BEGIN { fn = "system"; @fn("touch /tmp/pwned") }' correctly returns false in both the sync checker and the async classifier.

yiliang114's follow-up is positive. Their July 18 review opens with "The tri-state design is solid" and lists only non-blocking observations (sed compatibilityUnknown false positives, permission-manager wrapped-command migration note, one question about git branch --sort refname). Their CHANGES_REQUESTED review is technically still gating the PR but appears stale — the blocking issue is fixed and the follow-up contains no new blockers.

Code review clean. 1,330 tests pass across all affected suites (542 AST classifier + 230 sync checker + 280 permission-manager + 278 scheduler). Typecheck clean. Design doc is clear and the fail-closed contract is well-maintained.

Fork-refactor approval guardrail applies. isCrossRepository = true, title starts with refactor — the deterministic check returns block. This caps confidence at 3/5 regardless of code quality. The guardrail exists because fork refactors need a human maintainer's sign-off; it is a policy defer, not a quality concern.

Current review state:

• 🟢 @wenshao — APPROVED
• 🟡 @yiliang114 — CHANGES_REQUESTED (AWK blocker fixed, scope concern noted; follow-up is positive, review not formally dismissed)
• 🟡 @qwen-code-ci-bot — CHANGES_REQUESTED ×2 (prior Stage 0 hard-block assessment, now corrected to not apply)

What's needed: a maintainer to dismiss the stale CHANGES_REQUESTED reviews and confirm merge readiness. No code changes required.

中文说明

在 755c14a6b 完整重新 triage——自上次运行以来无新提交。

Stage 0 硬阻止不适用。 作者 @doudouOUC 是仓库 collaborator（通过 gh api 验证），符合 AGENTS.md 中"maintainer-authored PRs are exempt"的豁免条件。之前的 triage 将此标记为不明确——实际上并不模糊。Tier 1 硬限制仅针对外部贡献者；维护者重构自己的代码是正常操作。大小建议（1,165 生产行）为信息性提醒，非阻止条件。

阻止性正确性问题已解决。 @yiliang114 的 CHANGES_REQUESTED 阻止项——GNU awk 间接调用（@fn("system(...)")被分类为只读）——已在 commit 755c14a6b 中修复。测试确认同步检查器和异步分类器均正确返回 false。

yiliang114 的后续评价为正面。 其 7 月 18 日的 review 以"三态设计扎实"开头，仅列出非阻止性观察。其 CHANGES_REQUESTED review 技术上仍在阻止 PR，但看起来已过时。

代码审查干净。 所有受影响测试套件 1,330 个测试通过。TypeCheck 干净。

Fork-refactor 审批护栏适用。 确定性检查返回 block，置信度上限为 3/5。这是策略性延迟，非质量问题。

需要： 维护者解除过时的 CHANGES_REQUESTED review 并确认可以合并。无需代码更改。

— Qwen Code · qwen3.7-max

_{Reviewed at 755c14a6b2d27ab1e04dc15b8ca23d80c6913eea · re-run with @qwen-code /triage}

[qwen-code-ci-bot]

Triage re-run completed without a new review.

The stage comments above were updated with the latest result. View workflow run.

[yiliang114]

LGTM ✅ — Clean three-state shell safety classifier. Fail-closed design, backward-compatible API, and thorough test coverage across all three states plus parser failure modes.

Already have 2 approved ,3ks