fix(acp): sweep review worktree leases at the end of each prompt turn

[wenshao]

Problem

Cancelling (or crashing out of) a /review in a Web Shell / daemon session leaves the review worktree .qwen/tmp/review-pr-<n> and the qwen-review/pr-<n> branch behind. They stay on disk until the next review of the same PR runs its stale-state cleanup, or someone runs qwen review cleanup pr-<n> by hand.

The root cause is one level below the missing cleanup call: the ACP prompt path never enters promptIdContext. Only the TUI (useGeminiStream.ts) and headless (nonInteractiveCli.ts) entry points wrap the turn in promptIdContext.run, so in daemon sessions getShellContextEnvVars emits an empty QWEN_CODE_PROMPT_ID, and createReviewWorktreeLease (called by qwen review fetch-pr) silently no-ops — the lease that interruption cleanup depends on is never recorded in the first place.

Fix

Two small changes in Session.ts, mirroring the existing surfaces:

• Bind the prompt ID in #executePromptInner right after it is computed, via promptIdContext.enterWith(promptId) — the analogue of the sessionIdContext.run wrapper in #executePrompt. enterWith (rather than run) keeps the ~500-line turn body unnested; the binding dies with the turn's async scope. Shell subprocesses now see the real prompt ID and the lease gets recorded.
• Sweep the leases in the turn-wide finally (the one that already emits ConversationFinishedEvent, which runs on every terminal path: end_turn, cancelled, thrown). Unconditional like the headless finally in nonInteractiveCli.ts — the ACP loop runs whole turns, so unlike the TUI's per-continuation submitQuery this can never fire mid-review. When the review completed normally, its own cleanup step already released the lease and the sweep is a no-op.

config.getProjectRoot() is the correct lease root even for worktree-isolated sessions: relocateWorkingDirectory updates targetDir when the daemon relocates a session into its worktree, matching the process.cwd() that fetch-pr records.

Tests

New Session.review-lease.test.ts (reuses the Session.worktree.test.ts harness — real Session, no module-level core mock):

• RL1: the turn body observes the prompt ID in promptIdContext (the binding shell subprocesses inherit).
• RL2: a completed prompt sweeps its leases with the exact {sessionId, promptId, repositoryRoot} triple.
• RL3: the sweep still runs when the model stream throws — the interrupted-review case this exists for.
• RL4: consecutive prompts sweep under their own prompt IDs.

The Session.worktree.test.ts mock config gains getProjectRoot (the new finally calls it on every turn; Session.test.ts and acpAgent.test.ts mocks already had it).

Full src/acp-integration suite: 1064/1064 passing.

Not covered (intentionally)

• Cron/notification turns still don't enter promptIdContext; they don't run /review, so kept out of scope.
• A SIGKILL of the daemon child mid-review still can't run the finally; the next same-PR review's stale-state cleanup remains the backstop, as before.

中文版

问题

在 Web Shell / daemon 会话中取消(或异常中断)一次 /review,会把 review worktree .qwen/tmp/review-pr-<n> 和分支 qwen-review/pr-<n> 留在磁盘上,直到下次审同一个 PR 时的陈旧状态清理兜底,或者手动执行 qwen review cleanup pr-<n>。

根因比"少了一处清理调用"更深一层:ACP prompt 通路从未进入 promptIdContext。只有 TUI(useGeminiStream.ts)和 headless(nonInteractiveCli.ts)入口用 promptIdContext.run 包裹整个 turn,因此 daemon 会话里 getShellContextEnvVars 注入的 QWEN_CODE_PROMPT_ID 是空串,qwen review fetch-pr 调用的 createReviewWorktreeLease 静默 no-op —— 中断清理所依赖的 lease 从一开始就没有被记录。

修复

Session.ts 里两处小改动,与既有通路对齐:

• 绑定 prompt ID:在 #executePromptInner 计算出 promptId 后立即 promptIdContext.enterWith(promptId),对应 #executePrompt 里已有的 sessionIdContext.run 包裹。用 enterWith(而非 run)是为了不给约 500 行的 turn 主体增加一层嵌套;绑定随 turn 的 async 作用域消亡。此后 shell 子进程能看到真实的 prompt ID,lease 得以记录。
• 清扫 lease:放在 turn 级 finally(即已发出 ConversationFinishedEvent 的那个,覆盖 end_turn / cancelled / 抛错所有终止路径)。与 headless 的 nonInteractiveCli.ts 一样无条件执行 —— ACP 循环一次跑完整个 turn,不像 TUI 的 submitQuery 按 tool-result continuation 分次调用,因此绝不会在 review 进行中途触发。review 正常完成时其自身的 cleanup 步骤已释放 lease,这里的清扫是 no-op。

对 worktree 隔离会话,config.getProjectRoot() 也是正确的 lease 根:daemon 把会话迁入 worktree 时 relocateWorkingDirectory 会更新 targetDir,与 fetch-pr 记录的 process.cwd() 一致。

