diff --git a/apps/desktop/src/app/chat/composer/hooks/use-composer-draft.test.tsx b/apps/desktop/src/app/chat/composer/hooks/use-composer-draft.test.tsx index 7707c8ae75..92d89ea947 100644 --- a/apps/desktop/src/app/chat/composer/hooks/use-composer-draft.test.tsx +++ b/apps/desktop/src/app/chat/composer/hooks/use-composer-draft.test.tsx @@ -1,9 +1,11 @@ -import { act, cleanup, render } from '@testing-library/react' +import { act, cleanup, render, waitFor } from '@testing-library/react' import { useLayoutEffect } from 'react' import { afterEach, describe, expect, it, vi } from 'vitest' -import { type ComposerAttachment, mainComposerScope, stashSessionDraft } from '@/store/composer' +import { clearSessionDraft, type ComposerAttachment, mainComposerScope, stashSessionDraft } from '@/store/composer' +import { $connection } from '@/store/session' +import { useComposerActions } from '../../hooks/use-composer-actions' import type { QueueEditState } from '../composer-utils' import { type ComposerTarget, getActiveComposer, markActiveComposer } from '../focus' import { type ComposerScope, ComposerScopeProvider, MAIN_COMPOSER_SCOPE } from '../scope' @@ -52,6 +54,11 @@ describe('useComposerDraft — attachment scope stays coherent with the committe afterEach(() => { cleanup() mainComposerScope.clear() + clearSessionDraft('session-A') + clearSessionDraft('session-B') + delete (window as unknown as { hermesDesktop?: unknown }).hermesDesktop + vi.unstubAllGlobals() + $connection.set(null) }) it('clears the outgoing session attachments by the layout phase right after switching sessions', () => { @@ -84,6 +91,81 @@ describe('useComposerDraft — attachment scope stays coherent with the committe // fired the instant B renders must never ship session A's attachment. expect(snapshots[0]).toEqual([]) }) + + it('applies a delayed image preview after its attachment survives an A → B → A draft round-trip', async () => { + const fullResolution = + 'data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAQAAAC1HAwCAAAAC0lEQVR42mP8/x8AAwMCAO+GkZcAAAAASUVORK5CYII=' + + const readFileDataUrl = vi.fn(async () => fullResolution) + + ;( + window as unknown as { + hermesDesktop: { readFileDataUrl: typeof readFileDataUrl } + } + ).hermesDesktop = { readFileDataUrl } + + let resolveBitmap!: (bitmap: { close: () => void; height: number; width: number }) => void + + vi.stubGlobal( + 'fetch', + vi.fn(async () => ({ blob: async () => new Blob([new Uint8Array([0])], { type: 'image/png' }) })) + ) + vi.stubGlobal( + 'createImageBitmap', + vi.fn( + () => + new Promise<{ close: () => void; height: number; width: number }>(resolve => { + resolveBitmap = resolve + }) + ) + ) + + class MockOffscreenCanvas { + getContext = () => ({ drawImage: vi.fn() }) + convertToBlob = vi.fn(async () => new Blob(['thumbnail'], { type: 'image/png' })) + constructor(_width: number, _height: number) {} + } + + vi.stubGlobal('OffscreenCanvas', MockOffscreenCanvas) + + let actions!: ReturnType + + function PreviewHarness({ activeQueueSessionKey }: { activeQueueSessionKey: string }) { + useComposerDraft({ + activeQueueSessionKey, + focusKey: null, + inputDisabled: false, + queueEditRef: { current: null as QueueEditState | null }, + sessionId: activeQueueSessionKey + }) + actions = useComposerActions({ activeSessionId: null, currentCwd: '', requestGateway: vi.fn() }) + + return null + } + + const { rerender } = render() + let pending!: Promise + + act(() => { + pending = actions.attachImagePath('/tmp/round-trip.png') + }) + + await waitFor(() => expect(createImageBitmap).toHaveBeenCalledOnce()) + + act(() => rerender()) + act(() => rerender()) + + expect(mainComposerScope.$attachments.get()).toHaveLength(1) + expect(mainComposerScope.$attachments.get()[0]?.thumbnailUrl).toBeUndefined() + + resolveBitmap({ close: vi.fn(), height: 3000, width: 4000 }) + + await act(async () => { + await pending + }) + + expect(mainComposerScope.$attachments.get()[0]?.thumbnailUrl).toMatch(/^data:image\/png;base64,/) + }) }) describe('useComposerDraft — rehydrate diagnostic log stays redacted', () => { diff --git a/apps/desktop/src/app/chat/hooks/use-composer-actions.ts b/apps/desktop/src/app/chat/hooks/use-composer-actions.ts index c5a3c80713..82c5c17ca0 100644 --- a/apps/desktop/src/app/chat/hooks/use-composer-actions.ts +++ b/apps/desktop/src/app/chat/hooks/use-composer-actions.ts @@ -12,6 +12,7 @@ import { normalize } from '@/lib/text' import { addComposerAttachment, type ComposerAttachment, + createComposerAttachmentOccurrenceId, mainComposerScope, removeComposerAttachment, setComposerTerminalSelection, @@ -470,6 +471,7 @@ export function useComposerActions({ const baseAttachment: ComposerAttachment = { id: attachmentId('image', filePath), + occurrenceId: createComposerAttachmentOccurrenceId(), kind: 'image', label: pathLabel(filePath), detail: filePath, @@ -485,10 +487,10 @@ export function useComposerActions({ // Keep only the bounded thumbnail in composer state. The full source // is read on demand for lightbox/download and separately at submit // for the model, so retaining 72 multi-MB data URLs serves no purpose. - // Bind the late preview to this exact attachment occurrence. The id is - // path-derived, so remove + reattach can create a replacement with the - // same id while this decode is still pending; updating by id alone would - // apply stale pixels to the replacement. + // Bind the late preview to this attachment occurrence's stable token. + // Object identity is insufficient because a session draft round-trip + // clones attachments; id alone is insufficient because remove + + // reattach of the same path reuses it. scope.updateIfCurrent( baseAttachment, thumbnailUrl ? { ...baseAttachment, thumbnailUrl } : { ...baseAttachment, previewUrl } diff --git a/apps/desktop/src/store/composer.test.ts b/apps/desktop/src/store/composer.test.ts index 37cf4b7793..3860bb4e37 100644 --- a/apps/desktop/src/store/composer.test.ts +++ b/apps/desktop/src/store/composer.test.ts @@ -6,6 +6,7 @@ import { addComposerAttachment, clearSessionDraft, type ComposerAttachment, + createComposerAttachmentOccurrenceId, createComposerAttachmentScope, migrateSessionDraft, removeComposerAttachment, @@ -65,8 +66,20 @@ describe('updateComposerAttachment', () => { it('updates only the exact attachment occurrence captured before an async operation', () => { const scope = createComposerAttachmentScope() - const first = attachment({ id: 'image:a', kind: 'image', path: '/tmp/a.png' }) - const replacement = attachment({ id: 'image:a', kind: 'image', path: '/tmp/a.png' }) + + const first = attachment({ + id: 'image:a', + kind: 'image', + occurrenceId: createComposerAttachmentOccurrenceId(), + path: '/tmp/a.png' + }) + + const replacement = attachment({ + id: 'image:a', + kind: 'image', + occurrenceId: createComposerAttachmentOccurrenceId(), + path: '/tmp/a.png' + }) scope.add(first) scope.remove(first.id) @@ -79,6 +92,26 @@ describe('updateComposerAttachment', () => { ) expect(scope.$attachments.get()[0]?.thumbnailUrl).toBe('data:image/png;base64,current') }) + + it('recognizes the same attachment occurrence after a session-draft clone', () => { + const scope = createComposerAttachmentScope() + + const original = attachment({ + id: 'image:draft', + kind: 'image', + occurrenceId: createComposerAttachmentOccurrenceId(), + path: '/tmp/draft.png' + }) + + stashSessionDraft('session-a', '', [original]) + const restored = takeSessionDraft('session-a').attachments[0]! + scope.add(restored) + + expect(restored).not.toBe(original) + expect(scope.updateIfCurrent(original, { ...original, thumbnailUrl: 'data:image/png;base64,current' })).toBe(true) + expect(scope.$attachments.get()[0]?.thumbnailUrl).toBe('data:image/png;base64,current') + clearSessionDraft('session-a') + }) }) describe('session drafts', () => { diff --git a/apps/desktop/src/store/composer.ts b/apps/desktop/src/store/composer.ts index b9cf116c14..3b9ea81240 100644 --- a/apps/desktop/src/store/composer.ts +++ b/apps/desktop/src/store/composer.ts @@ -5,6 +5,10 @@ import { triggerHaptic } from '@/lib/haptics' export interface ComposerAttachment { id: string + /** Renderer-lifetime identity for one attachment occurrence. Unlike `id`, + * which is content/path-derived, this survives draft cloning but changes + * when the user removes and re-adds the same attachment. */ + occurrenceId?: string kind: 'file' | 'folder' | 'image' | 'review' | 'terminal' | 'url' label: string detail?: string @@ -31,6 +35,7 @@ export const $composerTerminalSelections = atom>({}) export const $voiceConversationStartRequest = atom(0) let nextVoiceStartRequest = 0 let handledVoiceStartRequest = 0 +export const createComposerAttachmentOccurrenceId = (): string => crypto.randomUUID() export const requestVoiceConversationStart = (): void => $voiceConversationStartRequest.set(++nextVoiceStartRequest) @@ -112,7 +117,12 @@ export function createComposerAttachmentScope($attachments = atom item === expected) + + const index = current.findIndex(item => + expected.occurrenceId === undefined + ? item === expected + : item.id === expected.id && item.occurrenceId === expected.occurrenceId + ) if (index < 0) { return false