Skip to content

Commit 2e7082f

Browse files
authored
fix(mothership): keep any queued send the server may already hold from being edited (#8723)
* fix(mothership): keep any queued send the server may already hold from being edited #8717 blocked edits only for a first message the unmount queued itself. A send handed to another surface, or re-queued after its dispatch got no answer, also carries an earlier attempt's id and was still editable; an edit dropped that id and could send a second message. The queue store now marks every message resuming an earlier attempt as admissionUnknown unless the writer knows the server refused it or never got it (a busy refusal, a superseded 409, a Stop that never settled), which also makes a held message editable again once the server refuses it. * fix(mothership): keep earlier-attempt uncertainty through a failed Stop and a reload A Send-now whose Stop did not settle sent nothing, but was treated as proof the server lacked the message, so a resumed message became editable. Only a refusal of its id clears that now; an attempt that never left keeps the earlier uncertainty. Queues saved before the guard are normalized when the session restores them.
1 parent 38942a7 commit 2e7082f

6 files changed

Lines changed: 326 additions & 13 deletions

File tree

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

Lines changed: 193 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2151,11 +2151,6 @@ describe('useChat remount send recovery', () => {
21512151
expect(allQueuedMessages()).toHaveLength(0)
21522152
})
21532153

2154-
/**
2155-
* After a Stop of the first message, only the follow-up was the user's
2156-
* intent: the Stop's POST is left to the server, nothing withdraws it, and the
2157-
* next mount sends just the follow-up, once.
2158-
*/
21592154
/**
21602155
* A first message held at the queue head after a remount may already be a
21612156
* turn on the server. Editing it would send different text under a new id,
@@ -2204,6 +2199,199 @@ describe('useChat remount send recovery', () => {
22042199
})
22052200
})
22062201

2202+
/** A chat with a turn running, so anything sent to it waits in its queue. */
2203+
function renderBusyChat(id: string) {
2204+
const history: MothershipChatHistory = {
2205+
id,
2206+
mode: 'agent',
2207+
title: 'Busy',
2208+
messages: [],
2209+
activeStreamId: 'turn-still-running',
2210+
resources: [],
2211+
}
2212+
mockRequestJson.mockImplementation(() => Promise.resolve({ chat: history }))
2213+
vi.stubGlobal('fetch', async (input: RequestInfo | URL, init?: RequestInit) => {
2214+
if (String(input).includes('/api/mothership/chat/stream')) {
2215+
if (String(input).includes('batch=true')) {
2216+
return Response.json({ success: true, events: [], status: 'streaming' })
2217+
}
2218+
return new Response(new ReadableStream<Uint8Array>(), {
2219+
headers: { 'Content-Type': 'text/event-stream' },
2220+
})
2221+
}
2222+
return fetchStub(input, init)
2223+
})
2224+
return { history, ...renderUseChatInChat(id, history) }
2225+
}
2226+
2227+
/**
2228+
* A send another surface withdrew arrives here under its original id, through
2229+
* the send event or the stored handoff. Queued behind a running turn, it may
2230+
* already be a turn on the server, so it can't be edited either.
2231+
*/
2232+
it('does not let a withdrawn send handed to a busy chat be edited', async () => {
2233+
const { history, getResult } = renderBusyChat('chat-busy-on-handoff')
2234+
await waitFor(() => getResult().isSending)
2235+
await act(async () => {
2236+
await getResult().sendMessage('handed over from another surface', undefined, undefined, {
2237+
resumeUserMessageId: 'withdrawn-attempt',
2238+
})
2239+
})
2240+
const queued = useMothershipQueueStore.getState().queues[history.id]?.[0]
2241+
expect(queued?.resumeUserMessageId).toBe('withdrawn-attempt')
2242+
2243+
let edited: ReturnType<ReturnType<typeof useChat>['editQueuedMessage']>
2244+
await act(async () => {
2245+
edited = getResult().editQueuedMessage(queued?.id ?? '')
2246+
})
2247+
2248+
expect(edited).toBeUndefined()
2249+
expect(getResult().editingQueuedId).toBeNull()
2250+
})
2251+
2252+
/** A follow-up whose dispatch got no answer may have reached the server too. */
2253+
it('does not let a queued follow-up be edited after its send got no answer', async () => {
2254+
const history: MothershipChatHistory = {
2255+
id: 'chat-follow-up-unanswered',
2256+
mode: 'agent',
2257+
title: 'Unanswered',
2258+
messages: [],
2259+
activeStreamId: null,
2260+
resources: [],
2261+
}
2262+
mockRequestJson.mockImplementation(() => Promise.resolve({ chat: history }))
2263+
vi.stubGlobal('fetch', async (input: RequestInfo | URL, init?: RequestInit) => {
2264+
if (String(input) === '/api/mothership/chat' && init?.method === 'POST') {
2265+
state.postBodies.push(JSON.parse(String(init.body)))
2266+
throw new TypeError('Failed to fetch')
2267+
}
2268+
return fetchStub(input, init)
2269+
})
2270+
useMothershipQueueStore
2271+
.getState()
2272+
.enqueue(history.id, { id: 'follow-up', content: 'and the second invoice' })
2273+
const { getResult } = renderUseChatInChat(history.id, history)
2274+
await waitFor(
2275+
() =>
2276+
useMothershipQueueStore.getState().queues[history.id]?.[0]?.resumeUserMessageId !==
2277+
undefined
2278+
)
2279+
2280+
let edited: ReturnType<ReturnType<typeof useChat>['editQueuedMessage']>
2281+
await act(async () => {
2282+
edited = getResult().editQueuedMessage('follow-up')
2283+
})
2284+
2285+
expect(edited).toBeUndefined()
2286+
expect(useMothershipQueueStore.getState().queues[history.id]?.[0]).toMatchObject({
2287+
content: 'and the second invoice',
2288+
resumeUserMessageId: state.postBodies[0].userMessageId,
2289+
})
2290+
})
2291+
2292+
/**
2293+
* Send-now on a resumed message whose Stop of the running turn does not
2294+
* settle sends nothing. That says nothing about the earlier attempt the
2295+
* message resumes, so it must stay uneditable.
2296+
*/
2297+
it('keeps a resumed message uneditable when its Send-now Stop does not settle', async () => {
2298+
state.abortSettlements = [false, false, false, false]
2299+
const { getResult } = renderUseChatInChat('chat-a')
2300+
await act(async () => {
2301+
void getResult().sendMessage('Original request')
2302+
})
2303+
await waitFor(() => state.postBodies.length === 1 && getResult().isSending)
2304+
await act(async () => {
2305+
await getResult().sendMessage('handed over from another surface', undefined, undefined, {
2306+
resumeUserMessageId: 'withdrawn-attempt',
2307+
})
2308+
})
2309+
await waitFor(() => useMothershipQueueStore.getState().queues['chat-a']?.length === 1)
2310+
2311+
await act(async () => {
2312+
await getResult()
2313+
.sendNow()
2314+
.catch(() => {})
2315+
await sleep(200)
2316+
})
2317+
const queued = useMothershipQueueStore.getState().queues['chat-a']?.[0]
2318+
let edited: ReturnType<ReturnType<typeof useChat>['editQueuedMessage']>
2319+
await act(async () => {
2320+
edited = getResult().editQueuedMessage(queued?.id ?? '')
2321+
})
2322+
2323+
expect(state.postBodies).toHaveLength(1)
2324+
expect(queued).toMatchObject({
2325+
content: 'handed over from another surface',
2326+
resumeUserMessageId: 'withdrawn-attempt',
2327+
admissionUnknown: true,
2328+
})
2329+
expect(edited).toBeUndefined()
2330+
})
2331+
2332+
/**
2333+
* A held message the server then refuses as busy is known not to be a turn
2334+
* there: the server answers a retry of an admitted id as a duplicate, never
2335+
* as busy. The user can edit it again.
2336+
*/
2337+
it('lets a held message be edited again once the server refuses it as busy', async () => {
2338+
const history: MothershipChatHistory = {
2339+
id: 'chat-held-then-refused',
2340+
mode: 'agent',
2341+
title: 'Refused',
2342+
messages: [],
2343+
activeStreamId: null,
2344+
resources: [],
2345+
}
2346+
mockRequestJson.mockImplementation(() => Promise.resolve({ chat: history }))
2347+
vi.stubGlobal('fetch', async (input: RequestInfo | URL, init?: RequestInit) => {
2348+
const url = String(input)
2349+
if (url === '/api/mothership/chat' && init?.method === 'POST') {
2350+
state.postBodies.push(JSON.parse(String(init.body)))
2351+
return Response.json(
2352+
{
2353+
error: 'A response is already in progress for this chat.',
2354+
activeStreamId: 'turn-from-another-tab',
2355+
},
2356+
{ status: 409 }
2357+
)
2358+
}
2359+
if (url.includes('/api/mothership/chat/stream')) {
2360+
if (url.includes('batch=true')) {
2361+
return Response.json({ success: true, events: [], status: 'streaming' })
2362+
}
2363+
return new Response(new ReadableStream<Uint8Array>(), {
2364+
headers: { 'Content-Type': 'text/event-stream' },
2365+
})
2366+
}
2367+
return fetchStub(input, init)
2368+
})
2369+
useMothershipQueueStore.getState().enqueue(history.id, {
2370+
id: 'held-first',
2371+
content: 'inspect the workspace',
2372+
resumeUserMessageId: 'first-attempt',
2373+
admissionUnknown: true,
2374+
})
2375+
const { getResult } = renderUseChatInChat(history.id, history)
2376+
await waitFor(() => state.postBodies.length === 1)
2377+
await waitFor(
2378+
() => useMothershipQueueStore.getState().queues[history.id]?.[0]?.id === 'held-first'
2379+
)
2380+
2381+
let edited: ReturnType<ReturnType<typeof useChat>['editQueuedMessage']>
2382+
await act(async () => {
2383+
edited = getResult().editQueuedMessage('held-first')
2384+
})
2385+
2386+
expect(edited?.content).toBe('inspect the workspace')
2387+
expect(getResult().editingQueuedId).toBe('held-first')
2388+
})
2389+
2390+
/**
2391+
* After a Stop of the first message, only the follow-up was the user's
2392+
* intent: the Stop's POST is left to the server, nothing withdraws it, and the
2393+
* next mount sends just the follow-up, once.
2394+
*/
22072395
it('sends only the follow-up after a failed Stop when the new-chat surface remounts', async () => {
22082396
stubFirstPostPendingThenAdmitted()
22092397
const first = renderHomeLikeSurface()

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

Lines changed: 31 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -234,6 +234,18 @@ interface WithdrawnSendResult {
234234
busy?: boolean
235235
/** Not sent at all (its Stop handoff failed); kept queued for the user to send. */
236236
held?: boolean
237+
/**
238+
* The server refused this id outright (busy, or a predecessor still shutting
239+
* down). It answers a retry of an admitted id as a duplicate instead, so the
240+
* server is known not to have it, and its queue entry can be edited.
241+
*/
242+
notAdmitted?: boolean
243+
/**
244+
* This attempt never reached the server (its Stop did not settle). That says
245+
* nothing about an earlier attempt the message resumes, whose uncertainty it
246+
* keeps.
247+
*/
248+
neverSent?: boolean
237249
}
238250

239251
/**
@@ -3887,7 +3899,7 @@ export function useChat(
38873899
setError(getErrorMessage(err, 'Failed to stop the previous response'))
38883900
/* Nothing was sent. Hand the message back so it stays in its chat's queue
38893901
even if the user has switched chats since the Stop began. */
3890-
return { userMessageId, held: true }
3902+
return { userMessageId, held: true, neverSent: true }
38913903
}
38923904
}
38933905

@@ -4004,7 +4016,7 @@ export function useChat(
40044016
}
40054017
if (viewOnSend)
40064018
setError('Previous response is still shutting down; queued message was restored.')
4007-
return { userMessageId, held: true }
4019+
return { userMessageId, held: true, notAdmitted: true }
40084020
}
40094021
/** Withdraws this refused send so the queue retries it, under the same id, later. */
40104022
const releaseRefusedSend = () => {
@@ -4040,7 +4052,7 @@ export function useChat(
40404052
exact: true,
40414053
refetchType: 'none',
40424054
})
4043-
return { userMessageId, busy: true }
4055+
return { userMessageId, busy: true, notAdmitted: true }
40444056
}
40454057
/* "Already sent" with no stream for it means the earlier attempt is still
40464058
in flight on the server (or died before starting a turn), not that a turn
@@ -4414,6 +4426,11 @@ export function useChat(
44144426
: {}),
44154427
...(result.held ? { retryRequired: true } : {}),
44164428
...(result.busy ? busyRetry(1) : {}),
4429+
admissionUnknown: result.notAdmitted
4430+
? false
4431+
: result.neverSent
4432+
? options?.resumeUserMessageId !== undefined
4433+
: true,
44174434
...((result.unreachable || result.busy) && activeChatKey.startsWith(PENDING_CHAT_KEY_PREFIX)
44184435
? { heldSurface: heldSendSurface }
44194436
: {}),
@@ -5043,6 +5060,17 @@ export function useChat(
50435060
? { heldSurface: heldSendSurface }
50445061
: {}),
50455062
...(withdrawnUserMessageId ? { resumeUserMessageId: withdrawnUserMessageId } : {}),
5063+
/* A refusal of this id settles it; an attempt that never left keeps the
5064+
earlier uncertainty; any other withdrawal may have reached the server. */
5065+
...(withdrawn
5066+
? {
5067+
admissionUnknown: withdrawn.notAdmitted
5068+
? false
5069+
: withdrawn.neverSent
5070+
? dispatched.admissionUnknown === true
5071+
: true,
5072+
}
5073+
: {}),
50465074
})
50475075
}
50485076

