Skip to content
Merged
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
127 changes: 125 additions & 2 deletions apps/desktop/src/app/chat/hooks/use-composer-actions.test.ts
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -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')
Expand All @@ -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<File, string> } {
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<number | string, unknown> = { 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<File, string> }
}

describe('extractDroppedFiles', () => {
afterEach(() => {
vi.unstubAllGlobals()
})

const stubBridge = (transfer: DataTransfer & { _pathByFile: Map<File, string> }) => {
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<File, string> }

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<File, string> }

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<File, string> }

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<File, string> }

stubBridge(transfer)

const result = extractDroppedFiles(transfer)

expect(result).toHaveLength(1)
expect(result[0]?.isDirectory).toBe(true)
})
})
95 changes: 57 additions & 38 deletions apps/desktop/src/app/chat/hooks/use-composer-actions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -108,39 +109,50 @@ export function extractDroppedFiles(transfer: DataTransfer): DroppedFile[] {
// Malformed payload — fall through to native files.
}

const fileList = transfer.files
// 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
}

if (fileList) {
for (let i = 0; i < fileList.length; i += 1) {
const file = fileList.item(i)
seenFiles.add(file)
let path = ''

if (!file || seenFiles.has(file)) {
continue
if (getPath) {
try {
path = getPath(file) || ''
} catch {
path = ''
}
}

seenFiles.add(file)
let path = ''

if (getPath) {
try {
path = getPath(file) || ''
} catch {
path = ''
}
}
if (path && seenPaths.has(path)) {
return
}

if (path && seenPaths.has(path)) {
continue
}
if (path) {
seenPaths.add(path)
}

if (isDirectory) {
if (path) {
seenPaths.add(path)
result.push({ isDirectory: true, path })
}

result.push({ file, 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) {
Expand All @@ -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)) {
continue
}
if (fileList) {
for (let i = 0; i < fileList.length; i += 1) {
const file = fileList.item(i)

if (path) {
seenPaths.add(path)
if (!file) {
continue
}

result.push({ file, path })
pushNativeEntry(file, false)
}
}

Expand Down
Loading