fix(core): Reclaim command hook process trees

[doudouOUC]

What this PR does

This PR makes every command hook own a process group on POSIX and reclaims the group with a bounded SIGTERM-to-SIGKILL sequence when the hook times out or is cancelled. Windows cancellation invokes the absolute System32 taskkill.exe path asynchronously with /F /T, avoiding event-loop blocking while retaining a direct-child fallback. Cancellation and timeout races share one cleanup operation, final stdout and stderr are drained before the result is returned, and a one-second close boundary prevents stream shutdown from waiting indefinitely.

Why it's needed

Command hooks can launch nested shells, package managers, and installers. The previous direct-child cleanup used ChildProcess.killed, which records signal delivery rather than process exit, so escalation could be skipped and descendants could survive after the cancellation result. In session-start flows, those orphan processes amplify an initialization timeout into persistent resource leakage and make subsequent sessions less reliable.

Reviewer Test Plan

How to verify

On POSIX, start a command hook whose descendant records SIGTERM and keeps running. Cancel or time out the hook and confirm the descendant receives SIGTERM, the process group is escalated after approximately two seconds, and neither the root nor descendant is running when the result returns. Also verify that final stdout and stderr written during cancellation are present in the result, while a child that never emits close returns after the one-second drain boundary. On Windows, confirm cancellation starts taskkill.exe asynchronously with /F /T /PID, does not block the abort call, and force-kills the direct child if taskkill reports an error.

Evidence (Before & After)

N/A — non-UI process lifecycle change.

Tested on

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

Environment (optional)

macOS 26.4.1 (25E253), Node.js v22.22.3, npm 10.9.8. Focused core build, typecheck, lint, formatting, and 49 hook tests passed. The repository-wide build is currently blocked by unrelated existing Ink selection type errors under the CLI package.

Risk & Scope

• Main risk or tradeoff: POSIX returns after SIGKILL is accepted rather than polling indefinitely for every PID to disappear; Windows depends on the operating system's taskkill tree semantics.
• Not validated / out of scope: Real Windows and Linux execution were not tested locally. ACP initialization deadline propagation, ACP child-process ownership, and deliberately daemonized processes that leave the owned process group are not part of this PR.
• Breaking changes / migration notes: None. Existing hook result messages, timeout defaults, exit-code handling, and normal completion semantics are preserved.

Linked Issues

Closes #10099

中文说明

本 PR 做了什么

本 PR 在 POSIX 上让每个命令 Hook 拥有独立进程组，并在 Hook 超时或取消时通过有界的 SIGTERM 到 SIGKILL 流程回收整个进程组。Windows 取消路径异步调用 System32 下的绝对路径 taskkill.exe 并传入 /F /T，避免阻塞事件循环，同时保留直接子进程兜底。取消与超时竞态共享同一个清理操作，最终 stdout 和 stderr 会在结果返回前排空；如果流始终不关闭，则通过一秒的 close 边界避免无限等待。

为什么需要

命令 Hook 可能启动多层 shell、包管理器和安装程序。原有的直接子进程清理使用 ChildProcess.killed，该属性只表示信号已经发送，并不表示进程已经退出，因此可能跳过强制升级并让后代进程在取消结果返回后继续运行。在 SessionStart 链路中，这类孤儿进程会把一次初始化超时放大成持续的资源泄漏，并降低后续会话的可靠性。

Reviewer 测试计划

如何验证

在 POSIX 上启动一个命令 Hook，使其后代进程记录 SIGTERM 后继续运行。取消 Hook 或让其超时，确认后代收到 SIGTERM，进程组在约两秒后升级回收，并且结果返回时根进程和后代进程均不再运行。同时确认取消期间写出的最终 stdout 和 stderr 被保留，而永远不触发 close 的子进程会在一秒排空边界后返回。在 Windows 上，确认取消会异步启动带 /F /T /PID 参数的 taskkill.exe，不会阻塞 abort 调用，并在 taskkill 返回错误时强制终止直接子进程。

证据（前后对比）

N/A — 非 UI 的进程生命周期变更。

测试平台

操作系统	状态
🍏 macOS	✅ 已测试
🪟 Windows	⚠️ 未测试
🐧 Linux	⚠️ 未测试

环境（可选）

macOS 26.4.1（25E253）、Node.js v22.22.3、npm 10.9.8。Core 聚焦 build、typecheck、lint、format 以及 49 个 Hook 测试均已通过。全仓库 build 当前被 CLI 包中与本 PR 无关的既有 Ink selection 类型错误阻断。

风险与范围

• 主要风险或权衡：POSIX 在 SIGKILL 被接受后返回，不会无限轮询等待每个 PID 消失；Windows 依赖操作系统的 taskkill 进程树语义。
• 未验证或范围外：本地未进行真实 Windows 和 Linux 执行验证。ACP 初始化 deadline 传播、ACP 子进程所有权，以及主动离开所拥有进程组的 daemonized 进程不属于本 PR。
• 破坏性变更或迁移说明：无。现有 Hook 结果消息、超时默认值、退出码处理和正常完成语义均保持不变。

关联 Issue

Closes #10099

[qwen-code-ci-bot]

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

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

[doudouOUC]

E2E test report

Environment: macOS 26.4.1 (25E253), Node.js v22.22.3, npm 10.9.8.

Verified a real POSIX command-hook tree whose descendant records SIGTERM and ignores it. On cancellation, the root exited first, the descendant remained in the owned process group, group SIGKILL ran after the two-second grace period, and neither PID was running when the Hook result returned. Repeated real-process runs passed without residual PIDs or temporary files.

Verified cancellation-time output draining with a real child that writes final stdout and stderr from its SIGTERM handler before closing. Both streams were preserved in the cancellation result. The bounded fallback for a child that never emits close, timeout/Abort races, root-close-before-descendant cleanup, and Windows taskkill success/failure paths are covered by focused tests.

Focused verification:

Core build: passed
Core typecheck: passed
Focused ESLint and Prettier: passed
Hook tests: 49/49 passed
git diff --check: passed

