fix(chat): stop losing sends aborted during mount-settling (#6525)

* fix(chat): stop losing sends aborted during mount-settling

* fix(chat): detect aborts by signal state, not error identity

fetch rejects with the RAW abort reason when its signal carries one —
abort('unmount:client_cleanup') surfaces as a plain string, so every
err.name === 'AbortError' check missed it and the restore path never ran
(verified live). The test stub now rejects with the raw reason like real
fetch, which turns this gap red.

* fix(chat): hand an aborted chatless send to the next mount

The mount-settling cycle is a full remount — the pending chat key is
regenerated per instance, so restoring the aborted send into the dead
instance's queue orphaned it (verified live). A chatless send now
re-persists as a one-shot MothershipHandoffStorage handoff the next
mount's consumer re-sends; chat-bound sends keep the queue restore.

* fix(chat): deliver an aborted chatless send to the live replacement surface

The settling remount's consumer checks handoff storage before the
restore microtask re-persists it, so the stored handoff sat unread until
a navigation. The replacement surface's send listener IS registered by
restore time — deliver the message directly through the claimable send
event, keeping the stored handoff as the no-surface fallback.

* refactor(chat): thread the recoverable-abort outcome through the send result

Replaces the restorableCleanupAbortRef reset choreography with a widened
startSendMessage return ('recoverable_cleanup_abort'), so the restore
decision is ordinary data flow and the second caller cannot leave a stale
flag behind.

* fix(chat): carry attachments through the cross-mount send handoff

The recoverable-abort delivery excluded attachment-bearing sends, so
they restored under the dead instance's pending key and were silently
lost. The claimable send event now carries fileAttachments end to end
(dispatcher, home listener, restore path); only the storage fallback —
whose shape cannot hold attachments — still queue-restores them.

* fix(panel): forward event attachments to the copilot send

* fix(chat): carry attachments through the stored handoff lane too

The unclaimed-event fallback excluded attachment sends and restored
them under the disposed mount's pending key. The persisted handoff now
carries fileAttachments (they are plain references to already-uploaded
files), the home consumer forwards them, and the recovery branch always
hands off — no stranded lane remains.

* fix(chat): probe the orphaned stream before re-sending a withdrawn send

The cleanup-abort recovery treated "no response headers yet" as "the server
never got it" and re-sent. It is not the same thing: the mothership chat route
never reads `request.signal`, so a request it had already accepted still runs
to completion — resolveOrCreateChat, persistUserMessage, and the billed turn
all commit even though the client socket is gone. Re-sending blind therefore
left the user with two chats and two billed runs for one message.

Recovery now carries the withdrawn send's `userMessageId` as a stream id
through both lanes (the live `mothership-send-message` event and the stored
one-shot handoff) and through a restored queue entry. Before re-sending, the
dispatcher polls that stream: when it resolves to a chat, the server already
has the message, so the chat is adopted instead of sent again. Only a stream
the server has no record of — a 404, i.e. genuinely never accepted — re-sends.
Timing out re-sends too, which is the safe direction.

Also corrects the root cause recorded in the comments. A Suspense hide/reveal
cannot run this cleanup: React 19 disappears layout effects only, and this is
a passive effect (verified against react-dom 19.2.4). What does run it is
StrictMode's dev double-mount and a real client-side navigation away, both
mid-flight — and because MothershipHandoffStorage consumes atomically, the
replacement mount finds nothing left to retry.

* fix(chat): never re-send on an unresolved probe, and reconnect after adopting

Two defects in the orphaned-stream probe, both found by Bugbot.

A probe cut short by an epoch change (unmount, chat switch) returned the same
`undefined` as "the server has no such stream", so the dispatcher fell through
to `startSendMessage`. After unmount the teardown has already dropped the abort
controller, so that send opened a POST nothing could cancel — duplicating the
very message this recovery exists to protect. The probe now reports
`superseded` distinctly and the dispatcher leaves the entry queued, keeping its
`recoverStreamId` so a later mount probes again.

Adopting the recovered chat also invalidated only the chat list. Hydration
reconnects to a live turn solely on `chatHistory.activeStreamId`, and that
query is cached for MOTHERSHIP_CHAT_HISTORY_STALE_TIME — on a chat-bound
recover the client normally holds a copy predating this stream, so the adopted
chat rendered with the running response invisible. Adoption now invalidates the
chat detail too.

Both regression tests were confirmed to fail without their fix: the first
re-sends (2 POSTs instead of 1), the second never invalidates. The probe stub
gained a `pending` mode because a `gone` probe answers on the first attempt and
leaves nothing in flight to interrupt — the earlier draft of the first test
passed with the guard removed and proved nothing.

* test(chat): cover the departing surface's own recovery-event claim

Greptile flagged that a surface being torn down could claim the recovery event
its own cleanup emits — which would return `true`, suppress the storage
fallback, and strand the message under a disposed pending key. It cannot: React
removes the listener during the same synchronous unmount commit, while the
recovery runs from the fetch rejection a microtask later, so by then nothing of
the departing surface is listening.

That ordering was previously only argued, never asserted — the suite unmounted a
bare hook with no listener attached. This mounts a home.tsx-shaped surface that
both drives useChat and registers the claiming listener, and asserts the
departing listener claims zero times while the handoff still reaches storage.
Confirmed meaningful: neutering the listener's removeEventListener cleanup so it
survives teardown makes it claim, and the test fails.

* fix(chat): hand off a chatless send when the probe is superseded

The previous commit made a superseded probe leave the entry queued rather than
re-send it. That is the right retry for a chat-bound key, which is the stable
chat id, but wrong for a chatless one: a `pending::` key is regenerated every
mount, so anything left under it is unreachable and the message is stranded —
the same loss this PR exists to prevent, just reached by a different route.

A superseded probe on a pending key now goes through the same recovery lanes as
the cleanup-abort path (live replacement surface, else a one-shot stored
handoff), still carrying the stream id so the next surface probes before it
sends. Skipped when the entry is no longer under that key, since adoption
migrating it to a live chat already leaves it recoverable there. The lane is
extracted so both call sites share one implementation.

The existing superseded test only asserted that nothing sent, which this bug
satisfied trivially; it now also asserts the message survives. Confirmed red
without the fix.

---------

Co-authored-by: Waleed Latif <walif6@gmail.com>
This commit is contained in:
Justin Blumencranz
2026-08-11 00:19:14 -07:00
committed by GitHub
co-authored by Waleed Latif
parent a64ce49af9
commit 155192330c
8 changed files with 866 additions and 22 deletions
@@ -341,7 +341,9 @@ export function Home({ chatId, userName, userId, tableViewsEnabled }: HomeProps)
const detail = (e as CustomEvent<MothershipSendMessageDetail>).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 ?? []
@@ -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<typeof import('@/lib/api/client/request')>()),
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<Uint8Array>({
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<Response> {
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<Response>((_, 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<typeof useChat>
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<typeof useChat> | undefined
function Probe() {
result = useChat('ws-1', undefined)
return null
}
act(() => {
root.render(
<QueryClientProvider client={queryClient}>{(<Probe />) as ReactNode}</QueryClientProvider>
)
})
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(
<StrictMode>
<QueryClientProvider client={queryClient}>{(<Probe />) as ReactNode}</QueryClientProvider>
</StrictMode>
)
})
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<typeof useChat>
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<typeof useChat> | 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(
<QueryClientProvider client={queryClient}>{(<HomeLike />) as ReactNode}</QueryClientProvider>
)
})
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<void> {
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)
})
})
})
@@ -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<void>
stopGeneration: () => Promise<void>
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<void> | null,
onOptimisticSendApplied?: () => void,
queuedSendHandoff?: QueuedSendHandoffSeed
) => {
): Promise<StartSendMessageResult> => {
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<RecoveredSendProbe> => {
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 () => {
@@ -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)
+22 -1
View File
@@ -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 {
+18 -1
View File
@@ -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<MothershipSendMessageDetail>(MOTHERSHIP_SEND_MESSAGE_EVENT, {
message: trimmed,
contexts,
fileAttachments,
...(recoverStreamId ? { recoverStreamId } : {}),
})
logger.info('Dispatched mothership message event', { messageLength: trimmed.length, consumed })
return consumed
+8 -1
View File
@@ -99,7 +99,14 @@ export const useMothershipQueueStore = create<MothershipQueueState>()(
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,
@@ -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`.