fix(web-shell): stop stacking duplicate copies when restoring prompt text - #7134
Conversation
…text Several restoration paths return a failed prompt's text to the composer by prepending it above the current draft: failed queue submits, failed mid-turn inserts, and queue clears. More than one of them can fire for the same prompt across reconnects and refreshes, and a user retrying an identical message produces identical text — each pass stacked another copy, which surfaced as multiple sent messages concatenated back into the input box after a page refresh (QwenLM#7128). Extract the merge into mergeRestoredPromptText() and make it idempotent: restoring text that is already at the top of the editor is a no-op. Restoring different text still prepends above the draft. This addresses the text-stacking defect (bug 3 in the triage analysis). The SSE-reconnect-on-prompt question (bug 1) is a behavioral decision left to maintainers. Fixes QwenLM#7128 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Verification ReportBase: 1. Unit contract — the exact reported sequence
2. Browser regression — real daemon + Playwright ChromiumAgainst a live 3. Static checks
Scope notes
中文版本验证报告基线: 1. 单元契约 — 精确复刻 issue 中的序列
2. 浏览器回归 — 真实 daemon + Playwright Chromium对真实 3. 静态检查typecheck / web-shell 包构建 / eslint / prettier 全部通过。 范围说明本 PR 解决 triage 分析中的 bug 3(文本堆叠)。bug 1(SSE 重连默认值)与 bug 2(发送失败的编辑器清空语义)是刻意留给 maintainer 的行为决策——如有倾向方向乐意跟进。 🤖 Generated with Claude Code |
|
Thanks for the PR! Template looks good ✓ Problem: observed bug with a clear 100%-reproducible scenario in #7128 — sending the same message twice, refreshing the page, and seeing the two copies concatenated in the input box. This is a real defect, not theoretical hardening. Direction: aligned. The fix targets exactly the stacking defect (triage bug 3 from the issue analysis) while leaving the SSE-reconnect default and editor-clear semantics for separate discussion. Scoped to one pure function and one call site — minimal blast radius. Size: not applicable — changes are in Approach: the scope feels right. Extracting Moving on to code review. 🔍 中文说明感谢贡献! 模板完整 ✓ 问题:已观测的 bug,#7128 中有 100% 可复现的场景——同一消息发送两次后刷新页面,输入框中出现拼接文本。这是真实缺陷,不是理论性加固。 方向:对齐。修复精确针对堆叠缺陷(issue 分析中的 triage bug 3),将 SSE 重连默认值和编辑器清空语义留给后续单独讨论。范围控制在一个纯函数和一个调用点——最小影响半径。 规模:不适用——变更在 方案:范围合理。将 进入代码审查 🔍 — Qwen Code · qwen3.7-max Reviewed at |
🖼️ web-shell visual previewRendered against a mock daemon (no real backend): the PR base vs this PR head Screenshots · before / afterFull-resolution recordings (.webm) are attached to the workflow run. — Qwen Code · web-shell visuals |
Code ReviewClean implementation. The pure function Reuse check: no existing shared function covers this merge-dedup logic; the extraction is justified. No correctness issues, no AGENTS.md violations. TestingAll 6 unit tests pass. The test suite covers:
ESLint: clean. TypeScript typecheck ( Real-scenario testing (tmux): not applicable — this is a browser-side React hook in 中文说明代码审查实现干净。纯函数 复用检查: 没有已有的共享函数覆盖此合并去重逻辑;抽取合理。 无正确性问题,无 AGENTS.md 违规。 测试6 个单测全部通过。测试覆盖:空编辑器填充、不同草稿前置、完全相同 no-op、顶部重复 no-op、CJK 消息重复恢复幂等(#7128 的精确场景)、同前缀不同首行正确不去重。 ESLint:通过。TypeScript typecheck(web-shell 包):通过。 真实场景测试(tmux): 不适用——这是 — Qwen Code · qwen3.7-max Reviewed at |
|
Confidence: 5/5 This is a clean, minimal fix for a real bug. The No concerns to escalate. The change is scoped to Verdict: Approve ✅ 中文说明置信度:5/5 这是一个干净、最小化的真实 bug 修复。 无需上报的顾虑。变更范围限于 结论:通过 ✅ — Qwen Code · qwen3.7-max Reviewed at |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
LGTM, looks ready to ship. ✅
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed — no blockers. Suggestions are inline.
— qwen3.7-max via Qwen Code /review
| if (images && images.length > 0) { | ||
| editorRef.current?.restoreImages(images); | ||
| } |
There was a problem hiding this comment.
[Suggestion] restoreImages is outside the if (next !== current) dedup guard — images are unconditionally appended on every restore call via setPastedImages((prev) => [...prev, ...images]), while text dedup correctly skips setText. — Failure scenario: a prompt with attached images fails to submit, restore fires, then a reconnect/retry triggers a second restore of the same prompt → text stays correct (dedup works) but images silently double, producing an inconsistent editor state.
| if (images && images.length > 0) { | |
| editorRef.current?.restoreImages(images); | |
| } | |
| if (next !== current) { | |
| editorRef.current?.setText(next); | |
| if (images && images.length > 0) { | |
| editorRef.current?.restoreImages(images); | |
| } | |
| } |
— qwen3.7-max via Qwen Code /review
There was a problem hiding this comment.
Good catch — fixed in follow-up PR #7169: restoreImages now runs only alongside an actual text change, so a deduplicated restore no longer doubles the attachments.
✅ Local verification report — LGTM, recommend mergeVerified at head Summary: the fix is correct, minimal, and targeted. Repeated restores of the same prompt text collapse to one copy; restoring different text still prepends above the user's draft, byte-for-byte unchanged. 1. Real production-path controlled experiment (the decisive one)I wrote a hook-level test that renders the actual
This proves the bug reproduces on the real production path and that the new guard is load-bearing. 2. Deterministic Before/After matrixReal exported 3. Automated coverage & regression
* The only initial red was ** Notes for merge
Verdict: ✅ ready to merge. 中文说明(点击展开)✅ 本地验证报告 — LGTM,建议合并在隔离 worktree 上于 head 结论: 修复正确、最小化、精准。同一条 prompt 文本重复恢复会收敛为一份;恢复不同文本时仍照旧前置到用户草稿之上,逐字节不变。 1. 真实生产路径的对照实验(最关键)我写了一个 hook 级测试,渲染真实的
这证明该缺陷在真实生产路径上确实复现,且新 guard 是关键性修复。(截图见上方英文部分) 2. 确定性的 Before/After 矩阵真实导出的 3. 自动化覆盖与回归
* 唯一初始报红的是 ** 在此符号链接 worktree 中 合并备注
结论:✅ 可以合并。 |
…t handler (QwenLM#7169) Post-merge review follow-ups from QwenLM#7134 and QwenLM#7135: - restoreImages now runs only alongside an actual text change: it appends to the pasted-image list, so a deduplicated restore of the same prompt (reconnect/retry) previously doubled the attachments while the text correctly stayed single. - The Cmd+B/Ctrl+B handler no longer toggles the sidebar while a non-composer editable element is focused (sidebar search, session rename, settings inputs) — matching the codebase's isEditableTarget convention; the composer keeps the editor-convention behavior and still toggles, verified in a real browser. - All state updates moved out of React updater functions (purity contract, mirroring the hamburger handler); the effect re-binds on the involved state so the listener closure stays fresh. - A drawer forced open on a wide viewport now takes the drawer path: the shortcut closes the overlay instead of collapsing the rail invisibly underneath it. Refs QwenLM#7102 QwenLM#7128 Co-authored-by: Claude Fable 5 <noreply@anthropic.com>



What this PR does
Makes the composer's prompt-restoration merge idempotent, so a prompt whose text is already sitting at the top of the editor is not stacked again. Restoration paths (failed queue submits, failed mid-turn inserts, queue clears racing an abort) prepend a failed prompt's text above the current draft; several of them can fire for the same prompt across reconnects and page refreshes, and a user retrying an identical message restores identical text — each pass previously prepended another copy. The merge logic is extracted into an exported pure function,
mergeRestoredPromptText(current, text), with the duplicate check documented in place; restoring different text still prepends above the draft exactly as before.Why it's needed
#7128 reports that after two failed sends of the same message, a page refresh shows the two texts concatenated into one string in the input box. The triage analysis identified three defects; this PR fixes the text-stacking one (bug 3):
restoreTextToEditorunconditionally computed`${text}\n${current}`, so duplicate restore passes and identical retried messages accumulated copies. The suggested fix directions were "replace instead of prepend, track restored prompts, or clear before batch restoration" — this PR implements the minimal-behavior-change variant: keep the prepend (a user's in-progress draft must not be destroyed), but make an already-restored text a no-op. The SSE-reconnect-on-prompt question (bug 1) is a behavioral default best decided by maintainers and is intentionally out of scope.Reviewer Test Plan
How to verify
Automated coverage: 6 new unit tests for
mergeRestoredPromptText— empty editor, prepend-above-draft, exact-duplicate no-op, duplicate-at-top no-op, idempotence across repeated restores of the reported CJK message, and a same-prefix-different-line case that must NOT be deduplicated.npm run typecheck, the web-shell package build, eslint and prettier are clean.Evidence (Before & After)
msginto empty editormsgmsgmsgmsg\nmsg❌msg✅msg\nmsg\nmsg❌msg✅msgabove a draftdraftmsg\ndraftmsg\ndraft(unchanged)Regression safety was additionally checked in a real browser: a Playwright run against a live
qwen servedaemon exercised the normal send path (submit accepted, editor cleared) and a network-refused direct submit — both behave identically before and after this change. The exact multi-restore browser reproduction depends on a mid-stream reconnect timing that is impractical to script deterministically, which is why the contract is pinned at the unit level.Tested on
Environment (optional)
macOS (Darwin 24.6), Node v22.23.1; vitest for the unit contract, Playwright Chromium against a live daemon for send-path regression.
Risk & Scope
restartEventStreamOnPromptdefault (triage bug 1) and the editor-clearing semantics on failed sends inApp.tsx/ChatPane.tsx(triage bug 2) — both are behavioral decisions that deserve their own review.mergeRestoredPromptTextis a new export used internally.Linked Issues
Fixes #7128
中文说明
本 PR 做了什么
让 composer 的 prompt 恢复合并逻辑幂等:若某条 prompt 的文本已位于编辑器顶部,不再重复堆叠。恢复路径(排队提交失败、mid-turn 插入失败、清空队列与 abort 竞争)会把失败 prompt 的文本前置到当前草稿之上;同一条 prompt 在重连/刷新过程中可能触发多条恢复路径,用户重试相同消息也会恢复相同文本——此前每次都会再前置一份副本。合并逻辑抽取为导出的纯函数
mergeRestoredPromptText(current, text)并附注释;恢复不同文本时仍照旧前置到草稿之上。为什么需要
#7128 报告:同一消息两次发送失败后,刷新页面看到两段文本拼接成一条出现在输入框。triage 分析识别出三个缺陷;本 PR 修复其中的文本堆叠缺陷(bug 3):
restoreTextToEditor无条件计算${text}\n${current},重复的恢复触发与重试的相同消息不断累积副本。triage 给出的修复方向是「改覆盖、跟踪已恢复、或批量恢复前清空」——本 PR 实现行为变更最小的变体:保留前置(不能破坏用户正在输入的草稿),但对已恢复的文本作 no-op。SSE 重连默认值问题(bug 1)属于应由 maintainer 决定的行为默认值,刻意不在本 PR 范围内。审阅测试计划
如何验证
自动化覆盖:
mergeRestoredPromptText的 6 个新单测——空编辑器、草稿上方前置、完全相同 no-op、顶部重复 no-op、对 issue 中中文消息的重复恢复幂等、以及不应被去重的同前缀不同首行用例。typecheck / web-shell 包构建 / eslint / prettier 全部通过。证据(Before & After)
见上方表格:重复恢复同一消息由
msg\nmsg\nmsg(❌)变为始终一份(✅);向草稿上方恢复不同文本的行为不变。另在真实浏览器中做了回归检查:Playwright 驱动真实qwen servedaemon 验证正常发送路径(提交成功、编辑器清空)与网络拒绝的直接提交在改动前后行为一致。精确的多次恢复浏览器复现依赖流中重连时序、难以确定性脚本化,故契约以单测层固定。测试平台
macOS 已本地验证(✅);Windows / Linux 依赖 CI(⚠️ )。
环境
macOS(Darwin 24.6)、Node v22.23.1;vitest 单测 + Playwright Chromium 对真实 daemon 的发送路径回归。
风险与范围
restartEventStreamOnPrompt默认值(triage bug 1)与App.tsx/ChatPane.tsx发送失败时的编辑器清空语义(triage bug 2)——两者都是值得单独评审的行为决策。mergeRestoredPromptText为新导出、仅内部使用。关联 Issue
Fixes #7128
🤖 Generated with Claude Code