fix(web): restore failed sends atomically (#1326)

* fix(web): restore failed sends atomically

* fix(web): wait for composer draft hydration

* fix(web): count restored attachments after success

* fix(web): keep scratchlist copy available

* fix(web): move suppressed retry errors to target session
This commit is contained in:
KorenKrita
2026-08-03 09:25:01 +08:00
committed by GitHub
parent 0725fabe84
commit fb3988a81f
12 changed files with 983 additions and 76 deletions
@@ -0,0 +1,413 @@
import { act, cleanup, fireEvent, render, screen, waitFor } from '@testing-library/react'
import type { ReactNode, TextareaHTMLAttributes } from 'react'
import { useRef, useState } from 'react'
import { afterEach, describe, expect, it, vi } from 'vitest'
import { I18nProvider } from '@/lib/i18n-context'
import type { PendingSchedule } from '@/components/AssistantChat/ScheduleTimePicker'
import { HappyComposer, type ComposerSendError } from './HappyComposer'
/**
* HappyComposer owns the recovery guard, while assistant-ui owns the live
* composer store. This focused harness supplies the small subset of that
* store necessary to exercise send → user interaction → delayed error races.
*/
type FakeAttachment = { id: string; status: { type: 'complete' } }
type MockComposerInputProps = TextareaHTMLAttributes<HTMLTextAreaElement> & {
maxRows?: number
submitOnEnter?: boolean
cancelOnEscape?: boolean
}
type FakeRuntimeState = {
composer: { text: string; attachments: FakeAttachment[] }
thread: { isRunning: boolean; isDisabled: boolean }
}
const runtime = vi.hoisted(() => ({
snapshot: {
composer: { text: '', attachments: [] as FakeAttachment[] },
thread: { isRunning: false, isDisabled: false },
} as FakeRuntimeState,
setSnapshot: null as null | ((updater: (current: FakeRuntimeState) => FakeRuntimeState) => void),
}))
vi.mock('@assistant-ui/react', async () => {
const React = await import('react')
return {
useAui: () => ({
composer: () => ({
setText: (text: string) => {
runtime.setSnapshot!((current) => ({
...current,
composer: { ...current.composer, text },
}))
},
send: () => {
runtime.setSnapshot!((current) => ({
...current,
composer: { text: '', attachments: [] },
}))
},
addAttachment: async () => {},
}),
thread: () => ({ cancelRun: () => {} }),
}),
useAuiState: (selector: (state: typeof runtime.snapshot) => unknown) => selector(runtime.snapshot),
ComposerPrimitive: {
Root: ({ children, onSubmit }: { children: ReactNode; onSubmit?: () => void }) => (
<form onSubmit={onSubmit}>{children}</form>
),
Input: React.forwardRef<HTMLTextAreaElement, MockComposerInputProps>(
({ onChange, maxRows: _maxRows, submitOnEnter: _submitOnEnter, cancelOnEscape: _cancelOnEscape, ...props }, ref) => (
<textarea
{...props}
ref={ref}
value={runtime.snapshot.composer.text}
onChange={(event) => {
runtime.setSnapshot!((current) => ({
...current,
composer: { ...current.composer, text: event.target.value },
}))
onChange?.(event)
}}
/>
),
),
Attachments: () => null,
},
}
})
vi.mock('@/lib/composerSegments', () => ({ isRichComposerMentionsEnabled: () => false }))
vi.mock('@/hooks/useComposerDraft', () => ({
useComposerDraft: (sessionId: string | undefined) => ({ sessionId, complete: true, restoredAny: false }),
}))
vi.mock('@/hooks/useComposerEnterBehavior', () => ({ useComposerEnterBehavior: () => ({ composerEnterBehavior: 'send' }) }))
vi.mock('@/hooks/usePlatform', () => ({ usePlatform: () => ({ haptic: { impact: () => {}, notification: () => {} }, isTouch: false }) }))
vi.mock('@/hooks/usePWAInstall', () => ({ usePWAInstall: () => ({ isStandalone: false, isIOS: false }) }))
vi.mock('@/hooks/useActiveWord', () => ({ useActiveWord: () => null }))
vi.mock('@/hooks/useActiveSuggestions', () => ({ useActiveSuggestions: () => [[], -1, () => {}, () => {}, () => {}] }))
vi.mock('@/components/ChatInput/FloatingOverlay', () => ({ FloatingOverlay: ({ children }: { children: ReactNode }) => <>{children}</> }))
vi.mock('@/components/ChatInput/Autocomplete', () => ({ Autocomplete: () => null }))
vi.mock('@/components/AssistantChat/StatusBar', () => ({ StatusBar: () => null }))
vi.mock('./PiModelPanel', () => ({ PiModelPanel: () => null }))
vi.mock('./PiThinkingLevelPanel', () => ({ PiThinkingLevelPanel: () => null }))
vi.mock('@/components/AssistantChat/ComposerButtons', () => ({
ComposerButtons: (props: {
onSend: () => void
onSchedule: (pending: PendingSchedule) => void
onClearSchedule: () => void
pendingSchedule: PendingSchedule | null
}) => (
<div>
<button type="button" onClick={props.onSend}>send</button>
<button type="button" onClick={() => props.onSchedule({ type: 'absolute', ms: 9000 })}>select schedule</button>
<button type="button" onClick={props.onClearSchedule}>clear schedule</button>
<output data-testid="pending-schedule">{JSON.stringify(props.pendingSchedule)}</output>
</div>
),
}))
type HarnessControls = {
setError: (error: ComposerSendError | null) => void
addAttachment: () => void
removeAttachments: () => void
acceptAndClearSchedule: () => void
remount: () => void
programmaticSetText: (text: string) => void
getClearErrorCalls: () => number
}
function ComposerHarness(props: { initialText: string; initialSchedule?: PendingSchedule | null; controls: { current: HarnessControls | null } }) {
const [snapshot, setSnapshot] = useState<FakeRuntimeState>(() => ({
composer: { text: props.initialText, attachments: [] },
thread: { isRunning: false, isDisabled: false },
}))
const [schedule, setSchedule] = useState<PendingSchedule | null>(props.initialSchedule ?? null)
const [sendError, setSendError] = useState<ComposerSendError | null>(null)
const [composerKey, setComposerKey] = useState('composer-a')
const clearErrorCallsRef = useRef(0)
runtime.snapshot = snapshot
runtime.setSnapshot = setSnapshot
props.controls.current = {
setError: sendError => setSendError(sendError),
addAttachment: () => setSnapshot((current) => ({
...current,
composer: {
...current.composer,
attachments: [{ id: 'new-attachment', status: { type: 'complete' } }],
},
})),
removeAttachments: () => setSnapshot((current) => ({
...current,
composer: { ...current.composer, attachments: [] },
})),
acceptAndClearSchedule: () => setSchedule(null),
remount: () => setComposerKey((key) => key === 'composer-a' ? 'composer-b' : 'composer-a'),
programmaticSetText: (text) => setSnapshot((current) => ({
...current,
composer: { ...current.composer, text },
})),
getClearErrorCalls: () => clearErrorCallsRef.current,
}
return (
<I18nProvider>
<HappyComposer
key={composerKey}
sessionId={composerKey}
pendingSchedule={schedule}
onSchedule={setSchedule}
onClearSchedule={() => setSchedule(null)}
sendError={sendError}
onClearSendError={() => {
clearErrorCallsRef.current += 1
setSendError(null)
}}
onSuppressSendErrorRestore={(id) => setSendError((current) =>
current && current.id === id
? { ...current, restoreSuppressed: true }
: current
)}
/>
</I18nProvider>
)
}
function renderComposer(initialText = 'failed text', initialSchedule: PendingSchedule | null = { type: 'absolute', ms: 1234 }) {
const controls: { current: HarnessControls | null } = { current: null }
render(<ComposerHarness initialText={initialText} initialSchedule={initialSchedule} controls={controls} />)
return controls
}
function fail(
id: number,
text = 'failed text',
scheduledAt: number | null = 1234,
mutationStarted = true,
): ComposerSendError {
return { id, text, scheduledAt, mutationStarted, restoreSuppressed: false, message: `failed-${id}` }
}
function send() {
fireEvent.click(screen.getByRole('button', { name: 'send' }))
}
function acceptAndClearSchedule(controls: { current: HarnessControls | null }) {
act(() => controls.current!.acceptAndClearSchedule())
}
function setError(controls: { current: HarnessControls | null }, error: ComposerSendError) {
act(() => controls.current!.setError(error))
}
function input(): HTMLTextAreaElement {
return screen.getByRole('textbox') as HTMLTextAreaElement
}
describe('HappyComposer send-error atomic restore', () => {
afterEach(() => {
cleanup()
runtime.setSnapshot = null
})
it('restores untouched text and its absolute schedule after accepted-send clear', async () => {
const controls = renderComposer()
send()
acceptAndClearSchedule(controls)
setError(controls, fail(1))
await waitFor(() => expect(input()).toHaveValue('failed text'))
expect(screen.getByTestId('pending-schedule')).toHaveTextContent('{"type":"absolute","ms":1234}')
})
it('restores text but preserves the original schedule when rejection happens before mutation acceptance', async () => {
const controls = renderComposer()
send()
setError(controls, fail(1, 'failed text', 1234, false))
await waitFor(() => expect(input()).toHaveValue('failed text'))
expect(screen.getByTestId('pending-schedule')).toHaveTextContent('{"type":"absolute","ms":1234}')
})
it('waits for delayed accepted-send clear when the mutation error arrives first', async () => {
const controls = renderComposer()
send()
setError(controls, fail(1))
expect(input()).toHaveValue('')
expect(screen.getByTestId('pending-schedule')).toHaveTextContent('{"type":"absolute","ms":1234}')
acceptAndClearSchedule(controls)
await waitFor(() => expect(input()).toHaveValue('failed text'))
expect(screen.getByTestId('pending-schedule')).toHaveTextContent('{"type":"absolute","ms":1234}')
})
it('restores after a keyed composer remount when no new draft interaction occurs', async () => {
const controls = renderComposer()
send()
acceptAndClearSchedule(controls)
act(() => controls.current!.remount())
setError(controls, fail(1))
await waitFor(() => expect(input()).toHaveValue('failed text'))
expect(screen.getByTestId('pending-schedule')).toHaveTextContent('{"type":"absolute","ms":1234}')
})
it('does not implicitly restore after a keyed remount receives a new draft interaction', async () => {
const controls = renderComposer()
send()
acceptAndClearSchedule(controls)
act(() => controls.current!.remount())
fireEvent.change(input(), { target: { value: 'new session draft' } })
setError(controls, fail(1))
await waitFor(() => expect(input()).toHaveValue('new session draft'))
expect(screen.getByTestId('pending-schedule')).toHaveTextContent('null')
})
it('clears a safely restored error after a programmatic text replacement so a remount preserves the replacement', async () => {
const controls = renderComposer()
send()
acceptAndClearSchedule(controls)
setError(controls, fail(1))
await waitFor(() => expect(input()).toHaveValue('failed text'))
expect(screen.getByTestId('composer-send-error')).toBeTruthy()
act(() => controls.current!.programmaticSetText('queued replacement'))
await waitFor(() => expect(screen.queryByTestId('composer-send-error')).toBeNull())
expect(input()).toHaveValue('queued replacement')
act(() => controls.current!.remount())
expect(input()).toHaveValue('queued replacement')
expect(screen.queryByTestId('composer-send-error')).toBeNull()
})
it('clears a safely restored error after a programmatic attachment replacement', async () => {
const controls = renderComposer()
send()
acceptAndClearSchedule(controls)
setError(controls, fail(1))
await waitFor(() => expect(input()).toHaveValue('failed text'))
act(() => controls.current!.addAttachment())
await waitFor(() => expect(screen.queryByTestId('composer-send-error')).toBeNull())
})
it('keeps the restored error through a direct retry clear, then evaluates a new error id', async () => {
const controls = renderComposer()
send()
acceptAndClearSchedule(controls)
setError(controls, fail(1))
await waitFor(() => expect(input()).toHaveValue('failed text'))
const clearCallsBeforeRetry = controls.current!.getClearErrorCalls()
send()
await waitFor(() => expect(input()).toHaveValue(''))
expect(screen.getByTestId('composer-send-error')).toBeTruthy()
expect(controls.current!.getClearErrorCalls()).toBe(clearCallsBeforeRetry)
// Simulates the A -> B -> A keyed remount during the retry. The route
// keeps the old alert visible but marks it restore-suppressed.
act(() => controls.current!.remount())
expect(input()).toHaveValue('')
expect(screen.getByTestId('composer-send-error')).toBeTruthy()
// A route success clears the retained alert without restoring text.
act(() => controls.current!.setError(null))
expect(screen.queryByTestId('composer-send-error')).toBeNull()
expect(input()).toHaveValue('')
// A later failed retry is a new, unsuppressed id and restores normally.
acceptAndClearSchedule(controls)
setError(controls, fail(2, 'retry failed', 5678))
await waitFor(() => expect(input()).toHaveValue('retry failed'))
expect(screen.getByTestId('pending-schedule')).toHaveTextContent('{"type":"absolute","ms":5678}')
})
it('keeps a new text draft and does not restore the old schedule', async () => {
const controls = renderComposer()
send()
acceptAndClearSchedule(controls)
fireEvent.change(input(), { target: { value: 'new draft' } })
setError(controls, fail(1))
await waitFor(() => expect(input()).toHaveValue('new draft'))
expect(screen.getByTestId('pending-schedule')).toHaveTextContent('null')
})
it('does not restore after a user types then deletes back to empty', async () => {
const controls = renderComposer()
send()
acceptAndClearSchedule(controls)
fireEvent.change(input(), { target: { value: 'replacement' } })
fireEvent.change(input(), { target: { value: '' } })
setError(controls, fail(1))
await waitFor(() => expect(input()).toHaveValue(''))
expect(screen.getByTestId('pending-schedule')).toHaveTextContent('null')
})
it('does not restore after a new attachment is added then removed', async () => {
const controls = renderComposer()
send()
acceptAndClearSchedule(controls)
act(() => controls.current!.addAttachment())
act(() => controls.current!.removeAttachments())
setError(controls, fail(1))
await waitFor(() => expect(input()).toHaveValue(''))
expect(screen.getByTestId('pending-schedule')).toHaveTextContent('null')
})
it('handles an attachments-only failed send without restoring text or a schedule', async () => {
const controls = renderComposer('', null)
act(() => controls.current!.addAttachment())
send()
setError(controls, fail(1, '', null))
await waitFor(() => expect(input()).toHaveValue(''))
expect(screen.getByTestId('pending-schedule')).toHaveTextContent('null')
})
it('does not restore after the user selects then clears a new schedule', async () => {
const controls = renderComposer()
send()
acceptAndClearSchedule(controls)
fireEvent.click(screen.getByRole('button', { name: 'select schedule' }))
fireEvent.click(screen.getByRole('button', { name: 'clear schedule' }))
setError(controls, fail(1))
await waitFor(() => expect(input()).toHaveValue(''))
expect(screen.getByTestId('pending-schedule')).toHaveTextContent('null')
})
it('evaluates a later error id against a new send instead of deduping matching text', async () => {
const controls = renderComposer()
send()
acceptAndClearSchedule(controls)
setError(controls, fail(1, 'same text', 1234))
await waitFor(() => expect(input()).toHaveValue('same text'))
send()
acceptAndClearSchedule(controls)
setError(controls, fail(2, 'same text', 5678))
await waitFor(() => expect(input()).toHaveValue('same text'))
expect(screen.getByTestId('pending-schedule')).toHaveTextContent('{"type":"absolute","ms":5678}')
})
it('restores text alone for an immediate failed send', async () => {
const controls = renderComposer('immediate', null)
send()
setError(controls, fail(1, 'immediate', null))
await waitFor(() => expect(input()).toHaveValue('immediate'))
expect(screen.getByTestId('pending-schedule')).toHaveTextContent('null')
})
})
@@ -83,6 +83,10 @@ export type ComposerSendError = {
text: string
message: string
scheduledAt: number | null
/** False for guards that reject before the underlying message mutation starts. */
mutationStarted: boolean
/** True once the user has retried; retain UI but never restore this id again. */
restoreSuppressed: boolean
action?: {
label: string
onClick: () => void
@@ -106,6 +110,7 @@ export function useRichComposerBridge(
setInputState: (state: TextInputState) => void,
sendError: ComposerSendError | null,
onClearSendError?: () => void,
onUserEdit?: () => void,
) {
const onValueChange = useCallback((text: string) => {
flushTapSync(() => {
@@ -118,8 +123,9 @@ export function useRichComposerBridge(
}, [setInputState])
const onEdit = useCallback(() => {
onUserEdit?.()
if (sendError && onClearSendError) onClearSendError()
}, [sendError, onClearSendError])
}, [sendError, onClearSendError, onUserEdit])
return { onValueChange, onMirrorChange, onEdit }
}
@@ -248,6 +254,7 @@ export function HappyComposer(props: {
// inline error affordance until the user dismisses or starts editing.
sendError?: ComposerSendError | null
onClearSendError?: () => void
onSuppressSendErrorRestore?: (id: number) => void
/** Chip hover / aria-label resolver (SessionChat → useSessions). */
resolveSessionMentionTooltip?: (id: string, title: string) => SessionMentionResolveResult
}) {
@@ -301,6 +308,7 @@ export function HappyComposer(props: {
onClearSchedule: onClearScheduleProp,
sendError = null,
onClearSendError,
onSuppressSendErrorRestore,
resolveSessionMentionTooltip,
} = props
@@ -388,16 +396,43 @@ export function HappyComposer(props: {
const textareaRef = useRef<HTMLTextAreaElement>(null)
const richInputRef = useRef<RichComposerInputHandle>(null)
// `composer.text === ''` alone is not enough to identify the empty state
// created by a send. A user can type and delete a fresh draft before the
// failed mutation reports back. Keep monotonic interaction generations so
// history still wins over a visually identical empty composer.
const userEditGenerationRef = useRef(0)
const userScheduleGenerationRef = useRef(0)
const userAttachmentGenerationRef = useRef(0)
const observedAttachmentIdsRef = useRef(new Set(attachments.map((attachment) => attachment.id)))
const sendRestoreGuardRef = useRef<{
userEditGeneration: number
userScheduleGeneration: number
userAttachmentGeneration: number
} | null>(null)
// Kill-switch only (?richMentions=0 / localStorage=0 / VITE=false). Mount-time
// read — hard reload required, so no per-keystroke localStorage/URL parse.
const [richMentionsEnabled] = useState(() => isRichComposerMentionsEnabled())
const prevControlledByUser = useRef(controlledByUser)
const recordUserEdit = useCallback(() => {
userEditGenerationRef.current += 1
}, [])
const handleUserEdit = useCallback(() => {
recordUserEdit()
// Editing the restored text is the operator's "I'm handling it"
// signal -- drop the inline error so the affordance doesn't shout
// at them while they fix the message.
if (sendError && onClearSendError) {
onClearSendError()
}
}, [recordUserEdit, sendError, onClearSendError])
const {
onValueChange: handleRichValueChange,
onMirrorChange: handleRichMirrorChange,
onEdit: handleRichEdit,
} = useRichComposerBridge(api, setInputState, sendError, onClearSendError)
} = useRichComposerBridge(api, setInputState, sendError, onClearSendError, recordUserEdit)
const attachmentDrafts = attachments.flatMap((attachment) => {
if (!attachment.file) return []
@@ -409,7 +444,7 @@ export function HappyComposer(props: {
previewUrl: upload.previewUrl,
}]
})
useComposerDraft(
const draftHydration = useComposerDraft(
sessionId,
composerText,
attachmentDrafts,
@@ -419,38 +454,113 @@ export function HappyComposer(props: {
)
// assistant-ui clears `composer.text` synchronously the moment a send is
// invoked AND `SessionChat.handleSend` clears `pendingSchedule` the
// moment the mutation is accepted, so by the time the mutation's
// onError fires both the typed text and the schedule are gone. When
// the route hands us a `sendError`, splice both back in -- once per
// `sendError.id` so a second failure with the same text still triggers
// a fresh restore.
// invoked AND `SessionChat.handleSend` clears `pendingSchedule` after the
// mutation is accepted. A failure must put the text and absolute schedule
// back as one atomic recovery unit, but only while the composer still
// reflects that send's cleared state. A blank composer alone is not enough:
// a user might type then delete a replacement draft before onError arrives.
const restoredErrorIdRef = useRef<number | null>(null)
const restoredErrorSnapshotRef = useRef<{ id: number; text: string; observed: boolean } | null>(null)
useEffect(() => {
if (!sendError) {
if (!sendError || restoredErrorIdRef.current === sendError.id) {
return
}
if (restoredErrorIdRef.current === sendError.id) {
if (sendError.restoreSuppressed) {
// Route retains the error UI for the retry attempt, but this id
// must never repopulate a composer after a keyed remount.
restoredErrorIdRef.current = sendError.id
return
}
const guard = sendRestoreGuardRef.current
// A resolved inactive session navigates to a keyed, fresh composer
// before the target mutation can fail. That new instance has no local
// send snapshot, but its zero mount-time generations still prove no
// user interaction has happened there. Treat that as an implicit guard;
// any edit, schedule interaction, or newly observed attachment makes
// the error terminally unsafe just like the explicit snapshot path.
if (!guard) {
// The implicit guard is only for a keyed remount. Wait until the
// session-keyed draft hydration has conclusively run: its RAF may
// still restore a persisted replacement after this effect.
if (draftHydration.sessionId !== sessionId || !draftHydration.complete) return
if (draftHydration.restoredAny) {
restoredErrorIdRef.current = sendError.id
onClearSendError?.()
return
}
}
const interactionChanged = guard
? userEditGenerationRef.current !== guard.userEditGeneration
|| userScheduleGenerationRef.current !== guard.userScheduleGeneration
|| userAttachmentGenerationRef.current !== guard.userAttachmentGeneration
: userEditGenerationRef.current !== 0
|| userScheduleGenerationRef.current !== 0
|| userAttachmentGenerationRef.current !== 0
const textOrAttachmentChanged = composerText.length !== 0 || attachments.length !== 0
if (interactionChanged || textOrAttachmentChanged) {
// This error id is now conclusively unsafe. Do not retry if the
// user later deletes their replacement text or attachment. Clear
// route-level state as well: a remount otherwise loses this local
// consumed marker and can replay the stale error over the draft.
restoredErrorIdRef.current = sendError.id
onClearSendError?.()
return
}
if (sendError.mutationStarted && pendingSchedule !== null) {
// SessionChat clears an accepted send's schedule asynchronously.
// The mutation's onError can arrive before that parent render, so
// wait for its null cleared state before restoring text + schedule
// as one unit. User-selected schedules were rejected above by the
// schedule generation check.
return
}
restoredErrorIdRef.current = sendError.id
// Only restore when the composer is empty. If the user has already
// typed something new (rare -- composer is `disabled` during send,
// but possible if isSending toggles before this effect runs), we
// would otherwise stomp on their fresh input.
if (composerText.length === 0 && sendError.text.length > 0) {
api.composer().setText(sendError.text)
}
// Restore the pending schedule too. `scheduledAt` was already
// resolved to an absolute epoch-ms before the failed send (presets
// are computed at send time -- see `resolvePendingSchedule`), so
// we feed it back as an 'absolute' PendingSchedule. The existing
// shouldAutoClearPendingSchedule effect in SessionChat handles the
// case where the absolute time has passed by the time we restore.
restoredErrorSnapshotRef.current = { id: sendError.id, text: sendError.text, observed: false }
api.composer().setText(sendError.text)
// `scheduledAt` is already absolute (presets resolve at send time), so
// restore it through the normal controlled schedule path in the same
// effect as text. For a pre-mutation rejection this updates the still
// present source schedule to its send-time absolute instant.
if (sendError.scheduledAt !== null && onScheduleProp) {
onScheduleProp({ type: 'absolute', ms: sendError.scheduledAt })
}
}, [sendError, api, composerText, onScheduleProp])
}, [sendError, api, attachments, composerText, draftHydration, onClearSendError, onScheduleProp, pendingSchedule, sessionId])
// A successful automatic restore keeps its inline error visible so the
// operator understands why the draft returned. If another path replaces
// the restored text or inserts an attachment without firing a textarea or
// rich-input event (scratchlist/queued/draft programmatic writes), consume
// the route-level error before a keyed remount can replay it over that new
// state. The exact restored text is intentionally exempt from this check.
useEffect(() => {
const restored = restoredErrorSnapshotRef.current
if (!sendError || !restored || restored.id !== sendError.id) return
if (!restored.observed) {
if (composerText === restored.text && attachments.length === 0) {
restored.observed = true
}
return
}
if (composerText === restored.text && attachments.length === 0) return
onClearSendError?.()
}, [sendError, attachments, composerText, onClearSendError])
// A user-added attachment must make a recovery unsafe even if it is removed
// again before the send error arrives. This runs even without a local send
// guard so a post-resume composer gets the same protection. Attachment IDs
// already present at mount are the baseline; assistant-ui's send clear only
// removes IDs and therefore does not look like a user addition.
useEffect(() => {
for (const attachment of attachments) {
if (observedAttachmentIdsRef.current.has(attachment.id)) continue
observedAttachmentIdsRef.current.add(attachment.id)
userAttachmentGenerationRef.current += 1
}
}, [attachments])
useEffect(() => {
if (richMentionsEnabled) {
@@ -505,6 +615,10 @@ export function HappyComposer(props: {
markSkillUsed(suggestion.text.slice(1))
}
// Suggestions edit composer content programmatically, so neither the
// textarea onChange nor RichComposerInput.onEdit sees this path.
handleUserEdit()
if (richMentionsEnabled && richInputRef.current) {
// insert*/apply* emit mirror state via onMirrorChange (keep inputState in mirror space).
if (suggestion.sessionMention) {
@@ -553,7 +667,7 @@ export function HappyComposer(props: {
}, 0)
haptic('light')
}, [api, suggestions, inputState, autocompletePrefixes, haptic, richMentionsEnabled])
}, [api, suggestions, inputState, autocompletePrefixes, haptic, richMentionsEnabled, handleUserEdit])
const abortDisabled = controlsDisabled || isAborting || !threadIsRunning
const switchDisabled = controlsDisabled || isSwitching || !controlledByUser
@@ -662,13 +776,46 @@ export function HappyComposer(props: {
[permissionModeOptions]
)
const handleUserSchedule = useCallback((nextPendingSchedule: PendingSchedule) => {
userScheduleGenerationRef.current += 1
if (sendError) onClearSendError?.()
setPendingSchedule(nextPendingSchedule)
}, [onClearSendError, sendError, setPendingSchedule])
const handleUserClearSchedule = useCallback(() => {
userScheduleGenerationRef.current += 1
if (sendError) onClearSendError?.()
if (isControlled) {
onClearScheduleProp?.()
} else {
setPendingScheduleLocal(null)
}
}, [isControlled, onClearScheduleProp, onClearSendError, sendError])
// Preserve the original controlled-mode contract: without a parent clear
// handler the schedule button opens the picker instead of claiming it can
// clear a value it does not own.
const onUserClearSchedule = isControlled && !onClearScheduleProp
? undefined
: handleUserClearSchedule
/** Flush rich chips → `[title](/sessions/<id>)` into composer.text, then send. */
const flushAndSend = useCallback(() => {
if (richMentionsEnabled && richInputRef.current) {
richInputRef.current.flushSerializedText()
}
// 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
// restore watcher before the clear so it cannot consume that error.
restoredErrorSnapshotRef.current = null
if (sendError) onSuppressSendErrorRestore?.(sendError.id)
sendRestoreGuardRef.current = {
userEditGeneration: userEditGenerationRef.current,
userScheduleGeneration: userScheduleGenerationRef.current,
userAttachmentGeneration: userAttachmentGenerationRef.current,
}
api.composer().send()
}, [api, richMentionsEnabled])
}, [api, attachments, onSuppressSendErrorRestore, richMentionsEnabled, sendError])
const handleKeyDown = useCallback((e: ReactKeyboardEvent<HTMLTextAreaElement | HTMLDivElement>) => {
const key = e.key
@@ -791,13 +938,8 @@ export function HappyComposer(props: {
end: e.target.selectionEnd
}
setInputState({ text: e.target.value, selection })
// Editing the restored text is the operator's "I'm handling it"
// signal -- drop the inline error so the affordance doesn't shout
// at them while they fix the message.
if (sendError && onClearSendError) {
onClearSendError()
}
}, [sendError, onClearSendError])
handleUserEdit()
}, [handleUserEdit])
const handleSelect = useCallback((e: ReactSyntheticEvent<HTMLTextAreaElement>) => {
const target = e.target as HTMLTextAreaElement
@@ -1518,8 +1660,8 @@ export function HappyComposer(props: {
onVoiceMicToggle={dictationActive ? undefined : onVoiceMicToggle}
onSend={handleSend}
pendingSchedule={pendingSchedule}
onSchedule={setPendingSchedule}
onClearSchedule={isControlled ? onClearScheduleProp : () => setPendingScheduleLocal(null)}
onSchedule={handleUserSchedule}
onClearSchedule={onUserClearSchedule}
hasAttachments={hasAttachments}
piModelLabel={piModelLabel}
piModelDisabled={controlsDisabled || !piHasModels}
@@ -375,3 +375,49 @@ describe('ScratchlistPanel', () => {
expect(screen.queryByTestId('scratchlist-entry-age')).toBeNull()
})
})
describe('ScratchlistDrawer disabled operations', () => {
it('disables and synchronously ignores move, delete, and promote actions while the parent send is pending', async () => {
const { ScratchlistDrawer } = await import('./ScratchlistPanel')
const entry = makeEntry({ id: 'pending-entry', text: 'held message' })
const onMove = vi.fn()
const onDelete = vi.fn()
const onPromoteToComposer = vi.fn()
const onPromoteToQueue = vi.fn(async () => true)
render(
<I18nProvider>
<ScratchlistDrawer
entries={[entry]}
sessionId={SID}
api={{} as never}
disabled
onMove={onMove}
onDelete={onDelete}
onPromoteToComposer={onPromoteToComposer}
onPromoteToQueue={onPromoteToQueue}
/>
</I18nProvider>,
)
const mutationButtons = [
...screen.getAllByRole('button', { name: 'Move entry up' }),
...screen.getAllByRole('button', { name: 'Move entry down' }),
screen.getByRole('button', { name: 'Copy into composer' }),
screen.getByRole('button', { name: 'Send to queue' }),
screen.getByRole('button', { name: 'Delete entry' }),
]
for (const button of mutationButtons) {
expect(button).toBeDisabled()
fireEvent.click(button)
}
// Copy is read-only and remains available while a chat send is pending.
expect(screen.getByRole('button', { name: 'Copy text to clipboard (not images)' })).not.toBeDisabled()
await Promise.resolve()
expect(onMove).not.toHaveBeenCalled()
expect(onDelete).not.toHaveBeenCalled()
expect(onPromoteToComposer).not.toHaveBeenCalled()
expect(onPromoteToQueue).not.toHaveBeenCalled()
})
})
@@ -313,6 +313,7 @@ function ScratchlistInventory({
onMove,
sessionId,
api,
disabled = false,
}: {
entries: ScratchlistEntry[]
busyEntryId: string | null
@@ -322,6 +323,7 @@ function ScratchlistInventory({
onMove: (entry: ScratchlistEntry, direction: 'up' | 'down') => void
sessionId?: string
api?: ApiClient
disabled?: boolean
}) {
const { t } = useTranslation()
const { copiedEntryId, signalCopied } = useCopiedFeedback()
@@ -351,6 +353,7 @@ function ScratchlistInventory({
const isFirst = index === 0
const isLast = index === entries.length - 1
const isBusy = busyEntryId === entry.id
const mutationsDisabled = disabled || isBusy
return (
<li
key={entry.id}
@@ -383,7 +386,7 @@ function ScratchlistInventory({
aria-label={t('scratchlist.action.moveUp')}
title={t('scratchlist.action.moveUp')}
onClick={() => onMove(entry, 'up')}
disabled={isFirst || isBusy}
disabled={isFirst || mutationsDisabled}
className="flex h-6 w-6 items-center justify-center rounded hover:bg-[var(--app-subtle-bg)] hover:text-[var(--app-fg)] disabled:cursor-not-allowed disabled:opacity-30"
>
<ArrowUpIcon />
@@ -393,7 +396,7 @@ function ScratchlistInventory({
aria-label={t('scratchlist.action.moveDown')}
title={t('scratchlist.action.moveDown')}
onClick={() => onMove(entry, 'down')}
disabled={isLast || isBusy}
disabled={isLast || mutationsDisabled}
className="flex h-6 w-6 items-center justify-center rounded hover:bg-[var(--app-subtle-bg)] hover:text-[var(--app-fg)] disabled:cursor-not-allowed disabled:opacity-30"
>
<ArrowDownIcon />
@@ -403,7 +406,7 @@ function ScratchlistInventory({
aria-label={t('scratchlist.action.promoteToComposer')}
title={t('scratchlist.action.promoteToComposer')}
onClick={() => onPromoteToComposer(entry)}
disabled={isBusy}
disabled={mutationsDisabled}
className="flex h-6 w-6 items-center justify-center rounded hover:bg-[var(--app-subtle-bg)] hover:text-[var(--app-fg)] disabled:cursor-not-allowed disabled:opacity-30"
>
<PencilIcon />
@@ -413,7 +416,7 @@ function ScratchlistInventory({
aria-label={t('scratchlist.action.promoteToQueue')}
title={t('scratchlist.action.promoteToQueue')}
onClick={() => onPromoteToQueue(entry)}
disabled={isBusy}
disabled={mutationsDisabled}
className="flex h-6 w-6 items-center justify-center rounded hover:bg-[var(--app-subtle-bg)] hover:text-[var(--app-fg)] disabled:cursor-not-allowed disabled:opacity-30"
>
<SendIcon />
@@ -442,7 +445,7 @@ function ScratchlistInventory({
aria-label={t('scratchlist.action.delete')}
title={t('scratchlist.action.delete')}
onClick={() => onDelete(entry)}
disabled={isBusy}
disabled={mutationsDisabled}
className="flex h-6 w-6 items-center justify-center rounded hover:bg-[var(--app-subtle-bg)] hover:text-[var(--app-fg)] disabled:cursor-not-allowed disabled:opacity-30"
>
<TrashIcon />
@@ -471,6 +474,7 @@ export function ScratchlistDrawer({
onPromoteToQueue,
sessionId,
api,
disabled = false,
}: {
entries: ScratchlistEntry[]
onMove: (id: string, direction: 'up' | 'down') => void
@@ -479,6 +483,7 @@ export function ScratchlistDrawer({
onPromoteToQueue: (entry: ScratchlistEntry) => Promise<boolean>
sessionId: string
api: ApiClient
disabled?: boolean
}) {
const { t } = useTranslation()
const [busyEntryId, setBusyEntryId] = useState<string | null>(null)
@@ -490,6 +495,7 @@ export function ScratchlistDrawer({
}, [entries.length, t])
const handleDelete = useCallback((entry: ScratchlistEntry) => {
if (disabled) return
if (shouldConfirmDelete(entry)) {
const confirmed = typeof window !== 'undefined'
? window.confirm(t('scratchlist.confirmDelete'))
@@ -497,18 +503,20 @@ export function ScratchlistDrawer({
if (!confirmed) return
}
onDelete(entry.id)
}, [onDelete, t])
}, [disabled, onDelete, t])
const handleMove = useCallback((entry: ScratchlistEntry, direction: 'up' | 'down') => {
if (disabled) return
onMove(entry.id, direction)
}, [onMove])
}, [disabled, onMove])
const handlePromoteToComposer = useCallback((entry: ScratchlistEntry) => {
if (disabled) return
void onPromoteToComposer(entry)
}, [onPromoteToComposer])
}, [disabled, onPromoteToComposer])
const handlePromoteToQueue = useCallback(async (entry: ScratchlistEntry) => {
if (busyEntryId) return
if (disabled || busyEntryId) return
setBusyEntryId(entry.id)
try {
const accepted = await onPromoteToQueue(entry)
@@ -516,7 +524,7 @@ export function ScratchlistDrawer({
} finally {
setBusyEntryId(null)
}
}, [busyEntryId, onDelete, onPromoteToQueue])
}, [busyEntryId, disabled, onDelete, onPromoteToQueue])
return (
<div className="mx-auto w-full max-w-content mb-1">
@@ -549,6 +557,7 @@ export function ScratchlistDrawer({
busyEntryId={busyEntryId}
sessionId={sessionId}
api={api}
disabled={disabled}
onPromoteToComposer={handlePromoteToComposer}
onPromoteToQueue={handlePromoteToQueue}
onDelete={handleDelete}
+9 -2
View File
@@ -333,9 +333,11 @@ export function ScratchlistDrawerHost(props: {
onDelete: ReturnType<typeof useHubScratchlist>['remove']
onSend: (text: string, attachments?: AttachmentMetadata[], scheduledAt?: number | null) => Promise<boolean>
onExitScratchlistMode: () => void
disabled?: boolean
}) {
const assistantApi = useAui()
const handlePromoteToComposer = useCallback(async (entry: ScratchlistEntry) => {
if (props.disabled) return
assistantApi.composer().setText(entry.text)
// Exit scratchlist mode before rehydrating attachments so addAttachment
// uses the normal chat upload adapter (not the scratchlist hub adapter).
@@ -350,8 +352,9 @@ export function ScratchlistDrawerHost(props: {
assistantApi.composer()
)
}
}, [assistantApi, props.api, props.onExitScratchlistMode, props.sessionId])
}, [assistantApi, props.api, props.disabled, props.onExitScratchlistMode, props.sessionId])
const handlePromoteToQueue = useCallback(async (entry: ScratchlistEntry) => {
if (props.disabled) return false
let attachments: AttachmentMetadata[] | undefined
if (entry.attachments && entry.attachments.length > 0) {
attachments = await stageScratchlistAttachmentsForComposeSend(
@@ -365,7 +368,7 @@ export function ScratchlistDrawerHost(props: {
props.onExitScratchlistMode()
}
return accepted
}, [props.api, props.onSend, props.onExitScratchlistMode, props.sessionId])
}, [props.api, props.disabled, props.onSend, props.onExitScratchlistMode, props.sessionId])
return (
<ScratchlistDrawer
entries={props.entries}
@@ -375,6 +378,7 @@ export function ScratchlistDrawerHost(props: {
onDelete={props.onDelete}
onPromoteToComposer={handlePromoteToComposer}
onPromoteToQueue={handlePromoteToQueue}
disabled={props.disabled}
/>
)
}
@@ -435,6 +439,7 @@ type SessionChatProps = {
// user dismisses or starts editing.
sendError?: ComposerSendError | null
onClearSendError?: () => void
onSuppressSendErrorRestore?: (id: number) => void
initialOutlineOpen?: boolean
onInitialOutlineConsumed?: () => void
}
@@ -1413,6 +1418,7 @@ function SessionChatInner(props: SessionChatProps) {
onDelete={scratchlist.remove}
onSend={props.onSend}
onExitScratchlistMode={() => setScratchlistMode(false)}
disabled={props.isSending}
/>
) : null}
<QueuedMessagesBar
@@ -1579,6 +1585,7 @@ function SessionChatInner(props: SessionChatProps) {
onScratchlistToggle={handleScratchlistToggle}
sendError={props.sendError ?? null}
onClearSendError={props.onClearSendError}
onSuppressSendErrorRestore={props.onSuppressSendErrorRestore}
/>
</div>
</DragDropZone>
@@ -172,8 +172,9 @@ describe('useSendMessage', () => {
await waitFor(() => {
expect(onError).toHaveBeenCalledTimes(1)
})
const info = onError.mock.calls[0][0] as { text: string; error: unknown }
const info = onError.mock.calls[0][0] as { text: string; error: unknown; mutationStarted: boolean }
expect(info.text).toBe('keep this text on 503')
expect(info.mutationStarted).toBe(true)
expect(info.error).toBeInstanceOf(Error)
expect((info.error as Error).message).toContain('503')
expect(onSuccess).not.toHaveBeenCalled()
@@ -199,8 +200,9 @@ describe('useSendMessage', () => {
await waitFor(() => {
expect(onError).toHaveBeenCalledTimes(1)
})
const info = onError.mock.calls[0][0] as { text: string; error: unknown }
const info = onError.mock.calls[0][0] as { text: string; error: unknown; mutationStarted: boolean }
expect(info.text).toBe('keep this on a dropped fetch')
expect(info.mutationStarted).toBe(true)
expect(info.error).toBeInstanceOf(TypeError)
})
@@ -594,8 +596,9 @@ describe('useSendMessage', () => {
await waitFor(() => {
expect(onError).toHaveBeenCalledTimes(1)
})
const info = onError.mock.calls[0][0] as { text: string; error: unknown; sessionId: string }
const info = onError.mock.calls[0][0] as { text: string; error: unknown; sessionId: string; mutationStarted: boolean }
expect(info.text).toBe('hello inactive')
expect(info.mutationStarted).toBe(true)
expect(info.sessionId).toBe('session-A')
expect(info.error).toBeInstanceOf(ApiError)
const apiErr = info.error as ApiError
@@ -629,8 +632,9 @@ describe('useSendMessage', () => {
await waitFor(() => {
expect(onError).toHaveBeenCalledTimes(1)
})
const info = onError.mock.calls[0][0] as { text: string; error: unknown; sessionId: string }
const info = onError.mock.calls[0][0] as { text: string; error: unknown; sessionId: string; mutationStarted: boolean }
expect(info.text).toBe('hello pre-mutation')
expect(info.mutationStarted).toBe(false)
// Keyed by the ORIGINAL sessionId: pre-mutation never navigated.
expect(info.sessionId).toBe('session-A')
expect(info.error).toBe(resumeError)
+6 -2
View File
@@ -57,6 +57,8 @@ export type SendErrorInfo = {
text: string
error: unknown
scheduledAt: number | null
/** True only after the message mutation was started. */
mutationStarted: boolean
}
type UseSendMessageOptions = {
@@ -197,7 +199,8 @@ export function useSendMessage(
sessionId: input.sessionId,
text: input.text,
error,
scheduledAt: input.scheduledAt ?? null
scheduledAt: input.scheduledAt ?? null,
mutationStarted: true,
})
},
})
@@ -246,7 +249,8 @@ export function useSendMessage(
sessionId,
text,
error,
scheduledAt: scheduledAt ?? null
scheduledAt: scheduledAt ?? null,
mutationStarted: false,
})
return false
} finally {
+144 -5
View File
@@ -1,5 +1,6 @@
import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'
import { renderHook, act } from '@testing-library/react'
import { renderHook, act, render, screen } from '@testing-library/react'
import { createElement, useEffect, useState } from 'react'
// Mock composer-drafts module
vi.mock('@/lib/composer-drafts', () => ({
@@ -51,15 +52,18 @@ describe('useComposerDraft', () => {
mockGetDraft.mockReturnValue('saved text')
const setText = vi.fn()
renderHook(() => useComposerDraft('session-1', '', [], true, setText, vi.fn()))
const { result } = renderHook(() => useComposerDraft('session-1', '', [], true, setText, vi.fn()))
// Before rAF fires, setText should not have been called
// Before rAF fires, setText should not have been called and hydration
// must prevent failed-send recovery from racing ahead of persistence.
expect(setText).not.toHaveBeenCalled()
expect(result.current).toEqual({ sessionId: 'session-1', complete: false, restoredAny: false })
// Flush rAF
// Flush rAF + attachment hydration.
await act(async () => flushRAF())
expect(mockGetDraft).toHaveBeenCalledWith('session-1')
expect(setText).toHaveBeenCalledWith('saved text')
expect(result.current).toEqual({ sessionId: 'session-1', complete: true, restoredAny: true })
})
it('does not restore draft if composer already has text', async () => {
@@ -138,10 +142,11 @@ describe('useComposerDraft', () => {
mockGetDraftAttachments.mockResolvedValue([file])
const addAttachment = vi.fn(async () => {})
renderHook(() => useComposerDraft('session-1', '', [], true, vi.fn(), addAttachment))
const { result } = renderHook(() => useComposerDraft('session-1', '', [], true, vi.fn(), addAttachment))
await act(async () => flushRAF())
expect(addAttachment).toHaveBeenCalledWith(file)
expect(result.current).toEqual({ sessionId: 'session-1', complete: true, restoredAny: true })
})
it('does not duplicate saved attachments when the composer already has files', async () => {
@@ -171,4 +176,138 @@ describe('useComposerDraft', () => {
expect(addAttachment).not.toHaveBeenCalled()
expect(mockSaveDraftAttachments).not.toHaveBeenCalled()
})
it('reports immediate complete hydration when no session exists', () => {
const { result } = renderHook(() => useComposerDraft(undefined, '', [], true, vi.fn(), vi.fn()))
expect(result.current).toEqual({ sessionId: undefined, complete: true, restoredAny: false })
})
it('returns to pending hydration when the session changes', async () => {
const { result, rerender } = renderHook(
({ sessionId }) => useComposerDraft(sessionId, '', [], false, vi.fn(), vi.fn()),
{ initialProps: { sessionId: 'session-1' as string | undefined } },
)
await act(async () => flushRAF())
expect(result.current).toEqual({ sessionId: 'session-1', complete: true, restoredAny: false })
rerender({ sessionId: 'session-2' })
expect(result.current).toEqual({ sessionId: 'session-2', complete: false, restoredAny: false })
await act(async () => flushRAF())
expect(result.current).toEqual({ sessionId: 'session-2', complete: true, restoredAny: false })
})
it('lets persisted replacement hydration win over an implicit failed-send restore', async () => {
mockGetDraft.mockReturnValue('persisted replacement')
function DraftVsImplicitRestore() {
const [text, setText] = useState('')
const [errorCleared, setErrorCleared] = useState(false)
const hydration = useComposerDraft('session-race', text, [], false, setText, vi.fn())
// Mirrors HappyComposer's guard===null branch: no old error text
// may be written until this session's hydration is complete.
useEffect(() => {
if (hydration.sessionId !== 'session-race' || !hydration.complete) return
if (hydration.restoredAny) {
setErrorCleared(true)
return
}
setText('stale failed-send text')
}, [hydration])
return createElement('output', { 'data-testid': 'draft-race' }, `${text}|${errorCleared}`)
}
render(createElement(DraftVsImplicitRestore))
expect(screen.getByTestId('draft-race')).toHaveTextContent('|false')
await act(async () => flushRAF())
expect(screen.getByTestId('draft-race')).toHaveTextContent('persisted replacement|true')
expect(screen.getByTestId('draft-race')).not.toHaveTextContent('stale failed-send text')
})
it('ignores a deferred old-session attachment restore after the session changes', async () => {
let resolveOldFiles: ((files: File[]) => void) | undefined
const oldFiles = new Promise<File[]>((resolve) => { resolveOldFiles = resolve })
const oldAttachment = new File(['old'], 'old.png', { type: 'image/png' })
mockGetDraftAttachments.mockImplementation((sessionId) => (
sessionId === 'session-1' ? oldFiles : Promise.resolve([])
))
const addAttachment = vi.fn(async () => {})
const { result, rerender } = renderHook(
({ sessionId }) => useComposerDraft(sessionId, '', [], true, vi.fn(), addAttachment),
{ initialProps: { sessionId: 'session-1' } },
)
await act(async () => flushRAF())
expect(result.current).toEqual({ sessionId: 'session-1', complete: false, restoredAny: false })
rerender({ sessionId: 'session-2' })
expect(result.current).toEqual({ sessionId: 'session-2', complete: false, restoredAny: false })
await act(async () => {
resolveOldFiles!([oldAttachment])
await Promise.resolve()
await Promise.resolve()
})
expect(result.current).toEqual({ sessionId: 'session-2', complete: false, restoredAny: false })
expect(addAttachment).not.toHaveBeenCalled()
await act(async () => flushRAF())
expect(result.current).toEqual({ sessionId: 'session-2', complete: true, restoredAny: false })
})
it('does not mark hydration restored when every saved attachment rejects', async () => {
const file = new File(['broken'], 'broken.png', { type: 'image/png' })
mockGetDraftAttachments.mockResolvedValue([file])
const addAttachment = vi.fn(async () => { throw new Error('upload failed') })
const { result } = renderHook(() => useComposerDraft('session-1', '', [], true, vi.fn(), addAttachment))
await act(async () => flushRAF())
expect(addAttachment).toHaveBeenCalledWith(file)
expect(result.current).toEqual({ sessionId: 'session-1', complete: true, restoredAny: false })
})
it('marks hydration restored when at least one saved attachment succeeds', async () => {
const rejected = new File(['broken'], 'broken.png', { type: 'image/png' })
const restored = new File(['ok'], 'ok.png', { type: 'image/png' })
mockGetDraftAttachments.mockResolvedValue([rejected, restored])
const addAttachment = vi.fn(async (file: File) => {
if (file === rejected) throw new Error('upload failed')
})
const { result } = renderHook(() => useComposerDraft('session-1', '', [], true, vi.fn(), addAttachment))
await act(async () => flushRAF())
expect(addAttachment).toHaveBeenCalledTimes(2)
expect(result.current).toEqual({ sessionId: 'session-1', complete: true, restoredAny: true })
})
it('allows implicit failed-send restoration when every persisted attachment fails', async () => {
const file = new File(['broken'], 'broken.png', { type: 'image/png' })
mockGetDraftAttachments.mockResolvedValue([file])
function AttachmentFailureVsImplicitRestore() {
const [text, setText] = useState('')
const [errorCleared, setErrorCleared] = useState(false)
const hydration = useComposerDraft('session-race', text, [], true, setText, async () => {
throw new Error('upload failed')
})
useEffect(() => {
if (hydration.sessionId !== 'session-race' || !hydration.complete) return
if (hydration.restoredAny) {
setErrorCleared(true)
return
}
setText('failed-send text')
}, [hydration])
return createElement('output', { 'data-testid': 'attachment-race' }, `${text}|${errorCleared}`)
}
render(createElement(AttachmentFailureVsImplicitRestore))
await act(async () => flushRAF())
expect(screen.getByTestId('attachment-race')).toHaveTextContent('failed-send text|false')
})
})
+83 -16
View File
@@ -1,4 +1,4 @@
import { useEffect, useRef } from 'react'
import { useEffect, useRef, useState } from 'react'
import { getDraft, saveDraft } from '@/lib/composer-drafts'
import {
getDraftAttachments,
@@ -6,6 +6,14 @@ import {
type AttachmentDraftInput,
} from '@/lib/composer-attachment-drafts'
export type ComposerDraftHydration = {
/** Session represented by this status; prevents a previous session's ready state leaking across a key change. */
sessionId: string | undefined
complete: boolean
/** True when this hydration found and applied a persisted text or attachment draft. */
restoredAny: boolean
}
/**
* Manages draft save/restore lifecycle for a composer.
*
@@ -14,6 +22,10 @@ import {
* - On unmount: saves current text and attachment files as a draft
* - The `draftReady` guard prevents saving before the initial restore completes,
* avoiding the case where the runtime's empty initial text overwrites a real draft.
*
* The returned status is deliberately session-keyed. Consumers that must not
* overwrite persisted drafts (for example failed-send recovery after a keyed
* remount) can wait until `complete` and then respect `restoredAny`.
*/
export function useComposerDraft(
sessionId: string | undefined,
@@ -22,7 +34,7 @@ export function useComposerDraft(
canRestoreAttachments: boolean,
setText: (text: string) => void,
addAttachment: (file: File) => Promise<void>,
): void {
): ComposerDraftHydration {
const composerTextRef = useRef(composerText)
composerTextRef.current = composerText
const attachmentsRef = useRef(attachments)
@@ -30,31 +42,84 @@ export function useComposerDraft(
const draftReadyRef = useRef(false)
const attachmentsReadyRef = useRef(false)
const [hydration, setHydration] = useState<ComposerDraftHydration>(() => ({
sessionId,
complete: sessionId === undefined,
restoredAny: false,
}))
useEffect(() => {
if (!sessionId) return
if (!sessionId) {
setHydration({ sessionId: undefined, complete: true, restoredAny: false })
return
}
draftReadyRef.current = false
attachmentsReadyRef.current = false
setHydration({ sessionId, complete: false, restoredAny: false })
let disposed = false
const frame = requestAnimationFrame(() => {
const draft = getDraft(sessionId)
if (draft && !composerTextRef.current) {
setText(draft)
const restoreText = Boolean(draft && !composerTextRef.current)
if (restoreText) {
// Mark before the external composer store gets its render so a
// consumer never mistakes this persisted replacement for empty.
setHydration({ sessionId, complete: !canRestoreAttachments, restoredAny: true })
setText(draft!)
}
draftReadyRef.current = true
if (canRestoreAttachments) {
void getDraftAttachments(sessionId).then(async (files) => {
if (!disposed && attachmentsRef.current.length === 0) {
for (const file of files) {
if (disposed) break
if (!canRestoreAttachments) {
if (!restoreText) setHydration({ sessionId, complete: true, restoredAny: false })
return
}
void getDraftAttachments(sessionId).then(async (files) => {
// The promise belongs to this session's effect. A later keyed
// session can already be hydrating when it settles, so never
// publish old status or rehydrate old files after disposal.
if (disposed) return
const restoreAttachments = attachmentsRef.current.length === 0 && files.length > 0
// Text is already known to be restored; attachment presence by
// itself is not. An upload can fail, so only successful adds
// contribute to restoredAny in the final completion update.
setHydration((current) => current.sessionId === sessionId
? {
sessionId,
complete: false,
restoredAny: restoreText || current.restoredAny,
}
: current)
let restoredAttachment = false
if (restoreAttachments) {
for (const file of files) {
if (disposed) break
try {
await addAttachment(file)
restoredAttachment = true
} catch {
// Continue restoring remaining files; one failed
// attachment must not discard a successful sibling.
}
}
}).catch(() => {
// Attachment draft restoration is best effort.
}).finally(() => {
if (!disposed) attachmentsReadyRef.current = true
})
}
}
return restoredAttachment
}).catch(() => {
// Attachment draft read is best effort.
return false
}).then((restoredAttachment) => {
if (!disposed) {
attachmentsReadyRef.current = true
setHydration((current) => current.sessionId === sessionId
? {
...current,
complete: true,
restoredAny: current.restoredAny || Boolean(restoredAttachment),
}
: current)
}
})
})
return () => {
@@ -70,4 +135,6 @@ export function useComposerDraft(
attachmentsReadyRef.current = false
}
}, [sessionId, canRestoreAttachments]) // eslint-disable-line react-hooks/exhaustive-deps
return hydration
}
+28
View File
@@ -0,0 +1,28 @@
import { describe, expect, it } from 'vitest'
import { migrateSuppressedSendError } from './suppressed-send-error'
type ErrorRecord = { id: number; restoreSuppressed: boolean; label: string }
const suppressed: ErrorRecord = { id: 1, restoreSuppressed: true, label: 'retry A' }
const ordinary: ErrorRecord = { id: 2, restoreSuppressed: false, label: 'ordinary A' }
describe('migrateSuppressedSendError', () => {
it('moves a suppressed retry record from source to resolved session', () => {
expect(migrateSuppressedSendError({ A: suppressed }, 'A', 'B')).toEqual({ B: suppressed })
})
it('does not move an unsuppressed record', () => {
const errors = { A: ordinary }
expect(migrateSuppressedSendError(errors, 'A', 'B')).toBe(errors)
})
it('is a no-op when source and resolved session are the same', () => {
const errors = { A: suppressed }
expect(migrateSuppressedSendError(errors, 'A', 'A')).toBe(errors)
})
it('supersedes a stale target record with the in-flight suppressed retry', () => {
const target: ErrorRecord = { id: 99, restoreSuppressed: true, label: 'stale B' }
expect(migrateSuppressedSendError({ A: suppressed, B: target }, 'A', 'B')).toEqual({ B: suppressed })
})
})
+21
View File
@@ -0,0 +1,21 @@
/**
* Moves a retry-suppressed inline send error to the session selected by an
* inactive-session resume. The record stays visible while the retry runs, but
* must follow the eventual mutation target so success/error can resolve it.
*/
export function migrateSuppressedSendError<T extends { restoreSuppressed: boolean }>(
errors: Readonly<Record<string, T>>,
sourceSessionId: string,
resolvedSessionId: string,
): Record<string, T> {
if (sourceSessionId === resolvedSessionId) return errors as Record<string, T>
const source = errors[sourceSessionId]
if (!source?.restoreSuppressed) return errors as Record<string, T>
const next = { ...errors }
delete next[sourceSessionId]
// The retry being resumed is the authoritative in-flight operation, so it
// intentionally supersedes any stale target-session error record.
next[resolvedSessionId] = source
return next
}
+28 -1
View File
@@ -46,6 +46,7 @@ import { inactiveSessionCanResume } from '@/lib/sessionResume'
import { markSessionSeen } from '@/lib/sessionLastSeen'
import { useSessionBrowserTitle } from '@/hooks/useSessionBrowserTitle'
import { clearCodexImportedSession } from '@/lib/codexImportedSessions'
import { migrateSuppressedSendError } from '@/lib/suppressed-send-error'
import FilesPage from '@/routes/sessions/files'
import FilePage from '@/routes/sessions/file'
import TerminalPage from '@/routes/sessions/terminal'
@@ -369,6 +370,8 @@ function SessionPage() {
message: string
code: string | null
scheduledAt: number | null
mutationStarted: boolean
restoreSuppressed: boolean
}
const [sendErrors, setSendErrors] = useState<Record<string, RawSendError>>({})
const [reopeningSessionId, setReopeningSessionId] = useState<string | null>(null)
@@ -382,6 +385,17 @@ function SessionPage() {
})
}, [sessionId])
const suppressSendErrorRestore = useCallback((id: number) => {
setSendErrors((prev) => {
const current = prev[sessionId]
if (!current || current.id !== id || current.restoreSuppressed) return prev
return {
...prev,
[sessionId]: { ...current, restoreSuppressed: true }
}
})
}, [sessionId])
// Reopen recovery (#918): one-click affordance attached to the inline
// composer error when the rejected send was inactive-session. Mirrors
// SessionList's Reopen UX -- POST /sessions/:id/reopen via
@@ -443,6 +457,8 @@ function SessionPage() {
text: rawSendError.text,
message: rawSendError.message,
scheduledAt: rawSendError.scheduledAt,
mutationStarted: rawSendError.mutationStarted,
restoreSuppressed: rawSendError.restoreSuppressed,
action: rawSendError.code === 'session_inactive' && canOfferInactiveReopen
? {
label: t('chat.sendError.sessionInactive.action'),
@@ -482,7 +498,9 @@ function SessionPage() {
text: info.text,
message,
code,
scheduledAt: info.scheduledAt
scheduledAt: info.scheduledAt,
mutationStarted: info.mutationStarted,
restoreSuppressed: false,
}
}))
},
@@ -522,6 +540,14 @@ function SessionPage() {
}
},
onSessionResolved: (resolvedSessionId) => {
// A direct retry retains its old alert with restoreSuppressed=true.
// Move it to the target session before navigation so the mutation's
// onSuccess/onError can clear or replace the same record.
setSendErrors((previous) => migrateSuppressedSendError(
previous,
sessionId,
resolvedSessionId,
))
void (async () => {
if (api) {
if (session) {
@@ -721,6 +747,7 @@ function SessionPage() {
availableSlashCommands={slashCommands}
sendError={sendError}
onClearSendError={clearSendError}
onSuppressSendErrorRestore={suppressSendErrorRestore}
initialOutlineOpen={outline}
onInitialOutlineConsumed={handleInitialOutlineConsumed}
/>