Skip to content

Commit 10dcc32

Browse files
heavygeecursoragent
andcommitted
fix(web): remove failed optimistic row on send error
Address PR review (Major #2): the mutation onError was leaving the optimistic message in the visible thread as `failed`, which duplicated the restored composer text -- operator saw a failed bubble plus the same text in the composer, and could stack a stale failed turn next to a fresh send. The composer-restore + inline alert is the single retry surface for this failure mode now, so the optimistic row is dropped via removeOptimisticMessage() in the same handler. retryMessage stays exposed for any future code path that still produces failed rows. Test pins the new behaviour: the row is removed (not flipped to failed) on send failure. Co-authored-by: Cursor <cursoragent@cursor.com>
1 parent 2b48cf4 commit 10dcc32

2 files changed

Lines changed: 43 additions & 1 deletion

File tree

web/src/hooks/mutations/useSendMessage.test.tsx

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ vi.mock('@/lib/message-window-store', () => ({
99
appendOptimisticMessage: vi.fn(),
1010
getMessageWindowState: vi.fn(() => ({ messages: [], pending: [] })),
1111
updateMessageStatus: vi.fn(),
12+
removeOptimisticMessage: vi.fn(),
1213
}))
1314

1415
vi.mock('@/hooks/usePlatform', () => ({
@@ -278,6 +279,38 @@ describe('useSendMessage', () => {
278279
const info = onError.mock.calls[0][0] as { scheduledAt: number | null }
279280
expect(info.scheduledAt).toBeNull()
280281
})
282+
283+
it('removes the optimistic row on failure so the composer-restore path is the single retry surface', async () => {
284+
// Without this, the thread keeps a stale `failed` bubble next to
285+
// the restored composer text, and the operator can stack a
286+
// duplicate by retrying from either surface.
287+
const onError = vi.fn()
288+
const api = createMockApi(async () => {
289+
throw new Error('HTTP 503')
290+
})
291+
292+
const { removeOptimisticMessage, updateMessageStatus } = await import('@/lib/message-window-store')
293+
const removeMock = vi.mocked(removeOptimisticMessage)
294+
const updateMock = vi.mocked(updateMessageStatus)
295+
296+
const { result } = renderHook(
297+
() => useSendMessage(api, 'session-A', { onError }),
298+
{ wrapper: createWrapper() },
299+
)
300+
301+
act(() => {
302+
result.current.sendMessage('hello')
303+
})
304+
305+
await waitFor(() => {
306+
expect(onError).toHaveBeenCalledTimes(1)
307+
})
308+
// The optimistic row is removed instead of being kept as failed.
309+
expect(removeMock).toHaveBeenCalledWith('session-A', 'local-id-1')
310+
// Defensive: nothing else should have transitioned the row to
311+
// 'failed' on this path -- we removed it outright.
312+
expect(updateMock.mock.calls.some((call) => call[2] === 'failed')).toBe(false)
313+
})
281314
})
282315

283316
it('does not call onSuccess when blocked', () => {

web/src/hooks/mutations/useSendMessage.ts

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import { makeClientSideId } from '@/lib/messages'
66
import {
77
appendOptimisticMessage,
88
getMessageWindowState,
9+
removeOptimisticMessage,
910
updateMessageStatus,
1011
} from '@/lib/message-window-store'
1112
import { usePlatform } from '@/hooks/usePlatform'
@@ -137,7 +138,15 @@ export function useSendMessage(
137138
options?.onSuccess?.(input.sessionId)
138139
},
139140
onError: (error, input) => {
140-
updateMessageStatus(input.sessionId, input.localId, 'failed')
141+
// Drop the optimistic row from the thread. The composer
142+
// restore path (see onError below) puts the original text
143+
// back into the input + surfaces an inline error, so leaving
144+
// the row as `failed` would visually duplicate the message
145+
// (failed bubble in thread + same text in composer) and let
146+
// the operator stack a stale failed turn next to a fresh
147+
// send. The composer affordance is the single retry surface
148+
// for this failure mode.
149+
removeOptimisticMessage(input.sessionId, input.localId)
141150
haptic.notification('error')
142151
// Surface the original text + scheduledAt so the composer can
143152
// restore both. assistant-ui clears the composer eagerly when

0 commit comments

Comments
 (0)