测试

新增 Session.review-lease.test.ts(复用 Session.worktree.test.ts 的 harness —— 真实 Session,不做模块级 core mock):

• RL1:turn 主体内能观测到 promptIdContext 中的 prompt ID(即 shell 子进程继承的绑定)。
• RL2:prompt 正常完成后按精确的 {sessionId, promptId, repositoryRoot} 三元组清扫 lease。
• RL3:模型流抛错时清扫仍会执行 —— 正是本机制针对的"review 被中断"场景。
• RL4:连续多个 prompt 各自用自己的 prompt ID 清扫。

Session.worktree.test.ts 的 mock config 补充了 getProjectRoot(新的 finally 每个 turn 都会调用它;Session.test.ts 和 acpAgent.test.ts 的 mock 原本就有)。

src/acp-integration 全套:1064/1064 通过。

有意不覆盖

• cron/notification turn 仍未进入 promptIdContext;它们不会运行 /review,故不在本次范围内。
• review 进行中 daemon 子进程被 SIGKILL 时 finally 依然无法执行;和之前一样,由下次审同一 PR 的陈旧状态清理兜底。

[qwen-code-ci-bot]

✅ Qwen Triage finished — view run. See the stage comments above for the result.

✅ Qwen Triage 已完成 —— 查看运行。结果见上方各阶段评论。

[qwen-code-ci-bot]

Thanks for the PR!

Template: the body uses custom headings (Problem / Fix / Tests / Not covered) instead of the repo template's (What this PR does / Why it's needed / Reviewer Test Plan / Risk & Scope / Linked Issues). The content is thorough and covers the same ground, so not blocking on this — but worth aligning next time.

Problem: observed bug with a clear causal chain. The ACP prompt path never enters promptIdContext, so getShellContextEnvVars emits an empty QWEN_CODE_PROMPT_ID, createReviewWorktreeLease silently no-ops (its guard: if (!params.promptId) return), and an interrupted /review leaves .qwen/tmp/review-pr-<n> and its branch behind. The root cause analysis names the exact code paths that DO enter the context (useGeminiStream.ts, nonInteractiveCli.ts) and explains why ACP doesn't. This is a real resource leak, not theoretical hardening.

Direction: aligned — cleaning up after interrupted operations is basic reliability for the daemon/ACP surface. No CHANGELOG reference needed for a resource-leak fix.

Size: not applicable — all changes are in packages/cli/src/acp-integration/session/, not core paths. Production: 25 lines (Session.ts). Test: 248 lines (new test file + mock fix).

Approach: the scope is tight. Two changes in Session.ts — bind the prompt ID via enterWith, sweep leases in the turn's finally — both mirroring existing patterns in TUI and headless. No drive-by changes, no scope creep. The enterWith vs run choice is well-justified (avoids nesting the ~500-line turn body). The "Not covered" section shows good awareness of edge cases (cron turns, SIGKILL).

Moving on to code review. 🔍

中文说明

感谢贡献！

模板： PR 正文使用了自定义标题（Problem / Fix / Tests / Not covered），而非仓库模板的标题（What this PR does / Why it's needed / Reviewer Test Plan / Risk & Scope / Linked Issues）。内容充实，覆盖了相同的信息，因此不阻塞——但下次建议对齐模板格式。

问题： 已观测到的 bug，因果链清晰。ACP 的 prompt 路径从未进入 promptIdContext，导致 getShellContextEnvVars 输出空的 QWEN_CODE_PROMPT_ID，createReviewWorktreeLease 静默跳过（其守卫条件：if (!params.promptId) return），中断的 /review 会留下 .qwen/tmp/review-pr-<n> 及其分支。根因分析准确指出了确实进入该上下文的代码路径（useGeminiStream.ts、nonInteractiveCli.ts），并解释了 ACP 为何缺失。这是真实的资源泄漏，不是理论性加固。

方向： 对齐——中断操作后的清理是 daemon/ACP 层面的基本可靠性保障。

规模： 不适用——所有改动在 packages/cli/src/acp-integration/session/，不涉及核心路径。生产代码：25 行（Session.ts）。测试：248 行。

方案： 范围紧凑。Session.ts 中两处改动——通过 enterWith 绑定 prompt ID、在 turn 的 finally 中清理 lease——均镜像 TUI 和 headless 的现有模式。无顺手改动，无范围蔓延。

进入代码审查 🔍

— Qwen Code · qwen3.8-max-preview

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

[qwen-code-ci-bot]

Code Review

Independent proposal: given the root cause (ACP never enters promptIdContext, so lease creation silently no-ops), I would (1) bind the prompt ID in #executePromptInner right after it's computed, and (2) sweep leases in the turn-wide finally that already fires on every terminal path. Both changes mirror the existing TUI/headless patterns.

Comparison with the diff: the PR does exactly this. No simpler path missed.

