fix(web): migrate chat-path attachments on scratchlist park (#1227)

* fix(web): migrate chat-path attachments on scratchlist park (#1226)

When an image is attached via the normal upload adapter before scratchlist
mode is enabled, toggling mode swapped adapters and send() dropped the
metadata — park stored text-only and cleared chips. Migrate pending
chat-path files into hub scratchlist storage on send, and fail closed if
non-hub paths still reach the park wrapper.

Co-authored-by: Cursor <cursoragent@cursor.com>

* fix(web): defer chat-path cleanup until scratchlist park succeeds

send() migrates before scratchlist.add; deleting the original upload
there left retries pointing at a missing chat blob when park failed.
Stamp migratedFromPath and clean up only after the park attempt result.

Co-authored-by: Cursor <cursoragent@cursor.com>

* fix(web): park scratchlist before composer.send clears chips

assistant-ui empties text/attachments before onNew, so return false from
park could not restore retryable state and rejected cleanup deleted the
migrated hub blob. Intercept park from a live snapshot; clear only after
accept; releaseWithoutDelete so clearAttachments keeps parked hubs.

Co-authored-by: Cursor <cursoragent@cursor.com>

* fix(web): lock composer while scratchlist park is in flight

Disable input/send and hide chip remove during migrate+add so mid-flight
edits are not wiped on success and hub blobs are not deleted out from
under the accepted entry.

Co-authored-by: Cursor <cursoragent@cursor.com>

* fix(web): do not clear composer if park snapshot changed mid-flight

Compare post-await composer state to the pre-park snapshot before
clearing; disable DragDropZone and scratchlist promote while parking
so parent paths cannot add chips the clear would silently drop.

Addresses Codex Major on #1227 (preserve post-snapshot composer changes).

Co-authored-by: Cursor <cursoragent@cursor.com>

* fix(web): defer migrated chat-path cleanup until park snapshot clears

Return ScratchlistParkResult.beforeClear from onParkScratchlist so
finalizeMigratedScratchlistParkCleanup runs only after HappyComposer
confirms the composer was unchanged mid-flight.

Co-authored-by: Cursor <cursoragent@cursor.com>

* fix(web): flush rich mentions before scratchlist park snapshot

Park snapshots composer.text after flushSerializedText so session
@-mention chips serialize to markdown links before scratchlist add.

Co-authored-by: Cursor <cursoragent@cursor.com>

* fix(web): validate park snapshot before scratchlist add

Split prepare/commit/abort so mid-flight composer edits abort orphan
hub blobs instead of parking a duplicate. Reuse restored hub paths in
prepareScratchlistParkAttachments so remounted chips do not re-upload.

Co-authored-by: Cursor <cursoragent@cursor.com>

* fix(web): ignore scratchlist toggle hotkey while park is in flight

Co-authored-by: Cursor <cursoragent@cursor.com>

---------

Co-authored-by: Cursor <cursoragent@cursor.com>
This commit is contained in:
HeavyGee
2026-08-03 10:39:01 +08:00
committed by GitHub
co-authored by Cursor
parent c3a5522207
commit 2d904de76e
9 changed files with 941 additions and 51 deletions
@@ -1,5 +1,6 @@
import { AttachmentPrimitive, useThreadComposerAttachment } from '@assistant-ui/react'
import { Spinner } from '@/components/Spinner'
import { useComposerParking } from '@/components/AssistantChat/composerParkingContext'
function ErrorIcon() {
return (
@@ -32,6 +33,7 @@ function RemoveIcon() {
export function AttachmentItem() {
const { name, status } = useThreadComposerAttachment()
const isParking = useComposerParking()
const isUploading = status.type === 'running'
const isError = status.type === 'incomplete'
@@ -45,13 +47,15 @@ export function AttachmentItem() {
) : null}
<span className={`max-w-[150px] truncate ${isError ? 'text-red-500 line-through' : ''}`}>{name}</span>
{isError ? <span className="text-xs text-red-500 whitespace-nowrap">Upload failed</span> : null}
<AttachmentPrimitive.Remove
className="ml-auto flex h-5 w-5 items-center justify-center rounded text-[var(--app-hint)] transition-colors hover:text-[var(--app-fg)]"
aria-label="Remove attachment"
title="Remove attachment"
>
<RemoveIcon />
</AttachmentPrimitive.Remove>
{!isParking ? (
<AttachmentPrimitive.Remove
className="ml-auto flex h-5 w-5 items-center justify-center rounded text-[var(--app-hint)] transition-colors hover:text-[var(--app-fg)]"
aria-label="Remove attachment"
title="Remove attachment"
>
<RemoveIcon />
</AttachmentPrimitive.Remove>
) : null}
</AttachmentPrimitive.Root>
)
}
@@ -0,0 +1,34 @@
import { describe, expect, it } from 'vitest'
import { composerParkSnapshotUnchanged } from './HappyComposer'
describe('composerParkSnapshotUnchanged', () => {
it('returns true when text and attachment ids match', () => {
const snapshot = {
text: 'hello',
attachments: [{ id: 'a1' }, { id: 'a2' }],
}
expect(composerParkSnapshotUnchanged(snapshot, {
text: 'hello',
attachments: [{ id: 'a1' }, { id: 'a2' }],
})).toBe(true)
})
it('returns false when text or attachments changed during park', () => {
const snapshot = {
text: 'hello',
attachments: [{ id: 'a1' }],
}
expect(composerParkSnapshotUnchanged(snapshot, {
text: 'hello world',
attachments: [{ id: 'a1' }],
})).toBe(false)
expect(composerParkSnapshotUnchanged(snapshot, {
text: 'hello',
attachments: [{ id: 'a1' }, { id: 'a2' }],
})).toBe(false)
expect(composerParkSnapshotUnchanged(snapshot, {
text: 'hello',
attachments: [{ id: 'a2' }],
})).toBe(false)
})
})
@@ -38,6 +38,8 @@ import { StatusBar } from '@/components/AssistantChat/StatusBar'
import { ComposerButtons } from '@/components/AssistantChat/ComposerButtons'
import type { PendingSchedule } from '@/components/AssistantChat/ScheduleTimePicker'
import { AttachmentItem } from '@/components/AssistantChat/AttachmentItem'
import { ComposerParkingContext } from '@/components/AssistantChat/composerParkingContext'
import type { ScratchlistParkResult } from '@/lib/scratchlistAttachmentFlow'
import { useTranslation } from '@/lib/use-translation'
import { getModelOptionsForFlavor, getNextModelForFlavor } from './modelOptions'
import { getClaudeComposerEffortOptions } from './claudeEffortOptions'
@@ -143,6 +145,18 @@ export function useRichComposerBridge(
const defaultSuggestionHandler = async (): Promise<Suggestion[]> => []
/** True when composer text/attachment ids match a pre-park snapshot. */
export function composerParkSnapshotUnchanged(
snapshot: { text: string; attachments: readonly { id: string }[] },
current: { text: string; attachments: readonly { id: string }[] },
): boolean {
return current.text === snapshot.text
&& current.attachments.length === snapshot.attachments.length
&& current.attachments.every(
(attachment, index) => attachment.id === snapshot.attachments[index]?.id,
)
}
export function ModelEffortSettingsSection(props: {
agentFlavor?: string | null
options: Array<{ value: string; label: string }>
@@ -260,6 +274,16 @@ export function HappyComposer(props: {
scratchlistMode?: boolean
scratchlistCount?: number
onScratchlistToggle?: () => void
/**
* Prepare a scratchlist park (migrate only). Caller validates the
* composer snapshot, then commit()/abort()/beforeClear().
*/
onParkScratchlist?: (
text: string,
pending: readonly import('@assistant-ui/react').Attachment[],
) => Promise<ScratchlistParkResult>
/** Parent disables DragDropZone / scratchlist promote while park is in flight. */
onScratchlistParkingChange?: (parking: boolean) => void
// Set when the most recent send failed (4xx/5xx/network). The composer
// restores the original text once per `sendError.id` and renders an
// inline error affordance until the user dismisses or starts editing.
@@ -373,7 +397,15 @@ export function HappyComposer(props: {
}
}, [dictationActive, voiceInput.voiceMode, voiceStatus, onVoiceToggle, dictation.status, dictation.toggle])
const controlsDisabled = disabled || (!active && !allowSendWhenInactive) || threadIsDisabled
const [isParkingScratchlist, setIsParkingScratchlist] = useState(false)
const parkInFlightRef = useRef(false)
const onScratchlistParkingChange = props.onScratchlistParkingChange
useEffect(() => {
onScratchlistParkingChange?.(isParkingScratchlist)
}, [isParkingScratchlist, onScratchlistParkingChange])
const controlsDisabled = disabled || (!active && !allowSendWhenInactive) || threadIsDisabled || isParkingScratchlist
const trimmed = composerText.trim()
const hasText = trimmed.length > 0
const hasAttachments = attachments.length > 0
@@ -843,11 +875,49 @@ export function HappyComposer(props: {
? undefined
: handleUserClearSchedule
/** Flush rich chips → `[title](/sessions/<id>)` into composer.text, then send. */
const flushAndSend = useCallback(() => {
const handleSend = useCallback(async () => {
// Rich chips must be serialized into composer.text before any send or
// scratchlist park snapshot (RichComposerInput contract).
if (richMentionsEnabled && richInputRef.current) {
richInputRef.current.flushSerializedText()
}
// Scratchlist parks must not go through assistant-ui's send(): it
// empties text/chips before onNew, so a rejected add cannot restore
// retryable composer state (#1226 Major).
if (
props.scratchlistMode
&& pendingSchedule == null
&& props.onParkScratchlist
) {
if (!canSend || parkInFlightRef.current) return
parkInFlightRef.current = true
setIsParkingScratchlist(true)
try {
const snapshot = api.composer().getState()
const prepared = await props.onParkScratchlist(
snapshot.text,
snapshot.attachments,
)
if (!prepared) return
// Validate before irreversible add — otherwise a mid-flight
// composer edit leaves a parked duplicate while chips remain.
if (!composerParkSnapshotUnchanged(snapshot, api.composer().getState())) {
await prepared.abort()
return
}
if (!await prepared.commit()) {
return
}
await prepared.beforeClear()
api.composer().setText('')
await api.composer().clearAttachments()
} finally {
parkInFlightRef.current = false
setIsParkingScratchlist(false)
}
return
}
// A retry intentionally clears composer state synchronously. It is
// neither a replacement draft nor a dismissal: route onSuccess/onError
// owns the inline-error transition for this new attempt. Drop the old
@@ -860,7 +930,31 @@ export function HappyComposer(props: {
userAttachmentGeneration: userAttachmentGenerationRef.current,
}
api.composer().send()
}, [api, attachments, onSuppressSendErrorRestore, richMentionsEnabled, sendError])
// SessionChat owns clearing the schedule — it clears only after awaiting
// the send hook's accepted result, which covers both pre-mutation guards
// and async inactive-session resume failure. Clearing here unconditionally
// would race ahead of that check and drop the user's schedule on every
// rejected send path.
//
// The inline send-error affordance is intentionally NOT cleared here:
// the route-level state (`onSuccess`/`onError` in router.tsx) replaces
// or clears it based on the actual mutation result, so the user keeps
// the error context while the new attempt is in flight.
}, [
api,
attachments,
canSend,
onSuppressSendErrorRestore,
pendingSchedule,
props.onParkScratchlist,
props.scratchlistMode,
richMentionsEnabled,
sendError,
])
const flushAndSend = useCallback(() => {
void handleSend()
}, [handleSend])
const handleKeyDown = useCallback((e: ReactKeyboardEvent<HTMLTextAreaElement | HTMLDivElement>) => {
const key = e.key
@@ -956,7 +1050,7 @@ export function HappyComposer(props: {
permissionMode,
permissionModes,
canSend,
api,
handleSend,
haptic,
composerEnterBehavior,
richMentionsEnabled,
@@ -1122,20 +1216,6 @@ export function HappyComposer(props: {
const showAbortButton = true
const voiceEnabled = Boolean(effectiveVoiceToggle)
const handleSend = useCallback(() => {
flushAndSend()
// SessionChat owns clearing the schedule — it clears only after awaiting
// the send hook's accepted result, which covers both pre-mutation guards
// and async inactive-session resume failure. Clearing here unconditionally
// would race ahead of that check and drop the user's schedule on every
// rejected send path.
//
// The inline send-error affordance is intentionally NOT cleared here:
// the route-level state (`onSuccess`/`onError` in router.tsx) replaces
// or clears it based on the actual mutation result, so the user keeps
// the error context while the new attempt is in flight.
}, [flushAndSend])
// Pi: selected model info for UI labels and thinking level filtering
const piModelLabel = agentFlavor === 'pi'
? (selectedPiModel?.name ?? selectedPiModel?.modelId ?? 'Model')
@@ -1608,6 +1688,7 @@ export function HappyComposer(props: {
: 'max-h-[7.5rem] min-h-[1.5rem] flex-1 overflow-y-auto whitespace-pre-wrap break-words bg-transparent text-base leading-snug text-[var(--app-fg)] focus:outline-none'
return (
<ComposerParkingContext.Provider value={isParkingScratchlist}>
<div className={shellClassName} data-testid="composer-shell" data-expanded={isExpanded || undefined}>
<div className={innerClassName}>
<ComposerPrimitive.Root className={rootClassName} onSubmit={handleSubmit}>
@@ -1784,5 +1865,6 @@ export function HappyComposer(props: {
</ComposerPrimitive.Root>
</div>
</div>
</ComposerParkingContext.Provider>
)
}
@@ -0,0 +1,12 @@
import { createContext, useContext } from 'react'
/**
* True while HappyComposer is awaiting a scratchlist park. Attachment chips
* must not be removable mid-flight (would delete hub blobs the park still
* references).
*/
export const ComposerParkingContext = createContext(false)
export function useComposerParking(): boolean {
return useContext(ComposerParkingContext)
}
+116 -10
View File
@@ -45,10 +45,18 @@ import { ScratchlistMigrationBanner } from '@/components/AssistantChat/Scratchli
import { assignThreadMessageIds, useHappyRuntime } from '@/lib/assistant-runtime'
import type { OlderLoadOutcome } from '@/lib/message-window-store'
import { createAttachmentAdapter } from '@/lib/attachmentAdapter'
import { createScratchlistAttachmentAdapter } from '@/lib/scratchlistAttachmentAdapter'
import {
createScratchlistAttachmentAdapter,
type ScratchlistAttachmentAdapter,
} from '@/lib/scratchlistAttachmentAdapter'
import {
attachmentsNeedScratchlistMigration,
finalizeMigratedScratchlistParkCleanup,
prepareScratchlistParkAttachments,
rehydrateScratchlistAttachmentsToComposer,
stageScratchlistAttachmentsForComposeSend
stageScratchlistAttachmentsForComposeSend,
type PendingParkAttachment,
type ScratchlistParkResult,
} from '@/lib/scratchlistAttachmentFlow'
import type { ScratchlistEntry } from '@/lib/scratchlist'
import { isHubScratchlistAttachmentPath } from '@hapi/protocol'
@@ -203,6 +211,11 @@ export function isScratchlistHotkeyBlockedTarget(target: EventTarget | null): bo
* or to the regular chat send. Scratchlist entries support text and hub-
* stored attachments; scheduled sends still fall through to chat.
*
* Chat-path chips attached before scratchlist mode are migrated in the
* park-before-clear path (`prepareScratchlistParkAttachments`, #1226).
* This predicate still rejects non-hub paths as a fail-closed backstop
* for any leftover composer.send() route.
*
* Pure / exported so it can be unit tested without mounting SessionChat.
*/
export function shouldRouteToScratchlist(
@@ -548,14 +561,16 @@ function SessionChatInner(props: SessionChatProps) {
}
}, [allSessions, t])
const [scratchlistMode, setScratchlistMode] = useState(false)
const [isScratchlistParking, setIsScratchlistParking] = useState(false)
// Mode resets across sessions implicitly: SessionChat is keyed by
// session.id at the public-export boundary, so a session switch
// remounts SessionChatInner from scratch and `scratchlistMode`
// initializes to false again. (Previous effect-based reset was
// racy on first paint - see public-export comment for context.)
const handleScratchlistToggle = useCallback(() => {
if (isScratchlistParking) return
setScratchlistMode((m) => !m)
}, [])
}, [isScratchlistParking])
/**
* Global keyboard shortcut: Ctrl/Cmd + Shift + S toggles scratchlist
* mode (open/close drawer + flip composer routing).
@@ -580,6 +595,7 @@ function SessionChatInner(props: SessionChatProps) {
*/
useEffect(() => {
const onKeyDown = (e: globalThis.KeyboardEvent) => {
if (isScratchlistParking) return
if (!isScratchlistToggleHotkey(e)) return
if (isScratchlistHotkeyBlockedTarget(e.target)) return
e.preventDefault()
@@ -587,7 +603,7 @@ function SessionChatInner(props: SessionChatProps) {
}
window.addEventListener('keydown', onKeyDown)
return () => window.removeEventListener('keydown', onKeyDown)
}, [])
}, [isScratchlistParking])
/**
* onSend wrapper: when scratchlist mode is on AND the submission is
* not scheduled, route to scratchlist (text and/or hub attachments).
@@ -598,15 +614,98 @@ function SessionChatInner(props: SessionChatProps) {
* they can keep adding entries while sticky-mode is on. If add()
* returns false (empty after trim, at-cap), we resolve false so
* the composer keeps its text and the operator can fix it.
*
* Chat-path chips attached before scratchlist mode are migrated in
* the scratchlist attachment adapter's send() (#1226). If any
* non-hub path still reaches this wrapper, fail closed never park
* text-only and clear chips.
*/
// Stable handle so HappyComposer can release parked chips without
// deleting hub blobs when clearAttachments() calls adapter.remove().
const scratchlistAdapterRef = useRef<ScratchlistAttachmentAdapter | null>(null)
/**
* Park from a live composer snapshot *before* assistant-ui's
* `composer.send()` empties text/chips. Returning false leaves the
* composer intact for retry (at-cap / hub error).
*/
const onParkScratchlist = useCallback(
async (
text: string,
pending: readonly PendingParkAttachment[],
): Promise<ScratchlistParkResult> => {
let prepared
try {
prepared = await prepareScratchlistParkAttachments(
props.api,
props.session.id,
pending,
)
} catch {
return false
}
let aborted = false
const abort = async () => {
if (aborted) return
aborted = true
await finalizeMigratedScratchlistParkCleanup(
props.api,
props.session.id,
prepared,
false,
)
}
return {
abort,
commit: async () => {
const accepted = await scratchlist.add(text, prepared)
if (!accepted) {
await abort()
return false
}
return true
},
beforeClear: async () => {
await finalizeMigratedScratchlistParkCleanup(
props.api,
props.session.id,
prepared,
true,
)
scratchlistAdapterRef.current?.releaseWithoutDelete(
pending.map((chip) => chip.id),
)
},
}
},
[props.api, props.session.id, scratchlist],
)
const onSendForComposer = useCallback(
async (
text: string,
attachments?: AttachmentMetadata[],
scheduledAt?: number | null,
): Promise<boolean> => {
if (
scratchlistMode
&& scheduledAt == null
&& attachmentsNeedScratchlistMigration(attachments)
) {
return false
}
if (shouldRouteToScratchlist(scratchlistMode, attachments, scheduledAt)) {
return scratchlist.add(text, attachments)
// Legacy path if something still calls composer.send() while
// scratchlist mode is on. Prefer onParkScratchlist (clears
// only after accept).
const accepted = await scratchlist.add(text, attachments)
await finalizeMigratedScratchlistParkCleanup(
props.api,
props.session.id,
attachments,
accepted,
)
return accepted
}
// If the user uploaded while scratchlist mode was on, then toggled
// it off before send, pending items still carry hub paths. Stage
@@ -1334,11 +1433,16 @@ function SessionChatInner(props: SessionChatProps) {
const attachmentAdapter = useMemo(() => {
if (!props.session.active) {
scratchlistAdapterRef.current = null
return undefined
}
return scratchlistMode
? createScratchlistAttachmentAdapter(props.api, props.session.id)
: createAttachmentAdapter(props.api, props.session.id)
if (scratchlistMode) {
const adapter = createScratchlistAttachmentAdapter(props.api, props.session.id)
scratchlistAdapterRef.current = adapter
return adapter
}
scratchlistAdapterRef.current = null
return createAttachmentAdapter(props.api, props.session.id)
}, [props.api, props.session.id, props.session.active, scratchlistMode])
const runtime = useHappyRuntime({
@@ -1396,7 +1500,7 @@ function SessionChatInner(props: SessionChatProps) {
<AssistantRuntimeProvider runtime={runtime}>
<ShareSeedConsumer sessionId={props.session.id} sessionActive={props.session.active} />
<DragDropZone disabled={sessionInactive || props.isSending || pendingSchedule != null}>
<DragDropZone disabled={sessionInactive || props.isSending || pendingSchedule != null || isScratchlistParking}>
<HappyThread
// Key with prefix: different components under the same session
@@ -1472,7 +1576,7 @@ function SessionChatInner(props: SessionChatProps) {
onDelete={scratchlist.remove}
onSend={props.onSend}
onExitScratchlistMode={() => setScratchlistMode(false)}
disabled={props.isSending}
disabled={props.isSending || isScratchlistParking}
/>
) : null}
<QueuedMessagesBar
@@ -1639,6 +1743,8 @@ function SessionChatInner(props: SessionChatProps) {
scratchlistMode={scratchlistMode}
scratchlistCount={scratchlist.entries.length}
onScratchlistToggle={handleScratchlistToggle}
onParkScratchlist={onParkScratchlist}
onScratchlistParkingChange={setIsScratchlistParking}
sendError={props.sendError ?? null}
onClearSendError={props.onClearSendError}
onSuppressSendErrorRestore={props.onSuppressSendErrorRestore}
@@ -297,3 +297,103 @@ describe('createScratchlistAttachmentAdapter', () => {
expect(deleteScratchlistAttachment).toHaveBeenCalledWith('session-1', 'hub-race')
})
})
describe('createScratchlistAttachmentAdapter.send migrates chat-path chips (#1226)', () => {
it('uploads pending chat-path file to hub scratchlist and returns hub metadata', async () => {
const hubAttachment = {
id: 'hub-migrated',
filename: 'before-mode.png',
mimeType: 'image/png',
size: 4,
path: 'hapi-hub:scratchlist/default/session-1/hub-migrated-before-mode.png',
}
const uploadScratchlistAttachment = vi.fn().mockResolvedValue({
success: true,
attachment: hubAttachment,
})
const deleteUploadFile = vi.fn().mockResolvedValue(undefined)
const api = { uploadScratchlistAttachment, deleteUploadFile } as never
const adapter = createScratchlistAttachmentAdapter(api, 'session-1')
const file = new File([new Uint8Array([137, 80, 78, 71])], 'before-mode.png', { type: 'image/png' })
const complete = await adapter.send({
id: 'composer-chat-1',
type: 'file',
name: 'before-mode.png',
contentType: 'image/png',
file,
status: { type: 'requires-action', reason: 'composer-send' },
path: '/tmp/hapi-blobs/session-1/before-mode.png',
previewUrl: 'data:image/png;base64,iVBORw0KGgo=',
} as never)
expect(uploadScratchlistAttachment).toHaveBeenCalledWith(
'session-1',
'before-mode.png',
expect.any(String),
'image/png',
)
// Chat-path cleanup is deferred until scratchlist.add succeeds (#1226 review).
expect(deleteUploadFile).not.toHaveBeenCalled()
expect(complete.content).toEqual([
{
type: 'text',
text: JSON.stringify({
__attachmentMetadata: {
...hubAttachment,
previewUrl: 'data:image/png;base64,iVBORw0KGgo=',
migratedFromPath: '/tmp/hapi-blobs/session-1/before-mode.png',
},
}),
},
])
})
it('throws when chat-path migrate upload fails so park does not silently drop', async () => {
const uploadScratchlistAttachment = vi.fn().mockResolvedValue({
success: false,
error: 'quota',
})
const api = { uploadScratchlistAttachment, deleteUploadFile: vi.fn() } as never
const adapter = createScratchlistAttachmentAdapter(api, 'session-1')
const file = new File([new Uint8Array([1])], 'x.png', { type: 'image/png' })
await expect(adapter.send({
id: 'composer-chat-2',
type: 'file',
name: 'x.png',
contentType: 'image/png',
file,
status: { type: 'requires-action', reason: 'composer-send' },
path: '/tmp/hapi-blobs/x.png',
} as never)).rejects.toThrow(/quota|Failed to migrate/i)
})
it('releaseWithoutDelete makes remove a no-op so clearAttachments keeps parked hubs', async () => {
const deleteScratchlistAttachment = vi.fn()
const deleteUploadFile = vi.fn()
const api = { deleteScratchlistAttachment, deleteUploadFile } as never
const adapter = createScratchlistAttachmentAdapter(api, 'session-1')
const pending = {
id: 'composer-hub-1',
type: 'file' as const,
name: 'a.png',
contentType: 'image/png',
status: { type: 'requires-action' as const, reason: 'composer-send' as const },
path: 'hapi-hub:scratchlist/default/session-1/hub-1-a.png',
hubAttachment: {
id: 'hub-1',
filename: 'a.png',
mimeType: 'image/png',
size: 1,
path: 'hapi-hub:scratchlist/default/session-1/hub-1-a.png',
},
}
adapter.releaseWithoutDelete([pending.id])
await adapter.remove(pending as never)
expect(deleteScratchlistAttachment).not.toHaveBeenCalled()
expect(deleteUploadFile).not.toHaveBeenCalled()
})
})
+85 -14
View File
@@ -1,6 +1,9 @@
import type { AttachmentAdapter, Attachment, CompleteAttachment, PendingAttachment } from '@assistant-ui/react'
import type { ScratchlistAttachmentMetadata } from '@hapi/protocol'
import { parseHubScratchlistAttachmentPath } from '@hapi/protocol'
import {
isHubScratchlistAttachmentPath,
parseHubScratchlistAttachmentPath,
} from '@hapi/protocol'
import type { ApiClient } from '@/api/client'
import { getRestoredUploadMetadata } from '@/lib/composer-attachment-drafts'
import { isImageMimeType } from '@/lib/fileAttachments'
@@ -40,8 +43,21 @@ export function hubAttachmentFromRestoredDraft(
}
}
export function createScratchlistAttachmentAdapter(api: ApiClient, sessionId: string): AttachmentAdapter {
export type ScratchlistAttachmentAdapter = AttachmentAdapter & {
/**
* After a successful park, `clearAttachments()` still calls `remove()`.
* Mark those chip ids so remove skips hub/chat deletes (blobs now live
* on the scratchlist entry).
*/
releaseWithoutDelete(ids: Iterable<string>): void
}
export function createScratchlistAttachmentAdapter(
api: ApiClient,
sessionId: string,
): ScratchlistAttachmentAdapter {
const cancelledAttachmentIds = new Set<string>()
const releasedWithoutDeleteIds = new Set<string>()
return {
// assistant-ui uses the exact "*" sentinel for an allow-all adapter.
@@ -49,6 +65,12 @@ export function createScratchlistAttachmentAdapter(api: ApiClient, sessionId: st
// this adapter's add() method can run.
accept: '*',
releaseWithoutDelete(ids: Iterable<string>): void {
for (const id of ids) {
releasedWithoutDeleteIds.add(id)
}
},
async *add({ file }): AsyncGenerator<PendingAttachment> {
const contentType = file.type || 'application/octet-stream'
const restored = getRestoredUploadMetadata(file)
@@ -163,17 +185,67 @@ export function createScratchlistAttachmentAdapter(api: ApiClient, sessionId: st
},
async remove(attachment: Attachment): Promise<void> {
if (releasedWithoutDeleteIds.delete(attachment.id)) {
return
}
cancelledAttachmentIds.add(attachment.id)
const pending = attachment as PendingScratchlistAttachment
const hubId = pending.hubAttachment?.id
if (hubId) {
await api.deleteScratchlistAttachment(sessionId, hubId).catch(() => {})
return
}
// Chat-path chip attached before scratchlist mode was enabled (#1226).
if (pending.path && !isHubScratchlistAttachmentPath(pending.path)) {
await api.deleteUploadFile(sessionId, pending.path).catch(() => {})
}
},
async send(attachment: PendingAttachment): Promise<CompleteAttachment> {
const pending = attachment as PendingScratchlistAttachment
const hubAttachment = pending.hubAttachment
let hubAttachment = pending.hubAttachment
let previewUrl = pending.previewUrl
let migratedFromPath: string | undefined
// Attach-before-mode: composer still holds a chat-path pending from
// the normal upload adapter. Migrate into hub scratchlist storage
// so park keeps the image (#1226). Fail closed (throw) — empty
// content would park text-only and clear chips silently.
//
// Do NOT delete the chat-path upload here: send() runs before
// scratchlist.add succeeds. If park is rejected (at-cap / 409 /
// network), the composer keeps chips that still reference the
// chat path; deleting early poisons retry and orphans the hub
// blob. Cleanup is deferred to onSendForComposer.
if (!hubAttachment) {
const file = attachment.file
if (!file) {
throw new Error('Cannot park scratchlist attachment without file bytes')
}
const contentType = attachment.contentType || file.type || 'application/octet-stream'
const content = await fileToBase64(file)
const result = await api.uploadScratchlistAttachment(
sessionId,
attachment.name,
content,
contentType
)
if (!result.success || !result.attachment) {
throw new Error(
result.error ?? 'Failed to migrate attachment to scratchlist storage'
)
}
hubAttachment = result.attachment
if (
pending.path
&& !isHubScratchlistAttachmentPath(pending.path)
) {
migratedFromPath = pending.path
}
if (!previewUrl && isImageMimeType(contentType) && file.size <= MAX_PREVIEW_BYTES) {
previewUrl = await fileToDataUrl(file)
}
}
return {
id: attachment.id,
@@ -181,17 +253,16 @@ export function createScratchlistAttachmentAdapter(api: ApiClient, sessionId: st
name: attachment.name,
contentType: attachment.contentType,
status: { type: 'complete' },
content: hubAttachment
? [{
type: 'text',
text: JSON.stringify({
__attachmentMetadata: {
...hubAttachment,
previewUrl: pending.previewUrl
}
})
}]
: []
content: [{
type: 'text',
text: JSON.stringify({
__attachmentMetadata: {
...hubAttachment,
previewUrl,
...(migratedFromPath ? { migratedFromPath } : {}),
}
})
}]
}
}
}
@@ -0,0 +1,300 @@
import { describe, expect, it, vi } from 'vitest'
import type { AttachmentMetadata } from '@/types/api'
import {
attachmentsNeedScratchlistMigration,
finalizeMigratedScratchlistParkCleanup,
migrateChatPathAttachmentsToScratchlist,
prepareScratchlistParkAttachments,
type ParkAttachmentMetadata,
} from './scratchlistAttachmentFlow'
function chatAttachment(path = '/tmp/hapi-blobs/a.png'): AttachmentMetadata {
return {
id: 'att-1',
filename: 'a.png',
mimeType: 'image/png',
size: 4,
path,
previewUrl: 'data:image/png;base64,aaaa',
}
}
function hubAttachment(): AttachmentMetadata {
return {
id: 'hub-1',
filename: 'a.png',
mimeType: 'image/png',
size: 4,
path: 'hapi-hub:scratchlist/default/session-1/hub-1-a.png',
}
}
describe('attachmentsNeedScratchlistMigration (#1226)', () => {
it('is false for empty / already-hub payloads', () => {
expect(attachmentsNeedScratchlistMigration(undefined)).toBe(false)
expect(attachmentsNeedScratchlistMigration([])).toBe(false)
expect(attachmentsNeedScratchlistMigration([hubAttachment()])).toBe(false)
})
it('is true when any attachment still has a chat upload path', () => {
expect(attachmentsNeedScratchlistMigration([chatAttachment()])).toBe(true)
expect(attachmentsNeedScratchlistMigration([hubAttachment(), chatAttachment()])).toBe(true)
})
})
describe('migrateChatPathAttachmentsToScratchlist (#1226)', () => {
it('re-uploads chat-path items via scratchlist/upload and leaves hub items alone', async () => {
const migrated = {
id: 'hub-new',
filename: 'a.png',
mimeType: 'image/png',
size: 4,
path: 'hapi-hub:scratchlist/default/session-1/hub-new-a.png',
}
const uploadScratchlistAttachment = vi.fn().mockResolvedValue({
success: true,
attachment: migrated,
})
const deleteUploadFile = vi.fn().mockResolvedValue(undefined)
const api = { uploadScratchlistAttachment, deleteUploadFile } as never
const hub = hubAttachment()
const chat = chatAttachment()
const contentBase64 = 'iVBORw0KGgo='
const result = await migrateChatPathAttachmentsToScratchlist(
api,
'session-1',
[hub, chat],
async (att) => {
expect(att.path).toBe(chat.path)
return contentBase64
},
)
expect(uploadScratchlistAttachment).toHaveBeenCalledTimes(1)
expect(uploadScratchlistAttachment).toHaveBeenCalledWith(
'session-1',
'a.png',
contentBase64,
'image/png',
)
// Chat-path delete is deferred until park succeeds.
expect(deleteUploadFile).not.toHaveBeenCalled()
expect(result).toEqual([
hub,
{ ...migrated, previewUrl: chat.previewUrl, migratedFromPath: chat.path },
])
})
it('rolls back newly uploaded hub blobs and throws when one migrate fails', async () => {
const uploadScratchlistAttachment = vi.fn()
.mockResolvedValueOnce({
success: true,
attachment: {
id: 'hub-ok',
filename: 'a.png',
mimeType: 'image/png',
size: 1,
path: 'hapi-hub:scratchlist/default/session-1/hub-ok-a.png',
},
})
.mockResolvedValueOnce({ success: false, error: 'too big' })
const deleteScratchlistAttachment = vi.fn().mockResolvedValue(undefined)
const deleteUploadFile = vi.fn().mockResolvedValue(undefined)
const api = {
uploadScratchlistAttachment,
deleteScratchlistAttachment,
deleteUploadFile,
} as never
await expect(migrateChatPathAttachmentsToScratchlist(
api,
'session-1',
[chatAttachment('/tmp/a.png'), chatAttachment('/tmp/b.png')],
async () => 'YmFzZTY0',
)).rejects.toThrow(/too big|Failed to migrate/i)
expect(deleteScratchlistAttachment).toHaveBeenCalledWith('session-1', 'hub-ok')
})
})
describe('prepareScratchlistParkAttachments (#1226)', () => {
it('passes hub chips through and migrates chat-path chips without deleting the chat upload', async () => {
const hubMeta = {
id: 'hub-1',
filename: 'hub.png',
mimeType: 'image/png',
size: 2,
path: 'hapi-hub:scratchlist/default/session-1/hub-1-hub.png',
}
const migrated = {
id: 'hub-new',
filename: 'chat.png',
mimeType: 'image/png',
size: 1,
path: 'hapi-hub:scratchlist/default/session-1/hub-new-chat.png',
}
const uploadScratchlistAttachment = vi.fn().mockResolvedValue({
success: true,
attachment: migrated,
})
const deleteUploadFile = vi.fn()
const api = { uploadScratchlistAttachment, deleteUploadFile } as never
const chatFile = new File([new Uint8Array([1])], 'chat.png', { type: 'image/png' })
const result = await prepareScratchlistParkAttachments(api, 'session-1', [
{
id: 'chip-hub',
name: 'hub.png',
contentType: 'image/png',
hubAttachment: hubMeta,
previewUrl: 'data:image/png;base64,hub',
},
{
id: 'chip-chat',
name: 'chat.png',
contentType: 'image/png',
file: chatFile,
path: '/tmp/hapi-blobs/chat.png',
previewUrl: 'data:image/png;base64,chat',
},
])
expect(uploadScratchlistAttachment).toHaveBeenCalledTimes(1)
expect(deleteUploadFile).not.toHaveBeenCalled()
expect(result).toEqual([
{ ...hubMeta, previewUrl: 'data:image/png;base64,hub' },
{
...migrated,
previewUrl: 'data:image/png;base64,chat',
migratedFromPath: '/tmp/hapi-blobs/chat.png',
},
])
})
it('reuses a restored hub path without re-uploading', async () => {
const uploadScratchlistAttachment = vi.fn()
const api = { uploadScratchlistAttachment } as never
const hubPath = 'hapi-hub:scratchlist/default/session-1/aaaaaaaa-bbbb-4ccc-8ddd-eeeeeeeeeeee-photo.png'
const file = new File([new Uint8Array([1, 2])], 'photo.png', { type: 'image/png' })
const result = await prepareScratchlistParkAttachments(api, 'session-1', [
{
id: 'chip-restored',
name: 'photo.png',
contentType: 'image/png',
file,
path: hubPath,
previewUrl: 'data:image/png;base64,x',
},
])
expect(uploadScratchlistAttachment).not.toHaveBeenCalled()
expect(result).toEqual([{
id: 'aaaaaaaa-bbbb-4ccc-8ddd-eeeeeeeeeeee',
filename: 'photo.png',
mimeType: 'image/png',
size: 2,
path: hubPath,
previewUrl: 'data:image/png;base64,x',
}])
})
it('rolls back hub blobs when a later chip fails to migrate', async () => {
const uploadScratchlistAttachment = vi.fn()
.mockResolvedValueOnce({
success: true,
attachment: {
id: 'hub-ok',
filename: 'a.png',
mimeType: 'image/png',
size: 1,
path: 'hapi-hub:scratchlist/default/session-1/hub-ok-a.png',
},
})
.mockResolvedValueOnce({ success: false, error: 'quota' })
const deleteScratchlistAttachment = vi.fn().mockResolvedValue(undefined)
const api = { uploadScratchlistAttachment, deleteScratchlistAttachment } as never
const file = new File([new Uint8Array([1])], 'a.png', { type: 'image/png' })
await expect(prepareScratchlistParkAttachments(api, 'session-1', [
{ id: '1', name: 'a.png', contentType: 'image/png', file, path: '/tmp/a.png' },
{ id: '2', name: 'b.png', contentType: 'image/png', file, path: '/tmp/b.png' },
])).rejects.toThrow(/quota|Failed to migrate/i)
expect(deleteScratchlistAttachment).toHaveBeenCalledWith('session-1', 'hub-ok')
})
})
describe('finalizeMigratedScratchlistParkCleanup (#1226)', () => {
const hubPath = 'hapi-hub:scratchlist/default/session-1/hub-1-a.png'
const chatPath = '/tmp/hapi-blobs/a.png'
it('deletes chat uploads after accepted park', async () => {
const deleteUploadFile = vi.fn().mockResolvedValue(undefined)
const deleteScratchlistAttachment = vi.fn()
const api = { deleteUploadFile, deleteScratchlistAttachment } as never
await finalizeMigratedScratchlistParkCleanup(
api,
'session-1',
[{
id: 'hub-1',
filename: 'a.png',
mimeType: 'image/png',
size: 1,
path: hubPath,
migratedFromPath: chatPath,
} as ParkAttachmentMetadata],
true,
)
expect(deleteUploadFile).toHaveBeenCalledWith('session-1', chatPath)
expect(deleteScratchlistAttachment).not.toHaveBeenCalled()
})
it('deletes orphan hub blobs after rejected park so retry can remigrate', async () => {
const deleteUploadFile = vi.fn()
const deleteScratchlistAttachment = vi.fn().mockResolvedValue(undefined)
const api = { deleteUploadFile, deleteScratchlistAttachment } as never
await finalizeMigratedScratchlistParkCleanup(
api,
'session-1',
[{
id: 'hub-1',
filename: 'a.png',
mimeType: 'image/png',
size: 1,
path: hubPath,
migratedFromPath: chatPath,
} as ParkAttachmentMetadata],
false,
)
expect(deleteScratchlistAttachment).toHaveBeenCalledWith('session-1', 'hub-1')
expect(deleteUploadFile).not.toHaveBeenCalled()
})
it('no-ops when nothing was migrated from a chat path', async () => {
const deleteUploadFile = vi.fn()
const deleteScratchlistAttachment = vi.fn()
const api = { deleteUploadFile, deleteScratchlistAttachment } as never
await finalizeMigratedScratchlistParkCleanup(
api,
'session-1',
[{
id: 'hub-1',
filename: 'a.png',
mimeType: 'image/png',
size: 1,
path: hubPath,
}],
true,
)
expect(deleteUploadFile).not.toHaveBeenCalled()
expect(deleteScratchlistAttachment).not.toHaveBeenCalled()
})
})
+182 -1
View File
@@ -1,7 +1,11 @@
import type { ScratchlistAttachmentMetadata } from '@hapi/protocol'
import {
isHubScratchlistAttachmentPath,
type ScratchlistAttachmentMetadata,
} from '@hapi/protocol'
import type { ApiClient } from '@/api/client'
import type { AttachmentMetadata } from '@/types/api'
import { isImageMimeType } from '@/lib/fileAttachments'
import { hubAttachmentFromRestoredDraft } from '@/lib/scratchlistAttachmentAdapter'
async function blobToBase64(blob: Blob): Promise<string> {
return new Promise((resolve, reject) => {
@@ -20,6 +24,183 @@ async function blobToBase64(blob: Blob): Promise<string> {
})
}
/** Client-only marker stashed on park payloads after chat→hub migrate (#1226). */
export type ParkAttachmentMetadata = AttachmentMetadata & {
migratedFromPath?: string
}
/** True when any composer attachment still lives on the normal chat upload path. */
export function attachmentsNeedScratchlistMigration(
attachments: AttachmentMetadata[] | undefined
): boolean {
return (attachments ?? []).some((att) => !isHubScratchlistAttachmentPath(att.path))
}
/** Result of preparing a scratchlist park (migrate only — add happens in commit). */
export type ScratchlistParkPrepared = {
/** Drop orphan hub blobs created during prepare (snapshot changed / give up). */
abort: () => Promise<void>
/** Persist the entry. On false, orphans are already cleaned. */
commit: () => Promise<boolean>
/** After successful commit + snapshot still matches: delete chat uploads + release chips. */
beforeClear: () => Promise<void>
}
export type ScratchlistParkResult = false | ScratchlistParkPrepared
/**
* - accepted: drop the original chat uploads (hub blobs are on the entry)
* - rejected: drop the orphan hub blobs so retry can re-migrate from chat paths
*/
export async function finalizeMigratedScratchlistParkCleanup(
api: ApiClient,
sessionId: string,
attachments: AttachmentMetadata[] | undefined,
accepted: boolean,
): Promise<void> {
const list = (attachments ?? []) as ParkAttachmentMetadata[]
const migrated = list.filter((att) => typeof att.migratedFromPath === 'string' && att.migratedFromPath.length > 0)
if (migrated.length === 0) return
if (accepted) {
await Promise.allSettled(
migrated.map((att) => api.deleteUploadFile(sessionId, att.migratedFromPath!))
)
return
}
await Promise.allSettled(
migrated.map((att) => api.deleteScratchlistAttachment(sessionId, att.id))
)
}
/**
* Pending composer chip shape used when parking *before* assistant-ui
* `composer.send()` empties the UI (#1226 follow-up Major).
*/
export type PendingParkAttachment = {
id: string
name: string
contentType?: string
file?: File
path?: string
hubAttachment?: ScratchlistAttachmentMetadata
previewUrl?: string
}
/**
* Build hub park metadata from live composer chips without clearing them.
* Chat-path items are migrated into hub storage and stamped with
* `migratedFromPath`; the original chat upload is left in place until
* `finalizeMigratedScratchlistParkCleanup(accepted=true)`.
*/
export async function prepareScratchlistParkAttachments(
api: ApiClient,
sessionId: string,
pending: readonly PendingParkAttachment[],
): Promise<ParkAttachmentMetadata[]> {
const prepared: ParkAttachmentMetadata[] = []
const createdHubIds: string[] = []
try {
for (const chip of pending) {
const contentType = chip.contentType || chip.file?.type || 'application/octet-stream'
const hubAttachment = chip.hubAttachment
?? (chip.path && chip.file
? hubAttachmentFromRestoredDraft(chip.path, chip.file, contentType)
: null)
if (hubAttachment) {
prepared.push({
...hubAttachment,
previewUrl: chip.previewUrl,
})
continue
}
const file = chip.file
if (!file) {
throw new Error(`Cannot park attachment ${chip.name} without file bytes`)
}
const content = await blobToBase64(file)
const result = await api.uploadScratchlistAttachment(
sessionId,
chip.name,
content,
contentType,
)
if (!result.success || !result.attachment) {
throw new Error(
result.error ?? `Failed to migrate attachment ${chip.name}`
)
}
createdHubIds.push(result.attachment.id)
const migratedFromPath =
chip.path && !isHubScratchlistAttachmentPath(chip.path)
? chip.path
: undefined
prepared.push({
...result.attachment,
previewUrl: chip.previewUrl,
...(migratedFromPath ? { migratedFromPath } : {}),
})
}
return prepared
} catch (error) {
await Promise.allSettled(
createdHubIds.map((id) => api.deleteScratchlistAttachment(sessionId, id))
)
throw error
}
}
/**
* Re-upload chat-path attachments into hub scratchlist storage (#1226).
*
* Does **not** delete the original chat upload callers must run
* `finalizeMigratedScratchlistParkCleanup` after the park attempt so a
* rejected add can retry from the chat path. On failure, newly created
* hub blobs are deleted so a partial migrate does not leak quota.
*/
export async function migrateChatPathAttachmentsToScratchlist(
api: ApiClient,
sessionId: string,
attachments: AttachmentMetadata[],
readContentBase64: (attachment: AttachmentMetadata) => Promise<string>
): Promise<ParkAttachmentMetadata[]> {
const migrated: ParkAttachmentMetadata[] = []
const createdHubIds: string[] = []
try {
for (const attachment of attachments) {
if (isHubScratchlistAttachmentPath(attachment.path)) {
migrated.push(attachment)
continue
}
const content = await readContentBase64(attachment)
const result = await api.uploadScratchlistAttachment(
sessionId,
attachment.filename,
content,
attachment.mimeType
)
if (!result.success || !result.attachment) {
throw new Error(
result.error ?? `Failed to migrate attachment ${attachment.filename}`
)
}
createdHubIds.push(result.attachment.id)
migrated.push({
...result.attachment,
previewUrl: attachment.previewUrl,
migratedFromPath: attachment.path,
})
}
return migrated
} catch (error) {
await Promise.allSettled(
createdHubIds.map((id) => api.deleteScratchlistAttachment(sessionId, id))
)
throw error
}
}
export async function stageScratchlistAttachmentsForComposeSend(
api: ApiClient,
sessionId: string,