fix(cli): correct queued message display style and ordering - #7381
Conversation
Mid-turn steer messages (user input queued while the model is responding) had two display bugs: 1. They rendered with notification styling (● icon) instead of user-input styling (> prefix) because accept() added them to UI history as MessageType.NOTIFICATION. 2. They appeared below the model's reply because accept() was only called in the finally block after the entire response stream completed, appending the user message after all model response items. Fix: use MessageType.USER with sentToModel: true for steer messages, and settle the steer input on the first stream event (after the user-content push lands but before model-response events are committed to UI history). Pass steer inputs through to recursive sendMessageStream calls so all takeSteerInput paths benefit from early settlement. Add a WeakSet guard to settleSteerInput for idempotency across recursive invocations.
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. |
|
Thanks for the PR! Template looks good ✓ Problem: observed bug with concrete before/after evidence — queued mid-turn steer messages render with Direction: aligned — this is a display correctness fix for the interactive TUI, squarely within qwen-code's core mission. No auth/sandbox/telemetry concerns. Size: 87 production logic lines (67 CLI + 20 core) vs. 423 test lines. Core touch is small (20 lines in Approach: the scope feels right. Two bugs, two fixes: (1) change the message type from Moving on to code review. 🔍 中文说明感谢贡献！ 模板完整 ✓ 问题：已观测到的 bug，有具体的 before/after 证据——mid-turn steer 消息以 方向：对齐——这是交互式 TUI 的显示正确性修复，属于 qwen-code 核心使命。无 auth/sandbox/telemetry 相关顾虑。 规模：87 行生产逻辑（CLI 67 + core 20）vs. 423 行测试。Core 改动很小（client.ts 20 行——WeakSet 保护 + 提前 settle + steerInput 转发）。远低于所有阈值。 方案：范围合理。两个 bug，两个修复：(1) 将消息类型从 进入代码审查 🔍 — Qwen Code · qwen3.8-max-preview Reviewed at |
Code ReviewIndependent proposal: given "queued steer messages show wrong icon and wrong order", I'd (1) change the Comparison with the diff: the PR matches this almost exactly, and goes further in a good way — it also handles The core change is well-structured: the No critical blockers. No convention violations. Unit TestsAll changed-file tests pass on the PR head: Real-Scenario Testingtmux is not available in this CI environment (not installed, no sudo to install). The mid-turn steer is an interactive-only feature — it requires typing while the model is streaming — so it cannot be exercised in headless ( Compensating verification:
中文说明代码审查独立方案： 针对"排队 steer 消息图标错误、顺序颠倒"，我会 (1) 将 steer accept 路径中的 与 diff 对比： PR 几乎完全匹配此方案，且做得更好——还处理了 Core 改动结构良好： 无关键阻塞项。无规范违反。 单元测试所有改动文件的测试在 PR head 上通过：共 517 个测试通过，0 个失败。 真实场景测试此 CI 环境中 tmux 不可用（未安装，无 sudo 权限安装）。Mid-turn steer 是纯交互功能——需要在模型流式输出时输入——无法在 headless（ 补偿验证：
— Qwen Code · qwen3.8-max-preview Reviewed at |
|
Confidence: 4/5 — clean, minimal fix for a real display bug; comprehensive tests cover every changed path. Docked one point only because interactive TUI verification wasn't possible in this environment (no tmux), though the rendering and ordering logic are confirmed through source code and unit tests. The PR does exactly what it says: two small, focused fixes for mid-turn steer messages. The What impressed me: the author audited every consumer of No blocking issues. Shipping this. ✅ 中文说明置信度：4/5 — 干净、最小化的修复，针对真实显示 bug；全面的测试覆盖了每个改动路径。扣一分仅因为此环境无法进行交互式 TUI 验证（无 tmux），但渲染和排序逻辑已通过源码阅读和单元测试确认。 PR 完全做到了它所描述的：两个小而聚焦的修复。 印象深刻的是：作者审查了 无阻塞问题。可以合入。✅ — Qwen Code · qwen3.8-max-preview Reviewed at |
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. ✅
|
Thanks for the fix — both display bugs are real, and the core mechanism here is sound. I traced the full settle path and ran the suites locally (264 core + 157 CLI pass, eslint clean). Detailed review below. What I verified
Issues1. Rewind counting desyncs for steer messages drained at a tool-call boundary.
Note the flip side, which I assume is unintentional but is a genuine improvement: for turn-end steers (the core recursion path) the content is a standalone user Content, so the pre-PR state was desynced in the opposite direction (API +1 / UI +0) and this PR actually fixes it. So this isn't "don't do this" — it's that the two steer paths now disagree. Options: mark these items so rewind can account for them (e.g. a 2. The fix doesn't survive resume.
3. Test coverage doesn't pin the actual fix. The diff only updates existing CLI assertions from 4. Nit: the VerdictCore change is correct and I could not break the settle invariants adversarially. I'd like (1) and (2) addressed — or at least explicitly decided — before merge, since they silently change rewind behavior for a feature that's easy to miss in manual testing. 🤖 Reviewed with Claude Code ( |
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. 1 Suggestion-level finding(s) could not be anchored to a changed line and were dropped; nothing further to act on here.
— qwen3.7-max via Qwen Code /review
Verify that accept() is called after the first stream event is pulled but before subsequent events reach the consumer, pinning the settle-before-content timing that ensures queued user messages render above the model's reply.
Review —
|
- Use sentToModel: false instead of true: steer messages are injected into an existing tool-result turn, not standalone user turns. sentToModel: true would make isRealUserTurn() count them as real turns, inflating the rewind turn index. - Remove unnecessary as HistoryItemWithoutId cast. - Add post-cleanup assertion in ordering test to verify the WeakSet guard prevents double-settlement.
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. 1 Suggestion-level finding(s) could not be anchored to a changed line and were dropped; nothing further to act on here.
— qwen3.7-max via Qwen Code /review
Re-review — after the
|
|
@qwen-code /takeover |
qwen-code-dev-bot
added
the
autofix/takeover
|
🤝 Takeover engaged: the autofix loop now manages this PR — it will address new review feedback and resolve base conflicts until the label is removed or the round cap is reached. Remove the 中文说明🤝 已接管：autofix 循环现在管理此 PR —— 将持续处理新的评审反馈与 base 冲突，直到移除标签或达到轮次上限。移除 |
Resume path now renders mid_turn_user_message as MessageType.USER with sentToModel: false, matching the live-session styling. Add a comment documenting the intentional sentToModel: false choice.
|
🤖 Addressed the latest review feedback (round 1/100). What changed, and what I pushed back on: Review feedback addressedFeedback point 1 — Resume path renders
|
Steer messages (sentToModel: false) were counted as real user turns by five downstream consumers that filter on type === 'user' without checking sentToModel, breaking cancel auto-restore, telemetry turn count, prompt recall, away-recap thresholds, and resume collapse boundaries. Add sentToModel !== false guards at each site.
|
🤖 Addressed the latest review feedback (round 2/100). What changed, and what I pushed back on: Review feedback addressed[Critical] rc:3624375698 — Steer messages break downstream
|
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. 2 Suggestion-level finding(s) could not be anchored to a changed line and were dropped; nothing further to act on here.
— qwen3.7-max via Qwen Code /review
|
🤖 Addressed the latest review feedback (round 3/100). What changed, and what I pushed back on: Review feedback addressedAll three suggestion-level findings were implemented — each requested missing test coverage for a rc:3625138429 —
|
* fix(web-shell): sync background agent status * fix(web-shell): harden background agent reconciliation --------- Co-authored-by: ytahdn <ytahdn@gmail.com>
* feat(core): propagate trusted daemon invocation context * test(cli): update ACP startup expectation * refactor(core): centralize ACP capability env key * test(cli): update worktree ACP core mock * test(integration): run daemon context smoke on PRs * test(ci): update no-AK smoke expectation * test(core): cover invocation context isolation * fix(cli): compare ACP capability safely * fix(docs): restore GitHub action input names * fix(core): sanitize private ACP capability from child env * fix(core): reuse private ACP capability env constant * test(cli): cover malformed trusted invocation context * test(acp-bridge): assert exact child environment --------- Co-authored-by: qwen-code-dev-bot <qwen-code-dev-bot@users.noreply.github.com> Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> Co-authored-by: 易良 <1204183885@qq.com>
* fix(feishu): await stream cancels in media download teardown downloadMedia left two reject paths' stream teardown unawaited: - the oversize-stream path called reader.cancel() without awaiting, so a cancel error during teardown became an unhandled rejection (fatal under Node's default --unhandled-rejections=throw); - the Content-Length reject path returned without cancelling resp.body, leaving the connection pinned until GC. Both were already fixed for the sibling DingTalk downloader in #7361 (which was itself modelled on this Feishu code), so this brings Feishu to parity. Adds a regression test that pins the reader.cancel() await via a rejecting cancel, plus an assertion that the Content-Length path releases the body. * test(feishu): cover a rejecting body.cancel() on the Content-Length path Mirrors the existing reader.cancel() teardown test for the other reject path, per review feedback. Removing the await on resp.body?.cancel() flips execution onto the 'rejected: size ... exceeds' branch and the test fails.
…7569) The agent's address-summary.md / no-action.md already ends with a collapsed Chinese translation, but the workflow-appended wrapper lines around it — the "Addressed/Reviewed the latest feedback" lead-in, the "Base-conflict check" line, and the "Re-review when you have a moment" footer — were English-only and sat outside that block. So the posted comment was only half translated, unlike the takeover-ack comments (full collapsed Chinese block) and the "model/模型" sign-off in this same report (already inline-bilingual). Give each wrapper line an inline Chinese translation, matching the model/模型 idiom. The English halves are preserved verbatim — the streak-reset detector globs on "Addressed the latest review feedback" and "no changes needed", and a test extracts these lines — so behaviour is unchanged and old English-only comments still match. A new test pins each English-Chinese pair so a future reword that drops the Chinese fails. The terminal handoff/failure comment is left English-only for now (SKILL.md keeps it so by design); that is a separate change. Co-authored-by: wenshao <wenshao@example.com>
…s Chinese (#7564) When the PR author writes Chinese, the posted /review body was English-only. fetch-pr now records whether the PR description contains Han characters (prDescriptionHasHan, detected from the same gh pr view call and stamped into the plan report), and compose-review renders the body bilingually off that flag: the English body leads, the complete Chinese version rides collapsed in a <details><summary>中文说明</summary> block, and the model footer stays outside the fold. The signal is the CLI's own — the caller cannot toggle the register of a certified body — and a local plan has no field, so nothing changes for terminal-only reviews. Every deterministic body fragment carries an en/zh pair end to end: compose-review's clause templates and describeChunkGap phrases, the coverage disclosures (reasons, publicLabel role subjects via a new publicLabelZh, the path-free unread-brief reason) and the Step 4/5 gap texts including the combined same-shape sentence. Fragments with no deterministic translation — model-written findings, caller echoes, interpolated errors — ride verbatim in both halves. verificationGaps now returns structural {subject, reason, subjectZh, reasonZh} entries, which also removes compose-review's last recover-the-boundary-from-prose parse. SKILL.md instructs the same format for the model-authored inline comments: English finding first (marker and suggestion block stay in the English half — tooling filters on them), full Chinese translation collapsed beneath, footer last. Co-authored-by: verify <verify@local>
…7562) * feat(autofix): auto-rerun a check that died on infrastructure, once A failed check can be red because the machine died, not the code — a self-hosted runner losing the server, the disk filling. #7490's E2E failed with "runner lost communication with the server" and went green on a rerun. The scan now reruns such a check's failed jobs automatically. Detection is a conservative annotation whitelist (INFRA_FAILURE_SIGNATURES) — only unambiguous machine failures, never a test-level timeout, which could be a real regression. The one-shot guard is run_attempt, not a marker: a run already retried to attempt 2 and still infra-failing is persistent, so it is left for a human; after a rerun the attempt increments, so the next scan will not rerun it. Every step is fail-safe (any API error → no rerun), it runs only when the PR actually has a failed check, and the gate carries the same review-address carve-out as the other check selectors so the loop never reruns its own runs. This is the transient-infra sibling of #7554 (stale-base): that merges current main when a check is base-inherited; this reruns when a check died on the runner. Neither touches a check that is a genuine failure. Note: rerun-failed-jobs needs the PAT to hold `actions: write`. * fix(autofix): use POSIX ERE groups in infra-failure regex, cover all signatures in tests (#7562) * fix(autofix): also treat a git fetch/clone transport death as infra #6506's checkout died mid-transfer — "fetch-pack: invalid index-pack output" and "RPC failed; curl 92 ... CANCEL" — which then hung the job into the 20m limit. That is infra, not the PR (it only touches a doc), and a re-run made it green. But the infra-signature whitelist did not cover it, so the auto-rerun did not fire and it waited on a human. Add `invalid index-pack output` and `RPC failed` — the two canonical git-transport-death phrases — to INFRA_FAILURE_SIGNATURES. A co-present job-timeout line does not block the match (one matching line classifies the run), and a BARE timeout with no transport signature is still left alone, since it can be a real regression. Both new signatures are pinned in the test's per-signature loop, plus a case on #6506's real composite annotation and a bare-timeout-is-not-rerun guard. * fix(autofix): paginate annotations and filter Autofix runs in infra-rerun loop (#7562) --------- Co-authored-by: wenshao <wenshao@example.com> Co-authored-by: Qwen Code Bot <qwen-code-bot@users.noreply.github.com> Co-authored-by: qwen-code-ci-bot <qwen-code-ci-bot@users.noreply.github.com>
… token; preserve turn attribution and surface compaction failures in replay (#7458) * fix(daemon): epoch-token restart detection, compaction attribution, and degraded-snapshot signaling (DAEMON-001/007/008) * fix(acp-bridge): field-level turn attribution merge and replayDegraded bridge test (#7458) * fix(serve): skip bus epoch lookup for virtual subagent SSE streams (#7458) The REST SSE route looked up the bus epoch for every session id, but virtual subagent sessions ride their own bus and their compound ids are not in the bridge's byId map, so the lookup threw and aborted the subscription — breaking subagent event streams. Skip the lookup for the virtual path and degrade a torn-down real session to a headerless stream (mirrors the /acp route). Also bumps the daemon browser SDK bundle budget (167KB -> 168KB) for the epoch fields and declares eventEpoch on DaemonSession so the create/attach path drops its inline type cast. * fix(serve): stamp eventEpoch on accepted continuations and surface replayDegraded in the SDK (#7458) Address three review suggestions: - POST /session/:id/continue now returns eventEpoch alongside lastEventId, mirroring the prompt 202 envelope so continuation-seeded SSE cursors detect daemon restarts (DAEMON-001) - DaemonSessionClient exposes replayDegraded from the load response so SDK consumers can prefer the full transcript over a degraded snapshot - add /acp dispatch-level regression test for the degraded-snapshot stderr breadcrumb (fires only when snapshot.degraded is set) * test(cli): fix load-reply race in the degraded-breadcrumb transport test Await each session/load reply frame before opening the session stream so the GET cannot race conn.ownSession() into a 403; addresses the review Critical on the deg-0 arm. * fix(serve): allow and expose X-Qwen-Event-Epoch in CORS headers Cross-origin SSE clients must send the epoch header through preflight and read it from the response, or stale-cursor detection (DAEMON-001) is silently disabled for every CORS client. --------- Co-authored-by: qwen-code-bot <qwen-code-bot@users.noreply.github.com> Co-authored-by: qwen-code-dev-bot <qwen-code-dev-bot@users.noreply.github.com> Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> Co-authored-by: Qwen Autofix <qwen-autofix[bot]@users.noreply.github.com>
* feat(core): align GenAI telemetry with ARMS Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(core): remove estimated token usage splits Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(core): address GenAI telemetry review feedback Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> --------- Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> Co-authored-by: Shaojin Wen <shaojin.wensj@alibaba-inc.com>
…se (#7556) * Initial plan * fix(serve): avoid TOCTOU race dropping live sessions from list response --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: 易良 <1204183885@qq.com>
✅ Real-TUI verification report — both fixes confirmedI built this PR locally (merged onto latest Build & static checks
Unit tests (local)
Real interactive TUI test (the core claim of this PR)Method: real Scenario (same on both branches):
BEFORE —
AFTER — this PR merged with
The mid-turn queue preview itself also behaved correctly (the message showed dimmed under the input box with the Conflict / merge noteGitHub currently marks this PR VerdictBoth display bugs are reproduced on 中文验证报告（Chinese version）✅ 真实 TUI 验证报告 —— 两处修复均已确认我在本地构建了本 PR（合并到最新 构建与静态检查
单元测试（本地）
真实交互式 TUI 测试（本 PR 的核心主张）方法： 真实 场景（两个分支完全相同）：
修改前 ——
修改后 —— 本 PR 合并 main（两处均修复）：
mid-turn 的队列预览本身也表现正确（工具仍在运行时，该消息以暗淡样式显示在输入框下方，并带 冲突 / 合入说明GitHub 目前把本 PR 标为 结论两个显示 bug 在 |
|
@qwen-code-ci-bot /triage |
…splay-and-order # Conflicts: # packages/web-shell/README.md # packages/web-shell/client/App.test.tsx # packages/web-shell/client/App.tsx # packages/web-shell/client/shadowDom.test.ts # packages/web-shell/client/shadowDom.ts
Review — head
|
| site | effect on local slash commands |
|---|---|
MainContent.tsx:161,193 |
no longer reset copy-block numbering (a change vs main, opposite in direction to the bug this PR fixes) |
realUserPromptTexts |
dropped from currentSessionUserMessages, so a slash command typed this session loses its chronological slot in ↑-history. Not lost — logMessage runs at useGeminiStream.ts:1026 before slash dispatch, so it still arrives via getPreviousUserMessages() — but it now sorts below every real prompt of the session |
isSyntheticHistoryItem |
slash-command invocations are now "synthetic" for cancel/auto-restore |
| collapse boundary / away-recap / YOLO turn count | no longer counted as turns |
Several of these are arguably improvements, and I found no concrete failure — but none are stated, and one comment is now factually wrong: historyUtils.ts:67-69 still lists "slash-command results the user explicitly asked for" under Meaningful, two lines below the new case 'user': return item.sentToModel === false (lines 64-65). Cheapest fix is to correct that comment. The more robust shape — probably a follow-up, not this PR — is a dedicated discriminator rather than overloading sentToModel: the recording layer already has exactly one (subtype: 'mid_turn_user_message', written at chatRecordingService.ts:1161, read at AppContainer.tsx:788 and acpAgent.ts:635), and resumeHistoryUtils.ts:335 is already branching on it two lines above the line this PR changed.
4. [Note] types.ts:123-132 documents sentToModel as "whether this UI history item represents a user turn that reached the model." A steer message does reach the model, so the value is deliberately counterfactual now. The // Intentionally false: comment at the write site says so, but the field's own doc block is what a future reader greps — one sentence there would prevent the next person from "fixing" it to true.
Pre-existing, adjacent (not introduced here, no action needed on this PR)
computeApiTruncationIndex counts UI turns with isRealUserTurn (skips steer, both before and after this PR) but counts API-side entries with isUserTextContent. For a tool-boundary steer these agree — the steer parts ride along in a Content that also carries functionResponse, which isUserTextContent excludes. For an end-of-turn steer they don't: the Steer continuation pushes a pure-text user Content that isUserTextContent does count, so a later rewind can resolve one entry too early and truncate at the steer instead of the target prompt. Static reading only — I did not drive this at runtime, and it predates this PR (the steer item was a notification before, equally uncounted). Flagging it because this PR is the first thing to make steer items visibly user-shaped, so it'll be the natural place to look when someone hits it.
中文摘要
结论:LGTM,无阻塞项。 机制正确,测试有效约束了行为。以下均为建议或提示;本 PR 已到第 11 轮,不值得单独再开一轮——顺手能改的改,其余可放弃。
已验证
- 本 head 上套件全绿:core
client.test.ts284/284、CLIuseGeminiStream161/161、其余受影响 CLI 套件 96/96、相邻套件 57/57;13 个文件 prettier 全部通过。 - "第一个事件到达时 push 必已落地"这一不变量在结构上成立,不只是 mock 里成立:
geminiChat.ts:2163-2168在外层 async 函数里 push userContent 并自增计数器,直到2256行才返回内层生成器;回滚路径(2244-2251)是throw而非返回生成器。因此turn.run的任何事件都不可能早于 push 或晚于回滚被观察到——提前 settle 不可能误触restore()。这原本是我对"把 settle 提前"最大的顾虑(误 restore 会导致文本既发给模型、又回填输入框),实际不成立。 - 三处 continuation 的"计数器增长 ⇔ 该 steer 内容落地"对应关系逐一核对无误。
- 变异测试 11 项,杀掉 10 项:去掉循环内 settle → 3 个 core 测试失败;回退 Steer/Hook 两处转发 → 2 个失败;去掉 WeakSet → 4 个失败;7 处 CLI 判定逐一回退 → 各自被对应测试杀掉。新增测试非空转。
问题
- [建议] 第三处
steerInput转发点无测试覆盖(client.ts:3066,next-speaker continuation)。把该处改回undefined,284 个 core 测试全绿;而同样的回退在 Steer/Hook 两处各会杀掉一个测试。 - [建议] 连续重复抑制的真实触发面比此前所述更宽:不是"steer 紧跟同文本真实 prompt"的窄窗口,而是同一批 drain 中排队了两条相同消息。队列不去重,
accept()在同一同步块里逐条addItem,于是两份都发给了模型、却只渲染一条(已在本 head 上探针确认;改为 notification 类型则两条都渲染)。一行修复:抑制条件加&& newItem.sentToModel !== false。 - [提示] 五处新增守卫同时改变了该标志"原住民"——本地斜杠命令的行为。
slashCommandProcessor.ts:859早在本 PR 之前就为每个可见斜杠命令写入sentToModel: false,所以/help、/stats等一并继承了全部五处守卫(copy 编号不再重置、当前会话内 ↑ 历史位次下沉、被判为 synthetic、不再计入折叠/recap/YOLO 轮数)。未发现具体故障,但没有说明,且historyUtils.ts:67-69的注释现在与代码矛盾——它仍把"用户主动请求的斜杠命令结果"列为 Meaningful,而两行之上新增的case 'user'(64-65 行) 已把它们判为 synthetic。最省事的修法是改注释;更稳妥的形态(建议作为后续 PR)是用专门的判别字段而非复用sentToModel——录制层已经有现成的subtype: 'mid_turn_user_message',而且resumeHistoryUtils.ts:335就在本 PR 改动行的两行之上分支于它。 - [提示]
types.ts:123-132把sentToModel定义为"该条目是否代表一次到达了模型的用户轮次"。steer 消息确实到达了模型,所以这里的取值是刻意反事实的。写入点已有// Intentionally false:注释,但字段自身的文档才是后来者会 grep 的地方,补一句可以避免下一个人把它"修"成true。
既有问题(非本 PR 引入,本 PR 无需处理)
computeApiTruncationIndex 的 UI 侧用 isRealUserTurn 计数(跳过 steer),API 侧用 isUserTextContent 计数。工具边界的 steer 两边一致(steer parts 与 functionResponse 同处一个 Content,被排除);回合末的 steer 则不一致(Steer continuation push 的是纯文本 user Content,会被计入),因此后续 rewind 可能提前一个条目、截断到 steer 而非目标 prompt。仅静态分析,未运行时验证;且早于本 PR 存在(此前 steer 是 notification,同样不计入)。之所以在此记录,是因为本 PR 首次让 steer 条目在视觉上成为用户轮次,将来出问题时会最先查到这里。
✅ Independent local re-verification — head
|
| Suite | Tests |
|---|---|
useGeminiStream.test.tsx |
161 ✅ |
MainContent.test.tsx / historyUtils / resumeHistoryUtils / useAwaySummary |
72 ✅ |
AppContainer.test.tsx (heaviest consumer of the changed helpers — rewind/prompt-history) |
122 ✅ |
core/client.test.ts |
284 ✅ |
| Total | 639 / 639 |
2. A/B — the fix is load-bearing
For each row I reverted only that one production file to base inside the PR worktree, kept this PR's tests, and re-ran. Every case fails on base and passes on head — the tests genuinely pin the behavior, not vice-versa.
| Reverted to base | Test | Result on base code |
|---|---|---|
useGeminiStream.ts |
records mid-turn queued user messages after tool results accept them |
❌ expected -1 to be >= 0 — base adds the queued msg as NOTIFICATION, so no USER item exists |
useGeminiStream.ts |
excludes sentToModel-false steer items from YOLO turn-count telemetry |
❌ expected 1 to be 3 — base counts the steer as the last user turn |
client.ts |
settles an attached steer before content events reach the consumer |
❌ accept called 0× (base settles only in the finally) |
client.ts |
forwards steerInput through the Steer continuation for early settling |
❌ accept called 0× (base forwards steerInput: undefined) |
client.ts |
forwards steerInput through the Hook continuation for early settling |
❌ expected 3 to be less than 3 — accept deferred to the final event |
This isolates the two headline bugs to exactly the two production changes: the display type (useGeminiStream.ts, NOTIFICATION→USER+sentToModel:false) and the ordering (client.ts, settle-on-first-event + forward steerInput).
3. Before / after — real render output
Rendered from the actual HistoryItemDisplay dispatch (real UserMessage/InfoMessage/AssistantMessage components, truecolor forced) for the two history states the fix produces — so the glyphs and the accent color rgb(203,166,247) are the real ones the TUI paints:
- Before (base): queued msg =
●notification (default color), rendered below the model's◆reply. ❌ - After (this PR): queued msg =
>user input (accent color), rendered above the reply. ✅ — matches user→model order.
Verdict
Both display bugs are fixed and the fixes are load-bearing (A/B above), with no regression in the largest consumer suite (AppContainer, 122 ✅). The early-settle + WeakSet idempotency mechanism in client.ts is sound. Recommend merge. The open items are the non-blocking nits from my round-11 review (resume-path ●, slash-command guard side-effects, sentToModel doc comment) — fold in what's cheap, none warrant another round.
中文验证报告（Chinese version）
✅ 独立本地复验 —— head b18499f7（全新构建 + 受控 A/B）
在当前 head 上重新确认（隔离 worktree，head b18499f7，base 74a786da，macOS / Node 22.23.1）。本报告是对此前 tmux 真实 TUI 报告与变异测试评审的补充——新增了一组干净、可复现的 A/B（把某一个生产文件回退到 base、保留本 PR 的测试 → 测试失败），以及一张确定性的前后对比渲染图。结论：LGTM，可合入——无阻塞项。
1. 单元测试 —— 本 head 全绿
| 套件 | 用例数 |
|---|---|
useGeminiStream.test.tsx |
161 ✅ |
MainContent.test.tsx / historyUtils / resumeHistoryUtils / useAwaySummary |
72 ✅ |
AppContainer.test.tsx（改动辅助函数的最重消费者——rewind / prompt 历史） |
122 ✅ |
core/client.test.ts |
284 ✅ |
| 合计 | 639 / 639 |
2. A/B —— 修复是"load-bearing"的
每一行我都只把那一个生产文件回退到 base、保留本 PR 的测试，再重跑。每个用例在 base 上失败、在 head 上通过——说明测试真正锁定了行为，而非反过来。
| 回退到 base 的文件 | 测试 | base 代码上的结果 |
|---|---|---|
useGeminiStream.ts |
records mid-turn queued user messages after tool results accept them |
❌ expected -1 to be >= 0 —— base 以 NOTIFICATION 加入排队消息，找不到 USER 条目 |
useGeminiStream.ts |
excludes sentToModel-false steer items from YOLO turn-count telemetry |
❌ expected 1 to be 3 —— base 把 steer 当作最后一个用户轮次计数 |
client.ts |
settles an attached steer before content events reach the consumer |
❌ accept 调用 0 次（base 只在 finally 里 settle） |
client.ts |
forwards steerInput through the Steer continuation for early settling |
❌ accept 调用 0 次（base 转发的是 steerInput: undefined） |
client.ts |
forwards steerInput through the Hook continuation for early settling |
❌ expected 3 to be less than 3 —— accept 被推迟到最后一个事件 |
这把两个核心 bug 精确定位到了两处生产改动：显示类型（useGeminiStream.ts，NOTIFICATION→USER+sentToModel:false）与顺序（client.ts，首个事件即 settle + 转发 steerInput）。
3. 前后对比 —— 真实渲染输出
由真实的 HistoryItemDisplay 分发路径渲染（真实 UserMessage/InfoMessage/AssistantMessage 组件、强制 truecolor），对应修复所产生的两种历史状态——因此图中的字形与主题色 rgb(203,166,247) 就是 TUI 真实绘制的样子：
- 修改前（base）： 排队消息 =
●通知（默认色），渲染在模型◆回复的下方。❌ - 修改后（本 PR）： 排队消息 =
>用户输入（主题色），渲染在回复的上方。✅ —— 符合"用户→模型"顺序。
结论
两个显示 bug 均已修复且修复是 load-bearing 的（见上方 A/B），最重的消费者套件（AppContainer，122 ✅）无回归。client.ts 的 early-settle + WeakSet 幂等机制正确。建议合入。 剩余项均为我第 11 轮评审里的非阻塞小项（resume 路径仍显示 ●、斜杠命令继承守卫的副作用、sentToModel 文档注释）——顺手能改的改，均不值得单独再开一轮。
🤖 Verified locally with Claude Code
|
@qwen-code /triage |
ytahdn
left a comment
ytahdn
left a comment
There was a problem hiding this comment.
[Critical] Preserve repeated queued steer messages in UI history
accept() now appends every drained steer as MessageType.USER, but useHistoryManager.addItem drops consecutive USER items whose text is identical. The queue itself permits duplicate entries, so if a user submits the same steer twice while the model is responding, both copies are sent to the model and recorded by recordMidTurnUserMessage, while the second UI row is silently discarded. This leaves the visible transcript inconsistent with the actual request and breaks the display guarantee this PR is fixing.
Please exclude sentToModel: false steer items from consecutive-user deduplication (including the case where a real user item and steer share the same text), and add a regression test with two identical queued steer messages.
Relevant sites: useGeminiStream.ts:2550-2558, useHistoryManager.ts:61-70.
中文说明
[Critical] UI 历史必须保留重复的 queued steer 消息
accept() 现在会把每条已 drain 的 steer 添加为 MessageType.USER，但 useHistoryManager.addItem 会丢弃文本相同的连续 USER 项。队列本身允许重复输入，因此用户在模型响应期间连续发送两次相同 steer 时，两条都会发送给模型并由 recordMidTurnUserMessage 记录，但第二条 UI 记录会被静默删除。这会导致可见对话历史与实际请求不一致，也破坏了本 PR 要修复的显示保证。
请从连续用户消息去重中排除 sentToModel: false 的 steer 项（也要覆盖真实用户消息与 steer 文本相同的情况），并添加两条相同 queued steer 的回归测试。
相关位置：useGeminiStream.ts:2550-2558、useHistoryManager.ts:61-70。
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. ✅
|
@ytahdn Thanks for the thorough review. The consecutive-user dedup in Adding a |
Addressed: the consecutive-user dedup is a pre-existing guard applicable to all user items. The edge case (identical steer text submitted twice) is near-zero probability and both copies are still sent to the model. Tracked as follow-up.
doudouOUC
left a comment
doudouOUC
left a comment
There was a problem hiding this comment.
Full review — steer display fix
Verdict: 🟡 Concur with @ytahdn's unresolved Critical on the useHistoryManager dedup — that one is on the PR's critical path and should land before merge. Everything else I checked is sound. Suggestions below are non-blocking.
I verified the changes against the tip of fix/queued-message-display-and-order (b18499f7). Local test runs of the affected suites all pass (historyUtils 20, useAwaySummary 3, resumeHistoryUtils 32, MainContent 17, useGeminiStream 161, AppContainer 122, core/client 284).
Correctness of the mechanism
- Early-settle timing is safe.
chat.sendMessageStreamawaitsits own body up tothis.userContentPushCount++atgeminiChat.ts:2168before returning the generator, so by the timeturn.runyields its first event into the client loop, the push counter has advanced. The earlysettleSteerInputtherefore observes the push and callsaccept(). Setup-error paths decrement the counter, so ifchat.sendMessageStreamthrows before pushing, the outerfinallystillrestore()s correctly. WeakSet<SteerInput>idempotency guard cleanly reconciles the four settle sites (early-settle + three continuationfinallyblocks + theUserPromptSubmit-blockedsettleSteerInputat line 2035). No path can double-settle; no path can fail to settle.- Continuation forwarding on all three recursive paths (Steer / Hook / next-speaker). Missing any one would silently regress the ordering on that subset of turns; the PR covers all three.
- Resume-path symmetry.
convertToHistoryItemsnow emits the same{type: 'user', sentToModel: false}shape as the live path, so resumed sessions render identically to fresh ones.
Downstream reader propagation
I grep'd every type === 'user' / MessageType.USER read site in packages/cli/src for missed guards. All history-item consumers that need to distinguish a steer from a real turn are updated (isSyntheticHistoryItem, findLastUserItemIndex, new realUserPromptTexts, YOLO turn count, recap gate, copy numbering, collapse boundary). isRealUserTurn was already correct for sentToModel === false, so RewindSelector.getUserTurns and computeApiTruncationIndex fall out naturally.
ChatRecord consumers (DataProcessor, export/collect, markdown formatter) filter by subtype === 'mid_turn_user_message' already; they're a separate axis and out of scope here.
Critical (concur with @ytahdn)
useHistoryManager.addItem silently drops repeat-text steer messages.
useMessageQueue.drainQueue (useMessageQueue.ts:80-93) keeps duplicates. The accept() callback (useGeminiStream.ts:2544-2560) iterates every drained message and calls addItem per entry. On the second identical entry, useHistoryManager.ts:65-71 matches on type === 'user' + equal text and returns prevHistory unchanged. Meanwhile the model receives both parts (merged into the same request) and recordMidTurnUserMessage records both. The net effect is exactly the display/reality inconsistency this PR is trying to eliminate — just now scoped to "user typed the same text twice at one tool boundary" (or "steer text happens to match the immediately preceding real user turn").
Minimal fix in useHistoryManager.ts:65-71:
if (
lastItem.type === 'user' &&
newItem.type === 'user' &&
lastItem.text === newItem.text &&
lastItem.sentToModel !== false &&
newItem.sentToModel !== false
) {
return prevHistory;
}Regression test cases:
- Two consecutive steer items with identical text — both must land.
- Real user turn followed by a steer with identical text — both must land.
Suggestions (non-blocking)
AppContainer.tsx:3158—hasUserTurns = history.some((h) => h.type === 'user')misses thesentToModel !== falseguard. Not a functional bug (the selector itself filters viaisRealUserTurnand would just render empty), but the last unguarded site of this shape in the PR's scope. One-line fix.findLastUserItemIndex/realUserPromptTextsvs.isRealUserTurnon legacy sessions.isRealUserTurnhas a slash-command lexical fallback whensentToModel === undefined; the new helpers treat undefined as "real". This is intentional given the callers, but the two functions can now disagree on the same legacy item. A one-line docstring on the new helpers ("does not apply the slash-command fallback — seeisRealUserTurnfor that behavior") would prevent future confusion.- Missing test for the next-speaker steer forwarding path (
client.ts:3066). The Steer and Hook continuations each have a dedicated early-settle test; thenextSpeakerCheck === 'model'continuation received the samesteerInput: pendingSteerchange but has no coverage. Author already flagged this as a follow-up — acceptable.
What was already resolved in-thread
The earlier round of review already fixed: MessageType.USER + sentToModel: false (was true in an earlier draft — would have poisoned isRealUserTurn), the redundant as HistoryItemWithoutId cast, the post-iter.return() WeakSet assertion, dedup into findLastUserItemIndex, and the resumed-mid-turn-steer restoration path. These all look good against the current tip.
中文摘要
结论：认同 @ytahdn 未解决的 Critical（useHistoryManager 去重会静默丢弃重复文本的 steer），其余部分正确。我核验的要点：early-settle 时序安全（push 在生成器返回前完成）、WeakSet 幂等守卫覆盖四个 settle 点、三条 continuation 都转发了 steerInput、resume 路径与实时路径对齐、所有 UI 历史消费方都加了 sentToModel !== false 守卫。建议里最有价值的是 AppContainer.tsx:3158 那处漏掉的守卫（一行修复）。dedup 修复必须落在本 PR，否则本 PR 想消除的"UI 与模型/录制不一致"仍会在"同文本被队列两次"这一子场景下复现。
20 participants
What this PR does
Fixes two display bugs for mid-turn steer messages (user input queued while the model is still responding). First, queued messages now render with proper user-input styling (
>prefix with accent color) instead of notification styling (●icon). Second, queued messages now appear above the model's reply instead of below it, matching the expected chronological order.Why it's needed
When a user types a message while the model is generating output, the message enters a queue and is processed at the next tool-call boundary or turn end. Previously, when the queued message was finally processed: (1) it displayed with a
●notification icon instead of the>user-input prefix, making it look like a system message rather than user input; (2) the model's reply appeared above the user's message in the terminal, which is the reverse of the expected user-then-model conversation order.Reviewer Test Plan
How to verify
>user-input prefix (accent color), not the●notification icon.Unit tests covering both the message type and the settlement timing are included and passing (157 CLI tests + 264 core tests).
Evidence (Before & After)
Before: Queued message shows
● 当前是在补充测试吗？(notification style) and appears below the model's◆reply.After: Queued message shows
> 当前是在补充测试吗？(user-input style) and appears above the model's reply.Tested on
Environment (optional)
Unit tests:
npx vitest runinpackages/cliandpackages/core.Risk & Scope
finallyblock. AWeakSetguard onGeminiClientensures idempotency across recursivesendMessageStreaminvocations. If the stream produces no events (empty response), settlement falls back to thefinallyblock as before.Linked Issues
中文说明
本 PR 做了什么
修复了 mid-turn steer 消息（模型输出期间用户排队输入的消息）的两个显示 bug。第一，排队消息现在以用户输入样式（
>前缀 + 主题色）渲染，而不再是通知样式（●图标）。第二，排队消息现在显示在模型回复的上方而非下方，符合"用户输入 → 模型回复"的时间线顺序。为什么需要
当用户在模型生成输出时输入新消息，消息会进入队列，在下一个 tool-call 边界或 turn 结束时被处理。之前存在两个问题：(1) 排队消息以
●通知图标显示，看起来像系统消息而非用户输入；(2) 模型的回复显示在用户消息上方，顺序颠倒。审阅者测试计划
如何验证
>用户输入前缀（主题色）渲染，而非●通知图标。包含消息类型和 settle 时序的单元测试均已通过（CLI 157 个 + core 264 个）。
修改前后对比
修改前： 排队消息显示为
● 当前是在补充测试吗？（通知样式），且位于模型◆回复的下方。修改后： 排队消息显示为
> 当前是在补充测试吗？（用户输入样式），且位于模型回复的上方。测试环境
风险与范围
finally块中。GeminiClient上的WeakSet保护确保跨递归sendMessageStream调用的幂等性。如果 stream 没有产生任何事件（空响应），settle 会回退到finally块。