Session.ts — promptIdContext.enterWith(promptId) (line ~2430): correct placement — right after promptId is computed, before any shell subprocess can spawn. enterWith (vs run) is the right call here: it binds the async context without nesting the ~500-line turn body. The binding dies with the turn's async scope, so no leak across turns. Verified that getShellContextEnvVars reads promptIdContext.getStore() to populate QWEN_CODE_PROMPT_ID — this is the exact gap the fix closes.

Session.ts — cleanupReviewWorktreeLeases(...) in finally (line ~3024): correct placement — inside the finally that already emits ConversationFinishedEvent, which runs on every terminal path (end_turn, cancelled, thrown). The call signature matches the headless pattern exactly: {sessionId, promptId, repositoryRoot}. Unconditional sweep is correct — when the review completed normally, its own cleanup already released the lease and this is a no-op (the lease file is gone, readdirSync finds nothing). config.getProjectRoot() is the right root even for worktree-isolated sessions, as the PR explains.

Session.worktree.test.ts — getProjectRoot mock addition: necessary because the new finally calls config.getProjectRoot() on every turn. The existing mock was missing it.

Session.review-lease.test.ts: four focused tests (RL1–RL4) covering the binding, the sweep, the error path, and consecutive prompts. Reuses the Session.worktree.test.ts harness pattern (real Session, no module-level core mock). The cleanupReviewWorktreeLeases mock via vi.hoisted follows the project convention for mocks consumed by vi.mock.

No correctness bugs, security holes, or regressions found. No AGENTS.md violations — the change is minimal, follows existing patterns, and stays in the right package.

CI Test Evidence

Fetched via the checks API for commit bec44156:

Check	Status	Conclusion
precheck-pr / precheck	completed	✅ success
Classify PR	completed	✅ success
Real daemon E2E / Java 11	completed	✅ success
ubuntu-latest / Java 21	completed	✅ success
ubuntu-latest / Java 17	completed	✅ success
ubuntu-latest / Java 11	completed	✅ success
macos-latest / Java 21	completed	✅ success
windows-latest / Java 21	completed	✅ success
Test (ubuntu-latest, Node 22.x)	in_progress	— still running at review time
Test (macos-latest, Node 22.x)	completed	skipped
Test (windows-latest, Node 22.x)	completed	skipped
Integration Tests (CLI, No Sandbox)	completed	skipped

The Node test suite on ubuntu was still running after ~5 minutes of polling. All completed checks pass. No failures.

Not verified: real-scenario tmux testing (CI run — a maintainer can trigger the isolated @qwen-code /tmux job if the ACP lease behavior warrants live verification).

中文说明

代码审查

独立方案： 根据根因（ACP 从未进入 promptIdContext，导致 lease 创建静默跳过），我会 (1) 在 #executePromptInner 中计算 promptId 后立即绑定，(2) 在 turn 级别的 finally 中清理 lease。两处改动均镜像 TUI/headless 的现有模式。

与 diff 对比： PR 正是这样做的。没有遗漏更简路径。

Session.ts — promptIdContext.enterWith(promptId)： 位置正确——在 promptId 计算之后、任何 shell 子进程启动之前。enterWith（而非 run）是正确选择：绑定异步上下文而不嵌套 ~500 行的 turn 方法体。绑定随 turn 的异步作用域消亡，不会跨 turn 泄漏。

Session.ts — cleanupReviewWorktreeLeases(...) 在 finally 中： 位置正确——在已发出 ConversationFinishedEvent 的 finally 块内，该块在每个终止路径（end_turn、cancelled、thrown）都会执行。调用签名与 headless 模式完全一致。无条件清理是正确的——当 review 正常完成时，其自身清理已释放 lease，此处为 no-op。

测试： 四个聚焦测试（RL1–RL4）覆盖绑定、清理、错误路径和连续 prompt。复用 Session.worktree.test.ts 的测试框架模式。

未发现正确性 bug、安全漏洞或回归。无 AGENTS.md 违规。

CI 测试证据

通过 checks API 获取 commit bec44156 的结果：所有已完成的检查均通过。Node 测试套件（ubuntu）在审查时仍在运行。无失败。

未验证：真实场景 tmux 测试（CI 运行——如需验证 ACP lease 行为，维护者可触发隔离的 @qwen-code /tmux 任务）。

— Qwen Code · qwen3.8-max-preview

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

[qwen-code-ci-bot]

Confidence: 5/5 — clean, minimal fix that closes a real resource leak in the ACP path by mirroring the existing TUI/headless patterns exactly.

This is the kind of PR that's easy to review and easy to maintain. 25 production lines, two changes that each have a clear analogue in the codebase, and four focused tests. The root cause analysis in the PR description is precise — it traces the exact chain from missing promptIdContext binding → empty QWEN_CODE_PROMPT_ID → createReviewWorktreeLease no-op → orphaned worktree. The fix addresses both halves: the binding (so leases get recorded) and the sweep (so they get cleaned up on interruption).

