From 80f0623b0ab1c228883cb0f455d1289650b8c0e9 Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Mon, 5 Oct 2026 18:30:26 -0700 Subject: [PATCH 01/16] feat(file-drop): add element owner preparation plumbing --- .../window/dropped-path-preparation.test.ts | 180 ++++++++++ src/main/window/dropped-path-preparation.ts | 161 +++++++++ .../window/native-file-drop-relay.test.ts | 64 +++- src/main/window/native-file-drop-relay.ts | 314 ++++++++---------- src/preload/api-types.ts | 1 + src/preload/api/filesystem-api.ts | 5 + src/preload/api/fs-bridge.ts | 6 + .../app-restart-checkpoint-routing.test.ts | 34 +- src/preload/index.ts | 3 +- .../preload-native-file-drop-release.test.ts | 122 +++++++ src/preload/preload-runtime-support.ts | 22 ++ .../automation-editor-prompt-options.test.ts | 1 + .../automation-editor-prompt-options.ts | 1 + .../src/components/editor/DiffSectionBody.tsx | 1 + .../src/components/editor/DiffViewer.tsx | 1 + .../src/components/editor/IpynbCellEditor.tsx | 1 + .../src/components/editor/MonacoEditor.tsx | 1 + ...treeList.status-lane-lineage-drop.test.tsx | 22 +- .../sidebar/workspace-status-drag-data.ts | 15 +- .../sidebar/workspace-status.test.ts | 17 +- .../src/hooks/use-os-file-drop-owner.test.tsx | 274 +++++++++++++++ .../src/hooks/use-os-file-drop-owner.ts | 193 +++++++++++ .../os-file-drop-cancellation-guard.test.ts | 110 ++++++ .../lib/os-file-drop-cancellation-guard.ts | 60 ++++ .../os-file-drop-preload-integration.test.tsx | 136 ++++++++ src/renderer/src/main.tsx | 3 + src/renderer/src/popout.tsx | 3 + src/renderer/src/web/main.tsx | 3 + .../src/web/preload-api/web-filesystem-api.ts | 3 + .../src/web/preload-api/web-platform-api.ts | 1 + .../web/web-preload-api-filesystem.test.ts | 8 + src/shared/native-file-drop-preparation.ts | 15 + 32 files changed, 1574 insertions(+), 207 deletions(-) create mode 100644 src/main/window/dropped-path-preparation.test.ts create mode 100644 src/main/window/dropped-path-preparation.ts create mode 100644 src/preload/preload-native-file-drop-release.test.ts create mode 100644 src/renderer/src/hooks/use-os-file-drop-owner.test.tsx create mode 100644 src/renderer/src/hooks/use-os-file-drop-owner.ts create mode 100644 src/renderer/src/lib/os-file-drop-cancellation-guard.test.ts create mode 100644 src/renderer/src/lib/os-file-drop-cancellation-guard.ts create mode 100644 src/renderer/src/lib/os-file-drop-preload-integration.test.tsx create mode 100644 src/shared/native-file-drop-preparation.ts diff --git a/src/main/window/dropped-path-preparation.test.ts b/src/main/window/dropped-path-preparation.test.ts new file mode 100644 index 000000000000..d04aa90ef607 --- /dev/null +++ b/src/main/window/dropped-path-preparation.test.ts @@ -0,0 +1,180 @@ +import { join } from 'node:path' +import { beforeEach, describe, expect, it, vi } from 'vitest' +import type { DragTempCopyItemResult } from './dragged-temp-file-copy' +import type * as DragTempFileCopy from './dragged-temp-file-copy' +import type { NativeFileDropPayload } from '../../shared/native-file-drop' +import { createDragTempCopyLane, MAX_PENDING_DRAG_TEMP_COPIES } from './dropped-path-preparation' +import { createNativeFileDropQueue } from './native-file-drop-relay' + +const materialize = vi.hoisted(() => vi.fn()) +vi.mock('electron', () => ({ app: {}, ipcMain: {} })) +vi.mock('./dragged-temp-file-copy', async (original) => ({ + ...(await original()), + materializeDragTempPaths: materialize +})) +const TEMP = join('/', 'var', 'T', 'TemporaryItems', 'NSIRD_one', 'Shot.png') +const ORDINARY = join('/', 'files', 'notes.txt') +const COPY = join('/', 'var', 'T', 'orca-drops', 'orca-drop-abc123', 'Shot.png') +const env = { platform: 'darwin' as const, sourceTempRoot: '/var/T', copyRoot: '/var/T/drops' } + +function queue(lane = createDragTempCopyLane(), copyTimeoutMs = 1000) { + const controller = new AbortController() + const forwarded: NativeFileDropPayload[] = [] + const dispose = vi.fn() + const enqueue = createNativeFileDropQueue( + { + platform: 'darwin', + getCopyEnvironment: async () => env, + watchRenderer: () => ({ signal: controller.signal, dispose }), + forward: (payload) => forwarded.push(payload), + copyTimeoutMs + }, + lane + ) + return { enqueue, controller, forwarded, dispose } +} +function deferred() { + let resolve!: (value: DragTempCopyItemResult[]) => void + const promise = new Promise((settle) => { + resolve = settle + }) + return { promise, resolve } +} +async function settle() { + await new Promise((resolve) => setTimeout(resolve, 0)) +} +beforeEach(() => { + materialize.mockReset() +}) + +describe('dropped path preparation through the shared relay queue', () => { + it.each(['agent', 'main-reader'] as const)( + 'passes ordinary paths without copying for %s', + async (consumer) => { + const { enqueue } = queue() + await expect(enqueue.prepare({ paths: [ORDINARY], consumer })).resolves.toEqual({ + paths: [ORDINARY], + failures: [] + }) + expect(materialize).not.toHaveBeenCalled() + } + ) + + it.each(['agent', 'main-reader'] as const)( + 'withholds uncopied originals only from agents: %s', + async (consumer) => { + materialize.mockResolvedValue([ + { sourcePath: ORDINARY, status: 'imported', destPath: ORDINARY }, + { sourcePath: TEMP, status: 'uncopied', reason: 'storage-full' } + ]) + const { enqueue } = queue() + const result = await enqueue.prepare({ paths: [ORDINARY, TEMP], consumer }) + expect(result.paths).toEqual(consumer === 'agent' ? [ORDINARY] : [ORDINARY, TEMP]) + expect(result.failures).toEqual( + consumer === 'agent' + ? [ + { + byteLength: 0, + pathCount: 1, + reason: 'temp-copy-failed', + target: 'rejected', + commonReason: 'storage-full' + } + ] + : [] + ) + } + ) + + it('preserves ordinary successes and copied successes when another file fails', async () => { + materialize.mockResolvedValue([ + { sourcePath: ORDINARY, status: 'imported', destPath: ORDINARY }, + { sourcePath: TEMP, status: 'imported', destPath: COPY }, + { sourcePath: TEMP, status: 'failed', reason: 'missing' } + ]) + const result = await queue().enqueue.prepare({ + paths: [ORDINARY, TEMP, TEMP], + consumer: 'agent' + }) + expect(result.paths).toEqual([ORDINARY, COPY]) + expect(result.failures[0]).toMatchObject({ pathCount: 1, commonReason: 'missing' }) + }) + + it('serializes requests and the legacy relay together', async () => { + const first = deferred() + materialize + .mockReturnValueOnce(first.promise) + .mockResolvedValueOnce([{ sourcePath: TEMP, status: 'imported', destPath: COPY }]) + const { enqueue, forwarded } = queue() + enqueue({ paths: [TEMP], target: 'terminal' }) + const requested = enqueue.prepare({ paths: [TEMP], consumer: 'agent' }) + await settle() + expect(materialize).toHaveBeenCalledTimes(1) + first.resolve([{ sourcePath: TEMP, status: 'imported', destPath: COPY }]) + await expect(requested).resolves.toEqual({ paths: [COPY], failures: [] }) + await settle() + expect(forwarded).toEqual([{ paths: [COPY], target: 'terminal' }]) + expect(materialize).toHaveBeenCalledTimes(2) + }) + + it('shares serialization and the pending cap across replacement-window queues', async () => { + materialize.mockReturnValue(new Promise(() => undefined)) + const lane = createDragTempCopyLane() + const first = queue(lane) + const second = queue(lane) + const results = Array.from({ length: MAX_PENDING_DRAG_TEMP_COPIES }, () => + first.enqueue.prepare({ paths: [TEMP], consumer: 'agent' }).catch(() => undefined) + ) + await expect( + second.enqueue.prepare({ paths: [ORDINARY, TEMP], consumer: 'agent' }) + ).resolves.toMatchObject({ paths: [ORDINARY], failures: [{ commonReason: 'busy' }] }) + first.controller.abort(new Error('test ended')) + await Promise.all(results) + await settle() + expect(materialize).toHaveBeenCalledTimes(1) + }) + + it('lets another owner prepare ordinary paths while a copy waits', async () => { + const copy = deferred() + materialize.mockReturnValueOnce(copy.promise) + const { enqueue } = queue() + const pending = enqueue.prepare({ paths: [TEMP], consumer: 'agent' }) + await expect(enqueue.prepare({ paths: [ORDINARY], consumer: 'main-reader' })).resolves.toEqual({ + paths: [ORDINARY], + failures: [] + }) + copy.resolve([{ sourcePath: TEMP, status: 'imported', destPath: COPY }]) + await pending + }) + + it('aborts running and queued requests on reload and releases their listeners', async () => { + materialize.mockReturnValue(new Promise(() => undefined)) + const { enqueue, controller, dispose } = queue() + const running = enqueue.prepare({ paths: [TEMP], consumer: 'agent' }) + const queued = enqueue.prepare({ paths: [TEMP], consumer: 'agent' }) + const assertions = [ + expect(running).rejects.toThrow('reloaded'), + expect(queued).rejects.toThrow('reloaded') + ] + await settle() + controller.abort(new Error('reloaded')) + await Promise.all(assertions) + await settle() + expect(materialize).toHaveBeenCalledTimes(1) + expect(dispose).toHaveBeenCalledTimes(2) + }) + + it('reports timeout, keeps ordinary paths, and serves the next copy', async () => { + materialize + .mockReturnValueOnce(new Promise(() => undefined)) + .mockResolvedValueOnce([{ sourcePath: TEMP, status: 'imported', destPath: COPY }]) + const { enqueue } = queue(undefined, 5) + const first = enqueue.prepare({ paths: [ORDINARY, TEMP], consumer: 'agent' }) + const next = enqueue.prepare({ paths: [TEMP], consumer: 'agent' }) + await expect(first).resolves.toMatchObject({ + paths: [ORDINARY], + failures: [{ commonReason: 'timed-out' }] + }) + await expect(next).resolves.toEqual({ paths: [COPY], failures: [] }) + }) +}) diff --git a/src/main/window/dropped-path-preparation.ts b/src/main/window/dropped-path-preparation.ts new file mode 100644 index 000000000000..1012989a8712 --- /dev/null +++ b/src/main/window/dropped-path-preparation.ts @@ -0,0 +1,161 @@ +import type { NativeFileDropCopyFailureReason } from '../../shared/native-file-drop' +import type { + PrepareDroppedPathsRequest, + PreparedDroppedPaths +} from '../../shared/native-file-drop-preparation' +import { + materializeDragTempPaths, + mayNeedDragTempCopy, + type DragTempCopyEnvironment +} from './dragged-temp-file-copy' + +const DRAG_TEMP_COPY_TIMEOUT_MS = 2 * 60 * 1000 +export const MAX_PENDING_DRAG_TEMP_COPIES = 8 + +export type RendererDropLifetime = { signal: AbortSignal; dispose: () => void } +export type DroppedPathPreparationDeps = { + platform: NodeJS.Platform + getCopyEnvironment: () => Promise + watchRenderer: () => RendererDropLifetime + copyTimeoutMs?: number +} + +export function createDragTempCopyLane(): { tail: Promise; pending: number } { + return { tail: Promise.resolve(), pending: 0 } +} + +// Why: replacement windows still share the retained-copy budget with work already in flight. +export const windowDragTempCopyLane = createDragTempCopyLane() + +export function createDroppedPathPreparationQueue( + deps: DroppedPathPreparationDeps, + lane = createDragTempCopyLane() +): (request: PrepareDroppedPathsRequest) => PreparedDroppedPaths | Promise { + return (request) => { + if (!request.paths.some((path) => mayNeedDragTempCopy(path, deps.platform))) { + return { paths: [...request.paths], failures: [] } + } + if (lane.pending >= MAX_PENDING_DRAG_TEMP_COPIES) { + return failDroppedPathPreparation(request.paths, deps.platform, 'busy') + } + const lifetime = deps.watchRenderer() + lane.pending += 1 + const prepared = lane.tail.then(() => { + lifetime.signal.throwIfAborted() + return copyDroppedPaths(request, lifetime.signal, deps) + }) + lane.tail = prepared + .then( + () => undefined, + () => undefined + ) + .finally(() => { + lane.pending -= 1 + lifetime.dispose() + }) + // Why: a reloaded document must stop waiting even while its request is queued. + return rejectOnDropAbort(prepared, lifetime.signal) + } +} + +export async function prepareDroppedPaths( + request: PrepareDroppedPathsRequest, + env: DragTempCopyEnvironment, + signal?: AbortSignal +): Promise { + const results = await materializeDragTempPaths(request.paths, env, signal) + const acceptsOriginal = request.consumer === 'main-reader' + const paths = results.flatMap((result) => + result.status === 'imported' + ? [result.destPath] + : result.status === 'uncopied' && acceptsOriginal + ? [result.sourcePath] + : [] + ) + const unprepared = results.flatMap((result) => + result.status === 'imported' || (result.status === 'uncopied' && acceptsOriginal) + ? [] + : [result] + ) + const commonReason = unprepared.every((item) => item.reason === unprepared[0]?.reason) + ? unprepared[0]?.reason + : undefined + return { + paths, + failures: unprepared.length > 0 ? [copyFailure(unprepared.length, commonReason)] : [] + } +} + +function failDroppedPathPreparation( + paths: readonly string[], + platform: NodeJS.Platform, + reason?: NativeFileDropCopyFailureReason +): PreparedDroppedPaths { + const ordinary = paths.filter((path) => !mayNeedDragTempCopy(path, platform)) + const lost = paths.length - ordinary.length + return { + paths: ordinary, + failures: lost > 0 ? [copyFailure(lost, reason)] : [] + } +} + +async function copyDroppedPaths( + request: PrepareDroppedPathsRequest, + rendererSignal: AbortSignal, + deps: DroppedPathPreparationDeps +): Promise { + const timeout = new AbortController() + const timer = setTimeout( + () => timeout.abort(new Error('Copying the dropped files took too long')), + deps.copyTimeoutMs ?? DRAG_TEMP_COPY_TIMEOUT_MS + ) + const signal = AbortSignal.any([rendererSignal, timeout.signal]) + try { + return await rejectOnDropAbort( + deps.getCopyEnvironment().then((env) => prepareDroppedPaths(request, env, signal)), + signal + ) + } catch (error) { + if (rendererSignal.aborted) { + throw error + } + return failDroppedPathPreparation( + request.paths, + deps.platform, + timeout.signal.aborted ? 'timed-out' : undefined + ) + } finally { + clearTimeout(timer) + } +} + +export function rejectOnDropAbort(work: Promise, signal: AbortSignal): Promise { + return new Promise((resolve, reject) => { + const onAbort = (): void => reject(signal.reason) + if (signal.aborted) { + onAbort() + } else { + signal.addEventListener('abort', onAbort, { once: true }) + } + void work.then( + (value) => { + signal.removeEventListener('abort', onAbort) + resolve(value) + }, + (error) => { + signal.removeEventListener('abort', onAbort) + reject(error) + } + ) + }) +} + +function copyFailure(pathCount: number, commonReason?: NativeFileDropCopyFailureReason) { + return { + byteLength: 0, + pathCount, + reason: 'temp-copy-failed' as const, + target: 'rejected' as const, + ...(commonReason ? { commonReason } : {}) + } +} diff --git a/src/main/window/native-file-drop-relay.test.ts b/src/main/window/native-file-drop-relay.test.ts index 276387bc6a48..7200be84eba4 100644 --- a/src/main/window/native-file-drop-relay.test.ts +++ b/src/main/window/native-file-drop-relay.test.ts @@ -6,16 +6,20 @@ import type * as DragTempFileCopy from './dragged-temp-file-copy' import type { DragTempCopyItemResult } from './dragged-temp-file-copy' type IpcListener = (event: { sender: unknown }, payload: unknown) => void +type InvokeHandler = (event: { sender: unknown }, payload: unknown) => Promise -const { materializeMock, sweepMock, ipcListeners } = vi.hoisted(() => ({ +const { materializeMock, sweepMock, ipcListeners, ipcHandlers } = vi.hoisted(() => ({ materializeMock: vi.fn(), sweepMock: vi.fn(), - ipcListeners: new Map() + ipcListeners: new Map(), + ipcHandlers: new Map() })) vi.mock('electron', () => ({ app: { getPath: () => '/app-temp' }, ipcMain: { + handle: (channel: string, handler: InvokeHandler) => ipcHandlers.set(channel, handler), + removeHandler: (channel: string) => ipcHandlers.delete(channel), on: (channel: string, listener: IpcListener) => ipcListeners.set(channel, listener), removeListener: (channel: string, listener: IpcListener) => { if (ipcListeners.get(channel) === listener) { @@ -418,6 +422,7 @@ function relay(): IpcListener { describe('registerFileDropRelay', () => { beforeEach(() => { ipcListeners.clear() + ipcHandlers.clear() sweepMock.mockReset() }) @@ -452,6 +457,61 @@ describe('registerFileDropRelay', () => { expect(first.webContents.send).not.toHaveBeenCalled() }) + it('prepares ordinary paths only for its own live renderer and validates the request', async () => { + const { window, webContents, close } = createWindow() + registerFileDropRelay(window) + const handler = ipcHandlers.get('fs:prepareDroppedPaths')! + const request = { paths: [FINDER], consumer: 'agent' } + expect(() => handler({ sender: {} }, request)).toThrow('owning window') + for (const invalid of [ + null, + { paths: [FINDER], consumer: 'other' }, + { paths: [42], consumer: 'agent' } + ]) { + expect(() => handler({ sender: webContents }, invalid)).toThrow('Invalid') + } + await expect(handler({ sender: webContents }, request)).resolves.toEqual({ + paths: [FINDER], + failures: [] + }) + expect(webContents.send).not.toHaveBeenCalled() + close() + expect(ipcHandlers.has('fs:prepareDroppedPaths')).toBe(false) + expect(() => handler({ sender: webContents }, request)).toThrow('owning window') + }) + + it('enforces both request caps before preparing any paths', async () => { + const { window, webContents } = createWindow() + registerFileDropRelay(window) + const handler = ipcHandlers.get('fs:prepareDroppedPaths')! + await expect( + handler( + { sender: webContents }, + { paths: Array.from({ length: 257 }, () => DRAG_TEMP), consumer: 'agent' } + ) + ).resolves.toMatchObject({ + paths: [], + failures: [{ reason: 'too-many-paths', pathCount: 257 }] + }) + await expect( + handler({ sender: webContents }, { paths: ['x'.repeat(256 * 1024 + 1)], consumer: 'agent' }) + ).resolves.toMatchObject({ paths: [], failures: [{ reason: 'paths-too-large' }] }) + expect(materializeMock).not.toHaveBeenCalled() + }) + + it('keeps the replacement window handler when an older window closes late', async () => { + const first = createWindow() + const second = createWindow() + registerFileDropRelay(first.window) + registerFileDropRelay(second.window) + const handler = ipcHandlers.get('fs:prepareDroppedPaths')! + first.close() + expect(ipcHandlers.get('fs:prepareDroppedPaths')).toBe(handler) + await expect( + handler({ sender: second.webContents }, { paths: [FINDER], consumer: 'main-reader' }) + ).resolves.toEqual({ paths: [FINDER], failures: [] }) + }) + it.skipIf(process.platform !== 'darwin')( 'copies from the macOS user temp dir and sends nothing once the window is gone', async () => { diff --git a/src/main/window/native-file-drop-relay.ts b/src/main/window/native-file-drop-relay.ts index b8af442e8950..3d7d4c8f50bb 100644 --- a/src/main/window/native-file-drop-relay.ts +++ b/src/main/window/native-file-drop-relay.ts @@ -1,251 +1,219 @@ import { app, ipcMain } from 'electron' import type { BrowserWindow } from 'electron' import { + createRejectedNativeFileDropPayload, isNativeFileDropPayload, NATIVE_FILE_DROP_TARGET, - type NativeFileDropCopyFailureReason, + validateNativeFileDropPaths, type NativeFileDropPayload, type NativeFileDropRejectedPayload } from '../../shared/native-file-drop' +import type { + PrepareDroppedPathsRequest, + PreparedDroppedPaths +} from '../../shared/native-file-drop-preparation' import { abortWhenRendererGone } from '../ipc/renderer-lifetime-abort' +import { getDragTempCopyRoot, scheduleDragTempCopySweep } from './dragged-temp-file-copy' import { - getDragTempCopyRoot, - materializeDragTempPaths, - mayNeedDragTempCopy, - scheduleDragTempCopySweep, - type DragTempCopyEnvironment -} from './dragged-temp-file-copy' + createDroppedPathPreparationQueue, + type createDragTempCopyLane, + prepareDroppedPaths, + windowDragTempCopyLane, + type DroppedPathPreparationDeps +} from './dropped-path-preparation' +import type { DragTempCopyEnvironment } from './dragged-temp-file-copy' import { getDarwinUserTempDir } from './darwin-user-temp-dir' -// Why: copies run one at a time, so a hung copy must not hold every later copy forever. -const DRAG_TEMP_COPY_TIMEOUT_MS = 2 * 60 * 1000 -// Why: a drop is one user gesture; more than this waiting means copies are stuck, not busy. -export const MAX_PENDING_DRAG_TEMP_COPIES = 8 +export { MAX_PENDING_DRAG_TEMP_COPIES } from './dropped-path-preparation' type AcceptedNativeFileDropPayload = Exclude - -type RendererLifetime = { signal: AbortSignal; dispose: () => void } - -type NativeFileDropQueueDeps = { +type NativeFileDropQueueDeps = DroppedPathPreparationDeps & { forward: (payload: NativeFileDropPayload) => void - platform: NodeJS.Platform - getCopyEnvironment: () => Promise - watchRenderer: () => RendererLifetime - copyTimeoutMs?: number +} +type NativeFileDropQueue = ((payload: NativeFileDropPayload) => void) & { + prepare: (request: PrepareDroppedPathsRequest) => Promise } export function registerFileDropRelay(mainWindow: BrowserWindow): void { const channel = 'terminal:file-dropped-from-preload' + const prepareChannel = 'fs:prepareDroppedPaths' const mainWebContents = mainWindow.webContents const isWindowGone = (): boolean => mainWindow.isDestroyed() || mainWebContents.isDestroyed() ipcMain.removeAllListeners(channel) - const enqueue = createNativeFileDropQueue({ - // Why: one IPC event per drop gesture so the renderer gets the full path batch without timer-based reconstruction. - forward: (payload) => { - if (!isWindowGone()) { - mainWebContents.send('terminal:file-drop', payload) - } - }, - platform: process.platform, - getCopyEnvironment: async () => ({ + ipcMain.removeHandler(prepareChannel) + const enqueue = createNativeFileDropQueue( + { + forward: (payload) => { + if (!isWindowGone()) { + mainWebContents.send('terminal:file-drop', payload) + } + }, platform: process.platform, - sourceTempRoot: await getDarwinUserTempDir(), - copyRoot: getDragTempCopyRoot(app.getPath('temp')) - }), - watchRenderer: () => abortWhenRendererGone(mainWebContents) - }) + getCopyEnvironment: async () => ({ + platform: process.platform, + sourceTempRoot: await getDarwinUserTempDir(), + copyRoot: getDragTempCopyRoot(app.getPath('temp')) + }), + watchRenderer: () => abortWhenRendererGone(mainWebContents) + }, + windowDragTempCopyLane + ) const relayFileDrop = (event: Electron.IpcMainEvent, args: NativeFileDropPayload): void => { if (isWindowGone() || event.sender !== mainWebContents) { return } - // Why: only main reports a failed copy; a renderer claiming one is ignored. if (!isNativeFileDropPayload(args) || isTempCopyFailure(args)) { return } enqueue(args) } ipcMain.on(channel, relayFileDrop) + const prepareHandler = ( + event: Electron.IpcMainInvokeEvent, + args: unknown + ): Promise => { + if (isWindowGone() || event.sender !== mainWebContents) { + throw new Error('Dropped paths must come from the owning window') + } + if (!isPrepareDroppedPathsRequest(args)) { + throw new Error('Invalid dropped paths request') + } + const validation = validateNativeFileDropPaths(args.paths) + if (validation.status === 'rejected') { + return Promise.resolve({ + paths: [], + failures: [createRejectedNativeFileDropPayload(validation)] + }) + } + return enqueue.prepare(args) + } + ipcMain.handle(prepareChannel, prepareHandler) + activePrepareHandler = prepareHandler mainWindow.on('closed', () => { - // Why: macOS keeps the process alive after window close; drop the closure so the destroyed window isn't retained. ipcMain.removeListener(channel, relayFileDrop) + // Why: an old window closing late must not remove the replacement window's handler. + if (activePrepareHandler === prepareHandler) { + ipcMain.removeHandler(prepareChannel) + activePrepareHandler = undefined + } }) scheduleDragTempCopySweep(() => getDragTempCopyRoot(app.getPath('temp'))) } -/** - * A drop holding a macOS drag-temp path waits for main to copy it, because the - * PTY daemon cannot open the original. Those copies run one at a time, in order, - * under one byte budget. Other drops are forwarded at once unless an earlier drop - * is still queued, so drops reach the renderer in the order they were made. - */ +let activePrepareHandler: unknown + +/** Legacy delivery stays ordered; new owners share only the serialized copy work. */ export function createNativeFileDropQueue( - deps: NativeFileDropQueueDeps -): (payload: NativeFileDropPayload) => void { - let tail = Promise.resolve() - let queued = 0 - let pendingCopies = 0 - const enqueue = ( - deliver: (lifetime: RendererLifetime) => Promise | void, - isCopy: boolean - ): void => { - queued += 1 - pendingCopies += isCopy ? 1 : 0 - // Why: bind to the document that dropped now, so a reload while this waits discards it. - const lifetime = deps.watchRenderer() - // Why: never reject, or one failed drop would stall every drop queued behind it. - tail = tail - .then(() => (lifetime.signal.aborted ? undefined : deliver(lifetime))) + deps: NativeFileDropQueueDeps, + lane?: ReturnType +): NativeFileDropQueue { + const prepare = createDroppedPathPreparationQueue(deps, lane) + let deliveryTail = Promise.resolve() + let queuedDeliveries = 0 + const deliver = (payloads: NativeFileDropPayload[]): void => { + for (const payload of payloads) { + deps.forward(payload) + } + } + const enqueueDelivery = (work: () => Promise | void): void => { + queuedDeliveries += 1 + deliveryTail = deliveryTail + .then(work) .catch(() => undefined) .finally(() => { - queued -= 1 - pendingCopies -= isCopy ? 1 : 0 - lifetime.dispose() + queuedDeliveries -= 1 }) } const forwardInOrder = (payload: NativeFileDropPayload): void => { - // Why: a rejection carries no paths, so only path drops wait their turn. - if (payload.target === 'rejected' || queued === 0) { + if (payload.target === 'rejected' || queuedDeliveries === 0) { deps.forward(payload) - } else { - enqueue(() => deps.forward(payload), false) + return } + const lifetime = deps.watchRenderer() + enqueueDelivery(() => { + try { + if (!lifetime.signal.aborted) { + deps.forward(payload) + } + } finally { + lifetime.dispose() + } + }) } - return (payload) => { - if (payload.target === 'rejected' || !needsDragTempCopy(payload, deps.platform)) { + const enqueue = (payload: NativeFileDropPayload): void => { + if (payload.target === 'rejected') { forwardInOrder(payload) return } - if (pendingCopies >= MAX_PENDING_DRAG_TEMP_COPIES) { - for (const item of failWholeDrop(payload, deps.platform, 'busy')) { + const prepared = prepare({ paths: payload.paths, consumer: legacyConsumer(payload) }) + if (!(prepared instanceof Promise)) { + if (payload.paths.length === 0) { + forwardInOrder(payload) + return + } + for (const item of legacyPayloads(payload, prepared)) { forwardInOrder(item) } return } - enqueue((lifetime) => copyAndForward(payload, lifetime, deps), true) + // Why: observe rejection now, even while legacy delivery waits behind an earlier drop. + const result = prepared.then( + (value) => legacyPayloads(payload, value), + () => [] + ) + enqueueDelivery(async () => deliver(await result)) } + return Object.assign(enqueue, { + prepare: async (request: PrepareDroppedPathsRequest) => prepare(request) + }) } -/** The payloads to forward for one drop: its prepared paths, then a rejection for any it lost. */ export async function prepareNativeFileDrop( payload: AcceptedNativeFileDropPayload, env: DragTempCopyEnvironment, signal?: AbortSignal ): Promise { - const results = await materializeDragTempPaths(payload.paths, env, signal) - // Why: agents run under the PTY daemon, which cannot open an uncopied original; - // other targets are read by main, so the original still works there. - const acceptsOriginal = - payload.target !== NATIVE_FILE_DROP_TARGET.terminal && - payload.target !== NATIVE_FILE_DROP_TARGET.composer - const paths = results.flatMap((result) => - result.status === 'imported' - ? [result.destPath] - : result.status === 'uncopied' && acceptsOriginal - ? [result.sourcePath] - : [] - ) - const unprepared = results.flatMap((result) => - result.status === 'imported' || (result.status === 'uncopied' && acceptsOriginal) - ? [] - : [result] + return legacyPayloads( + payload, + await prepareDroppedPaths( + { paths: payload.paths, consumer: legacyConsumer(payload) }, + env, + signal + ) ) - const prepared: NativeFileDropPayload[] = paths.length > 0 ? [{ ...payload, paths }] : [] - if (unprepared.length > 0) { - const commonReason = unprepared.every((item) => item.reason === unprepared[0].reason) - ? unprepared[0].reason - : undefined - prepared.push(copyFailure(unprepared.length, commonReason)) - } - return prepared } -/** When the copy stage fails as a whole, still deliver the paths that never needed a copy. */ -function failWholeDrop( +function legacyConsumer( + payload: AcceptedNativeFileDropPayload +): PrepareDroppedPathsRequest['consumer'] { + return payload.target === NATIVE_FILE_DROP_TARGET.terminal || + payload.target === NATIVE_FILE_DROP_TARGET.composer + ? 'agent' + : 'main-reader' +} + +function legacyPayloads( payload: AcceptedNativeFileDropPayload, - platform: NodeJS.Platform, - reason: NativeFileDropCopyFailureReason | undefined + prepared: PreparedDroppedPaths ): NativeFileDropPayload[] { - const ordinary = payload.paths.filter((path) => !mayNeedDragTempCopy(path, platform)) - const lost = payload.paths.length - ordinary.length return [ - ...(ordinary.length > 0 ? [{ ...payload, paths: ordinary }] : []), - ...(lost > 0 ? [copyFailure(lost, reason)] : []) + ...(prepared.paths.length > 0 ? [{ ...payload, paths: prepared.paths }] : []), + ...prepared.failures ] } -function forwardAll(deps: NativeFileDropQueueDeps, payloads: NativeFileDropPayload[]): void { - for (const payload of payloads) { - deps.forward(payload) - } -} - function isTempCopyFailure(payload: NativeFileDropPayload): boolean { return payload.target === 'rejected' && payload.reason === 'temp-copy-failed' } -function needsDragTempCopy( - payload: AcceptedNativeFileDropPayload, - platform: NodeJS.Platform -): boolean { - return payload.paths.some((path) => mayNeedDragTempCopy(path, platform)) -} - -async function copyAndForward( - payload: AcceptedNativeFileDropPayload, - lifetime: RendererLifetime, - deps: NativeFileDropQueueDeps -): Promise { - const timeout = new AbortController() - const timer = setTimeout( - () => timeout.abort(new Error('Copying the dropped files took too long')), - deps.copyTimeoutMs ?? DRAG_TEMP_COPY_TIMEOUT_MS +function isPrepareDroppedPathsRequest(value: unknown): value is PrepareDroppedPathsRequest { + return ( + !!value && + typeof value === 'object' && + 'consumer' in value && + (value.consumer === 'agent' || value.consumer === 'main-reader') && + 'paths' in value && + Array.isArray(value.paths) && + value.paths.every((path: unknown) => typeof path === 'string') ) - const signal = AbortSignal.any([lifetime.signal, timeout.signal]) - let prepared: NativeFileDropPayload[] - try { - // Why: race the signal too, since a hung fs call never reaches the copy's abort checks. - prepared = await rejectOnAbort( - deps.getCopyEnvironment().then((env) => prepareNativeFileDrop(payload, env, signal)), - signal - ) - } catch { - // Why: an aborted drop has no renderer to report to; anything else must not vanish silently. - if (lifetime.signal.aborted) { - return - } - prepared = failWholeDrop( - payload, - deps.platform, - timeout.signal.aborted ? 'timed-out' : undefined - ) - } finally { - clearTimeout(timer) - } - // Why: outside the try, so a failed forward is not reported a second time as a failed copy. - forwardAll(deps, prepared) -} - -function rejectOnAbort(work: Promise, signal: AbortSignal): Promise { - return new Promise((resolve, reject) => { - const onAbort = (): void => reject(signal.reason) - if (signal.aborted) { - onAbort() - } else { - signal.addEventListener('abort', onAbort, { once: true }) - } - work.then(resolve, reject).finally(() => signal.removeEventListener('abort', onAbort)) - }) -} - -function copyFailure( - pathCount: number, - commonReason?: NativeFileDropCopyFailureReason -): NativeFileDropRejectedPayload { - return { - byteLength: 0, - pathCount, - reason: 'temp-copy-failed', - target: 'rejected', - ...(commonReason ? { commonReason } : {}) - } } diff --git a/src/preload/api-types.ts b/src/preload/api-types.ts index cf5060fab898..88483eac58be 100644 --- a/src/preload/api-types.ts +++ b/src/preload/api-types.ts @@ -70,6 +70,7 @@ import type { FolderWorkspacesApi, SparsePresetsApi, WorktreeApi } from './api/w type Merged = { [K in keyof T]: T[K] } export type PreloadApi = { + getPathForFile?: (file: File) => string app: AppApi orcaProfiles: OrcaProfileApi platform: PlatformApi diff --git a/src/preload/api/filesystem-api.ts b/src/preload/api/filesystem-api.ts index 07230ae04ff1..ded450a9e4aa 100644 --- a/src/preload/api/filesystem-api.ts +++ b/src/preload/api/filesystem-api.ts @@ -27,6 +27,10 @@ import type { PythonEnvironments } from '../../shared/notebook-kernel-types' import type { RuntimeUploadFileStreamRequest } from '../../shared/runtime-upload-staging-contract' +import type { + PrepareDroppedPathsRequest, + PreparedDroppedPaths +} from '../../shared/native-file-drop-preparation' export type ExportApi = { htmlToPdf: (args: { @@ -39,6 +43,7 @@ export type ExportApi = { export type FilesystemApi = { fs: { + prepareDroppedPaths: (args: PrepareDroppedPathsRequest) => Promise readFileChunk: (args: { filePath: string connectionId?: string diff --git a/src/preload/api/fs-bridge.ts b/src/preload/api/fs-bridge.ts index aeea9b07fd01..aa085eaa538e 100644 --- a/src/preload/api/fs-bridge.ts +++ b/src/preload/api/fs-bridge.ts @@ -1,5 +1,9 @@ import type { PathExistenceResult } from '../../shared/path-existence-batch' import { ipcRenderer } from 'electron' +import type { + PrepareDroppedPathsRequest, + PreparedDroppedPaths +} from '../../shared/native-file-drop-preparation' import type { SshMutationExpectation } from '../../shared/ssh-types' import type { LocalFileAccess } from '../../shared/local-file-access' import type { RuntimeUploadFileStreamRequest } from '../../shared/runtime-upload-staging-contract' @@ -19,6 +23,8 @@ import type { import type { PreloadApi } from '../api-types' export const fsApi = { + prepareDroppedPaths: (args: PrepareDroppedPathsRequest): Promise => + ipcRenderer.invoke('fs:prepareDroppedPaths', args), readFileChunk: (args: { filePath: string connectionId?: string diff --git a/src/preload/app-restart-checkpoint-routing.test.ts b/src/preload/app-restart-checkpoint-routing.test.ts index 2c44b9a5b7fe..ab64310b0ff9 100644 --- a/src/preload/app-restart-checkpoint-routing.test.ts +++ b/src/preload/app-restart-checkpoint-routing.test.ts @@ -6,14 +6,16 @@ import { } from '../shared/updater-renderer-events' import { KEYBOARD_LAYOUT_CHANGED_CHANNEL } from '../shared/keyboard-layout-events' -const { exposeInMainWorld, invoke, on, removeListener, send, sendSync } = vi.hoisted(() => ({ - exposeInMainWorld: vi.fn(), - invoke: vi.fn(), - on: vi.fn(), - removeListener: vi.fn(), - send: vi.fn(), - sendSync: vi.fn() -})) +const { exposeInMainWorld, invoke, on, removeListener, send, sendSync, getPathForFile } = + vi.hoisted(() => ({ + exposeInMainWorld: vi.fn(), + invoke: vi.fn(), + on: vi.fn(), + removeListener: vi.fn(), + send: vi.fn(), + sendSync: vi.fn(), + getPathForFile: vi.fn(() => '/files/notes.txt') + })) vi.mock('electron', () => ({ contextBridge: { exposeInMainWorld }, @@ -23,7 +25,7 @@ vi.mock('electron', () => ({ setZoomFactor: vi.fn(), setVisualZoomLevelLimits: vi.fn() }, - webUtils: { getPathForFile: vi.fn(() => '') } + webUtils: { getPathForFile } })) describe('native preload destructive app actions', () => { @@ -58,6 +60,20 @@ describe('native preload destructive app actions', () => { return exposeInMainWorld.mock.calls.find(([name]) => name === 'api')?.[1] as PreloadApi } + it('exposes synchronous file path resolution and a window-local preparation invoke', async () => { + const api = await loadApi() + const file = new File(['x'], 'notes.txt') + expect(api.getPathForFile?.(file)).toBe('/files/notes.txt') + expect(getPathForFile).toHaveBeenCalledWith(file) + const request = { paths: ['/files/notes.txt'], consumer: 'agent' as const } + invoke.mockResolvedValue({ paths: request.paths, failures: [] }) + await expect(api.fs.prepareDroppedPaths(request)).resolves.toEqual({ + paths: request.paths, + failures: [] + }) + expect(invoke).toHaveBeenCalledExactlyOnceWith('fs:prepareDroppedPaths', request) + }) + for (const action of ['reload', 'relaunch'] as const) { it(`prepares and awaits durability before ${action}`, async () => { const api = await loadApi() diff --git a/src/preload/index.ts b/src/preload/index.ts index 5f64a1e2e351..775b2848e3cd 100644 --- a/src/preload/index.ts +++ b/src/preload/index.ts @@ -1,4 +1,4 @@ -import { contextBridge, ipcRenderer } from 'electron' +import { contextBridge, ipcRenderer, webUtils } from 'electron' import type { PreloadApi } from './api-types' import { installBrowserFindListener, @@ -101,6 +101,7 @@ const telemetryGetConsentStateApi: PreloadApi['telemetryGetConsentState'] = () = ipcRenderer.invoke('telemetry:getConsentState') const api = { + getPathForFile: (file: File): string => webUtils.getPathForFile(file), app: appApi, orcaProfiles: orcaProfilesApi, platform: platformApi, diff --git a/src/preload/preload-native-file-drop-release.test.ts b/src/preload/preload-native-file-drop-release.test.ts new file mode 100644 index 000000000000..dd556fb2e5ea --- /dev/null +++ b/src/preload/preload-native-file-drop-release.test.ts @@ -0,0 +1,122 @@ +// @vitest-environment happy-dom +import { beforeAll, beforeEach, describe, expect, it, vi } from 'vitest' +import { OS_FILE_DROP_OWNER_ATTRIBUTE } from '../shared/native-file-drop-preparation' +import { ORCA_INTERNAL_FILE_DRAG_TYPE } from '../shared/native-file-drop' + +const electron = vi.hoisted(() => ({ + send: vi.fn(), + getPathForFile: vi.fn((file: File) => `/drop/${file.name}`), + on: vi.fn(), + removeListener: vi.fn() +})) +vi.mock('electron', () => ({ + ipcRenderer: electron, + webUtils: { getPathForFile: electron.getPathForFile } +})) +import { installNativeFileDropHandlers } from './preload-runtime-support' + +function dispatch(target: Element, type = 'drop', types = ['Files']) { + const event = new Event(type, { bubbles: true, cancelable: true, composed: true }) + const transfer = { types, files: [new File(['x'], 'notes.txt')], dropEffect: 'none' } + Object.defineProperty(event, 'dataTransfer', { value: transfer }) + const stop = vi.spyOn(event, 'stopPropagation') + target.dispatchEvent(event) + return { event, transfer, stop } +} + +function root(owner: boolean, legacy = false) { + const element = document.createElement('div') + if (owner) { + element.setAttribute(OS_FILE_DROP_OWNER_ATTRIBUTE, '') + } + if (legacy) { + element.dataset.nativeFileDropTarget = 'terminal' + } + document.body.append(element) + return element +} + +beforeAll(() => { + installNativeFileDropHandlers() + installNativeFileDropHandlers() +}) +beforeEach(() => { + document.body.replaceChildren() + electron.send.mockClear() + electron.getPathForFile.mockClear() +}) + +describe('preload migration release', () => { + it.each([false, true])( + 'keeps unmarked and legacy roots on the relay exactly once: legacy=%s', + (legacy) => { + const { event, stop } = dispatch(root(false, legacy)) + expect(event.defaultPrevented).toBe(true) + expect(stop).toHaveBeenCalledTimes(1) + expect(electron.send).toHaveBeenCalledExactlyOnceWith('terminal:file-dropped-from-preload', { + paths: ['/drop/notes.txt'], + target: legacy ? 'terminal' : 'editor' + }) + } + ) + + it.each(['drop', 'dragover'])('leaves %s inside a migrated owner untouched', (type) => { + const owner = root(true) + const child = document.createElement('span') + owner.append(child) + const { event, transfer, stop } = dispatch(child, type) + expect(event.defaultPrevented).toBe(false) + expect(stop).not.toHaveBeenCalled() + expect(transfer.dropEffect).toBe('none') + expect(electron.send).not.toHaveBeenCalled() + expect(electron.getPathForFile).not.toHaveBeenCalled() + }) + + it.each(['drop', 'dragover'])('keeps a nested legacy boundary on the legacy %s path', (type) => { + const owner = root(true) + const legacy = root(false, true) + owner.append(legacy) + const { event, transfer } = dispatch(legacy, type) + expect(event.defaultPrevented).toBe(true) + if (type === 'drop') { + expect(electron.send).toHaveBeenCalledExactlyOnceWith('terminal:file-dropped-from-preload', { + paths: ['/drop/notes.txt'], + target: 'terminal' + }) + } else { + expect(transfer.dropEffect).toBe('copy') + expect(electron.send).not.toHaveBeenCalled() + } + }) + + it('chooses an inner owner over an outer legacy boundary', () => { + const legacy = root(false, true) + const owner = root(true) + legacy.append(owner) + expect(dispatch(owner).event.defaultPrevented).toBe(false) + expect(electron.send).not.toHaveBeenCalled() + }) + + it.each(['drop', 'dragover'])( + 'leaves internal %s unchanged even inside a migrated owner', + (type) => { + const { event, transfer, stop } = dispatch(root(true), type, [ + 'Files', + ORCA_INTERNAL_FILE_DRAG_TYPE + ]) + expect(event.defaultPrevented).toBe(false) + expect(stop).not.toHaveBeenCalled() + expect(transfer.dropEffect).toBe('none') + expect(electron.send).not.toHaveBeenCalled() + } + ) + + it('keeps the existing non-file drop cancellation, regardless of owner markers', () => { + const event = new Event('drop', { bubbles: true, cancelable: true }) + Object.defineProperty(event, 'dataTransfer', { value: { types: ['text/plain'], files: [] } }) + root(true).dispatchEvent(event) + expect(event.defaultPrevented).toBe(true) + expect(electron.send).not.toHaveBeenCalled() + expect(dispatch(root(true), 'dragover', ['text/plain']).event.defaultPrevented).toBe(false) + }) +}) diff --git a/src/preload/preload-runtime-support.ts b/src/preload/preload-runtime-support.ts index 9861583b34a0..424e276418ed 100644 --- a/src/preload/preload-runtime-support.ts +++ b/src/preload/preload-runtime-support.ts @@ -4,6 +4,7 @@ import { createBrowserFindSubscriptions } from './browser-find-subscriptions' import { registerRendererRestartIpcRelays } from './renderer-restart-wiring' import { createUpdaterQuitAbortRelay } from '../shared/renderer-restart-preparation' import { ORCA_UPDATER_QUIT_AND_INSTALL_ABORTED_EVENT } from '../shared/updater-renderer-events' +import { OS_FILE_DROP_OWNER_ATTRIBUTE } from '../shared/native-file-drop-preparation' import { ORCA_INTERNAL_FILE_DRAG_TYPE, createNativeFileDropPayload, @@ -88,6 +89,21 @@ function resolveNativeFileDrop(event: DragEvent): NativeDropResolution | null { return resolveNativeFileDropPath(pathEntries) } +function nearestDropBoundaryIsOwner(event: DragEvent): boolean { + for (const entry of event.composedPath()) { + if (!(entry instanceof HTMLElement)) { + continue + } + if (entry.hasAttribute(OS_FILE_DROP_OWNER_ATTRIBUTE)) { + return true + } + if (entry.hasAttribute('data-native-file-drop-target')) { + return false + } + } + return false +} + /** Installs the one preload-side listener that converts native File objects to paths. */ export function installNativeFileDropHandlers(): void { // Preload entry points can be evaluated more than once in tests and during development reloads; @@ -101,6 +117,9 @@ export function installNativeFileDropHandlers(): void { if (event.dataTransfer && !hasNativeFileDragTypes(event.dataTransfer.types)) { return } + if (hasNativeFileDragTypes(event.dataTransfer?.types) && nearestDropBoundaryIsOwner(event)) { + return + } event.preventDefault() if (event.dataTransfer) { event.dataTransfer.dropEffect = 'copy' @@ -114,6 +133,9 @@ export function installNativeFileDropHandlers(): void { if (event.dataTransfer?.types.includes(ORCA_INTERNAL_FILE_DRAG_TYPE)) { return } + if (hasNativeFileDragTypes(event.dataTransfer?.types) && nearestDropBoundaryIsOwner(event)) { + return + } event.preventDefault() event.stopPropagation() const files = event.dataTransfer?.files diff --git a/src/renderer/src/components/automations/automation-editor-prompt-options.test.ts b/src/renderer/src/components/automations/automation-editor-prompt-options.test.ts index 5cc4d29c628c..966eb568465f 100644 --- a/src/renderer/src/components/automations/automation-editor-prompt-options.test.ts +++ b/src/renderer/src/components/automations/automation-editor-prompt-options.test.ts @@ -12,6 +12,7 @@ describe('buildAutomationPromptEditorOptions', () => { }) expect(options.find).toEqual(monacoFindOptions) + expect(options.dropIntoEditor).toEqual({ enabled: false }) expect(options.wordWrap).toBe('on') expect(options.lineNumbers).toBe('off') expect(options.minimap).toEqual({ enabled: false }) diff --git a/src/renderer/src/components/automations/automation-editor-prompt-options.ts b/src/renderer/src/components/automations/automation-editor-prompt-options.ts index f43a3160e356..0d47b3aeec4c 100644 --- a/src/renderer/src/components/automations/automation-editor-prompt-options.ts +++ b/src/renderer/src/components/automations/automation-editor-prompt-options.ts @@ -11,6 +11,7 @@ export function buildAutomationPromptEditorOptions(args: { ariaLabel: args.ariaLabel, automaticLayout: true, contextmenu: true, + dropIntoEditor: { enabled: false }, folding: false, fontFamily: args.fontFamily, fontSize: args.fontSize, diff --git a/src/renderer/src/components/editor/DiffSectionBody.tsx b/src/renderer/src/components/editor/DiffSectionBody.tsx index 02cb001093a8..0bf660fa3696 100644 --- a/src/renderer/src/components/editor/DiffSectionBody.tsx +++ b/src/renderer/src/components/editor/DiffSectionBody.tsx @@ -216,6 +216,7 @@ export function DiffSectionBody({ keepCurrentOriginalModel keepCurrentModifiedModel options={{ + dropIntoEditor: { enabled: false }, // Native EditContext focus scrolls its hidden input into the combined viewport. editContext: false, readOnly: !isEditable, diff --git a/src/renderer/src/components/editor/DiffViewer.tsx b/src/renderer/src/components/editor/DiffViewer.tsx index cf262bfb3bb0..94116d758b30 100644 --- a/src/renderer/src/components/editor/DiffViewer.tsx +++ b/src/renderer/src/components/editor/DiffViewer.tsx @@ -340,6 +340,7 @@ export default function DiffViewer({ keepCurrentOriginalModel keepCurrentModifiedModel options={{ + dropIntoEditor: { enabled: false }, readOnly: !editable, originalEditable: false, renderSideBySide: sideBySide, diff --git a/src/renderer/src/components/editor/IpynbCellEditor.tsx b/src/renderer/src/components/editor/IpynbCellEditor.tsx index e9b29768e441..4bd32f00232c 100644 --- a/src/renderer/src/components/editor/IpynbCellEditor.tsx +++ b/src/renderer/src/components/editor/IpynbCellEditor.tsx @@ -179,6 +179,7 @@ function IpynbSourceEditor({ const { lineHeight, paddingX, paddingY } = CODE_LAYOUT const model = monaco.editor.createModel(source, cell.language) const editorInstance = monaco.editor.create(container, { + dropIntoEditor: { enabled: false }, model, automaticLayout: true, fontFamily, diff --git a/src/renderer/src/components/editor/MonacoEditor.tsx b/src/renderer/src/components/editor/MonacoEditor.tsx index 3f1f27423cd8..10cb1bb7c808 100644 --- a/src/renderer/src/components/editor/MonacoEditor.tsx +++ b/src/renderer/src/components/editor/MonacoEditor.tsx @@ -252,6 +252,7 @@ export default function MonacoEditor({ onChange={contentSync.handleChange} onMount={handleMount} options={{ + dropIntoEditor: { enabled: false }, // `IGlobalEditorOptions`, not per-editor: setting it here pins it for every // Monaco surface (diff, Peek) too, so this is the only site that needs it. // Defense-in-depth only — it does NOT guard the Monarch embed recursion, diff --git a/src/renderer/src/components/sidebar/WorktreeList.status-lane-lineage-drop.test.tsx b/src/renderer/src/components/sidebar/WorktreeList.status-lane-lineage-drop.test.tsx index 4a8059db50ef..17a0e5846742 100644 --- a/src/renderer/src/components/sidebar/WorktreeList.status-lane-lineage-drop.test.tsx +++ b/src/renderer/src/components/sidebar/WorktreeList.status-lane-lineage-drop.test.tsx @@ -360,11 +360,12 @@ function findStatusLaneHeader(container: HTMLElement, status: WorkspaceStatus): async function dropWorktreesOnStatusLane( header: HTMLElement, - worktreeIds: readonly string[] + worktreeIds: readonly string[], + dataTransfer: DataTransfer = makeWorktreeIdDataTransfer(worktreeIds) ): Promise { const event = new Event('drop', { bubbles: true, cancelable: true }) Object.defineProperty(event, 'dataTransfer', { - value: makeWorktreeIdDataTransfer(worktreeIds) + value: dataTransfer }) await act(async () => { header.dispatchEvent(event) @@ -413,6 +414,23 @@ describe('WorktreeList status-lane drop carries visible lineage children (#9083) expect(updates.get('child')).toEqual({ workspaceStatus: 'todo' }) }) + it('ignores a hybrid file drag with a workspace id in plain text', async () => { + const container = await renderWorktreeList() + const todoHeader = findStatusLaneHeader(container, 'todo') + const hybridTransfer = { + types: ['Files', 'text/plain'], + files: [{ name: 'notes.txt' }], + getData: (type: string) => (type === 'text/plain' ? 'parent' : '') + } + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: The drop reader uses only types and getData from this fixture. + const dataTransfer = hybridTransfer as unknown as DataTransfer + + await dropWorktreesOnStatusLane(todoHeader, ['parent'], dataTransfer) + + expect(mockStore.updateWorktreeMeta).not.toHaveBeenCalled() + expect(mockStore.updateWorktreesMeta).not.toHaveBeenCalled() + }) + it('leaves worktrees in other lanes untouched', async () => { const container = await renderWorktreeList() const todoHeader = findStatusLaneHeader(container, 'todo') diff --git a/src/renderer/src/components/sidebar/workspace-status-drag-data.ts b/src/renderer/src/components/sidebar/workspace-status-drag-data.ts index 83e3d0aa1831..9f698b4e9b79 100644 --- a/src/renderer/src/components/sidebar/workspace-status-drag-data.ts +++ b/src/renderer/src/components/sidebar/workspace-status-drag-data.ts @@ -24,17 +24,7 @@ export function writeWorkspaceDragData( export function readWorkspaceDragData(dataTransfer: DataTransfer): string | null { const typed = readWorkspaceStatusDragPayload(dataTransfer, WORKSPACE_STATUS_DRAG_TYPE) - if (typed.status === 'ok') { - return typed.value - } - if (typed.status === 'too-large') { - return null - } - const plain = readWorkspaceStatusDragPayload(dataTransfer, 'text/plain') - if (plain.status === 'ok') { - return plain.value - } - return null + return typed.status === 'ok' ? typed.value : null } export function readWorkspaceDragDataIds(dataTransfer: DataTransfer): string[] { @@ -74,8 +64,7 @@ export function hasWorkspaceDragData(dataTransfer: DataTransfer): boolean { const types = Array.from(dataTransfer.types) return ( hasBoundedWorkspaceStatusDragPayload(dataTransfer, types, WORKSPACE_STATUS_DRAG_IDS_TYPE) || - hasBoundedWorkspaceStatusDragPayload(dataTransfer, types, WORKSPACE_STATUS_DRAG_TYPE) || - hasBoundedWorkspaceStatusDragPayload(dataTransfer, types, 'text/plain') + hasBoundedWorkspaceStatusDragPayload(dataTransfer, types, WORKSPACE_STATUS_DRAG_TYPE) ) } diff --git a/src/renderer/src/components/sidebar/workspace-status.test.ts b/src/renderer/src/components/sidebar/workspace-status.test.ts index 54d03714dcbc..4ed73919c921 100644 --- a/src/renderer/src/components/sidebar/workspace-status.test.ts +++ b/src/renderer/src/components/sidebar/workspace-status.test.ts @@ -12,10 +12,11 @@ import { class TestDataTransfer { effectAllowed = 'uninitialized' + files: unknown[] = [] private readonly values = new Map() get types(): string[] { - return [...this.values.keys()] + return this.files.length > 0 ? [...this.values.keys(), 'Files'] : [...this.values.keys()] } getData(type: string): string { @@ -67,19 +68,21 @@ describe('workspace status drag data', () => { expect(readWorkspaceDragDataIds(dataTransfer)).toEqual(['wt-1', 'wt-2']) }) - it('ignores oversized plain-text workspace drag fallbacks', () => { - const dataTransfer = new TestDataTransfer() as unknown as DataTransfer - const secret = 'workspace-drag-secret' - dataTransfer.setData('text/plain', secret + 'x'.repeat(WORKSPACE_STATUS_DRAG_PAYLOAD_MAX_BYTES)) + it('ignores a hybrid file drag carrying a workspace id as plain text', () => { + const transfer = new TestDataTransfer() + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: The reader uses only types and getData, both supplied by this test transfer. + const dataTransfer = transfer as unknown as DataTransfer + dataTransfer.setData('text/plain', 'wt-1') + transfer.files = [{ name: 'notes.txt' }] expect(readWorkspaceDragData(dataTransfer)).toBeNull() expect(readWorkspaceDragDataIds(dataTransfer)).toEqual([]) expect(hasWorkspaceDragData(dataTransfer)).toBe(false) }) - it('ignores multibyte oversized workspace drag fallbacks', () => { + it('ignores plain text without an Orca workspace drag type', () => { const dataTransfer = new TestDataTransfer() as unknown as DataTransfer - dataTransfer.setData('text/plain', '😀'.repeat(4097)) + dataTransfer.setData('text/plain', 'wt-1') expect(readWorkspaceDragData(dataTransfer)).toBeNull() expect(readWorkspaceDragDataIds(dataTransfer)).toEqual([]) diff --git a/src/renderer/src/hooks/use-os-file-drop-owner.test.tsx b/src/renderer/src/hooks/use-os-file-drop-owner.test.tsx new file mode 100644 index 000000000000..f6519bd180db --- /dev/null +++ b/src/renderer/src/hooks/use-os-file-drop-owner.test.tsx @@ -0,0 +1,274 @@ +// @vitest-environment happy-dom +import { act, cleanup, render } from '@testing-library/react' +import { useRef } from 'react' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { ORCA_INTERNAL_FILE_DRAG_TYPE } from '../../../shared/native-file-drop' +import { + WORKSPACE_FILE_DRAG_SOURCE_MIME, + WORKSPACE_FILE_PATHS_MIME +} from '../lib/workspace-file-drag' +import type { PreparedDroppedPaths } from '../../../shared/native-file-drop-preparation' +import { useOsFileDropOwner } from './use-os-file-drop-owner' + +type DropHandler = (prepared: PreparedDroppedPaths, context: { target: EventTarget | null }) => void + +const getPathForFile = vi.fn((file: File) => `/dropped/${file.name}`) +const prepareDroppedPaths = vi.fn( + async ({ paths }: { paths: string[] }): Promise => ({ + paths, + failures: [] + }) +) + +function Owner({ + onDrop, + canAccept, + children +}: { + onDrop: DropHandler + canAccept?: boolean + children?: React.ReactNode +}): React.JSX.Element { + const ownerElementRef = useRef(null) + const ownerRef = useOsFileDropOwner(ownerElementRef, { consumer: 'agent', onDrop, canAccept }) + return ( +
+ {children} +
+ ) +} + +function drag( + target: Element, + type: 'dragover' | 'drop', + files: File[] = [new File(['a'], 'a.txt')], + options: { trusted?: boolean; types?: string[] } = {} +): { event: Event; transfer: { dropEffect: string } } { + const transfer = { types: options.types ?? ['Files'], files, dropEffect: 'move' } + const event = new Event(type, { bubbles: true, cancelable: true, composed: true }) + Object.defineProperty(event, 'dataTransfer', { value: transfer }) + Object.defineProperty(event, 'isTrusted', { value: options.trusted ?? true }) + act(() => { + target.dispatchEvent(event) + }) + return { event, transfer } +} + +beforeEach(() => { + vi.stubGlobal('api', { getPathForFile, fs: { prepareDroppedPaths } }) + getPathForFile.mockClear() + prepareDroppedPaths.mockClear() +}) + +afterEach(() => { + cleanup() + vi.unstubAllGlobals() +}) + +describe('useOsFileDropOwner', () => { + it('registers the root with a callback ref and delivers one prepared drop to the nearest owner', async () => { + const outerDrop = vi.fn() + const innerDrop = vi.fn() + const view = render( + + + + + + ) + const roots = view.getAllByTestId('owner') + const target = view.getByTestId('target') + expect(roots.every((root) => root.hasAttribute('data-os-file-drop-owner'))).toBe(true) + + const hover = drag(target, 'dragover') + expect(hover.event.defaultPrevented).toBe(true) + expect(hover.transfer.dropEffect).toBe('copy') + drag(target, 'drop') + await act(async () => undefined) + + expect(prepareDroppedPaths).toHaveBeenCalledExactlyOnceWith({ + paths: ['/dropped/a.txt'], + consumer: 'agent' + }) + expect(innerDrop).toHaveBeenCalledExactlyOnceWith( + { paths: ['/dropped/a.txt'], failures: [] }, + { target } + ) + expect(outerDrop).not.toHaveBeenCalled() + view.unmount() + expect(roots.every((root) => !root.hasAttribute('data-os-file-drop-owner'))).toBe(true) + }) + + it('keeps a disabled nested owner as a refusal barrier', () => { + const outerDrop = vi.fn() + const innerDrop = vi.fn() + const view = render( + + + + + + ) + const target = view.getByTestId('target') + const hover = drag(target, 'dragover') + expect(hover.transfer.dropEffect).toBe('none') + drag(target, 'drop') + expect(outerDrop).not.toHaveBeenCalled() + expect(innerDrop).not.toHaveBeenCalled() + expect(getPathForFile).not.toHaveBeenCalled() + }) + + it('caps the file snapshot before resolving any path and reports the local rejection once', async () => { + const onDrop = vi.fn() + const view = render() + const files = Array.from({ length: 257 }, (_, index) => new File([], `${index}.txt`)) + drag(view.getByTestId('owner'), 'drop', files) + await act(async () => undefined) + expect(getPathForFile).not.toHaveBeenCalled() + expect(prepareDroppedPaths).not.toHaveBeenCalled() + expect(onDrop).toHaveBeenCalledOnce() + expect(onDrop.mock.calls[0][0]).toEqual({ + paths: [], + failures: [{ target: 'rejected', reason: 'too-many-paths', pathCount: 257, byteLength: 0 }] + }) + }) + + it('reports path byte limits and unresolved files without calling preparation', async () => { + const onDrop = vi.fn() + const view = render() + getPathForFile.mockReturnValueOnce('a'.repeat(256 * 1024 + 1)) + drag(view.getByTestId('owner'), 'drop') + await act(async () => undefined) + expect(onDrop.mock.calls[0][0].failures[0].reason).toBe('paths-too-large') + getPathForFile.mockReturnValueOnce('') + drag(view.getByTestId('owner'), 'drop') + await act(async () => undefined) + expect(onDrop.mock.calls[1][0]).toEqual({ + paths: [], + failures: [{ target: 'rejected', reason: 'unresolved-paths', pathCount: 1, byteLength: 0 }] + }) + expect(prepareDroppedPaths).not.toHaveBeenCalled() + }) + + it('ignores internal drags and untrusted drops', () => { + const onDrop = vi.fn() + const view = render() + const root = view.getByTestId('owner') + for (const type of [ + ORCA_INTERNAL_FILE_DRAG_TYPE, + WORKSPACE_FILE_PATHS_MIME, + WORKSPACE_FILE_DRAG_SOURCE_MIME + ]) { + const internal = drag(root, 'drop', undefined, { types: ['Files', type] }) + expect(internal.event.defaultPrevented).toBe(false) + } + drag(root, 'drop', undefined, { trusted: false }) + expect(onDrop).not.toHaveBeenCalled() + expect(prepareDroppedPaths).not.toHaveBeenCalled() + }) + + it('reports a preparation failure once without passing unprepared paths', async () => { + const onDrop = vi.fn() + prepareDroppedPaths.mockRejectedValueOnce(new Error('copy failed')) + const view = render() + drag(view.getByTestId('owner'), 'drop') + await act(async () => undefined) + expect(onDrop).toHaveBeenCalledOnce() + expect(onDrop.mock.calls[0][0]).toEqual({ + paths: [], + failures: [ + { + target: 'rejected', + reason: 'temp-copy-failed', + commonReason: 'copy-failed', + pathCount: 1, + byteLength: '/dropped/a.txt'.length + } + ] + }) + }) + + it('reports a missing web path bridge to the owner', async () => { + const onDrop = vi.fn() + vi.stubGlobal('api', { fs: { prepareDroppedPaths } }) + const view = render() + drag(view.getByTestId('owner'), 'drop') + await act(async () => undefined) + expect(onDrop.mock.calls[0][0].failures[0].reason).toBe('unresolved-paths') + expect(prepareDroppedPaths).not.toHaveBeenCalled() + }) + + it('delivers drops in order within an owner while another owner can finish', async () => { + const firstDrop = vi.fn() + const secondDrop = vi.fn() + let finishFirst: ((prepared: PreparedDroppedPaths) => void) | undefined + prepareDroppedPaths.mockImplementationOnce( + () => + new Promise((resolve) => { + finishFirst = resolve + }) + ) + const view = render( + <> + + + + ) + const [firstRoot, secondRoot] = view.getAllByTestId('owner') + drag(firstRoot, 'drop', [new File([], 'first.txt')]) + drag(firstRoot, 'drop', [new File([], 'later.txt')]) + drag(secondRoot, 'drop', [new File([], 'other.txt')]) + await act(async () => undefined) + expect(firstDrop).not.toHaveBeenCalled() + expect(secondDrop).toHaveBeenCalledExactlyOnceWith( + { paths: ['/dropped/other.txt'], failures: [] }, + { target: secondRoot } + ) + + await act(async () => { + finishFirst?.({ paths: ['/dropped/first.txt'], failures: [] }) + }) + expect(firstDrop.mock.calls.map(([prepared]) => prepared.paths)).toEqual([ + ['/dropped/first.txt'], + ['/dropped/later.txt'] + ]) + }) + + it('does not deliver a prepared drop after its owner detaches', async () => { + const onDrop = vi.fn() + let finish: ((prepared: PreparedDroppedPaths) => void) | undefined + prepareDroppedPaths.mockImplementationOnce( + () => + new Promise((resolve) => { + finish = resolve + }) + ) + const view = render() + drag(view.getByTestId('owner'), 'drop') + view.unmount() + await act(async () => { + finish?.({ paths: ['/dropped/a.txt'], failures: [] }) + }) + expect(onDrop).not.toHaveBeenCalled() + }) + + it('reports a callback failure and still delivers the next drop', async () => { + const reported = vi.spyOn(console, 'error').mockImplementation(() => undefined) + const onDrop = vi.fn() + onDrop.mockImplementationOnce(() => { + throw new Error('consumer failed') + }) + try { + const view = render() + const root = view.getByTestId('owner') + drag(root, 'drop', [new File([], 'first.txt')]) + drag(root, 'drop', [new File([], 'second.txt')]) + await act(async () => undefined) + expect(onDrop).toHaveBeenCalledTimes(2) + expect(reported).toHaveBeenCalledOnce() + expect(reported.mock.calls[0][1]).toEqual(new Error('consumer failed')) + } finally { + reported.mockRestore() + } + }) +}) diff --git a/src/renderer/src/hooks/use-os-file-drop-owner.ts b/src/renderer/src/hooks/use-os-file-drop-owner.ts new file mode 100644 index 000000000000..0ecc1a65c1a5 --- /dev/null +++ b/src/renderer/src/hooks/use-os-file-drop-owner.ts @@ -0,0 +1,193 @@ +import { useCallback, useLayoutEffect, useRef, type RefObject } from 'react' +import { + createRejectedNativeFileDropPayload, + NATIVE_FILE_DROP_MAX_PATHS, + validateNativeFileDropPaths, + type NativeFileDropRejectedPayload +} from '../../../shared/native-file-drop' +import { + OS_FILE_DROP_OWNER_ATTRIBUTE, + type DroppedPathConsumer, + type PreparedDroppedPaths +} from '../../../shared/native-file-drop-preparation' +import { hasOsFileDragTypes } from '../lib/os-file-drop-cancellation-guard' + +type OsFileDropOwnerOptions = { + consumer: DroppedPathConsumer + canAccept?: boolean + onDrop: ( + prepared: PreparedDroppedPaths, + context: { target: EventTarget | null } + ) => void | Promise +} + +const registeredRoots = new WeakMap() + +function isNearestOwner(root: HTMLElement, event: DragEvent): boolean { + for (const entry of event.composedPath()) { + if (entry instanceof HTMLElement && registeredRoots.has(entry)) { + return entry === root + } + } + return false +} + +function rejectedDrop( + reason: NativeFileDropRejectedPayload['reason'], + pathCount: number, + byteLength = 0 +): PreparedDroppedPaths { + return { paths: [], failures: [{ target: 'rejected', reason, pathCount, byteLength }] } +} + +/** Attach the returned callback ref to the element that owns an OS file drop. */ +export function useOsFileDropOwner( + ownerRef: RefObject, + options: OsFileDropOwnerOptions +): (root: HTMLElement | null) => void { + const optionsRef = useRef(options) + const detachRef = useRef<(() => void) | null>(null) + + useLayoutEffect(() => { + optionsRef.current = options + }, [options]) + + return useCallback( + (root: HTMLElement | null) => { + detachRef.current?.() + detachRef.current = null + ownerRef.current = root + if (!root) { + return + } + + let attached = true + let deliveryTail = Promise.resolve() + const queueDelivery = ( + prepared: PreparedDroppedPaths | Promise, + deliver: OsFileDropOwnerOptions['onDrop'], + target: EventTarget | null + ): void => { + deliveryTail = deliveryTail + .then(async () => { + const result = await prepared + if (attached) { + await deliver(result, { target }) + } + }) + .catch((error: unknown) => console.error('OS file drop owner callback failed', error)) + } + + const onDragOver = (event: DragEvent): void => { + if (!hasOsFileDragTypes(event.dataTransfer?.types) || !isNearestOwner(root, event)) { + return + } + event.preventDefault() + event.stopPropagation() + if (event.dataTransfer) { + event.dataTransfer.dropEffect = optionsRef.current.canAccept === false ? 'none' : 'copy' + } + } + + const onDrop = (event: DragEvent): void => { + if (!hasOsFileDragTypes(event.dataTransfer?.types) || !isNearestOwner(root, event)) { + return + } + event.preventDefault() + event.stopPropagation() + if (!event.isTrusted || optionsRef.current.canAccept === false) { + return + } + + const { consumer, onDrop: deliver } = optionsRef.current + const target = event.target + const files = Array.from(event.dataTransfer?.files ?? []) + if (files.length > NATIVE_FILE_DROP_MAX_PATHS) { + queueDelivery( + { + paths: [], + failures: [ + createRejectedNativeFileDropPayload({ + status: 'rejected', + reason: 'too-many-paths', + pathCount: files.length, + byteLength: 0 + }) + ] + }, + deliver, + target + ) + return + } + + const getPathForFile = window.api?.getPathForFile + if (!getPathForFile) { + queueDelivery(rejectedDrop('unresolved-paths', files.length), deliver, target) + return + } + + const paths: string[] = [] + for (const file of files) { + try { + const path = getPathForFile(file) + if (path) { + paths.push(path) + } + } catch { + // A virtual file may not have a local path. + } + } + if (paths.length === 0) { + queueDelivery(rejectedDrop('unresolved-paths', files.length), deliver, target) + return + } + + const validation = validateNativeFileDropPaths(paths) + if (validation.status === 'rejected') { + queueDelivery( + { paths: [], failures: [createRejectedNativeFileDropPayload(validation)] }, + deliver, + target + ) + return + } + + const preparation = (async (): Promise => { + let prepared: PreparedDroppedPaths + try { + prepared = await window.api.fs.prepareDroppedPaths({ paths, consumer }) + } catch { + prepared = { + paths: [], + failures: [ + { + target: 'rejected', + reason: 'temp-copy-failed', + commonReason: 'copy-failed', + pathCount: paths.length, + byteLength: validation.byteLength + } + ] + } + } + return prepared + })() + queueDelivery(preparation, deliver, target) + } + + root.setAttribute(OS_FILE_DROP_OWNER_ATTRIBUTE, '') + registeredRoots.set(root, true) + root.addEventListener('dragover', onDragOver, true) + root.addEventListener('drop', onDrop, true) + detachRef.current = () => { + attached = false + root.removeEventListener('dragover', onDragOver, true) + root.removeEventListener('drop', onDrop, true) + registeredRoots.delete(root) + root.removeAttribute(OS_FILE_DROP_OWNER_ATTRIBUTE) + } + }, + [ownerRef] + ) +} diff --git a/src/renderer/src/lib/os-file-drop-cancellation-guard.test.ts b/src/renderer/src/lib/os-file-drop-cancellation-guard.test.ts new file mode 100644 index 000000000000..f1cfffbc7c55 --- /dev/null +++ b/src/renderer/src/lib/os-file-drop-cancellation-guard.test.ts @@ -0,0 +1,110 @@ +// @vitest-environment happy-dom +import { afterEach, describe, expect, it, vi } from 'vitest' +import { ORCA_INTERNAL_FILE_DRAG_TYPE } from '../../../shared/native-file-drop' +import { installOsFileDropCancellationGuard } from './os-file-drop-cancellation-guard' +import { WORKSPACE_FILE_DRAG_SOURCE_MIME, WORKSPACE_FILE_PATHS_MIME } from './workspace-file-drag' + +let dispose: (() => void) | null = null + +function drag( + target: Element, + type: 'dragover' | 'drop', + types: string[] = ['Files'] +): { event: Event; transfer: { dropEffect: string } } { + const transfer = { types, dropEffect: 'move' } + const event = new Event(type, { bubbles: true, cancelable: true }) + Object.defineProperty(event, 'dataTransfer', { value: transfer }) + target.dispatchEvent(event) + return { event, transfer } +} + +afterEach(() => { + dispose?.() + dispose = null + document.body.replaceChildren() +}) + +describe('OS file drop cancellation guard', () => { + it('cancels unclaimed OS files in capture and sets none in bubble', () => { + dispose = installOsFileDropCancellationGuard() + const target = document.createElement('div') + document.body.append(target) + const seenAtTarget = vi.fn((event: Event) => event.defaultPrevented) + target.addEventListener('drop', seenAtTarget) + const hover = drag(target, 'dragover') + const drop = drag(target, 'drop') + expect(hover.event.defaultPrevented).toBe(true) + expect(hover.transfer.dropEffect).toBe('none') + expect(seenAtTarget).toHaveReturnedWith(true) + expect(drop.event.defaultPrevented).toBe(true) + expect(drop.transfer.dropEffect).toBe('none') + }) + + it('leaves internal and non-file drags alone', () => { + dispose = installOsFileDropCancellationGuard() + const target = document.createElement('div') + document.body.append(target) + for (const types of [ + ['text/plain'], + ['Files', ORCA_INTERNAL_FILE_DRAG_TYPE], + ['Files', WORKSPACE_FILE_PATHS_MIME], + ['Files', WORKSPACE_FILE_DRAG_SOURCE_MIME] + ]) { + const { event, transfer } = drag(target, 'drop', types) + expect(event.defaultPrevented).toBe(false) + expect(transfer.dropEffect).toBe('move') + } + }) + + it('lets an owner stop propagation before the unclaimed sink', () => { + dispose = installOsFileDropCancellationGuard() + const target = document.createElement('div') + document.body.append(target) + target.addEventListener( + 'drop', + (event) => { + event.preventDefault() + event.stopPropagation() + if (event.dataTransfer) { + event.dataTransfer.dropEffect = 'copy' + } + }, + true + ) + const drop = drag(target, 'drop') + expect(drop.event.defaultPrevented).toBe(true) + expect(drop.transfer.dropEffect).toBe('copy') + }) + + it('preserves a drop claimed by an earlier document capture listener', () => { + const claim = (event: DragEvent): void => { + event.preventDefault() + if (event.dataTransfer) { + event.dataTransfer.dropEffect = 'copy' + } + } + document.addEventListener('drop', claim, true) + try { + dispose = installOsFileDropCancellationGuard() + const target = document.createElement('div') + document.body.append(target) + const drop = drag(target, 'drop') + expect(drop.event.defaultPrevented).toBe(true) + expect(drop.transfer.dropEffect).toBe('copy') + } finally { + document.removeEventListener('drop', claim, true) + } + }) + + it('installs only once and removes the listeners on disposal', () => { + dispose = installOsFileDropCancellationGuard() + expect(installOsFileDropCancellationGuard()).toBe(dispose) + dispose() + dispose = null + const target = document.createElement('div') + document.body.append(target) + const drop = drag(target, 'drop') + expect(drop.event.defaultPrevented).toBe(false) + expect(drop.transfer.dropEffect).toBe('move') + }) +}) diff --git a/src/renderer/src/lib/os-file-drop-cancellation-guard.ts b/src/renderer/src/lib/os-file-drop-cancellation-guard.ts new file mode 100644 index 000000000000..df4a88b4b739 --- /dev/null +++ b/src/renderer/src/lib/os-file-drop-cancellation-guard.ts @@ -0,0 +1,60 @@ +import { hasNativeFileDragTypes } from '../../../shared/native-file-drop' +import { WORKSPACE_FILE_DRAG_SOURCE_MIME, WORKSPACE_FILE_PATHS_MIME } from './workspace-file-drag' + +let disposeInstalledGuard: (() => void) | null = null + +export function hasOsFileDragTypes( + types: Iterable | ArrayLike | null | undefined +): boolean { + if (!hasNativeFileDragTypes(types)) { + return false + } + const values = Array.from(types ?? []) + return ( + !values.includes(WORKSPACE_FILE_PATHS_MIME) && !values.includes(WORKSPACE_FILE_DRAG_SOURCE_MIME) + ) +} + +/** Cancels unclaimed OS file drops before the browser can navigate to them. */ +export function installOsFileDropCancellationGuard(): () => void { + if (disposeInstalledGuard) { + return disposeInstalledGuard + } + + const claimedBeforeGuard = new WeakSet() + const onCapture = (event: DragEvent): void => { + if (!hasOsFileDragTypes(event.dataTransfer?.types)) { + return + } + if (event.defaultPrevented) { + claimedBeforeGuard.add(event) + } + event.preventDefault() + } + const onUnclaimed = (event: DragEvent): void => { + if (!hasOsFileDragTypes(event.dataTransfer?.types) || claimedBeforeGuard.has(event)) { + return + } + event.preventDefault() + if (event.dataTransfer) { + event.dataTransfer.dropEffect = 'none' + } + } + + document.addEventListener('dragover', onCapture, true) + document.addEventListener('drop', onCapture, true) + document.addEventListener('dragover', onUnclaimed) + document.addEventListener('drop', onUnclaimed) + + const dispose = (): void => { + document.removeEventListener('dragover', onCapture, true) + document.removeEventListener('drop', onCapture, true) + document.removeEventListener('dragover', onUnclaimed) + document.removeEventListener('drop', onUnclaimed) + if (disposeInstalledGuard === dispose) { + disposeInstalledGuard = null + } + } + disposeInstalledGuard = dispose + return dispose +} diff --git a/src/renderer/src/lib/os-file-drop-preload-integration.test.tsx b/src/renderer/src/lib/os-file-drop-preload-integration.test.tsx new file mode 100644 index 000000000000..3ca3bd498ce3 --- /dev/null +++ b/src/renderer/src/lib/os-file-drop-preload-integration.test.tsx @@ -0,0 +1,136 @@ +// @vitest-environment happy-dom +import { act, cleanup, render } from '@testing-library/react' +import { useRef } from 'react' +import { afterEach, beforeAll, beforeEach, describe, expect, it, vi } from 'vitest' +import type { PreparedDroppedPaths } from '../../../shared/native-file-drop-preparation' + +const electron = vi.hoisted(() => ({ + on: vi.fn(), + removeListener: vi.fn(), + send: vi.fn(), + getPathForFile: vi.fn((file: File) => `/dropped/${file.name}`) +})) + +vi.mock('electron', () => ({ + ipcRenderer: electron, + webUtils: { getPathForFile: electron.getPathForFile } +})) + +import { installNativeFileDropHandlers } from '../../../preload/preload-runtime-support' +import { useOsFileDropOwner } from '../hooks/use-os-file-drop-owner' +import { installOsFileDropCancellationGuard } from './os-file-drop-cancellation-guard' + +const prepareDroppedPaths = vi.fn( + async ({ paths }: { paths: string[] }): Promise => ({ paths, failures: [] }) +) + +function OwnerProbe({ + onDrop, + legacyChild = false +}: { + onDrop: (prepared: PreparedDroppedPaths) => void + legacyChild?: boolean +}): React.JSX.Element { + const ownerElementRef = useRef(null) + const attach = useOsFileDropOwner(ownerElementRef, { + consumer: 'agent', + onDrop: (prepared) => onDrop(prepared) + }) + return ( +
+ +
+ ) +} + +function drag( + target: Element, + type: 'dragover' | 'drop' +): { + event: Event + transfer: { dropEffect: string } +} { + const transfer = { + types: ['Files'], + files: [new File(['a'], 'a.txt')], + dropEffect: 'move' + } + const event = new Event(type, { bubbles: true, cancelable: true, composed: true }) + Object.defineProperty(event, 'dataTransfer', { value: transfer }) + Object.defineProperty(event, 'isTrusted', { value: true }) + act(() => { + target.dispatchEvent(event) + }) + return { event, transfer } +} + +let disposeGuard: (() => void) | null = null + +beforeAll(() => { + installNativeFileDropHandlers() +}) + +beforeEach(() => { + vi.stubGlobal('api', { getPathForFile: electron.getPathForFile, fs: { prepareDroppedPaths } }) + electron.send.mockClear() + electron.getPathForFile.mockClear() + prepareDroppedPaths.mockClear() + disposeGuard = installOsFileDropCancellationGuard() +}) + +afterEach(() => { + cleanup() + disposeGuard?.() + disposeGuard = null + document.body.replaceChildren() + vi.unstubAllGlobals() +}) + +describe('preload and renderer file drop guard', () => { + it('preserves preload copy acceptance and one IPC delivery on unmarked chrome', () => { + const target = document.createElement('div') + document.body.append(target) + const hover = drag(target, 'dragover') + expect(hover.event.defaultPrevented).toBe(true) + expect(hover.transfer.dropEffect).toBe('copy') + + drag(target, 'drop') + expect(electron.send).toHaveBeenCalledOnce() + expect(electron.send.mock.calls[0][0]).toBe('terminal:file-dropped-from-preload') + expect(prepareDroppedPaths).not.toHaveBeenCalled() + }) + + it('lets a migrated owner claim the real event and keep the copy cursor', async () => { + const onDrop = vi.fn() + const view = render() + const target = view.getByTestId('target') + const hover = drag(target, 'dragover') + expect(hover.transfer.dropEffect).toBe('copy') + drag(target, 'drop') + await act(async () => undefined) + expect(electron.send).not.toHaveBeenCalled() + expect(prepareDroppedPaths).toHaveBeenCalledExactlyOnceWith({ + paths: ['/dropped/a.txt'], + consumer: 'agent' + }) + expect(onDrop).toHaveBeenCalledExactlyOnceWith({ + paths: ['/dropped/a.txt'], + failures: [] + }) + }) + + it('keeps a legacy child as a barrier inside a migrated owner', () => { + const onDrop = vi.fn() + const view = render() + const target = view.getByTestId('target') + const hover = drag(target, 'dragover') + expect(hover.transfer.dropEffect).toBe('copy') + drag(target, 'drop') + expect(electron.send).toHaveBeenCalledOnce() + expect(prepareDroppedPaths).not.toHaveBeenCalled() + expect(onDrop).not.toHaveBeenCalled() + }) +}) diff --git a/src/renderer/src/main.tsx b/src/renderer/src/main.tsx index 36239d525164..4b0b316ce1db 100644 --- a/src/renderer/src/main.tsx +++ b/src/renderer/src/main.tsx @@ -24,7 +24,10 @@ import { getOrCreateRendererRoot } from './lib/react-renderer-root' import { primeTerminalWebglAddon } from './lib/pane-manager/pane-webgl-renderer' import { SkillWarningPreviewLauncher } from './components/skills/SkillWarningPreviewLauncher' import { installBrowserClientPageRenderer } from './components/browser-pane/browser-client-page-renderer-installation' +import { installOsFileDropCancellationGuard } from './lib/os-file-drop-cancellation-guard' +const disposeOsFileDropGuard = installOsFileDropCancellationGuard() +import.meta.hot?.dispose(disposeOsFileDropGuard) recordRendererCrashBreadcrumb('renderer_bootstrap_started', { dev: import.meta.env.DEV }) installRendererCrashDiagnostics() installTypingLatencyDiagnostic() diff --git a/src/renderer/src/popout.tsx b/src/renderer/src/popout.tsx index 3c84112d1a32..61d038f3e5d0 100644 --- a/src/renderer/src/popout.tsx +++ b/src/renderer/src/popout.tsx @@ -22,7 +22,10 @@ import { useAppStore } from './store' import type { GlobalSettings } from '../../shared/global-settings-types' import { getOrCreateRendererRoot } from './lib/react-renderer-root' import { setReactCommitCascadeRendererSurface } from './lib/react-commit-cascade-telemetry' +import { installOsFileDropCancellationGuard } from './lib/os-file-drop-cancellation-guard' +const disposeOsFileDropGuard = installOsFileDropCancellationGuard() +import.meta.hot?.dispose(disposeOsFileDropGuard) // Why: the pop-out window is a separate BrowserWindow with its own React root, // so it must run the same renderer bootstrap as main.tsx (crash diagnostics, // theme, i18n, error boundary) rather than inheriting anything from the main diff --git a/src/renderer/src/web/main.tsx b/src/renderer/src/web/main.tsx index 577f2b57de9c..695858cca185 100644 --- a/src/renderer/src/web/main.tsx +++ b/src/renderer/src/web/main.tsx @@ -19,7 +19,10 @@ import { import { installWebPreloadApi } from './web-preload-api' import { I18nProvider } from '../i18n/I18nProvider' import { translate } from '../i18n/i18n' +import { installOsFileDropCancellationGuard } from '../lib/os-file-drop-cancellation-guard' +const disposeOsFileDropGuard = installOsFileDropCancellationGuard() +import.meta.hot?.dispose(disposeOsFileDropGuard) const App = lazy(() => import('../App')) function WebRoot(): React.JSX.Element { diff --git a/src/renderer/src/web/preload-api/web-filesystem-api.ts b/src/renderer/src/web/preload-api/web-filesystem-api.ts index a393e12fa469..e98f550f958f 100644 --- a/src/renderer/src/web/preload-api/web-filesystem-api.ts +++ b/src/renderer/src/web/preload-api/web-filesystem-api.ts @@ -22,6 +22,9 @@ import { noopUnsubscribe } from './web-storage' export function createFileApi(): NonNullable['fs']> { return { + prepareDroppedPaths: async () => { + throw new Error('Preparing dropped file paths is not supported in the web client') + }, readFileChunk: async ({ filePath, offset, length }) => { const file = await resolveRuntimeFilePath(filePath) return callRuntimeResult('files.readChunk', { diff --git a/src/renderer/src/web/preload-api/web-platform-api.ts b/src/renderer/src/web/preload-api/web-platform-api.ts index 22e88ff66713..35b950c42e5f 100644 --- a/src/renderer/src/web/preload-api/web-platform-api.ts +++ b/src/renderer/src/web/preload-api/web-platform-api.ts @@ -3,6 +3,7 @@ import { getBrowserPlatform } from './web-storage' export function createWebPlatformApi(): Partial { return { + getPathForFile: () => '', platform: { get: () => ({ platform: getBrowserPlatform(), diff --git a/src/renderer/src/web/web-preload-api-filesystem.test.ts b/src/renderer/src/web/web-preload-api-filesystem.test.ts index 74cd8612ff38..1929898505a9 100644 --- a/src/renderer/src/web/web-preload-api-filesystem.test.ts +++ b/src/renderer/src/web/web-preload-api-filesystem.test.ts @@ -16,6 +16,14 @@ describe('web file preload API', () => { vi.doUnmock('./web-runtime-client') }) + it('has no local file path bridge and refuses path preparation without remote calls', async () => { + const { api } = await installApi('Linux') + expect(api.getPathForFile?.(new File(['x'], 'notes.txt'))).toBe('') + await expect( + api.fs.prepareDroppedPaths({ paths: ['/files/notes.txt'], consumer: 'agent' }) + ).rejects.toThrow('Preparing dropped file paths is not supported in the web client') + }) + it('rejects native save-dialog downloads in paired web clients', async () => { const { api } = await installApi('Linux') diff --git a/src/shared/native-file-drop-preparation.ts b/src/shared/native-file-drop-preparation.ts new file mode 100644 index 000000000000..94c553c86cc2 --- /dev/null +++ b/src/shared/native-file-drop-preparation.ts @@ -0,0 +1,15 @@ +import type { NativeFileDropRejectedPayload } from './native-file-drop' + +export const OS_FILE_DROP_OWNER_ATTRIBUTE = 'data-os-file-drop-owner' + +export type DroppedPathConsumer = 'agent' | 'main-reader' + +export type PrepareDroppedPathsRequest = { + paths: string[] + consumer: DroppedPathConsumer +} + +export type PreparedDroppedPaths = { + paths: string[] + failures: NativeFileDropRejectedPayload[] +} From 2dcc2a94273260d6b3aec5371c2934634ae7c553 Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Mon, 5 Oct 2026 18:33:57 -0700 Subject: [PATCH 02/16] Fix terminal and chat file drop destination ownership --- .../native-chat-composer-drop-scope.test.tsx | 2 +- ...-external-attachments-destination.test.tsx | 183 +++++++++++++++ .../native-chat-resolved-path-ownership.ts | 2 + ...-native-chat-external-attachments.test.tsx | 28 ++- .../use-native-chat-external-attachments.ts | 26 ++- ...e-native-chat-resolved-path-attachments.ts | 16 +- .../TerminalPaneHeaderOverlay.test.tsx | 142 ++++++++++- .../TerminalPaneHeaderOverlay.tsx | 5 +- .../terminal-drop-handler.test.ts | 220 ++++++++++++------ .../terminal-pane/terminal-drop-handler.ts | 20 +- .../terminal-drop-internal-handler.test.ts | 201 +++++++++++----- .../terminal-drop-local-workspace.test.ts | 84 +++++++ .../terminal-drop-pane-resolution.test.ts | 39 ++++ .../terminal-drop-pane-resolution.ts | 26 +-- .../terminal-drop-transport-owner.test.ts | 39 ++++ .../terminal-drop-transport-owner.ts | 46 ++++ .../terminal-drop-worktree-path.test.ts | 58 +++++ .../terminal-drop-worktree-path.ts | 14 +- .../terminal-native-file-drop.ts | 27 +-- ...inal-pane-global-effects-file-drop.test.ts | 13 +- .../use-terminal-pane-global-effects.ts | 13 +- 21 files changed, 988 insertions(+), 216 deletions(-) create mode 100644 src/renderer/src/components/native-chat/native-chat-external-attachments-destination.test.tsx create mode 100644 src/renderer/src/components/terminal-pane/terminal-drop-local-workspace.test.ts create mode 100644 src/renderer/src/components/terminal-pane/terminal-drop-pane-resolution.test.ts create mode 100644 src/renderer/src/components/terminal-pane/terminal-drop-transport-owner.test.ts create mode 100644 src/renderer/src/components/terminal-pane/terminal-drop-transport-owner.ts create mode 100644 src/renderer/src/components/terminal-pane/terminal-drop-worktree-path.test.ts diff --git a/src/renderer/src/components/native-chat/native-chat-composer-drop-scope.test.tsx b/src/renderer/src/components/native-chat/native-chat-composer-drop-scope.test.tsx index 90029987cf44..73953f509a9e 100644 --- a/src/renderer/src/components/native-chat/native-chat-composer-drop-scope.test.tsx +++ b/src/renderer/src/components/native-chat/native-chat-composer-drop-scope.test.tsx @@ -32,7 +32,7 @@ const intake = vi.hoisted(() => ({ readFile: vi.fn(), upload: vi.fn() })) -vi.mock('@/store', () => ({ useAppStore: { getState: () => ({}) } })) +vi.mock('@/store', () => ({ useAppStore: { getState: () => ({ tabsByWorktree: {} }) } })) // Keeps the real notice strings so the silent-failure guards assert what users see. vi.mock('./native-chat-attachment-upload', async (importOriginal) => ({ ...(await importOriginal()), diff --git a/src/renderer/src/components/native-chat/native-chat-external-attachments-destination.test.tsx b/src/renderer/src/components/native-chat/native-chat-external-attachments-destination.test.tsx new file mode 100644 index 000000000000..3e9ec8b99491 --- /dev/null +++ b/src/renderer/src/components/native-chat/native-chat-external-attachments-destination.test.tsx @@ -0,0 +1,183 @@ +// @vitest-environment happy-dom +import { act, useRef, useState } from 'react' +import { createRoot, type Root } from 'react-dom/client' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import type * as AttachmentUploadModule from './native-chat-attachment-upload' +import type { NativeChatAttachmentOwner } from './native-chat-attachment-upload' +import { useNativeChatExternalAttachments } from './use-native-chat-external-attachments' +import { + clearNativeChatAttachmentCacheForTests, + useNativeChatComposerAttachments +} from './use-native-chat-composer-attachments' + +const mocks = vi.hoisted(() => { + const state: { + owner: NativeChatAttachmentOwner + remoteTarget: boolean + stat: ReturnType + upload: ReturnType + tabsByWorktree: Record + } = { + owner: { kind: 'local' }, + remoteTarget: false, + stat: vi.fn(), + upload: vi.fn(), + tabsByWorktree: { 'workspace-1': [{ id: 'tab-1' }] } + } + return state +}) +vi.mock('@/store', () => ({ + useAppStore: { getState: () => ({ tabsByWorktree: mocks.tabsByWorktree }) } +})) +vi.mock('@/i18n/i18n', () => ({ translate: (_key: string, fallback: string) => fallback })) +vi.mock('@/runtime/runtime-terminal-inspection', () => ({ + isRemoteRuntimePtyId: () => mocks.remoteTarget +})) +vi.mock('./native-chat-attachment-upload', async (importOriginal) => ({ + ...(await importOriginal()), + resolveNativeChatAttachmentOwner: () => mocks.owner, + resolveNativeChatAttachmentOwnerForWorktree: () => mocks.owner, + uploadNativeChatAttachmentPaths: mocks.upload +})) + +type Api = ReturnType & + ReturnType +let api: Api | undefined +let root: Root +let container: HTMLDivElement +let composing = true + +function Probe({ worktreeId }: { worktreeId?: string }): React.JSX.Element { + const [draft, setDraft] = useState('') + const [caret, setCaret] = useState(0) + const [notice, setNotice] = useState(null) + const textareaRef = useRef(null) + const attachments = useNativeChatComposerAttachments({ + attachmentScopeKey: 'drop-test', + caret, + disabled: false, + isComposing: () => composing, + resolveTarget: () => ({ ptyId: 'pty-1', settings: { activeRuntimeEnvironmentId: null } }), + textareaRef, + setCaret, + setDraft, + setNotice + }) + const external = useNativeChatExternalAttachments({ + terminalTabId: 'tab-1', + structuredWorktreeId: worktreeId, + disabled: false, + attachResolvedPaths: attachments.attachResolvedPaths, + setNotice + }) + api = { ...attachments, ...external } + return ( + <> +