Skip to content

Commit a3fb22f

Browse files
committed
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 80ee657 commit a3fb22f

4 files changed

Lines changed: 129 additions & 6 deletions

File tree

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

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2289,6 +2289,46 @@ describe('useChat remount send recovery', () => {
22892289
})
22902290
})
22912291

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+
22922332
/**
22932333
* A held message the server then refuses as busy is known not to be a turn
22942334
* there: the server answers a retry of an admitted id as a duplicate, never

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

Lines changed: 26 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -235,10 +235,17 @@ interface WithdrawnSendResult {
235235
/** Not sent at all (its Stop handoff failed); kept queued for the user to send. */
236236
held?: boolean
237237
/**
238-
* The server is known not to have it: it was never sent, or the server
239-
* refused it outright. Its queue entry can be edited.
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.
240241
*/
241242
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
242249
}
243250

244251
/**
@@ -3892,7 +3899,7 @@ export function useChat(
38923899
setError(getErrorMessage(err, 'Failed to stop the previous response'))
38933900
/* Nothing was sent. Hand the message back so it stays in its chat's queue
38943901
even if the user has switched chats since the Stop began. */
3895-
return { userMessageId, held: true, notAdmitted: true }
3902+
return { userMessageId, held: true, neverSent: true }
38963903
}
38973904
}
38983905

@@ -4419,7 +4426,11 @@ export function useChat(
44194426
: {}),
44204427
...(result.held ? { retryRequired: true } : {}),
44214428
...(result.busy ? busyRetry(1) : {}),
4422-
...(result.notAdmitted ? { admissionUnknown: false } : {}),
4429+
admissionUnknown: result.notAdmitted
4430+
? false
4431+
: result.neverSent
4432+
? options?.resumeUserMessageId !== undefined
4433+
: true,
44234434
...((result.unreachable || result.busy) && activeChatKey.startsWith(PENDING_CHAT_KEY_PREFIX)
44244435
? { heldSurface: heldSendSurface }
44254436
: {}),
@@ -5049,8 +5060,17 @@ export function useChat(
50495060
? { heldSurface: heldSendSurface }
50505061
: {}),
50515062
...(withdrawnUserMessageId ? { resumeUserMessageId: withdrawnUserMessageId } : {}),
5052-
/** This attempt's outcome decides; an earlier refusal says nothing about it. */
5053-
...(withdrawn ? { admissionUnknown: !withdrawn.notAdmitted } : {}),
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+
: {}),
50545074
})
50555075
}
50565076

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.ts‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import { createLogger } from '@sim/logger'
22
import { toError } from '@sim/utils/errors'
3+
import { toRecord, toRecordOrNull } from '@sim/utils/object'
34
import { create } from 'zustand'
45
import { createJSONStorage, devtools, persist } from 'zustand/middleware'
56
import type { MothershipQueueState, QueuedMothershipMessage } from '@/stores/mothership-queue/types'
@@ -63,6 +64,25 @@ function withAdmissionGuard(message: QueuedMothershipMessage): QueuedMothershipM
6364
return { ...message, admissionUnknown: true }
6465
}
6566

67+
function isQueuedMessage(value: unknown): value is QueuedMothershipMessage {
68+
const record = toRecordOrNull(value)
69+
return record !== null && typeof record.id === 'string' && typeof record.content === 'string'
70+
}
71+
72+
/**
73+
* Queues saved to this tab's session, guarded on the way back in: an entry
74+
* saved before `admissionUnknown` existed would otherwise be editable.
75+
*/
76+
function restoredQueues(persisted: unknown): Record<string, QueuedMothershipMessage[]> {
77+
const queues: Record<string, QueuedMothershipMessage[]> = {}
78+
for (const [chatKey, queue] of Object.entries(toRecord(toRecord(persisted).queues))) {
79+
if (!Array.isArray(queue)) continue
80+
const messages = queue.filter(isQueuedMessage).map(withAdmissionGuard)
81+
if (messages.length > 0) queues[chatKey] = messages
82+
}
83+
return queues
84+
}
85+
6686
const omitKey = <V>(record: Record<string, V>, key: string): Record<string, V> => {
6787
if (!(key in record)) return record
6888
const { [key]: _removed, ...rest } = record
@@ -263,6 +283,7 @@ export const useMothershipQueueStore = create<MothershipQueueState>()(
263283
// edit text is component-local and empty after reload, so a persisted
264284
// editing flag would render an in-edit row with nothing bound.
265285
partialize: (state) => ({ queues: state.queues }),
286+
merge: (persisted, current) => ({ ...current, queues: restoredQueues(persisted) }),
266287
}
267288
),
268289
{ name: 'mothership-queue-store' }

0 commit comments

Comments
 (0)