Skip to content

Commit e9a6a3f

Browse files
committed
improvement(mothership): contain hung chat hook tests and name the delete-lifting actions
- bound async act in the use-chat DOM tests, so a send that never settles fails its own test instead of every later one - note at the deduped stream lookup that a timeout abort is retried on purpose - split reopenChat into liftDelete (token required) and reopenRestoredChat
1 parent 6adf4c6 commit e9a6a3f

8 files changed

Lines changed: 66 additions & 22 deletions

File tree

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

Lines changed: 31 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@
1919
* request it aborted was accepted.
2020
*/
2121

22-
import { act, type ReactNode, StrictMode, useEffect, useState } from 'react'
22+
import { type ReactNode, act as reactAct, StrictMode, useEffect, useState } from 'react'
2323
import { authClientMock, authClientMockFns } from '@sim/testing/mocks/auth-client.mock'
2424
import { libDesktopMock, libDesktopMockFns } from '@sim/testing/mocks/lib-desktop.mock'
2525
import { nextNavigationMock, nextNavigationMockFns } from '@sim/testing/mocks/next-navigation.mock'
@@ -103,6 +103,36 @@ import { useExecutionStore } from '@/stores/execution/store'
103103
import { useMothershipEffortStore } from '@/stores/mothership-effort/store'
104104
import { useMothershipQueueStore } from '@/stores/mothership-queue/store'
105105

106+
/** Captured before any test fakes timers, so the act budget below runs in real time. */
107+
const realSetTimeout = globalThis.setTimeout
108+
const realClearTimeout = globalThis.clearTimeout
109+
/** Well under the 10s test timeout: the scope must end before the runner abandons the test. */
110+
const ACT_BUDGET_MS = 6_000
111+
112+
/**
113+
* React's `act`, with async callbacks bounded. A callback that never settles (a
114+
* regressed send stuck reconnecting) used to run into the test timeout while
115+
* still inside React's act scope, and the renders of every later test queued
116+
* behind it ("Hook result is not ready"). Failing the act after a budget ends
117+
* the scope, so one regression is one red test. Sync callbacks stay synchronous.
118+
*/
119+
function act(callback: () => unknown): Promise<void> {
120+
return reactAct((): undefined | Promise<void> => {
121+
const result = callback()
122+
if (!(result instanceof Promise)) return undefined
123+
let budget: ReturnType<typeof setTimeout> | undefined
124+
const budgetSpent = new Promise<never>((_, reject) => {
125+
budget = realSetTimeout(
126+
() => reject(new Error(`act callback still pending after ${ACT_BUDGET_MS}ms`)),
127+
ACT_BUDGET_MS
128+
)
129+
})
130+
return Promise.race([result.then(() => undefined), budgetSpent]).finally(() =>
131+
realClearTimeout(budget)
132+
)
133+
})
134+
}
135+
106136
authClientMockFns.mockUseSession.mockImplementation(() => ({
107137
data: { user: { id: 'test-viewer' } },
108138
}))

