feat(web-shell): support text file attachments in the composer - #9180
Conversation
github-actions
Bot
added
the
review/self-reported
E2E / Verification ReportCoverage approach: unit + integration level at every hop of the chain; no live-browser run yet (manual E2E plan maintained locally; happy to execute on request). Chain coverage
Suite results (macOS, Node 24)
|
|
Thanks for the PR — the composer attachment design here is thought through, and the linked issue makes the case well. Gate notes below. Template: complete ✓ Problem: real and observed, not theoretical. Issue #9179 documents the current behavior concretely — dropping a Direction: aligned. Sharing a log/output file with the agent is a core chat workflow, and the Web Shell currently has no way to do it. Claude Code's CHANGELOG has no direct text-file-drop reference, but attachment handling is an active area there, and the space is clearly relevant. This doesn't touch auth, sandbox, model selection, telemetry, or any public contract — no direction escalation needed. Size & branch state — the important one. The diff as displayed (2,116+/165−, 47 files) is inflated by a stale branch: it still carries the two original commits from PR #8862 (background cleanup for OpenAI API logs), which was squash-merged into main on Aug 10. That's why Approach: sound from the description and diff structure. Reusing the daemon's existing ACP embedded- Risk: no elevated risk signals — no Stage 1e high-risk paths among the production files (the only Moving on to code review. 🔍 中文说明感谢贡献——composer 附件的设计考虑得很周全,关联 issue 也把需求讲清楚了。以下是 gate 结论。 模板:完整 ✓ 问题:真实且已被观测到,不是理论性问题。Issue #9179 具体描述了现状——把 方向:对齐。把日志/输出文件分享给 agent 是核心聊天场景,而 Web Shell 目前完全做不到。Claude Code 的 CHANGELOG 没有文本文件拖放的直接参照,但附件处理在那边是活跃方向,该领域明显相关。本 PR 不涉及 auth、sandbox、模型选择、telemetry 或公共契约——无需方向层面升级。 规模与分支状态——重点。当前展示的 diff(2,116+/165−,47 文件)被过期分支放大了:分支上还带着 PR #8862(OpenAI API 日志后台清理)的两个原始 commit,而该 PR 已于 8 月 10 日 squash 合并进 main。这就是 方案:从描述和 diff 结构看是合理的。复用 daemon 已有的 ACP 内嵌 风险:无升级风险信号——生产文件中没有命中 Stage 1e 高风险路径(唯一触及 进入代码审查 🔍 — Qwen Code · qwen3.8-max Reviewed at |
Code reviewI read the full feature-commit diff (the last of the three commits — the first two are the already-merged #8862 housekeeping work, stale-branch artifacts as noted in the gate comment). The implementation matches and exceeds what I'd have proposed: classify at ingestion (MIME with an extension fallback), read as UTF-8 in the browser with NUL-byte binary rejection, one name normalization so chip label / resource URI / I verified the daemon-side contract against No critical blockers found. Two non-blocking observations:
Tests are substantial (~635 new test lines) and genuinely pin the change: NUL-byte binary rejection, independent image/text budgets, name sanitization and dedup, slash-command token omission, file-bearing queue entries never matching mid-turn injection, and the daemon-level sequenceDiagram
participant U as User
participant P1 as web-shell composer
participant P2 as webui session actions
participant P3 as daemon Session
participant P4 as Model turn
U->>P1: drop or paste text file
P1->>P1: classify by MIME or extension, read as UTF-8, sanitize name, 512 KB budget
P1->>P2: submit prompt with PromptFile entries
P2->>P3: text block with attachment token plus one resource block per file
P3->>P4: @uri text part plus File uri content part
P3-->>P1: transcript replay shows token text only (chips are local optimistic)
Files changed (30 of 39 shown — feature commit only, excluding the stale housekeeping diff)
CI test evidenceThe check suite on the reviewed commit currently contains only the bot orchestration jobs — and notably, the PR's own CI (
Sandboxed verification would settle the behavioural claim: 中文说明代码审查:我完整读了功能 commit 的 diff(三个 commit 中的最后一个——前两个是已合并的 #8862 housekeeping 工作,如 gate 评论所述,是过期分支的残留)。实现与我的独立设想一致且更完善:摄取时按 MIME(扩展名兜底)分类、浏览器内按 UTF-8 读取并以 NUL 字节拒绝二进制、一次性归一化文件名保证 chip 标签 / resource URI / 我对照 未发现关键阻塞项。两个非阻塞观察: 测试量充足(约 635 行新增测试)且真正钉住了改动:NUL 字节二进制拒绝、图片/文本预算独立、文件名归一与去重、slash 命令省略 token、带附件的排队条目不匹配 mid-turn 注入、daemon 层 CI 测试证据:被审 commit 上的 check 套件目前只有 bot 编排任务——值得注意,本 PR 自己的 CI( 沙箱验证可以落定行为性主张: — Qwen Code · qwen3.8-max Reviewed at |
|
Confidence: 3/5 — clean review with no blockers found; the defer is policy- and status-driven, not doubt about the code. Stepping back: this solves a real, demonstrated gap (the linked issue shows today's "1 unsupported image file(s) were skipped" behavior), and the approach is the right one — it reuses the daemon's existing ACP Three things keep this short of approval, and none of them is answerable from the code itself:
Suggested path forward for the author: rebase onto 中文说明置信度:3/5 —— 代码审查未发现阻塞项;缓议(转人工)是政策与状态原因,不是对代码本身有疑虑。 退一步看:这个 PR 解决的是真实且已被演示的缺口(关联 issue 展示了现状 "1 unsupported image file(s) were skipped"),方案也是对的——复用 daemon 已有的 ACP 有三点使它不能直接批准,且都不是代码本身能回答的:
给作者的建议路径:rebase 到 — Qwen Code · qwen3.8-max Reviewed at |
|
⏸️ Deferring to @pomelo-nwu @wenshao — Stage 0 policy escalation (cross-package ⏸️ 转交 @pomelo-nwu @wenshao —— Stage 0 政策升级(跨包 — Qwen Code · qwen3.8-max Reviewed at |
e898e30 to
3d6d01e
Compare
|
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）为单个提交。 |
Rebase note (conflict resolution)The branch was originally cut from an older tip and carried an unrelated base commit; it has been rebased onto
Also noteworthy for reviewers: main has since landed its own Verification after rebase (temp worktree @
|
🖼️ 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 10 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 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
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed.
Not explored to full depth (tool budget reached): "agent reverse-audit (round 2)": none — I completed every check I started within the tool budget.; "agent reverse-audit (round 1)": none — no check was cut short.; "agent reverse-audit (round 4)": none (finished within budget; ~17 tool calls used)..
Not reviewed: reverse audit — did not converge within the reverse-audit round cap of 5.
Test Plan (not a blocker): 3094 tests pass — this review observed 20051, 1556, 3610, 497, 494 passed; 467 tests pass — this review observed 20051, 1556, 3610, 497, 494 passed; 1502 tests pass — this review observed 20051, 1556, 3610, 497, 494 passed; 561 passing — this review observed 20051, 1556, 3610, 497, 494 passed.
中文说明
仅完成部分审查，审查缺口已披露。
未探索到全部深度（达到工具调用预算）："agent reverse-audit (round 2)"：none — I completed every check I started within the tool budget.；"agent reverse-audit (round 1)"：none — no check was cut short.；"agent reverse-audit (round 4)"：none (finished within budget; ~17 tool calls used).。
未审查：反向审计——在 5 轮的反审轮数上限内未收敛。
Test Plan（非阻断）：3094 tests pass — this review observed 20051, 1556, 3610, 497, 494 passed; 467 tests pass — this review observed 20051, 1556, 3610, 497, 494 passed; 1502 tests pass — this review observed 20051, 1556, 3610, 497, 494 passed; 561 passing — this review observed 20051, 1556, 3610, 497, 494 passed。
— qwen3.8-max via Qwen Code /review (v0.21.12)
| inputAnnotations ? { inputAnnotations } : undefined, | ||
| normalizedFiles, |
There was a problem hiding this comment.
[Suggestion] Both appendLocalUserMessage call sites (here in sendPrompt and ~597-601 in submitPrompt) pass normalizePromptFiles(...) output — which carries full text — as block metadata, contradicting the DaemonTextTranscriptBlock.files invariant this PR itself documents ("display metadata only — the content rides the prompt's resource blocks and is never stored on the block", sdk-typescript types.ts). The queue path (toStoreFiles in useQueuedPrompts.ts) strips text, so the two producers disagree, and actions.test.ts:1217 cements the violating shape. — Concrete cost: every optimistically rendered user message keeps a second copy of the attachment payload (up to the 512 KB aggregate budget) in transcript state for the session's lifetime; any future serializer/exporter of transcript blocks would emit file contents from what the type declares to be display-only metadata.
Pass a display-only projection to the store at both sites, keeping normalizedFiles (with text) for toDaemonPromptContent:
store.appendLocalUserMessage(
text,
normalizedImages,
inputAnnotations ? { inputAnnotations } : undefined,
normalizedFiles.map(({ name, mimeType }) => ({ name, mimeType })),
);中文说明
两处 appendLocalUserMessage 调用(sendPrompt 中此处与 submitPrompt 约 597-601 行)都把 normalizePromptFiles(...) 的输出——携带完整 text——作为块元数据传入,与本 PR 自己写下的 DaemonTextTranscriptBlock.files 不变量("仅展示元数据——内容随 prompt 的 resource 块传递,永不存放在块上",sdk-typescript types.ts)矛盾。队列路径(useQueuedPrompts.ts 的 toStoreFiles)会剥掉 text,两个生产者因此不一致,且 actions.test.ts:1217 把违规形状固化了下来。— 具体代价:每条乐观渲染的用户消息都会在 transcript 状态里保留附件载荷的第二份拷贝(最高 512 KB 聚合预算),持续整个会话生命周期;未来任何 transcript 块的序列化/导出器都会从类型声明为"仅展示元数据"的字段里输出文件内容。
修复:两处在传给 store 时改用仅展示用的投影,normalizedFiles(带 text)继续供 toDaemonPromptContent 使用(见代码块)。
— qwen3.8-max via Qwen Code /review (v0.21.12)
| actions | ||
| .sendPrompt(trimmed, { | ||
| ...(images && images.length ? { images } : {}), | ||
| ...(files && files.length ? { files } : {}), |
There was a problem hiding this comment.
[Suggestion] Pattern (6 hops): the new files wiring at each delivery/restore hop ships without test coverage — deleting any one hop's wiring leaves the whole suite green. This hop: ChatPane's forwarding of composer files into actions.sendPrompt — ChatPane.test.tsx contains no occurrence of files (grep-verified); every latestOnSubmit! invocation passes undefined in the files slot, so the empty-input guard, this spread, and the enqueuePrompt(..., files, ...) forwarding have no coverage. Hops: (1) useQueuedPrompts queue lifecycle/restore (~610); (2) this ChatPane → actions forwarding; (3) turn-error retry reattachment + draft restore (App.tsx ~9920/~9831); (4) ChatEditor chip render/remove (~2722); (5) App → webui happy-path handoff (App.tsx ~5675); (6) webui submitPrompt wiring (actions.ts ~604). — Failure scenario: deleting this spread leaves every test green; if it regresses, the prompt is admitted and sent but the attachment is silently dropped — the model never receives the file content. App-level tests cannot compensate: App.test.tsx's mock editor always passes undefined for files, and App.tsx does not render the ChatPane component.
Add a ChatPane test: submit via latestOnSubmit!('check this', undefined, [{ name: 'app.log', media_type: 'text/plain', text: 'x' }], commit) and assert sendPrompt received { files: [...] } (mirroring the existing image test at ~1129).
中文说明
模式问题(6 个环节):文件新布线在每个送达/恢复环节都没有测试覆盖——删掉任一环节的布线,整个套件仍然全绿。本环节:ChatPane 把输入框文件转发给 actions.sendPrompt——ChatPane.test.tsx 中 files 出现次数为零(grep 验证);所有 latestOnSubmit! 调用都在 files 槽传 undefined,因此空输入守卫、这个 spread、以及 enqueuePrompt(..., files, ...) 转发都没有覆盖。环节清单:(1)useQueuedPrompts 队列生命周期/恢复(约 610 行);(2)本环节 ChatPane → actions 转发;(3)turn-error 重试重附 + 草稿恢复(App.tsx 约 9920/9831 行);(4)ChatEditor chip 渲染/移除(约 2722 行);(5)App → webui 主路径交接(App.tsx 约 5675 行);(6)webui submitPrompt 布线(actions.ts 约 604 行)。— 失败场景:删掉这个 spread,所有测试仍绿;一旦回归,prompt 会照常准入并发送,附件被悄悄丢弃——模型永远收不到文件内容。App 层测试无法补偿:App.test.tsx 的 mock editor 总在 files 槽传 undefined,且 App.tsx 并不渲染 ChatPane 组件。
修复:新增 ChatPane 测试——用 latestOnSubmit!('check this', undefined, [{ name: 'app.log', media_type: 'text/plain', text: 'x' }], commit) 提交,断言 sendPrompt 收到 { files: [...] }(镜像约 1129 行的既有图片测试)。
— qwen3.8-max via Qwen Code /review (v0.21.12)
| let admissionStarted = false; | ||
| let admitted = false; | ||
| sendPrompt(retryText, retryImages, { | ||
| sendPrompt(retryText, retryImages, retryFiles, { |
There was a problem hiding this comment.
[Suggestion] Pattern (6 hops): the new files wiring at each delivery/restore hop ships without test coverage — deleting any one hop's wiring leaves the whole suite green. This hop: turn-error retry file reattachment (sendPrompt(retryText, retryImages, retryFiles, {...}) here) and the draft-restore wiring editor.restoreFiles(current.files) (~9831) — restoreFiles appears in App.test.tsx only as the mock editor's type + no-op stub; no test supplies prompt files or asserts restoreFiles was called, and every existing retry test was updated only to pass undefined in the new slot. Hops: (1) useQueuedPrompts queue lifecycle/restore; (2) ChatPane → actions forwarding; (3) this retry reattachment + draft restore; (4) ChatEditor chip render/remove; (5) App → webui happy-path handoff (~5675); (6) webui submitPrompt wiring. — Failure scenario: mutating this call to sendPrompt(retryText, retryImages, { leaves all tests green; if it regresses, a user who sends a prompt with an attachment, hits a send failure, and retries silently loses the file content. This coverage gap is also why the confirmed Critical in the retry stash/restore lineage shipped. Same shape for the draft-restore wiring: switching sessions away and back with a file-bearing draft silently drops the chips if the restore regresses.
Extend one retry test to submit with a PromptFile[], fail the send, trigger retry, and assert sendPrompt/sessionActions.sendPrompt receives the same files on the retry call.
中文说明
模式问题(6 个环节):文件新布线在每个送达/恢复环节都没有测试覆盖——删掉任一环节的布线,整个套件仍然全绿。本环节:turn-error 重试的文件重附(此处 sendPrompt(retryText, retryImages, retryFiles, {...}))与草稿恢复布线 editor.restoreFiles(current.files)(约 9831 行)——restoreFiles 在 App.test.tsx 中只以 mock editor 的类型 + 空实现出现;没有任何测试提供 prompt 文件或断言 restoreFiles 被调用,所有既有重试测试只是在新槽位补了 undefined。环节清单:(1)useQueuedPrompts 队列生命周期/恢复;(2)ChatPane → actions 转发;(3)本环节 重试重附 + 草稿恢复;(4)ChatEditor chip 渲染/移除;(5)App → webui 主路径交接(约 5675 行);(6)webui submitPrompt 布线。— 失败场景:把此处变异为 sendPrompt(retryText, retryImages, {,所有测试仍绿;一旦回归,用户发送带附件的 prompt、遭遇发送失败并重试时,文件内容会被悄悄丢失。这一覆盖缺口也正是重试暂存/恢复链路那个已确认 Critical 得以漏网的原因。草稿恢复布线同理:带着文件草稿切走再切回时,若恢复逻辑回归,chip 会被悄悄丢掉。
修复:扩展一个重试测试——以 PromptFile[] 提交、令发送失败、触发重试,断言重试调用收到相同的文件。
— qwen3.8-max via Qwen Code /review (v0.21.12)
| if (disabled) return; | ||
| core.removeFile(i); |
There was a problem hiding this comment.
[Suggestion] Pattern (6 hops): the new files wiring at each delivery/restore hop ships without test coverage — deleting any one hop's wiring leaves the whole suite green. This hop: ChatEditor's file-chip rendering and remove-button wiring — ChatEditor.test.tsx gained pastedFiles/removeFile mock plumbing (state slot, hasAttachments, removeFile: vi.fn(), render prop) but no test renders with pastedFiles populated, so this rendered path (the chip map with the core.removeFile(i) closure and disabled guard) executes in zero tests; the sibling image path has real disabled-guard and click coverage the file path lacks. Hops: (1) useQueuedPrompts queue lifecycle/restore; (2) ChatPane → actions forwarding; (3) turn-error retry reattachment + draft restore; (4) this chip render/remove wiring; (5) App → webui happy-path handoff; (6) webui submitPrompt wiring. — Failure scenario: a regression in the remove button's index closure (e.g. a stale i after reordering) or in the disabled guard removes the wrong file or none — the user then sends with a different attachment set than the chips display; useComposerCore.dom.test.tsx:927 tests the hook's removeFile filter, not this component's per-index onClick closure.
Render the mocked editor with two pastedFiles, assert both names appear, click the first remove button, and assert removeFile was called with index 0.
中文说明
模式问题(6 个环节):文件新布线在每个送达/恢复环节都没有测试覆盖——删掉任一环节的布线,整个套件仍然全绿。本环节:ChatEditor 的文件 chip 渲染与移除按钮布线——ChatEditor.test.tsx 增加了 pastedFiles/removeFile 的 mock 管道(状态槽、hasAttachments、removeFile: vi.fn()、render prop),但没有任何测试在 pastedFiles 有值时渲染,因此这条渲染路径(chip 映射、core.removeFile(i) 闭包与 disabled 守卫)在零个测试中执行;相邻的图片路径有真实的 disabled 守卫与点击覆盖,文件路径没有。环节清单:(1)useQueuedPrompts 队列生命周期/恢复;(2)ChatPane → actions 转发;(3)turn-error 重试重附 + 草稿恢复;(4)本环节 chip 渲染/移除布线;(5)App → webui 主路径交接;(6)webui submitPrompt 布线。— 失败场景:移除按钮的索引闭包回归(如重排后 i 过期)或 disabled 守卫回归时,会移除错误的文件或什么都不移除——用户最终发送的附件集合与 chip 显示不一致;useComposerCore.dom.test.tsx:927 测的是 hook 的 removeFile 过滤,不是本组件按索引的 onClick 闭包。
修复:以两个 pastedFiles 渲染 mock editor,断言两个名字都出现,点击第一个移除按钮,断言 removeFile 被以索引 0 调用。
— qwen3.8-max via Qwen Code /review (v0.21.12)
| const promptOptions: SendPromptOptionsWithRetry = { | ||
| images, | ||
| files, |
There was a problem hiding this comment.
[Suggestion] Pattern (6 hops): the new files wiring at each delivery/restore hop ships without test coverage — deleting any one hop's wiring leaves the whole suite green. This hop: the App-level happy-path handoff of composer files into sessionActions.sendPrompt (promptOptions.files, consumed at ~5712-5716) — App.test.tsx's mock composer submit hardcodes undefined in the files slot, and none of the mockSessionActions.sendPrompt assertion sites passes files; every toHaveBeenCalledWith merely gained an undefined in the new position. Hops: (1) useQueuedPrompts queue lifecycle/restore; (2) ChatPane → actions forwarding; (3) turn-error retry reattachment + draft restore; (4) ChatEditor chip render/remove; (5) this App → webui primary-send handoff; (6) webui submitPrompt wiring. — Failure scenario: dropping files, from promptOptions silently stops delivering attachments on every primary send while the whole suite stays green; typecheck cannot catch a missing optional property. Distinct hop from (2) ChatPane→actions and (3) retry.
Add an App.test.tsx case whose mock submit passes a non-empty files array through props.onSubmit and asserts mockSessionActions.sendPrompt receives it in options.files.
中文说明
模式问题(6 个环节):文件新布线在每个送达/恢复环节都没有测试覆盖——删掉任一环节的布线,整个套件仍然全绿。本环节:App 层把输入框文件交接给 sessionActions.sendPrompt 的主路径(promptOptions.files,在约 5712-5716 行被消费)——App.test.tsx 的 mock composer submit 在 files 槽硬编码 undefined,所有 mockSessionActions.sendPrompt 断言点都不传 files,每个 toHaveBeenCalledWith 只是在新位置多了个 undefined。环节清单:(1)useQueuedPrompts 队列生命周期/恢复;(2)ChatPane → actions 转发;(3)turn-error 重试重附 + 草稿恢复;(4)ChatEditor chip 渲染/移除;(5)本环节 App → webui 主发送交接;(6)webui submitPrompt 布线。— 失败场景:从 promptOptions 中删掉 files, 会让每次主发送都悄悄不再送达附件,而整个套件保持绿色;类型检查抓不住缺失的可选属性。与(2)ChatPane→actions、(3)重试是不同环节。
修复:新增 App.test.tsx 用例——mock 的 submit 通过 props.onSubmit 传入非空 files 数组,断言 mockSessionActions.sendPrompt 在 options.files 中收到它。
— qwen3.8-max via Qwen Code /review (v0.21.12)
| const promptRequest: Record<string, unknown> = { | ||
| prompt: toDaemonPromptContent(text, normalizedImages), | ||
| prompt: toDaemonPromptContent(text, normalizedImages, normalizedFiles), | ||
| }; |
There was a problem hiding this comment.
[Suggestion] Pattern (6 hops): the new files wiring at each delivery/restore hop ships without test coverage — deleting any one hop's wiring leaves the whole suite green. This hop: the files wiring in actions.submitPrompt — the queued-prompt delivery hop, fed by useQueuedPrompts.submitPendingPrompt (files: prompt.files, useQueuedPrompts.ts ~882). The only new test (actions.test.ts:1187) exercises actions.sendPrompt; the existing submitPrompt tests pass only signal/optimisticUserMessage, never files; the web-shell queued-prompt suites mock sessionActions, so the real submitPrompt never runs there. Hops: (1) useQueuedPrompts queue lifecycle/restore; (2) ChatPane → actions forwarding; (3) turn-error retry reattachment + draft restore; (4) ChatEditor chip render/remove; (5) App → webui happy-path handoff; (6) this webui submitPrompt wiring. — Failure scenario (probe): severing this wiring, actions.submitPrompt('check this', {files:[app.log]}) sends {"prompt":[{"type":"text","text":"check this"}]} — and a files-only queued prompt carries no resource block at all — while all 280 existing tests still pass: a mid-turn queued prompt with an attachment submits with the local chip rendered but the model receives no file content.
Add a submitPrompt counterpart to the new sendPrompt test: actions.submitPrompt('queued', { files: [{ name: 'app.log', text: 'line1', media_type: 'text/plain' }], ... }) asserting both the session.submitPrompt prompt array (text+token and resource block) and the appendLocalUserMessage fourth argument.
中文说明
模式问题(6 个环节):文件新布线在每个送达/恢复环节都没有测试覆盖——删掉任一环节的布线,整个套件仍然全绿。本环节:actions.submitPrompt 的文件布线——排队 prompt 的送达环节,由 useQueuedPrompts.submitPendingPrompt 供给(files: prompt.files,useQueuedPrompts.ts 约 882 行)。唯一的新测试(actions.test.ts:1187)只覆盖 actions.sendPrompt;既有的 submitPrompt 测试只传 signal/optimisticUserMessage,从不传 files;web-shell 的排队 prompt 套件 mock 了 sessionActions,因此真实的 submitPrompt 在那里从不执行。环节清单:(1)useQueuedPrompts 队列生命周期/恢复;(2)ChatPane → actions 转发;(3)turn-error 重试重附 + 草稿恢复;(4)ChatEditor chip 渲染/移除;(5)App → webui 主路径交接;(6)本环节 webui submitPrompt 布线。— 失败场景(探针):切断该布线后,actions.submitPrompt('check this', {files:[app.log]}) 发送 {"prompt":[{"type":"text","text":"check this"}]}——纯文件的排队 prompt 完全没有 resource 块——而现有 280 个测试全部通过:mid-turn 排队的带附件 prompt 提交时本地 chip 照常渲染,模型却收不到任何文件内容。
修复:为新 sendPrompt 测试补一个 submitPrompt 对照:actions.submitPrompt('queued', { files: [{ name: 'app.log', text: 'line1', media_type: 'text/plain' }], ... }),同时断言 session.submitPrompt 的 prompt 数组(文本+token 与 resource 块)与 appendLocalUserMessage 的第四个参数。
— qwen3.8-max via Qwen Code /review (v0.21.12)
… reset paths Review feedback on #9180: the turn-error retry lineage (CancelledRetryState, failedTurnErrorRetryRef, stash/restore sites) and the session-owner / identity-mismatch resets managed text/images/inputAnnotations but not files, which both dropped attachments on restored retries and could leak another session's attachment into a retry. Also fixes stale enqueuePrompt arity in the midTurnReconcile tests, chip a11y/truncation, and strips bidi/zero-width characters from attachment names.
Review round 1 — response summaryAll 3 [Critical] threads are addressed in
Additionally resolved 5 [Suggestion] threads in the same commit: failed-prompt rehydration keeps file chips, chip remove button Deferred (with rationale):
Suites after the fix: web-shell client 3617 passed (187 files, incl. the previously red midTurnReconcile file), typecheck/ESLint/Prettier clean. |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed.
Not reviewed: reverse audit — did not converge within the reverse-audit round cap of 5.
Test Plan (not a blocker): 3094 tests pass — this review observed 20051, 1556, 2101, 497, 494 passed; 467 tests pass — this review observed 20051, 1556, 2101, 497, 494 passed; 1502 tests pass — this review observed 20051, 1556, 2101, 497, 494 passed; 561 passing — this review observed 20051, 1556, 2101, 497, 494 passed.
中文说明
仅完成部分审查，审查缺口已披露。
未审查：反向审计——在 5 轮的反审轮数上限内未收敛。
Test Plan（非阻断）：3094 tests pass — this review observed 20051, 1556, 2101, 497, 494 passed; 467 tests pass — this review observed 20051, 1556, 2101, 497, 494 passed; 1502 tests pass — this review observed 20051, 1556, 2101, 497, 494 passed; 561 passing — this review observed 20051, 1556, 2101, 497, 494 passed。
— qwen3.8-max via Qwen Code /review (v0.21.12)
…ings Round-2 review on #9180: .ts/.mts report as video/mp2t (IANA binding) and .csv/.tsv as application/vnd.ms-excel on Windows+Office, so the non-empty MIME early-return made the extension allowlist unreachable for flagship source files. Extension and well-known-name (Dockerfile/Makefile/LICENSE/...) fallbacks now apply even when a conflicting MIME is present; the downstream NUL sniff still rejects actually-binary content. Also adds the failed-prompt retry variant with files, the full-shape file assertion, and the cancelled-retry no-resubmit pin from the round-2 test suggestions.
Review round 2 — response summaryNew [Critical] (OS MIME collisions) fixed in Three test-hardening **[Suggestion]**s also landed in the same commit: failed-prompt retry variant with files ( Deferred (with reasons in-thread): CI: the 4 cancelled One off-topic spam comment (non-member, unrelated language content) was left un-engaged; flagging for maintainer moderation if desired. Suites after the fix: web-shell client 3629 passed (187 files), typecheck/ESLint/Prettier clean. |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
No issues found. LGTM! ✅
Test Plan (not a blocker): 3094 tests pass — this review observed 20048, 1556, 3629, 497, 494 passed; 467 tests pass — this review observed 20048, 1556, 3629, 497, 494 passed; 1502 tests pass — this review observed 20048, 1556, 3629, 497, 494 passed; 561 passing — this review observed 20048, 1556, 3629, 497, 494 passed.
中文说明
未发现问题。LGTM！✅
Test Plan（非阻断）：3094 tests pass — this review observed 20048, 1556, 3629, 497, 494 passed; 467 tests pass — this review observed 20048, 1556, 3629, 497, 494 passed; 1502 tests pass — this review observed 20048, 1556, 3629, 497, 494 passed; 561 passing — this review observed 20048, 1556, 3629, 497, 494 passed。
— qwen3.8-max via Qwen Code /review (v0.21.12)
|
Released in v0.21.13. |
3 participants
What this PR does
The Web Shell composer now accepts text files via paste and drag-and-drop, in addition to the images it already supported. Dropped or pasted text files (.log, .txt, .md, .json, .csv, source files, …) appear as named attachment chips (icon + filename + size + remove button) next to the image thumbnails, and their contents travel with the prompt as ACP embedded
resourcecontent blocks, which the daemon already supports end-to-end — no daemon production changes were needed.Key behaviors:
text/*MIME types plus a smallapplication/*set (json, xml, yaml, toml, shell, sql, …), with a filename-extension fallback when the browser reports no meaningful type. Unsupported types keep the existing skip-notice behavior (copy generalized from "image file(s)" to "file(s)").@attachment:///<name>token always match exactly.@attachment:///<name>text token (the daemon transcript stores text blocks only), matching how image attachments replay today. Slash/shell commands drop attachment blocks on the daemon side (same as images), so token lines are omitted there to avoid dangling references.Why it's needed
Dropping a
.logfile into the Web Shell composer was previously rejected with "1 unsupported image file(s) were skipped" — the paste/drop channel was image-only. Sharing a log is one of the most common chat workflows, and the@pathmention can't help when the browser and the daemon run on different machines (the file only exists on the user's side). Closes #9179.Reviewer Test Plan
How to verify
npm run build, thenqwen serve) and open the Web Shell..logfile into the composer: a chip with the filename and size appears; no warning toast.@attachment:///...token instead of a chip (expected degraded display)..zip: toast "1 unsupported file(s) were skipped." Drop a > 512 KB text file: size-limit toast.Evidence (Before & After)
Before: dropping
app.loginto the composer produced the toast1 unsupported image file(s) were skipped.and nothing was attached. After: the file appears as a removable chip and its content reaches the model as aFile: attachment:///app.logreference block (asserted by a new daemon-level unit test). Unit-level evidence below; browser-level manual run not yet performed — the E2E plan is in the repo notes and can be executed on request.packages/web-shell: 3094 tests pass (177 files), including new ingestion/composer/chip/queue/dedup cases.packages/webui: 467 tests pass, includingtoDaemonPromptContentresource-block and token-rule cases.packages/sdk-typescript: 1502 tests pass, including local user-message file metadata.packages/cliSession tests: new prompt-path case proves aresourceblock is injected as aFile: <uri>reference part (561 passing; the 2 pre-existing cron-runtime failures are unrelated and reproduce on the base branch).Tested on
Environment (optional)
Local build + unit suites on macOS (Node 24). Daemon loopback verification was done at the unit level (
Session.test.tsprompt-path case), not against a live browser session.Risk & Scope
@attachment:///<name>token so the daemon-recorded transcript keeps a visible trace; the daemon also derives its own@uritoken from the resource block, so the model sees the token twice — cosmetic, invisible to the user. File contents are embedded in the prompt (they must be — a browser file has no server path), so very large logs require manual trimming beyond the 512 KB budget; non-BOM GBK-encoded files decode as mojibake.resourceblocks).Linked Issues
Closes #9179
中文说明
本 PR 做了什么
Web Shell 的输入框在原有图片支持之上,新增粘贴/拖拽文本文件的附件能力。拖入或粘贴的文本文件(.log、.txt、.md、.json、.csv、源代码文件等)会以命名的附件 chip(图标 + 文件名 + 大小 + 删除按钮)展示在图片缩略图旁,其内容以 ACP 内嵌
resource内容块随 prompt 发送——daemon 已端到端支持该格式,因此 daemon 端零生产代码改动。关键行为:
text/*MIME 加少量application/*(json、xml、yaml、toml、shell、sql 等),浏览器未给出有效类型时按扩展名白名单兜底;不支持的类型沿用原有跳过提示(文案从 "image file(s)" 泛化为 "file(s)")。@attachment:///<名字>token 三者完全一致。@attachment:///<名字>文本 token(daemon transcript 只存 text 块),与图片附件的重放行为一致。slash/shell 命令下附件块会被 daemon 丢弃(与图片一致),因此该场景不追加 token,避免悬空引用。为什么需要
此前把
.log文件拖进 Web Shell 输入框只会得到 "1 unsupported image file(s) were skipped" 提示——粘贴/拖拽通道仅限图片。分享日志是最高频的聊天场景,而浏览器与 daemon 不同机时@路径帮不上忙(文件只存在于用户本地)。关闭 #9179。评审验证计划
见上方英文部分:构建并启动 daemon 后拖入
.log验证 chip 出现、模型能引用日志内容、刷新后降级为 token;.zip与超大文件的拒绝提示;流式期间带附件消息排队行为。证据:web-shell 3094、webui 467、sdk-typescript 1502 单测全过;cli Session 新增 prompt 路径用例证明
resource块被注入为File: <uri>引用块(561 通过,2 个既有 cron 失败与本 PR 无关,基线可复现)。浏览器实机未跑,E2E 计划已留档可按需执行。仅在 macOS 验证。风险与范围
@attachment:///<名字>token 以便 transcript 保留痕迹,而 daemon 会从 resource 块再生成一个 token——模型会看到重复 token,属外观层面、用户不可见。文件内容必须嵌入 prompt(浏览器文件在服务端没有路径),超过 512KB 预算的大日志需手动截取;无 BOM 的 GBK 文件会乱码。resource块)。