fix(chat): keep a withdrawn send in its own chat, and release stranded claims

Audit follow-ups, two of them real defects in the previous commit.

A withdrawn send routed unconditionally through the cross-surface lanes. Those
deliver to whatever chat is mounted next, so sending in one chat and switching
to another re-sent the message into the second one. The dispatcher already drew
the distinction; the idle path now draws it too — a chat-bound key is the stable
chat id, so re-queueing under it both retries durably and keeps the message
where the user put it. Only a chatless key, which dies with its mount, goes to
the lanes.

The claim release sat in `catch`, so the two paths that return a response
without throwing — a rejected branch, and a missing chat — stranded an
in-progress claim for its full 60s TTL, and a retry inside that window got a
spurious "already sent" instead of the real error. Moved to `finally`.

Also: `userMessageId` is now length-bounded, since it becomes part of a Postgres
key and an oversized one would throw inside the claim; `requestId` was still
empty at claim time, so both dedup logs printed a blank prefix; the provider
segment said `mothership` on a handler that also serves the workflow copilot,
and now says what the key identifies; `retryFailures` was dead config, only read
by `executeWithIdempotency`, which this caller never invokes; the doc pointed at
`billingIdempotency`, which has no consumers, and now points at the live Stripe
analogue.

Trimmed: `sendClaimRecorded` folded into clearing `sendClaim`, the unread `kind`
discriminant dropped from a one-arm union, the single-use `claimedChatId`
inlined, and the prose on all three of those cut back to what the code does not
already say.

Author: waleedlatif1

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

@@ -145,6 +145,43 @@ function renderUseChat(): {
   }
 }
 
+/**
+ * As `renderUseChat`, but bound to an existing chat rather than chatless. The
+ * pathname has to match: the hook resets a chat-bound surface back to a fresh
+ * pending key when it finds itself on the home route.
+ */
+function renderUseChatInChat(chatId: string): {
+  getResult: () => ReturnType<typeof useChat>
+  unmount: () => void
+} {
+  navigationMocks.usePathname.mockReturnValue(`/workspace/ws-1/chat/${chatId}`)
+  ;(globalThis as { IS_REACT_ACT_ENVIRONMENT?: boolean }).IS_REACT_ACT_ENVIRONMENT = true
+  queryClient = new QueryClient({ defaultOptions: { queries: { retry: false } } })
+  const container = document.createElement('div')
+  const root = createRoot(container)
+  mountedRoots.push(root)
+  let result: ReturnType<typeof useChat> | undefined
+
+  function Probe() {
+    result = useChat('ws-1', chatId)
+    return null
+  }
+
+  act(() => {
+    root.render(
+      <QueryClientProvider client={queryClient}>{(<Probe />) as ReactNode}</QueryClientProvider>
+    )
+  })
+
+  return {
+    getResult: () => {
+      if (result === undefined) throw new Error('Hook result is not ready')
+      return result
+    },
+    unmount: () => act(() => root.unmount()),
+  }
+}
+
 /**
  * Mounts a surface shaped like `home.tsx`: it drives `useChat` AND registers the
  * `mothership-send-message` listener that claims the event with
@@ -260,6 +297,7 @@ async function waitFor(predicate: () => boolean, budgetMs = 2000): Promise<void>
 describe('useChat remount send recovery', () => {
   beforeEach(() => {
     vi.stubGlobal('fetch', fetchStub)
+    navigationMocks.usePathname.mockReturnValue('/workspace/ws-1/home')
     state.postBehavior = 'hang'
     state.postBodies = []
     mockRequestJson.mockResolvedValue({ chats: [] })
@@ -444,4 +482,31 @@ describe('useChat remount send recovery', () => {
       expect(state.postBodies[0].userMessageId).toBe('the-first-attempt')
     })
   })
+
+  /**
+   * A withdrawn send belongs to the chat it was sent to. The cross-surface
+   * lanes deliver to whatever chat is mounted next, so routing a chat-bound
+   * send through them would drop the message into a different conversation —
+   * exactly what happens if the user switches chats mid-send. Its key is the
+   * stable chat id, so re-queueing under that key is the durable retry.
+   */
+  it('re-queues a withdrawn chat-bound send instead of following the user', async () => {
+    const { getResult, unmount } = renderUseChatInChat('chat-a')
+
+    await act(async () => {
+      void getResult().sendMessage('belongs to chat-a')
+    })
+    await waitFor(() => state.postBodies.length === 1)
+
+    unmount()
+    await waitFor(() => allQueuedMessages().length === 1)
+
+    const queues = useMothershipQueueStore.getState().queues
+    expect(Object.keys(queues)).toEqual(['chat-a'])
+    expect(queues['chat-a'][0].content).toBe('belongs to chat-a')
+    // Reused on the retry so the server deduplicates it.
+    expect(queues['chat-a'][0].resumeUserMessageId).toBe(state.postBodies[0].userMessageId)
+    // Must NOT have gone to the cross-surface handoff.
+    expect(MothershipHandoffStorage.consume('ws-1')).toBeNull()
+  })
 })

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

