fix(core): expand QWEN_PROJECT_DIR in command hooks

[chandlerm923]

What this PR does

Expands the documented $QWEN_PROJECT_DIR placeholder in command hooks while preserving shell-safe project paths. It also preserves quoted command text passed to Windows cmd.exe.

Why it's needed

The hook environment exposes QWEN_PROJECT_DIR, and the documentation uses $QWEN_PROJECT_DIR/... in command hooks, but the command expansion path did not support it. On Windows, Node's default argument quoting also corrupted quoted commands passed to cmd.exe.

Reviewer Test Plan

How to verify

Run the documented placeholder-first form $QWEN_PROJECT_DIR/.qwen/hooks/security-check.sh through Bash from a project path containing spaces and shell metacharacters. On Windows cmd.exe, run the equivalent executable .cmd hook from the same kind of path and verify that commands containing double quotes are preserved. Confirm that PowerShell hooks and the existing GEMINI and CLAUDE placeholders still resolve correctly.

Evidence (Before & After)

Before this change, $QWEN_PROJECT_DIR was not expanded and quoted cmd.exe hooks could be corrupted by argument marshalling. After this change, the documented .sh placeholder-first form runs through Bash, the equivalent .cmd form runs through cmd.exe, quoted cmd.exe commands remain intact, and QWEN, GEMINI, and CLAUDE project-directory placeholders resolve without regressing paths containing spaces.

Tested on

OS	Status
Windows	Passed with cmd.exe, Git Bash, and PowerShell
Linux	Unit tests passed in CI
macOS	Not run

Risk & Scope

• Main risk or tradeoff: windowsVerbatimArguments is enabled only for Windows cmd.exe command hooks, changing argument marshalling for those hooks.
• Breaking changes / migration notes: None expected. Native shell environment syntax and existing placeholders remain supported.

Linked Issues

• Related to fix(core): pass verbatim arguments to cmd.exe for command hooks on Windows #9357.
• Related to fix(core): prefer Claude manifest when an extension ships multiple manifests #8646.

This PR is scoped to $QWEN_PROJECT_DIR support and includes the minimum cmd.exe quoting correction required for placeholder-first paths and quoted commands.

Contribution checklist

• Focused bug fix with regression coverage.
• Reviewer reproduction steps and before/after evidence included.

[qwen-code-ci-bot]

⚠️ Qwen Triage ended early — view run. It stopped before finishing; check the run log.

⚠️ Qwen Triage 提前结束 —— 查看运行。未跑完，请查看运行日志。

[qwen-code-ci-bot]

Partially reviewed — gaps disclosed.

Not explored to full depth (tool budget reached): "agent 1a": run hookRunner.test.ts / hook-runner.process.test.ts to confirm the new tests pass (worktree lacks installed dependencies).

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

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

[qwen-code-ci-bot]

[Suggestion] This new spawn option has no witness on any per-PR CI lane. Its only real tests are the describe.runIf(process.platform === 'win32') process tests, and the test_windows lane in .github/workflows/ci.yml is gated to merge_group (merge queue disabled since 2026-07-02 per the workflow's own comment), schedule, and workflow_dispatch — the pull_request trigger is off, and the new mock unit test asserts only the expanded command string, never the spawn options. Since the option is inert off-win32, Linux/macOS lanes are structurally incapable of observing it: a follow-up edit that deletes the option or drops the === 'cmd' qualifier merges green and re-breaks cmd.exe quoting for hook paths containing spaces/&/quotes, resurfacing at best on a nightly run. Please add mocked-platform unit assertions following the existing pattern in shellExecutionService.test.ts (~3260/3313): spy process.platform to 'win32' with a cmd shell config and assert spawn receives windowsVerbatimArguments: true; assert it is absent for a shell: 'powershell' hook override and on 'linux' — and confirm that new assertion goes red when the two added lines are removed.

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

[qwen-code-ci-bot]

[Suggestion] This encodes the "cmd.exe needs verbatim arguments" policy per-caller for the second time: shellExecutionService.ts:801 already carries the identical condition (with an explanatory comment), and a third caller of the same spawn shape — packages/core/src/tools/monitor.ts:370 — does not set the flag at all, which is the fragmentation cost in the wild: on Windows default cmd, a monitor command with a quoted path gets Node's MSVC escaping that cmd.exe cannot parse, the exact corruption this PR fixes for hooks. Any future correction to the escaping policy (a Node escaping change, a pwsh variant, a shell-detection fix) must now be applied in two files in two subsystems, and updating only one silently regresses the other. The new line also open-codes process.platform === 'win32' although shell-utils.ts exports isWindows(). Consider centralising the rule where the spawn pattern is owned — e.g. a flag on ShellConfiguration populated by getShellConfiguration(), or a thin spawnShellCommand(command, opts) helper in shell-utils.ts — consumed by hookRunner.ts, shellExecutionService.ts, and monitor.ts. The existing assertions in shellExecutionService.test.ts (~3260 cmd true / ~3313 PowerShell false) plus this PR's cmd process test pin the move; if monitor.ts adopts the shared config it needs an equivalent spawn-options assertion (it has none today).

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

[chandlerm923]

Fixed in e69998f:

• Bash placeholders now stay native to the child shell.
• QWEN/GEMINI/CLAUDE expansion uses one boundary-aware replacement.
• Added regression coverage for comments, repeated placeholders, and placeholder-like paths.

I left R1-2/R1-3 for follow-up because platform-mocked spawn assertions and a shared spawn helper are broader than this focused fix.

[qwen-code-ci-bot]

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

• R1-3 cmd.exe verbatim-args policy encoded per-caller a second time — already reported (comment 3880640000)

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

[qwen-code-ci-bot]

Reviewed.

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

• R1-2 windowsVerbatimArguments spawn option has no per-PR-runnable unit test — still standing, already reported (comment 3882536358)

Not explored to full depth (tool budget reached): "agent 1a": none — the full walk completed; only the worktree's vitest suite could not be executed (no node_modules), which I compensated by replaying the changed logic dir….

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

[wenshao]

Runtime verification of PR #10393 (maintainer review)

I built a real A/B environment for this change instead of reading the diff only. Both arms drive the actual HookRunner.executeHook() from a worktree — d853f09f52 (PR base) vs 7793f6f323 (PR head) — through real child shells:

• Windows arm — node.exe win-x64 v22.22.2 under Wine, so process.platform === 'win32', ComSpec = C:\windows\system32\cmd.exe, real cmd.exe, real spawn() argument marshalling, plus PowerShell 7.4 for shell: "powershell" hooks.
• POSIX arm — the same driver on native Linux/bash.
• 45 scenarios, identical on both arms; project directories deliberately named qwen hook (project) & 'quoted' …, qwen my project …, and a plain one.

Verdict: the Windows half of this change is real and it fixes more than the PR claims — but the expandCommand early return for bash is an undisclosed regression on Linux/macOS, and the exact hook form our own docs teach still does not work on Windows. Details and evidence below.

---

1. What the PR genuinely fixes (verified on real cmd.exe / PowerShell)

$QWEN_PROJECT_DIR resolves for cmd and PowerShell as advertised. More importantly, windowsVerbatimArguments turns out to fix a much larger pre-existing bug than the description says: on main, libuv quotes the /c argument and escapes every " as \", so any cmd hook whose command text contains a double quote is mangled today:

scenario	base	head
echo "hello world"	\"hello world\"	"hello world"
if exist "%QWEN_PROJECT_DIR%" (echo FOUND) else (echo MISSING) — pure native %VAR%, no placeholder	MISSING	FOUND
cd /d $QWEN_PROJECT_DIR && echo MIDOK	exit 1, File not found.	MIDOK

That third-party benefit is worth calling out in the PR body; it is the strongest argument for the change.

2. Regression: $GEMINI_PROJECT_DIR / $CLAUDE_PROJECT_DIR stop being safe on Linux/macOS

expandCommand() now returns the command untouched for shellType === 'bash'. On main those two placeholders were substituted through escapeShellArg(cwd, 'bash'), i.e. shell-quoted. After this PR they are left to plain, unquoted parameter expansion in the child, so bash word-splits any project path containing a space:

scenario               base              head
posix-gemini-spaces    FOUND       ->    MISSING
posix-claude-spaces    FOUND       ->    MISSING
posix-doc-hook-gemini  HOOK-SCRIPT-RAN -> exit 127
                       stderr: bash: line 1: /tmp/qwen: No such file or directory

This is a behaviour change on the default platform inside a PR scoped as a Windows fix, and the "Tested on" table lists Linux/macOS as unit tests only — the two pre-existing expandCommand tests only keep passing because the PR edited them to add shell: 'powershell', which is exactly the signal that documented POSIX behaviour changed.

The stated motivation (comments, single-quoted literals, $..._DIRS suffixes keeping native bash semantics) is real and I confirmed the improvement — but the (?!…) boundary lookahead this PR adds already solves the $QWEN_PROJECT_DIRS half of it. Keeping the escaped substitution for bash (with the new lookahead and the new QWEN alternative) would give the boundary fix without breaking spacey project paths. If the early return is intentional, it needs its own paragraph in the PR body, a POSIX regression test, and a note that macOS users with a space in their path will silently lose these hooks.

3. Residual gap: the hook form our documentation teaches still fails on Windows

docs/users/features/hooks.md only ever shows the placeholder at the start of the command:

"command": "$QWEN_PROJECT_DIR/.qwen/hooks/security-check.sh"

cmd /S /C strips the first and last quote of the command line (documented in cmd /?). Because the expansion emits "C:\…project…" as a quoted token, that quoting is destroyed exactly when the placeholder starts the command — and if the project path contains a &, the fragment after it is executed as a separate command:

project dir: C:\…\Temp\proj & echo INJECTED-PIBToR
hook:        $GEMINI_PROJECT_DIR\.qwen\hooks\check.cmd
base -> exit 9009, stdout <empty>                       (quotes survived, command just not found)
head -> exit 9009, stdout "INJECTED-PIBToR\.qwen\…"     (the text after `&` in the path RAN)

Note this is the same class of directory name the PR's own fixture uses (qwen hook (project) & 'quoted' ). The PR's new test passes only because if exist $QWEN_PROJECT_DIR … does not begin with the quote.

A one-line follow-up closes it, verified on the same runtime — pass the command wrapped in one extra quote pair so /S consumes the outer pair instead of the expansion's:

[...shellConfig.argsPrefix, useVerbatim ? `"${command}"` : command]

With that, the documented placeholder-first form runs (HOOK-SCRIPT-RAN, exit 0) and the PR's own if exist $QWEN_PROJECT_DIR … shape is unaffected (FOUND).

4. Test coverage of the change that carries the fix

Mutation testing against the PR's own suites (npx vitest run --root packages/core src/hooks):

mutation	result
drop the (?![0-9A-Za-z_\p{L}\p{N}]) lookahead	killed (2 tests)
drop if (shellType === 'bash') return command;	killed (1 test)
$(?:QWEN|GEMINI|CLAUDE)_ → $(?:GEMINI|CLAUDE)_	killed (3 tests)
windowsVerbatimArguments: <cond> → false	SURVIVES — 760 passed, 4 skipped

The expandCommand tests have real teeth. The windowsVerbatimArguments line — the change that actually repairs Windows — is covered only by the new describe.runIf(process.platform === 'win32') block, which is skipped on Linux/macOS, and .github/workflows/ci.yml's test_windows job runs only on merge_group / schedule / workflow_dispatch, never on pull_request. So it is gated at the merge queue but invisible on this PR — please expect the merge-queue Windows run to be the first real execution of those four cases.

5. Minor

$QWEN_PROJECT_DIR is substituted from input.cwd and therefore ignores a hook-level env: { QWEN_PROJECT_DIR: … } override, while %QWEN_PROJECT_DIR% in the same hook honours it — the two forms now disagree. Pre-existing for GEMINI/CLAUDE, newly inherited by QWEN; worth a doc line rather than a code change.

Recommendation

Very close. Please (a) restore escaped substitution for bash, or justify the change explicitly and add a POSIX regression test; (b) take the one-line "${command}" wrapper so the documented placeholder-first form works; (c) mention in the PR body that this also fixes every cmd hook containing a double quote — that is the headline. Happy to re-run the whole matrix on the updated branch.

Harness & limitations

• Worktrees /var/tmp/pr10393-{base,wt}, driver esbuild-bundled from each worktree's packages/core/src/hooks/hookRunner.ts (bundles verified to differ only by this PR's hunks).
• Windows behaviour is reproduced under Wine's cmd.exe, not Microsoft's. Every claim above rests on the documented cmd /? /S quoting rule and was cross-checked with a direct spawn probe. One observation I explicitly do not assert for real Windows: a plain-spaces path (no &/(/)) in the placeholder-first form also fails under Wine, but real cmd.exe retries longer command tokens and may recover it — the metacharacter case above does not depend on that.
• PowerShell hooks were exercised with PowerShell 7.4 win-x64 under Wine, not Windows PowerShell 5.1.

