Repository navigation
Conversation
…cover native chat paste - Accept Finder's filename text without its extension (screenshots hide it). - Compare NFC-normalized text and path; NSURL file URLs decode to NFD. - Read public.file-url first and skip the text read when no file was copied. - Native chat composer tries the copied image file before readImage (file icon).
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (17)
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 8 remain after this review. 📝 WalkthroughWalkthroughThe change adds bounded image-file reading, macOS clipboard file validation, and an IPC and preload API for saving copied clipboard images. Windows clipboard image reading now uses the shared reader. Native chat and terminal paste flows try the copied-file API before existing clipboard handling. The web API returns Priority: ➖ Normal Merge Risk: ⚪ Minimal · up to The copied-image paste change is mergeable after normal checks. A real Finder copy has not been tested manually, but no specific failure is established. Security Architecture ReviewSecurity architecture risk: 🟡 Moderate · up to Copied image files can now be read and pasted into local or remote sessions. Platform, window-trust, file-size, and image checks limit the new path, but its reliance on clipboard-provided file locations warrants review. Retained concerns
Security review detailsSecurity Blast Radius
Security Findings and Attack Paths
Trust Boundaries and Controls
Resilience and Maintainability Implications
Hardening Proposals
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![pullfrog[bot]](https://avatars.githubusercontent.com/in/1768019?s=60&v=4){kind=small}
There was a problem hiding this comment.
ℹ️ No critical issues — one coverage gap and two small robustness notes.
Reviewed changes
- macOS copied-file probe: new
clipboard:saveCopiedImageFileAsTempFileIPC readspublic.file-urlfirst, validates a single copied image file, and reuses the bounded target-aware saver (SSH/runtime included); returns null on Windows/Linux/popout/web. - Shared reader: the Windows file→PNG logic moved verbatim to
clipboard-image-file-read.ts; Windows now delegates to it. - Matcher:
clipboard-mac-image-file.tsaccepts the full path, basename, or extensionless Finder display name (NFC-normalized on both sides) and rejects multi-selections. - Terminal paste: probes the copied file before the text read across all three entry points; pastes the returned path as an image.
- Native chat composer: tries the copied-file handler, then falls back to
saveClipboardImageAsTempFile. - Preload/web: method exposed; web stub returns null.
The public.file-url and "readImage() is the file icon" premises are confirmed against Chromium's macOS clipboard source.
ℹ️ The markdown editor's image paste still misses Finder copies
rich-markdown-paste-image.ts is a third image-paste surface and still calls only saveClipboardImageAsTempFile, so a Finder-copied screenshot pasted into a note will not get the new behavior. If that path is reachable for a Finder copy (it gates on an image item in the ClipboardEvent), the same icon-vs-file problem remains there.
Technical details
# Finder-copied image files in the markdown editor
## Affected sites
- `src/renderer/src/components/editor/rich-markdown-paste-image.ts:78` — calls
`window.api.ui.saveClipboardImageAsTempFile({ connectionId })`, which on macOS
returns the clipboard icon for a Finder copy.
- `src/renderer/src/components/editor/rich-markdown-paste-handler.ts:29` — routes
the paste here before the text fallbacks.
## Required outcome
- Either route this path through `saveCopiedClipboardImageFileAsTempFile` first
(same fallback shape as the composer), or state explicitly that the markdown
editor is out of scope for this fix.
## Open questions for the human
- Is the markdown-editor paste surface in scope for this bug? The report names
the terminal/composer, but the root cause is identical.ℹ️ Nitpicks
src/main/window/clipboard-ipc-handlers.ts:164—clipboard.read('public.file-url')(and'NSFilenamesPboardType'at line 171) use raw pasteboard types with no/, which the publishedclipboard.read(format)contract calls invalid; the macOS backend accepts them today and Electron's own spec readsNSFilenamesPboardType, so this is only a future-upgrade risk. A short comment noting the reliance would help a future reader.
DeepSeek Flash (free via Pullfrog for OSS) ｜ 𝕏
| return null | ||
| } | ||
| // Why: public.file-url exposes only the first item of a Finder multi-selection. | ||
| if ((filenamesPlist.match(/<string>/g)?.length ?? 0) > 1) { |
There was a problem hiding this comment.
The multi-file guard counts <string> occurrences in NSFilenamesPboardType, but that pasteboard type has been deprecated since macOS 10.14 and Chromium no longer reads it; if Finder stops writing it (or writes a binary plist instead of XML), the guard goes inert and a multi-selection would fall through to the text check. Since public.file-url is documented as first-item-only, it is worth confirming on a real multi-select that the text fallback genuinely rejects it (i.e. the clipboard text is the newline-joined list of all names), or counting the file URLs instead. The ignores a multi-file selection test only covers the XML representation.
|
This still reproduces in the macOS desktop app on 1.4.224, paired to a Linux Orca Server with Codex in terminal mode. Cmd+V after copying one screenshot file in Finder inserts its filename rather than an image. A read-only native pasteboard check found one item with I refreshed this PR's copied-file-first fix against main at The refreshed patch passes 207 clipboard and terminal paste tests (one existing benchmark skipped), the full typecheck, the changed-code quality gate, and the Electron Vite build on macOS ARM64 with Node 24. Three screenshot regression cases fail against unpatched main and pass with the refresh, including using a paired runtime's returned image path. The remote upload checks are automated; a live two-machine paste and rendered desktop verification have not been performed. Could the existing PR be refreshed with this patch so it can land alongside #27346 and #27605? I would prefer to help this PR land rather than open a competing one. |
|
I refreshed the existing adaptation again against main The refresh retains main's upload-cleanup subscription and passes its new attachment-store lifecycle options through the copied-image callback. It also extracts clipboard file helpers to stay within the 300-line limit. A new regression showed that an opaque filename list could otherwise be treated as one image; the copied-image path now declines that case instead of silently taking only the first file. On this adaptation, 60 focused tests, Node typecheck and scoped lint pass. Renderer typecheck passed before the subsequent main-process helper extraction and filename-list changes. A fresh app/CLI build and a real Electron preload-to-main IPC test pass: a selected PNG is saved with the expected dimensions, and a two-file XML list is refused. That test replaces the clipboard read functions and does not change the user's clipboard. Separately, a native named scratch pasteboard confirmed one/two NSURL items produce one/two XML filename entries. This is not a real Finder Cmd+V or live paired-machine acceptance claim, and it does not establish support for binary filename plists. The original PR head has not been retested by this run. The adaptation now includes paired application acceptance in e6f52be12. A hidden desktop client pairs to an isolated headless host. Shipping preload/main IPC reads a synthetic selected PNG, uploads it through the real chunked pairing transport and writes identical PNG bytes into a dedicated host-side temp directory. XML multi-selection, opaque binary filename data and multiple newline-separated names without a legacy plist all return null without creating another host file. No direct SSH attempt occurs. Clipboard read functions alone are substituted; the user's OS clipboard is untouched. The first new fixture run failed before upload because Playwright's main-process evaluation does not support the dynamic import used to create a test directory. Moving directory creation into the test runner fixed setup; the same destination/byte/refusal assertions then pass with retries disabled. Scoped lint passes, and the failed trace is retained. The existing 82 native-chat tests pass on the adaptation: current main's generic copied-file intake supersedes the original chat-specific probe, so this does not add a duplicate chat implementation. This is support for the existing PR, not a competing submission. The adaptation is a credited source port without original-head ancestry, and its results are not acceptance of the original head. Opaque binary lists are refused, not decoded. Two paired application processes on one Mac do not establish physical two-machine or real Finder Cmd+V acceptance; native-focus testing still requires an isolated desktop. |
Description
On macOS, pasting a copied screenshot into an Orca terminal (or the native chat composer) could paste the file's name, or nothing useful, instead of the image. A user reported "I can't paste a screenshot from my clipboard."
There are two causes:
pasteTerminalClipboardpastes any text it finds and only asks for an image when the clipboard has no text. When you copy a screenshot file in Finder (Cmd+C), the clipboard holds the file's URL and its name as plain text, so the name wins. Screenshot tools that put both a PNG and a path string on the clipboard hit the same rule.clipboard:saveImageAsTempFilealready converts a copied image file into a PNG, but only on Windows (readWindowsClipboardImageFileAsPng). On macOS, Electron'sclipboard.readImage()for a Finder-copied file returns the 1024x1024 generic file icon, not the screenshot.What Changed
New main-process handler
clipboard:saveCopiedImageFileAsTempFile(macOS only; returns null on Windows, on Linux, and in the dashboard popout window). It readspublic.file-urlfirst and returns early if the clipboard has nofile://URL, so a normal text paste costs one small extra read. If there is a URL, it accepts it only when all of the following are true:NSFilenamesPboardType);.png,.jpgor.jpegat an absolute path that exists;The file is then loaded with the existing size, dimension and pixel-bomb checks and saved with the existing target-aware saver, so SSH and remote-runtime pastes upload it the same way image pastes already do.
Shared file reader. The file-to-PNG reader and its safety limits moved unchanged from
clipboard-windows-image-file.tsintoclipboard-image-file-read.ts. The Windows path and the new macOS path both use it.Terminal paste (
terminal-clipboard-paste.ts). Before reading text, it asks the new handler whether the clipboard holds a single copied image file. If so, it pastes the image path, just like a normal image paste. Otherwise the old text-first behaviour is unchanged. All three entry points pass the handler through: keyboard / paste-event, paste listener, and app menu.Native chat composer (
use-native-chat-composer-paste.ts). It tries the copied-file handler first and falls back to the oldsaveClipboardImageAsTempFile, so a Finder-copied screenshot is attached as the screenshot instead of the file icon. The SSHconnectionIdis passed through.Preload and web stub. The preload exposes the new method. The web client stub returns null, so the web client keeps its old behaviour.
Why
Alternative considered: always prefer an image over text. This would break the case where copying rich text also puts an image on the clipboard. On macOS it would still paste the file icon for Finder copies, because that is what
readImage()returns.What this PR does instead. It only changes the result when the clipboard text is just the name of the one copied image file. That is how Finder copies a file. Any other text (real content) still wins, so normal text paste is not affected.
Why reuse the Windows reader and target-aware saver. They already solve "copied image file becomes a pasted image" safely: they enforce size limits and handle SSH and runtime targets. Reusing them avoids writing a second version of that logic.
Fix evidence
1. Real macOS pasteboard probe (Electron 43.7.0, the same version as v1.4.215; main process only, no window,
ORCA_BACKGROUND_LAUNCH=1).A Swift helper wrote four clipboard shapes for a 640x400 PNG named
Screenshot 2026-09-28 at 09.30.00.png. The probe then applied the v1.4.215 decision logic:Screenshot 2026-09-28 at 09.30.00.png(andreadImage()is the 1024x1024 file icon)2. Unit tests: fail before, pass after.
Command (renderer paste logic reverted to
origin/main, new tests kept):Before:
After, on this branch, adding the main-process tests:
These main-process tests cover the matcher and the handler:
reads the image file behind a Finder Cmd+C (file URL + filename text)accepts the extension-hidden Finder display name as textmatches an NFD file URL against NFC filename text(uses a real file with a decomposed-form name)ignores a copied image file that no longer existssaves a Finder-copied image file for the paste target on macOSskips the text read when no file URL is on the clipboardRemoving the NFC normalization and the extension-less match makes 2 of these tests fail; this was verified in review.
3. Broader runs.
pnpm test src/main/window/ src/renderer/src/components/native-chat/ src/renderer/src/components/terminal-pane/terminal-clipboard-paste.test.tsgave 2405 passed and 1 skipped.pnpm tc,oxlinton the changed files, andpnpm run check:code-quality:changedare all clean.ELI5
When you copy a screenshot file in Finder, the Mac clipboard holds a note that says "this file" plus a label with the file's name. Orca read the label first and typed the name, instead of opening the file and handing over the picture. Now, if the label is just the name of that one copied picture, Orca opens the picture and pastes it like a normal image. If the clipboard holds real text, Orca still pastes the text.
Trade-offs
.png/.jpginto a plain shell (no agent running) now inserts a temporary file path such as/var/folders/.../orca-paste-*.pnginstead of the file name.file:///.file/id=…) inpublic.file-url, the extension check fails and paste falls back to the old text behaviour. That is no worse than today.Adversarial review
2 loops, clean on loop 2.
Loop 1: not clean, 5 findings.
.png. Fixed: the matcher also accepts the name without its extension.Loop 2: clean, no defects. The reviewer checked:
pnpm tc, oxlint and the code-quality gate are clean, and the branch merges cleanly with main.Linked crash reports
Linked Issue
None. This came from a crash/diagnostic report.
Visual Proof
N/A. Showing the change on screen would need a real Finder Cmd+C and a foreground window, which this background-only validation cannot do. The behaviour is shown by the pasteboard probe and the unit tests above.
Testing
Platforms: macOS, using unit tests and a real-pasteboard probe with Electron 43.7.0. Windows and Linux paths return null from the new handler and are covered by the existing tests.
Notes
Checklist
N/Awith reasonpnpm lint,pnpm typecheck,pnpm test, andpnpm buildpass (or CI will cover; local preferred)