diff --git a/web/src/lib/attachmentAdapter.test.ts b/web/src/lib/attachmentAdapter.test.ts index df08060b..3d76b4e6 100644 --- a/web/src/lib/attachmentAdapter.test.ts +++ b/web/src/lib/attachmentAdapter.test.ts @@ -1,5 +1,67 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +function stubUploadThenPreviewReadFailure(): void { + let readCount = 0 + + class FileReaderMock { + result: string | ArrayBuffer | null = null + onload: FileReader['onload'] = null + onerror: FileReader['onerror'] = null + + readAsDataURL(): void { + readCount += 1 + if (readCount === 1) { + this.result = 'data:image/png;base64,dXBsb2Fk' + this.onload?.call(this as unknown as FileReader, {} as ProgressEvent) + return + } + this.onerror?.call(this as unknown as FileReader, {} as ProgressEvent) + } + } + + vi.stubGlobal('FileReader', FileReaderMock) +} + +function stubUploadThenDeferredPreviewReadFailure(): { + previewStarted: Promise + failPreview: () => void +} { + let readCount = 0 + let resolvePreviewStarted!: () => void + let failPreviewRead: (() => void) | undefined + const previewStarted = new Promise((resolve) => { + resolvePreviewStarted = resolve + }) + + class FileReaderMock { + result: string | ArrayBuffer | null = null + onload: FileReader['onload'] = null + onerror: FileReader['onerror'] = null + + readAsDataURL(): void { + readCount += 1 + if (readCount === 1) { + this.result = 'data:image/png;base64,dXBsb2Fk' + this.onload?.call(this as unknown as FileReader, {} as ProgressEvent) + return + } + failPreviewRead = () => { + this.onerror?.call(this as unknown as FileReader, {} as ProgressEvent) + } + resolvePreviewStarted() + } + } + + vi.stubGlobal('FileReader', FileReaderMock) + return { + previewStarted, + failPreview: () => { + if (!failPreviewRead) throw new Error('Preview read did not start') + failPreviewRead() + } + } +} + describe('attachmentAdapter', () => { beforeEach(() => { vi.stubGlobal('indexedDB', undefined) @@ -46,4 +108,78 @@ describe('attachmentAdapter', () => { status: { type: 'requires-action', reason: 'composer-send' }, })]) }) + + it('keeps a successful upload ready when image preview generation fails', async () => { + stubUploadThenPreviewReadFailure() + const { createAttachmentAdapter } = await import('./attachmentAdapter') + const uploadFile = vi.fn().mockResolvedValue({ success: true, path: '/uploads/proof.png' }) + const deleteUploadFile = vi.fn().mockResolvedValue({ success: true }) + const adapter = createAttachmentAdapter({ uploadFile, deleteUploadFile } as never, 'session-1') + const file = new File(['proof'], 'proof.png', { type: 'image/png' }) + const states: import('@assistant-ui/react').PendingAttachment[] = [] + + for await (const state of adapter.add({ file }) as AsyncGenerator) { + states.push(state) + } + + const ready = states.at(-1) as import('@assistant-ui/react').PendingAttachment & { + path?: string + previewUrl?: string + } + expect(uploadFile).toHaveBeenCalledTimes(1) + expect(uploadFile).toHaveBeenCalledWith('session-1', 'proof.png', 'dXBsb2Fk', 'image/png') + expect(ready).toMatchObject({ + type: 'file', + name: 'proof.png', + status: { type: 'requires-action', reason: 'composer-send' }, + path: '/uploads/proof.png', + }) + expect(ready.id).toEqual(expect.any(String)) + expect(ready.previewUrl).toBeUndefined() + + const sent = await adapter.send(ready) + expect(JSON.parse((sent.content[0] as { text: string }).text)).toEqual({ + __attachmentMetadata: { + id: ready.id, + filename: 'proof.png', + mimeType: 'image/png', + size: file.size, + path: '/uploads/proof.png', + }, + }) + + await adapter.remove(ready) + expect(deleteUploadFile).toHaveBeenCalledWith('session-1', '/uploads/proof.png') + }) + + it('cleans up a successful upload when cancellation occurs during preview generation', async () => { + const preview = stubUploadThenDeferredPreviewReadFailure() + const { createAttachmentAdapter } = await import('./attachmentAdapter') + const uploadFile = vi.fn().mockResolvedValue({ success: true, path: '/uploads/proof.png' }) + const deleteUploadFile = vi.fn().mockResolvedValue({ success: true }) + const adapter = createAttachmentAdapter({ uploadFile, deleteUploadFile } as never, 'session-1') + const file = new File(['proof'], 'proof.png', { type: 'image/png' }) + const iter = adapter.add({ file }) as AsyncGenerator + + const initial = await iter.next() + const uploading = await iter.next() + expect(uploading.value).toMatchObject({ + status: { type: 'running', reason: 'uploading', progress: 50 }, + }) + expect((uploading.value as { path?: string }).path).toBeUndefined() + + const completion = iter.next() + await preview.previewStarted + expect(uploadFile).toHaveBeenCalledTimes(1) + await adapter.remove(uploading.value) + preview.failPreview() + + expect(await completion).toEqual({ done: true, value: undefined }) + expect([initial.value, uploading.value]).toEqual([ + expect.objectContaining({ status: { type: 'running', reason: 'uploading', progress: 0 } }), + expect.objectContaining({ status: { type: 'running', reason: 'uploading', progress: 50 } }), + ]) + expect(deleteUploadFile).toHaveBeenCalledTimes(1) + expect(deleteUploadFile).toHaveBeenCalledWith('session-1', '/uploads/proof.png') + }) }) diff --git a/web/src/lib/attachmentAdapter.ts b/web/src/lib/attachmentAdapter.ts index 06e099b9..9527db26 100644 --- a/web/src/lib/attachmentAdapter.ts +++ b/web/src/lib/attachmentAdapter.ts @@ -113,7 +113,16 @@ export function createAttachmentAdapter(api: ApiClient, sessionId: string): Atta // Generate preview URL for images under 5MB let previewUrl: string | undefined if (isImageMimeType(contentType) && file.size <= MAX_PREVIEW_BYTES) { - previewUrl = await fileToDataUrl(file) + try { + previewUrl = await fileToDataUrl(file) + } catch { + // Preview generation is optional after the upload has succeeded. + } + } + + if (cancelledAttachmentIds.has(id)) { + await deleteUpload(result.path) + return } yield { diff --git a/web/src/lib/scratchlistAttachmentAdapter.test.ts b/web/src/lib/scratchlistAttachmentAdapter.test.ts index a9899eae..89477866 100644 --- a/web/src/lib/scratchlistAttachmentAdapter.test.ts +++ b/web/src/lib/scratchlistAttachmentAdapter.test.ts @@ -1,9 +1,75 @@ -import { describe, expect, it, vi } from 'vitest' +import { afterEach, describe, expect, it, vi } from 'vitest' import { createScratchlistAttachmentAdapter, hubAttachmentFromRestoredDraft, } from './scratchlistAttachmentAdapter' +function stubUploadThenPreviewReadFailure(): void { + let readCount = 0 + + class FileReaderMock { + result: string | ArrayBuffer | null = null + onload: FileReader['onload'] = null + onerror: FileReader['onerror'] = null + + readAsDataURL(): void { + readCount += 1 + if (readCount === 1) { + this.result = 'data:image/png;base64,dXBsb2Fk' + this.onload?.call(this as unknown as FileReader, {} as ProgressEvent) + return + } + this.onerror?.call(this as unknown as FileReader, {} as ProgressEvent) + } + } + + vi.stubGlobal('FileReader', FileReaderMock) +} + +function stubUploadThenDeferredPreviewReadFailure(): { + previewStarted: Promise + failPreview: () => void +} { + let readCount = 0 + let resolvePreviewStarted!: () => void + let failPreviewRead: (() => void) | undefined + const previewStarted = new Promise((resolve) => { + resolvePreviewStarted = resolve + }) + + class FileReaderMock { + result: string | ArrayBuffer | null = null + onload: FileReader['onload'] = null + onerror: FileReader['onerror'] = null + + readAsDataURL(): void { + readCount += 1 + if (readCount === 1) { + this.result = 'data:image/png;base64,dXBsb2Fk' + this.onload?.call(this as unknown as FileReader, {} as ProgressEvent) + return + } + failPreviewRead = () => { + this.onerror?.call(this as unknown as FileReader, {} as ProgressEvent) + } + resolvePreviewStarted() + } + } + + vi.stubGlobal('FileReader', FileReaderMock) + return { + previewStarted, + failPreview: () => { + if (!failPreviewRead) throw new Error('Preview read did not start') + failPreviewRead() + } + } +} + +afterEach(() => { + vi.unstubAllGlobals() +}) + describe('hubAttachmentFromRestoredDraft', () => { it('reconstructs hub metadata from hapi-hub:scratchlist storage key', () => { const file = new File([new Uint8Array([1, 2, 3])], 'proof.png', { type: 'image/png' }) @@ -104,6 +170,95 @@ describe('createScratchlistAttachmentAdapter', () => { expect((ready as { path?: string }).path).toBe('/scratchlist/sessions/s1/proof.png') }) + it('keeps a successful upload ready when image preview generation fails', async () => { + stubUploadThenPreviewReadFailure() + const attachment = { + id: 'hub-proof', + filename: 'proof.png', + mimeType: 'image/png', + size: 5, + path: 'hapi-hub:scratchlist/default/session-1/hub-proof-proof.png', + } + const uploadScratchlistAttachment = vi.fn().mockResolvedValue({ success: true, attachment }) + const deleteScratchlistAttachment = vi.fn().mockResolvedValue(undefined) + const adapter = createScratchlistAttachmentAdapter( + { uploadScratchlistAttachment, deleteScratchlistAttachment } as never, + 'session-1' + ) + const file = new File(['proof'], 'proof.png', { type: 'image/png' }) + const states: import('@assistant-ui/react').PendingAttachment[] = [] + + for await (const state of adapter.add({ file }) as AsyncGenerator) { + states.push(state) + } + + const ready = states.at(-1) as import('@assistant-ui/react').PendingAttachment & { + path?: string + hubAttachment?: typeof attachment + previewUrl?: string + } + expect(uploadScratchlistAttachment).toHaveBeenCalledTimes(1) + expect(uploadScratchlistAttachment).toHaveBeenCalledWith('session-1', 'proof.png', 'dXBsb2Fk', 'image/png') + expect(ready).toMatchObject({ + type: 'file', + name: 'proof.png', + status: { type: 'requires-action', reason: 'composer-send' }, + path: attachment.path, + hubAttachment: attachment, + }) + expect(ready.id).toEqual(expect.any(String)) + expect(ready.previewUrl).toBeUndefined() + + const sent = await adapter.send(ready) + expect(JSON.parse((sent.content[0] as { text: string }).text)).toEqual({ + __attachmentMetadata: attachment, + }) + + await adapter.remove(ready) + expect(deleteScratchlistAttachment).toHaveBeenCalledWith('session-1', attachment.id) + }) + + it('cleans up a successful hub upload when cancellation occurs during preview generation', async () => { + const preview = stubUploadThenDeferredPreviewReadFailure() + const attachment = { + id: 'hub-proof', + filename: 'proof.png', + mimeType: 'image/png', + size: 5, + path: 'hapi-hub:scratchlist/default/session-1/hub-proof-proof.png', + } + const uploadScratchlistAttachment = vi.fn().mockResolvedValue({ success: true, attachment }) + const deleteScratchlistAttachment = vi.fn().mockResolvedValue(undefined) + const adapter = createScratchlistAttachmentAdapter( + { uploadScratchlistAttachment, deleteScratchlistAttachment } as never, + 'session-1' + ) + const file = new File(['proof'], 'proof.png', { type: 'image/png' }) + const iter = adapter.add({ file }) as AsyncGenerator + + const initial = await iter.next() + const uploading = await iter.next() + expect(uploading.value).toMatchObject({ + status: { type: 'running', reason: 'uploading', progress: 50 }, + }) + expect((uploading.value as { path?: string }).path).toBeUndefined() + expect((uploading.value as { hubAttachment?: unknown }).hubAttachment).toBeUndefined() + + const completion = iter.next() + await preview.previewStarted + expect(uploadScratchlistAttachment).toHaveBeenCalledTimes(1) + await adapter.remove(uploading.value) + preview.failPreview() + + expect(await completion).toEqual({ done: true, value: undefined }) + expect([initial.value, uploading.value]).toEqual([ + expect.objectContaining({ status: { type: 'running', reason: 'uploading', progress: 0 } }), + expect.objectContaining({ status: { type: 'running', reason: 'uploading', progress: 50 } }), + ]) + expect(deleteScratchlistAttachment).toHaveBeenCalledTimes(1) + expect(deleteScratchlistAttachment).toHaveBeenCalledWith('session-1', attachment.id) + }) + it('deletes hub blob when cancel races the in-flight upload completion', async () => { let pendingId = '' let adapter: ReturnType diff --git a/web/src/lib/scratchlistAttachmentAdapter.ts b/web/src/lib/scratchlistAttachmentAdapter.ts index 1fb1c9d4..cd40ff83 100644 --- a/web/src/lib/scratchlistAttachmentAdapter.ts +++ b/web/src/lib/scratchlistAttachmentAdapter.ts @@ -127,7 +127,16 @@ export function createScratchlistAttachmentAdapter(api: ApiClient, sessionId: st let previewUrl: string | undefined if (isImageMimeType(contentType) && file.size <= MAX_PREVIEW_BYTES) { - previewUrl = await fileToDataUrl(file) + try { + previewUrl = await fileToDataUrl(file) + } catch { + // Preview generation is optional after the upload has succeeded. + } + } + + if (cancelledAttachmentIds.has(id)) { + await api.deleteScratchlistAttachment(sessionId, result.attachment.id).catch(() => {}) + return } yield {