中文版报告

PR #10393 真实运行时验证（维护者复核）

我没有只读 diff，而是搭建了真实的 A/B 运行环境：两个 arm 都驱动 worktree 里真实的 HookRunner.executeHook()（base d853f09f52 vs head 7793f6f323），并真正拉起子 shell：

• Windows arm —— Wine 下的 node.exe win-x64 v22.22.2，因此 process.platform === 'win32'、ComSpec = C:\windows\system32\cmd.exe，是真实的 cmd.exe、真实的 spawn() 参数编排；PowerShell 钩子用 PowerShell 7.4。
• POSIX arm —— 同一个 driver 跑在原生 Linux/bash 上。
• 两个 arm 共 45 个场景；项目目录刻意命名为 qwen hook (project) & 'quoted' …、qwen my project … 以及一个普通目录。

结论：Windows 侧的修复是真实有效的，而且修好的范围比 PR 描述的更大；但 expandCommand 对 bash 的提前返回在 Linux/macOS 上引入了未被说明的回归，同时我们文档里教的那种钩子写法在 Windows 上依然不能用。

1. PR 确实修好的部分（真实 cmd.exe / PowerShell 验证）

$QWEN_PROJECT_DIR 在 cmd 和 PowerShell 下都能解析了。更关键的是 windowsVerbatimArguments 修掉了一个比描述中大得多的历史 bug：在 main 上，libuv 会给 /c 参数加引号并把每个 " 转义成 \"，因此任何命令文本里含双引号的 cmd 钩子今天都是坏的：

场景	base	head
echo "hello world"	\"hello world\"	"hello world"
if exist "%QWEN_PROJECT_DIR%" (...)（纯原生 %VAR%，不涉及占位符）	MISSING	FOUND
cd /d $QWEN_PROJECT_DIR && echo MIDOK	exit 1，File not found.	MIDOK

建议把这一点写进 PR 描述——这才是这个改动最有力的理由。

2. 回归：Linux/macOS 上 $GEMINI_PROJECT_DIR / $CLAUDE_PROJECT_DIR 不再安全

expandCommand() 现在对 shellType === 'bash' 原样返回。而在 main 上这两个占位符是经 escapeShellArg(cwd, 'bash') 替换的，也就是做过 shell 转义。改动后交给子进程里未加引号的参数展开，只要项目路径含空格，bash 就会按空格拆词：

场景                    base              head
posix-gemini-spaces    FOUND       ->    MISSING
posix-claude-spaces    FOUND       ->    MISSING
posix-doc-hook-gemini  HOOK-SCRIPT-RAN -> exit 127
                       stderr: bash: line 1: /tmp/qwen: No such file or directory

这是一个定位为 Windows 修复的 PR 在默认平台上的行为变更，而 "Tested on" 表格里 Linux/macOS 只写了 unit tests only。原有的两个 expandCommand 测试之所以还能通过，是因为 PR 给它们补了 shell: 'powershell'——这正是"已文档化的 POSIX 行为被改了"的信号。

PR 给出的动机（注释、单引号字面量、$..._DIRS 后缀保持 bash 原生语义）是成立的，我也确认了这部分确实变好了；但其中 $QWEN_PROJECT_DIRS 那一半，本 PR 新加的 (?!…) 边界断言已经解决了。对 bash 保留转义替换（同时保留新的边界断言和新增的 QWEN 分支）就能既拿到边界修复、又不破坏含空格的项目路径。如果这个提前返回是有意为之，需要在 PR 描述里单独说明、补一个 POSIX 回归测试，并提示路径含空格的 macOS 用户这些钩子会静默失效。

3. 遗留缺口：文档里教的那种写法在 Windows 上仍然失败

docs/users/features/hooks.md 里出现的写法，占位符都在命令开头：

"command": "$QWEN_PROJECT_DIR/.qwen/hooks/security-check.sh"

cmd /S /C 会剥掉命令行的第一个和最后一个引号（cmd /? 有明确说明）。由于展开产生的是带引号的 "C:\…project…"，占位符位于命令开头时这层引号恰好被吃掉；如果项目路径里含 &，& 之后的内容会被当成独立命令执行：

项目目录: C:\…\Temp\proj & echo INJECTED-PIBToR
钩子:     $GEMINI_PROJECT_DIR\.qwen\hooks\check.cmd
base -> exit 9009, stdout 空                            （引号还在，只是命令没找到）
head -> exit 9009, stdout "INJECTED-PIBToR\.qwen\…"     （路径里 `&` 之后的内容被执行了）

注意这正是 PR 自己 fixture 用的那类目录名（qwen hook (project) & 'quoted' ）。PR 新增测试能过，只是因为 if exist $QWEN_PROJECT_DIR … 并不以引号开头。

一行改动即可闭合，我在同一环境验证过——把命令再包一层引号，让 /S 去吃这层外层引号：

[...shellConfig.argsPrefix, useVerbatim ? `"${command}"` : command]

这样文档里的占位符开头写法可以正常执行（HOOK-SCRIPT-RAN，exit 0），而 PR 自己的 if exist $QWEN_PROJECT_DIR … 形态不受影响（仍为 FOUND）。

