feat: support drag and drop img in web-shell - #8696
Conversation
Allow Web Shell composers to ingest image files reliably while preserving the existing multimodal prompt protocol. - Share ordered image ingestion across desktop and mobile editors - Support image-only prompts and BMP preview and provider-safe handling - Preserve queued payloads across retries and uncertain outcomes - Add lifecycle guards, user feedback, unit coverage, and browser tests
Preserve complete prompt payloads and prevent duplicate or uncertain delivery states when admission responses race with queue lifecycle events. - Correlate admission, queue, and terminal events by prompt ID - Restore images and input annotations across retry and edit flows - Bound image reader concurrency and encoded attachment memory - Reconcile confirmed removals and explain ambiguous queue entries
Document the reviewed admission, recovery, and resource invariants. Keep the design aligned with the hardened Web Shell implementation. - Record bounded image ingestion and encoded-data budgeting - Clarify prompt lifecycle correlation and confirmed removal behavior - Describe annotation restoration and internal action boundaries - Update focused validation evidence and acceptance criteria
|
✅ Qwen Triage finished — CI landed green on ✅ Qwen Triage 已完成 —— |
|
Thanks for the PR! Template looks good ✓ Problem: real and observed — linked issue #8321 reports that Web Shell lacks the image drag-and-drop the TUI already supports (feature parity), and the issue carries the Direction: aligned — the TUI has handled image drag-and-drop since #3518, and this closes the gap for Web Shell. The claude-code CHANGELOG has no direct drag-and-drop reference, but image-input handling is an active area there. Size: large. ~2,069 production logic lines (web-shell + webui), ~2,139 test lines, and a 688-line design doc. The Approach: drag-and-drop ingestion reuses the existing paste/attachment pipeline, which is the right call, and the design doc is welcome. One question before deeper review: the PR also hardens the prompt admission/queue lifecycle (out-of-order start/terminal/HTTP events, duplicate prevention, ambiguous-failure handling). Some of that is entangled with image-only prompts and async ingestion, but parts look separable from drag-and-drop itself — was splitting that out considered? Risk: no elevated risk signals — no non-test files match the revert-correlated high-risk paths. Moving on to code review. 🔍 中文说明感谢贡献！ 模板完整 ✓ 问题：真实且已被观测到——关联 issue #8321 指出 Web Shell 缺少 TUI 已支持的图片拖拽（功能对齐），且该 issue 带有 方向：对齐——TUI 自 #3518 起支持图片拖拽，此 PR 补齐 Web Shell 的能力。claude-code CHANGELOG 没有直接的拖拽条目，但图片输入处理是其活跃领域。 规模：较大。约 2,069 行生产逻辑（web-shell + webui）、约 2,139 行测试、688 行设计文档。 方案：拖拽摄取复用了现有粘贴/附件管线，方向正确，设计文档也很好。一个问题：PR 同时加固了 prompt admission/队列生命周期（start/terminal/HTTP 乱序事件、去重、不确定失败处理）。其中一部分与纯图片 prompt、异步摄取耦合，但另一部分看起来可以与拖拽功能本身解耦——是否考虑过拆分？ 风险：无升级风险信号——没有非测试文件命中与 revert 相关的高风险路径。 进入代码审查 🔍 — 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 / afterℹ️ No screenshot changed against the PR base — but this PR edits 12 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 |
Code reviewI read the full diff against my own baseline proposal (reuse the paste pipeline for dropped files, preventDefault on dragover/drop, relax the submit guard for image-only prompts, bound the async reads). The PR matches that direction and goes further where it matters. What's done well:
No critical blockers found. One behavioral note reviewers should be aware of: the The prompt-admission lifecycle is the part most worth a reviewer's attention, so here is the flow as implemented: sequenceDiagram
participant P1 as Composer
participant P2 as Submit path
participant P3 as Daemon actions
participant P4 as Daemon
participant P5 as Queue lifecycle
P1->>P2: submit text and images
P2->>P3: sendPrompt with onAdmissionStarted
P3-->>P2: admission started
P3->>P4: submitPrompt request
alt response accepted
P4-->>P3: accepted with prompt id
P3-->>P2: onAdmitted, run or queue the turn
else response lost or ambiguous
P3-->>P2: error after start
P2->>P2: mark admission unknown, lock composer until resolved
else 413 or 501
P3-->>P2: definite rejection, restore payload for retry
end
P4-->>P5: started and terminal queue events
P5->>P5: correlate by prompt id before appending to transcript
Files changed (30 of 37 shown)
Testing evidence (from the PR's own CI, fetched via API — no PR code was executed here)The main unit suite is still running on the reviewed commit; nothing is red so far. The macOS/Windows test jobs and the CLI integration tests are skipped for this fork PR (authorization-gated), so the ubuntu jobs plus the PR's own focused suites are the available signal. The Final CI results for
One row per check name (latest run); skipped checks omitted; failures sort first. / 每个检查名一行（取最新一次运行），省略 skipped，失败项排在最前。 The test files themselves target the claimed races directly (out-of-order terminal/admission events, S1→S2→S1 owner changes, exactly-once transcript append, restore/discard of uncertain payloads), and the Playwright smoke spec drives a real Sandboxed verification would settle the remaining gap: 中文说明代码审查：已通读全部 diff。实现方向正确——拖拽摄取复用了现有粘贴/附件管线，新的 测试证据（来自 PR 自身的 CI，通过 API 获取——此处未执行任何 PR 代码）：ubuntu 主单元测试仍在运行，目前没有红色检查；macOS/Windows 测试与 CLI 集成测试因 fork PR 授权门控被跳过。表格会在 CI 结束后由 finalize 任务更新。测试文件直接针对所声称的竞态（乱序事件、S1→S2→S1 owner 切换、transcript 恰好追加一次、不确定载荷的恢复/丢弃），Playwright 冒烟用例用真实 DataTransfer 拖放事件断言了确切的 daemon 请求载荷。作者报告的测试数字（530 + 237）是其声明，未在此独立复跑。未验证：Windows/Linux 浏览器行为（作者未测试）。沙箱验证可补齐剩余缺口： — Qwen Code · qwen3.8-max Reviewed at |
|
Confidence: 4/5 — solid, well-tested feature work with no blocking findings; docking one point for the still-pending CI and the macOS-only manual testing. Stepping back: this started as "Web Shell should accept dropped images like the TUI does" (#8321) and the PR delivers exactly that, plus the lifecycle work needed to make image-only and queued prompts actually reliable. Comparing against my independent proposal, the PR goes further than I would have on the first pass — the ingestion lane with generation-based isolation and the prompt-id event correlation are things I might have deferred — but the tests demonstrate those races are real rather than speculative: out-of-order terminal/admission events, session owner changes mid-flight, exactly-once transcript appends all have dedicated coverage that pins the behavior. The code doesn't feel like it's trying too hard. The two new utilities are small and single-purpose, the drag-and-drop wiring reuses the existing paste pipeline rather than forking it, conventions are followed (design doc, bilingual strings, colocated tests), and core-package changes are test-only. My one standing reservation is the scope decision to let the admission-unknown lock affect text-only prompts too — it's the safe choice against double-sending, it's documented and tested, and a reviewer should still be aware the behavioral surface is wider than the title suggests. The author tested on macOS only; Windows/Linux browser behavior rests on CI and the synthetic-event Playwright spec. I'm approving because the implementation is genuinely good, not because I ran out of objections. CI is still running on the reviewed commit, so approval is deferred until CI lands green on 中文说明置信度：4/5 —— 扎实、测试充分的特性实现，无阻塞性问题；因 CI 尚未结束且作者仅在 macOS 上手动测试，扣一分。 整体来看：需求源于 #8321（Web Shell 应像 TUI 一样支持图片拖拽），PR 完整交付了该能力，并补齐了让纯图片 prompt 与排队 prompt 真正可靠所需的生命周期处理。与我独立的方案对比，PR 走得更远——带代际隔离的摄取泳道、prompt id 事件关联，这些我第一遍可能会推迟——但测试证明这些竞态是真实存在的而非臆测：乱序的 terminal/admission 事件、会话 owner 中途切换、transcript 恰好追加一次，都有专门的覆盖。代码没有过度设计：两个新工具模块小而专注，拖拽接线复用了现有粘贴管线而非另起炉灶，项目约定（设计文档、双语字符串、同目录测试）均被遵守，core 包仅改动测试。唯一的保留意见是 admission-unknown 锁定同样作用于纯文本 prompt——这是避免重复发送的稳妥选择，有文档和测试支撑，但审阅者应意识到其行为面比标题所示更宽。作者仅在 macOS 上测试，Windows/Linux 浏览器行为依赖 CI 与合成事件的 Playwright 用例。批准是因为实现确实好，而不是因为没有理由拒绝。由于 CI 仍在所审 commit 上运行，批准将推迟到 CI 全绿后由 finalize 任务提交（绑定到所审 commit；如有红色检查或 head 移动则不会批准）。 — Qwen Code · qwen3.8-max 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 — CI landed green after the review. ✅
|
@wenshao @yiliang114 Please take a look |
water-in-stone
changed the title
Review Summary — PR #8696Reviewed: 37 files, +4482/−414 at head of What this PR doesAdds image drag-and-drop to every Web Shell composer, reusing the existing paste/attachment/multimodal pipeline, and hardens the async prompt lifecycle (ordered bounded ingestion, prompt-ID correlation, dedup, safe retry/edit). Strengths
Observations[Info] Size/complexity. The PR is large and touches many lifecycle paths; the complexity is justified by the races described, but it's a lot to land at once. [Info] No blocking issues. CI green (Test, web-shell E2E, Capture visuals). Only ci-bot has approved so far. Approve. |
ytahdn
left a comment
ytahdn
left a comment
There was a problem hiding this comment.
LGTM. Well-engineered drag-drop ingestion (bounded, ordered, budgeted) and thorough prompt-lifecycle hardening (prompt-ID correlation, dedup, safe retry). Capture-phase preventDefault prevents navigation. Strong test coverage, CI green.
|
@yiliang114 Please take a look |
water-in-stone
changed the title
yiliang114
left a comment
yiliang114
left a comment
There was a problem hiding this comment.
LGTM. The hard parts check out: reader concurrency is genuinely bounded (worker-pool index handoff, batches serialized on a tail promise, cumulative 8 MiB budget inclusive at the boundary), stale readers are killed by lane identity + generation + abort with a post-await recheck, the race matrix is pinned by tests (terminal-before-response binding, no double append, removed-before-response no-append, exactly-once restore after definite rejection, unknown-payload restore/discard without resend), lost-response prompts are never auto-resent, and there is no new XSS surface (count-only notices, data-URL thumbnails, SVG stays blocked). The removed-before-dispatch vs started-then-cancelled distinction and the admission classifier are correct, and the core-package change is test-only.
One P2 worth fixing or testing away before merge: restoreQueuedPromptsToEditor drops the #7134 guard — images are now restored unconditionally after mergeRestoredPromptText, keyed by local row id. Same-row double restore is blocked, but if the payload text is already in the editor when a different row carrying the same payload is restored (re-typed/re-queued identical content, or a row re-materializing with a fresh id), the text dedupes while the images append again — exactly the case the old condition guarded. Suggested fix: skip image/annotation restore when nextText === currentText, or dedup on serverPromptId/payload hash; plus a restore-while-text-already-present regression test.
Minor notes: no content sniffing (extension/declared MIME trusted, parity with paste — the provider is the real validation boundary; worth a line in the design doc's trust-boundary section), drop protection is composer-scoped so drops on the message list still navigate (pre-existing), and the branch now conflicts with main and needs a rebase. Nothing blocks from my side.
…rop-img # Conflicts: # packages/web-shell/client/App.tsx
e6c85df
|
@ytahdn @yiliang114 All the conflicts have been resolved. Please take another look. |
|
@qwen-code-ci-bot @qwen-code-dev-bot Please take a look |
|
Qwen Code review timed out. Qwen review timed out after 21600 seconds (of the 360-minute budget). This run already used the maximum 360 minute timeout. See workflow logs. |
|
One P2 remains on the current head ( I reproduced this against the current head with a focused regression test: prefill the editor with Suggested minimal fix: when non-empty restored text is already present ( |
Skip payload attachments when restoring text is a no-op because the same prompt text already exists in the composer. - Restore images and annotations only when their text is inserted - Preserve image-only restoration regardless of the current draft - Add regression coverage for duplicate text with attachments
|
@ytahdn @yiliang114 Fixed in dc75bf6.
I added the requested regression test using an existing Validation:
Thanks for catching this. |
ytahdn
left a comment
ytahdn
left a comment
There was a problem hiding this comment.
Re-reviewed at dc75bf6. The duplicate attachment restoration issue is fixed: attachments are skipped when non-empty restored text is already present, while image-only restoration remains intact. The focused queue restoration suites pass 42/42 locally. No remaining blockers from my review.
yiliang114
left a comment
yiliang114
left a comment
There was a problem hiding this comment.
Re-approving at head dc75bf6 after my approval was dismissed by the new push. The P2 is fixed exactly as recommended: restoreQueuedPromptsToEditor now tracks textWasRestored (true only when mergeRestoredPromptText actually changes the editor text) and skips image/annotation restore when the payload text is already present — normal restores and image-only prompts are unaffected, annotation offset alignment holds in both branches, and the new dom test reproduces the exact duplicate scenario (payload text already in editor + 413 reject) asserting no restore calls while the queue drains. The accepted tradeoff is the safer direction: in the dedupe case a not-yet-restored attachment is silently dropped instead of duplicated, and it stays visible to the user. I also verified the main-merge resolutions in e6c85df are clean unions — all four conflicted files keep the full PR wiring (the fourth onImageIngestionNotice site correctly folded into main's artifactPanelSharedProps), so nothing from the original review was dropped. CI green on this head. Nothing blocks merge.
|
Released in v0.21.9. |
5 participants
What this PR does
This PR adds image drag-and-drop support to every Web Shell composer while reusing the existing paste, attachment preview, and multimodal prompt pipeline.
It supports PNG, JPEG, GIF, WebP, and BMP images, preserves attachment order across files and batches, and allows image-only prompts in the main chat, split panes, and side tasks. It also provides drop-state feedback, attachment removal, queued submission, edit, and retry behavior.
The prompt lifecycle has been hardened to:
The daemon wire format, ACP/Core protocol, and public Web Shell API remain unchanged.
Why it's needed
Web Shell already supported pasting images, but dropping image files onto the composer was not handled. Browser default drop behavior could also insert unwanted content or navigate away from the page.
The existing asynchronous ingestion and queue lifecycle had additional edge cases: attachments could be reordered, an image could be omitted if the prompt was submitted before reading completed, and response/event races could produce duplicate transcript entries or lose recoverable payload data.
This PR closes those gaps and makes image prompts reliable across direct, queued, image-only, retry, and editing flows.
Reviewer Test Plan
How to verify
git diff --checkpassed.Evidence (Before & After)
Automated Chromium coverage verifies PNG/BMP drop, preview ordering, image-only submission, deletion, request payload contents, transcript rendering, admission failure retry, and post-admission turn-error retry.
20260807-170544_._.s.video.meeting_0807_video.mp4
Tested on
Risk & Scope
Linked Issues
Closes #8321
中文说明
What this PR does
本 PR 为所有 Web Shell composer 增加图片拖放能力，并复用现有的图片粘贴、附件预览和多模态 prompt 链路。
支持 PNG、JPEG、GIF、WebP 和 BMP，能够保持多文件及多批次附件顺序，并让主聊天、split pane 和 side task 都可以发送 image-only prompt。同时覆盖拖放状态反馈、附件删除、排队发送、编辑和重试。
本次还强化了 prompt 生命周期：
daemon wire format、ACP/Core 协议及公开 Web Shell API 均保持不变。
Why it's needed
Web Shell 已经支持粘贴图片，但将图片文件拖入 composer 时不会被接管，浏览器默认行为还可能插入无效内容或离开当前页面。
原有异步摄取和队列生命周期也存在一些边界问题：附件可能因为读取完成顺序不同而乱序；读取完成前提交会遗漏图片；response 与生命周期事件发生竞态时，可能产生重复 transcript message 或丢失可恢复的完整 payload。
本 PR 补齐这些缺口，使 direct、queued、image-only、retry 和 edit 等图片 prompt 流程更加可靠。
Reviewer Test Plan
How to verify
git diff --check通过。Evidence (Before & After)
Chromium 自动化覆盖 PNG/BMP drop、预览顺序、image-only 提交、附件删除、请求内容、transcript 渲染、admission failure retry 和 post-admission turn-error retry。
20260807-170544_._.s.video.meeting_0807_video.mp4
Tested on
Environment (optional)
macOS 26.0、Node.js v22.22.3、本地 Web Shell mock daemon，以及 Chromium Playwright smoke tests。
Risk & Scope
Linked Issues
Closes #8321