Skip to content

Commit 30ccab5

Browse files
committed
fix(mothership): keep a withdrawn first message held at the queue head as written
A first message the unmount queues ahead of its follow-ups may already be a turn on the server. Editing it dropped its resume id, so the edit went out as a second message. The entry is now marked admissionUnknown: the store ignores edits to it, the hook refuses to open it for editing, and its Edit button is disabled. The ordering test's resend now gets the server's real dedupe 409.
1 parent 351b8b9 commit 30ccab5

6 files changed

Lines changed: 109 additions & 7 deletions

File tree

‎apps/sim/app/workspace/[workspaceId]/home/components/queued-messages/queued-messages.tsx‎

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,7 @@ export function QueuedMessages({
7474
{messageQueue.map((msg) => {
7575
const isEditing = msg.id === editingQueuedId
7676
const isDispatching = msg.id === dispatchingHeadId
77+
const editBlocked = isDispatching || msg.admissionUnknown === true
7778
return (
7879
<div
7980
key={msg.id}
@@ -141,9 +142,15 @@ export function QueuedMessages({
141142
<Tooltip.Root>
142143
<Tooltip.Trigger asChild>
143144
<button
144-
aria-label={isDispatching ? 'Sending now' : 'Edit queued message'}
145+
aria-label={
146+
isDispatching
147+
? 'Sending now'
148+
: msg.admissionUnknown
149+
? 'Already sent; editing it would send a second message'
150+
: 'Edit queued message'
151+
}
145152
type='button'
146-
disabled={isDispatching}
153+
disabled={editBlocked}
147154
onClick={(e) => {
148155
e.stopPropagation()
149156
onEdit(msg.id)
@@ -154,7 +161,11 @@ export function QueuedMessages({
154161
</button>
155162
</Tooltip.Trigger>
156163
<Tooltip.Content side='top' sideOffset={4}>
157-
{isDispatching ? 'Sending now' : 'Edit queued message'}
164+
{isDispatching
165+
? 'Sending now'
166+
: msg.admissionUnknown
167+
? 'Already sent; editing it would send a second message'
168+
: 'Edit queued message'}
158169
</Tooltip.Content>
159170
</Tooltip.Root>
160171

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

Lines changed: 71 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2053,9 +2053,11 @@ describe('useChat remount send recovery', () => {
20532053
})
20542054

20552055
/**
2056-
* The first POST on the new-chat surface never answers; later POSTs open a
2057-
* turn in the chat the first message created. The abort endpoint and the
2058-
* stream lookup fail, as they would for a Stop that cannot reach the server.
2056+
* The first POST on the new-chat surface reaches the server, which admits it,
2057+
* but its answer never arrives. A resend under that id gets the server's
2058+
* dedupe answer naming the chat it opened; any other POST opens a turn in
2059+
* that chat. Until the remount, the abort endpoint and the stream lookup
2060+
* fail, as they would for a Stop that cannot reach the server.
20592061
*/
20602062
function stubFirstPostPendingThenAdmitted() {
20612063
vi.stubGlobal('fetch', async (input: RequestInfo | URL, init?: RequestInit) => {
@@ -2069,6 +2071,17 @@ describe('useChat remount send recovery', () => {
20692071
})
20702072
})
20712073
}
2074+
const firstId = state.postBodies[0].userMessageId
2075+
if (state.postBodies.at(-1)?.userMessageId === firstId) {
2076+
return Response.json(
2077+
{
2078+
error: 'This message was already sent.',
2079+
activeStreamId: firstId,
2080+
chatId: DEDUPED_CHAT_ID,
2081+
},
2082+
{ status: 409 }
2083+
)
2084+
}
20722085
return new Response(
20732086
new ReadableStream<Uint8Array>({
20742087
start(controller) {
@@ -2113,6 +2126,13 @@ describe('useChat remount send recovery', () => {
21132126
})
21142127
await waitFor(() => allQueuedMessages().length === 1)
21152128
first.unmount()
2129+
/** Held first, as written: the server may already have it under its id. */
2130+
expect(
2131+
allQueuedMessages().map((message) => [message.content, message.admissionUnknown])
2132+
).toEqual([
2133+
['inspect the workspace', true],
2134+
['follow-up while admission pending', undefined],
2135+
])
21162136

21172137
const second = renderHomeLikeSurface()
21182138
await waitFor(() => state.postBodies.length >= 3, 4_000)
@@ -2136,6 +2156,54 @@ describe('useChat remount send recovery', () => {
21362156
* intent: the Stop's POST is left to the server, nothing withdraws it, and the
21372157
* next mount sends just the follow-up, once.
21382158
*/
2159+
/**
2160+
* A first message held at the queue head after a remount may already be a
2161+
* turn on the server. Editing it would send different text under a new id,
2162+
* a second message the user never meant to send.
2163+
*/
2164+
it('does not let a withdrawn first message held at the queue head be edited', async () => {
2165+
const history: MothershipChatHistory = {
2166+
id: 'chat-running-while-held',
2167+
mode: 'agent',
2168+
title: 'Held',
2169+
messages: [],
2170+
activeStreamId: 'turn-still-running',
2171+
resources: [],
2172+
}
2173+
mockRequestJson.mockImplementation(() => Promise.resolve({ chat: history }))
2174+
vi.stubGlobal('fetch', async (input: RequestInfo | URL, init?: RequestInit) => {
2175+
if (String(input).includes('/api/mothership/chat/stream')) {
2176+
if (String(input).includes('batch=true')) {
2177+
return Response.json({ success: true, events: [], status: 'streaming' })
2178+
}
2179+
return new Response(new ReadableStream<Uint8Array>(), {
2180+
headers: { 'Content-Type': 'text/event-stream' },
2181+
})
2182+
}
2183+
return fetchStub(input, init)
2184+
})
2185+
useMothershipQueueStore.getState().enqueue(history.id, {
2186+
id: 'held-first',
2187+
content: 'inspect the workspace',
2188+
resumeUserMessageId: 'first-attempt',
2189+
admissionUnknown: true,
2190+
})
2191+
const { getResult } = renderUseChatInChat(history.id, history)
2192+
await waitFor(() => getResult().isSending)
2193+
2194+
let edited: ReturnType<ReturnType<typeof useChat>['editQueuedMessage']>
2195+
await act(async () => {
2196+
edited = getResult().editQueuedMessage('held-first')
2197+
})
2198+
2199+
expect(edited).toBeUndefined()
2200+
expect(getResult().editingQueuedId).toBeNull()
2201+
expect(useMothershipQueueStore.getState().queues[history.id]?.[0]).toMatchObject({
2202+
content: 'inspect the workspace',
2203+
resumeUserMessageId: 'first-attempt',
2204+
})
2205+
})
2206+
21392207
it('sends only the follow-up after a failed Stop when the new-chat surface remounts', async () => {
21402208
stubFirstPostPendingThenAdmitted()
21412209
const first = renderHomeLikeSurface()

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5235,7 +5235,7 @@ export function useChat(
52355235
const activeChatKey = chatKeyRef.current
52365236
const queue = useMothershipQueueStore.getState().queues[activeChatKey] ?? EMPTY_MESSAGE_QUEUE
52375237
const msg = queue.find((m) => m.id === id)
5238-
if (!msg) return undefined
5238+
if (!msg || msg.admissionUnknown) return undefined
52395239
// Evict any sessionStorage handoff — a failed prior dispatch may have left
52405240
// a pre-edit content snapshot that the recovery effect would otherwise replay.
52415241
clearQueuedSendHandoffState(id)
@@ -5352,6 +5352,7 @@ export function useChat(
53525352
id: generateId(),
53535353
content: send.content,
53545354
resumeUserMessageId: send.userMessageId,
5355+
admissionUnknown: true,
53555356
...(send.fileAttachments ? { fileAttachments: send.fileAttachments } : {}),
53565357
...(send.contexts ? { contexts: send.contexts } : {}),
53575358
...(send.requestMode ? { requestMode: send.requestMode } : {}),

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

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,12 @@ export interface QueuedMessage {
3434
requestMode?: ChatRequestMode
3535
assistantSearch?: WorkspaceSearchFilters
3636
assistantSearchLevel?: AssistantSearchLevel
37+
/**
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.
41+
*/
42+
admissionUnknown?: boolean
3743
}
3844

3945
export const ToolCallStatus = {

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

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

3232
describe('replaceAt', () => {
33+
it('leaves a first message the server may already hold unchanged, with its id', () => {
34+
useMothershipQueueStore.getState().enqueue('chat-A', {
35+
id: 'm1',
36+
content: 'original',
37+
resumeUserMessageId: 'first-attempt',
38+
admissionUnknown: true,
39+
})
40+
useMothershipQueueStore.getState().replaceAt('chat-A', 'm1', { content: 'edited' })
41+
expect(useMothershipQueueStore.getState().queues['chat-A']?.[0]).toMatchObject({
42+
content: 'original',
43+
resumeUserMessageId: 'first-attempt',
44+
})
45+
})
46+
3347
it('editing preserves an unresolved Stop while replacing the prior request identity', () => {
3448
useMothershipQueueStore.getState().enqueue('chat-A', {
3549
id: 'm1',

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -96,6 +96,8 @@ export const useMothershipQueueStore = create<MothershipQueueState>()(
9696
const current = state.queues[chatKey] ?? []
9797
const index = current.findIndex((m) => m.id === id)
9898
if (index === -1) return state
99+
/** The server may already hold it as sent; an edit would become a second message. */
100+
if (current[index].admissionUnknown) return state
99101
const next = [...current]
100102
/** Editing changes the request identity, never an unresolved Stop dependency. */
101103
const {

0 commit comments

Comments
 (0)