@@ -151,7 +151,7 @@ export interface SendMessageOptions {
  * when an unmount cleanup withdrew it — `userMessageId` is what a retry reuses
  * so the server deduplicates the two attempts.
  */
-type StartSendMessageResult = boolean | { kind: 'withdrawn_by_cleanup'; userMessageId: string }
+type StartSendMessageResult = boolean | { userMessageId: string }
 
 interface StartSendMessageOptions {
   /** Awaited before dispatch. Defaults to the hook's in-flight stop, if any. */
@@ -3905,7 +3905,7 @@ export function useChat(
                server deduplicates it against that turn instead of billing
                another one. */
             rollbackOptimisticSend()
-            return { kind: 'withdrawn_by_cleanup', userMessageId }
+            return { userMessageId }
           }
           return consumedByTranscript
         }
@@ -4031,14 +4031,29 @@ export function useChat(
       }
 
       const result = await startSendMessage(message, fileAttachments, contexts, options)
-      if (typeof result === 'object') {
-        handOffWithdrawnSend({
-          content: message,
-          fileAttachments,
-          contexts,
-          userMessageId: result.userMessageId,
-        })
+      if (typeof result !== 'object') return
+
+      /* An unmount cleanup withdrew the send. A chat-bound key is the stable
+         chat id, so re-queueing under the key this was sent to is the durable
+         retry — and keeps the message in that chat rather than following the
+         user into whichever one they opened next. Only a chatless surface,
+         whose key dies with the mount, goes to the cross-surface lanes. */
+      const withdrawn = {
+        content: message,
+        fileAttachments,
+        contexts,
+        userMessageId: result.userMessageId,
       }
+      if (activeChatKey.startsWith(PENDING_CHAT_KEY_PREFIX)) {
+        handOffWithdrawnSend(withdrawn)
+        return
+      }
+      useMothershipQueueStore
+        .getState()
+        .enqueue(
+          activeChatKey,
+          createQueuedMessage(message, fileAttachments, contexts, result.userMessageId)
+        )
     },
     [workspaceId, createQueuedMessage, startSendMessage, handOffWithdrawnSend]
   )
@@ -4578,6 +4593,10 @@ export function useChat(
         useMothershipQueueStore.getState().remove(dispatchChatKey, msg.id)
       }
 
