feat(web-shell): adopt canonical Goal v3 controls - #9393
Conversation
Route WebShell Goal lifecycle through the canonical Goal v3 control plane instead of model-bound chat commands. Goals can be created before the first message, then inspected, edited, paused, resumed, replaced and cleared directly. - core: extend the Goal reducer/runtime and protocol with the v3 control actions and a revisioned snapshot. - serve/acp: expose typed Goal state and controls over the daemon HTTP routes and the ACP bridge, with a shared error taxonomy. - sdk/webui: surface the same revisioned snapshot and control actions so every consumer reads one source of truth. - web-shell: render the active Goal as a compact composer row sized to match the queued-message row, drop the clear confirmation, and hold ordinary messages in a local FIFO queue while a Goal runs — only the explicit Insert action enters the active turn. The TUI keeps its existing presentation and command flow. Token-budget UI and desktop-shell adoption are intentionally out of scope.
|
Re-run on the new head — three commits landed since the last pass at Template looks good ✓ Problem: Unchanged from earlier passes — a real gap, not a theoretical one. Goal v3 has been wired consumer by consumer — TUI (#8005), non-interactive CLI (#8324), ACP sessions (#8732) — and WebShell was the remaining consumer mixing Goal commands into the chat path. The described pain points (needing a conversation before Direction: Aligned. This is the next step of an established roadmap by the author who built Goal v3, and the declared exclusions (token-budget UI, desktop-shell adoption) keep the scope honest. The increment since the last pass is fix-driven; the shape of the feature did not change. Size: 10,994 changed lines across 83 files at this head: 4,198 production, 6,796 test (this count treats the Playwright e2e harness Approach: The new fix commit Risk: Stage 1e high-risk paths still match — Moving on to code review. 🔍 中文说明在新 head 上重跑——距上次审查 模板完整 ✓ 问题:与此前各轮一致——是真实缺口而非理论问题。Goal v3 一直按消费者逐个接入——TUI(#8005)、非交互 CLI(#8324)、ACP 会话(#8732)——WebShell 是仍在把 Goal 命令混入聊天路径的最后一个消费者。PR 描述的痛点与该缺口一致,且四轮审查已真实检验过相关语义。 方向:对齐。这是 Goal v3 作者既定路线的下一步,明确排除 token 预算 UI 和桌面端接入,范围克制。上次审查之后的增量由修复驱动,功能形态未变。 规模:本 head 上 83 个文件共 10,994 行:生产 4,198、测试 6,796(按门禁文件规则,Playwright e2e 脚手架 方案:新修复提交 风险:Stage 1e 高风险路径仍命中—— 进入代码审查 🔍 — Qwen Code · qwen3.8-max Reviewed at |
🖼️ web-shell visual previewRendered against a mock daemon (no real backend): the PR base vs this PR head Screenshots · before / afterFull-resolution recordings (.webm) are attached to the workflow run. — Qwen Code · web-shell visuals |
Code review (re-run at
|
| Check | Conclusion |
|---|---|
Capture web-shell visuals (ubuntu-latest, Node 22.x) |
✅ success |
Classify PR |
✅ success |
Dependency CVE audit |
✅ success |
Desktop Shell (ubuntu-22.04) |
✅ success |
Desktop Shell (windows-2022) |
✅ success |
Live Host (macos-latest) |
✅ success |
macos-latest / Java 21 |
✅ success |
Real daemon E2E / Java 11 |
✅ success |
Secret scan (TruffleHog) |
✅ success |
Serve A/B (ubuntu-latest, Node 22.x) |
✅ success |
Test (ubuntu-latest, Node 22.x) |
✅ success |
ubuntu-latest / Java 11 |
✅ success |
ubuntu-latest / Java 17 |
✅ success |
ubuntu-latest / Java 21 |
✅ success |
web-shell E2E Smoke (ubuntu-latest, Node 22.x) |
✅ success |
windows-latest / Java 21 |
✅ success |
One row per check name (latest run); skipped checks omitted; failures sort first. / 每个检查名一行(取最新一次运行),省略 skipped,失败项排在最前。
Because the three open Criticals are behavioral and no test pins them, a green suite would not settle them — and the one fix that did land (R3-2) is pinned only by this PR's own test, which CI hasn't confirmed at this head yet. Sandboxed verification would settle what CI cannot: @qwen-code /verify — an A/B run against the base build could confirm the R3-2 chain ordering holds, and probe the three open behaviors (that a detached mid-Goal session survives reaping, that end_turn carries the right promptId, that a lost insert response cannot double-execute). The author has write access, so the lane is directly available. @qwen-code /tmux — the claim that the TUI Goal flow stays visually and behaviorally unchanged is author-tested only (macOS); a maintainer-triggered tmux run would confirm it on this head.
Not verified: the three open Criticals (no test pins them, and no PR code was executed in this run); the R3-2 fix in CI (the suite was still running at review time); the merge-conflict resolutions beyond the Goal seams (green CI is the evidence there); Windows/Linux UI behavior (author marks them
中文说明
代码审查(在 a39b8243 上重跑)
距上次审查 c52a2231 之后,去掉合并的增量只有一个提交——bf66d843(useQueuedPrompts.ts +129/−47,测试 +38)——回应 R3-2 Critical,外加两次 upstream/main 合并。我在 head 代码中核实了 R3-2 修复,复核了三条没有修复提交的发现,并抽查确认合并未使此前的修复回退。
已在 head 核实修复——R3-2(drain 期间输入的 prompt 越过释放链): 机制真实存在,不只是提交说明。存活的释放链现在发布在 releaseChainRef 上(构建时钉住 owner),enqueuePrompt 的普通路径在 POST 前检查它:当前 owner 的链仍在 drain 时,新 prompt 被登记进 unreleasedPromptIdsRef(已标记但未 POST,owner 变更时其文本会被暂存),并经 drain 所用的同一个共享 releaseChainedPrompt 追加到 chain.tail——因此继承 owner 守卫、行存在性检查与逐 link 的 hold/write-block 回退。存活 owner 的再次 drain 会扩展现有链而非与之竞争;retireChainWhenDrained 在 tail 变新时重新挂接,而不是释放仍有 link 的链。由测试 holds a prompt typed mid-drain behind the release chain 钉住:挂起 link 1 的准入、在 drain 期间输入第三条 prompt,断言链在飞行中它不 POST,且最终顺序为 ['first with media', 'second plain', 'typed during drain']。
head 上仍未解决——第 3 轮三条 Critical,均无作者回应、无修复提交,逐条在 a39b8243 代码中复核确认: R3-16——Goal 运行中分离的会话会被 daemon 自身回收:busy 汇总 OR 进了 goalTurnActive,但保留机制从未看到它;entryHasLocalWork 仍只读 pendingPromptCount/pendingAgentNotificationCount,ACTIVE_WORK_HOLD_CATEGORIES 仍是 'agent' | 'notification' | 'shell' 没有 goal 类别,collectActiveWorkHolds() 仍只报 agent/notification/shell;条件关闭路径等轮次结算(goal 轮次不占 prompt 槽,所以会结算)后看到零 hold 即关闭,而"启动 Goal 后关掉标签页"正是 Goals 的典型用法。R3-17——#emitGoalEndTurn 仍在发送时从共享可变的 this.turn 重新推导 promptId(通知体内 sessionId + '########' + String(this.turn)),启动前被 closing/write barrier 拒绝的 goal 轮次在 catch 中结算、早于 finally 发送,会发布上一个(或 ########0、或抢占时下一个)轮次的 id。R3-21——显式 Insert 被拒时,owner 中途变更(或 reconcile 本身失败)就跳过 daemon 快照检查并清掉 midTurnMessageId,daemon 已提交的消息之后会经 drain 二次执行。
合并增量——此前的修复未回退: 两次 upstream 合并带入 prompt-terminal ledger 与 review 命令重构,冲突解决中保留了 Goal 接缝。抽查第 2/3 轮修复在 head 上均完好:带 core 协议漂移守卫的共享 canResumeGoal 门(goalGate.ts)、操作所有权的控制锁(App.tsx 的 goalControlOpSeqRef/goalControlOwnerRef 与 controlBusy 输入框守卫)、以及移入共享 releaseChainedPrompt 的链 owner 令牌与行存在性守卫。这些接缝之外的合并内容以绿色 CI 为证据。
测试证据(PR 自己的 CI——无人值守路径)
本次为无人值守 CI 重跑(issue_comment 触发):未构建或执行任何 PR 代码;TUI 无变化声明可由维护者触发隔离的 @qwen-code /tmux 任务验证。审查时点 a39b824 上,主套件 Test (ubuntu-latest, Node 22.x) 与 web-shell visuals、Serve A/B 仍在途——因此 R3-2 的新测试在 CI 上尚未转绿。已完成的检查全部为绿:Real daemon E2E、Desktop Shell(两平台)、Live Host、SDK 兼容矩阵(5 项)、TruffleHog、CVE 审计、Classify PR、precheck。macos/windows 测试与 CLI 集成测试按 ci.yml 仅在 merge queue 运行,属设计内跳过;review-pr 为机器人编排而非 PR CI。修复提交说明中的本地 3,986 项 web-shell 测试通过与 R3-2 守卫的变异翻转属作者声明,本审查依赖的证据是上方 CI。
由于三条未决 Critical 都是行为级且无测试钉住,即便套件转绿也不能定论;唯一落地的修复(R3-2)也只由 PR 自带测试钉住,CI 在本 head 上尚未确认。沙箱验证可以补足 CI 无法定论的部分:@qwen-code /verify——A/B 对照 base 构建即可确认 R3-2 链保序成立,并探测三条未决行为(分离会话在 Goal 中存活、end_turn 携带正确 promptId、丢失 Insert 响应不会二次执行)。作者有写权限,该通道直接可用。@qwen-code /tmux——TUI 无变化声明仅经作者 macOS 自测。
未验证:三条未决 Critical(无测试钉住,本跑也未执行 PR 代码);R3-2 修复的 CI 结果(审查时套件仍在途);Goal 接缝之外的合并冲突解决(以绿色 CI 为证据);Windows/Linux UI 行为(作者标记
— Qwen Code · qwen3.8-max
Reviewed at a39b82439635d28d509ad39ba76fcd6349c0200c · re-run with @qwen-code /triage
|
Confidence: 2/5 — the R3-2 fix landed cleanly and is verified, but three of the four round-3 Criticals are verified still open at this head with no reply or fix commit, one of them breaking the canonical Goals use case; not mergeable as-is. Stepping back: the increment since the last pass is a genuinely good single fix and two clean merges — and that is exactly the problem, because it addresses one of the four open items. The R3-2 fix is what a fix round should look like: the live chain is published on an owner-pinned ref, The other half is that three round-3 Criticals have no reply and no fix, and reading the code at head confirms they are still real, not stale:
None of these is a judgment-call item — they are mechanism gaps with witness probes in the review thread. The standing Two things that are not the author's fault but shape what happens next. First, the main unit suite was still in flight at review time, so even the R3-2 fix's new test has not confirmed green in CI at this head — it needs to land green before anything else. Second, and separately from all of the above, the Stage 0 escalation stands: a ⏸️ So: request changes on the three open Criticals, wait for the suite to land green, then the maintainer call remains. If those three land with pinning tests, this is close — the R3-2 fix shows exactly the bar the rest of the review surfaced, and it cleared it. 中文说明置信度:2/5 —— R3-2 修复干净落地且已核实,但第 3 轮四条 Critical 中有三条在本 head 上核实仍未解决、既无回应也无修复提交,其中一条破坏 Goals 的典型用法;当前状态不可合并。 退一步看:上次审查之后的增量是一个确实不错的单点修复加两次干净合并——而问题恰恰在于它只处理了四条未决项中的一条。R3-2 修复正是修复轮该有的样子:存活的链发布在 owner 钉住的 ref 上, 另一半是:第 3 轮有三条 Critical 既无回应也无修复,读 head 代码确认它们仍然真实存在而非过期:R3-16——启动 Goal 后关闭标签页,daemon 自身的保留机制会回收会话(busy 汇总知道 goalTurnActive,保留机制不知道,也没有 goal hold 类别可报),Goal 停留在 active 却无人推进,并从 GET /goals 无声消失——这正是 Goals 的典型用法;R3-17——#emitGoalEndTurn 仍在发送时计算 sessionId + '########' + String(this.turn),#executePromptInner 递增计数器前被拒的 goal 轮次会发布上一个轮次的 id;R3-21——响应丢失的显式 Insert 在会话中途切换(或 reconcile 失败)时跳过 daemon 快照检查并清掉 midTurnMessageId,daemon 已提交的消息之后会经 drain 二次执行。这三条都不是政策判断题,而是审查线程中带有探针证据的机制缺口。本 PR 上现有的 CHANGES_REQUESTED 仍然成立,因此本次不重复提交评审;门禁状态已反映该结论。@qqqys —— 需要回应上述三个线程(修复或有理有据的反驳),最好保持 R3-2 修复所体现的变异验证标准——那一处做得非常出色。 另有两点非作者之过、但决定后续走向。其一,审查时点主套件仍在途,连 R3-2 修复的新测试也尚未在 CI 上转绿——需先等套件转绿。其二,与上述无关的是 Stage 0 升级依然有效:触及 core 路径、约 4,200 行生产代码、横跨六个包的 feat 无论审查结果如何都不自动批准;最终 head 仍需 @yiliang114(或其他维护者)签核,覆盖代码本身与此前各轮留给维护者的降级支持策略问题。 ⏸️ 因此:就三条未决 Critical 请求修改,等套件转绿,之后维护者决定仍然必要。若这三条连同钉住它们的测试一起落地,这个 PR 就很接近终点了——R3-2 修复展示了其余审查所确立的标准,并且达标了。 — Qwen Code · qwen3.8-max Reviewed at |
🩺 serve daemon A/BBuilt the PR base vs this PR head ✅ No response changes against the PR base across 4 scenario(s). — Qwen Code · serve A/B |
The PR registers `POST /session/:id/goal` and `GET /session/:id/goal`, both `handler_resolved`, taking the legacy session telemetry catalog from 59 routes to 61 and the attribution split from 57/2 to 59/2. Three hardcoded totals in telemetry.test.ts and one in the drift guard still asserted the old counts, so the Test job failed even though the guard's real check — registered Express routes equal the catalog — passed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
@qwen-code /review |
|
Qwen Code review request accepted. Review is queued in workflow run. |
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: reverse audit — stopped before round 3 by the review time budget.
中文说明
仅完成部分审查,审查缺口已披露。
未审查:build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。
未审查:反向审计——评审时间预算不足,未能开始第 3 轮。
— qwen3.8-max via Qwen Code /review (v0.21.13)
|
Qwen Code review did not complete successfully. The review pipeline failed before a review could be posted. A transient error is retried automatically; if you are seeing this, retry with |
|
@qwen-code /takeover |
doudouOUC
left a comment
There was a problem hiding this comment.
Not reviewed: coverage — could not read the agents' transcripts (no subagent transcripts at C:\Users\jinye.djy.qwen\projects\c--users-jinye-djy--qoderwork-workspace-mspqz3u5etjh72hs-qwen-code\subagents\724dd642-0ca3-4fbe-a5d3-979607bc00c3 (ENOENT: no such file or directory, scandir 'C:\Users\jinye.djy.qwen\projects\c--users-jinye-djy--qoderwork-workspace-mspqz3u5etjh72hs-qwen-code\subagents\724dd642-0ca3-4fbe-a5d3-979607bc00c3'). The harness writes one per agent; if there are none, either no agents ran or the harness could not write them.), so this run cannot show that any of the diff was read.
Not reviewed: verification — could not check that Step 4 and Step 5 ran (no subagent transcripts at C:\Users\jinye.djy.qwen\projects\c--users-jinye-djy--qoderwork-workspace-mspqz3u5etjh72hs-qwen-code\subagents\724dd642-0ca3-4fbe-a5d3-979607bc00c3 (ENOENT: no such file or directory, scandir 'C:\Users\jinye.djy.qwen\projects\c--users-jinye-djy--qoderwork-workspace-mspqz3u5etjh72hs-qwen-code\subagents\724dd642-0ca3-4fbe-a5d3-979607bc00c3'). The harness writes one per agent; if there are none, either no agents ran or the harness could not write them.).
中文说明
未审查:覆盖情况——无法读取 agent 的运行记录(no subagent transcripts at C:\Users\jinye.djy.qwen\projects\c--users-jinye-djy--qoderwork-workspace-mspqz3u5etjh72hs-qwen-code\subagents\724dd642-0ca3-4fbe-a5d3-979607bc00c3 (ENOENT: no such file or directory, scandir 'C:\Users\jinye.djy.qwen\projects\c--users-jinye-djy--qoderwork-workspace-mspqz3u5etjh72hs-qwen-code\subagents\724dd642-0ca3-4fbe-a5d3-979607bc00c3'). The harness writes one per agent; if there are none, either no agents ran or the harness could not write them.),本次运行无法证明 diff 的任何部分被读过。
未审查:验证——无法检查步骤 4 与步骤 5 是否运行(no subagent transcripts at C:\Users\jinye.djy.qwen\projects\c--users-jinye-djy--qoderwork-workspace-mspqz3u5etjh72hs-qwen-code\subagents\724dd642-0ca3-4fbe-a5d3-979607bc00c3 (ENOENT: no such file or directory, scandir 'C:\Users\jinye.djy.qwen\projects\c--users-jinye-djy--qoderwork-workspace-mspqz3u5etjh72hs-qwen-code\subagents\724dd642-0ca3-4fbe-a5d3-979607bc00c3'). The harness writes one per agent; if there are none, either no agents ran or the harness could not write them.)。
— qwen-code-review via Qwen Code /review (v0.21.13)
Resolve the queue conflicts against QwenLM#9407 (backend-authoritative queue state). The unknown-admission row state that Goal v3 built on is gone upstream, so the explicit Insert path now lets the daemon's mid-turn snapshot decide: a confirmed message becomes a daemon-owned mid-turn row, a settled or promoted one leaves the local queue, and an insert the daemon cannot confirm returns to the local Goal hold instead of lingering as an "unknown" row. Local Goal-held rows still survive an owner change; daemon-owned rows are re-rendered from the next snapshot. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…e reads, and load Three ordering defects let a stale Goal frame win over the daemon's actual state, each of which then flips `holdQueuedPromptsLocally`, the Goal strip, and the manual-run gate off a goal that is not really there. - `selectGoalState` recorded an ordering identity only for a cleared goal, and `goal-runtime` attaches `clearedGoal` to a clear but not to a replace — so a pre-replace frame of the replaced goal passed every guard and reinstalled it over its replacement. Carry a bounded ledger of superseded goal identities (cleared and replaced alike) forward onto each accepted snapshot and reject frames at or behind one. - `getGoal()` reconciled a bare-null READ snapshot against whatever state existed at resolution time. A read the daemon answered while goal-less can land after a concurrent create, and with no `clearedGoal` tombstone it read as "clear whatever is current", wiping the new goal. Stamp the read with the goal observed at issue time: a bare-null response may only clear that goal. - The session-load path installed its snapshot behind a reference-equality guard instead of `selectGoalState`, so a frame arriving inside the load window discarded the authoritative response — and when none arrived, the raw install registered no tombstone and a later stale frame resurrected a cleared goal. Reconcile instead, and keep the synthesized empty snapshot for a failed fetch only while no state is known. Each fix is pinned by a test that fails when the fix is reverted. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
… fetch Only the fresh-connection branch (synthesize an idle snapshot) was covered, so a simplification that always synthesizes on rejection would ship green and replace a live goal with idle on a transient `goal()` failure. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…at started them
Four Goal-surface defects, all of the same shape — state applied to whatever
form, latch, or composer exists when an operation settles rather than to the
one that started it:
- `GoalEditDialog` live-synced its textarea to the `objective` prop, so a
concurrent edit from another client (or the refresh a failed save triggers)
silently overwrote the user's typed draft — the only copy. Adopt prop
refreshes only while the field is still pristine.
- `GoalsDialog` left its form dismissible while a submit was in flight, so
closing it mid-request handed that request's `resetForm()`/`setFormError` to
the next goal's form. Pass `dismissible={!submitting}`, matching
`GoalEditDialog`.
- ChatPane's `/goal` branch ran before the broken-connection guard applied
further down the same `handleSubmit`, so a control typed while the pane was
disconnected was consumed, written to the transcript, and then failed with
only a toast. Apply the guard inside the branch and keep the text.
- ChatPane's goal-control busy latch was session-keyed, so a server-side goal
replacement released it mid-operation and a second control could dispatch
against the same expected revision (one loses with a 409). Key the latch to
the operation and release it only from its owner.
Each fix is pinned by a test that fails when the fix is reverted.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ng held prompts - The `goalSnapshotRef` gates (`promptBlocked`, retry, language change, fast-model select) failed OPEN while `connection.goalState` was still hydrating: the session load clears `loadingTranscript` before its `goal()` fetch resolves, so a prompt typed in that window was submitted straight into a Goal the client had not learned about — while the queue-hold gate, `enqueueManualRun` and `tryFireBoundRun` all failed closed on the identical state. They now share one `isGoalGateBlocked()` predicate that treats an unknown Goal state on a real session as blocked. - Creating a Goal in an allocated session went through the workspace-scoped control, which — unlike `sessionActions.controlGoal` — never wrote `connection.goalState`; the only compensation was a conditional re-sync one round trip later. Until it landed the hold gate read false and no Goal strip rendered. A new `applyGoalSnapshot` session action installs the create response into the connection state, reconciled like any other snapshot. - Locally held Goal prompts were stashed under `(workspaceCwd, sessionId)` and could only be relocated when the session just left was the same one, so a workspace resolving while the user was on another session orphaned the stash under a key nothing looks up again — silently losing typed text. Any stash whose session half matches is now relocated and restored in queue order. Each fix is pinned by a test that fails when the fix is reverted. The App test harness now defaults to a hydrated (goal-less) snapshot, matching a loaded session; the tests that exercise the hydration window set it back to undefined. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Resolve the ChatPane composer conflict with QwenLM#9351: keep upstream's approval-owns-the-footer wrapper and take the Goal composer-status stack (queued prompts plus the Goal strip) inside it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Goal turns are driven inside the ACP child via its own prompt() call, so the daemon bridge never sees a session/prompt RPC boundary for them and published no turn terminal on SSE. Web Shell (and any SSE client) only settles its streaming state on turn_complete/turn_error/prompt_cancelled, so after a Goal turn the spinner spun forever and locally queued messages never drained — even though the daemon itself was already idle. The child now sends the existing `_qwencode/end_turn` notification with `source: 'goal'` and a turn-scoped promptId when a Goal turn settles, and the bridge translates that into a real `turn_complete` frame — the same contract the Web Shell goal e2e mock already assumed.
…bort registry - Releasing several locally held Goal prompts fired every submission at once, so a prompt awaiting media uploads could be overtaken by a later plain one and reach the daemon's queue out of order. `submitPendingPrompt` now returns its admission promise and the release chains them; the first release stays synchronous. - `explicitInsertAbortControllersRef` was populated and cleaned up but never read: nothing aborted its controllers, so the signal plumbing and the `abort.signal.aborted` recovery branches were unreachable. An explicit insert is meant to outlive an owner rotation and settle into the queue of the session it was started from (two tests pin exactly that), so the registry, the signal and the dead branches are gone rather than given a consumer. - Both mount sites rendered the Insert button enabled while streaming was idle and a Goal was active — precisely the state `insertQueuedPrompt` no-ops in. `canInsertMidTurn` now tracks the hook: the affordance appears only while a turn is running. Test coverage the review probes found missing, each verified by reverting the line it gates: the images and slash-command insert guards, the `isInserting` reset on both settle-unaccepted paths, the release order, the Insert affordance, and the held-prompt stash handoff (a prompt already dealt with must not come back from a stale owner key). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…wallowing input - `parseWebShellGoalCommand` returned an English sentence that `formatError` prefers over any localized fallback. It now returns the offending keyword and both composers render `goals.error.requiresObjective` from the dictionaries; the manual-run rejection gets `scheduledTasks.error.goalActive` for the same reason. - `handleGoalSlashCommand` returned true — wiping the composer — before the preconditions it checks asynchronously held, so a `/goal clear` typed without a session lost its text to a toast. The cheap preconditions are checked first and refuse the submit instead. - `enqueueManualRun` treated an unknown Goal state as "Goal is active", which also fires when no session is attached and no Goal can exist, so Run now on a fresh workspace always failed. It shares the hydration-aware gate now. - ChatPane consulted the host slash handler after its `/goal` intercept, so a host override applied in the main composer but not in a pane; and a bare `/goal` in a pane without a Goals view consumed the text silently. The pane now matches the composer's ordering and refuses what it cannot open. - GoalsDialog offered Edit on a completed Goal, which the reducer rejects, and fell back to the stale snapshot when the edited session left the list — turning the friendly "no longer available" path into a raw conflict error. - The versioned control request is built by one shared helper instead of two copies that had already drifted. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…e stranded-session rationale
- `GOAL_STATUS_ACTIVE_EVENT` lost its only production listener in this PR while
`GoalStatusMessage` kept dispatching it and two test files kept asserting the
dispatch — a contract that looks live but is dead. The dispatch, the
`activateFooter` prop, the `isLatest` plumbing that fed it, and the
assertions are gone.
- The built-in StatusBar now received `activeGoal={null}` permanently, so its
goal pill and `onOpenGoals` wiring could never fire while StatusBar.test.tsx
kept them green. The pill, its props, its elapsed-time ticker and that
pill-only test file are removed; the composer status stack is the goal
surface. Custom footers keep their `activeGoal` prop, which is still fed.
- `/language ui` skipped its daemon sync when `promptBlocked`, silently
switching the chrome while the agent kept answering in the old language for
the rest of a Goal run. It now refuses with the same feedback the language
picker gives for the identical condition.
- The `@container` block in GoalStatusStrip styled `.root`, which cannot match
its own container query, so half the responsive rule never applied. Trimmed
to the descendant rules that do, with the reason recorded.
- Restored the rationale comment above `strandedGoalSessionRef` — the mechanism
it documents is unchanged and still live.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
复审回应 / Response to the re-review先谢谢这轮复审——两个 Critical 都对,我逐环对着 head 复核过,其中 C2( 六条 inline finding 已全部修复并在各自的线程里回复+resolve:
C2 按"让 Insert 真的能进 Goal turn"修,而不是禁用按钮——PR 描述与 Reviewer Test Plan 都承诺了这个能力,且子进程本来就在 Goal turn 内部逐批 drain 同一个队列。顺带补上了同源的一环:Goal turn 不占 prompt slot,它最后一次 drain 之后入队的消息此前无人结算,现在由 goal 每条新增测试都做了探针验证(还原被测行即变红)。已跑:acp-bridge 未修的三条及原因 / Deliberately not fixed
两条 "需要人工确认" 的(降级 daemon 复活已清除 Goal、 All six inline findings are fixed and answered in their own threads (table above); C2 is fixed by making Insert actually work during a Goal turn rather than disabling the affordance, since the PR promises that capability and the child already drains this queue from inside the Goal turn. Every new test is probe-verified. Three items are deliberately not fixed — the mock-daemon busy model (a half-modelled busy state would misjudge existing specs, and the rejection logic is now pinned at the bridge layer instead), the latent 409 |
|
@qwen-code /triage |
|
Sandboxed verification: The verification job did not complete (checkout, runner, or setup error) and produced no report. See the workflow run for details. 中文 — 判定:
|
|
Triage re-run completed without a new review.
The stage comments above were updated with the latest result. View workflow run. 上方各阶段评论已更新为最新结果。查看工作流运行。 |
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: reverse audit — stopped before round 3 by the review time budget.
中文说明
仅完成部分审查,审查缺口已披露。
未审查:build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。
未审查:反向审计——评审时间预算不足,未能开始第 3 轮。
— qwen3.8-max via Qwen Code /review (v0.21.14)
| // Release serially: a prompt carrying media awaits its uploads before its | ||
| // admission POST, so firing the whole batch at once lets a later plain | ||
| // prompt overtake it and reach the daemon's queue out of order. | ||
| let release: Promise<void> | undefined; |
There was a problem hiding this comment.
[Critical] The serial release chain carries no owner/session token and nothing cancels it, while the owner-change stash only saves rows satisfying isLocallyHeldPrompt (serverState === undefined). Switching sessions mid-drain orphans every chain-marked row: the in-flight row is aborted and its catch early-returns on the stale owner token (no restore), and the remaining links drop silently (actions.submitPrompt throws on the sessionId mismatch before POSTing).
Failure scenario: a Goal ends on session 1 with 2+ held prompts (the first carries media, stretching the admission window seconds — the very delay this chain serializes); the drain stamps all rows serverState: 'submitting' and chains them; the user switches to session 2 mid-drain → the owner-change effect excludes all chain rows from the stash and aborts the in-flight admission → both typed prompts are permanently lost (queue empty on return, nothing restored to the editor). The window spans the whole serial drain, so a mid-drain session switch is a routine hazard, not a microsecond race.
Witness (probe through the real hook): BASE drainStart=['first with media'] afterSwitch=['first with media','second plain'] queueAfterReturn=[] editorRestores=[]; with a chain owner-token guard + stashing submitting rows, queueAfterReturn=['first with media:submitting','second plain:submitting']. Fix reverted after the probe.
Suggested fix: capture the owner token (and session id) when the chain is built; in each link, bail and revert the row when owner/session no longer matches. Also, in the owner-change effect, synchronously reset locally-held rows that are serverState: 'submitting' with no serverPromptId back to serverState: undefined before the stash filter, so the abort is paired with the text surviving in the stash.
中文说明
串行释放链(release chain)不携带 owner/session 令牌,也没有任何取消机制;而 owner 变更时的暂存(stash)只保存满足 isLocallyHeldPrompt(serverState === undefined)的行。在 drain 进行中切换会话会使所有被链标记的行成为孤儿:飞行中的行被中止,其 catch 因 owner 令牌过期直接返回(不恢复),其余环节静默丢弃(actions.submitPrompt 在 POST 前就因 sessionId 不匹配抛错)。
失败场景:会话 1 的 Goal 结束时有 2 条以上保留消息(第一条带媒体,把准入窗口拉长到秒级——正是本链要串行化的延迟);drain 把所有行标记为 serverState: 'submitting' 并串成链;用户在 drain 中途切到会话 2 → owner 变更 effect 把所有链上行排除在 stash 之外并中止飞行中的准入 → 两条已输入的消息永久丢失(返回时队列为空,也没有恢复到编辑器)。窗口覆盖整个串行 drain,因此 drain 中途切换会话是常规风险而非微秒级竞态。
证据(探针,走真实 hook):BASE drainStart=['first with media'] afterSwitch=['first with media','second plain'] queueAfterReturn=[] editorRestores=[];加上链 owner 令牌守卫并把 submitting 行纳入 stash 后,queueAfterReturn=['first with media:submitting','second plain:submitting']。探针后已还原修复。
建议修复:构建链时捕获 owner 令牌(及 session id);每个环节在 owner/session 不再匹配时退出并还原该行。同时在 owner 变更 effect 中,stash 过滤之前把本地持有、serverState: 'submitting' 且无 serverPromptId 的行同步重置回 serverState: undefined,使中止操作与文本在 stash 中存活配对。
— qwen3.8-max via Qwen Code /review (v0.21.14)
There was a problem hiding this comment.
Fixed in aa1d913 — both halves, with one deliberate narrowing I want on the record rather than buried.
Confirmed as described. The chain carried no owner token, so remaining links called submitPrompt against the old session, which throws before POSTing and was swallowed by the chain's own .catch(() => undefined); and the owner-change stash filters on isLocallyHeldPrompt (serverState === undefined), so every chain-marked row was excluded from it.
What landed:
- The chain pins
chainOwner = ownerTokenRef.currentwhen it is built, and each link bails when that token is no longer current. Pinned at build time, not read per link — the guard has to ask "is this still the owner the chain was built for", not "is there an owner". - A new
unreleasedPromptIdsRefholds exactly the rows the chain stampedsubmittingbut never handed tosubmitPendingPrompt. Each link drops its own id as it fires; the owner-change effect stashes those alongside the genuinely held rows and then empties the set. The optimistic stamp is dropped on the way into the stash, so the row comes back as a plain held prompt and the next drain re-releases it in order.
The narrowing — the row whose admission is actually in flight is NOT stashed. Your probe expected both rows back (queueAfterReturn=['first with media:submitting','second plain:submitting']), and I started there. It turns out that flips two existing tests: ignores an old submit response after an S1 to S2 to S1 owner change and fences an old submit before the replacement owner rerenders, which deliberately fence and drop an in-flight submission across an owner change. That contract looks right to me — the POST may well have landed server-side, so restoring the row risks a duplicate rather than a rescue. The rows the chain never POSTed carry no such ambiguity: nothing exists for them on the daemon, so stashing them cannot duplicate anything.
So this commit closes the unambiguous half in full and leaves the in-flight row on the existing fencing contract. If you want that contract reversed, say so and I will do it in the next round — it is a design call about duplicate-vs-loss, not something to change in passing by weakening two tests that were written on purpose.
New test (useQueuedPrompts.dom.test.tsx): stashes the undrained release chain when the session changes mid-drain — your exact scenario. Two held prompts, first mocked to hang its admission, session switches mid-drain, then back. It asserts the second link never POSTs into session-2, and that on return the row is in session-1's queue and the fresh drain submits it, in order: ['first with media', 'second plain'].
Mutation-verified, three ways, each turning that test red on its own:
| mutation | effect |
|---|---|
| remove the chain owner guard | link 2 POSTs into session-2 |
| remove the unreleased-row half of the stash filter | text lost, queue empty on return |
| keep the optimistic stamp on the way into the stash | row returns stranded as submitting, never re-drained |
A fourth mutation — moving the id retirement below the owner guard instead of above — survives, and I left it that way rather than writing a test for it: the owner-change effect empties the set after reading it, so the two orderings are genuinely equivalent. Flagging it so it does not read as an untested branch.
Verification: npx vitest run client/hooks/ client/utils/ client/components/GoalStatusStrip.test.tsx client/components/dialogs/GoalsDialog.test.tsx client/components/ChatPane.test.tsx in packages/web-shell → 1011 passed, vs 994 on the stashed tree, with an identical 17 pre-existing failures in the untouched hooks/useMessages.test.ts on both trees.
There was a problem hiding this comment.
这条在 r2 的提交里已经加了链 owner 令牌(chainOwner + 每个 link 的 isCurrentOwnerTokenRef 检查)和把 unreleasedPromptIdsRef 里的行纳入 owner 变更暂存。r3 指出的残留缺口(R3-24:delete 在 owner 检查之前执行,使提交前窗口里触发的 link 把自己从暂存集合里摘掉)已在 3d885b1321 关闭,并配了复现该窗口的测试。
留着不 resolve:这是本条 Critical 的类问题入口,等下一轮复审确认串行链在 owner 变更下已经收敛再关。
| if (!entry || !this.ownsSession(sessionId)) return; | ||
| if (source === 'goal') { | ||
| entry.goalTurnActive = false; | ||
| // Before the promptId validation below: a malformed id costs the |
There was a problem hiding this comment.
[Suggestion] No test pins the settle-before-validate ordering this comment declares load-bearing: this.onGoalTurnEnded?.(sessionId) runs BEFORE the malformed-promptId early return. The existing drop-without-promptId test constructs BridgeClient without an onGoalTurnEnded seam, and the bridge-level settle tests only send well-formed promptIds.
Failure scenario: a future refactor moves onGoalTurnEnded below the malformed-promptId early return; a child goal terminal with a missing/oversized promptId clears goalTurnActive but never settles the undrained mid-turn queue — queued ordinary messages sit un-promoted until an unrelated prompt arrives, and every existing test still passes.
Suggested fix: in the third test, construct the client with an onGoalTurnEnded spy as the trailing constructor arg and assert it is called with the sessionId even though publish is not; or add a bridge-level variant of the promote-on-settle test with a malformed promptId.
中文说明
没有测试钉住该注释声称至关重要的“先结算、后校验”顺序:this.onGoalTurnEnded?.(sessionId) 在格式错误的 promptId 提前返回之前执行。现有的“无 promptId 即丢弃”测试构造 BridgeClient 时没有传 onGoalTurnEnded 接缝,bridge 层的结算测试也只发送格式正确的 promptId。
失败场景:未来重构把 onGoalTurnEnded 移到格式错误 promptId 的提前返回之下;子端 Goal 终止信号缺少/超长 promptId 时会清掉 goalTurnActive 却从不结算未 drain 的 mid-turn 队列——排队的普通消息一直得不到提升,直到某个无关 prompt 到来,且所有现有测试仍然通过。
建议修复:在第三个测试中以尾部构造参数传入 onGoalTurnEnded spy,断言即使未 publish 也以 sessionId 被调用;或给 promote-on-settle 测试加一个 malformed promptId 的 bridge 层变体。
— qwen3.8-max via Qwen Code /review (v0.21.14)
| export interface GoalRecord { | ||
| goalId: string; |
There was a problem hiding this comment.
[Suggestion] The SDK's duplicated GoalRecord omits core's optional evidenceCheckpoint (packages/core/src/goals/goal-protocol.ts defines it on the same wire record; parseGoalRecord accepts it and goal-reducer.test.ts round-trips it). Daemon GET/POST /session/:id/goal responses can carry the field while SDK typings cannot see it.
Failure scenario: nothing is broken today (no consumer reads it, and the webui mapper rebuilds records field-by-field), but the first consumer that renders checkpoint evidence must cast to unknown or re-declare the shape — the exact silent drift the duplication's own comment is meant to prevent.
Suggested fix: add evidenceCheckpoint?: GoalEvidenceCheckpoint (plus the nested claim/proof-kind types) to the SDK copy, or leave a one-line note marking the omission intentional.
中文说明
SDK 复制的 GoalRecord 缺少 core 同一条线上记录中的可选字段 evidenceCheckpoint(packages/core/src/goals/goal-protocol.ts 定义了它;parseGoalRecord 接受该字段,goal-reducer.test.ts 有往返测试)。daemon 的 GET/POST /session/:id/goal 响应可能携带该字段,而 SDK 类型看不到它。
失败场景:今天没有实际破坏(无消费者读取,webui mapper 逐字段重建记录),但第一个渲染检查点证据的消费者必须强转 unknown 或重新声明形状——正是这份复制注释本要防止的静默漂移。
建议修复:在 SDK 副本中补上 evidenceCheckpoint?: GoalEvidenceCheckpoint(及嵌套的 claim/proof-kind 类型),或留一行注释说明该省略是有意的。
— qwen3.8-max via Qwen Code /review (v0.21.14)
| @@ -250,6 +252,486 @@ describe('mapWorkspaceSkills', () => { | |||
| }); | |||
|
|
|||
| describe('updateConnectionFromDaemonEvent', () => { | |||
There was a problem hiding this comment.
[Suggestion] The reconciliation suite never isolates revision dominance: no same-goalId fixture where revision and updatedAt disagree (higher revision + OLDER updatedAt), and no superseded-identity fixture with revision behind but updatedAt newer — so two mutations survive the full 29-test suite green.
Failure scenario (both mutations run): (1) reducing selectGoalState's same-goalId guard to an updatedAt-only comparison, and (2) reducing isSupersededGoalFrame to updatedAt-only — each proposed isolating fixture flips its mutation and passes on correct code. After a daemon clock step (NTP correction, VM restore) produces a lower-revision frame with a newer stamp, a shipped regression either accepts a stale-revision frame as an upgrade (the Goal UI silently regresses to an older objective/status) or rejects a legitimate revival.
Suggested fix: add the two isolating fixtures — same-goalId incoming { revision: 8, updatedAt: 20 } against current { revision: 7, updatedAt: 30 } must be rejected; a superseded-identity frame { revision: 5, updatedAt: 25 } against ledger identity { revision: 4, updatedAt: 30 } must be accepted.
中文说明
协调(reconciliation)套件从未隔离“revision 优先”:没有同 goalId 且 revision 与 updatedAt 不一致(更高 revision + 更旧 updatedAt)的 fixture,也没有 revision 落后但 updatedAt 更新的被取代身份 fixture——因此两个突变都能让全部 29 个测试绿着通过。
失败场景(两个突变都实际运行过):(1) 把 selectGoalState 的同 goalId 守卫降为只比较 updatedAt;(2) 把 isSupersededGoalFrame 降为只比较 updatedAt——每个建议的隔离 fixture 都能翻转对应突变并在正确代码下通过。daemon 时钟跳变(NTP 校正、VM 恢复)产生“低 revision + 新时间戳”的帧后,带病上线的回归要么把过期 revision 帧当作升级接受(Goal UI 静默回退到更旧的 objective/status),要么拒绝合法的复活。
建议修复:补两个隔离 fixture——同 goalId 传入 { revision: 8, updatedAt: 20 } 对当前 { revision: 7, updatedAt: 30 } 必须被拒绝;被取代身份帧 { revision: 5, updatedAt: 25 } 对台账身份 { revision: 4, updatedAt: 30 } 必须被接受。
— qwen3.8-max via Qwen Code /review (v0.21.14)
| } as DaemonEvent, | ||
| ); | ||
|
|
||
| expect(next.goalState?.goal?.limitKind).toBeUndefined(); |
There was a problem hiding this comment.
[Suggestion] 'drops an unknown limitKind rather than passing it through' only asserts limitKind is undefined — which passes equally if getGoalState rejected the WHOLE snapshot or nulled the goal. The 'drops (keeps the goal)' half of its own title is unpinned, unlike its sibling 'carries limitKind through from the wire', which pins goal survival via toMatchObject. The start state has no goalState, so a hardened parser returning undefined keeps the assertion green.
Failure scenario (mutation-proven): a hardened parser mutation returning undefined (dropping the entire snapshot) for an unknown limitKind keeps this test green. Shipped regression: a newer daemon emitting a new limitKind value causes the entire goal snapshot to be dropped — the goal vanishes from the web-shell UI (strip, GoalsDialog, prompt gates) instead of rendering without the field.
Suggested fix: add expect(next.goalState?.goal).toMatchObject({ goalId: 'goal-1', status: 'paused' }); before the existing assertion.
中文说明
'drops an unknown limitKind rather than passing it through' 只断言 limitKind 为 undefined——即使 getGoalState 拒绝了整个快照或把 goal 置空也同样通过。它标题中“丢弃(但保留 goal)”的另一半没有被钉住,不像兄弟测试 'carries limitKind through from the wire' 用 toMatchObject 钉住了 goal 存活。初始状态没有 goalState,因此一个强化后对未知 limitKind 返回 undefined 的解析器能让断言保持绿色。
失败场景(突变测试证实):未知 limitKind 时返回 undefined(丢弃整个快照)的强化解析器突变能让本测试保持绿色。带病上线的后果:更新的 daemon 发出新的 limitKind 值时整个 goal 快照被丢弃——goal 从 web-shell UI(状态条、GoalsDialog、prompt 门禁)消失,而不是不带该字段继续渲染。
建议修复:在现有断言前补 expect(next.goalState?.goal).toMatchObject({ goalId: 'goal-1', status: 'paused' });。
— qwen3.8-max via Qwen Code /review (v0.21.14)
| setConnection((current) => { | ||
| if (current.sessionId !== session.sessionId) return current; |
There was a problem hiding this comment.
[Suggestion] The session-scope guards in getGoal/controlGoal — the only protection against a delayed Goal response writing another session's snapshot into the connection state after a switch — are not pinned by any test (all five new Goal tests keep the harness connection on session-a, including the in-flight replaceConnection case).
Failure scenario (mutation-proven): deleting both guards keeps actions.test.ts + DaemonSessionProvider.test.tsx green (311/311); mirrored probe tests (deferred read/control on A, switch to B with goal-B installed mid-flight, resolve with A's non-null frame) show goalState.goal.goalId flip 'goal-b' → 'goal-a' under mutation — cross-session corruption of the strip and prompt-hold gating. selectGoalStateFromRead's stamp only intercepts bare-null responses, so a non-null different-goalId frame falls through to selectGoalState and is accepted as a replacement.
Suggested fix: tests mirroring the stale-read test — issue getGoal() against a deferred session.goal, replaceConnection to session-b with a goal-B snapshot, resolve the deferred read with session A's goal snapshot, assert goalState is still the goal-B reference; repeat the shape for controlGoal.
中文说明
getGoal/controlGoal 中的会话范围守卫——防止切换会话后迟到的 Goal 响应把另一个会话的快照写入 connection 状态的唯一保护——没有任何测试钉住(全部五个新 Goal 测试都把 harness 连接保持在 session-a,包括飞行中 replaceConnection 的用例)。
失败场景(突变测试证实):删除两个守卫后 actions.test.ts + DaemonSessionProvider.test.tsx 仍全绿(311/311);镜像探针测试(对 A 发起延迟读/控制,飞行中切换到已安装 goal-B 的 B,用 A 的非空帧 resolve)显示突变下 goalState.goal.goalId 从 'goal-b' 翻转为 'goal-a'——状态条与 prompt 保留门禁被跨会话污染。selectGoalStateFromRead 的 stamp 只拦截 bare-null 响应,非空且 goalId 不同的帧会落入 selectGoalState 并被当作替换接受。
建议修复:补镜像 stale-read 测试——对延迟的 session.goal 发起 getGoal(),replaceConnection 到带 goal-B 快照的 session-b,用 session A 的 goal 快照 resolve 延迟读取,断言 goalState 仍是 goal-B 引用;对 controlGoal 重复同样形状。
— qwen3.8-max via Qwen Code /review (v0.21.14)
| editLastQueuedPrompt.mockClear(); | ||
| clearQueuedPrompts.mockClear(); | ||
| transcriptDispatch.mockClear(); | ||
| appendLocalUserMessage.mockClear(); |
There was a problem hiding this comment.
[Suggestion] The optimistic transcript echo for pane /goal controls (store.appendLocalUserMessage(text) at ChatPane.tsx:760) is wired into this harness and reset here, but asserted only in the negative (the broken-connection test); no test pins the positive behavior.
Failure scenario (mutation-proven): deleting the echo line keeps ChatPane.test.tsx (103/103) and App.test.tsx (482/482) green. Shipped regression: pane /goal controls run but leave no user message in the transcript — the daemon does not echo control commands, which is why this call exists (mirrored from App.tsx) — so the typed command silently vanishes from the conversation record on the pane surface only.
Suggested fix: in 'builds the control request from the freshly fetched Goal', after the /goal set submit add expect(appendLocalUserMessage).toHaveBeenCalledWith('/goal set ship the other thing');.
中文说明
面板 /goal 控制的乐观转录回显(ChatPane.tsx:760 的 store.appendLocalUserMessage(text))接入了这个 harness 并在此重置,但只有反向断言(断连测试);没有测试钉住正向行为。
失败场景(突变测试证实):删除回显行后 ChatPane.test.tsx(103/103)与 App.test.tsx(482/482)仍全绿。带病上线的后果:面板 /goal 控制执行了但转录中没有留下用户消息——daemon 不会回显控制命令,这正是该调用存在的原因(与 App.tsx 镜像)——于是键入的命令只在面板这一侧从对话记录中静默消失。
建议修复:在 'builds the control request from the freshly fetched Goal' 中,/goal set 提交后追加 expect(appendLocalUserMessage).toHaveBeenCalledWith('/goal set ship the other thing');。
— qwen3.8-max via Qwen Code /review (v0.21.14)
| ); | ||
| }); | ||
|
|
||
| it('holds a composer prompt while Goal state is still hydrating', async () => { |
There was a problem hiding this comment.
[Suggestion] The four new goal-gating tests only exercise status: 'active' (blocked polarity); no unit test sets a paused/blocked/usage_limited/complete goal and asserts the gates OPEN — the takeover state the gates exist to reopen (no goalGate.test.ts exists).
Failure scenario: at the unit level, a regression wrongly blocking paused goals passes every gate test (manual run, retry, fast-model, /language). One correction to the filed claim, verified: the PR's own e2e (web-shell.goal.spec.ts) holds a prompt, pauses the goal, and polls until the prompt reaches the daemon — all gates share the one predicate, so the e2e does guard the paused polarity for the composer path; the gap is the unit-level open polarity, not an absolute mutation survival.
Suggested fix: add the open-polarity counterpart next to these tests — set a paused goal snapshot and assert run('do the thing', null) resolves and sendPrompt is called; ideally colocate a goalGate.test.ts pinning blocked/open across all five GoalStatus values.
中文说明
四个新的 Goal 门禁测试只演练了 status: 'active'(阻塞极性);没有任何单测设置 paused/blocked/usage_limited/complete 的 Goal 并断言门禁开放——而门禁存在的意义正是在这些状态下重新放开(也不存在 goalGate.test.ts)。
失败场景:在单测层面,错误阻塞 paused Goal 的回归能通过所有门禁测试(手动运行、重试、快速模型、/language)。对原始报告的一处更正(已核实):PR 自己的 e2e(web-shell.goal.spec.ts)会保留 prompt、暂停 Goal、轮询直到 prompt 到达 daemon——所有门禁共用同一个谓词,因此 e2e 确实为输入框路径守护了 paused 极性;缺口在单测层面的开放极性,而非绝对的突变存活。
建议修复:在这些测试旁补上开放极性的对照——设置 paused 的 Goal 快照,断言 run('do the thing', null) resolve 且 sendPrompt 被调用;最好再加一个 goalGate.test.ts,对全部五个 GoalStatus 值钉住阻塞/放开两种极性。
— qwen3.8-max via Qwen Code /review (v0.21.14)
Clears the three Critical findings from round 2 of the review on QwenLM#9393. R2-1 / R2-2 — both resume gates (`GoalStatusStrip.tsx`, `GoalsDialog.tsx`) keyed off `goal.limitKind === undefined` alone, while core's `isEvidenceLimited` (`goal-reducer.ts`) also treats a Goal as evidence-limited when `lastReason` is one of the two sentinel strings. Those sentinels shipped before the `limitKind` field did, so a Goal persisted in that window restores as `usage_limited` with no `limitKind` at all: the UI offered a Resume button that `reduceGoalControl` is guaranteed to answer with `GoalInvalidTransitionError`. Both gates now go through one `canResumeGoal` in `utils/goalGate.ts` — the module whose documented invariant is that every client Goal gate runs through a shared predicate so none can drift. It states the reducer's rule directly (complete/active refuse; `usage_limited` refuses when evidence-limited) rather than approximating it, and it keeps the evidence check scoped to `usage_limited` exactly as the reducer does, so a `paused` Goal carrying stale sentinel prose is not stranded without a Resume control. The Web Shell client bundles for the browser and cannot import `@qwen-code/qwen-code-core`, so the two sentinel strings are duplicated. A comment asking the next person to keep them in sync is not a mechanism: `goalGate.test.ts` reads `packages/core/src/goals/goal-protocol.ts` and fails if either literal moves, or if core grows a third sentinel branch. R2-3 — a session switch mid-drain silently destroyed queued prompts. The serial release chain stamps the whole batch `serverState: 'submitting'` up front and then releases it one link at a time (a prompt carrying media awaits its uploads, so that window is seconds, not microseconds). Two things went wrong inside it: the chain carried no owner token, so remaining links called `submitPrompt` against the old session, which throws before POSTing and was swallowed by the chain's own `.catch`; and the owner-change stash only saves rows matching `isLocallyHeldPrompt` (`serverState === undefined`), so every chain-marked row was excluded from it. Both typed prompts were gone with nothing restored to the editor. Each link now bails when the owner token it was built for is no longer current, and a new `unreleasedPromptIdsRef` tracks exactly the rows the chain stamped but never handed to `submitPendingPrompt`. The owner-change effect stashes those alongside the genuinely held rows, dropping the optimistic stamp on the way in so the next drain re-releases them in order. Deliberately narrower than the finding's suggested fix on one point: the row whose admission is actually in flight is NOT stashed. Restoring it would flip the behaviour pinned by 'ignores an old submit response after an S1 to S2 to S1 owner change' and 'fences an old submit before the replacement owner rerenders', which deliberately fence and drop an in-flight submission across an owner change — that POST may well have landed, so resurrecting the row risks a duplicate. The rows the chain never POSTed have no such ambiguity: nothing exists for them on the daemon, so stashing them cannot duplicate anything. Reversing the in-flight contract is a design call for the reviewer, not a drive-by. Also fixes R2-12 in passing, since the new tests need it: `MockGoal`'s snapshot `status` union omitted `'complete'` (a TS2322 against this file's own later usage) and had no `limitKind`. Verification: `npx vitest run client/hooks/ client/utils/ client/components/GoalStatusStrip.test.tsx client/components/dialogs/GoalsDialog.test.tsx client/components/ChatPane.test.tsx` in `packages/web-shell` — 1011 passed (994 on the stashed tree, so all 17 new tests pass), with an identical 17 pre-existing failures in the untouched `hooks/useMessages.test.ts` on both trees. eslint clean on all eight touched files. Every fix is mutation-verified. Dropping the sentinel fallback, widening it to any `lastReason`, or unscoping it from `usage_limited` each turns a distinct test red at both the unit and the component level; removing the chain owner guard, the unreleased-row half of the stash filter, or the stamp reset each turns the new mid-drain test red. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed.
2 Suggestion-level finding(s) this review confirmed are already reported on this PR and are not repeated:
- R2-8 benign-race error reporting in the ChatPane typed-/goal catch — already reported (comment 3816614512)
- R2-28 hardcoded English reportError fallback in both composers' /goal catches — already reported (comment 3816614622)
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.
Deferred under the convergence posture (round 3, not a blocker) — recorded, not requested in this round:
packages/web-shell/client/components/ChatPane.test.tsx:532 — [review] it.each transcript oracle targets the never-invoked dispatch mock instead of appendLocalUserMessagepackages/web-shell/client/components/ChatPane.test.tsx:1056 — [review] stale-edit reject branch does not pin the refresh-on-control-failure getGoal callpackages/web-shell/client/utils/goalCondition.ts:35 — [review] parseWebShellGoalCommand grammar mirror has no drift guardpackages/webui/src/daemon/session/types.ts:514 — [review] orphaned clearGoal methods and legacy route left behind by the control-plane migrationpackages/webui/src/daemon/workspace/actions.ts:814 — [review] droppedCount folds legacy-row drops into the probe-failure count the goals.dropped banner describespackages/web-shell/client/App.test.tsx:20707 — [review] latch release on control failure is unpinned (mutation survives with 482/482 green)packages/web-shell/client/App.tsx:164 — [review] isGoalClearCommand/isGoalClearKeyword dead exports left behindpackages/webui/src/daemon/workspace/actions.ts:808 — [review] hasGoalSnapshot protocol duplicate has no drift guard (nor does mappers.ts getGoalState)
中文说明
仅完成部分审查,审查缺口已披露。
本轮确认的 2 条建议级发现已在 PR 上报告过,不再重复发布(列表见上方英文部分)。
未审查:build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。
未审查:反向审计——评审时间预算不足,未能开始第 3 轮。
收敛姿态下延后(第 3 轮,非阻断)——已记录,本轮不要求修改:共 8 条(原文未翻译,列表见上方英文部分)。
— qwen3.8-max via Qwen Code /review (v0.21.14)
| }; | ||
| // The first release stays synchronous, so a single held prompt reaches | ||
| // the daemon exactly as it did before. | ||
| release = release ? release.then(submit) : submit(); |
There was a problem hiding this comment.
[Critical] R3-2: The serial release chain preserves order only within the batch it drains; a prompt sent through enqueuePrompt while the chain is still in flight bypasses it entirely and reaches the daemon ahead of the older held prompts it was typed after.
Failure scenario: a Goal ends with two or more locally held prompts queued, the first carrying media; link 1's admission takes seconds, and links 2..N wait on it. Inside that window the user sends a new prompt — nothing consults the active chain (nor does the App-level wrapper, nor the direct submit path), so submitPendingPrompt POSTs at once and the daemon admits it before links 2..N ever fire: the newly typed prompt executes before the follow-ups typed earlier, and if link 1's upload is still running it can even overtake prompt 1 and start the turn itself — defeating the ordering guarantee the chain's own comment names.
Witness (verification): probe on the unmodified PR — final admission order ["first with media","typed during drain","second plain"], the prompt typed during the drain overtaking "second plain" (its POST fired immediately); with a chain-tail ref serializing new sends: ["first with media","second plain","typed during drain"]. Fix reverted after the probe.
Suggested fix: serialize new sends behind an active drain — keep the chain's tail promise in a ref keyed by owner and append new submissions to it — or stamp the new prompt held while a chain for the current owner is alive.
中文说明
[Critical] R3-2:串行释放链只在它正在释放的批次内部保序;链仍在飞行时通过 enqueuePrompt 发出的新 prompt 会完全绕过链,比之前排队的旧 prompt 更早到达 daemon。
失败场景:Goal 结束时本地保留了 2 条以上排队的 prompt(第一条带媒体文件);link 1 的准入需要数秒,link 2..N 依次等待。在这个窗口内用户发送新 prompt——没有任何逻辑检查当前活跃的链(App 层封装和直接提交路径也没有),于是 submitPendingPrompt 立即 POST,daemon 在 link 2..N 触发之前就接收了它:新输入的 prompt 先于更早输入的内容执行;如果 link 1 的上传还在进行,新 prompt 甚至可能超过 prompt 1 直接开启 turn——与链自身注释声明的保序目标相悖。
证据(验证):在未修改的 PR 上探针——最终准入顺序 ["first with media","typed during drain","second plain"],drain 期间输入的 prompt 超过了更早输入的 "second plain"(其 POST 立即发出);用按 owner 键控的链尾 promise 串行化新发送后:["first with media","second plain","typed during drain"]。探针后已还原。
建议修复:把新发送串行化到活跃的 drain 之后——用按 owner 键控的 ref 保存链尾 promise 并把新提交追加到它后面;或者在当前 owner 的链存活期间把新 prompt 标记为保留。
— qwen3.8-max via Qwen Code /review (v0.21.14)
| goalTurnActive?: boolean; | ||
| /** Terminal error from the prior turn, cleared when the next turn starts. */ |
There was a problem hiding this comment.
[Critical] R3-16: The new child-driven Goal turn is invisible to the bridge's session-retention machinery: entryIsAutoCloseCandidate/entryHasLocalWork never consult goalTurnActive, and the child's collectActiveWorkHolds() reports no Goal hold — so a detached session mid-Goal is treated as idle and destroyed by the daemon's own reaping. This diff OR-ed goalTurnActive into every busy summary and the mid-turn enqueue gate precisely so consumers see Goal turns as busy; only the summaries got the flag — retention did not.
Failure scenario: the user starts a Goal and closes the tab (the canonical Goals use case). unregisterClient fires maybeCloseIdleSession('last_client_detached'); pendingPromptCount is 0 for the whole Goal turn (the very property this feature's comment celebrates); closeStoredSession(onlyIfUnheld) waits ≤ the drain budget for the running turn, sees zero holds, and closes — or closes instantly between goal turns. The session is torn down while the Goal stays active with nothing advancing it, and the goal drops from GET /goals with no feedback.
Witness (verification): probe in the bridge.test.ts harness — after _qwencode/start_turn {source:'goal'}, hasActivePrompt during goal turn = true yet sessionCount after detach mid goal turn = 0 (expected 1). Flip: adding goalTurnActive to entryHasLocalWork → sessionCount = 1. Fix reverted.
Suggested fix: treat entry.goalTurnActive === true as local work in entryIsAutoCloseCandidate/entryHasLocalWork/entryHasActiveWork, and report the active Goal as a child-side hold in collectActiveWorkHolds() so confirmChildUnheld refuses until the Goal ends or is cleared.
中文说明
[Critical] R3-16:新的子进程驱动 Goal turn 对 bridge 的会话保留机制不可见:entryIsAutoCloseCandidate/entryHasLocalWork 从不检查 goalTurnActive,子进程的 collectActiveWorkHolds() 也不报告 Goal hold——因此 Goal 运行中分离(detach)的会话会被当作空闲并被 daemon 自身的回收机制销毁。本 diff 把 goalTurnActive OR 进了所有 busy 汇总和中途入队门,就是为了让消费者把 Goal turn 视为忙碌;但只有汇总拿到了这个标志——保留机制没有。
失败场景:用户启动 Goal 后关闭标签页(Goals 的典型用法)。unregisterClient 触发 maybeCloseIdleSession('last_client_detached');整个 Goal turn 期间 pendingPromptCount 为 0(正是该特性注释所强调的属性);closeStoredSession(onlyIfUnheld) 等待运行中的 turn 不超过 drain 预算,看到零 hold 后关闭——或在两个 goal turn 之间立即关闭。Goal 仍为 active 但没有任何东西推进它,会话却被拆除,该 goal 从 GET /goals 中消失且无任何反馈。
证据(验证):bridge.test.ts 测试具架中探针——_qwencode/start_turn {source:'goal'} 之后,hasActivePrompt during goal turn = true 但 sessionCount after detach mid goal turn = 0(期望 1)。修复翻转:把 goalTurnActive 加入 entryHasLocalWork → sessionCount = 1。探针后已还原。
建议修复:在 entryIsAutoCloseCandidate/entryHasLocalWork/entryHasActiveWork 中把 entry.goalTurnActive === true 视为本地工作,并在 collectActiveWorkHolds() 中把活跃 Goal 报告为子进程侧 hold,使 confirmChildUnheld 在 Goal 结束或被清除前拒绝关闭。
— qwen3.8-max via Qwen Code /review (v0.21.14)
| source: 'goal', | ||
| promptId: this.config.getSessionId() + '########' + String(this.turn), |
There was a problem hiding this comment.
[Critical] R3-17: #emitGoalEndTurn re-derives the promptId from the shared mutable this.turn at emission time. Every Goal turn that ends before #executePromptInner increments this.turn — a queued turn aborted before it starts, a closing/write-barrier rejection, or a successor user prompt incrementing first under preemption — publishes turn_complete with a promptId that was never issued for this turn: the previous turn's id, ########0, or the NEXT turn's id.
Failure scenario: a queued Goal turn aborted before it starts (or rejected by the write barrier) emits end_turn carrying the previous turn's id; SSE/SDK consumers correlating by promptId settle a turn that already settled. Under the successor-preemption interleaving the id is the next turn's, so web-shell useQueuedPrompts treats that turn_complete as the new turn's settle and releases held queued prompts mid-turn. The existing tests pin only the success-path regex.
Witness (verification): probe in Session.test.ts — turn A completes, turn B is rejected by the write barrier before its body: end_turn promptIds = ["test-session-id########1","test-session-id########1"] — turn B published turn A's already-settled id. Flip (omit the id on never-started turns — the bridge tolerates a missing id and still settles the queue): ["test-session-id########1",null], and the PR's own success-path test stays green. Fix reverted.
Suggested fix: capture the turn's promptId where the turn number is authoritative and pass it to #emitGoalEndTurn, or emit no promptId on the never-started path.
中文说明
[Critical] R3-17:#emitGoalEndTurn 在发送时从共享可变的 this.turn 重新推导 promptId。任何在 #executePromptInner 递增 this.turn 之前结束的 Goal turn——启动前被中止的排队 turn、closing/写入屏障拒绝、或抢占场景下后续用户 prompt 先递增——都会发布一个从未为该 turn 签发的 promptId 的 turn_complete:上一个 turn 的 id、########0、或下一个 turn 的 id。
失败场景:启动前被中止(或被写入屏障拒绝)的排队 Goal turn 发出的 end_turn 携带上一个 turn 的 id;按 promptId 关联的 SSE/SDK 消费者会把一个已经结算的 turn 再次结算。在后续 prompt 抢占的交错下,id 是下一个 turn 的,于是 web-shell useQueuedPrompts 把该 turn_complete 当作新 turn 的结算,在 turn 进行中释放保留的排队 prompt。现有测试只钉住了成功路径的正则。
证据(验证):Session.test.ts 探针——turn A 完成,turn B 在主体运行前被写入屏障拒绝:end_turn promptIds = ["test-session-id########1","test-session-id########1"]——turn B 发布了 turn A 已结算的 id。修复翻转(对从未启动的 turn 不发送 id——bridge 能容忍缺失的 id 且仍会结算队列):["test-session-id########1",null],PR 自己的成功路径测试仍为绿色。探针后已还原。
建议修复:在 turn 编号权威处捕获该 turn 的 promptId 并传给 #emitGoalEndTurn;或对从未启动的路径不发送 promptId。
— qwen3.8-max via Qwen Code /review (v0.21.14)
| expect(next.goalState?.goal?.limitKind).toBeUndefined(); | ||
| }); |
There was a problem hiding this comment.
[Suggestion] R2-32 — still standing at aa1d913 (round-3 re-check): "drops an unknown limitKind rather than passing it through" only asserts limitKind is undefined — which passes equally if getGoalState rejected the WHOLE snapshot or nulled the goal. The "drops (keeps the goal)" half of its own title is unpinned, unlike its sibling which pins goal survival via toMatchObject.
Failure scenario (mutation-proven in round 2): a hardened parser mutation returning undefined (dropping the entire snapshot) for an unknown limitKind keeps this test green. Shipped regression: a newer daemon emitting a new limitKind value causes the entire goal snapshot to be dropped — the goal vanishes from the web-shell UI (strip, GoalsDialog, prompt gates) instead of rendering without the field.
Suggested fix:
| expect(next.goalState?.goal?.limitKind).toBeUndefined(); | |
| }); | |
| expect(next.goalState?.goal).toMatchObject({ goalId: 'goal-1', status: 'paused' }); | |
| expect(next.goalState?.goal?.limitKind).toBeUndefined(); | |
| }); |
中文说明
[Suggestion] R2-32 — 在 aa1d913 仍然存在(第 3 轮复查):"drops an unknown limitKind rather than passing it through" 只断言 limitKind 为 undefined——如果 getGoalState 拒绝了整个快照或把 goal 置空,该断言同样通过。标题中"丢弃(保留 goal)"的一半没有被钉住,不像其姊妹测试用 toMatchObject 钉住 goal 存活。
失败场景(第 2 轮突变证实):强化解析器的突变对未知 limitKind 返回 undefined(丢弃整个快照),该测试仍为绿色。出货回归:更新的 daemon 发出新的 limitKind 值会导致整个 goal 快照被丢弃——goal 从 web-shell UI(状态条、GoalsDialog、prompt 门禁)消失,而不是不带该字段继续渲染。
建议修复:见 suggestion 代码块(在现有断言前加 expect(next.goalState?.goal).toMatchObject({ goalId: 'goal-1', status: 'paused' });)。
— qwen3.8-max via Qwen Code /review (v0.21.14)
| setConnection((current) => { | ||
| if (current.sessionId !== session.sessionId) return current; | ||
| const goalState = selectGoalStateFromRead( |
There was a problem hiding this comment.
[Suggestion] R2-33 — still standing at aa1d913 (round-3 re-check): the session-scope guards in getGoal/controlGoal — the only protection against a delayed Goal response writing another session's snapshot into the connection state after a switch — are not pinned by any test (all five new Goal tests keep the harness connection on session-a).
Failure scenario (mutation-proven in round 2): deleting both guards keeps actions.test.ts + DaemonSessionProvider.test.tsx green (311/311); mirrored probes show goalState.goal.goalId flip 'goal-b' → 'goal-a' under mutation — cross-session corruption of the strip and prompt-hold gating. selectGoalStateFromRead's stamp only intercepts bare-null responses, so a non-null different-goalId frame falls through to selectGoalState and is accepted as a replacement.
Suggested fix: tests mirroring the stale-read test — issue getGoal() against a deferred session.goal, replaceConnection to session-b with a goal-B snapshot, resolve the deferred read with session A's goal snapshot, assert goalState is still the goal-B reference; repeat the shape for controlGoal.
中文说明
[Suggestion] R2-33 — 在 aa1d913 仍然存在(第 3 轮复查):getGoal/controlGoal 中的会话范围守卫——防止切换后迟到的 Goal 响应把另一个会话的快照写入 connection 状态的唯一保护——没有任何测试钉住(五个新 Goal 测试都让具架连接保持在 session-a)。
失败场景(第 2 轮突变证实):删除两个守卫,actions.test.ts + DaemonSessionProvider.test.tsx 仍为绿色(311/311);镜像探针显示突变下 goalState.goal.goalId 从 'goal-b' → 'goal-a' 翻转——状态条和 prompt 保留门禁的跨会话污染。selectGoalStateFromRead 的戳记只拦截 bare-null 响应,因此非空的不同 goalId 帧会落入 selectGoalState 并被当作替换接受。
建议修复:镜像过期读取测试——对挂起的 session.goal 发起 getGoal(),replaceConnection 到带 goal-B 快照的 session-b,用会话 A 的 goal 快照解析挂起读取,断言 goalState 仍是 goal-B 引用;对 controlGoal 重复同样形状。
— qwen3.8-max via Qwen Code /review (v0.21.14)
| transcriptDispatch.mockClear(); | ||
| appendLocalUserMessage.mockClear(); | ||
| catalogController.invalidateWorkspace.mockClear(); |
There was a problem hiding this comment.
[Suggestion] R2-34 — still standing at aa1d913 (round-3 re-check): the optimistic transcript echo for pane /goal controls (store.appendLocalUserMessage(text) at ChatPane.tsx:760) is wired into this harness and reset here, but asserted only in the negative — no test pins the positive behavior.
Failure scenario (mutation-proven in round 2): deleting the echo line keeps ChatPane.test.tsx (103/103) and App.test.tsx (482/482) green. Shipped regression: pane /goal controls run but leave no user message in the transcript — the daemon does not echo control commands, which is why this call exists — so the typed command silently vanishes from the conversation record on the pane surface only.
Suggested fix: in "builds the control request from the freshly fetched Goal", after the /goal set submit add expect(appendLocalUserMessage).toHaveBeenCalledWith('/goal set ship the other thing');.
中文说明
[Suggestion] R2-34 — 在 aa1d913 仍然存在(第 3 轮复查):面板 /goal 控制的乐观 transcript 回显(ChatPane.tsx:760 的 store.appendLocalUserMessage(text))在这个具架中接线并在此处重置,但只有负向断言——没有测试钉住正向行为。
失败场景(第 2 轮突变证实):删除该回显行,ChatPane.test.tsx(103/103)和 App.test.tsx(482/482)仍为绿色。出货回归:面板 /goal 控制执行但 transcript 中不留用户消息——daemon 不会回显控制命令,这正是该调用存在的原因——于是输入的命令仅在面板表面从对话记录中静默消失。
建议修复:在 "builds the control request from the freshly fetched Goal" 中,/goal set 提交后加 expect(appendLocalUserMessage).toHaveBeenCalledWith('/goal set ship the other thing');。
— qwen3.8-max via Qwen Code /review (v0.21.14)
| it('holds a composer prompt while Goal state is still hydrating', async () => { | ||
| // The session load clears `loadingTranscript` before its `goal()` fetch |
There was a problem hiding this comment.
[Suggestion] R2-35 — still standing at aa1d913 (round-3 re-check; re-discovered this round): the four new goal-gating tests only exercise status: 'active' (blocked polarity); no unit test sets a paused/blocked/usage_limited goal and asserts the gates OPEN — the takeover state the gates exist to reopen. The e2e guards the paused polarity for the composer path; the gap is unit-level.
Failure scenario: a maintainer widening the predicate (goal?.status === 'active' → goal !== null, a plausible reading of "a Goal owns the session") keeps every unit test green; while a Goal is paused — the point of Pause is handing the session back — composer submits, the queued-prompt hold, manual/scheduled runs, retry and /language all silently block.
Suggested fix: add the open-polarity pin — set a paused goal snapshot and assert run('do the thing', null) resolves and sendPrompt is called; ideally goalGate.test.ts asserting blocked/open across all five GoalStatus values.
中文说明
[Suggestion] R2-35 — 在 aa1d913 仍然存在(第 3 轮复查;本轮重新发现):四个新的 Goal 门禁测试只演练 status: 'active'(阻塞极性);没有单测设置 paused/blocked/usage_limited 的 Goal 并断言门禁开放——而门禁存在的意义正是在这些状态下重新放开。e2e 为输入框路径守护了 paused 极性;缺口在单测层面。
失败场景:维护者放宽谓词(goal?.status === 'active' → goal !== null,"Goal 拥有会话"的合理读法)时所有单测仍为绿色;Goal 暂停时——暂停的意义正是把会话交还用户——输入框提交、排队保留、手动/定时运行、重试和 /language 都会被静默阻塞。
建议修复:补上开放极性钉桩——设置 paused 的 Goal 快照并断言 run('do the thing', null) resolve 且 sendPrompt 被调用;最好再加 goalGate.test.ts 对全部五个 GoalStatus 值钉住阻塞/开放两种极性。
— qwen3.8-max via Qwen Code /review (v0.21.14)
| expect(mockSessionActions.controlGoal).toHaveBeenLastCalledWith({ | ||
| action: 'create', | ||
| objective: 'all tests pass', | ||
| }); |
There was a problem hiding this comment.
[Suggestion] R2-21 — still standing at aa1d913 (round-3 re-check): this retry assertion cannot distinguish the retry's create dispatch from the original identical-arg dispatch — toHaveBeenLastCalledWith matches the first call too, and nothing asserts controlGoal was called a second time. The same hole exists in 'forgets a rejected goal session after leaving the Goals page', whose retry never asserts a dispatch at all.
Failure scenario (mutation-proven in round 2): skipping the Goal create dispatch in the reuse branch leaves the first call as the last matching call — every assertion still passes. Shipped regression: the retry click creates no goal, no error surfaces, and the Goals page silently keeps or leaves the view.
Suggested fix:
| expect(mockSessionActions.controlGoal).toHaveBeenLastCalledWith({ | |
| action: 'create', | |
| objective: 'all tests pass', | |
| }); | |
| expect(mockSessionActions.clearSession).toHaveBeenCalledTimes(1); | |
| expect(mockSessionActions.controlGoal).toHaveBeenCalledTimes(2); | |
| expect(mockSessionActions.controlGoal).toHaveBeenLastCalledWith({ | |
| action: 'create', | |
| objective: 'all tests pass', | |
| }); |
and the same count-2 assertion after the retry in the leave-page test.
中文说明
[Suggestion] R2-21 — 在 aa1d913 仍然存在(第 3 轮复查):这个重试断言无法区分重试的 create 派发和参数完全相同的原始派发——toHaveBeenLastCalledWith 也会匹配第一次调用,且没有任何断言表明 controlGoal 被调用了第二次。同样的漏洞存在于 'forgets a rejected goal session after leaving the Goals page',其重试完全没有断言派发。
失败场景(第 2 轮突变证实):在复用分支中跳过 Goal create 派发,第一次调用仍是最后一次匹配调用——所有断言仍通过。出货回归:重试点击没有创建 goal,没有错误浮现,Goals 页面静默保持或离开视图。
建议修复:见 suggestion 代码块(加 toHaveBeenCalledTimes(2)),并在 leave-page 测试的重试后加同样的 count-2 断言。
— qwen3.8-max via Qwen Code /review (v0.21.14)
…other Four round-3 Criticals, each reproduced first and then mutation-verified. R3-4 / R3-19 — `goalControlBusy` was a shared latch with no owner. A create in an allocated session released it unconditionally in `finally`, and the composer `/goal` path never consulted it at all, so two controls could read the same snapshot, stamp the same expectedGoalId/expectedRevision, and let the daemon reject the loser with a 409. Ported ChatPane's twin pattern — `goalControlOpSeqRef`/`goalControlOwnerRef`, released only by the operation that still owns it — to both `controlCurrentGoal` and `createGoalForAllocatedSession`, and made `handleGoalSlashCommand` refuse (keeping the composer text) while a control is in flight. R3-24 — a chain link deleted its id from `unreleasedPromptIdsRef` before the owner check. The owner token is replaced in the render body while the stash is a passive effect flushed after commit, so a link firing in that window left a row that was neither locally held nor unreleased, and the stash dropped it: the typed prompt vanished with no POST, no abort, no toast. Delete moved below the owner check, as the review suggested. R3-20 — a queue clear mid-drain aborts only the in-flight link's controller; the chain's pending links have no controller yet, so they went on to POST prompts the user had explicitly cleared. The link now bails when its row has left the queue. Verification: 775 passing across App / useQueuedPrompts.dom / useQueuedPrompts.midTurnReconcile / ChatPane / GoalStatusStrip / GoalsDialog; full web-shell 3906 passing with the same 18 failures the branch has without this change (useMessages background-agent reconciliation, build-artifact — confirmed by stash-and-rerun). tsc unchanged at 66 pre-existing errors from unbuilt workspace deps. ESLint and Prettier clean on the touched files. Mutation-verified: each of the four fixes reverted in isolation turns exactly its own new test red.
|
@qwen-code /triage |
|
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: 28 passed · 0 failed · 28 total Flakiness gate: 中文 — 判定:✅ 通过 · 可合入(agent 判定)沙箱验证在隔离、无凭证的容器中执行了该 PR 的代码(与 base 构建 A/B 对照、无 mock harness 断言、定向门禁)。仅作为评审证据,不构成评审、批准或 CI 检查。 脚本断言:28 通过 · 0 失败 · 28 总计 抖动门: Verification reportPR #9393 verification —
|
| # | cell | oracle | head | base |
|---|---|---|---|---|
| 1 | control-plane create | POST /session/:id/goal {action:create} |
200, snapshot.goal.status=active, revision 1, objective intact |
404 (route absent) |
| 2 | model-free creation | chat-completion count across the create call | 0 requests | n/a — Goal had to travel POST /session/:id/prompt with /goal … |
| 3 | Goal turn runs | fake model sees Continue working on the active Goal |
yes, +31 ms after create | yes (positive control: base goal runtime fires turns too) |
| 4 | wire settles | SSE turn_complete with ########-scoped promptId |
yes — f24613ac…########1, stopReason end_turn |
0 goal-scoped frames in a 10 s window after 222 model requests (201 goal turns) (only the /goal prompt's own terminal arrived) |
| 5 | read surface | GET /session/:id/goal, GET /goals |
200 live snapshot; listed with droppedCount:0 |
404 |
| 6 | CAS | pause with superseded expectedRevision |
409 goal_conflict |
n/a |
| 7 | lifecycle controls | edit → pause → resume → clear | 200 each, statuses active→paused→active→null, edit bumps revision |
only POST …/goal/clear existed (200) |
| 8 | insert gate | POST …/mid-turn-message idle vs during a Goal turn |
idle: 200 {accepted:false}; during Goal turn: 200 {accepted:true} |
n/a |
Head arm 17/17, base arm 8/8 (every base cell is an assertion that the base fails as predicted — encoded as expected-failures, so all 28 count as passes). Captured as printed: 01-ab-head-control-plane.png (137 rows, full assertion ledger) and 02-ab-base-no-control-no-terminal.png (tail kept; the dropped top rows are fake-model churn, full logs in logs/base-arm-run.log).
Reviewer Test Plan walk (daemon-level half): step 1 ✅ (session + Goal via control, zero chat prompts); step 2 ✅ at the wire (edit/pause/resume/clear + CAS; the no-confirmation-dialog half is browser UI, not covered); step 3 ✅ at the wire (idle insert refused, explicit insert admitted mid-Goal-turn; the local-FIFO half is pinned by useQueuedPrompts* tests, 342/342 in the gate below); step 4 client-side drain not exercised end-to-end (see Not covered); steps 5–6 (row width, TUI) not covered.
Mutation matrix (vacuity of the pinned fixes)
One hunk reverted per row in a scratch HEAD worktree; the test the commit claims pins it re-run. Positive control: all three suites green unmutated in this round's gates. Captured in 03-mutation-matrix-three-hunks-killed.png.
| mutant | reverted hunk | pinned test | outcome |
|---|---|---|---|
| M1 | bridge accepts source:'goal' on _qwencode/end_turn |
bridgeClient.test.ts 'publishes a real turn_complete for a goal-turn end signal' |
KILLED — 2 failed in 4 s; expected "spy" to be called with {type:'turn_complete'…} / Number of calls: 0 |
| M2 | child emits _qwencode/end_turn in the goal-turn finally |
Session.test.ts 'notifies the bridge that the Goal turn ended' |
KILLED — 1 failed in 14 s; spy never called with _qwencode/end_turn |
| M3 | webui mapper rejects unknown limitKind |
mappers.test.ts 'drops an unknown limitKind rather than passing it through' |
KILLED — 1 failed in 2 s; expected 'not-a-kind' to be undefined |
3/3 killed, 0 survivors. Each failure is the intended behavioural assertion (expected-vs-actual), not a compile/import break.
Targeted gates (head)
| workspace | files | result |
|---|---|---|
| core | goals/goal-reducer.test.ts, goal-runtime.test.ts |
171/171 |
| acp-bridge | bridge, bridgeClient, transcript-replay |
905/905 |
| cli serve | routes/goals, server/telemetry*, server/error-response |
98/98 |
| cli acp | session/Session, session/history-replay-page |
686/686 |
| cli acp/serve | acpAgent, server |
1498 passed, 1 skipped |
| webui | session mappers/actions/DaemonSessionProvider, workspace goals.actions |
359/359 |
| sdk-typescript | DaemonClient, DaemonSessionClient |
413/413 |
| web-shell | 11 goal-surface files (gates, hooks, dialogs, strips) | 342/342 |
| web-shell | ChatPane, App, WebShellTranscript |
603/603 |
Total 5,075 passed / 1 skipped / 0 failed across the changed surface. The telemetry guard change (routes 59→61, split 59/2) is included and green.
Findings
No blocking findings. Non-blocking observations:
- Goal-turn loop has no pacing while the model answers with plain text. With the fake model never completing the Goal, the child fired goal turns back-to-back: 222 chat-completion requests (201 of them goal turns) in ~100 s on base, same shape on head until paused. This is pre-existing (identical on the base arm), not introduced by the PR — but the PR is also what first makes the loop stoppable from outside the model (pause/clear verified in cells 6–7). Worth the author's awareness, not a merge condition.
- Snapshot base drift. The metadata snapshot's
baseRefOid(43aea245, committed 2026-08-20 06:57) is newer than the merge ref's base (313f1911). The A/B ran against313f1911per the merge-ref contract; a trial merge into43aea245was not possible at depth 2 (git merge-treereports unrelated histories). If main has moved the goal surface since, a rebase-and-reverify is the maintainer's call.
Not covered
- Browser-level WebShell behavior: queued-message FIFO drain after pause/clear, Goal-row width parity, clear-without-confirmation, Insert affordance visibility. No browser in this container; the client logic is pinned by the web-shell gates above (945 tests in 14 files) and the wire contract by the A/B. The PR's own Playwright goal spec (
web-shell.goal.spec.ts) was not executed here. - TUI Goal flow (plan step 6) — unchanged surface per the PR; not exercised.
- Per-commit attribution — depth-2 checkout; only the aggregate
HEAD^1..HEADdiff was verified (27 commits in the snapshot vs 1 locally reachable). - Trial merge into the drifted snapshot base — impossible at shallow depth (see Finding 2).
- Windows/macOS paths; desktop shell and token-budget UI (declared out of scope by the PR).
Methodology
Environment: node:22-bookworm CI container, merge-ref checkout at depth 2 (HEAD=merge, HEAD^1=base, HEAD^2=verified head). Harness goal-ab.mjs boots the compiled daemon per arm with --token, an isolated QWEN_HOME, and OPENAI_BASE_URL pointed at an in-process fake OpenAI server that fingerprints goal-continuation prompts; assertions are scripted comparisons written to results-<arm>.json. The base arm used a git worktree at HEAD^1 rebuilt via npm run build with internal @qwen-code/* links re-pointed into the worktree (realpath-asserted) and nested node_modules mirrored from the unchanged lockfile. Mutation rows ran in a second scratch worktree with per-row git checkout restore. Raw logs: logs/ (per-arm daemon stderr/stdout, fake-model request log, base build log). Captures rendered with scripts/verify-capture.mjs. Scratch worktrees removed after capture.
Flakiness gate log
e2e suite, out of gate scope: packages/web-shell/client/e2e/web-shell.goal.spec.ts
rounds=5 files=33 skipped=1
file packages/acp-bridge/src/bridge.test.ts: (cd packages/acp-bridge) npx --no-install vitest run ./src/bridge.test.ts
file packages/acp-bridge/src/bridgeClient.test.ts: (cd packages/acp-bridge) npx --no-install vitest run ./src/bridgeClient.test.ts
file packages/acp-bridge/src/transcript-replay.test.ts: (cd packages/acp-bridge) npx --no-install vitest run ./src/transcript-replay.test.ts
file packages/cli/src/acp-integration/acpAgent.test.ts: (cd packages/cli) npx --no-install vitest run ./src/acp-integration/acpAgent.test.ts
file packages/cli/src/acp-integration/session/Session.test.ts: (cd packages/cli) npx --no-install vitest run ./src/acp-integration/session/Session.test.ts
file packages/cli/src/acp-integration/session/history-replay-page.test.ts: (cd packages/cli) npx --no-install vitest run ./src/acp-integration/session/history-replay-page.test.ts
file packages/cli/src/serve/routes/goals.test.ts: (cd packages/cli) npx --no-install vitest run ./src/serve/routes/goals.test.ts
file packages/cli/src/serve/server.test.ts: (cd packages/cli) npx --no-install vitest run ./src/serve/server.test.ts
file packages/cli/src/serve/server/error-response.test.ts: (cd packages/cli) npx --no-install vitest run ./src/serve/server/error-response.test.ts
file packages/cli/src/serve/server/telemetry-catalog.test.ts: (cd packages/cli) npx --no-install vitest run ./src/serve/server/telemetry-catalog.test.ts
file packages/cli/src/serve/server/telemetry.test.ts: (cd packages/cli) npx --no-install vitest run ./src/serve/server/telemetry.test.ts
file packages/core/src/goals/goal-reducer.test.ts: (cd packages/core) npx --no-install vitest run ./src/goals/goal-reducer.test.ts
file packages/core/src/goals/goal-runtime.test.ts: (cd packages/core) npx --no-install vitest run ./src/goals/goal-runtime.test.ts
file packages/sdk-typescript/test/unit/DaemonClient.test.ts: (cd packages/sdk-typescript) npx --no-install vitest run ./test/unit/DaemonClient.test.ts
file packages/sdk-typescript/test/unit/DaemonSessionClient.test.ts: (cd packages/sdk-typescript) npx --no-install vitest run ./test/unit/DaemonSessionClient.test.ts
file packages/web-shell/client/App.test.tsx: (cd packages/web-shell) npx --no-install vitest run ./client/App.test.tsx
file packages/web-shell/client/components/ChatPane.test.tsx: (cd packages/web-shell) npx --no-install vitest run ./client/components/ChatPane.test.tsx
file packages/web-shell/client/components/GoalStatusStrip.test.tsx: (cd packages/web-shell) npx --no-install vitest run ./client/components/GoalStatusStrip.test.tsx
file packages/web-shell/client/components/QueuedPromptDisplay.test.tsx: (cd packages/web-shell) npx --no-install vitest run ./client/components/QueuedPromptDisplay.test.tsx
file packages/web-shell/client/components/WebShellTranscript.dom.test.tsx: (cd packages/web-shell) npx --no-install vitest run ./client/components/WebShellTranscript.dom.test.tsx
file packages/web-shell/client/components/dialogs/GoalEditDialog.test.tsx: (cd packages/web-shell) npx --no-install vitest run ./client/components/dialogs/GoalEditDialog.test.tsx
file packages/web-shell/client/components/dialogs/GoalsDialog.test.tsx: (cd packages/web-shell) npx --no-install vitest run ./client/components/dialogs/GoalsDialog.test.tsx
file packages/web-shell/client/components/dialogs/ScheduledTasksDialog.test.tsx: (cd packages/web-shell) npx --no-install vitest run ./client/components/dialogs/ScheduledTasksDialog.test.tsx
file packages/web-shell/client/components/messages/SystemMessage.test.tsx: (cd packages/web-shell) npx --no-install vitest run ./client/components/messages/SystemMessage.test.tsx
file packages/web-shell/client/hooks/useQueuedPrompts.dom.test.tsx: (cd packages/web-shell) npx --no-install vitest run ./client/hooks/useQueuedPrompts.dom.test.tsx
file packages/web-shell/client/hooks/useQueuedPrompts.midTurnReconcile.test.tsx: (cd packages/web-shell) npx --no-install vitest run ./client/hooks/useQueuedPrompts.midTurnReconcile.test.tsx
file packages/web-shell/client/midTurnDedup.test.ts: (cd packages/web-shell) npx --no-install vitest run ./client/midTurnDedup.test.ts
file packages/web-shell/client/utils/goalCondition.test.ts: (cd packages/web-shell) npx --no-install vitest run ./client/utils/goalCondition.test.ts
file packages/web-shell/client/utils/goalGate.test.ts: (cd packages/web-shell) npx --no-install vitest run ./client/utils/goalGate.test.ts
file packages/webui/src/daemon/session/DaemonSessionProvider.test.tsx: (cd packages/webui) npx --no-install vitest run ./src/daemon/session/DaemonSessionProvider.test.tsx
file packages/webui/src/daemon/session/actions.test.ts: (cd packages/webui) npx --no-install vitest run ./src/daemon/session/actions.test.ts
file packages/webui/src/daemon/session/mappers.test.ts: (cd packages/webui) npx --no-install vitest run ./src/daemon/session/mappers.test.ts
file packages/webui/src/daemon/workspace/goals.actions.test.ts: (cd packages/webui) npx --no-install vitest run ./src/daemon/workspace/goals.actions.test.ts
per-file results (P=pass F=fail I=infra-exit, one letter per run):
packages/acp-bridge/src/bridge.test.ts: PPP
packages/acp-bridge/src/bridgeClient.test.ts: PPP
packages/acp-bridge/src/transcript-replay.test.ts: PPP
packages/cli/src/acp-integration/acpAgent.test.ts: PPP
packages/cli/src/acp-integration/session/Session.test.ts: PPP
packages/cli/src/acp-integration/session/history-replay-page.test.ts: PPP
packages/cli/src/serve/routes/goals.test.ts: PPP
packages/cli/src/serve/server.test.ts: PP
packages/cli/src/serve/server/error-response.test.ts: PP
packages/cli/src/serve/server/telemetry-catalog.test.ts: PP
packages/cli/src/serve/server/telemetry.test.ts: PP
packages/core/src/goals/goal-reducer.test.ts: PP
packages/core/src/goals/goal-runtime.test.ts: PP
packages/sdk-typescript/test/unit/DaemonClient.test.ts: PP
packages/sdk-typescript/test/unit/DaemonSessionClient.test.ts: PP
packages/web-shell/client/App.test.tsx: PP
packages/web-shell/client/components/ChatPane.test.tsx: PP
packages/web-shell/client/components/GoalStatusStrip.test.tsx: PP
packages/web-shell/client/components/QueuedPromptDisplay.test.tsx: PP
packages/web-shell/client/components/WebShellTranscript.dom.test.tsx: PP
packages/web-shell/client/components/dialogs/GoalEditDialog.test.tsx: PP
packages/web-shell/client/components/dialogs/GoalsDialog.test.tsx: PP
packages/web-shell/client/components/dialogs/ScheduledTasksDialog.test.tsx: PP
packages/web-shell/client/components/messages/SystemMessage.test.tsx: PP
packages/web-shell/client/hooks/useQueuedPrompts.dom.test.tsx: PP
packages/web-shell/client/hooks/useQueuedPrompts.midTurnReconcile.test.tsx: PP
packages/web-shell/client/midTurnDedup.test.ts: PP
packages/web-shell/client/utils/goalCondition.test.ts: PP
packages/web-shell/client/utils/goalGate.test.ts: PP
packages/webui/src/daemon/session/DaemonSessionProvider.test.tsx: PP
packages/webui/src/daemon/session/actions.test.ts: PP
packages/webui/src/daemon/session/mappers.test.ts: PP
packages/webui/src/daemon/workspace/goals.actions.test.ts: PP
verdict: timeout
summary: only 2 of 5 rounds fit the 15-minute budget; the completed rounds agreed
--- per-invocation detail (full copy in the artifact) ---
round 1 · packages/acp-bridge/src/bridge.test.ts: P (exit 0)
round 1 · packages/acp-bridge/src/bridgeClient.test.ts: P (exit 0)
round 1 · packages/acp-bridge/src/transcript-replay.test.ts: P (exit 0)
round 1 · packages/cli/src/acp-integration/acpAgent.test.ts: P (exit 0)
round 1 · packages/cli/src/acp-integration/session/Session.test.ts: P (exit 0)
round 1 · packages/cli/src/acp-integration/session/history-replay-page.test.ts: P (exit 0)
round 1 · packages/cli/src/serve/routes/goals.test.ts: P (exit 0)
round 1 · packages/cli/src/serve/server.test.ts: P (exit 0)
round 1 · packages/cli/src/serve/server/error-response.test.ts: P (exit 0)
round 1 · packages/cli/src/serve/server/telemetry-catalog.test.ts: P (exit 0)
round 1 · packages/cli/src/serve/server/telemetry.test.ts: P (exit 0)
round 1 · packages/core/src/goals/goal-reducer.test.ts: P (exit 0)
round 1 · packages/core/src/goals/goal-runtime.test.ts: P (exit 0)
round 1 · packages/sdk-typescript/test/unit/DaemonClient.test.ts: P (exit 0)
round 1 · packages/sdk-typescript/test/unit/DaemonSessionClient.test.ts: P (exit 0)
round 1 · packages/web-shell/client/App.test.tsx: P (exit 0)
round 1 · packages/web-shell/client/components/ChatPane.test.tsx: P (exit 0)
round 1 · packages/web-shell/client/components/GoalStatusStrip.test.tsx: P (exit 0)
round 1 · packages/web-shell/client/components/QueuedPromptDisplay.test.tsx: P (exit 0)
round 1 · packages/web-shell/client/components/WebShellTranscript.dom.test.tsx: P (exit 0)
round 1 · packages/web-shell/client/components/dialogs/GoalEditDialog.test.tsx: P (exit 0)
round 1 · packages/web-shell/client/components/dialogs/GoalsDialog.test.tsx: P (exit 0)
round 1 · packages/web-shell/client/components/dialogs/ScheduledTasksDialog.test.tsx: P (exit 0)
round 1 · packages/web-shell/client/components/messages/SystemMessage.test.tsx: P (exit 0)
round 1 · packages/web-shell/client/hooks/useQueuedPrompts.dom.test.tsx: P (exit 0)
round 1 · packages/web-shell/client/hooks/useQueuedPrompts.midTurnReconcile.test.tsx: P (exit 0)
round 1 · packages/web-shell/client/midTurnDedup.test.ts: P (exit 0)
round 1 · packages/web-shell/client/utils/goalCondition.test.ts: P (exit 0)
round 1 · packages/web-shell/client/utils/goalGate.test.ts: P (exit 0)
round 1 · packages/webui/src/daemon/session/DaemonSessionProvider.test.tsx: P (exit 0)
round 1 · packages/webui/src/daemon/session/actions.test.ts: P (exit 0)
round 1 · packages/webui/src/daemon/session/mappers.test.ts: P (exit 0)
round 1 · packages/webui/src/daemon/workspace/goals.actions.test.ts: P (exit 0)
...truncated -- full content in the run artifacts.
Evidence images
Harness scripts and raw logs are in the workflow run artifacts (7-day retention).
— Qwen Code · sandboxed verification
|
Triage re-run completed without a new review.
The stage comments above were updated with the latest result. View workflow run. 上方各阶段评论已更新为最新结果。查看工作流运行。 |
Five conflicts, all in web-shell's composer surface, against upstream's mid-turn file attachments work (QwenLM#9570): - App.tsx / ChatPane.tsx: upstream added `onAttachmentPreview` to `QueuedPromptDisplay` while this branch wrapped that same element in the composer status stack alongside `GoalStatusStrip`. Kept the stack and carried the new prop (plus this branch's `canInsertMidTurn`/`onInsert`) onto it. - MessageItem.tsx / SystemMessage.tsx: upstream's hunk carried both `onAttachmentPreview` and `isLatest`. Only the first is new -- `isLatest` is context this branch deliberately removed along with its sole consumer (`GoalStatusMessage activateFooter`), so taking the hunk whole would have reintroduced an unused prop that no longer typechecks. - useQueuedPrompts.ts: both sides added helpers at the same offset (this branch's owner-key/stash helpers, upstream's `annotatedFiles`). Kept both.
R3-2: the serial release chain preserved order only inside the batch it drained. It exists because the prompt at its head may await media uploads for seconds; a prompt typed inside that window went straight through `submitPendingPrompt`, so the daemon admitted it ahead of the older held rows it was typed after -- and while link 1's upload was still running it could overtake link 1 itself and start the turn. The chain is now published on `releaseChainRef` (owner-pinned, retired once its newest tail settles), and `enqueuePrompt`'s ordinary path appends to that tail instead of POSTing past it. The waiting row is registered in `unreleasedPromptIdsRef`, so it is stamped-but-not-POSTed exactly like the chain's own undrained rows and the owner-change stash saves its text rather than losing it. The per-link guards (owner pinned at build time, row-still-present, hold/write-block revert) move into a shared `releaseChainedPrompt` so both the drain and the appended send carry identical semantics. A re-drain for a live owner now extends the existing chain instead of racing it. Test: `holds a prompt typed mid-drain behind the release chain` -- two held prompts with link 1's admission hung, a third typed mid-drain; asserts it does not POST while the chain is in flight and that final order is ['first with media', 'second plain', 'typed during drain']. Mutation-verified: disabling the append arm turns the test red with the reported symptom (the mid-drain prompt POSTs immediately, order inverted). Verification: `npx tsc --noEmit` clean; `npx vitest run` in packages/web-shell -> 190 files / 3986 tests passed.
|
@qwen-code /triage |
|
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: 33 passed · 0 failed · 33 total Flakiness gate: 中文 — 判定:✅ 通过 · 可合入(agent 判定)沙箱验证在隔离、无凭证的容器中执行了该 PR 的代码(与 base 构建 A/B 对照、无 mock harness 断言、定向门禁)。仅作为评审证据,不构成评审、批准或 CI 检查。 脚本断言:33 通过 · 0 失败 · 33 总计 抖动门: Verification reportPR #9393 verification (round 2) —
|
| # | Previous finding (round 1, head c52a223145, base 313f1911) |
Severity | Status at new head a39b824396 |
|---|---|---|---|
| 1 | Goal-turn loop has no pacing while the model answers with plain text (222 requests in ~100 s on base; same shape on head until paused) | Observation, non-blocking, pre-existing | Stands — re-measured. Base arm at the new base fired 222 chat-completions (201 goal continuations) in the ~100 s window again (02-ab-base-no-control-no-terminal.png). The head loop is now externally stoppable and that was exercised: pause (H7a) and clear (H7d) both landed; after clear, 0 goal-continuation requests in 4 s (H9). |
| 2 | Snapshot base drift — the metadata baseRefOid was newer than the merge-ref base; trial merge impossible at depth 2 |
Observation | Resolved for this round. The snapshot baseRefOid (4839935e55) now matches the merge-ref base (HEAD^1) exactly, so the A/B ran directly against it. A trial merge into any newer main remains impossible here (depth 2, no network) — listed under Not covered. |
Round-1 verdict (merge-ready, 28/28) is superseded by this round: the input closure changed (3 new commits), so all A/B cells, mutants, and gates were re-executed rather than diffed against the old report.
Central claim and A/B proof
Central claim (unchanged from round 1): the Goal lifecycle is routed through the canonical Goal v3 control plane — created and controlled via POST /session/:id/goal with no model round-trip — and a child-driven Goal turn settles the SSE wire with a goal-scoped turn_complete frame (the missing terminal that left Web Shell spinners spinning and local queues undrained).
Both arms drive a real daemon (node packages/cli/dist/index.js serve, head build vs a HEAD^1 worktree build) whose ACP children talk to an in-process fake OpenAI server over loopback. Base control integrity: internal @qwen-code/* links re-pointed into the base worktree and realpath-asserted (readlink -f node_modules/@qwen-code/qwen-code-core → tmp/base-tree/packages/core, same for acp-bridge/sdk) before any run; package.json/package-lock.json are untouched by the PR (verified by diff), so reusing the root external deps and linking the per-package conflict-version node_modules from the head tree is a clean code-only control. Binary-level control: base bridgeClient.js contains 0 source === 'goal' branches; head has 1 (cell W1 on both arms).
| # | cell | oracle | head | base |
|---|---|---|---|---|
| S0 | session create | 200 + sessionId (+ daemon-minted clientId) | ✅ | ✅ |
| 1 | control-plane create | POST /session/:id/goal {action:create} |
200, goal.status=active, revision 1, objective intact |
404 (route absent) |
| 2 | model-free creation | chat-completion count across the create round-trip | 0 requests (19 ms round-trip) | n/a — /goal had to travel POST /session/:id/prompt (202) |
| 3 | Goal turn runs | fake model sees Continue working on the active Goal |
yes, +32 ms after create | yes (positive control: base goal turns fire too) |
| 4 | wire settles | SSE turn_complete with <sessionId>########<n> promptId |
yes — f299a3ce…########1, stopReason end_turn |
0 goal-scoped frames in the 10 s window after the /goal prompt's own terminal, while 201 goal turns hit the model (the defect) |
| 5 | read surface | GET /session/:id/goal; GET /goals |
200 live snapshot; listed with droppedCount:0; row carries the revisioned snapshot (PR delta) |
GET /session/:id/goal 404; GET /goals pre-existing (200, v:1) but rows are the old shape — keys [sessionId, displayName, condition, iterations, setAt, hasActivePrompt], no snapshot |
| 6 | CAS | pause with superseded expectedRevision |
409 goal_conflict with current snapshot |
n/a |
| 7 | lifecycle | pause → resume → edit → clear | 200 each: active→paused→active, edit bumps revision 1→2, clear returns goal:null + clearedGoal tombstone |
only legacy POST …/goal/clear existed (200) |
| 8 | insert gate | POST …/mid-turn-message idle vs during a Goal turn |
idle: 200 {accepted:false}; during Goal turn: 200 {accepted:true, messageId} |
n/a |
| 9 | loop stop | goal-continuation requests 4 s after clear | 0 | n/a (loop still running: 222 total requests) |
Head arm 18/18, base arm 11/11 — every base cell is an assertion that the base fails (or contrasts) as predicted, encoded as expected outcomes, so all count as passes. Witness captures: 01-ab-head-control-plane.png, 02-ab-base-no-control-no-terminal.png; raw logs in logs/head-arm-run.log, logs/base-arm-run.log, machine-readable cells in logs/results-head.json, logs/results-base.json.
Delta since round 1: bf66d843fe — keep a prompt typed mid-drain behind the release chain
The one substantive commit since the previous round fixes the serial release chain in useQueuedPrompts.ts: the chain preserved order only inside the batch it drained, so a prompt typed while link 1 awaited its admission (e.g. media uploads) POSTed straight past the older held rows. The fix publishes the chain on releaseChainRef and appends ordinary submissions to its tail.
- Test present and green at head:
holds a prompt typed mid-drain behind the release chaininsideuseQueuedPrompts.dom.test.tsx— two held prompts with link 1's admission hung, a third typed mid-drain; asserts it does not POST while the chain is in flight and that final order is['first with media', 'second plain', 'typed during drain']. Full file: 69/69. - Vacuity check (mutant M4): disabling the append arm (
if (chain && isCurrentOwnerTokenRef.current(chain.owner))→if (false && …)) turns exactly that test red with the reported symptom —expected [ 'first with media' ]but received[ 'first with media', 'typed during drain' ](the mid-drain prompt POSTs immediately while link 1 is hung); the other 68 tests stay green. The commit message's "mutation-verified" claim reproduces. - Shape reproduced at the hook level only: the real trigger (multi-second media uploads) needs a browser; see Not covered.
Mutation matrix (vacuity of the pinned fixes, re-run at new head)
One guard disabled per row in a scratch HEAD worktree; the suite the commit claims pins it re-run. Positive controls: all four suites are green unmutated in this round (cited in the gates table below — 115 bridgeClient tests, the pinned Session test among 686, 30 mapper tests, 69 queued-prompt tests). Captured live in 03-mutation-matrix-four-killed.png; machine-readable in logs/mutation-matrix.json.
| mutant | reverted/disabled guard | pinned test | outcome |
|---|---|---|---|
| M1 | bridge source === 'goal' branch on _qwencode/end_turn |
bridgeClient.test.ts goal-turn boundary tests |
KILLED — 3 failed | 112 passed; expected … turn_complete received background_notification_turn_complete |
| M2 | child emits _qwencode/end_turn in the goal-turn finally |
Session.test.ts 'notifies the bridge that the Goal turn ended' |
KILLED — 1 failed | 666 skipped; expected call _qwencode/end_turn, received _qwencode/start_turn (end never emitted) |
| M3 | webui mapper rejects unknown limitKind |
mappers.test.ts 'drops an unknown limitKind rather than passing it through' |
KILLED — 1 failed | 29 passed; expected 'not-a-kind' to be undefined |
| M4 | delta: enqueuePrompt appends a mid-drain prompt to the release chain |
useQueuedPrompts.dom.test.tsx 'holds a prompt typed mid-drain behind the release chain' |
KILLED — 1 failed | 68 passed; mid-drain prompt POSTs immediately, order inverted |
4/4 killed, 0 survivors. Every failure is the intended behavioural assertion (expected-vs-actual), not an import/compile break; every mutant file was restored (git status clean) after its run.
Corrections
- Round-1's base table listed
GET /goalsamong the 404 read-surface cells. At this round's base (4839935e55) the route exists (200,v:1) — the PR migrates the listing to the revisioned snapshot (+snapshotfield, filterscompleteinstead of!active, +12/−10 inroutes/goals.ts), it does not add the route. Round 1 ran against an older base (313f1911), where the 404 may have been accurate; the route evidently reached main between the two bases. Cells B2b/B3c above encode the corrected shape.
Targeted gates (head)
| workspace | suites | result | round 1 |
|---|---|---|---|
| core | goals/goal-reducer, goals/goal-runtime |
171/171 | 171 |
| acp-bridge | bridge, bridgeClient, transcript-replay |
905/905 | 905 |
| cli serve | routes/goals, server/telemetry, server/telemetry-catalog, server/error-response |
98/98 | 98 |
| cli acp | session/Session, session/history-replay-page |
686/686 | 686 |
| cli acp/serve | acpAgent, server |
1500 passed / 0 failed | 1498 + 1 skipped |
| webui | session mappers/actions/DaemonSessionProvider, workspace goals.actions |
362/362 | 359 |
| sdk-typescript | DaemonClient, DaemonSessionClient |
413/413 | 413 |
| web-shell | 11 goal-surface files incl. useQueuedPrompts.dom (delta tests) |
353/353 | 342 |
| web-shell | ChatPane, App, WebShellTranscript.dom |
603/603 | 603 |
Total 5,091 passed / 0 failed across the changed surface (+16 vs round 1 — the delta-fix tests). The telemetry-guard change (routes 59→61, split 59/2) is included and green (witness: 04-cli-serve-gate-98-pass.png). Gate logs: logs/gate-*.txt.
Findings
No blocking findings. Non-blocking observations:
- Goal-turn loop still has no pacing while the model answers with plain text (carried from round 1, re-measured). Base arm: 222 chat-completion requests, 201 goal continuations, in the ~100 s observation window; the head loop behaves identically until a control lands. Pre-existing on base — but the PR remains what first makes the loop externally stoppable, and both stop paths were exercised this round (pause H7a; clear → 0 further goal turns in 4 s, H9). Worth the author's awareness, not a merge condition.
- A no-progress Goal auto-transitions to
usage_limitedafter ~100 turns (limitKind: evidence_catalog, ~3 s with the instant fake model). This is the bounded-evidence-catalog guard working as designed — it surfaced while sequencing the lifecycle cells (a late pause hitsgoal_invalid_transitionbecause the Goal already leftactive). Not a defect; recorded so a future verifier of this surface sequences controls inside the active window or drives evidence progress.
Not covered
- Browser-level WebShell behavior — queued-message FIFO drain rendering after pause/clear, Goal-row width parity, clear-without-confirmation, Insert-affordance visibility (plan steps 2/4/5's UI halves). No browser in this container; the PR's Playwright goal spec was not executed. The delta fix's ordering logic is pinned at the hook level (M4), which reproduces the shape of the race (hung admission), not the browser-side cause (real multi-second media uploads).
- TUI Goal flow (plan step 6) — unchanged surface per the PR; not exercised.
- Per-commit attribution — depth-2 checkout: 1 commit locally reachable vs 30 in the metadata snapshot. Only the aggregate
HEAD^1..HEADdiff (83 files, +9,895/−1,099) was verified. - Trial merge into a newer main — impossible at shallow depth with no network; the snapshot base matched the merge-ref base exactly this round, so no drift was observed.
- Windows/macOS paths; desktop-shell styling and token-budget UI (declared out of scope by the PR).
Methodology
Environment: node:22-bookworm CI container (node v22.23.2), merge-ref checkout at depth 2 (HEAD=merge e93ea6e19a, HEAD^1=base 4839935e55, HEAD^2=verified head a39b824396); metadata snapshot at $QWEN_VERIFY_CONTEXT treated as untrusted input; no network calls. Harness harness/goal-ab.mjs boots the compiled daemon per arm with --token, an isolated QWEN_HOME, and OPENAI_BASE_URL pointed at an in-process fake OpenAI server that fingerprints goal-continuation prompts; every cell is a scripted comparison written to logs/results-<arm>.json. The base arm ran from a git worktree at HEAD^1 rebuilt per-package (tsc --build + copy_files.js, git-commit.ts regenerated) with internal @qwen-code/* links re-pointed into the worktree (realpath-asserted) and per-package conflict-version node_modules linked from the head tree (lockfile verified untouched by diff). Mutation rows ran in a second scratch worktree with per-row apply/restore (files verified clean afterwards). Gates ran in the main tree against the CI-built dist. Captures rendered with scripts/verify-capture.mjs. Scratch worktrees were removed after capture.
Flakiness gate log
e2e suite, out of gate scope: packages/web-shell/client/e2e/web-shell.goal.spec.ts
rounds=5 files=33 skipped=1
file packages/acp-bridge/src/bridge.test.ts: (cd packages/acp-bridge) npx --no-install vitest run ./src/bridge.test.ts
file packages/acp-bridge/src/bridgeClient.test.ts: (cd packages/acp-bridge) npx --no-install vitest run ./src/bridgeClient.test.ts
file packages/acp-bridge/src/transcript-replay.test.ts: (cd packages/acp-bridge) npx --no-install vitest run ./src/transcript-replay.test.ts
file packages/cli/src/acp-integration/acpAgent.test.ts: (cd packages/cli) npx --no-install vitest run ./src/acp-integration/acpAgent.test.ts
file packages/cli/src/acp-integration/session/Session.test.ts: (cd packages/cli) npx --no-install vitest run ./src/acp-integration/session/Session.test.ts
file packages/cli/src/acp-integration/session/history-replay-page.test.ts: (cd packages/cli) npx --no-install vitest run ./src/acp-integration/session/history-replay-page.test.ts
file packages/cli/src/serve/routes/goals.test.ts: (cd packages/cli) npx --no-install vitest run ./src/serve/routes/goals.test.ts
file packages/cli/src/serve/server.test.ts: (cd packages/cli) npx --no-install vitest run ./src/serve/server.test.ts
file packages/cli/src/serve/server/error-response.test.ts: (cd packages/cli) npx --no-install vitest run ./src/serve/server/error-response.test.ts
file packages/cli/src/serve/server/telemetry-catalog.test.ts: (cd packages/cli) npx --no-install vitest run ./src/serve/server/telemetry-catalog.test.ts
file packages/cli/src/serve/server/telemetry.test.ts: (cd packages/cli) npx --no-install vitest run ./src/serve/server/telemetry.test.ts
file packages/core/src/goals/goal-reducer.test.ts: (cd packages/core) npx --no-install vitest run ./src/goals/goal-reducer.test.ts
file packages/core/src/goals/goal-runtime.test.ts: (cd packages/core) npx --no-install vitest run ./src/goals/goal-runtime.test.ts
file packages/sdk-typescript/test/unit/DaemonClient.test.ts: (cd packages/sdk-typescript) npx --no-install vitest run ./test/unit/DaemonClient.test.ts
file packages/sdk-typescript/test/unit/DaemonSessionClient.test.ts: (cd packages/sdk-typescript) npx --no-install vitest run ./test/unit/DaemonSessionClient.test.ts
file packages/web-shell/client/App.test.tsx: (cd packages/web-shell) npx --no-install vitest run ./client/App.test.tsx
file packages/web-shell/client/components/ChatPane.test.tsx: (cd packages/web-shell) npx --no-install vitest run ./client/components/ChatPane.test.tsx
file packages/web-shell/client/components/GoalStatusStrip.test.tsx: (cd packages/web-shell) npx --no-install vitest run ./client/components/GoalStatusStrip.test.tsx
file packages/web-shell/client/components/QueuedPromptDisplay.test.tsx: (cd packages/web-shell) npx --no-install vitest run ./client/components/QueuedPromptDisplay.test.tsx
file packages/web-shell/client/components/WebShellTranscript.dom.test.tsx: (cd packages/web-shell) npx --no-install vitest run ./client/components/WebShellTranscript.dom.test.tsx
file packages/web-shell/client/components/dialogs/GoalEditDialog.test.tsx: (cd packages/web-shell) npx --no-install vitest run ./client/components/dialogs/GoalEditDialog.test.tsx
file packages/web-shell/client/components/dialogs/GoalsDialog.test.tsx: (cd packages/web-shell) npx --no-install vitest run ./client/components/dialogs/GoalsDialog.test.tsx
file packages/web-shell/client/components/dialogs/ScheduledTasksDialog.test.tsx: (cd packages/web-shell) npx --no-install vitest run ./client/components/dialogs/ScheduledTasksDialog.test.tsx
file packages/web-shell/client/components/messages/SystemMessage.test.tsx: (cd packages/web-shell) npx --no-install vitest run ./client/components/messages/SystemMessage.test.tsx
file packages/web-shell/client/hooks/useQueuedPrompts.dom.test.tsx: (cd packages/web-shell) npx --no-install vitest run ./client/hooks/useQueuedPrompts.dom.test.tsx
file packages/web-shell/client/hooks/useQueuedPrompts.midTurnReconcile.test.tsx: (cd packages/web-shell) npx --no-install vitest run ./client/hooks/useQueuedPrompts.midTurnReconcile.test.tsx
file packages/web-shell/client/midTurnDedup.test.ts: (cd packages/web-shell) npx --no-install vitest run ./client/midTurnDedup.test.ts
file packages/web-shell/client/utils/goalCondition.test.ts: (cd packages/web-shell) npx --no-install vitest run ./client/utils/goalCondition.test.ts
file packages/web-shell/client/utils/goalGate.test.ts: (cd packages/web-shell) npx --no-install vitest run ./client/utils/goalGate.test.ts
file packages/webui/src/daemon/session/DaemonSessionProvider.test.tsx: (cd packages/webui) npx --no-install vitest run ./src/daemon/session/DaemonSessionProvider.test.tsx
file packages/webui/src/daemon/session/actions.test.ts: (cd packages/webui) npx --no-install vitest run ./src/daemon/session/actions.test.ts
file packages/webui/src/daemon/session/mappers.test.ts: (cd packages/webui) npx --no-install vitest run ./src/daemon/session/mappers.test.ts
file packages/webui/src/daemon/workspace/goals.actions.test.ts: (cd packages/webui) npx --no-install vitest run ./src/daemon/workspace/goals.actions.test.ts
per-file results (P=pass F=fail I=infra-exit, one letter per run):
packages/acp-bridge/src/bridge.test.ts: PPP
packages/acp-bridge/src/bridgeClient.test.ts: PPP
packages/acp-bridge/src/transcript-replay.test.ts: PPP
packages/cli/src/acp-integration/acpAgent.test.ts: PPP
packages/cli/src/acp-integration/session/Session.test.ts: PP
packages/cli/src/acp-integration/session/history-replay-page.test.ts: PP
packages/cli/src/serve/routes/goals.test.ts: PP
packages/cli/src/serve/server.test.ts: PP
packages/cli/src/serve/server/error-response.test.ts: PP
packages/cli/src/serve/server/telemetry-catalog.test.ts: PP
packages/cli/src/serve/server/telemetry.test.ts: PP
packages/core/src/goals/goal-reducer.test.ts: PP
packages/core/src/goals/goal-runtime.test.ts: PP
packages/sdk-typescript/test/unit/DaemonClient.test.ts: PP
packages/sdk-typescript/test/unit/DaemonSessionClient.test.ts: PP
packages/web-shell/client/App.test.tsx: PP
packages/web-shell/client/components/ChatPane.test.tsx: PP
packages/web-shell/client/components/GoalStatusStrip.test.tsx: PP
packages/web-shell/client/components/QueuedPromptDisplay.test.tsx: PP
packages/web-shell/client/components/WebShellTranscript.dom.test.tsx: PP
packages/web-shell/client/components/dialogs/GoalEditDialog.test.tsx: PP
packages/web-shell/client/components/dialogs/GoalsDialog.test.tsx: PP
packages/web-shell/client/components/dialogs/ScheduledTasksDialog.test.tsx: PP
packages/web-shell/client/components/messages/SystemMessage.test.tsx: PP
packages/web-shell/client/hooks/useQueuedPrompts.dom.test.tsx: PP
packages/web-shell/client/hooks/useQueuedPrompts.midTurnReconcile.test.tsx: PP
packages/web-shell/client/midTurnDedup.test.ts: PP
packages/web-shell/client/utils/goalCondition.test.ts: PP
packages/web-shell/client/utils/goalGate.test.ts: PP
packages/webui/src/daemon/session/DaemonSessionProvider.test.tsx: PP
packages/webui/src/daemon/session/actions.test.ts: PP
packages/webui/src/daemon/session/mappers.test.ts: PP
packages/webui/src/daemon/workspace/goals.actions.test.ts: PP
verdict: timeout
summary: only 2 of 5 rounds fit the 15-minute budget; the completed rounds agreed
--- per-invocation detail (full copy in the artifact) ---
round 1 · packages/acp-bridge/src/bridge.test.ts: P (exit 0)
round 1 · packages/acp-bridge/src/bridgeClient.test.ts: P (exit 0)
round 1 · packages/acp-bridge/src/transcript-replay.test.ts: P (exit 0)
round 1 · packages/cli/src/acp-integration/acpAgent.test.ts: P (exit 0)
round 1 · packages/cli/src/acp-integration/session/Session.test.ts: P (exit 0)
round 1 · packages/cli/src/acp-integration/session/history-replay-page.test.ts: P (exit 0)
round 1 · packages/cli/src/serve/routes/goals.test.ts: P (exit 0)
round 1 · packages/cli/src/serve/server.test.ts: P (exit 0)
round 1 · packages/cli/src/serve/server/error-response.test.ts: P (exit 0)
round 1 · packages/cli/src/serve/server/telemetry-catalog.test.ts: P (exit 0)
round 1 · packages/cli/src/serve/server/telemetry.test.ts: P (exit 0)
round 1 · packages/core/src/goals/goal-reducer.test.ts: P (exit 0)
round 1 · packages/core/src/goals/goal-runtime.test.ts: P (exit 0)
round 1 · packages/sdk-typescript/test/unit/DaemonClient.test.ts: P (exit 0)
round 1 · packages/sdk-typescript/test/unit/DaemonSessionClient.test.ts: P (exit 0)
round 1 · packages/web-shell/client/App.test.tsx: P (exit 0)
round 1 · packages/web-shell/client/components/ChatPane.test.tsx: P (exit 0)
round 1 · packages/web-shell/client/components/GoalStatusStrip.test.tsx: P (exit 0)
round 1 · packages/web-shell/client/components/QueuedPromptDisplay.test.tsx: P (exit 0)
round 1 · packages/web-shell/client/components/WebShellTranscript.dom.test.tsx: P (exit 0)
round 1 · packages/web-shell/client/components/dialogs/GoalEditDialog.test.tsx: P (exit 0)
round 1 · packages/web-shell/client/components/dialogs/GoalsDialog.test.tsx: P (exit 0)
round 1 · packages/web-shell/client/components/dialogs/ScheduledTasksDialog.test.tsx: P (exit 0)
round 1 · packages/web-shell/client/components/messages/SystemMessage.test.tsx: P (exit 0)
round 1 · packages/web-shell/client/hooks/useQueuedPrompts.dom.test.tsx: P (exit 0)
round 1 · packages/web-shell/client/hooks/useQueuedPrompts.midTurnReconcile.test.tsx: P (exit 0)
round 1 · packages/web-shell/client/midTurnDedup.test.ts: P (exit 0)
round 1 · packages/web-shell/client/utils/goalCondition.test.ts: P (exit 0)
round 1 · packages/web-shell/client/utils/goalGate.test.ts: P (exit 0)
round 1 · packages/webui/src/daemon/session/DaemonSessionProvider.test.tsx: P (exit 0)
round 1 · packages/webui/src/daemon/session/actions.test.ts: P (exit 0)
round 1 · packages/webui/src/daemon/session/mappers.test.ts: P (exit 0)
round 1 · packages/webui/src/daemon/workspace/goals.actions.test.ts: P (exit 0)
rou
...truncated -- full content in the run artifacts.
Evidence images
Harness scripts and raw logs are in the workflow run artifacts (7-day retention).
— Qwen Code · sandboxed verification
|
Triage re-run completed without a new review.
The stage comments above were updated with the latest result. View workflow run. 上方各阶段评论已更新为最新结果。查看工作流运行。 |
ytahdn
left a comment
There was a problem hiding this comment.
LGTM, approving.
没问题,批准合入。
|
Released in v0.21.15. |







What this PR does
This PR adopts the canonical Goal v3 control plane in WebShell. Goals can be created before the first chat message, inspected, edited, paused, resumed, replaced, and cleared without routing commands through the model. WebShell presents the active Goal in a compact composer row, keeps that row at the queued-message width, and removes the delete confirmation.
While a Goal is running, ordinary messages remain in a local FIFO queue. They are submitted normally after the Goal stops; only the explicit Insert action enters the active turn. The change also exposes typed Goal state and controls through the daemon, SDK, ACP bridge, and shared WebUI data layer so WebShell has one canonical source of truth.
The TUI keeps its existing visual style and command flow. Token-budget UI and desktop-shell adoption are intentionally excluded.
Why it's needed
The previous WebShell behavior mixed Goal commands with chat prompts, required a conversation before setting a Goal, and allowed queued messages to interrupt autonomous execution unexpectedly. Goal state also had multiple UI-owned interpretations, which produced transient load errors and inconsistent lifecycle rendering.
The canonical control path makes Goal lifecycle changes deterministic, lets a new task start directly with
/goal, preserves queue intent, and gives all consumers the same revisioned Goal snapshot.Reviewer Test Plan
How to verify
/goal <objective>, and confirm a session is created with an active Goal while no ordinary chat prompt is sent.Automated browser coverage runs the complete fresh-session and active-Goal queue chains. A separate PR comment contains the E2E scenario report and captured evidence inventory.
Evidence (Before & After)
Before: WebShell treated Goal setup as part of chat/session state, could require an initial message, used independently sized Goal and queue rows, and could feed ordinary queued messages into the running turn.
After:
/goalcan allocate its own session, the compact Goal row matches the queued-message width, normal messages remain locally queued, explicit Insert is the only mid-turn path, and clear is immediate. TUI remains visually unchanged and was exercised through three tmux chains.Tested on
Environment (optional)
Node.js 22.17.0, local source build, Chromium Playwright against the WebShell daemon harness, and a 140x42 tmux TUI session without sandboxing.
Risk & Scope
Linked Issues
N/A
中文说明
本 PR 做了什么
本 PR 在 WebShell 中接入 canonical Goal v3 控制面。无需先发送聊天消息即可创建 Goal,并可直接查看、编辑、暂停、恢复、替换和清除,不再把这些命令发送给模型。WebShell 在输入框上方展示紧凑的 Goal 状态条,保持排队消息的宽度,同时删除 Goal 时不再弹出确认框。
Goal 运行时,普通消息保留在本地 FIFO 队列中;Goal 停止后再按正常消息发送。只有用户明确点击“插入”时,消息才会进入当前执行轮次。本次还通过 daemon、SDK、ACP bridge 和共享 WebUI 数据层暴露统一的 Goal 状态与控制能力,让 WebShell 使用同一份 canonical 状态。
TUI 保持现有视觉风格和命令流程。本 PR 不包含 token 预算 UI 和桌面端接入。
为什么需要
此前 WebShell 会把 Goal 命令和聊天消息混在一起,设置 Goal 前可能需要先创建会话;排队消息也可能意外中断自治执行。Goal 状态还存在多套 UI 侧解释,造成过瞬时加载错误和生命周期展示不一致。
canonical 控制路径让 Goal 生命周期变更更确定,支持新任务直接从
/goal开始,保留用户的排队意图,并让所有消费者读取同一份带版本的 Goal 快照。Reviewer Test Plan
验证方式
/goal <目标>,确认系统创建会话并激活 Goal,同时没有发送普通聊天消息。自动化浏览器用例覆盖首次创建会话和运行中 Goal 排队的完整链路。PR 的独立评论附有 E2E 场景报告与截图证据清单。
前后对比证据
Before:WebShell 的 Goal 设置依赖聊天/会话状态,可能要求先发送消息;Goal 与排队消息宽度独立;普通排队消息可能进入正在执行的轮次。
After:
/goal可自行分配会话,紧凑 Goal 状态条保持排队消息宽度,普通消息继续本地排队,只有明确点击“插入”才进入当前轮次,清除操作立即生效。TUI 视觉保持不变,并通过三条 tmux 链路验证。测试平台
环境
Node.js 22.17.0、本地源码构建、Chromium Playwright WebShell daemon harness,以及 140x42 无沙箱 tmux TUI 会话。
风险与范围
关联 Issue
N/A