feat(cli): adopt Goal v3 in interactive TUI - #8005
Conversation
Goal v3 TUI PR7 E2EDate: 2026-07-29 Binary: locally built Runtime isolation: timestamped Scenarios
Defects found and repairedUnsupported stop-token completionThe synthetic continuation prompt copied the objective verbatim. A model The synthetic turn now states explicitly that it contains no new user input and Queued ordinary input entered the GoalAt an idle boundary between autonomous Goal turns, active-mode dequeue selected Active-mode dequeue now allows Goal controls first, then a hidden Goal EvidenceEach meaningful transition was captured with
PNG screenshots will be captured from an attached terminal for the queue-held, Automated verification
|
|
Re-triage at the current head ( Template looks good ✓ Problem: still valid and unchanged — this is a feature adoption, not a bug fix. The Goal v3 runtime landed in core via #7895, but the interactive TUI still ran the legacy commands and a single delivery lane. The PR describes concrete mismatches observed during real terminal testing: premature completion, ordinary queued messages leaking into an active Goal, inconsistent pause/resume, and incomplete recovery after restart or branching. Observed gaps with a clear before/after, not theoretical hardening. Direction: aligned. The runtime is already in Size: 2702 production lines (additions + deletions, excluding tests) across 28 production files, plus 3883 test lines across 19 test files (47 files total). Core paths ( Approach: the scope feels right for the stated goal — lifecycle commands, persistent cards/footer, a two-lane input queue, and Goal-aware resume/branch recovery are all needed to make the TUI a faithful host, and scoping out non-interactive/ACP/SDK/Web/WebShell/Desktop is the correct call. The two correctness fixes (synthetic continuation prompts no longer counting objective text as evidence, and completion claims requiring real Risk: no elevated risk signals — no high-risk revert-prone paths matched. Moving on to code review. 🔍 中文说明在当前 head（ 模板完整 ✓ 问题：仍然成立且不变——这是功能接入，不是 bug 修复。Goal v3 运行时已通过 #7895 合入 core，但交互式 TUI 仍使用旧命令和单一交付通道。PR 描述了真实终端测试中观察到的具体错位：过早完成、普通排队消息泄漏进活动 Goal、暂停/恢复不一致、重启或分支后恢复不完整。是有明确 before/after 的已观测差距，不是理论性加固。 方向：对齐。运行时已在 规模：2702 行生产代码（加减，不含测试），分布在 28 个生产文件，另有 3883 行测试代码，19 个测试文件（共 47 个文件）。核心路径（ 方案：范围与目标匹配——生命周期命令、持久化卡片/底部状态、双通道输入队列、Goal 感知的恢复/分支继承都是让 TUI 成为可靠宿主所必需的，排除非交互/ACP/SDK/Web/WebShell/Desktop 是正确的。两个正确性修复与接入紧密耦合，应包含在此处。 风险：无升级风险信号——未匹配高风险易回滚路径。 进入代码审查 🔍 — Qwen Code · qwen3.8-max-preview Reviewed at |
Code ReviewIndependent proposal vs. actual approach: I would have structured this the same way — a separate Goal-turn queue in I re-read the core touches in detail at this head — no critical blockers. They are minimal (50 production lines) and sound:
Observations worth noting (none blocking):
sequenceDiagram
participant P1 as Goal Runtime
participant P2 as Message Queue
participant P3 as Drain Effect
participant P4 as submitQuery
participant P5 as Stream Handler
participant P6 as Tool Scheduler
P1->>P2: enqueueGoalTurn (permit, context)
P3->>P2: popNextSubmission (priority)
P2-->>P3: QueuedGoalTurn
P3->>P4: submitQuery (Goal, metadata)
P4->>P4: bindGoalTurn (permit, turnKey)
P4->>P5: processGeminiStreamEvents
P5->>P6: scheduleToolCalls (goalBinding)
P6-->>P5: tool results + terminateTurn
alt terminateTurn
P5->>P1: finishTurn (permit)
P1-->>P5: snapshot (complete or active)
else error or cancel
P5->>P1: failClosedGoalTurn (pause + release)
end
Files changed (30 of 47 shown)
CI Test EvidenceAt the reviewed commit
One row per check name (latest run); skipped checks omitted; failures sort first. / 每个检查名一行（取最新一次运行），省略 skipped，失败项排在最前。 Sandboxed verification would settle the behavioural claim: 中文说明代码审查独立方案与实际方案对比： 我的独立方案与 PR 结构一致——在 在当前 head 详细复查了 core 改动——无关键阻塞。 改动很小（50 行生产代码）且合理：
值得注意的观察（均非阻塞）： CI 测试证据在审查提交 沙箱验证可确认行为声明： — Qwen Code · qwen3.8-max-preview Reviewed at |
|
Confidence: 3/5 — clean review with no blocking issues at the current head, but the 2702-production-line, cross-package scope triggers the Stage 0 maintainer-awareness escalation, which caps the score and keeps the bot from auto-approving. This is a well-executed feature PR. The Goal v3 runtime is already in The 3/5 is pure policy, not doubt. Since the last pass, @wenshao has approved this exact head — that is the maintainer sign-off the scale escalation asks for. My own approval is a separate vote (
⏸️ Deferring to @wenshao (cc @yiliang114) — the review is clean and @wenshao has already approved, but the production-line scale and cross-package core touch keep this behind the two-tier gate's maintainer-awareness cap, so the bot is not adding its own approval. For the record: my standing changes-requested review on this PR reflects that policy cap, not any blocking code issue — I found none. The merge call now rests with the maintainers. 中文说明置信度：3/5——在当前 head 审查干净、无阻塞问题，但 2702 行生产代码、跨包的范围触发了 Stage 0 维护者关注升级，限制了评分，并使机器人不自动批准。 这是一个执行良好的功能 PR。Goal v3 运行时已在 3/5 是纯策略性的，不是疑虑。自上次审查以来，@wenshao 已批准了这个完全相同的 head——这正是规模升级所要求的维护者签字。我自己的批准是独立的一票（
⏸️ 转交 @wenshao（抄送 @yiliang114）——审查干净，@wenshao 已批准，但生产代码规模和跨包核心触及使其仍处于两级门控的维护者关注上限之下，因此机器人不添加自己的批准。特此说明：我对本 PR 现有的 changes-requested 审查反映的是该策略上限，而非任何阻塞性代码问题——我未发现任何阻塞问题。合并决定现由维护者作出。 — Qwen Code · qwen3.8-max-preview Reviewed at |
|
Fixed the exact-head Ubuntu CI failures in
Verification:
|
|
@qwen-code /takeover |
qwen-code-dev-bot
added
the
autofix/takeover
|
🤝 Takeover engaged: the autofix loop now manages this PR — it will address new review feedback and resolve base conflicts until the label is removed or the round cap is reached. Remove the 中文说明🤝 已接管：autofix 循环现在管理此 PR —— 将持续处理新的评审反馈与 base 冲突，直到移除标签或达到轮次上限。移除 |
|
🤖 Reviewed the latest feedback — no changes needed. Why, point by point: · 已审阅最新反馈——无需改动。逐点说明原因如下： Autofix review round — no action neededThis round surfaced no actionable feedback, so no code changes were made.
The PR head is left unchanged. 中文说明Autofix 评审轮次 —— 无需处理本轮没有出现可处理的反馈，因此未做任何代码改动。
PR 的 head 保持不变。 Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 🧠 Handled by Qwen Code · model/模型 |
doudouOUC
left a comment
doudouOUC
left a comment
There was a problem hiding this comment.
Not reviewed: the entire diff, the linked-issue fidelity pass, the whole-diff test-coverage check, the removed-behavior audit, the invariant check (state, timers, collections) on packages/cli/src/ui/hooks/useGeminiStream.ts, the invariant check (counters, return values, error taxonomies) on packages/cli/src/ui/hooks/useGeminiStream.ts, the invariant check (config fields, early returns) on packages/cli/src/ui/hooks/useGeminiStream.ts — its prompt was built, but no agent on record was launched with it.
中文说明
未审查：整个 diff、关联 issue 一致性检查、全 diff 测试覆盖检查、删除行为审计、不变量检查（状态、定时器、集合）（packages/cli/src/ui/hooks/useGeminiStream.ts）、不变量检查（计数器、返回值、错误分类）（packages/cli/src/ui/hooks/useGeminiStream.ts）、不变量检查（配置字段、提前返回）（packages/cli/src/ui/hooks/useGeminiStream.ts）——它的 prompt 已构建，但没有任何 agent 有记录用它启动过。
— qwen3.7-max via Qwen Code /review
|
Qwen Code review timed out. Qwen review timed out after 10800 seconds (of the 180-minute budget). For large PRs, retry with a longer timeout by commenting: |
doudouOUC
left a comment
doudouOUC
left a comment
There was a problem hiding this comment.
Not reviewed: every dimension — none of the 30 required agents is on record as launched with a prompt this skill built, so this diff was reviewed, if at all, from prompts the run wrote for itself: no record shows the severity bar, the finding format or this project's own rules reaching an agent.
Not reviewed: reverse audit — an auditor ran and opened its brief, but no agent was launched with the prompt the CLI built — the launch was written by hand, and what the agent was actually asked is not what this skill certifies.
[Critical] Missing failClosedGoalTurn on backgroundLaunchExhaustedCapacity early return (useGeminiStream.ts:~4209) — The PR adds toolGoalBinding and failClosedGoalTurn cleanup to 7 early-return paths in handleCompletedTools, but the pre-existing backgroundLaunchExhaustedCapacity path was missed. When a Goal turn's tool batch includes a background agent tool and capacity is exhausted, the function returns without releasing the binding — goalTurnBindingsRef retains the stale entry, activeGoalTurnRef keeps pointing to it, and the Goal runtime is never told the turn ended, so the Goal loop silently stalls. Fix: add if (toolGoalBinding) { await failClosedGoalTurn(toolGoalBinding, 'Goal tool continuation stopped: background agent capacity exhausted'); } before the return.
中文说明
未审查：所有维度——30 个必需 agent 中没有任何一个有记录表明是用本 skill 构建的 prompt 启动的，这个 diff 即便被审查过，也是基于这次 run 自行编写的 prompt：没有记录表明严重级别标准、发现格式或本项目自己的规则到达过任何 agent。
未审查：反向审计——有审计 agent 运行并打开了自己的 brief，但没有 agent 是用 CLI 构建的 prompt 启动的——启动 prompt 是手写的，agent 实际被要求做的并不是本 skill 所认证的内容。
[Critical] Missing failClosedGoalTurn on backgroundLaunchExhaustedCapacity early return (useGeminiStream.ts:~4209) — The PR adds toolGoalBinding and failClosedGoalTurn cleanup to 7 early-return paths in handleCompletedTools, but the pre-existing backgroundLaunchExhaustedCapacity path was missed. When a Goal turn's tool batch includes a background agent tool and capacity is exhausted, the function returns without releasing the binding — goalTurnBindingsRef retains the stale entry, activeGoalTurnRef keeps pointing to it, and the Goal runtime is never told the turn ended, so the Goal loop silently stalls. Fix: add if (toolGoalBinding) { await failClosedGoalTurn(toolGoalBinding, 'Goal tool continuation stopped: background agent capacity exhausted'); } before the return.
— qwen3.7-max via Qwen Code /review
Maintainer verification — local real-run build & E2E at
|
| Head verified | ba45e268697d32b9186c1069f4c40b22e1c905ff |
| Base | 289fe94c9e (merge-base with main) |
| Environment | macOS arm64, Node 22, isolated worktree, own npm ci (vite 7.0.0 / vitest 3.2.4, lockfile-exact) |
Finding 1 — a queued ordinary message stalls the Goal loop indefinitely (new, blocking)
This is the one I'd fix before merge. It contradicts the PR's own acceptance criterion #2, which says a queued message should remain queued across Goal turns (plural).
Repro: set a Goal, wait for it to run, type an ordinary message, press Ctrl+Q to queue it. The Goal loop runs one more turn and then stops forever, while the footer keeps showing ◎ /goal active and no in-flight indicator is displayed. There is no error and no visible sign anything is wrong.
Single-variable A/B on the built bundle — same binary, same objective, same timings; the only variable is an env-gated neutralisation of peekNextUserBatchKey():
| arm | new Goal turns in 30 s | queued text leaked to model | turn in flight |
|---|---|---|---|
head ba45e268 |
1 | 0 | false |
| reservation neutralised | 21 | 0 | true |
| control (nothing queued) | 20–27 | — | true |
Neutralising the reservation restores the loop and still withholds the message, so the hold policy itself is correct — only the turn reservation is wrong.
Mechanism:
useMessageQueue.ts:141—peekNextUserBatchKey()returns the first queued non-slash message key unconditionally. It never asks whether the two-lane gate will actually release that message now.client.ts:3503-3505— at the end of a Goal turn that key is fed togoalRuntime.beginTurn(...), reserving the next turn.goal-runtime.ts:776—finishTurnthen bindscurrentPermitto that reserved key and setsactivity: 'running', soqueueContinuation()at:789never fires.useMessageQueue.ts:271-274— butdrainQueue'sgoalTurnActivegate refuses to release that very message while the Goal is active.
Deadlock: the runtime waits for a user batch the queue will not release until the Goal terminates.
Also affects plain Enter (steer), not just Ctrl+Q — same stall. Escape hatches that do work: /goal clear (releases the message, verified — 3 requests carried it afterwards) and /goal pause + /goal resume (recovers for a couple of turns, then stalls again on the same reservation).
Suggested direction: make the reservation respect the same gate as drainQueue — only report a batch key for a message that is actually deliverable now.
Finding 2 — backgroundLaunchExhaustedCapacity leaks the Goal turn (confirms @doudouOUC, but not Critical)
The blocking review comment is factually correct: the PR threads toolGoalBinding cleanup through the other early-return paths in handleCompletedTools, but misses the pre-existing one at useGeminiStream.ts:4208.
I reproduced it with the real createGoalRuntime wired into the real hook, so the oracle is runtime state rather than a spy:
| measurement | head | + failClosedGoalTurn on that path |
|---|---|---|
| runtime still holds the turn permit | true | false |
| goal status after the batch | active | paused |
| activity after the batch | running (stuck) | idle |
| host preempt calls | ["Goal create"] |
["Goal create","Goal pause"] |
The reviewer's proposed fix is correct and I verified it flips every measurement.
Two corrections to the severity, though:
- Reachability is narrow. The guard requires
getMaxConcurrentBackgroundAgents() === 1. The default is 10 — reaching 1 needs an explicitagents.maxParallelAgents: 1setting orQWEN_CODE_MAX_BACKGROUND_AGENTS=1. It is not a default-path defect. - It is recoverable. I tested this rather than assuming:
/goal pause+/goal resumegets the loop moving again. It does not self-heal, but the user is not permanently stuck.
So: real, worth fixing, but I read it as Medium rather than Critical. Note also that both CHANGES_REQUESTED reviews lead with "Not reviewed: …" self-reports — the second one's finding still stands on its own merits, as verified above.
What I verified works
Real pty against the built dist/cli.js, two independent sessions so the queue scenario cannot contaminate the Escape scenario. 7/8 scenarios pass (S2b is Finding 1).
| # | Scenario (from your test plan) | Result |
|---|---|---|
| S1 | Goal set as the first interaction renders the lifecycle card + footer pill, no prior chat message needed | ✅ ◎ Goal running + ◎ /goal active |
| S2 | Queued ordinary input is withheld from the model while a Goal is active | ✅ 0 requests carried it; still visibly queued |
| S2b | Goal continuations keep running while a message is queued | ❌ Finding 1 — 1 turn, then stalls |
| S3 | Escape pauses the Goal instead of clearing it | ✅ footer becomes ! /goal paused |
| S3b | /goal edit on a paused Goal is accepted and retained |
✅ |
| S4 | /goal clear applies with no confirmation dialog |
✅ |
| S4b | Clearing releases the held ordinary message | ✅ 3 requests carried it after clear |
| S5 | /goal resume restarts the loop on the edited objective |
✅ 7 turns after resume |
Gates and suites — all green
typecheck (6 workspaces) · lint · prettier --check (all 40 changed files) · check-i18n · npm run bundle — all pass.
Focused suites at head: 1230 passed, 0 failed — cli 224 (queue/branch/resume/goalCommand/goal-runtime/history/GoalPill/GoalStatusMessage/HistoryItemDisplay) + cli 230 (AppContainer, slashCommandProcessor) + cli 174 (useGeminiStream) + core 602 (coreToolScheduler + all 14 goals/ files).
Mutation matrix — the new tests are load-bearing
Each mutant breaks one claimed invariant; I then ran only the test that claims it and required RED. Every mutant was checked for a no-op edit first, and each run was verified to have actually executed its target it() (a name matching nothing reads as a false survivor).
| mutant | result | |
|---|---|---|
| M1 | synthetic continuation copies the objective into the prompt | ✅ killed |
| M2 | verifier prompt drops the user_input evidence requirement |
✅ killed |
| M3 | direct-user turn binds as 'runtime' instead of 'user' |
✅ killed |
| M4 | two-lane drain gate removed | ✅ killed |
One note on the user_input evidence fix (non-blocking)
The second correctness fix is a prompt-text-only change to the verifier system instruction, and its test is a toContain string assertion. That is a reasonable design given the verifier is an LLM, but it is worth being explicit that nothing enforces the rule in code — a model that ignores the instruction will still accept an unsupported completion. Worth a follow-up behavioural check against a real verifier model.
中文说明
维护者验证 —— 本地真实构建与端到端测试 ba45e268
我在隔离的 worktree 中用独立 npm ci 构建了本 PR，运行了改动涉及的测试套件，对新增不变量做了变异测试，并在真实 pty 中驱动构建产物 dist/cli.js，配合一个会记录每次请求的 OpenAI 兼容假服务器（因此"什么真正到达了模型"是网络层事实，而非 spy 断言）。
结论：修复后可合并。 功能本身是好的 —— 评审测试计划的 8 个场景中有 7 个完全按描述复现，所有门禁均通过。有两个缺陷需要处理。当前阻塞评审的那条意见属实，但并非 Critical；我真正建议卡住合并的是另一个更容易触发的缺陷。
| 验证的 head | ba45e268697d32b9186c1069f4c40b22e1c905ff |
| base | 289fe94c9e（与 main 的 merge-base） |
| 环境 | macOS arm64、Node 22、隔离 worktree、独立 npm ci（vite 7.0.0 / vitest 3.2.4，与 lockfile 完全一致） |
问题 1 —— 排队的普通消息会让 Goal 循环永久停摆（新发现，建议阻塞）
这是我建议合并前修复的问题。它与本 PR 自己的验收标准第 2 条相矛盾 —— 该条要求排队消息在多个 Goal 轮次之间保持排队。
复现： 设置 Goal，等它开始运行，输入一条普通消息，按 Ctrl+Q 排队。Goal 循环再跑一轮后就永久停止，而底部状态仍显示 ◎ /goal active，也没有任何进行中指示。没有报错，没有任何可见异常迹象。
在构建产物上的单变量 A/B —— 相同二进制、相同目标、相同时序；唯一变量是通过环境变量屏蔽 peekNextUserBatchKey()：
| 实验组 | 30 秒内新增 Goal 轮次 | 排队文本泄漏到模型 | 有轮次进行中 |
|---|---|---|---|
head ba45e268 |
1 | 0 | false |
| 屏蔽预留 | 21 | 0 | true |
| 对照组（不排队任何消息） | 20–27 | — | true |
屏蔽预留后循环恢复，且消息依然被正确扣留，说明扣留策略本身是对的 —— 错的只是轮次预留。
机制：
useMessageQueue.ts:141——peekNextUserBatchKey()无条件返回第一条排队的非斜杠消息的 key，从不判断双通道门禁此刻是否真的会释放这条消息。client.ts:3503-3505—— Goal 轮次结束时该 key 被送入goalRuntime.beginTurn(...)，预留了下一个轮次。goal-runtime.ts:776——finishTurn随后把currentPermit绑定到这个被预留的 key 并置为activity: 'running'，导致:789的queueContinuation()永远不触发。useMessageQueue.ts:271-274—— 但drainQueue的goalTurnActive门禁在 Goal 活动期间拒绝释放的正是这条消息。
死锁：运行时在等待一个用户批次，而队列在 Goal 结束前不会释放它。
普通 Enter（steer）同样触发，不只是 Ctrl+Q。有效的逃生路径：/goal clear（会释放消息，已验证 —— 之后有 3 次请求携带了该文本），以及 /goal pause + /goal resume（能恢复几轮，然后因同一预留再次停摆）。
建议方向：让预留遵循与 drainQueue 相同的门禁 —— 只为当下真正可交付的消息报告批次 key。
问题 2 —— backgroundLaunchExhaustedCapacity 泄漏 Goal 轮次（确认 @doudouOUC，但并非 Critical）
阻塞评审的那条意见属实：本 PR 为 handleCompletedTools 中其他提前返回路径都串上了 toolGoalBinding 清理，唯独漏掉了 useGeminiStream.ts:4208 这处既有分支。
我用真实的 createGoalRuntime 接入真实的 hook 复现，因此判据是运行时状态而非 spy：
| 观测项 | head | 加上该路径的 failClosedGoalTurn |
|---|---|---|
| 运行时仍持有轮次 permit | true | false |
| 批次结束后的 goal 状态 | active | paused |
| 批次结束后的 activity | running（卡住） | idle |
| 宿主 preempt 调用 | ["Goal create"] |
["Goal create","Goal pause"] |
评审提出的修复方案是正确的，我验证了它能翻转上述每一项。
但对严重级别有两点更正：
- 触发面很窄。 该守卫要求
getMaxConcurrentBackgroundAgents() === 1。默认值是 10 —— 要变成 1 需要显式配置agents.maxParallelAgents: 1或QWEN_CODE_MAX_BACKGROUND_AGENTS=1。这不是默认路径上的缺陷。 - 可恢复。 这一点我是实测而非推断：
/goal pause+/goal resume能让循环重新运转。它不会自愈，但用户不会被永久卡死。
所以：问题真实、值得修，但我认为是 Medium 而非 Critical。另外值得注意的是，两条 CHANGES_REQUESTED 评审开头都是 "Not reviewed: …" 的自述；不过第二条的发现如上文所验证，本身是成立的。
验证通过的部分
真实 pty 驱动构建产物 dist/cli.js，分两个独立会话，避免队列场景污染 Escape 场景。8 个场景通过 7 个（S2b 即问题 1）。
| # | 场景（来自你的测试计划） | 结果 |
|---|---|---|
| S1 | 把设置 Goal 作为第一次交互即可渲染生命周期卡片与底部状态，无需先聊天 | ✅ ◎ Goal running + ◎ /goal active |
| S2 | Goal 活动期间排队的普通输入不会交付给模型 | ✅ 0 次请求携带该文本；仍可见地排队 |
| S2b | 消息排队期间 Goal 续轮继续运行 | ❌ 问题 1 —— 1 轮后停摆 |
| S3 | Escape 暂停 Goal 而非清除 | ✅ 底部变为 ! /goal paused |
| S3b | 对暂停的 Goal 执行 /goal edit 被接受并保留 |
✅ |
| S4 | /goal clear 生效且不弹确认框 |
✅ |
| S4b | 清除后释放被扣留的普通消息 | ✅ 清除后有 3 次请求携带该文本 |
| S5 | /goal resume 在编辑后的目标上重启循环 |
✅ 恢复后 7 轮 |
门禁与测试套件 —— 全绿
typecheck（6 个 workspace）· lint · prettier --check（全部 40 个改动文件）· check-i18n · npm run bundle —— 全部通过。
head 上的聚焦套件：1230 通过、0 失败 —— cli 224（queue/branch/resume/goalCommand/goal-runtime/history/GoalPill/GoalStatusMessage/HistoryItemDisplay）+ cli 230（AppContainer、slashCommandProcessor）+ cli 174（useGeminiStream）+ core 602（coreToolScheduler 与全部 14 个 goals/ 文件）。
变异测试矩阵 —— 新增测试确实起作用
每个变异体破坏一条声称的不变量，然后只运行声称覆盖它的那个测试并要求变红。每个变异体都先确认不是空改动，每次运行也都确认目标 it() 真的执行了（名字匹配不到任何用例会被误读为"存活"）。
| 变异体 | 结果 | |
|---|---|---|
| M1 | 合成续轮把目标文本复制进提示 | ✅ 已杀死 |
| M2 | 校验器提示词删除 user_input 证据要求 |
✅ 已杀死 |
| M3 | 直接用户轮次绑定为 'runtime' 而非 'user' |
✅ 已杀死 |
| M4 | 移除双通道 drain 门禁 | ✅ 已杀死 |
关于 user_input 证据修复的一点说明（不阻塞）
第二个正确性修复是纯提示词文本改动（校验器系统指令），其测试是 toContain 字符串断言。鉴于校验器本身是 LLM，这个设计是合理的，但值得明确指出：代码层面没有任何强制，忽略该指令的模型仍会接受缺乏依据的完成声明。建议后续针对真实校验器模型补一个行为验证。
Verified locally by @wenshao — isolated worktree at ba45e268, own npm ci, real pty against the built bundle. Evidence images on pr-assets/pr-8005-verify.
) Reserve the next Goal turn only for a user batch the queue will actually release: peekNextUserBatchKey now mirrors the two-lane drain gate, so an active Goal turn no longer binds a permit to a held plain message and the loop keeps running. Also release the Goal turn binding on the background-capacity early return, drop a redundant getSteerInput spread, recognise the legacy /goal clear aliases in interactive mode, and restore the rollback assertion for loadPausedBackgroundAgents.
|
@qwen-code /takeover |
|
🤖 Addressed the latest review feedback (round 1/100). What changed, and what I pushed back on: · 已处理最新评审反馈（第 1/100 轮）。改动内容与我反驳保留之处如下： Review feedback addressed[Critical] Missing
|
|
🔄 Takeover re-armed: the round counter starts a fresh window (previous rounds no longer count toward the cap); management continues. 中文说明🔄 已重新武装：轮次计数开启新窗口（此前轮次不再计入上限），托管继续。 |
| it('rejects invalid set and edit commands before runtime admission', async () => { | ||
| const { runtime } = makeRuntime(noGoalSnapshot()); | ||
| const { context, getGoalRuntimeReady } = makeContext(runtime); |
There was a problem hiding this comment.
[Suggestion] Legacy non-interactive /goal path (runLegacyGoalCommand) lost all test coverage — Concrete cost: seven deleted tests covered trust-folder blocking, hooks-disabled blocking, set/clear card emission, terminal-event forwarding, chat-recording, and terminal-summary formatting. The function remains reachable via executionMode !== 'interactive'. A regression (e.g. inverting the !config.isTrustedFolder() guard) would ship undetected in headless/ACP modes.
中文说明
[Suggestion] 非交互式 /goal 旧路径（runLegacyGoalCommand）失去了所有测试覆盖 — 具体代价：七个被删除的测试覆盖了信任文件夹阻断、钩子禁用阻断、set/clear 卡片发射、终端事件转发、聊天记录和终端摘要格式化。该函数仍可通过 executionMode !== 'interactive' 到达。回归（例如反转 !config.isTrustedFolder() 守卫）将在无头/ACP 模式下不被检测地发布。
— qwen3.8-max-preview via Qwen Code /review
There was a problem hiding this comment.
Declined — the legacy path (runLegacyGoalCommand) is thin glue code delegating to well-tested core functions (getActiveGoal, setActiveGoal, registerGoalHook, installGoalTerminalObserver). The interactive path has comprehensive coverage. Re-adding seven tests for a private wrapper is disproportionate diff growth for a review round; the regression risk in this thin layer is low relative to the cost.
中文说明
已拒绝 — 旧路径（runLegacyGoalCommand）是委托给经过充分测试的核心函数（getActiveGoal、setActiveGoal、registerGoalHook、installGoalTerminalObserver）的薄封装代码。交互式路径已有全面覆盖。为一个私有封装函数重新添加七个测试对于审查轮次来说是不成比例的差异增长；此薄层中的回归风险相对于成本较低。
# Conflicts: # packages/cli/src/ui/AppContainer.test.tsx
|
🤖 Reviewed the latest feedback — no changes needed. Why, point by point: · 已审阅最新反馈——无需改动。逐点说明原因如下： No action takenNo actionable review feedback was found on PR #8005. Failed check triage:
All substantive CI checks passed:
No review findings: There are no inline comments, no review verdicts, and no maintainer feedback requiring code changes. 中文说明未采取任何操作在 PR #8005 上未找到需要处理的审查反馈。 失败检查分析：
所有实质性 CI 检查均已通过：
无审查发现： 没有行内评论、没有审查结论、也没有需要代码更改的维护者反馈。 Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 🧠 Handled by Qwen Code · model/模型 |
|
@qwen-code /triage |
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 integration suite was not run locally (the PR's changed unit tests passed locally: 668 CLI + 468 core).
[Critical] packages/core/src/goals/goal-runtime.ts:307-309 — The budget-exhaustion callback in queueContinuation guards only on snapshot.goal?.status !== 'active' and does NOT verify goal identity (goalId/revision), unlike its siblings handleStartFailure (isCurrentPermit(startedPermit)) and recordVerificationOutcome (isCurrentVerificationAttempt(attempt)). The callback is enqueued from within finishTurn's running operation AFTER its await options.journal.recordGoalState(...). A dispatch({ action: 'replace' }) issued during that await chains onto the same dispatchTail, so the order becomes op_finishTurn -> op_replace -> op_budget. The replace creates a fresh goal (turnCount 0, status 'active', new goalId) and calls queueContinuation(); the budget callback then sees an active goal and transitions the REPLACEMENT goal to usage_limited with a bogus 'Goal exceeded the 50-turn continuation budget' reason. Probe-verified: driving turnCount to MAX_GOAL_CONTINUATION_TURNS while firing a replace from the journal append hook usage-limits the 0-turn replacement goal; adding an identity guard (capture goalId/revision at enqueue, return early on mismatch) leaves it active. This is distinct from the previously-fixed /goal resume re-limiting (comment 3693706358, fixed via turnCount reset on resume); it is relocated to the body because it shares its (path, line) with that comment but is a different bug. Suggested fix: capture snapshot.goal.goalId/revision when enqueuing and return if they no longer match, matching the codebase's existing identity-guard pattern.
中文说明
未审查：build-and-test — 'Integration Tests (CLI, No Sandbox)' was skipped in CI and its integration suite was not run locally (the PR's changed unit tests passed locally: 668 CLI + 468 core)。
[Critical] packages/core/src/goals/goal-runtime.ts:307-309 — The budget-exhaustion callback in queueContinuation guards only on snapshot.goal?.status !== 'active' and does NOT verify goal identity (goalId/revision), unlike its siblings handleStartFailure (isCurrentPermit(startedPermit)) and recordVerificationOutcome (isCurrentVerificationAttempt(attempt)). The callback is enqueued from within finishTurn's running operation AFTER its await options.journal.recordGoalState(...). A dispatch({ action: 'replace' }) issued during that await chains onto the same dispatchTail, so the order becomes op_finishTurn -> op_replace -> op_budget. The replace creates a fresh goal (turnCount 0, status 'active', new goalId) and calls queueContinuation(); the budget callback then sees an active goal and transitions the REPLACEMENT goal to usage_limited with a bogus 'Goal exceeded the 50-turn continuation budget' reason. Probe-verified: driving turnCount to MAX_GOAL_CONTINUATION_TURNS while firing a replace from the journal append hook usage-limits the 0-turn replacement goal; adding an identity guard (capture goalId/revision at enqueue, return early on mismatch) leaves it active. This is distinct from the previously-fixed /goal resume re-limiting (comment 3693706358, fixed via turnCount reset on resume); it is relocated to the body because it shares its (path, line) with that comment but is a different bug. Suggested fix: capture snapshot.goal.goalId/revision when enqueuing and return if they no longer match, matching the codebase's existing identity-guard pattern.
— qwen3.8-max-preview via Qwen Code /review
| if (toolGoalBinding?.controller.signal.aborted) { | ||
| drainedSteer?.restore(); | ||
| await failClosedGoalTurn( | ||
| toolGoalBinding, | ||
| 'Goal tool continuation was preempted', | ||
| ); |
There was a problem hiding this comment.
[Suggestion] The goal-turn preemption path has no direct unit test — Concrete cost: preemptGoalTurn aborts activeGoalAdmissionRef.current.controller, and handleCompletedTools checks toolGoalBinding?.controller.signal.aborted to fail-close the continuation with 'Goal tool continuation was preempted'. AppContainer.test.tsx:1433 only asserts preemptGoalTurn is called, not this downstream effect. A future refactor that severs the link between the preemptGoalTurn abort and this controller.signal.aborted check (e.g. binding a fresh controller in handleCompletedTools) would let a preempted goal turn keep submitting tool continuations against an edited/replaced goal, and nothing in the suite would fail. Add a useGeminiStream.test.tsx case that binds a goal turn, aborts its controller via preemptGoalTurn before capturedOnComplete fires, and asserts the failClosedGoalTurn effects (pause/finishTurn dispatch), the 'Goal tool continuation was preempted' error item, and no further sendMessageStream call.
中文说明
[Suggestion] Goal 轮次的抢占（preemption）路径没有直接的单元测试 — 具体代价：preemptGoalTurn 会中止 activeGoalAdmissionRef.current.controller，而 handleCompletedTools 通过检查 toolGoalBinding?.controller.signal.aborted 来以 'Goal tool continuation was preempted' 失败关闭续轮。AppContainer.test.tsx:1433 只断言了 preemptGoalTurn 被调用，并未断言此处的下游效果。如果未来的重构切断了 preemptGoalTurn 的中止与这个 controller.signal.aborted 检查之间的联系（例如在 handleCompletedTools 中绑定新的 controller），被抢占的 Goal 轮次就会继续针对已编辑/已替换的 Goal 提交工具续轮，而套件中没有任何用例会失败。建议新增一个 useGeminiStream.test.tsx 用例：绑定一个 Goal 轮次，在 capturedOnComplete 触发前通过 preemptGoalTurn 中止其 controller，并断言 failClosedGoalTurn 的效果（pause/finishTurn dispatch）、'Goal tool continuation was preempted' 错误项，以及不再有 sendMessageStream 调用。
— qwen3.8-max-preview via Qwen Code /review
There was a problem hiding this comment.
Declined — writing this test correctly requires interleaving preemptGoalTurn during the async window between bindGoalTurn (which sets activeGoalAdmissionRef) and the controller.signal.aborted check inside handleCompletedTools. The binding is created fresh via bindGoalTurn, so the abort must land between that synchronous call and the subsequent async finalization — a timing-dependent setup that would make the test fragile. The preemptGoalTurn call site is already covered by AppContainer.test.tsx:1433, and the single-line guard is structurally coupled to the binding mechanism. Deferring to a follow-up that can refactor the binding lifecycle to make this path directly testable.
中文说明
已拒绝 — 正确编写此测试需要在 bindGoalTurn（设置 activeGoalAdmissionRef）和 handleCompletedTools 内部的 controller.signal.aborted 检查之间的异步窗口中插入 preemptGoalTurn。绑定通过 bindGoalTurn 新建，因此中止必须落在该同步调用和后续异步终结之间——这是一个依赖时序的设置，会使测试变得脆弱。preemptGoalTurn 的调用点已由 AppContainer.test.tsx:1433 覆盖，且该单行守卫与绑定机制结构耦合。推迟到可以重构绑定生命周期以使此路径可直接测试的后续 PR。
|
Sandboxed verification: ✅ passed — merge-ready (agent verdict) - workflow run Ran the PR in an isolated, token-free container: A/B against the base build, mock-free harness assertions, targeted gates. Advisory evidence for human reviewers — not a review, an approval, or a CI check. Scripted assertions: 12 passed · 0 failed · 12 total 中文 — 判定：✅ 通过 · 可合入（agent 判定）沙箱验证在隔离、无凭证的容器中执行了该 PR 的代码（与 base 构建 A/B 对照、无 mock harness 断言、定向门禁）。仅作为评审证据，不构成评审、批准或 CI 检查。 脚本断言：12 通过 · 0 失败 · 12 总计 Verification report<!-- qwen-triage:verify --> Sandboxed verification: ✅ passed — merge-ready (agent verdict) Ran the PR in an isolated, token-free container: A/B against the base build, mock-free harness assertions, targeted gates. Advisory evidence for human reviewers — not a review, an approval, or a CI check. Scripted assertions: 12 passed · 0 failed · 12 total 中文 — 判定：✅ 通过 · 可合入（agent 判定）沙箱验证在隔离、无凭证的容器中执行了该 PR 的代码（与 base 构建 A/B 对照、无 mock harness 断言、定向门禁）。仅作为评审证据，不构成评审、批准或 CI 检查。 脚本断言：12 通过 · 0 失败 · 12 总计 Verification reportPR #8005 — Deep Verification Report (follow-up round 2)Verdict: 中文 — 判定：✅ 通过 · 可合入（agent 判定）这是第二轮跟进。上一轮（round 1）在 head
数字以下方表格为准，不在散文里重复。 Previous-finding status tableThe previous round (round 1, head
No prior finding regressed (there were none). No prior verified claim flipped to red. Scope47-file / +5416 −1169 feature PR wiring the interactive TUI to the Goal v3 runtime. Because the head is unchanged from round 1, scope this round = re-prove the five carried-forward load-bearing claims with fresh witnesses, plus one deepening probe that converts round 1's gate-only consumer-side coverage into a two-cell A/B:
A/B proofsEach A/B runs the PR's own test as the head-side oracle, then a single-hunk revert reproducing base / pre-fix behaviour as the control. A red control cell is the expected outcome (it proves the head behaviour is load-bearing and the test non-vacuous) and is encoded as a pass. Rerunnable:
Every control cell produced its expected outcome, so #6 Consumer-side
|
| cell | build | result |
|---|---|---|
| HEAD | PR head | PASS — all five assertions hold (01-terminate-consumer-ab.png, head cell) |
| CONTROL (== base) | head with toolCall.response.terminateTurn === true → … === false (reproduces base, which had no consumer block at all) |
expected red — terminatesGoalTurn is false, so the entire if (terminatesGoalTurn && toolGoalBinding) { … } block is skipped: addHistory/finishTurn/the goal_state card never fire, and the test fails at expect(client.addHistory).toHaveBeenCalledWith({ role: 'user', … }) (01-terminate-consumer-ab.png, control cell; logs/terminate-consumer-base.log) |
This is the load-bearing consumption half of the terminateTurn chain that round 1 left at "gate-covered only": the producer copy (#2) puts the flag on the wire-internal response, and #6 proves the stream loop actually reads it and terminates the Goal turn. With #2 + #6 both A/B-proven, the producer→consumer path is verified end to end. Restored clean (git status --porcelain packages/ empty).
#4 Resume continuation-budget reset (carried-forward, re-measured)
Oracle: goal-reducer.test.ts › "resets the continuation turn budget when resuming an exhausted goal".
| cell | build | result |
|---|---|---|
| HEAD | PR head | turnCount: 0, status: 'active' — PASS |
| CONTROL (== base) | head with the turnCount: 0, line removed from the resume branch |
expected red: - "turnCount": 0, + "turnCount": 50, (02-resume-budget-ab.png) |
#5 State-aware queue gating / stranding fix (carried-forward, re-measured)
Oracle: AppContainer.test.tsx › "holds ordinary input while the Goal is active and drains it once paused" + "treats paused, blocked and usage_limited Goals as drain-eligible".
| cell | build | result |
|---|---|---|
| HEAD | PR head | both PASS (popNextSubmission called with 'priority' while active, 'normal' otherwise) |
| CONTROL (pre-fix) | head with if (status === 'active') → if (status) |
expected red, both: expected "spy" to be called with arguments: [ 'normal' ] (03-state-gating-ab.png) |
#1 Two-lane drain gate (carried-forward, re-measured)
Oracle: useMessageQueue.test.ts (40 tests). Faithful base mutation: head predicate (goalTurnActive ? GOAL_COMMAND_RE.test(text) : !isSlashCommand(text)) → verbatim base predicate (!isSlashCommand(text) || (!includeDeferred && GOAL_COMMAND_RE.test(text))).
| cell | build | result |
|---|---|---|
| HEAD | PR head | 40/40 PASS |
| CONTROL (faithful base) | head with the base predicate | expected red, 2 tests (38 passed | 2 failed) |
#2 terminateTurn producer propagation (carried-forward, re-measured)
Oracle: coreToolScheduler.test.ts › "propagates a tool turn-termination boundary to the host".
| cell | build | result |
|---|---|---|
| HEAD | PR head | PASS |
| CONTROL (== base) | head with the copy line ...(toolResult.terminateTurn ? { terminateTurn: true } : {}) removed |
expected red: expected undefined to be true |
#3 Verifier user_input rule on the wire (carried-forward, re-measured)
Oracle: goal-verifier.test.ts › "uses a tool-free deterministic side query with bounded fields" (asserts request.systemInstruction carries the user_input rule).
| cell | build | result |
|---|---|---|
| HEAD | PR head | PASS |
| CONTROL (== base) | git show HEAD^1:…/goal-verifier.ts |
expected absence: grep -c user_input = 0 (base) vs 1 (head) |
The model's resulting accept/reject decision is non-deterministic and is not asserted; the load-bearing deterministic part — the rule reaching the wire request — is.
Corrections
None to the PR code, and none to round 1's report this time: round 1's vacuity-count correction (3 → 2 for the drain gate) was already correct, and my faithful-base mutation this round reproduces exactly 2 reds, confirming it. The one substantive change versus round 1 is not a correction but an addition: the consumer-side terminateTurn item moves from Not covered to a proven A/B (#6).
Findings
None blocking. No defect reproduced against the PR head. All red cells above are expected control/mutation outcomes that prove the head behaviour, not findings against the PR.
Not covered
- Interactive tmux E2E full flow (first-message Goal creation, pause/edit/resume, Escape-cancel, process/branch recovery, clear-without-confirmation, verifier-rejection cards): not reproduced in this sandbox. The PR body references a separately posted tmux report; per the verify contract PR text is untrusted and that report was not available to me, so I neither relied on it nor re-ran it.
- Verifier model decision: non-deterministic; only the prompt-on-the-wire was asserted (如何自定义密钥文件 .env可能与其他文件冲突 #3).
- Stream-glue fixes still gate-covered only (their tests are green but I did not independently A/B them): the orphan-prompt strip on cancel, the
/goal setclear-keyword bypass,goalTerminalErrorRef, and the "clear stalelastReasonon edit" fix. (Note: the consumer-sideterminateTurnhonouring that round 1 listed here is no longer in this list — it is now OpenAI API Error: 401 Incorecct API Key provided #6.) - Out-of-scope surfaces (non-interactive CLI, ACP, SDK, Web, WebShell, Desktop): unchanged-by-design per the PR; not exercised.
- Per-commit attribution: the checkout is depth-2 (shallow,
git rev-parse --is-shallow-repository= true); the 23 commits in the metadata are not individually reachable, so I verified the aggregateHEAD^1..HEADdiff only. - Merge freshness: I did not trial-merge into current
main; the A/B is valid against the merge-ref base actually built (HEAD^1=2bbd82f14). Because the head is unchanged from round 1, this carries the same caveat round 1 reported.
Targeted gates
| gate | scope | result |
|---|---|---|
| cli affected tests | all 13 changed cli test files (AppContainer, goalCommand, GoalPill, HistoryItemDisplay, GoalStatusMessage, slashCommandProcessor, useBranchCommand, useGeminiStream, useMessageQueue, useResumeCommand, goal-runtime, historyUtils, resumeHistoryUtils) | 13 files / 668 passed / 0 failed (logs/gate-cli.log) |
| core affected tests | client-goal, coreToolScheduler, goal-reducer, goal-runtime.integration, goal-runtime, goal-verifier | 6 files / 468 passed / 0 failed (logs/gate-core.log) |
| affected surface total | 19 files | 1136 passed / 0 failed |
| typecheck | npm run typecheck -w packages/cli -w packages/core |
exit 0, 0 error TS (logs/typecheck.log) |
Gate liveness (proven, not assumed): the cli gate is live because the #1 mutation turned 2 useMessageQueue tests red and the #5 mutation turned 2 AppContainer tests red and the #6 mutation turned the useGeminiStream consumer test red; the core gate is live because the #4 mutation turned the reducer test red, the #2 mutation turned the scheduler test red, and #3's base grep-absence distinguishes the prompt. A green-by-no-match artefact is ruled out. (Repo-wide npm run typecheck --workspaces exits 1 only because several unrelated workspaces — mobile-mcp, vscode-ide-companion, channels/*, web-templates — define no typecheck script; the two workspaces this PR touches both typecheck clean.)
Methodology
Environment: CI verify container, refs/pull/8005/merge at depth 2 (HEAD merge f081795d7, HEAD^1 base tip 2bbd82f14, HEAD^2 PR head b292a2af9), npm ci + npm run build pre-run at HEAD; this is follow-up round 2 (previous-report.md present, prior head also b292a2af9 — i.e. identical tree, proven by HEAD^{tree} == HEAD^2^{tree} == 77110a58…). Each A/B drove the real source module under vitest (jsdom for the React hooks via renderHook/act; node for core) — the reducer, the scheduler copy path, the verifier prompt construction, the queue gate, the status→mode mapping, and the stream-loop consumer are real code, never stubbed (the verifier test stubs only the model client at the transport boundary; the stream test stubs the scheduler callback and the Goal runtime at their boundaries but exercises the real terminatesGoalTurn branch). Base/pre-fix controls are single-hunk reverts of the head source, each confirmed confound-free (the queue hook's runtime imports are unchanged; package.json/lockfile untouched; the base reducer/scheduler/verifier/consumer deltas isolated to the reverted line/block; the drain-gate control uses the verbatim base predicate including !includeDeferred &&, per round 1's faithfulness correction). Every control cell produced its expected outcome (red for a behavioural revert, grep-absence for the base prompt), so assertions.json counts the 12 A/B cells as pass with fail: 0. Evidence PNGs were produced by scripts/verify-capture.mjs (the captured side-by-side cell runs' ANSI rendered through @xterm/headless + sharp); a non-zero exit from a captured control run is the intended red witness. Raw per-cell logs are in logs/; the rerunnable A/B scripts are harness/ab.sh and harness/cells.sh. All mutations were restored (git checkout -- packages/) and git status --porcelain packages/ confirmed empty after every claim and after the gates/typecheck.
Evidence images
Harness scripts and raw logs are in the workflow run artifacts (7-day retention).
— Qwen Code · sandboxed verification
Evidence images
Harness scripts and raw logs are in the workflow run artifacts (7-day retention).
— Qwen Code · sandboxed verification
Local verification report — real model, real interactive TUII built this PR in an isolated worktree and drove the built interactive CLI through the Reviewer Test Plan in real tmux sessions against a real provider. Six of the seven listed behaviors reproduce as described. One blocking defect surfaced that is only observable on the real stack: tool results never enter the Goal evidence catalog, so any objective whose completion depends on external state can never be verified and the Goal loop runs until it is cleared or hits the usage limit. Environment: PR head Focused unit tests on the changed surface: Reviewer Test Plan results
Happy path end to end — a conversational objective is proposed, independently verified and completed in one turn (36.9s):
Lifecycle controls in one frame — Escape → paused,
Two-lane queue — an ordinary message stays visibly queued while the Goal drives turn after turn:
…and is delivered only once the Goal reaches a terminal state:
Recovery — relaunch with continue mode restores the paused Goal, and a branch created from that session inherits objective, lifecycle state and footer:
Blocking: tool results never reach the Goal evidence catalogObjective under test: create a file named
The recorded session shows why. Every assistant record in a Goal turn carries Root cause, confirmed by temporary instrumentation on the real binary (added, traced, then reverted): the interactive TUI records tool results itself, at the two-argument call site in The recording block itself predates this PR ( Suggested direction: either give Suggestions (non-blocking)
RecommendationEverything the PR claims about the TUI lifecycle, the two-lane queue, cancellation, clear and recovery holds up on the real stack. I would hold the merge until the evidence-catalog gap is fixed or explicitly scoped out in writing, because in its current state an interactive Goal over any file- or command-based objective cannot terminate on its own. 中文版本地验证报告 —— 真实模型、真实交互式 TUI我在隔离 worktree 里构建了本 PR，并在真实 tmux 会话中用真实模型跑完了 Reviewer Test Plan。七项行为里有六项与描述一致。另外发现一个只有在真实环境才能暴露的阻塞缺陷：工具结果永远不会进入 Goal 证据目录，因此任何依赖外部状态的目标都无法通过验证，Goal 循环会一直跑到被清除或触发用量限制。 环境：PR head 改动面聚焦单测： Reviewer Test Plan 结果
截图依次为：完整成功路径（对话型目标 36.9s 内经独立验证完成）、生命周期控制（Escape → 暂停 → 编辑 → 恢复到 revision 2）、排队消息在 Goal 运行期间被保持、终态后才被交付、以及 continue 重启与 阻塞问题：工具结果从未进入 Goal 证据目录测试目标：在当前目录创建 会话记录说明了原因：Goal 轮次里每一条 assistant 记录都带 根因（在真实二进制上临时插桩确认后已回滚）：交互式 TUI 自己在 这段记录代码本身早于本 PR（ 建议方向：要么把 非阻塞建议
结论PR 声称的 TUI 生命周期、双通道队列、取消、清除与恢复行为在真实环境下都成立。但建议在证据目录这个缺口修复、或明确写清不在本 PR 范围之前先不要合并，因为按当前状态，交互式 Goal 对任何涉及文件或命令的目标都无法自行终止。 |
…to tool-result recording (QwenLM#8005) The budget-exhaustion callback in queueContinuation only checked goal status, not identity. A replace dispatched during the journal append could create a fresh goal that the stale callback then incorrectly usage-limited. Capture goalId/revision at enqueue time and return early on mismatch, matching the existing identity-guard pattern used by handleStartFailure and recordVerificationOutcome. The TUI recorded tool results without goalContext, so the evidence catalog never admitted tool-result records and any Goal whose objective depended on external state could not be verified. Pass request.goalContext at both recordToolResult call sites in useGeminiStream, tagging get_goal/update_goal results as goal_runtime to match the CoreToolScheduler pattern.
|
🤖 Addressed the latest review feedback (round 1/100). What changed, and what I pushed back on: · 已处理最新评审反馈（第 1/100 轮）。改动内容与我反驳保留之处如下： Review feedback addressed[Critical] Budget-exhaustion callback lacks identity guard — FIXEDThe budget-exhaustion callback in Fix: Capture Test: Added [Blocking] Tool results never reach the Goal evidence catalog (@wenshao) — FIXEDThe interactive TUI recorded tool results via Fix: Pass Test: Added [Suggestion] rc:3695190171 — Preemption path test — DECLINEDThe suggestion asks for a @wenshao non-blocking suggestions — NOTED
Verification
中文说明已处理的评审反馈[Critical] 预算耗尽回调缺少身份守卫 — 已修复
修复： 在入队时捕获 测试： 新增 [Blocking] 工具结果从未进入 Goal 证据目录（@wenshao）— 已修复交互式 TUI 通过 修复： 在 测试： 新增 [Suggestion] rc:3695190171 — 抢占路径测试 — 已拒绝该建议要求新增一个 @wenshao 非阻塞建议 — 已记录
验证
Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 Re-review when you have a moment. After round 100 this bot stops and leaves the PR for a human. · 有空请复审；第 100 轮后本 bot 停止并将 PR 交给人工。 🧠 Handled by Qwen Code · model/模型 |
e1d6c77
…oom (QwenLM#8257) * fix(autofix): budget the whole round, not just the agent step The primary attempt took run-agent.mjs's 50-minute default while its step capped at 80, so a third of that step was unreachable and every "ran out of time" round ended for a reason this file never named. Raising it surfaced the larger problem: the budget was never checked as a whole. Measured on one scan (run 30646547838): setup 5-7m in earlier steps, so it never competes with the agent step; Triage and address 50m03s on QwenLM#8005 round 9 (its own timer) and 12m45s on QwenLM#8211; the Verification gate 22m48s on QwenLM#8211 — the largest consumer in the job, and unbounded; push, report and finalize 3-4s. The old arithmetic ("80 + 20 leaves ~50 for setup, two verification passes, and reporting") predates that measurement: the real worst case was 7 + 80 + 23 + 20 + 23 = 153 against a 150-minute job, and a JOB timeout cancels the always() reporters, which is the silent round the design exists to prevent. Sized for the PRs that actually exhaust the budget (47 and 35 changed files): agent 120m under a 130m step cap, each verification gate bounded at 60m — 2.6x the measured pass, and a graceful degrade because both gates already carry continue-on-error, so a bound turns a job kill into the ordinary verification-failure path that reports. Worst case 7 + 130 + 60 + 20 + 60 + 3 = 280 against a 300-minute job, itself under the 360-minute ceiling ubuntu-latest imposes regardless. Raising the budget does not make a doomed round cheaper — each exhausted budget still pushes nothing, which is why TIMEOUT_WINDOW_CAP stops a PR after three in one window. It buys rounds that were close to finishing. The test now asserts the SUM against the job cap and the count of bounded steps, because asserting the numbers individually is exactly what let 153-against-150 pass. Mutations checked: budget over its step cap, either verification bound removed, job cap back to 150, job cap over the runner ceiling, and continue-on-error dropped are each caught. * fix(autofix): bound every long step and cap the timeout override (QwenLM#8257) * fix(autofix): enforce the timeout ceiling and trim review feedback (QwenLM#8257) * fix(autofix): force base-10 clamp and align stale bound with job cap (QwenLM#8257) * fix(autofix): close the clamp int64 escape and pin it with a bash replay (QwenLM#8257) * fix(autofix): give the timeout clamp a floor, not only a ceiling The review's Finding 1: the guard clamped only the ceiling, and the uncovered side is the likelier typo. Every comment in this file, the PR body and the operator message speak in MINUTES; this one variable wants MILLISECONDS. A maintainer told to "raise the agent time budget" who sets QWEN_AUTOFIX_TIMEOUT_MS=120 armed a 120 ms timer — every round SIGTERMs instantly, writes agent-timeout, and reports "ran out of time (timeout (120ms))" until TIMEOUT_WINDOW_CAP trips and AutoFix stops on the PR, advising the human to raise the budget they just raised. No warning anywhere in that loop, which is the exact misreport the clamp exists to prevent, reached from the other direction. A 60000 ms floor rejects every minutes-shaped value, and it also closes the `0`/`000` hole the review noted alongside it — those passed the bare regex while the message asserted the value had to be positive. The message now names the units, because a units confusion is the whole failure mode. Replayed the review's own table against the extracted block, stdout and stderr separated: 7200000, 3600000 and the floor itself pass untouched; 120, 60, 0, 000 and 59999 all clamp with a warning, alongside the over-cap, malformed, octal and int64 cases the previous round closed. The test pins both boundaries from each side (59999 clamps, 60001 does not) and asserts the warning names MILLISECONDS. --------- Co-authored-by: verify <verify@local> Co-authored-by: qwen-code-dev-bot <qwen-code-dev@service.alibaba.com> Co-authored-by: Qwen Autofix <qwen-autofix@users.noreply.github.com>
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed. Not reviewed: reverse audit — an auditor ran and opened its brief, but no agent was launched with the prompt the CLI built — the launch was written by hand, and what the agent was actually asked is not what this skill certifies.
中文说明
已审查。 未审查：反向审计——有审计 agent 运行并打开了自己的 brief，但没有 agent 是用 CLI 构建的 prompt 启动的——启动 prompt 是手写的，agent 实际被要求做的并不是本 skill 所认证的内容。
— qwen3.8-max-preview via Qwen Code /review
|
🤖 Reviewed the latest feedback — no changes needed. Why, point by point: · 已审阅最新反馈——无需改动。逐点说明原因如下： Autofix review round — no action takenThe only feedback newer than the last evaluation is a single
This is an informational comment about the There is nothing to address in code this round:
The earlier 中文说明Autofix 审查轮次——未采取任何操作自上次评估以来唯一的新反馈是来自自动审查器的一条
这是一条关于 本轮在代码层面没有任何需要处理的内容：
较早的 Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 🧠 Handled by Qwen Code · model/模型 |
Review 总结变更概述在交互式 TUI 中采用 Goal v3，47 文件，+5629/-1174。涵盖 Goal 命令、GoalPill 组件、GoalStatusMessage、goal-runtime、goal-reducer、goal-verifier、useGeminiStream 集成、useMessageQueue 集成、resume/history 适配、i18n 等。 代码质量评估
结论无 Critical 缺陷，可以合并。👍 |
ytahdn
left a comment
ytahdn
left a comment
There was a problem hiding this comment.
LGTM. Goal v3 TUI 集成结构清晰，core/UI 分层明确，测试覆盖充分。
— qwen3.7-plus via Qwen Code /review
|
Released in v0.21.3. |
8 participants
What this PR does
This PR connects the interactive TUI to the Goal v3 runtime. It adds the canonical
/goallifecycle commands, persistent lifecycle cards and footer status, Goal-aware resume and branch recovery, and a two-lane input queue that keeps ordinary messages queued while Goal continuations are running. Explicit Goal control commands can still preempt the loop, and ordinary queued messages are released only after the Goal reaches a terminal state or is cleared.It also closes two correctness gaps found during real terminal testing: synthetic continuation prompts can no longer count objective text as user evidence, and completion claims that depend on user input must cite real
user_inputevidence.Why it's needed
The Goal v3 runtime was available below the UI, but the interactive TUI still used the legacy command and queue behavior. That mismatch caused premature completion, ordinary queued messages leaking into an active Goal, inconsistent pause/resume behavior, and incomplete recovery after restart or branching. This PR makes the TUI a faithful host for the existing runtime without pulling the non-interactive, ACP, SDK, Web, WebShell, or Desktop surfaces into the same review.
Reviewer Test Plan
How to verify
Evidence (Before & After)
Before: the interactive TUI used legacy Goal commands and a single effective delivery lane, allowing unsupported completion and ordinary queued input to enter the active Goal loop.
After: the tmux E2E report posted as a separate PR comment covers first-message creation, normal continuation, real stop-input completion, unsupported-completion rejection, queued-message hold and release, pause/edit/resume, Escape cancellation, process recovery, branch recovery, and clear-without-confirmation. Screenshots will be attached to that report.
Tested on
Environment (optional)
macOS arm64, Node.js 24, no sandbox, interactive tmux sessions against the local development bundle.
Risk & Scope
Linked Issues
Follow-up to #7895.
中文说明
本 PR 做了什么
本 PR 将交互式 TUI 接入 Goal v3 运行时，补齐规范化的
/goal生命周期命令、持久化生命周期卡片与底部状态、Goal 感知的恢复和分支继承，以及双通道输入队列。Goal 自动续轮期间，普通消息会持续排队；显式 Goal 控制命令仍可抢占循环；普通排队消息只会在 Goal 进入终态或被清除后释放。真实终端测试还发现并修复了两个正确性问题：合成续轮提示不再把目标文本误当作用户证据；依赖用户输入的完成声明必须引用真实的
user_input证据。为什么需要
Goal v3 运行时已经存在于底层，但交互式 TUI 仍使用旧命令与旧队列行为。这个错位会导致目标过早完成、普通排队消息泄漏进活动 Goal、暂停/恢复行为不一致，以及重启或分支后的恢复不完整。本 PR 让 TUI 成为现有运行时的可靠宿主，同时不把非交互 CLI、ACP、SDK、Web、WebShell 或 Desktop 一并塞进同一次评审。
Reviewer Test Plan
如何验证
证据（Before & After）
Before：交互式 TUI 使用旧 Goal 命令和单一的有效交付通道，可能接受缺少依据的完成提案，也可能让普通排队输入进入活动 Goal 循环。
After：单独发布的 tmux E2E 报告覆盖首次消息创建、正常续轮、真实停止输入完成、无依据完成拒绝、排队消息保持与释放、暂停/编辑/恢复、Escape 取消、进程恢复、分支恢复和无确认清除。截图会附在该报告中。
测试平台
环境（可选）
macOS arm64、Node.js 24、无沙箱，使用本地开发 bundle 在 tmux 交互会话中测试。
风险与范围
关联
承接 #7895。