feat(web-shell): support workspace file uploads - #8874
Merged
Conversation
Collaborator
Author
E2E / Interactive Test ReportEnvironment: macOS, local Web Shell + daemon development setup. Interactive development validation covered the Automated regression evidence:
Not yet manually validated: real model consumption of uploaded text/image content, Windows UI behavior, and Linux UI behavior. These remain explicit reviewer checks rather than being inferred from file creation and composer insertion alone. |
Collaborator
ytahdn
added
the
autofix/takeover
Collaborator
|
Thanks for the PR! Template looks good ✓ Problem: this is a feature, not a bug fix, and the capability gap is real and observable: the Web Shell can reference workspace files via Direction: aligned. The Web Shell is a first-class surface in this repo, and letting a browser-based client place files into the workspace is table stakes for that surface. Claude Code's CHANGELOG has no direct match, but adjacent products do attach client-side files/photos into remote coding sessions, so the area is clearly relevant. Size: this is a cross-package change (cli/serve daemon routes + sdk-typescript + web-shell), so the core-infrastructure gate applies. Breakdown: ~1,455 production logic lines (607 in cli/serve, 188 in the SDK, 660 in web-shell TS; plus 112 CSS and 4 build-script), 1,635 test lines, 304 docs. As a Approach: the structure reads as focused — a separate binary upload route rather than extending the text-write route is the right call (binary safety and an independent 50 MiB limit), and the admission gate, no-clobber publish, deterministic conflict renaming, SDK parity, and composer UI each map to a stated requirement. No unrelated changes or drive-by refactors spotted in the file list. The web shell consumes Risk: no elevated risk signals from the revert-history path check. The daemon-route surface (workspace trust boundary, qualified-workspace scoping) is where this PR needs the most careful eyes in code review, and that's where I'll focus. Moving on to code review. 🔍 中文说明感谢贡献！ 模板完整 ✓ 问题：这是一个功能（feature）而非 bug 修复，能力缺口真实可见：Web Shell 可以通过 方向：对齐。Web Shell 是本仓库的一等界面，让浏览器端客户端把文件放入工作区是该界面的基本能力。Claude Code 的 CHANGELOG 没有直接对应条目，但同类产品确实支持把客户端文件/照片附加到远程编码会话，说明这个方向明确相关。 规模：这是跨包改动（cli/serve daemon 路由 + sdk-typescript + web-shell），适用核心基础设施门禁。明细：约 1,455 行生产逻辑（cli/serve 607、SDK 188、web-shell TS 660；另有 CSS 112、构建脚本 4），测试 1,635 行，文档 304 行。作为 方案：结构上很聚焦——用独立的二进制上传路由而不是扩展文本写入路由是正确的选择（二进制安全 + 独立的 50 MiB 上限），准入门、不覆盖已有文件的原子发布、确定性重名编号、SDK 对齐和输入框 UI 每一项都对应一个已声明的需求。文件列表中未发现无关改动或顺手重构。web shell 依赖 风险：回滚历史路径检查无升级风险信号。daemon 路由面（工作区信任边界、qualified workspace 作用域）是本 PR 在代码审查中最需要仔细的部分，我会重点看那里。 进入代码审查 🔍 — Qwen Code · qwen3.8-max Reviewed at |
Collaborator
Code reviewI wrote my independent proposal before reading the diff — separate binary route with its own size policy, content-length pre-check, bounded concurrency, atomic no-clobber publish with deterministic numbering, capability advertisement, SDK method with progress/abort, composer hook — and this PR matches or exceeds it everywhere. The places it goes beyond my sketch are all justified: temp-name What I verified against the base tree, per the daemon-route rules:
No critical blockers found. One honest caveat: this is ~1,450 production lines reviewed statically; the behavioral surface is only as proven as the test suite and the live lanes below. sequenceDiagram
participant P1 as Composer (web-shell)
participant P2 as SDK DaemonClient
participant P3 as Upload route (daemon)
participant P4 as WorkspaceFileSystem
P1->>P2: uploadWorkspaceFile with file and path
P2->>P3: POST octet-stream with progress events
P3->>P3: trust, content-type, size, dir admission
P3->>P4: writeBytesAtomic on a free candidate
P4-->>P3: size and hash, confirmed path
P3-->>P2: 201 with server-confirmed path
P2-->>P1: result fires inline file tag insert
Files changed (30 of 35 shown)
Testing evidence — the PR's own CI (unattended run; PR code is never executed here)The decisive suites for this commit were still running at review time — the unit suite The one red check, Final CI results for
One row per check name (latest run); skipped checks omitted; failures sort first. / 每个检查名一行（取最新一次运行），省略 skipped，失败项排在最前。 Not verified here: live upload behavior through a running daemon and the composer UI — unattended review is static; the author's manual testing (macOS only, per the PR body) is their claim, not evidence. Sandboxed verification would settle this: 中文说明代码审查：我在读 diff 之前先独立写下了自己的方案——独立二进制路由（独立大小策略）、Content-Length 预检、有界并发、原子不覆盖发布 + 确定性编号、能力声明、带进度/取消的 SDK 方法、输入框 hook——本 PR 处处达到或超过该方案。超出部分的细节也都有正当理由：255 字节文件名的临时文件 ENAMETOOLONG 处理、编号候选的 UTF-8 安全截断、入口/锁内/发布前的 generation 检查点、0o600 创建模式。 按 daemon 路由审查规则逐项核对：两条路由（legacy-primary 与 workspace-qualified）都在 strict 变更门禁之后，共享一个四槽准入门约束缓冲；qualified 路由复用现有 runtime 解析/信任助手，unknown/draining/untrusted 分别 400/503/403，且新测试逐状态钉死"绝不落回主工作区"；legacy 路由仅在主工作区信任权威时接入信任判断，与相邻写入路由一致；大小策略保持分离（文本 5 MiB 不变，上传 50 MiB，三处校验同一常量）；不覆盖语义由 fs 原语保证，编号策略在路由层，同名并发上传落在不同候选（含竞态测试），工作区内符号链接被编号绕过、越界链接直接失败，发布阶段即使中途出现符号链接也会拒绝；客户端 hook 在切换工作区时中止在传任务，选择器打开后重新校验目标，纯图片拖放仍走原图片附件流程。所有新增字段/选项我都 grep 了读取点，无死开关。未发现关键阻塞问题。 测试证据（无人值守运行，绝不执行 PR 代码）：本 commit 的关键套件（ubuntu 单元测试、Serve A/B、web-shell 视觉捕获）在审查时仍在运行，下表由 finalize 任务在 CI 落定后原地刷新。已完成且通过：Desktop Shell（ubuntu/windows）、Live Host（macOS）、Java 矩阵（ubuntu 11/17/21、macOS 21、Windows 21）、Real daemon E2E / Java 11、route、Classify PR、review-scan；macOS/Windows 的 Node 测试本轮被跳过，由 Linux 覆盖。唯一红色检查 takeover-ack 是 bot 编排噪音：日志显示其 ack 评论步骤因 GitHub API 限流（submitted too quickly）失败，该任务不构建也不运行 PR 代码。 此处未验证：真实 daemon 上的上传行为与输入框 UI（无人值守审查为静态审查；作者自述仅在 macOS 上手工验证，属作者声明而非证据）。沙箱验证可以补足： — 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 / afterℹ️ No screenshot changed against the PR base — but this PR edits 6 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 |
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
|
Confidence: 4/5 — the shipped code matches two green sandboxed verifications, a maintainer real-stack verification, and a clean post-merge fidelity check; the missing point is the five inline findings that merged as follow-ups. Status: merged. @wenshao squash-merged this PR as What this re-run still checked after the merge:
Shipped unresolved — now follow-ups on Housekeeping note: the 中文说明置信度：4/5 —— 已合入的代码与两次绿色沙箱验证、一次维护者真实栈验证、以及本次合并后一致性检查全部吻合；扣掉的一分是五条 inline 意见以"后续跟进"状态合入。 状态：已合并。 @wenshao 已于 2026-08-13 06:42 UTC 将本 PR 以 squash 方式合入（ 本次 re-run 在合并后仍核查了以下内容：
已合入但未解决——现在成为 备注：合并时本 PR 携带的 — Qwen Code · qwen3.8-max Reviewed at |
Collaborator
🩺 serve daemon A/BBuilt the PR base vs this PR head
|
| field | PR base (before) | this PR (after) |
|---|---|---|
features[] |
— | "workspace_file_upload" |
— Qwen Code · serve A/B
Collaborator
|
🤝 Takeover engaged: the autofix loop now manages this PR — it will address new review feedback and resolve base conflicts until the label is removed or the round cap is reached. Remove the 中文说明🤝 已接管：autofix 循环现在管理此 PR —— 将持续处理新的评审反馈与 base 冲突，直到移除标签或达到轮次上限。移除 |
Collaborator
|
🔀 Base updated: red check(s) [Test (ubuntu-latest, Node 22.x)] pass on current main — merged current main via update-branch; CI will re-run. 中文说明🔀 已更新 base：红色检查 [Test (ubuntu-latest, Node 22.x)] 在当前 main 上通过 —— 已通过 update-branch 合入当前 main，CI 将重新运行。 |
Collaborator
Collaborator
|
🤖 AutoFix ran out of time before finishing (idle-timeout (no output for 1200000ms — the sandbox likely hung at startup)) (attempt 1/100) — it will retry on the next scan. What I found before stopping: See the Qwen Autofix agent step logs for model/tool output. Run log: https://github.com/QwenLM/qwen-code/actions/runs/31391895896 🧠 Handled by Qwen Code · model/模型 |
Collaborator
|
🤖 AutoFix updated a stale base — the fix did not pass verification, but this PR was behind What I found before stopping: See the Qwen Autofix agent step logs for model/tool output. Run log: https://github.com/QwenLM/qwen-code/actions/runs/31394772225 🧠 Handled by Qwen Code · model/模型 |
Collaborator
|
🤖 AutoFix ran out of time before finishing (idle-timeout (no output for 1200000ms — the sandbox likely hung at startup)) (attempt 3/100) — it will retry on the next scan. What I found before stopping: See the Qwen Autofix agent step logs for model/tool output. Run log: https://github.com/QwenLM/qwen-code/actions/runs/31397491013 🧠 Handled by Qwen Code · model/模型 |
Collaborator
|
🔀 Base updated: red check(s) [Test (ubuntu-latest, Node 22.x), web-shell E2E Smoke (ubuntu-latest, Node 22.x), Post Coverage Comment (ubuntu-latest, 22.x)] pass on current main — merged current main via update-branch; CI will re-run. 中文说明🔀 已更新 base：红色检查 [Test (ubuntu-latest, Node 22.x), web-shell E2E Smoke (ubuntu-latest, Node 22.x), Post Coverage Comment (ubuntu-latest, 22.x)] 在当前 main 上通过 —— 已通过 update-branch 合入当前 main，CI 将重新运行。 |
Collaborator
|
🤖 Addressed the latest review feedback (round 4/100). What changed, and what I pushed back on: · 已处理最新评审反馈（第 4/100 轮）。改动内容与我反驳保留之处如下： Autofix address-review summary — PR #8874Feedback triage
Changes
Conflict notes
Deferred / remaining
VerificationCommands actually run this round, in order:
Not run (with reason): integration tests after 中文说明Autofix address-review 摘要 — PR #8874反馈分类
变更内容
冲突说明
暂缓 / 遗留
验证本轮实际执行的命令（按顺序）：
未执行的命令（附原因）： Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 Re-review when you have a moment. After round 100 this bot stops and leaves the PR for a human. · 有空请复审；第 100 轮后本 bot 停止并将 PR 交给人工。 🧠 Handled by Qwen Code · model/模型 |
Collaborator
|
🤖 AutoFix stopped: this counting window now contains 3 time-budget exhaustions (pushed rounds in between included; this round itself may have failed differently). That is 3 full agent runs that pushed nothing. 3 of those were silent-sandbox (idle) timeouts that no budget increase can cure — investigate the sandbox image and runner docker daemon for those. A human should investigate the sandbox image and runner docker daemon, then comment What I found before stopping: See the Qwen Autofix agent step logs for model/tool output. Run log: https://github.com/QwenLM/qwen-code/actions/runs/31408890753 🧠 Handled by Qwen Code · model/模型 |
Collaborator
|
⏸️ Takeover paused: this PR reached its round cap (100/100). Comment 中文说明⏸️ 托管已暂停：本 PR 达到轮次上限（100/100）。评论 |
qwen-code-ci-bot
requested changes
Aug 10, 2026
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
Collaborator
There was a problem hiding this comment.
Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.
Not explored to full depth (tool budget reached): You are review agent reverse-audit — Reverse audit agen...: none — all checks above completed within budget.; You are review agent reverse-audit — Reverse audit agen...: none — all checks above completed within budget.; You are review agent reverse-audit — Reverse audit agen...: none — all checks above completed within budget.; chunk 3: could not execute workspace-file-system.test.ts -t "writeBytesAtomic" — worktree dependency install is incomplete (missing lowlight , @vitest/coverage-v8 ),…; You are review agent reverse-audit — Reverse audit agen...: none — all checks I started completed within budget., and 6 more.
Not reviewed: reverse audit — did not converge within the reverse-audit round cap of 5.
中文说明
未审查：build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。
未探索到全部深度（达到工具调用预算）：You are review agent reverse-audit — Reverse audit agen...：none — all checks above completed within budget.；You are review agent reverse-audit — Reverse audit agen...：none — all checks above completed within budget.；You are review agent reverse-audit — Reverse audit agen...：none — all checks above completed within budget.；chunk 3：could not execute workspace-file-system.test.ts -t "writeBytesAtomic" — worktree dependency install is incomplete (missing lowlight , @vitest/coverage-v8 ),…；You are review agent reverse-audit — Reverse audit agen...：none — all checks I started completed within budget.，另有 6 条。
未审查：反向审计——在 5 轮的反审轮数上限内未收敛。
— qwen3.8-max via Qwen Code /review (v0.21.9)
Collaborator
|
🤖 Addressed the latest review feedback (round 1/100). What changed, and what I pushed back on: · 已处理最新评审反馈（第 1/100 轮）。改动内容与我反驳保留之处如下： Round-6 review findings — all 17 addressedCommit: Critical findings
Suggestions
Every new behavioral fix is covered by a focused Vitest regression test; the five highest-risk fixes (R6-8, R6-17, R6-19, R6-12, R6-2) were additionally mutation-verified (the fix temporarily removed → the new test fails; fix restored → green). VerificationCommands actually run on the final committed tree (all required gates re-run after every mutation restore):
Not run (not applicable): integration tests after 中文说明第 6 轮评审发现 — 全部 17 项已处理提交： Critical 发现
Suggestion 发现
每个新的行为修复都有聚焦的 Vitest 回归测试覆盖；风险最高的五项修复（R6-8、R6-17、R6-19、R6-12、R6-2）还做了变异验证（临时移除修复 → 新测试失败；恢复修复 → 全部通过）。 验证在最终提交的代码树上实际执行的命令（每次变异恢复后均重新执行所有必需门禁）：
未执行（不适用）： Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 Re-review when you have a moment. After round 100 this bot stops and leaves the PR for a human. · 有空请复审；第 100 轮后本 bot 停止并将 PR 交给人工。 🧠 Handled by Qwen Code · model/模型 |
…/pr-8874-review-fixes # Conflicts: # .qwen/review-context.json # packages/cli/src/commands/review/lib/manifest-repository-context.committed.test.ts
Collaborator
|
🤖 Reviewed the latest feedback — no changes needed. Why, point by point: · 已审阅最新反馈——无需改动。逐点说明原因如下： 🤖 Autofix review round: no action taken The feedback collected for this round contains nothing new to address:
No code changes were made and no commit was created. The PR head remains at the last reviewed commit. 中文说明🤖 Autofix 评审轮次：未采取任何操作 本轮收集到的反馈中没有需要处理的新内容：
未做任何代码改动，也没有创建新的提交。PR 的 head 仍停留在上一次已评审的提交上。 Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 🧠 Handled by Qwen Code · model/模型 |
yiliang114
previously approved these changes
Aug 13, 2026
yiliang114
left a comment
yiliang114
left a comment
Collaborator
There was a problem hiding this comment.
Approved after a security-focused pass over the daemon ingress path at head 2bf40fa.
Security boundary (verified in code, not just tests)
- Auth: both
/file/uploadand/workspaces/:workspace/file/uploadregister AFTER the globalbearerAuth(server.ts:1699) and behindmutate({ strict: true }), which requires a token even on loopback defaults; untrusted workspaces get 403. No new unauthenticated surface in LAN mode. Covered by therequires a token(401) anduntrusted_workspace(403) tests. - Containment: admission resolves the parent with
fs.resolve(dir, 'write')and re-resolves every numbered candidate from the literal query dir through the pre-existingresolveWithinWorkspace(same boundary as the reviewed read/write routes). Null bytes,./..basenames, trailing slashes, and suspicious patterns (UNC/device/8.3/drive-colon) are rejected before buffering;../escape returnspath_outside_workspacewith nothing written. - Symlinks: never writes through in-workspace links (numbers on), rejects escaping links and symlinked parents, treats ELOOP cycles as occupied names, and
assertCreateTargetAbsentrefuses link targets. The documented residual parent-symlink-swap TOCTOU is defense-in-depth guarded and identical to the existing text-write path — not new exposure. - Limits before buffering: 50 MiB cap shared by one constant across the Content-Length pre-check,
express.raw({ limit, inflate: false })(encoded bodies rejected, chunked can't bypass), andenforceWriteSizein the fs layer; 4-slot concurrency gate bounds worst-case memory and releases on finish/close/abort (tested end-to-end). - Overwrite semantics are explicit: uploads NEVER clobber. Publish uses
link()(atomic EEXIST guarantee closing the POSIX rename-overwrite race), O_EXCL temp reservation, temp-identity re-checks, and temp cleanup on failure; occupied names auto-number with a 1000-candidate cap → 409.
Correctness: no phantom 0-byte file on mid-admission disconnect (req.aborted || res.closed guard + test); distinct errorKind envelopes (413/415/400/403/409/429); client hook aborts in-flight + queued uploads on workspace/session switch with synchronous generation invalidation during render (addresses the round-6 R6-8 race); batch capped at 100 files; SDK transports (fetch/XHR) share one auth/timeout/error path. Capability workspace_file_upload + limits.maxWorkspaceFileUploadBytes plumbed through daemon → SDK → hook; all 15 i18n keys registered in EN and ZH.
Test coverage: ~1150 lines of route tests hit exactly the security boundaries (traversal, symlink escape/cycle/dangling, oversize declared + buffered + exact-limit, 401/403/415, 255/256-byte names, candidate exhaustion, gate slot leak scenarios, concurrent same-name landing on distinct candidates), plus 722 lines of SDK upload tests.
Round-6 findings: addressed by 72a89c1, merged into this head; remaining bot items are Suggestion-level test-pinning nits (R3-*), fine as follow-ups, none blocking.
Nits (non-blocking)
- P3: worst-case 4 × 50 MiB in-memory buffering is fine for a token-gated daemon; a streaming-to-temp variant would be the future direction if the cap grows.
- P3: a few R3-* suggestions about assertion pinning (e.g. abort-listener identity,
xhr.timeoutinheritance) could harden the suite later.
CI on head: 27 check runs all success/skipped except review-pr (the bot review workflow itself, still running) — no test failures, no known flakes.
Collaborator
Author
|
@qwen-code /verify |
Collaborator
|
Sandboxed verification: ✅ passed — merge-ready (agent verdict) - workflow run Ran the PR in an isolated, token-free container: A/B against the base build, mock-free harness assertions, targeted gates. Advisory evidence for human reviewers — not a review, an approval, or a CI check. Scripted assertions: 2166 passed · 0 failed · 2166 total 中文 — 判定：✅ 通过 · 可合入（agent 判定）沙箱验证在隔离、无凭证的容器中执行了该 PR 的代码（与 base 构建 A/B 对照、无 mock harness 断言、定向门禁）。仅作为评审证据，不构成评审、批准或 CI 检查。 脚本断言：2166 通过 · 0 失败 · 2166 总计 Verification reportPR 8874 Deep Verification (follow-up round) —
|
| # | Finding (previous round) | Severity | Status at new head 2bf40faf |
|---|---|---|---|
| F1 | NUL byte in any path → 500 internal_error echoing a daemon-internal absolute path (shared fs layer) |
note, pre-existing | Partially fixed. The round-6 commit added a pre-buffer NUL reject to the upload admission: live cells C6e–C6g → 400 parse_error "path must not contain null bytes", no path echo, nothing written; mutant R1 (guard removed) is killed by the round-6 test rejects the NUL-bearing path with parse_error and no denied event (expected 500 to be 400). The shared layer still 500s elsewhere: /file/write NUL → 500 + absolute-path echo on both head (cell N2) and base (cell B6) — A/A attribution unchanged; the residual gap is not caused by this PR and lives outside its surface. |
| F2 | Control/RTL characters in filenames accepted as literal POSIX names | nit | Stands. Re-measured at the new head (04-hostile-names-probe.png): bad\nname.bin, bad\rname.bin, bad\tname.bin, \u202edoc.bin (RTL override), ZWJ name — 5/5 accepted, 201, byte-identical on disk. Daemon-side correct; the Web Shell tag renderer should remain aware of deceptive RTL-override display. No action required for merge. |
| F3 | SDK browser-bundle headroom ~0.3 KB under the budget the PR bumps | observation | Stands. Re-measured: dist/daemon/index.js = 192,252 B (187.7 KB) vs the 188 KB budget → 260 B headroom. Gate proven live this round: real budget passes, a planted 180 KB budget throws Browser daemon SDK bundle is 192252 bytes; expected <= 184320. |
| F4 | 2 reds in manifest-repository-context.committed.test.ts from a root-owned dirty .qwen/review-context.json in the verify container |
environmental | Resolved. The merge of main took the new-base versions of both .qwen/review-context.json and the test (PR diff against HEAD^1 is empty for those paths); the file on disk now matches HEAD (git status clean) and the suite runs 5/5 green at HEAD in this same container. |
| note | PR description says "11 SDK upload tests"; the suite contains 28 | nit | Stands (harmless undercount) — suite still 28 tests, all green. |
Delta since the previous round
72a89c1fround-6 review fixes (12 files, the only PR-side code delta): (1) no-clobber collisions inwriteBytesAtomicno longer emitfs.deniedaudit events (expected control flow of the candidate loop); (2) candidates re-resolve from the literal request directory (queryDir) instead of the canonicalized one — uploads through symlinked parents whose target name trips the suspicious-pattern check (e.g.docs -> aux) now succeed; (3)express.rawgetsinflate: falseplus an explicit 415unsupported_media_typeenvelope for encoded bodies; (4)ECONNABORTED/aborted mid-body parses are swallowed (routine for large uploads); (5) trailing-slash paths rejected beforedirname/basenamenormalization; (6) NUL-byte paths rejected pre-buffer with 400parse_error(the F1 fix on this route's surface); (7) ELOOP symlink cycles occupying a candidate are treated as occupancy (number on) instead ofsymlink_escapeerrors.2bf40fafmerge of main → new base02f1e90a. Conflict surfaces.qwen/review-context.jsonandmanifest-repository-context.committed.test.tsresolved identically to the new base (zero PR diff on those paths). One stray artifact:packages/cli/src/ui/hooks/use-effort-command.tsdiffers from base by a single whitespace-only hunk (re-wrapped const declaration, 1@@hunk, no token-level behavior change) — a merge-resolution formatting remnant, noted, not a finding.
Central claim and A/B proof
Central claim (carried, re-measured): the daemon exposes capability-gated binary upload ingress (POST /file/upload, POST /workspaces/:workspace/file/upload) that publishes bytes byte-identically, never overwrites (deterministic name (n).ext numbering), enforces a 50 MiB binary policy distinct from the 5 MiB text policy, bounds concurrency at 4 shared slots, and honors trust + workspace-qualification semantics without fallback to the primary runtime — while leaving existing text-write behavior unchanged.
Round-6 sub-claims (new this round): symlinked-parent resolution fix, NUL pre-buffer reject, trailing-slash reject, encoded-body 415, abort tolerance, ELOOP-as-occupancy, audit suppression for collisions.
Driven against a real qwen serve daemon (compiled dist/, bearer token, folder trust enabled via settings.json + trustedFolders.json source file) with raw HTTP/chunked-socket requests — no stubs of the unit under test. Harness: upload-matrix.mjs (this dir); raw logs head-run/matrix-head.log, base-run/matrix-base.log.
| Cell group | Observable oracle | HEAD (79) | BASE (11) |
|---|---|---|---|
| C1 capability | features array contains workspace_file_upload; limits.maxWorkspaceFileUploadBytes = 52428800 |
✅ both | ✅ absent tag + absent limit key |
| C1.0 / B2 trust control | GET /workspace/trust → effective {state: trusted, source: file}, folderTrustEnabled: true |
✅ | ✅ (symmetric control) |
| C2 byte identity, mode | 1 MiB → 201, confirmed path, sha256 on disk, mode 0o600 |
✅ 4/4 | route 404, no file (B3) |
C3 unicode/%/space names |
exact-name round-trip + bytes | ✅ 6/6 | n/a |
| C4 no-clobber numbering | dup.txt→(1)→(2); dir occupancy; .env (1) + original untouched; 255-byte fit ≤255 chars |
✅ 8/8 | n/a |
| C5 traversal/absolute | ../, /etc/…, sub/../../, x/../../y → 4×400 path_outside_workspace; nothing outside; no .tmp litter |
✅ 6/6 | n/a |
| C6 suspicious + NUL (round-6) | CON, CON.txt, photo., trail → 400 parse_error; bad\0name.txt → 400 parse_error naming null bytes, no absolute-path echo |
✅ 7/7 | n/a (base has no route; B6 covers shared layer) |
| C7 byte cap | exactly 50 MiB → 201 identical (236 ms); 50 MiB+1 → 413 file_too_large + maxBytes; no partial |
✅ 5/5 | n/a |
| C8 chunked oversize | no Content-Length, 51 MiB streamed → 413 envelope; slot released (next 201) | ✅ 2/2 | n/a |
| C9 admission gate | 4 held chunked uploads → 5th 429 upload_busy + Retry-After: 1; all 4 complete 201; retry 201 |
✅ 7/7 | n/a |
| C10 symlinks | in-workspace symlink → alias (1).txt, target untouched; escaping symlink → 400 symlink_escape, nothing written |
✅ 4/4 | n/a |
| C11 content-type / missing path | JSON body → 415; no ?path → 400 |
✅ 2/2 | n/a |
| N1 queryDir (round-6) | upload through docs -> aux (DOS-name target) → 201, bytes land via symlink, confirmed path canonical (aux/probe.bin); two-hop chain also 201 |
✅ 3/3 | n/a (no route on base) |
| N3 trailing slash (round-6) | sub2/dir/ → 400 parse_error even though the dir exists |
✅ | n/a |
| N4 encoded body (round-6) | Content-Encoding: gzip → 415 unsupported_media_type; nothing written |
✅ 2/2 | n/a |
| N5 mid-body abort (round-6) | headers + partial body + socket destroy → daemon healthy, next upload 201 (slot freed), stderr clean, no partial file | ✅ 4/4 | n/a |
| N6 ELOOP cycles (round-6) | self-cycle cyc.txt -> cyc.txt → cyc (1).txt, symlink untouched; two-hop ca<->cb → ca (1) |
✅ 3/3 | n/a |
| N2 / B6 F1 attribution | /file/write NUL → 500 + path echo |
✅ (persists, shared layer) | ✅ identical (A/A) |
| C12 text-write control (both arms) | create 201 + content intact; >5 MiB replace (with expectedHash) → 413 file_too_large |
✅ 3/3 | ✅ 3/3 identical |
| C13 qualified semantics | unknown id → 400 workspace_mismatch, no fallback write; by-path selector → 201 into that workspace |
✅ 4/4 | qualified route 404 (B4) |
| C14 untrusted daemon | legacy + qualified upload → 403 untrusted_workspace; nothing written |
✅ 3/3 | n/a |
Totals: HEAD 79/79, BASE 11/11. Witness: 01-head-upload-matrix.png, 02-base-upload-matrix.png.
Contract clarifications observed (not defects): the confirmed path is the canonical workspace-relative form — uploading through docs -> aux confirms aux/probe.bin (the client tag inserts the canonical path); and the daemon serves a bootstrap /capabilities envelope for ~1 s after listening (features present, runtime-derived limits keys absent until the workspace runtime is up) — the harness polls /workspace/trust to 200 before asserting, as SDK clients do with Retry-After.
Corrections
None this round (no inaccurate prior review/bot claims to correct).
Findings (new this round)
No blocking findings. All 79 head cells and every gate green; 9/9 mutants killed. Two informational notes are folded into the table above (canonical confirmed path; bootstrap capabilities window) — both are documented contracts, not requested changes.
Gate and vacuity results
| Gate | Result |
|---|---|
CLI changed test files (7): workspace-file-write.test.ts + fs/workspace-file-system.test.ts (198), bridge-file-system-adapter, workspace-file-read, server.test.ts, server/telemetry, workspace-qualified-rest (1104) |
1302 passed / 0 failed |
manifest-repository-context.committed.test.ts (prev-round environmental red) |
5/5 passed at HEAD (see status table F4) |
Integration cli/qwen-serve-routes.test.ts (real daemon, incl. capabilities baseline with workspace_file_upload) |
36/36 passed (05-integration-serve-routes.png) |
SDK test/unit/DaemonClient.upload.test.ts |
28/28 passed |
Web Shell changed test files (6): App, AtMentionPanel, ChatEditor, useAtMentionMenu, useComposerCore.dom, useFileUpload |
689 passed / 0 failed |
| SDK build incl. 188 KB browser budget | exit 0; bundle 192,252 B; gate liveness proven (planted 180 KB budget → throws) |
| Typecheck/build | cli+core tsc --build on both arms (base rebuilt from HEAD^1 sources), SDK build (tsc), web-shell tsc -p tsconfig.json --noEmit — all green |
Mutation matrix — 9/9 killed, zero survivors. Positive controls: the unmutated suites above (198 CLI + 28 web-shell green). Every mutant failed the intended behavioral assertion, not an import/compile break (witness 03-mutation-matrix.png, per-mutant logs mutant-*-killed.log):
| Mutant | Change | Killed by | Verdict |
|---|---|---|---|
| M1 | writeBytesAtomic: MAX_UPLOAD_BYTES → MAX_WRITE_BYTES (5 MiB text cap) |
3 tests: fs-layer accepts a payload above the 5 MiB text cap (binary policy applies) + HTTP accepts an upload above the 5 MiB text cap (binary policy at HTTP) + accepts an upload of exactly MAX_UPLOAD_BYTES: expected 413 to be 201 |
killed |
| M2 | route: no-clobber continue → surface error |
11 numbering-family tests (.env (1) dotfiles, taken candidates, 255-byte trims incl. multi-byte boundary, concurrency race, dir occupancy, suspicious-root, 1000-candidate cap, no-fs.denied audit): expected 409 to be 201 |
killed |
| M3 | web-shell hook: resultPath: result.path → targetPath |
invokes onUploaded exactly once with the server-confirmed path: expected 'report.pdf' to be 'report (1).pdf' |
killed |
| G4 | admission: remove declared-Content-Length pre-check | rejects an oversized declared Content-Length with the upload 413 envelope (hangs/timeout without the guard) |
killed |
| R1 | round-6: remove NUL-byte admission check | rejects the NUL-bearing path with parse_error and no denied event (×2 shapes): expected 500 to be 400 — the mutant reproduces the exact pre-fix F1 behavior |
killed |
| R2 | round-6: remove inflate: false |
rejects an encoded request body instead of silently decoding it: expected 201 to be 415 — without the guard the gzip body is silently decoded and published |
killed |
| R3 | round-6: remove ELOOP-cycle continue |
numbers past a symlink cycle… + …two-hop symlink cycle…: expected 400 to be 201 |
killed |
| R4 | round-6: revert audit suppression (collisions audit again) | does not audit a no-clobber collision as fs.denied (got 1) + emits no fs.denied events for a successful auto-numbered upload (got 3, expected 0) |
killed |
| R5 | round-6: revert queryDir: dir → canonicalized workspaceRelative(req, resolvedDir) |
uploads through a symlinked directory whose target trips the pattern check: expected 400 to be 201 — exactly one test, the one added for this fix |
killed |
R2's surviving form is the dangerous one (silent publication of bytes the client never sent); the mutant proves the guard is load-bearing, not cosmetic.
Not covered
- Browser-rendered Web Shell UI (drag styling, progress strip cadence, tag insertion): container has no browser; covered by the jsdom suites (689 green) and code reading only. XHR upload progress events are browser-only by design (SDK throws pre-send where
XMLHttpRequestis undefined) — real progress cadence unmeasured. - Per-commit attribution: depth-2 shallow checkout; the two delta commit objects (
72a89c1f,2bf40faf) are present and their diffs verified directly, butgit rev-listacross the shallow boundary is unreliable, so the 22-commit metadata list was not individually exercised. Verified the aggregateHEAD^1..HEADdiff. - Live
fs.deniedobservation: audit suppression is proven at the unit layer via captured bridge-event lists (R4's kill shows the events that would appear); no live SSE sink was subscribed. - Draining/removed qualified workspaces (503
workspace_runtime_unavailable): pinned by the executedworkspace-qualified-rest.test.tssuite; the live matrix covered the unknown-workspace cell only. - Repo-wide lint: not run; per-workspace builds/typechecks above stand in. Repo-wide typecheck replaced by the same.
- A/B of the queryDir fix against round-5 head (
59d8d4dd): not rebuilt as a third daemon arm; proven instead by mutant R5 + live N1 cells. - PR-declared out-of-scope surfaces: resumable/chunked uploads, folder uploads, overwrite-in-place, ACP-HTTP parity, Windows/Linux manual UI.
Methodology
Linux container (Node v22.23.2), merge-ref checkout at depth 2 (HEAD merge ed9fd301ff, HEAD^1 base 02f1e90a, HEAD^2 head 2bf40faf). Base arm: git worktree at HEAD^1 with root + per-package node_modules symlinked from the already-installed head tree, scripts/generate-git-commit-info.js run for the gitignored generated file, then npm run build of packages/cli only (base dist stamped 02f1e90a0a; workspace_file_upload absent from its capabilities — verified by grep). Control cleanliness: PR diff empty for packages/core, package.json, package-lock.json (so reusing the head node_modules is a clean control); @qwen-code/qwen-code-core realpath resolves into the head tree whose core sources are byte-identical to base; packages/cli/src/serve/** contains zero @qwen-code/sdk imports (grep), so the changed SDK cannot leak into either arm. Harnesses booted real daemons (dist/index.js serve --port 0 --token … --workspace <mkdtemp>) with scratch HOME/QWEN_HOME, security.folderTrust.enabled=true in settings.json, trust granted/withheld via QWEN_CODE_TRUSTED_FOLDERS_PATH; waited past the bootstrap envelope by polling /workspace/trust to 200; drove them with fetch + raw http chunked sockets (gate holds, oversize streams, mid-body aborts) and asserted on bytes-on-disk + response envelopes. Mutants applied by exact single-occurrence string replacement, run through vitest, restored via git checkout -- with clean-status verification after each (worktree ended clean). Evidence images produced with scripts/verify-capture.mjs; raw logs and harness scripts live alongside this report. Base worktree removed after the A/B cells were captured.
Evidence images
Harness scripts and raw logs are in the workflow run artifacts (7-day retention).
— Qwen Code · sandboxed verification
…pload # Conflicts: # packages/web-shell/client/components/ChatEditor.tsx
Collaborator
|
🤖 Addressed the latest review feedback (round 2/100). What changed, and what I pushed back on: · 已处理最新评审反馈（第 2/100 轮）。改动内容与我反驳保留之处如下： Autofix round summary — PR #8874 (address-review, conflict resolution)Feedback triageNo actionable review feedback arrived since the last evaluation:
Conflict resolution (
|
qwen-code-ci-bot
added a commit
that referenced
this pull request
Aug 13, 2026
wenshao
added a commit
that referenced
this pull request
Aug 13, 2026
wenshao
reviewed
Aug 13, 2026
| // `clientRef` is reassigned during render, so an in-flight completion | ||
| // landing between commit and the reset effect below would otherwise pass | ||
| // the old-generation guard while already holding the NEW target's client. | ||
| if (prevTargetKeyRef.current !== targetKey) { |
Collaborator
There was a problem hiding this comment.
Aborting live uploads from the render phase is unsafe. controller.abort() and the generationRef bump are side effects executed while rendering, and React does not guarantee that a render commits. Under concurrent rendering, a transition that renders ChatEditor with a new sessionId/workspace and is then interrupted (higher-priority update rebases it away) will have already aborted the in-flight XHR and every queued controller, even though the committed tree still shows the old targetKey — the user's uploads disappear with no visible cause and the rows only clear when the (never-run) effect for that key would have fired.
The stated motivation (a completion landing between commit and the reset effect while clientRef already holds the new client) can be handled without render-phase mutation, e.g. by capturing the client alongside the generation when the item is dequeued (const activeClient = clientRef.current is already read per item) and comparing that captured client on completion, or by moving the invalidation into useLayoutEffect.
wenshao
reviewed
Aug 13, 2026
| // the picker closed without any upload, so give the query back. | ||
| restore?.(); | ||
| } | ||
| } else { |
Collaborator
There was a problem hiding this comment.
When the picker resolves after the target changed, the selected files are dropped with no user-visible trace. capturedKey !== uploadTargetKey takes the else branch and calls restore?.() — but restore is already undefined here: the useLayoutEffect keyed on uploadTargetKey (line ~1585) fired at the moment of the change and consumed/cleared uploadPickerRestoreRef, and this handler re-reads that ref. So the branch is a no-op.
Scenario: user opens the @ panel, picks "Upload file", and while the OS dialog is open the session id changes or the daemon capabilities refresh flips uploadTarget (e.g. workspaces[].trusted briefly re-reported). The user selects three files, the dialog closes, and nothing happens at all — no upload, no error row, no restored query. Consider pushing an error row (a new errorCode, e.g. targetChanged) instead of silently returning.
wenshao
reviewed
Aug 13, 2026
| ); | ||
| if (req.onProgress) { | ||
| return await this.uploadWithProgress(url, label, req, headers); | ||
| } |
Collaborator
There was a problem hiding this comment.
The non-progress path inherits the client-wide 30 s DEFAULT_FETCH_TIMEOUT_MS, which is far too short for the 50 MiB cap this route advertises. fetchWithTimeout keeps the abort signal armed for the whole request+consume window, so an SDK caller doing client.uploadWorkspaceFile({ path, data }) with a 40 MiB file on anything slower than ~12 Mbps uplink gets a TimeoutError mid-body and the daemon sees an aborted request (gate slot freed, no file published). The Web Shell only escapes this because it explicitly passes timeoutMs: 0.
Since MAX_UPLOAD_BYTES is 10x MAX_WRITE_BYTES, this operation should not silently take the generic REST default. Either default req.timeoutMs to 0 here (cancellation is already available via signal), or scale it off the payload size, and make the JSDoc on DaemonWorkspaceFileUploadRequest.timeoutMs say what the inherited default actually is.
Collaborator
Maintainer verification report (local real-stack run)I verified this PR end-to-end on a real local stack — no mocked daemon: Verdict: everything in the reviewer test plan checks out. No blocking issues found. 1. HTTP route contract (curl against the PR daemon)
2. Web Shell UI (Chromium via Playwright, real daemon behind it)
3. Automated tests (PR head, local)
Evidence“Upload file” entry in the
Batch done: conflict renamed to
Bounded progress with cancel affordance (throttled upload, “Uploading 10%”):
Image-only drop stays on the image attachment path (thumbnail chip, no upload):
Oversized file rejected inline, 50 MiB policy:
Old daemon (no
Untrusted workspace: upload entry hidden (server also rejects with
中文版本维护者验证报告（本地真实栈）在本地搭建了完全真实的环境验证本 PR——没有 mock daemon：daemon 直接从 PR 源码启动（head 结论：Reviewer 测试计划中的各项全部通过，未发现阻塞问题。 1. HTTP 路由合约（curl 直连 PR daemon）
2. Web Shell UI（Playwright 驱动 Chromium，背后是真实 daemon）
3. 自动化测试（PR head，本地）
截图见上方英文部分（同一组证据，托管于 |
wenshao
reviewed
Aug 13, 2026
| const matches = (uploadWorkspace.capabilities?.workspaces ?? []).filter( | ||
| (w) => w.cwd === atWorkspaceCwd, | ||
| ); | ||
| if (matches.length !== 1 || matches[0].trusted === false) |
Collaborator
There was a problem hiding this comment.
The two trust gates disagree on how to treat a missing trusted field. The qualified branch fails open (matches[0].trusted === false), the primary branch a few lines below fails closed (primaryMatches[0].trusted !== true). DaemonWorkspaceCapability.trusted is optional in the SDK type, so a daemon (or a proxy/older build) that omits it enables the upload entry points for a qualified workspace while disabling them for the primary one — the more permissive half being the one that can target an arbitrary registered workspace. Uploads would then 403 at requireTrustedWorkspaceRuntime, but only after the whole body has been sent. Suggest making both !== true.
wenshao
reviewed
Aug 13, 2026
| ...uploadsRef.current, | ||
| { | ||
| id: `upload-${uploadIdCounter}`, | ||
| file: files[accepted.length] as File, |
Collaborator
There was a problem hiding this comment.
The overflow notice row borrows files[accepted.length] — the first rejected file — as its file, and ChatEditor's strip renders {upload.file.name} next to the status text. Dropping 105 files therefore renders a row reading photo101.jpg — 5 more files were not added (limit per batch), which reads as if that specific file failed while the other four unnamed ones are unaccounted for. The row is synthetic; consider making file optional on FileUploadItem (or adding a notice status) and having the strip render only the message for it.
wenshao
approved these changes
Aug 13, 2026
Collaborator
|
@qwen-code /triage |
wenshao
enabled auto-merge
August 13, 2026 06:38
Collaborator
|
Sandboxed verification: ✅ passed — merge-ready (agent verdict) - workflow run Ran the PR in an isolated, token-free container: A/B against the base build, mock-free harness assertions, targeted gates. Advisory evidence for human reviewers — not a review, an approval, or a CI check. Scripted assertions: 2172 passed · 0 failed · 2172 total 中文 — 判定：✅ 通过 · 可合入（agent 判定）沙箱验证在隔离、无凭证的容器中执行了该 PR 的代码（与 base 构建 A/B 对照、无 mock harness 断言、定向门禁）。仅作为评审证据，不构成评审、批准或 CI 检查。 脚本断言：2172 通过 · 0 失败 · 2172 总计 Verification reportPR 8874 Deep Verification (follow-up round 2) —
|
| # | Finding (previous round) | Severity | Status at new head 015f1b09 |
|---|---|---|---|
| F1 | NUL byte in any path → 500 internal_error echoing a daemon-internal absolute path (shared fs layer) |
note, pre-existing | Partially fixed — holds. Re-measured live: upload admission cells C6e–C6h → 400 parse_error "path must not contain null bytes", no path echo, nothing written (4/4 pass). Mutant R1 (guard removed) killed by rejects the NUL-bearing path with parse_error and no denied event ×2 with expected 500 to be 400 — the mutant reproduces the exact pre-fix F1 behavior. The shared layer still 500s elsewhere: /file/write NUL → 500 + absolute-path echo on both head (cell N2) and base (cell B6) — A/A attribution unchanged; residual gap is not caused by this PR. |
| F2 | Control/RTL characters in filenames accepted as literal POSIX names | nit | Stands. Re-measured at the new head (cells in 01-head-upload-matrix.png): bad\nname.bin, bad\rname.bin, bad\tname.bin, \u202edoc.bin (RTL override), ZWJ name — 5/5 accepted, 201, byte-identical on disk. Daemon-side correct; the Web Shell tag renderer should remain aware of deceptive RTL-override display. No action required for merge. |
| F3 | SDK browser-bundle headroom under the budget the PR bumps | observation | Stands, improved. Re-measured: dist/daemon/index.js = 192,550 B (was 192,252 B; +298 B from main-merge SDK growth) vs the budget the PR now sets at 189 KB (193,536 B, bumped 186→188→189) → 986 B headroom (was 260 B). Gate liveness proven: planted 180 KB budget → build fails with Browser daemon SDK bundle is 192550 bytes; expected <= 184320; real budget passes. |
| F4 | reds in manifest-repository-context.committed.test.ts from a root-owned dirty .qwen/review-context.json in the verify container |
environmental | Resolved — holds. PR diff against the new base is empty for both .qwen/review-context.json and the test file; the suite runs 5/5 green at HEAD in this same container. |
| note | PR description says "11 SDK upload tests"; the suite contains 28 | nit | Stands (harmless undercount) — suite still 28 tests, all green. |
Delta since the previous round
015f1b09merge of origin/main (the only PR-side commit since head2bf40faf, per the metadata snapshot — its parents are unreachable in the depth-2 clone, so the delta is attributed from commit metadata + the aggregate diff, not from a direct three-way): conflict surface waspackages/web-shell/client/components/ChatEditor.tsx. Verified the resolution kept both sides: the effective diffHEAD^1..HEADstill carries the full upload wiring (useFileUploadintegration, capability gate onworkspace_file_upload, drop/picker plumbing,maxWorkspaceFileUploadBytesread fromcapabilities.limits), and only two removed lines exist outside upload code — two import lines that reappear in the restructured import block with all three symbols (ReactNode,RefObject,DAEMON_APPROVAL_MODES) still imported and used. No conflict markers anywhere underpackages/web-shell/client/. The 689-test Web Shell suite (including the 1174-lineChatEditor.test.tsxdelta) pins this surface and is green.- New base
5af31316(v0.21.11). The merge also moved the SDK budget comment trail (186→188for the upload surface,188→189for session reasoning-effort APIs merged in from main) — the live bundle measurement above (192,550 B) confirms the 189 KB budget is correct at the new head. - Carried:
packages/cli/src/ui/hooks/use-effort-command.tsstill differs from base by a single whitespace-only hunk (re-wrapped const declaration, no token-level change) — merge-resolution formatting remnant, noted, not a finding.
Central claim and A/B proof
Central claim (carried, re-measured): the daemon exposes capability-gated binary upload ingress (POST /file/upload, POST /workspaces/:workspace/file/upload) that publishes bytes byte-identically, never overwrites (deterministic name (n).ext numbering), enforces a 50 MiB binary policy distinct from the 5 MiB text policy, bounds concurrency at 4 shared slots, and honors trust + workspace-qualification semantics without fallback to the primary runtime — while leaving existing text-write behavior unchanged.
Driven against a real qwen serve daemon (compiled dist/, bearer token, folder trust enabled via settings.json + trustedFolders.json source file) with fetch + raw chunked-socket requests — no stubs of the unit under test. Harness: upload-matrix.mjs; raw logs head-run/matrix-head.log, base-run/matrix-base.log.
| Cell group | Observable oracle | HEAD (87) | BASE (11) |
|---|---|---|---|
| C1 capability | features contains workspace_file_upload; limits.maxWorkspaceFileUploadBytes = 52428800 |
✅ 2/2 | ✅ tag absent + limit key absent (B1) |
| C1.0 / B2 trust control | GET /workspace/trust → folderTrustEnabled: true, effective {state: trusted, source: file} |
✅ 3/3 | ✅ symmetric (B2) |
| C2 byte identity, mode | 1 MiB → 201, confirmed path, sha256 on disk, mode 0o600 |
✅ 4/4 | route 404, no file (B3) |
C3 unicode/%/space names |
exact-name round-trip + bytes (6 names) | ✅ 6/6 | n/a |
| C4 no-clobber numbering | dup.txt→(1)→(2); dir occupancy; .env (1) + original untouched; 255-byte fit exactly 255 B; multi-byte code-point-boundary trim |
✅ 8/8 | n/a |
| C5 traversal/absolute | ../, /etc/…, sub/../../, x/../../y → 4×400 path_outside_workspace; nothing outside; no .tmp litter |
✅ 6/6 | n/a |
| C6 suspicious + NUL | CON, CON.txt, photo., trail → 400 parse_error; bad\0name.txt → 400 naming null bytes, no absolute-path echo, nothing written |
✅ 8/8 | n/a (base has no route; B6 covers shared layer) |
| C7 byte cap | exactly 50 MiB → 201 identical (264 ms); declared CL 50 MiB+1 → 413 file_too_large + maxBytes; no partial |
✅ 5/5 | n/a |
| C8 chunked oversize | no Content-Length, 51 MiB streamed → 413 envelope; slot released (next 201); no partial | ✅ 3/3 | n/a |
| C9 admission gate | 4 held chunked uploads → 5th 429 upload_busy + Retry-After: 1; all 4 complete 201 (4/4); retry 201 |
✅ 4/4 | n/a |
| C10 symlinks | in-workspace symlink → alias (1).txt, target untouched; escaping symlink → 400 symlink_escape, nothing outside |
✅ 4/4 | n/a |
| C11 content-type / missing path | JSON body → 415; no ?path → 400 |
✅ 2/2 | n/a |
| N1 queryDir | upload through docs -> aux (DOS-name target) → 201, confirmed path canonical aux/probe.bin, bytes land via symlink; two-hop chain → 201 |
✅ 4/4 | n/a (no route on base) |
| N3 trailing slash | sub2/dir/ → 400 parse_error even though the dir exists |
✅ | n/a |
| N4 encoded body | Content-Encoding: gzip → 415 unsupported_media_type; nothing written |
✅ 2/2 | n/a |
| N5 mid-body abort | headers + partial body + socket destroy → daemon alive, next upload 201 (slot freed), no partial file, stderr clean of unhandled errors | ✅ 4/4 | n/a |
| N6 ELOOP cycles | self-cycle cyc.txt -> cyc.txt → cyc (1).txt, symlink untouched; two-hop ca<->cb → ca (1).txt |
✅ 3/3 | n/a |
| N2 / B6 F1 attribution | /file/write NUL → 500 + absolute-path echo |
✅ (persists, shared layer) | ✅ identical (A/A) |
| C12 / B5 text-write control | create 201 + content intact; >5 MiB replace (with expectedHash) → 413 file_too_large |
✅ 3/3 | ✅ 3/3 identical |
| C13 qualified semantics | POST /workspaces registers secondary; by-path selector → 201 into SECONDARY (not primary); unknown id → 400 workspace_mismatch; no fallback write |
✅ 5/5 | qualified route 404 (B4) |
| C14 untrusted daemon | legacy + qualified upload → 403 untrusted_workspace; nothing written |
✅ 3/3 | n/a |
| F2 probe | 5 control/RTL/ZWJ names accepted, byte-identical (documented posture, carried finding) | ✅ 5/5 | n/a |
Totals: HEAD 87/87, BASE 11/11. Witnesses: 01-head-upload-matrix.png, 02-base-upload-matrix.png.
Contract clarifications re-observed (not defects): the confirmed path is the canonical workspace-relative form (docs -> aux confirms aux/probe.bin); the daemon serves a bootstrap /capabilities envelope shortly after listening, so the harness polls /workspace/trust to 200 before asserting, as SDK clients do.
Corrections
None this round.
Findings (new this round)
No blocking findings. All 87 head cells and every gate green; 9/9 mutants killed.
Two informational observations, neither caused by this PR:
- Keep-alive desync after an early 413 (pre-existing stack class). A client that declares a Content-Length above the cap, sends a short body, receives the early 413, and then reuses the same keep-alive socket desyncs the connection — the server parser is still owed the declared body bytes, so the next request is misparsed (empty-body 400 or hang). Measured on head: the upload admission pre-check answers 413 instantly, next reused-socket request →
socket hang up. Measured on base through the pre-existingexpress.json({limit:'10mb'})path: the same lying-CL request gets no early answer at all (hangs until client timeout), then the reused socket gets a stray 404 — the same class, strictly worse. Honest clients (browsers, undici/fetch, the SDK) do not reuse a connection whose request body was not fully sent; the loopback daemon has no shared-front-end smuggling surface. This round's harness hit it itself (agent: falseper raw request was the fix). Not a PR finding; recorded so a future reviewer of connection-lifetime issues finds the attribution already done. - Whitespace-only hunk in
use-effort-command.ts(carried from the previous round's delta note) — still present against the new base; zero token-level change.
Gate and vacuity results
| Gate | Result |
|---|---|
CLI changed test files (7): workspace-file-write.test.ts + fs/workspace-file-system.test.ts, bridge-file-system-adapter, workspace-file-read, server.test.ts, server/telemetry, workspace-qualified-rest |
1303 passed / 0 failed (prev round 1302; +1 from the new base) |
manifest-repository-context.committed.test.ts (round-1 environmental red) |
5/5 passed at HEAD (status table F4) |
Integration cli/qwen-serve-routes.test.ts (real daemon, capabilities baseline incl. workspace_file_upload) |
36/36 passed |
SDK test/unit/DaemonClient.upload.test.ts |
28/28 passed |
Web Shell changed test files (6): App, AtMentionPanel, ChatEditor, useAtMentionMenu, useComposerCore.dom, useFileUpload |
689 passed / 0 failed |
| SDK build incl. 189 KB browser budget | exit 0; bundle 192,550 B; gate liveness proven (planted 180 KB → expected <= 184320) |
| Typecheck/build | web-shell tsc -p tsconfig.json --noEmit green; head npm run build pre-run by CI; base arm rebuilt packages/cli (+ packages/core via project references) from HEAD^1 sources with tsc --build green |
Mutation matrix — 9/9 killed, zero survivors. Positive controls: the unmutated suites above (1303 CLI / 689 web-shell / 28 SDK green). Every mutant failed the intended behavioral assertion, not an import/compile break (witness 03-mutation-matrix.png, per-mutant logs mutant-*.log):
| Mutant | Change | Killed by | Verdict |
|---|---|---|---|
| M1 | writeBytesAtomic: cap → 5 MiB text cap (inline literal) |
3 tests: fs-layer accepts a payload above the 5 MiB text cap (binary policy applies) + HTTP binary policy at HTTP + accepts an upload of exactly MAX_UPLOAD_BYTES: expected 413 to be 201 |
killed |
| M2 | route: no-clobber collision surfaces an error instead of numbering on | 11 numbering-family tests (.env (1) dotfiles, taken candidates, 255-byte trims incl. multi-byte boundary, concurrency race, dir occupancy, suspicious-root, 1000-candidate cap, no-fs.denied audit): 10× expected 409 to be 201, 1× expected 409 to be 400 |
killed |
| M3 | web-shell hook: resultPath: result.path → targetPath |
invokes onUploaded exactly once with the server-confirmed path: expected 'report.pdf' to be 'report (1).pdf' |
killed |
| G4 | admission: remove declared-Content-Length pre-check | rejects an oversized declared Content-Length with the upload 413 envelope (hangs → fails at 1501 ms without the guard) |
killed |
| R1 | round-6: remove NUL-byte admission check | rejects the NUL-bearing path with parse_error and no denied event (×2 shapes): expected 500 to be 400 — reproduces pre-fix F1 |
killed |
| R2 | round-6: remove inflate: false |
rejects an encoded request body instead of silently decoding it: expected 201 to be 415 — without the guard the gzip body is silently decoded and published |
killed |
| R3 | round-6: remove ELOOP-cycle continue |
numbers past a symlink cycle… + …two-hop symlink cycle…: expected 400 to be 201 |
killed |
| R4 | round-6: revert audit suppression (collisions audit again) | does not audit a no-clobber collision as fs.denied (got 1) + emits no fs.denied events for a successful auto-numbered upload (got 3, expected 0) |
killed |
| R5 | round-6: candidates re-resolve from canonicalized dir, not literal queryDir |
uploads through a symlinked directory whose target trips the pattern check (+ the workspace-root-pattern sibling): expected 400 to be 201 |
killed |
Not covered
- Browser-rendered Web Shell UI (drag styling, progress strip cadence, tag insertion): container has no browser; covered by the jsdom suites (689 green) and code reading only. XHR upload progress events are browser-only by design — real progress cadence unmeasured.
- Per-commit attribution: depth-2 shallow checkout.
git rev-list --count HEAD^1..HEAD^2returns1against the metadata's 23 commits — silent truncation at the shallow boundary, so the metadata list was not individually exercised; the aggregateHEAD^1..HEADdiff (43 files, +7241/−163) is what was verified. The round delta is attributed from commit metadata (015f1b09= merge of origin/main; previous head2bf40fafunreachable locally). - Live
fs.deniedobservation: audit suppression is proven at the unit layer via captured bridge-event lists (R4's kill quotes the event counts that would appear); no live SSE sink was subscribed. - Draining/removed qualified workspaces (503
workspace_runtime_unavailable): pinned by the executedworkspace-qualified-rest.test.tssuite (inside the 1303); the live matrix covered unknown-workspace and by-path-selector cells only. - Repo-wide lint: not run; per-workspace builds/typechecks above stand in.
- A/B against previous head
2bf40faf: not locally reachable; delta proven by the aggregate diff + suites instead. - The keep-alive desync observation was characterized, not fixed-and-retested — it is pre-existing and outside this PR's surface.
- PR-declared out-of-scope surfaces: resumable/chunked uploads, folder uploads, overwrite-in-place, ACP-HTTP parity, Windows/Linux manual UI.
Methodology
Linux container (Node v22.23.2, 64 cores), merge-ref checkout at depth 2 (HEAD merge e108d5b1, HEAD^1 base 5af31316 = v0.21.11, HEAD^2 head 015f1b09). Base arm: git worktree at HEAD^1; root node_modules entries symlinked from the already-installed head tree, but all @qwen-code/* workspace links re-created deliberately — qwen-code-core, qwen-code-cli (→ cli), sdk, web-shell point into the base tree (readlink -f node_modules/@qwen-code/qwen-code-core → …/tmp/base-tree/packages/core, quoted as required), while the PR-untouched workspaces (web-templates, webui, acp-bridge, audio-capture, all channels, chrome-bridge, external-context, mobile-mcp) point at the head tree's builds — clean controls because git diff HEAD^1..HEAD is empty for every one of them (only cli, sdk-typescript, web-shell are touched). scripts/generate-git-commit-info.js run for the gitignored generated file, then tsc --build of packages/cli (which built packages/core via project references). Base markers verified: workspace_file_upload absent from base dist capabilities (grep 0, head 1) and no file/upload route in base dist; base daemon boots with an identical /workspace/trust envelope (B2). Harnesses booted real daemons (dist/index.js serve --port 0 --token … --workspace <mkdtemp>) with scratch HOME/QWEN_HOME, security.folderTrust.enabled=true in settings.json, trust granted/withheld via QWEN_CODE_TRUSTED_FOLDERS_PATH; waited past the bootstrap envelope by polling /workspace/trust to 200; drove them with fetch + raw http sockets (agent: false after the keep-alive desync above bit the harness itself) and asserted on bytes-on-disk + response envelopes. Mutants applied by exact single-occurrence string replacement, run through vitest, restored byte-exact with git status --porcelain verified clean after each. Evidence images produced with scripts/verify-capture.mjs (03 is assembled from the saved per-mutant logs because the R5 pattern was corrected after the live capture pass); raw logs and harness scripts live alongside this report. Base worktree removed after the A/B cells were captured.
Evidence images
Harness scripts and raw logs are in the workflow run artifacts (7-day retention).
— Qwen Code · sandboxed verification
yiliang114
approved these changes
Aug 13, 2026
Collaborator
|
Triage re-run completed without a new review.
The stage comments above were updated with the latest result. View workflow run. 上方各阶段评论已更新为最新结果。查看工作流运行。 |
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
Adds direct workspace file uploads to the Web Shell composer. Users can drop ordinary files onto the composer or choose “Upload file” while browsing the
@file panel; multiple files upload sequentially with progress, cancellation, automatic conflict renaming, and inline file tags using the server-confirmed paths. Pure supported-image drops continue to use the existing image attachment flow.Adds capability-gated legacy-primary and workspace-qualified binary upload routes, a 50 MiB binary policy distinct from text-write limits, a shared four-request admission gate, atomic no-clobber publication, and deterministic numbered filenames for occupied targets. The TypeScript SDK exposes the same upload operation with browser progress, abort, and timeout support.
Why it's needed
The Web Shell could reference workspace files but could not bring local files into the workspace. Users had to copy files through another tool before mentioning them, and naïvely extending the text-write route would corrupt binary data or weaken existing text-write limits.
Reviewer Test Plan
How to verify
@file panel, and confirm “Upload file” has an upload icon and opens a multi-file picker without leaving the trigger@in the composer.%, and an occupied filename. Confirm uploads are byte-identical, occupied names becomename (1).ext, progress appears above the input surface, successful rows disappear after three seconds, and each result becomes a removable inline file tag.Automated verification:
npm run build,npm run typecheck, andnpm run lint; 244 relevant CLI tests, 11 SDK upload tests, and 231 Web Shell composer/upload tests passed locally.Evidence (Before & After)
Before: local files had to be copied into the workspace outside the Web Shell before they could be referenced. After: the composer and
@file browser upload local files directly, show bounded progress state, and insert the confirmed path as an inline file tag.Tested on
Environment (optional)
Local Web Shell and daemon development environment on macOS; Node.js 22 workspace tests.
Risk & Scope
Linked Issues
N/A
中文说明
本 PR 的改动
为 Web Shell 输入框增加工作区文件直传能力。用户可以把普通文件拖入输入框，或在
@文件面板浏览目录时选择“上传文件”；多文件按顺序上传，支持进度、取消、重名自动改名，并使用服务端确认的路径生成行内文件标签。仅包含受支持图片的拖放仍使用现有图片附件流程。新增由能力标签控制的主工作区和 qualified workspace 二进制上传路由；二进制上限为 50 MiB，与文本写入上限相互独立；两个路由共享四请求准入门，使用原子且不覆盖已有文件的发布方式，并为被占用的目标生成确定性的编号文件名。TypeScript SDK 同步提供上传接口，支持浏览器进度、取消和超时。
为什么需要
Web Shell 原本只能引用工作区里已经存在的文件，无法把本地文件带入工作区。用户必须先通过其他工具复制文件才能在输入框中引用；直接扩展文本写入路由则可能破坏二进制内容，或削弱现有文本写入限制。
Reviewer 测试计划
验证方法
@文件面板中确认“上传文件”带有上传图标、可打开多文件选择器，并且输入框中不会残留触发面板的@。%和重名情况的文件。确认字节完全一致，重名文件变为name (1).ext，进度显示在输入框上方，成功状态三秒后消失，并且每个结果都成为可移除的行内文件标签。自动化验证：本地通过
npm run build、npm run typecheck和npm run lint；通过 244 个相关 CLI 测试、11 个 SDK 上传测试和 231 个 Web Shell 输入框/上传测试。前后对比证据
改动前：本地文件必须先在 Web Shell 之外复制进工作区，之后才能引用。改动后：输入框和
@文件浏览器可以直接上传本地文件，显示有界的进度状态，并将服务端确认的路径插入为行内文件标签。测试平台
环境（可选）
macOS 本地 Web Shell 与 daemon 开发环境；Node.js 22 工作区测试。
风险与范围
关联 Issue
N/A