My independent proposal matched the PR's approach exactly — I didn't find a simpler path. The enterWith vs run choice is well-reasoned, the unconditional sweep in finally is correct (no-op when the lease was already released), and the "Not covered" section shows the author thought about the boundaries (cron turns, SIGKILL).

If I had to maintain this in six months, I'd thank the author — the comments explain the why, the tests cover the key paths, and the change doesn't add any new abstraction.

中文说明

置信度：5/5 — 干净、最小化的修复，通过精确镜像 TUI/headless 的现有模式，关闭了 ACP 路径中的真实资源泄漏。

这是一个容易审查、容易维护的 PR。25 行生产代码，两处改动各有明确的代码库先例，四个聚焦测试。PR 描述中的根因分析精确——追踪了从缺失 promptIdContext 绑定 → 空 QWEN_CODE_PROMPT_ID → createReviewWorktreeLease 静默跳过 → 孤立 worktree 的完整链条。修复同时解决了两个环节：绑定（使 lease 被记录）和清理（使中断时 lease 被清除）。

我的独立方案与 PR 完全一致——没有找到更简路径。enterWith vs run 的选择有充分理由，finally 中的无条件清理是正确的（lease 已释放时为 no-op），"Not covered" 部分表明作者考虑了边界情况。

— Qwen Code · qwen3.8-max-preview

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

[qwen-code-ci-bot]

LGTM, looks ready to ship. ✅

[qwen-code-ci-bot]

⚠️ Downgraded from Approve to Comment: CI still running. Reviewed.

中文说明

⚠️ 已从批准降级为评论：CI still running。 已审查。

— qwen3.7-max via Qwen Code /review

[wenshao]

Code Review — fix(acp): sweep review worktree leases at the end of each prompt turn

Verdict: ⚠️ Request changes. The root-cause analysis is right and promptIdContext.enterWith is the correct half of the fix. But the unconditional sweep in the turn-wide finally also fires on a stop that ACP deliberately makes resumable, and force-deletes a live review worktree out from under the user. Reproduced with an executable test against the real Session.

What it does

Binds promptId into promptIdContext at the top of #executePromptInner (Session.ts:2439) so shell subprocesses inherit QWEN_CODE_PROMPT_ID and qwen review fetch-pr can record its lease, then sweeps that prompt's leases in the turn-wide finally (Session.ts:3032).

Verified ✅

1. Root cause is exactly as described. getShellContextEnvVars writes QWEN_CODE_PROMPT_ID = promptIdContext.getStore() ?? '' (shellContextEnv.ts:136-137) and createReviewWorktreeLease returns early on a falsy promptId (review-worktree-lease.ts:66). Only nonInteractiveCli.ts:357 and useGeminiStream.ts:2768 entered the context; ACP never did.
2. getProjectRoot() is relocation-safe. It returns this.targetDir (config.ts:4420-4422) and relocateWorkingDirectory assigns this.targetDir = expected (config.ts:4380). The claim in the PR body holds.
3. The finally can't mask the turn's own error — cleanupReviewWorktreeLeases wraps its whole body in try/catch and only debug-logs.
4. A poisoned lease file can't be turned into arbitrary deletion — removeLeaseWorktree re-derives the expected branch from target and rejects any worktreePath outside <root>/.qwen/tmp.
5. Suite is green: src/acp-integration → 1061 passed here. (authPreflight.test.ts failed to load in my worktree on a missing generated src/generated/git-commit.js — environmental, unrelated.)

🔴 Blocking — the sweep fires on ACP's resumable permission-cancel stop

The PR body argues: "the ACP loop runs whole turns, so unlike the TUI's per-continuation submitQuery this can never fire mid-review." ACP does have a mid-turn stop that is meant to be resumed — stopAfterPermissionCancel.

Clicking Reject on any permission prompt maps to ToolConfirmationOutcome.Cancel (permissionUtils.ts:23-27, reject_once), which returns stopAfterPermissionCancel: true (Session.ts:7293/7337 → 7201/7249). #executePromptInner then calls #preserveStoppedToolRun — which deliberately preserves the unsent tool responses in history so a later prompt continues the same work — and returns from inside the try at Session.ts:2966-2973. The new finally at 3016 therefore runs and force-removes .qwen/tmp/review-pr-<n> plus git branch -D qwen-review/pr-<n>. The user then says "continue", and the review agents — pinned to working_dir: "<worktreePath>" by SKILL.md Step 3 — resume against a directory that no longer exists.

Verified against the real Session using your own harness (one tool call, requestPermission → {outcome:{outcome:'cancelled'}}):

STOP REASON: {"stopReason":"end_turn"}
SWEEP CALLS: 1 [{"sessionId":"lease-test-session",
                 "promptId":"lease-test-session########1",
                 "repositoryRoot":"/repo"}]

