Uh oh!
There was an error while loading. Please reload this page.
feat(composer): drop non-image files as filesystem paths - #8549
Conversation
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:
Warning Your free Security trial is over. An organization admin can activate Security or dismiss this notice. Comment |
| focusComposer(); | ||
| // Images become attachments; everything else (audio, PDF, video, …) is | ||
| // handed over as a path so the agent opens it from disk itself. | ||
| const images = files.filter((file) => file.type.startsWith("image/")); |
There was a problem hiding this comment.
🟠 Highchat/ChatComposer.tsx:2882
Dropped HEIC files with an empty or application/octet-stream MIME type are treated as non-images, so desktop inserts their filesystem path instead of attaching/converting them and browser drops reject them. Use isHeicImageFile in both filters, matching the supported image handling elsewhere.
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/web/src/components/chat/ChatComposer.tsx around line 2882:
Dropped HEIC files with an empty or `application/octet-stream` MIME type are treated as non-images, so desktop inserts their filesystem path instead of attaching/converting them and browser drops reject them. Use `isHeicImageFile` in both filters, matching the supported image handling elsewhere.
| readonly primaryEnvironmentId: string | undefined; | ||
| readonly threadEnvironmentId: string | undefined; | ||
| }): boolean { | ||
| if (input.primarySource !== "desktop-managed") { |
There was a problem hiding this comment.
🟠 Highchat/droppedFilePaths.ts:35
In WSL-only mode, this predicate returns true for matching desktop-managed IDs and allows a Windows path such as C:\Users\... into a Linux agent prompt, where the agent cannot open it. The check must distinguish a WSL primary (or translate the path) before permitting dropped-path insertion.
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/web/src/components/chat/droppedFilePaths.ts around line 35:
In WSL-only mode, this predicate returns `true` for matching `desktop-managed` IDs and allows a Windows path such as `C:\Users\...` into a Linux agent prompt, where the agent cannot open it. The check must distinguish a WSL primary (or translate the path) before permitting dropped-path insertion.
There was a problem hiding this comment.
One finding: the new image/non-image split in addDroppedFiles drops HEIC photos out of the attachment path they previously took.
Posted via Macroscope — UI Consistency
| const images = files.filter((file) => file.type.startsWith("image/")); | ||
| const nonImages = files.filter((file) => !file.type.startsWith("image/")); |
There was a problem hiding this comment.
This split uses only file.type.startsWith("image/"), but Finder and some browsers omit the MIME type when dragging HEIC photos (see the comment on isHeicImageFile). A dropped IMG_1234.HEIC with type: "" now lands in nonImages: previously addComposerImages accepted it and converted it to a JPEG attachment, and the paste handler above still does. In a browser tab or a remote-environment thread the photo is now discarded with an "Unable to add to chat" toast instead of being attached.
Consider reusing the same predicate as onComposerPaste so HEIC keeps the attachment route:
| constimages=files.filter((file)=>file.type.startsWith("image/")); | |
| constnonImages=files.filter((file)=>!file.type.startsWith("image/")); | |
| constisDroppedImage=(file: File)=> | |
| file.type.startsWith("image/")||isHeicImageFile(file); | |
| constimages=files.filter(isDroppedImage); | |
| constnonImages=files.filter((file)=>!isDroppedImage(file)); |
Posted via Macroscope — UI Consistency
Uh oh!
There was an error while loading. Please reload this page.
ApprovabilityVerdict: Not approved Macroscope's review found this PR not approvable — This PR adds a user-facing filesystem-path workflow to the existing composer drop path, spanning Electron preload integration, prompt content, and local-versus-remote environment handling. Unresolved findings identify concrete classification and cross-platform path issues, including a condition that currently prevents the new path branch from running. Not approved because:
Adjust the Minimum Blocking Severity for this repo — including turning it Off — in Settings. You can add or adjust custom eligibility rules. Learn more. |
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.
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.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.
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.
The drop resolved a path through the local desktop bridge and inserted it regardless of where the thread actually runs. On a remote environment — a LAN or Tailscale host, a relay, a tunnel, another desktop acting as server — that path names a file on the wrong machine, so the agent either cannot find it or finds a different one. Gate the insert on the thread's environment being the one this desktop app supervises (`desktop-managed`, matching environment ids) rather than on the bridge merely existing, and say so plainly when it is not. contracts/ipc.ts already states the rule this broke: resolve by environmentId rather than reaching through the local desktop bridge. The predicate is pure and covered: matching ids, mismatched ids, each non-desktop source, and either id still unknown (which refuses — bootstrapping is not evidence the thread runs here).
920ff0b to
f52c963CompareThere was a problem hiding this comment.
One concrete issue in the dropped-file split in ChatComposer.addDroppedFiles: the video guard compares against undefined, which is always true, so the new path-insertion branch is unreachable. Details inline.
Posted via Macroscope — UI Consistency
| const attachable = files.filter( | ||
| (file) => | ||
| file.type.startsWith("image/") || | ||
| videoMimeType({ name: file.name, mimeType: file.type }) !== undefined, | ||
| ); |
There was a problem hiding this comment.
videoMimeType returns string | null (apps/web/src/types.ts), never undefined, so videoMimeType(...) !== undefined is always true. Every dropped file therefore lands in attachable, byPath is always empty, and insertDroppedFilePaths never runs — the feature this PR adds is dead at the call site (and the no-overlap comparison should also fail typecheck).
Switching to !== null re-exposes the classification gap flagged earlier: Finder omits the MIME type for IMG_1234.HEIC, so file.type.startsWith("image/") routes it (and a typeless photo.jpg) to the path branch, where a browser tab or a remote-environment thread discards it with an "Unable to add to chat" toast instead of attaching it, unlike onComposerPaste. Reusing the already-imported classifyComposerAttachmentFile keeps drop and paste on the same rules and fixes both:
| constattachable=files.filter( | |
| (file)=> | |
| file.type.startsWith("image/")|| | |
| videoMimeType({name: file.name,mimeType: file.type})!==undefined, | |
| ); | |
| constattachable=files.filter( | |
| (file)=> | |
| classifyComposerAttachmentFile(file)!=="file"|| | |
| videoMimeType({name: file.name,mimeType: file.type})!==null, | |
| ); |
Posted via Macroscope — UI Consistency
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit f52c963. Configure here.
| (file) => | ||
| file.type.startsWith("image/") || | ||
| videoMimeType({ name: file.name, mimeType: file.type }) !== undefined, | ||
| ); |
There was a problem hiding this comment.
Path insert never runs
High Severity
The attachable filter treats every dropped file as attachable because videoMimeType returns null for non-videos, and null !== undefined is always true. byPath stays empty, so insertDroppedFilePaths never runs and non-image drops keep going through addComposerAttachments as before.
Reviewed by Cursor Bugbot for commit f52c963. Configure here.
Reopening #7749, which was closed with this reason:
That was correct, and this fixes it. I could not reopen #7749 through the API (
reopenPullRequestreturns "Could not open the pull request",PATCH state=openreturns 422), hence a fresh PR rather than ignoring the request to explain what was missed.What Changed
Dropping a non-image file (audio, PDF, video, …) on the composer inserts its filesystem path as text, so the agent opens the file itself instead of a 40MB attachment crossing the wire — the same gesture as dropping a file into a terminal. Images are unaffected and still attach.
The insert is gated on the thread's environment being the one this desktop app supervises:
When it refuses, the composer says why rather than dropping the file on the floor:
Why
The earlier version resolved a path through the local desktop bridge and inserted it regardless of where the thread ran. On a remote environment — a LAN or Tailscale host, a relay, a tunnel, another desktop acting as server — that path names a file on the wrong machine, so the agent either cannot find it or, worse, finds a different one.
packages/contracts/src/ipc.tsalready states the rule that broke:desktop-managedis the single environment this app supervises on this machine; every otherKnownEnvironmentSource(configured,manual,window-origin) names a server somewhere else. A missing id on either side also refuses — bootstrapping is not evidence the thread runs here, and guessing wrong reproduces exactly the silent failure that got #7749 closed.Two smaller decisions carried over from the earlier review:
setThreadError. A mixed drop's images own that banner, and a path failure there reads as though the whole drop failed.UI Changes
No visual change. The only new surface is an error toast on a drop that cannot be honored, where the previous behavior was silence.
Test plan
apps/web/src/components/chat/droppedFilePaths.test.ts— 15 tests: quoting (whitespace, embedded quotes, paths that must not be trimmed), formatting, and the environment predicate (matching ids, mismatched ids, each non-desktop source, either id unknown).pnpm --filter @t3tools/web typecheckclean, targeted lint clean.Rebased onto current
main.Checklist
Written with Claude Opus 5 in Claude Code.
Note
Medium Risk
Changes drop handling and prompt content in a high-traffic composer path; environment gating limits wrong-machine paths but incorrect ID matching could still block or allow inserts incorrectly.
Overview
Dropping non-image files on the chat composer (PDFs, audio, archives, etc.) now inserts their local filesystem path into the prompt instead of rejecting them or uploading large blobs—similar to dropping a file into a terminal. Images and video still attach as before; mixed drops handle both paths.
The desktop preload exposes
desktopBridge.getPathForFilevia ElectronwebUtils(replacing removedFile.path), with the contract updated accordingly.Path insertion runs only when the thread’s
environmentIdmatches the desktop-managed primary environment on this machine (droppedPathsResolveInEnvironment). Browser tabs, remote/SSH/relay connections, mismatched environments, or unknown IDs get explicit error toasts instead of silent wrong-path inserts. Paths are quoted when they contain whitespace (without trimming legal trailing spaces).Composer focus is skipped immediately after a successful path insert so Lexical reconciliation does not wipe the new text.
Reviewed by Cursor Bugbot for commit f52c963. Bugbot is set up for automated code reviews on this repo. Configure here.
Note
Insert non-image dropped files as filesystem paths in
ChatComposerdesktopBridge.getPathForFile(file)to the preload bridge, using Electron'swebUtilsto resolve a filesystem path for OS-originatedFileobjects (returnsnullotherwise)ChatComposer.addDroppedFiles: images/videos attach as before; all other types are resolved to local paths and inserted as quoted text via the newinsertDroppedFilePathshelperdroppedPathsResolveInEnvironment(requiresprimarySource === 'desktop-managed'and matching environment ids),quoteDroppedFilePath(wraps paths with whitespace in double quotes), andformatDroppedFilePaths(filters empties and joins with spaces)toastManagerwhen the desktop bridge is absent, the thread environment does not match the local desktop-managed environment, no paths resolve, or the composer is busyMacroscope summarized f52c963.