fix(web-shell): fall back after stale mid-turn rejection - #9732
Conversation
E2E test reportTested the reported missing-session flow against the current branch and a clean Sequence and observed requests:
The exact four-step missing-session flow behaves the same on clean Environment: macOS, local Vite Web Shell, mock daemon. |
|
✅ Qwen Triage finished — CI landed green on ✅ Qwen Triage 已完成 —— |
|
Thanks for the PR!
Moving on to code review. 🔍 中文说明感谢贡献!
进入代码审查 🔍 — Qwen Code · qwen3.8-max Reviewed at |
Code reviewMy independent take before reading the diff: the flattened What I verified in the code:
No blockers found. Test evidenceEvidence carried: the PR's own CI on the reviewed commit, fetched once via the API (in-flight checks reported as pending, not polled). The Ubuntu unit suite — which contains this PR's new tests — and the web-shell visual capture are still running; everything completed so far is green, and the macOS/Windows unit legs plus CLI integration were skipped on this commit, not failed. Final CI results for
One row per check name (latest run); skipped checks omitted; failures sort first. / 每个检查名一行(取最新一次运行),省略 skipped,失败项排在最前。 Sandboxed verification would settle the remaining gap: 中文说明代码审查:我在看 diff 之前的独立判断是——压平后的 已核对: 测试证据:引用的是该 commit 自身 CI(经 API 一次性拉取,未轮询)。Ubuntu 单测套件(含本 PR 新增测试)与 web-shell 视觉采集仍在运行;已完成的全部为绿(双平台 Desktop Shell、依赖 CVE 审计、密钥扫描、PR 分类),macOS/Windows 单测与 CLI 集成测试在该 commit 上为跳过(skipped),不是失败。 沙箱验证建议:降级行为目前由 mock daemon 的单测钉住,作者仅在 macOS 上验证过; — 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, minimal fix for a documented race; the remaining point is that the timing claim rests on mocked-daemon unit tests plus the author's macOS run, which is exactly what the This is the kind of follow-up fix that's easy to under-rate: it adds no new machinery, it just stops discarding information that was already available. The flattened What settled it for me: the predicate re-keying is exactly equivalent at every site except the one intended change; the failure mode is bounded (a rejected fallback submit restores the text to the editor and surfaces the normal error — no retry loop, no silent drop); both panes are covered; and the tests pin the flattening removal at the App/ChatPane layer and the fallback plus annotation preservation at the hook layer. The Approval is deferred until CI lands green on 中文说明置信度:4/5 —— 对一个有记录的竞态来说,这是干净且最小的修复;扣掉的 1 分在于时序结论目前依赖 mock daemon 的单测和作者仅在 macOS 上的验证,这正是上方 这是一个容易被低估的后续修复:它没有引入任何新机制,只是不再丢弃本来就有的信息。压平后的 让我信服的点:谓词重锚在除唯一目标位置外的每一处都严格等价;失败面有界(降级提交被拒时文本会还原到编辑器并正常报错——无重试循环、无静默丢失);主面板与分屏都覆盖了;测试在 App/ChatPane 层钉住了压平的移除、在 hook 层钉住了降级与 annotation 保留; 审批延迟到 CI 在 — Qwen Code · qwen3.8-max Reviewed at |
ytahdn
left a comment
There was a problem hiding this comment.
Review — fix(web-shell): fall back after stale mid-turn rejection
Static review of the single commit a0244078 (App.tsx, ChatPane.tsx, useQueuedPrompts.ts + three test files, ~64 production lines). No tests/builds were run; every claim below was re-checked against the head tree.
对单提交 a0244078 的纯静态审查(App.tsx、ChatPane.tsx、useQueuedPrompts.ts 及三个测试文件,约 64 行生产逻辑)。未运行任何测试/构建,以下每条结论均已在 head tree 上复核。
What I verified / 已核实的内容
The fix is surgically minimal. / 修复收敛得很干净。 The raw-vs-derived split is done precisely: a new latestRawStreamingStateRef is read at exactly one site — the outright-rejection fallback in useQueuedPrompts.ts (~line 1805). Every other decision point (sessionActive / latestSessionActiveRef) preserves the previous flattened semantics. I audited all references to confirm nothing else silently switched from flattened to raw, and nothing still reads the removed flattened value.
raw 与派生状态的拆分非常克制:新增的 latestRawStreamingStateRef 只在一处被读取——useQueuedPrompts.ts(约 1805 行)的明确拒绝降级分支。其余所有判定点(sessionActive / latestSessionActiveRef)都保持原先的压平语义。我逐条核对了全部引用,确认没有别处悄悄从压平态切到 raw 态,也没有残留对已删除压平值的读取。
Keying the fallback on raw-idle is the correct asymmetry. / 降级以真实 idle 为条件是正确的不对称设计。 A definitive accepted:false while raw SSE is idle means the turn genuinely ended and the live-state hasActivePrompt was stale — so resubmitting through the ordinary path is right. While genuinely streaming (raw responding), the rejection still surfaces as before, which matches the stated intent and avoids masking a real mid-turn refusal.
当真实 SSE 已 idle 却收到明确的 accepted:false,说明轮次确实已结束、live-state 的 hasActivePrompt 是过期信号——此时改走普通路径重发是正确的。而真正在流式输出(raw responding)时,拒绝仍按原样上抛,与 PR 意图一致,也不会掩盖真实的轮中拒绝。
"Falls back once" is structurally guaranteed. / "只降级一次"在结构上得到保证。 The rejected admission is removed from pendingMidTurnAdmissionsRef and requeued as an ordinary pending prompt (not a mid-turn admission), so there is no path that re-enters the mid-turn insert and loops. The drain effect is gated on sessionActive, and it only re-releases prompts carrying midTurnFailedAction or locally-held prompts, so it cannot double-send the fallback row.
被拒的 admission 会从 pendingMidTurnAdmissionsRef 移除,并以普通待提交 prompt(而非 mid-turn admission)重新入队,因此不存在再次进入 mid-turn 插入并循环的路径。drain 逻辑以 sessionActive 为闸门,且只重新释放携带 midTurnFailedAction 或本地保留的 prompt,不会对降级行二次发送。
Payload fidelity on the ordinary path. / 普通路径上的载荷保真。 Switching the fallback spread from pendingAdmission to restoreAdmission is the right call: restoreAdmission carries the original trimmed text plus inputAnnotations and payloadCompleteness:'complete' rather than the mid-turn display form (displayText + summary-only). The dedicated test asserts the original text and the @file annotation both reach submitPrompt, and the previously-uploaded mid-turn attachments are cleaned up first.
降级分支的展开对象从 pendingAdmission 换成 restoreAdmission 是对的:restoreAdmission 携带原始 trimmed 文本、inputAnnotations 与 payloadCompleteness:'complete',而不是 mid-turn 的展示形态(displayText + summary-only)。专门的测试断言了原始文本与 @file 标注都进入了 submitPrompt,且此前为 mid-turn 上传的附件会先被清理。
Tests track the new behavior. / 测试与新行为一致。 The zero-token-window tests now assert the raw state is reported idle with the active-prompt flag true (instead of a synthetic responding); the new hook tests cover the fallback, annotation preservation, and no-resubmit-on-late-accept; the missing-session test confirms the first message after recovery uses the ordinary path. Read-only confirmation only.
零 token 窗口的测试现在断言 raw 状态上报为 idle 且 active-prompt 标志为 true(而非人工的 responding);新增的 hook 测试覆盖了降级、标注保真、以及迟到 accept 不重发;missing-session 测试确认恢复后首条消息走普通路径。仅静态确认。
Observations / 观察
- The acknowledged tradeoff —
accepted:falsedoesn't say why the daemon rejected, so a wrong guess costs one visible ordinary-send failure rather than silent loss — is reasonable, and the ordinary resubmission keeps its own daemon-side admission guard. This matches the PR's own Risk section; nothing more is needed here.
PR 已坦承的权衡——accepted:false不说明 daemon 拒绝的原因,因此误判的代价是一次可见的普通发送失败而非静默丢消息——是合理的,且普通重提仍带 daemon 端准入守卫。这与 PR 自述的风险一致,此处无需再加东西。 - CI note: at review time the head's
Test (ubuntu-latest)was still in progress (visuals already green). Worth letting it land before merge; nothing in this review anticipates a failure.
CI 说明:评审时 head 的Test (ubuntu-latest)仍在运行(visuals 已绿)。建议等其通过再合并;本轮审查未发现预示失败的迹象。
Praise / 值得肯定
-
Minimal, well-targeted change: passing the two signals separately instead of flattening is exactly the right fix, and it avoids the daemon-side alternative that was correctly scoped out.
-
The raw-vs-derived discipline is easy to get wrong and was not gotten wrong — a single, clearly-intended raw read.
-
Solid regression coverage for a race that is inherently hard to pin down, including the annotation-preservation edge.
-
改动最小且目标精准:把两个信号分开下传而非压平,正是恰当的修法,也正确地排除了 daemon 端方案。
-
raw 与派生状态的边界很容易写错,但这里没有写错——只有一处、意图清晰的 raw 读取。
-
对一个本质上难以复现的竞态给出了扎实的回归覆盖,包括标注保真这一边界。
Conclusion / 结论
💬 COMMENT — no blockers. This is a clean, correct, well-tested fix for the stale mid-turn rejection race. Approve is available if you want it.
💬 COMMENT——无阻塞项。这是对过期 mid-turn 拒绝竞态的一次干净、正确、测试充分的修复。如需批准请告知。
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. |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
LGTM, looks ready to ship — CI landed green after the review. ✅
chiga0
left a comment
There was a problem hiding this comment.
Scope: mid-turn fallback path; removal in App.tsx and ChatPane.tsx.
CI: ubuntu-latest Test ✅ · E2E Smoke ✅ · Visual regression ✅. review-pr was pending at review time (CI automation, not build/test). Windows/macOS tests SKIPPED; no platform-specific code touched.
Checked:
- Fallback condition uses
latestRawStreamingStateRef.current === 'idle'(raw SSE state only), not the compositesessionActive. This is the key design decision: fall back only when the session has definitively gone idle on the wire, not just becausesessionHasActivePromptis stale. Code matches the PR description. - The fallback re-queues with
restoreAdmission(trimmed text +inputAnnotations) rather thanpendingAdmission(display text + uploaded attachment files). Correct for the ordinary submit path — the test "preserves file annotations when a live-state insert falls back" pins this. - Uploaded attachments are cleaned up via
removeUploadedAttachments()before the fallback re-queue, so no stale media references on the ordinary path. - All other "session idle" guards (drain effect, settledAtIdle, deletePrompt) are correctly updated to use the composite
latestSessionActiveRef, keeping the queued-prompt state machine consistent withsessionHasActivePrompt. queuedPromptStreamingStatesynthetic var removed from both App.tsx and ChatPane.tsx; logic moved inside the hook. Callers now pass rawstreamingState+sessionHasActivePromptseparately — cleaner contract.- Double-submission risk: the re-queued prompt gets
midTurnState: undefinedandmidTurnMessageId: undefined, soshouldInsertMidTurncannot pick it up for a second mid-turn attempt. Test confirmssubmitPromptcalled exactly once.
No blockers.
Reviewed with AI assistance.
|
Released in v0.22.2. |
What this PR does
This change keeps the raw SSE streaming state separate from the daemon-authoritative active-prompt signal. An active-prompt signal still causes an ordinary message to try mid-turn insertion first. If the daemon definitively rejects that insertion while the raw SSE state is already idle, Web Shell submits the message once through the ordinary prompt path instead of showing a rejection toast. The fallback preserves text, images, files, file annotations, and completion callbacks in both the main chat and split panes.
Why it's needed
The live session snapshot can briefly continue reporting an active prompt after the turn has already become idle. Previously Web Shell converted that combination into a synthetic responding state, which discarded the raw idle information. A message submitted during that window tried mid-turn insertion, and a definitive idle rejection from the daemon surfaced as
Daemon rejected mid-turn messageinstead of sending the user's message normally.Reviewer Test Plan
How to verify
With a session whose raw SSE state is idle while live state still reports an active prompt, submit an ordinary text message and have the daemon reject the mid-turn admission. Confirm Web Shell first attempts mid-turn insertion, then submits exactly one ordinary prompt without an error toast. Repeat with an
@filereference and confirm the original text and annotation reach the ordinary prompt path. With a genuinely responding session, reject the mid-turn admission and confirm Web Shell retains the existing rejection behavior rather than falling back. From a 404 or 410 missing-session screen, create a new conversation and submit its first message; confirm it uses the ordinary prompt path and does not attempt mid-turn insertion.Evidence (Before & After)
Before:
streamingState=idleplushasActivePrompt=truewas flattened to a synthetic responding state. A definitive mid-turn rejection produced an error toast and did not retry through the ordinary prompt path.After: the same state still attempts mid-turn insertion first, but a definitive rejection falls back once when the raw SSE state is idle. Genuine streaming rejections continue to surface normally.
Tested on
Environment (optional)
Local Web Shell unit tests, TypeScript typecheck, ESLint, production build, and browser-level missing-session flow against the mock daemon.
Risk & Scope
accepted:falsedoes not identify whether the daemon rejected because the session became idle, was closing, or reached another admission limit. The fallback is therefore deliberately limited to cases where the raw SSE state is idle; an ordinary submission can still return its normal error if the session cannot accept it.Linked Issues
Related: #9667
中文说明
本 PR 做了什么
本改动将真实 SSE 流状态与 daemon 权威的 active-prompt 信号分开保存。active-prompt 信号仍会让普通消息优先尝试 mid-turn 插入。如果 daemon 明确拒绝该插入,且真实 SSE 状态已经是 idle,Web Shell 会改用普通 prompt 路径提交一次,而不是展示拒绝错误。该降级路径在主会话和分屏会话中都会保留文本、图片、附件、文件 annotation 和完成回调。
为什么需要
在 turn 已经进入 idle 后,live session snapshot 可能短暂继续报告 active prompt。之前 Web Shell 会将这个组合转换为人工的 responding 状态,从而丢失真实的 idle 信息。用户在该窗口期间发送消息时,前端会尝试 mid-turn 插入;当 daemon 明确以 idle 拒绝时,界面会显示
Daemon rejected mid-turn message,而不是正常发送用户消息。Reviewer 测试计划
验证方法
在真实 SSE 状态为 idle、但 live state 仍报告 active prompt 的会话中提交普通文本,并让 daemon 拒绝 mid-turn admission。确认 Web Shell 先尝试 mid-turn 插入,然后只提交一次普通 prompt,且不显示错误 toast。使用
@file引用重复测试,确认原始文本和 annotation 进入普通 prompt 路径。在真正 responding 的会话中拒绝 mid-turn admission,确认 Web Shell 保留现有的拒绝处理,而不是降级发送。从 404 或 410 的会话不存在页面新建会话并提交第一条消息,确认该消息走普通 prompt,而不是 mid-turn 插入。前后对比证据
修复前:
streamingState=idle加hasActivePrompt=true会被合并为人工的 responding 状态。mid-turn 被明确拒绝后会显示错误 toast,也不会通过普通 prompt 路径重试。修复后:同样的状态仍会先尝试 mid-turn 插入,但在被明确拒绝后,若真实 SSE 状态为 idle,则降级发送一次。真正 streaming 时的拒绝仍然正常显示。
测试环境
环境(可选)
本地 Web Shell 单元测试、TypeScript typecheck、ESLint、生产构建,以及基于 mock daemon 的会话不存在浏览器流程。
风险与范围
accepted:false不会指明 daemon 是因为 session 变为 idle、正在关闭,还是达到了其他 admission 限制而拒绝。因此降级严格限定在真实 SSE 状态为 idle 的情况;如果 session 无法接收消息,普通提交仍可能返回其正常错误。关联问题
相关:#9667