fix(desktop): preserve image preview ownership across drafts
This commit is contained in:
committed by
Teknium
parent
6dfe63e1e2
commit
7d5e9757ec
@@ -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<typeof useComposerActions>
|
||||
|
||||
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(<PreviewHarness activeQueueSessionKey="session-A" />)
|
||||
let pending!: Promise<boolean>
|
||||
|
||||
act(() => {
|
||||
pending = actions.attachImagePath('/tmp/round-trip.png')
|
||||
})
|
||||
|
||||
await waitFor(() => expect(createImageBitmap).toHaveBeenCalledOnce())
|
||||
|
||||
act(() => rerender(<PreviewHarness activeQueueSessionKey="session-B" />))
|
||||
act(() => rerender(<PreviewHarness activeQueueSessionKey="session-A" />))
|
||||
|
||||
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', () => {
|
||||
|
||||
@@ -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 }
|
||||
|
||||
@@ -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', () => {
|
||||
|
||||
@@ -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<Record<string, string>>({})
|
||||
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<ComposerAttach
|
||||
},
|
||||
updateIfCurrent(expected, attachment) {
|
||||
const current = $attachments.get()
|
||||
const index = current.findIndex(item => 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
|
||||
|
||||
Reference in New Issue
Block a user