From cc291dba1dcf62be1dd9473efdcc863073d23c39 Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Thu, 8 Oct 2026 10:30:49 -0700 Subject: [PATCH] Keep picked skills and pending uploads when native chat composers close (#26538) * Preserve native chat skills and uploads when composers close * Own menu image pastes before saving begins * Keep paste ownership independent of image presentation * Show pending pastes so stalled saves can be removed * Register confirmed image pastes and preserve preview ownership * Keep clipboard image presence and saving on one source selector * Describe clipboard image formats in upload routing fixtures --- ...ard-agent-session-attachment-paste.test.ts | 6 +- .../window/clipboard-image-source.test.ts | 72 +++++ src/main/window/clipboard-image-source.ts | 33 +++ .../window/clipboard-ipc-handlers.test.ts | 129 ++++----- src/main/window/clipboard-ipc-handlers.ts | 24 +- .../clipboard-runtime-owned-ssh-paste.test.ts | 3 +- .../window/clipboard-windows-image-file.ts | 13 +- .../NativeChatComposer.paste-remount.test.tsx | 229 +++++++++++++-- .../native-chat/NativeChatComposer.tsx | 1 + .../native-chat/NativeChatComposerField.tsx | 7 +- ...ative-chat-composer-draft-addition.test.ts | 41 +++ .../native-chat-composer-draft-addition.ts | 5 +- .../native-chat-composer-draft-journal.ts | 1 - .../native-chat-composer-draft-store.ts | 6 +- .../native-chat-composer-paste-options.ts | 23 ++ ...e-chat-composer-upload-completion.test.tsx | 230 +++++++++++++++ .../native-chat/native-chat-draft-cache.ts | 9 +- ...tive-chat-pending-attachment-cache.test.ts | 40 +-- .../native-chat-pending-attachment-cache.ts | 40 +-- .../native-chat-prompt-document-text.test.ts | 66 +++++ .../native-chat-prompt-document-text.ts | 59 ++++ .../native-chat-prompt-document.ts | 13 +- .../native-chat-session-attachment-drop.ts | 8 +- ...-native-chat-composer-attachments.test.tsx | 40 ++- .../use-native-chat-composer-attachments.ts | 89 +++--- ...-native-chat-composer-image-input.test.tsx | 6 +- ...omposer-paste-session-attachments.test.tsx | 17 +- .../use-native-chat-composer-paste.test.tsx | 36 ++- .../use-native-chat-composer-paste.ts | 114 ++++---- ...-native-chat-external-attachments.test.tsx | 13 +- ...e-native-chat-menu-paste-lifetime.test.tsx | 273 ++++++++++++++++++ .../use-native-chat-paste-lifetime.test.tsx | 141 +++++++++ .../use-native-chat-paste-lifetime.ts | 87 ++++-- 33 files changed, 1524 insertions(+), 350 deletions(-) create mode 100644 src/main/window/clipboard-image-source.test.ts create mode 100644 src/main/window/clipboard-image-source.ts create mode 100644 src/renderer/src/components/native-chat/native-chat-composer-paste-options.ts create mode 100644 src/renderer/src/components/native-chat/native-chat-composer-upload-completion.test.tsx create mode 100644 src/renderer/src/components/native-chat/native-chat-prompt-document-text.test.ts create mode 100644 src/renderer/src/components/native-chat/native-chat-prompt-document-text.ts create mode 100644 src/renderer/src/components/native-chat/use-native-chat-menu-paste-lifetime.test.tsx create mode 100644 src/renderer/src/components/native-chat/use-native-chat-paste-lifetime.test.tsx diff --git a/src/main/window/clipboard-agent-session-attachment-paste.test.ts b/src/main/window/clipboard-agent-session-attachment-paste.test.ts index 8db77235abf..ba32e82c3b9 100644 --- a/src/main/window/clipboard-agent-session-attachment-paste.test.ts +++ b/src/main/window/clipboard-agent-session-attachment-paste.test.ts @@ -10,7 +10,11 @@ const mocks = vi.hoisted(() => ({ vi.mock('electron', () => ({ app: { getPath: vi.fn(() => '/tmp/client-user-data') }, - clipboard: { readImage: mocks.readImage }, + clipboard: { + readImage: mocks.readImage, + availableFormats: () => ['image/png'], + readBuffer: () => Buffer.alloc(0) + }, ipcMain: { removeHandler: vi.fn(), handle: mocks.handle }, nativeImage: { createFromBuffer: vi.fn() } })) diff --git a/src/main/window/clipboard-image-source.test.ts b/src/main/window/clipboard-image-source.test.ts new file mode 100644 index 00000000000..cebd03f1959 --- /dev/null +++ b/src/main/window/clipboard-image-source.test.ts @@ -0,0 +1,72 @@ +import { describe, expect, it, vi } from 'vitest' +import { readClipboardImageSource } from './clipboard-image-source' + +function clipboardReader(filePath = '', itemCount = 1, formats: string[] = ['FileNameW']) { + const shellItems = Buffer.alloc(4 + 4 * (itemCount + 1)) + shellItems.writeUInt32LE(itemCount) + const buffers: Record = { + FileNameW: Buffer.from(`${filePath}\0`, 'utf16le'), + 'Shell IDList Array': shellItems + } + return { + availableFormats: vi.fn(() => formats), + readBuffer: vi.fn((format: string) => buffers[format] ?? Buffer.alloc(0)) + } +} + +describe('readClipboardImageSource', () => { + it.each(['darwin', 'linux', 'win32'] as const)( + 'recognizes native image formats on %s without decoding', + (platform) => { + const clipboard = clipboardReader('', 1, ['text/plain', 'image/png']) + expect(readClipboardImageSource(clipboard, platform)).toEqual({ + kind: 'native', + windowsFileFormats: null + }) + } + ) + + it.each(['darwin', 'linux', 'win32'] as const)('ignores ordinary text on %s', (platform) => { + expect(readClipboardImageSource(clipboardReader('', 1, ['text/plain']), platform)).toBeNull() + }) + + it.each([ + 'C:\\Users\\alice\\图片\\shot.PNG', + '\\\\server\\share\\shot.jpeg', + '\\\\?\\C:\\Users\\alice\\shot.jpg', + '\\\\?\\UNC\\server\\share\\shot.png' + ])('recognizes a supported single Windows image file: %s', (filePath) => { + const clipboard = clipboardReader(filePath) + const source = readClipboardImageSource(clipboard, 'win32') + expect(source?.kind).toBe('windows-file') + expect(source?.windowsFileFormats).toEqual({ + fileNameW: clipboard.readBuffer('FileNameW'), + shellIdListArray: clipboard.readBuffer('Shell IDList Array') + }) + }) + + it.each([ + ['C:\\shot.pdf', 1], + ['C:\\shot.webp', 1], + ['C:\\shot.png', 2], + ['shot.png', 1], + ['C:shot.png', 1], + ['\\\\server\\pipe\\shot.png', 1], + ['C:\\one.png\0C:\\two.png', 1] + ])('rejects an unsupported file source: %s (%s items)', (filePath, itemCount) => { + expect(readClipboardImageSource(clipboardReader(filePath, itemCount), 'win32')).toBeNull() + }) + + it.each(['darwin', 'linux'] as const)('never reads Windows file formats on %s', (platform) => { + const clipboard = clipboardReader('C:\\shot.png') + expect(readClipboardImageSource(clipboard, platform)).toBeNull() + expect(clipboard.readBuffer).not.toHaveBeenCalled() + }) + + it('keeps the copied-file fallback beside advertised native image data', () => { + const clipboard = clipboardReader('C:\\shot.png', 1, ['image/png', 'FileNameW']) + const source = readClipboardImageSource(clipboard, 'win32') + expect(source?.kind).toBe('native') + expect(source?.windowsFileFormats?.fileNameW).toEqual(clipboard.readBuffer('FileNameW')) + }) +}) diff --git a/src/main/window/clipboard-image-source.ts b/src/main/window/clipboard-image-source.ts new file mode 100644 index 00000000000..f5dc72a0247 --- /dev/null +++ b/src/main/window/clipboard-image-source.ts @@ -0,0 +1,33 @@ +import { clipboardFormatsIncludeImage } from '../../shared/clipboard-image' +import { + readWindowsCopiedImageFilePath, + type WindowsClipboardFileFormats +} from './clipboard-windows-image-file' + +type ClipboardImageReader = { + availableFormats: () => string[] + readBuffer: (format: string) => Buffer +} + +type ClipboardImageSource = + | { kind: 'native'; windowsFileFormats: WindowsClipboardFileFormats | null } + | { kind: 'windows-file'; windowsFileFormats: WindowsClipboardFileFormats } + +/** Select image sources without decoding pixels or reading a copied file. */ +export function readClipboardImageSource( + clipboard: ClipboardImageReader, + platform: NodeJS.Platform = process.platform +): ClipboardImageSource | null { + const formats = + platform === 'win32' + ? { + fileNameW: clipboard.readBuffer('FileNameW'), + shellIdListArray: clipboard.readBuffer('Shell IDList Array') + } + : null + const windowsFileFormats = formats && readWindowsCopiedImageFilePath(formats) ? formats : null + if (clipboardFormatsIncludeImage(clipboard.availableFormats())) { + return { kind: 'native', windowsFileFormats } + } + return windowsFileFormats ? { kind: 'windows-file', windowsFileFormats } : null +} diff --git a/src/main/window/clipboard-ipc-handlers.test.ts b/src/main/window/clipboard-ipc-handlers.test.ts index 2d283a50de2..dc7168f7d65 100644 --- a/src/main/window/clipboard-ipc-handlers.test.ts +++ b/src/main/window/clipboard-ipc-handlers.test.ts @@ -226,6 +226,7 @@ describe('registerClipboardHandlers', () => { clipboardReadBufferMock.mockReset() clipboardReadBufferMock.mockReturnValue(Buffer.alloc(0)) clipboardWriteTextMock.mockReset() + clipboardAvailableFormatsMock.mockReturnValue(['image/png']) clipboardReadImageMock.mockReset() clipboardWriteImageMock.mockReset() clipboardWriteBufferMock.mockReset() @@ -571,62 +572,72 @@ describe('registerClipboardHandlers', () => { } }) - it('routes a Windows Explorer FileNameW image through the target-aware attachment flow', async () => { - const platformSpy = vi.spyOn(process, 'platform', 'get').mockReturnValue('win32') - const sourcePath = 'C:\\Users\\alice\\图片\\copied-image.png' - const png = Buffer.from([4, 3, 2, 1]) - clipboardReadImageMock.mockReturnValue({ isEmpty: () => true }) - clipboardReadBufferMock.mockImplementation((format: string) => - format === 'FileNameW' ? Buffer.from(`${sourcePath}\0`, 'utf16le') : shellIdListArray(1) - ) - const source = Buffer.alloc(24) - Buffer.from([137, 80, 78, 71, 13, 10, 26, 10]).copy(source) - source.writeUInt32BE(13, 8) - source.write('IHDR', 12, 'ascii') - source.writeUInt32BE(1, 16) - source.writeUInt32BE(1, 20) - const close = vi.fn().mockResolvedValue(undefined) - fsOpenMock.mockResolvedValue({ - close, - stat: vi.fn().mockResolvedValue({ isFile: () => true, size: source.byteLength }), - read: vi.fn(async (buffer: Buffer, offset: number, length: number, position: number) => { - const bytesRead = Math.min(Math.max(source.byteLength - position, 0), length) - source.copy(buffer, offset, position, position + bytesRead) - return { buffer, bytesRead } + it.each([false, true])( + 'routes a Windows Explorer image with native format advertised: %s', + async (advertisesImage) => { + setTrustedClipboardRendererWebContentsId(17) + const platformSpy = vi.spyOn(process, 'platform', 'get').mockReturnValue('win32') + clipboardAvailableFormatsMock.mockReturnValue( + advertisesImage ? ['image/png', 'FileNameW'] : ['FileNameW'] + ) + const sourcePath = 'C:\\Users\\alice\\图片\\copied-image.png' + const png = Buffer.from([4, 3, 2, 1]) + clipboardReadImageMock.mockReturnValue({ isEmpty: () => true }) + clipboardReadBufferMock.mockImplementation((format: string) => + format === 'FileNameW' ? Buffer.from(`${sourcePath}\0`, 'utf16le') : shellIdListArray(1) + ) + const source = Buffer.alloc(24) + Buffer.from([137, 80, 78, 71, 13, 10, 26, 10]).copy(source) + source.writeUInt32BE(13, 8) + source.write('IHDR', 12, 'ascii') + source.writeUInt32BE(1, 16) + source.writeUInt32BE(1, 20) + const close = vi.fn().mockResolvedValue(undefined) + fsOpenMock.mockResolvedValue({ + close, + stat: vi.fn().mockResolvedValue({ isFile: () => true, size: source.byteLength }), + read: vi.fn(async (buffer: Buffer, offset: number, length: number, position: number) => { + const bytesRead = Math.min(Math.max(source.byteLength - position, 0), length) + source.copy(buffer, offset, position, position + bytesRead) + return { buffer, bytesRead } + }) + }) + nativeImageCreateFromBufferMock.mockReturnValue({ + getSize: () => ({ height: 1, width: 1 }), + isEmpty: () => false, + toPNG: () => png + }) + const writeFileBase64 = vi.fn().mockResolvedValue(undefined) + getSshFilesystemProviderMock.mockReturnValue({ + getTempDir: vi.fn().mockResolvedValue('/var/tmp'), + writeFileBase64 }) - }) - nativeImageCreateFromBufferMock.mockReturnValue({ - getSize: () => ({ height: 1, width: 1 }), - isEmpty: () => false, - toPNG: () => png - }) - const writeFileBase64 = vi.fn().mockResolvedValue(undefined) - getSshFilesystemProviderMock.mockReturnValue({ - getTempDir: vi.fn().mockResolvedValue('/var/tmp'), - writeFileBase64 - }) - try { - registerClipboardHandlers({} as never) + try { + registerClipboardHandlers({} as never) - const handler = getRegisteredHandlers().get('clipboard:saveImageAsTempFile') - await expect(handler?.(makeClipboardEvent(), { connectionId: 'ssh-1' })).resolves.toBe( - '/var/tmp/orca-paste-1760000000000-00000000-0000-4000-8000-000000000000.png' - ) - expect(clipboardReadBufferMock).toHaveBeenCalledWith('FileNameW') - expect(clipboardReadBufferMock).toHaveBeenCalledWith('Shell IDList Array') - expect(fsOpenMock).toHaveBeenCalledWith(sourcePath, 'r') - expect(nativeImageCreateFromBufferMock).toHaveBeenCalledWith(source) - expect(close).toHaveBeenCalled() - expect(writeFileBase64).toHaveBeenCalledWith( - '/var/tmp/orca-paste-1760000000000-00000000-0000-4000-8000-000000000000.png', - png.toString('base64') - ) - expect(fsWriteFileMock).not.toHaveBeenCalled() - } finally { - platformSpy.mockRestore() + const probe = getRegisteredHandlers().get('clipboard:hasImage') + expect(probe?.(makeClipboardEvent())).toBe(true) + expect(() => probe?.(makeClipboardEvent({ id: 42 }))).toThrow() + const handler = getRegisteredHandlers().get('clipboard:saveImageAsTempFile') + await expect(handler?.(makeClipboardEvent(), { connectionId: 'ssh-1' })).resolves.toBe( + '/var/tmp/orca-paste-1760000000000-00000000-0000-4000-8000-000000000000.png' + ) + expect(clipboardReadBufferMock).toHaveBeenCalledWith('FileNameW') + expect(clipboardReadBufferMock).toHaveBeenCalledWith('Shell IDList Array') + expect(fsOpenMock).toHaveBeenCalledWith(sourcePath, 'r') + expect(nativeImageCreateFromBufferMock).toHaveBeenCalledWith(source) + expect(close).toHaveBeenCalled() + expect(writeFileBase64).toHaveBeenCalledWith( + '/var/tmp/orca-paste-1760000000000-00000000-0000-4000-8000-000000000000.png', + png.toString('base64') + ) + expect(fsWriteFileMock).not.toHaveBeenCalled() + } finally { + platformSpy.mockRestore() + } } - }) + ) it('saves clipboard images through the selected remote runtime host', async () => { const png = Buffer.alloc(512 * 1024) @@ -874,18 +885,4 @@ describe('registerClipboardHandlers', () => { expect(nativeImageCreateFromBufferMock).toHaveBeenCalled() expect(clipboardWriteImageMock).not.toHaveBeenCalled() }) - - it.each([ - [['text/plain'], false], - [['text/plain', 'image/png'], true] - ])('reports image presence for %j from the format list without decoding', (formats, expected) => { - setTrustedClipboardRendererWebContentsId(17) - clipboardAvailableFormatsMock.mockReturnValue(formats) - // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: registering handlers never reads the store for clipboard image presence. - registerClipboardHandlers({} as never) - const probe = getRegisteredHandlers().get('clipboard:hasImage') - expect(probe?.(makeClipboardEvent())).toBe(expected) - expect(clipboardReadImageMock).not.toHaveBeenCalled() - expect(() => probe?.(makeClipboardEvent({ id: 42 }))).toThrow() - }) }) diff --git a/src/main/window/clipboard-ipc-handlers.ts b/src/main/window/clipboard-ipc-handlers.ts index 05a09a54bb8..80dad3bd02b 100644 --- a/src/main/window/clipboard-ipc-handlers.ts +++ b/src/main/window/clipboard-ipc-handlers.ts @@ -25,7 +25,6 @@ import { assertClipboardImageBase64LengthWithinLimit, assertClipboardImageByteLengthWithinLimit, assertClipboardImageDimensionsWithinLimit, - clipboardFormatsIncludeImage, type ClipboardImageThumbnail } from '../../shared/clipboard-image' import { @@ -41,6 +40,7 @@ import { import { saveClipboardImageBufferInRuntime } from './clipboard-runtime-image-upload' import { uploadPastedImageToAgentSessionAttachments } from '../ipc/agent-session-attachment-upload' import { readWindowsClipboardImageFileAsPng } from './clipboard-windows-image-file' +import { readClipboardImageSource } from './clipboard-image-source' import { readClipboardCopiedFilePaths } from './clipboard-copied-file-paths' import { buildClipboardImageThumbnail } from './clipboard-image-thumbnail' import { writeClipboardTextAndVerify } from './clipboard-text-write-verify' @@ -141,7 +141,7 @@ export function registerClipboardHandlers(store: Store): void { }) ipcMain.handle('clipboard:hasImage', (event): boolean => { assertTrustedClipboardSender(event) - return clipboardFormatsIncludeImage(clipboard.availableFormats()) + return readClipboardImageSource(clipboard) !== null }) // Why: a file-manager copy also carries the files' names as text, which a paste must not type. ipcMain.handle('clipboard:readFilePaths', (event): string[] => { @@ -155,21 +155,19 @@ export function registerClipboardHandlers(store: Store): void { 'clipboard:saveImageAsTempFile', async (event, args?: SaveClipboardImageAsTempFileArgs) => { assertTrustedClipboardSender(event) + const source = readClipboardImageSource(clipboard) + if (!source) { + return null + } const image = clipboard.readImage() if (image.isEmpty()) { - if (process.platform !== 'win32') { + if (!source.windowsFileFormats) { return null } - const copiedFilePng = await readWindowsClipboardImageFileAsPng( - { - fileNameW: clipboard.readBuffer('FileNameW'), - shellIdListArray: clipboard.readBuffer('Shell IDList Array') - }, - { - createImageFromBuffer: (buffer) => nativeImage.createFromBuffer(buffer), - openFile: (filePath) => open(filePath, 'r') - } - ) + const copiedFilePng = await readWindowsClipboardImageFileAsPng(source.windowsFileFormats, { + createImageFromBuffer: (buffer) => nativeImage.createFromBuffer(buffer), + openFile: (filePath) => open(filePath, 'r') + }) return copiedFilePng ? saveClipboardImageBufferForTarget(copiedFilePng, args) : null } assertClipboardImageDimensionsWithinLimit(image.getSize()) diff --git a/src/main/window/clipboard-runtime-owned-ssh-paste.test.ts b/src/main/window/clipboard-runtime-owned-ssh-paste.test.ts index 1bc2915508f..e2844455c38 100644 --- a/src/main/window/clipboard-runtime-owned-ssh-paste.test.ts +++ b/src/main/window/clipboard-runtime-owned-ssh-paste.test.ts @@ -15,13 +15,14 @@ const PNG = Buffer.from([0, 1, 2, 3]) vi.mock('electron', () => ({ app: { getPath: vi.fn(() => '/tmp') }, clipboard: { + availableFormats: () => ['image/png'], readImage: () => ({ getSize: () => ({ height: 1, width: 1 }), isEmpty: () => false, toPNG: () => PNG }), readText: vi.fn(), - readBuffer: vi.fn(), + readBuffer: vi.fn(() => Buffer.alloc(0)), writeText: vi.fn(), writeImage: vi.fn(), writeBuffer: vi.fn() diff --git a/src/main/window/clipboard-windows-image-file.ts b/src/main/window/clipboard-windows-image-file.ts index 39099332958..17baa100846 100644 --- a/src/main/window/clipboard-windows-image-file.ts +++ b/src/main/window/clipboard-windows-image-file.ts @@ -84,6 +84,15 @@ export function readWindowsCopiedFilePath({ return hasAtMostOneShellItem(shellIdListArray) ? decodeFileNameW(fileNameW) : null } +export function readWindowsCopiedImageFilePath( + formats: WindowsClipboardFileFormats +): string | null { + const filePath = readWindowsCopiedFilePath(formats) + return filePath && IMAGE_FILE_EXTENSION_SET.has(win32.extname(filePath).toLowerCase()) + ? filePath + : null +} + function readPngDimensions(source: Buffer): { height: number; width: number } | null { if ( source.byteLength < 24 || @@ -164,8 +173,8 @@ export async function readWindowsClipboardImageFileAsPng( formats: WindowsClipboardFileFormats, { createImageFromBuffer, openFile }: WindowsClipboardImageFileDeps ): Promise { - const filePath = readWindowsCopiedFilePath(formats) - if (!filePath || !IMAGE_FILE_EXTENSION_SET.has(win32.extname(filePath).toLowerCase())) { + const filePath = readWindowsCopiedImageFilePath(formats) + if (!filePath) { return null } diff --git a/src/renderer/src/components/native-chat/NativeChatComposer.paste-remount.test.tsx b/src/renderer/src/components/native-chat/NativeChatComposer.paste-remount.test.tsx index f5509b5b10d..327397db8b0 100644 --- a/src/renderer/src/components/native-chat/NativeChatComposer.paste-remount.test.tsx +++ b/src/renderer/src/components/native-chat/NativeChatComposer.paste-remount.test.tsx @@ -4,23 +4,41 @@ // replaces the composer. The real composer, attachment and paste hooks keep the upload, and Send // waits for it, whenever the composer comes back; a rich-text paste's image is owed from the start. -import { act, cleanup, render } from '@testing-library/react' +import { act, cleanup, fireEvent, render, screen, waitFor } from '@testing-library/react' import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' -import type { ClipboardEventLike } from './native-chat-clipboard-payload' +import { createRef, type RefObject } from 'react' +import type { NativeChatComposerHandle } from './native-chat-composer-types' +import type { NativeChatAttachmentOwner } from './native-chat-attachment-upload' +import type { + NativeChatComposerField as NativeChatComposerFieldComponent, + NativeChatComposerFieldProps, + NativeChatComposerImageAttachment +} from './NativeChatComposerField' type FieldProps = { - onPaste?: (event: ClipboardEventLike) => void - imageAttachments?: { path: string; pending?: boolean }[] + imageAttachments?: readonly NativeChatComposerImageAttachment[] sendButtonDisabled?: boolean + onRemoveImageAttachment: (id: string) => void } const mocks = vi.hoisted(() => { - const state: { fieldProps: FieldProps | null } = { fieldProps: null } + const state: { fieldProps: FieldProps | null; owner: NativeChatAttachmentOwner } = { + fieldProps: null, + owner: { + kind: 'runtime-session', + environmentId: 'env-1', + pairingRevision: 1, + sessionId: 'session-1' + } + } return { state, prepare: vi.fn(), save: vi.fn() } }) vi.mock('../../store', () => { const state = { + tabsByWorktree: {}, + worktrees: [], + repos: [], dictationState: 'idle', settings: { voice: { enabled: false }, nativeChatSessionOptions: {} }, agentStatusByPaneKey: {}, @@ -43,24 +61,25 @@ vi.mock('@/lib/native-chat-telemetry', () => ({ emitNativeChatPickerOpened: vi.fn(), emitNativeChatSendClassified: vi.fn() })) -vi.mock('./NativeChatComposerField', () => ({ - NativeChatComposerField: (props: FieldProps) => { - mocks.state.fieldProps = props - return
+vi.mock('./NativeChatComposerField', async (importOriginal) => { + const original = await importOriginal<{ + NativeChatComposerField: typeof NativeChatComposerFieldComponent + }>() + return { + NativeChatComposerField: (props: NativeChatComposerFieldProps) => { + mocks.state.fieldProps = props + return + } } -})) +}) +vi.mock('./NativeChatComposerActions', () => ({ NativeChatComposerActions: () => null })) vi.mock('./use-native-chat-skills', () => ({ useNativeChatSkills: () => ({ status: 'ready', skills: [], error: null, retry: () => {} }) })) vi.mock('./use-native-chat-external-attachments', () => ({ useNativeChatExternalAttachments: () => ({ attachExternalPaths: vi.fn(), - resolveAttachmentOwner: () => ({ - kind: 'runtime-session', - environmentId: 'env-1', - pairingRevision: 1, - sessionId: 'session-1' - }) + resolveAttachmentOwner: () => mocks.state.owner }) })) vi.mock('./native-chat-attachment-upload', async (importOriginal) => ({ @@ -71,6 +90,7 @@ vi.mock('./native-chat-attachment-upload', async (importOriginal) => ({ import { NativeChatComposer } from './NativeChatComposer' import { clearNativeChatDraftCacheForTests, + readNativeChatDraftCache, writeNativeChatDraftCache } from './native-chat-draft-cache' import { @@ -81,9 +101,13 @@ import { const PANE = 'tab-1::session-1' const STORED = '/srv/agent-session-attachments/0b6f8a52-4a3e-4c4e-9a59-1d5d1f2b8c01/orca-paste.png' -function composer(): React.JSX.Element { +function composer( + acceptsImages = true, + composerRef?: RefObject +): React.JSX.Element { return ( ): void { }) } -function imagePaste(text?: string): ClipboardEventLike { +function imagePaste(text?: string): ClipboardEvent { const data = new DataTransfer() data.items.add(new File(['image'], 'image.png', { type: 'image/png' })) if (text) { @@ -147,6 +172,12 @@ beforeEach(() => { clearNativeChatAttachmentCacheForTests() clearNativeChatDraftCacheForTests() mocks.state.fieldProps = null + mocks.state.owner = { + kind: 'runtime-session', + environmentId: 'env-1', + pairingRevision: 1, + sessionId: 'session-1' + } mocks.prepare.mockResolvedValue({ ok: true, target: { @@ -163,7 +194,9 @@ beforeEach(() => { }) afterEach(() => { + vi.useRealTimers() cleanup() + vi.restoreAllMocks() vi.unstubAllGlobals() }) @@ -177,7 +210,7 @@ describe('a paste into a chat on a paired server', () => { ) installPreloadApi({ saveClipboardImageAsTempFile: mocks.save }) const view = render(composer()) - await act(async () => mocks.state.fieldProps?.onPaste?.(imagePaste())) + await act(async () => fireEvent(screen.getByRole('textbox'), imagePaste())) expect(mocks.state.fieldProps?.imageAttachments).toMatchObject([{ pending: true }]) // The agent asks a question: its card takes the composer's place while the image uploads. @@ -196,7 +229,7 @@ describe('a paste into a chat on a paired server', () => { // Something typed, so only the pending image can hold Send. writeNativeChatDraftCache(PANE, 'look at this') const first = render(composer()) - await act(async () => mocks.state.fieldProps?.onPaste?.(imagePaste())) + await act(async () => fireEvent(screen.getByRole('textbox'), imagePaste())) first.unmount() // Back from the prompt card while the image is still on its way to the server. @@ -217,7 +250,7 @@ describe('a paste into a chat on a paired server', () => { writeNativeChatDraftCache(PANE, 'look at this caption') render(composer()) expect(mocks.state.fieldProps?.sendButtonDisabled).toBe(false) - await act(async () => mocks.state.fieldProps?.onPaste?.(imagePaste('caption'))) + await act(async () => fireEvent(screen.getByRole('textbox'), imagePaste('caption'))) // The text is in; the image is not yet anywhere, but it is owed to this message. expect(mocks.state.fieldProps?.sendButtonDisabled).toBe(true) @@ -229,7 +262,7 @@ describe('a paste into a chat on a paired server', () => { const finishUpload = holdUpload() writeNativeChatDraftCache(PANE, 'look at this caption') const first = render(composer()) - await act(async () => mocks.state.fieldProps?.onPaste?.(imagePaste('caption'))) + await act(async () => fireEvent(screen.getByRole('textbox'), imagePaste('caption'))) // A prompt card replaces the composer while the server is still being asked. first.unmount() render(composer()) @@ -254,3 +287,155 @@ describe('a paste into a chat on a paired server', () => { expect(mocks.state.fieldProps?.imageAttachments).toMatchObject([{ path: STORED }]) }) }) + +it('keeps a local image paste after the real composer closes and reopens', async () => { + mocks.state.owner = { kind: 'local' } + mocks.save.mockClear() + const finishUpload = holdUpload() + const first = render(composer()) + await act(async () => fireEvent(screen.getByRole('textbox'), imagePaste())) + await waitFor(() => expect(mocks.save).toHaveBeenCalled()) + expect(mocks.state.fieldProps?.imageAttachments).toMatchObject([{ pending: true }]) + first.unmount() + await act(async () => finishUpload('/local/native-chat-pastes/orca-paste-1-image.png')) + render(composer()) + expect(mocks.state.fieldProps?.imageAttachments).toMatchObject([ + { path: '/local/native-chat-pastes/orca-paste-1-image.png' } + ]) + await waitFor(() => expect(mocks.state.fieldProps?.imageAttachments?.[0]?.pending).toBeFalsy()) +}) + +it.each([ + { kind: 'local', acceptsImages: false }, + { kind: 'ssh', acceptsImages: false }, + { kind: 'local', acceptsImages: true }, + { kind: 'ssh', acceptsImages: true } +] as const)( + 'can remove a stalled $kind paste after reopen with acceptsImages=$acceptsImages', + async ({ kind, acceptsImages }) => { + mocks.state.owner = + kind === 'local' + ? { kind: 'local' } + : { + kind: 'ssh', + connectionId: 'ssh-1', + worktreePath: '/remote/folder', + expectedExecutionHostId: 'ssh:ssh-1', + expectedSshTargetId: 'ssh-1', + expectedSshConnectionGeneration: 1 + } + vi.useFakeTimers({ toFake: ['setTimeout', 'clearTimeout', 'Date'] }) + const finishUpload = holdUpload() + writeNativeChatDraftCache(PANE, 'look at this') + const first = render(composer(acceptsImages)) + expect(mocks.state.fieldProps?.sendButtonDisabled).toBe(false) + await act(async () => fireEvent(screen.getByRole('textbox'), imagePaste())) + first.unmount() + render(composer(acceptsImages)) + await act(async () => vi.advanceTimersByTimeAsync(24 * 60 * 60 * 1000)) + expect(mocks.state.fieldProps?.sendButtonDisabled).toBe(true) + expect(screen.getByRole('button', { name: 'Saving pasted image…' })).toBeTruthy() + await act(async () => + fireEvent.click(screen.getByRole('button', { name: 'Remove attachment' })) + ) + expect(mocks.state.fieldProps?.sendButtonDisabled).toBe(false) + await act(async () => + finishUpload( + kind === 'local' + ? '/local/native-chat-pastes/orca-paste-1-image.png' + : '/tmp/orca-paste-1-image.png' + ) + ) + expect(readNativeChatAttachmentCache(PANE)).toEqual([]) + expect(readNativeChatDraftCache(PANE)).toBe('look at this') + expect(mocks.state.fieldProps?.imageAttachments).toEqual([]) + } +) + +it.each([true, false])( + 'pastes ordinary menu text without image feedback or holding Send, acceptsImages=%s', + async (acceptsImages) => { + mocks.state.owner = { kind: 'local' } + mocks.save.mockClear() + mocks.save.mockReturnValue(new Promise(() => {})) + installPreloadApi({ + saveClipboardImageAsTempFile: mocks.save, + readClipboardText: async () => 'ordinary text', + readClipboardFilePaths: async () => [], + readClipboardImageThumbnail: async () => null, + clipboardHasImage: async () => false + }) + const handle = createRef() + writeNativeChatDraftCache(PANE, 'look at this') + render(composer(acceptsImages, handle)) + await act(async () => handle.current?.pasteFromClipboard()) + expect(screen.getByRole('textbox').textContent).toContain('ordinary text') + expect(screen.queryByRole('button', { name: 'Saving pasted image…' })).toBeNull() + expect(mocks.state.fieldProps?.sendButtonDisabled).toBe(false) + expect(mocks.save).not.toHaveBeenCalled() + } +) + +it.each([true, false])( + 'releases all previews when a paired paste is removed before acceptance, acceptsImages=%s', + async (acceptsImages) => { + const prepare = Promise.withResolvers<{ + ok: true + target: { + environmentId: string + sessionId: string + expectedEnvironmentPairingRevision: number + } + }>() + mocks.prepare.mockReturnValue(prepare.promise) + const finishUpload = holdUpload() + const allocated = new Set() + const released = new Set() + vi.spyOn(URL, 'createObjectURL').mockImplementation(() => { + const url = 'blob:late-acceptance' + allocated.add(url) + return url + }) + vi.spyOn(URL, 'revokeObjectURL').mockImplementation((url) => { + released.add(url) + }) + const view = render(composer(acceptsImages)) + await act(async () => fireEvent(screen.getByRole('textbox'), imagePaste('caption'))) + await act(async () => + fireEvent.click(screen.getByRole('button', { name: 'Remove attachment' })) + ) + await act(async () => + prepare.resolve({ + ok: true, + target: { + environmentId: 'env-1', + sessionId: 'session-1', + expectedEnvironmentPairingRevision: 1 + } + }) + ) + await act(async () => finishUpload(STORED)) + expect(readNativeChatAttachmentCache(PANE)).toEqual([]) + view.unmount() + expect([...allocated].filter((url) => !released.has(url))).toEqual([]) + } +) + +it('starts no upload when the composer closes before image presence is known', async () => { + mocks.state.owner = { kind: 'local' } + mocks.save.mockClear() + const presence = Promise.withResolvers() + installPreloadApi({ + saveClipboardImageAsTempFile: mocks.save, + readClipboardText: async () => '', + readClipboardFilePaths: async () => [], + clipboardHasImage: () => presence.promise + }) + const handle = createRef() + const view = render(composer(true, handle)) + await act(async () => handle.current?.pasteFromClipboard()) + view.unmount() + await act(async () => presence.resolve(true)) + expect(mocks.save).not.toHaveBeenCalled() + expect(readNativeChatAttachmentCache(PANE)).toEqual([]) +}) diff --git a/src/renderer/src/components/native-chat/NativeChatComposer.tsx b/src/renderer/src/components/native-chat/NativeChatComposer.tsx index f9519ed859f..62315c377ec 100644 --- a/src/renderer/src/components/native-chat/NativeChatComposer.tsx +++ b/src/renderer/src/components/native-chat/NativeChatComposer.tsx @@ -197,6 +197,7 @@ const NativeChatComposerPane = forwardRef !attachment.hidden) const draftNotSaved = useNativeChatComposerDraftUnsaved(draftScopeKey) const optionCount = autocomplete.mode === 'slash' @@ -282,9 +279,9 @@ export function NativeChatComposerField({ '[contain:paint]' )} > - {shownAttachments.length > 0 ? ( + {imageAttachments.length > 0 ? (
- {shownAttachments.map((attachment) => ( + {imageAttachments.map((attachment) => ( { expect(next.drafts.readNativeChatDraftCache('agent-session:s2')).toBe('please go\n\ngo') expect(storage.drafts.get('agent-session:s2')?.text).toBe('please go\n\ngo') }) + + it.each(['delayed load', 'crash replay'])( + 'keeps picked nodes when appending through %s', + async (mode) => { + const document = { + type: 'doc', + content: [ + { + type: 'paragraph', + content: [ + { type: 'nativeChatSkill', attrs: { token: '$review' } }, + { type: 'text', text: '-long' } + ] + } + ] + } + storage.drafts.set(SCOPE, { text: '$review-long', document, images: [], savedAt: 1 }) + let next: DraftModules + if (mode === 'crash replay') { + const crashed = await reload({ + using: { ...storage, write: () => new Promise(() => {}) } + }) + crashed.drafts.appendNativeChatDraftCache(SCOPE, '@/stored/file.pdf') + crashed.store.clearNativeChatComposerDraftsForTests() + next = await reload() + } else { + const { using, land } = slowLoading() + next = await reload({ using, hydrate: false }) + await next.store.waitForNativeChatComposerDrafts(1) + next.drafts.appendNativeChatDraftCache(SCOPE, '@/stored/file.pdf') + land() + await next.store.waitForNativeChatComposerDrafts(1_000) + } + await next.store.nativeChatComposerDraftWritesSettled() + expect(next.drafts.readNativeChatDraftCache(SCOPE)).toBe('$review-long\n\n@/stored/file.pdf') + expect(next.store.readNativeChatComposerDraft(SCOPE).document?.content?.[0]).toEqual( + document.content[0] + ) + expect(storage.drafts.get(SCOPE)?.document?.content?.[0]).toEqual(document.content[0]) + } + ) }) diff --git a/src/renderer/src/components/native-chat/native-chat-composer-draft-addition.ts b/src/renderer/src/components/native-chat/native-chat-composer-draft-addition.ts index 76b82af5393..441ce2264cc 100644 --- a/src/renderer/src/components/native-chat/native-chat-composer-draft-addition.ts +++ b/src/renderer/src/components/native-chat/native-chat-composer-draft-addition.ts @@ -3,6 +3,7 @@ // loaded later, or replayed from the journal after a crash. import { appendReturnedDraftText } from '../../../../shared/returned-draft-text' +import { appendPromptDocumentText } from './native-chat-prompt-document-text' import { basename } from '@/lib/path' import type { NativeChatComposerDraft, @@ -25,7 +26,7 @@ export function withNativeChatComposerDraftAddition( draft: NativeChatComposerDraft, addition: NativeChatComposerDraftAddition, options: { once?: boolean } = {} -): Pick { +): Pick { const text = appendReturnedDraftText(draft.text, addition.text ?? '') const images = [...draft.images] for (const { id, path, connectionId } of addition.images ?? []) { @@ -42,5 +43,5 @@ export function withNativeChatComposerDraftAddition( images[placeholder] = image } } - return { text, images } + return { text, images, document: appendPromptDocumentText(draft.document, draft.text, text) } } diff --git a/src/renderer/src/components/native-chat/native-chat-composer-draft-journal.ts b/src/renderer/src/components/native-chat/native-chat-composer-draft-journal.ts index c1b979e3645..dc4a210d6cc 100644 --- a/src/renderer/src/components/native-chat/native-chat-composer-draft-journal.ts +++ b/src/renderer/src/components/native-chat/native-chat-composer-draft-journal.ts @@ -145,7 +145,6 @@ export function replayNativeChatComposerDraftJournal( drafts.set(scopeKey, { ...from, ...next, - ...(next.text === from.text ? {} : { document: undefined }), savedAt: at }) changed.add(scopeKey) diff --git a/src/renderer/src/components/native-chat/native-chat-composer-draft-store.ts b/src/renderer/src/components/native-chat/native-chat-composer-draft-store.ts index 60dddf06d2f..2842251fd6a 100644 --- a/src/renderer/src/components/native-chat/native-chat-composer-draft-store.ts +++ b/src/renderer/src/components/native-chat/native-chat-composer-draft-store.ts @@ -189,7 +189,7 @@ function withAddition( options: { once?: boolean } = {} ): T { const next = withNativeChatComposerDraftAddition(draft, addition, options) - return { ...draft, ...next, ...(next.text === draft.text ? {} : { document: undefined }) } + return { ...draft, ...next } } /** @@ -206,13 +206,13 @@ export function appendToNativeChatComposerDraft( ): boolean { const before = records.get(scopeKey) const current = readNativeChatComposerDraft(scopeKey) - const { text, images } = withAddition(current, addition) + const { text, images, document } = withAddition(current, addition) updateNativeChatComposerDraft( scopeKey, { text, images, - ...(text === current.text ? {} : { document: undefined }), + document, ...(owner ? { owner } : {}) }, 'immediate', diff --git a/src/renderer/src/components/native-chat/native-chat-composer-paste-options.ts b/src/renderer/src/components/native-chat/native-chat-composer-paste-options.ts new file mode 100644 index 00000000000..1d4a17d41f9 --- /dev/null +++ b/src/renderer/src/components/native-chat/native-chat-composer-paste-options.ts @@ -0,0 +1,23 @@ +import type { AgentType } from '../../../../shared/agent-status-types' +import type { NativeChatAttachmentOwner } from './native-chat-attachment-upload' + +export type UseNativeChatComposerPasteArgs = { + targetKey?: string + attachmentScopeKey?: string + agent: AgentType + /** Re-read after awaits so a mid-paste disable guards the composer. */ + disabled: boolean + caret: number + /** Resolved at paste time: SSH panes must save the clipboard image on the + * remote host, or the attached path names a file the agent cannot read. */ + resolveAttachmentOwner: () => NativeChatAttachmentOwner + attachResolvedPaths: (paths: string[], connectionId?: string | null) => void + beginPendingImageAttachment: (previewUrl?: string, pendingName?: string) => string | null + resolvePendingImageAttachment: (id: string, path: string, connectionId?: string | null) => void + /** Supplies a pending chip's thumbnail. */ + revealPendingImageAttachment?: (id: string, previewUrl?: string) => void + dropPendingImageAttachment: (id: string) => void + insertTypedText: (text: string) => boolean + setCaret: (caret: number) => void + setNotice: (notice: string | null, errorText?: string) => void +} diff --git a/src/renderer/src/components/native-chat/native-chat-composer-upload-completion.test.tsx b/src/renderer/src/components/native-chat/native-chat-composer-upload-completion.test.tsx new file mode 100644 index 00000000000..97d06b69b52 --- /dev/null +++ b/src/renderer/src/components/native-chat/native-chat-composer-upload-completion.test.tsx @@ -0,0 +1,230 @@ +// @vitest-environment happy-dom +import { createRef } from 'react' +import { act, cleanup, render, renderHook } from '@testing-library/react' +import { afterEach, beforeEach, expect, it, vi } from 'vitest' +import { useNativeChatComposerAttachments } from './use-native-chat-composer-attachments' +import { attachNativeChatSessionAttachmentPaths } from './native-chat-session-attachment-drop' +import { NativeChatPromptEditor } from './NativeChatPromptEditor' +import type { NativeChatComposerInput } from './native-chat-composer-input' +import { + clearNativeChatComposerDraftsForTests, + hydrateNativeChatComposerDrafts, + nativeChatComposerDraftWritesSettled, + readNativeChatComposerDraft, + structuredAgentSessionDraftScopeKey as scope, + updateNativeChatComposerDraft +} from './native-chat-composer-draft-store' +import { + createMemoryNativeChatComposerDraftStorage, + setNativeChatComposerDraftStorageForTests +} from './native-chat-composer-draft-storage' +import { + clearNativeChatPendingAttachmentsForTests, + nativeChatPendingAttachmentSnapshot, + takeNativeChatPendingAttachment +} from './native-chat-pending-attachment-cache' + +// Only host replies are controlled; pending-chip ownership and upload completion are production code. +vi.mock('@/runtime/runtime-rpc-client', () => ({ + callRuntimeRpc: vi.fn(async () => ({ + runtimeId: 'paired-host', + capabilities: ['agent-session.attachments.v1'] + })) +})) +let previousApi: typeof window.api +beforeEach(async () => { + previousApi = window.api + setNativeChatComposerDraftStorageForTests(createMemoryNativeChatComposerDraftStorage()) + await hydrateNativeChatComposerDrafts() +}) +afterEach(async () => { + cleanup() + await nativeChatComposerDraftWritesSettled() + clearNativeChatPendingAttachmentsForTests() + clearNativeChatComposerDraftsForTests() + window.api = previousApi +}) +async function uploadFixture( + extension: string, + { removed = false, withSkill = false, switchScope = false } = {} +) { + const path = `/srv/agent-session-attachments/u1/report.${extension}` + const sourcePath = `/local/report.${extension}` + let finish = () => {} + Object.defineProperty(window, 'api', { + configurable: true, + writable: true, + value: { + fs: { + uploadPathsToAgentSessionAttachments: () => + new Promise((resolve) => { + finish = () => resolve({ uploaded: [{ sourcePath, path }], skipped: [], failed: [] }) + }) + } + } + }) + updateNativeChatComposerDraft(scope('a'), { text: 'next question' }, 'immediate') + if (withSkill) { + const inputRef = createRef() + const picker = render( + + updateNativeChatComposerDraft(scope('a'), { text: input.value }, 'immediate') + } + onSelect={() => {}} + /> + ) + act(() => { + inputRef.current?.insertSkill?.(0, 4, '$review') + if (inputRef.current) { + inputRef.current.value = '$review-long' + } + }) + await vi.waitFor(() => + expect(picker.container.querySelector('[data-native-chat-skill]')).not.toBeNull() + ) + picker.unmount() + } + const textareaRef = createRef() + const view = renderHook( + ({ draftScope }) => { + return useNativeChatComposerAttachments({ + attachmentScopeKey: draftScope, + allowWithoutTarget: true, + caret: 0, + disabled: false, + isComposing: () => false, + resolveTarget: () => null, + textareaRef, + setCaret: () => {}, + setDraft: (updater) => + updateNativeChatComposerDraft( + draftScope, + { text: updater(readNativeChatComposerDraft(draftScope).text) }, + 'immediate' + ), + setNotice: () => {} + }) + }, + { initialProps: { draftScope: scope('a') } } + ) + let uploaded = Promise.resolve() + act(() => { + uploaded = attachNativeChatSessionAttachmentPaths({ + paths: [sourcePath], + owner: { + kind: 'runtime-session', + environmentId: 'paired', + sessionId: 'a', + pairingRevision: 1 + }, + chips: view.result.current.pendingChips, + isAbandoned: () => false, + ownerStillCurrent: () => true, + setNotice: () => {} + }) + }) + await Promise.resolve() + await Promise.resolve() + const chip = nativeChatPendingAttachmentSnapshot(scope('a'))[0] + if (!chip) { + throw new Error('Expected pending upload chip') + } + expect(chip.pendingName).toBe(`report.${extension}`) + const targetScope = scope('a') + if (switchScope) { + act(() => view.rerender({ draftScope: scope('b') })) + render( + + updateNativeChatComposerDraft(scope('b'), { text: input.value }, 'immediate') + } + onSelect={() => {}} + /> + ) + } + if (removed) { + act(() => takeNativeChatPendingAttachment(targetScope, chip.id)) + } + if (!switchScope) { + view.unmount() + } + finish() + await uploaded + return { + path, + targetScope, + other: readNativeChatComposerDraft(scope('b')), + otherInput: textareaRef.current?.value, + target: readNativeChatComposerDraft(targetScope), + pending: nativeChatPendingAttachmentSnapshot(targetScope) + } +} +it.each(['pdf', 'png'])( + 'settles an unmounted paired %s upload in its owning draft', + async (extension) => { + const result = await uploadFixture(extension) + expect(result.pending).toEqual([]) + if (extension === 'pdf') { + expect(result.target.text).toContain(`@${result.path}`) + } else { + expect(result.target.images[0]?.path).toBe(result.path) + } + expect(result.other.text).toBe('') + expect(result.other.images).toEqual([]) + } +) + +it('does not attach a file reference removed while uploading', async () => { + const result = await uploadFixture('pdf', { removed: true }) + expect(result.pending).toEqual([]) + expect(result.target.text).toBe('next question') +}) + +it('keeps the original file owner when the mounted composer switches draft scopes', async () => { + const result = await uploadFixture('pdf', { switchScope: true }) + expect(result.pending).toEqual([]) + expect(result.target.text).toBe(`next question\n\n@${result.path}`) + expect(result.otherInput).toBe('other question') + expect(result.other.images).toEqual([]) +}) + +it.each(['pdf', 'png'])( + 'preserves a picked skill and its literal suffix after an unmounted %s upload', + async (extension) => { + const result = await uploadFixture(extension, { withSkill: true }) + expect(result.pending).toEqual([]) + const inputRef = createRef() + const view = render( + {}} + onSelect={() => {}} + /> + ) + expect(inputRef.current?.value).toContain('$review-long') + if (extension === 'pdf') { + expect(inputRef.current?.value).toContain(`@${result.path}`) + } else { + expect(result.target.images[0]?.path).toBe(result.path) + } + await vi.waitFor(() => { + expect(view.container.querySelectorAll('[data-native-chat-skill]')).toHaveLength(1) + expect(view.container.querySelector('[data-native-chat-skill]')?.textContent).toBe('Review') + }) + } +) diff --git a/src/renderer/src/components/native-chat/native-chat-draft-cache.ts b/src/renderer/src/components/native-chat/native-chat-draft-cache.ts index 8d086d5ac35..f4f7eee53bd 100644 --- a/src/renderer/src/components/native-chat/native-chat-draft-cache.ts +++ b/src/renderer/src/components/native-chat/native-chat-draft-cache.ts @@ -1,5 +1,6 @@ import type { NativeChatComposerImageAttachment } from './NativeChatComposerField' import type { JSONContent } from '@tiptap/react' +import type { NativeChatComposerDraftOwner } from './native-chat-composer-draft-storage' // The composer's in-progress draft text and its editor document, keyed by the same stable pane // scope as image attachments. The composer unmounts when the pane toggles back to the hosted // terminal, so without this the typed-but-unsent draft would be lost on every TUI/GUI round-trip. @@ -40,13 +41,17 @@ const appendListeners = new Map /** Puts text back after whatever is typed, and tells a mounted composer to show it. True once it * is durable, so the copy it came from may go. */ -export function appendNativeChatDraftCache(scopeKey: string, text: string): boolean { +export function appendNativeChatDraftCache( + scopeKey: string, + text: string, + owner?: NativeChatComposerDraftOwner +): boolean { if (text === '') { return true } const previous = readNativeChatDraftCache(scopeKey) // Durable now: the copy it came from (a send handed back, a queued card) goes right after this. - const durable = appendToNativeChatComposerDraft(scopeKey, { text }) + const durable = appendToNativeChatComposerDraft(scopeKey, { text }, owner) appendListeners.get(scopeKey)?.forEach((listener) => listener(text, previous)) return durable } diff --git a/src/renderer/src/components/native-chat/native-chat-pending-attachment-cache.test.ts b/src/renderer/src/components/native-chat/native-chat-pending-attachment-cache.test.ts index 3dccd946f89..96a187899a1 100644 --- a/src/renderer/src/components/native-chat/native-chat-pending-attachment-cache.test.ts +++ b/src/renderer/src/components/native-chat/native-chat-pending-attachment-cache.test.ts @@ -1,13 +1,12 @@ -import { afterEach, describe, expect, it, vi } from 'vitest' +import { afterEach, describe, expect, it } from 'vitest' import { addNativeChatPendingAttachment, clearNativeChatPendingAttachmentsForTests, dropNativeChatPendingAttachmentsForTab, dropNativeChatPendingAttachmentsOwnedBy, nativeChatPendingAttachmentSnapshot, - revealNativeChatPendingAttachment, settleNativeChatPendingAttachment, - subscribeToNativeChatPendingAttachments, + settleNativeChatPendingAttachmentReferences, takeNativeChatPendingAttachment } from './native-chat-pending-attachment-cache' import { @@ -58,22 +57,6 @@ describe('the pane pending attachment cache', () => { expect(takeNativeChatPendingAttachment('pane-b', 'b1')).toBeUndefined() }) - it('shows a hidden chip and tells every subscriber, with a new snapshot only on change', () => { - const listener = vi.fn() - const unsubscribe = subscribeToNativeChatPendingAttachments('pane-c', listener) - addNativeChatPendingAttachment('pane-c', { id: 'c1', path: '', pending: true, hidden: true }) - const hidden = nativeChatPendingAttachmentSnapshot('pane-c') - - expect(nativeChatPendingAttachmentSnapshot('pane-c')).toBe(hidden) - revealNativeChatPendingAttachment('pane-c', 'c1') - - expect(nativeChatPendingAttachmentSnapshot('pane-c')).toEqual([ - { id: 'c1', path: '', pending: true } - ]) - expect(listener).toHaveBeenCalledTimes(2) - unsubscribe() - }) - it('settles into a draft owned by the workspace it began in, after its chat stopped naming one', () => { const conversation = structuredAgentSessionDraftScopeKey('session-1') let open = true @@ -89,6 +72,25 @@ describe('the pane pending attachment cache', () => { expect(readNativeChatComposerDraft(conversation).images).toEqual([]) }) + it('keeps reference settlement owned by the workspace after its chat closes', () => { + const conversation = structuredAgentSessionDraftScopeKey('session-1') + let open = true + setNativeChatComposerDraftOwnerResolver((scopeKey) => + open && scopeKey === conversation ? OWNER : undefined + ) + addPending(conversation, 'file') + open = false + + settleNativeChatPendingAttachmentReferences(conversation, [ + { id: 'file', path: '/store/a.pdf' } + ]) + + expect(readNativeChatComposerDraft(conversation).text).toBe('@/store/a.pdf') + expect(nativeChatPendingAttachmentSnapshot(conversation)).toEqual([]) + deleteNativeChatComposerDraftsOwnedBy(OWNER) + expect(readNativeChatComposerDraft(conversation).text).toBe('') + }) + it('drops the chips begun in a removed workspace on that host only', () => { const resolved = new Map([ ['tab-1:leaf', OWNER], diff --git a/src/renderer/src/components/native-chat/native-chat-pending-attachment-cache.ts b/src/renderer/src/components/native-chat/native-chat-pending-attachment-cache.ts index 3d4e477d0ce..a2c771a6568 100644 --- a/src/renderer/src/components/native-chat/native-chat-pending-attachment-cache.ts +++ b/src/renderer/src/components/native-chat/native-chat-pending-attachment-cache.ts @@ -1,15 +1,11 @@ -// A composer's chips still on their way (a save, an upload, a server's answer), per pane scope, -// owned here rather than by the composer. A prompt card unmounts the composer, so anything owed to -// the message must outlive it: the composer that comes back shows the same chips, Send waits for -// them, and each settles into the scope's draft whichever composer, if any, is showing. They are -// never saved with the draft, so a restored draft cannot bring back an upload as if it were -// attached. Every chip dies with its operation: settled or dropped when the save or upload settles -// (each bounded by its call timeout), removed by the user, or dropped with the drafts of a tab or -// workspace the user closed or removed, so none settles into a draft nothing could find or delete. +// Pending operations belong to draft scopes and outlive their composers. They are not persisted. +// Completion, failure, explicit removal, draft/workspace deletion or renderer exit ends ownership. import { useCallback, useSyncExternalStore } from 'react' import type { NativeChatComposerImageAttachment } from './NativeChatComposerField' import type { NativeChatComposerDraftOwner } from './native-chat-composer-draft-storage' +import { appendNativeChatDraftCache } from './native-chat-draft-cache' +import { formatNativeChatFileReference } from '../../../../shared/agent-image-paste' import { appendToNativeChatComposerDraft, nativeChatDraftScopeTabId, @@ -115,17 +111,23 @@ export function settleNativeChatPendingAttachment( return true } -/** Shows a pending chip that was held out of sight, such as while a server was asked first. */ -export function revealNativeChatPendingAttachment(scopeKey: string, id: string): void { - const current = nativeChatPendingAttachmentSnapshot(scopeKey) - if (current.some((attachment) => attachment.id === id && attachment.hidden)) { - writePending( - scopeKey, - current.map((attachment) => { - const { hidden: _hidden, ...shown } = attachment - return attachment.id === id ? shown : attachment - }) - ) +/** File results settle only while their operation still belongs to this draft. */ +export function settleNativeChatPendingAttachmentReferences( + scopeKey: string, + references: { id: string; path: string }[], + insertAtCaret?: (paths: string[]) => void +): void { + const owner = pendingOwners.get(scopeKey) + const paths = references.flatMap(({ id, path }) => + takeNativeChatPendingAttachment(scopeKey, id) ? [path] : [] + ) + if (paths.length === 0) { + return + } + if (insertAtCaret) { + insertAtCaret(paths) + } else { + appendNativeChatDraftCache(scopeKey, paths.map(formatNativeChatFileReference).join(' '), owner) } } diff --git a/src/renderer/src/components/native-chat/native-chat-prompt-document-text.test.ts b/src/renderer/src/components/native-chat/native-chat-prompt-document-text.test.ts new file mode 100644 index 00000000000..24f3056c1ee --- /dev/null +++ b/src/renderer/src/components/native-chat/native-chat-prompt-document-text.test.ts @@ -0,0 +1,66 @@ +import { expect, it } from 'vitest' +import { withNativeChatComposerDraftAddition } from './native-chat-composer-draft-addition' +import type { NativeChatComposerDraft } from './native-chat-composer-draft-storage' + +it.each(['spaces', 'hard breaks', 'empty paragraphs'])( + 'trims trailing %s while keeping picked skills and earlier paragraphs', + (trailing) => { + const first = { type: 'paragraph', content: [{ type: 'text', text: 'first' }] } + const picked = { type: 'nativeChatSkill', attrs: { token: '$review' } } + const draft: NativeChatComposerDraft = { + text: `first\n$review${trailing === 'spaces' ? ' ' : '\n\n'}`, + images: [], + document: { + type: 'doc', + content: [ + first, + { + type: 'paragraph', + content: [ + picked, + ...(trailing === 'spaces' + ? [{ type: 'text', text: ' ' }] + : trailing === 'hard breaks' + ? [{ type: 'hardBreak' }, { type: 'hardBreak' }] + : []) + ] + }, + ...(trailing === 'empty paragraphs' + ? [ + { type: 'paragraph', content: [] }, + { type: 'paragraph', content: [] } + ] + : []) + ] + } + } + const result = withNativeChatComposerDraftAddition(draft, { text: 'go' }) + expect(result.text).toBe('first\n$review\n\ngo') + expect(result.document?.content).toEqual([ + first, + { type: 'paragraph', content: [picked] }, + { type: 'paragraph', content: [] }, + { type: 'paragraph', content: [{ type: 'text', text: 'go' }] } + ]) + } +) + +it('replaces an all-whitespace document with literal returned text', () => { + const result = withNativeChatComposerDraftAddition( + { + text: ' \n', + images: [], + document: { + type: 'doc', + content: [ + { type: 'paragraph', content: [{ type: 'text', text: ' ' }] }, + { type: 'paragraph', content: [] } + ] + } + }, + { text: '$review' } + ) + expect(result.document?.content).toEqual([ + { type: 'paragraph', content: [{ type: 'text', text: '$review' }] } + ]) +}) diff --git a/src/renderer/src/components/native-chat/native-chat-prompt-document-text.ts b/src/renderer/src/components/native-chat/native-chat-prompt-document-text.ts new file mode 100644 index 00000000000..6151a994c95 --- /dev/null +++ b/src/renderer/src/components/native-chat/native-chat-prompt-document-text.ts @@ -0,0 +1,59 @@ +import type { JSONContent } from '@tiptap/react' + +export function promptTextContent(text: string): JSONContent { + return { + type: 'doc', + content: text.split('\n').map((line) => ({ + type: 'paragraph', + content: line ? [{ type: 'text', text: line }] : [] + })) + } +} + +/** Applies an appended text change without replacing existing picked nodes. */ +export function appendPromptDocumentText( + document: JSONContent | undefined, + before: string, + after: string +): JSONContent | undefined { + if (!document || before === after) { + return document + } + const kept = before.trimEnd() + if (kept === '') { + return promptTextContent(after) + } + const blocks = [...(document.content ?? [])] + while (blocks.length > 0) { + const block = blocks.pop() + const content = [...(block?.content ?? [])] + while (content.length > 0) { + const node = content.at(-1) + if (node?.type === 'hardBreak') { + content.pop() + } else if (node?.type === 'text') { + const text = node.text?.trimEnd() ?? '' + if (text === '') { + content.pop() + } else { + content[content.length - 1] = { ...node, text } + break + } + } else { + break + } + } + if (content.length > 0 && block) { + const [first, ...rest] = promptTextContent(after.slice(kept.length)).content ?? [] + return { + ...document, + content: [ + ...blocks, + { ...block, content: [...content, ...(first?.content ?? [])] }, + ...rest + ] + } + } + } + return promptTextContent(after) +} diff --git a/src/renderer/src/components/native-chat/native-chat-prompt-document.ts b/src/renderer/src/components/native-chat/native-chat-prompt-document.ts index 7ed5fd8c130..96bba1a75fd 100644 --- a/src/renderer/src/components/native-chat/native-chat-prompt-document.ts +++ b/src/renderer/src/components/native-chat/native-chat-prompt-document.ts @@ -1,6 +1,7 @@ import { NativeChatSkillPill } from './NativeChatSkillPill' -import { ReactNodeViewRenderer, Node, type JSONContent } from '@tiptap/react' +import { ReactNodeViewRenderer, Node } from '@tiptap/react' import type { Node as ProseMirrorNode } from '@tiptap/pm/model' +export { promptTextContent } from './native-chat-prompt-document-text' export const NativeChatSkill = Node.create({ name: 'nativeChatSkill', @@ -24,16 +25,6 @@ export const NativeChatSkill = Node.create({ ] }) -export function promptTextContent(text: string): JSONContent { - return { - type: 'doc', - content: text.split('\n').map((line) => ({ - type: 'paragraph', - content: line ? [{ type: 'text', text: line }] : [] - })) - } -} - /** Each boundary maps a plain-text caret to a document position, including atomic skills. */ export function promptTextMap(doc: ProseMirrorNode): { text: string; positions: number[] } { let text = '' diff --git a/src/renderer/src/components/native-chat/native-chat-session-attachment-drop.ts b/src/renderer/src/components/native-chat/native-chat-session-attachment-drop.ts index 0d655d8de8a..e51be382d94 100644 --- a/src/renderer/src/components/native-chat/native-chat-session-attachment-drop.ts +++ b/src/renderer/src/components/native-chat/native-chat-session-attachment-drop.ts @@ -18,7 +18,7 @@ export type NativeChatPendingAttachmentChips = { drop: (id: string) => boolean /** Adds stored files as `@path` references to the scope's draft, which keeps them through an * input-method composition or a remount. */ - attachReferences: (paths: string[]) => void + attachReferences: (references: { id: string; path: string }[]) => void } /** @@ -86,7 +86,7 @@ export async function attachNativeChatSessionAttachmentPaths(args: { return } const notAttached: { name: string; reason: string }[] = [] - const references: string[] = [] + const references: { id: string; path: string }[] = [] for (const { path, chipId } of pending) { const storedPath = stored.get(path) if (!storedPath) { @@ -100,9 +100,7 @@ export async function attachNativeChatSessionAttachmentPaths(args: { continue } // Other files become `@path` references, as every attach does, unless their chip was removed. - if (args.chips.drop(chipId)) { - references.push(storedPath) - } + references.push({ id: chipId, path: storedPath }) } // Inserting at the caret clears the notice, so what failed is said after. if (references.length > 0) { diff --git a/src/renderer/src/components/native-chat/use-native-chat-composer-attachments.test.tsx b/src/renderer/src/components/native-chat/use-native-chat-composer-attachments.test.tsx index 5208dfe6e8a..ddf55907789 100644 --- a/src/renderer/src/components/native-chat/use-native-chat-composer-attachments.test.tsx +++ b/src/renderer/src/components/native-chat/use-native-chat-composer-attachments.test.tsx @@ -477,13 +477,14 @@ describe('useNativeChatComposerAttachments', () => { it('keeps an upload that finishes while the composer is unmounted, for when it returns', async () => { const probe = await renderProbe('pty-gone') const chips = probe.latest().pendingChips - const begun: { image?: string | null; removed?: string | null } = {} + const begun: { image?: string | null; removed?: string | null; reference?: string | null } = {} act(() => { begun.image = chips.begin(undefined, 'shot.png') begun.removed = chips.begin(undefined, 'old.png') + begun.reference = chips.begin(undefined, 'notes.pdf') }) - const { image, removed } = begun - if (!image || !removed) { + const { image, removed, reference } = begun + if (!image || !removed || !reference) { throw new Error('expected pending chips') } act(() => probe.latest().removeImageAttachment(removed)) @@ -492,7 +493,7 @@ describe('useNativeChatComposerAttachments', () => { chips.resolve(image, '/srv/agent-session-attachments/u1/shot.png') chips.resolve(removed, '/srv/agent-session-attachments/u2/old.png') - chips.attachReferences(['/srv/agent-session-attachments/u3/notes.pdf']) + chips.attachReferences([{ id: reference, path: '/srv/agent-session-attachments/u3/notes.pdf' }]) expect(readNativeChatAttachmentCache('pty-gone')).toEqual([ { id: image, path: '/srv/agent-session-attachments/u1/shot.png' } @@ -531,11 +532,33 @@ describe('useNativeChatComposerAttachments', () => { act(() => back.root.unmount()) }) + it('gives uploads unique IDs across composers remounted in the same clock tick', async () => { + const now = vi.spyOn(Date, 'now').mockReturnValue(1_000) + try { + const first = await renderProbe('pty-ids', true) + const ids: (string | null)[] = [] + act(() => ids.push(first.latest().pendingChips.begin(undefined, 'first.pdf'))) + act(() => first.root.unmount()) + const next = await renderProbe('pty-ids', true) + act(() => ids.push(next.latest().pendingChips.begin(undefined, 'second.pdf'))) + expect(new Set(ids).size).toBe(2) + expect(nativeChatPendingAttachmentSnapshot('pty-ids')).toHaveLength(2) + act(() => next.root.unmount()) + } finally { + now.mockRestore() + } + }) + it('inserts a stored file at the caret while the composer is showing and not composing', async () => { const probe = await renderProbe('pty-caret', true) - act(() => - probe.latest().pendingChips.attachReferences(['/srv/agent-session-attachments/u6/a.pdf']) - ) + act(() => { + const chips = probe.latest().pendingChips + const id = chips.begin(undefined, 'a.pdf') + if (!id) { + throw new Error('expected pending chip') + } + chips.attachReferences([{ id, path: '/srv/agent-session-attachments/u6/a.pdf' }]) + }) expect(probe.draft()).toBe('@/srv/agent-session-attachments/u6/a.pdf ') expect(readNativeChatDraftCache('pty-caret')).toBe('') act(() => probe.root.unmount()) @@ -555,8 +578,7 @@ describe('useNativeChatComposerAttachments', () => { const id: string = chipId // The upload finishes mid-composition: the reference waits for the composition to settle. act(() => { - expect(chips.drop(id)).toBe(true) - chips.attachReferences(['/srv/agent-session-attachments/u4/notes.pdf']) + chips.attachReferences([{ id, path: '/srv/agent-session-attachments/u4/notes.pdf' }]) }) // A prompt card takes the composer's place before the composition settles. act(() => probe.root.unmount()) diff --git a/src/renderer/src/components/native-chat/use-native-chat-composer-attachments.ts b/src/renderer/src/components/native-chat/use-native-chat-composer-attachments.ts index b360492493f..2eb20ac7954 100644 --- a/src/renderer/src/components/native-chat/use-native-chat-composer-attachments.ts +++ b/src/renderer/src/components/native-chat/use-native-chat-composer-attachments.ts @@ -1,12 +1,5 @@ import type { NativeChatComposerInput } from './native-chat-composer-input' -import { - useCallback, - useEffect, - useMemo, - useRef, - useSyncExternalStore, - type RefObject -} from 'react' +import { useCallback, useEffect, useMemo, useSyncExternalStore, type RefObject } from 'react' import { nativeChatComposerTargetIsRemote, nativeChatLocalAttachmentUnsupportedNotice, @@ -26,17 +19,15 @@ import type { NativeChatPendingAttachmentChips } from './native-chat-session-att import { addNativeChatPendingAttachment, clearNativeChatPendingAttachments, - revealNativeChatPendingAttachment, settleNativeChatPendingAttachment, + settleNativeChatPendingAttachmentReferences, + nativeChatPendingAttachmentSnapshot, takeNativeChatPendingAttachment, useNativeChatPendingAttachments } from './native-chat-pending-attachment-cache' -import { - appendNativeChatAttachmentCache, - appendNativeChatDraftCache -} from './native-chat-draft-cache' +import { appendNativeChatAttachmentCache } from './native-chat-draft-cache' import { useNativeChatComposerAttachmentPreviews } from './use-native-chat-composer-attachment-previews' -import { formatNativeChatFileReference } from '../../../../shared/agent-image-paste' +import { createUuidV4 } from '../../../../shared/uuid-v4' export type UseNativeChatComposerAttachmentsArgs = { attachmentScopeKey: string @@ -75,11 +66,7 @@ export function useNativeChatComposerAttachments({ clearImageAttachments: () => void flushPendingAttachments: () => void removeImageAttachment: (id: string) => void - beginPendingImageAttachment: ( - previewUrl?: string, - pendingName?: string, - options?: { hidden?: true } - ) => string | null + beginPendingImageAttachment: (previewUrl?: string, pendingName?: string) => string | null resolvePendingImageAttachment: (id: string, path: string, connectionId?: string | null) => void revealPendingImageAttachment: (id: string, previewUrl?: string) => void dropPendingImageAttachment: (id: string) => boolean @@ -117,19 +104,18 @@ export function useNativeChatComposerAttachments({ ], [pending, previews, restoring, settled] ) - const imageAttachmentCounter = useRef(0) - const mountedRef = useRef(true) + const mountedRef = useMemo( + () => ({ scopeKey: attachmentScopeKey, current: true }), + [attachmentScopeKey] + ) useEffect(() => { mountedRef.current = true return () => { mountedRef.current = false } - }, []) + }, [mountedRef]) - const nextAttachmentId = useCallback((): string => { - imageAttachmentCounter.current += 1 - return `${Date.now()}-${imageAttachmentCounter.current}` - }, []) + const nextAttachmentId = useCallback(() => createUuidV4(), []) // Client-local paths cannot cross into a runtime target; workspace-owned // paths may only bypass this after the internal drop ownership gate. @@ -181,10 +167,8 @@ export function useNativeChatComposerAttachments({ // Placeholder chip shown the instant a paste starts, so a clipboard image that // takes a beat to save (or upload over SSH) never reads as a dropped paste. const beginPendingImageAttachment = useCallback( - (previewUrl?: string, pendingName?: string, options?: { hidden?: true }): string | null => { - // Without image input the saved paste is attached by path once it lands, so no image chip; - // a named upload chip is a file, not an image, and still shows. - if (disabledRef.current || (!acceptsImages && pendingName === undefined)) { + (previewUrl?: string, pendingName?: string): string | null => { + if (disabledRef.current) { return null } if (attachmentTargetBlocked()) { @@ -196,14 +180,12 @@ export function useNativeChatComposerAttachments({ id, path: '', pending: true, - ...(pendingName ? { pendingName } : {}), - ...(options?.hidden ? { hidden: true } : {}) + ...(pendingName ? { pendingName } : {}) }) setPreview(id, previewUrl) return id }, [ - acceptsImages, attachmentScopeKey, attachmentTargetBlocked, disabledRef, @@ -213,17 +195,35 @@ export function useNativeChatComposerAttachments({ ] ) + const attachPendingReferences = useCallback( + (references: { id: string; path: string }[]) => { + settleNativeChatPendingAttachmentReferences( + attachmentScopeKey, + references, + mountedRef.current && !disabledRef.current && !isComposing() + ? (paths) => attachResolvedPaths(paths, null) + : undefined + ) + }, + [attachResolvedPaths, attachmentScopeKey, disabledRef, isComposing, mountedRef] + ) + const resolvePendingImageAttachment = useCallback( (id: string, path: string, connectionId?: string | null) => { - settleNativeChatPendingAttachment(attachmentScopeKey, id, path, connectionId) + if (acceptsImages) { + settleNativeChatPendingAttachment(attachmentScopeKey, id, path, connectionId) + } else { + attachPendingReferences([{ id, path }]) + } }, - [attachmentScopeKey] + [acceptsImages, attachPendingReferences, attachmentScopeKey] ) const revealPendingImageAttachment = useCallback( (id: string, previewUrl?: string) => { - setPreview(id, previewUrl) - revealNativeChatPendingAttachment(attachmentScopeKey, id) + if (nativeChatPendingAttachmentSnapshot(attachmentScopeKey).some((chip) => chip.id === id)) { + setPreview(id, previewUrl) + } }, [attachmentScopeKey, setPreview] ) @@ -245,23 +245,12 @@ export function useNativeChatComposerAttachments({ drop: dropPendingImageAttachment, // At the caret, as every attach does; mid-composition or once the composer is gone, into the // scope's draft, which keeps it until the composition settles or the composer comes back. - attachReferences: (paths) => { - if (mountedRef.current && !isComposing()) { - attachResolvedPaths(paths, null) - return - } - appendNativeChatDraftCache( - attachmentScopeKey, - paths.map(formatNativeChatFileReference).join(' ') - ) - } + attachReferences: attachPendingReferences }), [ - attachResolvedPaths, - attachmentScopeKey, + attachPendingReferences, beginPendingImageAttachment, dropPendingImageAttachment, - isComposing, resolvePendingImageAttachment ] ) diff --git a/src/renderer/src/components/native-chat/use-native-chat-composer-image-input.test.tsx b/src/renderer/src/components/native-chat/use-native-chat-composer-image-input.test.tsx index 93e289e4462..71536868356 100644 --- a/src/renderer/src/components/native-chat/use-native-chat-composer-image-input.test.tsx +++ b/src/renderer/src/components/native-chat/use-native-chat-composer-image-input.test.tsx @@ -87,10 +87,10 @@ describe('structured composer image input', () => { expect(result.pendingChip).not.toBeNull() }) - it('references an image by path, with no chip, when the agent takes none', async () => { + it('references an image by path, with a removable pending operation, when the agent takes none', async () => { const result = await attach(false, '/tmp/shot.png') - expect(result.chips).toEqual([]) - expect(result.pendingChip).toBeNull() + expect(result.chips).toEqual(['']) + expect(result.pendingChip).not.toBeNull() expect(result.draft).toContain('/tmp/shot.png') }) diff --git a/src/renderer/src/components/native-chat/use-native-chat-composer-paste-session-attachments.test.tsx b/src/renderer/src/components/native-chat/use-native-chat-composer-paste-session-attachments.test.tsx index 421462ae144..e08c92eea4f 100644 --- a/src/renderer/src/components/native-chat/use-native-chat-composer-paste-session-attachments.test.tsx +++ b/src/renderer/src/components/native-chat/use-native-chat-composer-paste-session-attachments.test.tsx @@ -49,7 +49,7 @@ vi.stubGlobal('URL', { import { useNativeChatComposerPaste } from './use-native-chat-composer-paste' type HookApi = ReturnType -type Chip = { id: string; path: string; pending: boolean; hidden?: true } +type Chip = { id: string; path: string; pending: boolean } const sessionOwner: NativeChatAttachmentOwner = { kind: 'runtime-session', @@ -87,20 +87,18 @@ async function renderPaste(args: { setCaret: () => {}, resolveAttachmentOwner: () => sessionOwner, attachResolvedPaths: args.attachResolvedPaths ?? (() => {}), - beginPendingImageAttachment: (_preview, _name, options) => { + beginPendingImageAttachment: () => { counter += 1 chips.push({ id: `chip-${counter}`, path: '', - pending: true, - ...(options?.hidden ? { hidden: true } : {}) + pending: true }) return `chip-${counter}` }, revealPendingImageAttachment: (id) => { const chip = chips.find((candidate) => candidate.id === id) if (chip) { - delete chip.hidden revealed += 1 } }, @@ -151,7 +149,7 @@ beforeEach(() => { vi.resetAllMocks() mocks.readClipboardText.mockResolvedValue('') mocks.readClipboardImageThumbnail.mockResolvedValue(null) - mocks.clipboardHasImage.mockResolvedValue(false) + mocks.clipboardHasImage.mockResolvedValue(true) mocks.readClipboardFilePaths.mockResolvedValue([]) mocks.prepareNativeChatSessionAttachmentUpload.mockResolvedValue({ ok: true, @@ -195,11 +193,12 @@ describe('pasting into a structured chat on a paired server', () => { expect(probe.chips).toEqual([{ id: 'chip-1', path: storedPath, pending: false }]) }) - it('attaches the server path when no placeholder chip was shown', async () => { + it('settles the server path when no thumbnail was shown', async () => { const attachResolvedPaths = vi.fn() const probe = await renderPaste({ attachResolvedPaths }) await act(async () => probe.api().pasteFromClipboard()) - expect(attachResolvedPaths).toHaveBeenCalledExactlyOnceWith([storedPath], null) + expect(probe.chips).toEqual([{ id: 'chip-1', path: storedPath, pending: false }]) + expect(attachResolvedPaths).not.toHaveBeenCalled() }) it('refuses on a server without the attachment store and saves nothing', async () => { @@ -236,7 +235,7 @@ describe('pasting into a structured chat on a paired server', () => { await act(async () => probe.api().handlePaste(imagePasteEvent('caption'))) expect(insertTypedText).toHaveBeenCalledWith('caption') expect(setNotice).not.toHaveBeenCalledWith('needs newer server') - // The image was owed to the message only out of sight: no chip showed, so none flashed. + // The operation ends quietly when this server cannot store its image. expect(probe.begun()).toBe(1) expect(probe.revealed()).toBe(0) expect(probe.chips).toEqual([]) diff --git a/src/renderer/src/components/native-chat/use-native-chat-composer-paste.test.tsx b/src/renderer/src/components/native-chat/use-native-chat-composer-paste.test.tsx index cca232c9140..8f42d083208 100644 --- a/src/renderer/src/components/native-chat/use-native-chat-composer-paste.test.tsx +++ b/src/renderer/src/components/native-chat/use-native-chat-composer-paste.test.tsx @@ -53,6 +53,7 @@ function createChipStore(): { chips: FakeChip[] begin: (previewUrl?: string) => string | null resolve: (id: string, path: string, connectionId?: string | null) => void + reveal: (id: string, previewUrl?: string) => void drop: (id: string) => void connectionIds: (string | null | undefined)[] } { @@ -62,6 +63,12 @@ function createChipStore(): { return { chips, connectionIds, + reveal: (id, previewUrl) => { + const chip = chips.find((candidate) => candidate.id === id) + if (chip) { + chip.previewUrl = previewUrl + } + }, begin: (previewUrl) => { counter += 1 const id = `chip-${counter}` @@ -93,6 +100,7 @@ type ProbeArgs = { attachResolvedPaths: (paths: string[], connectionId?: string | null) => void beginPendingImageAttachment: (previewUrl?: string) => string | null resolvePendingImageAttachment: (id: string, path: string, connectionId?: string | null) => void + revealPendingImageAttachment: (id: string, previewUrl?: string) => void dropPendingImageAttachment: (id: string) => void insertTypedText: (text: string) => boolean setNotice: (notice: string | null) => void @@ -134,8 +142,9 @@ async function renderProbe(args: { disabled, resolveAttachmentOwner: args.resolveAttachmentOwner, attachResolvedPaths: args.attachResolvedPaths ?? (() => {}), - beginPendingImageAttachment: store.begin, + beginPendingImageAttachment: args.agent === 'omp' ? () => null : store.begin, resolvePendingImageAttachment: store.resolve, + revealPendingImageAttachment: store.reveal, dropPendingImageAttachment: store.drop, insertTypedText: args.insertTypedText ?? (() => true), setNotice: args.setNotice ?? (() => {}), @@ -175,7 +184,7 @@ beforeEach(() => { vi.resetAllMocks() mocks.readClipboardText.mockResolvedValue('') mocks.readClipboardImageThumbnail.mockResolvedValue(null) - mocks.clipboardHasImage.mockResolvedValue(false) + mocks.clipboardHasImage.mockResolvedValue(true) mocks.readClipboardFilePaths.mockResolvedValue([]) mocks.saveClipboardImageAsTempFile.mockResolvedValue(null) }) @@ -260,11 +269,7 @@ describe('useNativeChatComposerPaste', () => { } }) expect(mocks.saveClipboardImageAsTempFile).toHaveBeenCalledWith({ connectionId: 'conn-1' }) - if (source === 'event') { - expect(store.chips[0]?.path).toBe('/remote/tmp/omp.png') - } else { - expect(attachResolvedPaths).toHaveBeenCalledWith(['/remote/tmp/omp.png'], 'conn-1') - } + expect(attachResolvedPaths).toHaveBeenCalledWith(['/remote/tmp/omp.png'], 'conn-1') expect(setNotice.mock.calls.every(([notice]) => notice === null)).toBe(true) } ) @@ -389,7 +394,7 @@ describe('useNativeChatComposerPaste', () => { expect(store.chips[0]).toMatchObject({ path: '/tmp/orca-paste-2.png', pending: false }) }) - it('attaches directly when no clipboard preview was available', async () => { + it('settles the owned operation when no clipboard preview was available', async () => { mocks.readClipboardImageThumbnail.mockResolvedValue(null) mocks.saveClipboardImageAsTempFile.mockResolvedValue('C:\\Temp\\orca-paste-3.png') const store = createChipStore() @@ -402,8 +407,8 @@ describe('useNativeChatComposerPaste', () => { await act(async () => { probe.latest().pasteFromClipboard() }) - expect(store.chips).toHaveLength(0) - expect(attachResolvedPaths).toHaveBeenCalledWith(['C:\\Temp\\orca-paste-3.png'], null) + expect(store.chips[0]).toMatchObject({ path: 'C:\\Temp\\orca-paste-3.png', pending: false }) + expect(attachResolvedPaths).not.toHaveBeenCalled() }) it('inserts text independently of a failed image save', async () => { @@ -425,8 +430,7 @@ describe('useNativeChatComposerPaste', () => { }) it('still falls through to text when the clipboard holds no image', async () => { - mocks.readClipboardImageThumbnail.mockResolvedValue(null) - mocks.saveClipboardImageAsTempFile.mockResolvedValue(null) + mocks.clipboardHasImage.mockResolvedValue(false) mocks.readClipboardText.mockResolvedValue('hello') const insertTypedText = vi.fn() const store = createChipStore() @@ -787,17 +791,21 @@ describe('file-manager copies', () => { mocks.saveClipboardImageAsTempFile.mockResolvedValue('/tmp/shot.png') const insertTypedText = vi.fn(() => true) const attachResolvedPaths = vi.fn() + const store = createChipStore() const probe = await renderProbe({ resolveAttachmentOwner: () => ({ kind: 'local' }), insertTypedText, - attachResolvedPaths + attachResolvedPaths, + store }) await act(async () => probe.latest().pasteFromClipboard()) expect(insertTypedText).not.toHaveBeenCalled() - expect(attachResolvedPaths).toHaveBeenCalledExactlyOnceWith(['/tmp/shot.png'], null) + expect(store.chips[0]).toMatchObject({ path: '/tmp/shot.png', pending: false }) + expect(attachResolvedPaths).not.toHaveBeenCalled() }) it('types the label when no image came with the files', async () => { + mocks.clipboardHasImage.mockResolvedValue(false) mocks.readClipboardText.mockResolvedValue('notes.txt') mocks.readClipboardFilePaths.mockResolvedValue(['/Users/me/notes.txt']) const insertTypedText = vi.fn(() => true) diff --git a/src/renderer/src/components/native-chat/use-native-chat-composer-paste.ts b/src/renderer/src/components/native-chat/use-native-chat-composer-paste.ts index 1701c069b34..b9d5cc465cc 100644 --- a/src/renderer/src/components/native-chat/use-native-chat-composer-paste.ts +++ b/src/renderer/src/components/native-chat/use-native-chat-composer-paste.ts @@ -3,7 +3,6 @@ import { useNativeChatPasteLifetime } from './use-native-chat-paste-lifetime' import { nativeChatAttachmentOwnerUnchanged } from './native-chat-resolved-path-ownership' import { assertClipboardTextWithinLimit } from '../../../../shared/clipboard-text' import { setNativeChatPasteFailure } from './native-chat-composer-notice' -import type { AgentType } from '../../../../shared/agent-status-types' import { NATIVE_CHAT_CONTEXT_PASTE_MAX_BYTES } from './native-chat-composer-target' import { nativeChatPasteUnavailableNotice } from '@/lib/native-chat-paste-request' import { @@ -22,33 +21,12 @@ import { saveNativeChatClipboardImage } from './native-chat-clipboard-image-save' -export type UseNativeChatComposerPasteArgs = { - targetKey?: string - agent: AgentType - /** Live composer-disabled state (no pty / presence-lock); read at await-resume - * via a ref so a flip mid-paste doesn't write into a guarded composer. */ - disabled: boolean - caret: number - /** Resolved at paste time: SSH panes must save the clipboard image on the - * remote host, or the attached path names a file the agent cannot read. */ - resolveAttachmentOwner: () => NativeChatAttachmentOwner - attachResolvedPaths: (paths: string[], connectionId?: string | null) => void - beginPendingImageAttachment: ( - previewUrl?: string, - pendingName?: string, - options?: { hidden?: true } - ) => string | null - resolvePendingImageAttachment: (id: string, path: string, connectionId?: string | null) => void - /** Shows a chip begun hidden, with its thumbnail. */ - revealPendingImageAttachment?: (id: string, previewUrl?: string) => void - dropPendingImageAttachment: (id: string) => void - insertTypedText: (text: string) => boolean - setCaret: (caret: number) => void - setNotice: (notice: string | null, errorText?: string) => void -} +import type { UseNativeChatComposerPasteArgs } from './native-chat-composer-paste-options' +export type { UseNativeChatComposerPasteArgs } from './native-chat-composer-paste-options' export function useNativeChatComposerPaste({ targetKey, + attachmentScopeKey, disabled, caret, resolveAttachmentOwner, @@ -71,6 +49,7 @@ export function useNativeChatComposerPaste({ const { lifetime, track, startImageChip, keepStoreUploadAfterUnmount } = useNativeChatPasteLifetime({ targetKey, + attachmentScopeKey, beginPendingImageAttachment, resolvePendingImageAttachment, revealPendingImageAttachment, @@ -83,6 +62,15 @@ export function useNativeChatComposerPaste({ setNotice(nativeChatPasteUnavailableNotice()) } }, [lifetime, setNotice]) + const dropImagePaste = useCallback( + (pendingId: string | null) => { + if (pendingId) { + lifetime.pending.delete(pendingId) + dropPendingImageAttachment(pendingId) + } + }, + [lifetime, dropPendingImageAttachment] + ) // Image failures do not decide whether text can be inserted. Beside pasted text, a server too old // to store the image drops only the image rendition, as an owner that takes no images does. @@ -112,15 +100,11 @@ export function useNativeChatComposerPaste({ [canPaste, setNotice] ) - /** Settle the chip started at paste time, or attach directly when the paste - * produced no placeholder (no clipboard preview was available). */ + /** Settle the operation started at paste time, or attach directly if chips are unavailable. */ const settleImagePaste = useCallback( (pendingId: string | null, path: string, originalOwner: NativeChatAttachmentOwner) => { if (!nativeChatAttachmentOwnerUnchanged(originalOwner, resolveAttachmentOwner())) { - if (pendingId) { - lifetime.pending.delete(pendingId) - dropPendingImageAttachment(pendingId) - } + dropImagePaste(pendingId) setNotice(nativeChatWorktreeNotReadyNotice()) return } @@ -135,7 +119,7 @@ export function useNativeChatComposerPaste({ [ attachResolvedPaths, lifetime, - dropPendingImageAttachment, + dropImagePaste, resolveAttachmentOwner, resolvePendingImageAttachment, setNotice @@ -185,28 +169,23 @@ export function useNativeChatComposerPaste({ // Why: snapshot the caret before the async temp-file round-trip — `caret` // state can move (further typing/selection) while the await is in flight. const caretAtPaste = caret - // Beside pasted text, a server too old to store the image drops it quietly, so its chip stays - // out of sight until the server answers; Send waits for it from the start all the same. + // Wait for server acceptance before minting a preview; the operation is visible from the start. const awaitServer = Boolean(text) && owner.kind === 'runtime-session' - const chip = startImageChip(owner, imageFile, { - hidden: awaitServer, + const { id: pendingId, reveal } = startImageChip(owner, imageFile, { + deferPreview: awaitServer, canShow: () => ownerAcceptsClipboardImage(owner) && canPaste() }) - const pendingId = chip.id void (async () => { const saved = await saveClipboardImageForOwner( owner, async () => Boolean(text), - awaitServer ? chip.reveal : undefined + awaitServer ? reveal : undefined ) if (keepStoreUploadAfterUnmount(pendingId, saved)) { return } if (saved.status !== 'saved' || !canPaste()) { - if (pendingId) { - lifetime.pending.delete(pendingId) - dropPendingImageAttachment(pendingId) - } + dropImagePaste(pendingId) return } settleImagePaste(pendingId, saved.tempPath, owner) @@ -217,9 +196,8 @@ export function useNativeChatComposerPaste({ }, [ canPaste, - lifetime, caret, - dropPendingImageAttachment, + dropImagePaste, insertTypedText, showPasteUnavailable, resolveAttachmentOwner, @@ -278,26 +256,44 @@ export function useNativeChatComposerPaste({ } return } - const thumbnailPromise = window.api.ui.readClipboardImageThumbnail().catch(() => null) - const savePromise = saveClipboardImageForOwner(owner, async () => { + const hasImage = await window.api.ui.clipboardHasImage().catch((error) => { + if (canPaste()) { + setNativeChatPasteFailure(setNotice, error) + } + return null + }) + if (hasImage !== true) { + const read = await textRead + if (read?.labelsFiles) { + insertText(read.text) + } + return + } + if (!canPaste()) { + return + } + // Ownership starts before saving; a thumbnail only supplies its preview. + const pendingId = beginPendingImageAttachment() + if (pendingId) { + track(pendingId, '', owner) + } + void window.api.ui + .readClipboardImageThumbnail() + .catch(() => null) + .then((thumbnail) => { + if (pendingId && thumbnail) { + revealPendingImageAttachment?.(pendingId, thumbnail.dataUrl) + } + }) + const saved = await saveClipboardImageForOwner(owner, async () => { const read = await textRead return Boolean(read?.text) && read?.labelsFiles !== true }) - const thumbnail = await thumbnailPromise - const pendingId = - thumbnail && canPaste() ? beginPendingImageAttachment(thumbnail.dataUrl) : null - if (pendingId) { - track(pendingId, thumbnail?.dataUrl ?? '', owner) - } - const saved = await savePromise if (keepStoreUploadAfterUnmount(pendingId, saved)) { return } if (!canPaste() || saved.status !== 'saved') { - if (pendingId) { - lifetime.pending.delete(pendingId) - dropPendingImageAttachment(pendingId) - } + dropImagePaste(pendingId) // A file's label is typed only when no image came with it. const read = saved.status === 'empty' ? await textRead : null if (read?.labelsFiles) { @@ -309,9 +305,9 @@ export function useNativeChatComposerPaste({ })() }, [ beginPendingImageAttachment, + revealPendingImageAttachment, canPaste, - lifetime, - dropPendingImageAttachment, + dropImagePaste, insertTypedText, keepStoreUploadAfterUnmount, track, diff --git a/src/renderer/src/components/native-chat/use-native-chat-external-attachments.test.tsx b/src/renderer/src/components/native-chat/use-native-chat-external-attachments.test.tsx index 74d257e5af1..f92095aaad1 100644 --- a/src/renderer/src/components/native-chat/use-native-chat-external-attachments.test.tsx +++ b/src/renderer/src/components/native-chat/use-native-chat-external-attachments.test.tsx @@ -587,9 +587,9 @@ describe('useNativeChatExternalAttachments', () => { '/srv/agent-session-attachments/u1/shot.png', null ) - expect(chips.drop).toHaveBeenCalledExactlyOnceWith('chip-2') + expect(chips.drop).not.toHaveBeenCalled() expect(chips.attachReferences).toHaveBeenCalledExactlyOnceWith([ - '/srv/agent-session-attachments/u2/notes.md' + { id: 'chip-2', path: '/srv/agent-session-attachments/u2/notes.md' } ]) // The client never reads its own disk for these paths: the server stores the bytes. expect(mocks.stat).not.toHaveBeenCalled() @@ -653,11 +653,11 @@ describe('useNativeChatExternalAttachments', () => { ) ) expect(chips.attachReferences).toHaveBeenCalledExactlyOnceWith([ - '/srv/agent-session-attachments/u1/notes.md' + { id: 'chip-1', path: '/srv/agent-session-attachments/u1/notes.md' } ]) }) - it('does not insert a file whose chip the user removed while it uploaded', async () => { + it('keeps each file operation ID through reference settlement, including removed chips', async () => { const upload = deferred() mocks.uploadNativeChatSessionAttachmentPaths.mockReturnValueOnce(upload.promise) const chips = trackingChips() @@ -681,7 +681,8 @@ describe('useNativeChatExternalAttachments', () => { ) expect(chips.attachReferences).toHaveBeenCalledExactlyOnceWith([ - '/srv/agent-session-attachments/u2/notes.md' + { id: 'chip-1', path: '/srv/agent-session-attachments/u1/report.pdf' }, + { id: 'chip-2', path: '/srv/agent-session-attachments/u2/notes.md' } ]) }) @@ -709,7 +710,7 @@ describe('useNativeChatExternalAttachments', () => { expect(chips.drop).toHaveBeenCalledWith('chip-1') expect(chips.attachReferences).toHaveBeenCalledExactlyOnceWith([ - '/srv/agent-session-attachments/u2/notes.md' + { id: 'chip-2', path: '/srv/agent-session-attachments/u2/notes.md' } ]) // The cause, the size limit here, rides in the same notice. expect(notices).toEqual([ diff --git a/src/renderer/src/components/native-chat/use-native-chat-menu-paste-lifetime.test.tsx b/src/renderer/src/components/native-chat/use-native-chat-menu-paste-lifetime.test.tsx new file mode 100644 index 00000000000..cbfed516c27 --- /dev/null +++ b/src/renderer/src/components/native-chat/use-native-chat-menu-paste-lifetime.test.tsx @@ -0,0 +1,273 @@ +// @vitest-environment happy-dom +import { createRef } from 'react' +import { act, cleanup, renderHook } from '@testing-library/react' +import { afterEach, beforeEach, expect, it, vi } from 'vitest' +import { useNativeChatComposerPaste } from './use-native-chat-composer-paste' +import { useNativeChatComposerAttachments } from './use-native-chat-composer-attachments' +import type { NativeChatComposerInput } from './native-chat-composer-input' +import type { NativeChatAttachmentOwner } from './native-chat-attachment-upload' +import { nativeChatImageSendBlock } from './native-chat-image-reattach' +import { + clearNativeChatComposerDraftsForTests, + hydrateNativeChatComposerDrafts, + nativeChatComposerDraftWritesSettled, + readNativeChatComposerDraft, + setNativeChatComposerDraftOwnerResolver, + updateNativeChatComposerDraft, + structuredAgentSessionDraftScopeKey as scope +} from './native-chat-composer-draft-store' +import { + createMemoryNativeChatComposerDraftStorage, + setNativeChatComposerDraftStorageForTests +} from './native-chat-composer-draft-storage' +import { + clearNativeChatPendingAttachmentsForTests, + dropNativeChatPendingAttachmentsOwnedBy, + nativeChatPendingAttachmentSnapshot +} from './native-chat-pending-attachment-cache' + +let previousApi: typeof window.api +const draftOwner = { workspaceId: 'folder-1', executionHostId: 'local' as const } +beforeEach(async () => { + previousApi = window.api + setNativeChatComposerDraftStorageForTests(createMemoryNativeChatComposerDraftStorage()) + await hydrateNativeChatComposerDrafts() + setNativeChatComposerDraftOwnerResolver(() => draftOwner) +}) +afterEach(async () => { + vi.useRealTimers() + cleanup() + await nativeChatComposerDraftWritesSettled() + clearNativeChatPendingAttachmentsForTests() + clearNativeChatComposerDraftsForTests() + window.api = previousApi +}) + +async function fixture( + kind: 'local' | 'ssh', + acceptsImages = true, + source: 'menu' | 'event' = 'menu' +) { + const save = Promise.withResolvers() + const thumbnail = Promise.withResolvers<{ + dataUrl: string + width: number + height: number + } | null>() + const owner: NativeChatAttachmentOwner = + kind === 'ssh' + ? { + kind: 'ssh', + connectionId: 'ssh-1', + worktreePath: '/remote/folder', + expectedExecutionHostId: 'ssh:ssh-1', + expectedSshTargetId: 'ssh-1', + expectedSshConnectionGeneration: 1 + } + : { kind: 'local' } + const path = + kind === 'ssh' + ? '/tmp/orca-paste-1-image.png' + : '/local/native-chat-pastes/orca-paste-1-image.png' + const saveCall = vi.fn(() => save.promise) + Object.defineProperty(window, 'api', { + configurable: true, + writable: true, + value: { + ui: { + readClipboardText: async () => '', + readClipboardFilePaths: async () => [], + clipboardHasImage: async () => true, + readClipboardImageThumbnail: () => thumbnail.promise, + saveClipboardImageAsTempFile: saveCall + } + } + }) + const input = createRef() + const view = renderHook(() => { + const attachments = useNativeChatComposerAttachments({ + attachmentScopeKey: scope('menu'), + allowWithoutTarget: true, + acceptsImages, + caret: 0, + disabled: false, + isComposing: () => false, + resolveTarget: () => null, + textareaRef: input, + setCaret: () => {}, + setDraft: (update) => + updateNativeChatComposerDraft( + scope('menu'), + { + text: update(readNativeChatComposerDraft(scope('menu')).text) + }, + 'immediate' + ), + setNotice: () => {} + }) + const paste = useNativeChatComposerPaste({ + targetKey: 'menu', + attachmentScopeKey: scope('menu'), + agent: acceptsImages ? 'claude' : 'omp', + disabled: false, + caret: 0, + resolveAttachmentOwner: () => owner, + ...attachments, + insertTypedText: () => true, + setCaret: () => {}, + setNotice: () => {} + }) + return { paste, attachments } + }) + await act(async () => { + if (source === 'menu') { + view.result.current.paste.pasteFromClipboard() + } else { + const data = new DataTransfer() + data.items.add(new File(['png'], 'image.png', { type: 'image/png' })) + view.result.current.paste.handlePaste(new ClipboardEvent('paste', { clipboardData: data })) + } + }) + await vi.waitFor(() => + expect(saveCall).toHaveBeenCalledExactlyOnceWith( + kind === 'ssh' ? { connectionId: 'ssh-1' } : { forNativeChatDraft: true } + ) + ) + return { view, save, thumbnail, path } +} + +const referencePastes = [ + { kind: 'local', source: 'menu' }, + { kind: 'ssh', source: 'menu' }, + { kind: 'local', source: 'event' }, + { kind: 'ssh', source: 'event' } +] as const + +it.each( + referencePastes.flatMap((paste) => + [true, false].map((acceptsImages) => ({ ...paste, acceptsImages })) + ) +)( + 'can abandon stalled $kind $source operations after reopen with acceptsImages=$acceptsImages', + async ({ kind, source, acceptsImages }) => { + vi.useFakeTimers({ toFake: ['setTimeout', 'clearTimeout', 'Date'] }) + const { view, save, thumbnail, path } = await fixture(kind, acceptsImages, source) + view.unmount() + const reopened = renderHook(() => + useNativeChatComposerAttachments({ + attachmentScopeKey: scope('menu'), + acceptsImages, + allowWithoutTarget: true, + caret: 0, + disabled: false, + isComposing: () => false, + resolveTarget: () => null, + textareaRef: createRef(), + setCaret: () => {}, + setDraft: () => {}, + setNotice: () => {} + }) + ) + await act(async () => vi.advanceTimersByTimeAsync(24 * 60 * 60 * 1000)) + expect(reopened.result.current.imageAttachments).toHaveLength(1) + expect(nativeChatImageSendBlock(reopened.result.current.imageAttachments).holdsSend).toBe(true) + const id = reopened.result.current.imageAttachments[0].id + await act(async () => reopened.result.current.removeImageAttachment(id)) + expect(nativeChatImageSendBlock(reopened.result.current.imageAttachments).holdsSend).toBe(false) + await act(async () => save.resolve(path)) + await act(async () => + thumbnail.resolve({ dataUrl: 'data:image/png;base64,AA', width: 1, height: 1 }) + ) + expect(nativeChatPendingAttachmentSnapshot(scope('menu'))).toEqual([]) + expect(readNativeChatComposerDraft(scope('menu')).images).toEqual([]) + expect(readNativeChatComposerDraft(scope('menu')).text).toBe('') + } +) +it.each(referencePastes)( + 'keeps a $kind $source paste as a reference after close', + async ({ kind, source }) => { + const { view, save, thumbnail, path } = await fixture(kind, false, source) + await act(async () => + thumbnail.resolve({ dataUrl: 'data:image/png;base64,AA', width: 1, height: 1 }) + ) + expect(view.result.current.attachments.imageAttachments).toHaveLength(1) + view.unmount() + await act(async () => save.resolve(path)) + expect(nativeChatPendingAttachmentSnapshot(scope('menu'))).toEqual([]) + expect(readNativeChatComposerDraft(scope('menu')).text).toContain(path) + expect(readNativeChatComposerDraft(scope('menu')).images).toEqual([]) + } +) +it.each(referencePastes)( + 'keeps mounted $kind $source paste references without image chips', + async ({ kind, source }) => { + const { save, thumbnail, path } = await fixture(kind, false, source) + await act(async () => thumbnail.resolve(null)) + await act(async () => save.resolve(path)) + expect(readNativeChatComposerDraft(scope('menu')).text).toContain(path) + expect(readNativeChatComposerDraft(scope('menu')).images).toEqual([]) + } +) + +it.each(['local', 'ssh'] as const)( + 'preserves menu %s uploads closed before a delayed or absent thumbnail', + async (kind) => { + let completed = 0 + for (const preview of ['delayed', 'missing', 'present'] as const) { + const { view, save, thumbnail, path } = await fixture(kind) + if (preview !== 'delayed') { + await act(async () => + thumbnail.resolve( + preview === 'present' + ? { dataUrl: 'data:image/png;base64,AA', width: 1, height: 1 } + : null + ) + ) + } + view.unmount() + await act(async () => save.resolve(path)) + expect(nativeChatPendingAttachmentSnapshot(scope('menu'))).toEqual([]) + expect(readNativeChatComposerDraft(scope('menu')).images).toHaveLength(++completed) + expect(readNativeChatComposerDraft(scope('menu')).images.at(-1)).toMatchObject({ + path, + ...(kind === 'ssh' ? { connectionId: 'ssh-1' } : {}) + }) + // The save also settles when a thumbnail has not answered at all. + if (preview === 'delayed') { + await act(async () => thumbnail.resolve(null)) + } + } + } +) + +it.each(['local', 'ssh'] as const)( + 'settles menu %s images without a thumbnail while mounted', + async (kind) => { + const { save, thumbnail, path } = await fixture(kind) + await act(async () => thumbnail.resolve(null)) + await act(async () => save.resolve(path)) + expect(readNativeChatComposerDraft(scope('menu')).images[0]).toMatchObject({ path }) + } +) + +it.each([true, false])( + 'ends pending operations for acceptsImages=%s on removal, deletion or failure', + async (acceptsImages) => { + for (const end of ['remove', 'workspace', 'failure'] as const) { + const { view, save, thumbnail, path } = await fixture('local', acceptsImages) + if (end === 'remove') { + await act(async () => view.result.current.attachments.clearImageAttachments()) + } else if (end === 'workspace') { + dropNativeChatPendingAttachmentsOwnedBy(draftOwner) + } + view.unmount() + await act(async () => save.resolve(end === 'failure' ? null : path)) + await act(async () => + thumbnail.resolve({ dataUrl: 'data:image/png;base64,AA', width: 1, height: 1 }) + ) + expect(nativeChatPendingAttachmentSnapshot(scope('menu'))).toEqual([]) + expect(readNativeChatComposerDraft(scope('menu')).images).toEqual([]) + expect(readNativeChatComposerDraft(scope('menu')).text).toBe('') + } + } +) diff --git a/src/renderer/src/components/native-chat/use-native-chat-paste-lifetime.test.tsx b/src/renderer/src/components/native-chat/use-native-chat-paste-lifetime.test.tsx new file mode 100644 index 00000000000..59a8d22d0f9 --- /dev/null +++ b/src/renderer/src/components/native-chat/use-native-chat-paste-lifetime.test.tsx @@ -0,0 +1,141 @@ +// @vitest-environment happy-dom +import { cleanup, renderHook } from '@testing-library/react' +import { afterEach, beforeEach, expect, it } from 'vitest' +import { useNativeChatPasteLifetime } from './use-native-chat-paste-lifetime' +import { + clearNativeChatComposerDraftsForTests, + hydrateNativeChatComposerDrafts, + nativeChatComposerDraftWritesSettled, + readNativeChatComposerDraft, + setNativeChatComposerDraftOwnerResolver, + structuredAgentSessionDraftScopeKey as scope +} from './native-chat-composer-draft-store' +import { + type NativeChatComposerDraftOwner, + createMemoryNativeChatComposerDraftStorage, + setNativeChatComposerDraftStorageForTests +} from './native-chat-composer-draft-storage' +import { + addNativeChatPendingAttachment, + clearNativeChatPendingAttachmentsForTests, + dropNativeChatPendingAttachmentsOwnedBy, + nativeChatPendingAttachmentSnapshot, + settleNativeChatPendingAttachment, + takeNativeChatPendingAttachment +} from './native-chat-pending-attachment-cache' +import type { NativeChatAttachmentOwner } from './native-chat-attachment-upload' + +const WORKSPACE: NativeChatComposerDraftOwner = { + workspaceId: 'workspace-1', + executionHostId: 'local' +} +const owners: NativeChatAttachmentOwner[] = [ + { kind: 'local' }, + { + kind: 'ssh', + connectionId: 'ssh-1', + worktreePath: '/remote/worktree', + expectedExecutionHostId: 'ssh:ssh-1', + expectedSshTargetId: 'ssh-1', + expectedSshConnectionGeneration: 1 + }, + { kind: 'runtime-session', environmentId: 'paired', sessionId: 'a', pairingRevision: 1 } +] + +beforeEach(async () => { + setNativeChatComposerDraftStorageForTests(createMemoryNativeChatComposerDraftStorage()) + await hydrateNativeChatComposerDrafts() + setNativeChatComposerDraftOwnerResolver(() => WORKSPACE) +}) +afterEach(async () => { + cleanup() + await nativeChatComposerDraftWritesSettled() + clearNativeChatPendingAttachmentsForTests() + clearNativeChatComposerDraftsForTests() +}) + +function fixture(owner: NativeChatAttachmentOwner) { + const view = renderHook( + ({ draftScope }) => + useNativeChatPasteLifetime({ + targetKey: draftScope, + attachmentScopeKey: draftScope, + beginPendingImageAttachment: () => 'upload', + resolvePendingImageAttachment: (id, path, connectionId) => { + settleNativeChatPendingAttachment(draftScope, id, path, connectionId) + }, + dropPendingImageAttachment: (id) => { + takeNativeChatPendingAttachment(draftScope, id) + } + }), + { initialProps: { draftScope: scope('a') } } + ) + addNativeChatPendingAttachment(scope('a'), { id: 'upload', path: '', pending: true }) + view.result.current.track('upload', '', owner) + return { view, lifetime: view.result.current } +} + +it.each(owners)('keeps a $kind upload when its composer closes', (owner) => { + const { view, lifetime } = fixture(owner) + view.unmount() + expect(nativeChatPendingAttachmentSnapshot(scope('a'))).toHaveLength(1) + expect( + lifetime.keepStoreUploadAfterUnmount('upload', { + status: 'saved', + tempPath: '/stored/image.png' + }) + ).toBe(true) + expect(nativeChatPendingAttachmentSnapshot(scope('a'))).toEqual([]) + expect(readNativeChatComposerDraft(scope('a')).images).toEqual([ + { + id: 'upload', + path: '/stored/image.png', + ...(owner.kind === 'ssh' ? { connectionId: 'ssh-1' } : {}) + } + ]) + expect(readNativeChatComposerDraft(scope('b')).images).toEqual([]) +}) + +it.each(owners)('keeps a $kind upload with its original scope after a target switch', (owner) => { + const { view, lifetime } = fixture(owner) + view.rerender({ draftScope: scope('b') }) + expect( + lifetime.keepStoreUploadAfterUnmount('upload', { + status: 'saved', + tempPath: '/stored/image.png' + }) + ).toBe(true) + expect(readNativeChatComposerDraft(scope('a')).images).toHaveLength(1) + expect(readNativeChatComposerDraft(scope('b')).images).toEqual([]) +}) + +it.each(owners)('does not settle a removed $kind upload', (owner) => { + const { view, lifetime } = fixture(owner) + takeNativeChatPendingAttachment(scope('a'), 'upload') + view.unmount() + lifetime.keepStoreUploadAfterUnmount('upload', { status: 'saved', tempPath: '/stored/image.png' }) + expect(readNativeChatComposerDraft(scope('a')).images).toEqual([]) +}) + +it.each(owners)('drops a failed $kind upload after teardown', (owner) => { + const { view, lifetime } = fixture(owner) + view.unmount() + expect(lifetime.keepStoreUploadAfterUnmount('upload', { status: 'error' })).toBe(true) + expect(nativeChatPendingAttachmentSnapshot(scope('a'))).toEqual([]) + expect(readNativeChatComposerDraft(scope('a')).images).toEqual([]) +}) + +it.each(owners)( + 'does not recreate a draft after its workspace is removed during a $kind upload', + (owner) => { + const { view, lifetime } = fixture(owner) + view.unmount() + dropNativeChatPendingAttachmentsOwnedBy(WORKSPACE) + lifetime.keepStoreUploadAfterUnmount('upload', { + status: 'saved', + tempPath: '/stored/image.png' + }) + expect(nativeChatPendingAttachmentSnapshot(scope('a'))).toEqual([]) + expect(readNativeChatComposerDraft(scope('a')).images).toEqual([]) + } +) diff --git a/src/renderer/src/components/native-chat/use-native-chat-paste-lifetime.ts b/src/renderer/src/components/native-chat/use-native-chat-paste-lifetime.ts index f801303dab1..41ee0f68438 100644 --- a/src/renderer/src/components/native-chat/use-native-chat-paste-lifetime.ts +++ b/src/renderer/src/components/native-chat/use-native-chat-paste-lifetime.ts @@ -1,19 +1,14 @@ import { useCallback, useLayoutEffect, useMemo, useRef } from 'react' import type { NativeChatAttachmentOwner } from './native-chat-attachment-upload' +import { nativeChatPendingAttachmentSnapshot } from './native-chat-pending-attachment-cache' /** - * The pending chips of pastes this composer started, released when it unmounts or changes target. - * A paste uploading into a paired server's store outlives the composer, which a prompt card - * unmounts: its chip stays pending in the scope's attachment cache, where its result settles for - * the composer's return (`native-chat-pending-attachment-cache.ts`). + * Preview URLs belong to this composer; pending uploads belong to the draft's attachment cache. */ export function useNativeChatPasteLifetime(args: { targetKey?: string - beginPendingImageAttachment: ( - previewUrl?: string, - pendingName?: string, - options?: { hidden?: true } - ) => string | null + attachmentScopeKey?: string + beginPendingImageAttachment: (previewUrl?: string, pendingName?: string) => string | null /** Files the result into the composer's scope even once this instance is unmounted. */ resolvePendingImageAttachment: (id: string, path: string, connectionId?: string | null) => void revealPendingImageAttachment?: (id: string, previewUrl?: string) => void @@ -21,19 +16,23 @@ export function useNativeChatPasteLifetime(args: { }): { lifetime: { active: boolean; pending: Map } track: (pendingId: string, preview: string, owner: NativeChatAttachmentOwner) => void - /** A paste's chip, shown at once with the clipboard's own thumbnail, or held out of sight - * (`hidden`) until `reveal`; `id` is null when the composer refused it. */ + /** Register the visible operation; `reveal` supplies its preview once the host accepts it. */ startImageChip: ( owner: NativeChatAttachmentOwner, imageFile: Blob, - options: { hidden: boolean; canShow: () => boolean } + options: { deferPreview: boolean; canShow: () => boolean } ) => { id: string | null; reveal: () => void } keepStoreUploadAfterUnmount: ( pendingId: string | null, saved: { status: string; tempPath?: string } ) => boolean } { - const { targetKey, beginPendingImageAttachment, revealPendingImageAttachment } = args + const { + targetKey, + attachmentScopeKey, + beginPendingImageAttachment, + revealPendingImageAttachment + } = args const { resolvePendingImageAttachment, dropPendingImageAttachment } = args const dropPendingRef = useRef(dropPendingImageAttachment) useLayoutEffect(() => { @@ -42,11 +41,12 @@ export function useNativeChatPasteLifetime(args: { const lifetime = useMemo( () => ({ targetKey, + attachmentScopeKey, active: false, pending: new Map(), - storeUploads: new Set() + uploads: new Map() }), - [targetKey] + [attachmentScopeKey, targetKey] ) useLayoutEffect(() => { lifetime.active = true @@ -56,19 +56,23 @@ export function useNativeChatPasteLifetime(args: { if (preview.startsWith('blob:')) { URL.revokeObjectURL(preview) } - if (!lifetime.storeUploads.has(id)) { + if ( + lifetime.uploads.get(id)?.kind !== 'runtime-session' && + !( + attachmentScopeKey && + nativeChatPendingAttachmentSnapshot(attachmentScopeKey).some((chip) => chip.id === id) + ) + ) { dropPendingRef.current(id) } } lifetime.pending.clear() } - }, [lifetime]) + }, [attachmentScopeKey, lifetime]) const track = useCallback( (pendingId: string, preview: string, owner: NativeChatAttachmentOwner) => { lifetime.pending.set(pendingId, preview) - if (owner.kind === 'runtime-session') { - lifetime.storeUploads.add(pendingId) - } + lifetime.uploads.set(pendingId, owner) }, [lifetime] ) @@ -76,16 +80,22 @@ export function useNativeChatPasteLifetime(args: { ( owner: NativeChatAttachmentOwner, imageFile: Blob, - options: { hidden: boolean; canShow: () => boolean } + options: { deferPreview: boolean; canShow: () => boolean } ): { id: string | null; reveal: () => void } => { - if (options.hidden) { - const id = beginPendingImageAttachment(undefined, undefined, { hidden: true }) + if (options.deferPreview) { + const id = beginPendingImageAttachment() if (id) { track(id, '', owner) } // Through the scope's cache, so it shows even in a composer that came back since. const reveal = (): void => { - if (!id) { + if ( + !id || + (attachmentScopeKey && + !nativeChatPendingAttachmentSnapshot(attachmentScopeKey).some( + (chip) => chip.id === id + )) + ) { return } const previewUrl = lifetime.active ? URL.createObjectURL(imageFile) : undefined @@ -108,21 +118,42 @@ export function useNativeChatPasteLifetime(args: { } return { id, reveal: () => {} } }, - [beginPendingImageAttachment, lifetime, revealPendingImageAttachment, track] + [attachmentScopeKey, beginPendingImageAttachment, lifetime, revealPendingImageAttachment, track] ) const keepStoreUploadAfterUnmount = useCallback( (pendingId: string | null, saved: { status: string; tempPath?: string }): boolean => { - if (!pendingId || lifetime.active || !lifetime.storeUploads.has(pendingId)) { + if (!pendingId || lifetime.active) { + if (pendingId) { + lifetime.uploads.delete(pendingId) + } return false } + const owner = lifetime.uploads.get(pendingId) + if ( + owner?.kind !== 'runtime-session' && + !( + attachmentScopeKey && + nativeChatPendingAttachmentSnapshot(attachmentScopeKey).some( + (chip) => chip.id === pendingId + ) + ) + ) { + lifetime.uploads.delete(pendingId) + return false + } + lifetime.uploads.delete(pendingId) if (saved.status === 'saved' && saved.tempPath) { - resolvePendingImageAttachment(pendingId, saved.tempPath, null) + resolvePendingImageAttachment( + pendingId, + saved.tempPath, + owner?.kind === 'ssh' ? owner.connectionId : null + ) } else { dropPendingImageAttachment(pendingId) } return true }, - [dropPendingImageAttachment, lifetime, resolvePendingImageAttachment] + [attachmentScopeKey, dropPendingImageAttachment, lifetime, resolvePendingImageAttachment] ) return { lifetime, track, startImageChip, keepStoreUploadAfterUnmount } }