feat(composer): drop non-image files as filesystem paths - #7749
Conversation
Dropping an audio file, PDF or video onto the composer was rejected with 'Please attach image files only'. Images still become attachments; everything else now inserts its filesystem path as text, so the agent opens the file from disk itself and a large recording never crosses the wire — the same thing dropping a file into a terminal does. The renderer cannot read a dropped file's path (Electron removed File.path in v32), so preload exposes webUtils.getPathForFile through the desktop bridge. In a browser tab, where no path exists, the drop says so instead of failing silently. Note the deliberate absence of focusComposer() on the path branch: focusing synchronously after the insert makes the not-yet-reconciled Lexical editor sync its stale empty state back over the text, which is the same footgun makeComposerMentionDragHandlers documents for the file-tree mention drop.
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
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 |
github-actions
Bot
added
vouch:unvouched
![macroscopeapp[bot]](https://avatars.githubusercontent.com/in/900172?s=60&v=4){kind=small}
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
| if (text.length === 0) { | ||
| setThreadError(activeThreadId, "Could not read the location of the dropped file(s)."); | ||
| return; | ||
| } |
There was a problem hiding this comment.
Mixed drops overwrite image errors
Medium Severity
When a drop mixes images and non-images, insertDroppedFilePaths always calls setThreadError on path failure and can replace a message just set by addComposerImages. In the browser, a successful image attach still shows the desktop-only path error, which reads like the whole drop failed and can lead to retrying with duplicate attachments.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 602ce81. Configure here.
There was a problem hiding this comment.
All three are fair — fixed in 82f2be0. Path failures now go to a toast instead of setThreadError, so they can't overwrite the banner the image attach owns or make a successful image attach read as a failed drop. The 'composer is busy' refusal is suppressed when the same drop also had images, since that refusal comes from the state that already rejected them. And insertDroppedFilePaths now returns whether it inserted, so focusComposer() is skipped only when text actually landed — the stale-state rationale only applies to a successful insert. Re-verified the drop end to end after the refactor.
| // documents in makeComposerMentionDragHandlers. | ||
| insertDroppedFilePaths(nonImages); | ||
| return; | ||
| } |
There was a problem hiding this comment.
Mixed drops fire duplicate toasts
Low Severity
A mixed image and non-image drop while plan questions are pending triggers two separate error toasts: addComposerImages rejects with its plan-questions message, then insertDroppedFilePaths fails insertComposerTextAtEnd and toasts that the composer is busy.
Additional Locations (2)
Reviewed by Cursor Bugbot for commit 602ce81. Configure here.
There was a problem hiding this comment.
All three are fair — fixed in 82f2be0. Path failures now go to a toast instead of setThreadError, so they can't overwrite the banner the image attach owns or make a successful image attach read as a failed drop. The 'composer is busy' refusal is suppressed when the same drop also had images, since that refusal comes from the state that already rejected them. And insertDroppedFilePaths now returns whether it inserted, so focusComposer() is skipped only when text actually landed — the stale-state rationale only applies to a successful insert. Re-verified the drop end to end after the refactor.
| // documents in makeComposerMentionDragHandlers. | ||
| insertDroppedFilePaths(nonImages); | ||
| return; | ||
| } |
There was a problem hiding this comment.
Failed path insert skips focus
Low Severity
The non-image branch always returns after insertDroppedFilePaths, which skips focusComposer even when no text was inserted. The Lexical stale-state rationale only applies after a successful applyPromptReplacement; on path failure with images still attaching, focus never runs.
Reviewed by Cursor Bugbot for commit 602ce81. Configure here.
There was a problem hiding this comment.
All three are fair — fixed in 82f2be0. Path failures now go to a toast instead of setThreadError, so they can't overwrite the banner the image attach owns or make a successful image attach read as a failed drop. The 'composer is busy' refusal is suppressed when the same drop also had images, since that refusal comes from the state that already rejected them. And insertDroppedFilePaths now returns whether it inserted, so focusComposer() is skipped only when text actually landed — the stale-state rationale only applies to a successful insert. Re-verified the drop end to end after the refactor.
Two ways the formatter could hand the agent a path that does not exist:
- trimming every entry destroyed a leading or trailing space, which is legal in
a POSIX filename ('/tmp/report ' became '/tmp/report'). Emptiness is now
tested on a trimmed copy while the original path is what gets inserted.
- a double quote is also legal in a filename, so wrapping '/tmp/a " b.pdf' in
quotes made the inner quote read as the closing delimiter. Inner quotes are
escaped before wrapping.
github-actions
Bot
added
size:L
ApprovabilityVerdict: Not approved Macroscope's review found this PR not approvable — This PR introduces new user-facing feature behavior (non-image file drops now insert filesystem paths) with new IPC bridge methods, new utility logic, and modified composer drop handling. New features warrant human review to ensure the behavior meets product expectations. You can add or adjust custom eligibility rules. Learn more. |
A mixed drop (images + other files) reported path failures through setThreadError, the single banner the image attach path also writes: a successful image attach alongside an unsupported path read as though the whole drop had failed. Path failures now go to a toast, which coexists with the banner, and the 'composer is busy' refusal is suppressed when images were in the same drop, since that refusal comes from the state that already rejected them — one drop never says it twice. insertDroppedFilePaths now reports whether it inserted, so focusComposer() is skipped only when text actually landed. The stale-state rationale applies to a successful insert; on failure there is nothing to lose and focus behaves as it always did.
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.
There are 4 total unresolved issues (including 3 from previous reviews).
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 82f2be0. Configure here.
Suppressing the busy toast for any drop that contained an image assumed the image path had already reported the same refusal. It only does for pending plan questions — the one guard addComposerImages shares with insertComposerTextAtEnd. While connecting, awaiting approval, or with no project selected, a mixed drop would attach the images and drop the paths without a word, which is the silent failure this branch exists to avoid. The suppression now requires that shared condition to actually hold.
1 participant
What changed
Dropping an audio file, PDF or video onto the composer is rejected with "Unsupported file type … Please attach image files only." Images still become attachments; everything else now inserts its filesystem path as text, so the agent opens the file from disk itself and a 40MB recording never has to cross the wire as an attachment — the same thing dropping a file into a terminal does.
ChatComposer.tsx:addDroppedFilessplits the drop — images keep the existing attachment path, non-images go toinsertDroppedFilePaths, which resolves each path and appends it to the prompt.droppedFilePaths.ts(new, + unit tests): formats the paths — quotes one containing whitespace so where it ends stays unambiguous, skips unresolvable entries.preload.ts/contracts/ipc.ts: the renderer cannot read a dropped file's path (Electron removedFile.pathin v32), so preload exposeswebUtils.getPathForFilethrough the desktop bridge as an optional member. In a browser tab, where no path exists, the drop says so instead of failing silently.One subtlety worth flagging for review: the path branch deliberately does not call
focusComposer(). Focusing synchronously after the insert makes the not-yet-reconciled Lexical editor sync its stale empty state back over the text — the insert reports success and the composer stays empty. That is the same footgunmakeComposerMentionDragHandlersalready documents for the file-tree mention drop, which is why that path does not focus either;applyPromptReplacementfocuses on the next frame instead.Why
Attaching a voice memo, a PDF or a screen recording is a normal thing to want, and the agent can already read any of them from disk — it just needed the path. Sending the bytes would be the expensive way to do the same thing.
+126 / −2 across five files, no new dependencies.
Before / after
Dropping
standup note.m4aonto the composer:Test plan
vp test(238 web unit tests incl. 5 new fordroppedFilePaths),tsgo --noEmitforapps/webandapps/desktop,vp lint,vp fmt --check— all greenNote
Low Risk
Composer UX plus a small optional preload helper; no auth, IPC command, or file-content handling changes. Browser drops now toast instead of ignoring non-images.
Overview
Dropping PDFs, audio, video, and other non-images onto the composer now inserts their OS paths as prompt text so the agent can open them from disk. Images still attach as before.
The desktop preload exposes
getPathForFilevia ElectronwebUtils(needed afterFile.pathwas removed). Paths with whitespace are quoted. Browser tabs toast that path attach needs the desktop app instead of failing silently.Focus is skipped after a successful path insert so Lexical does not overwrite the new text with a stale empty editor state.
Reviewed by Cursor Bugbot for commit 4186b1a. Bugbot is set up for automated code reviews on this repo. Configure here.
Note
Insert dropped non-image files as filesystem paths in
ChatComposerdesktopBridge.getPathForFilevia ElectronwebUtilsin preload.ts to resolve aFileobject's OS path, returningnullon error or empty resultsformatDroppedFilePathsandquoteDroppedFilePathin droppedFilePaths.ts to quote paths containing whitespace and join valid entries into a single stringChatComposer.addDroppedFilesin ChatComposer.tsx to split dropped files into images (attached as before) and non-images (inserted as formatted path text); errors are surfaced via toastsMacroscope summarized 82f2be0.