diff --git a/apps/desktop/src/app/chat/hooks/use-composer-actions.test.ts b/apps/desktop/src/app/chat/hooks/use-composer-actions.test.ts index 55d5bc20380..5f3f91b0402 100644 --- a/apps/desktop/src/app/chat/hooks/use-composer-actions.test.ts +++ b/apps/desktop/src/app/chat/hooks/use-composer-actions.test.ts @@ -1,6 +1,6 @@ -import { describe, expect, it } from 'vitest' +import { afterEach, describe, expect, it, vi } from 'vitest' -import { type DroppedFile, partitionDroppedFiles } from './use-composer-actions' +import { type DroppedFile, extractDroppedFiles, HERMES_PATHS_MIME, partitionDroppedFiles } from './use-composer-actions' // A Finder/Explorer drop carries a native File handle; an in-app drag (project // tree, gutter line ref) is path-only. The split decides whether a drop becomes @@ -39,6 +39,18 @@ describe('partitionDroppedFiles', () => { expect(inAppRefs).toEqual([lineRef]) }) + it('routes an OS folder drop (path-only, isDirectory) to inAppRefs, not the upload pipeline', () => { + // extractDroppedFiles emits a dropped directory as a path-only entry so it + // stays a @folder: ref instead of hitting file.attach, which can't stage a + // directory ("file not found on gateway and no data_url provided"). + const folder = inAppRef('/Users/jeff/projects/hermes', { isDirectory: true }) + + const { inAppRefs, osDrops } = partitionDroppedFiles([folder]) + + expect(osDrops).toEqual([]) + expect(inAppRefs).toEqual([folder]) + }) + it('splits a mixed drop and preserves order within each group', () => { const a = inAppRef('a.ts') const b = osDrop('/abs/b.pdf') @@ -55,3 +67,114 @@ describe('partitionDroppedFiles', () => { expect(partitionDroppedFiles([])).toEqual({ inAppRefs: [], osDrops: [] }) }) }) + +// Minimal DataTransfer stand-in. A real OS drop populates BOTH `items` (which +// alone carries webkitGetAsEntry for folder detection) and `files`; the mock +// mirrors that so the dedup path is exercised too. +interface StubEntry { + path: string + isDirectory: boolean +} + +function stubTransfer(entries: StubEntry[], internalRaw = ''): DataTransfer & { _pathByFile: Map } { + const files = entries.map(entry => new File(['x'], entry.path.split('/').pop() || 'f')) + const pathByFile = new Map(files.map((file, i) => [file, entries[i].path])) + + const items: Record = { length: entries.length } + entries.forEach((entry, i) => { + items[i] = { + kind: 'file' as const, + getAsFile: () => files[i], + webkitGetAsEntry: () => ({ isDirectory: entry.isDirectory, isFile: !entry.isDirectory }) + } + }) + + return { + getData: (mime: string) => (mime === HERMES_PATHS_MIME ? internalRaw : ''), + files: { + length: files.length, + item: (i: number) => files[i] ?? null + }, + items, + _pathByFile: pathByFile + } as unknown as DataTransfer & { _pathByFile: Map } +} + +describe('extractDroppedFiles', () => { + afterEach(() => { + vi.unstubAllGlobals() + }) + + const stubBridge = (transfer: DataTransfer & { _pathByFile: Map }) => { + vi.stubGlobal('window', { + hermesDesktop: { + getPathForFile: (file: File) => transfer._pathByFile.get(file) ?? '' + } + }) + } + + it('emits a dropped directory as a path-only entry with isDirectory (no File to upload)', () => { + const transfer = stubTransfer([ + { path: '/Users/jeff/projects/hermes', isDirectory: true } + ]) as DataTransfer & { _pathByFile: Map } + + stubBridge(transfer) + + const result = extractDroppedFiles(transfer) + + expect(result).toHaveLength(1) + expect(result[0]?.isDirectory).toBe(true) + expect(result[0]?.path).toBe('/Users/jeff/projects/hermes') + // A directory carries no bytes — it must NOT ride the File/upload pipeline. + expect(result[0]?.file).toBeUndefined() + // And it partitions as an in-app ref (→ @folder:), never an OS upload drop. + expect(partitionDroppedFiles(result).osDrops).toEqual([]) + }) + + it('still emits a dropped file with its native File handle for the upload pipeline', () => { + const transfer = stubTransfer([ + { path: '/Users/jeff/Downloads/report.pdf', isDirectory: false } + ]) as DataTransfer & { _pathByFile: Map } + + stubBridge(transfer) + + const result = extractDroppedFiles(transfer) + + expect(result).toHaveLength(1) + expect(result[0]?.isDirectory).toBeFalsy() + expect(result[0]?.path).toBe('/Users/jeff/Downloads/report.pdf') + expect(result[0]?.file).toBeInstanceOf(File) + expect(partitionDroppedFiles(result).osDrops).toHaveLength(1) + }) + + it('classifies a mixed folder+file drop independently', () => { + const transfer = stubTransfer([ + { path: '/abs/src', isDirectory: true }, + { path: '/abs/notes.txt', isDirectory: false } + ]) as DataTransfer & { _pathByFile: Map } + + stubBridge(transfer) + + const result = extractDroppedFiles(transfer) + const { inAppRefs, osDrops } = partitionDroppedFiles(result) + + expect(inAppRefs.map(entry => entry.path)).toEqual(['/abs/src']) + expect(inAppRefs[0]?.isDirectory).toBe(true) + expect(osDrops.map(entry => entry.path)).toEqual(['/abs/notes.txt']) + }) + + it('does not duplicate a folder that appears in both items and files', () => { + // Chromium lists a dropped folder in transfer.files too (as a size-0 File); + // the items pass claims its path first so the files fallback skips it. + const transfer = stubTransfer([ + { path: '/abs/project', isDirectory: true } + ]) as DataTransfer & { _pathByFile: Map } + + stubBridge(transfer) + + const result = extractDroppedFiles(transfer) + + expect(result).toHaveLength(1) + expect(result[0]?.isDirectory).toBe(true) + }) +}) diff --git a/apps/desktop/src/app/chat/hooks/use-composer-actions.ts b/apps/desktop/src/app/chat/hooks/use-composer-actions.ts index a8afdd12830..5facd58f42a 100644 --- a/apps/desktop/src/app/chat/hooks/use-composer-actions.ts +++ b/apps/desktop/src/app/chat/hooks/use-composer-actions.ts @@ -44,7 +44,8 @@ export interface DroppedFile { file?: File /** Absolute filesystem path. Empty when an OS drop didn't carry one. */ path: string - /** True if the entry is a directory. Currently only set by in-app drags. */ + /** True if the entry is a directory. Set by in-app drags, and by OS drops via + * DataTransferItem.webkitGetAsEntry(). */ isDirectory?: boolean /** First line number for in-app line-ref drags (source view gutter). */ line?: number @@ -108,39 +109,50 @@ export function extractDroppedFiles(transfer: DataTransfer): DroppedFile[] { // Malformed payload — fall through to native files. } - const fileList = transfer.files - - if (fileList) { - for (let i = 0; i < fileList.length; i += 1) { - const file = fileList.item(i) - - if (!file || seenFiles.has(file)) { - continue - } - - seenFiles.add(file) - let path = '' - - if (getPath) { - try { - path = getPath(file) || '' - } catch { - path = '' - } - } - - if (path && seenPaths.has(path)) { - continue - } - - if (path) { - seenPaths.add(path) - } - - result.push({ file, path }) + // Add a native OS-drop entry. A dropped directory has no byte content to + // upload, so it's emitted as a path-only entry with `isDirectory: true` — + // that routes it to a `@folder:` ref / folder attachment (like the folder + // picker) instead of the file-upload pipeline, which can't stage a directory + // (the gateway can't read its bytes and there's no data_url to send). + const pushNativeEntry = (file: File, isDirectory: boolean) => { + if (seenFiles.has(file)) { + return } + + seenFiles.add(file) + let path = '' + + if (getPath) { + try { + path = getPath(file) || '' + } catch { + path = '' + } + } + + if (path && seenPaths.has(path)) { + return + } + + if (path) { + seenPaths.add(path) + } + + if (isDirectory) { + if (path) { + result.push({ isDirectory: true, path }) + } + + return + } + + result.push({ file, path }) } + // Process items first: DataTransferItem.webkitGetAsEntry() is the only + // synchronous way to tell a dropped folder from a file, and it lives only on + // items (not transfer.files). Must be read here, inside the drop handler, + // before the DataTransfer detaches. const items = transfer.items if (items) { @@ -151,32 +163,39 @@ export function extractDroppedFiles(transfer: DataTransfer): DroppedFile[] { continue } + let isDirectory = false + + try { + const entry = typeof item.webkitGetAsEntry === 'function' ? item.webkitGetAsEntry() : null + isDirectory = entry?.isDirectory === true + } catch { + isDirectory = false + } + const file = item.getAsFile() - if (!file || seenFiles.has(file)) { + if (!file) { continue } - seenFiles.add(file) - let path = '' + pushNativeEntry(file, isDirectory) + } + } - if (getPath) { - try { - path = getPath(file) || '' - } catch { - path = '' - } - } + // Fallback for environments that populate transfer.files but not items. + // webkitGetAsEntry isn't available on this path, so directory detection + // relies on the items pass above; anything reaching here is treated as a file. + const fileList = transfer.files - if (path && seenPaths.has(path)) { + if (fileList) { + for (let i = 0; i < fileList.length; i += 1) { + const file = fileList.item(i) + + if (!file) { continue } - if (path) { - seenPaths.add(path) - } - - result.push({ file, path }) + pushNativeEntry(file, false) } }