fix(web-shell): preserve pasted text in composer - #7824
Merged
Conversation
Collaborator
Author
E2E test reportTested locally on macOS with Chromium and the Web Shell mock daemon.
|
Collaborator
ytahdn
force-pushed
the
fix/web-shell-paste
branch
from
July 27, 2026 08:48
33919b7 to
374ab38
Compare
Contributor
|
Please do not rebase or force-push to an active PR as it invalidates existing review comments. Note for future reference, the bots always squash all changes into a single commit automatically as part of the integration. 中文请勿对活跃的 PR 执行 rebase 或 force-push，因为这会使已有的评审评论失效。另外，供日后参考：作为集成流程的一部分，机器人始终会自动将所有改动压缩（squash）为单个提交。 |
Collaborator
|
Thanks for the PR! Template looks good ✓ Problem: this is labelled Direction: this is the part I'd like a maintainer's eye on. The CLI keeps its own Size: not applicable — Approach: clean and minimal. The whole placeholder subsystem goes out together, the removed exports aren't referenced anywhere else in the web-shell package, and there's a new e2e test covering a 1200+ char paste plus the unchanged image path. One thing in its favor: the composer already has two paste paths, and the non-CodeMirror one never collapsed large pastes — it always fell through to the native paste. This change makes the CodeMirror path match it, removing an inconsistency. No drive-by changes. Flagging the direction question for discussion before I weigh the code itself. 中文说明感谢贡献！ 模板完整 ✓ 问题： 这个 PR 标的是 方向： 这是我希望 maintainer 帮忙看的部分。CLI 在 规模： 不适用—— 方案： 干净、最小。整个占位符子系统一起移除，被删的导出在 web-shell 包内没有其它引用，并新增了一个 e2e 测试覆盖 1200+ 字符粘贴和未改动的图片路径。有一点支持本 PR：输入框本来就有两条粘贴路径，非 CodeMirror 那条从不做折叠——一直走原生粘贴。本改动让 CodeMirror 路径与之一致，消除了一处不一致。没有夹带其它改动。 先把方向问题提出来讨论，再深入看代码。 — Qwen Code · qwen3.8-max-preview Reviewed at |
Collaborator
|
Code review I read the diff against my own baseline (I'd expect exactly this: delete the placeholder subsystem, let CodeMirror's native paste handle text, keep image handling). The PR matches that cleanly.
No correctness, security, or convention issues found. My only reservation is the product-direction one from Stage 1, not the code. CI evidence Real PR CI is running on this commit but hasn't settled yet — I'm reporting status as-is, not guessing the outcome. The finalize pass updates the table below once CI completes. Final CI results for
One row per check name (latest run); skipped checks omitted; failures sort first. / 每个检查名一行（取最新一次运行），省略 skipped，失败项排在最前。 Real-scenario testing This is an unattended CI run, so I'm not driving the product live here. The change is user-visible (composer paste behavior), so the behavioral proof would come from the isolated lanes — and since this branch is in-repo (author has write access), a maintainer can trigger 中文说明代码审查 我对照自己的预期读了 diff（我的预期正是：删掉占位符子系统、让 CodeMirror 原生粘贴处理文本、保留图片处理）。PR 与之一致，且干净。
未发现正确性、安全性或规范问题。我唯一的保留是 Stage 1 提到的产品方向问题，而非代码本身。 CI 证据 本 commit 的真实 PR CI 正在运行但尚未出结果——我如实报告状态，不猜测结果。CI 完成后 finalize 流程会更新下方表格。 真实场景测试 这是无人值守的 CI 运行，所以我在这里不实际驱动产品。该改动对用户可见（输入框粘贴行为），行为层面的证据应来自隔离测试通道——由于本分支在仓库内（作者有写权限），maintainer 可以触发 — Qwen Code · qwen3.8-max-preview Reviewed at |
Collaborator
|
Confidence: 3/5 — the code is clean and I'd merge it on quality alone, but reversing a deliberate paste-collapsing feature with no linked issue is a product call I shouldn't make by myself. Stepping back: the implementation is exactly what I'd have written — a clean, complete removal that leaves the two composer paste paths consistent and adds the right e2e test. If this were a code-quality question, it's a yes. The reason I'm not approving is the direction question from Stage 1, which I couldn't resolve from the diff, the tests, or the PR description:
So I'm deferring rather than approving or requesting changes: the code deserves a green tick, the direction needs a human who knows whether cross-surface paste consistency was intentional. CI is also still in flight on this commit, so there's no test result to lean on yet either. ⏸️ Deferring to @wenshao (cc @chinesepowered) — is the web-shell meant to keep the large-paste placeholder for consistency with the CLI and the reference product, or is dropping it in favor of native editable paste the direction you want? The code itself is ready; this is purely a product-direction call. 中文说明置信度：3/5 —— 代码很干净，单看质量我会直接合，但在没有关联 issue 的情况下反转一个刻意设计的粘贴折叠功能，这个产品决定不该由我单独做出。 退一步看：实现正是我会写的样子——干净、彻底的移除，让两条输入框粘贴路径保持一致，并补上了恰当的 e2e 测试。如果这只是代码质量问题，答案是肯定的。 我不批准的原因在于 Stage 1 的方向问题，而我无法从 diff、测试或 PR 描述中解决它：
所以我选择暂缓，既不批准也不要求修改：代码值得一个绿勾，但方向需要一个了解"跨界面粘贴一致性是否刻意为之"的人来决定。本 commit 的 CI 也还在跑，暂时没有测试结果可依据。 ⏸️ 转交 @wenshao（cc @chinesepowered）—— web-shell 是否为了与 CLI 和参考产品保持一致而应保留大段粘贴占位符，还是你们希望改用原生可编辑粘贴的方向？代码本身已就绪，这纯粹是产品方向的取舍。 — 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 changes against the PR base. 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
Maintainer verification — built and tested locallyI built this branch from source and ran it against a pre-PR baseline to check the claims independently. Two isolated worktrees, same toolchain, same mock daemon: PR head Verdict: verified, and the fix is more valuable than the description claims. The placeholder was not just an editability annoyance — on 1. The bug is worse than described: editing the placeholder loses the pasteThe PR says "editing the placeholder could also discard the cached original text." Measured, that is silent data loss on a single keystroke. Scenario, run identically on both sides: paste 1,209 characters, press Backspace once, submit. I read the text back out of the mock daemon's
On
Root cause worth recording: 2. The stated risk — large text living in the editor — does not materialiseThis is the PR's own listed tradeoff, so I measured it rather than assuming. Paste sizes from 1.4 kB to 1 MB, timing the paste, the next 10 keystrokes, and submit-to-daemon:
The differences are inside run-to-run noise in both directions. CodeMirror virtualises the document, so the DOM node count is unchanged even at 1 M characters — the placeholder was buying no measurable performance, only the collapsed visual. Layout containment holds, as the description claims: a 200,000-character paste grows the composer to 222 px in a 720 px viewport and scrolls internally. The existing responsive-cap tests still pass at 1000/800/600 px.
3. The new test is a real regression testI copied the PR's new It also lands in the CI smoke lane ( 4. Suite results and side-effect checks
5. Two notes for the merge decision — neither is a blocker
Minor: the new EnvironmentmacOS 24.6.0 · Node 22 · Playwright chromium · two isolated git worktrees at 中文报告维护者验证 —— 已在本地构建并实测我从源码构建了该分支，并与合入基线做了独立对比验证。两个隔离 worktree、相同工具链、相同 mock daemon：PR head 结论：验证通过，且这个修复的价值比描述中说的更大。 占位符不只是「不方便编辑」的问题 —— 在 1. 实际问题比描述更严重：编辑占位符会丢失粘贴内容PR 描述说「修改占位符还可能导致缓存的原始文本丢失」。实测下来，这是一次按键就会触发的静默数据丢失。 两侧执行完全相同的场景：粘贴 1,209 个字符，按一次 Backspace，然后提交。文本从 mock daemon 的
在 根因值得记录： 2. 描述中列出的风险 —— 大文本留在编辑器里 —— 实测并不成立这是 PR 自己列出的权衡，所以我做了实测而不是假设。粘贴规模从 1.4 kB 到 1 MB，分别测量粘贴、粘贴后 10 次按键、以及提交到 daemon 的耗时：
差异在两个方向上都落在单次运行的噪声范围内。CodeMirror 对文档做了虚拟化，即使 100 万字符 DOM 节点数也不变 —— 占位符并没有带来可测量的性能收益，只带来了折叠后的视觉效果。 布局约束也如描述所说成立：粘贴 200,000 字符后输入框在 720 px 视口中增长到 222 px 并内部滚动。现有的 1000/800/600 px 响应式高度上限测试依然通过。 3. 新增测试是真实的回归测试我把 PR 新增的 该测试也进入了 CI smoke 通道（ 4. 测试套件结果与副作用检查
5. 两点供合并决策参考 —— 都不是阻塞项
小问题：新增的 环境macOS 24.6.0 · Node 22 · Playwright chromium · 两个隔离 git worktree，分别位于 |
Collaborator
|
@qwen-code /verify |
Collaborator
|
Sandboxed verification: infrastructure failure - workflow run The verification job did not complete (checkout, runner, or setup error) and produced no report. See the workflow run for details. — Qwen Code · sandboxed verification |
qwen-code-ci-bot
approved these changes
Jul 27, 2026
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
Collaborator
There was a problem hiding this comment.
No issues found. LGTM! ✅
— qwen3.7-max via Qwen Code /review
Collaborator
First live
|
Collaborator
|
@qwen-code /verify |
Collaborator
|
Sandboxed verification: infrastructure failure - workflow run The verification job did not complete (checkout, runner, or setup error) and produced no report. See the workflow run for details. — Qwen Code · sandboxed verification |
Collaborator
|
Sandboxed verification: infrastructure failure - workflow run The verification job did not complete (checkout, runner, or setup error) and produced no report. See the workflow run for details. — Qwen Code · sandboxed verification |
Collaborator
|
@qwen-code /verify |
ytahdn
commented
Jul 27, 2026
ytahdn
left a comment
ytahdn
left a comment
Collaborator
Author
There was a problem hiding this comment.
中文说明
— qwen3.8-max-preview via Qwen Code /review
Collaborator
|
Sandboxed verification: infrastructure failure - workflow run The verification job did not complete (checkout, runner, or setup error) and produced no report. See the workflow run for details. — Qwen Code · sandboxed verification |
Collaborator
|
@qwen-code /verify |
Collaborator
|
Sandboxed verification: infrastructure failure - workflow run The verification job did not complete (checkout, runner, or setup error) and produced no report. See the workflow run for details. — Qwen Code · sandboxed verification |
ytahdn
enabled auto-merge
July 27, 2026 12:22
Collaborator
|
@qwen-code /verify |
Collaborator
|
Sandboxed verification: infrastructure failure - workflow run The verification job did not complete (checkout, runner, or setup error) and produced no report. See the workflow run for details. — Qwen Code · sandboxed verification |
doudouOUC
approved these changes
Jul 27, 2026
doudouOUC
left a comment
doudouOUC
left a comment
Collaborator
There was a problem hiding this comment.
No issues found. LGTM! ✅
中文说明
未发现问题。LGTM！✅
— qwen3.7-max via Qwen Code /review
chiga0
approved these changes
Jul 27, 2026
chiga0
left a comment
chiga0
left a comment
Collaborator
There was a problem hiding this comment.
Review — APPROVE
Reviewed at HEAD 374ab38. No Critical or Major issues. This is a clean, well-scoped removal: the large-paste placeholder mechanism is deleted completely, plain text now follows the editor's native paste path, and the image-paste path is preserved unchanged.
What I verified
Complete removal, no dangling references: Fetched the actual file content at HEAD (not the search index) and grepped — zero leftover references to normalizePastedText, isLargePaste, createLargePastePlaceholder, prunePendingPastes, expandLargePastePlaceholders, LargePastePlaceholderResult, pendingPastesRef, nextPasteIdRef, or the LARGE_PASTE_* thresholds in either useComposerCore.ts or useComposerCore.test.ts. The removed exports had no external consumers (only the file and its own test referenced them). Note the CLI's packages/cli/src/ui/components/InputPrompt.tsx defines its own independent LARGE_PASTE_* constants for the Ink terminal UI — correctly untouched by this PR.
Paste flow coherence at HEAD: Both paste handlers remain parallel and correct. The CodeMirror domEventHandlers.paste (line 2549) collects clipboard images and preventDefault + returns true only when an image is present; otherwise returns false so CodeMirror's native paste inserts the plain text. The mobile handleMobilePaste (line 1866) mirrors this for the textarea backend. The submit path now binds text directly to the inline-tag-replaced raw text (no placeholder expansion), and the composerUpdateListener / clear-path resets for the removed refs are gone. Image + text mixed paste still drops the text via preventDefault, but that is pre-existing behavior, not a regression from this PR.
Test coverage: The removed unit tests (-97) correspond exactly to the deleted pure functions — correct to drop. The new e2e test pastes long plain text as editable composer content is a strong integration-level replacement: it writes a 1360+ char string to the system clipboard, asserts the full text renders (and not.toContainText('Pasted Content')), types edited to prove editability, submits, and asserts the prompt body contains the edited content exactly. The pasteComposerText helper grants clipboard permissions per-origin and uses the platform-correct shortcut (Meta+V / Control+V).
Cross-Validation
| Finding | Other Reviewers | My Assessment |
|---|---|---|
| (none raised) | ci-bot APPROVE, doudouOUC APPROVE, ytahdn self-COMMENT — all "no issues" | Confirmed — independent blind review found no Critical/Major |
Additional Audit Coverage
Areas I independently checked beyond the existing LGTMs:
- Leftover-reference audit at HEAD: grepped actual file bytes (search index was stale pre-merge) — confirmed zero dangling symbols in both source and test.
- Submit-path data flow: verified the
textWithInlineTags→textrename preservesreplaceInlineTagPlacementsbehavior; inline tags still expand correctly without the placeholder layer. - Adversarial pass: paste→edit→submit (covered by e2e), multiple sequential pastes (native cursor insertion, no ID tracking needed), undo after paste (native paste integrates cleanly with CodeMirror undo history — an improvement over the prior single placeholder transaction), and draft persistence (the real text is now in the doc, so session drafts save the actual content rather than a placeholder whose cached original could be lost — also an improvement).
- Cross-package boundary: confirmed the CLI's separate large-paste constants are independent and unaffected.
Collaborator
|
Sandboxed verification: infrastructure failure - workflow run The verification job did not complete (checkout, runner, or setup error) and produced no report. See the workflow run for details. — Qwen Code · sandboxed verification |
1 similar comment
Collaborator
|
Sandboxed verification: infrastructure failure - workflow run The verification job did not complete (checkout, runner, or setup error) and produced no report. See the workflow run for details. — Qwen Code · sandboxed verification |
Collaborator
|
Released in v0.21.1. |
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
5 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
Removes the Web Shell composer behavior that replaced large pasted text with a
[Pasted Content N chars]placeholder. Plain text now follows the editor's native paste path, so the complete clipboard content remains visible and editable. Image paste behavior is unchanged.Why it's needed
Text longer than 1000 characters or 10 lines was hidden behind a placeholder, so users could not modify the actual pasted input before submitting it. Editing the placeholder could also discard the cached original text. The composer already has a maximum height and scrolls long content internally, so large pasted text does not need a separate collapsing mechanism.
Reviewer Test Plan
How to verify
Paste text longer than 1000 characters into the Web Shell composer. Confirm that the full text is displayed instead of a placeholder, append or remove text, submit the prompt, and verify that the submitted prompt exactly matches the edited content. Paste a supported image and confirm that it is still added as an attachment.
Evidence (Before & After)
Before: large plain-text pastes appeared as
[Pasted Content N chars]and the original text could not be edited in place.After: the complete pasted text appears in the composer, remains editable, and is submitted exactly as edited. The existing composer height limit keeps large pastes contained within the layout. Automated browser coverage verifies a 1200+ character system-clipboard paste and the existing image-paste path.
Tested on
Environment (optional)
Local Chromium through Playwright with the Web Shell mock daemon.
Risk & Scope
Linked Issues
N/A
中文说明
此 PR 的改动
移除 Web Shell 输入框将大段粘贴文本替换为
[Pasted Content N chars]占位符的行为。纯文本现在使用编辑器原生粘贴路径，因此剪贴板完整内容会保持可见且可编辑。图片粘贴行为不变。为什么需要
超过 1000 个字符或 10 行的文本此前会隐藏在占位符后面，导致用户提交前无法修改实际粘贴的输入内容，而且修改占位符还可能导致缓存的原始文本丢失。输入框本身已有最大高度限制，长内容会在内部滚动，因此大段粘贴文本不需要额外的折叠机制。
Reviewer 测试计划
验证方式
在 Web Shell 输入框中粘贴超过 1000 个字符的文本。确认显示完整文本而不是占位符，追加或删除文本后提交，并验证提交内容与编辑后的内容完全一致。再粘贴一张支持的图片，确认图片仍会作为附件添加。
前后对比证据
之前：大段纯文本粘贴会显示为
[Pasted Content N chars]，原始文本无法直接编辑。之后：完整粘贴文本会显示在输入框中，可以继续编辑，并严格按编辑后的内容提交。现有输入框高度限制会将大段粘贴内容约束在布局内。浏览器自动化测试覆盖了 1200 多字符的系统剪贴板粘贴和现有图片粘贴路径。
测试平台
环境（可选）
使用 Web Shell mock daemon 的本地 Playwright Chromium。
风险与范围
关联 Issue
无