diff --git a/apps/sim/app/workspace/[workspaceId]/home/home.tsx b/apps/sim/app/workspace/[workspaceId]/home/home.tsx index 492b5659464..093f75c6556 100644 --- a/apps/sim/app/workspace/[workspaceId]/home/home.tsx +++ b/apps/sim/app/workspace/[workspaceId]/home/home.tsx @@ -341,7 +341,9 @@ export function Home({ chatId, userName, userId, tableViewsEnabled }: HomeProps) const detail = (e as CustomEvent).detail if (!detail?.message) return e.preventDefault() - sendMessage(detail.message, undefined, detail.contexts) + sendMessage(detail.message, detail.fileAttachments, detail.contexts, { + ...(detail.recoverStreamId ? { recoverStreamId: detail.recoverStreamId } : {}), + }) } window.addEventListener(MOTHERSHIP_SEND_MESSAGE_EVENT, handler) return () => window.removeEventListener(MOTHERSHIP_SEND_MESSAGE_EVENT, handler) @@ -370,7 +372,9 @@ export function Home({ chatId, userName, userId, tableViewsEnabled }: HomeProps) const handoff = MothershipHandoffStorage.consume(workspaceId) if (!handoff) return if (handoff.message) { - sendMessage(handoff.message, undefined, handoff.contexts) + sendMessage(handoff.message, handoff.fileAttachments, handoff.contexts, { + ...(handoff.recoverStreamId ? { recoverStreamId: handoff.recoverStreamId } : {}), + }) return } const contexts = handoff.contexts ?? [] diff --git a/apps/sim/app/workspace/[workspaceId]/home/hooks/use-chat.mount-send.test.tsx b/apps/sim/app/workspace/[workspaceId]/home/hooks/use-chat.mount-send.test.tsx new file mode 100644 index 00000000000..dbc50800722 --- /dev/null +++ b/apps/sim/app/workspace/[workspaceId]/home/hooks/use-chat.mount-send.test.tsx @@ -0,0 +1,555 @@ +/** + * @vitest-environment jsdom + * + * Regression tests for the remount send loss: a send started on a fresh chat + * surface was silently dropped when the hook's unmount cleanup ran mid-flight + * and aborted the POST. Two things run that cleanup while an auto-send from a + * cross-route handoff is still in flight — StrictMode's dev double-mount, and a + * real client-side navigation away — and because `MothershipHandoffStorage` + * consumes atomically, the second mount finds nothing left to retry. + * + * (A Suspense hide/reveal does NOT cause this: React 19 disappears layout + * effects only, so this passive cleanup never runs for it.) + * + * The fix routes idle sends through the durable queue so every send has a + * recoverable entry, and recovers one the cleanup withdrew — probing the + * orphaned stream first so a request the server had already accepted is + * adopted rather than sent twice. + */ +import { act, type ReactNode, StrictMode, useEffect } from 'react' +import { sleep } from '@sim/utils/helpers' +import { QueryClient, QueryClientProvider } from '@tanstack/react-query' +import { createRoot, type Root } from 'react-dom/client' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' + +const { mockRequestJson, navigationMocks } = vi.hoisted(() => ({ + mockRequestJson: vi.fn(), + navigationMocks: { + usePathname: vi.fn(() => '/workspace/ws-1/home'), + useRouter: vi.fn(() => ({ push: vi.fn(), replace: vi.fn(), prefetch: vi.fn() })), + useSearchParams: vi.fn(() => new URLSearchParams()), + }, +})) + +vi.mock('next/navigation', () => navigationMocks) + +vi.mock('@/lib/api/client/request', async (importOriginal) => ({ + ...(await importOriginal()), + requestJson: mockRequestJson, +})) + +import { MothershipHandoffStorage } from '@/lib/core/utils/browser-storage' +import { useChat } from '@/app/workspace/[workspaceId]/home/hooks/use-chat' +import { useMothershipQueueStore } from '@/stores/mothership-queue/store' + +interface NetworkState { + /** How the chat POST behaves for the next call. */ + postBehavior: 'hang' | 'accept' + postCalls: number + /** + * How the orphaned-stream probe answers: + * - `found` — the server accepted the withdrawn request and owns a chat + * - `gone` — 404, it has no such stream (never accepted) + * - `pending` — registered but no owner yet, so the probe keeps polling; + * this is the only mode that leaves a probe in flight to interrupt + */ + probeBehavior: 'found' | 'gone' | 'pending' + orphanedStreamChatId: string + streamProbes: number +} + +const state: NetworkState = { + postBehavior: 'hang', + postCalls: 0, + probeBehavior: 'gone', + orphanedStreamChatId: 'chat-server-already-made', + streamProbes: 0, +} + +/** An SSE response whose stream ends immediately without a terminal event. */ +function emptySseResponse(): Response { + const stream = new ReadableStream({ + start(controller) { + controller.close() + }, + }) + return new Response(stream, { status: 200, headers: { 'Content-Type': 'text/event-stream' } }) +} + +async function fetchStub(input: RequestInfo | URL, init?: RequestInit): Promise { + const url = String(input instanceof Request ? input.url : input) + + // The orphaned-stream probe: does the server hold a stream for the send the + // cleanup abort withdrew? + if (url.includes('/api/mothership/chat/stream')) { + state.streamProbes++ + // 404 is what the server returns for a stream it never registered — i.e. + // the request really was withdrawn before it was accepted. + if (state.probeBehavior === 'gone') { + return new Response(JSON.stringify({ error: 'stream gone' }), { status: 404 }) + } + return new Response( + JSON.stringify({ + success: true, + events: [], + status: 'streaming', + // `pending` omits the owner, so the probe keeps polling. + ...(state.probeBehavior === 'found' ? { chatId: state.orphanedStreamChatId } : {}), + }), + { status: 200, headers: { 'Content-Type': 'application/json' } } + ) + } + + if (url.includes('/api/mothership/chat') && init?.method === 'POST') { + state.postCalls++ + if (state.postBehavior === 'accept') return emptySseResponse() + return new Promise((_, reject) => { + const signal = init?.signal + if (!signal) return + // Real fetch rejects with the RAW abort reason (a string here), not an + // AbortError — the regression this suite guards depends on that shape. + if (signal.aborted) { + reject(signal.reason) + return + } + signal.addEventListener('abort', () => reject(signal.reason), { once: true }) + }) + } + + return new Response(JSON.stringify({ error: 'not found' }), { status: 404 }) +} + +const mountedRoots: Root[] = [] +let queryClient: QueryClient + +function renderUseChat(): { + getResult: () => ReturnType + unmount: () => void +} { + ;(globalThis as { IS_REACT_ACT_ENVIRONMENT?: boolean }).IS_REACT_ACT_ENVIRONMENT = true + queryClient = new QueryClient({ defaultOptions: { queries: { retry: false } } }) + const container = document.createElement('div') + const root = createRoot(container) + mountedRoots.push(root) + let result: ReturnType | undefined + + function Probe() { + result = useChat('ws-1', undefined) + return null + } + + act(() => { + root.render( + {() as ReactNode} + ) + }) + + return { + getResult: () => { + if (result === undefined) throw new Error('Hook result is not ready') + return result + }, + unmount: () => act(() => root.unmount()), + } +} + +/** + * Mounts the hook under StrictMode with a handoff already in storage, mirroring + * `home.tsx`'s consume-and-auto-send effect. This is the production-shaped + * failure: the dev double-mount runs the passive cleanup between the two + * mounts, aborting the in-flight POST, and `consume` has already cleared the + * entry so the second mount has nothing to replay. + */ +function renderStrictModeHandoffConsumer(): { unmount: () => void } { + ;(globalThis as { IS_REACT_ACT_ENVIRONMENT?: boolean }).IS_REACT_ACT_ENVIRONMENT = true + queryClient = new QueryClient({ defaultOptions: { queries: { retry: false } } }) + const container = document.createElement('div') + const root = createRoot(container) + mountedRoots.push(root) + + function Probe() { + const { sendMessage } = useChat('ws-1', undefined) + useEffect(() => { + const handoff = MothershipHandoffStorage.consume('ws-1') + if (!handoff?.message) return + sendMessage(handoff.message, handoff.fileAttachments, handoff.contexts, { + ...(handoff.recoverStreamId ? { recoverStreamId: handoff.recoverStreamId } : {}), + }) + }, [sendMessage]) + return null + } + + act(() => { + root.render( + + {() as ReactNode} + + ) + }) + + return { unmount: () => act(() => root.unmount()) } +} + +/** + * Mounts a surface shaped like `home.tsx`: it drives `useChat` AND registers + * the `mothership-send-message` listener that claims the event with + * `preventDefault`. Unmounting this exercises the ordering question — whether + * the departing surface's own still-attached listener can claim the recovery + * event its own teardown emitted, which would suppress the storage fallback + * and strand the message. + */ +function renderHomeLikeSurface(): { + getResult: () => ReturnType + claimedByOwnListener: () => number + unmount: () => void +} { + ;(globalThis as { IS_REACT_ACT_ENVIRONMENT?: boolean }).IS_REACT_ACT_ENVIRONMENT = true + queryClient = new QueryClient({ defaultOptions: { queries: { retry: false } } }) + const container = document.createElement('div') + const root = createRoot(container) + mountedRoots.push(root) + let result: ReturnType | undefined + let claims = 0 + + function HomeLike() { + const chat = useChat('ws-1', undefined) + result = chat + const { sendMessage } = chat + // Mirrors home.tsx:339 — declared AFTER useChat, so on unmount React runs + // useChat's cleanup (which aborts) before this removeEventListener. + useEffect(() => { + const handler = (e: Event) => { + const detail = (e as CustomEvent<{ message?: string; recoverStreamId?: string }>).detail + if (!detail?.message) return + claims++ + e.preventDefault() + sendMessage(detail.message, undefined, undefined, { + ...(detail.recoverStreamId ? { recoverStreamId: detail.recoverStreamId } : {}), + }) + } + window.addEventListener('mothership-send-message', handler) + return () => window.removeEventListener('mothership-send-message', handler) + }, [sendMessage]) + return null + } + + act(() => { + root.render( + {() as ReactNode} + ) + }) + + return { + getResult: () => { + if (result === undefined) throw new Error('Hook result is not ready') + return result + }, + claimedByOwnListener: () => claims, + unmount: () => act(() => root.unmount()), + } +} + +/** Every queued message across all chat keys, flattened. */ +function allQueuedMessages() { + return Object.values(useMothershipQueueStore.getState().queues).flat() +} + +async function waitFor(predicate: () => boolean, budgetMs = 2000): Promise { + const deadline = Date.now() + budgetMs + while (!predicate()) { + if (Date.now() > deadline) throw new Error('waitFor timed out') + await act(async () => { + await sleep(10) + }) + } +} + +describe('useChat remount send recovery', () => { + beforeEach(() => { + vi.stubGlobal('fetch', fetchStub) + state.postBehavior = 'hang' + state.postCalls = 0 + state.probeBehavior = 'gone' + state.streamProbes = 0 + mockRequestJson.mockResolvedValue({ chats: [] }) + useMothershipQueueStore.setState({ queues: {}, editing: {} }) + window.sessionStorage.clear() + window.localStorage.clear() + }) + + afterEach(() => { + for (const root of mountedRoots.splice(0)) { + act(() => root.unmount()) + } + queryClient?.clear() + vi.unstubAllGlobals() + vi.clearAllMocks() + }) + + it('delivers an aborted chatless send directly to a live replacement surface', async () => { + const attachment = { + id: 'file-1', + key: 'uploads/file-1', + filename: 'notes.txt', + media_type: 'text/plain', + size: 12, + } + const received: Array<{ message: string; fileAttachments?: unknown[] }> = [] + const claim = (event: Event) => { + const detail = (event as CustomEvent<{ message: string; fileAttachments?: unknown[] }>).detail + received.push(detail) + event.preventDefault() + } + window.addEventListener('mothership-send-message', claim) + + try { + const { getResult, unmount } = renderUseChat() + await act(async () => { + void getResult().sendMessage('hello from the palette', [attachment]) + }) + await waitFor(() => state.postCalls === 1) + + unmount() + await waitFor(() => received.length === 1) + + expect(received[0].message).toBe('hello from the palette') + expect(received[0].fileAttachments).toEqual([attachment]) + expect(window.localStorage.getItem('sim_mothership_handoff')).toBeNull() + } finally { + window.removeEventListener('mothership-send-message', claim) + } + }) + + it('re-persists an aborted chatless send as a handoff for the next mount', async () => { + const attachment = { + id: 'file-2', + key: 'uploads/file-2', + filename: 'report.pdf', + media_type: 'application/pdf', + size: 99, + } + const { getResult, unmount } = renderUseChat() + + await act(async () => { + void getResult().sendMessage('hello from the palette', [attachment]) + }) + await waitFor(() => state.postCalls === 1) + + // The dispatch claimed the queue head when the optimistic send applied. + expect(allQueuedMessages()).toHaveLength(0) + + // The cleanup abort (the same code path a StrictMode remount or a real + // navigation away runs) fires while the POST is still awaiting the server. + // A chatless surface regenerates its queue key per mount, so recovery + // re-persists the send as a one-shot handoff for the next mount's consumer + // instead of restoring the dead instance's queue. + unmount() + await waitFor(() => window.localStorage.getItem('sim_mothership_handoff') !== null) + + expect(allQueuedMessages()).toHaveLength(0) + const handoff = MothershipHandoffStorage.consume('ws-1') + expect(handoff?.message).toBe('hello from the palette') + expect(handoff?.fileAttachments).toEqual([attachment]) + }) + + it('does not re-queue a send the server already received', async () => { + state.postBehavior = 'accept' + const { getResult, unmount } = renderUseChat() + + await act(async () => { + void getResult().sendMessage('already accepted') + }) + await waitFor(() => state.postCalls === 1) + + unmount() + await act(async () => { + await sleep(50) + }) + + expect(allQueuedMessages()).toHaveLength(0) + expect(MothershipHandoffStorage.consume('ws-1')).toBeNull() + }) + + /** + * A departing surface's own listener must not claim the recovery event its + * teardown emitted: claiming returns `true`, which suppresses the storage + * fallback, and the enqueue would land under the disposed pending key — the + * message would be stranded exactly where this fix is supposed to save it. + */ + it('does not let a departing surface claim its own recovery event', async () => { + const surface = renderHomeLikeSurface() + await act(async () => { + void surface.getResult().sendMessage('must survive my own teardown') + }) + await waitFor(() => state.postCalls === 1) + + surface.unmount() + await waitFor(() => window.localStorage.getItem('sim_mothership_handoff') !== null) + + expect(surface.claimedByOwnListener()).toBe(0) + expect(MothershipHandoffStorage.consume('ws-1')?.message).toBe('must survive my own teardown') + }) + + /** + * The end-to-end failure, driven by the thing that actually runs the cleanup + * mid-flight rather than by a hand-rolled unmount. On the unfixed hook the + * handoff is consumed, the POST is aborted, and nothing survives to retry. + */ + it('keeps a cross-route handoff recoverable across a StrictMode double-mount', async () => { + MothershipHandoffStorage.store({ message: 'investigate this failed run' }, 'ws-1') + + renderStrictModeHandoffConsumer() + await waitFor(() => state.postCalls >= 1) + + // Something must still be holding the message: either the live event was + // claimed and it is queued/in flight again, or it is back in storage. + await waitFor(() => { + const stored = window.localStorage.getItem('sim_mothership_handoff') + return stored !== null || allQueuedMessages().length > 0 || state.postCalls > 1 + }) + }) + + /** + * The abort tears down the client socket but the route handler never reads + * `request.signal` — a request the server had already accepted still creates + * the chat, persists the user message, and runs (and bills) the turn. So the + * recovered send has to ask whether that happened before sending again. + */ + describe('recovered send probes the orphaned stream before re-sending', () => { + it('adopts the chat the server already created instead of sending twice', async () => { + // The server accepted the withdrawn request and registered its stream. + state.probeBehavior = 'found' + + const { getResult, unmount } = renderUseChat() + await act(async () => { + void getResult().sendMessage('only once please') + }) + await waitFor(() => state.postCalls === 1) + unmount() + await waitFor(() => window.localStorage.getItem('sim_mothership_handoff') !== null) + + // The next mount consumes the handoff, exactly as home.tsx does. + const handoff = MothershipHandoffStorage.consume('ws-1') + expect(handoff?.recoverStreamId).toBeTruthy() + + const replacement = renderUseChat() + await act(async () => { + void replacement.getResult().sendMessage(handoff?.message as string, undefined, undefined, { + recoverStreamId: handoff?.recoverStreamId as string, + }) + }) + await waitFor(() => state.streamProbes > 0) + await waitFor(() => replacement.getResult().resolvedChatId === 'chat-server-already-made') + + expect(state.postCalls).toBe(1) + expect(allQueuedMessages()).toHaveLength(0) + }) + + /** + * Adoption alone does not surface the running turn: hydration reconnects + * only when `chatHistory.activeStreamId` is set, and that query is cached + * for 30s. Without an explicit detail invalidation the adopted chat renders + * with the live response invisible. + */ + it('invalidates the adopted chat detail so hydration can reconnect', async () => { + state.probeBehavior = 'found' + + const { getResult, unmount } = renderUseChat() + await act(async () => { + void getResult().sendMessage('surface the running turn') + }) + await waitFor(() => state.postCalls === 1) + unmount() + await waitFor(() => window.localStorage.getItem('sim_mothership_handoff') !== null) + + const handoff = MothershipHandoffStorage.consume('ws-1') + const replacement = renderUseChat() + const invalidateSpy = vi.spyOn(queryClient, 'invalidateQueries') + + await act(async () => { + void replacement.getResult().sendMessage(handoff?.message as string, undefined, undefined, { + recoverStreamId: handoff?.recoverStreamId as string, + }) + }) + await waitFor(() => + invalidateSpy.mock.calls.some(([arg]) => { + const key = (arg as { queryKey?: unknown[] } | undefined)?.queryKey + return Array.isArray(key) && key.includes('chat-server-already-made') + }) + ) + + expect(state.postCalls).toBe(1) + }) + + /** + * A probe cut short by unmount answers "unknown", not "safe to send". + * Falling through to `startSendMessage` there would open a POST whose + * abort controller the teardown already dropped — an uncancellable request + * that duplicates the send. The entry must stay queued instead. + */ + it('does not send when the probe is cut short by unmount', async () => { + state.probeBehavior = 'gone' + const { getResult, unmount } = renderUseChat() + await act(async () => { + void getResult().sendMessage('do not zombie me') + }) + await waitFor(() => state.postCalls === 1) + unmount() + await waitFor(() => window.localStorage.getItem('sim_mothership_handoff') !== null) + + const handoff = MothershipHandoffStorage.consume('ws-1') + + /* `pending` keeps the probe polling instead of answering on the first + attempt, which is what leaves one in flight to interrupt. A `gone` + probe answers immediately and the re-send would already have happened + before the unmount — that version of this test cannot fail. */ + state.probeBehavior = 'pending' + const replacement = renderUseChat() + await act(async () => { + void replacement.getResult().sendMessage(handoff?.message as string, undefined, undefined, { + recoverStreamId: handoff?.recoverStreamId as string, + }) + }) + await waitFor(() => state.streamProbes > 0) + const postsBeforeUnmount = state.postCalls + replacement.unmount() + + // Well past the probe's poll budget: nothing may send after teardown. + await act(async () => { + await sleep(3000) + }) + expect(state.postCalls).toBe(postsBeforeUnmount) + + /* Not sending is only half of it — the message must still be + recoverable. A chatless surface's `pending::` key is regenerated per + mount, so leaving the entry there would strand it just as surely as + re-sending would have duplicated it. */ + expect(MothershipHandoffStorage.consume('ws-1')?.message).toBe('do not zombie me') + expect(allQueuedMessages()).toHaveLength(0) + }) + + it('re-sends when the server has no stream for it', async () => { + // 404 from the probe: the request really was withdrawn before acceptance. + state.probeBehavior = 'gone' + + const { getResult, unmount } = renderUseChat() + await act(async () => { + void getResult().sendMessage('please actually send me') + }) + await waitFor(() => state.postCalls === 1) + unmount() + await waitFor(() => window.localStorage.getItem('sim_mothership_handoff') !== null) + + const handoff = MothershipHandoffStorage.consume('ws-1') + const replacement = renderUseChat() + await act(async () => { + void replacement.getResult().sendMessage(handoff?.message as string, undefined, undefined, { + recoverStreamId: handoff?.recoverStreamId as string, + }) + }) + await waitFor(() => state.postCalls === 2) + + expect(state.streamProbes).toBeGreaterThan(0) + }) + }) +}) diff --git a/apps/sim/app/workspace/[workspaceId]/home/hooks/use-chat.ts b/apps/sim/app/workspace/[workspaceId]/home/hooks/use-chat.ts index e3ce3327301..1055f20b703 100644 --- a/apps/sim/app/workspace/[workspaceId]/home/hooks/use-chat.ts +++ b/apps/sim/app/workspace/[workspaceId]/home/hooks/use-chat.ts @@ -82,6 +82,7 @@ import { executeTerminalToolOnClient } from '@/lib/copilot/tools/client/terminal import { setCurrentChatTraceparent } from '@/lib/copilot/tools/client/trace-context' import { isUserLocalVfsToolCall } from '@/lib/copilot/tools/local-filesystem' import { isWorkflowToolName } from '@/lib/copilot/tools/workflow-tools' +import { MothershipHandoffStorage } from '@/lib/core/utils/browser-storage' import { readSSELines } from '@/lib/core/utils/sse' import { getDesktopBridge, getDesktopChatCapabilities } from '@/lib/desktop' import { @@ -91,6 +92,7 @@ import { migrateDesktopChatScopes, PENDING_CHAT_KEY_PREFIX, } from '@/lib/desktop/chat-scope' +import { sendMothershipMessage } from '@/lib/mothership/events' import { initTerminalTransport } from '@/lib/terminal/transport' import { getQueryClient } from '@/app/_shell/providers/get-query-client' import { useFilePreviewController } from '@/app/workspace/[workspaceId]/home/hooks/preview' @@ -134,6 +136,34 @@ import type { ToolCallInfo, } from '../types' +export interface SendMessageOptions { + /** + * Stream id of a send a cleanup abort withdrew, when this call is the + * recovery of it. The dispatcher probes the id before sending and adopts the + * chat the server already created instead, when there is one. + */ + recoverStreamId?: string +} + +/** + * `true` when the send owns the transcript (rendered, or handed to reconnect), + * `false` when the caller should restore the queue entry, and the object form + * when a cleanup abort withdrew the send while its request was on the wire — + * `streamId` is what recovery probes before re-sending. + */ +type StartSendMessageResult = boolean | { kind: 'recoverable_cleanup_abort'; streamId: string } + +/** + * Outcome of asking the server whether it accepted the request a cleanup abort + * withdrew. `superseded` is deliberately distinct from `not_found`: the former + * means the answer is unknown because this dispatch is no longer current, and + * must not be treated as licence to re-send. + */ +type RecoveredSendProbe = + | { status: 'adopted'; chatId: string } + | { status: 'not_found' } + | { status: 'superseded' } + export interface UseChatReturn { messages: ChatMessage[] isSending: boolean @@ -145,7 +175,8 @@ export interface UseChatReturn { sendMessage: ( message: string, fileAttachments?: FileAttachmentForApi[], - contexts?: ChatContext[] + contexts?: ChatContext[], + options?: SendMessageOptions ) => Promise stopGeneration: () => Promise resources: MothershipResource[] @@ -174,6 +205,16 @@ const RECONNECT_MAX_DELAY_MS = 30_000 const STREAM_BATCH_FETCH_TIMEOUT_MS = 10_000 const STREAM_CHAT_ID_RESOLVE_TIMEOUT_MS = 10_000 const CHAT_HISTORY_RECOVERY_TIMEOUT_MS = 10_000 +/** + * How long a recovered send waits to find out whether the request its cleanup + * abort withdrew had in fact been accepted by the server. The server registers + * the stream early in the request (before it responds), so a short poll is + * enough; the ceiling is deliberately low because every millisecond here delays + * a send the user is watching for. Timing out re-sends, which is the safe + * direction: a lost message is worse than a rare duplicate. + */ +const RECOVERED_SEND_PROBE_TIMEOUT_MS = 2500 +const RECOVERED_SEND_PROBE_INTERVAL_MS = 250 const STOP_REQUEST_TIMEOUT_MS = 15_000 const QUEUED_SEND_HANDOFF_STORAGE_KEY = `${STREAM_STORAGE_KEY}:queued-send-handoff` const QUEUED_SEND_HANDOFF_CLAIM_STORAGE_KEY = `${STREAM_STORAGE_KEY}:queued-send-handoff-claim` @@ -3345,7 +3386,8 @@ export function useChat( ( message: string, fileAttachments?: FileAttachmentForApi[], - contexts?: ChatContext[] + contexts?: ChatContext[], + recoverStreamId?: string ): QueuedMothershipMessage => { const id = generateId() const handoffChatId = selectedChatIdRef.current ?? chatIdRef.current @@ -3365,6 +3407,7 @@ export function useChat( content: message, fileAttachments, contexts, + ...(recoverStreamId ? { recoverStreamId } : {}), ...(supersededStreamId || handoffChatId ? { queuedSendHandoff: { @@ -3454,7 +3497,7 @@ export function useChat( pendingStopOverride?: Promise | null, onOptimisticSendApplied?: () => void, queuedSendHandoff?: QueuedSendHandoffSeed - ) => { + ): Promise => { if (!message.trim() || !workspaceId) return false const pendingStop = pendingStopOverride ?? pendingStopPromiseRef.current const pendingStopStreamId = pendingStop @@ -3465,6 +3508,8 @@ export function useChat( : undefined let consumedByTranscript = false + let sendReachedServer = false + let sendAbortSignal: AbortSignal | null = null setError(null) setTransportStreaming() @@ -3697,6 +3742,7 @@ export function useChat( } const abortController = new AbortController() abortControllerRef.current = abortController + sendAbortSignal = abortController.signal const resourceAttachments = buildResourceAttachments( resourcesRef.current, @@ -3725,6 +3771,7 @@ export function useChat( }), signal: abortController.signal, }) + sendReachedServer = true // Capture for propagation on side-channel calls + non-React // tool-completion callbacks (via trace-context singleton). @@ -3823,7 +3870,29 @@ export function useChat( } } } catch (err) { - if (err instanceof Error && err.name === 'AbortError') return consumedByTranscript + /* fetch rejects with the RAW abort reason (here a plain string) when + its signal was aborted with abort(reason) — an `err.name` check alone + misses those, so abort detection also consults the signal itself. */ + const sendWasAborted = + (err instanceof Error && err.name === 'AbortError') || sendAbortSignal?.aborted === true + if (sendWasAborted) { + if (sendAbortSignal?.reason === 'unmount:client_cleanup' && !sendReachedServer) { + /* A remount (StrictMode's dev double-mount, or a real navigation + away) ran the unmount cleanup before this send's response headers + arrived. Nothing was rendered from it, so withdraw the optimistic + pair and report the distinct outcome so the dispatcher recovers + the message. + + `sendReachedServer` only rules out a send whose RESPONSE landed — + the request itself may well have been accepted, and the route + never reads `request.signal`, so it runs to completion either + way. Recovery therefore carries `userMessageId` as the stream id + to probe before re-sending. */ + rollbackOptimisticSend() + return { kind: 'recoverable_cleanup_abort', streamId: userMessageId } + } + return consumedByTranscript + } if (isStreamSchemaValidationError(err)) { setError(err.message) if (gen !== undefined && streamGenRef.current === gen) { @@ -3874,7 +3943,12 @@ export function useChat( ] ) const sendMessage = useCallback( - async (message: string, fileAttachments?: FileAttachmentForApi[], contexts?: ChatContext[]) => { + async ( + message: string, + fileAttachments?: FileAttachmentForApi[], + contexts?: ChatContext[], + options?: SendMessageOptions + ) => { if (!message.trim() || !workspaceId) return const queueStore = useMothershipQueueStore.getState() @@ -3901,20 +3975,33 @@ export function useChat( queueStore.setEditing(activeChatKey, null) } + const queued = createQueuedMessage( + message, + fileAttachments, + contexts, + options?.recoverStreamId + ) + if (sendingRef.current) { - queueStore.enqueue(activeChatKey, createQueuedMessage(message, fileAttachments, contexts)) + queueStore.enqueue(activeChatKey, queued) return } if (pendingStopPromiseRef.current) { - queueStore.enqueue(activeChatKey, createQueuedMessage(message, fileAttachments, contexts)) + queueStore.enqueue(activeChatKey, queued) void enqueueQueueDispatchRef.current({ type: 'send_head' }) return } - await startSendMessage(message, fileAttachments, contexts) + /* Even an idle-path send goes through the durable queue: a direct + startSendMessage has no backing entry, so a cleanup abort mid-flight + silently drops it. The dispatch loop claims the head in the same tick + for a chatless send; a chat-bound one yields once on `cancelQueries` + first, so it can briefly show as queued. */ + queueStore.enqueue(activeChatKey, queued) + void enqueueQueueDispatchRef.current({ type: 'send_head' }) }, - [workspaceId, startSendMessage, createQueuedMessage] + [workspaceId, createQueuedMessage] ) useEffect(() => { if (typeof window === 'undefined') return @@ -4422,6 +4509,49 @@ export function useChat( ] ) + /** + * Answers "did the server accept the request that a cleanup abort withdrew?" + * by polling for the stream it would have registered. + * + * The abort tears down the client's socket but the route handler never reads + * `request.signal`, so an accepted request still creates the chat, persists + * the user message, and runs the turn. Re-sending in that case bills a second + * run and leaves the user with two chats, so recovery adopts the existing + * chat instead whenever this resolves one. + * + * @returns The chat the orphaned stream belongs to, or `undefined` when the + * server has no such stream (never accepted, or already gone) — in which case + * the caller re-sends. + */ + const resolveRecoveredSendChatId = useCallback( + async (streamId: string, epoch: number): Promise => { + const deadline = Date.now() + RECOVERED_SEND_PROBE_TIMEOUT_MS + while (true) { + const resolve = resolveDetachedChatForStreamRef.current + if (!resolve) return { status: 'not_found' } + /* "Superseded" is NOT the same answer as "the server has no such + stream", and the caller must not conflate them: adopting rewrites + the URL, and re-sending after an unmount would open a POST whose + abort controller the teardown has already dropped — a zombie request + nothing can cancel, duplicating the very send this recovery exists + to protect. Reported distinctly so the caller leaves the entry + queued for a later mount instead. */ + if (epoch !== queueDispatchEpochRef.current) return { status: 'superseded' } + const resolution = await resolve(streamId) + if (epoch !== queueDispatchEpochRef.current) return { status: 'superseded' } + if (resolution.chatId) return { status: 'adopted', chatId: resolution.chatId } + // A terminal status means the stream existed and finished without a + // durable owner; polling cannot improve on that. + if (resolution.terminal) return { status: 'not_found' } + if (Date.now() + RECOVERED_SEND_PROBE_INTERVAL_MS >= deadline) { + return { status: 'not_found' } + } + await sleep(RECOVERED_SEND_PROBE_INTERVAL_MS) + } + }, + [] + ) + const dispatchQueuedMessage = useCallback( async ( msg: QueuedMothershipMessage, @@ -4456,12 +4586,40 @@ export function useChat( useMothershipQueueStore.getState().remove(dispatchChatKey, msg.id) } - const restoreQueuedMessage = (handoff?: QueuedSendHandoffSeed) => { + /** + * Hands a chatless send to whatever surface comes next, because its + * `pending::` queue key is regenerated per mount and anything left under + * this one is unreachable. Prefers the live replacement surface's + * listener and falls back to a one-shot stored handoff for a real + * navigation away. Both lanes carry attachments and the stream id, so the + * next surface probes before it sends. + */ + const handOffChatlessRecovery = (recoverStreamId?: string) => { + if ( + !sendMothershipMessage(msg.content, msg.contexts, msg.fileAttachments, recoverStreamId) + ) { + MothershipHandoffStorage.store( + { + message: msg.content, + ...(msg.contexts?.length ? { contexts: msg.contexts } : {}), + ...(msg.fileAttachments?.length ? { fileAttachments: msg.fileAttachments } : {}), + ...(recoverStreamId ? { recoverStreamId } : {}), + }, + workspaceId + ) + } + } + + const restoreQueuedMessage = (handoff?: QueuedSendHandoffSeed, recoverStreamId?: string) => { + const recoverableCleanupAbort = recoverStreamId !== undefined if (!handoff) { clearQueuedSendHandoffState(msg.id) } clearQueuedSendHandoffClaim(msg.id) - if (!removedFromQueue || options.epoch !== queueDispatchEpochRef.current) { + if (!removedFromQueue) { + return + } + if (options.epoch !== queueDispatchEpochRef.current && !recoverableCleanupAbort) { return } // If the user explicitly removed this message during dispatch, honor @@ -4469,7 +4627,23 @@ export function useChat( if (userRemovedDuringDispatchRef.current.delete(msg.id)) { return } - useMothershipQueueStore.getState().insertAt(dispatchChatKey, originalIndex, msg) + /* A pending (chatless) surface regenerates its chat key per mount, and + the cleanup that aborted this send belongs to a full remount — a + queue restore would orphan the message under the dead instance's + key. Deliver to the replacement surface instead: its send listener + is live by the time this microtask executes. When nothing claims the + event (a real navigation away), a one-shot handoff covers the next + mount; both lanes carry attachments and the stream id to probe. + Chat-bound sends keep the queue restore — their key is the stable + chat id — and carry the stream id on the restored entry. */ + if (recoverableCleanupAbort && dispatchChatKey.startsWith(PENDING_CHAT_KEY_PREFIX)) { + handOffChatlessRecovery(recoverStreamId) + return + } + useMothershipQueueStore.getState().insertAt(dispatchChatKey, originalIndex, { + ...msg, + ...(recoverStreamId ? { recoverStreamId } : {}), + }) } let activeQueuedSendHandoff: QueuedSendHandoffSeed | undefined = @@ -4487,7 +4661,53 @@ export function useChat( // between dispatch scheduling and this send. const liveMsg = queueAtSend[currentIndex] activeQueuedSendHandoff = options.queuedSendHandoff ?? liveMsg.queuedSendHandoff - const consumed = await startSendMessage( + + /* This entry is the recovery of a send a cleanup abort withdrew while + its request was already on the wire. The server never sees that + abort, so if it had accepted the request it created the chat and + persisted the message regardless — re-sending would duplicate both + the chat and the billed run. Probe the orphaned stream first and + adopt its chat instead when it exists. */ + if (liveMsg.recoverStreamId) { + const probe = await resolveRecoveredSendChatId(liveMsg.recoverStreamId, options.epoch) + /* Unknown, not "safe to send". A chat-bound key is the stable chat + id, so leaving the entry queued IS the retry — the next mount's + drain probes again. A chatless `pending::` key is regenerated per + mount, so the same move would strand the message under a dead key; + hand it to the recovery lanes instead, still carrying the stream + id. Skipped when the entry is no longer under this key (adoption + migrated it to a live chat), where it is already recoverable. */ + if (probe.status === 'superseded') { + if (dispatchChatKey.startsWith(PENDING_CHAT_KEY_PREFIX)) { + const queueStore = useMothershipQueueStore.getState() + const stillUnderDeadKey = (queueStore.queues[dispatchChatKey] ?? []).some( + (queued) => queued.id === msg.id + ) + if (stillUnderDeadKey) { + queueStore.remove(dispatchChatKey, msg.id) + handOffChatlessRecovery(liveMsg.recoverStreamId) + } + } + return + } + if (probe.status === 'adopted') { + removeQueuedMessage() + adoptResolvedChatId(probe.chatId, { + replaceHomeHistory: true, + invalidateList: true, + }) + /* Adoption alone does not surface the running turn. Hydration only + reconnects when `chatHistory.activeStreamId` is set, and that + query is cached for `MOTHERSHIP_CHAT_HISTORY_STALE_TIME` — on a + chat-bound recover the client usually holds a copy predating this + stream, so without an explicit detail invalidation the adopted + chat renders with the live response invisible. */ + invalidateChatQueries({ includeDetail: true, targetChatId: probe.chatId }) + return + } + } + + const sendResult = await startSendMessage( liveMsg.content, liveMsg.fileAttachments, liveMsg.contexts, @@ -4496,8 +4716,11 @@ export function useChat( activeQueuedSendHandoff ) - if (!consumed) { - restoreQueuedMessage(activeQueuedSendHandoff) + if (sendResult !== true) { + restoreQueuedMessage( + activeQueuedSendHandoff, + typeof sendResult === 'object' ? sendResult.streamId : undefined + ) } } catch { restoreQueuedMessage(activeQueuedSendHandoff) @@ -4507,7 +4730,13 @@ export function useChat( userRemovedDuringDispatchRef.current.delete(msg.id) } }, - [startSendMessage] + [ + startSendMessage, + workspaceId, + resolveRecoveredSendChatId, + adoptResolvedChatId, + invalidateChatQueries, + ] ) const runQueueDispatchLoop = useCallback(async () => { diff --git a/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/panel.tsx b/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/panel.tsx index 57a97042c96..feba0bec19e 100644 --- a/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/panel.tsx +++ b/apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/panel.tsx @@ -489,7 +489,9 @@ export const Panel = memo(function Panel() { if (!detail?.message) return e.preventDefault() setActiveTab('copilot') - copilotSendMessage(detail.message, undefined, detail.contexts) + copilotSendMessage(detail.message, detail.fileAttachments, detail.contexts, { + ...(detail.recoverStreamId ? { recoverStreamId: detail.recoverStreamId } : {}), + }) } window.addEventListener(MOTHERSHIP_SEND_MESSAGE_EVENT, handler) return () => window.removeEventListener(MOTHERSHIP_SEND_MESSAGE_EVENT, handler) diff --git a/apps/sim/lib/core/utils/browser-storage.ts b/apps/sim/lib/core/utils/browser-storage.ts index af43319d717..61776fa1618 100644 --- a/apps/sim/lib/core/utils/browser-storage.ts +++ b/apps/sim/lib/core/utils/browser-storage.ts @@ -4,6 +4,7 @@ */ import { createLogger } from '@sim/logger' +import type { FileAttachmentForApi } from '@/app/workspace/[workspaceId]/home/types' import type { ChatContext } from '@/stores/panel' const logger = createLogger('BrowserStorage') @@ -307,6 +308,15 @@ export interface MothershipHandoff { message?: string /** Structured contexts to attach — e.g. a `logs` mention tagging a run. */ contexts?: ChatContext[] + /** Already-uploaded attachment references riding along with the message. */ + fileAttachments?: FileAttachmentForApi[] + /** + * Set only when a cleanup abort withdrew an in-flight send and this handoff + * is the recovery of it: the aborted send's stream id. The consuming chat + * probes it before sending, so a request the server had already accepted is + * adopted rather than sent a second time. + */ + recoverStreamId?: string } interface StoredHandoff extends MothershipHandoff { @@ -353,6 +363,8 @@ export class MothershipHandoffStorage { contexts: message ? contexts : [...MothershipHandoffStorage.pendingContexts(workspaceId), ...contexts], + ...(handoff.fileAttachments?.length ? { fileAttachments: handoff.fileAttachments } : {}), + ...(handoff.recoverStreamId ? { recoverStreamId: handoff.recoverStreamId } : {}), workspaceId, timestamp: Date.now(), }) @@ -409,7 +421,16 @@ export class MothershipHandoffStorage { return null } - return { ...(data.message ? { message: data.message } : {}), contexts } + return { + ...(data.message ? { message: data.message } : {}), + contexts, + ...(Array.isArray(data.fileAttachments) && data.fileAttachments.length > 0 + ? { fileAttachments: data.fileAttachments } + : {}), + ...(typeof data.recoverStreamId === 'string' && data.recoverStreamId + ? { recoverStreamId: data.recoverStreamId } + : {}), + } } static clear(): boolean { diff --git a/apps/sim/lib/mothership/events.ts b/apps/sim/lib/mothership/events.ts index d376ee0080f..9259051e6ed 100644 --- a/apps/sim/lib/mothership/events.ts +++ b/apps/sim/lib/mothership/events.ts @@ -1,4 +1,5 @@ import { createLogger } from '@sim/logger' +import type { FileAttachmentForApi } from '@/app/workspace/[workspaceId]/home/types' import type { ChatContext } from '@/stores/panel' const logger = createLogger('MothershipEvents') @@ -24,6 +25,15 @@ export interface MothershipSendMessageDetail { message: string /** Structured contexts to attach — e.g. a `logs` mention tagging a run. */ contexts?: ChatContext[] + /** Already-uploaded attachments riding along with the message. */ + fileAttachments?: FileAttachmentForApi[] + /** + * Set only when a cleanup abort withdrew an in-flight send and this is the + * recovery of it: the aborted send's stream id. The receiving chat probes it + * before sending, so a request the server had already accepted is adopted + * rather than sent a second time. + */ + recoverStreamId?: string } /** @@ -35,7 +45,12 @@ export interface MothershipSendMessageDetail { * was listening — callers that can fall back (e.g. cross-route navigation) use * this to decide whether to persist a handoff instead. */ -export function sendMothershipMessage(message: string, contexts?: ChatContext[]): boolean { +export function sendMothershipMessage( + message: string, + contexts?: ChatContext[], + fileAttachments?: FileAttachmentForApi[], + recoverStreamId?: string +): boolean { const trimmed = message.trim() if (!trimmed) { logger.warn('sendMothershipMessage called with empty message') @@ -44,6 +59,8 @@ export function sendMothershipMessage(message: string, contexts?: ChatContext[]) const consumed = dispatchClaimable(MOTHERSHIP_SEND_MESSAGE_EVENT, { message: trimmed, contexts, + fileAttachments, + ...(recoverStreamId ? { recoverStreamId } : {}), }) logger.info('Dispatched mothership message event', { messageLength: trimmed.length, consumed }) return consumed diff --git a/apps/sim/stores/mothership-queue/store.ts b/apps/sim/stores/mothership-queue/store.ts index 5697d9ac475..a70eea369f0 100644 --- a/apps/sim/stores/mothership-queue/store.ts +++ b/apps/sim/stores/mothership-queue/store.ts @@ -99,7 +99,14 @@ export const useMothershipQueueStore = create()( const next = [...current] // Strip `queuedSendHandoff` — references the stream active at // original enqueue time; the dispatcher mints a fresh one at send. - const { queuedSendHandoff: _stale, ...rest } = next[index] + // Strip `recoverStreamId` too: it dedupes against a server-side copy + // of the PRE-edit text, which the edited message is no longer a + // duplicate of, so probing it would wrongly suppress this send. + const { + queuedSendHandoff: _stale, + recoverStreamId: _staleRecover, + ...rest + } = next[index] next[index] = { ...rest, content: patch.content, diff --git a/apps/sim/stores/mothership-queue/types.ts b/apps/sim/stores/mothership-queue/types.ts index b39dff9b8e4..dde4808a422 100644 --- a/apps/sim/stores/mothership-queue/types.ts +++ b/apps/sim/stores/mothership-queue/types.ts @@ -10,6 +10,15 @@ export interface QueuedSendHandoffSeed { export type QueuedMothershipMessage = QueuedMessage & { queuedSendHandoff?: QueuedSendHandoffSeed + /** + * Stream id (the aborted send's `userMessageId`) of a send a cleanup abort + * withdrew while its request was already on the wire. The dispatcher probes + * it before re-sending: the server does not observe `request.signal`, so a + * request it had already accepted still creates the chat and persists the + * message, and re-sending blind would duplicate both. Persisted, so a + * nav-back restore probes too. + */ + recoverStreamId?: string } // Mutable fields an in-place edit overwrites; id and index are preserved by `replaceAt`.