feat(cli): paste copied Windows files - #7957
Conversation
Windows File Explorer clipboard demoTested on Windows 11 with Qwen Code v0.21.0 using BeforeCopying either the PNG or TXT file from File Explorer leaves the prompt unchanged. 
After
|
|
✅ Qwen Triage finished — CI landed green on ✅ Qwen Triage 已完成 —— |
|
Re-ran the gate at the current head. The branch has moved a long way since the July pass (14+ commits, four maintainer verification rounds), so this replaces that assessment rather than repeating it. Template looks good ✓ — every required section is filled in, including the Tested-on matrix, the risk/scope bullets, and the bilingual summary. Problem: observed, not theoretical. #2605 is open, and the thread carries two independent reproductions: the author's Explorer-copy demo, and @wenshao's round-4 A/B, which drove the real built bundle in a real terminal against a live clipboard with the six production files reverted to the merge base as the negative control. Copied files yield nothing on the base arm and an attachment chip or a working reference on the PR arm; the control flips 24 tests red, all inside the feature area, so the tests are not vacuous. That is the strongest form of evidence this gate asks for. Direction: aligned. This is Windows parity for a clipboard path that already ships on macOS/Linux — it extends the existing image-paste flow instead of adding a parallel one. CHANGELOG signal found: the reference agent invests steadily in exactly this area ("Pasted and clipboard images are read without blocking the event loop", "native image paste, clipboard … add-ons now load") and in Windows path handling generally (the Size: cross-package ( Approach: scope feels right, and it matches what I'd have written. Before reading the diff my own plan was a win32-gated Risk: no elevated risk signals — no changed file matches the revert-correlated path set. Moving on to code review. 🔍 中文说明按当前 head 重新跑了一遍准入检查。分支自 7 月那次以来变化很大（14+ 次提交、维护者 4 轮验证），所以这次是替换当时的判断，而不是重复它。 模板 完整 ✓ —— 各必填部分都写了，包括测试平台矩阵、风险与范围条目和中文说明。 问题： 是已观测到的问题，不是理论性加固。#2605 仍处于 open 状态，讨论串里有两份独立复现：作者的「资源管理器复制」演示，以及 @wenshao 第 4 轮 A/B —— 用真实构建产物、真实终端、真实剪贴板驱动，并把 6 个生产文件回退到合并基线作为负对照。基线侧复制文件没有任何反应，PR 侧得到附件或可用引用；负对照让 24 个测试变红且全部落在本功能范围内，说明这些测试不是空转。这是本准入流程要求的最高等级证据。 方向： 对齐。这是给 macOS/Linux 上已有的剪贴板路径补齐 Windows 能力 —— 扩展现有图片粘贴流程，而不是另起一套。CHANGELOG 信号：参考产品在这一块持续投入（「粘贴与剪贴板图片读取不再阻塞事件循环」「原生图片粘贴、剪贴板等插件可加载」），在 Windows 路径处理上也持续修（ 规模： 跨包（ 方案： 范围合理，也和我自己的写法一致。看 diff 之前我的方案是：在现有 风险： 无升级风险信号 —— 变更文件均未命中与回滚相关的路径集合。 进入代码审查 🔍 — Qwen Code · qwen3.8-max-2026-09-02 Reviewed at |
Code reviewThree Cross-platform test failure — fixed. The Inert non-image paths — fixed. Non-image selections are no longer inserted raw. The
So the change is confined to strings beginning What I would genuinely act on is smaller and cheaper than the thread's rewrite:
Per AGENTS.md this PR is past five review rounds, so I'm separating those from blockers deliberately: none of the four above is a correctness, security, data-loss or regression defect at this head, and the remaining Suggestion-level threads are of the same character. One Suggestion I'd flag for explicit sign-off rather than let lapse: clipboard-supplied paths reach filesystem resolution unvalidated, so a UNC selection triggers an outbound SMB connection and NTLMv2 authentication to a host named on the clipboard. It is narrow (the user copied it themselves, and any typed UNC reference does the same today), but the reference agent shipped an explicit hardening for the adjacent vector — rejecting Windows NT-namespace paths "against the NTLM credential-leak vector" — so this is a known-bad class rather than a hypothetical one. Worth a decision on the record either way. sequenceDiagram
participant P1 as User (Explorer copy)
participant P2 as KeypressContext
participant P3 as clipboardUtils
participant P4 as Native clipboard module
participant P5 as InputPrompt
participant P6 as atCommandProcessor
P1->>P2: Alt+V, or a terminal empty bracketed paste
P2->>P3: readClipboardFiles(onUnavailable)
P3->>P4: hasFormat files, then getFiles
P4-->>P3: absolute paths, or nothing
P3-->>P2: paths (win32 only, empty elsewhere)
alt file list is non-empty
P2->>P5: broadcast paste carrying clipboardFiles as references
P5->>P5: classifyPastedImagePaths
alt every path is an image
P5->>P5: promotePastedImagePaths to attachment chips
P5->>P5: failed paths fall back to inserted references
else any path is not an image
P5->>P5: large-paste placeholder guard, else insert references
end
else file list is empty
P2->>P3: clipboardHasImage (the unchanged image path)
P3-->>P5: pasteImage broadcast
end
P5->>P6: on submit, references resolve through unescapePath
Files changed (12)
Testing evidenceThis is an unattended run, so I did not build or execute anything from this branch — the evidence below is the PR's own CI, read through the API at the reviewed commit, plus measurements already in the thread that I attribute to their author. Nothing is red at this head. Seven substantive lanes were still in flight when I fetched, including the unit suite and lint, so the honest statement is that CI has not settled yet — not that it passed. I did not poll for it; the finalize pass rewrites the table below once the runs complete. Two lanes deserve a mention beyond the table. Final CI results for
One row per check name (latest run); skipped checks omitted; failures sort first. / 每个检查名一行（取最新一次运行），省略 skipped，失败项排在最前。 Everything else I report is not mine and I want it labelled that way: the 523-passed CLI suite, the 24-test negative control, the mutation probes, the simulated-win32 core run and the live TUI captures are @wenshao's measurements from round 4 at Not verified by me: live Windows behaviour (no Windows lane runs, and this run executes nothing from the branch); the current head's unit/lint/integration results (still in flight); and whether the mixed-separator reference shape resolves on a real Windows filesystem (the thread's own disclosed assumption). Sandboxed verification would settle the gap that actually matters here: 中文说明代码审查这个 PR 上有三条来自早前 跨平台测试失败 —— 已修复。 非图片路径成为无效文本 —— 已修复。 非图片选择不再以原始路径插入。
所以这处改动被限制在以 我真正建议动手的，比线程里那套改写要小得多、也便宜得多：
按 AGENTS.md，这个 PR 已超过 5 轮评审，所以我是刻意把上面这些与阻断项分开的：在当前 head 上，这四条都不是正确性、安全、数据丢失或回归缺陷，其余 Suggestion 级线程性质相同。 有一条 Suggestion 我建议明确签字而不是任其失效：剪贴板提供的路径未经校验就进入文件系统解析，因此一个 UNC 选择会触发对外 SMB 连接，并向剪贴板上指定的主机做 NTLMv2 认证。它很窄（是用户自己复制的，而且今天手输的 UNC 引用也一样），但参考产品对相邻向量做过明确加固 —— 拒绝 Windows NT 命名空间路径以「防范 NTLM 凭据泄露向量」—— 所以这属于已知的坏类别，而非假想。无论结论如何，值得在记录上做一次决定。 测试证据这是无人值守运行，因此我没有构建或执行本分支的任何代码 —— 上面的证据是我通过 API 在被审提交上读到的本 PR 自己的 CI，外加线程中已有的、我明确标注了归属的实测结果。 当前 head 上没有红灯。我抓取时仍有 7 条实质 lane 在跑，包括单元测试与 lint，所以诚实的说法是 CI 尚未定论，而不是它已通过。我没有轮询等待；CI 跑完后 finalize 流程会就地改写上面的表格。 有两条 lane 值得单独一提。 其余一切都不是我做的，我要明确标注：523 项通过的 CLI 套件、24 项负对照、变异探针、模拟 win32 的 core 运行以及实时 TUI 截图，都是 @wenshao 第 4 轮在 未由我验证： Windows 真实行为（没有 Windows lane 在跑，且本次运行不执行分支代码）；当前 head 的单元/lint/集成结果（仍在进行中）；以及混合分隔符引用形态在真实 Windows 文件系统上能否解析（该线程自己披露的假设）。 沙箱验证能真正补上这里的缺口： — Qwen Code · qwen3.8-max-2026-09-02 Reviewed at |
|
Confidence: 4/5 — solid, and the Windows claim now rests on measured behaviour rather than a demo; what's left is non-blocking, but I'm naming it below rather than waving it through. Stepping back against my own Stage 2a proposal: this landed close to what I'd have written, including the de-duplication of the two empty-paste broadcast blocks that I'd have done for the same reason. The one place I'd have gone differently is the core Does it solve something users care about? Yes, and the burden of proof here is about as discharged as it gets: an open welcome-pr issue, the author's before/after, and then a maintainer who drove the real bundle in a real terminal against a live clipboard, with the production files reverted to the merge base as a negative control that flips 24 tests red inside the feature area. I did not re-run any of that — this run executes nothing from the branch — but I also don't need to take it on faith, because the two Criticals that would have undermined it are fixed at this head and I read the fixes in the code myself. Would I curse this in six months? Mostly no. The code is straightforward and reuses what was already there. Two things would annoy me: the win32 branch of the core change being executed by no CI lane, so a future regression there is silent; and the The one thing I'd want a decision on rather than a silent lapse is the UNC/NTLM note in Stage 2. It's narrow and arguably pre-existing, but the reference agent hardened the adjacent vector explicitly, so it deserves an answer on the record instead of drifting. Verdict: approve, deferred. CI has not settled at this head — the unit suite and lint were still in flight when I read them, and I did not poll. Approving now would attest to a result that doesn't exist yet, so this run posts no approval; the deferred approval lands once every check on the reviewed commit completes green, and is withheld if anything lands red or the head moves. Worth knowing for the merge state: 中文说明Confidence: 4/5 —— 扎实，而且 Windows 这一论断现在建立在实测行为上，而不是一段演示；剩下的都是非阻断项，但我选择在下面点名，而不是直接放行。 回到我在 Stage 2a 自己写的方案：落地实现与我会写的很接近，包括把两处重复的空粘贴广播块合并 —— 我也会出于同样的理由这么做。我唯一会走不同路的地方是 core 的 它解决的是用户在意的问题吗？是，而且这里的举证责任基本已经尽到位：一个 open 的 welcome-pr issue、作者的前后对比，然后是一位维护者用真实构建产物、真实终端、真实剪贴板驱动，并把生产文件回退到合并基线作为负对照 —— 该对照让功能范围内 24 个测试变红。这些我都没有重跑 —— 本次运行不执行分支里的任何代码 —— 但我也无需凭信任接受，因为会动摇它的两条 Critical 在当前 head 上已修复，而且修复是我自己在代码里读过的。 半年后我会骂它吗？大体不会。代码直白，复用了已有的东西。有两点会让我烦：core 改动的 win32 分支没有任何 CI lane 执行，所以那里将来回归是无声的；以及 我唯一希望得到一个决定、而不是任其无声失效的，是 Stage 2 里那条 UNC/NTLM 说明。它很窄，也可以说是既有问题，但参考产品对相邻向量做过明确加固，所以它值得在记录上有一个答复，而不是漂走。 结论：批准，延后执行。 CI 在当前 head 上尚未定论 —— 我读取时单元测试与 lint 仍在进行，而我没有轮询。现在批准等于为一个尚不存在的结果背书，所以本次运行不提交批准；延后的批准会在被审提交上的所有检查全绿后落地，若有红灯或 head 移动则不予执行。 关于合并状态，有一点值得知道： — Qwen Code · qwen3.8-max-2026-09-02 Reviewed at |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed — no blockers. Suggestions are inline.
— qwen3.8-max-preview via Qwen Code /review
| const clipboardFiles = await readClipboardFiles(onUnavailable); | ||
| const hasImage = | ||
| clipboardFiles.length === 0 && !clipboardImageUnavailable | ||
| ? await clipboardHasImage(onUnavailable) | ||
| : false; |
There was a problem hiding this comment.
[Suggestion] This clipboard-read → conditional-image-check → broadcast block is duplicated verbatim in broadcastPasteFromRaw (~line 1399); the only difference is await vs .then(). — Concrete cost: the two paste paths are selected by terminal behavior (kitty-protocol keypress paste-end vs raw bracketed-paste flush), indistinguishable to the user. Any future change to the broadcast shape or the file-vs-image priority must be made in two places; fixing one and missing the other yields divergent paste behavior that no test currently pins. Consider extracting a shared helper, e.g.:
async function broadcastClipboardPaste(broadcast: BroadcastFn): Promise<void> {
let clipboardImageUnavailable = false;
const onUnavailable = () => {
clipboardImageUnavailable = true;
};
const clipboardFiles = await readClipboardFiles(onUnavailable);
const hasImage =
clipboardFiles.length === 0 && !clipboardImageUnavailable
? await clipboardHasImage(onUnavailable)
: false;
broadcast({
name: '', ctrl: false, meta: false, shift: false,
paste: true, pasteImage: hasImage, clipboardImageUnavailable,
sequence: clipboardFiles.join('\n'),
});
}then await broadcastClipboardPaste(broadcast) here and void broadcastClipboardPaste(broadcast) in the raw path.
— qwen3.8-max-preview via Qwen Code /review
| if (process.platform !== 'win32') { | ||
| return []; | ||
| } |
There was a problem hiding this comment.
[Suggestion] readClipboardFiles has a single happy-path test; the non-win32 guard, the module-unavailable onUnavailable callback, the hasFormat('files') === false branch, and the getFiles()-throws catch are all untested — clipboardHasImage has equivalent coverage for each. — Concrete cost: this guard is the only thing stopping macOS/Linux from attempting a native module load (and firing onUnavailable, surfacing a spurious "clipboard unavailable" warning) on every paste, since the function is called unconditionally in both KeypressContext paste paths and handleClipboardPaste. A refactor that drops or reorders it would regress silently. Consider mirroring the clipboardHasImage cases: non-win32 → [] without loading the module; win32 + failLoad → [] and onUnavailable called; win32 + empty clipboard → []; win32 + getFiles() throws → [].
— qwen3.8-max-preview via Qwen Code /review
| const pastedImagePaths = classifyPastedImagePaths(pasted); | ||
| if (pastedImagePaths.allImages) { | ||
| await promotePastedImagePaths(pastedImagePaths.imagePaths, pasted); |
There was a problem hiding this comment.
[Suggestion] The all-images-from-files branch — the headline behavior where an image-only Explorer selection becomes attachments via promotePastedImagePaths — has no component-level test. The new test covers the non-image path (buffer.insert) and the empty-files fallthrough, and classifyPastedImagePaths is unit-tested for Windows paths in isolation, but nothing drives the paste shortcut with readClipboardFiles returning image-only paths and asserts attachments are created. — Concrete cost: if this condition were inverted or promotePastedImagePaths received wrong arguments, copying only .png files and pressing Alt+V would insert raw path text instead of attachments, with no test to catch it. Consider adding a case that mocks readClipboardFiles to return ['C:\\Users\\mochi\\image.png'], sends the shortcut, and asserts the promote path fires (attachments updated / fs.copyFile invoked) rather than buffer.insert.
— qwen3.8-max-preview via Qwen Code /review
| if (pastedImagePaths.allImages) { | ||
| await promotePastedImagePaths(pastedImagePaths.imagePaths, pasted); | ||
| } else { | ||
| buffer.insert(pasted, { paste: false }); |
There was a problem hiding this comment.
[Suggestion] This inserts the full multi-file path blob directly, bypassing the LARGE_PASTE_CHAR_THRESHOLD / LARGE_PASTE_LINE_THRESHOLD placeholder guard that the key.paste broadcast path applies (~line 1067). — Concrete cost: on Windows, selecting many files in Explorer (15+, or long C:\Users\...\AppData\... paths — easily >10 lines / >1000 chars) and pressing Alt+V floods the composer with the raw blob, while the same content delivered through the terminal's native paste is collapsed into a nextLargePastePlaceholder. The two paste entry points behave inconsistently for the same clipboard content. Consider routing this insert through the same large-paste threshold check, or having handleClipboardPaste reuse the key.paste handler so both entry points share one code path.
— qwen3.8-max-preview via Qwen Code /review
| expect(mockFsStat).toHaveBeenCalledWith(imagePath); | ||
| expect(mockFsCopyFile).toHaveBeenCalledWith( | ||
| imagePath, |
There was a problem hiding this comment.
[Critical] This new test (promotes copied image files to attachments on the clipboard shortcut) fails on every non-Windows platform, breaking the suite on Linux/macOS CI. It asserts fs.stat/fs.copyFile receive the bare Windows path C:\Users\mochi\image.png, but promotePastedImagePaths (InputPrompt.tsx:813-816) computes path.isAbsolute(imagePath) ? imagePath : path.resolve(cwd, imagePath). Under POSIX node:path, path.isAbsolute('C:\Users\mochi\image.png') is false, so the path is resolved against cwd and fs.stat actually receives <cwd>/C:\Users\mochi\image.png — not the verbatim path asserted here. The test is a plain it(...) (not gated to Windows), so it runs on ubuntu CI. — Failure scenario: vitest run for packages/cli exits 1 on every Linux run (confirmed locally: 1 failed | 343 passed). Production behavior on real Windows is correct (path.isAbsolute recognizes C:\ there); this is a cross-platform test defect, not a Windows runtime bug.
Fix: make the expectation platform-aware (compute the source the same way the code does), or gate the test to Windows via the file's existing isWindows/describeConditional pattern:
const expectedSource = path.isAbsolute(imagePath)
? imagePath
: path.resolve(cwd, imagePath);
expect(mockFsStat).toHaveBeenCalledWith(expectedSource);
expect(mockFsCopyFile).toHaveBeenCalledWith(expectedSource, expect.any(String));中文说明
这个新测试（promotes copied image files to attachments on the clipboard shortcut）在所有非 Windows 平台上都会失败，导致 Linux/macOS CI 上的测试套件变红。它断言 fs.stat/fs.copyFile 收到裸 Windows 路径 C:\Users\mochi\image.png，但 promotePastedImagePaths（InputPrompt.tsx:813-816）会计算 path.isAbsolute(imagePath) ? imagePath : path.resolve(cwd, imagePath)。在 POSIX 的 node:path 下，path.isAbsolute('C:\Users\mochi\image.png') 返回 false，路径会基于 cwd 解析，fs.stat 实际收到 <cwd>/C:\Users\mochi\image.png，而非此处断言的原始路径。该测试是普通 it(...)（未限定 Windows），因此会在 ubuntu CI 上运行。— 失败场景：packages/cli 的 vitest run 在每次 Linux 运行都以退出码 1 结束（本地已确认：1 失败 | 343 通过）。真实 Windows 上的生产行为是正确的（path.isAbsolute 在那里能识别 C:\）；这是跨平台测试缺陷，而非 Windows 运行时 bug。修复方式：让断言感知平台（按代码相同的方式计算源路径），或按本文件已有的 isWindows/describeConditional 模式将测试限定为 Windows。
— qwen3.8-max-preview via Qwen Code /review
| await waitFor(() => { | ||
| expect(keyHandler).toHaveBeenCalledWith( | ||
| expect.objectContaining({ | ||
| paste: true, | ||
| sequence: | ||
| 'C:\\Users\\mochi\\image.png\nC:\\Users\\mochi\\notes.txt', | ||
| }), | ||
| ); | ||
| }); |
There was a problem hiding this comment.
[Suggestion] This assertion doesn't pin pasteImage: false, so a regression in broadcastClipboardPaste that broadcasts pasteImage: true when files are present would survive the test. — Concrete cost: mutate the ternary in broadcastClipboardPaste (KeypressContext.tsx:767-770) from : false to : true and the test still passes (objectContaining ignores unlisted keys, and clipboardHasImage is still not called). Downstream, InputPrompt.tsx:1079 checks if (key.pasteImage) first and would call handleClipboardImage(true) — attempting to save a non-existent clipboard image — instead of inserting the copied file paths, so a user pasting Explorer files gets an image-save attempt rather than the expected path text.
| await waitFor(() => { | |
| expect(keyHandler).toHaveBeenCalledWith( | |
| expect.objectContaining({ | |
| paste: true, | |
| sequence: | |
| 'C:\\Users\\mochi\\image.png\nC:\\Users\\mochi\\notes.txt', | |
| }), | |
| ); | |
| }); | |
| await waitFor(() => { | |
| expect(keyHandler).toHaveBeenCalledWith( | |
| expect.objectContaining({ | |
| paste: true, | |
| pasteImage: false, | |
| sequence: | |
| 'C:\\Users\\mochi\\image.png\nC:\\Users\\mochi\\notes.txt', | |
| }), | |
| ); | |
| }); |
中文说明
此断言没有固定 pasteImage: false，因此 broadcastClipboardPaste 中一个在有文件时广播 pasteImage: true 的回归将能逃过该测试。— 具体代价：把 broadcastClipboardPaste（KeypressContext.tsx:767-770）中的三元表达式从 : false 改成 : true，测试仍然通过（objectContaining 忽略未列出的键，且 clipboardHasImage 仍不会被调用）。在下游，InputPrompt.tsx:1079 会先检查 if (key.pasteImage)，从而调用 handleClipboardImage(true)——尝试保存一个并不存在的剪贴板图片——而不是插入复制的文件路径，于是粘贴资源管理器文件的用户会得到一次图片保存尝试，而非预期的路径文本。建议在 objectContaining 断言中加入 pasteImage: false。
— qwen3.8-max-preview via Qwen Code /review
Review —
|
| Check | Result |
|---|---|
clipboardUtils.test.ts |
33/33 pass |
KeypressContext.test.tsx |
110/110 pass |
InputPrompt.test.tsx |
201/201 pass, 0 failed — the earlier [Critical] POSIX failure on the bare C:\... assertion is fixed |
prettier --check on all 7 changed files |
clean |
eslint on the 6 changed TS/TSX files |
clean |
I also probed the pinned native module directly (@teddyzhu/clipboard@0.0.5, darwin binary — so this validates the JS-visible contract, not the Windows implementation):
hasFormat('files') -> false (accepted format name)
hasFormat('totally-bogus') -> THROWS "Unsupported format: totally-bogus-format"
getFiles() with empty clipboard -> THROWS "Failed to get files: no files"
Two useful conclusions: 'files' is a real format name (a typo would have thrown, not silently returned false), and getFiles() throws rather than returning [] — so the hasFormat guard plus the try/catch in readClipboardFiles are both load-bearing, including for the TOCTOU window where the clipboard changes between the two calls. That part is correct as written.
1. Alt+V inserts a bare path where Ctrl+V inserts an @-reference (should fix)
handleClipboardPaste inserts with { paste: false }, but the terminal Ctrl+V route ends in buffer.handleInput(key) → insert(seq, { paste: true }), and paste: true is what triggers tryExtractFilePaths → @-reference conversion. Measured buffer text for the same two-file Explorer selection (real useTextBuffer, isValidPath: () => true, which is what real Windows gives you since the copied files exist):
paste: true (Ctrl+V route) "@C:\Users\mochi\notes.txt @C:\Users\mochi\todo.txt "
paste: false (Alt+V, this PR) "C:\Users\mochi\notes.txt\nC:\Users\mochi\todo.txt"
single file, paste: true "@C:\Users\mochi\notes.txt "
single file, paste: false "C:\Users\mochi\notes.txt"
Why this matters beyond cosmetics:
- The PR's own test plan says
Ctrl+VandAlt+Vshould "expect the same results". For non-image files they don't — one produces a working file reference, the other inert text. - add image paste from clipboard on windows command prompt #2605 asks for this specifically: "to easy paste a file copied instead to add the path of it". The
Alt+Vpath currently delivers the path, which is the thing the issue asks to avoid. TheCtrl+Vpath already delivers the reference. - It's the same consistency argument that motivated routing this branch through
insertLargePastePlaceholder— worth finishing the job on the sibling line.
Fix is one token:
} else if (!insertLargePastePlaceholder(pasted)) {
buffer.insert(pasted, { paste: true });
}This is strictly additive: tryExtractFilePaths returns null when any token isn't an existing file (folders included, since isValidPath requires isFile()), and the raw insert then behaves exactly as today. It's also a no-op in shell mode, which insert already guards.
I applied that change as a mutant to see what it breaks — exactly one test, the new inserts copied non-image file paths on the clipboard shortcut, which pins { paste: false } (1 failed | 11 passed). So the change is: flip the flag, update that one expectation, and ideally assert the @-prefixed result with a real buffer rather than the mockBuffer spy so the user-visible outcome is what's pinned.
2. The new !clipboardImageUnavailable short-circuit is untested (mutant survives)
In broadcastClipboardPaste, this guard is what stops a second native-module load attempt after readClipboardFiles already reported the module missing:
const hasImage =
clipboardFiles.length === 0 && !clipboardImageUnavailable
? await clipboardHasImage(onUnavailable)
: false;I deleted && !clipboardImageUnavailable (verified the edit landed) and ran the suite: 110/110 still pass. Worth one case — win32 + failLoad — asserting clipboardHasImage is not called and the broadcast carries clipboardImageUnavailable: true. Cheap, and it's the only new logic in that file with no coverage.
3. The file-wide vi.mock('node:fs/promises') is a broad default (test hygiene)
vi.mock('node:fs/promises', async (importOriginal) => ({ ...(await importOriginal()), stat: mockFsStat, ... }));
// beforeEach:
mockFsStat.mockRejectedValue(new Error('file not found'));This applies to the whole 5.9k-line suite's module graph, not just promotePastedImagePaths, and makes "stat always fails" the default for every one of the 201 tests. Nothing depends on it today (suite is green), so this is a future-vacuity risk rather than a live bug: a later test that legitimately stats a file silently takes the error branch and can pass for the wrong reason. Consider defaulting the three mocks to the real implementations and overriding only inside the two tests that need them — the blast radius drops to zero at no cost to the new assertions.
4. Mock implementations wired inside the vi.mock factory (clipboardUtils.test.ts)
mockClipboardHasFormat.mockImplementation(...) / mockClipboardGetFiles.mockImplementation(...) are set as a side effect of the module factory, which only re-runs on the next lazy import('@teddyzhu/clipboard') after vi.resetModules(). It works today because getClipboardModule() imports lazily inside each test, after vi.clearAllMocks(). But any test that calls clearAllMocks (or reaches the clipboard twice across a reset boundary) gets hasFormat returning undefined → falsy → readClipboardFiles returns [] → a green test that proved nothing. Moving both mockImplementation calls into beforeEach after clearAllMocks removes the ordering dependency.
Also minor: the module-unavailable case now asserts both clipboardHasImage and readClipboardFiles inside one it() (and mutates process.platform mid-test), so a failure doesn't say which surface regressed. Worth splitting.
5. Minor notes
- Large file lists lose their identity. 11 files trips
LARGE_PASTE_LINE_THRESHOLDand the composer shows[Pasted Content 287 chars]. Correct for consistency (and the paths still expand on submit), but for a file selection specifically, something like[Pasted 11 files]would be much more legible. Fine as a follow-up. - Folders. Explorer copies directories too, and
getFiles()returns them. They're not images, so they land as text — reasonable, and with{ paste: true }above they'd stay as text rather than becoming a broken@-ref. A test case would document the intent. - Failure is silent. A
getFiles()throw logs viadebugLogger.error(invisible unless debug logging is on) and falls through to the image check, so a real native failure looks like "the shortcut does nothing" — same as pre-PR, so not a regression, just noting the diagnosability ceiling. - Doc wording. "files copied in Explorer paste as image attachments or their paths" reads ambiguously; something like "image files become attachments, other files insert their paths" is clearer. If pre-release: fix ci #1 lands, this line needs updating anyway.
allImagesguard.handleClipboardPastecheckspastedImagePaths.allImageswhile thekey.pastehandler checksallImages && imagePaths.length > 0. The extra guard is redundant (allImagesis seeded fromtokens.length > 0), but the asymmetry invites a reader to wonder which one is right.
Landscape
Two open PRs touch the same files and are worth sequencing against:
- fix(cli): enable clipboard text and image paste on WSL2/Linux #6829
fix(cli): enable clipboard text and image paste on WSL2/Linux— +152/-3 inclipboardUtils.ts, +20/-4 inInputPrompt.tsx. Direct overlap; also relevant to theprocess.platform !== 'win32'gate here, since the native module exposesgetFiles()on macOS/Linux too and a Finder/Nautilus copy is the natural next step. - feat(cli): paste base64 / data URL images, drag image files, with [Image #N] placeholders #3519
feat(cli): paste base64 / data URL images, drag image files…— +197/-30 inInputPrompt.tsx, rewrites the same paste-classification region.
Verdict
Approve after #1. The architecture is right, the earlier round's findings are genuinely fixed, and the win32 gate keeps macOS/Linux regression risk at essentially zero. #1 is a one-token change that makes the two entry points actually agree and delivers what #2605 asked for; #2 is a five-line test that stops a real mutant. CI note: Test (ubuntu-latest) hadn't reported at review time, and the review-pr job failure is bot-workflow infra, not this diff.
中文说明
审阅提交 0173db4。改动干净利落：一个新工具函数 readClipboardFiles、每个调用点各抽一个 helper，之前重复的空粘贴广播块也合并成了单一的 broadcastClipboardPaste。上一轮评审的六条意见看起来都已处理。
本地验证（macOS，PR head 的独立 worktree）：clipboardUtils.test.ts 33/33 通过；KeypressContext.test.tsx 110/110 通过；InputPrompt.test.tsx 201/201 通过、0 失败——之前那条关于裸 C:\... 断言在 POSIX 上失败的 [Critical] 问题确已修复；7 个改动文件 prettier --check 干净，6 个 TS/TSX 文件 eslint 干净。我还直接探测了锁定版本的原生模块（@teddyzhu/clipboard@0.0.5 的 darwin 二进制，因此只验证 JS 层契约、不验证 Windows 实现）：hasFormat('files') 返回 false 但不抛错，而 hasFormat('totally-bogus') 会抛 Unsupported format——说明 'files' 是真实的格式名（写错会抛错而非静默返回 false）；空剪贴板下 getFiles() 会抛错而非返回 []——因此 hasFormat 守卫和 try/catch 都是必要的，也覆盖了两次调用之间剪贴板被改动的 TOCTOU 窗口。这部分实现是正确的。
1（建议修复）：Alt+V 插入裸路径，而 Ctrl+V 插入 @ 引用。 handleClipboardPaste 用 { paste: false } 插入，而终端 Ctrl+V 路径最终走 buffer.handleInput(key) → insert(seq, { paste: true })，只有 paste: true 才会触发 tryExtractFilePaths 的 @ 引用转换。对同一组两文件选择的实测缓冲区文本（真实 useTextBuffer，isValidPath: () => true，与真实 Windows 上文件存在的情况一致）：paste: true 得到 "@C:\Users\mochi\notes.txt @C:\Users\mochi\todo.txt "，paste: false 得到 "C:\Users\mochi\notes.txt\nC:\Users\mochi\todo.txt"；单文件分别是 "@C:\Users\mochi\notes.txt " 与 "C:\Users\mochi\notes.txt"。这不只是外观问题：PR 自己的测试计划声称 Ctrl+V 与 Alt+V「预期结果相同」，但非图片文件并不相同——一个产生可用的文件引用，另一个只是无效文本；#2605 恰恰要求「不要只是加上路径」，而 Alt+V 目前给的正是路径，Ctrl+V 反而已经给出了引用；这也正是把该分支接入 insertLargePastePlaceholder 时采用的一致性理由，相邻这一行值得一并完成。修复只需一个 token：buffer.insert(pasted, { paste: true })。该改动是纯增量的——当任一 token 不是已存在的文件时（包括文件夹，因为 isValidPath 要求 isFile()），tryExtractFilePaths 返回 null，随后的原始插入与现状完全一致；在 shell 模式下 insert 本身已有守卫，同样无影响。我把这个改动当作 mutant 应用后跑了一遍，只挂掉一个测试：新增的 inserts copied non-image file paths on the clipboard shortcut（它固定了 { paste: false }），结果为 1 failed | 11 passed。因此改法是：翻转该 flag、更新这一条期望，并最好用真实 buffer 而非 mockBuffer spy 来断言带 @ 前缀的结果，这样固定住的是用户可见的行为。
2（测试缺口）：新增的 !clipboardImageUnavailable 短路没有测试。 这个守卫的作用是在 readClipboardFiles 已经报告原生模块缺失后，避免第二次加载尝试。我删掉 && !clipboardImageUnavailable（已确认改动生效）后重跑，110/110 仍全部通过。建议补一条 win32 + failLoad 用例，断言 clipboardHasImage 未被调用、且广播中带有 clipboardImageUnavailable: true。成本很低，而这是该文件里唯一没有覆盖的新逻辑。
3（测试卫生）：文件级 vi.mock('node:fs/promises') 的默认值过宽。 该 mock 作用于整个 5900 行套件的模块图，而非只作用于 promotePastedImagePaths，并让「stat 总是失败」成为全部 201 个测试的默认行为。目前没有测试依赖它（套件是绿的），所以这是未来的「空测试」风险而非现存缺陷：日后某个确实需要 stat 成功的测试会静默走错误分支，并因错误原因通过。建议三个 mock 默认使用真实实现，仅在需要的两条测试内覆盖，即可把影响面降为零且不损失新断言。
4（clipboardUtils.test.ts）：mock 实现写在 vi.mock 工厂内部。 mockClipboardHasFormat / mockClipboardGetFiles 的 mockImplementation 作为模块工厂的副作用设置，而工厂只在 vi.resetModules() 之后下一次惰性 import('@teddyzhu/clipboard') 时重新执行。目前能工作，是因为 getClipboardModule() 在每个测试内、vi.clearAllMocks() 之后才惰性导入。但任何调用 clearAllMocks（或跨 reset 边界两次访问剪贴板）的测试都会让 hasFormat 返回 undefined → falsy → readClipboardFiles 返回 [] → 得到一个什么都没证明的绿色测试。把两处 mockImplementation 移到 clearAllMocks 之后的 beforeEach 即可消除这个顺序依赖。另外一个小点：模块不可用的用例现在在同一个 it() 里同时断言 clipboardHasImage 和 readClipboardFiles（并在测试中途改写 process.platform），失败时无法判断是哪个面回归，建议拆开。
5（次要）：大文件列表会丢失身份信息——11 个文件即触发 LARGE_PASTE_LINE_THRESHOLD，输入框只显示 [Pasted Content 287 chars]；为一致性这是对的（提交时路径仍会展开），但对文件选择而言 [Pasted 11 files] 之类会易读得多，可作为后续改进。文件夹：资源管理器也能复制目录，getFiles() 会返回它们；它们不是图片，因此作为文本落入输入框——这合理，且采用上面的 { paste: true } 后它们会保持为文本而不会变成无效的 @ 引用，建议补一条测试记录该意图。失败是静默的：getFiles() 抛错时只经 debugLogger.error 记录（未开调试日志时不可见）并回落到图片检查，因此真实的原生失败表现为「快捷键没反应」——与改动前一致，不算回归，只是说明可诊断性的上限。文档措辞：「files copied in Explorer paste as image attachments or their paths」有歧义，改成「图片文件成为附件，其他文件插入路径」更清楚；若第 1 点落地，这行本来也需要更新。allImages 守卫：handleClipboardPaste 只检查 pastedImagePaths.allImages，而 key.paste 处理器检查 allImages && imagePaths.length > 0；多出的守卫其实是冗余的（allImages 由 tokens.length > 0 初始化），但这种不对称会让读者怀疑哪一处才是对的。
关联 PR：#6829 fix(cli): enable clipboard text and image paste on WSL2/Linux（clipboardUtils.ts +152/-3、InputPrompt.tsx +20/-4）与本 PR 直接重叠，也关系到这里的 process.platform !== 'win32' 门禁——原生模块在 macOS/Linux 上同样提供 getFiles()，Finder/Nautilus 的复制是自然的下一步；#3519 feat(cli): paste base64 / data URL images, drag image files…（InputPrompt.tsx +197/-30）重写了同一段粘贴分类逻辑。
结论：处理第 1 点后可以合入。架构方向正确，上一轮的意见确实修好了，win32 门禁也把 macOS/Linux 的回归风险压到基本为零。第 1 点是一个 token 的改动，能让两个入口真正一致并交付 #2605 所要求的行为；第 2 点是五行测试，能挡住一个真实存活的 mutant。CI 说明：评审时 Test (ubuntu-latest) 尚未出结果，review-pr 任务的失败属于 bot 工作流基础设施问题，与本 diff 无关。
doudouOUC
left a comment
doudouOUC
left a comment
There was a problem hiding this comment.
中文说明
— qwen3.7-max via Qwen Code /review
| } else if (!insertLargePastePlaceholder(pasted)) { | ||
| buffer.insert(pasted, { paste: false }); | ||
| } |
There was a problem hiding this comment.
[Critical] Non-image copied files are inserted with { paste: false }, so text-buffer.ts's insert() never runs tryExtractFilePaths (it only runs when paste === true) and the paths land as inert text. The terminal Ctrl+V route inserts with { paste: true }, so the same Explorer selection becomes working @-references there. — Failure scenario: on Windows, copy notes.txt in Explorer and press Alt+V → the prompt gets the bare string C:\Users\mochi\notes.txt (inert text the model cannot read), while Ctrl+V yields @C:\Users\mochi\notes.txt. The two paste routes diverge for non-image files, contrary to the PR's own test plan ("expect the same results"), and a maintainer review gated approval on this. Fix: insert with paste: true so file paths are converted to references, and update the inserts copied non-image file paths... test, which currently pins { paste: false }.
| } else if (!insertLargePastePlaceholder(pasted)) { | |
| buffer.insert(pasted, { paste: false }); | |
| } | |
| } else if (!insertLargePastePlaceholder(pasted)) { | |
| buffer.insert(pasted, { paste: true }); | |
| } |
中文说明
非图片的复制文件以 { paste: false } 插入，因此 text-buffer.ts 的 insert() 不会执行 tryExtractFilePaths（该方法仅在 paste === true 时运行），路径会作为无效文本落入输入框。而终端 Ctrl+V 路径以 { paste: true } 插入，同样的资源管理器选择在那里会变成可用的 @ 引用。— 失败场景：在 Windows 上复制 notes.txt 并按 Alt+V，输入框得到裸字符串 C:\Users\mochi\notes.txt（模型无法读取的无效文本），而 Ctrl+V 得到 @C:\Users\mochi\notes.txt。两条粘贴路径对非图片文件的结果不一致，与 PR 自己的测试计划（「预期结果相同」）相悖，且已有维护者评审以此作为合入条件。修复方式：以 paste: true 插入，使文件路径被转换为引用，并更新目前固定了 { paste: false } 的 inserts copied non-image file paths... 测试。
— qwen3.8-max-preview via Qwen Code /review
| } catch (error) { | ||
| debugLogger.error('Error reading clipboard files:', error); | ||
| return []; | ||
| } |
There was a problem hiding this comment.
[Suggestion] When getFiles() throws at runtime (e.g. another process holds the clipboard open — antivirus scan, RDP session), the error is swallowed into debugLogger.error and [] is returned without signaling the caller, so handleClipboardPaste / broadcastClipboardPaste silently falls through and the user sees nothing. — Concrete cost: the shortcut appears to do nothing; the only evidence is a debug-level log that requires DEBUG=* to see, so a "file paste doesn't work on Windows" report needs a developer to reproduce with debug logging to diagnose. Note the module-load path above does call onUnavailable, but a runtime read failure is not "module unavailable" — and in broadcastClipboardPaste reusing onUnavailable would set clipboardImageUnavailable and wrongly skip a still-working image check. Consider distinguishing "no files" from "read failed" (a distinct callback or result type) so the caller can surface a message without disabling the image fallback.
中文说明
当 getFiles() 在运行时抛错（例如另一个进程占用剪贴板——杀毒软件扫描、远程桌面会话）时，错误被吞进 debugLogger.error 并返回 []，且不通知调用方，因此 handleClipboardPaste / broadcastClipboardPaste 会静默回退，用户什么都看不到。— 具体代价：快捷键看起来毫无反应；唯一证据是一条需要 DEBUG=* 才能看到的 debug 级日志，因此「Windows 上文件粘贴无效」的反馈需要开发者开启调试日志复现才能定位。注意上方模块加载失败路径确实调用了 onUnavailable，但运行时读取失败并非「模块不可用」——而且在 broadcastClipboardPaste 中复用 onUnavailable 会置位 clipboardImageUnavailable，从而错误地跳过仍然可用的图片检查。建议区分「没有文件」与「读取失败」（用独立回调或返回结构），让调用方能弹出提示而不禁用图片回退。
— qwen3.8-max-preview via Qwen Code /review
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed — no blockers. Suggestions are inline.
中文说明
已审查——无阻断问题。 建议见行内评论。
— qwen3.8-max-preview via Qwen Code /review
| const normalizedPath = /^(?:[A-Za-z]:\\|\\\\)/.test(filePath) | ||
| ? filePath.replaceAll('\\', '/') | ||
| : filePath; |
There was a problem hiding this comment.
[Suggestion] The UNC alternative in this regex (the branch that matches paths beginning with two backslashes, e.g. a network share) has no test exercising it — a mutation deleting that alternative survives. The drive-letter branch is covered by the inserts copied non-image files as references test, and the unescapePath side is tested for UNC paths in paths.test.ts, but the format side of the round-trip is not. — Concrete cost: if a future refactor drops the UNC alternative, a path copied from a network share would keep its backslashes instead of being normalized to the forward-slash form (@ + //server/share/...); the unescapePath guard (which only recognizes the forward-slash @C:/ and @// shapes) would then not recognize the reference and it would fail to resolve. Consider adding a UNC case to the non-image-reference test asserting the buffer shows the forward-slash @//server/share/My Report.txt form.
中文说明
[Suggestion] 这个正则中的 UNC 分支（匹配以两个反斜杠开头的路径，例如网络共享）没有测试覆盖——删除该分支的 mutation 会存活。盘符分支已由 inserts copied non-image files as references 测试覆盖，unescapePath 一侧也在 paths.test.ts 中针对 UNC 路径做了测试，但往返的格式化一侧没有。— 具体代价：如果未来重构删除了 UNC 分支，从网络共享复制的路径会保留反斜杠，而不会被规范化为正斜杠形式（@ + //server/share/...）；此时 unescapePath 的守卫（只识别正斜杠的 @C:/ 和 @// 形式）将无法识别该引用，导致其解析失败。建议在非图片引用测试中补充一个 UNC 用例，断言缓冲区显示正斜杠形式 @//server/share/My Report.txt。
— qwen3.8-max-preview via Qwen Code /review
There was a problem hiding this comment.
Addressed and pushed in a5a07390f. The direct clipboard-shortcut test now covers both drive-letter and UNC paths, including \\server\share\My Report.txt → @//server/share/My\ Report.txt. I also added round-trip coverage through the empty bracketed-paste/TextBuffer path so the generated reference remains resolvable by handleAtCommand.
While reviewing the complete diff, I fixed two related cross-entry regressions: raw Windows paths from empty bracketed paste now use the same reference formatter, and escaped shell metacharacters in normalized image references are unescaped before attachment promotion.
Validation: InputPrompt + Keypress 321 passed / 1 skipped; at-command + clipboard 93 passed / 2 skipped; core path tests 124 passed / 12 skipped; focused image/classifier tests 20 passed. Prettier, ESLint, CLI typecheck, and diff check also passed. The final complete 10-file diff review returned no findings.
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed — no blockers. Suggestions are inline.
中文说明
已审查——无阻断问题。 建议见行内评论。
— qwen3.8-max-preview via Qwen Code /review
| const hasImage = | ||
| clipboardFiles.length === 0 && !clipboardImageUnavailable | ||
| ? await clipboardHasImage(onUnavailable) | ||
| : false; |
There was a problem hiding this comment.
[Suggestion] The image-data branch of this new helper — no copied files and the native module available, so clipboardHasImage() decides pasteImage — has no test. A mutation const hasImage = false survives every added test: the "unavailable native module" test only asserts clipboardHasImage was not called (the mutant agrees), and the two "copied Windows files" cases short-circuit to false regardless. — Concrete cost: a future regression breaking terminal image-paste (an empty bracketed paste with an image on the clipboard and no copied files) on macOS/Linux would merge with every KeypressContext test green, because that distinguishing input is never exercised. Add a case that drives the branch:
it('broadcasts pasteImage for image data on an empty paste when no files are copied', async () => {
mockReadClipboardFiles.mockResolvedValue([]);
mockClipboardHasImage.mockResolvedValue(true);
// render + subscribe a keyHandler, then:
act(() => stdin.sendPaste(''));
await waitFor(() =>
expect(keyHandler).toHaveBeenCalledWith(
expect.objectContaining({ paste: true, pasteImage: true }),
),
);
expect(mockClipboardHasImage).toHaveBeenCalled();
});中文说明
这个新 helper 的图片数据分支——没有复制文件、原生模块可用，因此由 clipboardHasImage() 决定 pasteImage——没有测试覆盖。变异 const hasImage = false 能在所有新增测试下存活：「原生模块不可用」测试只断言 clipboardHasImage 未被调用（与变异体一致），而两个「复制了 Windows 文件」用例无论如何都会短路为 false。— 具体代价：未来若某个回归破坏了 macOS/Linux 上的终端图片粘贴（空 bracketed paste、剪贴板有图片、无复制文件），所有 KeypressContext 测试仍会是绿的，因为那个区分性输入从未被演练。建议补充一个驱动该分支的用例（见上方代码）。
— qwen3.8-max-preview via Qwen Code /review
| const isForwardSlashWindowsAtReference = /^@(?:[A-Za-z]:\/|\/\/)/.test( | ||
| filePath, | ||
| ); | ||
| if (os.platform() === 'win32' && !isForwardSlashWindowsAtReference) { |
There was a problem hiding this comment.
[Suggestion] The load-bearing Windows branch of unescapePath — the isForwardSlashWindowsAtReference guard that lets the new @C:/… / @//… clipboard references unescape and resolve on Windows — is exercised only by tests gated behind skipIf(!isWindows) (paths.test.ts) and runIf(process.platform === 'win32') (atCommandProcessor.test.ts). PR-level CI runs Linux only (the Windows job runs in the merge queue), and the platform-agnostic classifyPastedImagePaths tests run where os.platform() is 'linux', so the guard's regex is computed but never load-bearing there. — Concrete cost: if a future change simplifies this guard back toward a blanket win32 no-op, or breaks the ^@(?:[A-Za-z]:\/|\/\/) regex, then on Windows @C:/My\ Photos/pic.png would stay escaped and fail to resolve — silently breaking the whole Explorer-file-paste feature — while PR-level CI stays green. Cover the decision table without a Windows runner, e.g. spy os.platform (or factor the platform check out) so a Linux test asserts a win32 @C:/…/@//… reference is unescaped while a win32 backslash path without @ is preserved:
vi.spyOn(os, 'platform').mockReturnValue('win32');
expect(unescapePath('@C:/My\\ Photos/pic.png')).toBe('@C:/My Photos/pic.png');
expect(unescapePath('C:\\Users\\mochi\\notes.txt')).toBe('C:\\Users\\mochi\\notes.txt');中文说明
unescapePath 中承重的 Windows 分支——即让新的 @C:/… / @//… 剪贴板引用能在 Windows 上反转义并解析的 isForwardSlashWindowsAtReference 门禁——只被 skipIf(!isWindows)（paths.test.ts）和 runIf(process.platform === 'win32')（atCommandProcessor.test.ts）门禁后的测试覆盖。PR 级 CI 只跑 Linux（Windows 任务在合并队列中运行），而平台无关的 classifyPastedImagePaths 测试运行在 os.platform() 为 'linux' 的环境，因此该门禁的正则在那里只是被计算、却从未真正起作用。— 具体代价：如果未来某个改动把这个门禁简化回一刀切的 win32 no-op，或破坏了 ^@(?:[A-Za-z]:\/|\/\/) 正则，那么在 Windows 上 @C:/My\ Photos/pic.png 会保持转义状态而无法解析——悄无声息地破坏整个资源管理器文件粘贴功能——而 PR 级 CI 仍是绿的。建议在没有 Windows runner 的情况下覆盖该决策表，例如 spy os.platform（或将平台判断抽离），让一个 Linux 测试断言 win32 的 @C:/…/@//… 引用会被反转义、而不带 @ 的 win32 反斜杠路径会被保留（见上方代码）。
— qwen3.8-max-preview via Qwen Code /review
| const normalized = unescapePath( | ||
| token.replace(/^["']|["']$/g, ''), // strip surrounding quotes | ||
| ) | ||
| .replace(/^@/, '') // strip the `@` reference prefix | ||
| .replace(/^["']|["']$/g, '') // strip surrounding quotes | ||
| .replace(/\\ /g, ' '); // unescape shell-escaped spaces |
There was a problem hiding this comment.
[Suggestion] The reordering moved quote-stripping before @-stripping (so unescapePath can see the @ for its Windows guard), but this regresses quoted @-reference tokens like @"/var/tmp/screenshot.png": ^["'] cannot match because @ is the first character, so only the trailing quote is stripped, and after @ is removed a stray leading quote remains ("/var/tmp/screenshot.png). The end-anchored extension regex still matches .png, so the malformed path enters promotePastedImagePaths, where path.isAbsolute('"/var/tmp/screenshot.png') is false → fs.stat throws → the fallback inserts inert text. The old order (strip @ first) promoted such a paste to an attachment. Confirmed by probe: the current code returns ['"/var/tmp/screenshot.png']; adding the second quote-strip returns ['/var/tmp/screenshot.png']. The diff's own formatClipboardFileReference never wraps in quotes, so only externally-pasted quoted references are affected.
| const normalized = unescapePath( | |
| token.replace(/^["']|["']$/g, ''), // strip surrounding quotes | |
| ) | |
| .replace(/^@/, '') // strip the `@` reference prefix | |
| .replace(/^["']|["']$/g, '') // strip surrounding quotes | |
| .replace(/\\ /g, ' '); // unescape shell-escaped spaces | |
| const normalized = unescapePath( | |
| token.replace(/^["']|["']$/g, ''), // strip surrounding quotes | |
| ) | |
| .replace(/^@/, '') // strip the `@` reference prefix | |
| .replace(/^["']|["']$/g, '') // strip quotes exposed by `@` removal | |
| .replace(/\\ /g, ' '); // unescape shell-escaped spaces |
中文说明
这次重排把去引号移到了去 @ 之前（好让 unescapePath 能看到 @ 以触发其 Windows 门禁），但这会让带引号的 @ 引用 token（如 @"/var/tmp/screenshot.png"）发生回归：因为首字符是 @，^["'] 无法匹配，所以只有末尾引号被去掉；在去掉 @ 之后会残留一个多余的前导引号（"/var/tmp/screenshot.png）。由于扩展名正则是末尾锚定的，仍会匹配 .png，于是这个畸形路径进入 promotePastedImagePaths，其中 path.isAbsolute('"/var/tmp/screenshot.png') 为 false → fs.stat 抛错 → 回退插入无效文本。旧顺序（先去 @）会把这种粘贴提升为附件。已通过探针确认：当前代码返回 ['"/var/tmp/screenshot.png']；补上第二次去引号后返回 ['/var/tmp/screenshot.png']。本 PR 自身的 formatClipboardFileReference 从不在外面包引号，因此只有外部粘贴的带引号引用会受影响。
— qwen3.8-max-preview via Qwen Code /review
There was a problem hiding this comment.
Fixed and pushed in 7e47a6a44. Quoted @ references now remove the quote immediately after the reference prefix before calling unescapePath, so the Windows forward-slash @C:/… / @//… dispatch remains intact while inputs such as @"/var/tmp/screenshot.png" and @"C:/Photos/image\\(1\\).png" normalize correctly.
I added parameterized regression coverage for both shapes. Validation: focused classifier tests 8/8; full InputPrompt tests 207 passed / 1 skipped; Prettier, ESLint, CLI build, typecheck, and diff check passed. An independent review of the complete 10-file PR diff returned No findings.
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed — no blockers. Suggestions are inline.
中文说明
已审查——无阻断问题。 建议见行内评论。
— qwen3.8-max-preview via Qwen Code /review
| if (!clipboard.hasFormat('files')) { | ||
| return []; | ||
| } | ||
| return clipboard.getFiles(); |
There was a problem hiding this comment.
[Suggestion] Clipboard-supplied paths reach filesystem operations unvalidated, so a UNC path forces an outbound SMB connection and NTLMv2 authentication to an attacker-named host. — Failure scenario: a copied-file selection may contain a UNC path (\\attacker-host\share\image.png); the PR even blesses UNC input in a test. An image UNC path reaches fs.stat/fs.copyFile in promotePastedImagePaths, and a non-image UNC path becomes an @//host/share/file reference resolved through atCommandProcessor on submit — both open a remote SMB session that sends the user's NTLMv2 credentials. Any process that can write the clipboard can stage the path; the user only has to paste. (Writing the clipboard generally needs same-user code execution, which limits exploitability — hence Suggestion, not Critical.)
Suggested fix: before using a clipboard path, reject remote/UNC paths (skip entries whose raw form starts with two backslashes, or whose normalized form starts with @//), or warn the user that pasting will contact a remote host; at minimum document the trust assumption.
中文说明
剪贴板提供的路径未经校验就进入文件系统操作，因此 UNC 路径会强制向攻击者指定的域名发起出站 SMB 连接并进行 NTLMv2 认证。— 失败场景：复制的文件列表中可能包含 UNC 路径（\\attacker-host\share\image.png），PR 甚至在测试中认可了 UNC 输入。图片型 UNC 路径会进入 promotePastedImagePaths 的 fs.stat/fs.copyFile，非图片型 UNC 路径会变成 @//host/share/file 引用并在提交时经 atCommandProcessor 解析——两者都会打开远程 SMB 会话并发送用户的 NTLMv2 凭据。任何能写剪贴板的进程都可以布置该路径，用户只需粘贴即可触发。（写剪贴板通常需要同用户代码执行权限，限制了可利用性——因此定为 Suggestion 而非 Critical。）
建议修复：在使用剪贴板路径前，拒绝远程/UNC 路径（跳过原始形式以两个反斜杠开头、或规范化后以 @// 开头的条目），或在粘贴会联系远程主机时向用户告警；至少应记录该信任假设。
— qwen3.8-max-preview via Qwen Code /review
| it.runIf(process.platform === 'win32')( | ||
| 'should resolve clipboard-formatted Windows references with escaped spaces', | ||
| async () => { |
There was a problem hiding this comment.
[Suggestion] This is the only integration test for the formatClipboardFileReference → handleAtCommand round-trip, and it is gated to Windows, so it never runs on Linux CI. — Concrete cost: the round-trip is equally valid on Linux (formatClipboardFileReference still calls escapePath; unescapePath has no early return there), yet the runIf guard skips it. The existing non-Windows test at line 382 manually constructs the escaped path rather than going through formatClipboardFileReference, so a future regression in the escape/unescape round-trip for paths with spaces — at this integration boundary — would ship uncaught.
| it.runIf(process.platform === 'win32')( | |
| 'should resolve clipboard-formatted Windows references with escaped spaces', | |
| async () => { | |
| it( | |
| 'should resolve clipboard-formatted Windows references with escaped spaces', | |
| async () => { |
中文说明
这是 formatClipboardFileReference → handleAtCommand 往返的唯一集成测试，且被限定为 Windows，因此从不在 Linux CI 上运行。— 具体代价：该往返在 Linux 上同样有效（formatClipboardFileReference 仍会调用 escapePath；unescapePath 在那里没有提前返回），但 runIf 守卫跳过了它。第 382 行现有的非 Windows 测试是手工构造转义路径，而非经过 formatClipboardFileReference，因此未来在此集成边界上针对含空格路径的转义/反转义往返若发生回归，将不会被捕获。
— qwen3.8-max-preview via Qwen Code /review
| const normalized = unescapePath( | ||
| token | ||
| .replace(/^@(["'])/, '@') // strip a quote after the `@` prefix | ||
| .replace(/^["']|["']$/g, ''), // strip surrounding quotes | ||
| ) | ||
| .replace(/^@/, '') // strip the `@` reference prefix | ||
| .replace(/^["']|["']$/g, '') // strip surrounding quotes | ||
| .replace(/\\ /g, ' '); // unescape shell-escaped spaces |
There was a problem hiding this comment.
[Suggestion] unescapePath already unescapes \ (space is in SHELL_SPECIAL_CHARS), so the trailing .replace(/\\ /g, ' ') double-unescapes a \\ (literal-backslash-then-space) sequence on Unix, losing the literal backslash. — Failure scenario: pasting @/tmp/foo\\ bar.png on Unix → unescapePath yields /tmp/foo\ bar.png, then the trailing replace strips the remaining \ → /tmp/foo bar.png; the literal backslash is lost, promotePastedImagePaths stats the wrong path and falls back to raw text instead of attaching. Rare trigger (a literal backslash before a space in an image filename); degraded paste, not data loss.
| const normalized = unescapePath( | |
| token | |
| .replace(/^@(["'])/, '@') // strip a quote after the `@` prefix | |
| .replace(/^["']|["']$/g, ''), // strip surrounding quotes | |
| ) | |
| .replace(/^@/, '') // strip the `@` reference prefix | |
| .replace(/^["']|["']$/g, '') // strip surrounding quotes | |
| .replace(/\\ /g, ' '); // unescape shell-escaped spaces | |
| const normalized = unescapePath( | |
| token | |
| .replace(/^@(["'])/, '@') // strip a quote after the `@` prefix | |
| .replace(/^["']|["']$/g, ''), // strip surrounding quotes | |
| ) | |
| .replace(/^@/, ''); // strip the `@` reference prefix |
中文说明
unescapePath 已经会反转义 \ （空格在 SHELL_SPECIAL_CHARS 中），因此末尾的 .replace(/\\ /g, ' ') 会在 Unix 上对 \\ （字面反斜杠后跟空格）序列二次反转义，丢失字面反斜杠。— 失败场景：在 Unix 上粘贴 @/tmp/foo\\ bar.png → unescapePath 得到 /tmp/foo\ bar.png，随后末尾的 replace 去掉剩余的 \ → /tmp/foo bar.png；字面反斜杠丢失，promotePastedImagePaths 对错误路径执行 stat 并回退为原始文本而非附件。触发条件罕见（图片文件名中空格前出现字面反斜杠）；属于粘贴体验降级，非数据丢失。
— qwen3.8-max-preview via Qwen Code /review
| if (pastedImagePaths.allImages) { | ||
| await promotePastedImagePaths(pastedImagePaths.imagePaths, pasted); | ||
| } else if (!insertLargePastePlaceholder(pasted, fileReferences)) { |
There was a problem hiding this comment.
[Suggestion] When every copied image file fails to stat/copy, the promotePastedImagePaths fallback inserts the raw Windows paths (pasted) instead of the formatted @-references (fileReferences), producing text that is not resolvable on submit. — Failure scenario: on Windows, copy two .png files from a temporarily unreachable network share and paste; every fs.stat fails, the else branch (~line 847) inserts the raw, newline-separated paths (e.g. \\server\share\img1.png, no @ prefix). The non-image branch two lines below correctly inserts fileReferences (@//server/share/...), so the all-image fallback is inconsistent for the same clipboard content.
| if (pastedImagePaths.allImages) { | |
| await promotePastedImagePaths(pastedImagePaths.imagePaths, pasted); | |
| } else if (!insertLargePastePlaceholder(pasted, fileReferences)) { | |
| if (pastedImagePaths.allImages) { | |
| await promotePastedImagePaths(pastedImagePaths.imagePaths, fileReferences); | |
| } else if (!insertLargePastePlaceholder(pasted, fileReferences)) { |
中文说明
当所有复制的图片文件 stat/copy 都失败时，promotePastedImagePaths 的回退会插入原始 Windows 路径（pasted），而非格式化后的 @ 引用（fileReferences），产生提交时无法解析的文本。— 失败场景：在 Windows 上从临时不可达的网络共享复制两个 .png 文件并粘贴；每次 fs.stat 都失败，else 分支（约第 847 行）插入原始的、以换行分隔的路径（例如 \\server\share\img1.png，无 @ 前缀）。下方两行的非图片分支会正确插入 fileReferences（@//server/share/...），因此全图片回退对相同剪贴板内容的处理不一致。
— qwen3.8-max-preview via Qwen Code /review
There was a problem hiding this comment.
Fixed and pushed in de3e8237c. The all-image path now passes the preformatted file references as the promotion fallback, so complete stat/copy failure inserts resolvable @ references instead of raw Windows paths. The regression uses two images to cover both a missing source and a copy failure, and asserts that both formatted references are preserved. Validation: the full InputPrompt suite passed 212 tests with 1 skipped; CLI typecheck, Prettier, ESLint, the build, and diff checks passed. The independent complete-diff review returned No findings.
|
@qwen-code /resolve |
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
|
Qwen Code resolved the merge conflicts and pushed the branch update. Merge summary — PR #7957 (feat(cli): paste copied Windows files)Root causemain's #8120 (commit Textual or semanticDocs: same table row, different columns — #8120 added KeypressContext: purely adjacent. #8120 generalized the macOS composed-glyph rewrite into an What is load-bearing
What I could not verifyNo build, lint, or tests were run. 中文说明冲突来源：main 上的 #8120（ 关键约束：两条粘贴流程互相独立——合成 Alt+v 走 |
ReviewOverviewAdds Windows Explorer file paste through the existing clipboard shortcut (Alt+V) and the empty bracketed-paste path. Overall this is a well-scoped, well-tested change. The design choices are sound: files take precedence over image pixels, non-Windows platforms early-return untouched, and image-only selections converge on the existing attachment flow. What I verified
Issues / suggestions
Test coverageGood: drive-letter and UNC cases, both entry paths (shortcut and empty bracketed paste), large file lists, native-module unavailable, Security / performanceNo concerns. Clipboard reads happen only on user-initiated paste and are win32-gated; references are inserted as plain buffer text with |
|
Follow-up pushed in Addressed in this narrow round:
Kept out of scope: the hand-typed mixed-separator Windows edge and the divergent VS Code companion copy remain follow-up candidates because generated references use forward slashes and the current clipboard path does not flow through the companion. The existing UNC trust and runtime clipboard-error UX suggestions are also unchanged. Validation: the full |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed — no blockers. Suggestions are inline.
中文说明
已审查——无阻断问题。 建议见行内评论。
— qwen3.8-max-preview via Qwen Code /review (v0.21.3)
| broadcast({ | ||
| name: '', | ||
| ctrl: false, | ||
| meta: false, | ||
| shift: false, | ||
| paste: true, | ||
| pasteImage: hasImage, | ||
| clipboardImageUnavailable, | ||
| sequence: clipboardFiles.map(formatClipboardFileReference).join('\n'), | ||
| }); |
There was a problem hiding this comment.
[Suggestion] broadcastClipboardPaste puts the synthesized clipboard-file @ references into the broadcast paste sequence, which fans out to every keypress subscriber — not just InputPrompt. SettingsDialog.tsx:795 has an if (key.paste && key.sequence) branch that appends key.sequence into the setting's edit buffer. Before this change an empty bracketed paste broadcast sequence: '', so that branch was dead; now it fires.
— Failure scenario: on Windows with files copied in Explorer, open the Settings dialog and edit a setting, then trigger an empty paste (Ctrl+V). The fabricated references land in the edit field — a text setting receives @C:/Users/mochi/My\ Notes/notes.txt; a number setting's replace(/[^0-9\-+.]/g, '') filter mangles @C:/Users/mochi/notes-1.txt into -1.. Confirmed with a probe that renders the real SettingsDialog inside the real KeypressProvider: the references appeared in the edit field, and reverting to sequence: '' made both cases pass cleanly.
Suggested fix: mark the synthesized event so non-input consumers can ignore it — add a field to the broadcast and guard SettingsDialog:
// KeypressContext.tsx — broadcastClipboardPaste
broadcast({
...
paste: true,
pasteImage: hasImage,
clipboardImageUnavailable,
clipboardFiles: clipboardFiles.length > 0, // synthesized, not user-typed
sequence: clipboardFiles.map(formatClipboardFileReference).join('\n'),
});
// SettingsDialog.tsx:795
if (key.paste && key.sequence && !key.clipboardFiles) {(Alternatively, avoid fabricating sequence for a genuinely empty terminal paste and let InputPrompt pull the files through its own handler.)
中文说明
broadcastClipboardPaste 把合成出来的剪贴板文件 @ 引用放进了广播粘贴事件的 sequence，而该广播会发送给所有按键订阅者，并不只是 InputPrompt。SettingsDialog.tsx:795 有一个 if (key.paste && key.sequence) 分支，会把 key.sequence 追加进设置项的编辑缓冲区。本次改动之前，空的 bracketed paste 广播的是 sequence: ''，所以那个分支不会触发；现在会触发了。
— 失败场景：在 Windows 上资源管理器里复制了文件时，打开设置对话框并编辑某个设置项，然后触发一次空粘贴（Ctrl+V）。伪造的引用会落进编辑框——文本设置会收到 @C:/Users/mochi/My\ Notes/notes.txt；数字设置的 replace(/[^0-9\-+.]/g, '') 过滤会把 @C:/Users/mochi/notes-1.txt 变成 -1.。已用 probe 验证：在真实的 KeypressProvider 中渲染真实的 SettingsDialog，引用确实出现在编辑框中；改回 sequence: '' 后两种情况都恢复正常。
修复建议：给合成的粘贴事件加一个标记，让非输入类消费者忽略它——在广播里加一个字段并在 SettingsDialog 中加守卫（见上方代码）。或者，对于真正的空终端粘贴不要伪造 sequence，让 InputPrompt 通过自己的处理函数去读取文件。
— qwen3.8-max-preview via Qwen Code /review (v0.21.3)
There was a problem hiding this comment.
Fixed in 9eacd6a0d. The empty-paste broadcast now carries formatted clipboard file references in a dedicated clipboardFiles field while keeping the shared sequence empty, so SettingsDialog and other generic text inputs cannot insert prompt-only @ references. InputPrompt consumes the dedicated field explicitly, and partial image promotion now preserves formatted references for any files that fail to stat or copy. Regression coverage exercises string and number SettingsDialog editors, regular text paste, both KeypressContext paste paths, and partial image-promotion failure. Validation: 404 passed / 1 skipped across InputPrompt, SettingsDialog, KeypressContext, and SettingInputPrompt; the localized ToolApproval test passed 2/2 after syncing current main; Prettier, ESLint, CLI typecheck/build, and git diff --check passed. An independent complete-diff re-review returned No findings.
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed — no blockers. Suggestions are inline. 1 Suggestion-level finding(s) could not be anchored to a changed line and were dropped; nothing further to act on here.
中文说明
已审查——无阻断问题。 建议见行内评论。 1 条建议级发现无法锚定到改动行，已丢弃；此处无需进一步处理。
— qwen3.8-max-preview via Qwen Code /review (v0.21.3)
| if (!stats.isFile()) { | ||
| failedImagePaths.push(imagePath); | ||
| continue; |
There was a problem hiding this comment.
[Suggestion] The new failedImagePaths.push(imagePath) in the !stats.isFile() branch has no test — every mockFsStat in this suite resolves { isFile: () => true }, and the failure-path tests only exercise the catch branch via rejections. — Failure scenario: probe-confirmed — mutating this back to if (!stats.isFile()) continue; leaves all 214 tests green. On Windows, copying a folder named tripod.png alongside a real photo.png and pasting would silently drop the folder's reference instead of retaining @C:/.../tripod.png, losing an item with no feedback.
Add a partial-promotion case where one stat resolves to a directory:
mockFsStat.mockResolvedValueOnce({ isFile: () => false }); // tripod.png is a directory
mockFsStat.mockResolvedValueOnce({ isFile: () => true }); // photo.png is a file
// trigger the shortcut with two copied files, one of them the directory
expect(mockFsCopyFile).toHaveBeenCalledTimes(1);
expect(mockBuffer.insert).toHaveBeenCalledWith(
expect.stringContaining('@C:/'), // the directory is retained as a reference
expect.anything(),
);中文说明
!stats.isFile() 分支中新加的 failedImagePaths.push(imagePath) 没有测试覆盖——本套件里所有 mockFsStat 都返回 { isFile: () => true }，而失败路径的测试只通过 reject 触发 catch 分支。— 失败场景：经探针确认——把这行改回 if (!stats.isFile()) continue; 后全部 214 个测试仍为绿色。在 Windows 上，把一个名为 tripod.png 的文件夹和真实的 photo.png 一起复制并粘贴时，文件夹的引用会被静默丢弃，而不是保留为 @C:/.../tripod.png，条目无声消失。建议补一条部分提升用例，让某个 stat 返回目录（见上方代码）。
— qwen3.8-max-preview via Qwen Code /review (v0.21.3)
| const handleClipboardFilePaste = useCallback( | ||
| async (pasted: string, fileReferences: string) => { | ||
| const pastedImagePaths = classifyPastedImagePaths(pasted); |
There was a problem hiding this comment.
[Suggestion] handleClipboardFilePaste's pasted parameter has no single contract: handleClipboardPaste (the Ctrl+V shortcut) passes raw OS paths (clipboardFiles.join('\\n')), while the key.clipboardFiles branch passes pre-formatted @-references. Both work only because classifyPastedImagePaths happens to tolerate both shapes, and nothing documents that. — Failure scenario: a maintainer who tightens classifyPastedImagePaths (or normalizes pasted here) assuming one shape silently breaks exactly one entry point; since the shortcut tests and the bracketed-paste tests live in separate blocks, a partial break can pass CI and ship.
Give the parameter one documented shape (e.g. carry raw paths on the Key so both callers pass raw paths and formatting happens once here), or at minimum document the dual contract:
// `pasted` may be raw OS paths (Ctrl+V) or pre-formatted `@`-references
// (bracketed paste); `classifyPastedImagePaths` must keep tolerating both.
async (pasted: string, fileReferences: string) => {中文说明
handleClipboardFilePaste 的 pasted 参数没有单一契约：handleClipboardPaste（Ctrl+V 快捷键）传入原始操作系统路径（clipboardFiles.join('\\n')），而 key.clipboardFiles 分支传入预格式化的 @ 引用。两者都能工作，仅仅因为 classifyPastedImagePaths 恰好同时容忍这两种形态，且没有任何注释说明这一点。— 失败场景：若维护者假设其中一种形态而收紧 classifyPastedImagePaths（或在此处归一化 pasted），会静默破坏恰好其中一个入口；由于快捷键测试与 bracketed-paste 测试分属不同代码块，局部破坏可以通过 CI 并合入。建议给该参数一个有文档的单一形态（例如把原始路径放在 Key 上，让两个调用方都传原始路径、格式化只在此处发生），或至少注明这一双重契约（见上方代码）。
— qwen3.8-max-preview via Qwen Code /review (v0.21.3)
| const normalized = unescapePath( | ||
| token | ||
| .replace(/^@(["'])/, '@') // strip a quote after the `@` prefix | ||
| .replace(/^["']|["']$/g, ''), // strip surrounding quotes | ||
| ).replace(/^@/, ''); // strip the `@` reference prefix |
There was a problem hiding this comment.
[Suggestion] unescapePath must run while the @ prefix is still attached — its win32 carve-out keys off @C:/ / @// and is a no-op otherwise — but the @ is stripped immediately afterward, with nothing marking the ordering as load-bearing. — Failure scenario: a maintainer refactoring to the more intuitive "strip @ then unescape" order would, on Windows, feed unescapePath('C:/Photos/image\\(1\\).png') (no @) → gate fails → no-op → \\( retained → fs.stat fails → copied images silently stop promoting and fall back to inserting reference text. No test or comment warns that reordering breaks Windows.
| const normalized = unescapePath( | |
| token | |
| .replace(/^@(["'])/, '@') // strip a quote after the `@` prefix | |
| .replace(/^["']|["']$/g, ''), // strip surrounding quotes | |
| ).replace(/^@/, ''); // strip the `@` reference prefix | |
| const normalized = unescapePath( | |
| token | |
| .replace(/^@(["'])/, '@') // strip a quote after the `@` prefix | |
| .replace(/^["']|["']$/g, ''), // strip surrounding quotes | |
| ).replace(/^@/, ''); // strip the `@` reference prefix (must follow unescapePath: its win32 carve-out keys off the `@` prefix) |
中文说明
unescapePath 必须在 @ 前缀仍然保留时运行——它的 win32 特例判断依赖 @C:/ / @//，否则就是 no-op——但紧接着 @ 就被剥离了，没有任何注释标明这个顺序是承重的。— 失败场景：若维护者重构成更直观的「先剥 @ 再反转义」顺序，在 Windows 上会传入 unescapePath('C:/Photos/image\\(1\\).png')（无 @）→ 门禁不匹配 → no-op → \\( 保留 → fs.stat 失败 → 复制的图片静默停止提升、回退为插入引用文本。没有测试或注释警告「调换顺序会破坏 Windows」。建议在剥离 @ 的那一行注明它必须跟在 unescapePath 之后（见上方 suggestion）。
— qwen3.8-max-preview via Qwen Code /review (v0.21.3)
Review — head
|
| Check | Result |
|---|---|
InputPrompt.test.tsx |
214/214 pass |
SettingsDialog.test.tsx |
64/64 pass |
KeypressContext.test.tsx |
120/120 pass |
clipboardUtils.test.ts |
33/33 pass |
atCommandProcessor.test.ts |
71 pass / 1 skipped |
packages/core paths.test.ts |
134 pass / 2 skipped |
The skips are the win32-gated cases — the unescapePath carve-out and the formatClipboardFileReference → handleAtCommand round-trip still get zero execution outside the merge queue, which matches the open thread on paths.ts:356.
I also re-checked the unescapePath blast radius at this head. handleAtCommand is the only caller that passes an @-prefixed string (atCommandProcessor.ts:148, with the @ deliberately retained); every core tool-param normalizer passes a bare path, so the ^@(?:[A-Za-z]:/|//) gate can never fire there. I'm satisfied that carve-out is safe as scoped.
Two mutation probes, and one item resolved:
- Resolved: the UNC alternative in
formatClipboardFileReferenceis now covered. Narrowing the regex to/^(?:[A-Za-z]:\\)/fails 4 tests acrossKeypressContext.test.tsxandInputPrompt.test.tsx. The earlier "UNC format side untested" thread can be closed.
Findings
1. [Suggestion] The new SettingsDialog guard is unreachable, and its 101 lines of tests don't pin it.
SettingsDialog.tsx:795 adds if (key.clipboardFiles) return;, but broadcastClipboardPaste now always broadcasts sequence: '' — so if (key.paste && key.sequence) on the next line cannot fire, and the trailing single-character branch rejects ch = '' (ch.length === 1 is false). Deleting the guard entirely leaves all three new Clipboard Paste cases green (3 passed / 61 skipped), and the full file green at 64/64.
Concrete cost: a maintainer reading the new describe('Clipboard Paste') block will believe it protects the guard. It doesn't — it pins behavior that the sequence: '' change already guarantees, so a later refactor that repopulates sequence and drops the guard would ship silently. Either drop the guard (the isolation is already achieved by the Key field split, which is the better fix) and keep one test as a regression pin on sequence staying empty, or keep the guard and note in the test why it is defense-in-depth rather than the thing under test.
2. [Suggestion] !stats.isFile() → failedImagePaths.push is still untested (InputPrompt.tsx:825, previously flagged, unchanged at this head).
Probe-confirmed: reverting to if (!stats.isFile()) continue; leaves all 214 tests green. Every mockFsStat in the suite resolves { isFile: () => true }; the partial-failure tests only reach the catch branch via rejections, so the directory case has no coverage. Failure scenario: copy a folder named photo.png in Explorer alongside a real image — its reference is silently dropped instead of retained. One case with mockFsStat.mockResolvedValueOnce({ isFile: () => false }) closes it.
3. [Suggestion — product] The non-image half of the feature silently no-ops for the most common Explorer flow.
This is the one I'd most like a decision on before merge. A non-image selection becomes an @ reference and nothing else. On submit, atCommandProcessor.ts:356-364 skips any path that is neither under the workspace nor under the Qwen temp dir, with an onDebugMessage only — and :564-583 then returns { processedQuery: [{ text: query }], shouldProceed: true }. So copying report.pdf from Downloads or the Desktop and pressing Alt+V produces a prompt that looks correct, submits without warning, and sends the model only the literal path string. No file content, no error, nothing outside DEBUG=*.
The PR body documents this as out of scope, which is honest. But the shipped behavior for "copy a file in Explorer, paste it" is: works if the file was already in your workspace (where @relative/path would have worked anyway), silently does nothing if it wasn't. Options, cheapest first: (a) surface a paste-time notice when a reference resolves outside the workspace, so the failure is visible before submit; (b) mirror the image flow and copy non-image files into the Qwen temp dir too, which makes them resolvable by construction. (a) is small and would make the current scope defensible on its own.
4. [Suggestion — minor] readClipboardFiles returns the native module's value unvalidated.
clipboardUtils.ts:370 does return clipboard.getFiles(); where getClipboardModule() is typed Promise<any | null>, so the declared Promise<string[]> is unchecked. hasFormat('files') plus the try/catch cover the known failure mode (my earlier probe showed getFiles() throws on an empty clipboard), but a non-array return would surface as a TypeError on clipboardFiles.length inside broadcastClipboardPaste / handleClipboardPaste — both invoked as void-ed promises, so it becomes an unhandled rejection rather than a caught error. const files = clipboard.getFiles(); return Array.isArray(files) ? files.filter((f) => typeof f === 'string') : []; closes it inside the existing try.
Verdict
No Critical at this head. Finding 1 is cleanup on the newest commit; finding 2 is a small test gap; finding 4 is one line. Finding 3 is a scope call rather than a defect — I'd like your read on whether a paste-time warning is worth adding now or tracked as a follow-up, given how many rounds this PR has already been through. The Windows-only coverage gap (the carve-out and the at-reference round-trip only execute in the merge queue) remains the standing risk on a change whose central claim is Windows runtime behavior.
|
Thanks for calling this out. For finding 3, I'd like to keep #7957 scoped to adding the Windows File Explorer clipboard ingestion path. The current outside-workspace limitation is already documented in the PR's Risk & Scope section. After this PR lands, I'll address the non-image-file behavior consistently in a separate follow-up PR, including eliminating the current silent skip, rather than expanding the scope of #7957 further. |
|
@qwen-code /resolve |
|
Qwen Code attempted to resolve merge conflicts but the run did not complete successfully. Check the workflow run for full logs. |
🩺 serve daemon A/BBuilt the PR base vs this PR head ✅ No response changes against the PR base across 12 scenario(s). — Qwen Code · serve A/B |
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.
3 Suggestion-level finding(s) this review confirmed are already reported on this PR and are not repeated:
- R2-1 packages/cli/src/ui/components/InputPrompt.tsx:856 — the !stats.isFile() branch's failedImagePaths.push still has no test — already reported (comment 3699335073)
- R2-2 packages/cli/src/ui/components/InputPrompt.tsx:894 — handleClipboardFilePaste's pasted parameter still has two contracts — already reported (comment 3699335074)
- R2-3 packages/cli/src/ui/components/InputPrompt.tsx:148 — the unescape-before-@-strip ordering is still unmarked — already reported (comment 3699335076)
Not reviewed: build-and-test — Test (windows-latest, Node 22.x) was skipped in CI and the win32-gated suites did not run locally either.
Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.
Not reviewed: build-and-test — Test (macos-latest, Node 22.x) was skipped in CI and the macOS clipboard-unavailability path did not run locally.
Not explored to full depth (tool budget reached): "agent reverse-audit (round 2)": could not confirm the native hasFormat('files') contract — only @teddyzhu/clipboard-linux-x64-gnu@0.0.5 is installed here and its README documents just 'te…; "agent 6c": did not run the vitest suites ( packages/core/src/utils/paths.test.ts , packages/cli/src/ui/utils/clipboardUtils.test.ts , InputPrompt.test.tsx , KeypressCon…; "agent 6c": could not verify that @teddyzhu/clipboard 's hasFormat('files') maps to CF_HDROP on Windows — only index.js / index.d.ts ship in node_modules (the declar….
Not reviewed: reverse audit — stopped before round 7 by the review time budget.
Deferred under the convergence posture (round 3, not a blocker) — recorded, not requested in this round:
packages/core/src/utils/paths.test.ts:215 — [probe] The win32 carve-out's only tests are host-gated and Windows is not a PR gatepackages/cli/src/ui/contexts/KeypressContext.tsx:791 — [probe] The image-side unavailability leg lost its only testpackages/cli/src/ui/components/SettingsDialog.tsx:803 — [probe] The new clipboardFiles guard is inert and its tests cannot tellpackages/cli/src/ui/utils/clipboardUtils.ts:23 — [probe] escapePath's set is narrower than the at-parser's break setpackages/cli/src/ui/utils/clipboardUtils.ts:19 — [probe] The new exported formatter has no oracle that can move independentlypackages/cli/src/ui/components/InputPrompt.tsx:896 — [probe] The all-images branch bypasses the large-paste placeholder cappackages/cli/src/ui/components/InputPrompt.tsx:856 — [probe] Unbounded copy into a temp dir whose only bound is 100 files
[Critical] R3-2: [certifies-falsely] Nothing bounds how many copied files one keystroke promotes, and cleanupOldClipboardImages — fired unawaited by the same block that just attached them — reclaims by ascending atimeMs against a count budget, so one large Explorer selection unlinks the first files that paste created. The attachment chips stay on screen pointing at deleted files and submit drops each one with only a debug-level message. (Relocated here from an inline anchor at packages/cli/src/ui/components/InputPrompt.tsx:895, a line an existing thread on this pull request already occupies.) On Windows, select 101 or more PNGs in Explorer and press Alt+V, or Ctrl+V where the terminal forwards an empty bracketed paste: readClipboardFiles returns the whole selection with no cap, promotePastedImagePaths copies every file into <runtimeBase>/tmp/clipboard and pushes one attachment chip per file, then the sweep with MAX_IMAGES = 100, CLEANUP_COUNT = 50 and removeCount = Math.min(CLEANUP_COUNT, imageFiles.length - MAX_IMAGES + CLEANUP_COUNT) over an ascending-atimeMs sort removes the 50 oldest — on a fresh or recently trimmed directory those are positions 0 to 49 of the batch this same paste just wrote. At submit each chip becomes @<relative temp path>, fs.realpath throws ENOENT, sawNotFound is set, and atCommandProcessor.ts:473-475 emits only onDebugMessage, so the user sees every chip, the model receives the rest, and nothing reports the loss. The same call is unawaited against a directory shared by every concurrent CLI session, so the 50 removed may instead be another live session's pending chips. Before this diff the loop was reachable only by pasting 101 or more image paths as text; one Explorer selection now triggers it with no path ever visible to the user. Witness, a probe driving the real cleanupOldClipboardImages over seeded directories with controlled atimeMs: fresh directory with paste=120 gives remaining=70 removed=50 and allRemovedWereFromThisPaste=true with removedBatchPositions 0 to 49; the boundary paste=100 removes 0; paste=101 leaves remaining=51 removed=50; paste=110, 150 and 200 each remove 50; and with 90 pre-existing older files the new batch is safe while 50 old ones are removed instead. InputPrompt.test.tsx mocks cleanupOldClipboardImages to vi.fn(), so no existing test can observe the unlink. Fix: bound the promotion before copying — when pastedImagePaths.imagePaths.length exceeds a cap, promote only the first cap and route the remainder through the failedImagePaths reference branch this diff already added, so the overflow lands as references instead of copies; better, exempt the in-flight batch by passing the just-written destination paths to the sweep and skipping them, or sweep before writing rather than after. Do not simply raise MAX_IMAGES: the boundary moves but the mechanism does not. The fix must not violate const MAX_IMAGES = 100; and const CLEANUP_COUNT = 50; at packages/cli/src/ui/utils/clipboardUtils.ts:639-640, consumed at :665-671 as removeCount = Math.min(CLEANUP_COUNT, imageFiles.length - MAX_IMAGES + CLEANUP_COUNT) over an ascending-atimeMs sort, so any new promotion cap must sit at or below MAX_IMAGES or the cleanup the same call triggers still unlinks this paste's own files; the interaction is measured, not hypothetical — changing the sweep's extension filter at clipboardUtils.ts:647 without this bound turns a 101-file uppercase-extension paste from zero dangling attachments into 50, so the two must not land separately. Please add a case in packages/cli/src/ui/components/InputPrompt.test.tsx that mocks readClipboardFiles with more than MAX_IMAGES image paths, drives the clipboard shortcut, and asserts both that mockFsCopyFile was called at most cap times and that the overflow came back as formatClipboardFileReference text via mockBuffer.insert, then remove the cap and confirm that test reds — the witness has to be on the new bound, because the existing suite cannot catch the deletion itself while cleanupOldClipboardImages is mocked at InputPrompt.test.tsx:86.
中文说明
仅完成部分审查，审查缺口已披露。
本轮确认的 3 条建议级发现已在 PR 上报告过，不再重复发布（列表见上方英文部分）。
未审查（原文为英文）：build-and-test — Test (windows-latest, Node 22.x) was skipped in CI and the win32-gated suites did not run locally either.
未审查（原文为英文）：build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.
未审查（原文为英文）：build-and-test — Test (macos-latest, Node 22.x) was skipped in CI and the macOS clipboard-unavailability path did not run locally.
未探索到全部深度（达到工具调用预算）："agent reverse-audit (round 2)"：could not confirm the native hasFormat('files') contract — only @teddyzhu/clipboard-linux-x64-gnu@0.0.5 is installed here and its README documents just 'te…；"agent 6c"：did not run the vitest suites ( packages/core/src/utils/paths.test.ts , packages/cli/src/ui/utils/clipboardUtils.test.ts , InputPrompt.test.tsx , KeypressCon…；"agent 6c"：could not verify that @teddyzhu/clipboard 's hasFormat('files') maps to CF_HDROP on Windows — only index.js / index.d.ts ship in node_modules (the declar…。
未审查：反向审计——评审时间预算不足，未能开始第 7 轮。
收敛姿态下延后（第 3 轮，非阻断）——已记录，本轮不要求修改：共 7 条（原文未翻译，列表见上方英文部分）。
[Critical] R3-2: [certifies-falsely] Nothing bounds how many copied files one keystroke promotes, and cleanupOldClipboardImages — fired unawaited by the same block that just attached them — reclaims by ascending atimeMs against a count budget, so one large Explorer selection unlinks the first files that paste created. The attachment chips stay on screen pointing at deleted files and submit drops each one with only a debug-level message. (Relocated here from an inline anchor at packages/cli/src/ui/components/InputPrompt.tsx:895, a line an existing thread on this pull request already occupies.) On Windows, select 101 or more PNGs in Explorer and press Alt+V, or Ctrl+V where the terminal forwards an empty bracketed paste: readClipboardFiles returns the whole selection with no cap, promotePastedImagePaths copies every file into <runtimeBase>/tmp/clipboard and pushes one attachment chip per file, then the sweep with MAX_IMAGES = 100, CLEANUP_COUNT = 50 and removeCount = Math.min(CLEANUP_COUNT, imageFiles.length - MAX_IMAGES + CLEANUP_COUNT) over an ascending-atimeMs sort removes the 50 oldest — on a fresh or recently trimmed directory those are positions 0 to 49 of the batch this same paste just wrote. At submit each chip becomes @<relative temp path>, fs.realpath throws ENOENT, sawNotFound is set, and atCommandProcessor.ts:473-475 emits only onDebugMessage, so the user sees every chip, the model receives the rest, and nothing reports the loss. The same call is unawaited against a directory shared by every concurrent CLI session, so the 50 removed may instead be another live session's pending chips. Before this diff the loop was reachable only by pasting 101 or more image paths as text; one Explorer selection now triggers it with no path ever visible to the user. Witness, a probe driving the real cleanupOldClipboardImages over seeded directories with controlled atimeMs: fresh directory with paste=120 gives remaining=70 removed=50 and allRemovedWereFromThisPaste=true with removedBatchPositions 0 to 49; the boundary paste=100 removes 0; paste=101 leaves remaining=51 removed=50; paste=110, 150 and 200 each remove 50; and with 90 pre-existing older files the new batch is safe while 50 old ones are removed instead. InputPrompt.test.tsx mocks cleanupOldClipboardImages to vi.fn(), so no existing test can observe the unlink. Fix: bound the promotion before copying — when pastedImagePaths.imagePaths.length exceeds a cap, promote only the first cap and route the remainder through the failedImagePaths reference branch this diff already added, so the overflow lands as references instead of copies; better, exempt the in-flight batch by passing the just-written destination paths to the sweep and skipping them, or sweep before writing rather than after. Do not simply raise MAX_IMAGES: the boundary moves but the mechanism does not. The fix must not violate const MAX_IMAGES = 100; and const CLEANUP_COUNT = 50; at packages/cli/src/ui/utils/clipboardUtils.ts:639-640, consumed at :665-671 as removeCount = Math.min(CLEANUP_COUNT, imageFiles.length - MAX_IMAGES + CLEANUP_COUNT) over an ascending-atimeMs sort, so any new promotion cap must sit at or below MAX_IMAGES or the cleanup the same call triggers still unlinks this paste's own files; the interaction is measured, not hypothetical — changing the sweep's extension filter at clipboardUtils.ts:647 without this bound turns a 101-file uppercase-extension paste from zero dangling attachments into 50, so the two must not land separately. Please add a case in packages/cli/src/ui/components/InputPrompt.test.tsx that mocks readClipboardFiles with more than MAX_IMAGES image paths, drives the clipboard shortcut, and asserts both that mockFsCopyFile was called at most cap times and that the overflow came back as formatClipboardFileReference text via mockBuffer.insert, then remove the cap and confirm that test reds — the witness has to be on the new bound, because the existing suite cannot catch the deletion itself while cleanupOldClipboardImages is mocked at InputPrompt.test.tsx:86.
— qwen3.8-max via Qwen Code /review (v0.23.0)
| */ | ||
| export function unescapePath(filePath: string): string { | ||
| if (os.platform() === 'win32') { | ||
| const isForwardSlashWindowsAtReference = /^@(?:[A-Za-z]:\/|\/\/)/.test( |
There was a problem hiding this comment.
[Critical] R3-1: [certifies-falsely] [regression] The new win32 carve-out in unescapePath decides "this string is one of our generated @ references" from a two-alternative prefix regex, and that proxy is wrong in both directions: it eats separator backslashes from mixed-separator references the previous code preserved, and it still declines to unescape references this PR's own formatter can emit.
atCommandProcessor.ts:147-148 hands the whole token to unescapePath with the @ still attached — its own comment says "unescapePath expects the @ symbol to be present" — and Windows accepts / and \ interchangeably, so @C:/repo\#docs\readme.md is a valid reference to a real file. The prefix test matches @C:/, unescapeShellSpecials then removes the separator backslash in front of # (which is in SHELL_SPECIAL_CHARS), and the reference resolves to C:/repo#docs\readme.md, a path that exists nowhere. read_many_files gets ENOENT and atCommandProcessor.ts:473-475 skips it with only an onDebugMessage, so the prompt is submitted and answered as though the file had been attached. The sharpest part is that C:/repo\#docs\readme.md is the exact string this diff's own new test at paths.test.ts:210-212 pins as must-be-preserved; it survives only because that test omits the @ the real consumer always supplies. At the merge base unescapePath was an unconditional win32 no-op, so every one of these came back unchanged and resolved.
The same predicate is too narrow for what this PR emits. formatClipboardFileReference escapes unconditionally but normalizes \ to / only for drive-letter and UNC shapes, so the new failedImagePaths fallback at InputPrompt.tsx:879-884 handed a non-drive token produces @notes\my\ image.png; the predicate does not match, win32 no-ops, and the inserted reference is dead on both platforms (win32 keeps the literal backslash-space, the Linux arm truncates it to notes\my). And the doc comment this diff adds at paths.ts:351-354 states the proxy as a general fact — "Generated @ references use forward-slash absolute Windows paths" — which four of the five in-tree generators violate, so the next maintainer reads them as handled and neither gets fixed.
The escape side and the unescape side live in different packages with nothing keeping them in step, so this cannot be closed one path shape at a time. Discriminate on the invariant the prefix is standing in for — every backslash in the value is an escape, not a separator — and derive both sides from one exported helper here, built from SHELL_SPECIAL_CHARS the same way UNESCAPE_REGEX already is at paths.ts:59-63, so the formatter cannot emit a shape unescapePath refuses and unescapePath cannot eat a separator the formatter left:
// derived from SHELL_SPECIAL_CHARS, not a hand-written class
const isGeneratedAtReference =
/^@(?:[A-Za-z]:\/|\/\/)/.test(filePath) &&
!BACKSLASH_SEPARATOR.test(filePath);If you would rather not carry any of this in core, the narrower-depth alternative is to leave unescapePath a win32 no-op and apply the reference unescape at the only two sites that ever see an @-prefixed value, atCommandProcessor.ts:148 and InputPrompt.tsx:144. Either way, narrow the comment at paths.ts:351-354 to the producer whose output it actually describes.
Witness:
run against the built packages/core/dist/src/utils/paths.js, os.platform() -> 'win32':
"C:/repo\#docs\readme.md" -> "C:/repo\#docs\readme.md" UNCHANGED <- the string paths.test.ts:210 pins
"@C:/repo\#docs\readme.md" -> "@C:/repo#docs\readme.md" CHANGED <- the same string as the consumer passes it
"@C:/tools\[dev]\cli.exe" -> "@C:/tools[dev]\cli.exe" CHANGED
"@C:/Program\ Files/Qwen/qwen.exe" -> "@C:/Program Files/Qwen/qwen.exe" CHANGED <- the intended fix, still works
"C:\(v2)\file.txt" -> "C:\(v2)\file.txt" UNCHANGED <- must stay preserved
base arm, git show 077c57d2005b:packages/core/src/utils/paths.ts:
if (os.platform() === 'win32') { return filePath; } <- unconditional no-op, so all of the above came back UNCHANGED
A/B over the real generator->consumer chain (escapePath -> reference -> real extractAtPathCommands):
BASE raw="C:/repo\#docs\readme.md" extracted=["C:/repo\#docs\readme.md"] resolves-as-typed=true
PR raw="C:/repo\#docs\readme.md" extracted=["C:/repo#docs\readme.md"] resolves-as-typed=false
under-unescape direction, real functions, win32 spied:
format in="notes\my image.png" ref="@notes\my\ image.png" extractAt=["notes\my\ image.png"]
host(linux) arm: extractAt=["notes\my"]
generator survey - 4 of 5 violate the new comment:
G1 formatClipboardFileReference -> "@C:/Users/me/My\ Notes/notes.txt" carve-out TRUE
G2 text-buffer.ts:2017/:2095 -> "@C:\Users\me\My\ Notes\notes.txt" carve-out FALSE
G3 useAtCompletion.ts:506 -> "@src/my\ report.ts" carve-out FALSE
G4 input-prompt-model.ts:936 -> same shape as G3 carve-out FALSE
declared assumption: that Windows resolves a mixed-separator path at all could not be exercised on a
Linux harness - the string-level regression is measured, the filesystem acceptance is assumed.
The fix must not violate what this PR already pins: unescapePath('C:\\(v2)\\file.txt') unchanged (packages/core/src/utils/paths.test.ts:206), unescapePath('C:/repo\\#docs\\readme.md') unchanged (:210-212), '@C:/Program\\ Files/Qwen/qwen.exe' unescaping to '@C:/Program Files/Qwen/qwen.exe' (:218) and '@//server/share/My\\ File.txt' to '@//server/share/My File.txt' (:221-222), plus the reference literals at packages/cli/src/ui/contexts/KeypressContext.test.tsx:1107-1110 — because unescapePath has 19 non-test call sites in packages/core and 4 in the CLI that all pass bare paths and rely on the win32 no-op, and the discriminator must be derived from SHELL_SPECIAL_CHARS (paths.ts:47), whose own comment states it is "Kept as the single source of truth for the escape set so platform-specific unescapers cannot drift from it".
Please add expect(unescapePath('@C:/repo\\#docs\\readme.md')).toBe('@C:/repo\\#docs\\readme.md') to the win32 block in packages/core/src/utils/paths.test.ts, de-gated with vi.spyOn(os, 'platform').mockReturnValue('win32') so it runs on the Linux PR lane, then remove the new discriminator and confirm that test reds while the existing @C:/Program\\ Files/... case stays green — reverting to the unconditional no-op is not a fix.
中文说明
unescapePath 中新增的 win32 特例判断用一个只有两个分支的前缀正则来决定「这个字符串是我们自己生成的 @ 引用」，而这个替代判据在两个方向上都是错的：它会吃掉混合分隔符引用中的分隔反斜杠（旧代码是保留的），同时又仍然拒绝反转义本 PR 自己的格式化函数可能产出的引用。
atCommandProcessor.ts:147-148 会把整个 token 连同 @ 一起传给 unescapePath（该处注释写明「unescapePath expects the @ symbol to be present」），而 Windows 允许 / 与 \ 混用，因此 @C:/repo\#docs\readme.md 是指向真实文件的合法引用。前缀判断匹配到 @C:/，unescapeShellSpecials 随后会去掉 # 前面的分隔反斜杠（# 属于 SHELL_SPECIAL_CHARS），引用被解析为 C:/repo#docs\readme.md——一个不存在的路径。read_many_files 得到 ENOENT，atCommandProcessor.ts:473-475 仅通过 onDebugMessage 跳过它，于是提示词照常提交并被回答，就好像文件已经附加成功。最关键的一点是：C:/repo\#docs\readme.md 正是本 diff 自己在 paths.test.ts:210-212 新增测试中断言「必须保持不变」的字符串；它之所以没被破坏，只是因为该测试没有带上真实调用方总会带上的 @。在合并基线上 unescapePath 是无条件的 win32 no-op，上述所有输入都原样返回并能正常解析。
同一个判据对本 PR 产出的引用又太窄。formatClipboardFileReference 无条件转义，但只对盘符和 UNC 形态把 \ 归一化为 /，因此 InputPrompt.tsx:879-884 新增的 failedImagePaths 回退分支在拿到非盘符 token 时会产出 @notes\my\ image.png；前缀判断不匹配，win32 下 no-op，插入的引用在两个平台上都是死的（win32 保留字面的反斜杠加空格，Linux 分支在未转义的空格处截断为 notes\my）。此外本 diff 在 paths.ts:351-354 新增的注释把这个替代判据当作普遍事实来陈述——「Generated @ references use forward-slash absolute Windows paths」——而仓库内五个生成器中有四个并不满足，于是后来的维护者会以为它们已被覆盖，两处都不会去修。
转义侧与反转义侧位于不同包中，没有任何机制让二者保持同步，因此这个问题无法靠逐个路径形态来收尾。应当针对前缀真正想代替的那个不变量来做判断——即值中的每个反斜杠都是转义而非分隔符——并且在 paths.ts 中用一个导出的辅助函数同时驱动两侧，像 paths.ts:59-63 的 UNESCAPE_REGEX 那样从 SHELL_SPECIAL_CHARS 派生，这样格式化函数就不可能产出 unescapePath 拒绝处理的形态，unescapePath 也不可能吃掉格式化函数留下的分隔符。若不希望把这部分逻辑放在 core，更浅的替代方案是让 unescapePath 在 win32 保持 no-op，只在真正会看到 @ 前缀值的两处（atCommandProcessor.ts:148 与 InputPrompt.tsx:144）执行引用反转义。无论采用哪种，都应把 paths.ts:351-354 的注释收窄到它真正描述的那个生成器。
证据见上方英文部分的 Witness 代码块（针对已构建的 packages/core/dist/src/utils/paths.js 运行，并与合并基线做了 A/B 对比）。
修复不得违反本 PR 已经固定的断言：unescapePath('C:\\(v2)\\file.txt') 保持不变（packages/core/src/utils/paths.test.ts:206）、unescapePath('C:/repo\\#docs\\readme.md') 保持不变（:210-212）、'@C:/Program\\ Files/Qwen/qwen.exe' 反转义为 '@C:/Program Files/Qwen/qwen.exe'（:218）、'@//server/share/My\\ File.txt' 反转义为 '@//server/share/My File.txt'（:221-222），以及 packages/cli/src/ui/contexts/KeypressContext.test.tsx:1107-1110 中的引用字面量——因为 unescapePath 在 packages/core 有 19 处非测试调用点、在 CLI 另有 4 处，它们都传入裸路径并依赖 win32 的 no-op；同时判据必须从 SHELL_SPECIAL_CHARS（paths.ts:47）派生，该常量的注释明确写着它是「转义集合的唯一事实来源，以免各平台的反转义实现与其漂移」。
请在 packages/core/src/utils/paths.test.ts 的 win32 区块中补上 expect(unescapePath('@C:/repo\\#docs\\readme.md')).toBe('@C:/repo\\#docs\\readme.md')，并用 vi.spyOn(os, 'platform').mockReturnValue('win32') 去掉平台门禁，使其在 Linux PR 流水线上真正执行；随后移除新增判据，确认该测试变红、而既有的 @C:/Program\\ Files/... 用例仍然为绿——退回到无条件 no-op 并不是修复。
— qwen3.8-max via Qwen Code /review (v0.23.0)
Verification round 4 — head
|
| arm | source |
|---|---|
| PR | 4908b46 as-is |
| main | 4908b46 with the 6 production files reverted to merge base c39e83e |
Confirmed the swap actually happened before trusting either run: hasFormat("files") appears once in the PR bundle chunks and zero times in the main bundle.
Real TUI, Alt+V, same clipboard — PR vs main
| clipboard contents | PR arm | main arm |
|---|---|---|
C:\Users\mochi\photo.png |
attachment chip | nothing |
C:\Users\mochi\notes.txt |
@C:/Users/mochi/notes.txt in the prompt |
nothing |
| two PNGs | two attachment chips | — |
| PNG + TXT (mixed) | both stay references, no promotion | — |
\\server\share\report.docx |
@//server/share/report.docx |
— |
| empty file list | nothing — falls through to the image check | — |
And the regression that matters most, since this PR inserts itself ahead of the existing image path — a real 64×64 PNG placed on the live X11 clipboard, read back through the real binding:
Both arms produce an attachment. The new file check does not disturb image-pixels paste.
Executing the win32-only path that no CI lane runs
Test (windows-latest) is skipping on this PR, so unescapePath's new carve-out and the formatClipboardFileReference → handleAtCommand round trip get zero execution anywhere today. Two measurements close that:
packages/core/src/utils/paths.test.tsunder simulated win32: 128 passed / 18 skipped, and the twoit.skipIf(!isWindows)unescapePathcases — the ones this PR added or rewrote — both pass. On Linux they are simply skipped (144 passed / 2 skipped).- A probe that drives the real
handleAtCommandwith the reference this PR generates forC:\Users\mochi\Q3 notes.txt. Same file, same setup, two bundles:
The PR arm resolves the reference and reads the file's contents; the main arm leaves @C:/Users/mochi/Q3\ notes.txt as inert text. That is the concrete answer to "does the pasted reference actually work" — the escaped space survives only because of the core carve-out.
The native contract, measured against the real binary
The first round probed the darwin binary with an empty clipboard. This round populates a real selection on Linux. Two facts worth recording: 'files' is a validated format name (a typo throws rather than returning false), and getFiles()'s return shape is platform-dependent — Linux hands back file:// URIs, darwin throws on an empty selection, Windows gives plain paths. The process.platform !== 'win32' gate, the hasFormat guard and the try/catch are each load-bearing.
Suites, at this head
| suite | PR arm | with the 6 production files reverted |
|---|---|---|
clipboardUtils.test.ts |
36/36 | 5 fail |
KeypressContext.test.tsx |
120/120 | 3 fail |
InputPrompt.test.tsx |
232/232 | 14 fail |
SettingsDialog.test.tsx |
64/64 | 2 fail |
atCommandProcessor.test.ts |
72 (1 skipped) | unchanged |
core/paths.test.ts |
144 (2 skipped) | unchanged — 144 (2 skipped) |
523 passed / 1 skipped on the CLI side. The negative control flips 24 tests red, all inside the feature area — the tests are not vacuous. The one cell worth staring at is the last: reverting paths.ts changes nothing on Linux, because its only coverage is win32-gated.
Mutation probes at the PR head, to find out which guards the tests actually hold down:
| mutation | tests that fail |
|---|---|
formatClipboardFileReference drops the UNC alternative |
4 |
broadcastClipboardPaste checks image data before copied files |
5 |
classifyPastedImagePaths split loses the C:/ form |
1 |
readClipboardFiles loses its win32 guard |
1 |
delete the key.clipboardFiles guard in SettingsDialog.tsx |
0 |
Gates
prettier --check clean and eslint --max-warnings 0 clean on all 12 changed files; tsc --noEmit clean for packages/core and packages/cli.
The red lane is not this PR
Test (ubuntu-latest, Node 22.x) fails on one assertion in scripts/tests/ci-platform-lanes.test.js — an extra Check retired WebUI dependency step in the lint lane that the test's expected payload doesn't list. Both inputs (.github/workflows/ci.yml, the test file) are byte-identical at this head to main@077c57d2, the commit this branch merged; this PR touches no workflow file. Current origin/main passes the same test 36/36 after c516740a27 (#11095) removed that step. Merging current main into the branch clears it.
Still open, none blocking
- The
SettingsDialog.tsxguard is still dead code (raised in round 3, unchanged). Deletingif (key.clipboardFiles) return;atSettingsDialog.tsx:800leaves the file at 64/64 — including the three newClipboard Pastecases, which actually pinKeypressContext'ssequence: ''broadcast, not the guard. A reader will believe those 101 test lines protect it. Either drop the guard (theKeyfield split already achieves the isolation) or add a comment saying it is defence-in-depth. getFiles()shape — fine today because the feature is win32-gated, but a future extension to Linux/macOS (add image paste from clipboard on windows command prompt #2605 is not Windows-specific) would produce@file:///...references. Worth a comment at the win32 gate.- Out-of-workspace references are still silently skipped. Documented in the Risk & Scope section and explicitly deferred by the author; re-confirmed unchanged, no objection.
What this rig could not do
- No real Explorer copy: the CF_HDROP payload shape is scripted from the documented Windows behaviour and the author's demo, not captured from a real
explorer.exe. path.isAbsolute('C:/…')isfalseon Linux, so the harness materialises each copied file at the exact path the product's ownisAbsolute ? p : resolve(cwd, p)produces;fs.stat/fs.copyFilethen see a real file, as they would on Windows. That one step is a no-op on the real platform.- The
@-completion dropdown hangs under simulated win32 on this box. This is a rig artifact, not a PR behaviour — typing a bare@on the main bundle freezes identically. That is why the submit path is proved at thehandleAtCommandlevel above rather than by pressing Enter in the TUI. - The one failure under simulated win32 is the PR's own
it.runIf(win32)atCommandProcessorcase, and it is the same artifact: its root comes fromos.tmpdir(), which is POSIX here, soformatClipboardFileReferencenever takes the drive-letter branch and the@C:/carve-out never fires. The probe above covers the identical round trip with a Windows-shaped root and passes.
中文说明
验证第 4 轮 —— head 4908b46（仅增量）
此前几轮（1、2、3）已分别覆盖 0173db4、de3e823 与 9eacd6a 的代码。此后分支只多了一个提交（4908b46，纯测试：为输入框套件补一个默认的 readClipboardFiles mock）加一次 main 合并。所以这一轮不再重读代码，而是补上此前每一轮都指出的缺口：独立执行一次 Windows 行为，并复核尚未关闭的条目。
结论：我这边认为可以合入。 Windows 的主张现在有实测支撑，而不只是作者的演示。唯一的红 lane 来自过期的 main，与本 PR 无关。
如何在 Linux 上跑出 Windows 行为
这套环境跑的是真实构建产物、真实终端（node-pty + xterm.js）、真实的 @teddyzhu/clipboard@0.0.5 原生绑定，并在 Xvfb 下驱动真实的 X11 剪贴板。只有两处是模拟的，都如实披露：
- 在 bundle 加载之前把
process.platform改成win32，于是os.platform()、PASTE_CLIPBOARD_IMAGE绑定表（keyBindings.ts:238在 win32 上选的是Alt+V/Win+V，不是Ctrl+V）、readClipboardFiles的平台门以及unescapePath的例外分支都会走 Windows 分支。原生模块在切换之前加载，因此切换后真实的 linux 绑定仍然可用——这一点在切换后调用它验证过。 - 复制到剪贴板的文件列表被整形为资源管理器 CF_HDROP 的形状（裸
C:\Users\...路径）。这是必要的，因为 Linux 绑定不会返回这种形状——见下面的原生契约实测。
两份 bundle，除 6 个生产文件外完全一致（两边测试文件都不动）：
| arm | 来源 |
|---|---|
| PR | 4908b46 原样 |
| main | 4908b46，但 6 个生产文件回退到 merge base c39e83e |
在信任任何一次运行之前，先确认切换确实生效：hasFormat("files") 在 PR bundle 的 chunk 中出现 1 次，在 main bundle 中 0 次。
真实 TUI、Alt+V、相同剪贴板 —— PR 对 main
| 剪贴板内容 | PR arm | main arm |
|---|---|---|
C:\Users\mochi\photo.png |
出现附件条 | 无反应 |
C:\Users\mochi\notes.txt |
输入框插入 @C:/Users/mochi/notes.txt |
无反应 |
| 两个 PNG | 两个附件条 | — |
| PNG + TXT 混合 | 全部作为引用，不做附件提升 | — |
\\server\share\report.docx |
@//server/share/report.docx |
— |
| 空文件列表 | 无反应 —— 回落到图片检查 | — |
以及最关键的回归项（本 PR 把文件检查插在既有图片路径之前）：把一张真实的 64×64 PNG 放到 X11 剪贴板上，通过真实绑定读回，两个 arm 都生成附件。新增的文件检查没有干扰图片像素粘贴。
执行没有任何 CI lane 会跑的 win32 路径
本 PR 的 Test (windows-latest) 状态是 skipping，所以 unescapePath 新增的例外分支、以及 formatClipboardFileReference → handleAtCommand 的往返，今天在任何地方都是零执行。两项实测补上这个缺口：
- 在模拟 win32 下跑
packages/core/src/utils/paths.test.ts：128 通过 / 18 跳过，本 PR 新增或改写的两个it.skipIf(!isWindows)unescapePath用例都通过。在 Linux 上它们只是被跳过（144 通过 / 2 跳过）。 - 一个探针，用本 PR 为
C:\Users\mochi\Q3 notes.txt生成的引用去驱动真实的handleAtCommand。同一个文件、同一套配置、两份 bundle：PR arm 解析出引用并读到文件内容；main arm 让@C:/Users/mochi/Q3\ notes.txt停留为无效文本。这就是「粘出来的引用到底能不能用」的正面回答——被转义的空格能活下来，正是靠 core 的那处例外。
对着真实二进制实测原生契约
第一轮探的是空剪贴板下的 darwin 二进制。这一轮在 Linux 上放入了真实选区。有两点值得记录：'files' 是经过校验的格式名（写错会抛异常，而不是静默返回 false）；getFiles() 的返回形状与平台相关——Linux 返回 file:// URI，darwin 在空选区上抛异常，Windows 给裸路径。process.platform !== 'win32' 门、hasFormat 守卫和 try/catch 三者都是受力构件。
本 head 的测试套件
| 套件 | PR arm | 回退 6 个生产文件后 |
|---|---|---|
clipboardUtils.test.ts |
36/36 | 5 失败 |
KeypressContext.test.tsx |
120/120 | 3 失败 |
InputPrompt.test.tsx |
232/232 | 14 失败 |
SettingsDialog.test.tsx |
64/64 | 2 失败 |
atCommandProcessor.test.ts |
72（1 跳过） | 不变 |
core/paths.test.ts |
144（2 跳过） | 不变 —— 144（2 跳过） |
CLI 侧 523 通过 / 1 跳过。负对照让 24 个测试翻红，全部落在本特性范围内——测试不是空转。唯一值得盯的是最后一行：在 Linux 上回退 paths.ts 毫无影响，因为它的覆盖全部被 win32 门挡住了。
在 PR head 上做的突变探针，用来看测试到底压住了哪些守卫：
| 突变 | 失败测试数 |
|---|---|
formatClipboardFileReference 去掉 UNC 分支 |
4 |
broadcastClipboardPaste 把图片检查放到文件检查之前 |
5 |
classifyPastedImagePaths 的切分丢掉 C:/ 形式 |
1 |
readClipboardFiles 去掉 win32 守卫 |
1 |
删掉 SettingsDialog.tsx 里的 key.clipboardFiles 守卫 |
0 |
各项门禁
12 个改动文件上 prettier --check 与 eslint --max-warnings 0 均干净；packages/core 与 packages/cli 的 tsc --noEmit 均通过。
红 lane 不是本 PR 造成的
Test (ubuntu-latest, Node 22.x) 的失败来自 scripts/tests/ci-platform-lanes.test.js 的一条断言——lint lane 里多出一个 Check retired WebUI dependency 步骤，而测试的预期清单里没有它。两个输入文件（.github/workflows/ci.yml 和该测试文件）在本 head 上与本分支所合并的 main@077c57d2 逐字节一致；本 PR 没有改动任何 workflow 文件。当前 origin/main 在 c516740a27（#11095）移除该步骤后，同一测试 36/36 通过。把当前 main 合进分支即可消红。
仍未关闭的条目，均不阻塞
SettingsDialog.tsx的守卫仍是死代码（第 3 轮提出，未变）。删掉SettingsDialog.tsx:800的if (key.clipboardFiles) return;后该文件仍是 64/64，包括三个新增的Clipboard Paste用例——它们实际压住的是KeypressContext广播的sequence: ''，不是这个守卫。读代码的人会以为那 101 行测试保护了它。要么删掉守卫（Key字段拆分已经实现了隔离），要么加一行注释说明它是纵深防御。getFiles()的形状 —— 今天没问题，因为特性被 win32 门挡住；但如果将来扩展到 Linux/macOS（add image paste from clipboard on windows command prompt #2605 本身并非 Windows 专属），会产出@file:///...这类引用。值得在 win32 门那里加一行注释。- 工作区外的引用仍会被静默跳过。 已写入 Risk & Scope，作者明确推迟到后续 PR；本轮复核未变，我没有异议。
这套环境做不到的事
- 没有真实的资源管理器复制：CF_HDROP 的载荷形状是依据 Windows 文档行为和作者演示脚本化的，不是从真实
explorer.exe抓来的。 - 在 Linux 上
path.isAbsolute('C:/…')为false，所以夹具把每个被复制的文件落在产品自身isAbsolute ? p : resolve(cwd, p)算出的确切路径上，让fs.stat/fs.copyFile看到真实文件，与 Windows 上一致；这一步在真实平台上是空操作。 - 模拟 win32 下这台机器上的
@补全下拉会卡住。这是环境产物而非 PR 行为——在 main bundle 上只键入一个@同样会冻结。所以提交路径改用上面handleAtCommand层面的证据，而不是在 TUI 里按回车。 - 模拟 win32 下唯一的失败是本 PR 自己的
it.runIf(win32)atCommandProcessor用例，成因同上：它的根目录来自os.tmpdir()，这里是 POSIX 路径，于是formatClipboardFileReference走不到盘符分支，@C:/例外也就不会触发。上面那个用 Windows 形状根目录的探针覆盖了完全相同的往返，并且通过。
—
🤖 Generated with Claude Code — Claude Opus 5 (1M context)
|
@qwen-code /triage |
|
Sandboxed verification: The verification job did not complete (checkout, runner, or setup error) and produced no report. See the workflow run for details. 中文 — 判定：
|
|
Triage re-run completed without a new review.
The stage comments above were updated with the latest result. View workflow run. 上方各阶段评论已更新为最新结果。查看工作流运行。 |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
LGTM, looks ready to ship — CI landed green after the review. ✅
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.
9 Suggestion-level finding(s) this review confirmed are already reported on this PR and are not repeated:
- S4-1 host-gated win32 carve-out tests — already reported (comment 3687782877)
- S4-2 inert SettingsDialog clipboardFiles guard — already reported (round-3 deferral list, review 5121634474)
- S4-3 UNC clipboard path reaching fs.stat/fs.copyFile — already reported (comment 3691398129)
- S4-4 out-of-workspace clipboard references attaching nothing — already reported (PR Risk & Scope plus author comment 5290215842, maintainer accepted the deferral)
- S4-5 two contracts for handleClipboardFilePaste's pasted parameter — already reported (comment 3699335074)
- S4-6 all-images branch bypassing the large-paste placeholder cap — already reported (round-3 deferral list, review 5121634474)
- S4-7 untested !stats.isFile() promotion route — already reported (comment 3699335073)
- S4-8 image-side unavailability leg that lost its only test — already reported (round-3 deferral list, review 5121634474)
- S4-9 second @-reference formatter beside text-buffer.ts — already reported (folded into the R3-1 class finding, comment 3940953223, whose producer survey carries it)
Not reviewed: build-and-test — Test (windows-latest, Node 22.x) was skipped in CI and the win32-gated suites did not run locally either.
Not reviewed: build-and-test — Test (macos-latest, Node 22.x) was skipped in CI and the macOS clipboard-unavailability path did not run locally.
Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.
Not reviewed: test-efficacy mutation probe — the harness control produced no verdict (harnessValidated: null, it ran and died before answering); all 6 whole-file revert probes returned inconclusive with reason no-output because the probe's per-file vitest invocation tripped the globalSetup build guard, so 0 mutants and 0 hunk probes ran and there is no mutation-level evidence that the new tests would catch a regression.
Not reviewed: agent 6a — could not verify whether ANSI in tool/model output is filtered before reaching the terminal, so the terminal-escape-injection widening could be neither ruled in nor out — stopped at the agent tool budget.
Not reviewed: agent 6a — did not read useCompletion.ts closely enough to determine whether programmatically inserting an @-token via buffer.insert opens the completion popup and lets Tab/Enter accept a suggestion instead of submitting — stopped at the agent tool budget.
Deferred under the convergence posture (round 4, not a blocker) — recorded, not requested in this round:
packages/cli/src/ui/components/InputPrompt.tsx:1145 — [probe] Clipboard-file paste is not gated on shell mode
Convergence: round 4 posted 3 inline comment(s), 1 of them reported for the first time; the previous round posted 1 (1 new). The rate of new findings is not falling. Batching the remaining fixes and verifying them before the next push, or dropping this PR's reviews to --severity-floor critical, keeps the loop from re-deriving the same set. (Observation only — nothing was withheld from this review because of this observation.)
中文说明
仅完成部分审查，审查缺口已披露。
本轮确认的 9 条建议级发现已在 PR 上报告过，不再重复发布（列表见上方英文部分）。
未审查（原文为英文）：build-and-test — Test (windows-latest, Node 22.x) was skipped in CI and the win32-gated suites did not run locally either.
未审查（原文为英文）：build-and-test — Test (macos-latest, Node 22.x) was skipped in CI and the macOS clipboard-unavailability path did not run locally.
未审查（原文为英文）：build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.
未审查（原文为英文）：test-efficacy mutation probe — the harness control produced no verdict (harnessValidated: null, it ran and died before answering); all 6 whole-file revert probes returned inconclusive with reason no-output because the probe's per-file vitest invocation tripped the globalSetup build guard, so 0 mutants and 0 hunk probes ran and there is no mutation-level evidence that the new tests would catch a regression.
未审查（原文为英文）：agent 6a — could not verify whether ANSI in tool/model output is filtered before reaching the terminal, so the terminal-escape-injection widening could be neither ruled in nor out — stopped at the agent tool budget.
未审查（原文为英文）：agent 6a — did not read useCompletion.ts closely enough to determine whether programmatically inserting an @-token via buffer.insert opens the completion popup and lets Tab/Enter accept a suggestion instead of submitting — stopped at the agent tool budget.
收敛姿态下延后（第 4 轮，非阻断）——已记录，本轮不要求修改：共 1 条（原文未翻译，列表见上方英文部分）。
收敛情况：第 4 轮发布了 3 条行内评论，其中 1 条是首次提出；上一轮发布了 1 条（其中 1 条首次提出）。新发现的产出速度没有下降。把剩余修复攒成一批、验证后再推送，或将本 PR 的评审降到 --severity-floor critical，可以避免循环反复推导同一组发现。（仅为观察——本轮评审未因此扣留任何内容。）
— qwen3.8-max via Qwen Code /review (v0.23.0)
| const isForwardSlashWindowsAtReference = /^@(?:[A-Za-z]:\/|\/\/)/.test( | ||
| filePath, | ||
| ); |
There was a problem hiding this comment.
[Critical] R3-1: [certifies-falsely] [regression] The win32 carve-out in unescapePath decides "this string is one of our generated @ references" from a two-alternative prefix regex, and that proxy is wrong in both directions. This is the round-3 blocker re-asserted: it still stands at this commit, and the anchored file is untouched since it was filed.
It eats separator backslashes from mixed-separator references the merge base preserved. atCommandProcessor.ts:147-148 hands the whole token to unescapePath with the @ still attached — its own comment says "unescapePath expects the @ symbol to be present" — and Windows accepts / and \ interchangeably, so @C:/repo\#docs\readme.md is a valid reference to a real file. The prefix test matches @C:/, unescapeShellSpecials then removes the separator backslash in front of # (which is in SHELL_SPECIAL_CHARS), and the reference resolves to C:/repo#docs\readme.md, a path that exists nowhere. read_many_files gets ENOENT and atCommandProcessor.ts:473-475 skips it with only an onDebugMessage, so the prompt is submitted and answered as though the file had been attached. The sharpest part is that C:/repo\#docs\readme.md is the exact string this diff's own new test at paths.test.ts:210-212 pins as must-be-preserved; it survives only because that test omits the @ the real consumer always supplies.
The same predicate is too narrow for what the repo emits. @docs/my\ image.png — what useAtCompletion.ts:506 produces, because the crawler emits posix-relative paths — and @C:\Photos\my\ image.png — what text-buffer.ts:2095 produces — are both rejected, stay escaped on win32, and never resolve. And the doc comment this diff adds states the proxy as a general fact, "Generated @ references use forward-slash absolute Windows paths", which most in-tree generators violate, so the next maintainer reads them as handled and neither gets fixed.
This cannot be closed one path shape at a time. A whole-diff pass enumerated 6 in-tree @-reference producers, of which the predicate recognizes 1, and ran a 224-cell matrix (28 shapes x 2 producer forms x 2 @-attached consumers x 2 platforms) with 66 failing cells, 47 of them win32. It also measured the escape set and the unescape set as the identical 23 characters, so the defect is the platform gate, not set asymmetry. Discriminate on the invariant the prefix is standing in for — every backslash in the value is an escape, not a separator — and derive both sides from one exported helper here, built from SHELL_SPECIAL_CHARS the same way UNESCAPE_REGEX already is at paths.ts:59-63, so the formatter cannot emit a shape unescapePath refuses and unescapePath cannot eat a separator the formatter left. If you would rather not carry any of this in core, the narrower-depth alternative is to leave unescapePath a win32 no-op and apply the reference unescape at the only two sites that ever see an @-prefixed value, atCommandProcessor.ts:148 and InputPrompt.tsx:144. Either way, narrow the added comment to the producer whose output it actually describes.
Witness:
real unescapePath from each arm's built packages/core/dist, os.platform() stubbed to 'win32':
PR unescapePath("@C:/repo\#docs\readme.md") = "@C:/repo#docs\readme.md" <- separator backslash eaten
BASE unescapePath("@C:/repo\#docs\readme.md") = "@C:/repo\#docs\readme.md" <- preserved
PR unescapePath("@docs/my\ image.png") = "@docs/my\ image.png" <- declines (identical on BASE)
PR unescapePath("@C:\Photos\my\ image.png") = "@C:\Photos\my\ image.png" <- declines (identical on BASE)
PR unescapePath("C:/repo\#docs\readme.md") = "C:/repo\#docs\readme.md" <- the shape paths.test.ts:210 pins
base arm proof: grep isForwardSlashWindowsAtReference in the base tree -> 0 hits;
base unescapePath is `if (os.platform() === 'win32') { return filePath; }`
consumer sweep (extractAtPathCommands -> unescapePath), restricted to producers that exist at base:
BASE 13/16 fail -> PR 14/16 fail, the single new failure being the mixed-separator '#' reference
The fix must not violate what this PR already pins: unescapePath('C:\(v2)\file.txt') unchanged (packages/core/src/utils/paths.test.ts:206), unescapePath('C:/repo\#docs\readme.md') unchanged (:210-212), '@C:/Program\ Files/Qwen/qwen.exe' unescaping to '@C:/Program Files/Qwen/qwen.exe' (:218) and '@//server/share/My\ File.txt' to '@//server/share/My File.txt' (:221-222), plus the reference literals at packages/cli/src/ui/contexts/KeypressContext.test.tsx:1107-1110 — because unescapePath has ~19 non-test call sites in packages/core and 4 in the CLI that all pass bare paths and rely on the win32 no-op, and the discriminator must be derived from SHELL_SPECIAL_CHARS (paths.ts:47), whose own comment states it is "Kept as the single source of truth for the escape set so platform-specific unescapers cannot drift from it". Please add expect(unescapePath('@C:/repo\#docs\readme.md')).toBe('@C:/repo\#docs\readme.md') to the win32 block in packages/core/src/utils/paths.test.ts, de-gated with vi.spyOn(os, 'platform').mockReturnValue('win32') so it runs on the Linux PR lane, then remove the new discriminator and confirm that test reds while the existing @C:/Program\ Files/... case stays green — reverting to the unconditional no-op is not a fix.
中文说明
R3-1（第 3 轮阻断项，本轮仍然成立）：unescapePath 中新增的 win32 特例判断用一个只有两个分支的前缀正则来决定「这个字符串是我们自己生成的 @ 引用」，而这个替代判据在两个方向上都是错的。被锚定的文件自该问题提出以来未发生改动。
它会吃掉混合分隔符引用中的分隔反斜杠（合并基线是保留的）。atCommandProcessor.ts:147-148 会把整个 token 连同 @ 一起传给 unescapePath（该处注释写明「unescapePath expects the @ symbol to be present」），而 Windows 允许 / 与 \ 混用，因此 @C:/repo\#docs\readme.md 是指向真实文件的合法引用。前缀判断匹配到 @C:/，unescapeShellSpecials 随后会去掉 # 前面的分隔反斜杠（# 属于 SHELL_SPECIAL_CHARS），引用被解析为 C:/repo#docs\readme.md——一个不存在的路径。read_many_files 得到 ENOENT，atCommandProcessor.ts:473-475 仅通过 onDebugMessage 跳过它，于是提示词照常提交并被回答，就好像文件已经附加成功。最关键的一点是：C:/repo\#docs\readme.md 正是本 diff 自己在 paths.test.ts:210-212 新增测试中断言「必须保持不变」的字符串；它之所以没被破坏，只是因为该测试没有带上真实调用方总会带上的 @。
同一个判据对仓库产出的引用又太窄。@docs/my\ image.png（useAtCompletion.ts:506 的产物，因为 crawler 输出 posix 相对路径）与 @C:\Photos\my\ image.png（text-buffer.ts:2095 的产物）都不匹配，win32 下保持转义状态，永远无法解析。此外本 diff 新增的注释把这个替代判据当作普遍事实来陈述——「Generated @ references use forward-slash absolute Windows paths」——而仓库内大多数生成器并不满足，于是后来的维护者会以为它们已被覆盖，两处都不会去修。
这个问题无法靠逐个路径形态来收尾。一次全文 diff 专项审查枚举出仓库内 6 个 @ 引用生成器，而该判据只识别其中 1 个；并跑了一个 224 格矩阵（28 种形态 x 2 种生成器形式 x 2 个带 @ 的消费者 x 2 个平台），其中 66 格失败，47 格属于 win32。同时测得转义集合与反转义集合是完全相同的 23 个字符，因此缺陷在于平台判断，而不是两个集合不对称。应当针对前缀真正想代替的那个不变量来做判断——即值中的每个反斜杠都是转义而非分隔符——并在 paths.ts 中用一个导出的辅助函数同时驱动两侧，像 paths.ts:59-63 的 UNESCAPE_REGEX 一样从 SHELL_SPECIAL_CHARS 派生。如果不想把这部分放在 core，较浅的替代方案是让 unescapePath 在 win32 保持 no-op，并把引用反转义放到唯一两个会看到带 @ 值的位置：atCommandProcessor.ts:148 与 InputPrompt.tsx:144。无论哪种方式，都应把新增注释收窄到它真正描述的那个生成器。
修复不得违反本 PR 已固定的行为：unescapePath('C:\(v2)\file.txt') 保持不变（paths.test.ts:206）、unescapePath('C:/repo\#docs\readme.md') 保持不变（:210-212）、'@C:/Program\ Files/Qwen/qwen.exe' 反转义为 '@C:/Program Files/Qwen/qwen.exe'（:218）、'@//server/share/My\ File.txt' 反转义为 '@//server/share/My File.txt'（:221-222），以及 KeypressContext.test.tsx:1107-1110 的引用字面量——因为 unescapePath 在 packages/core 有约 19 个非测试调用点、在 CLI 有 4 个，它们都传入裸路径并依赖 win32 no-op；且判据必须从 SHELL_SPECIAL_CHARS（paths.ts:47）派生，该常量自己的注释写明它是「转义集合的唯一真源，以免各平台的反转义实现与之漂移」。请在 paths.test.ts 的 win32 区块中加入 expect(unescapePath('@C:/repo\#docs\readme.md')).toBe('@C:/repo\#docs\readme.md')，并用 vi.spyOn(os, 'platform').mockReturnValue('win32') 取消平台门控使其在 Linux PR 通道上运行；然后移除新的判据，确认该测试变红而既有的 @C:/Program\ Files/... 用例仍然通过——退回到无条件 no-op 不是修复。
— qwen3.8-max via Qwen Code /review (v0.23.0)
| await promotePastedImagePaths( | ||
| pastedImagePaths.imagePaths, | ||
| fileReferences, |
There was a problem hiding this comment.
[Critical] R3-2: [certifies-falsely] Nothing bounds how many copied files one keystroke promotes, and cleanupOldClipboardImages — fired unawaited by the same block that just attached them — reclaims by ascending atimeMs against a count budget, so one large Explorer selection unlinks the first files that paste created. This is the round-3 blocker re-asserted: it still stands at this commit, and it now has an inline anchor rather than living only in the review body.
On Windows, select 101 or more PNGs in Explorer and press Alt+V, or Ctrl+V where the terminal forwards an empty bracketed paste. readClipboardFiles returns the whole selection with no cap, promotePastedImagePaths copies every file into <runtimeBase>/tmp/clipboard and pushes one attachment chip per file, then the sweep with MAX_IMAGES = 100, CLEANUP_COUNT = 50 and removeCount = Math.min(CLEANUP_COUNT, imageFiles.length - MAX_IMAGES + CLEANUP_COUNT) over an ascending-atimeMs sort removes the 50 oldest — on a fresh or recently trimmed directory those are positions 0 to 49 of the batch this same paste just wrote. At submit each chip becomes @<relative temp path>, fs.realpath throws ENOENT, sawNotFound is set, and atCommandProcessor.ts:473-475 emits only onDebugMessage, so the user sees every chip, the model receives the rest, and nothing reports the loss. The same call is unawaited against a directory shared by every concurrent CLI session, so the 50 removed may instead be another live session's pending chips. Before this diff the loop was reachable only by pasting 101 or more image paths as text; one Explorer selection now triggers it with no path ever visible to the user.
The directory also grows monotonically, because the trim removes at most CLEANUP_COUNT per invocation while a single paste can add far more: a second identical paste leaves 102 files where 51 remained after the first.
Bound the promotion before copying — when pastedImagePaths.imagePaths.length exceeds a cap, promote only the first cap and route the remainder through the failedImagePaths reference branch this diff already added, so the overflow lands as references instead of copies. Better still, exempt the in-flight batch by passing the just-written destination paths to the sweep and skipping them, or sweep before writing rather than after. Do not simply raise MAX_IMAGES: the boundary moves but the mechanism does not.
Witness:
probe driving the REAL cleanupOldClipboardImages over a real temp dir populated by the
verbatim promotePastedImagePaths copy loop (clipboard-${Date.now()}-${i}${ext} + fs.copyFile):
uncapped, one 101-file paste: {"pasted":101,"remainingInDir":51,"deadCount":50,
"deadIndicesFirst10":[0..9],"deadIndicesLast5":[45..49]}
second identical paste: {"secondPasteCount":101,"remainingAfterSecondPaste":102,
"secondPasteFilesAlreadyDead":0}
PROBE_CAP=100 (the implied one-line bound): {"pasted":100,"remainingInDir":100,"deadCount":0}
The probe flips with the bound, so it distinguishes buggy from correct.
Base arm of the same probe: deadCount 50, indices 0..49 — identical, and cleanupOldClipboardImages
diffs IDENTICAL between arms, so the trim is not what this PR changed; the uncapped producer is.
The fix must not violate const MAX_IMAGES = 100; and const CLEANUP_COUNT = 50; at packages/cli/src/ui/utils/clipboardUtils.ts:639-640, consumed at :665-671 as removeCount = Math.min(CLEANUP_COUNT, imageFiles.length - MAX_IMAGES + CLEANUP_COUNT) over an ascending-atimeMs sort, so any new promotion cap must sit at or below MAX_IMAGES or the cleanup the same call triggers still unlinks this paste's own files; the interaction is measured, not hypothetical — changing the sweep's extension filter at clipboardUtils.ts:647 without this bound turns a 101-file uppercase-extension paste from zero dangling attachments into 50, so the two must not land separately. Please add a case in packages/cli/src/ui/components/InputPrompt.test.tsx that mocks readClipboardFiles with more than MAX_IMAGES image paths, drives the clipboard shortcut, and asserts both that mockFsCopyFile was called at most cap times and that the overflow came back as formatClipboardFileReference text via mockBuffer.insert, then remove the cap and confirm that test reds — the witness has to be on the new bound, because the existing suite cannot catch the deletion itself while cleanupOldClipboardImages is mocked to vi.fn() at InputPrompt.test.tsx:86.
中文说明
R3-2（第 3 轮阻断项，本轮仍然成立）：一次按键能提升多少复制文件没有任何上限，而 cleanupOldClipboardImages——由刚刚完成附加的同一个代码块以未 await 的方式触发——按 atimeMs 升序、在数量预算内回收文件，因此一次较大的资源管理器选择会删除这次粘贴自己刚创建的前一批文件。本轮它有了行内锚点，不再只存在于评审正文中。
在 Windows 上于资源管理器中选择 101 个或更多 PNG 并按 Alt+V（或在终端会转发空 bracketed paste 时按 Ctrl+V）：readClipboardFiles 不加限制地返回整个选择，promotePastedImagePaths 把每个文件复制进 <runtimeBase>/tmp/clipboard 并为每个文件压入一个附件标签，随后在 MAX_IMAGES = 100、CLEANUP_COUNT = 50、removeCount = Math.min(CLEANUP_COUNT, imageFiles.length - MAX_IMAGES + CLEANUP_COUNT) 且按 atimeMs 升序排序的清理中删除最旧的 50 个——在空目录或刚清理过的目录上，这 50 个正是本次粘贴刚写入的第 0 到第 49 个。提交时每个标签变成 @<相对临时路径>，fs.realpath 抛出 ENOENT，sawNotFound 被置位，而 atCommandProcessor.ts:473-475 只发出 onDebugMessage，于是用户看到全部标签、模型收到剩下的文件，而没有任何地方报告这次丢失。同一个调用未 await，且操作的是所有并发 CLI 会话共享的目录，因此被删除的 50 个也可能是另一个活跃会话待用的标签。在本 diff 之前，只有以文本形式粘贴 101 个以上图片路径才能进入这个循环；现在一次资源管理器选择即可触发，而且用户从未看到任何路径。
该目录还会单调增长，因为每次调用最多只清理 CLEANUP_COUNT 个，而单次粘贴可以写入远多于此：第二次相同粘贴后目录中剩 102 个文件，而第一次之后剩 51 个。
请在复制之前限制提升数量——当 pastedImagePaths.imagePaths.length 超过上限时，只提升前 cap 个，并把其余的通过本 diff 已经加入的 failedImagePaths 引用分支处理，使溢出部分以引用而非副本落地。更好的做法是豁免本批次：把刚写入的目标路径传给清理函数并跳过它们，或者先清理再写入。不要只是提高 MAX_IMAGES：边界会移动，机制不变。
修复不得违反 packages/cli/src/ui/utils/clipboardUtils.ts:639-640 的 const MAX_IMAGES = 100; 与 const CLEANUP_COUNT = 50;，它们在 :665-671 以 removeCount = Math.min(CLEANUP_COUNT, imageFiles.length - MAX_IMAGES + CLEANUP_COUNT) 且按 atimeMs 升序排序的方式被消费，因此任何新的提升上限都必须不高于 MAX_IMAGES，否则同一次调用触发的清理仍会删除本次粘贴自己的文件；这一交互是实测的而非假设的——在没有该上限的情况下修改 clipboardUtils.ts:647 的扩展名过滤，会让一次 101 个大写扩展名的粘贴从 0 个悬空附件变成 50 个，因此两者不能分开合入。请在 packages/cli/src/ui/components/InputPrompt.test.tsx 中新增一个用例：mock readClipboardFiles 返回超过 MAX_IMAGES 个图片路径，触发剪贴板快捷键，断言 mockFsCopyFile 最多被调用 cap 次，且溢出部分通过 mockBuffer.insert 以 formatClipboardFileReference 文本返回；然后移除该上限并确认该测试变红——证据必须落在新的上限上，因为在 InputPrompt.test.tsx:86 处 cleanupOldClipboardImages 被 mock 为 vi.fn()，现有测试无法观察到删除本身。
— qwen3.8-max via Qwen Code /review (v0.23.0)
| const normalized = unescapePath( | ||
| token | ||
| .replace(/^@(["'])/, '@') // strip a quote after the `@` prefix |
There was a problem hiding this comment.
[Critical] R4-1: [fails-closed] [regression] Replacing the pre-diff unconditional .replace(/\\ /g, ' ') with unescapePath makes this call site platform-divergent, and it is wrong on both platforms at once.
On Windows it under-unescapes. unescapePath returns the token untouched unless it starts with @C:/ or @//, so pasted image tokens that used to normalize no longer do: fs.stat then fails on a path that exists, the token falls into the new failedImagePaths list, and the attachment the user expected never appears. Worse, when a sibling image does promote, the new fallback runs formatClipboardFileReference over the failed token, whose replaceAll('\\', '/') converts the surviving ESCAPE backslash into a path separator — so the composer receives a reference that looks valid and points at a location that exists nowhere.
On POSIX it over-unescapes. The widening from one character (space) to all 22 of SHELL_SPECIAL_CHARS is applied to every token, including raw paths no in-tree generator ever escaped, so a POSIX filename containing a literal backslash before a shell special is corrupted before fs.stat.
This is the call-site sibling of the paths.ts carve-out blocker, and fixing that one in core does not close this one: its own fix constraint requires the win32 no-op to keep preserving backslash-separated paths, which is exactly what leaves the bare-token case here unnormalized. Decide the unescape from the evidence that the token was generated, which is still in hand at this point — keep the pre-diff .replace(/\\ /g, ' ') for tokens that do not start with @, and apply the full unescape only to @-prefixed tokens, the dialect formatClipboardFileReference and text-buffer.ts:2017/:2095 actually emit. The @ strip already happens after the unescape, so the prefix is available for the branch. Alternatively keep the raw token alongside the normalized one in classifyPastedImagePaths and prefer whichever fs.stat accepts, or retry fs.stat on the un-unescaped token in promotePastedImagePaths before pushing to failedImagePaths.
Witness:
Windows arm — the REAL classifyPastedImagePaths from each arm, os.platform() stubbed to 'win32',
same six tokens:
BASE: 6/6 CLASSIFY-PASS "@docs/my\ image.png" -> imagePaths=["docs/my image.png"]
"@C:\Photos\My\ Vacation.png" -> imagePaths=["C:\Photos\My Vacation.png"]
PR : 3/6 CLASSIFY-PASS "@docs/my\ image.png" -> imagePaths=["docs/my\ image.png"] FAIL
"@C:\Photos\My\ Vacation.png" -> imagePaths=["C:\Photos\My\ Vacation.png"] FAIL
fallback re-format -> "@C:/Photos/My/\ Vacation.png"
-> ["C:/Photos/My/ Vacation.png"] fallbackOk=false
POSIX arm — real file on disk at /tmp/probe-7957-f1/report\#3.png (literal backslash, stat: true),
hunk 3 of InputPrompt.tsx reverted with git's own patch engine (applied: true) for the BASE arm:
PR classify: {"imagePaths":["/tmp/probe-7957-f1/report#3.png"],"allImages":true}
-> stat: STAT THREW: ENOENT ; normalized === real? false
BASE classify: {"imagePaths":["/tmp/probe-7957-f1/report\#3.png"],"allImages":true}
-> stat: true ; normalized === real? true
Both probes flip on the reverted hunk, so both discriminate.
The PR's own primary flow (Explorer copy -> formatted forward-slash references) is unaffected:
those cells pass in both arms.
Two measured limits on the fix. packages/cli/src/ui/components/InputPrompt.test.tsx, 'preserves a raw Windows image path with a space-leading segment', pins that under os.platform() === 'win32' the NON-@ raw path 'C:\data\ archive\img.png' returns unchanged, so the discriminator must be @-presence and not the platform — calling unescapeShellSpecials unconditionally reds that test. And restricting the widened unescape to @-prefixed tokens on its own breaks the diff's existing "/a/my image.png" and '@"/var/tmp/screenshot.png"' cases, so the @-gate must be paired with keeping the space-only unescape for bare tokens rather than dropping normalization for them. Please add a case in describe('classifyPastedImagePaths') with vi.spyOn(os, 'platform').mockReturnValue('win32') asserting classifyPastedImagePaths('@docs/my\\ image.png') equals { imagePaths: ['docs/my image.png'], allImages: true } — it returns docs/my\ image.png today — plus a platform-independent case asserting classifyPastedImagePaths('/tmp/report\\#3.png') keeps the backslash; removing either half of the fix must red the corresponding case, and the diff-added 'unescapes a shell-escaped Unix image path exactly once' must stay green.
中文说明
R4-1（本轮新发现）：把 diff 之前无条件的 .replace(/\\ /g, ' ') 换成 unescapePath，使这个调用点变得依赖平台，而且在两个平台上同时是错的。
在 Windows 上它反转义不足。unescapePath 只有在 token 以 @C:/ 或 @// 开头时才会处理，否则原样返回，因此过去能归一化的粘贴图片 token 现在不再归一化：fs.stat 会在一个确实存在的路径上失败，token 落入新增的 failedImagePaths，用户期望的附件不会出现。更糟的是，当同批中有图片提升成功时，新的回退分支会对失败 token 调用 formatClipboardFileReference，其 replaceAll('\\', '/') 会把残留的转义反斜杠变成路径分隔符——于是输入框收到一个看起来有效、却指向不存在位置的引用。
在 POSIX 上它反转义过度。从单个字符（空格）扩大到全部 22 个 SHELL_SPECIAL_CHARS 的处理被应用到每一个 token，包括仓库内任何生成器都从未转义过的裸路径，因此文件名中在 shell 特殊字符前带有字面反斜杠的 POSIX 路径会在 fs.stat 之前被破坏。
这是 paths.ts 特例判断阻断项在调用点上的同源问题，而在 core 中修复那一个并不能关闭这一个：它自己的修复约束要求 win32 no-op 继续保留反斜杠分隔的路径，而这恰恰使这里的裸 token 情形得不到归一化。应当依据「该 token 是生成出来的」这一在此处仍然可得的证据来决定反转义方式——对不以 @ 开头的 token 保留 diff 之前的 .replace(/\\ /g, ' ')，只对带 @ 前缀的 token 应用完整反转义，也就是 formatClipboardFileReference 与 text-buffer.ts:2017/:2095 实际产出的那种形式。@ 的剥离本来就发生在反转义之后，因此分支判断可以拿到该前缀。或者在 classifyPastedImagePaths 中同时保留原始 token 与归一化 token，优先使用 fs.stat 能接受的那个；也可以在 promotePastedImagePaths 中于压入 failedImagePaths 之前，对未反转义的 token 重试一次 fs.stat。
修复有两个实测限制。packages/cli/src/ui/components/InputPrompt.test.tsx 的 'preserves a raw Windows image path with a space-leading segment' 固定在 os.platform() === 'win32' 下不带 @ 的裸路径 'C:\data\ archive\img.png' 原样返回，因此判据必须是「是否带 @」而不是平台——无条件调用 unescapeShellSpecials 会使该测试变红。而单独把扩大的反转义限制在带 @ 的 token 上，会破坏本 diff 既有的 "/a/my image.png" 与 '@"/var/tmp/screenshot.png"' 用例，因此 @ 门控必须与「对裸 token 保留仅空格的反转义」配对，而不是直接放弃对它们的归一化。请在 describe('classifyPastedImagePaths') 中新增一个使用 vi.spyOn(os, 'platform').mockReturnValue('win32') 的用例，断言 classifyPastedImagePaths('@docs/my\\ image.png') 等于 { imagePaths: ['docs/my image.png'], allImages: true }（当前返回 docs/my\ image.png），另加一个与平台无关的用例断言 classifyPastedImagePaths('/tmp/report\\#3.png') 保留反斜杠；移除修复的任一部分都应使对应用例变红，且本 diff 新增的 'unescapes a shell-escaped Unix image path exactly once' 必须保持通过。
— qwen3.8-max via Qwen Code /review (v0.23.0)
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.
3 Suggestion-level finding(s) this review confirmed are already reported on this PR and are not repeated:
- packages/cli/src/ui/components/SettingsDialog.tsx:803 inert clipboardFiles guard — already reported (round-3 deferral list, review 5121634474; round-4 S4-2)
- packages/cli/src/ui/contexts/KeypressContext.test.tsx:1064 image-side unavailability leg lost its only test — already reported (round-3 deferral list, review 5121634474; round-4 S4-8)
- packages/cli/src/ui/components/InputPrompt.tsx:905 multi-file failedImagePaths test gap — already reported (comment 3699335073; round-4 S4-7)
Not reviewed: build-and-test — Test (windows-latest, Node 22.x) never reported at this commit and the win32-gated suites could not run locally, so the Windows-only decode branch and the four it.runIf cases are unexecuted.
Not reviewed: build-and-test — Test (macos-latest, Node 22.x) never reported at this commit and the macOS clipboard-unavailability path could not run locally.
Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) never reported at this commit and its suite did not run locally.
Not reviewed: test-efficacy mutation probe — the harness control produced no verdict (harnessValidated: null, it ran and died before answering) and every whole-file revert probe returned inconclusive with reason no-output, so 0 mutants and 0 hunk probes ran and there is no mutation-level evidence.
Not explored to full depth (tool budget reached): "agent reverse-audit (round 2)": none — I did not walk the InputPrompt.test.tsx promotion tests at diff lines ~1740-2030 or the SettingsDialog / KeypressContext / atCommandProcessor / clipbo….
Not reviewed: reverse audit — did not converge within the reverse-audit round cap of 5.
7 Suggestion(s) were drafted inline past the resolved critical posting floor — the floor engaged early: the first-time-finding rate has not fallen for 2 consecutive round(s); the CLI moved them into the deferral list below (floor enforcement).
Deferred under the convergence posture (round 5, not a blocker) — the floor engaged early: the first-time-finding rate has not fallen for 2 consecutive round(s) — recorded, not requested in this round:
packages/cli/src/ui/components/InputPrompt.test.tsx:74 — [review] R5-5: This paste-directory mock is a plain async function rather than a spy, and every copyFile assertion in the five new promotion tests matches only the tail /[\\/]paste…packages/cli/src/ui/components/InputPrompt.test.tsx:6539 — [review] R5-3: Both rows of this new preserves the path dialect on %s block produce the same expectation with or without the os.platform() ternary in classifyPastedImagePaths ,…packages/cli/src/ui/components/InputPrompt.tsx:135 — [review] R5-7: This new pastedImageTokens helper was inserted between classifyPastedImagePaths ' JSDoc block and its export function line, so the doc block now documents the wrong fu…packages/cli/src/ui/components/InputPrompt.tsx:905 — [review] R5-8: This new partial-failure insert appends the failed references at the cursor with no boundary character. buffer.insert leaves the cursor immediately after what it inserted…packages/cli/src/ui/hooks/atCommandProcessor.ts:415 — [review] R5-6: This new win32 branch adopts the decoded spelling whenever the raw spelling is merely *absent*. But for a genuine Windows path a \ before a shell-special character is a …packages/cli/src/ui/utils/clipboard-paste-directory.ts:17 — [review] R5-9: This memo returns the resolved directory path forever with no revalidation against the filesystem, so once anything other than this process's own exit cleanup remove…packages/cli/src/ui/utils/clipboard-paste-directory.ts:21 — [review] R5-4: No case in the new clipboard-paste-directory.test.ts makes initialization fail, so this memo-eviction-on-rejection path is untested — and in production a single re…packages/cli/src/ui/components/InputPrompt.test.tsx:1846 — [probe] No promotion test asserts that a promoted image becomes an attachment, so setAttachments is unpinnedpackages/cli/src/ui/contexts/KeypressContext.tsx:804 — [probe] The empty-array guard on Key.clipboardFiles is unpinned and both consumers test it by truthiness
Convergence: round 5 posted 3 inline comment(s), 3 of them reported for the first time; the previous round posted 3 (1 new). Findings keep coming back to the same files: packages/cli/src/ui/components/InputPrompt.tsx (findings in rounds 3, 4; 1 more now). A cluster that keeps producing siblings usually means the fixes are treating instances of a shared root cause — triaging that cause before the next round, or splitting an independent cluster into its own pull request, tends to end the loop faster than fixing them one at a time. (Observation only — nothing was withheld from this review because of this observation.)
Mechanism health: this round did not close cleanly, so it withholds the incremental anchor — and the round it recovered had no anchor this round could use either — none at all, one with no certifier, one certified by an identity other than the one this round runs under, or one this round's fetch refused or resolved to the head — so the next review re-reads the whole diff unless recovery grafts an earlier own anchor that the round running it can use onto the complete work list this round leaves behind, and keeps doing so until a round's marker carries an anchor again or a graft lands that the round running it can use. (Stated, not acted on — this changes nothing about what the round posts.)
中文说明
仅完成部分审查，审查缺口已披露。
本轮确认的 3 条建议级发现已在 PR 上报告过，不再重复发布（列表见上方英文部分）。
未审查（原文为英文）：build-and-test — Test (windows-latest, Node 22.x) never reported at this commit and the win32-gated suites could not run locally, so the Windows-only decode branch and the four it.runIf cases are unexecuted.
未审查（原文为英文）：build-and-test — Test (macos-latest, Node 22.x) never reported at this commit and the macOS clipboard-unavailability path could not run locally.
未审查（原文为英文）：build-and-test — Integration Tests (CLI, No Sandbox) never reported at this commit and its suite did not run locally.
未审查（原文为英文）：test-efficacy mutation probe — the harness control produced no verdict (harnessValidated: null, it ran and died before answering) and every whole-file revert probe returned inconclusive with reason no-output, so 0 mutants and 0 hunk probes ran and there is no mutation-level evidence.
未探索到全部深度（达到工具调用预算）："agent reverse-audit (round 2)"：none — I did not walk the InputPrompt.test.tsx promotion tests at diff lines ~1740-2030 or the SettingsDialog / KeypressContext / atCommandProcessor / clipbo…。
未审查：反向审计——在 5 轮的反审轮数上限内未收敛。
7 条 Suggestion 在已解析的 critical 发布下限之外被起草为行内评论——发布下限因首次发现速率连续 2 轮未下降而提前生效；CLI 已将其移入下方延后清单（下限强制执行）。
收敛姿态下延后（第 5 轮，非阻断）——发布下限因首次发现速率连续 2 轮未下降而提前生效——已记录，本轮不要求修改：共 9 条（原文未翻译，列表见上方英文部分）。
收敛情况：第 5 轮发布了 3 条行内评论，其中 3 条是首次提出；上一轮发布了 3 条（其中 1 条首次提出）。发现反复回到同一批文件：packages/cli/src/ui/components/InputPrompt.tsx（第 3、4 轮已出过发现，本轮又有 1 条）。一个不断再生兄弟发现的簇，通常意味着逐条修复只在处理同一根因的实例——先定位并处理该根因，或把独立的簇拆成单独的 PR，通常比逐条修复更快结束循环。（仅为观察——本轮评审未因此扣留任何内容。）
机制健康：本轮未能干净收尾，因而扣留了增量锚点，而它恢复到的那一轮也没有留下本轮可用的锚点——要么完全没有、要么没有认证者、要么由本轮运行身份之外的身份认证、要么被本轮的获取拒绝或解析为头提交——因此下一次评审将重读整个 diff，除非恢复流程把本轮能使用的更早自有锚点嫁接到本轮留下的完整工作清单上；并会一直如此，直到某一轮的标记重新带上锚点，或落地的嫁接能被运行该轮的评审使用。（仅陈述，不据此行动——这不改变本轮发布的任何内容。）
— qwen3.8-max via Qwen Code /review (v0.23.0)
| formatClipboardFileReference, | ||
| } from '../utils/clipboardUtils.js'; | ||
| import * as path from 'node:path'; | ||
| import * as os from 'node:os'; |
There was a problem hiding this comment.
[Critical] R5-1: [certifies-falsely] [regression] This adds import * as os from 'node:os' while the new test at InputPrompt.test.tsx:20 adds import os from 'node:os'. Those are two different objects, so vi.spyOn(os, 'platform').mockReturnValue('win32') patches the CJS default-export object and the ESM namespace binding that classifyPastedImagePaths reads at line 154 never observes it. The new test preserves a raw Windows image path with a space-leading segment therefore fails on every non-Windows runner: it expects imagePaths: ['C:\data\ archive\img.png'] and receives ['C:\data archive\img.png'], which is the non-win32 token.replace(/\\ /g, ' ') arm running where the win32 arm should have preserved the backslash. The test carries no runIf/skipIf, so nothing gates it off Linux, and the Ubuntu lane is the only one that runs vitest for a pull request. Past the red gate, the win32 arm this diff exists to add has no test that can exercise it on Linux — the sibling platform-mocked cases are all platform-insensitive by construction.
Witness:
build-test at this commit: build 19/19 workspaces exit 0; npm test --workspace=packages/cli exit 1
test-delta vs built merge base 9b188ea38: netNew = src/ui/components/InputPrompt.test.tsx
(base fails only src/config/settings.test.ts in this workspace)
isolated: vitest run src/ui/components/InputPrompt.test.tsx -t "preserves a raw Windows image path" -> 1 failed
- Expected "C:\\data\\ archive\\img.png"
+ Received "C:\\data archive\\img.png"
binding, measured: ns === default is false; the namespace's `platform` descriptor is
configurable: false (cannot be spied), the default export's is configurable: true
probe: PROBE-OS defaultImport=win32 namespaceImport=linux
| import * as os from 'node:os'; | |
| import os from 'node:os'; |
Do not fix this by spying the namespace object instead — its platform property is configurable: false, so vi.spyOn cannot redefine it (measured: TypeError: Cannot redefine property: platform). The constraint the fix must respect is the repo's own working precedent, which pairs a default import on both sides: packages/core/src/utils/paths.ts:9 and packages/core/src/utils/paths.test.ts:8 are both import os from 'node:os';, which is why the identical vi.spyOn(os, 'platform') in this PR's own new core test does reach unescapePath and passes; the three spy sites in the test file (lines 6544, 6565, 6579) must all keep working.
Please confirm the witness on the fix: classifyPastedImagePaths > preserves a raw Windows image path with a space-leading segment must go green on Linux, and must go red again if you delete the win32 arm (? token) at lines 152-156 — that mutation is the proof the branch is now genuinely pinned.
中文说明
[Critical] R5-1：[certifies-falsely] [regression] 这里新增了 import * as os from 'node:os'，而 InputPrompt.test.tsx:20 新增的测试用的是 import os from 'node:os'。这是两个不同的对象，因此 vi.spyOn(os, 'platform').mockReturnValue('win32') 打的是 CJS 默认导出对象，而 classifyPastedImagePaths 在第 154 行读取的 ESM 命名空间绑定完全感知不到它。于是新增测试 preserves a raw Windows image path with a space-leading segment 在所有非 Windows runner 上都会失败：它期望 imagePaths: ['C:\data\ archive\img.png']，实际收到 ['C:\data archive\img.png']，也就是本该由 win32 分支保留反斜杠的地方走了非 win32 的 token.replace(/\\ /g, ' ')。该测试没有 runIf/skipIf，因此在 Linux 上不会被跳过，而 Ubuntu 通道是唯一会为 pull request 运行 vitest 的通道。除了 CI 变红之外，本 diff 想要新增的 win32 分支在 Linux 上没有任何测试能够执行到——同批其他带平台 mock 的用例在构造上就与平台无关。
证据（Witness）见上方英文部分的代码块：本次提交上 19/19 个 workspace 构建退出码 0，npm test --workspace=packages/cli 退出码 1；与已构建的合并基线 9b188ea 做 test-delta 对比，净新增失败文件是 src/ui/components/InputPrompt.test.tsx（基线在该 workspace 只失败 src/config/settings.test.ts）；单独运行该用例 1 failed；直接测量绑定关系得到命名空间与默认导出不是同一对象，且命名空间的 platform 属性 configurable: false（无法被 spy），默认导出的为 configurable: true。
| import * as os from 'node:os'; | |
| import os from 'node:os'; |
请不要改为去 spy 命名空间对象——它的 platform 属性是 configurable: false，vi.spyOn 无法重定义（实测报错 TypeError: Cannot redefine property: platform）。修复必须遵守的约束是仓库自身已有的可行先例，即两侧都使用默认导入：packages/core/src/utils/paths.ts:9 与 packages/core/src/utils/paths.test.ts:8 都是 import os from 'node:os';，这也是本 PR 自己在 core 中新增的测试里同样的 vi.spyOn(os, 'platform') 能够作用到 unescapePath 并通过的原因；测试文件中三处 spy（第 6544、6565、6579 行）都必须继续有效。
请对修复本身验证证据：classifyPastedImagePaths > preserves a raw Windows image path with a space-leading segment 必须在 Linux 上变绿，并且在删除第 152-156 行的 win32 分支（? token）后必须重新变红——该变异才能证明这个分支真的被固定住了。
— qwen3.8-max via Qwen Code /review (v0.23.0)
| // Only reap directories whose owning process is gone. Active attachments may | ||
| // be queued for a later turn, even after their composer has been cleared. | ||
| for (const entry of await fs.readdir(root, { withFileTypes: true })) { | ||
| const owner = /^paste-(\d+)-[a-zA-Z0-9]{6}$/.exec(entry.name); |
There was a problem hiding this comment.
[Critical] R3-2: (fix-induced) [certifies-falsely] [new-surface] The round-4 blocker's reported mechanism is fixed by this commit — the unawaited cleanupOldClipboardImages sweep is gone from the promotion path, and copies now land in per-process paste-<pid>-<rand> subdirectories that neither flat pruner can enumerate (your own 101-file regression test passes). But the fix opened a new defect at the same site: this reaper decides directory ownership from a bare PID number with no namespace, boot or start-time identity, and that token is wrong in both directions.
Deletion direction: a PID that does not resolve here reads as "owner dead", so a session sharing ~/.qwen across a PID namespace or machine boundary deletes a LIVE sibling session's staged paste images. A host session (pid 4242) pastes an Explorer screenshot, and that copy is the only backing store for its attachment chip — InputPrompt.tsx:893-897 stores {id, path, filename}, no bytes, read back lazily at submit. A devcontainer session with the host home mounted, or a second machine on an NFS home, pastes for the first time: createDirectory enumerates the shared root, process.kill(4242, 0) runs in its own namespace and throws ESRCH, and the fs.rm deletes the host's directory. The host still shows the chip, submits, and the turn proceeds with the image silently absent — atCommandProcessor.ts:497-501 emits only an onDebugMessage, and the original clipboard content is already gone. Container PID spaces are small and low-numbered, so the collision is likely rather than theoretical, and the sweep runs on the first paste of every session.
Accumulation direction: a recycled PID reads as "owner alive", so after a kill -9 or a reboot an orphaned directory is kept forever, and nothing else in the repo can see it.
Witness:
real getClipboardPasteDirectory from this commit, real kernel namespace split (unshare --user --pid --fork),
host owner process ALIVE in both arms (host: kill -0 869904 -> OK; inside ns: No such process;
namespaces pid:[4026531836] vs pid:[4026533376]):
ARM A (reaper in the SAME namespace) stagedImageAfter: "HOST SESSION STAGED IMAGE BYTES"
entries: [paste-869904-a1b2c3, paste-869943-N4WAb2]
ARM B (reaper in a DIFFERENT namespace) ownerFromThisNamespace: "throws code=ESRCH"
stagedImageAfter: "READ FAIL ENOENT"
entries: [paste-12-A7uZXI]
accumulation arm: dead-PID orphan reaped 1/1; live/recycled-PID orphan kept 3/3, contents intact,
across three simulated later runs
no other sweeper can see paste-*: clipboardUtils.ts:646 and imageHandler.ts:52 do a non-recursive
readdir filtered on startsWith('clipboard-'); toolResultCleanup.ts and the
housekeeping sweeper never descend into it
Give the ownership token an identity that survives PID reuse and namespace boundaries, and never sweep a foreign tag. Core already exports the helpers: packages/core/src/index.ts:703 publishes isPidAlive, readLocalBootId, readPidNamespaceId and isSameProcess from process-liveness.ts, whose own header states the rule this reaper breaks — "A bare PID is not enough on its own: PIDs are recycled…". Record the owner's pidNs + bootId + process-start time beside the PID (the paste-<pid>-<6 rand> name has no room, so use a marker file inside the directory or a wider name); skip any entry whose namespace or boot identity is not this process's, and require the start token to match before reaping. That one change closes both directions. Do not use a bare mtime cutoff — it can reap a directory whose attachments are still queued in a genuinely long-lived session, which is exactly what your own comment at lines 29-30 warns against.
The constraint is the precedent already in this repo: packages/core/src/services/session-registry.ts:536-542 says "A record from another PID namespace describes PIDs that do not resolve in ours: kill(pid, 0) reports ESRCH for a process that is alive over there … Neither listing nor sweeping is safe — leave it to a reader on the writer's own side", followed by if (record.pidNs !== ownNamespace) return; and a boot-id guard at :544-559 whose comment names the NFS/AFS shared-home case. Note also that readPidNamespaceId() returns null on every non-Linux platform (process-liveness.ts:183), so an identity field must degrade to "cannot compare → do not sweep" on Windows and macOS — never to "absent means local", which is the shipping path for this Windows-only feature. The reaper regex at line 32, the fs.mkdtemp prefix at line 44 and the fixtures at clipboard-paste-directory.test.ts:57-59 all read the name format and must change in the same commit.
Please confirm the witness on the fix, in clipboard-paste-directory.test.ts beside reaps only recognized directories whose owner is gone: (1) a directory carrying a FOREIGN namespace/boot identity under a PID that throws ESRCH must survive getClipboardPasteDirectory(root) while the local dead one is removed — deleting the identity gate must red it; and (2) a directory whose PID is alive but whose recorded start time does not match must survive — deleting the start-time comparison must red it. Today's suite cannot catch either, because it mocks process.kill to throw ESRCH for the PID regardless of identity, so it pins only the sweep's action and never its premise.
中文说明
[Critical] R3-2：（由修复引入）[certifies-falsely] [new-surface] 第 4 轮阻断项所报告的机制已被本次提交修好——提升路径里那个未 await 的 cleanupOldClipboardImages 清理调用已经移除，副本现在落在按进程隔离的 paste-<pid>-<rand> 子目录中，两个扁平清理器都无法枚举到它（你们自己新增的 101 文件回归测试是通过的）。但这次修复在同一位置打开了一个新缺陷：这个回收器仅凭一个裸 PID 数字判定目录归属，没有命名空间、boot 或进程启动时间等身份信息，而这个判据在两个方向上都是错的。
删除方向：一个在此处无法解析的 PID 会被当作「属主已死」，因此一个跨 PID 命名空间或跨机器共享 ~/.qwen 的会话，会删除另一个仍然存活的兄弟会话已暂存的粘贴图片。宿主会话（pid 4242）粘贴了一张资源管理器截图，而该副本是其附件标签唯一的存储——InputPrompt.tsx:893-897 只保存 {id, path, filename}，不保存字节，提交时才惰性读回。一个挂载了宿主 home 的 devcontainer 会话，或 NFS home 上的另一台机器，第一次粘贴时：createDirectory 枚举共享根目录，process.kill(4242, 0) 在它自己的命名空间里抛出 ESRCH，随后 fs.rm 删掉了宿主的目录。宿主端标签仍在，提交后该轮对话照常进行，而图片已无声缺失——atCommandProcessor.ts:497-501 只发出一个 onDebugMessage，且原始剪贴板内容已经不可恢复。容器 PID 空间小且编号靠前，因此这种碰撞是大概率而非理论上的，而且该清理会在每个会话的第一次粘贴时运行。
累积方向：被回收复用的 PID 会被当作「属主仍活着」，因此在 kill -9 或重启之后，遗留目录会被永久保留，而仓库中没有任何其他清理器能看到它。
证据（Witness）见上方英文部分代码块：使用本提交的真实 getClipboardPasteDirectory，配合真实内核命名空间隔离（unshare --user --pid --fork），两个分支中宿主属主进程都存活（宿主侧 kill -0 869904 成功；命名空间内报 No such process；命名空间分别为 pid:[4026531836] 与 pid:[4026533376]）。A 分支（同命名空间）暂存图片内容仍在；B 分支（不同命名空间）ownerFromThisNamespace 抛 ESRCH，暂存图片读取变为 READ FAIL ENOENT。累积分支：PID 真死的遗留目录 1/1 被回收，PID 存活/被复用的遗留目录在三次模拟后续运行中 3/3 被保留且内容完好。另已逐个读过其他清理器确认都看不到 paste-*：clipboardUtils.ts:646 与 imageHandler.ts:56 是非递归 readdir 且按 startsWith('clipboard-') 过滤，toolResultCleanup.ts 与 housekeeping 清理器都不会进入该目录。
请为归属判据加上能在 PID 复用与命名空间边界下仍然成立的身份信息，并且绝不清理带外来标记的目录。core 已经导出了所需的辅助函数：packages/core/src/index.ts:703 通过 process-liveness.ts 公开了 isPidAlive、readLocalBootId、readPidNamespaceId 与 isSameProcess，而该模块自己的文件头正写着本回收器违反的规则——「A bare PID is not enough on its own: PIDs are recycled…」。请把属主的 pidNs + bootId + 进程启动时间与 PID 一起记录下来（paste-<pid>-<6 rand> 这个名字没有空间，可用目录内的标记文件或更宽的名字）；对命名空间或 boot 身份不属于本进程的条目一律跳过，并且在回收前要求启动时间标记匹配。这一处改动即可同时关闭两个方向。不要使用单纯的 mtime 时间阈值——它可能回收一个附件仍在排队、会话确实长期存活的目录，而这正是你们自己在第 29-30 行注释中警告的情况。
约束来自仓库中已有的先例：packages/core/src/services/session-registry.ts:536-542 写着「A record from another PID namespace describes PIDs that do not resolve in ours: kill(pid, 0) reports ESRCH for a process that is alive over there … Neither listing nor sweeping is safe — leave it to a reader on the writer's own side」，随后是 if (record.pidNs !== ownNamespace) return;，以及 :544-559 处 boot-id 保护，其注释点名了 NFS/AFS 共享 home 的场景。另请注意 readPidNamespaceId() 在所有非 Linux 平台返回 null（process-liveness.ts:183），因此身份字段在 Windows 与 macOS 上必须退化为「无法比较 → 不清理」，绝不能退化为「缺失即本地」——而这个仅限 Windows 的功能，其上线路径正是 Windows。第 32 行的回收正则、第 44 行的 fs.mkdtemp 前缀，以及 clipboard-paste-directory.test.ts:57-59 的测试夹具都读取该名字格式，必须在同一次提交中一起改。
请对修复本身验证证据，在 clipboard-paste-directory.test.ts 中 reaps only recognized directories whose owner is gone 旁边补两个用例：（1）一个带外来命名空间/boot 身份、但 PID 会抛 ESRCH 的目录，必须在 getClipboardPasteDirectory(root) 之后仍然存在，而本地已死的那个被删除——移除身份门控后该用例必须变红；（2）一个 PID 存活但记录的启动时间不匹配的目录必须仍然存在——移除启动时间比较后该用例必须变红。现有测试无法发现这两点，因为它把 process.kill mock 成对该 PID 一律抛 ESRCH，因此只固定了清理动作，从未固定其前提。
— qwen3.8-max via Qwen Code /review (v0.23.0)
| const normalizedPath = /^(?:[A-Za-z]:\\|\\\\)/.test(filePath) | ||
| ? filePath.replaceAll('\\', '/') | ||
| : filePath; | ||
| return `@${escapePath(normalizedPath)}`; |
There was a problem hiding this comment.
[Critical] R5-2: [certifies-falsely] [new-surface] The escaping contract this new producer establishes is incomplete. escapePath's SHELL_SPECIAL_CHARS (packages/core/src/utils/paths.ts:47) escapes only ASCII space and tab among whitespace, while the at-command tokenizer that consumes these references (atCommandProcessor.ts:134, /[,\s;!?()[\]{}]/) breaks the token at every JS \s character — U+3000, U+00A0, U+2000-200A, U+202F, U+FEFF, all of them legal in NTFS filenames.
So a Windows user who copies C:\docs\報告書 最終版.pdf (U+3000 IDEOGRAPHIC SPACE, plausible in CJK filenames) in Explorer and presses the paste shortcut gets @C:/docs/報告書 最終版.pdf with the U+3000 bare. On submit parseAllAtCommands terminates the token there: the at-path becomes C:/docs/報告書 and 最終版.pdf leaks into the query as literal text. If a sibling named 報告書 exists in that directory, its contents are attached and read into the model context — the wrong file, with shouldProceed: true. If it does not exist, the only signal is an onDebugMessage, invisible by default, so the paste is silently not attached.
Witness:
probe against the intact PR:
SHELL_SPECIAL_CHARS.source = "[ \t()[\]{};|*?$`'\"#&<>!~,]"
escapePath(U+3000) = "a b" <- NOT escaped
escapePath(NBSP) = "a b" <- NOT escaped
escapePath(ASCII space) = "a\ b" <- CONTROL: escaped
reference = "@C:/docs/報告書 最終版.pdf", codepoints carry 0x3000 bare
TOKENS(U+3000) = ["C:/docs/報告書"] <- TRUNCATED
TOKENS(NBSP) = ["C:/docs/a"]
TOKENS(U+2009) = ["C:/docs/a"]
TOKENS(ASCII space) = ["C:/docs/report final.pdf"] <- CONTROL: intact
producer-only patch arm (escape the extra whitespace in formatClipboardFileReference):
TOKENS(U+3000) = ["C:/docs/報告書\ 最終版.pdf"] <- no longer truncated, but a literal
0x5c survives, because UNESCAPE_REGEX (derived from SHELL_SPECIAL_CHARS.source.slice(1,-1)
at paths.ts:59-62) has no \<U+3000> alternative; on win32 that backslash is a separator,
so it resolves to .../報告書/ 最終版.pdf
Align the three sets at their source of truth rather than patching this producer — the producer-only arm above was measured and does not work. Either widen SHELL_SPECIAL_CHARS in packages/core/src/utils/paths.ts:47 from [ \t...] to [\s...] (UNESCAPE_REGEX then follows automatically, since it is composed from SHELL_SPECIAL_CHARS.source), or derive the tokenizer's break regex in atCommandProcessor.ts from the same constant so the sets cannot drift.
Two constraints. UNESCAPE_REGEX is composed via SHELL_SPECIAL_CHARS.source.slice(1, -1) (paths.ts:60-63), so any change to the class must keep that slice-based derivation a valid character class; and the win32 existence probe at atCommandProcessor.ts:392-415 relies on decoding being the exact inverse of escaping. Worth knowing for scoping: the mismatch itself is not new — escapePath already feeds @-references from useAtCompletion.ts:506 and text-buffer.ts:2017/:2095, so a Linux user completing such a filename hits the same truncation today. What this diff adds is a one-keystroke producer that emits these references automatically from paths the user never typed or saw, which is what makes it newly reachable. This was recorded as a round-3 deferral at Suggestion severity; it is confirmed Critical this round on the measured wrong-file-attachment scenario.
Please confirm the witness on the fix with a non-platform-gated round-trip test: in atCommandProcessor.test.ts, extractAtPathCommands(formatClipboardFileReference('C:\docs\a\u3000b.txt')) must return the single full token (it returns ['C:/docs/a'] today), and in packages/core/src/utils/paths.test.ts, escapePath('a\u3000b') === 'a\\\u3000b' with unescapeShellSpecials inverting it (also red today). Removing either half of the set alignment must red the corresponding case.
中文说明
[Critical] R5-2：[certifies-falsely] [new-surface] 这个新的生成方所建立的转义约定是不完整的。escapePath 使用的 SHELL_SPECIAL_CHARS（packages/core/src/utils/paths.ts:47）在空白字符中只转义 ASCII 空格与制表符，而消费这些引用的 at-command 分词器（atCommandProcessor.ts:134，/[,\s;!?()[\]{}]/）会在每一个 JS \s 字符处截断 token——U+3000、U+00A0、U+2000-200A、U+202F、U+FEFF，而这些在 NTFS 文件名中都是合法的。
因此 Windows 用户在资源管理器中复制 C:\docs\報告書 最終版.pdf（U+3000 表意空格，在 CJK 文件名中很常见）并按下粘贴快捷键时，得到的是 @C:/docs/報告書 最終版.pdf，其中 U+3000 是裸的。提交时 parseAllAtCommands 会在该处截断 token：at-path 变成 C:/docs/報告書，而 最終版.pdf 作为字面文本泄漏进查询。如果该目录下正好存在名为 報告書 的同级文件，它的内容会被附加并读入模型上下文——即错误的文件，且 shouldProceed: true；如果不存在，唯一的信号是一个默认不可见的 onDebugMessage，于是这次粘贴无声地没有附加任何内容。
证据（Witness）见上方英文部分代码块：针对未修改的 PR 运行探针，escapePath 对 U+3000 与 NBSP 都不转义，而对 ASCII 空格转义（对照组）；分词结果 U+3000、NBSP、U+2009 三种都被截断，ASCII 空格对照组完整。只改生成方的分支也已实测：token 不再被截断，但残留一个字面 0x5c，因为 UNESCAPE_REGEX（由 paths.ts:59-62 的 SHELL_SPECIAL_CHARS.source.slice(1,-1) 派生）没有 \<U+3000> 这一分支，而在 win32 上该反斜杠是分隔符，于是解析成 .../報告書/ 最終版.pdf。
请在唯一真源处让这三个集合对齐，而不要修补这个生成方——上面「只改生成方」的分支已经实测过，行不通。可以把 packages/core/src/utils/paths.ts:47 的 SHELL_SPECIAL_CHARS 从 [ \t...] 扩为 [\s...]（UNESCAPE_REGEX 会自动跟随，因为它由 SHELL_SPECIAL_CHARS.source 组合而成），或者让 atCommandProcessor.ts 的截断正则从同一常量派生，使两个集合无法漂移。
两个约束：UNESCAPE_REGEX 是通过 SHELL_SPECIAL_CHARS.source.slice(1, -1) 组合的（paths.ts:60-63），因此对该字符类的任何修改都必须保证这种基于 slice 的派生仍是合法字符类；同时 atCommandProcessor.ts:392-415 的 win32 存在性探测依赖「解码是转义的严格逆运算」。关于范围界定还有一点值得知道：这个不一致本身并不是新的——escapePath 已经在为 useAtCompletion.ts:506 与 text-buffer.ts:2017/:2095 生成 @ 引用，所以 Linux 用户今天补全这类文件名时会遇到同样的截断。本 diff 新增的是一个「一键」生成方，它会自动从用户从未输入也从未看见的路径产出这类引用，这才是它变得新近可达的原因。该问题在第 3 轮曾以 Suggestion 级别被记录进延后清单；本轮基于实测的「附加错误文件」场景确认为 Critical。
请用一个不受平台门控的往返测试来验证修复：在 atCommandProcessor.test.ts 中，extractAtPathCommands(formatClipboardFileReference('C:\docs\a\u3000b.txt')) 必须返回单个完整 token（当前返回 ['C:/docs/a']）；在 packages/core/src/utils/paths.test.ts 中，escapePath('a\u3000b') === 'a\\\u3000b' 且 unescapeShellSpecials 能将其还原（当前同样为红）。移除集合对齐的任一半，都必须使对应用例变红。
— qwen3.8-max via Qwen Code /review (v0.23.0)
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.
5 Suggestion-level finding(s) this review confirmed are already reported on this PR and are not repeated:
- packages/cli/src/ui/components/InputPrompt.test.tsx:74 — paste-directory root not pinned by the promotion tests — already reported (round-5 deferral R5-5, review 5125833845)
- packages/cli/src/ui/components/InputPrompt.test.tsx:1847 — promoted-image attachment not asserted — already reported (round-5 deferral list, review 5125833845)
- packages/cli/src/ui/components/InputPrompt.tsx:930 — clipboard-file insert with no boundary character — already reported (round-5 deferral R5-8, review 5125833845)
- packages/cli/src/ui/components/SettingsDialog.tsx:804 — inert clipboardFiles guard — already reported (round-3 deferral list, review 5121634474; round-4 S4-2, review 5122622637)
- packages/cli/src/ui/utils/clipboard-paste-directory.ts:26 — memo-eviction-on-rejection path untested — already reported (round-5 deferral R5-4, review 5125833845)
Not reviewed: build-and-test — Test (windows-latest, Node 22.x) never reported at this commit and the win32-gated suites could not run locally, so the Windows-only decode branch and the four it.runIf cases are unexecuted.
Not reviewed: build-and-test — Test (macos-latest, Node 22.x) never reported at this commit and the macOS clipboard-unavailability path could not run locally.
Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) never reported at this commit and its suite did not run locally.
Not reviewed: test-efficacy mutation probe — the harness control produced no verdict (harnessValidated: null, it ran and died before answering: every probe vitest invocation tripped the globalSetup build guard), so 0 mutants and 0 hunk probes ran and there is no mutation-level evidence.
Not reviewed: reverse audit — stopped before round 4 by the review time budget.
Deferred under the convergence posture (round 6, not a blocker) — recorded, not requested in this round:
packages/cli/src/ui/components/InputPrompt.test.tsx:1760 — [probe] The new isValidPath stubs can never be called, so the…packages/cli/src/ui/components/InputPrompt.tsx:862 — [probe] A quote-stripped @-token is re-inserted verbatim, losing…packages/cli/src/ui/contexts/KeypressContext.tsx:790 — [probe] The pasteImage: true production path is pinned by nothing…packages/cli/src/ui/hooks/atCommandProcessor.test.ts:745 — [probe] No new decode test asserts the reference text that reaches…packages/cli/src/ui/hooks/atCommandProcessor.ts:408 — [probe] The non-ENOENT side of the new existence probe has no test…packages/cli/src/ui/hooks/atCommandProcessor.ts:415 — [probe] The decode substitution is not re-validated against the…packages/cli/src/ui/utils/clipboard-paste-directory.test.ts:49 — [probe] The keeps accepted files through shared cleanup …packages/cli/src/ui/utils/clipboard-paste-directory.ts:37 — [probe] The hand-rolled ownership decision under-reaps every paste…packages/cli/src/ui/utils/clipboard-paste-directory.ts:60 — [probe] Hand-rolled liveness probe instead of the exported…packages/cli/src/ui/utils/clipboardUtils.ts:368 — [probe] Nothing pins that readClipboardFiles signals…packages/core/src/utils/paths.ts:311 — [probe] On win32 a backslash is both separator and escape, so…
Mechanism health: this round did not close cleanly, so it withholds the incremental anchor — and the round it recovered had no anchor this round could use either — none at all, one with no certifier, one certified by an identity other than the one this round runs under, or one this round's fetch refused or resolved to the head — so the next review re-reads the whole diff unless recovery grafts an earlier own anchor that the round running it can use onto the complete work list this round leaves behind, and keeps doing so until a round's marker carries an anchor again or a graft lands that the round running it can use. (Stated, not acted on — this changes nothing about what the round posts.)
中文说明
仅完成部分审查，审查缺口已披露。
本轮确认的 5 条建议级发现已在 PR 上报告过，不再重复发布（列表见上方英文部分）。
未审查（原文为英文）：build-and-test — Test (windows-latest, Node 22.x) never reported at this commit and the win32-gated suites could not run locally, so the Windows-only decode branch and the four it.runIf cases are unexecuted.
未审查（原文为英文）：build-and-test — Test (macos-latest, Node 22.x) never reported at this commit and the macOS clipboard-unavailability path could not run locally.
未审查（原文为英文）：build-and-test — Integration Tests (CLI, No Sandbox) never reported at this commit and its suite did not run locally.
未审查（原文为英文）：test-efficacy mutation probe — the harness control produced no verdict (harnessValidated: null, it ran and died before answering: every probe vitest invocation tripped the globalSetup build guard), so 0 mutants and 0 hunk probes ran and there is no mutation-level evidence.
未审查：反向审计——评审时间预算不足，未能开始第 4 轮。
收敛姿态下延后（第 6 轮，非阻断）——已记录，本轮不要求修改：共 11 条（原文未翻译，列表见上方英文部分）。
机制健康：本轮未能干净收尾，因而扣留了增量锚点，而它恢复到的那一轮也没有留下本轮可用的锚点——要么完全没有、要么没有认证者、要么由本轮运行身份之外的身份认证、要么被本轮的获取拒绝或解析为头提交——因此下一次评审将重读整个 diff，除非恢复流程把本轮能使用的更早自有锚点嫁接到本轮留下的完整工作清单上；并会一直如此，直到某一轮的标记重新带上锚点，或落地的嫁接能被运行该轮的评审使用。（仅陈述，不据此行动——这不改变本轮发布的任何内容。）
— qwen3.8-max via Qwen Code /review (v0.23.0)
| workspaceContext.isPathWithinWorkspace(candidate) | ||
| ); | ||
| }; | ||
| const decodedPath = unescapeShellSpecials(pathName); |
There was a problem hiding this comment.
[Critical] R6-1: [certifies-falsely] [new-surface] On Windows, a clipboard @ reference that gets skipped because it points outside the workspace is handed to the model in its escaped spelling, which is not a path that exists — while the same paste on Linux hands over the decoded one.
unescapeShellSpecials is called only here, and the win32 substitution it feeds is gated by (isAllowedPath(pathName) || isAllowedPath(decodedPath)). When the copied file lives outside the project both are false, so the decode never runs; the token is dropped with only an onDebugMessage, and buildInitialQueryText re-emits part.content, which came from unescapePath(rawAtPath) — a no-op on win32. The escaped spelling is therefore what ships. That also falsifies the contract this diff writes down at paths.ts:352, "Escaped references are decoded by their consumer": in the skipped case no consumer decodes anything.
Copy C:\Users\mochi\Downloads\Q3 report.pdf in Explorer and press Alt+V with the project rooted elsewhere. The prompt receives @C:/Users/mochi/Downloads/Q3\ report.pdf, the path is outside the workspace so it is skipped, and that escaped string is what the model sees. If the model acts on it and calls read_file, Node's own path.win32 normalizes it to six segments ending in " report.pdf" and the read fails with ENOENT — so the model reports that a file the user just copied does not exist, which is not true. The identical paste on Linux emits the clean decoded spelling, so the two platforms diverge on text the user can see. Spaces are ordinary in Windows filenames, so a file copied from Downloads or Desktop lands here rather than in some corner.
Witness — the real handleAtCommand driven with a reference built by the real formatClipboardFileReference, over a file that genuinely exists outside the workspace and has a space in its name; same input in both arms, only the platform stub differs:
reference (identical both arms): "@/tmp/n1-outside-0NXgtI/Q3\ report.pdf"
win32: processedQuery: [{ "text": "inspect @/tmp/n1-outside-0NXgtI/Q3\ report.pdf" }] <- escaped spelling reaches the model
linux: processedQuery: [{ "text": "inspect @/tmp/n1-outside-0NXgtI/Q3 report.pdf" }] <- decoded spelling
Node's own path.win32 on the win32-emitted spelling:
win32Normalize: "C:\Users\mochi\Downloads\Q3\ report.pdf"
segments: ["C:","Users","mochi","Downloads","Q3"," report.pdf"] <- not a spelling of the file; read_file ENOENTs
merge base 92a8a8d179, git grep -c over packages/cli/src + packages/core/src:
formatClipboardFileReference / readClipboardFiles / clipboardFiles / getClipboardPasteDirectory: no hits
Decode the emitted text of a token that has already been decided to be skipped, at the emission site rather than at the policy site: in buildInitialQueryText, when a part has no resolved spec and process.platform === 'win32', emit the unescaped spelling instead of part.content.
// buildInitialQueryText — the branch that re-emits an unresolved at-path part
text +=
process.platform === 'win32'
? unescapeShellSpecials(part.content)
: part.content;That leaves isAllowedPath and the existence probe untouched and puts nothing on the read path, because a skipped token has no read path. It is also worth surfacing the drop the way the existing Git-ignored: summary does, so a pasted out-of-project file is not discarded in silence — the skip itself is already declared as intended behaviour in this PR's Risk & Scope, and is not what this comment is about.
The fix must stay off the resolution path: atCommandProcessor.ts:398-401 states "Probe existence only (never contents) before applying policy, so an existing but ignored, inaccessible, or out-of-root path cannot select an unrelated decoded sibling", so decode only the text of a token already decided to be skipped, and do not widen isAllowedPath (atCommandProcessor.ts:386-390).
Please add a case in packages/cli/src/ui/hooks/atCommandProcessor.test.ts modelled on should only allow actual temp directory paths outside the workspace, submitting formatClipboardFileReference(<an out-of-workspace file whose name contains a space>) with the platform mocked to win32 — the vi.spyOn(os, 'platform') pattern paths.test.ts already uses, so it runs on the Linux lane — and asserting processedQuery equals [{ text: '@' + <the decoded path> }] with no backslash-space in it. That case is red today, and removing the decode-on-skip afterwards must turn it red again.
中文说明
[Critical] R6-1：[certifies-falsely] [new-surface] 在 Windows 上，一个因为指向工作区之外而被跳过的剪贴板 @ 引用，会以转义后的拼写交给模型，而那个拼写并不是一个真实存在的路径；同一次粘贴在 Linux 上交给模型的却是解码后的拼写。
unescapeShellSpecials 只在这一行被调用，而它所服务的 win32 替换由 (isAllowedPath(pathName) || isAllowedPath(decodedPath)) 把守。当被复制的文件位于项目之外时，两者都为 false，于是解码根本不会执行；该 token 只留下一条 onDebugMessage 就被跳过，随后 buildInitialQueryText 原样重新发出 part.content，而它来自 unescapePath(rawAtPath) —— 在 win32 上是 no-op。因此最终送达的就是带转义的拼写。这也与本 diff 在 paths.ts:352 写下的约定相矛盾：「Escaped references are decoded by their consumer（转义过的引用由其消费方解码）」——在被跳过的情形下，没有任何消费方会去解码。
在资源管理器中复制 C:\Users\mochi\Downloads\Q3 report.pdf，在项目根目录位于别处时按下 Alt+V：输入框得到 @C:/Users/mochi/Downloads/Q3\ report.pdf，该路径不在工作区内因此被跳过，模型看到的就是这个带转义的字符串。如果模型据此调用 read_file，Node 自己的 path.win32 会把它归一化成 6 段、最后一段是 " report.pdf"，读取以 ENOENT 失败——于是模型会报告「用户刚刚复制的文件不存在」，而这并不成立。完全相同的一次粘贴在 Linux 上发出的是干净的解码拼写，因此两个平台在用户可见的文本上出现了分歧。空格在 Windows 文件名中非常常见，所以从 Downloads 或 Desktop 复制的文件会落进这条路径，而不是什么边角情形。
证据（Witness）——用真实的 handleAtCommand，配合由真实 formatClipboardFileReference 构造的引用，针对一个确实存在于工作区之外、文件名含空格的文件；两个分支输入完全相同，只有平台 stub 不同：
reference (identical both arms): "@/tmp/n1-outside-0NXgtI/Q3\ report.pdf"
win32: processedQuery: [{ "text": "inspect @/tmp/n1-outside-0NXgtI/Q3\ report.pdf" }] <- 带转义的拼写送达模型
linux: processedQuery: [{ "text": "inspect @/tmp/n1-outside-0NXgtI/Q3 report.pdf" }] <- 解码后的拼写
Node's own path.win32 on the win32-emitted spelling:
win32Normalize: "C:\Users\mochi\Downloads\Q3\ report.pdf"
segments: ["C:","Users","mochi","Downloads","Q3"," report.pdf"] <- 不是该文件的任何一种拼写；read_file 报 ENOENT
merge base 92a8a8d179, git grep -c over packages/cli/src + packages/core/src:
formatClipboardFileReference / readClipboardFiles / clipboardFiles / getClipboardPasteDirectory: no hits
请在发出端而不是策略端，对「已被判定跳过」的 token 解码其发出文本：在 buildInitialQueryText 中，当某个 part 没有 resolved spec 且 process.platform === 'win32' 时，发出反转义后的拼写，而不是 part.content。
// buildInitialQueryText —— 重新发出未解析 at-path part 的那个分支
text +=
process.platform === 'win32'
? unescapeShellSpecials(part.content)
: part.content;这样既不改动 isAllowedPath，也不改动存在性探测，并且不会把任何东西放到读取路径上——因为被跳过的 token 本来就没有读取路径。另外建议按现有 Git-ignored: 摘要的方式把这次丢弃显示出来，使粘贴进来的项目外文件不至于无声消失；「跳过」这件事本身已在本 PR 的 Risk & Scope 中声明为预期行为，不是本条评论要谈的内容。
修复必须停留在解析路径之外：atCommandProcessor.ts:398-401 写着「Probe existence only (never contents) before applying policy, so an existing but ignored, inaccessible, or out-of-root path cannot select an unrelated decoded sibling（在应用策略之前只探测存在性、绝不读取内容，这样一个已存在但被忽略、不可访问或超出根目录的路径就不会选中一个无关的解码后同级路径）」，因此只对一个已被判定跳过的 token 的文本做解码，并且不要放宽 isAllowedPath（atCommandProcessor.ts:386-390）。
请在 packages/cli/src/ui/hooks/atCommandProcessor.test.ts 中，参照 should only allow actual temp directory paths outside the workspace 新增一个用例：提交 formatClipboardFileReference(<一个工作区之外、文件名含空格的文件>)，并把平台 mock 成 win32——即 paths.test.ts 已在使用的 vi.spyOn(os, 'platform') 方式，因此它能在 Linux 通道上运行——断言 processedQuery 等于 [{ text: '@' + <解码后的路径> }]，其中不含反斜杠加空格。该用例今天是红的；修复落地后再移除「跳过时解码」这一逻辑，它必须重新变红。
— qwen3.8-max via Qwen Code /review (v0.23.0)
There was a problem hiding this comment.
Thanks for the detailed witness. I agree that a skipped reference can retain its escaped spelling on Windows. I am leaving this behavior unchanged in this PR and propose treating R6-1 as a non-blocking follow-up rather than a Critical.
The demonstrated result is a difference in the prompt text after the reference has been skipped. Not automatically expanding out-of-workspace files is already an explicit scope limitation of this PR. The subsequent failure assumes the model passes that spelling unchanged to a file tool; the witness does not demonstrate that model-to-tool interaction. This does not rule out a usability problem, but it does not establish a blocking file-read regression either.
The proposed unconditional unescape at the emission site also affects manually entered, unresolved Windows paths, not just references produced by the clipboard formatter. For example, @C:\repo\#docs\file.txt would become @C:\repo#docs\file.txt, changing a real directory separator in the text sent to the model. Moving the conversion off the automatic resolution path avoids an immediate read, but does not make that conversion unambiguous.
A separate improvement could preserve the original clipboard path/provenance and use it when presenting skipped references, with coverage for both generated references and literal Windows paths. For this PR, I prefer to retain the current workspace policy and path-preservation behavior rather than broaden normalization for all unresolved references.
6 participants
What this PR does
This adds Windows support for pasting files copied in File Explorer through the existing clipboard shortcut and empty terminal paste path. Image-only selections become attachments through the existing image-path flow, while selections containing other file types insert their paths into the prompt.
The keyboard shortcut reference now documents the Windows file-copy behavior.
Why it's needed
File Explorer stores copied files as a native file list rather than clipboard text or image pixels. Qwen Code already ships a native clipboard integration that can read this format, but the input flow only checked for image pixels, so copying a file and pressing Alt+V or a terminal-provided Ctrl+V produced no input.
Reviewer Test Plan
How to verify
Evidence (Before & After)
Before: a copied Explorer file produced no prompt input because the clipboard contained a native file list that was not read.
After: the built Windows clipboard path round-tripped one copied file successfully, the clipboard utility suite passed 30/30 tests, the keypress suite passed 109/109 tests, and the focused prompt regressions passed 2/2 tests.
Visual demo: see the Windows File Explorer clipboard before/after demo.
Tested on
Environment (optional)
Windows 10.0.26100.8875, PowerShell 7.6.3, CP936, Node.js 22.11.0. The CLI package build, typecheck, and changed-file lint all passed. The full prompt test file passed 197 tests with one skipped test and one pre-existing Windows path-separator expectation failure unrelated to this change.
Risk & Scope
@references but are not copied. Files outside the workspace or an allowed Qwen temporary directory may therefore be skipped by at-command workspace checks. Pure image selections are copied into a Qwen temporary directory before attachment promotion. Partial attachment promotion for mixed image/non-image selections is also out of scope; mixed selections remain references.Linked Issues
Fixes #2605
中文说明
此 PR 的改动
此改动让 Windows 可以通过现有剪贴板快捷键和终端空粘贴路径，粘贴在文件资源管理器中复制的文件。仅包含图片的选择会通过现有图片路径流程变成附件；包含其他文件类型的选择则会把路径插入输入框。
键盘快捷键文档也补充了 Windows 文件复制行为。
为什么需要
文件资源管理器会把复制的文件存为原生文件列表，而不是剪贴板文本或图片像素。Qwen Code 已经随包提供了能够读取该格式的原生剪贴板集成，但输入流程此前只检查图片像素，因此复制文件后按 Alt+V，或者使用终端提供的 Ctrl+V，都不会产生输入。
Reviewer Test Plan
验证方式
前后证据
改动前：从资源管理器复制的文件不会产生输入，因为应用没有读取剪贴板中的原生文件列表。
改动后：构建后的 Windows 剪贴板路径成功往返读取一个复制文件；剪贴板工具测试 30/30 通过，按键测试 109/109 通过，输入框定向回归 2/2 通过。
可视化演示：见 Windows 文件资源管理器剪贴板前后对比录屏。
测试平台
环境
Windows 10.0.26100.8875、PowerShell 7.6.3、CP936、Node.js 22.11.0。CLI 包构建、类型检查和变更文件 lint 均通过。完整输入框测试中 197 项通过、1 项跳过，另有 1 项与本改动无关的既有 Windows 路径分隔符预期失败。
风险与范围
@引用，但不会被复制；因此位于工作区或允许的 Qwen 临时目录之外的文件，可能被 at-command 的工作区检查跳过。仅图片选择会先复制到 Qwen 临时目录，再提升为附件。图片和非图片混合选择的部分附件提升也不在本轮范围内；混合选择仍全部作为引用插入。关联 Issue
Fixes #2605