Not tested on real Windows or Linux hosts. The repository-wide build is currently blocked by unrelated existing Ink selection type errors under packages/cli/src/ui/**; the changed core package builds and typechecks successfully.

[qwen-code-ci-bot]

Re-run after the round-5 fix — the gate verdict stands, updated for the new head fb0ed9fe.

Template: complete — all required sections present, including the bilingual summary ✓

Problem: exists, verified in the current code. executeCommandHook signalled only the direct shell, and its SIGKILL escalation was gated on child.killed — a flag that turns true the moment a signal is delivered, not when the process exits — so escalation effectively never fired and descendants survived cancellation. #10099 carries a concrete reproduction recipe. Self-reported, but not speculative: the mechanism is plain POSIX semantics, directly verifiable in the code.

Direction: aligned. The claude-code CHANGELOG has shipped this exact class of fix (orphaned Bash-tool process trees, SIGTERM-before-SIGKILL teardown), and this repo's shell tool already spawns detached: !isWindows and kills the process group — this PR extends the established house pattern to command hooks.

Size: touches packages/core/src/** — 351 production lines (hookRunner.ts +301/−50), 1103 test lines (hookRunner.test.ts +670/−1, hook-runner.process.test.ts +432), 38 design-doc lines. Under the 500-line threshold.

Approach: still minimal. The round-5 commit is exactly what the round-5 Critical (R5-1) asked for — fire-and-forget hooks exempted from the parent-exit fallback, plus the requested process-level survival test — and nothing else: +9/−2 production lines, +92 test lines, no drive-by edits. Earlier deferrals (taskkill dedup, cross-package tree-kill consolidation, observability logging) remain genuinely out of scope for this bugfix.

Risk: no match on the revert-correlated high-risk paths. The earlier tradeoff note still applies: with detached: true a hook's tree no longer receives the terminal's SIGINT directly — reclamation rides the CLI's shutdown and the exit-path backstop for registered hooks, while MessageDisplay and async hooks are deliberately left running per the documented contract.

Moving on to code review. 🔍

中文说明

复审（第 5 轮修复之后的重新运行）——门槛结论不变，按新 head fb0ed9fe 更新数据。

模板：完整 —— 所有必需章节齐全，包含双语说明 ✓

问题：真实存在，已在当前代码中核实。executeCommandHook 原本只向直接子 shell 发信号，且 SIGKILL 升级以 child.killed 为门槛——该标志在信号发出时即为 true，与进程是否退出无关——因此升级实际从未触发，后代进程会在取消后存活。#10099 提供了具体复现方案。虽是自报，但并非臆测：机制就是基本的 POSIX 语义，可直接在代码中验证。

方向：对齐。claude-code 的 CHANGELOG 已上线过同类修复（Bash 工具进程树孤儿化、先 SIGTERM 后 SIGKILL 的清理顺序），本仓库 shell 工具也早已采用 detached: !isWindows 加进程组回收——本 PR 把这一既有模式扩展到命令 Hook。

规模：触及 packages/core/src/** —— 生产代码 351 行（hookRunner.ts +301/−50），测试 1103 行（hookRunner.test.ts +670/−1、hook-runner.process.test.ts +432），设计文档 38 行。低于 500 行门槛。

方案：仍然是最小改动。第 5 轮提交恰好是第 5 轮 Critical（R5-1）所要求的内容——即发即忘 Hook 从父进程退出兜底中豁免，外加所要求的进程级存活测试——别无其他：生产代码 +9/−2 行、测试 +92 行，无夹带改动。此前的推迟项（taskkill 去重、跨包进程树回收整合、可观测性日志）仍然确实超出本 bug 修复的范围。

风险：未命中与 revert 相关的高风险路径。此前的权衡提示仍然适用：detached: true 之后 Hook 进程树不再直接收到终端 SIGINT——对已登记的 Hook，回收由 CLI 退出流程与退出路径兜底承担；MessageDisplay 与异步 Hook 则按文档契约有意保留运行。

进入代码审查。🔍

— Qwen Code · qwen3.8-max

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

[qwen-code-ci-bot]

Code review

Re-run after the round-5 fix. The full-diff audit from earlier rounds (termination-flow race safety, TERM→KILL escalation gating, drain bound, Windows taskkill path, parent-exit registry lifecycle) still holds; this pass verified the incremental +9/−2 production lines in fb0ed9fe against round 5's Critical (R5-1):

• R5-1 (exit fallback kills documented fire-and-forget hooks) — fixed via exemption, verified end to end. MessageDisplay hooks and async: true command hooks are no longer registered in activePosixHookProcesses, so the 'exit' fallback and the temporary signal handlers never touch them. I checked both dispatch chains rather than taking the condition on faith: fireMessageDisplayEvent passes HookEventName.MessageDisplay straight through executeHooks* into executeCommandHook, and async hooks arrive via executeAsyncHook → executeCommandHookInBackground with async: true intact on the config, so isAsyncHook fires regardless of event. Cleanup is symmetric — exempt hooks skip both register and unregister, so registry occupancy (which installs/uninstalls the parent-exit handlers) is unaffected. The shipped contract in docs/users/features/hooks.md ("The hook process is not killed on exit; it is left to finish on its own") holds again at this commit.
• Exempt ≠ unkillable. Cancellation semantics for the exempt classes are unchanged: timeout and abort still terminate the whole group through terminateHookProcessTree, which acts on child.pid directly and never consults the registry. Only the parent-exit backstop skips them, which is exactly the documented behavior.
• Test pinning is real. The new process-level matrix drives the actual HookRunner in a child Node process for both exempt classes, waits until the hook is running, then process.exit(0)s and asserts the hook runs to completion ('completed' file lands). On the previous head registration was unconditional, so the exit fallback SIGKILLed the group and this assertion could not pass — the witness direction was already demonstrated in the R5-1 probe, and CI now runs the test itself.

One non-blocking nit, recorded per the round-6 convergence posture rather than requested as a change: the design doc's fallback paragraph still says the exit path kills "every active hook group" without mentioning that fire-and-forget hooks are now exempt from registration — worth a sentence in a follow-up.

sequenceDiagram
    participant P1 as Hook caller
    participant P2 as HookRunner
    participant P3 as Termination op
    participant P4 as Process group
    participant P5 as Parent exit path
    P1->>P2: run command hook
    P2->>P4: spawn detached (owns group)
    alt not a fire-and-forget hook
        P2->>P5: register group
    end
    alt timeout or abort
        P1->>P2: abort or timeout
        P2->>P3: start once, idempotent
        P3->>P4: SIGTERM group
        P3->>P3: poll liveness, up to 2 seconds
        alt group exits
            P3-->>P2: done
        else group still alive
            P3->>P4: SIGKILL group
            P3-->>P2: done
        end
        P2->>P2: drain close, up to 1 second
        P2-->>P1: cancelled or timeout result
    else parent exits or unhandled signal
        P5->>P4: SIGKILL every registered group
        P5->>P5: re-raise signal when no app handler
    end
    P2->>P5: unregister when hook settles

Loading

Testing

Unattended run — PR code is never executed here; the evidence below is the PR's own CI on the reviewed commit, read via the API. CI has settled: all checks that ran are green, including Test (ubuntu-latest, Node 22.x), which executes the new survival matrix and the full hook suite on a real Linux runner.

Check	Conclusion
Classify PR	✅ success
Dependency CVE audit	✅ success
Desktop Shell (ubuntu-22.04)	✅ success
Desktop Shell (windows-2022)	✅ success
Secret scan (TruffleHog)	✅ success
Test (ubuntu-latest, Node 22.x)	✅ success
web-shell E2E Smoke (ubuntu-latest, Node 22.x)	✅ success

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

Evidence notes, same shape as earlier rounds: Test (macos-latest / windows-latest, Node 22.x) and the integration tests are skipped on this fork PR, so the Windows taskkill path remains covered only by mocked unit tests — no lane available here runs real Windows. The POSIX claims are pinned by real-process tests that CI actually executed green on ubuntu, now including the fire-and-forget survival matrix.

Sandboxed verification would settle the last general gap: @qwen-code /verify — an A/B run proving the suite (especially the new survival matrix and the parent-exit reaping tests) fails on the base build rather than passing identically with and without the diff. This is a sponsored run since the author lacks write access: a maintainer's @qwen-code /verify comment approves the head it names, and the run carries a pre-execution risk screen plus a workspace wipe — read its report with the same skepticism as the fork's own CI logs.

中文说明

代码审查：本轮针对第 5 轮 Critical（R5-1）验证 fb0ed9fe 的增量改动（生产代码 +9/−2 行）。R5-1（退出兜底误杀文档承诺的即发即忘 Hook）已经以豁免方式修复，并端到端核实：MessageDisplay Hook 与 async: true 命令 Hook 不再登记进 activePosixHookProcesses，'exit' 兜底与临时信号处理器都不会触碰它们。两条派发链都实际核对过：fireMessageDisplayEvent 将 HookEventName.MessageDisplay 原样传入 executeCommandHook；异步 Hook 经 executeAsyncHook → executeCommandHookInBackground 到达，配置上 async: true 完好，isAsyncHook 对任何事件都会命中。清理逻辑对称——豁免 Hook 同时跳过登记与注销，注册表占用（决定父进程退出处理器的安装/卸载）不受影响。docs/users/features/hooks.md 中"退出时不杀 Hook 进程、由其自行完成"的已发布契约在本提交重新成立。豁免不等于不可杀：超时与取消仍经 terminateHookProcessTree 直接作用于进程组、不查询注册表，故豁免类别的取消语义不变，只有父进程退出兜底跳过它们——正是文档所述行为。测试钉住是真实的：新增的进程级矩阵用例在子进程中驱动真实 HookRunner，覆盖两种豁免类别，待 Hook 运行后以 process.exit(0) 退出父进程，断言 Hook 跑完全程；上一提交登记是无条件的，该断言不可能通过（R5-1 的证据探针已展示翻转方向，CI 此次实际运行了该测试）。一项非阻塞小项按第 6 轮收敛姿态记录而不作为修改要求：设计文档兜底段落仍写"击杀每个活跃 Hook 组"，未提及即发即忘 Hook 现已豁免登记，值得后续补一句。

测试：无人值守运行，此处不执行 PR 代码；以上证据为通过 API 读取的该提交自身 CI。CI 已收敛：实际运行的检查全部为绿，其中 Test (ubuntu-latest, Node 22.x) 在真实 Linux 环境执行了新增存活矩阵与全部 Hook 测试套件。macOS/Windows 测试与集成测试在该 fork PR 上为 skipped，因此 Windows taskkill 路径仍仅有 mock 单测覆盖——此处没有任何通道可运行真实 Windows。POSIX 侧由真实进程测试钉住，且 CI 已在 ubuntu 上实际跑绿。剩余通用空白可用 @qwen-code /verify 的 A/B 运行补足（赞助运行：作者无写权限，由维护者触发；该运行附带执行前风险筛查与工作区清理，报告应以审视 fork CI 日志的同等怀疑态度阅读）。

— Qwen Code · qwen3.8-max

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

[qwen-code-ci-bot]

Confidence: 4/5 — the round-5 Critical is fixed the right way and pinned by a test that provably fails without it; docking the last point for the Windows path still resting on mocked coverage and a one-sentence design-doc lag.

Stepping back: this is the sixth look at this PR, and the picture at fb0ed9fe is exactly what the long review earned. R5-1 was the one real contract break left — the exit backstop SIGKILLing hooks the shipped docs promise will survive — and the fix takes the exemption route rather than rewriting the docs: MessageDisplay and async hooks never enter the registry, so the backstop cannot touch them, while timeout/abort cancellation still reaches their groups through the direct termination path. I verified both dispatch chains instead of trusting the condition, and the new survival test drives the real HookRunner through process.exit(0) for both exempt classes — on the previous head, where registration was unconditional, it could not pass. Underneath all of it the round-1 verdict still stands: minimal implementation of the right fix, escalation gated on real liveness instead of the broken child.killed, result shape and exit-code semantics untouched, and every accepted finding across five rounds pinned by a test. If I'm maintaining this in six months, the registry-plus-exemption reads as boring in the best sense.

Remaining reservations, none blocking: the Windows tree-kill has never run on a real Windows host (mocked unit tests only, and no lane here can change that), and the design doc's fallback paragraph hasn't caught up with the exemption — recorded as a follow-up, not a change request, per the convergence posture at this round count. The deferred Suggestions (taskkill dedup, cross-package tree-kill consolidation, observability logging, Windows parent-exit gap) stay deferred with their reasons in-thread.

Verdict: approve. CI is settled green on the reviewed commit, so the approval is posted now, pinned to fb0ed9fe654a94d1ba78f9320c535e7f2e773c51.

中文说明

置信度：4/5 —— 第 5 轮 Critical 以正确方式修复，并由一个缺少修复便必然失败的测试钉住；扣掉一分是因为 Windows 路径仍只有 mock 覆盖，且设计文档还差一句话。

退一步看：这是本 PR 的第六次审查，fb0ed9fe 的全貌正是漫长 review 应得的成果。R5-1 是最后剩下的真实契约破坏——退出兜底会 SIGKILL 掉文档承诺存活的 Hook——修复选择了豁免路线而非改写文档：MessageDisplay 与异步 Hook 从不进入注册表，兜底便无法触碰它们，而超时/取消仍能通过直接终止路径到达它们的进程组。我核实了两条派发链而非轻信条件本身；新增的存活测试针对两种豁免类别驱动真实 HookRunner 经历 process.exit(0)——在登记还是无条件的上一提交上，该测试不可能通过。其下第一轮的核心结论依然成立：正确修复的最小实现、升级门槛基于真实存活探测而非损坏的 child.killed、结果结构与退出码语义未动，五轮中每一项被接受的发现都有测试钉住。半年后维护这段代码，注册表加豁免部分读起来是"无聊"的最好含义。

剩余保留意见（均不阻塞）：Windows 进程树回收从未在真实 Windows 主机运行过（仅 mock 单测，此处也没有任何通道能改变这一点）；设计文档的兜底段落尚未跟上豁免改动——按当前轮次的收敛姿态记为后续跟进，不作为修改要求。被推迟的建议（taskkill 去重、跨包进程树回收整合、可观测性日志、Windows 父进程退出缺口）保持推迟，理由均在主题中。

结论：通过。该提交的 CI 已全绿收敛，批准即时发布，钉住 fb0ed9fe654a94d1ba78f9320c535e7f2e773c51。

— Qwen Code · qwen3.8-max

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

[doudouOUC]

@qwen-code /triage

[qwen-code-ci-bot]

Sandboxed verification: ✅ passed — merge-ready (agent verdict) - workflow run

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

Scripted assertions: 844 passed · 0 failed · 844 total

Flakiness gate: ✅ 2 changed test file(s) x 5 identical rounds, no divergence

中文 — 判定：✅ 通过 · 可合入（agent 判定）

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

脚本断言：844 通过 · 0 失败 · 844 总计

抖动门：✅ 2 changed test file(s) x 5 identical rounds, no divergence

Verification report

PR #10100 verification — fix(core): Reclaim command hook process trees

Verdict: merge-ready — 844/844 scripted assertions passed (0 unexpected failures), verified head 1959a40e01a91e717bc86e8108ded985c9f72d74 (merge-ref HEAD^2; base tip 7b69293266).

中文摘要

• 结论: merge-ready。844 条脚本化断言全部通过,0 条意外失败。
• A/B 结论: 中心声明成立。在 head 上,超时/取消会以 SIGTERM→SIGKILL 回收整个进程组,结果返回时忽略 SIGTERM 的后代进程已死亡(S1/S2,3.0s/2.0s 有界返回);在 base 控制组上同一后代进程在结果返回后仍存活(泄漏复现)。最尖锐的对照是 S4:setsid 逃逸者持有 stdout 管道时,base 永不返回(8s 观察窗内挂起),head 在 ~1.9s 内按 1 秒 close 边界有界返回且保留已写出的 stdout。正常完成路径(退出码 0/1/2、后台后代存活)两臂完全一致,无回归。
• 测试归因: PR 新增的 49 个 Hook 测试在 base 上 10 红 39 绿,红的恰为新增取消测试(含真实进程测试);变异矩阵中 M0–M4 五个守卫均被自身测试捕获,唯一存活者 M5(executeCommandHook 内 signal.aborted 预检查)是覆盖缺口,已由 A/B 的 S5 单元在行为层面钉住。
• Findings: 无阻塞项。两条覆盖缺口建议(见 Findings),一条设计语义提示(detached 使 Hook 脱离终端信号组,取消仅经 AbortSignal 传播)。
• 未覆盖: 真实 Windows 执行(taskkill 路径仅有 mock 单测)、非 root 下的 EPERM 信号失败分支、仓库级 typecheck(仅 core;CI 在 HEAD 的构建已覆盖)。

Central claim and A/B proof

Central claim: on timeout or cancellation, a command hook's entire process tree is reclaimed (SIGTERM to the owned POSIX process group, escalation to SIGKILL after a 2 s grace) before the result returns; the base only signalled the direct child and let descendants survive.

Harness ab-tree-reclaim.mjs drives the compiled dist/ HookRunner with real child processes (no mocks), once per arm. Control build = tmp/base-tree worktree at HEAD^1, rebuilt with the root node_modules (PR leaves package.json/lockfile untouched) plus a symlinked per-package node_modules; readlink -f on the base hookRunner.js resolves inside the base tree, and the harness imports each arm by explicit absolute path, so no workspace link can leak head code into the control. Witness: evidence/01-ab-head-vs-base.png (full output of both arms plus the table below).

cell	head (PR)	base (control)
S1 timeout, SIGTERM-ignoring descendant	descendant dead at return; SIGTERM delivered to group; dur=3005 ms	descendant ALIVE after return; never signalled; dur=1007 ms
S2 abort via AbortSignal, same tree	descendant dead; SIGTERM delivered; dur=2002 ms	descendant ALIVE; dur=2 ms
S3 final stdout/stderr written before cancellation	preserved (FINAL-OUT/FINAL-ERR)	preserved (identical)
S4 setsid escaper holds stdout/stderr pipes	bounded return dur=1853 ms; pre-termination stdout drained; escaper survives (documented non-goal)	never resolves within 8 s (hang)
S5 pre-aborted signal at executeCommandHook level	cancelled in 52 ms	ignores abort, runs to success in 3003 ms
S6 exit-0/1/2 normal completion semantics	unchanged	identical to head
S7 backgrounded descendant after normal exit 0	survives (no group signal on success)	identical

All 40 A/B assertions and 6 S7 assertions passed on both arms (head 22+3, base 18+3; three consecutive runs, deterministic). The base arm's expected-broken cells (leak, hang, ignored pre-abort) are encoded as expectations, so their reproduction counts as a pass.

Secondary claims

1. Drain + 1 s close boundary — S3/S4: final output survives cancellation; when streams can never close, head returns within the 1 s boundary instead of base's permanent hang. Pinned by unit tests bounds the output drain wait… and drains final output… (mutation M3 turns 2 tests red).
2. Single shared termination for timeout/abort races — unit test shares one termination when timeout and abort race… asserts exactly one SIGTERM and one SIGKILL; mutation M4 (drop the terminationPromise idempotency guard) turns it red.
3. Windows taskkill path — unit-mocked only (/System32/taskkill.exe, /f /t /pid, 2 s cap, SIGKILL fallback on taskkill error); not executable on this Linux container (see Not covered).

Vacuity and attribution

Base attribution: the PR's 49 hook tests run against the base build give 10 failed / 39 passed — the 10 reds are exactly the new cancellation tests, including the real-process test reaps a descendant that ignores SIGTERM before returning (and bounds the output drain wait… times out at 15 s on base, i.e. base waits forever). The 39 greens are pre-existing tests that pass identically on both sides. Witness: logs/base-attribution.log.

Mutation matrix (scratch worktree at head, one mutation at a time, file restored between runs; witness evidence/02-mutation-matrix.png, raw logs logs/mut-*.log):

mutant	change	result vs 49 tests	classification
control	none	49/49 green	harness live
M0 (positive control)	cancellation message string	6 red (5 unit + process test)	proves the runner collects and fails on this file
M1	detached: false	2 red (owns a POSIX process group…, process test)	pinned
M2	drop SIGKILL escalation	3 red (escalates to SIGKILL…, race test, process test)	pinned
M3	drop 1 s drain bound + stream destroy	2 red (bounds the output drain wait…, waits for close after a cancellation-time child error)	pinned
M4	drop termination idempotency guard	1 red (race test)	pinned
M5	drop signal.aborted pre-check in executeCommandHook	0 red — survivor	coverage gap (see F1)

No mutant regressed a killed-to-survived test relative to the control; the M5 survivor is real but bounded (F1).

Findings

F1 — Suggestion (coverage gap, not blocking): the signal.aborted pre-check added inside executeCommandHook is not pinned by any test. Mutation M5 leaves all 49 tests green; the behavior is nonetheless real and correct — harness S5 shows head cancels a pre-aborted direct call in 52 ms while base runs the hook to success (3003 ms). The public executeHook already filters pre-aborted signals, so this guard only matters for direct executeCommandHook callers (the async-hook background executor) and for aborts racing spawn; it is defense-in-depth. Suggested follow-up: a unit test that calls executeCommandHook (or triggers abort between the executeHook check and spawn) asserting immediate cancellation.

F2 — Nice to have: the non-ESRCH failed branch of signalProcessGroup (EPERM → fall back to direct-child kill) is unpinned and unreachable in this environment (the harness runs as root, so group signals never fail). Unit tests cover the ESRCH gone path only. Note only — the fallback is conservative and the escalation still SIGKILLs the group.

F3 — Note (design semantics, by construction): detached: true moves hook children out of the CLI's terminal process group. Terminal-generated signals (Ctrl+C SIGINT, SIGHUP) no longer reach a running hook directly; cancellation flows exclusively through the AbortSignal, which the codebase threads into every hook event (verified statically: hookEventHandler.ts passes signal through all event methods, and async hooks reuse the same fixed executeCommandHook path). S4/S7 confirm the accepted tradeoffs behave as documented (daemonized escapers survive; normal completion never signals the group). Reviewers should be aware that a hook whose signal never aborts will now run to its timeout where previously a terminal kill of the whole group could have ended it.

Not covered

• Real Windows execution — the taskkill.exe path is verified only by mocked unit tests; this container is Linux. The PR author also reports Windows untested.
• EPERM-class signal failures (F2) — requires a non-root environment.
• Zombie-reaping under a non-reaping PID 1 — this container reaped normally; the harness (like the PR's test) treats zombies as not-running, matching the design's stated "return after SIGKILL accepted" contract.
• Repo-wide typecheck/build — only packages/core was typechecked here (tsc --noEmit, clean). The CI environment had already built the repo at HEAD before this round; the author's "repo-wide build blocked by Ink selection errors" did not reproduce in this container.
• Base worktree build log carries one unrelated pre-existing type error (@lydell/node-pty typings resolution) that is a worktree artifact of root-node_modules type resolution; it does not touch the unit under test, whose emitted base dist was verified correct (grep -c terminateHookProcessTree = 0) and runtime-importable.
• Per-commit attribution — single commit (1959a40e), fully reachable locally; git rev-list HEAD^1..HEAD^2 matches the snapshot's commits array.

Methodology

Environment: CI verify container (node:22-bookworm), merge-ref checkout at depth 2; HEAD^1 = base tip, HEAD^2 = PR head. Harnesses (ab-tree-reclaim.mjs, normal-path-descendants.mjs) import the compiled dist/ HookRunner of each tree by absolute path and run real hook subprocesses (bash + node fixtures writing pid/ready/term files), asserting per-arm expectations with scripted checks; timing windows are asserted as ranges. Control build: scratch git worktree at HEAD^1, tsc --build with shared root node_modules plus symlinked per-package node_modules; isolation asserted via readlink -f. Mutation matrix and base attribution ran vitest 3.2.7 against scratch/base worktrees; gates ran the 25-file hook suite (742 tests) and tsc --noEmit at head. Raw logs: logs/ab-head.log, logs/ab-base.log, logs/s7-*.log, logs/base-attribution.log, logs/mut-*.log. Assertion counts: A/B 40, S7 6, head hook suite 742, base attribution 49 (39 green-as-predicted, 10 red-as-predicted), mutation matrix 7 = 844.

Flakiness gate log

rounds=5 files=2 skipped=0
file packages/core/src/hooks/hook-runner.process.test.ts: (cd packages/core) npx --no-install vitest run ./src/hooks/hook-runner.process.test.ts
file packages/core/src/hooks/hookRunner.test.ts: (cd packages/core) npx --no-install vitest run ./src/hooks/hookRunner.test.ts


per-file results (P=pass F=fail I=infra-exit, one letter per run):
  packages/core/src/hooks/hook-runner.process.test.ts: PPPPP
  packages/core/src/hooks/hookRunner.test.ts: PPPPP

verdict: pass
summary: 2 changed test file(s) x 5 identical rounds, no divergence

--- per-invocation detail (full copy in the artifact) ---
round 1 · packages/core/src/hooks/hook-runner.process.test.ts: P (exit 0)
round 1 · packages/core/src/hooks/hookRunner.test.ts: P (exit 0)
round 2 · packages/core/src/hooks/hook-runner.process.test.ts: P (exit 0)
round 2 · packages/core/src/hooks/hookRunner.test.ts: P (exit 0)
round 3 · packages/core/src/hooks/hook-runner.process.test.ts: P (exit 0)
round 3 · packages/core/src/hooks/hookRunner.test.ts: P (exit 0)
round 4 · packages/core/src/hooks/hook-runner.process.test.ts: P (exit 0)
round 4 · packages/core/src/hooks/hookRunner.test.ts: P (exit 0)
round 5 · packages/core/src/hooks/hook-runner.process.test.ts: P (exit 0)
round 5 · packages/core/src/hooks/hookRunner.test.ts: P (exit 0)

Evidence images

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

— Qwen Code · sandboxed verification

[qwen-code-ci-bot]

LGTM, looks ready to ship — CI landed green after the review. ✅

[qwen-code-ci-bot]

Not explored to full depth (tool budget reached): "agent 4": none — no checks were cut short.; "agent 1a": none — no check was cut short. One caveat: I did not run the Windows-path behavior on a real Windows host (mocked-unit coverage only, which is what the PR provi…; "agent 6b": none — but disclosing two checks I did not finish for honesty: (1) I did not trace runExitCleanup 's registered callbacks to determine whether they abort in-fl….

中文说明

未探索到全部深度（达到工具调用预算）："agent 4"：none — no checks were cut short.；"agent 1a"：none — no check was cut short. One caveat: I did not run the Windows-path behavior on a real Windows host (mocked-unit coverage only, which is what the PR provi…；"agent 6b"：none — but disclosing two checks I did not finish for honesty: (1) I did not trace runExitCleanup 's registered callbacks to determine whether they abort in-fl…。

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

[doudouOUC]

Addressed the latest review round in 6359a9546b.

• Fixed the Critical parent-exit orphan path by tracking active POSIX hook process groups, cleaning them from Node exit, and covering default SIGHUP/SIGINT/SIGTERM without overriding existing application handlers.
• Added fallback coverage for POSIX group-signal failures, missing PIDs, and synchronous Windows taskkill failures, plus debug breadcrumbs for bounded escalation/drain paths.
• Added real process tests for explicit process.exit, default signal exit, and a pre-existing once(SIGTERM) handler completing graceful AbortSignal cleanup.
• Documented the Windows root-already-exited limitation, detached-hook /dev/tty limitation, and untrappable SIGKILL boundary.
• Deferred repository-wide taskkill abstraction and production/test liveness-helper consolidation because both would broaden this bugfix beyond hook cancellation.

Verification: 57 focused tests passed; core ESLint, typecheck, build, formatting, and diff checks passed. Independent process verification confirmed preserved exit semantics and full root/descendant cleanup. Replied to all 8 review threads; resolving those handled threads now.

[doudouOUC]

@qwen-code /triage

[qwen-code-ci-bot]

Sandboxed verification: ✅ passed — merge-ready (agent verdict) - workflow run

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

Scripted assertions: 911 passed · 0 failed · 911 total

Flakiness gate: ✅ 2 changed test file(s) x 5 identical rounds, no divergence

中文 — 判定：✅ 通过 · 可合入（agent 判定）

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

脚本断言：911 通过 · 0 失败 · 911 总计

抖动门：✅ 2 changed test file(s) x 5 identical rounds, no divergence

Verification report

PR #10100 verification (round 2) — fix(core): Reclaim command hook process trees

Verdict: merge-ready — 911/911 scripted assertions passed (0 unexpected failures), verified head 6359a9546b59b35eeac231888dd359a52ff6112d (merge-ref HEAD^2; base tip aa7a0f05431c, which advanced since round 1's 7b69293266 — the A/B was re-measured against the new base, and the working tree under test is the merge commit itself).

This is a follow-up round. The PR gained a second commit (6359a954, "codex: address PR review feedback") after round 1 verified 1959a40e as merge-ready. That earlier commit is not reachable in this depth-2 checkout, so the delta cannot be isolated locally; the aggregate HEAD^1..HEAD diff was verified end to end, including the surfaces round 1 did not cover (see the status table and Not covered).

中文 — 判定：✅ 通过 · 可合入（agent 判定）· 第 2 轮

• 结论: merge-ready。911 条脚本化断言全部通过，0 条意外失败（详见下方各表）。
• 上一轮发现的状态: 见"上轮发现状态表"。F1（signal.aborted 预检查无测试钉住）仍然存在（变异 M5 在新 head 上再次存活）；F2（EPERM 分支）因新增 mock 测试 + 本轮半真实探针而基本关闭；F3（detached 脱离终端信号组）被本 PR 新增的父进程兜底机制在设计上缓解（S8a/S8b/S8c 三组 A/B 证据）。
• A/B 结论: 中心声明在新 base 上重新验证成立：超时/取消时 head 以 SIGTERM→2s grace→SIGKILL 回收整个进程组，忽略 SIGTERM 的后代在结果返回时已死亡（S1 3.5s / S2 2.0s 有界返回），base 对照组后代泄漏；escaper 持有管道时 base 永不返回而 head 按 1s close 边界有界返回（S4）；新增的父进程退出/信号兜底在 head 上三种模式均回收进程树，base 全部泄漏（S8a/S8b/S8c）。正常完成路径（S6/S7）两臂逐字段一致。
• 测试归因与变异: base 上 18 红全部恰为 PR 新增的修复性测试，39 绿为既有测试；变异矩阵 8 个变异 + 对照全部符合预期红集（M6/M7 钉住新的父进程兜底守卫），唯一存活者仍是 M5（F1）。
• 未覆盖: 真实 Windows、跨 uid 真实 EPERM（探针为半真实）、逐提交归因（shallow 边界）、仓库级门禁（仅 core）。

Previous-finding status (round 1 → this head)

#	Finding (round 1)	Severity	Status at new head 6359a954
F1	signal.aborted pre-check inside executeCommandHook unpinned by any test	Suggestion	Stands. Mutation M5 survives again at the new head (57/57 tests green with the pre-check deleted; matrix row M5, witness 03-mutation-matrix.png). The behavior remains real and correct — A/B cell S5 re-measured at this head: head cancels a pre-aborted direct call in 52 ms while base runs the hook to success in ~2.0 s — but it is still pinned only at the A/B level, not by the suite. Still a suggested follow-up test, not blocking.
F2	Non-ESRCH failed branch of signalProcessGroup unpinned and unreachable (round 1 ran as root)	Nice to have	Substantially resolved. (1) The branch is now pinned by a mocked unit test (falls back to the direct child when POSIX group signals fail) — it goes red under mutant M2. (2) The error shape is real in this container, which runs as uid 1000: kill(-7, 0) on a root-owned group returns EPERM. (3) A semi-real probe drove the compiled dist's cancellation path with a real child while denying only group signals: 5/5 assertions — 43 group-signal EPERMs, direct-child fallback killed the real child, cancellation resolved bounded at 2000 ms (witness logs/probe-eperm.log). Residual only: a genuinely cross-uid hook group requires privileges to construct.
F3	detached: true moves hooks out of the terminal process group; terminal signals can no longer end a hook whose AbortSignal never aborts	Note (design)	Mitigated by design at this head. The PR adds a parent-side fallback (design doc: "synchronous process-exit fallback" + temporary signal handlers), which round 1 had not exercised. Re-measured: parent process.exit(0) → tree SIGKILLed (S8a); parent SIGTERM with no app handler → tree killed, handler re-raises (S8b, driver death 3 ms); parent SIGTERM with an app handler → fallback defers, cancellation path runs the 2 s grace, driver exits 77 (S8c, elapsed 2005 ms). Residual accepted semantics: an app handler that swallows the signal without aborting still leaves the hook running to its timeout.

Central claim and A/B proof

Central claim: on timeout or cancellation, a command hook's entire process tree is reclaimed (SIGTERM to the owned POSIX process group, escalation to SIGKILL after a 2 s grace) before the result returns; base signalled only the direct child and leaked descendants.

Harnesses ab-main.mjs and ab-parent-fallback.mjs drive each arm's compiled dist/src/hooks/hookRunner.js by absolute path with real child processes (no mocks), and were run twice each (initial + captured re-run; all four runs green — deterministic). Control build: tmp/base-tree worktree at HEAD^1, rebuilt with the shared root node_modules (the PR touches no package.json/lockfile — verified empty diff — so the shared tree is a clean control); readlink -f on the base hookRunner.js resolves inside the base tree, and its emitted code contains zero occurrences of terminateHookProcessTree/detached. Witnesses: 01-ab-tree-cells-head-vs-base.png, 02-parent-fallback-cells-head-vs-base.png (live re-runs of both arms plus the digests below).

cell	head (PR)	base (control)
S1 timeout, SIGTERM-ignoring descendant	descendant dead at return; SIGTERM delivered to group (received); dur=3506 ms (1.5 s timeout + 2 s grace)	descendant ALIVE after return; never signalled
S2 abort via AbortSignal, same tree	descendant dead; dur=2002 ms	descendant ALIVE (leak)
S3 final stdout/stderr written before cancellation	preserved (FINAL-OUT/FINAL-ERR)	preserved (identical)
S4 setsid escaper holds stdout/stderr pipes	bounded return dur=1051 ms (abort + 1 s close boundary); pre-abort stdout drained; escaper survives (documented non-goal)	never resolves within 8 s (hang)
S5 pre-aborted signal at executeCommandHook level	cancelled in 52 ms	ignores abort, runs to success in 2006 ms
S6 exit-0/1/2 normal completion semantics	stdout/stderr/success/error fields identical across arms	identical to head
S7 backgrounded descendant after normal exit 0	survives (no group signal on success)	survives (identical)
S8a parent process.exit(0)	tree reaped by the exit fallback	tree survives parent exit (leak)
S8b parent SIGTERM, no app handler	tree reaped; HookRunner handler re-raises SIGTERM (driver dies in 3 ms)	tree survives parent SIGTERM (leak)
S8c parent SIGTERM, application handler present	fallback defers: cancellation path runs the 2 s grace (elapsed 2005 ms), tree dead, driver exit 77, upper.completed written	descendant ALIVE; fast return 9 ms; driver exit 77
S9 listener registry hygiene	SIGHUP/SIGINT/SIGTERM/exit listener counts identical before/after a hook (0→0)	n/a (base adds no listeners)

A/B assertions: S1–S7 head 28 + base 24 = 52; S8–S9 head 15 + base 15 = 30. All base-arm expected-broken cells (leak, hang, ignored pre-abort) are encoded as expectations, so their reproduction counts as pass.

Secondary claims

1. Drain + 1 s close boundary — S3/S4: final output survives cancellation; when streams can never close, head returns within the 1 s boundary instead of base's permanent hang. Pinned by the suite: mutant M3 (boundary raised to 60 s) turns bounds the output drain wait… red.
2. Single shared termination for timeout/abort races — unit test shares one termination when timeout and abort race…; mutant M4 (drop the terminationPromise idempotency guard) turns exactly that test red.
3. Parent-exit/signal fallback (new since round 1) — S8a/S8b/S8c above plus unit tests; mutants M1 (no group ownership) and M6 (no registration) turn the parent-fallback tests red, showing the guards interlock: group ownership is a precondition of the fallback's kill(-pid).
4. Windows taskkill path — mocked unit tests only (System32/taskkill.exe, /f /t /pid, 2 s cap, direct-child fallback on taskkill error/throw); not executable in this Linux container (see Not covered).

Vacuity and attribution

Base attribution (PR's test files staged into the base worktree, run against base source; witness 04-base-attribution.png, raw logs/base-attribution.stdout.log): 18 failed / 39 passed of 57. The 18 reds are byte-exactly the PR's new fix-specific tests (the diff adds 18 it/it.each blocks = 20 cases; 18 red on base). The 39 greens = 37 pre-existing tests (none red on base) plus 2 new regression guards that pin behavior the base already had (drains final output…, removes cancellation handling after a spawn error) — the right shape: regression guards green on base, fix proofs red on base. Six scripted meta-assertions verify the exact red/green partition (logs/attribution-assert.log).

Mutation matrix at head (scratch worktree, one hunk per mutant, source restored between runs — git status clean at the end; witness 03-mutation-matrix.png, junit per mutant logs/mut-*.junit.xml):

mutant	change	result vs 57 tests	classification
control	none	57/57 green	harness live
M0 (positive control)	cancellation message string altered	6 red — exactly the tests asserting that message	proves the runner collects and fails on this file
M1	detached: false	5 red (group-ownership test, process test, all 3 parent-fallback process tests)	pinned; guards interlock
M2	drop SIGKILL escalation	5 red (escalation, race, process test, handled-signal-exit, POSIX-signal-fail fallback)	pinned
M3	1 s close boundary → 60 s	1 red (bounds the output drain wait…)	pinned
M4	drop termination idempotency guard	1 red (race test)	pinned
M5	drop signal.aborted pre-check	0 red — survivor (F1 stands)	coverage gap
M6	drop active-hook registration	4 red (exit-fallback unit test, process-exit, signal-exit, leaves-parent-signals)	pinned
M7	drop "other listener present" early return	1 red (leaves parent signals to an existing application handler)	pinned

No mutant regressed a killed-to-survived test relative to the control. The M1/M2/M6 red sets extend beyond the minimally expected tests for mechanically verified reasons (noted in the table) — layered guards, not collateral damage: handled-signal-exit stays green under M6 because the cancellation path is independent of the parent-driven path.

Findings

F1 — Suggestion (stands from round 1, coverage gap, not blocking): the signal.aborted pre-check inside executeCommandHook is still not pinned by any test. Mutant M5 leaves all 57 tests green at the new head. The guard is real and correct — cell S5 shows head cancelling a pre-aborted direct call in 52 ms where base runs the hook to success — and matters for direct executeCommandHook callers (the async-hook background executor) and aborts racing spawn, since the public executeHook filters pre-aborted signals at entry. Suggested follow-up: a test that calls the command path with an already-aborted signal past the executeHook entry check.

F2 — Note (downgraded from round 1's Nice-to-have): the EPERM failed branch is now pinned and probed; only genuinely cross-uid execution remains unverified. See the status table for the three strands of evidence. No action needed unless the reviewer wants a privilege-separated integration test.

F3 — Note (design semantics, mitigated): detached: true still moves hook children out of the CLI's terminal process group, but the new parent-side fallback (S8a/S8b) now covers parent death for the default-signal case, and the exit fallback covers graceful shutdown; when an application handler owns the signal, HookRunner deliberately defers to it (S8c, unit test leaves parent signals…). The design doc lists the controlling-terminal loss as an explicit non-goal. Residual, as documented: a handler that swallows the signal without aborting leaves the hook running to its timeout.

No new findings this round.

Not covered

• Real Windows execution — the taskkill.exe path is verified only by mocked unit tests; this container is Linux and the PR author also reports Windows untested.
• Per-commit attribution — the depth-2 checkout reaches only the merge commit, the base tip, and the PR head; 1959a40e (round 1's head) is absent (git cat-file confirms), and git rev-list HEAD^1..HEAD^2 returns 1 commit while the snapshot lists 2 — a shallow-boundary gap. The aggregate HEAD^1..HEAD diff was verified; what the feedback commit changed relative to round 1 cannot be isolated locally. Round 1's report makes no mention of the parent-exit/signal fallback machinery, and this round verifies it fully (S8/S9, M6/M7, four new tests), so the new surface is covered whichever commit introduced it.
• Genuinely cross-uid EPERM on a hook's own group — constructing it requires privileges; the F2 probe is semi-real (real child, real cancellation flow, only the group-signal syscall denied). Labelled as such.
• Repo-wide gates — packages/core only: hooks suite 750/750, tsc --noEmit clean at head in the main tree (liveness-proven: a planted TS2322 was reported). The CI environment built the repo at HEAD before this round. The @lydell/node-pty TS7016 error reproduces identically in BOTH scratch worktrees (base and head) and not in the main tree — a worktree artifact of symlinked-node_modules type resolution, pre-existing and unrelated to the PR.
• Out-of-scope by the PR's own declaration (unchanged since round 1): ACP initialization deadline propagation, ACP child-process ownership, deliberately daemonized escapers (S4 confirms the documented behavior: escaper survives, return stays bounded).
• Shape vs cause: the A/B reproduces the wire/process-level shape of Command hook cancellation can leave descendant processes running #10099 (descendants surviving cancellation) directly; it does not reproduce the original session-start timeout amplification in a live ACP session.

Methodology

Environment: CI verify container (node:22-bookworm), uid 1000 (round 1 ran as root — the difference enabled the F2 EPERM probe), merge-ref checkout at depth 2; HEAD^1 = base tip aa7a0f0543, HEAD^2 = PR head 6359a954. Harnesses (harness/ab-main.mjs, harness/ab-parent-fallback.mjs, harness/parent-fallback-driver.mjs, harness/probe-eperm-fallback.mjs) import each arm's compiled dist/ HookRunner by absolute path and drive real hook subprocesses (node fixtures writing pid/ready/term files; isRunning treats zombies as not-running, mirroring the PR's own helper); per-arm expectations are scripted and timing windows asserted as ranges. Control build: git worktree at HEAD^1, tsc --build (one unrelated pre-existing worktree type error; emitted base dist verified PR-free by grep and importable), shared root node_modules with lockfile-unchanged verification and readlink -f isolation checks. Mutation matrix and attribution ran vitest 3.x against scratch/base worktrees; gates ran the 25-file hooks suite and tsc --noEmit at head, both liveness-proven (M0 reds / planted TS2322). Evidence images were captured live with scripts/verify-capture.mjs. Assertion counts: A/B S1–S7 52, A/B S8–S9 30, mutation verdicts 9, base attribution 57 (all outcomes constrained) + 6 meta, EPERM probe 5, head hooks suite 750, typecheck 1, gate liveness 1 = 911; each A/B harness was additionally re-run once under capture, green both times (determinism), those re-runs not double-counted. Raw logs: logs/ab-{head,base}.log, logs/pf-{head,base}.log, logs/matrix.log, logs/mut-*.junit.xml, logs/base-attribution.stdout.log, logs/attribution-assert.log, logs/probe-eperm.log, logs/gate-hooks-suite.log, logs/typecheck-core.log, logs/base-build.log.

Flakiness gate log

rounds=5 files=2 skipped=0
file packages/core/src/hooks/hook-runner.process.test.ts: (cd packages/core) npx --no-install vitest run ./src/hooks/hook-runner.process.test.ts
file packages/core/src/hooks/hookRunner.test.ts: (cd packages/core) npx --no-install vitest run ./src/hooks/hookRunner.test.ts


per-file results (P=pass F=fail I=infra-exit, one letter per run):
  packages/core/src/hooks/hook-runner.process.test.ts: PPPPP
  packages/core/src/hooks/hookRunner.test.ts: PPPPP

verdict: pass
summary: 2 changed test file(s) x 5 identical rounds, no divergence

--- per-invocation detail (full copy in the artifact) ---
round 1 · packages/core/src/hooks/hook-runner.process.test.ts: P (exit 0)
round 1 · packages/core/src/hooks/hookRunner.test.ts: P (exit 0)
round 2 · packages/core/src/hooks/hook-runner.process.test.ts: P (exit 0)
round 2 · packages/core/src/hooks/hookRunner.test.ts: P (exit 0)
round 3 · packages/core/src/hooks/hook-runner.process.test.ts: P (exit 0)
round 3 · packages/core/src/hooks/hookRunner.test.ts: P (exit 0)
round 4 · packages/core/src/hooks/hook-runner.process.test.ts: P (exit 0)
round 4 · packages/core/src/hooks/hookRunner.test.ts: P (exit 0)
round 5 · packages/core/src/hooks/hook-runner.process.test.ts: P (exit 0)
round 5 · packages/core/src/hooks/hookRunner.test.ts: P (exit 0)

Evidence images

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

— Qwen Code · sandboxed verification

[qwen-code-ci-bot]

Triage re-run completed without a new review.

⚠️ The bot has neither a verdict nor a deferral on 6359a9546b59b35eeac231888dd359a52ff6112d — no APPROVED, CHANGES_REQUESTED, or COMMENTED review of its own. A DISMISSED one does not count: dismiss_stale_reviews voids the bot's approval on every push, which is exactly when a fresh one is needed. If this re-run was meant to review or approve, it did not, and an approval left by another account is a separate vote that does not count as the bot's own.

⚠️ 机器人在 6359a9546b59b35eeac231888dd359a52ff6112d 上既没有裁决也没有 defer —— 没有属于它自己的 APPROVED、CHANGES_REQUESTED 或 COMMENTED 评审。DISMISSED 不算：dismiss_stale_reviews 会在每次推送时作废机器人的批准，而那恰恰是需要一次新批准的时刻。如果这次重跑本应评审或批准，那么它没有做到；而其他账号留下的批准是另一张票，不能算作机器人自己的。

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

上方各阶段评论已更新为最新结果。查看工作流运行。

[qwen-code-ci-bot]

Partially reviewed — gaps disclosed.

Not reviewed: build-and-test — Test (windows-latest, Node 22.x) was skipped in CI and its suite did not run locally.

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

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

• packages/core/src/hooks/hookRunner.ts:91 — [probe] Where group signalling is denied, every cancellation pays the full 2 s grace for nothing

中文说明

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

未审查：build-and-test — Test (windows-latest, Node 22.x) was skipped in CI and its suite did not run locally。

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

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

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

[doudouOUC]

Review round 2 handled at abdafee6c6.

Item	Decision	Action
R2-1	Fixed	Pinned all POSIX-only cancellation tests to Linux so Windows runs cannot enter the mocked taskkill path.
R2-2	Fixed	Reclaim owned hook groups before yielding to an application signal handler; the real-process test now covers graceful SIGTERM shutdown without aborting the hook.
R2-3	Not taken	Synchronous stderr writes from exit/signal handlers would change CLI shutdown output and introduce a new output contract.
R2-4	Fixed	Added overlapping-hook listener-retention coverage and verified the exit fallback remains installed with an application handler.
R2-5	Fixed	Added SIGHUP/SIGINT cleanup and same-signal re-raise coverage.
R2-6	Not taken	Async hooks remain agent-owned work; allowing them to outlive the parent would preserve the orphan-process behavior this PR is fixing.
R2-7	Fixed	Added SIGQUIT cleanup/re-raise handling, tests, and design documentation.

Verification:

• Core build and typecheck passed.
• Hook unit and process tests passed: 61/61.
• Focused ESLint and Prettier checks passed.
• Independent before/after probes reproduced both Critical findings on 6359a954 and verified the fix.
• The full repository build reaches the CLI package but is blocked by existing unrelated Ink selection typings; no unrelated CLI changes were made.

[qwen-code-ci-bot]

Partially reviewed — gaps disclosed. Suggestions are inline.

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

• WINDOWS_TASKKILL constant and taskkill-call duplication — already reported (comment 3861650968), author deferred the consolidation
• cross-package tree-kill consolidation — already reported (comment 3861650968), author deferred the consolidation
• exit/signal fallback observability logging — already reported as R2-3 (comment 3863811425), author declined

Not reviewed: build-and-test — Test (windows-latest, Node 22.x) was skipped in CI and its suite did not run locally.

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

Not explored to full depth (tool budget reached): "agent 1b": none.** All checks I planned completed within budget..

Convergence: round 3 posted 1 inline comment(s), 1 of them reported for the first time; the previous round posted 7 (7 new). Findings keep coming back to the same files: packages/core/src/hooks/hookRunner.ts (findings in round 2; 1 more now). A cluster that keeps producing siblings usually means the fixes are treating instances of a shared root cause — triaging that cause before the next round, or splitting an independent cluster into its own pull request, tends to end the loop faster than fixing them one at a time. No Critical finding is open on this round, so merging and moving the remaining Suggestion threads to a follow-up issue is available as an ending — a merged pull request cannot diverge further. (Observation only — nothing was withheld from this review because of this observation.)

中文说明

仅完成部分审查，审查缺口已披露。 建议见行内评论。

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

未审查：build-and-test — Test (windows-latest, Node 22.x) was skipped in CI and its suite did not run locally。

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

未探索到全部深度（达到工具调用预算）："agent 1b"：none.** All checks I planned completed within budget.。

收敛情况：第 3 轮发布了 1 条行内评论，其中 1 条是首次提出；上一轮发布了 7 条（其中 7 条首次提出）。发现反复回到同一批文件：packages/core/src/hooks/hookRunner.ts（第 2 轮已出过发现，本轮又有 1 条）。一个不断再生兄弟发现的簇，通常意味着逐条修复只在处理同一根因的实例——先定位并处理该根因，或把独立的簇拆成单独的 PR，通常比逐条修复更快结束循环。本轮没有未决的 Critical，因此"合入后把剩余 Suggestion 线程转到后续 issue"是一个可选的结束方式——已合入的 PR 不会继续发散。（仅为观察——本轮评审未因此扣留任何内容。）

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

[doudouOUC]

Review feedback update for commit 86ecb96639:

Item	Decision	Action
R3-1: simplify parent signal listener identity	Fixed	Registered one shared handler for SIGHUP, SIGINT, SIGQUIT, and SIGTERM; removed four wrappers and the unchecked ternary. Core build/typecheck, lint/format, and 61 focused hook tests pass.

No items were rejected or deferred in this batch.

[qwen-code-ci-bot]

Partially reviewed — gaps disclosed.

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

• WINDOWS_TASKKILL constant and taskkill-call / POSIX tree-kill duplication — already reported (comment 3861650968), author deferred the consolidation

Not reviewed: build-and-test — the test harness aborted before its test phase on a pre-existing packages/sdk-typescript compile failure untouched by this PR; the hook suites (61 tests) ran green via direct agent execution, but the wider affected suite was not exercised by the harness.

Not reviewed: build-and-test — Test (windows-latest, Node 22.x) was skipped in CI and its suite did not run locally.

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

Not explored to full depth (tool budget reached): "agent 1d": none — no check was cut short.; "agent 3c": running packages/core/src/hooks/hook-runner.process.test.ts — the review worktree has no node_modules installed and vitest cannot start, so the refactor was….

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

• packages/core/src/hooks/hookRunner.ts:906 — [probe] already-aborted signal fast path has no test witness

中文说明

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

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

未审查：build-and-test — the test harness aborted before its test phase on a pre-existing packages/sdk-typescript compile failure untouched by this PR; the hook suites (61 tests) ran green via direct agent execution, but the wider affected suite was not exercised by the harness。

未审查：build-and-test — Test (windows-latest, Node 22.x) was skipped in CI and its suite did not run locally。

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

未探索到全部深度（达到工具调用预算）："agent 1d"：none — no check was cut short.；"agent 3c"：running packages/core/src/hooks/hook-runner.process.test.ts — the review worktree has no node_modules installed and vitest cannot start, so the refactor was…。

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

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

[doudouOUC]

@qwen-code /triage

[doudouOUC]

@qwen-code /triage

[qwen-code-ci-bot]

Sandboxed verification: ✅ passed — merge-ready (agent verdict) - workflow run

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

Scripted assertions: 130 passed · 0 failed · 130 total

Flakiness gate: ✅ 2 changed test file(s) x 5 identical rounds, no divergence

中文 — 判定：✅ 通过 · 可合入（agent 判定）

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

脚本断言：130 通过 · 0 失败 · 130 总计

抖动门：✅ 2 changed test file(s) x 5 identical rounds, no divergence

Verification report

PR #10100 verification (round 3) — fix(core): Reclaim command hook process trees

Verdict: merge-ready — 130/130 scripted assertions passed (0 unexpected failures), verified head 86ecb966397b67e8e4ca33a77ee433762b890d4d (git rev-parse HEAD^2; base tip a82a11a0a4 = HEAD^1, which advanced again since round 2's aa7a0f0543 and since the metadata snapshot's 7b69293266 — the A/B was re-measured against the new base, and the working tree under test is the merge commit itself, i.e. a trial merge into the checkout-time base tip).

This is a follow-up round. The PR gained two more feedback commits (abdafee6, 86ecb966) after round 2 verified 6359a954; neither older head is reachable in this depth-2 checkout (git cat-file confirms both absent), so the delta cannot be isolated via git. It was isolated by measurement instead: the suite at head collects 61 tests vs round 2's 57 (+4 cases), the hooks gate counts 754 vs 750 (+4), and base attribution shows the 4 additional tests are all red on base (fix-proving). See the delta section.

中文 — 判定：✅ 通过 · 可合入（agent 判定）· 第 3 轮

• 结论: merge-ready。130 条脚本化断言全部通过，0 条意外失败（详见下方各表，不在此复述数字）。
• 上一轮发现的状态: 见"上轮发现状态表"。F1（executeCommandHook 内 signal.aborted 预检查无测试钉住）仍然存在——变异 M5 在新 head 上第三次存活（61/61 全绿）；本轮新增的 4 个测试也没有钉住它。F2（EPERM failed 分支）维持关闭：mock 测试钉住（M2 红集包含它）+ 半真实探针 6/6。F3（detached 脱离终端信号组）维持"设计上已缓解"：S8a/S8b/S8c 在新 head 重新测量，head 三种模式均在 ≤8ms 内回收进程树，base 三种模式全部泄漏（S8c base 驱动永久挂起）。
• 与第 2 轮的增量: 新增 4 个测试用例（套件 61 vs 57，hooks 门禁 754 vs 750），全部为修复证明型（base 上 22 红，其中 4 个即新增测试），且全部被本轮新增变异 M8/M9/M10 及 M6 钉住；中心声明的 A/B 在新 base 上重新成立。
• 未覆盖: 真实 Windows、逐提交归因（shallow 边界，仅能按计数与枚举推断 +4）、跨 uid 真实 EPERM（探针为半真实）、仓库级门禁（仅 core）、ACP deadline 传播等 PR 自声明非目标。

Previous-finding status (round 2 → this head)

#	Finding (round 2)	Severity	Status at new head 86ecb966
F1	signal.aborted pre-check inside executeCommandHook unpinned by any test	Suggestion	Stands. Mutant M5 survives for the third round: with the pre-check deleted, all 61 tests stay green (matrix row M5; witness 03-mutation-matrix.png, logs/vitest-M5.json). The four tests added since round 2 pin other guards (M6/M8/M9/M10), not this one. The behavior remains real — A/B cell S5 re-measured at this head: head cancels a pre-aborted direct executeCommandHook call in 53 ms while base ignores the abort and runs the hook to success in 1205 ms — but it is still pinned only at the A/B level, not by the suite. Still a suggested follow-up test, not blocking.
F2	Non-ESRCH failed branch of signalProcessGroup unpinned/unreachable	Note	Stays resolved. (1) Mocked unit test falls back to the direct child when POSIX group signals fail pins it — red under mutant M2. (2) Semi-real probe re-run at this head: real child, real cancellation flow, only kill(-pid, …) denied with EPERM — 6/6 assertions: 43 group-signal EPERMs observed, direct-child fallback killed the real child, cancellation resolved bounded at 2184 ms (witness 05-eperm-fallback-probe.png, logs/probe-eperm.log). Residual unchanged: genuinely cross-uid execution needs privileges this container lacks.
F3	detached: true moves hooks out of the terminal process group	Note (design)	Still mitigated by design. Re-measured at this head (witness 02-parent-fallback-cells-head-vs-base.png): parent process.exit(0) → tree reaped by the exit fallback (S8a, elapsed 1 ms); parent SIGTERM with no app handler → tree reaped, handler re-raised SIGTERM (S8b, 4 ms); parent SIGTERM with an app handler → fallback kills the tree then defers, driver exits 77 with upper.completed written (S8c, 8 ms). Base leaks the tree in all three modes; base S8c additionally hangs forever awaiting a result nothing can produce. Residual accepted semantics (documented non-goal): an app handler that swallows the signal without aborting leaves the hook to its timeout.

Central claim and A/B proof

Central claim: on timeout or cancellation, a command hook's entire process tree is reclaimed (SIGTERM to the owned POSIX process group, escalation to SIGKILL after a 2 s grace) before the result returns; base signalled only the direct child via ChildProcess.killed-gated killChild() and leaked descendants.

Harnesses harness/ab-main.mjs and harness/ab-parent-fallback.mjs drive both arms in one process each, importing each arm's hookRunner.ts via tsx with real child processes (no mocks). Each was run twice (initial + captured re-run; all four runs green — deterministic). Symmetric tsx-source arms, plus harness/dist-smoke.mjs proving the CI-built head dist contains the mechanism and behaves identically on an abort cell (8/8), so the A/B transfers to the shipped artifact. Control build: tmp/base-tree worktree at HEAD^1; the PR touches no package.json/lockfile (diff-stat shows only 4 files), so the shared root node_modules is a clean control; every import under test is relative (verified — hookRunner.ts imports only node: builtins and relative modules), so no workspace symlink can smuggle head code into the base arm; readlink -f on the base source resolves inside the base tree, and the emitted base dist contains zero occurrences of terminateHookProcessTree/detached. Witnesses: 01-ab-tree-cells-head-vs-base.png, 02-parent-fallback-cells-head-vs-base.png (live re-runs).

cell	head (PR)	base (control)
S1 timeout 1500 ms, SIGTERM-ignoring descendant	descendant received SIGTERM (received), dead at return, root dead; dur=3504 ms (1.5 s timeout + 2 s grace)	descendant never signalled, ALIVE after return (leak); dur ≈ timeout only
S2 abort via AbortSignal, same tree	descendant received SIGTERM, dead at return; dur=2152 ms	descendant ALIVE (leak)
S3 final stdout/stderr written before cancellation	preserved (FINAL-OUT/FINAL-ERR), bounded	preserved (identical)
S4 setsid escaper holds stdout/stderr pipes	bounded return, elapsed=1050 ms from abort (1 s close boundary); pre-abort stdout drained; escaper survives (documented non-goal)	never resolves within 15 s watchdog (hang)
S5 pre-aborted signal at executeCommandHook level	cancelled in 53 ms, hook never ran	ignores abort, runs to success in 1205 ms
S6 exit-0/1/2 normal completion semantics	success/stdout/stderr/error fields identical across arms (12 cross-arm comparisons)	identical to head
S7 backgrounded descendant after normal exit 0	survives (no group signal on success)	survives (identical)
S8a parent process.exit(0)	tree reaped by exit fallback (1 ms)	tree survives parent exit (leak)
S8b parent SIGTERM, no app handler	tree reaped; HookRunner handler re-raises SIGTERM (driver dies of SIGTERM, 4 ms)	tree survives (leak)
S8c parent SIGTERM, app handler present	fallback reaps tree then defers; driver exits 77, upper.completed written (8 ms)	driver hangs forever; tree alive
S9 listener registry hygiene	exit/SIGHUP/SIGINT/SIGTERM counts rise while hook active, restored exactly after	unchanged throughout (base registers nothing)

A/B assertions: ab-main 74 (S1–S9 both arms incl. 12 S6 cross-arm field comparisons), ab-parent-fallback 14, dist-smoke 8. All base-arm expected-broken cells (leak, hang, ignored pre-abort) are encoded as expectations in the harness, so their reproduction counts as pass.

Delta since round 2 (measured, not diffed)

The two feedback commits are not individually reachable (depth-2 checkout; git rev-list HEAD^1..HEAD^2 returns 1 commit while the snapshot lists 4 — shallow-boundary gap). Measured delta: the two PR test files now collect 61 cases vs 57 at round 2, and the hooks gate counts 754 vs 750. Cross-referencing round 2's test enumeration, the four tests it does not name are keeps parent cleanup registered while another hook is active, waits for close after a cancellation-time child error, force-kills the direct child when cancellation has no pid, and falls back to the direct child when taskkill throws synchronously — labelled as inference from the two reports' enumerations, not from a git diff. All four are red on base (fix-proving, attribution below) and each is pinned by a mutant this round (M6/M7-cascade, M10, M8, M9 respectively). No behavioral regression vs round 2's recorded observations: every S-cell and mutant expectation carried over unchanged except where the larger suite extends a red set (noted per row below).

Vacuity, attribution, and the mutation matrix

Base attribution (PR's test files staged into the base worktree, run against base source; witness 04-base-attribution.png, raw logs/vitest-base-attribution.json): 22 failed / 39 passed of 61. The 22 reds are byte-exactly the PR's fix-specific tests (24 added cases − the 2 regression guards that pin behavior base already had: drains final output before resolving cancellation, removes cancellation handling after a spawn error — both green on base, the right shape). All 37 pre-existing tests stay green on base — no pre-existing test was driven red. Six scripted adjudication assertions verify the partition (harness/adjudicate.mjs).

Mutation matrix at head (scratch worktree tmp/head-scratch at the merge commit, one hunk per mutant, source restored between runs — git status clean at the end; witness 03-mutation-matrix.png, per-mutant vitest JSON in logs/vitest-M*.json):

mutant	change	result vs 61 tests	classification
control	none	61/61 green	harness live
M0 (positive control)	cancellation message string altered	6 red — exactly the tests asserting that message, same 6 as round 2	proves the runner collects and fails on this file
M1	detached: false	5 red (group-ownership, process test, all 3 parent modes)	pinned
M2	SIGKILL escalation removed	4 red (reaps, escalates, race, POSIX-signal-fail fallback)	pinned — contrast: round 2 additionally listed handled-signal-exit under its M2; at this head that test is green under the precise escalation-only revert (it depends on the parent fallback's direct SIGKILL, not on escalation), and red under M6 instead
M3	1 s close boundary → 60 s	1 red (bounds the output drain wait…)	pinned
M4	termination idempotency guard removed	1 red (race test)	pinned
M5	signal.aborted pre-check removed	0 red — survivor (F1 stands)	coverage gap (behavior real per S5, nothing asserts it)
M6	active-hook registration removed	9 red — all parent-fallback surface: 3 process modes (tree-leak timeouts), exit-fallback unit, leaves-parent-signals, SIGHUP/SIGINT/SIGQUIT re-raise, keeps-parent-cleanup	pinned; red set larger than round 2's 4 because every listener-discovery test now fails the toBeDefined lookup when nothing registers
M7	application-handler deferral removed	5 red — leaves parent signals… fails its direct assertion (expected "kill" to not be called with [pid, 'SIGTERM']); SIGHUP/SIGINT/SIGQUIT + keeps-parent-cleanup fail via a mechanical cascade (the interrupted test never reaches its close event, leaking the handler, which the next tests' listenersBefore then excludes)	pinned
M8	no-pid SIGKILL branch removed	1 red — exactly force-kills the direct child when cancellation has no pid	pinned (new test non-vacuous)
M9	taskkill sync-throw fallback kill removed	1 red — exactly falls back to the direct child when taskkill throws synchronously	pinned (new test non-vacuous)
M10	error-during-cancellation guard removed	1 red — exactly waits for close after a cancellation-time child error	pinned (new test non-vacuous)

No mutant regressed a killed-to-survived test relative to the control. The M6/M7 red sets extend beyond the minimally expected tests for mechanically verified reasons (quoted failure messages in logs/vitest-M6.json/vitest-M7.json) — layered guards, not collateral damage. M8/M9/M10 additionally run the mutation in reverse on the four new tests: each new test is the unique red of its mutant, so the tests added since round 2 are confirmed non-vacuous and correctly attributed.

Findings

F1 — Suggestion (stands from rounds 1–2, coverage gap, not blocking): the signal.aborted pre-check inside executeCommandHook is still not pinned by any test. Mutant M5 leaves all 61 tests green at the new head, including the four added since round 2. The guard is real and correct — cell S5 shows head cancelling a pre-aborted direct call in 53 ms where base runs the hook to success in 1205 ms — and matters for direct executeCommandHook callers and aborts racing spawn, since the public executeHook filters pre-aborted signals at entry and executeHooksSequential/Parallel re-check before each hook. Suggested follow-up: a test that reaches executeCommandHook with an already-aborted signal past the executeHook entry check (e.g. through the async-hook background path, or a direct call with startTime + aborted signal).

F2 — Note (stays resolved): the EPERM failed branch is pinned by the mocked unit test and probed semi-real (43 denied group signals, direct-child fallback killed the real child, bounded 2184 ms). Only genuinely cross-uid execution remains unverifiable in this container.

F3 — Note (design semantics, still mitigated): detached: true moves hook children out of the CLI's terminal process group, with the parent-side fallback covering parent death/exit (S8a/S8b) and deliberate deferral to an application handler (S8c). The controlling-terminal loss is an explicit non-goal in the design doc; the residual (a handler that swallows the signal without aborting leaves the hook to its timeout) is documented and accepted.

No new findings this round.

Not covered

• Real Windows execution — the taskkill.exe path is verified only by mocked unit tests (tree-kill args/options asserted verbatim); this container is Linux and the PR author also reports Windows untested.
• Per-commit attribution — depth-2 checkout; 6359a954/abdafee6 are absent (git cat-file confirms), git rev-list HEAD^1..HEAD^2 returns 1 of the snapshot's 4 commits. The delta was isolated by measurement (counts + enumeration, see the delta section) rather than by diff; labelled as inference where applicable.
• Genuinely cross-uid EPERM on a hook's own group — constructing it requires privileges; the F2 probe is semi-real (real child, real cancellation flow, only the group-signal syscall denied). Labelled as such.
• Repo-wide gates — packages/core only: hooks suite 754/754 and tsc --noEmit clean at head (both liveness-proven: M0's 6 reds prove the vitest collection fails on this file; a planted TS2322 was reported by the identical tsc invocation before the source was restored). The tsc --build of the two scratch/base worktrees emits 63 identical type errors in both trees, all in files the PR never touches (gitIgnoreParser, qwenIgnoreParser, schemaValidator — dependency-type-resolution artifacts of nested worktrees sharing the root node_modules); emission still occurred and was verified PR-free by grep and import.
• Out-of-scope by the PR's own declaration (unchanged since rounds 1–2): ACP initialization deadline propagation, ACP child-process ownership, deliberately daemonized escapers (S4 confirms the documented behavior: escaper survives, return stays bounded).
• Shape vs cause: the A/B reproduces the process-level shape of Command hook cancellation can leave descendant processes running #10099 (descendants surviving cancellation/timeout, bounded vs unbounded returns) directly; it does not reproduce the original session-start timeout amplification in a live ACP session.
• Flakiness gate: owned by the workflow, not re-run here.

Methodology

Environment: CI verify container (node:22-bookworm), uid 1000, node v22.23.2, merge-ref checkout at depth 2; HEAD^1 = base tip a82a11a0a4 (newer than both round 2's base and the metadata snapshot's baseRefOid, so the merge-ref is a fresh trial merge into that tip), HEAD^2 = PR head 86ecb966. Harnesses (harness/ab-main.mjs, harness/ab-parent-fallback.mjs, harness/probe-eperm.mjs, harness/dist-smoke.mjs, harness/debug-s4.mjs) import each arm's hookRunner.ts by absolute path under --import=tsx/esm (imports are relative-only, verified) and drive real hook subprocesses (node fixtures writing pid/ready/term files; isRunning treats zombies as not-running, mirroring the PR's own helper); per-arm expectations are scripted and timing windows asserted as ranges. Head dist equivalence proven by dist-smoke.mjs (8/8). Control build: git worktree at HEAD^1 with tsc --build (63 unrelated pre-existing worktree type errors, identical in a head worktree built the same way; emitted base dist verified PR-free by grep and importable); base vitest runs use the base tree's own config and the staged PR test files. Mutation matrix and attribution ran vitest 3.2.7 against scratch/base worktrees with per-run JSON reporters; gates ran the 25-file hooks suite and tsc --noEmit at head, both liveness-proven. harness/adjudicate.mjs re-parses every log/JSON and encodes all expectations, emitting assertions.json (harness checks 102 + log-integrity 4 + matrix verdicts 15 + attribution verdicts 6 + gate verdicts 3 = 130; each A/B harness was additionally re-run once under scripts/verify-capture.mjs, green both times, those re-runs not double-counted). Evidence images were captured live with scripts/verify-capture.mjs. Raw logs: logs/ab-main.log, logs/ab-pf.log, logs/probe-eperm.log, logs/dist-smoke.log, logs/matrix.log, logs/vitest-{control,M0…M10,base-attribution,hooks-suite}.json, logs/base-attribution.stdout.log, logs/gate-hooks-suite.log, logs/typecheck-core.log, logs/typecheck-liveness.log, logs/base-build.log, logs/head-scratch-build.log.

Flakiness gate log

rounds=5 files=2 skipped=0
file packages/core/src/hooks/hook-runner.process.test.ts: (cd packages/core) npx --no-install vitest run ./src/hooks/hook-runner.process.test.ts
file packages/core/src/hooks/hookRunner.test.ts: (cd packages/core) npx --no-install vitest run ./src/hooks/hookRunner.test.ts


per-file results (P=pass F=fail I=infra-exit, one letter per run):
  packages/core/src/hooks/hook-runner.process.test.ts: PPPPP
  packages/core/src/hooks/hookRunner.test.ts: PPPPP

verdict: pass
summary: 2 changed test file(s) x 5 identical rounds, no divergence

--- per-invocation detail (full copy in the artifact) ---
round 1 · packages/core/src/hooks/hook-runner.process.test.ts: P (exit 0)
round 1 · packages/core/src/hooks/hookRunner.test.ts: P (exit 0)
round 2 · packages/core/src/hooks/hook-runner.process.test.ts: P (exit 0)
round 2 · packages/core/src/hooks/hookRunner.test.ts: P (exit 0)
round 3 · packages/core/src/hooks/hook-runner.process.test.ts: P (exit 0)
round 3 · packages/core/src/hooks/hookRunner.test.ts: P (exit 0)
round 4 · packages/core/src/hooks/hook-runner.process.test.ts: P (exit 0)
round 4 · packages/core/src/hooks/hookRunner.test.ts: P (exit 0)
round 5 · packages/core/src/hooks/hook-runner.process.test.ts: P (exit 0)
round 5 · packages/core/src/hooks/hookRunner.test.ts: P (exit 0)

Evidence images

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

— Qwen Code · sandboxed verification

[qwen-code-ci-bot]

Triage re-run completed without a new review.

⚠️ The bot's only review on 86ecb966397b67e8e4ca33a77ee433762b890d4d is a COMMENTED one, which carries no vote — so it has no verdict of its own on this commit, and main needs two approving reviews: an approval left by another account is a separate vote and does not count as the bot's own. Two different things look like this, and the stage-3 comment above says which: the triage skill deferring on purpose at 3/5 — a fork refactor hitting the approval guardrail, or a core change escalated for maintainer awareness, both normal outcomes — or an earlier approval that a push dismissed, leaving only the comment behind, which needs a fresh review.

⚠️ 机器人在 86ecb966397b67e8e4ca33a77ee433762b890d4d 上唯一的评审是 COMMENTED，不带票 —— 因此它在该 commit 上没有自己的裁决，而 main 需要两个批准（其他账号的批准是另一张票）。有两种情况长这样，上方的 stage-3 评论会说明是哪一种：triage skill 在 3/5 时有意 defer（fork refactor 命中审批护栏，或核心改动被升级交由维护者把关，两者都是正常结果）；或者更早的批准被一次推送作废、只剩下这条评论，此时需要重新评审。

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

上方各阶段评论已更新为最新结果。查看工作流运行。

[qwen-code-ci-bot]

Partially reviewed — gaps disclosed.

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

• WINDOWS_TASKKILL constant and taskkill-call duplication — already reported (comment 3861650968), author deferred the consolidation
• cross-package tree-kill consolidation — already reported (comment 3861650968), author deferred the consolidation
• exit/signal fallback observability logging — already reported as R2-3 (comment 3863811425), author declined
• already-aborted signal fast path test witness — already reported in round 4 (deferred item in review 5033829402)

Not reviewed: build-and-test — Test (windows-latest, Node 22.x) was skipped in CI and its suite did not run locally.

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

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

• packages/core/src/hooks/hookRunner.ts:150 — [review] No Windows parent-exit hook reclamation; the design doc's Non-goals does not record the gap

中文说明

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

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

未审查：build-and-test — Test (windows-latest, Node 22.x) was skipped in CI and its suite did not run locally。

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

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

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

[doudouOUC]

Round 5 Critical addressed in fb0ed9f.

Review item	Decision	Action
R5-1 fire-and-forget hooks killed on parent exit	Agreed	Exempted MessageDisplay and async: true command hooks from parent-exit cleanup without changing explicit timeout/AbortSignal cancellation; added real-process regression coverage for both behaviors.

Verification: exact-head reproduction failed for both rows before the fix; after the fix, the 6-test process-tree suite, core build, core typecheck, targeted ESLint, and Prettier checks pass. Repository-wide build remains affected by the pre-existing unrelated CLI Ink typing mismatch, so this change does not modify that area.

[doudouOUC]

@qwen-code /triage

[qwen-code-ci-bot]

Sandboxed verification: ❌ not passed — the PR could not be built - workflow run

The PR could not be built because npm ci failed twice in a row before any verification started. This is treated as a PR failure verdict rather than an infrastructure failure.

中文 — 判定：❌ 不通过 · PR 构建失败

由于 npm ci 在验证开始前失败（连续两次），无法构建该 PR。判定为 PR 问题而非基础设施故障；安装日志见下方折叠块。

Install/build log

$ npm ci --prefer-offline --no-audit --progress=false --cache "$RUNNER_TEMP/npm-cache"
npm warn deprecated rimraf@3.0.2: Rimraf versions prior to v4 are no longer supported
npm warn deprecated prebuild-install@7.1.3: No longer maintained. Please contact the author of the relevant native addon; alternatives are available.
npm warn deprecated node-domexception@1.0.0: Use your platform's native DOMException instead
npm warn deprecated inflight@1.0.6: This module is not supported, and leaks memory. Do not use it. Check out lru-cache if you want a good and tested way to coalesce async requests by a key value, which is much more comprehensive and powerful.
npm warn deprecated @humanwhocodes/object-schema@2.0.3: Use @eslint/object-schema instead
npm warn deprecated @humanwhocodes/config-array@0.13.0: Use @eslint/config-array instead
npm warn deprecated glob@7.2.3: Glob versions prior to v9 are no longer supported
npm warn deprecated eslint@8.57.1: This version is no longer supported. Please see https://eslint.org/version-support for other options.
npm warn deprecated glob@7.2.3: Glob versions prior to v9 are no longer supported

> @qwen-code/qwen-code@0.22.2 postinstall
> patch-package

patch-package 8.0.1
Applying patches...
ink@7.0.3 ✔

> @qwen-code/qwen-code@0.22.2 prepare
> node scripts/prepare.js


> @qwen-code/qwen-code@0.22.2 build
> cross-env NODE_OPTIONS="--max-old-space-size=3072" node scripts/build.js


> @qwen-code/qwen-code@0.22.2 generate
> node scripts/generate-git-commit-info.js


> @qwen-code/qwen-code-core@0.22.2 build
> node ../../scripts/build_package.js

src/core/client.telemetrySwap.test.ts(103,5): error TS1117: An object literal cannot have multiple properties with the same name.
node:internal/errors:983
  const err = new Error(message);
              ^

Error: Command failed: tsc --build
    at genericNodeError (node:internal/errors:983:15)
    at wrappedFn (node:internal/errors:537:14)
    at checkExecSyncError (node:child_process:916:11)
    at execSync (node:child_process:988:15)
    at file:///__w/qwen-code/qwen-code/scripts/build_package.js:38:1
    at ModuleJob.run (node:internal/modules/esm/module_job:343:25)
    at async onImport.tracePromise.__proto__ (node:internal/modules/esm/loader:681:26)
    at async asyncRunEntryPointWithESMLoader (node:internal/modules/run_main:117:5) {
  status: 1,
  signal: null,
  output: [ null, null, null ],
  pid: 2427,
  stdout: null,
  stderr: null
}

Node.js v22.23.2
npm error Lifecycle script `build` failed with error:
npm error code 1
npm error path /__w/qwen-code/qwen-code/packages/core
npm error workspace @qwen-code/qwen-code-core@0.22.2
npm error location /__w/qwen-code/qwen-code/packages/core
npm error command failed
npm error command sh -c node ../../scripts/build_package.js
node:internal/errors:983
  const err = new Error(message);
              ^

Error: Command failed: npm run build --workspace=packages/core
    at genericNodeError (node:internal/errors:983:15)
    at wrappedFn (node:internal/errors:537:14)
    at checkExecSyncError (node:child_process:916:11)
    at execSync (node:child_process:988:15)
    at file:///__w/qwen-code/qwen-code/scripts/build.js:90:3
    at ModuleJob.run (node:internal/modules/esm/module_job:343:25)
    at async onImport.tracePromise.__proto__ (node:internal/modules/esm/loader:681:26)
    at async asyncRunEntryPointWithESMLoader (node:internal/modules/run_main:117:5) {
  status: 1,
  signal: null,
  output: [ null, null, null ],
  pid: 2407,
  stdout: null,
  stderr: null
}

Node.js v22.23.2
prepare: npm run build exited with status 1
npm error code 1
npm error path /__w/qwen-code/qwen-code
npm error command failed
npm error command sh -c node scripts/prepare.js
npm error A complete log of this run can be found in: /__w/_temp/npm-cache/_logs/2026-08-27T07_35_57_113Z-debug-0.log

npm ci failed with exit code 1; retrying once.
$ npm ci --prefer-offline --no-audit --progress=false --cache "$RUNNER_TEMP/npm-cache"
npm warn deprecated glob@7.2.3: Glob versions prior to v9 are no longer supported
npm warn deprecated eslint@8.57.1: This version is no longer supported. Please see https://eslint.org/version-support for other options.

> @qwen-code/qwen-code@0.22.2 postinstall
> patch-package

patch-package 8.0.1
Applying patches...
ink@7.0.3 ✔

> @qwen-code/qwen-code@0.22.2 prepare
> node scripts/prepare.js


> @qwen-code/qwen-code@0.22.2 build
> cross-env NODE_OPTIONS="--max-old-space-size=3072" node scripts/build.js


> @qwen-code/qwen-code@0.22.2 generate
> node scripts/generate-git-commit-info.js


> @qwen-code/qwen-code-core@0.22.2 build
> node ../../scripts/build_package.js

src/core/client.telemetrySwap.test.ts(103,5): error TS1117: An object literal cannot have multiple properties with the same name.
node:internal/errors:983
  const err = new Error(message);
              ^

Error: Command failed: tsc --build
    at genericNodeError (node:internal/errors:983:15)
    at wrappedFn (node:internal/errors:537:14)
    at checkExecSyncError (node:child_process:916:11)
    at execSync (node:child_process:988:15)
    at file:///__w/qwen-code/qwen-code/scripts/build_package.js:38:1
    at ModuleJob.run (node:internal/modules/esm/module_job:343:25)
    at async onImport.tracePromise.__proto__ (node:internal/modules/esm/loader:681:26)
    at async asyncRunEntryPointWithESMLoader (node:internal/modules/run_main:117:5) {
  status: 1,
  signal: null,
  output: [ null, null, null ],
  pid: 2671,
  stdout: null,
  stderr: null
}

Node.js v22.23.2
npm error Lifecycle script `build` failed with error:
npm error code 1
npm error path /__w/qwen-code/qwen-code/packages/core
npm error workspace @qwen-code/qwen-code-core@0.22.2
npm error location /__w/qwen-code/qwen-code/packages/core
npm error command failed
npm error command sh -c node ../../scripts/build_package.js
node:internal/errors:983
  const err = new Error(message);
              ^

Error: Command failed: npm run build --workspace=packages/core
    at genericNodeError (node:internal/errors:983:15)
    at wrappedFn (node:internal/errors:537:14)
    at checkExecSyncError (node:child_process:916:11)
    at execSync (node:child_process:988:15)
    at file:///__w/qwen-code/qwen-code/scripts/build.js:90:3
    at ModuleJob.run (node:internal/modules/esm/module_job:343:25)
    at async onImport.tracePromise.__proto__ (node:internal/modules/esm/loader:681:26)
    at async asyncRunEntryPointWithESMLoader (node:internal/modules/run_main:117:5) {
  status: 1,
  signal: null,
  output: [ null, null, null ],
  pid: 2651,
  stdout: null,
  stderr: null
}

Node.js v22.23.2
prepare: npm run build exited with status 1
npm error code 1
npm error path /__w/qwen-code/qwen-code
npm error command failed
npm error command sh -c node scripts/prepare.js
npm error A complete log of this run can be found in: /__w/_temp/npm-cache/_logs/2026-08-27T07_37_29_547Z-debug-0.log

npm ci failed with exit code 1 after 2 attempts.

— Qwen Code · sandboxed verification

[qwen-code-ci-bot]

LGTM, looks ready to ship. ✅

[yiliang114]

Reviewed at the current head — spot-checked hookRunner.ts (the whole source delta) plus the design doc and test coverage:

• POSIX: detached makes the hook child a process-group leader, cancellation signals the whole group (SIGTERM → 2s grace → SIGKILL), with ESRCH handled as "gone" and a direct-child fallback when group signaling fails. Windows path uses taskkill /f /t with its own timeout and direct-kill fallback.
• Parent-exit leak closed: active hook children are registered in a module-level set and force-killed on parent exit / SIGHUP / SIGINT / SIGQUIT / SIGTERM (re-raising the signal when no other listener owns it); the registration is unregistered and the listeners removed again once the set drains. MessageDisplay / async hooks are correctly exempted as surviving the parent.
• The settle path is single-shot (finish), cancellation waits for child close (1s bound) before destroying stdio so a cancelled hook cannot hang the caller, and the pre-aborted signal fast path is covered.
• CI green (only the review-pr bot lane running). Aligns with the earlier review rounds' resolution.

[wenshao]

Partially reviewed — gaps disclosed.

Not reviewed: build-and-test — Test (windows-latest, Node 22.x) was skipped in CI and the Windows process-tree path did not run locally.

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

• docs/design/hook-process-tree-cancellation.md:18 — [review] The design document says every POSIX command hook is registered for parent-exit cleanup, contradicting the intentional MessageDisplay and async-hook exemptions.
• packages/core/src/hooks/hookRunner.ts:127 — [review] The parent-exit direct-child fallback for a failed process-group SIGKILL has no regression test.

Convergence: round 6 posted 2 inline comment(s), 2 of them reported for the first time. Findings keep coming back to the same files: packages/core/src/hooks/hookRunner.ts (findings in round 5; 2 more now). (Evidence: the previous round was recovered from a marker this account did not post, so those rounds may not be this account's own.) A cluster that keeps producing siblings usually means the fixes are treating instances of a shared root cause — triaging that cause before the next round, or splitting an independent cluster into its own pull request, tends to end the loop faster than fixing them one at a time. (Observation only — nothing was withheld from this review because of this observation.)

中文说明

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

未审查：build-and-test — Test (windows-latest, Node 22.x) was skipped in CI and the Windows process-tree path did not run locally。

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

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

— gpt-5.6-sol@954e5164 via Qwen Code /review (v0.22.2)

[wenshao]

[Critical] R5-1: (fix-induced) Fire-and-forget hooks exempted from parent-exit SIGKILL still die when they write to parent-owned stdout or stderr after the parent exits.

A MessageDisplay or async hook can outlive Qwen, but every hook is still spawned with parent-owned stdout/stderr pipes. Once Qwen exits, a later write receives SIGPIPE/EPIPE, terminating the hook before its audit, notification, or logging side effect completes.

Witness:

PR: both augmented survival tests failed with `Condition not met within 3000ms` (2 failed, 4 passed).
With parent-independent stdout/stderr: both passed (6 passed).

Give hooks intended to survive parent exit stdio that remains valid independently of the parent, such as spool files that HookRunner reads while alive, or explicitly revise the survival contract and clean them up. Extend both process-level survival fixtures to write stdout and stderr before creating their completion marker, and verify that removing the independent-stdio fix makes those tests fail.

中文说明

被排除在父进程退出 SIGKILL 清理之外的即发即忘 Hook，仍会在父进程退出后写入父进程持有的 stdout/stderr 管道时死亡。Qwen 退出会关闭管道读取端，Hook 随后的输出会触发 SIGPIPE/EPIPE，在审计、通知或日志副作用完成前终止。探针中，当前实现的两个增强生存测试均超时失败；将生存 Hook 的 stdout/stderr 改为独立于父进程后，6 个进程测试全部通过。请为需要存活的 Hook 提供独立 stdio，并让两个生存用例在创建完成标记前写 stdout/stderr；移除修复后测试必须失败。

— gpt-5.6-sol@954e5164 via Qwen Code /review (v0.22.2)

[wenshao]

[Critical] R6-1: The parent-exit exemption omits synchronous StopFailure command hooks even though their documented contract and production callers launch them fire-and-forget.

docs/users/features/hooks.md explicitly defines StopFailure as fire-and-forget and documents rate-limit monitoring and authentication-failure logging as use cases. Core, CLI UI, and ACP callers start fireStopFailureEvent(...) without awaiting it. Because this predicate exempts only MessageDisplay and async: true, a headless error or loop-detection exit registers the synchronous StopFailure hook for parent-exit cleanup and SIGKILLs it before its alert or audit record completes.

Witness: not run — the review harness could not create the required isolated scratch tree for this verifier; the documented contract and complete production call chain are directly present in the reviewed commit.

Include HookEventName.StopFailure in the fire-and-forget parent-exit exemption and audit any other event whose production delivery is explicitly non-awaited, while retaining timeout and AbortSignal cancellation. Add a process-level synchronous StopFailure test whose parent exits immediately, and verify that removing the exemption makes the completion-marker assertion fail.

中文说明

父进程退出豁免遗漏了同步 StopFailure 命令 Hook，但其文档契约和生产调用点都明确采用即发即忘方式。用户文档将 StopFailure 定义为 fire-and-forget，并把限流告警、认证失败日志列为用途；Core、CLI UI 和 ACP 调用点都不会等待 fireStopFailureEvent(...)。当前判断只豁免 MessageDisplay 与 async: true，因此 headless 错误或循环检测退出时会把同步 StopFailure 注册到父退出清理并在告警/审计记录完成前 SIGKILL。请把 HookEventName.StopFailure 纳入豁免，并补一个父进程立即退出的同步 StopFailure 进程测试；移除豁免后完成标记断言必须失败。

— gpt-5.6-sol@954e5164 via Qwen Code /review (v0.22.2)

[qwen-code-ci-bot]

Released in v0.22.3.