‎apps/sim/app/workspace/[workspaceId]/home/types.ts‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -35,9 +35,10 @@ export interface QueuedMessage {
3535
assistantSearch?: WorkspaceSearchFilters
3636
assistantSearchLevel?: AssistantSearchLevel
3737
/**
38-
* A first message withdrawn before the server answered. The server may
39-
* already hold it as sent, so it goes out exactly as written, under its
40-
* original id, and cannot be edited into a different message.
38+
* An earlier attempt at this message got no answer, so the server may
39+
* already hold it as sent. It goes out exactly as written, under that
40+
* attempt's id, and cannot be edited into a different message. False once
41+
* the server is known not to have it (it refused it, or it was never sent).
4142
*/
4243
admissionUnknown?: boolean
4344
}
Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
/**
2+
* @vitest-environment jsdom
3+
*/
4+
import { beforeEach, describe, expect, it } from 'vitest'
5+
import { useMothershipQueueStore } from '@/stores/mothership-queue/store'
6+
7+
describe('useMothershipQueueStore rehydration', () => {
8+
beforeEach(() => {
9+
useMothershipQueueStore.getState().reset()
10+
sessionStorage.clear()
11+
})
12+
13+
it('treats a resumed message saved before the edit guard as possibly sent', async () => {
14+
sessionStorage.setItem(
15+
'mothership-queue',
16+
JSON.stringify({
17+
state: {
18+
queues: {
19+
'chat-A': [
20+
{ id: 'saved-before', content: 'original', resumeUserMessageId: 'attempt-1' },
21+
{
22+
id: 'refused',
23+
content: 'original',
24+
resumeUserMessageId: 'attempt-2',
25+
admissionUnknown: false,
26+
},
27+
{ id: 'plain', content: 'never sent' },
28+
],
29+
},
30+
},
31+
version: 0,
32+
})
33+
)
34+
35+
await useMothershipQueueStore.persist.rehydrate()
36+
37+
const [savedBefore, refused, plain] = useMothershipQueueStore.getState().queues['chat-A'] ?? []
38+
expect(savedBefore?.admissionUnknown).toBe(true)
39+
expect(refused?.admissionUnknown).toBe(false)
40+
expect(plain?.admissionUnknown).toBeUndefined()
41+
})
42+
})

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

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,24 @@ describe('useMothershipQueueStore', () => {
3030
})
3131