Note the stop reason is end_turn, not cancelled — the host renders a normally completed turn, so nothing tells the user their review worktree was just deleted.

This is precisely why the TUI gates its sweep behind cleanupReviewLease, set only on UserCancelled (2890), loopDetected (2948) and the error path (2990) — never on an ordinary tool-loop stop.

Suggested fix — keep it unconditional except on the one resumable path:

// in #executePromptInner, before the try
let sweepReviewLease = true;
...
if (toolRun.stopAfterPermissionCancel) {
  // The tool run is preserved for the user to resume;
  // the review worktree has to survive with it.
  sweepReviewLease = false;
  ...
}
...
} finally {
  ...
  if (sweepReviewLease) {
    cleanupReviewWorktreeLeases({ ... });
  }
}

loopDetected can keep sweeping — the TUI does the same.

Worth widening the question once while you're here: the sweep assumes nothing holding a lease outlives the turn. That happens to hold for /review's own agents today only because working_dir launches are forced to the foreground — see agent.ts:1099-1100, whose rationale is literally "the caller owns the worktree lifecycle and could remove it while a background agent is still running." It would stop holding the day a review is launched from a background agent.

🟠 Test isolation — the real sweep now runs unmocked in the ACP unit suites

Session.worktree.test.ts, Session.test.ts and acpAgent.test.ts do not mock ../../services/review-worktree-lease.js (only the new file does), so every prompt turn in them now executes the real sweep against the mocked project root — /tmp and /workspace. That means real existsSync/readdirSync, real execFileSync('git', …), and a real recursive rmSync.

Not hypothetical — I planted a matching lease and ran the suite unchanged:

$ mkdir -p /tmp/.qwen/tmp/review-pr-999/marker
$ cat > /tmp/.qwen/tmp/qwen-review-lease-pr-999.json   # sessionId wt-test-session, promptId ...########1
$ npx vitest run src/acp-integration/session/Session.worktree.test.ts
  Tests  8 passed (8)
$ ls -R /tmp/.qwen/tmp
  qwen-review-lease-pr-999.json          # review-pr-999/ was deleted by the unit test

Please add vi.mock('../../services/review-worktree-lease.js') to those three suites as well (or point their getProjectRoot at a per-test tmpdir). As it stands, a developer who happens to have /tmp/.qwen/tmp gets git subprocesses and directory deletions out of a unit run.

🟠 The sweep is synchronous, and it is now on the daemon's event loop

cleanupReviewWorktreeLeases is fully blocking: execFileSync with GIT_TIMEOUT_MS = 120_000 per call, plus a recursive rmSync of an entire checkout on the fallback path. In headless that runs once at process end; in the daemon it runs at the end of every turn, on the event loop shared by every other session and by the ACP JSON-RPC transport. One slow git worktree remove stalls all of them.

nonInteractiveCli.ts:405-407 already passes gitTimeout: 1_000 for its exit-handler path — the same reasoning applies more strongly here. Suggest passing a short gitTimeout, and ideally moving the sweep off the turn's critical path (it currently sits inside withInteractionSpan, so its blocking time also lands in the interaction-duration telemetry). At minimum, note that the steady-state existsSync + readdirSync of <root>/.qwen/tmp now runs on every turn.

🔵 Nit — the enterWith comment overstates the guarantee

enterWith (not run) so the 500-line turn body below stays unnested; the binding dies with this async scope.

enterWith has no scope exit — it writes the store into the current async context and everything downstream keeps it. Standalone it does escape upward past an await:

async function inner() { await Promise.resolve(); als.enterWith('turn-1'); }
als.getStore();   // undefined
await inner();
als.getStore();   // 'turn-1'  ← leaked to the caller

In Session.ts it is in fact contained — I confirmed promptIdContext.getStore() is undefined after await session.prompt(...) on Node 22.22.2 — but only because #executePromptInner runs inside Storage.runWithRuntimeBaseDir(...) / sessionIdContext.run(...), whose frame restore reverts it. Worth rewording so the comment credits those wrappers, otherwise a later refactor that drops one silently starts leaking prompt IDs into cron/notification turns.

Test coverage

RL1–RL4 are well built — real Session, no module-level core mock, and RL3 covers the throw path, which is the case that matters most. The gap is exactly the case that is wrong: no test for a turn ending via stopAfterPermissionCancel, and none asserting the sweep is skipped for a resumable stop. Worth adding alongside the fix; the harness already supports it (I only had to add getMaxToolCallsPerTurn, getHistoryFunctionResponseIds and a few tool-path config methods).

---

Nice diagnosis — the promptIdContext half is clearly right and worth landing on its own. It's the sweep's trigger condition that needs narrowing before this is safe in a daemon.

中文说明

结论：⚠️ 建议修改后合并。 根因分析准确，promptIdContext.enterWith 是正确的一半修复。但 turn 级 finally 里的无条件清扫同样会在 ACP 有意设计为可恢复的那个停止点上触发，把用户正在使用的 review worktree 强制删掉。已用针对真实 Session 的可执行测试复现。

