feat(web-shell): insert active-turn messages by default - #8224
Closed
ytahdn wants to merge 1 commit into
Closed
Conversation
Collaborator
|
⏳ Approval still deferred — 1 PR CI workflow run(s) still in progress for ⏳ 审批仍在延迟中 —— |
Collaborator
|
Thanks for the PR! Template looks good ✓ Problem: this is a UX enhancement, not a bug fix — sending during an active turn currently lands in the ordinary queue and needs a second "Insert" click to reach the running turn. That friction is real and observable (it's the explicit point of the removed Insert action), and the change builds on the queue-visibility work from #8065 and supersedes #8210. Not theoretical hardening. Direction: aligned. "Send while a turn is running = make it available to that turn" is the intuitive meaning of send, and making injection the default removes a step users had to learn. It stays inside the Web Shell package, reuses the existing daemon mid-turn API (the old Insert button already called Size: not applicable — no core paths ( Approach: the scope feels right. Extracting Risk: no elevated risk signals — none of the changed files match the high-revert paths. One thing worth a maintainer's product judgment (not a blocker): this changes a default behavior and removes a previously-shipped explicit action, so it's the kind of UX call a human may want to eyeball. The daemon contract itself is unchanged, which keeps the blast radius small. Moving on to code review. 🔍 中文说明感谢贡献！ 模板完整 ✓ 问题：这是一个 UX 增强，而非 bug 修复——回合运行中发送的消息目前会进入普通队列，需要再点一次"插入"才能送达当前回合。这个摩擦是真实可见的（被移除的插入操作正是为此而设），改动基于 #8065 的队列可见性工作，并替代 #8210。不属于理论性加固。 方向：对齐。"回合运行中发送 = 让当前回合看到它"是 send 的直觉含义，把插入设为默认省去了一步用户需要学习的操作。改动限定在 Web Shell 包内，复用已有的 daemon mid-turn 接口（旧的插入按钮本就调用 规模：不适用——未触及核心路径（ 方案：范围合理。把 风险：无升级风险信号——改动文件均未命中高回滚路径。 有一点值得 maintainer 从产品角度判断（非阻塞）：这改变了默认行为并移除了一个已发布的显式操作，属于人类可能想亲自看一眼的 UX 决策。daemon 契约本身未变，因此影响范围较小。 进入代码审查 🔍 — Qwen Code · qwen3.8-max-preview Reviewed at |
Collaborator
Code reviewI wrote my independent proposal first (just from the title + "why"): gate mid-turn delivery on The part I scrutinized most was the race handling, because there are three async things that can claim the same prompt: the admission result, the idle transition, and the injection event. The design holds up:
I checked the No correctness bugs, security issues, or regressions found, and no AGENTS.md convention violations. The sequenceDiagram
participant P1 as User
participant P2 as useQueuedPrompts
participant P3 as Daemon mid-turn API
participant P4 as Injection event
participant P5 as Ordinary submit
P1->>P2: send plain text during active turn
P2->>P2: queue row, midTurnState submitting
P2->>P3: enqueueMidTurnMessage
alt accepted and turn still running
P3-->>P2: accepted true
P2->>P2: midTurnState queued, row stays visible
P4-->>P2: injection batch matches text
P2->>P2: remove row, fire onComplete
else rejected or turn went idle
P3-->>P2: accepted false, or idle seen
P2->>P5: fallbackToPendingPrompt then submitPrompt
P2->>P2: row becomes ordinary submitting
end
Files changed (13 of 13 shown)
TestingThis is an unattended CI run, so I did not build or run any PR code — the evidence below is the PR's own CI, read via the API. No failures so far; the primary ubuntu unit suite and the web-shell visual capture are still running. The macOS/Windows/integration checks report CI results for
One row per check name (latest run); skipped checks omitted; failures sort first. / 每个检查名一行（取最新一次运行），省略 skipped，失败项排在最前。 The author reports Web Shell unit tests (394) and focused post-commit tests (33) passing locally on macOS, plus typecheck and ESLint — that's the author's claim, not independently re-run here. The central claim is behavioural (a message sent mid-turn actually reaches the running turn, and the row disappears only on the daemon's injection event), and the dom tests mock both 中文说明代码审查我先独立写了方案（仅凭标题和"为什么"）：以 我最仔细审视的是竞态处理，因为有三个异步来源会认领同一条消息：受理结果、idle 转换、注入事件。设计成立：
我检查了 未发现正确性 bug、安全问题或回归，也无 AGENTS.md 约定违规。 （时序图见英文部分。） 测试这是无人值守的 CI 运行，因此我没有构建或运行任何 PR 代码——以下证据是 PR 自身的 CI，通过 API 读取。目前无失败；ubuntu 主单测套件与 web-shell 视觉截取仍在运行。macOS/Windows/集成检查对该 commit 报告 作者报告本地 macOS 上 Web Shell 单测（394）与提交后聚焦测试（33）通过，以及类型检查和 ESLint——这是作者的声明，未在此独立复跑。 核心主张是行为性的（mid-turn 发送的消息确实送达运行中的回合，且行只在 daemon 注入事件后消失），而 dom 测试 mock 了 — Qwen Code · qwen3.8-max-preview Reviewed at |
Collaborator
|
Confidence: 4/5 — clean, well-tested change that does exactly what it sets out to; the only reservation is that the real-daemon wire path rests on mocked dom tests plus the author's macOS browser check, and it's a default-behavior change worth a maintainer's glance. Stepping back: my independent proposal and the PR converge, which is a good sign the approach isn't over-engineered. The hard part of this feature isn't calling the mid-turn API — it's not losing or duplicating a message across the admission/idle/injection races — and that's exactly where the implementation is careful. The single atomic claim in Every change in the diff is in service of the stated goal — removing the Insert path is the flip side of making injection the default, not scope creep, and the Why not 5/5: the dom suite mocks Approval is deferred until CI lands green on the reviewed commit; the primary ubuntu unit suite and the web-shell visual capture are still running. Once they complete green, the approval is posted automatically against this exact commit. 中文说明置信度：4/5 —— 干净、测试充分的改动，完全达成其目标；唯一的保留是真实 daemon 链路依赖 mock 的 dom 测试加作者的 macOS 浏览器验证，且这是一个默认行为变更，值得 maintainer 看一眼。 退一步看：我的独立方案与 PR 收敛，这说明方案没有过度设计。这个功能的难点不在于调用 mid-turn 接口，而在于在受理/idle/注入竞态中不丢失、不重复消息——而这正是实现谨慎之处。 diff 中每一处改动都服务于既定目标——移除插入路径是把插入设为默认的一体两面，不是范围蔓延； 为何不是 5/5：dom 套件 mock 了 审批推迟到 CI 在被审 commit 上变绿；ubuntu 主单测套件与 web-shell 视觉截取仍在运行。一旦它们绿色完成，将自动针对这一确切 commit 发布审批。 — Qwen Code · qwen3.8-max-preview Reviewed at |
Collaborator
🖼️ web-shell visual previewRendered against a mock daemon (no real backend): the PR base vs this PR head Screenshots · before / afterℹ️ No screenshot changed against the PR base — but this PR edits 5 render-shaping files:
Either the change has no visual effect (logic, plumbing, a state the scenarios never reach), or no scenario renders this UI — in which case the preview cannot see it, and an empty result is a coverage gap rather than a clean bill of health. To make it visible, add a scenario to Full-resolution recordings (.webm) are attached to the workflow run. — Qwen Code · web-shell visuals |
Contributor
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. |
Collaborator
Author
|
Closing this partial version so default mid-turn insertion, daemon-backed delete/edit, capability gating, and race handling can be reviewed together as one complete feature. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this PR does
Messages sent while a model turn is running now enter the existing mid-turn delivery path by default. The message remains visible as queued until the daemon confirms that it was actually injected, then disappears. If injection is rejected or the turn ends first, the same message automatically continues through the ordinary next-turn queue. The separate insert action and its unused UI asset are removed.
Why it's needed
Sending during an active turn should mean “make this available to the current turn” without requiring a second click. Treating request admission as successful insertion hides messages too early, while keeping every accepted message in the ordinary server queue makes users manually insert it. This change makes the visible queue represent only messages that have not yet reached the running turn and preserves every message through idle and transport races.
Reviewer Test Plan
How to verify
Start a long-running turn and send a second plain-text message. Confirm the message shows “Queued...” without a separate insert action, the current generation continues, and the row disappears only when the running turn receives the message. End a turn before another injection boundary and confirm the waiting message starts as the next ordinary turn. Image prompts, slash commands, and an unavailable or older daemon should continue through ordinary submission. Sending identical text through ordinary and mid-turn paths must remove only the actually injected row.
Evidence (Before & After)
Before: a message sent during generation entered the ordinary pending queue and required a separate insert action. After: send attempts mid-turn delivery automatically; the row remains while delivery is pending and disappears on the daemon injection event. Manual browser verification was intentionally left to the requester; focused state-machine and UI tests cover accepted delivery, simultaneous injection/idle, rejected delivery, late admission, image/command fallback, and same-text reconciliation.
Tested on
Environment (optional)
Web Shell unit tests: 394 passed. Focused post-commit tests: 33 passed. Web Shell TypeScript typecheck and ESLint passed. No global build was run.
Risk & Scope
Linked Issues
Supersedes the closed #8210 and builds on the queue visibility behavior from #8065.
中文说明
本 PR 做了什么
模型回合运行期间发送的消息现在默认进入已有的 mid-turn 投递路径。消息会继续显示在队列中，直到 daemon 确认它已被真实插入，随后才从页面消失。如果插入被拒绝或当前回合先结束，同一条消息会自动转入普通的下一回合队列。独立的“插入”操作及其不再使用的 UI 资源已移除。
为什么需要
在活动回合中点击发送，应当直接表达“让当前回合看到这条消息”，不应要求用户再次点击。把接口受理误当成真实插入会过早隐藏消息，而让所有受理消息继续停留在普通服务器队列又会迫使用户手动插入。本改动让页面队列只表示尚未到达当前运行回合的消息，并在 idle 与网络竞态中保证消息不丢失。
Reviewer Test Plan
如何验证
启动一个长时间运行的回合，并发送第二条纯文本消息。确认消息显示“排队中...”，没有独立插入操作，当前生成继续运行，并且只有在运行中的回合真实收到消息后队列行才消失。在下一次插入边界前结束回合，确认等待消息会作为下一普通回合启动。图片消息、Slash 命令以及不支持该接口的旧 daemon 应继续走普通提交路径。普通队列和 mid-turn 队列存在相同文本时，只能删除真实被插入的那一行。
证据（Before & After）
Before：生成期间发送的消息进入普通待处理队列，需要额外点击插入。After：发送默认尝试 mid-turn 投递；投递未完成时保留队列行，收到 daemon 注入事件后才消失。按需求未代替用户进行浏览器验证；聚焦状态机与 UI 测试覆盖了受理成功、注入与 idle 同时发生、拒绝投递、延迟受理、图片/命令回退以及同文本精确关联。
测试平台
环境（可选）
Web Shell 单元测试 394 条通过；提交后聚焦测试 33 条通过；Web Shell TypeScript 类型检查和 ESLint 通过。未运行全局 build。
风险与范围
关联
替代已关闭的 #8210，并基于 #8065 引入的队列可见性行为实现。