3232
describe('replaceAt', () => {
33+
it('treats any message resuming an earlier attempt as possibly sent, unless told otherwise', () => {
34+
useMothershipQueueStore
35+
.getState()
36+
.enqueue('chat-A', { id: 'resumed', content: 'original', resumeUserMessageId: 'attempt-1' })
37+
useMothershipQueueStore.getState().insertAt('chat-A', 0, {
38+
id: 'refused',
39+
content: 'original',
40+
resumeUserMessageId: 'attempt-2',
41+
admissionUnknown: false,
42+
})
43+
useMothershipQueueStore.getState().replaceAt('chat-A', 'resumed', { content: 'edited' })
44+
useMothershipQueueStore.getState().replaceAt('chat-A', 'refused', { content: 'edited' })
45+
46+
const [refused, resumed] = useMothershipQueueStore.getState().queues['chat-A'] ?? []
47+
expect(resumed).toMatchObject({ content: 'original', resumeUserMessageId: 'attempt-1' })
48+
expect(refused?.content).toBe('edited')
49+
})
50+
3351
it('leaves a first message the server may already hold unchanged, with its id', () => {
3452
useMothershipQueueStore.getState().enqueue('chat-A', {
3553
id: 'm1',
@@ -50,6 +68,8 @@ describe('useMothershipQueueStore', () => {
5068
content: 'original',
5169
retryRequired: true,
5270
resumeUserMessageId: 'prior-request',
71+
/** Its Stop never settled, so it was never sent: the server cannot hold it. */
72+
admissionUnknown: false,
5373
queuedSendHandoff: {
5474
id: 'm1',
5575
chatId: 'chat-A',

0 commit comments

Comments
 (0)