做了什么：在 #executePromptInner 顶部把 promptId 绑定进 promptIdContext（Session.ts:2439），使 shell 子进程继承 QWEN_CODE_PROMPT_ID、qwen review fetch-pr 得以记录 lease；随后在 turn 级 finally 清扫该 prompt 的 lease（Session.ts:3032）。

已验证 ✅

1. 根因与描述完全一致：getShellContextEnvVars 写入 QWEN_CODE_PROMPT_ID = promptIdContext.getStore() ?? ''（shellContextEnv.ts:136-137），createReviewWorktreeLease 在 promptId 为假值时直接返回（review-worktree-lease.ts:66）。只有 nonInteractiveCli.ts:357 与 useGeminiStream.ts:2768 进入了该 context，ACP 从未进入。
2. getProjectRoot() 对 relocate 是安全的：它返回 this.targetDir（config.ts:4420-4422），而 relocateWorkingDirectory 会赋值 this.targetDir = expected（config.ts:4380）。PR 描述中的说法成立。
3. finally 不会掩盖 turn 自身的异常：cleanupReviewWorktreeLeases 整体包在 try/catch 中，只做 debug 日志。
4. 被污染的 lease 文件无法变成任意删除：removeLeaseWorktree 会从 target 重新推导期望的分支名，并拒绝任何位于 <root>/.qwen/tmp 之外的 worktreePath。
5. 测试通过：src/acp-integration 本地 1061 passed。（authPreflight.test.ts 在我的 worktree 里因缺少构建生成的 src/generated/git-commit.js 而加载失败，属环境问题，与本 PR 无关。）

🔴 阻塞项 —— 清扫会在 ACP 的「权限被拒绝」这个可恢复停止点上触发

PR 描述认为："ACP 循环一次跑完整个 turn，不像 TUI 的 submitQuery 按 continuation 分次调用，因此绝不会在 review 进行中途触发。" 但 ACP 确实存在一个设计为可恢复的中途停止点 —— stopAfterPermissionCancel。

权限弹窗上点 Reject 会映射为 ToolConfirmationOutcome.Cancel（permissionUtils.ts:23-27，reject_once），进而返回 stopAfterPermissionCancel: true（Session.ts:7293/7337 → 7201/7249）。此时 #executePromptInner 会调用 #preserveStoppedToolRun —— 它有意把未发送的 tool response 保留进 history，好让后续 prompt 接着做同一件事 —— 然后从 try 内部的 Session.ts:2966-2973 处 return。于是 3016 的新 finally 照常执行，强制删除 .qwen/tmp/review-pr-<n> 并 git branch -D qwen-review/pr-<n>。用户接着说"继续"，而 review agent 被 SKILL.md Step 3 用 working_dir: "<worktreePath>" 钉死在一个已经不存在的目录上。

用你自己的 harness 针对真实 Session 验证（一次 tool call，requestPermission → {outcome:{outcome:'cancelled'}}）：

STOP REASON: {"stopReason":"end_turn"}
SWEEP CALLS: 1 [{"sessionId":"lease-test-session",
                 "promptId":"lease-test-session########1",
                 "repositoryRoot":"/repo"}]

注意 stopReason 是 end_turn 而非 cancelled —— 宿主渲染出来是一个正常结束的 turn，用户完全不会知道自己的 review worktree 刚被删了。

这正是 TUI 用 cleanupReviewLease 把清扫挡住的原因：它只在 UserCancelled（2890）、loopDetected（2948）和错误路径（2990）置位，绝不在普通的 tool-loop 停止上置位。

建议改法 —— 保持无条件，仅排除这一条可恢复路径：

// #executePromptInner 中，try 之前
let sweepReviewLease = true;
...
if (toolRun.stopAfterPermissionCancel) {
  // tool run 被保留下来供用户恢复，review worktree 必须一起存活
  sweepReviewLease = false;
  ...
}
...
} finally {
  ...
  if (sweepReviewLease) {
    cleanupReviewWorktreeLeases({ ... });
  }
}

loopDetected 可以继续清扫 —— TUI 也是这么做的。

顺带值得想一遍：这个清扫假定"没有任何持有 lease 的东西会活得比 turn 更久"。这一点今天对 /review 自己的 agent 成立，仅仅是因为带 working_dir 的启动被强制为前台 —— 见 agent.ts:1099-1100，其理由原文就是*"caller 拥有 worktree 生命周期，可能在后台 agent 仍在运行时把它删掉"*。哪天 review 被从后台 agent 里拉起，这个假定就不成立了。

🟠 测试隔离 —— 真实的清扫函数现在会在 ACP 单测里无 mock 执行

