feat(web-shell): support file attachments in mid-turn messages - #9570
Merged
Conversation
Collaborator
|
✅ Qwen Triage finished — CI landed green on ✅ Qwen Triage 已完成 —— |
Collaborator
|
Thanks for the PR! Template looks good ✓ Problem: real and verifiable. In the current code Direction: aligned. This completes parity between image and file attachments in Web Shell mid-turn sends, an area with active investment (mid-turn queue lifecycle, session attachments). Claude Code's CHANGELOG has no direct reference to mid-turn file attachments, but queued-message and attachment handling is clearly a live area there too. Size: cross-package (web-shell + webui), so the core gate applies. Breakdown: ~420 production logic lines, ~570 test lines, 19 lines of design doc. Well under the 500-line maintainer-awareness threshold and the 1000-line advisory. No size concerns. Approach: the scope feels right. It reuses the existing Risk: no elevated risk signals — no match against the revert-correlated high-risk paths. Moving on to code review. 🔍 中文说明感谢贡献！ 模板完整 ✓ 问题：真实且可验证。当前代码中 方向：对齐。这补齐了 Web Shell mid-turn 发送中图片与文件附件的一致性，该方向正在持续投入（mid-turn 队列生命周期、会话附件）。Claude Code 的 CHANGELOG 没有直接提到 mid-turn 文件附件，但排队消息与附件处理在那边也是活跃领域。 规模：跨包改动（web-shell + webui），适用核心门禁。拆分：约 420 行生产逻辑、约 570 行测试、19 行设计文档。远低于 500 行维护者关注阈值和 1000 行大 PR 建议线。规模无顾虑。 方案：范围合理。复用现有 风险：无升级风险信号——未命中与回滚相关的高风险路径。 进入代码审查 🔍 — Qwen Code · qwen3.8-max Reviewed at |
Collaborator
Code reviewNo critical issues found. The implementation extends the existing image path instead of building a parallel one, and the failure modes are conservative throughout. What I checked:
sequenceDiagram
participant P1 as Composer
participant P2 as useQueuedPrompts
participant P3 as Workspace reader
participant P4 as Session actions
participant P5 as Daemon
P1->>P2: enqueuePrompt, text with file annotation
P2->>P2: validate annotations, all must be files
P2->>P3: read file bytes, bounded at 8 MB
P3-->>P2: blob
P2->>P4: uploadAttachment, name and data, session-guarded
P4->>P5: store session attachment
P5-->>P2: attachment reference
P2->>P4: enqueueMidTurnMessage, display text plus references
P4->>P5: insert into the running turn
P5-->>P2: admission echo, reconcile restores the file row
P1->>P2: delete the queued message
P2->>P5: removeMidTurnMessage
P5-->>P2: removed
P2->>P4: removeAttachment, only after confirmation
Files changed (18 of 18 shown)
Test evidence (PR's own CI, fetched via API — PR code was not executed locally)At fetch time the two PR-CI workflow runs ( Final CI results for
One row per check name (latest run); skipped checks omitted; failures sort first. / 每个检查名一行（取最新一次运行），省略 skipped，失败项排在最前。 The new unit tests pin the hook logic meaningfully (they assert the bounded read call, the upload payload and session guard, the stripped display text, the resource references handed to the daemon, and cleanup after confirmed deletion), but they mock the daemon boundary. The end-to-end claim — real browser against a real daemon — rests on the author's macOS run per the test plan; Windows and Linux were not validated by the author. Sandboxed verification would settle this: 中文说明代码审查未发现关键问题。实现是在扩展现有图片链路而非另起炉灶，失败路径全程保守。核查要点：
测试证据拉取时 PR 自身的两条 CI 流水线仍在运行；安全与 Desktop Shell 检查已绿。macOS/Windows 测试与集成测试显示 skipped——这是仓库既有的 CI 设计（这些任务仅在 merge_group 运行），与本 PR 无关。目前无红色检查。表格由 finalize 任务在 CI 结束后原地更新。 新增单元测试实质性地钉住了 hook 逻辑（断言受限读取调用、上传载荷与会话守卫、裁剪后的展示文本、交给 daemon 的 resource 引用、确认删除后的清理），但 daemon 边界是 mock 的。端到端结论（真实浏览器 + 真实 daemon）目前依赖作者的 macOS 验证；Windows 和 Linux 未验证。 可用沙箱验证补上这一环： — Qwen Code · qwen3.8-max Reviewed at |
Collaborator
|
Confidence: 4/5 — clean review across every stage; the only gap is that CI has not landed yet and the real browser+daemon evidence is the author's macOS run, which the Stepping back: this is exactly the kind of PR the mid-turn attachment surface needed. The problem is not hypothetical — the current gate demonstrably routes any message with a file or an My independent take before reading the diff was essentially what the PR does, so there is no simpler route being missed here; if anything the PR is slightly more careful than the minimal version (the pre-await-snapshot race fix in the deletion path earns its place). The diff carries no unrelated changes. The two reservations are non-blocking: attachment display names fall back to the attachment id until the daemon protocol grows display-name metadata (declared out of scope, acceptable), and the end-to-end story rests on the author's macOS run plus mocked-daemon unit tests (the Approval is deferred until CI lands green on this commit — the two PR-CI runs (unit suite and web-shell visuals) were still in flight at review time. 中文说明置信度：4/5 —— 各阶段审查都很干净；唯一的缺口是 CI 尚未结束，且真实浏览器 + daemon 证据只有作者的 macOS 验证，可通过上面点名的 整体来看：这正是 mid-turn 附件能力需要的那种 PR。问题不是假设性的——当前门禁确实会把任何带文件或 我在读 diff 之前的独立方案与 PR 基本一致，因此不存在被错过的更简路径；PR 甚至比最小版本更谨慎（删除路径上对等待前旧快照的竞态修复是应得的）。diff 中没有无关改动。两点保留意见均不阻塞：附件展示名在 daemon 协议增加展示名元数据之前回退为 attachment id（已声明不在范围内，可接受）；端到端证据依赖作者的 macOS 验证加 mock daemon 的单元测试（审查评论中的 批准推迟到 CI 在本提交上变绿——审查时两条 PR CI 流水线（单测套件与 web-shell visuals）仍在运行。 — Qwen Code · qwen3.8-max Reviewed at |
Collaborator
🖼️ 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 |
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. |
qwen-code-ci-bot
approved these changes
Aug 20, 2026
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
Collaborator
There was a problem hiding this comment.
LGTM, looks ready to ship — CI landed green after the review. ✅
chiga0
reviewed
Aug 20, 2026
chiga0
left a comment
chiga0
left a comment
Collaborator
There was a problem hiding this comment.
No blocking findings.
Checked:
annotatedFiles()— offset arithmetic for leading/trailing whitespace, mixed-annotation fallback to server queue, empty-annotation case returns{displayText, paths:[]}so plain-text mid-turn still works. ✓- Upload ordering:
Promise.allSettled([...imageList, ...fileList, ...annotatedFileList]).uploadedAttachmentReferences.slice(imageList.length)aligns withsourceFiles = [...fileList, ...annotatedFileList]— JS spec preserves order, early-exit guard checks total count before slicing. ✓ - Session-ID propagation:
uploadAttachmentpassessessionId: targetSessionId;actions.tsguard throws on mismatch before delegating to session;removeAttachmenton deletion usestarget.sessionId(correct for old-session cleanup after switch). ✓ contentToFiles()fix: now setsattachmentId: record['attachmentId']so server-reconciled prompts can be cleaned up on deletion — previouslyattachmentIdwas absent andremoveAttachmentwould have been skipped silently. ✓restoreAdmission: preserves originaltext(with @-tokens),files(user-dropped only),inputAnnotations;restoreQueuedPromptsToEditorcorrectly callseditor.restoreFilesandeditor.restoreInputAnnotations. ✓payloadCompleteness: 'summary-only'only whenannotatedFileList.length > 0(async workspace read needed); regularfileListstays'complete'(data already in memory). ✓- Test validity: new tests for upload, @ annotation, restore-on-failure, cross-session delete all cover concrete failure paths. Upload mock discriminates image vs resource by
attachment.name, matching the'name' in attachmentbranch inactions.ts. ✓
Scope: source only. Not reviewed — docs/ and packages/*/README* changes (none in diff); Windows/macOS path behaviour (no host).
CI: E2E Smoke ✓ · Coverage ✓ · review-pr in_progress.
Reviewed with AI assistance.
chiga0
approved these changes
Aug 20, 2026
chiga0
left a comment
chiga0
left a comment
Collaborator
There was a problem hiding this comment.
LGTM — no blocking findings from the review.
qqqys
added a commit
to qqqys/qwen-code
that referenced
this pull request
Aug 20, 2026
Five conflicts, all in web-shell's composer surface, against upstream's mid-turn file attachments work (QwenLM#9570): - App.tsx / ChatPane.tsx: upstream added `onAttachmentPreview` to `QueuedPromptDisplay` while this branch wrapped that same element in the composer status stack alongside `GoalStatusStrip`. Kept the stack and carried the new prop (plus this branch's `canInsertMidTurn`/`onInsert`) onto it. - MessageItem.tsx / SystemMessage.tsx: upstream's hunk carried both `onAttachmentPreview` and `isLatest`. Only the first is new -- `isLatest` is context this branch deliberately removed along with its sole consumer (`GoalStatusMessage activateFooter`), so taking the hunk whole would have reintroduced an unused prop that no longer typechecks. - useQueuedPrompts.ts: both sides added helpers at the same offset (this branch's owner-key/stash helpers, upstream's `annotatedFiles`). Kept both.
Collaborator
|
Released in v0.21.15. |
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
3 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
This PR lets Web Shell insert file attachments into an active turn through the same session-attachment path already used for images. It supports both files attached in the composer and workspace files selected with
@, shows queued files beside image previews, opens them in the existing attachment preview panel, restores file rows from reconciliation and injection events, and removes uploaded file data after a queued mid-turn message is successfully deleted.Older daemons that do not advertise session attachments keep the existing next-turn server queue behavior. Prompts containing unsupported or mixed annotations also remain on that conservative fallback path so no reference semantics are dropped.
Why it's needed
While a turn is running, attached files previously bypassed mid-turn insertion and appeared as server-queued prompts, even though images could be inserted immediately.
@workspace file references had the same limitation. This made file behavior inconsistent with images and with an ordinary prompt sent together with a file.Reviewer Test Plan
How to verify
@and send it. Confirm the file is uploaded and inserted into the running turn, the@token is omitted from the displayed message text, and the injected message renders the same attachment row as an ordinary prompt with a file.Automated coverage verifies direct attachments,
@files, upload failure restoration, old-daemon fallback, transcript reconstruction, previews, successful deletion cleanup, failed deletion preservation, and deletion that completes after switching sessions.Evidence (Before & After)
Before: file attachments sent during a running turn appeared as “Server queued”, and
@workspace files could not follow the image insertion path.After: supported files are uploaded on send, shown as local mid-turn queued attachments, inserted into the running turn, previewable from the right-side row, and cleaned up when the queued message is deleted.
Tested on
Environment (optional)
Local package tests and TypeScript checks with the repository Node.js toolchain.
Risk & Scope
@files are read through the selected trusted workspace and retain the existing attachment size limit; unsupported or mixed annotations deliberately fall back to the next-turn queue.Linked Issues
N/A
中文说明
这个 PR 做了什么
这个 PR 让 Web Shell 可以通过图片已经使用的同一套会话附件链路，把文件附件插入正在运行的对话轮次。它同时支持输入框中附加的文件和通过
@选择的工作区文件；排队中的文件显示在图片预览旁边，可以在现有附件预览面板中打开；刷新队列或收到插入回显后能够恢复文件行；成功删除排队中的 mid-turn 消息后，也会清理已经上传的文件数据。未声明会话附件能力的旧 daemon 会保留现有的下一轮服务器排队行为。包含不支持或混合注解的消息也会走这一保守回退路径，避免丢失任何引用语义。
为什么需要这个改动
此前在一个轮次运行期间发送附件文件时，文件不会走 mid-turn 插入，而是显示为服务器排队；图片却可以立即插入。通过
@引用工作区文件也有同样限制。这导致文件行为既不符合图片行为，也不同于正常发送 prompt 加文件的效果。Reviewer 测试计划
如何验证
@选择一个工作区文件并发送。确认文件被上传并插入当前轮次，展示文本中不再保留@token，而且插入后的消息显示与普通 prompt 加文件相同的附件行。自动化测试覆盖了直接附件、
@文件、上传失败恢复、旧 daemon 回退、会话记录重建、附件预览、删除成功后的清理、删除失败时保留附件，以及切换会话后才完成删除的情况。前后对比证据
改动前：运行中的轮次收到文件附件时会显示“服务器排队中”，
@工作区文件也不能像图片一样插入。改动后：受支持的文件在发送时上传，显示为本地 mid-turn 排队附件，插入当前轮次，可从右侧附件行预览，并在删除排队消息时一并清理。
测试平台
环境（可选）
使用仓库 Node.js 工具链执行本地 package 测试和 TypeScript 检查。
风险与范围
@文件通过选中的受信任工作区读取，并沿用现有附件大小限制；不支持或混合的注解会有意回退到下一轮队列。关联 Issue
无