‎apps/sim/app/workspace/[workspaceId]/home/hooks/use-chat.ts‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4025,9 +4025,10 @@ export function useChat(
40254025
message. Retry it later like a busy refusal; the server's claim settles.
40264026
This is checked before adopting the chat the answer names, so a retried
40274027
message stays under the key it was sent from. A lookup that fails for
4028-
another reason proves nothing either way, so it is retried too: the server
4029-
deduplicates the retry by id. Only a lookup this send aborted (Stop, or the
4030-
user moving on) is not retried. */
4028+
another reason proves nothing either way, so it is retried on purpose; that
4029+
includes the lookup's own timeout abort, which leaves this send's signal
4030+
untouched. The server deduplicates the retry by id. Only an abort of this
4031+
send itself (Stop, or the user moving on) is not retried. */
40314032
const dedupedStreamExists = await fetchStreamBatch(
40324033
conflictStreamId,
40334034
'0',

‎apps/sim/hooks/queries/mothership-chat-history-restore.test.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -90,7 +90,7 @@ describe('lifting a chat delete', () => {
9090
useMothershipQueueStore.getState().clearChat(history.id)
9191
const answer = deferredAnswer({ chat: history })
9292
const read = fetchMothershipChatHistory(history.id)
93-
useMothershipQueueStore.getState().reopenChat(history.id)
93+
useMothershipQueueStore.getState().reopenRestoredChat(history.id)
9494
useMothershipQueueStore.getState().clearChat(history.id)
9595
answer()
9696
await read
@@ -110,7 +110,7 @@ describe('lifting a chat delete', () => {
110110
useMothershipQueueStore.getState().clearChat(history.id)
111111

112112
await restore(history.id, () => {
113-
useMothershipQueueStore.getState().reopenChat(history.id)
113+
useMothershipQueueStore.getState().reopenRestoredChat(history.id)
114114
useMothershipQueueStore.getState().clearChat(history.id)
115115
})
116116

‎apps/sim/hooks/queries/mothership-chats.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -321,7 +321,7 @@ export async function fetchMothershipChatHistory(
321321
): Promise<MothershipChatHistory> {
322322
const deleteSeen = useMothershipQueueStore.getState().cleared[chatId]
323323
const history = await readMothershipChatHistory(chatId, signal)
324-
if (deleteSeen !== undefined) useMothershipQueueStore.getState().reopenChat(chatId, deleteSeen)
324+
if (deleteSeen !== undefined) useMothershipQueueStore.getState().liftDelete(chatId, deleteSeen)
325325
return history
326326
}
327327

@@ -385,7 +385,7 @@ export function useRestoreMothershipChat(owner?: MothershipChatOwner) {
385385
onMutate: (chatId) => ({ deleteSeen: useMothershipQueueStore.getState().cleared[chatId] }),
386386
onSuccess: (_data, chatId, context) => {
387387
if (context?.deleteSeen === undefined) return
388-
useMothershipQueueStore.getState().reopenChat(chatId, context.deleteSeen)
388+
useMothershipQueueStore.getState().liftDelete(chatId, context.deleteSeen)
389389
},
390390
onSettled: () => {
391391
queryClient.invalidateQueries({ queryKey: mothershipChatKeys.ownerLists(owner) })

‎apps/sim/hooks/use-mothership-chat-events.ts‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -125,7 +125,8 @@ export function handleMothershipChatStatusEvent(
125125
return
126126
}
127127
/** A restore is published as `created`; the chat takes queued sends again. */
128-
if (payload.type === 'created') useMothershipQueueStore.getState().reopenChat(payload.chatId)
128+
if (payload.type === 'created')
129+
useMothershipQueueStore.getState().reopenRestoredChat(payload.chatId)
129130
if (payload.type === 'renamed') {
130131
/**
131132
* The lists invalidated above carry the title every surface renders; the

‎apps/sim/stores/mothership-queue/store.test.ts‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -92,15 +92,15 @@ describe('useMothershipQueueStore', () => {
9292
it('lifts only the delete a restore saw, never a later one', () => {
9393
useMothershipQueueStore.getState().clearChat('chat-X')
9494
const seen = useMothershipQueueStore.getState().cleared['chat-X']
95-
useMothershipQueueStore.getState().reopenChat('chat-X')
95+
useMothershipQueueStore.getState().reopenRestoredChat('chat-X')
9696
useMothershipQueueStore.getState().clearChat('chat-X')
9797

98-
useMothershipQueueStore.getState().reopenChat('chat-X', seen)
98+
useMothershipQueueStore.getState().liftDelete('chat-X', seen)
9999
useMothershipQueueStore.getState().enqueue('chat-X', message('after-stale-restore'))
100100
expect(useMothershipQueueStore.getState().queues['chat-X']).toBeUndefined()
101101

102102
const latest = useMothershipQueueStore.getState().cleared['chat-X']
103-
useMothershipQueueStore.getState().reopenChat('chat-X', latest)
103+
useMothershipQueueStore.getState().liftDelete('chat-X', latest)
104104
useMothershipQueueStore.getState().enqueue('chat-X', message('after-restore'))
105105
expect(useMothershipQueueStore.getState().queues['chat-X']?.map((m) => m.id)).toEqual([
106106
'after-restore',

‎apps/sim/stores/mothership-queue/store.ts‎

Lines changed: 13 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -212,13 +212,19 @@ export const useMothershipQueueStore = create<MothershipQueueState>()(
212212
cleared: { ...state.cleared, [chatKey]: ++deleteCount },
213213
})),
214214

215-
reopenChat: (chatKey, deleteToken) =>
216-
set((state) => {
217-
const current = state.cleared[chatKey]
218-
if (current === undefined) return state
219-
if (deleteToken !== undefined && deleteToken !== current) return state
220-
return { cleared: omitKey(state.cleared, chatKey) }
221-
}),
215+
liftDelete: (chatKey, deleteToken) =>
216+
set((state) =>
217+
state.cleared[chatKey] === deleteToken
218+
? { cleared: omitKey(state.cleared, chatKey) }
219+
: state
220+
),
221+
222+
reopenRestoredChat: (chatKey) =>
223+
set((state) =>
224+
state.cleared[chatKey] === undefined
225+
? state
226+
: { cleared: omitKey(state.cleared, chatKey) }
227+
),
222228

223229
reset: () => set(initialState),
224230
}),

‎apps/sim/stores/mothership-queue/types.ts‎

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -71,9 +71,15 @@ export interface MothershipQueueState {
7171
adoptHeldSends: (toKey: string, surface: string) => void
7272
clearChat: (chatKey: string) => void
7373
/**
74-
* Lifts `cleared` for a restored chat. Given the delete token an operation
75-
* saw when it began, lifts only that delete, never one that landed after it.
74+
* Lifts the delete an operation saw when it began (`cleared[chatKey]` read
75+
* then), for a read or restore that found the chat. A delete that landed
76+
* after it has a newer token and stays.
7677
*/
77-
reopenChat: (chatKey: string, deleteToken?: number) => void
78+
liftDelete: (chatKey: string, deleteToken: number) => void
79+
/**
80+
* Lifts any delete of a chat the server announced as restored (its `created`
81+
* event, published after every delete before it).
82+
*/
83+
reopenRestoredChat: (chatKey: string) => void
7884
reset: () => void
7985
}

0 commit comments

Comments
 (0)