fix(desktop): preserve in-flight attachment replacements

This commit is contained in:
Jakub Wolniewicz
2026-08-13 18:56:53 +02:00
committed by Teknium
parent 462e7d8e6b
commit e2a2fba4aa
6 changed files with 161 additions and 5 deletions
@@ -250,7 +250,7 @@ export function useSessionTileActions({ runtimeId, scope, storedSessionId }: Ses
syncAttachmentsForSubmit,
updateSessionState: (sessionId, updater) => sessionTileDelegate()!.updateSession(sessionId, updater),
scope: {
clearAttachments: scope.attachments.clear,
removeAttachments: attachments => scope.attachments.removeOccurrences(attachments),
readAttachments: () => scope.attachments.$attachments.get(),
// Busy/messages flow through updateSession -> the tile's state slice;
// the primary view atoms must never see a tile turn.
@@ -206,4 +206,41 @@ describe('session tile attachment occurrence ownership', () => {
expect(scope.attachments.$attachments.get()).toEqual([replacement])
})
it('preserves a same-path replacement added while a successful tile submit is in flight', async () => {
const scope = createScope()
const original = makeAttachment()
const replacement = makeAttachment()
const attach = deferred<{ attached: boolean; path: string }>()
requestGateway.mockImplementation(async (method: string) => {
if (method === 'image.attach_bytes') {
return attach.promise
}
return {}
})
scope.attachments.add(original)
const { result } = renderHook(() =>
useSessionTileActions({ runtimeId: RUNTIME_ID, scope, storedSessionId: STORED_ID })
)
let submitted!: Promise<boolean>
act(() => {
submitted = result.current.submitText('describe this')
})
await waitFor(() => expect(requestGateway).toHaveBeenCalledWith('image.attach_bytes', expect.anything()))
scope.attachments.remove(original.id)
scope.attachments.add(replacement)
attach.resolve({ attached: true, path: STAGED_PATH })
await act(async () => {
await expect(submitted).resolves.toBe(true)
})
expect(scope.attachments.$attachments.get()).toEqual([replacement])
})
})
@@ -2517,6 +2517,7 @@ describe('usePromptActions restoreToMessage', () => {
describe('usePromptActions file attachment sync', () => {
afterEach(() => {
cleanup()
$composerAttachments.set([])
$connection.set(null)
$currentCwd.set('')
vi.restoreAllMocks()
@@ -2725,6 +2726,68 @@ describe('usePromptActions file attachment sync', () => {
})
})
it('preserves a same-path replacement added while a successful main submit is in flight', async () => {
$connection.set({ mode: 'local' } as never)
$currentCwd.set('/root')
const hostPath = 'C:\\Users\\alice\\Pictures\\photo.jpg'
const original: ComposerAttachment = {
detail: hostPath,
id: 'image:photo.jpg',
kind: 'image',
label: 'photo.jpg',
occurrenceId: 'occurrence-original',
path: hostPath
}
const replacement: ComposerAttachment = {
...original,
occurrenceId: 'occurrence-replacement'
}
let resolveAttach!: (value: { attached: boolean; path: string }) => void
const attachResult = new Promise<{ attached: boolean; path: string }>(resolve => {
resolveAttach = resolve
})
Object.defineProperty(window, 'hermesDesktop', {
configurable: true,
value: { readFileDataUrl: vi.fn(async () => 'data:image/jpeg;base64,aGVsbG8=') }
})
const requestGateway = vi.fn(async (method: string) => {
if (method === 'image.attach_bytes') {
return (await attachResult) as never
}
return {} as never
})
$composerAttachments.set([original])
let handle: HarnessHandle | null = null
await actRender(
<Harness onReady={h => (handle = h)} refreshSessions={async () => undefined} requestGateway={requestGateway} />
)
let submitted!: Promise<boolean>
act(() => {
submitted = handle!.submitTextRaw('describe this')
})
await waitFor(() => expect(requestGateway).toHaveBeenCalledWith('image.attach_bytes', expect.anything()))
$composerAttachments.set([replacement])
resolveAttach({ attached: true, path: '/root/tmp/photo.jpg' })
await act(async () => {
await expect(submitted).resolves.toBe(true)
})
expect($composerAttachments.get()).toEqual([replacement])
})
it('uploads file bytes when the terminal backend is a container (docker)', async () => {
// Container backends have their own filesystem: the host drop path would
// dangle inside the sandbox, so the bytes must cross via file.attach's
@@ -14,8 +14,8 @@ import {
} from '@/lib/voice-playback'
import {
$composerAttachments,
clearComposerAttachments,
type ComposerAttachment,
mainComposerScope,
terminalContextBlocksFromDraft
} from '@/store/composer'
import { $hudMode } from '@/store/hud'
@@ -77,7 +77,7 @@ interface SubmitPromptDeps {
* (defaults); a session tile injects its own so a tile submit never writes
* the primary view's $busy/$messages or clears the main attachment chips. */
scope?: {
clearAttachments: () => void
removeAttachments: (attachments: readonly ComposerAttachment[]) => void
readAttachments: () => ComposerAttachment[]
setAwaitingResponse: (awaiting: boolean) => void
setBusy: (busy: boolean) => void
@@ -88,7 +88,7 @@ interface SubmitPromptDeps {
// Stable identity — a fresh default object per render would churn the
// useCallback below on every render.
const MAIN_SUBMIT_SCOPE: NonNullable<SubmitPromptDeps['scope']> = {
clearAttachments: clearComposerAttachments,
removeAttachments: attachments => mainComposerScope.removeOccurrences(attachments),
readAttachments: () => $composerAttachments.get(),
setAwaitingResponse,
setBusy,
@@ -714,7 +714,10 @@ export function useSubmitPrompt(deps: SubmitPromptDeps) {
}
if (usingComposerAttachments) {
scope.clearAttachments()
// A submit owns only the occurrences captured before its first await.
// Preserve chips added or replaced while staging / prompt.submit was
// in flight instead of clearing the composer's newer draft state.
scope.removeAttachments(attachments)
}
// Submit landed — the turn now runs (busy stays true), but the submit
+38
View File
@@ -137,6 +137,44 @@ describe('updateComposerAttachment', () => {
thumbnailUrl: 'data:image/png;base64,current'
})
})
it('removes submitted occurrences while preserving unrelated attachments', () => {
const scope = createComposerAttachmentScope()
const submitted = attachment({
id: 'image:submitted',
kind: 'image',
occurrenceId: 'occurrence-submitted'
})
const other = attachment({ id: 'file:other', occurrenceId: 'occurrence-other' })
scope.add(submitted)
scope.add(other)
scope.removeOccurrences([submitted])
expect(scope.$attachments.get()).toEqual([other])
})
it('preserves a same-id replacement of a submitted occurrence', () => {
const scope = createComposerAttachmentScope()
const submitted = attachment({
id: 'image:submitted',
kind: 'image',
occurrenceId: 'occurrence-submitted'
})
const replacement = attachment({
...submitted,
occurrenceId: 'occurrence-replacement'
})
scope.add(replacement)
scope.removeOccurrences([submitted])
expect(scope.$attachments.get()).toEqual([replacement])
})
})
describe('session drafts', () => {
+15
View File
@@ -64,6 +64,7 @@ export interface ComposerAttachmentScope {
add(attachment: ComposerAttachment): void
clear(): void
remove(id: string): ComposerAttachment | null
removeOccurrences(attachments: readonly ComposerAttachment[]): void
setUploadState(id: string, uploadState?: ComposerAttachment['uploadState']): void
update(attachment: ComposerAttachment): boolean
updateIfCurrent(expected: ComposerAttachment, patch: ComposerAttachmentPatch): boolean
@@ -99,6 +100,20 @@ export function createComposerAttachmentScope($attachments = atom<ComposerAttach
return removed
},
removeOccurrences(attachments) {
const current = $attachments.get()
const submitted = new Set(attachments.map(attachment => `${attachment.id}\u0000${attachment.occurrenceId ?? ''}`))
const next = current.filter(
attachment => !submitted.has(`${attachment.id}\u0000${attachment.occurrenceId ?? ''}`)
)
// Preserve clear()'s notification semantics even when no captured
// occurrence remains. Some composer consumers settle local state on the
// successful-submit store emission.
$attachments.set(next)
},
setUploadState(id, uploadState) {
const current = $attachments.get()
const index = current.findIndex(attachment => attachment.id === id)