4. 真正承载修复的那行代码的测试覆盖

对 PR 自带用例做变异测试（npx vitest run --root packages/core src/hooks）：

变异	结果
删掉 (?![0-9A-Za-z_\p{L}\p{N}]) 断言	被杀死（2 个用例）
删掉 if (shellType === 'bash') return command;	被杀死（1 个用例）
$(?:QWEN|GEMINI|CLAUDE)_ → $(?:GEMINI|CLAUDE)_	被杀死（3 个用例）
windowsVerbatimArguments: <cond> → false	存活 —— 760 passed、4 skipped

expandCommand 的用例有真牙齿；但真正修好 Windows 的 windowsVerbatimArguments 只被新增的 describe.runIf(process.platform === 'win32') 覆盖，该块在 Linux/macOS 上被跳过，而 .github/workflows/ci.yml 的 test_windows 只在 merge_group / schedule / workflow_dispatch 触发，不在 pull_request 上跑。所以它在合并队列里是有门禁的，但在本 PR 上完全不可见——这 4 个用例的首次真实执行会发生在合并队列。

5. 次要问题

$QWEN_PROJECT_DIR 是从 input.cwd 替换的，因此会忽略 hook 级 env: { QWEN_PROJECT_DIR: … } 覆盖；而同一个钩子里的 %QWEN_PROJECT_DIR% 会遵循该覆盖——两种写法现在不一致。这对 GEMINI/CLAUDE 是既有行为，QWEN 只是继承了它；补一句文档即可，不必改代码。

结论与建议

已经很接近可合并了。建议：(a) 对 bash 恢复转义替换，或明确说明该变更并补 POSIX 回归测试；(b) 采纳 "${command}" 这一行改动，让文档里的占位符开头写法可用；(c) 在 PR 描述里点明这同时修复了所有含双引号的 cmd 钩子——这才是最大的价值点。分支更新后我可以把整套矩阵重跑一遍。

环境与局限

• Worktree /var/tmp/pr10393-{base,wt}，driver 由各自 worktree 的 packages/core/src/hooks/hookRunner.ts 用 esbuild 打包（已核对两个 bundle 仅差本 PR 的 hunk）。
• Windows 行为是在 Wine 的 cmd.exe 上复现的，不是微软的 cmd.exe。上述结论都基于 cmd /? 中记录的 /S 引号规则，并用直接的 spawn 探针交叉验证。有一条我不声称适用于真实 Windows：仅含空格（不含 &/(/)）的路径在占位符开头写法下于 Wine 中也失败，但真实 cmd.exe 会尝试更长的命令 token，可能可以恢复——上面关于元字符的结论不依赖这一点。
• PowerShell 钩子用的是 Wine 下的 PowerShell 7.4 win-x64，不是 Windows PowerShell 5.1。

[chandlerm923]

Applied the suggested "${command}" wrapper exactly in b48d078047. I also restored escaped Bash placeholder substitution and updated the PR description to mention the double-quote cmd hook behavior identified in your review.

On Microsoft cmd.exe, the placeholder-first case still fails because the expanded path is "C:\project path"\hook.cmd. Quoting the complete executable path as "C:\project path\hook.cmd" resolves it. Would you prefer that additional adjustment in this PR?

[wenshao]

Re-verified at b48d078047 — round 2

Same harness as before, now a four-arm matrix over 63 scenarios: main d853f09f52 · v1 7793f6f323 (what I reviewed) · v2 b48d078047 (current head) · a proposal arm. Windows arm is node.exe win-x64 under Wine (process.platform === 'win32', real cmd.exe, PowerShell 7.4); POSIX arm is native Linux/bash. Both drive the real HookRunner.executeHook() built from each revision.

Both of my round-1 findings are fixed, and the fix for one of them opened a new regression on the other side. One blocking item, one nit, and a direct answer to your cmd.exe question.

✅ Confirmed fixed

• Finding Where is the config saved? #2 (POSIX): $GEMINI_/CLAUDE_PROJECT_DIR are escaped again — posix-gemini-spaces, posix-claude-spaces back to FOUND; and $QWEN_PROJECT_DIR now works unquoted too (MISSING → FOUND, exit 127 → HOOK-SCRIPT-RAN). Re-adding the bash early return is now killed by a test — real teeth.
• Finding 如何自定义密钥文件 .env可能与其他文件冲突 #3 (Windows): the documented placeholder-first form runs, including forward slashes and a project path containing & — the INJECTED-… command-splitting I demonstrated is gone.

win-cmd-doc-hook, -spacey, -fwdslash, -fwdslash-spacey, win-cmd-amp-dochook-*, win-cmd-literal-quoted-exe: all FAIL 9009 on main → HOOK-SCRIPT-RAN on b48d078047. Every v1 win is kept (redirect, pipe, chain, unbalanced-quote, trailing-backslash, native-percent).

❌ Blocking: the quoted placeholder now regresses against main on Linux/macOS

Restoring escaped substitution also added QWEN to it, and $QWEN_PROJECT_DIR was a plain env var for bash on main. So the idiomatic, correct shell form breaks:

ls -d "$QWEN_PROJECT_DIR"   ->   ls -d "'/tmp/qwen hook (project) & …'"    exit 2

scenario	main	v1	v2 b48d078047
ls -d "$QWEN_PROJECT_DIR"	FOUND	FOUND	exit 2
"$QWEN_PROJECT_DIR/.qwen/hooks/check.sh"	ran	ran	exit 2
"$QWEN_PROJECT_DIR/.qwen/hooks/check.sh" (spaces-only path)	ran	ran	exit 127
cd "$QWEN_PROJECT_DIR" && pwd	ok	ok	exit 2
echo '[$QWEN_PROJECT_DIR]'	[$QWEN_PROJECT_DIR]	[$QWEN_PROJECT_DIR]	[/tmp/qwenplain…]

Quoting "$VAR" is what every shell style guide and our own hook authors will write, so this hits real hooks. The same shape gets worse on Windows too — where main returned a benign wrong answer, b48d078047 is a hard parse error:

