fix(web-shell): route goal messages by session activity - #9667
Conversation
E2E / verification reportIndependent pre-PR verification passed with no source changes and no blocking findings.
Automated evidence: Web Shell focused tests 17 passed; image direct-send and attachment retry tests 2 passed; Goal gate suite 10 passed; ACP active-Goal admission test 1 passed; |
|
Thanks for the PR! Re-run ( Template: complete ✓ — all required sections present, bilingual body included. Problem: observed and measured, not theoretical. The session load clears Direction: aligned. Routing ordinary-message admission by session activity instead of an auxiliary Goal read is the right instinct; slash commands and run controls keep the fail-closed Goal gate. The load-bearing premise was checked in the first pass and re-confirmed since: the runtime's permit machinery ( Size: not applicable — no core paths touched (all seven files under Approach: focused, and it actively removes special-casing. Every edit serves the stated goal; the post-first-review commits respond point-for-point to review feedback (E2E spec rewritten to the new contract, Goal-specific rejection toasts) and to the A/B report's §5 (daemon Risk: no high-risk-path matches; no elevated risk signals. Moving on to code review. 🔍 中文说明感谢贡献！ Re-run（ 模板：完整 ✓，包含双语正文。 问题：已观测且有测量数据，不是理论问题。会话加载会先清掉 方向：对齐。普通消息准入按会话活动状态路由、而不是等辅助的 Goal 查询，方向正确；斜杠命令和运行控制保留 fail-closed Goal 门。关键前提在首轮已核实、本轮再次确认：goal runtime 的 permit 机制（ 规模：不适用——未触及核心路径（七个文件全部在 方案：聚焦，且在主动移除特判。每处改动都服务于目标；首轮之后的提交逐条响应了 review 反馈（E2E spec 按新契约重写、Goal 专属拒绝提示）和 A/B 报告 §5（把 daemon 风险：未命中高风险路径，无升级风险信号。 进入代码审查。🔍 — Qwen Code · qwen3.8-max Reviewed at |
Code reviewRe-run — this pass covers the full diff at the current head, with focus on the five commits since the first pass. The load-bearing premise still holds (verified in the first pass, and independently re-verified by two reviewers with file:line citations since): the goal runtime's permit machinery gates ordinary submissions server-side, so routing ordinary messages purely by session activity is safe, and the Goal gate correctly stays in place for slash commands and the run controls. The routing contract at this head:
What I checked in the new commits:
No correctness blockers found. Four suggestions from the fourth Test evidence (this PR's own CI)Unattended run — I do not execute PR code; this quotes the PR's CI via the API at the reviewed commit.
One row per check name (latest run); skipped checks omitted; failures sort first. / 每个检查名一行（取最新一次运行），省略 skipped，失败项排在最前。 Everything green on the reviewed commit — notably the web-shell E2E smoke, which was the round-1 blocker. Beyond CI: a maintainer ran a real-daemon + real-browser A/B (PR arm vs merge base against one daemon, request-by-request ledger) at the earlier head Not verified: real browser behavior on Windows/Linux (author marked N/A); images/attachments through the new routing (parity claimed in the description, not exercised by the A/B); live behavior at the exact reviewed head (A/B was at 中文说明代码审查Re-run——本轮覆盖当前 head 的完整 diff，重点是首轮之后的五个提交。关键前提依然成立（首轮已核实，此后两位审查者又独立复核并给出文件:行号引用）：goal runtime 的 permit 机制在服务端拦截普通提交，因此普通消息完全可以只按会话活动状态路由，Goal 门也正确地保留给了斜杠命令和运行控制。 当前 head 的路由契约：普通消息——空闲直发、流式中中途插入、首 token 前窗口也中途插入（新：daemon 新提交核实情况： 未发现正确性阻塞问题。第四轮 测试证据（本 PR 自己的 CI）无人值守运行——不执行 PR 代码；以上为 API 读取的该 commit 的 CI 结果。本 commit 全绿——尤其是 round-1 的阻塞项 web-shell E2E smoke。macOS/Windows 测试矩阵与 CLI 集成测试在该事件下为条件性跳过。作者此前提到的 此外：一位 maintainer 在较早的 head 未验证：Windows/Linux 真实浏览器行为（作者标记 N/A）；图片/附件走新路由（描述声称一致，A/B 未实测）；精确审查 head 上的实运行（A/B 在 — 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 |
|
Confidence: 4/5 — clean re-review at the new head: the risky premise verified, CI fully green, the maintainer's A/B concern addressed by design; four recorded non-blocking follow-ups named below. Stepping back: this PR still matches what I would have proposed — route ordinary messages by session activity alone, keep the Goal gate only where ownership genuinely matters, and let retry wait for idle. What changed since the first pass is that the one real behavioral gap found along the way (the A/B report's §5: an ordinary submit in the pre-first-token window preempting the in-flight Goal turn at zero tokens) was answered the right way — not with documentation, but by folding the daemon-authoritative Reservations, all non-blocking and all already recorded inline by the fourth review round: ChatPane rejects a Goal-blocked slash command silently where the main composer toasts (three-line fix proposed inline — cheapest of the four if the author wants it in before merge); the Verdict: approve. All checks are green on the reviewed commit, so the approval is posted immediately, pinned to it. 中文说明置信度：4/5 —— 新 head 上的复审干净：关键前提已核实、CI 全绿、maintainer A/B 的疑虑已从设计上解决；四个不阻塞的后续项见下。 退一步看：这个 PR 仍然与我的独立方案一致——普通消息只按会话活动状态路由，Goal 门只保留在归属真正重要的地方，重试等待空闲。首轮之后真正的变化是：过程中发现的唯一真实行为缺口（A/B 报告 §5：首 token 前窗口的普通提交在零 token 处抢占在途 Goal turn）得到了正确方式的回应——不是改文档，而是把 daemon 权威的 保留意见，均不阻塞且均已由第四轮 review 记录在行内：ChatPane 对 Goal 拦截的斜杠命令静默而主输入框有 toast（行内已给出三行修复，若想合并前带上它，这是四条中成本最低的）； 结论：批准。该 commit 的所有检查均为绿色，批准立即发出并固定于该 commit。 — Qwen Code · qwen3.8-max Reviewed at |
Code Coverage Summary
CLI Package - Full Text ReportCore Package - Full Text ReportFor detailed HTML reports, please see the 'coverage-reports-22.x-ubuntu-latest' artifact from the main CI run. |
chiga0
left a comment
chiga0
left a comment
There was a problem hiding this comment.
E2E Smoke CI: ❌ FAIL — blocker present.
Scope
Reviewed: App.tsx (ordinary-message routing, slash-command enqueueBlockedCommand, handleRetry guard, streamingStateRef synchronization), ChatPane.tsx (submission gate refactor), goalGate.ts (docblock), App.test.tsx + ChatPane.test.tsx (updated unit tests).
Checked: blockLocalCommandDuringTurn definition (toast + return false — correct for fail-closed Goal block); holdQueuedPromptsLocally fully removed from both App.tsx and ChatPane.tsx at head; streamingStateRef.current = streamingState during render is the standard latest-ref pattern in React (correct); enqueueBlockedCommand branching logic (goal-blocked → toast; streaming-blocked, no goal → enqueue); retry guard drop (button hidden via streamingState !== 'idle' when session is active — stale-callback test confirms no double-send).
Not reviewed: macOS/Windows CI (SKIPPED); integration tests (SKIPPED).
F1 — Blocker: E2E smoke fails because holdQueuedPromptsLocally removal drops the explicit local-queue UX path
CI witness (3/3 retries, all identical):
web-shell.goal.spec.ts:48 runs the canonical Goal and explicit queue interaction chain @smoke
Error: expect(locator).toContainText(expected) failed
Locator: locator('[data-web-shell-queued-prompts]')
Expected substring: "stay queued until I choose"
Timeout: 10000ms — element(s) not found
93 | await submitComposer(page, 'stay queued until I choose');
94 | const queue = page.locator('[data-web-shell-queued-prompts]');
> 95 | await expect(queue).toContainText('stay queued until I choose');
Mechanism: The test submits a message while a Goal is active (session is idle, between turns). Under the old code, holdQueuedPromptsLocally was true (Goal state active → hold), so the message went into the local queue and appeared in [data-web-shell-queued-prompts]. Under the new code the hold is removed: for non-command text at streamingStateRef.current === 'idle', the submission goes directly to sendPrompt, the queue element never renders, and daemon.promptRequests() would show 1 request rather than 0.
The test then uses the queued message to exercise the explicit "Insert" button (mid-turn insertion with user choice). That full E2E flow — submit while Goal-idle → see in queue → click Insert during turn → verify mid-turn request — is the explicit local-queue UX path. The Reviewer Test Plan covers "send while a turn is responding → inserted automatically" but does not cover the "queue for manual insertion" path this smoke test exercises.
What to check: Was the explicit local-queue affordance (queue while idle, Insert during turn) intentionally removed by this PR? If yes, the E2E spec needs to be updated or replaced with a test that validates the new direct-send path. If not, the holdQueuedPromptsLocally removal needs to be scoped so Goal-active-between-turns still populates the queue.
Reviewed with AI assistance.
ytahdn
left a comment
ytahdn
left a comment
There was a problem hiding this comment.
Review of PR 9667 — fix(web-shell): route goal messages by session activity
Verdict: 💬 COMMENT — one blocking item: the canonical Goal E2E smoke spec still encodes the removed local-hold UX, and CI is red on exactly that assertion.
Head reviewed: 66aadc4e. Scope: 5 files, +208/−125. Unit suite green on CI; web-shell E2E Smoke failed (details in F1).
Load-bearing premise — verified ✅
This PR deliberately reverses the earlier fail-closed client-side Goal hold, so I verified the server-side admission machinery first, at source level:
goalRequiresExactPermit(packages/core/src/goals/goal-protocol.ts:146-151): an active Goal (or running activity) requires an exact turn permit.- UserQuery admission (
packages/core/src/core/client.ts:2749-2775): an ordinary submission under an active Goal takesgoalOrigin = 'user'and obtainspermitForTurn(goalTurnKey) ?? beginTurn(goalTurnKey); if neither is available it throws, surfacing "Goal turn admission failed" — a visible error, not a silent drop. beginTurn(packages/core/src/goals/goal-runtime.ts:1101-1133): between turns it grants the permit immediately; while another turn runs (or verifying/proposal/checkpoint is pending) it queues the turn key and returns undefined.releaseTurn(goal-runtime.ts:1134-1180) promotes the queued turn key on release, andclaimGoalTurn(packages/cli/src/acp-integration/session/Session.ts:570-614) waits on the runtime subscription until promotion or abort.
So a direct send from an idle session is admitted as a user-origin Goal turn, a send during a running turn is enqueued/promoted by the runtime, and neither path can silently bypass the Goal. The premise holds; the client no longer needs to fail closed for ordinary messages.
Client-side changes — verified ✅
streamingStateRefis now assigned during render (App.tsx:4244-4245) instead of in a passive effect. This closes the real race where a child layout-effect submit during the transition render read the stale'idle'value; the new test harness (onChatEditorLayoutin the ChatEditor mock) pins exactly that window. The pattern matches existing code (e.g.writeBlockedRefinuseQueuedPrompts.ts:413-414).- Ordinary path routes by streaming state only (
App.tsx:9958-9968); slash commands keep the fail-closed Goal gate and are deduped throughenqueueBlockedCommand(App.tsx:8915-8925), withblockLocalCommandDuringTurnstill toasting (App.tsx:6661-6664). - All remaining
isGoalGateBlockedcall sites are command/run guards:/languagesync (App.tsx:7082), manual run enqueue (App.tsx:8466), composer commands (App.tsx:8915), fast-model picker (App.tsx:10743).goalGate.tsbody unchanged, doc narrowed to the new convention. - Retry hardening is consistent on both layers: the retry affordance is hidden while a turn runs (
App.tsx:12402-12406),handleFailedPromptRetryguards onstreamingStateRef.current !== 'idle'(App.tsx:6129-6136), and the stale-retry test proves a captured callback cannot resubmit once the session becomes active.handleRetrykeeps its own idle guard, so removing the Goal gate there is safe. ChatPane.tsx:791-798gates only/-prefixed submissions on the Goal gate; ordinary submissions route by streaming state, mirroring App.
Findings
🔴 F1 — web-shell.goal.spec.ts @smoke still asserts the removed hold UX; CI fails on it (blocking)
Witness: web-shell E2E Smoke (ubuntu-latest, Node 22.x) on 66aadc4e (run 32483364983) failed on all 3 attempts with expect(locator).toContainText('stay queued until I choose') — element not found — at client/e2e/web-shell.goal.spec.ts:95.
The spec "runs the canonical Goal and explicit queue interaction chain" encodes the old contract this PR removes:
- lines 93-97: with an active Goal and no turn running, an ordinary submission must land in the local queue (
[data-web-shell-queued-prompts]) with zero daemon prompt requests and zero mid-turn requests; - lines 105-117: the explicit Insert button moves that held prompt into a running turn;
- lines 126-130: a second prompt is held locally and flushed only when the Goal is paused.
Because the PR removes holdQueuedPromptsLocally from production (the useQueuedPrompts call in App.tsx no longer passes it; the hook default is false, useQueuedPrompts.ts:406), the line-93 submission is now a direct sendPrompt against the idle session — the queue strip never renders the text and a prompt request reaches the daemon, so the assertion fails deterministically (3/3 retries, not flaky). The unit suite was updated for the new contract; this spec was not, and it is the only remaining consumer of [data-web-shell-queued-prompts] in the e2e tree.
Suggested fix direction: rewrite the interaction chain to the new contract — idle + active Goal → expect the daemon prompt request (user-origin Goal turn); running turn → submission becomes a mid-turn insertion request; rework or drop the explicit-Insert segment accordingly, and update the 02-goal-with-local-queue.png capture stage. Until then this check blocks merge (and ci-bot's deferred approval is gated on it too).
Not re-raised (already discussed)
holdQueuedPromptsLocallyremains in theuseQueuedPromptssignature and hook tests while dead in production — ci-bot stage 3 already named this as a non-blocking cleanup follow-up.- ChatPane's slash-command block returns
falsewithout a toast while App toasts — pre-existing asymmetry, also already noted by ci-bot.
🎉 What stands out
The reversal is justified at the right layer: admission semantics move to the runtime's permit machinery, which is strictly stronger than a client-side hold (a held prompt could be stranded by a client reload; a permitted/queued turn survives because the runtime owns ordering). The render-phase ref fix and the stale-retry hardening each come with a test that reproduces the exact race window. Net −65 lines of gate logic replaced by a single routing predicate.
中文说明
结论：💬 COMMENT — 一个阻塞项：canonical Goal E2E smoke spec 仍在断言已被移除的本地 hold 行为，CI 正是断在这一条上。
审查 head：66aadc4e。范围：5 个文件，+208/−125。单元测试在 CI 上通过；web-shell E2E Smoke 失败（见 F1）。
关键前提 — 已验证 ✅
本 PR 有意反转了早先 fail-closed 的前端 Goal hold，因此先逐层核实了服务端准入机制：goalRequiresExactPermit（goal-protocol.ts:146-151）要求 active Goal 下的提交必须持有精确 turn permit；UserQuery 准入（client.ts:2749-2775）把普通提交标记为 goalOrigin = 'user' 并走 permitForTurn ?? beginTurn，拿不到 permit 时抛错并显示 "Goal turn admission failed"，是可见错误而非静默丢弃；beginTurn（goal-runtime.ts:1101-1133）在两个 turn 之间立即发放 permit，运行中则排队；releaseTurn（1134-1180）释放时提升排队者，claimGoalTurn（Session.ts:570-614）通过订阅等待提升或中止。结论：空闲会话的直接发送会作为用户触发的 Goal turn 被准入，运行中的发送由运行时排队/提升，任何路径都不会静默绕过 Goal。前提成立，前端不再需要对普通消息 fail-closed。
前端改动 — 已验证 ✅
streamingStateRef 改为渲染期赋值（App.tsx:4244-4245），修复了过渡渲染中子组件 layout effect 提交读到过期 'idle' 的真实竞态，新测试夹具（ChatEditor mock 的 onChatEditorLayout）正好钉住该窗口；模式与既有代码一致（useQueuedPrompts.ts:413-414 的 writeBlockedRef）。普通消息路径只按 streaming state 路由（App.tsx:9958-9968）；斜杠命令保留 fail-closed Goal 门并经 enqueueBlockedCommand 去重（App.tsx:8915-8925），blockLocalCommandDuringTurn 仍弹 toast（App.tsx:6661-6664）。剩余所有 isGoalGateBlocked 调用点都是命令/运行守卫（App.tsx:7082 /language、8466 手动运行、8915 命令、10743 fast-model 选择器）；goalGate.ts 函数体未变，仅收窄文档。重试加固两层一致：运行中隐藏重试入口（App.tsx:12402-12406）、handleFailedPromptRetry 增加 streamingStateRef.current !== 'idle' 守卫（App.tsx:6129-6136），stale-retry 测试证明已捕获的旧回调在会话转为活跃后无法重复提交；handleRetry 自身仍保留 idle 守卫，移除其中的 Goal 门是安全的。ChatPane.tsx:791-798 仅对 / 前缀提交启用 Goal 门，与 App 保持一致。
发现
🔴 F1 — web-shell.goal.spec.ts @smoke 仍断言已移除的 hold 行为，CI 因此失败（阻塞）
见证：66aadc4e 上的 web-shell E2E Smoke (ubuntu-latest, Node 22.x)（run 32483364983）三次重试全部失败于 expect(locator).toContainText('stay queued until I choose') — element not found — 位置 client/e2e/web-shell.goal.spec.ts:95。
spec "runs the canonical Goal and explicit queue interaction chain" 编码的正是本 PR 移除的旧契约：93-97 行要求 active Goal 且无 turn 运行时，普通提交进入本地队列（[data-web-shell-queued-prompts]）且不产生任何 daemon prompt/mid-turn 请求；105-117 行通过显式 Insert 按钮把排队消息插入运行中的 turn；126-130 行要求第二条消息本地保留、直到 Goal 被暂停才冲刷。由于 PR 在生产路径移除了 holdQueuedPromptsLocally（App.tsx 调用 useQueuedPrompts 不再传参，hook 默认 false，useQueuedPrompts.ts:406），第 93 行的提交现在是对空闲会话的直接 sendPrompt —— 队列条不会出现该文本，prompt 请求会到达 daemon，断言确定性失败（3/3 次重试，非偶发）。单元测试已按新契约更新，此 spec 未更新，且它是 e2e 树中 [data-web-shell-queued-prompts] 的唯一消费者。
建议修复方向：把该交互链改写为新契约 —— 空闲 + active Goal → 断言 daemon prompt 请求（用户触发的 Goal turn）；运行中的 turn → 提交变为 mid-turn 插入请求；相应重写或删除显式 Insert 段落，并更新 02-goal-with-local-queue.png 截图阶段。修复前该检查阻塞合并（ci-bot 的延迟批准也以它为门槛）。
不再重复提出（已有讨论）
holdQueuedPromptsLocally 仍留在 useQueuedPrompts 签名与 hook 测试中但生产已不使用 —— ci-bot stage 3 已将其列为非阻塞清理项；ChatPane 对斜杠命令的阻止返回 false 而不弹 toast，与 App 不一致 —— 既有差异，ci-bot 亦已提及。
🎉 亮点
反转发生在正确的层：准入语义移交给运行时 permit 机制，严格强于前端 hold（被 hold 的提示可能因客户端刷新而滞留，而 permit/排队由运行时持有排序，不受前端状态影响）。渲染期 ref 修复与 stale-retry 加固各自配备了能复现精确竞态窗口的测试。净 −65 行门控逻辑被单一按活动状态路由的判定取代。
chiga0
left a comment
chiga0
left a comment
There was a problem hiding this comment.
F1 fixed — E2E smoke now passes on all checks.
What the fix commit does: test(web-shell): align goal smoke with message routing rewrites the Goal E2E spec to match the new routing semantics. The old test submitted while Goal was active + session idle and expected a local queue; the new test submits during an active turn (after assistantTextEvent) and verifies the message is auto-inserted mid-turn. The after-turn submission ('run while the goal stays active') now correctly expects daemon.promptRequests().length === 1 (direct send, not queued), which is the intent described in the PR.
CI (new head 4fc8f0d6):
web-shell E2E Smoke (ubuntu)✅Test (ubuntu, Node 22.x)✅Capture web-shell visuals✅- macos / windows / integration: SKIPPED (not gated on this change)
Reviewed with AI assistance.
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed — no blockers. Suggestions are inline.
中文说明
已审查——无阻断问题。 建议见行内评论。
— qwen3.8-max via Qwen Code /review (v0.21.15)
wenshao
left a comment
wenshao
left a comment
There was a problem hiding this comment.
Reviewed. Suggestions are inline.
中文说明
已审查。 建议见行内评论。
— qwen3.8-max via Qwen Code /review (v0.21.14)
| }), | ||
| ); | ||
| await queue.getByRole('button', { name: 'Insert' }).click(); | ||
| await submitComposer(page, 'stay queued until I choose'); |
There was a problem hiding this comment.
[Suggestion] The rewrite drops the only E2E click of the queued-prompts Insert button (the removed queue.getByRole('button', { name: 'Insert' }).click()). The button still ships — QueuedPromptDisplay renders it and the App wires canInsertMidTurn/onInsert — but nothing in client/e2e/ exercises manual mid-turn insertion anymore: the ChatPane tests only assert the data-can-insert-mid-turn dataset, and hook-level tests call insertQueuedPrompt directly, bypassing the button and the App-level gating. A regression breaking the manual Insert wiring would ship with the smoke suite green. Consider keeping one E2E step that holds a queued prompt while a turn runs and clicks Insert, asserting the resulting mid-turn request.
中文说明
重写移除了 E2E 中唯一一次点击队列 Insert 按钮的操作。该按钮仍在线上提供（QueuedPromptDisplay 渲染、App 连接 canInsertMidTurn/onInsert），但 client/e2e/ 中不再有任何用例验证手动中途插入：ChatPane 测试只断言 data-can-insert-mid-turn 数据集，hook 层测试直接调用 insertQueuedPrompt，绕过了按钮和 App 层的门控。若手动 Insert 接线出现回归，smoke 套件仍会绿灯通过。建议保留一个 E2E 步骤：turn 运行时让一条消息留在队列中并点击 Insert，断言产生的 mid-turn 请求。
— qwen3.8-max via Qwen Code /review (v0.21.14)
|
CI note: the new |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed. Suggestions are inline.
Not explored to full depth (tool budget reached): "agent reverse-audit (round 1)": running the web-shell unit tests (App.test.tsx / ChatPane.test.tsx) to confirm the diff's tests execute green; "agent 1a": typecheck/unit-test run for packages/web-shell (no node_modules installed in the worktree or parent checkout; full install exceeded the tool budget).
中文说明
已审查。 建议见行内评论。
未探索到全部深度（达到工具调用预算）："agent reverse-audit (round 1)"：running the web-shell unit tests (App.test.tsx / ChatPane.test.tsx) to confirm the diff's tests execute green；"agent 1a"：typecheck/unit-test run for packages/web-shell (no node_modules installed in the worktree or parent checkout; full install exceeded the tool budget)。
— qwen3.8-max via Qwen Code /review (v0.21.15)
Local verification report — real daemon + real browser, A/B against
|
| Piece | What it was |
|---|---|
| Daemon | real qwen serve started from the PR source tree (scripts/dev.js serve, one instance, :5323), real ACP child, isolated QWEN_HOME, real workspace |
| Front ends | two Vite dev servers against the same daemon — PR head on :5324, merge base main on :5325 |
| Model | local OpenAI-compatible provider with controllable time-to-first-token and streaming cadence, plus a per-request ledger that records whether the request socket was aborted and how many chunks it delivered |
| Driver | Playwright Chromium, full request ledger per page (POST /prompt, POST /mid-turn-message, GET /goal, …) |
| Platform | macOS 26.6.2, Node v24.18.1 |
Because both arms talk to one daemon and one model, every difference below is attributable to the client diff.
Static / test gates on PR head
| Gate | Result |
|---|---|
packages/web-shell full vitest suite |
4043 passed, 0 failed |
App.test.tsx + ChatPane.test.tsx + goalGate |
622 passed |
tsc --noEmit (packages/web-shell) |
clean |
eslint on the 4 changed source files |
clean |
web-shell.goal.spec.ts (Playwright, PR head) |
2 passed |
Non-vacuity: PR test files run against main source |
19 failed / 612 |
Non-vacuity: PR e2e spec run against main source |
fails at midTurnRequests(daemon).length → 1 |
1. Idle session, Goal read still in flight — the case the PR is about ✅
Reopened an existing idle session with GET /session/:id/goal delayed by 12 s, then typed an ordinary message.
main |
PR #9667 | |
|---|---|---|
POST /prompt timing after reopen |
+12 149 ms (waits out the whole Goal read) | +336 ms |
| Queued badge shown | yes | no |
2. Forwarded slash command in the same window ✅ (stricter — see note)
main |
PR #9667 | |
|---|---|---|
| Behaviour | queued, then forwarded to the daemon once the Goal read landed (POST /prompt with /deploy production) |
refused immediately with the new queue.commandGoalBlocked toast; 0 POST /prompt, ever |
| Composer | cleared | text preserved — re-submitting after hydration succeeds (verified) |
Note: on main a slash command typed in the unknown window eventually ran; on this PR it is dropped and the user must press send again. That is the intended fail-closed trade, and it costs one keypress because the composer keeps the text — flagging it only so the choice is explicit.
3. Ordinary message during a streaming Goal turn ✅
Goal turn streaming line-1…line-40 at 1.5 s intervals; composer correctly reads esc to cancel on both arms.
main |
PR #9667 | |
|---|---|---|
| Requests | none — held locally behind an explicit “Insert” | 1 × POST /mid-turn-message, badge reads “Queued…” |
| Reached the model? | not until the user clicks Insert | yes — provider ledger shows the Goal continuation context and the user text in the same model request |
4. Failed-send retry while a Goal turn runs ✅
Produced a genuinely rejected send (413, the isDefinitelyRejectedPromptAdmission path), then resumed a parked Goal so a turn starts without a new user message.
main |
PR #9667 | |
|---|---|---|
| While the Goal turn streams | “Failed to send / Try again” still rendered; clicking it produced 0 requests, 0 mid-turn messages, no toast — a dead affordance blocked by the Goal gate | affordance withdrawn |
| After the session goes idle again | — | affordance returns, message still marked failed (verified) |
5. ⚠️ Worth a decision: the pre-first-token window of a Goal turn
streamingState only becomes responding once the turn produces output. Between “Goal turn started” and “first streamed token” the client still reads idle, so this PR takes the direct submit path there rather than mid-turn insertion.
I forced a 40 s time-to-first-token to make the window observable. Provider ledger for the PR arm:
02:57:18.296 → 02:57:27.655 aborted=true chunks=0 [Goal turn]
02:57:27.654 POST /session/:id/prompt → 202 ← user message lands
02:57:27.685 → 02:58:11.692 aborted=false chunks=6 [Goal turn + user message]
The in-flight Goal turn is preempted with zero tokens delivered and immediately restarted as a Goal turn that carries the user's text. On main the same message was held locally and the in-flight Goal turn ran undisturbed.
Assessment: not data loss — the message reaches the model, the Goal stays active (status: active, activity: running), nothing is dropped or duplicated. But it does discard the in-flight turn's work, and it is not the "drained into that turn" path the PR description promises for a running Goal turn. In practice the window is the provider's TTFT, so it is small but non-zero. Two options if you want it closed: route ordinary messages on the daemon's turn status rather than on client streamingState, or state the preemption explicitly in the PR description.
6. Minor: dead option left behind
holdQueuedPromptsLocally no longer has any production caller — both call sites are removed by this PR, yet packages/web-shell/client/hooks/useQueuedPrompts.ts still carries the option and 13 references whose branches are now only reachable from the hook's own tests. Worth removing in a follow-up, or keeping with a comment saying why.
Not covered
- Images / attachments through the same routing (the description claims parity; I did not exercise it).
- Multi-client and channel-origin sessions.
- Windows / Linux — macOS only.
中文版报告
本地真实环境验证报告 —— 真实 daemon + 真实浏览器，与 main 做 A/B
我在本地搭了完整栈，把本 PR 的 head（d72a093）与它的 merge base（04886c4）放在完全相同的后端条件下并排跑，逐个请求比对前端行为。结论：改动与描述一致，新增测试非空洞，未发现正确性回归。 有两点建议在合并前明确决策，证据见下。
验证栈
| 组件 | 说明 |
|---|---|
| daemon | 用 PR 源码起的真实 qwen serve（scripts/dev.js serve，单实例，:5323），真实 ACP 子进程，隔离的 QWEN_HOME，真实 workspace |
| 前端 | 两个 Vite dev server 指向同一个 daemon —— PR head 在 :5324，merge base main 在 :5325 |
| 模型 | 本地 OpenAI 兼容假服务，可控首 token 延迟与流式节奏，并逐请求记录 socket 是否被 abort、发出了多少 chunk |
| 驱动 | Playwright Chromium，逐页记录完整请求台账（POST /prompt、POST /mid-turn-message、GET /goal 等） |
| 平台 | macOS 26.6.2，Node v24.18.1 |
两条腿共用一个 daemon 和一个模型，因此下面所有差异都可归因于前端 diff。
PR head 上的静态与测试门
| 门 | 结果 |
|---|---|
packages/web-shell 全量 vitest |
4043 通过，0 失败 |
App.test.tsx + ChatPane.test.tsx + goalGate |
622 通过 |
tsc --noEmit（packages/web-shell） |
干净 |
4 个改动源文件的 eslint |
干净 |
web-shell.goal.spec.ts（Playwright，PR head） |
2 通过 |
非空洞验证： 把 PR 的测试文件拿到 main 源码上跑 |
19 失败 / 612 |
非空洞验证： 把 PR 的 e2e spec 拿到 main 源码上跑 |
在 midTurnRequests(daemon).length → 1 处失败 |
1. 空闲会话、Goal 查询仍在途 —— 本 PR 要解决的场景 ✅
重开一个已有的空闲会话，把 GET /session/:id/goal 延迟 12 秒，然后输入一条普通消息。
main |
PR #9667 | |
|---|---|---|
重开后 POST /prompt 的时刻 |
+12 149 ms（要等完整个 Goal 查询） | +336 ms |
| 是否出现排队标识 | 是 | 否 |
2. 同一窗口内的转发型 Slash 命令 ✅（更严格 —— 见备注）
main |
PR #9667 | |
|---|---|---|
| 行为 | 先排队，Goal 查询返回后转发给 daemon（POST /prompt，内容 /deploy production） |
立刻用新的 queue.commandGoalBlocked toast 拒绝；始终 0 次 POST /prompt |
| 输入框 | 被清空 | 文本保留 —— 等 hydration 结束后再次提交可成功（已验证） |
备注：在 main 上，未知窗口里输入的 slash 命令最终仍会执行；本 PR 会丢弃它，用户需要再按一次发送。这是刻意的 fail-closed 取舍，因为输入框保留了文本，代价只是一次按键 —— 列出来只是让这个取舍被明确知晓。
3. Goal turn 正在流式输出时发普通消息 ✅
Goal turn 以 1.5 秒间隔流式输出 line-1…line-40；两条腿的输入框都正确显示 esc to cancel。
main |
PR #9667 | |
|---|---|---|
| 请求 | 无 —— 本地扣留，需要用户显式点 “Insert” | 1 次 POST /mid-turn-message，标识显示 “Queued…” |
| 是否到达模型 | 用户点 Insert 之前不会 | 会 —— provider 台账显示同一次模型请求里同时含 Goal continuation context 与用户文本 |
4. Goal turn 运行期间的失败重发入口 ✅
先制造一次明确被拒的发送（413，走 isDefinitelyRejectedPromptAdmission 分支），再 resume 一个已 park 的 Goal，让 turn 在没有新用户消息的情况下启动。
main |
PR #9667 | |
|---|---|---|
| Goal turn 流式期间 | 仍然渲染 “Failed to send / Try again”；点击后 0 请求、0 mid-turn、无 toast —— 被 Goal 门挡住的死按钮 | 入口被收起 |
| 会话恢复空闲之后 | — | 入口重新出现，消息仍标记为失败（已验证） |
5. ⚠️ 建议明确决策：Goal turn 的「首 token 之前」窗口
streamingState 只有在 turn 产生输出后才变成 responding。在「Goal turn 已开始」到「第一个流式 token」之间，前端读到的仍是 idle，因此本 PR 在这个窗口里走的是直接提交路径，而不是中途插入。
我把首 token 延迟强制拉到 40 秒以便观测。PR 这条腿的 provider 台账：
02:57:18.296 → 02:57:27.655 aborted=true chunks=0 [Goal turn]
02:57:27.654 POST /session/:id/prompt → 202 ← 用户消息到达
02:57:27.685 → 02:58:11.692 aborted=false chunks=6 [Goal turn + 用户消息]
在途的 Goal turn 被在零 token 情况下抢占，随即以携带用户文本的新 Goal turn 重启。在 main 上，同一条消息被本地扣留，在途的 Goal turn 不受干扰。
评估：不是数据丢失 —— 消息到达了模型，Goal 保持 active（status: active、activity: running），既没丢也没重复。但它确实丢弃了在途 turn 已做的工作，也不是 PR 描述里承诺的「drain 到当前 turn」路径。实际窗口约等于供应商的首 token 延迟，很小但不为零。若要封掉，有两条路：普通消息按 daemon 侧 turn 状态而不是前端 streamingState 来路由；或者在 PR 描述里把这种抢占写清楚。
6. 次要：留下了一个已无调用方的选项
holdQueuedPromptsLocally 已经没有任何生产调用方 —— 两处调用点都被本 PR 删掉了，但 packages/web-shell/client/hooks/useQueuedPrompts.ts 里仍保留该选项及 13 处引用，其分支现在只能被该 hook 自己的测试触达。建议后续 PR 清理，或保留并注明原因。
未覆盖
- 图片 / 附件走同一路由（描述里声称一致，我没有实测）。
- 多客户端与 channel 来源的会话。
- Windows / Linux —— 只在 macOS 上验证。
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed — no blockers. Suggestions are inline.
Deferred under the convergence posture (round 3, not a blocker) — recorded, not requested in this round:
packages/web-shell/client/App.tsx:8925 — [probe] enqueueBlockedCommand's forwarded images/files/inputAnnotations are pinned by no test
中文说明
已审查——无阻断问题。 建议见行内评论。
收敛姿态下延后（第 3 轮，非阻断）——已记录，本轮不要求修改：共 1 条（原文未翻译，列表见上方英文部分）。
— qwen3.8-max via Qwen Code /review (v0.21.15)
| const skillPrompt = `/${skillArg}`; | ||
| if (promptBlocked) { | ||
| return enqueuePrompt( | ||
| skillPrompt, | ||
| images, | ||
| files, | ||
| undefined, | ||
| commitComposerAccepted, | ||
| metadata?.inputAnnotations, | ||
| ); | ||
| if (commandBlocked) { | ||
| return enqueueBlockedCommand(skillPrompt); |
There was a problem hiding this comment.
[Suggestion] R2-2: Round-2 ledger entry, still standing in reduced scope after re-checking at d72a093f. The Goal-blocked half is now fixed — the final commit's it.each exercises all four enqueue-style sites (hidden command via the wired hiddenSlashCommands, /model --fast, /skills, /rename --auto) with an active Goal + idle streaming, asserting refusal and the new queue.commandGoalBlocked toast. Residual gap: none of these four sites has a test submitting the command with streamingState === 'responding' and a Goal-idle session, so the enqueue branch of enqueueBlockedCommand is pinned only at the generic forwarded-command site. Concretely, replacing enqueueBlockedCommand(...) with blockCommand() at any one of the four sites leaves the whole suite green while a user who submits e.g. /skills deployer mid-turn with no Goal gets it refused with a toast instead of queued. Dropping the if (commandBlocked) guard entirely IS caught by the new Goal-active it.each; only the enqueue→block substitution survives.
Witness: probe — mutant enqueueBlockedCommand → blockCommand() at the /skills site and at all four sites simultaneously: App.test.tsx 503/503 pass (mutant survives).
Consider adding one running-turn, Goal-idle case per enqueue-style site asserting rawEnqueuePrompt is called with the command text.
中文说明
[Suggestion]（建议）R2-2：第二轮台账条目，在 d72a093f 复查后仍以缩小后的范围成立。Goal 拦截的一半已修复——最后一个提交的 it.each 已覆盖全部四个入队式命令点（通过接入的 hiddenSlashCommands 覆盖隐藏命令、/model --fast、/skills、/rename --auto），在 active Goal + streaming 空闲下断言拒绝并弹出新的 queue.commandGoalBlocked 提示。剩余缺口：这四个调用点没有任何测试在 streamingState === 'responding' 且 Goal 空闲的会话下提交命令，因此 enqueueBlockedCommand 的入队分支只在通用转发命令点被钉住。具体而言，把任一调用点的 enqueueBlockedCommand(...) 替换为 blockCommand()，整套测试依旧全绿，而用户在无 Goal 的 turn 运行中提交例如 /skills deployer 时，会收到一个 toast 拒绝而不是入队。完全删掉 if (commandBlocked) 守卫的变异会被新增的 Goal-active it.each 捕获；只有入队→拦截的替换变异能够存活。
见证：探针——在 /skills 调用点及四处调用点同时将 enqueueBlockedCommand 变异为 blockCommand()：App.test.tsx 503/503 通过（变异存活）。
建议为每个入队式调用点补一个「turn 运行中 + Goal 空闲」的用例，断言 rawEnqueuePrompt 以该命令文本被调用。
— qwen3.8-max via Qwen Code /review (v0.21.15)
| const blockCommand = useCallback((): false => { | ||
| pushToast( | ||
| 'error', | ||
| t( | ||
| isGoalGateBlocked() | ||
| ? 'queue.commandGoalBlocked' | ||
| : 'queue.commandBlocked', |
There was a problem hiding this comment.
[Suggestion] The non-Goal branch of this new toast ternary (queue.commandBlocked, "Slash commands can't be queued while a turn is running.") is asserted by no test: the string appears only in i18n.tsx, and no test blocks a command mid-turn without a Goal via any blockCommand call site (/language, /branch, /fork, /plan, /mcp install, the language picker, the fast-model picker). Emitting queue.commandGoalBlocked unconditionally instead leaves the suite green, and every command blocked during an ordinary running turn — no Goal anywhere — would show "Slash commands are unavailable while a Goal owns the session or its state is loading.", naming a Goal that does not exist.
Witness: probe — mutant blockCommand pushing queue.commandGoalBlocked unconditionally (ternary removed): App.test.tsx 503/503 pass (mutant survives); the commandBlocked string appears in no test file.
Consider one mid-turn, Goal-idle refusal test (e.g. a language-picker change or /branch with streamingState === 'responding' and a known-inactive Goal) asserting accepted === false and the queue.commandBlocked toast.
中文说明
[Suggestion]（建议）这个新 toast 三元表达式的非 Goal 分支（queue.commandBlocked，即 "Slash commands can't be queued while a turn is running."）没有任何测试断言：该字符串只出现在 i18n.tsx 中，也没有测试在无 Goal 的 turn 运行中通过任何 blockCommand 调用点（/language、/branch、/fork、/plan、/mcp install、语言选择器、fast-model 选择器）拦截过命令。若改为无条件弹 queue.commandGoalBlocked，整套测试依旧全绿，而普通运行中（完全没有 Goal）被拦截的每个命令都会显示 "Slash commands are unavailable while a Goal owns the session or its state is loading."——指向一个并不存在的 Goal。
见证：探针——将 blockCommand 变异为无条件推送 queue.commandGoalBlocked（删除三元判断）：App.test.tsx 503/503 通过（变异存活）；commandBlocked 文案未出现在任何测试文件中。
建议补一个「turn 运行中 + Goal 空闲」的拒绝用例（例如在 streamingState === 'responding' 且 Goal 已知非 active 时切换语言选择器或执行 /branch），断言 accepted === false 且弹出 queue.commandBlocked 提示。
— qwen3.8-max via Qwen Code /review (v0.21.15)
|
Addressed the zero-token in-flight gap in 54a075f. The main and split-pane composers now treat the daemon-authoritative current-session hasActivePrompt signal as active even while the client streaming state is still idle. That effective active state is also passed into the prompt queue hook, so an ordinary message reaches the real mid-turn insertion path instead of being reclassified as a direct prompt inside the hook. When hasActivePrompt returns false, routing falls back to idle/direct send as before. Retry actions are hidden and fail closed through the same active window. Goal ownership still does not affect ordinary-message routing, and shell-command behavior is unchanged. Regression coverage pins App, ChatPane, queue-hook state, state rollback, and both retry entry points; App 506/506 and ChatPane 112/112 pass. |
…uting # Conflicts: # packages/web-shell/client/components/ChatPane.tsx
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed — no blockers. Suggestions are inline.
Deferred under the convergence posture (round 4, not a blocker) — recorded, not requested in this round:
packages/web-shell/client/App.test.tsx:11069 — [probe] hydrating-window command tests do not assert the refusal toastpackages/web-shell/client/e2e/web-shell.goal.spec.ts:121 — [review] rewritten E2E smoke request-count oracles no longer catch a double-send regression
中文说明
已审查——无阻断问题。 建议见行内评论。
收敛姿态下延后（第 4 轮，非阻断）——已记录，本轮不要求修改：共 2 条（原文未翻译，列表见上方英文部分）。
— qwen3.8-max via Qwen Code /review (v0.21.15)
| goalState: connection.goalState, | ||
| }) | ||
| }); | ||
| if (commandBlockedByGoal) return false; |
There was a problem hiding this comment.
[Suggestion] ChatPane's rewritten command gate rejects a Goal-blocked slash command with a bare return false and no feedback, while the equivalent main-composer path (enqueueBlockedCommand → blockCommand in App.tsx) shows the queue.commandGoalBlocked toast, and every other refusal in this same callback calls reportError. Before this PR the same submission was visibly held in the local queue through the removed holdQueuedPromptsLocally gate, so the diff turns a visible queue entry into a silent no-op: type a slash command into a split-view/side-task pane while a Goal is active or its state is still hydrating, press Enter, and nothing happens — the text stays in the composer with no toast, no hint, no queue entry, while the identical submission in the main composer explains itself.
Witness: probe — submitting /deploy production with an active Goal and an onError spy fails against the PR code (the error callback is never called: Number of calls: 0) and passes once the reportError wiring below is applied; ChatPane.test.tsx stays 113/113 green with the fix.
| if (commandBlockedByGoal) return false; | |
| if (commandBlockedByGoal) { | |
| reportError( | |
| new Error(t('queue.commandGoalBlocked')), | |
| t('queue.commandGoalBlocked'), | |
| ); | |
| return false; | |
| } |
中文说明
[Suggestion]（建议）ChatPane 重写后的命令门在斜杠命令被 Goal 拦截时只返回 return false，没有任何反馈；而主输入框的等价路径（App.tsx 中的 enqueueBlockedCommand → blockCommand）会弹出 queue.commandGoalBlocked 提示，且本回调中其他所有拒绝分支都会调用 reportError。在本 PR 之前，同样的提交会通过已被移除的 holdQueuedPromptsLocally 门可见地进入本地队列，因此本 diff 把一个可见的队列条目变成了静默无操作：在 Goal active 或其状态仍在加载时，于 split-view/侧边任务面板中输入斜杠命令并回车，什么都不会发生——文本留在输入框中，没有提示、没有队列条目；而同样的提交在主输入框中会得到解释性提示。
见证：探针——在 active Goal 下提交 /deploy production 并挂 onError 监听：对 PR 代码探针失败（错误回调从未被调用：Number of calls: 0）；应用下方的 reportError 修复后探针通过，且 ChatPane.test.tsx 保持 113/113 全绿。
（修复代码见上方英文部分的 ```suggestion 块。）
— qwen3.8-max via Qwen Code /review (v0.21.15)
| const sessionActive = | ||
| streamingStateRef.current !== 'idle' || | ||
| sessionHasActivePromptRef.current; |
There was a problem hiding this comment.
[Suggestion] This diff routes every submission path through the new composite sessionActive — ordinary prompts (~9986), slash commands via commandBlocked (8948), both retry gates (6151, 10195), the language picker (7109) and the fast-model picker (10781) — but not the ! shell path: the shell gate (App.tsx:9930) and the shell-queue drain trigger (App.tsx:7251) still read only raw streamingState. At the merge base nothing read sessionHasActivePrompt, so every path was consistently ungated for the silent mid-turn gap; this diff moves every sibling to the composite and leaves shell out, newly creating the inconsistency.
The concrete shape: in the pre-first-stream-event window your own tests construct ('inserts a prompt before the first stream event reaches the client' — streamingState idle while the daemon still reports the in-flight prompt), submit !ls. Every other submission is held or enqueued and the UI renders the session as running, but the shell branch sees raw idle and fires sendShellCommand immediately — bridge.executeShellCommand has no turn-active check, so the output interleaves with the active turn, the exact harm the /extensions install block comment cites ("block it mid-turn so it can't split the active turn"). Queued shell commands can likewise drain the moment raw streamingState flips idle even while sessionHasActivePrompt is still true.
Witness: probe — with sessionHasActivePrompt = true and streamingState = 'idle', submitting !ls calls sendShellCommand once with ["ls"] against the PR code; with the one-line composite-gate fix below the probe passes (the command is queued instead) — a clean flip, and all 29 'App shell command queueing' tests still pass with the fix, including 'runs an idle shell command immediately while a Goal is active' (the deliberate Goal bypass is preserved).
Suggested fix (at the two read sites):
// App.tsx:9930 (the `!` gate)
if (streamingStateRef.current !== 'idle' || sessionHasActivePromptRef.current) {
// App.tsx:7251 — key the drain-start effect on queuedPromptStreamingState
// (or the same composite) instead of raw streamingStateplus a silent-gap test mirroring the prompt-enqueue one.
中文说明
[Suggestion]（建议）本 diff 把所有提交路径都改走新的组合判定 sessionActive——普通消息（约 9986 行）、经 commandBlocked 的斜杠命令（8948）、两处重试门（6151、10195）、语言选择器（7109）、快模型选择器（10781）——唯独没有包括 ! shell 路径：shell 门（App.tsx:9930）与 shell 队列 drain 触发（App.tsx:7251）仍然只读原始 streamingState。在 merge base 上没有任何代码读 sessionHasActivePrompt，所有路径对这个「静默 mid-turn 窗口」的处理是一致的（都不设防）；本 diff 把其他所有兄弟路径迁移到组合判定，唯独落下 shell，从而新制造了这个不一致。
具体场景：在本 PR 自己的测试所构造的首个流事件之前的窗口（'inserts a prompt before the first stream event reaches the client'——streamingState 为 idle 而 daemon 仍报告有在途 prompt）提交 !ls：其他所有提交都会被暂留或入队、UI 显示会话运行中，但 shell 分支看到原始 idle 就会立即触发 sendShellCommand——bridge.executeShellCommand 没有 turn-active 检查，输出会与活跃 turn 交错，正是 /extensions install 拦截注释所引用的危害（"mid-turn 拦截，避免其割裂活跃 turn"）。同样，排队的 shell 命令也会在原始 streamingState 翻回 idle 的瞬间被 drain，即使 sessionHasActivePrompt 仍为 true。
见证：探针——sessionHasActivePrompt = true、streamingState = 'idle' 时提交 !ls：对 PR 代码，sendShellCommand 被以 ["ls"] 调用一次；应用下面的一行组合门修复后探针通过（命令改为入队）——翻转干净，且修复后全部 29 个 'App shell command queueing' 测试依旧通过，包括 'runs an idle shell command immediately while a Goal is active'（有意保留的 Goal 旁路不受影响）。
建议修复（两处读取点）：App.tsx:9930 的 ! 门改为 if (streamingStateRef.current !== 'idle' || sessionHasActivePromptRef.current)；App.tsx:7251 的 drain 启动 effect 改挂 queuedPromptStreamingState（或同一组合判定），并补一个与 prompt 入队用例对应的静默窗口测试。
— qwen3.8-max via Qwen Code /review (v0.21.15)
| isPreparingPrompt || | ||
| streamingState !== 'idle' || | ||
| sessionHasActivePrompt |
There was a problem hiding this comment.
[Suggestion] The streaming-state half of this new retry gate is untested. The stale-retry tests this diff adds flip only sessionHasActivePrompt; nothing asserts retry visibility/inertness with streamingState = 'responding' while sessionHasActivePrompt is false. That matters because sessionHasActivePrompt comes from the session-catalog live-state subscription, which can lag the stream or be absent on daemons without workspace_session_live_state, while streamingState is already 'responding' — in that window this half is the only thing hiding the retry, and a failed-prompt retry clicked there would fire sendPrompt into the running turn.
Witness: probe — deleting streamingState !== 'idle' from both this gate and the matching handleFailedPromptRetry guard (~6150) keeps the whole suite green (App.test.tsx 508/508, main.test.tsx 1/1 — the mutant survives); the discriminator probe fails against the mutant (<button data-testid="failed-prompt-retry"> still rendered during streamingState = 'responding') and passes against the PR code.
Consider extending a retry test (e.g. a sibling of 'marks and retries a failed prompt while an active Goal is known') to set testState.streamingState = 'responding' with sessionHasActivePrompt false and assert the retry affordance is hidden and the captured stale onRetryFailedPrompt does not re-send.
中文说明
[Suggestion]（建议）这个新重试门的 streaming-state 一半没有任何测试覆盖。本 diff 新增的过期重试测试只翻转 sessionHasActivePrompt；没有测试断言在 streamingState = 'responding' 且 sessionHasActivePrompt 为 false 时重试入口的可见/失效。这一点要紧，因为 sessionHasActivePrompt 来自 session-catalog 的 live-state 订阅，它可能滞后于流，也可能在不支持 workspace_session_live_state 的 daemon 上缺失，而 streamingState 此时已经是 'responding'——在那个窗口里，这半个条件是唯一隐藏重试入口的东西，此时点击失败消息重试会向正在运行的 turn 触发 sendPrompt。
见证：探针——把 streamingState !== 'idle' 从本门和对应的 handleFailedPromptRetry 守卫（约 6150 行）中同时删掉，整套测试依旧全绿（App.test.tsx 508/508、main.test.tsx 1/1——变异存活）；判别探针对变异体失败（streamingState = 'responding' 期间 <button data-testid="failed-prompt-retry"> 仍然渲染），对 PR 代码通过。
建议扩展一个重试测试（例如 'marks and retries a failed prompt while an active Goal is known' 的兄弟用例）：置 testState.streamingState = 'responding' 且 sessionHasActivePrompt 为 false，断言重试入口隐藏、已捕获的过期 onRetryFailedPrompt 不会重发。
— qwen3.8-max via Qwen Code /review (v0.21.15)
| metadata?.inputAnnotations, | ||
| ); | ||
| if (commandBlocked) { | ||
| return enqueueBlockedCommand(skillPrompt); |
There was a problem hiding this comment.
[Suggestion] R2-2: Round-2 ledger entry, still standing at this round's head. The Goal-blocked half is fixed — the it.each now exercises all four enqueue-style sites (hidden command via the wired hiddenSlashCommands, /model --fast, /skills, /rename --auto) with an active Goal + idle streaming, asserting refusal and the queue.commandGoalBlocked toast. Residual gap: none of these four sites has a test submitting the command with streamingState === 'responding' and a Goal-idle session, so the enqueue branch of enqueueBlockedCommand is pinned only at the generic forwarded-command site ('enqueues a forwarded command while the session is active and Goal is idle'). Concretely, replacing enqueueBlockedCommand(...) with blockCommand() at any one of the four sites leaves the whole suite green while a user who submits e.g. /skills deployer mid-turn with no Goal gets it refused with a toast instead of queued; dropping the if (commandBlocked) guard entirely IS caught by the Goal-active it.each, only the enqueue→block substitution survives.
Witness: probe (round 3) — mutant enqueueBlockedCommand → blockCommand() at the /skills site and at all four sites simultaneously: App.test.tsx 503/503 pass (mutant survives). Re-checked at this round's head: no responding-state test exists at the four enqueue-style sites.
Consider adding one running-turn, Goal-idle case per enqueue-style site asserting rawEnqueuePrompt is called with the command text.
中文说明
[Suggestion]（建议）R2-2：第二轮台账条目，在本轮 head 上仍然成立。Goal 拦截的一半已修复——it.each 现已覆盖全部四个入队式调用点（通过接入的 hiddenSlashCommands 覆盖隐藏命令、/model --fast、/skills、/rename --auto），在 active Goal + streaming 空闲下断言拒绝并弹出 queue.commandGoalBlocked 提示。剩余缺口：这四个调用点仍没有任何测试在 streamingState === 'responding' 且 Goal 空闲的会话下提交命令，因此 enqueueBlockedCommand 的入队分支只在通用转发命令点（'enqueues a forwarded command while the session is active and Goal is idle'）被钉住。具体而言，把任一调用点的 enqueueBlockedCommand(...) 替换为 blockCommand()，整套测试依旧全绿，而用户在无 Goal 的 turn 运行中提交例如 /skills deployer 时会收到 toast 拒绝而不是入队；完全删掉 if (commandBlocked) 守卫的变异会被 Goal-active 的 it.each 捕获，只有入队→拦截的替换变异能够存活。
见证：探针（第三轮）——在 /skills 调用点及四处调用点同时将 enqueueBlockedCommand 变异为 blockCommand()：App.test.tsx 503/503 通过（变异存活）。本轮 head 复查：四个入队式调用点仍无 responding 状态测试。
建议为每个入队式调用点补一个「turn 运行中 + Goal 空闲」的用例，断言 rawEnqueuePrompt 以该命令文本被调用。
— qwen3.8-max via Qwen Code /review (v0.21.15)
|
@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: 763 passed · 0 failed · 763 total Flakiness gate: ✅ 2 changed test file(s) x 5 identical rounds, no divergence 中文 — 判定：✅ 通过 · 可合入（agent 判定）沙箱验证在隔离、无凭证的容器中执行了该 PR 的代码（与 base 构建 A/B 对照、无 mock harness 断言、定向门禁）。仅作为评审证据，不构成评审、批准或 CI 检查。 脚本断言：763 通过 · 0 失败 · 763 总计 抖动门：✅ 2 changed test file(s) x 5 identical rounds, no divergence Verification reportPR #9667 Deep Verification —
|
| Cell | Environment | Oracle | Result |
|---|---|---|---|
| head unit | HEAD worktree, vitest (mocked catalog hooks) |
App.test.tsx + ChatPane.test.tsx + goalGate.test.ts pass counts | 630/630 green (01-head-unit-green.png) |
| base unit | HEAD^1 worktree + head test files copied in |
same suites | 18 red / 490 green (App), 6 red / 106 green (ChatPane) (02-ab-chatpane-base-red.png, 03-ab-app-base-red.png) |
| head e2e | real chromium + vite dev server + scripted daemon harness | web-shell.goal.spec.ts |
2/2 pass; mid-turn insert reaches daemon with exact body (05-e2e-head-green.png) |
| base e2e | base tree, same spec copied in | same | 1 red: midTurnRequests stays 0 where head delivers 1 (06-ab-e2e-base-red.png, 07-base-e2e-failure-screenshot.png) |
| adjacent gate | head | useQueuedPrompts.midTurnReconcile.test.tsx |
88/88 green |
| typecheck | head | tsc -p tsconfig.json --noEmit |
clean |
All 24 base reds are behavioral assertion mismatches in exactly the tests this PR adds/rewrites (names listed in logs/base-app-junit.xml and the captures) — e.g. base sends nothing where head sends 'hello during hydration', base forwards a slash command during hydration (accepted === true) where head blocks it, base emits the old toast string. The control cell in the other direction also holds: inserts a hydrating composer prompt while the session is active (mid-turn enqueue) passes on both arms — pre-existing behavior preserved.
Control purity: the base worktree has no node_modules of its own (resolution walks to the root install); the only internal packages the suites load are @qwen-code/sdk and @qwen-code/webui (realpath-verified), both untouched by the diff — the PR changes nothing outside packages/web-shell/client/, no package.json/lockfile changes. The one nested node_modules inside packages/web-shell was symlinked into the base tree (identical content, lockfile unchanged).
Corrections
None — first verification round for this PR; no earlier report or bot comment to correct.
Reviewer Test Plan, per step
- Idle session, Goal still loading → immediate submit, no queued badge — ✅ unit level, both App and ChatPane (
sends an idle composer prompt while Goal state is still hydratingand pane twin; green on head, red on base). Not reproduced in a real browser (see Not covered). - Active Goal between turns → message starts a Goal turn — ✅ client side proven (
sends an idle composer prompt when an active Goal is known); the Goal-turn ownership itself is runtime behavior, out of this PR's diff (see Not covered). - Turn responding → next ordinary message inserted into the running turn — ✅ wire-proven end to end: head delivers exactly 1 mid-turn request with the message body to the scripted daemon; base delivers 0.
- Forwarded slash commands blocked while Goal unknown/active — ✅ ChatPane
blocks a forwarded slash command while Goal state is hydrating (idle|responding)+ Appdoes not enqueue a forwarded command while Goal state is hydrating+ 4×refuses an enqueue-style …while a Goal owns the session./goalitself stays usable while blocked (handled before the gate in both App and ChatPane — the user can still pause/clear). - Retry hidden during another turn; stale retry callback cannot re-submit —
⚠️ partially pinned: visibility is pinned (matrix rows M4/M5/M6), but the failed-prompt stale-callback assertion races its own async chain — see Finding 1.
Mutation matrix (vacuity check, head tree)
12 surgical guard reverts plus one positive control, each run against its targeted tests (harness/mutate.mjs, logs in logs/, 04-mutation-matrix.png):
| Mutant | Guard reverted | Result | Tests turned red |
|---|---|---|---|
| M1 | App: ordinary messages consult goalBlocked again |
killed | 3 (idle-hydration send, active-Goal send, allocated-session Goal window) |
| M2 | ChatPane: coarse gate reverted to base shape | killed | 4 (2 ordinary sends + 2 slash blocks) |
| M3 | ChatPane: only the slash-command Goal early return removed | killed | 2 slash-block cases — clean attribution within M2's set |
| M4 | handleRetry stale-callback guard removed |
killed | turn-error retry test |
| M5 | retry-hint visibility (!sessionHasActivePrompt) removed |
killed | turn-error retry test |
| M6 | failedPromptMessageId visibility condition removed |
killed | failed-prompt retry test |
| M7a/M7b | queuedPromptStreamingState promotion removed (App / ChatPane) |
killed | first-stream-event test / Insert-affordance + pane twin |
| M8 | streamingStateRef assignment moved back to useEffect |
killed | inserts a prompt when the session becomes active before effects run |
| M9 | Goal-specific toast ternary removed | killed | 7 toast tests |
| M10 | enqueueBlockedCommand Goal guard removed |
killed | 6 refuse-enqueue tests |
| M11 | handleFailedPromptRetry activity guard removed |
SURVIVED shipped test (508/508 green) — adjudicated below | |
| C1 | positive control: mid-turn enqueue disabled | killed | proves the harness catches regressions on long-pinned behavior |
No vacuous new tests: every guard except M11's is pinned by the test the commit says pins it, and M11's guard is pinned once the test oracle is fixed (Finding 1). The layered retry guards (M4/M5/M6) each fail the shared target test on their own assertion (callback no-op / hint hidden / failed-prompt button hidden), so no layer masks another — no combination row needed.
Findings
1. [Suggestion] The failed-prompt stale-retry assertion races its async chain — it pins nothing today; one-line test fix (verified)
App.test.tsx › marks and retries a failed prompt while an active Goal is known captures onRetryFailedPrompt, flips sessionHasActivePrompt = true, asserts the button disappears (pinned, M6), then invokes the stale callback and asserts no second send:
act(() => staleRetry?.());
expect(mockSessionActions.sendPrompt).toHaveBeenCalledTimes(1);Measurement:
- Removing the guard this assertion exists to protect (
handleFailedPromptRetry'sstreamingStateRef.current !== 'idle' || sessionHasActivePromptRef.current, mutant M11) leaves the entire 508-test suite green — the shipped assertion never observes the resend, becausehandleFailedPromptRetryis synchronous but the resend runs through the asyncsendPromptchain, and the assertion lands before it reachessessionActions.sendPrompt. - Same mutant + strengthened oracle → red:
expected "spy" to be called 1 times, but got 2 times— i.e. without the guard the stale callback genuinely re-submits the failed prompt into a session whose daemon prompt is active (a duplicate send). - Unmutated head + strengthened oracle → green. The fix is proven behavior-preserving and pins the axis:
await act(async () => {
staleRetry?.();
await flush();
});The production guard is correct and load-bearing; only the test is defective. Contrast: the sibling handleRetry guard (M4) is properly pinned by its test. Severity is Suggestion rather than blocker because the shipping code is right; the risk is a future regression silently passing this suite. Suggested location: packages/web-shell/client/App.test.tsx (the stale-retry block of that one test).
Observation (no action requested)
useQueuedPrompts's holdQueuedPromptsLocally machinery now has no production caller passing true (both App and ChatPane stopped passing it; only the hook's own midTurnReconcile tests exercise it, 88/88 green). This reads as deliberate API retention — flagging only so the author can confirm whether a follow-up cleanup is intended.
Not covered
- Per-commit attribution: depth-2 checkout;
git rev-list HEAD^1..HEAD^2returns 1 (graft boundary) vs 8 commits in the metadata snapshot — only the aggregateHEAD^1..HEADdiff was exercised. - Runtime Goal-ownership semantics ("submission during a running Goal turn is drained", "late insert promoted to the next Goal turn", "idle submission under an active Goal becomes a user-origin Goal turn"): daemon/runtime behavior, untouched by this PR's diff; the e2e harness's scripted daemon verifies the client's wire shape only.
- Manual-browser hydration window (Reviewer Test Plan step 1 in a literal browser): covered at unit level with mocked catalog hooks; the e2e spec has no hydrating-
goalStatescenario, so no browser-level witness of the original bug's exact window was produced. - Base A/A for the dev harness: not needed — both arms boot the identical vite+daemon stack and the first e2e test passes on both; the base red is a wire-shape reproduction through the real loopback path, not an environment artifact.
- Remaining web-shell suites beyond the touched files (full-workspace run not performed — targeted gates per the affected files plus the adjacent hook suite); visuals/mobile playwright projects; the PR's macOS-only manual testing.
- No injection-style instructions were present in the PR metadata; none to report.
Methodology
Single container (node v22.23.2), merge-ref checkout of refs/pull/9667/merge with npm ci + npm run build pre-run. A/B: git worktree add tmp/base-tree HEAD^1, head test files copied in, nested node_modules symlinked (lockfile untouched by the PR); internal dependency realpaths verified (@qwen-code/sdk, @qwen-code/webui → unchanged packages). Mutation matrix in a second HEAD worktree (harness/mutate.mjs): each mutant applied by exact unique-string replacement, targeted vitest run, file restored, git diff --stat confirmed clean after every run; full-suite adjudication for the M11 survivor and a flush-strengthened oracle probe in both directions. E2e: playwright 1.61.1 with chromium headless shell installed in-container, vite dev server + the repo's scripted-daemon harness (createWebShellDaemonScenario), asserting both wire sides (promptRequests, midTurnRequests bodies). Image evidence via scripts/verify-capture.mjs. Raw logs: logs/*.log (per-mutant), logs/base-app-run.log, logs/base-app-junit.xml, logs/mutation-results.json; harnesses in harness/.
Flakiness gate log
e2e suite, out of gate scope: packages/web-shell/client/e2e/web-shell.goal.spec.ts
rounds=5 files=2 skipped=1
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
per-file results (P=pass F=fail I=infra-exit, one letter per run):
packages/web-shell/client/App.test.tsx: PPPPP
packages/web-shell/client/components/ChatPane.test.tsx: PPPPP
verdict: pass
summary: 2 changed test file(s) x 5 identical rounds, no divergence
--- per-invocation detail (full copy in the artifact) ---
round 1 · packages/web-shell/client/App.test.tsx: P (exit 0)
round 1 · packages/web-shell/client/components/ChatPane.test.tsx: P (exit 0)
round 2 · packages/web-shell/client/App.test.tsx: P (exit 0)
round 2 · packages/web-shell/client/components/ChatPane.test.tsx: P (exit 0)
round 3 · packages/web-shell/client/App.test.tsx: P (exit 0)
round 3 · packages/web-shell/client/components/ChatPane.test.tsx: P (exit 0)
round 4 · packages/web-shell/client/App.test.tsx: P (exit 0)
round 4 · packages/web-shell/client/components/ChatPane.test.tsx: P (exit 0)
round 5 · packages/web-shell/client/App.test.tsx: P (exit 0)
round 5 · packages/web-shell/client/components/ChatPane.test.tsx: P (exit 0)
Evidence images
Harness scripts and raw logs are in the workflow run artifacts (7-day retention).
— Qwen Code · sandboxed verification
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
LGTM, looks ready to ship. ✅
chiga0
left a comment
chiga0
left a comment
There was a problem hiding this comment.
Scope: Ordinary-message routing + Goal gate separation in App.tsx and ChatPane.tsx.
CI: ubuntu-latest Test ✅ · E2E Smoke ✅ · Visual regression ✅. Windows/macOS tests not run; no platform-dependent code in this diff.
Core logic checked:
- Ordinary messages now gate on
sessionActive = streamingState !== 'idle' || sessionHasActivePromptRef.current— Goal state is no longer a barrier. Verified both App.tsx (~line 9986) and ChatPane.tsx (~line 815) follow this consistently. - Slash commands gate on
commandBlocked = sessionActive || goalBlockedin App.tsx; ChatPane gates them with a directisGoalGateBlockedcheck and returns false silently (confirmed R4-1 — not a regression; old ChatPane behavior was also silent for this case). holdQueuedPromptsLocallyfully removed from App.tsx at head; replaced byqueuedPromptStreamingStatesynthesizing 'responding' during Goal-active/session-idle window. Queued prompts now show Insert rather than a local hold.streamingStateRefupdate moved fromuseEffectto synchronous render assignment — correct pattern for a ref that mirrors state.- Retry UI (
showCurrentRetryHint,failedPromptMessageId) hidden whensessionHasActivePrompt— matches PR description's stated intent. - E2E smoke test rewritten to match new routing semantics: submit after
assistantTextEventfires (turn actually running) and assert mid-turn insertion.
No blockers. Existing CI bot round-4 suggestions (R4-1/R4-2/R4-3/R2-2) are all test-coverage improvements, none structural.
Reviewed with AI assistance.
doudouOUC
left a comment
doudouOUC
left a comment
There was a problem hiding this comment.
审查结果：无新阻塞项
对 PR #9667 的完整审查已完成。该 PR 已通过多个审查轮次，最新的 qwen-code-ci-bot 已 APPROVE。
验证结果
-
现有 3 个 blocker 线程：
isGoalGateBlocked()从 retry 路径移除 — 已在71056e284a补充 active Goal 覆盖测试（thread 3831225116，作者已回复）- ChatPane 命令门仅测试 hydrating 场景 — 已在
71056e284a扩展为 idle/responding 两种 streaming state（thread 3831179663，作者已回复） commandBlocked分支测试覆盖不全 — 已在d72a093f补充四个 enqueue 站点的it.each测试（thread 3834665229，未回复但已修复）
-
代码审查要点：
queuedPromptStreamingState公式正确：streamingState === 'idle' && sessionHasActivePrompt ? 'responding' : streamingStatesessionHasActivePromptRef同步更新（非 useEffect），避免回调闭包过期enqueueBlockedCommand正确区分 Goal 阻塞（toast）与会话活跃（入队）blockCommand根据isGoalGateBlocked()显示差异化 toasthandleRetry移除isGoalGateBlocked()，改用sessionHasActivePromptRef.current— 设计正确goalGate.tsJSDoc 准确反映收窄后的作用域
-
未发现新 Critical 问题。代码变更设计合理，测试覆盖充分。
仍需跟踪的已记录建议（非阻塞）
- ChatPane 命令门在 Goal 阻塞时无 toast 提示（thread 3835642174）
commandBlocked测试仅在 2/5+ 命令站点覆盖（thread 3834665229）- retry 门的 streaming state 半分支未测试（thread 3835642183）
queue.commandBlockedtoast 字符串未被任何测试断言（thread 3834950588）
Reviewed by Qwen Code
Re-verification of
|
| # | Item | Status on 2bf1f50 |
|---|---|---|
| 1 | Idle session, Goal read in flight | ✅ unchanged (+336 ms vs main +12 149 ms) |
| 2 | Slash command in the hydration window | ✅ unchanged (toast, text preserved, 0 × POST /prompt) |
| 3 | Ordinary message during a streaming Goal turn | ✅ unchanged (1 × POST /mid-turn-message) |
| 4 | Failed-send retry during a Goal turn | ✅ unchanged (withdrawn while busy, restored when idle) |
| 5 | Pre-first-token preemption | ✅ fixed |
| 6 | Dead holdQueuedPromptsLocally option |
➖ still there |
| 7 | Message dropped when the live-state read is stale at turn end | |
| 8 | Residual preemption window ≈ one live-state poll at turn start |
Gates on 2bf1f50
packages/web-shell vitest 4097 passed / 195 files, 0 failed · tsc --noEmit clean · eslint clean on the changed files · web-shell.goal.spec.ts 2 passed · PR test files against main source: 24 failed / 620 (was 19 — the new active-prompt cases are non-vacuous too).
5. Fixed ✅
Same probe as before: a Goal turn that emits no token for 40 s. The composer now reads esc to cancel in that window, the message takes POST /mid-turn-message, and the provider ledger shows the in-flight Goal turn is not aborted.
7. ⚠️ New: a message can be dropped when the live-state read is stale at turn end
Repro. Delay GET /workspaces/:cwd/sessions/live-state by 5 s (models a slow or queued live-state read), run a turn, poll GET /session/:id/status until the daemon reports hasActivePrompt: false, then submit an ordinary message ~0.26 s later.
main |
PR 2bf1f50 |
|
|---|---|---|
| Route | POST /prompt |
POST /mid-turn-message |
| Daemon answer | 202 | HTTP 200 {"accepted": false} |
| Reached the model | yes | no |
| In the transcript | yes | no |
| Left in the queue / composer | — | no — gone |
| User sees | nothing | toast: “Daemon rejected mid-turn message” |
Cause. useQueuedPrompts already has the right fallback for a rejected insert:
if (!result.accepted) {
…
// "If the turn has meanwhile ended, send the message through the ordinary
// path … instead of dropping it."
if (targetIsCurrent() && latestStreamingStateRef.current === 'idle') { … re-send … }
// else: drop the row, reconcile, reportError('Daemon rejected mid-turn message')
}latestStreamingStateRef is fed by the hook's streamingState option, and 54a075f now supplies the inflated queuedPromptStreamingState. Inside the stale window it reads 'responding', so the fallback is skipped and the else branch discards the message. Suggested fix: hand the hook the raw streamingState for that fallback (or pass raw and effective separately) so accepted:false still falls back to a direct send when the session is genuinely idle.
Reachability. With a local daemon answering live-state in <1 ms I could not hit it naturally — 5/5 attempts submitting 108–328 ms after the daemon reported turn end routed correctly, because turnCompleted → recordSessionActivity settles fast. It needs a slow/queued live-state response. Worth noting though that when live-state is enabled turnCompleted deliberately does not patch hasActivePrompt: false (only recordSessionActivity), so there is no optimistic clear — a slow read is the only ingredient needed, and the drop is silent apart from an error toast.
8. ⚠️ Residual: the turn-start window is now one live-state poll, not zero
SESSION_LIVE_STATE_POLL_MS = 2000, and the optimistic hasActivePrompt: true patch in promptAdmitted only fires for client-initiated prompts — a Goal turn gets no such patch, so at Goal-turn start the client depends entirely on the next poll. With a Goal turn that emits no token for 20 s, resuming a parked Goal and submitting after N ms:
| submit at | route | in-flight Goal turn |
|---|---|---|
| +0.50 s | POST /prompt |
aborted, 0 tokens delivered |
| +1.20 s | POST /prompt |
aborted, 0 tokens delivered |
| +2.04 s | POST /mid-turn-message |
intact |
| +3.15 s | POST /mid-turn-message |
intact |
So the exposure went from “the whole time-to-first-token” to “up to one 2 s poll after a Goal turn starts”. Much better; your call whether that is small enough to leave, or whether the Goal runtime should publish turn start the way promptAdmitted does.
c5573a9 (settings commands during active prompts)
Its two new cases (blocks fast-model selection / settings language change before the first stream event reaches the client) both fail against main source, so they are non-vacuous. I did not drive the Settings panel's fast-model and language pickers in the real browser — that path is unit-test-only in this report.
Not covered
Images/attachments through the same routing · multi-client and channel-origin sessions · Windows/Linux.
中文版报告
对 2bf1f50 的复验（新增 54a075f + c5573a9）
沿用同一套栈并在新 SHA 上重建：一个从 PR 源码起的真实 qwen serve daemon，两个 Vite 前端指向它（PR head 2bf1f50 在 :5324，merge base main 6bbb273 在 :5325），本地 provider 可控首 token 延迟并记录请求是否被 abort，Playwright 同时驱动两条腿。
结论：我上次报的缺口在主要场景下确实已修复，1–4 项复现一致无回归。但发现了一条新的消息丢失路径，以及一个残留窗口。
| # | 项目 | 在 2bf1f50 上的状态 |
|---|---|---|
| 1 | 空闲会话、Goal 查询在途 | ✅ 一致（+336 ms，main 为 +12 149 ms） |
| 2 | hydration 窗口内的 slash 命令 | ✅ 一致（toast、文本保留、0 次 POST /prompt） |
| 3 | Goal turn 流式期间的普通消息 | ✅ 一致（1 次 POST /mid-turn-message） |
| 4 | Goal turn 期间的失败重发入口 | ✅ 一致（忙时收起，空闲后恢复） |
| 5 | 首 token 之前的抢占 | ✅ 已修复 |
| 6 | 无调用方的 holdQueuedPromptsLocally |
➖ 仍在 |
| 7 | turn 结束时 live-state 陈旧会丢消息 | |
| 8 | turn 开始处残留约一个 live-state 轮询的抢占窗口 |
2bf1f50 上的各项门
packages/web-shell vitest 4097 通过 / 195 个文件，0 失败 · tsc --noEmit 干净 · 改动文件 eslint 干净 · web-shell.goal.spec.ts 2 通过 · 把 PR 的测试文件放到 main 源码上跑：24 失败 / 620（此前为 19，说明新增的 active-prompt 用例同样非空洞）。
5. 已修复 ✅
同一个探针：Goal turn 在 40 秒内不产出任何 token。现在这个窗口里输入框显示 esc to cancel，消息走 POST /mid-turn-message，provider 台账显示在途的 Goal turn 没有被 abort。
7. ⚠️ 新增：turn 结束时 live-state 陈旧会导致消息被丢弃
复现：把 GET /workspaces/:cwd/sessions/live-state 延迟 5 秒（模拟一次缓慢或排队的 live-state 读），跑一个 turn，轮询 GET /session/:id/status 直到 daemon 报 hasActivePrompt: false，随后约 0.26 秒提交一条普通消息。
main |
PR 2bf1f50 |
|
|---|---|---|
| 路由 | POST /prompt |
POST /mid-turn-message |
| daemon 应答 | 202 | HTTP 200 {"accepted": false} |
| 是否到达模型 | 是 | 否 |
| 是否进入转录 | 是 | 否 |
| 是否留在队列/输入框 | — | 否 —— 已消失 |
| 用户看到 | 无 | toast：“Daemon rejected mid-turn message” |
成因：useQueuedPrompts 本来就有针对插入被拒的兜底：
if (!result.accepted) {
…
// “若此时 turn 已结束，就改走普通发送路径……而不是丢弃它。”
if (targetIsCurrent() && latestStreamingStateRef.current === 'idle') { … 重发 … }
// 否则：删掉该行、reconcile、reportError('Daemon rejected mid-turn message')
}latestStreamingStateRef 来自该 hook 的 streamingState 入参，而 54a075f 现在传入的是被抬高的 queuedPromptStreamingState。在陈旧窗口里它读到 'responding'，于是兜底被跳过，走到 else 分支把消息丢掉。建议修法：这条兜底改用原始 streamingState（或把 raw 与 effective 分别传入），使得会话真正空闲时 accepted:false 仍能回落到直接发送。
可达性：本地 daemon 的 live-state 响应不到 1 ms，自然情况下我复现不出来 —— 在 daemon 报 turn 结束后 108–328 ms 提交的 5 次尝试全部路由正确，因为 turnCompleted → recordSessionActivity 收敛很快。它需要一次缓慢/排队的 live-state 响应。但要注意：启用 live-state 时 turnCompleted 刻意不打 hasActivePrompt: false 的乐观补丁（只做 recordSessionActivity），所以客户端没有乐观清零，只要读变慢就足以触发，而丢弃过程除了一条错误 toast 之外是无声的。
8. ⚠️ 残留：turn 开始处的窗口变成了一个 live-state 轮询周期
SESSION_LIVE_STATE_POLL_MS = 2000，而 promptAdmitted 里那次 hasActivePrompt: true 的乐观补丁只对客户端发起的 prompt 生效 —— Goal turn 没有这个补丁，因此 Goal turn 启动时客户端完全依赖下一次轮询。让 Goal turn 在 20 秒内不产出 token，resume 一个已 park 的 Goal，然后在 N 毫秒后提交：
| 提交时刻 | 路由 | 在途 Goal turn |
|---|---|---|
| +0.50 s | POST /prompt |
被 abort，0 token |
| +1.20 s | POST /prompt |
被 abort，0 token |
| +2.04 s | POST /mid-turn-message |
完好 |
| +3.15 s | POST /mid-turn-message |
完好 |
也就是说暴露面从「整个首 token 延迟」缩到了「Goal turn 启动后最多一个 2 秒轮询」。改善很大；是否可以就这样保留，还是应当让 Goal 运行时像 promptAdmitted 那样主动发布 turn 启动，由你判断。
c5573a9（活跃 prompt 期间拦截设置类命令）
它新增的两个用例（blocks fast-model selection / settings language change before the first stream event reaches the client）在 main 源码上都会失败，因此非空洞。我没有在真实浏览器里驱动设置面板的 fast-model 与语言选择器 —— 这条路径在本报告中仅由单测覆盖。
未覆盖
图片/附件走同一路由 · 多客户端与 channel 来源的会话 · Windows / Linux。
|
Released in v0.22.0. |
5 participants
What this PR does
Ordinary Web Shell messages now follow session activity instead of Goal hydration or Goal status: an idle session submits immediately, while a running session uses mid-turn insertion. Goal state remains a fail-closed guard for slash commands and scheduled/manual run controls, and failed-message retry is unavailable while another turn is running.
This does not change Goal ownership in the runtime. An ordinary submission made while an active Goal is between turns becomes a user-origin Goal turn; a submission made during a running Goal turn is drained into that turn when possible, and a late undrained insert is promoted to the next Goal turn. Supported images and attachments follow the same runtime paths.
Why it's needed
Opening an old idle session creates a window where the transcript is usable but the separate Goal read has not settled. The previous client-side Goal hold treated that unknown state as active ownership, so a normal message appeared queued even though no turn was running. Message admission should not wait for an auxiliary Goal read.
Reviewer Test Plan
How to verify
Open an idle session with Goal state still loading and send an ordinary message; it should submit immediately without a queued badge. Repeat with an active Goal between turns and confirm the message starts a Goal turn. While a turn is responding, send another ordinary message and confirm it is inserted into the running turn. Confirm forwarded slash commands remain blocked while Goal state is unknown or active. Create a retryable send failure, start another turn, and confirm retry is hidden and a stale retry callback cannot submit again.
Evidence (Before & After)
Before: an idle session could show a newly submitted ordinary message as queued solely because Goal state was unresolved or active. After: ordinary messages are direct when idle and inserted when running; Goal state no longer changes ordinary-message admission.
Tested on
Environment (optional)
Web Shell and ACP bridge unit tests on Node.js 22 workspace dependencies.
Risk & Scope
Linked Issues
Related PR: #9393
中文说明
此 PR 做了什么
Web Shell 的普通消息现在只根据会话活动状态路由，不再依赖 Goal 是否加载完成或 Goal 状态：会话空闲时立即提交，会话运行中时使用中途插入。Goal 状态仍然以 fail-closed 方式限制斜杠命令以及定时/手动运行控制；当另一个 turn 正在运行时，失败消息的重试入口不可用。
这不会改变运行时对 Goal 的归属处理。active Goal 处于两个 turn 之间时，普通提交会成为用户触发的 Goal turn；Goal turn 正在运行时，提交会在可行时被 drain 到当前 turn；太晚而未被当前 turn 消费的插入会提升为下一个 Goal turn。受支持的图片和附件遵循相同的运行时路径。
为什么需要
打开一个很久未使用的空闲会话时，会出现 transcript 已可用但独立 Goal 查询尚未结束的窗口。之前前端的 Goal hold 会把未知状态当作 Goal 正在占用，因此即使没有 turn 在运行，普通消息也会显示为排队中。消息准入不应该等待辅助的 Goal 查询。
Reviewer 测试计划
如何验证
打开一个 Goal 状态仍在加载的空闲会话并发送普通消息，消息应立即提交且不出现排队标识。对处于两个 turn 之间的 active Goal 重复操作，确认消息会启动 Goal turn。在 turn 正在响应时发送另一条普通消息，确认它被插入当前 turn。确认 Goal 状态未知或 active 时，转发的斜杠命令仍被阻止。制造可重试的发送失败，再启动另一个 turn，确认重试入口隐藏且旧重试回调不会重复提交。
证据（修改前后）
修改前：空闲会话可能仅因 Goal 状态未知或 active，就把新提交的普通消息显示为排队中。修改后：普通消息空闲时直接发送、运行中中途插入，Goal 状态不再改变普通消息准入。
测试环境
环境（可选）
使用 Node.js 22 工作区依赖运行 Web Shell 和 ACP bridge 单元测试。
风险与范围
关联 Issue
关联 PR：#9393