Session.worktree.test.ts、Session.test.ts 和 acpAgent.test.ts 都没有 mock ../../services/review-worktree-lease.js（只有新增文件 mock 了），因此这些套件里的每个 prompt turn 现在都会拿 mock 的 project root（/tmp 和 /workspace）去执行真实清扫：真实的 existsSync/readdirSync、真实的 execFileSync('git', …)、真实的递归 rmSync。

不是假设 —— 我放了一个匹配的 lease，然后原样跑套件：

$ mkdir -p /tmp/.qwen/tmp/review-pr-999/marker
$ cat > /tmp/.qwen/tmp/qwen-review-lease-pr-999.json   # sessionId wt-test-session, promptId ...########1
$ npx vitest run src/acp-integration/session/Session.worktree.test.ts
  Tests  8 passed (8)
$ ls -R /tmp/.qwen/tmp
  qwen-review-lease-pr-999.json          # review-pr-999/ 被单测删掉了

建议在这三个套件里同样加上 vi.mock('../../services/review-worktree-lease.js')（或把它们的 getProjectRoot 指向每个测试独立的临时目录）。按现状，只要开发者机器上恰好有 /tmp/.qwen/tmp，一次单测运行就会拉起 git 子进程并删除真实目录。

🟠 清扫是同步的，而它现在跑在 daemon 的事件循环上

cleanupReviewWorktreeLeases 完全阻塞：每次 git 调用用 execFileSync 且 GIT_TIMEOUT_MS = 120_000，回退路径还会对整个 checkout 做递归 rmSync。headless 下它只在进程退出时跑一次；daemon 下它在每个 turn 结束时跑，且占用的是所有其他会话和 ACP JSON-RPC 传输共用的事件循环。一次慢的 git worktree remove 会把它们全部卡住。

nonInteractiveCli.ts:405-407 已经为退出钩子路径传了 gitTimeout: 1_000 —— 同样的理由在这里只会更强。建议传一个较短的 gitTimeout，并尽量把清扫移出 turn 的关键路径（目前它位于 withInteractionSpan 内部，阻塞时间还会计入 interaction 时长遥测）。至少要意识到：稳态下每个 turn 都会对 <root>/.qwen/tmp 做一次 existsSync + readdirSync。

🔵 小问题 —— enterWith 的注释把保证说过头了

用 enterWith（而非 run）是为了不给约 500 行的 turn 主体增加一层嵌套；绑定随 turn 的 async 作用域消亡。

enterWith 没有作用域退出这回事 —— 它把 store 写进当前 async context，下游一切都会继续持有。单独看，它确实会越过 await 向上泄漏到调用方：

async function inner() { await Promise.resolve(); als.enterWith('turn-1'); }
als.getStore();   // undefined
await inner();
als.getStore();   // 'turn-1'  ← 泄漏到了调用方

在 Session.ts 里它确实被兜住了 —— 我在 Node 22.22.2 上确认 await session.prompt(...) 返回后 promptIdContext.getStore() 是 undefined —— 但那只是因为 #executePromptInner 跑在 Storage.runWithRuntimeBaseDir(...) / sessionIdContext.run(...) 内部，其 frame 恢复顺带把它还原了。建议改写注释，把功劳记在这些包装上；否则日后有人重构掉其中一层，prompt ID 就会悄悄泄漏进 cron / notification turn。

测试覆盖

RL1–RL4 写得不错 —— 真实 Session、没有模块级 core mock，RL3 覆盖了抛错路径，那是最要紧的一条。缺口恰好就是出问题的那条：没有覆盖以 stopAfterPermissionCancel 结束的 turn，也没有断言可恢复停止时不应清扫。建议随修复一起补上；harness 本身已经够用（我只额外补了 getMaxToolCallsPerTurn、getHistoryFunctionResponseIds 和几个 tool 路径上的 config 方法）。

---

诊断很漂亮 —— promptIdContext 那一半明显是对的，单独合入就有价值。需要收窄的是清扫的触发条件，这样在 daemon 里才安全。

---

🤖 Generated with Claude Code — Claude Opus 5 (1M context)

[doudouOUC]

No issues found. LGTM! ✅

— qwen3.7-max via Qwen Code /review

[doudouOUC]