if exist "$QWEN_PROJECT_DIR" (echo FOUND) else (echo MISSING)   ->  cmd: Syntax error: unexpected (   (exit 255)
if (Test-Path '$QWEN_PROJECT_DIR') { … }                        ->  pwsh: ParserError … ''C:\users\root\…''

One change fixes all of it: make expandCommand quote-aware

Scan the command once, tracking single/double-quote regions, and pick the substitution per region instead of always emitting the escaped form:

region	substitute
inside "…"	the bare path (cmd/PowerShell have no $VAR there; bash re-quotes it itself)
inside '…'	bash: leave the placeholder literal · PowerShell: bare path with '' doubling
elsewhere	escapeShellArg(cwd, shell) — exactly today's behaviour

~25 lines replacing the single .replace(). In my matrix that arm is green on every placeholder scenario across bash, cmd and PowerShell (last column of Fig 6/7) — it is a strict superset of main, v1 and v2, and it also fixes the pre-existing "$GEMINI_PROJECT_DIR" wart that main never handled. The PR's existing suite passes unchanged under it: 760 passed | 5 skipped (765), no test edits.

Your question: should the complete executable path be quoted?

Yes — take it, and it is free. Fig 9 shows the literal text the expansion produces today versus with the adjustment:

b48d0780   "C:\…\qwen my project 5TXiY0"\.qwen\hooks\check.cmd
proposal   "C:\…\qwen my project QsH86Y\.qwen\hooks\check.cmd"

It is ~8 more lines on top of the quote-aware scan (cmd only, outside quotes: consume the literal suffix up to the first cmd delimiter and put it inside the same quote pair). I ran that arm too — identical results on all 63 scenarios, suite still 760 | 5 skipped. I cannot run Microsoft's cmd.exe, so your real-Windows observation should win over my Wine result; the point is that this form is correct under both parsings, so it is the safe one to ship.

Nits

• packages/core/src/hooks/hook-runner.process.test.ts:135 still has a debug statement: if (!result.success) console.log('placeholder-first result', result);
• Coverage is unchanged from round 1: mutating either the cmd wrapper (useVerbatimArguments ? '"' + command + '"' : command → command) or windowsVerbatimArguments: … → false survives the whole Linux hooks suite (760 passed, 5 skipped). Those two lines are only gated by the runIf(win32) block, and ci.yml's test_windows never runs on pull_request. Not something to fix here — just know the merge-queue Windows run is their first execution.

Recommendation

Land it once the quoted-placeholder regression is closed. My preference, in order: (1) the quote-aware expandCommand + your executable-path quoting, with unit tests for "$QWEN_PROJECT_DIR", '$QWEN_PROJECT_DIR' and the bare form on each shell; (2) if you would rather keep this PR minimal, drop QWEN from the bash substitution list only — that removes the regression, though it leaves the unquoted $QWEN_PROJECT_DIR bash case unfixed. Either way the Windows half is in good shape now. Happy to re-run the full matrix on whatever you push.

中文版报告

在 b48d078047 上重新验证 —— 第二轮

沿用上一轮的环境，这次是四臂矩阵、63 个场景：main d853f09f52 · v1 7793f6f323（我上轮 review 的版本）· v2 b48d078047（当前 head）· 一个提案臂。Windows 臂是 Wine 下的 node.exe win-x64（process.platform === 'win32'、真实 cmd.exe、PowerShell 7.4），POSIX 臂是原生 Linux/bash；两臂都驱动各自版本编译出的真实 HookRunner.executeHook()。

上轮两个问题都修好了，但修好其中一个的同时在另一侧引入了新的回归。 下面是一个阻塞项、一个小问题，以及对你关于 cmd.exe 提问的直接答复。

✅ 已确认修复

• 问题 Where is the config saved? #2（POSIX）：$GEMINI_/CLAUDE_PROJECT_DIR 恢复了转义替换——posix-gemini-spaces、posix-claude-spaces 回到 FOUND；并且 $QWEN_PROJECT_DIR 在未加引号时也能用了（MISSING → FOUND，exit 127 → HOOK-SCRIPT-RAN）。把 bash 提前返回加回去现在会被测试杀死——测试有真牙齿了。
• 问题 如何自定义密钥文件 .env可能与其他文件冲突 #3（Windows）：文档里"占位符开头"的写法可以跑通了，包括正斜杠以及项目路径含 & 的情况——我上轮演示的 INJECTED-… 命令拆分已经消失。

win-cmd-doc-hook、-spacey、-fwdslash、-fwdslash-spacey、win-cmd-amp-dochook-*、win-cmd-literal-quoted-exe 在 main 上全是 FAIL 9009，在 b48d078047 上全部 HOOK-SCRIPT-RAN；v1 的收益也都保留了（redirect、pipe、chain、unbalanced-quote、trailing-backslash、native-percent）。

❌ 阻塞项：加引号的占位符写法相对 main 出现回归（Linux/macOS）

恢复转义替换的同时把 QWEN 也加进了替换列表，而在 main 上 $QWEN_PROJECT_DIR 对 bash 来说只是一个普通环境变量。于是符合规范的写法反而坏了：

ls -d "$QWEN_PROJECT_DIR"   ->   ls -d "'/tmp/qwen hook (project) & …'"    exit 2

场景	main	v1	v2 b48d078047
ls -d "$QWEN_PROJECT_DIR"	FOUND	FOUND	exit 2
"$QWEN_PROJECT_DIR/.qwen/hooks/check.sh"	执行成功	执行成功	exit 2
同上（仅含空格的路径）	执行成功	执行成功	exit 127
cd "$QWEN_PROJECT_DIR" && pwd	正常	正常	exit 2
echo '[$QWEN_PROJECT_DIR]'	[$QWEN_PROJECT_DIR]	[$QWEN_PROJECT_DIR]	[/tmp/qwenplain…]

给变量加引号 "$VAR" 是所有 shell 规范、也是我们的钩子作者会写的写法，所以这会影响真实的钩子。同样的写法在 Windows 上也变得更糟——main 只是返回一个良性的错误结果，而 b48d078047 直接是解析错误：

if exist "$QWEN_PROJECT_DIR" (echo FOUND) else (echo MISSING)   ->  cmd: Syntax error: unexpected (   (exit 255)
if (Test-Path '$QWEN_PROJECT_DIR') { … }                        ->  pwsh: ParserError … ''C:\users\root\…''

一个改动可以全部解决：让 expandCommand 感知引号

扫描命令一次，跟踪单/双引号区域，按区域选择替换方式，而不是永远输出转义形式：

区域	替换为
位于 "…" 内	裸路径（cmd/PowerShell 在这里没有 $VAR 语义；bash 自己会正确处理引号）
位于 '…' 内	bash：保留占位符字面量 · PowerShell：裸路径并把 ' 加倍
其他位置	escapeShellArg(cwd, shell) —— 与现在完全一致

约 25 行，替换掉那一处 .replace()。在我的矩阵里这个臂在 bash + cmd + PowerShell 的所有占位符场景上全绿（图 6/7 最后一列），是 main、v1、v2 三者的严格超集，并且顺带修好了 main 从未处理过的 "$GEMINI_PROJECT_DIR" 老问题。PR 现有用例在该改动下无需修改即可全部通过：760 passed | 5 skipped (765)。

回答你的问题：是否应该把完整可执行路径包进引号？

应该，而且这个改动是"免费"的。 图 9 展示了当前展开产生的字面文本与调整后的对比：

b48d0780   "C:\…\qwen my project 5TXiY0"\.qwen\hooks\check.cmd
提案       "C:\…\qwen my project QsH86Y\.qwen\hooks\check.cmd"

在感知引号的扫描之上再加约 8 行即可（仅 cmd、仅引号外：把占位符后面直到第一个 cmd 分隔符的字面后缀一起放进同一对引号）。我把这个臂也跑了一遍——63 个场景结果完全一致，用例仍然 760 | 5 skipped。我无法运行微软的 cmd.exe，所以在这一点上应以你的真实 Windows 观察为准；关键在于这种形式在两种解析下都正确，因此是更安全的选择。

小问题

• packages/core/src/hooks/hook-runner.process.test.ts:135 还留着一行调试语句：if (!result.success) console.log('placeholder-first result', result);
• 覆盖情况和上一轮一样：把 cmd 包装（useVerbatimArguments ? '"' + command + '"' : command）改成 command，或把 windowsVerbatimArguments 改成 false，都能在整个 Linux hooks 套件下存活（760 passed、5 skipped）。这两行只被 runIf(win32) 块覆盖，而 ci.yml 的 test_windows 从不在 pull_request 上触发。这不需要在本 PR 里解决，知悉即可：合并队列里的 Windows 运行会是它们的首次真实执行。

结论与建议

把"加引号的占位符"这个回归解决后即可合并。我的偏好排序：(1) 采用感知引号的 expandCommand + 你提出的可执行路径加引号，并为每种 shell 补上 "$QWEN_PROJECT_DIR"、'$QWEN_PROJECT_DIR' 和裸写法的单测；(2) 如果希望本 PR 保持最小改动，就只把 QWEN 从 bash 的替换列表里去掉——这样能消除回归，但未加引号的 $QWEN_PROJECT_DIR bash 场景就仍然没修。无论选哪种，Windows 这一半现在的状态都很好。你推新的提交后我可以把整套矩阵再跑一遍。

[chandlerm923]

Implemented option (1): quote-aware expandCommand.

expandProjectDirPlaceholders scans the command once, tracks the quote region a placeholder sits in, and substitutes per-region instead of always emitting the escaped form (bash '...' left literal, bash/cmd/powershell "..." get the bare path with region-appropriate escaping, everything else unchanged via escapeShellArg).

Also applied the executable-path quoting from my earlier comment (#5461194729) — findCmdTokenEnd now pulls the literal suffix after the placeholder into the same quote pair, so $QWEN_PROJECT_DIR/.qwen/hooks/check.cmd resolves on real cmd.exe.

Added unit tests for the quoted/unquoted cases per shell. Dropped the stray console.log at hook-runner.process.test.ts:135.

Full suite + typecheck + lint pass; ran hook-runner.process.test.ts through actual cmd.exe/PowerShell.

[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）为单个提交。

[wenshao]

Re-verified at ae9bf7402a — round 3

86 scenarios, four arms: main d853f09f52 · v2 b48d078047 · v5 ae9bf7402a (current head) · a "head + fix" arm. Windows arm is node.exe win-x64 under Wine (real cmd.exe, PowerShell 7.4); POSIX arm is native Linux/bash. New this round: a project directory literally named qwen $x `tick` "dq" \bs 'sq' … on Linux and qwen $x `tick` 'q' (a) & b, c; d=e %F% !G! … on Windows, to attack the new per-region escaping.

The quote-aware implementation is right, and the tests are now genuinely strong. Every round-2 regression is gone and nothing from rounds 1–2 came back. Two small things remain, both in the new scanner, and both fixed by ~15 lines I verified.

✅ Everything from rounds 1 and 2 is closed

"$QWEN_PROJECT_DIR", '$QWEN_PROJECT_DIR', the bare form, the documented placeholder-first form, &-in-path, forward slashes, suffix-plus-arguments, suffix-plus-redirect — all green on bash, cmd and PowerShell, including on the metacharacter-heavy project directory. win-ps-double-quoted and win-ps-meta-quoted never worked on any earlier revision and work now. The stray console.log is gone.

Mutation teeth are much better than last round — 5 of 6 mutants killed, including the cmd wrapper and each per-region branch:

mutation	result
cmd wrapper '"' + command + '"' → command	killed (2 tests)
bash "…" branch → escapeShellArg	killed (2 tests)
findCmdTokenEnd → return start	killed (1 test)
bash '…' branch → escapeShellArg	killed (1 test)
PowerShell "…" branch → escapeShellArg	killed (1 test)
windowsVerbatimArguments: … → false	SURVIVES — win32-only, as before

⚠️ 1. The bash scanner does not know about comments or backslash escapes

expandProjectDirPlaceholders toggles inSingleQuote / inDoubleQuote on every ' and ", including ones inside a # comment or preceded by a backslash. An apostrophe in a comment — # don't touch this — is common, and it makes the scanner believe the rest of the command is inside a single-quoted region:

scenario	main	v2	v5 ae9bf7402a
# don't touch this ⏎ ls -d $GEMINI_PROJECT_DIR …	FOUND	FOUND	MISSING
# note: 24" wide ⏎ ls -d $GEMINI_PROJECT_DIR …	FOUND	FOUND	exit 2
echo \" >/dev/null; ls -d $QWEN_PROJECT_DIR …	MISSING	FOUND	exit 2

The first two are regressions against main; the third is a regression against b48d078047. When the project path contains a shell metacharacter the failure is hard, not silent:

bash: -c: line 2: syntax error near unexpected token `('
bash: -c: line 2: `ls -d /tmp/qwen hook (project) & 'quoted' …'

Controls behave correctly, so the region tracking itself is sound — # see the "docs" first (balanced), echo "don't", and echo 'say "hi"' all stay FOUND.

Fix (~12 lines, inside the existing character loop): skip the character after an escape char (\ for bash, ` for PowerShell, ^ for cmd) when not inside single quotes, and treat an unquoted # at a word boundary as a comment running to the next newline — leaving any placeholder inside a comment as written. Last column of Fig 10 is that arm: all three rows green, everything else unchanged.

⚠️ 2. findCmdTokenEnd can swallow the next placeholder

Its delimiter set is [ \t & | < > ( )]. cmd also separates arguments on , ; =, and " is not a delimiter either, so the absorbed suffix can contain a quote and desync the scanner. Concretely, a second placeholder gets pulled inside the first quoted token and is never expanded:

hook:  echo $QWEN_PROJECT_DIR;$QWEN_PROJECT_DIR
v2     "C:\…\qwenplainxNpfif";"C:\…\qwenplainxNpfif"
v5     "C:\…\qwenplaintywhOM;$QWEN_PROJECT_DIR"        <- second placeholder left literal
fix    "C:\…\qwenplainN4Bcac";"C:\…\qwenplainN4Bcac"

The realistic shape is set PATH=$QWEN_PROJECT_DIR\bin;%PATH%, which today expands to set PATH="C:\proj\bin;%PATH%" — the quote ends up inside the value. echo A=$QWEN_PROJECT_DIR=B similarly becomes A="C:\…=B" instead of A="C:\…"=B.

Fix: add , ; = " ^ to the delimiter set. I ran that arm across all 55 Windows scenarios — identical to ae9bf7402a everywhere else, and both cases above corrected.

Both fixes together keep the PR's own suite exactly as it is: 767 passed | 5 skipped (772), no test edits.

Note for the docs, not a bug

'$QWEN_PROJECT_DIR' / '$GEMINI_PROJECT_DIR' are now left literal on bash (correct shell semantics, and intended by this design), where main substituted the path. Worth one line in docs/users/features/hooks.md alongside the placeholder description, since it is a deliberate behaviour change.

Recommendation

Add the two scanner fixes and this is good to merge from my side — the design is right and I could not break it any other way across 86 scenarios on three shells. If you would rather land the current head and follow up, say so and I will file the two edge cases as an issue instead; they are much narrower than what rounds 1–2 turned up. Either way, test_windows still only runs in the merge queue, so that run will be the first real execution of the new runIf(win32) cases.

中文版报告

在 ae9bf7402a 上重新验证 —— 第三轮

86 个场景、四臂：main d853f09f52 · v2 b48d078047 · v5 ae9bf7402a（当前 head） · 一个"head + 修复"臂。Windows 臂是 Wine 下的 node.exe win-x64（真实 cmd.exe、PowerShell 7.4），POSIX 臂是原生 Linux/bash。本轮新增：Linux 上项目目录字面命名为 qwen $x `tick` "dq" \bs 'sq' …，Windows 上为 qwen $x `tick` 'q' (a) & b, c; d=e %F% !G! …，专门冲击新的分区域转义逻辑。

感知引号的实现方向是对的，测试这次也真的有牙齿。 第二轮的回归全部消失，第一、二轮的问题也没有复发。还剩两个小问题，都在新的扫描器里，我都验证了修复（约 15 行）。

✅ 第一、二轮的问题全部关闭

"$QWEN_PROJECT_DIR"、'$QWEN_PROJECT_DIR'、裸写法、文档里的占位符开头写法、路径含 &、正斜杠、后缀加参数、后缀加重定向——在 bash、cmd、PowerShell 上全绿，包括在元字符密集的项目目录下。win-ps-double-quoted 和 win-ps-meta-quoted 在此前任何版本上都不工作，现在可以了。遗留的 console.log 也已删除。

变异测试的牙齿比上一轮强得多——6 个变异杀死 5 个，包括 cmd 包装和每个分区域分支：

变异	结果
cmd 包装 '"' + command + '"' → command	被杀死（2 个用例）
bash "…" 分支 → escapeShellArg	被杀死（2 个用例）
findCmdTokenEnd → return start	被杀死（1 个用例）
bash '…' 分支 → escapeShellArg	被杀死（1 个用例）
PowerShell "…" 分支 → escapeShellArg	被杀死（1 个用例）
windowsVerbatimArguments: … → false	存活 —— 仍然只有 win32 覆盖

⚠️ 1. bash 扫描器不认识注释和反斜杠转义

expandProjectDirPlaceholders 对每一个 ' 和 " 都会翻转 inSingleQuote / inDoubleQuote，包括位于 # 注释里或被反斜杠转义的那些。注释里出现撇号——# don't touch this——非常常见，这会让扫描器认为后面整条命令都处于单引号区域：

场景	main	v2	v5 ae9bf7402a
# don't touch this ⏎ ls -d $GEMINI_PROJECT_DIR …	FOUND	FOUND	MISSING
# note: 24" wide ⏎ ls -d $GEMINI_PROJECT_DIR …	FOUND	FOUND	exit 2
echo \" >/dev/null; ls -d $QWEN_PROJECT_DIR …	MISSING	FOUND	exit 2

前两条是相对 main 的回归，第三条是相对 b48d078047 的回归。当项目路径含 shell 元字符时，失败是硬失败而非静默：

bash: -c: line 2: syntax error near unexpected token `('
bash: -c: line 2: `ls -d /tmp/qwen hook (project) & 'quoted' …'

对照组行为正确，说明区域跟踪本身是对的——# see the "docs" first（引号成对）、echo "don't"、echo 'say "hi"' 都仍是 FOUND。

修复（约 12 行，就在现有字符循环里）：在非单引号区域内遇到转义字符（bash 用 \、PowerShell 用 `、cmd 用 ^）时跳过下一个字符；并把词边界上未被引号包裹的 # 视为注释直到下一个换行——注释里的占位符保持原样。图 10 最后一列就是这个臂：三行全绿，其他一切不变。

⚠️ 2. findCmdTokenEnd 会吞掉下一个占位符

它的分隔符集合是 [ \t & | < > ( )]。cmd 还会在 , ; = 上切分参数，而且 " 也不在集合里，所以被吸收的后缀可能含引号并让扫描器失步。具体表现是第二个占位符被拉进第一个引号 token 内、永远不会被展开：

钩子:  echo $QWEN_PROJECT_DIR;$QWEN_PROJECT_DIR
v2     "C:\…\qwenplainxNpfif";"C:\…\qwenplainxNpfif"
v5     "C:\…\qwenplaintywhOM;$QWEN_PROJECT_DIR"        <- 第二个占位符成了字面量
修复   "C:\…\qwenplainN4Bcac";"C:\…\qwenplainN4Bcac"

现实中的形态是 set PATH=$QWEN_PROJECT_DIR\bin;%PATH%，现在会展开成 set PATH="C:\proj\bin;%PATH%"——引号跑进了变量值里。echo A=$QWEN_PROJECT_DIR=B 同样会变成 A="C:\…=B" 而不是 A="C:\…"=B。

修复：把 , ; = " ^ 加进分隔符集合。我把该臂在全部 55 个 Windows 场景上跑了一遍——除上述两处外与 ae9bf7402a 完全一致。

两处修复合在一起，PR 自己的用例仍然原样通过：767 passed | 5 skipped (772)，无需改动测试。

一条文档提示，不是 bug

'$QWEN_PROJECT_DIR' / '$GEMINI_PROJECT_DIR' 在 bash 下现在保持字面量（这是正确的 shell 语义，也是本设计的意图），而 main 会替换成路径。建议在 docs/users/features/hooks.md 的占位符说明处补一句，因为这是一个有意的行为变更。

结论与建议

补上这两处扫描器修复，我这边就可以合并了——设计方向是对的，我在三种 shell、86 个场景下没能再找出别的破法。如果你更希望先合入当前 head、后续再跟进，告诉我一声，我会把这两个边界情况另开 issue；它们比前两轮发现的问题窄得多。无论怎么选，test_windows 仍然只在合并队列里跑，那次运行会是新增 runIf(win32) 用例的首次真实执行。

[chandlerm923]

Added the two scanner fixes from round 3 (comment/escape handling, delimiter set).

Merged main to resolve a conflict with #10288's hook supervisor, which landed without this PR's windowsVerbatimArguments fix — re-threaded it through both spawn paths and added a real-cmd.exe test for the supervisor path (previously no per-PR win32 coverage there).

Also extended comment detection to PowerShell (it uses # too) and added newline to findCmdTokenEnd's delimiter set.

Full suite + typecheck + lint pass; verified on real cmd.exe/PowerShell.

[qwen-code-ci-bot]

Not explored to full depth (tool budget reached): "agent 1a": running packages/core vitest suites ( hookRunner.test.ts , hook-runner.process.test.ts ) to confirm the new tests pass — node_modules is not installed in t….

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

[qwen-code-ci-bot]

[Critical] R4-1: [certifies-falsely] [regression] The quote tracker that decides how each placeholder is substituted models only flat quote regions, # comments and one escape character — it has no state for bash heredocs, $(…)/backtick command substitution, $'…' ANSI-C quoting, PowerShell here-strings, or PowerShell <# #> block comments. A quote character inside one of those hidden contexts flips the tracked state, and a later genuinely-unquoted placeholder is then spliced with the escaping of the wrong region — with a hostile session cwd (a malicious archive top-level folder, a shared CI workspace) that becomes command execution in the hook. This is the same unbounded, grammar-shaped surface the round-1 blocker (comment 3880639989) named: the quote-aware scanner closed that round's demonstrated entrances — the maintainer's round-2/round-3 matrices verified it — but three new entrances were demonstrated by execution at this commit, so the class itself remains open.

Concrete shape: a bash hook cat <<'EOF'\n"\nEOF\necho $QWEN_PROJECT_DIR run from a project directory named /tmp/x;touch CANARY;# — the tracker reads the heredoc body's lone " as opening a double-quote region, splices the cwd escaping only \ $ ", and the injected touchruns. Also demonstrated:wc -c "$(cat $QWEN_PROJECT_DIR/log.txt)"with a spacey cwd (the space lands unescaped inside the substitution body, so even benign spacey paths break), andprintf $'it's' 'cat $QWEN_PROJECT_DIR', where the $'…'parity flip leaves the path unquoted (canary created as well). Pre-PR none of these fired: GEMINI/CLAUDE were always wrapped viaescapeShellArgand$QWEN_PROJECT_DIR` was left to native bash expansion.

Witness:

PR arm (executeHook + real bash, hostile cwd):
  expansion = cat <<'EOF'\n"\nEOF\necho /tmp/r41-Tjbr0c/x;touch /tmp/r41-Tjbr0c/CANARY;#
  result    = { success: true, exitCode: 0, canaryExists: true }
BASE arm (pre-PR revert): canaryExists: false

Close it structurally rather than entrance by entrance: splice a fully-escaped token that closes and reopens the author's quotes (bash double-quote region: emit " + escapeShellArg(cwd, 'bash') + " so the author's region resumes), or teach the tracker the missing constructs (heredoc <<['"]?WORD, $(…), $'…', PS here-strings, PS <# #>), or constrain substitution to the documented isolated-token forms and leave every other context to native expansion — the three variables are already exported into the hook env (hookRunner.ts:1088-1090). The invariant to restore: no cwd byte may reach the command text outside some form of quoting, regardless of tracker state.

The fix must keep the non-nesting behavior pinned by this diff's own test splices a bare path into a bash double-quoted placeholder instead of nesting quotes (expects ls -d "/tmp/qwen hook (project)", hookRunner.test.ts:1082). Please pin the fix with a test: command cat <<'EOF'\n"\nEOF\necho $QWEN_PROJECT_DIR with cwd /tmp/x;touch pwned;# must produce output with no raw ; outside quotes — removing the fix must turn that test red.

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

[qwen-code-ci-bot]

[Suggestion] R4-8: Four new cmd.exe tests in this describe('expandCommand') block each paste the identical ~22-line setup/teardown block (platform spy, MSYSTEM save+delete, ComSpec save+set, and the symmetric finally restore) — this anchor resolves to the first copy, but the same block repeats at ~1269, ~1314 and ~1360. The env contract of getShellConfiguration() is now encoded in 4 copies — and that contract also reads TERM (shell-utils.ts:186), which these tests do not control.

Any change to the mock contract must then be applied in 4 places; missing one leaves that test silently selecting a bash/PowerShell config (a TERM containing msys/cygwin selects Git-Bash) while asserting cmd-shaped expectations — failing only on environments where the leaked env var differs.

Witness:

grep: four vi.spyOn(process, 'platform', 'get').mockReturnValue('win32') sites
      inside expandCommand (hookRunner.test.ts:1223/1269/1314/1360),
      each followed line-for-line by the same MSYSTEM/ComSpec save-set-restore block
shell-utils.ts:186: const term = process.env['TERM'] || '';

Consider grouping the four tests in a nested describe('cmd shell configuration', …) and doing the platform spy plus MSYSTEM/ComSpec save/restore once in beforeEach/afterEach (or a local withCmdShell(body) helper in the file). The four existing tests pin the behavior before and after such a refactor.

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

[qwen-code-ci-bot]

[Suggestion] R4-13: Adjacent bare placeholders expand wrongly: $ is not in findCmdTokenEnd's delimiter set, so this cmd suffix absorption swallows the second placeholder as literal text that is never expanded (cmd has no $VAR semantics — the hook receives the literal $GEMINI_PROJECT_DIR inside the token), and the PowerShell bare splice emits two adjacent single-quoted literals that PS tokenizes as ONE literal with a spurious ' between the paths. bash concatenates correctly. The round-3 maintainer fix pins only the ;-separated shape (test at :1220); the delimiter-less adjacency is untested.

Concrete shape: echo $QWEN_PROJECT_DIR$GEMINI_PROJECT_DIR with cwd C:\proj expands for cmd to echo "C:\proj$GEMINI_PROJECT_DIR" (second placeholder never expanded) and for PowerShell to echo 'C:\proj''C:\proj' — PS reads that as the single literal C:\proj'C:\proj.

Witness:

CMD_OUT::  "echo \"C:\\proj$GEMINI_PROJECT_DIR\""   <- second placeholder swallowed
PS_OUT::   "echo 'C:\\proj''C:\\proj'"              <- merged literal with injected quote
BASH_OUT:: "echo /test/project/test/project"        <- correct
Flip (add '$' delimiter): CMD_OUT "echo \"C:\\proj\"\"C:\\proj\"";
  the ';'-separated and placeholder-first shapes unchanged; suite 81/81 green with the fix

(Expansion layer executed; cmd/PS-side parsing is documented semantics — no cmd.exe/pwsh on this runner.)

Fix: treat a $ that begins another (QWEN|GEMINI|CLAUDE)_PROJECT_DIR match as an absorption boundary in findCmdTokenEnd (or stop the bare-branch absorption there), and coalesce a run of consecutive placeholders into one substitution before splicing, which also fixes the PS merged-literal shape. Caveat: adding $ to the delimiter set fixes only the cmd half — the PS manifestation is a separate mechanism needing adjacency-aware substitution — and it also stops absorption at any $, so a non-placeholder suffix like $QWEN_PROJECT_DIR/foo$bar.cmd would no longer be absorbed whole.

Regular suffix absorption must stay intact — the test at :1388 pins '""C:\\qwen hook & project/.qwen/hooks/check.cmd""' for the documented placeholder-first shape, and the ;-separated two-placeholder test (:1220) must keep passing. Please pin the fix with cmd and powershell tests asserting echo $QWEN_PROJECT_DIR$GEMINI_PROJECT_DIR expands with no literal $GEMINI_PROJECT_DIR remaining and no ' inserted between the spliced paths.

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

[chandlerm923]

Addressed the bot's round-4 findings: R4-2 (cmd ^ no longer suppresses quote-tracking inside "..."), R4-3 (an escaped placeholder like \$QWEN_PROJECT_DIR is left untouched), R4-4 (bash comment detection now recognizes a word boundary after ; | & < > ( ), not just whitespace; PowerShell drops the boundary requirement entirely), R4-5 (strengthened the surviving-hook cmd.exe test so its two branches are actually distinguishable), R4-6 (added a cmd unit test for a literal single quote — verified it catches the tracksSingleQuotes mutation), R4-7 (documented the placeholder expansion semantics in hooks.md), R4-9 (expandCommand now logs the expanded result, not just the input), R4-10 (cmd quote state resets on newline instead of leaking across lines).

Left R4-1 (full shell-grammar awareness: heredocs, command substitution, ANSI-C quoting, PS here-strings) and R4-13 (adjacent placeholders with no separator) open — both are real but a lot bigger than this fix, happy to file them as follow-up issues if useful. Skipped R4-8 (test setup duplication) as a pure style suggestion, not a bug.

Full suite + typecheck + lint pass; verified the strengthened cmd.exe test on real Windows.

[qwen-code-ci-bot]

Partially reviewed — gaps disclosed.

⚠️ Round 5, and the diff has grown 14.3x since this review first measured it (21 → 300 source diff lines). The findings below are anchored to the current patch, so they can only say where this approach leaks — never that a different approach would retire all of them at once. Before fixing them, a human should decide whether the shape of the change is still right. Advisory only: this does not affect the verdict, and nothing here is a blocker.

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

• R4-7 cmd/PowerShell escape-character logic untested — still standing, already reported (comment 3887117024)

Not reviewed: win32 process tests — could not run the win32-gated process tests (describe.runIf(process.platform === 'win32')) or verify the cmd verbatim-arguments path on an actual Windows host — stopped at the agent tool budget.

Not explored to full depth (tool budget reached): "agent 6a": could not run the win32 process tests ( describe.runIf(process.platform === 'win32') ) or verify the cmd verbatim-arguments path on an actual Windows host..

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

• packages/core/src/hooks/hookRunner.ts:1653 — [review] cmd/PowerShell in-quote splice escape branches unpinned — identity mutants survive the whole suite while the bash control mutant turns it red
• packages/core/src/hooks/hookRunner.ts:1555 — [review] no test places a placeholder inside a # comment — deleting the inComment early-continue keeps the suite green
• packages/core/src/hooks/hookRunner.ts:1101 — [review] the "${command}" wrap is only an identity transform because cmd's argsPrefix carries /s — cross-module invariant documented nowhere
• packages/core/src/hooks/hookRunner.ts:1653 — [review] cmd %VAR% expansion corrupts project paths containing a matched %…% pair — newly opened for QWEN-on-cmd by this diff
• packages/core/src/hooks/hookRunner.ts:1543 — [review] $$QWEN_PROJECT_DIR fuses into $'…' ANSI-C quoting — cwd backslash sequences reinterpreted by bash
• packages/core/src/hooks/hookRunner.ts:1474 — [review] Unicode identifier lookahead over-reaches for bash — $QWEN_PROJECT_DIRé falls through to raw unquoted env expansion (regresses the twins)

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

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

[qwen-code-ci-bot]

[Critical] R4-1: [certifies-falsely] [regression] Still standing from round 4 — the quote tracker that decides how each placeholder is substituted models only flat quote regions, # comments and one escape character; it has no state for bash heredocs, $(…)/backtick command substitution, $'…' ANSI-C quoting, PowerShell here-strings, or PowerShell <# #> block comments. A quote character inside one of those hidden contexts flips the tracked state, and a later genuinely-unquoted placeholder is spliced with the escaping of the wrong region. This round re-demonstrated the class by execution at this commit, and added a new consequence: the idiomatic quoted form inside "$(…)" now silently breaks on any spacey project path while still exiting 0. Patching entrances one by one cannot close this surface — five rounds of corner fixes have each opened new corners. The author deferred this in the thread, but no follow-up issue exists and no maintainer has signed off on the deferral, so it stays a blocker.

Witness:

heredoc arm (executeHook + real bash, cwd /tmp/qw-probe-dir/x y;touch /tmp/qw-pwned):
  expansion = cat <<EOF\n"\nEOF\nls -d /tmp/qw-probe-dir/x y;touch /tmp/qw-pwned
  result    = canary file created (raw splice executed)
injection arm: directory name containing \nEOF\necho PWNED\n# -> bash printed PWNED
$( ) arm: echo "$(cat "$QWEN_PROJECT_DIR/f")" (spacey cwd)
  -> echo "$(cat "'/tmp/my dir'/f")" ; cat: No such file or directory; exit 0

Close it structurally — cheapest first: (1) splice a fully-escaped token that closes and reopens the author's quotes so their region resumes; (2) teach the tracker the missing constructs (heredoc <<['"]?WORD, $(…), $'…', PS here-strings, PS <# #>); (3) constrain substitution to the documented isolated-token forms and leave every other context to native expansion — the three variables are already exported into the hook env (hookRunner.ts:1088-1090). The invariant to restore: no cwd byte may reach the command text outside some form of quoting, regardless of tracker state. The fix must keep the non-nesting splice pinned by hookRunner.test.ts:1082, and it should land with a test: command cat <<'EOF'\n"\nEOF\necho $QWEN_PROJECT_DIR with cwd /tmp/x;touch pwned;# must produce no raw ; outside quotes — removing the fix must turn that test red.

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

[qwen-code-ci-bot]

[Critical] R4-3: (fix-induced) [certifies-falsely] [regression] The round-5 fix for the cmd newline quote-leak (this unconditional per-line reset) has no exception for cmd's ^ line continuation. cmd assembles ^+newline continuations into one logical line before quote parsing, so a quoted region the author carries across a continuation is force-closed in the tracker, the later real closing quote is misread as opening one, and a bare placeholder is raw-spliced into genuinely unquoted command text. With a hostile-but-legal directory name like C:\x & calc & y the & segments execute as separate commands; a benign spacey cwd word-splits and silently corrupts the hook. At the previous reviewed head the identical input took the safely-quoted path, so this reset is what regresses it.

Witness:

PR probe (HEAD, real scanner, cmd):
  expandProjectDirPlaceholders('echo "a^\r\nb" $QWEN_PROJECT_DIR\dest', 'C:\x & calc & y')
    -> echo "a^\r\nb" C:\x & calc & y\dest        <- raw splice, path unquoted
BASE arm (round-4 head, same input):
    -> echo "a^\r\nb" "C:\x & calc & y\dest"      <- safely quoted
candidate fix (skip reset on caret continuation): flips HEAD to the quoted splice, suite 87/87 green
cmd.exe half: witness: not run — no cmd.exe/wine oracle on this Linux runner; caret-continuation semantics are documented cmd behavior

Suggested change

	if (shellType === 'cmd' && (ch === '\n' || ch === '\r')) {
	// cmd lexes line-by-line; a stray/odd quote on an earlier line
	// must not leak its quote state into the next one.
	inDoubleQuote = false;
	}
	if (shellType === 'cmd' && (ch === '\n' || ch === '\r')) {
	// cmd lexes line-by-line; a stray/odd quote on an earlier line
	// must not leak its quote state into the next one — except across
	// a `^` line continuation, which cmd assembles before quote parsing.
	const prev =
	ch === '\n' && command[i - 1] === '\r' ? command[i - 2] : command[i - 1];
	if (prev !== '^') {
	inDoubleQuote = false;
	}
	}

The round-5 test resets cmd quote tracking on a newline, since cmd lexes line-by-line pins that a caret-less newline still resets — the exception must key on the caret, not drop the reset. Please pin the fix with a cmd test for echo "a^\nb" $QWEN_PROJECT_DIR (cwd C:\my proj) asserting the unquoted-branch output echo "a^\nb" "C:\my proj" — removing the continuation exception must turn it red.

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

[qwen-code-ci-bot]

[Suggestion] R4-5: (fix-induced) The round-5 widening of the bash comment boundary tests the RAW previous character — but that character may be the escaped half of \X consumed by the escape branch, so an escaped boundary-class character followed by # is misclassified as a comment start and the rest of the line's placeholders are silently left unexpanded. Real bash treats \;/\ as literal word constituents, so # after one is mid-word, not a comment; the placeholder then expands at runtime from the exported env WITHOUT the auto-quoting this feature exists for, and a spacey cwd word-splits. The widening opened the escaped-metacharacter manifestations (\; \| \& \( \) \< \>); the escaped-whitespace one (\ ) pre-existed round 4.

Witness:

PR probe (HEAD): 'cp \;# note $QWEN_PROJECT_DIR/f /dest' -> placeholder left literal
bash oracle: bash -c 'echo TOKEN\;#MARKER comment-here' -> TOKEN;#MARKER comment-here (no comment)
runtime arm (cwd /tmp/my project): ARG=<cp> ARG=<;#> ARG=<note> ARG=</tmp/my> ARG=<project/f> ARG=</dest>
round-4-boundary mutant: the \; case flips back to cp \;# note '/tmp/my project'/f /dest; the \  case stays broken

Track the semantic word state while scanning instead of re-reading the raw string: keep a flag set when the escape branch consumes an escaped character (and on any other non-delimiter character), reset on unquoted whitespace/metacharacters, and use it in the # boundary test in place of command[i - 1]. The round-5 test at hookRunner.test.ts:1226 pins that ;# keeps starting a comment for UNESCAPED metacharacters — it must stay green. Please pin the fix with a bash test for echo \ # $QWEN_PROJECT_DIR (cwd /tmp/my dir) asserting echo \ # '/tmp/my dir' — with the raw-command[i-1] boundary still in place the assertion is red.

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

[qwen-code-ci-bot]

[Suggestion] R4-9: Still standing from round 4 — the cmd-shell environment setup/teardown block (platform spy + MSYSTEM/ComSpec save, clear, set, and the symmetric finally restore, 19 lines) is pasted verbatim into every cmd test in this file — now seven tests (round 4 said four). Any future change to shell-selection env inputs must be edited in seven places, and a miss in one leaks MSYSTEM/ComSpec state into later tests or makes that test depend on the host environment.

Witness:

grep: const previousMsystem appears exactly 7 times (lines 1293, 1339, 1384, 1429, 1472, 1516, 1562)
two blocks verified byte-for-byte identical (1290-1332 and 1559-1603)

Extract a local helper (e.g. withCmdShellEnvironment(fn)) that performs the spy + env save/clear/set and restores MSYSTEM/ComSpec in its own finally, and call it from all seven tests. The describe-level afterEach (hookRunner.test.ts:46-49) restores the platform spy but does NOT restore process.env, so the helper must keep the MSYSTEM/ComSpec restore inside its own finally so it runs even when an assertion throws.

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

[qwen-code-ci-bot]

[Suggestion] R4-11: Still standing from round 4 — for cmd, the literal-suffix absorption after a bare placeholder (findCmdTokenEnd) has no $ delimiter, so an adjacent second placeholder is swallowed as literal text inside the quoted path and never expanded, and placeholderPattern.lastIndex = suffixEnd skips the regex past it. $QWEN_PROJECT_DIR\$GEMINI_PROJECT_DIR.cmd with cwd C:\proj expands to "C:\proj\$GEMINI_PROJECT_DIR.cmd" — the hook runs against a nonexistent path, while the docs promise all three placeholder forms are expanded; ;- and space-separated variants expand both correctly.

Witness:

probe (real scanner, intact code):
  V1_ADJACENT: "$QWEN_PROJECT_DIR\$GEMINI_PROJECT_DIR.cmd" (cwd C:\proj, cmd)
    -> "C:\proj\$GEMINI_PROJECT_DIR.cmd"   <- second placeholder literal
  V1_SEMI / V1_SPACE variants -> both placeholders expanded
candidate fix capping the suffix at the next placeholder match: both expand, 87/87 green

Clamp the absorption to the next placeholder: after computing suffixEnd, also find the next placeholderPattern match at/after afterPlaceholder and use Math.min(suffixEnd, nextMatch.index) so the second placeholder is reached by the main loop (do not add $ to the delimiter set blindly — $ is a legal character in Windows path segments). The existing test absorbs the literal path suffix after a bare cmd placeholder pins $QWEN_PROJECT_DIR/.qwen/hooks/check.cmd absorption — literal suffixes must still be absorbed into the same quoted region. Please add a cmd test with command $QWEN_PROJECT_DIR\$GEMINI_PROJECT_DIR.cmd expecting both paths expanded — it goes red against the current absorption.

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

[qwen-code-ci-bot]

[Suggestion] The new paragraph reads "(including any literal path suffix immediately after it, e.g. on Windows cmd.exe)" as a general guarantee, but suffix absorption into the quoted region is implemented only in the cmd branch — the bash and PowerShell unquoted branches quote only the cwd and leave the suffix outside the quotes. A hook author writing "$QWEN_PROJECT_DIR/my scripts/check.sh" with cwd /tmp/my dir gets '/tmp/my dir'/my scripts/check.sh, which word-splits into /tmp/my dir/my + scripts/check.sh and fails with a confusing not-found — the documented suffix-quoting never happens outside cmd.exe (and even cmd stops absorbing at a space in the suffix).

Witness:

probe (real scanner):
  bash:       $QWEN_PROJECT_DIR/my scripts/check.sh (cwd '/tmp/my dir') -> '/tmp/my dir'/my scripts/check.sh
  powershell: -> 'C:/my dir'/my scripts/check.ps1
  cmd:        -> "C:/my dir/my" scripts/check.cmd   (absorbs only up to the space delimiter)

Suggested change

	the shell itself. Outside quotes the expanded path is auto-quoted for you
	(including any literal path suffix immediately after it, e.g. on Windows
	`cmd.exe`); inside `"..."` the raw path is spliced into your existing quotes.
	the shell itself. Outside quotes the expanded path is auto-quoted for you;
	on Windows `cmd.exe`, any literal path suffix immediately after the
	placeholder (e.g. `$QWEN_PROJECT_DIR/.qwen/hooks/check.cmd`) is pulled into
	the same quoted region, since cmd does not concatenate quoted and unquoted
	tokens. Inside `"..."` the raw path is spliced into your existing quotes.

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

[qwen-code-ci-bot]

[Suggestion] The docs justify leaving a single-quoted placeholder untouched by quoting bash semantics ("nothing expands inside '...'"), but the sibling PowerShell branch substitutes inside '...' (embedded ' doubled) — and PowerShell single-quoted strings have the exact same no-expansion semantics. A team sharing one hooks config across Linux and Windows developers writes validate '$QWEN_PROJECT_DIR/config.json' and gets the literal placeholder under bash but the expanded path under shell: 'powershell' — silently different behavior per shell, with nothing in the docs to consult.

Witness:

probe (real executeHook, identical command both shells):
  bash       (cwd=/home/team/proj) => validate '$QWEN_PROJECT_DIR/config.json'
  powershell (cwd=C:/team/proj)    => validate 'C:/team/proj/config.json'

Add one sentence in this paragraph documenting the PowerShell behavior (inside PowerShell '...' the path is spliced in with embedded ' doubled), or align the two branches — note the PowerShell splice is pinned by the test doubles an embedded single quote when splicing into a PowerShell single-quoted placeholder, which an alignment fix must update deliberately.

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

[qwen-code-ci-bot]

[Suggestion] The round-5 boundary class treats every ) as a word boundary, but a ) closing a $(…) command substitution, $((…)) arithmetic expansion, or <(…)/>(…) process substitution is a word constituent in bash — # right after it is NOT a comment, yet the scanner enters inComment and leaves the following placeholder literal, so the runtime env fallback expands it unquoted and word-splits. echo $((1+1))#$QWEN_PROJECT_DIR, diff <(sort a)#$QWEN_PROJECT_DIR and echo $(cat f)#$QWEN_PROJECT_DIR with cwd /tmp/my dir all come back unchanged at HEAD and word-split at runtime ([2#x][/tmp/my][dir] instead of one argument) — a regression vs the round-4 \s-only boundary, which expanded the $(…) shape correctly. Note the fix suggested for the $(…) instance alone was proven insufficient by patching: process substitution stayed broken until the fix classified what opened the paren.

Witness:

PR probe (HEAD): 'echo $((1+1))#$QWEN_PROJECT_DIR' and 'diff <(sort a)#$QWEN_PROJECT_DIR' -> unchanged
bash oracle: bash -c 'echo $((1+1))#MARKER1 tail' -> 2#MARKER1 tail (no comment)
             bash -c 'printf "%s\n" <(echo hi)#MARKER' -> /dev/fd/63#MARKER (no comment)
runtime arm: printf receives [2#x] [/tmp/my] [dir] (path word-split into two arguments)
round-4-boundary arm: 'echo $(cat f)#$QWEN_PROJECT_DIR' -> echo $(cat f)#'/tmp/my dir'
unified fix (classify the matching '(' predecessor over $/</>): 6/6 probe cases green, suite 87/87 green

When command[i-1] is ), scan back to the matching ( and treat ) as a boundary only if that ( is a plain subshell — not preceded by $ (for $((…)) also check the character before the second (), <, or >. A plain-subshell ) must REMAIN a boundary (bash -c '(echo hi)#tail' prints only hi), the $((16#ff)) radix form stays unaffected, and the ;# pin at hookRunner.test.ts:1226 must stay green. Please pin the fix with tests for echo $(cat f)#$QWEN_PROJECT_DIR, echo $((1+1))#$QWEN_PROJECT_DIR and diff <(sort a)#$QWEN_PROJECT_DIR (cwd /tmp/my dir) asserting the quoted expansion — with the naive all-)-are-boundaries behavior they are red.

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

[qwen-code-ci-bot]

[Suggestion] The shellType === 'powershell' short-circuit treats every unquoted # as a PowerShell comment start, but real PowerShell starts comments only at a TOKEN boundary — a mid-bareword # is literal there. Test-Path foo#$QWEN_PROJECT_DIR is misfiled as comment text; PowerShell then evaluates $QWEN_PROJECT_DIR as an undefined PS variable (env vars need $env:) — an empty string — so a gate hook probes foo instead of the project path and decides allow/deny on the wrong file. The round-5 any-# model (this PR's fix for the PowerShell facet of the round-4 comment-boundary finding) is falsified by the platform, and the pinning test added for it is itself invalid PowerShell.

Witness:

pwsh 7.4.19 oracle:
  Write-Output foo#bar -> foo#bar              (mid-bareword # is literal)
  echo a;#b            -> a                    (comment after a token boundary)
  Test-Path foo#$QWEN_PROJECT_DIR (env set) -> False
  foo#$env:QWEN_PROJECT_DIR                   -> foo#C:/proj
pinning-test fixture "echo foo#it's fine" -> ParserError: The string is missing the terminator: '
PR probe: 'Test-Path foo#$QWEN_PROJECT_DIR' returned unchanged by the scanner
fix arm (word-boundary rule for PS): -> Test-Path foo#'C:/proj' ; 86/87 green — the single failure is exactly the pinning test

Give the PowerShell branch the same word-boundary requirement as bash (start of command, or preceded by whitespace/a shell metacharacter), and update the pinning test recognizes a PowerShell comment starting mid-token, not just at a word boundary in the same commit — its fixture is not valid PowerShell, so it cannot stay asserting the falsified model.

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