+      /* What actually went out. `msg` is the snapshot from when the dispatch was
+         scheduled; the send below uses the re-read live entry, so recovery
+         tracks that rather than assuming the two still match. */
+      let dispatched = msg
       const restoreQueuedMessage = (
         handoff?: QueuedSendHandoffSeed,
         withdrawnUserMessageId?: string
@@ -4604,15 +4623,15 @@ export function useChat(
            the queue itself is the durable retry. */
         if (withdrawnByCleanup && dispatchChatKey.startsWith(PENDING_CHAT_KEY_PREFIX)) {
           handOffWithdrawnSend({
-            content: msg.content,
-            fileAttachments: msg.fileAttachments,
-            contexts: msg.contexts,
+            content: dispatched.content,
+            fileAttachments: dispatched.fileAttachments,
+            contexts: dispatched.contexts,
             userMessageId: withdrawnUserMessageId,
           })
           return
         }
         useMothershipQueueStore.getState().insertAt(dispatchChatKey, originalIndex, {
-          ...msg,
+          ...dispatched,
           ...(withdrawnUserMessageId ? { resumeUserMessageId: withdrawnUserMessageId } : {}),
         })
       }
@@ -4631,6 +4650,7 @@ export function useChat(
         // Re-read live: the user may have applied an in-place edit (`replaceAt`)
         // between dispatch scheduling and this send.
         const liveMsg = queueAtSend[currentIndex]
+        dispatched = liveMsg
         activeQueuedSendHandoff = options.queuedSendHandoff ?? liveMsg.queuedSendHandoff
 
         const sendResult = await startSendMessage(

apps/sim/lib/copilot/chat/post.test.ts

@@ -148,7 +148,7 @@ describe('handleUnifiedChatPost', () => {
     resetDbChainMock()
     atomicallyClaimChatSend.mockResolvedValue({
       claimed: true,
-      normalizedKey: 'chat-send:mothership:msg-1:userId=user-1',
+      normalizedKey: 'chat-send:user-message:msg-1:userId=user-1',
       storageMethod: 'database',
       claimToken: 'claim-1',
     })
@@ -758,7 +758,7 @@ describe('handleUnifiedChatPost', () => {
     it('answers an already-claimed send with the chat the first attempt opened', async () => {
       atomicallyClaimChatSend.mockResolvedValue({
         claimed: false,
-        normalizedKey: 'chat-send:mothership:msg-1:userId=user-1',
+        normalizedKey: 'chat-send:user-message:msg-1:userId=user-1',
         storageMethod: 'database',
         existingResult: { success: true, status: 'completed', result: { chatId: 'chat-first' } },
       })
@@ -798,7 +798,7 @@ describe('handleUnifiedChatPost', () => {
         })
       )
 
-      expect(atomicallyClaimChatSend).toHaveBeenCalledWith('mothership', 'msg-1', {
+      expect(atomicallyClaimChatSend).toHaveBeenCalledWith('user-message', 'msg-1', {
         userId: 'user-1',
       })
     })
@@ -817,7 +817,7 @@ describe('handleUnifiedChatPost', () => {
       )
 
       expect(storeChatSendResult).toHaveBeenCalledWith(
-        'chat-send:mothership:msg-1:userId=user-1',
+        'chat-send:user-message:msg-1:userId=user-1',
         expect.objectContaining({ result: { chatId: 'chat-1' } }),
         'database',
         'claim-1'

apps/sim/lib/copilot/chat/post.ts

@@ -260,7 +260,10 @@ const ChatContextSchema = z
 
 const ChatMessageSchema = z.object({
   message: z.string().min(1, 'Message is required'),
-  userMessageId: z.string().optional(),
+  /* Bounded because it becomes part of a Postgres key in `chatSendIdempotency`;
+     a client-supplied id longer than the btree entry limit would throw there.
+     A generated id is 36 chars. */
+  userMessageId: z.string().max(128).optional(),
   chatId: z.string().optional(),
   workflowId: z.string().optional(),
   workspaceId: z.string().optional(),
@@ -945,26 +948,22 @@ async function resolveBranch(params: {
   }
 }
 
-/** Namespace-local provider segment for {@link chatSendIdempotency} keys. */
-const CHAT_SEND_IDEMPOTENCY_PROVIDER = 'mothership'
+/** Names what the key identifies: `chat-send:user-message:<id>:userId=<id>`. */
+const CHAT_SEND_IDEMPOTENCY_PROVIDER = 'user-message'
 
 /**
  * Claims this send so a retry of it can be recognised.
  *
- * Fails open. Deduplication is an optimization here — it saves a duplicate chat
- * and a duplicate billed turn — whereas the send itself is the user's message,
- * and refusing it because a bookkeeping store is unreachable would take chat
- * down for everyone rather than degrade it. An unavailable store therefore
- * returns `undefined`, which sends normally with no claim to finalize.
+ * Fails open: a missed deduplication costs a duplicate chat and turn, but
+ * refusing the send loses the user's message. Returns `undefined` when the
+ * store is unreachable, which sends normally with no claim to finalize.
  *
- * The key is scoped to the caller: `userMessageId` is client-supplied, so an
- * unscoped one would let a user probe another's sends and read back their chat
- * id.
+ * The key is scoped to the caller — `userMessageId` is client-supplied, so an
+ * unscoped one would let a user probe another's sends for their chat id.
  */
 async function claimChatSend(
   userMessageId: string,
-  userId: string,
-  requestId: string
+  userId: string
 ): Promise<AtomicClaimResult | undefined> {
   try {
     return await chatSendIdempotency.atomicallyClaim(
@@ -973,20 +972,14 @@ async function claimChatSend(
       { userId }
     )
   } catch (error) {
-    logger.warn(`[${requestId}] Could not claim chat send; proceeding without deduplication`, {
+    logger.warn('Could not claim chat send; proceeding without deduplication', {
       userMessageId,
       error: getErrorMessage(error, 'Unknown error'),
     })
     return undefined
   }
 }
 
-/** Chat a previously-claimed send resolved to, when it got that far. */
-function claimedChatId(claim: AtomicClaimResult): string | undefined {
-  const chatId = claim.existingResult?.result?.chatId
-  return typeof chatId === 'string' && chatId ? chatId : undefined
-}
-
 /**
  * Answers a send whose `userMessageId` was already claimed.
  *
@@ -997,13 +990,10 @@ function claimedChatId(claim: AtomicClaimResult): string | undefined {
  * first attempt got far enough to resolve one, letting a chatless client adopt
  * it without a stream-to-chat lookup.
  */
-function duplicateChatSendResponse(
-  claim: AtomicClaimResult,
-  userMessageId: string,
-  requestId: string
-): NextResponse {
-  const chatId = claimedChatId(claim)
-  logger.info(`[${requestId}] Deduplicated a repeated chat send`, { userMessageId, chatId })
+function duplicateChatSendResponse(claim: AtomicClaimResult, userMessageId: string): NextResponse {
+  const claimed = claim.existingResult?.result?.chatId
+  const chatId = typeof claimed === 'string' && claimed ? claimed : undefined
+  logger.info('Deduplicated a repeated chat send', { userMessageId, chatId })
   return NextResponse.json(
     {
       error: 'This message was already sent.',
@@ -1018,13 +1008,8 @@ export async function handleUnifiedChatPost(req: NextRequest) {
   let actualChatId: string | undefined
   let userMessageId = ''
   let chatStreamLockAcquired = false
-  /* Held from the moment this send is claimed until the chat it belongs to is
-     recorded. Released on the error path so a failed send can be retried, and
-     deliberately NOT released once recorded — by then the chat exists and the
-     user message is persisted, so a retry must resolve to it rather than open
-     a second chat. */
+  /** Cleared once the chat is recorded against it, which makes it permanent. */
   let sendClaim: AtomicClaimResult | undefined
-  let sendClaimRecorded = false
   // Started once we've parsed the body (need userMessageId to stamp as
   // streamId). Every subsequent span (persistUserMessage,
   // createRunSegment, the whole SSE stream, etc.) nests under this
@@ -1059,9 +1044,9 @@ export async function handleUnifiedChatPost(req: NextRequest) {
     const normalizedContexts = normalizeContexts(body.contexts) ?? []
     userMessageId = body.userMessageId || generateId()
 
-    sendClaim = await claimChatSend(userMessageId, authenticatedUserId, requestId)
+    sendClaim = await claimChatSend(userMessageId, authenticatedUserId)
     if (sendClaim?.claimed === false) {
-      return duplicateChatSendResponse(sendClaim, userMessageId, requestId)
+      return duplicateChatSendResponse(sendClaim, userMessageId)
     }
 
     otelRoot = startCopilotOtelRoot({
@@ -1156,13 +1141,12 @@ export async function handleUnifiedChatPost(req: NextRequest) {
         }
       }
 
-      /* Record the chat against this send as soon as it is known, the earliest
-         a retry can be answered with somewhere to go. From here the claim is
-         permanent: the user message lands moments later, so a retry must
-         resolve to this chat rather than open another. Failing to record only
-         costs deduplication, so it must not fail the send. */
+      /* Record the chat as soon as it is known — the earliest a retry can be
+         answered with somewhere to go. The claim becomes permanent here: the
+         user message lands moments later, so a retry must resolve to this chat
+         rather than open another. */
       if (sendClaim?.claimToken && actualChatId) {
-        sendClaimRecorded = await chatSendIdempotency
+        const recorded = await chatSendIdempotency
           .storeResult(
             sendClaim.normalizedKey,
             { success: true, status: 'completed', result: { chatId: actualChatId } },
@@ -1176,6 +1160,7 @@ export async function handleUnifiedChatPost(req: NextRequest) {
             })
             return false
           })
+        if (recorded) sendClaim = undefined
       }
 
       if (chatIsNew && actualChatId && body.resourceAttachments?.length) {
@@ -1482,19 +1467,6 @@ export async function handleUnifiedChatPost(req: NextRequest) {
     if (chatStreamLockAcquired && actualChatId && userMessageId) {
       await releasePendingChatStream(actualChatId, userMessageId)
     }
-    // Nothing was recorded against this send, so let a retry of it through
-    // rather than deduplicating against a chat that was never opened. A failed
-    // release just leaves the claim to expire on its own short in-progress TTL.
-    if (sendClaim?.claimToken && !sendClaimRecorded) {
-      await chatSendIdempotency
-        .release(sendClaim.normalizedKey, sendClaim.storageMethod, sendClaim.claimToken)
-        .catch((releaseError) => {
-          logger.warn(`[${requestId}] Could not release the claim for a failed send`, {
-            userMessageId,
-            error: getErrorMessage(releaseError, 'Unknown error'),
-          })
-        })
-    }
     otelRoot?.finish('error', error)
 
     if (isZodError(error)) {
@@ -1524,5 +1496,20 @@ export async function handleUnifiedChatPost(req: NextRequest) {
       },
       { status: 500 }
     )
+  } finally {
+    /* A claim still held here never recorded a chat — the send threw, or
+       returned early on a rejected branch or a missing chat. Release it so a
+       retry is treated as new rather than deduplicated against a chat that was
+       never opened. Must be `finally`: those early returns skip `catch`. */
+    if (sendClaim?.claimToken) {
+      await chatSendIdempotency
+        .release(sendClaim.normalizedKey, sendClaim.storageMethod, sendClaim.claimToken)
+        .catch((releaseError) => {
+          logger.warn('Could not release the claim for an unfinished send', {
+            userMessageId,
+            error: getErrorMessage(releaseError, 'Unknown error'),
+          })
+        })
+    }
   }
 }