Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 10 additions & 1 deletion apps/desktop/src/preload.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import type {
DesktopPreviewTabState,
} from "@t3tools/contracts";
import { exposeClerkBridge } from "@clerk/electron/preload";
import { contextBridge, ipcRenderer } from "electron";
import { contextBridge, ipcRenderer, webUtils } from "electron";

import * as IpcChannels from "./ipc/channels.ts";

Expand Down Expand Up @@ -35,6 +35,15 @@ contextBridge.exposeInMainWorld("desktopBridge", {
}
return result as ReturnType<DesktopBridge["getAppBranding"]>;
},
getPathForFile: (file: File) => {
// Throws for a File that never came from the OS (e.g. built by the page).
try {
const path = webUtils.getPathForFile(file);
return path.length > 0 ? path : null;
} catch {
return null;
}
},
getSystemLocale: () => {
const result = ipcRenderer.sendSync(IpcChannels.GET_SYSTEM_LOCALE_CHANNEL);
return typeof result === "string" ? result : null;
Expand Down
63 changes: 61 additions & 2 deletions apps/web/src/components/chat/ChatComposer.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,7 @@ import {
submitComposerDraft,
} from "./composerSubmission";
import { ComposerPromptLengthValidation } from "./ComposerPromptLengthValidation";
import { formatDroppedFilePaths } from "./droppedFilePaths";

type ComposerCommandMenuPosition = {
bottom: number;
Expand Down Expand Up @@ -2563,6 +2564,48 @@ export const ChatComposer = memo(function ChatComposer(props: ChatComposerProps)
void addComposerImages(imageFiles);
};

/**
* Insert dropped non-image files as filesystem paths, reporting to a toast
* rather than the thread error banner: a mixed drop's images own that banner,
* and a path failure there would read as though the whole drop failed. Only
* the desktop app can resolve a path from a dropped File, so in a browser tab
* this says so instead of silently swallowing the drop. Returns whether text
* was actually inserted.
*/
const insertDroppedFilePaths = (files: File[], hadImages: boolean): boolean => {
const reportFailure = (description: string) => {
toastManager.add({ type: "error", title: "Unable to add to chat", description });
};
const resolvePath = window.desktopBridge?.getPathForFile;
if (!resolvePath) {
reportFailure(
"Attaching files by path needs the desktop app. Paste an image, or type the path.",
);
return false;
}
const paths = files
.map((file) => resolvePath(file))
.filter((path): path is string => path !== null);
const text = formatDroppedFilePaths(paths);
if (text.length === 0) {
reportFailure("Could not read the location of the dropped file(s).");
return false;
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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)
Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 602ce81. Configure here.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

if (!insertComposerTextAtEnd(text, { ensureLeadingBoundary: true })) {
// Pending plan questions is the ONLY refusal `addComposerImages` shares
// with the insert, so that is the only case a mixed drop has already been
// told about. Staying silent for the others — connecting, approval,
// project selection — would attach the images and drop the paths without
// a word, which is the failure this whole branch exists to avoid.
const alreadyReported = hadImages && pendingUserInputs.length > 0;
if (!alreadyReported) {
reportFailure("The composer is busy; try again once it is ready.");
}
Comment thread
cursor[bot] marked this conversation as resolved.
return false;
}
return true;
};

const insertComposerTextAtEnd = (
text: string,
options?: { ensureLeadingBoundary?: boolean },
Expand Down Expand Up @@ -2686,8 +2729,24 @@ export const ChatComposer = memo(function ChatComposer(props: ChatComposerProps)
composerEditorRef.current?.focusAt(cursor);
},
addDroppedFiles: (files: File[]) => {
void addComposerImages(files);
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/"));
const nonImages = files.filter((file) => !file.type.startsWith("image/"));
if (images.length > 0) {
void addComposerImages(images);
}
const insertedPath =
nonImages.length > 0 && insertDroppedFilePaths(nonImages, images.length > 0);
// Focus unless a path just landed. `applyPromptReplacement` focuses on the
// next frame, once Lexical has reconciled; focusing synchronously right
// after the insert makes the not-yet-reconciled editor sync its stale empty
// state back over the text, so the drop looks like it silently did nothing
// — the footgun makeComposerMentionDragHandlers documents for the mention
// drop. Nothing inserted means nothing to lose, so focus as before.
if (!insertedPath) {
focusComposer();
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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)
Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 602ce81. Configure here.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 602ce81. Configure here.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

},
insertTextAtEnd: insertComposerTextAtEnd,
openModelPicker: () => {
Expand Down
41 changes: 41 additions & 0 deletions apps/web/src/components/chat/droppedFilePaths.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
import { describe, expect, it } from "vite-plus/test";

import { formatDroppedFilePaths, quoteDroppedFilePath } from "./droppedFilePaths";

describe("quoteDroppedFilePath", () => {
it("leaves a path without whitespace alone", () => {
expect(quoteDroppedFilePath("/Users/me/notes.pdf")).toBe("/Users/me/notes.pdf");
});

it("escapes a double quote inside a quoted path", () => {
expect(quoteDroppedFilePath('/tmp/a " b.pdf')).toBe('"/tmp/a \\" b.pdf"');
});

it("leaves a double quote alone when there is no whitespace to quote for", () => {
expect(quoteDroppedFilePath('/tmp/a"b.pdf')).toBe('/tmp/a"b.pdf');
});

it("quotes a path containing spaces", () => {
expect(quoteDroppedFilePath("/Users/me/Voice Memos/note 1.m4a")).toBe(
'"/Users/me/Voice Memos/note 1.m4a"',
);
});
});

describe("formatDroppedFilePaths", () => {
it("joins several paths with a space", () => {
expect(formatDroppedFilePaths(["/a/one.opus", "/b/two.pdf"])).toBe("/a/one.opus /b/two.pdf");
});

it("preserves a trailing space in a filename instead of trimming it", () => {
expect(formatDroppedFilePaths(["/tmp/report "])).toBe('"/tmp/report "');
});

it("skips empty and whitespace-only entries", () => {
expect(formatDroppedFilePaths(["", " ", "/a/one.opus"])).toBe("/a/one.opus");
});

it("returns an empty string when nothing resolved", () => {
expect(formatDroppedFilePaths([])).toBe("");
});
});
38 changes: 38 additions & 0 deletions apps/web/src/components/chat/droppedFilePaths.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
/**
* Files dropped from the OS that aren't images are handed to the agent by
* *path*, not by content: it can open the file itself, so a 40MB recording or
* a PDF never has to cross the wire as an attachment. This mirrors dropping a
* file into a terminal, where the shell receives the path.
*
* Paths are only available in the desktop app (Electron's `webUtils`); in a
* browser tab the File object carries no filesystem path at all.
*/

/**
* Quote a path for the prompt when whitespace would make where it ends
* ambiguous. This is prompt text, not a shell command — the goal is a clear
* boundary for the reader, not shell-injection safety.
*/
export function quoteDroppedFilePath(path: string): string {
if (!/\s/.test(path)) {
return path;
}
// A double quote is legal in a POSIX filename, so escape any before wrapping —
// otherwise the first inner quote reads as the closing delimiter.
return `"${path.replace(/"/g, '\\"')}"`;
}

/**
* The text inserted into the composer for a set of dropped paths. Entries that
* are empty or all whitespace are skipped (a bridge that can't resolve a file
* returns null, which the caller filters, but be defensive about "" too).
* A path that survives is never altered: leading and trailing spaces are legal
* in a filename, so trimming one would point the agent at a file that does not
* exist. Quoting keeps such a path readable instead.
*/
export function formatDroppedFilePaths(paths: ReadonlyArray<string>): string {
return paths
.filter((path) => path.trim().length > 0)
.map(quoteDroppedFilePath)
Comment thread
macroscopeapp[bot] marked this conversation as resolved.
.join(" ");
}
7 changes: 7 additions & 0 deletions packages/contracts/src/ipc.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1070,6 +1070,13 @@ export interface DesktopBridge {
* regardless of OS settings.
*/
getSystemLocale?: () => string | null;
/**
* The filesystem path of a dropped/pasted `File`, which the renderer cannot
* read for itself (Electron removed `File.path` in v32). Returns null when
* the object has no path — e.g. a file synthesised in-page rather than
* dragged in from the OS.
*/
getPathForFile?: (file: File) => string | null;
// One bootstrap per pool instance currently registered with bootstrap
// info (omits instances whose backend hasn't produced a config yet).
// The primary backend is identified by id === PRIMARY_LOCAL_ENVIRONMENT_ID.
Expand Down
Loading