Reviewed. Suggestions are inline. Not reviewed: You are review agent 1c — Agent 1c: Cross-file tracer., You are review agent 5 — Agent 5: Test coverage., You are review agent 6b — Agent 6b: Undirected audit — ..., You are review agent 7 — Agent 7: Build & test verifica..., You are review agent 1a — Agent 1a: Line-by-line correc..., You are review agent 3 — Agent 3: Code quality., You are review agent 2 — Agent 2: Security., You are review agent 0 — Agent 0: Issue fidelity & root..., You are review agent 6a — Agent 6a: Undirected audit — ... — the agent made no tool call: it read nothing. Not reviewed: Agent 0: Issue fidelity & root-cause ownership — never opened its brief (/Users/jinye.djy/Projects/qwen-code/.qwen/tmp/qwen-review-pr-7694-fetch-prompts/0.brief.md), so it reviewed without the instructions it was launched to follow. Not reviewed: Agent 1a: Line-by-line correctness — never opened its brief (/Users/jinye.djy/Projects/qwen-code/.qwen/tmp/qwen-review-pr-7694-fetch-prompts/1a.brief.md), so it reviewed without the instructions it was launched to follow. Not reviewed: Agent 2: Security — never opened its brief (/Users/jinye.djy/Projects/qwen-code/.qwen/tmp/qwen-review-pr-7694-fetch-prompts/2.brief.md), so it reviewed without the instructions it was launched to follow. Not reviewed: Agent 3: Code quality — never opened its brief (/Users/jinye.djy/Projects/qwen-code/.qwen/tmp/qwen-review-pr-7694-fetch-prompts/3.brief.md), so it reviewed without the instructions it was launched to follow. Not reviewed: Agent 5: Test coverage — never opened its brief (/Users/jinye.djy/Projects/qwen-code/.qwen/tmp/qwen-review-pr-7694-fetch-prompts/5.brief.md), so it reviewed without the instructions it was launched to follow. Not reviewed: Agent 6a: Undirected audit — attacker mindset — never opened its brief (/Users/jinye.djy/Projects/qwen-code/.qwen/tmp/qwen-review-pr-7694-fetch-prompts/6a.brief.md), so it reviewed without the instructions it was launched to follow. Not reviewed: Agent 6b: Undirected audit — 3 AM oncall mindset — never opened its brief (/Users/jinye.djy/Projects/qwen-code/.qwen/tmp/qwen-review-pr-7694-fetch-prompts/6b.brief.md), so it reviewed without the instructions it was launched to follow. Not reviewed: Agent 1c: Cross-file tracer — never opened its brief (/Users/jinye.djy/Projects/qwen-code/.qwen/tmp/qwen-review-pr-7694-fetch-prompts/1c.brief.md), so it reviewed without the instructions it was launched to follow. Not reviewed: Agent 7: Build & test verification — never opened its brief (/Users/jinye.djy/Projects/qwen-code/.qwen/tmp/qwen-review-pr-7694-fetch-prompts/7.brief.md), so it reviewed without the instructions it was launched to follow. Not reviewed: reverse audit — no auditor was launched with a prompt this skill builds — the pass that hunts what the rest of the review missed ran, if at all, without the method its brief carries. Not reviewed: verification — the review posts findings, but no verifier was launched with a prompt this skill builds — they were ruled on, if at all, without the verdict bar its brief carries.

— qwen3.7-max via Qwen Code /review

[doudouOUC]

[Critical] The sweep fires unconditionally, but stopAfterPermissionCancel is a resumable stop — the turn returns from inside the try at line ~2970 after #preserveStoppedToolRun deliberately keeps the tool run in history for the user to resume. This finally then deletes .qwen/tmp/review-pr-<n> and git branch -D qwen-review/pr-<n>. When the user says "continue", review agents pinned to working_dir: "<worktreePath>" by SKILL.md Step 3 operate against a deleted directory.

This is exactly why the TUI gates its sweep behind cleanupReviewLease, set only on UserCancelled, loopDetected, and the error path — never on an ordinary tool-loop stop.

Failure scenario: user clicks Reject on a permission prompt during /review → stopAfterPermissionCancel: true → this finally deletes the worktree → user says "continue" → agents fail on missing directory.

Suggested change

	cleanupReviewWorktreeLeases({
	sessionId: this.config.getSessionId(),
	promptId,
	repositoryRoot: this.config.getProjectRoot(),
	});
	if (!toolRun?.stopAfterPermissionCancel) {
	cleanupReviewWorktreeLeases({
	sessionId: this.config.getSessionId(),
	promptId,
	repositoryRoot: this.config.getProjectRoot(),
	});
	}

Secondary concern: the sweep is fully synchronous (execFileSync with 120 s default timeout + recursive rmSync) on the daemon's event loop, blocking every other session and the ACP transport. Consider passing a short gitTimeout (the headless path already uses gitTimeout: 1_000 at nonInteractiveCli.ts:405-407).

— qwen3.7-max via Qwen Code /review

[doudouOUC]

[Suggestion] Session.worktree.test.ts, Session.test.ts, and acpAgent.test.ts do not mock ../../services/review-worktree-lease.js — only the new Session.review-lease.test.ts does. Every prompt turn in those suites now runs the real sweep against the mocked project root (/tmp), executing real existsSync/readdirSync, execFileSync('git', …), and recursive rmSync.

Concrete cost: a developer with /tmp/.qwen/tmp/review-pr-<n> on disk gets real git subprocesses and directory deletions from a unit test run.

Suggested fix: add vi.mock('../../services/review-worktree-lease.js') to those three suites as well.

— qwen3.7-max via Qwen Code /review