Skip to content

Commit cb91ded

Browse files
committed
test(mothership): cover Send-now's re-checks after its history read
Send-now no longer stops a turn in a chat the user moved to during the read, or from a surface that unmounted during it, and leaves a message the drain dispatched meanwhile to that dispatch. applyHeldResend is renamed applyResendVerdict.
1 parent 2c5608b commit cb91ded

2 files changed

Lines changed: 158 additions & 11 deletions

File tree

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

Lines changed: 150 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2882,11 +2882,6 @@ describe('useChat remount send recovery', () => {
28822882
expect(getResult().messageQueue.map((message) => message.content)).toEqual(['Follow-up'])
28832883
})
28842884

2885-
/**
2886-
* Send-now on a message that may already be a turn checks the chat's history
2887-
* first, as the queue drain does: if the server shows the id accepted, the
2888-
* message is already in the chat and sending it again could run a second turn.
2889-
*/
28902885
/**
28912886
* Send-now reads the history before it stops the running turn. A message the
28922887
* user removes during that read is no longer theirs to send, so the running
@@ -2937,6 +2932,156 @@ describe('useChat remount send recovery', () => {
29372932
expect(getResult().isSending).toBe(true)
29382933
})
29392934

2935+
/**
2936+
* The drain may already be reading the history for the message the user then
2937+
* sends by hand. When the drain's read lands first it dispatches the message,
2938+
* and Send-now must leave that dispatch alone rather than stop the turn it
2939+
* just started.
2940+
*/
2941+
it('does not stop the turn the drain started for a Send-now waiting on the same history read', async () => {
2942+
const history: MothershipChatHistory = {
2943+
id: 'chat-a',
2944+
mode: 'agent',
2945+
title: 'A',
2946+
messages: [],
2947+
activeStreamId: null,
2948+
resources: [],
2949+
}
2950+
const answers: Array<() => void> = []
2951+
mockRequestJson.mockImplementation(
2952+
() =>
2953+
new Promise((resolve) => {
2954+
answers.push(() => resolve({ chat: history }))
2955+
})
2956+
)
2957+
useMothershipQueueStore.getState().enqueue('chat-a', {
2958+
id: 'resumed',
2959+
content: 'sent earlier with no answer',
2960+
resumeUserMessageId: 'earlier-attempt',
2961+
admissionUnknown: true,
2962+
})
2963+
const { getResult } = renderUseChatInChat('chat-a', history)
2964+
await waitFor(() => answers.length > 0)
2965+
2966+
await act(async () => {
2967+
void getResult().sendNow('resumed')
2968+
})
2969+
await act(async () => {
2970+
for (const answer of answers) answer()
2971+
await sleep(100)
2972+
})
2973+
2974+
expect(state.abortBodies).toHaveLength(0)
2975+
expect(state.postBodies).toHaveLength(1)
2976+
expect(state.postBodies[0]).toMatchObject({ userMessageId: 'earlier-attempt' })
2977+
expect(getResult().isSending).toBe(true)
2978+
})
2979+
2980+
/**
2981+
* A Send-now belongs to the chat it was pressed in. If the user moves to
2982+
* another chat during its history read (which the move may cancel), the turn
2983+
* running there is not the one it was meant to stop.
2984+
*/
2985+
it("does not stop another chat's turn for a Send-now whose chat was left during its history read", async () => {
2986+
const chat = (id: string): MothershipChatHistory => ({
2987+
id,
2988+
mode: 'agent',
2989+
title: id,
2990+
messages: [],
2991+
activeStreamId: null,
2992+
resources: [],
2993+
})
2994+
let answerHistory: (() => void) | undefined
2995+
mockRequestJson.mockImplementation(
2996+
() =>
2997+
new Promise((resolve) => {
2998+
answerHistory = () => resolve({ chat: chat('chat-a') })
2999+
})
3000+
)
3001+
useMothershipQueueStore.getState().enqueue('chat-a', {
3002+
id: 'resumed',
3003+
content: 'sent earlier with no answer',
3004+
resumeUserMessageId: 'earlier-attempt',
3005+
admissionUnknown: true,
3006+
hold: 'user',
3007+
})
3008+
const { getResult, navigate } = renderUseChatInChat('chat-a', chat('chat-a'))
3009+
await act(async () => {
3010+
void getResult().sendNow('resumed')
3011+
})
3012+
await waitFor(() => answerHistory !== undefined)
3013+
const answerChatA = answerHistory
3014+
mockRequestJson.mockImplementation(() => Promise.resolve({ chat: chat('chat-b') }))
3015+
3016+
navigate('chat-b', chat('chat-b'))
3017+
await act(async () => {
3018+
void getResult().sendMessage('Chat B request')
3019+
})
3020+
await waitFor(() => state.postBodies.length === 1)
3021+
await act(async () => {
3022+
answerChatA?.()
3023+
await sleep(100)
3024+
})
3025+
3026+
expect(state.abortBodies).toHaveLength(0)
3027+
expect(state.postBodies).toHaveLength(1)
3028+
expect(getResult().isSending).toBe(true)
3029+
expect(useMothershipQueueStore.getState().queues['chat-a']?.map((queued) => queued.id)).toEqual(
3030+
['resumed']
3031+
)
3032+
})
3033+
3034+
/** A surface that unmounts during the read leaves the running turn to whoever owns it next. */
3035+
it('does not stop the running turn for a Send-now whose surface unmounted during its history read', async () => {
3036+
const { getResult, unmount } = renderUseChatInChat('chat-a')
3037+
await act(async () => {
3038+
void getResult().sendMessage('Original request')
3039+
})
3040+
await waitFor(() => state.postBodies.length === 1 && getResult().isSending)
3041+
let answerHistory: (() => void) | undefined
3042+
mockRequestJson.mockImplementation(
3043+
() =>
3044+
new Promise((resolve) => {
3045+
answerHistory = () =>
3046+
resolve({
3047+
chat: {
3048+
id: 'chat-a',
3049+
mode: 'agent',
3050+
title: 'A',
3051+
messages: [],
3052+
activeStreamId: null,
3053+
resources: [],
3054+
},
3055+
})
3056+
})
3057+
)
3058+
useMothershipQueueStore.getState().enqueue('chat-a', {
3059+
id: 'resumed',
3060+
content: 'sent earlier with no answer',
3061+
resumeUserMessageId: 'earlier-attempt',
3062+
admissionUnknown: true,
3063+
})
3064+
await act(async () => {
3065+
void getResult().sendNow('resumed')
3066+
})
3067+
await waitFor(() => answerHistory !== undefined)
3068+
const abortsBeforeUnmount = state.abortBodies.length
3069+
3070+
unmount()
3071+
await act(async () => {
3072+
answerHistory?.()
3073+
await sleep(100)
3074+
})
3075+
3076+
expect(state.abortBodies).toHaveLength(abortsBeforeUnmount)
3077+
expect(state.postBodies).toHaveLength(1)
3078+
})
3079+
3080+
/**
3081+
* Send-now on a message that may already be a turn checks the chat's history
3082+
* first, as the queue drain does: if the server shows the id accepted, the
3083+
* message is already in the chat and sending it again could run a second turn.
3084+
*/
29403085
it('drops a Send-now whose id the server already accepted instead of resending it', async () => {
29413086
const cached: MothershipChatHistory = {
29423087
id: 'chat-send-now-accepted',

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

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -5068,7 +5068,7 @@ export function useChat(
50685068
)
50695069

50705070
/** Holds back a message the verdict says must wait, or discards one already sent. */
5071-
const applyHeldResend = useCallback(
5071+
const applyResendVerdict = useCallback(
50725072
(chatKey: string, msg: QueuedMothershipMessage, verdict: 'wait' | 'drop') => {
50735073
/** Sent by hand meanwhile: that dispatch owns the entry now. */
50745074
if (queuedMessageDispatchIds.has(msg.id)) return
@@ -5109,7 +5109,7 @@ export function useChat(
51095109
if (queueState.editing[activeChatKey] === msg.id) continue
51105110
const verdict = await checkResend(activeChatKey, msg)
51115111
if (verdict !== 'send') {
5112-
applyHeldResend(activeChatKey, msg, verdict)
5112+
applyResendVerdict(activeChatKey, msg, verdict)
51135113
continue
51145114
}
51155115

@@ -5127,7 +5127,7 @@ export function useChat(
51275127
void queueDispatchLoopRef.current()
51285128
}
51295129
})
5130-
}, [dispatchQueuedMessage, hasPendingChatAdmission, checkResend, applyHeldResend])
5130+
}, [dispatchQueuedMessage, hasPendingChatAdmission, checkResend, applyResendVerdict])
51315131
queueDispatchLoopRef.current = runQueueDispatchLoop
51325132

51335133
const enqueueQueueDispatch = useCallback((action: QueueDispatchActionInput) => {
@@ -5159,11 +5159,13 @@ export function useChat(
51595159
hold it back here; the user asked to send it, and the server deduplicates it
51605160
while the earlier attempt's claim lasts. */
51615161
if ((await checkResend(chatKey, msg)) === 'drop') {
5162-
applyHeldResend(chatKey, msg, 'drop')
5162+
applyResendVerdict(chatKey, msg, 'drop')
51635163
return
51645164
}
51655165
/* The read took time. Only what is still this view's queued, unedited and
5166-
undispatched message may stop the running turn and go out. */
5166+
undispatched message may stop the running turn and go out. An entry that
5167+
needed the read cannot be in the editor (editing refuses one possibly
5168+
sent); the editing check covers an entry that skipped it. */
51675169
const afterRead = useMothershipQueueStore.getState()
51685170
if (
51695171
chatKeyRef.current !== chatKey ||
@@ -5226,7 +5228,7 @@ export function useChat(
52265228
scopeKey,
52275229
hasPendingChatAdmission,
52285230
checkResend,
5229-
applyHeldResend,
5231+
applyResendVerdict,
52305232
]
52315233
)
52325234

0 commit comments

Comments
 (0)