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, with [Image #N] placeholders#3519callmeYe wants to merge 3 commits into
Conversation
|
@callmeYe Thank you for your feedback and code contribution. However, I must point out that the issue you encountered with cmd+v not being able to paste is not caused by either of the two reasons. To verify this, you can try using Ctrl+v to paste an image. cmd+v is blocked by some command-line applications on macOS; for example, you cannot use cmd+v to paste images in iTerm2, but it works in the VS Code command line. Therefore, we provide Ctrl+v as a backup. Similarly, Ctrl+v is also blocked by most command-line applications on Windows, so we use Alt+v as a backup shortcut for pasting. If you compare, you'll find that claudecode has also implemented the same handling, as this is a common issue encountered by command-line applications. |
…age #N] placeholders Adds three new ways to attach images to the prompt, all converging on a shared [Image #N] placeholder UX inserted at the cursor: - Paste a `data:image/...;base64,...` URL as text. Decoded after a conservative magic-byte check. - Paste raw base64. Only accepted when the decoded prefix matches a known image magic (PNG/JPEG/GIF/WebP/BMP/TIFF) so JWTs and regular base64-looking text aren't hijacked. - Drag an image file into the terminal. Supported both when the terminal wraps the drop in bracketed paste AND when it synthesizes the drop as a rapid burst of individual keystrokes (macOS Terminal.app), via a simple keystroke-burst heuristic (>= 4 chars with < 10 ms gaps) + a debounced scan of the buffer tail. The existing Cmd+V clipboard path now also renders as [Image #N] for consistency; attachments added without a placeholder fall back to the legacy [filename] chip so other callers are unchanged. At submit time, [Image #N] tokens in the message text are substituted with @<relative path> so the model sees each image exactly where the user placed it. Counter resets on submit. Files ----- - clipboardUtils.ts: add `tryDecodeBase64Image`, `saveDecodedImage`, `detectDraggedImagePath` (path is validated to exist, be a regular file, and have a recognized image extension). Accepts single-/double-quoted paths and escaped spaces as used by drag-drop. - InputPrompt.tsx: unified paste branch (pasteImage -> base64 -> drag-drop paste -> large paste -> small paste), keystroke-burst detection for drag-drops that arrive as typing, [Image #N] allocator, attachment placeholder substitution on submit. - Tests: 143/143 pass (1 Windows skip). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
0cfd47c to
02e181e
Compare
callmeYe
changed the title
|
Hi @LaZzyMan — thanks for the pointer about the Ctrl+V / Alt+V fallbacks, that context is useful and I'll keep it in mind. Just to clarify the evidence behind #3517 (not this PR, which intentionally leaves the Cmd+V path untouched): My reproduction wasn't at the terminal layer. I ran the installed const m = await import('@teddyzhu/clipboard');
const c = new m.ClipboardManager();
c.hasFormat('image'); // => false
m.getClipboardImage(); // throws: "Failed to get image: no image data"Since If Ctrl+V works reliably on your setup, I suspect we're on different darwin architectures (mine is arm64, Node 22.17) or different versions of the native addon. Happy to attach the direct-Node reproduction and my env details to #3517 if that would help — or if you'd rather close it as "works as designed with Ctrl+V," I'm fine with that too; it's orthogonal to this PR. For #3519 specifically: no code change needed from this discussion — the feature paths (base64 data URL paste, raw base64 paste, drag-drop file) don't go through |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| it('accepts escaped spaces (terminal drag-drop style)', async () => { | ||
| const imagePath = path.join(tmp, 'a b.png'); | ||
| await fs.writeFile(imagePath, PNG_MAGIC); | ||
| const escaped = imagePath.replace(/ /g, '\\ '); |
wenshao
left a comment
wenshao
left a comment
There was a problem hiding this comment.
[Critical] Deleting an image attachment only removes it from attachments; it does not remove the corresponding inline [Image #N] token from buffer.text. That leaves a stale placeholder behind, and handleSubmitAndClear() will then send literal text like [Image #1] instead of an image reference after the user deletes the attachment. Please keep attachment deletion and inline placeholders synchronized.
— gpt-5.4 via Qwen Code /review
| if (key.pasteImage) { | ||
| handleClipboardImage(true); | ||
| } else if (decodedImage) { | ||
| handleBase64ImagePaste(decodedImage); |
There was a problem hiding this comment.
[Critical] This starts the base64-image materialization asynchronously and returns immediately. If the user pastes an image and hits Enter right away, handleSubmitAndClear() can run before the attachment state and [Image #N] placeholder are inserted, so the submitted prompt silently drops the image.
Please block submit while image materialization is in flight, or await this path before normal submission can proceed.
— gpt-5.4 via Qwen Code /review
wenshao
left a comment
wenshao
left a comment
There was a problem hiding this comment.
[Critical] Two key test gaps in the most important new code paths:
-
[Image #N]→@pathsubmit substitution is not tested. No test pastes an image, types text, presses Enter, and assertsonSubmitreceives the substituted@path. This is the most complex new logic inhandleSubmitAndClear(theIMAGE_PLACEHOLDER_REreplace +refByPlaceholdermap +path.relative). A regression would silently send literal[Image #1]to the model. -
Drag-burst keystroke detection has zero test coverage. All drag-and-drop tests use bracketed-paste sequences (
\x1b[200~...~\x1b[201~), exercising only thekey.pastebranch. The entire keystroke-burst fallback —burstCharCountRef,dragCheckTimerRef,scanBufferTailForDraggedImage— is untested. This is the feature's fallback for macOS Terminal.app and iTerm without bracketed paste.
Additionally: imagePlaceholderCounter.current = 0 reset on submit (line 366) is not tested — after submitting with [Image #1]/[Image #2], a new paste should produce [Image #1] again.
— qwen3.7-max via Qwen Code /review
| if (buffer.length === 0) return null; | ||
| const sniffed = detectByMagic(buffer); | ||
| const mimeType = sniffed?.mimeType ?? declaredMime; | ||
| const ext = sniffed?.ext ?? EXT_BY_MIME[declaredMime] ?? 'bin'; |
There was a problem hiding this comment.
[Suggestion] Data URL branch accepts non-image content when magic bytes don't match
When detectByMagic returns null for a data URL, the code falls back to the declared MIME type and may return ext: 'bin'. This is asymmetric with the raw-base64 branch (line 311: if (!sniffed) return null). For example, data:image/png;base64,SGVsbG8gV29ybGQ= (decodes to "Hello World") passes all checks and returns { mimeType: 'image/png', ext: 'png' }.
| const ext = sniffed?.ext ?? EXT_BY_MIME[declaredMime] ?? 'bin'; | |
| const sniffed = detectByMagic(buffer); | |
| if (!sniffed) return null; | |
| return { buffer, mimeType: sniffed.mimeType, ext: sniffed.ext }; |
— qwen3.7-max via Qwen Code /review
| export function tryDecodeBase64Image( | ||
| text: string, | ||
| ): { buffer: Buffer; mimeType: string; ext: string } | null { | ||
| if (!text) return null; |
There was a problem hiding this comment.
[Suggestion] No upper bound on base64 payload size
tryDecodeBase64Image is called synchronously on every paste (InputPrompt.tsx:635) before the large-paste threshold check. A very large base64 string (e.g. from a paste-jacking website) causes unbounded Buffer.from allocation on the main thread.
| if (!text) return null; | |
| if (trimmed.length < 32) return null; | |
| if (trimmed.length > 64 * 1024 * 1024) return null; |
— qwen3.7-max via Qwen Code /review
| ); | ||
| cleanupOldClipboardImages(Storage.getGlobalTempDir()).catch(() => {}); | ||
| addImageAttachment(imagePath); | ||
| } catch (error) { |
There was a problem hiding this comment.
[Suggestion] Silent discard of pasted text on filesystem error
If saveDecodedImage throws (disk full, permission error, temp dir deleted), the catch block only logs. The paste event has already been consumed (key.paste handler returns true), so the user sees neither the image attachment nor the original pasted text — it's silently lost.
Consider falling back to inserting the raw pasted text into the buffer on error:
} catch (error) {
debugLogger.error('Error handling base64 image paste:', error);
buffer.insert(rawText, { paste: false }); // pass rawText as a parameter
}— qwen3.7-max via Qwen Code /review
| } | ||
| } | ||
| if (refByPlaceholder.size > 0) { | ||
| finalValue = finalValue.replace(IMAGE_PLACEHOLDER_RE, (match) => |
There was a problem hiding this comment.
[Suggestion] Orphaned @ref silently dropped when [Image #N] text is deleted from buffer
If the user backspaces over [Image #1] in the input text but doesn't remove the attachment from the attachment bar, the replace() call never matches that placeholder. The @ref is silently dropped — the model never receives the image reference, and no warning is shown.
Consider collecting unconsumed refByPlaceholder entries and falling back to prependRefs:
const consumed = new Set<string>();
finalValue = finalValue.replace(IMAGE_PLACEHOLDER_RE, (match) => {
if (refByPlaceholder.has(match)) {
consumed.add(match);
return refByPlaceholder.get(match)!;
}
return match;
});
for (const [placeholder, ref] of refByPlaceholder) {
if (!consumed.has(placeholder)) prependRefs.push(ref);
}— qwen3.7-max via Qwen Code /review
| dragCheckTimerRef.current = null; | ||
| burstCharCountRef.current = 0; | ||
| scanBufferTailRef.current?.(); | ||
| }, DRAG_CHECK_DEBOUNCE_MS); |
There was a problem hiding this comment.
[Suggestion] Drag-burst timer fires without checking shellModeActive
The setTimeout callback doesn't check shellModeActive. If a timer is armed before toggling shell mode (via ! on an empty buffer), it can fire and call scanBufferTailForDraggedImage on a shell command buffer, potentially corrupting the command by stripping a trailing image-path token.
| }, DRAG_CHECK_DEBOUNCE_MS); | |
| dragCheckTimerRef.current = setTimeout(() => { | |
| dragCheckTimerRef.current = null; | |
| burstCharCountRef.current = 0; | |
| if (!shellModeActiveRef.current) { | |
| scanBufferTailRef.current?.(); | |
| } | |
| }, DRAG_CHECK_DEBOUNCE_MS); |
— qwen3.7-max via Qwen Code /review
| if (!text) return; | ||
| // Match an optional single- or double-quoted path, followed by an optional | ||
| // trailing space that some terminals add after a file drop. | ||
| const match = text.match(/('([^']+)'|"([^"]+)"|(\S+))\s?$/); |
There was a problem hiding this comment.
[Suggestion] Greedy quoted-path regex in scanBufferTailForDraggedImage
The '([^']+)' alternative is greedy: it matches from the first single quote in the entire buffer to the last single quote. If the user typed text with an earlier single-quoted segment before dragging an image (e.g., it's a photo '/path/to/image.png'), the regex spans across both quotes, producing an invalid token that detectDraggedImagePath rejects — silently missing the drag-drop.
| const match = text.match(/('([^']+)'|"([^"]+)"|(\S+))\s?$/); | |
| const match = text.match(/(?:^|\s)(?:'([^']+)'|"([^"]+)"|(\S+))\s?$/); |
— qwen3.7-max via Qwen Code /review
| setAttachments([]); | ||
| setIsAttachmentMode(false); | ||
| setSelectedAttachmentIndex(-1); | ||
| imagePlaceholderCounter.current = 0; |
There was a problem hiding this comment.
[Suggestion] imagePlaceholderCounter reset on submit is not tested
After submitting a message with [Image #1] and [Image #2], pasting a new image should produce [Image #1] again (counter resets to 0). No test verifies this behavior. If this line is accidentally removed, the second message would start at [Image #3].
— qwen3.7-max via Qwen Code /review
DragonnZhang
left a comment
DragonnZhang
left a comment
There was a problem hiding this comment.
One additional finding not covered by the existing inline comments.
The core design (unified [Image #N] placeholders with @path substitution at submit time) is sound, and the test coverage for the new utility functions is thorough. The existing comments from @wenshao cover the most critical issues (race condition on async paste + immediate submit, stale placeholders on attachment deletion, missing size bound, untested submit substitution). This comment adds one more defensive-programming concern.
— qwen-code via Qwen Code /review
| await fs.mkdir(tempDir, { recursive: true }); | ||
| const filePath = path.join( | ||
| tempDir, | ||
| `clipboard-${Date.now()}.${ext || 'bin'}`, |
There was a problem hiding this comment.
[Suggestion] Potential filename collision in saveDecodedImage when two images are pasted within the same millisecond.
Date.now() is used as the sole entropy for the filename. Since handleBase64ImagePaste is called fire-and-forget (not awaited in the paste handler at InputPrompt.tsx:643), two rapid base64 paste events could invoke saveDecodedImage concurrently. If both calls read Date.now() before either await fs.writeFile completes, the second write overwrites the first file — the first attachment then points at the second image's data.
The attachment ID already incorporates imagePlaceholderCounter for uniqueness (${Date.now()}-${counter}). Consider applying the same pattern to the filename, e.g.:
`clipboard-${Date.now()}-${Math.random().toString(36).slice(2, 8)}.${ext || 'bin'}`Or accept a counter/nonce parameter from the caller.
— qwen-code via Qwen Code /review
|
@callmeYe heads up — this PR currently has merge conflicts with Conflicting files:
The rest merges cleanly. Thanks! 中文@callmeYe 提个醒 —— 这个 PR 目前和 冲突文件:
其余文件可以自动合并。谢谢! |
|
@qwen-code /resovle |
| } | ||
| } | ||
| if (refByPlaceholder.size > 0) { | ||
| finalValue = finalValue.replace(IMAGE_PLACEHOLDER_RE, (match) => |
There was a problem hiding this comment.
[Correctness] IMAGE_PLACEHOLDER_RE (with the g flag) replaces all [Image #N] patterns in finalValue at submit time — not just the auto-inserted placeholders. If a user types text that happens to contain [Image #1] (e.g. a Markdown reference link, a discussion about the placeholder format itself, or content copied from documentation) and also pastes an image, their typed [Image #1] is silently replaced with @<path>.
This is distinct from the orphaned-placeholder issue already raised (that one is about the attachment surviving after the placeholder is deleted; this is about unrelated user text being clobbered).
Fix: Make the placeholder token unambiguous so user-typed text can never collide. Two common approaches:
- Use a zero-width character sentinel inside the token (e.g.
[Image \u200B#1]) — invisible to the user but invisible to naive regex. - Use a private-use Unicode character or an internal-only marker that is extremely unlikely to appear in natural text (e.g.
[\uE001Image#1]or[qwen-image:1]).
Either way the key idea is that the substitution regex should only match tokens the component itself inserted.
|
@qwen-code /triage |
|
@qwen-code /resolve |
Merge origin/main into feat/base64-image-paste, resolving conflicts in:
- clipboardUtils.ts: combined imports (statSync, unescapePath from HEAD;
fsConstants, execSync/spawn, randomUUID, wrapForMultiplexer from main)
- InputPrompt.tsx: kept IMAGE_PLACEHOLDER_RE (HEAD) alongside
PASTED_IMAGE_EXTENSIONS + classifyPastedImagePaths (main); merged all
three paste handler variables and dependencies into if/else chain
- clipboardUtils.test.ts: kept PNG_MAGIC/JPEG_MAGIC constants (HEAD)
and mock infrastructure (main); adapted saveDecodedImage and
detectDraggedImagePath tests to use vi.unmock + dynamic imports
so they work with main's vi.mock('node:fs/promises') overrides
|
Qwen Code resolved the merge conflicts and pushed the branch update. PR #3519 — Merge Conflict Resolution SummaryBranch: Conflicts Resolved (3 files, 6 conflicts)1.
|
|
Qwen Code did not run conflict resolution for this request. PR #3519 does not currently have merge conflicts with main. |
| import * as os from 'node:os'; | ||
| import * as path from 'node:path'; | ||
| import { EventEmitter } from 'node:events'; | ||
| import { |
There was a problem hiding this comment.
[Critical] [linter] ESLint: saveDecodedImage and detectDraggedImagePath are only used as types in this test file but imported as values. Use type imports:
| import { | |
| import type { | |
| saveDecodedImage, | |
| tryDecodeBase64Image, | |
| detectDraggedImagePath, | |
| } from './clipboardUtils.js'; | |
| import { | |
| clipboardHasImage, | |
| saveClipboardImage, | |
| cleanupOldClipboardImages, | |
| } from './clipboardUtils.js'; |
— qwen3.7-max via Qwen Code /review
| expect(decoded!.mimeType).toBe('image/jpeg'); | ||
| expect(decoded!.ext).toBe('jpg'); | ||
| }); | ||
|
|
There was a problem hiding this comment.
[Critical] [test] 4 test failures from mock leakage. The afterEach in earlier describe blocks installs vi.mock('node:fs/promises', ...) that bleeds into these saveDecodedImage and detectDraggedImagePath test blocks despite vi.unmock + vi.resetModules() in beforeEach. The production code operates on mocked fs functions that silently no-op, so readFile returns undefined and statSync can't see the real file.
Fix: restructure the mock lifecycle — either move these test blocks above the blocks that install the fs mock, or use vi.doUnmock before vi.resetModules() and re-import the module in each test.
— qwen3.7-max via Qwen Code /review
| await fs.mkdir(tempDir, { recursive: true }); | ||
| const filePath = path.join( | ||
| tempDir, | ||
| `clipboard-${Date.now()}.${ext || 'bin'}`, |
There was a problem hiding this comment.
[Critical] TOCTOU / symlink attack: saveDecodedImage generates predictable filenames (clipboard-${Date.now()}.${ext}) with no randomUUID() and no O_EXCL. The same file already demonstrates the correct pattern — saveClipboardImage (line 532) uses clipboard-${timestamp}-${randomUUID()}.png, and saveFromCommand opens with O_WRONLY | O_CREAT | O_EXCL.
On shared systems, an attacker can pre-create a symlink at the predictable path and the next paste overwrites an arbitrary target file with attacker-controlled binary content.
| `clipboard-${Date.now()}.${ext || 'bin'}`, | |
| const filePath = path.join( | |
| tempDir, | |
| `clipboard-${Date.now()}-${randomUUID()}.${ext || 'bin'}`, | |
| ); | |
| const fd = await fs.open(filePath, fsConstants.O_WRONLY | fsConstants.O_CREAT | fsConstants.O_EXCL); | |
| await fd.writeFile(buffer); | |
| await fd.close(); |
— qwen3.7-max via Qwen Code /review
| return null; | ||
| } | ||
| return candidate; | ||
| } |
There was a problem hiding this comment.
[Critical] detectDraggedImagePath doesn't enforce absolute paths. A relative path like image.png passes statSync (resolved against CWD), is returned as-is, and handleSubmitAndClear's path.relative(config.getTargetDir(), att.path) with a relative second argument produces a broken @ref — the model receives an incorrect path and the user gets no error.
| } | |
| if (!path.isAbsolute(candidate)) return null; | |
| const stats = statSync(candidate); |
— qwen3.7-max via Qwen Code /review
| } else { | ||
| burstCharCountRef.current = 1; | ||
| } | ||
| if (burstCharCountRef.current >= DRAG_MIN_BURST_CHARS) { |
There was a problem hiding this comment.
[Suggestion] DRAG_MIN_BURST_CHARS = 4 is too low — terminal multiplexers (tmux, mosh), CJK input methods, and SSH connections with buffered delivery routinely produce 4+ character bursts within 10ms. When the heuristic fires, scanBufferTailForDraggedImage silently strips the trailing token from the buffer and replaces it with an [Image #N] placeholder.
Consider raising DRAG_MIN_BURST_CHARS to 15+ (real file paths are typically 20+ chars), or requiring the burst to constitute the entire trailing token.
— qwen3.7-max via Qwen Code /review
|
|
||
| const IMAGE_PLACEHOLDER_RE = /\[Image #(\d+)\]/g; | ||
|
|
||
| const PASTED_IMAGE_EXTENSIONS = /\.(png|jpe?g|gif|webp|bmp)$/i; |
There was a problem hiding this comment.
[Suggestion] Extension list inconsistency: PASTED_IMAGE_EXTENSIONS here covers png|jpe?g|gif|webp|bmp, while IMAGE_EXTENSIONS in clipboardUtils.ts (line 721) also includes tif, tiff, heic, heif. A .heic file dragged onto the terminal is recognized by detectDraggedImagePath, but the same path pasted via bracketed paste goes through classifyPastedImagePaths and is NOT recognized.
Export IMAGE_EXTENSIONS from clipboardUtils.ts and derive PASTED_IMAGE_EXTENSIONS from it to keep a single source of truth.
— qwen3.7-max via Qwen Code /review
| check: (b) => b.length >= 2 && b[0] === 0x42 && b[1] === 0x4d, | ||
| }, | ||
| { | ||
| ext: 'tiff', |
There was a problem hiding this comment.
[Suggestion] BMP magic is only 2 bytes (0x42 0x4d = ASCII "BM"). Any raw base64 string starting with Qk0 decodes to those bytes and gets misidentified as a BMP image, silently consuming the paste. Consider requiring b.length >= 14 (minimum BMP file header size) to reduce false positives.
— qwen3.7-max via Qwen Code /review
|
@qwen-code /triage |
|
Thanks for the PR @callmeYe! Template: substantive content provided ✓ (uses different headings than the template but covers the intent — Summary, Test Plan, Backwards Compatibility, Notes all present). Problem: real UX gap. Users currently have only one way to attach images (Cmd+V binary clipboard), which doesn't cover data URLs from devtools, raw base64 from chat messages, or drag-drop from Finder. Closes #3518 — this is an observed need, not theoretical hardening. Direction: aligned. Expanding image input methods is a natural CLI UX improvement — Claude Code has similar capabilities. The Size: not applicable — no core module paths touched. All 4 files are in Approach: the three new paths (data URL, raw base64, drag-drop) converging into a unified Moving on to code review. 🔍 中文说明感谢 @callmeYe 的贡献！ 模板：已包含实质内容 ✓（标题与模板不同，但覆盖了模板意图——Summary、Test Plan、Backwards Compatibility、Notes 均有）。 问题：真实的 UX 缺口。用户目前只能通过 Cmd+V 二进制剪贴板附加图片，无法处理 devtools 的 data URL、聊天消息中的 base64、或从 Finder 拖入的文件。关联 #3518——这是已观测到的需求，不是理论性加固。 方向：对齐。扩展图片输入方式是 CLI UX 的自然改进——Claude Code 也有类似能力。 规模：不适用——未触及核心模块路径。4 个文件均在 方案：三种新路径（data URL、raw base64、拖放）统一收敛到 进入代码审查 🔍 — Qwen Code · qwen3.7-max |
Code ReviewThe implementation is well-structured — Two issues found: 1. Build failure — unused import (blocker).
2. Four test failures in vitest 3.2.4. The These 4 tests pass in the author's environment (claimed 143/0) but fail here. The No other issues in production code. The Real-Scenario Testing
(This is because Unit test results from the PR's changed files:
Once the build error and test failures are fixed, I'd like to re-run tmux interactive testing to confirm the feature works end-to-end. 中文说明代码审查实现结构良好—— 发现两个问题： 1. 构建失败——未使用的导入（阻塞）。 2. vitest 3.2.4 中 4 个测试失败。 真实场景测试由于构建失败， 单元测试结果：
修复构建错误和测试失败后，需要重新运行 tmux 交互测试以确认功能端到端正常工作。 — Qwen Code · qwen3.7-max |
|
Stepping back: this is a well-designed feature PR that solves a real problem. The The two blocking issues are both in the test file and are straightforward fixes:
Neither issue reflects a design problem — they're test-infrastructure bugs that the author likely didn't hit in their local vitest version. I can't approve while the build is broken, but once these two are addressed, this is ready to ship. Not requesting tmux re-test until the build passes. 中文说明总体来看：这是一个设计良好的功能 PR，解决了真实问题。 两个阻塞问题都在测试文件中，修复很简单：
这两个问题都不反映设计缺陷——它们是测试基础设施 bug，作者可能在本地 vitest 版本中没有遇到。 构建失败时无法批准，但修复后即可合入。构建通过后再进行 tmux 交互测试。 — Qwen Code · qwen3.7-max |
| import * as path from 'node:path'; | ||
| import { EventEmitter } from 'node:events'; | ||
| import { | ||
| clipboardHasImage, |
There was a problem hiding this comment.
[Critical] [build] Unused imports cause TS6133 build error
clipboardHasImage, saveClipboardImage, and cleanupOldClipboardImages are imported here as values but only used inside dynamically-imported test modules (lines 178-180). These top-level imports are never referenced in the new test blocks (tryDecodeBase64Image, saveDecodedImage, detectDraggedImagePath) that use the real functions via await import().
TypeScript strict mode (noUnusedLocals) flags these as TS6133 errors, breaking the CLI package build.
Remove clipboardHasImage, saveClipboardImage, and cleanupOldClipboardImages from this import block, keeping only the functions actually used by the new tests:
| clipboardHasImage, | |
| import { | |
| saveDecodedImage, | |
| tryDecodeBase64Image, | |
| detectDraggedImagePath, | |
| } from './clipboardUtils.js'; |
— qwen3.7-max via Qwen Code /review
| let saveDecodedImageReal: typeof saveDecodedImage; | ||
|
|
||
| beforeEach(async () => { | ||
| vi.unmock('node:fs/promises'); |
There was a problem hiding this comment.
[Critical] [test] 4 test failures from vi.mock/vi.unmock leakage
The vi.unmock('node:fs/promises') + vi.resetModules() in this beforeEach does not fully restore the real node:fs/promises module. The top-level vi.mock('node:fs/promises', ...) factory (installed for earlier test blocks) persists in vitest's mock registry, so writeFile, readFile, and stat remain mocked no-ops.
This causes:
saveDecodedImagetest:readFilereturnsundefinedinstead of the written buffer- 3
detectDraggedImagePathtests:writeFileis a no-op (files never created on disk), sostatSyncthrows ENOENT
Fix: use vi.doMock/vi.doUnmock instead of vi.mock/vi.unmock, or restructure these test blocks to avoid depending on real filesystem operations while the module-level mock is active.
— qwen3.7-max via Qwen Code /review
| // Used to catch drag-drop on terminals that don't use bracketed paste: | ||
| // the path arrives as a burst of individual keystrokes, and by the time this | ||
| // runs the path is already sitting at the end of buffer.text. | ||
| const scanBufferTailForDraggedImage = useCallback(() => { |
There was a problem hiding this comment.
[Critical] \S+ branch cannot match paths with backslash-escaped spaces (Linux drag-drop)
The regex /('([^']+)'|"([^"]+)"|(\S+))\s?$/ uses \S+ for the unquoted branch. On Linux terminals (GNOME Terminal, Konsole, xfce4-terminal), drag-dropped paths with spaces are encoded with backslash-escaped spaces, e.g. /home/user/my\ photo.png. The literal space in the buffer causes \S+ to match only photo.png, truncating the path. detectDraggedImagePath('photo.png') then fails (relative path, not found), and the drag-drop is silently dropped.
The downstream unescapePath call inside detectDraggedImagePath is designed to handle backslash escapes, but it never receives the full path because this regex already truncated it.
Suggested fix — replace \S+ with a pattern that also consumes backslash-escaped characters:
| const scanBufferTailForDraggedImage = useCallback(() => { | |
| const match = text.match(/('([^']+)'|"([^"]+)"|((?:\\.|[^\s])+))\s?$/); |
— qwen3.7-max via Qwen Code /review
Suggestions — commit
|
| File | Issue | Suggested fix |
|---|---|---|
InputPrompt.tsx:1053-1097 |
Drag-burst detection heuristic (~40 lines) has zero test coverage — no tests exercise burst accumulation, timer callback, or false-positive guard for human typing | Add integration tests simulating rapid single-char key events with <10ms deltas followed by a 150ms+ pause, asserting scanBufferTailForDraggedImage fires |
InputPrompt.tsx:853-857 |
promotePastedImagePaths creates attachments without [Image #N] placeholders, causing inconsistent submit behavior (images prepended instead of inline) compared to the new addImageAttachment path |
Have promotePastedImagePaths call addImageAttachment per resolved image for consistent placeholder-based substitution |
InputPrompt.tsx:1129 |
classifyPastedImagePaths(pasted) runs unconditionally on every paste, even when earlier branches (pasteImage, decodedImage, draggedImagePath) short-circuit |
Move the call inside the else if branch that consumes it |
clipboardUtils.ts:838 |
detectDraggedImagePath uses synchronous statSync, blocking the event loop on every paste; inconsistent with the rest of the file which uses fs/promises |
Replace statSync with await fs.stat() and make callers async |
clipboardUtils.ts:742 |
isLikelyBase64 rejects valid unpadded base64 via s.length % 4 !== 0; several real-world encoders produce unpadded output |
Pad to multiple of 4 before checking: s += '='.repeat((4 - s.length % 4) % 4) |
InputPrompt.tsx:296-298 |
DRAG_BURST_MAX_INTERVAL_MS, DRAG_CHECK_DEBOUNCE_MS, DRAG_MIN_BURST_CHARS declared inside component body; existing module-level constants set the pattern |
Move to module scope alongside LARGE_PASTE_CHAR_THRESHOLD |
clipboardUtils.ts:773-777 |
try { Buffer.from(payload, 'base64') } catch is dead code — Node.js never throws for string+base64 encoding |
Remove the try/catch or add a comment explaining it's intentionally defensive |
InputPrompt.tsx:1133 |
handleBase64ImagePaste(decodedImage) is a floating promise without void; nearby promotePastedImagePaths uses void |
Add void prefix for consistency with promotePastedImagePaths |
— qwen3.7-max via Qwen Code /review
wenshao
left a comment
wenshao
left a comment
There was a problem hiding this comment.
Reviewed — no blockers. Suggestions are inline.
| file.startsWith('clipboard-') && | ||
| (file.endsWith('.png') || | ||
| file.endsWith('.jpg') || | ||
| file.endsWith('.jpeg') || |
There was a problem hiding this comment.
[Suggestion] .bin files produced by saveDecodedImage are never cleaned up
saveDecodedImage falls back to ext = 'bin' when the MIME type is unrecognized (line ~827: EXT_BY_MIME[declaredMime] ?? 'bin'), producing files like clipboard-<ts>.bin. However, this cleanup filter only matches .png, .jpg, .jpeg, .webp, .heic, .heif, .tiff, .gif, .bmp — .bin is absent, so those files accumulate indefinitely.
| file.endsWith('.jpeg') || | |
| file.endsWith('.bmp') || | |
| file.endsWith('.bin')) |
— qwen3.7-max via Qwen Code /review
| // scan of buffer.text's trailing token and promote it to an image | ||
| // attachment when it points at a real image file. | ||
| if ( | ||
| !shellModeActive && |
There was a problem hiding this comment.
[Suggestion] No test for keystroke-burst drag detection path
The non-bracketed-paste drag detection (burst timing + debounce timer + scanBufferTailForDraggedImage) is the most complex new code path but has zero test coverage. The existing drag-drop tests only exercise the bracketed-paste path (\x1b[200~...\x1b[201~), which goes through a completely different branch.
Consider adding a test that simulates rapid single-character keystrokes spelling out an image path, waits for the debounce timer, and asserts addImageAttachment is called with the detected path.
— qwen3.7-max via Qwen Code /review
8 participants
Summary
Adds three new ways to attach images to the prompt and unifies them with the existing Cmd+V path under a single
[Image #N]placeholder UX.Closes #3518
What's new
data:image/<type>;base64,<payload>(devtools "Copy as data URL", chat messages containing embedded images, etc.).< 10 mskeystroke-gap heuristic + a 150 ms debounced scan ofbuffer.text's trailing token.Unified UX
All four sources — Cmd+V binary clipboard (existing), data URL, raw base64, drag-drop — converge on:
[Image #N]placeholder inserted at the cursor.[Image #N]→@<relative path>substitution at submit time so the model sees each image exactly where the user placed it.Attachments added without a placeholder (legacy code paths) keep rendering as
[filename], so other callers are unchanged.Code
clipboardUtils.ts:tryDecodeBase64Image(text)— data URL + raw base64, magic-byte sniff.saveDecodedImage(buf, ext, dir)— persists decoded bytes to the shared clipboard temp dir.detectDraggedImagePath(text)— validates a token is an existing local image file; supports single/double-quoted paths and escaped spaces.clipboardHasImage/saveClipboardImage/cleanupOldClipboardImagesunchanged.InputPrompt.tsx:pasteImage→ data URL / raw base64 → drag-drop paste path → large paste placeholder → small paste.DRAG_BURST_MAX_INTERVAL_MS = 10,DRAG_MIN_BURST_CHARS = 4,DRAG_CHECK_DEBOUNCE_MS = 150).[Image #N]allocator +[Image #N]→@pathsubstitution inhandleSubmitAndClear.Test plan
npm test --workspace packages/cli -- clipboardUtils InputPrompt— 143 pass, 1 Windows-skip, 0 failtryDecodeBase64Image: data URL, declared-vs-sniffed MIME (magic wins), raw base64 PNG, raw base64 JPEG, rejects text / JWT-like / empty / short / non-base64 data URL.saveDecodedImageround-trips bytes to disk.detectDraggedImagePath: existing file, quoted path, escaped-space path, non-image extension, missing file, directory, empty input, multi-line.InputPrompt > base64 / data URL paste: single paste, sequential[Image #1]/[Image #2], fall-through to large-paste placeholder for non-images.InputPrompt > drag-and-drop image paste: image path →[Image #1], non-image path → regular buffer handling.npm run build --workspace packages/clipasses.Backwards compatibility
clipboardHasImage,saveClipboardImage,cleanupOldClipboardImages).[filename]when no placeholder is set.@pathmentions outside the image paste flow.Notes
@teddyzhu/clipboardbinding issues on some platforms (Clipboard image paste (Cmd+V) silently fails on macOS — two root causes #3517). This PR leaves that path untouched.