Skip to content

Commit 39e2a84

Browse files
committed
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.
1 parent d6a5238 commit 39e2a84

1 file changed

Lines changed: 79 additions & 0 deletions

File tree

apps/sim/app/workspace/[workspaceId]/home/hooks/use-chat.mount-send.test.tsx

Lines changed: 79 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -190,6 +190,65 @@ function renderStrictModeHandoffConsumer(): { unmount: () => void } {
190190
return { unmount: () => act(() => root.unmount()) }
191191
}
192192

193+
/**
194+
* Mounts a surface shaped like `home.tsx`: it drives `useChat` AND registers
195+
* the `mothership-send-message` listener that claims the event with
196+
* `preventDefault`. Unmounting this exercises the ordering question — whether
197+
* the departing surface's own still-attached listener can claim the recovery
198+
* event its own teardown emitted, which would suppress the storage fallback
199+
* and strand the message.
200+
*/
201+
function renderHomeLikeSurface(): {
202+
getResult: () => ReturnType<typeof useChat>
203+
claimedByOwnListener: () => number
204+
unmount: () => void
205+
} {
206+
;(globalThis as { IS_REACT_ACT_ENVIRONMENT?: boolean }).IS_REACT_ACT_ENVIRONMENT = true
207+
queryClient = new QueryClient({ defaultOptions: { queries: { retry: false } } })
208+
const container = document.createElement('div')
209+
const root = createRoot(container)
210+
mountedRoots.push(root)
211+
let result: ReturnType<typeof useChat> | undefined
212+
let claims = 0
213+
214+
function HomeLike() {
215+
const chat = useChat('ws-1', undefined)
216+
result = chat
217+
const { sendMessage } = chat
218+
// Mirrors home.tsx:339 — declared AFTER useChat, so on unmount React runs
219+
// useChat's cleanup (which aborts) before this removeEventListener.
220+
useEffect(() => {
221+
const handler = (e: Event) => {
222+
const detail = (e as CustomEvent<{ message?: string; recoverStreamId?: string }>).detail
223+
if (!detail?.message) return
224+
claims++
225+
e.preventDefault()
226+
sendMessage(detail.message, undefined, undefined, {
227+
...(detail.recoverStreamId ? { recoverStreamId: detail.recoverStreamId } : {}),
228+
})
229+
}
230+
window.addEventListener('mothership-send-message', handler)
231+
return () => window.removeEventListener('mothership-send-message', handler)
232+
}, [sendMessage])
233+
return null
234+
}
235+
236+
act(() => {
237+
root.render(
238+
<QueryClientProvider client={queryClient}>{(<HomeLike />) as ReactNode}</QueryClientProvider>
239+
)
240+
})
241+
242+
return {
243+
getResult: () => {
244+
if (result === undefined) throw new Error('Hook result is not ready')
245+
return result
246+
},
247+
claimedByOwnListener: () => claims,
248+
unmount: () => act(() => root.unmount()),
249+
}
250+
}
251+
193252
/** Every queued message across all chat keys, flattened. */
194253
function allQueuedMessages() {
195254
return Object.values(useMothershipQueueStore.getState().queues).flat()
@@ -311,6 +370,26 @@ describe('useChat remount send recovery', () => {
311370
expect(MothershipHandoffStorage.consume('ws-1')).toBeNull()
312371
})
313372

373+
/**
374+
* A departing surface's own listener must not claim the recovery event its
375+
* teardown emitted: claiming returns `true`, which suppresses the storage
376+
* fallback, and the enqueue would land under the disposed pending key — the
377+
* message would be stranded exactly where this fix is supposed to save it.
378+
*/
379+
it('does not let a departing surface claim its own recovery event', async () => {
380+
const surface = renderHomeLikeSurface()
381+
await act(async () => {
382+
void surface.getResult().sendMessage('must survive my own teardown')
383+
})
384+
await waitFor(() => state.postCalls === 1)
385+
386+
surface.unmount()
387+
await waitFor(() => window.localStorage.getItem('sim_mothership_handoff') !== null)
388+
389+
expect(surface.claimedByOwnListener()).toBe(0)
390+
expect(MothershipHandoffStorage.consume('ws-1')?.message).toBe('must survive my own teardown')
391+
})
392+
314393
/**
315394
* The end-to-end failure, driven by the thing that actually runs the cleanup
316395
* mid-flight rather than by a hand-rolled unmount. On the unfixed hook the

0 commit comments

Comments
 (0)