Skip to content

Commit 45013a4

Browse files
committed
fix(chat): hand off a chatless send when the probe is superseded
The previous commit made a superseded probe leave the entry queued rather than re-send it. That is the right retry for a chat-bound key, which is the stable chat id, but wrong for a chatless one: a `pending::` key is regenerated every mount, so anything left under it is unreachable and the message is stranded — the same loss this PR exists to prevent, just reached by a different route. A superseded probe on a pending key now goes through the same recovery lanes as the cleanup-abort path (live replacement surface, else a one-shot stored handoff), still carrying the stream id so the next surface probes before it sends. Skipped when the entry is no longer under that key, since adoption migrating it to a live chat already leaves it recoverable there. The lane is extracted so both call sites share one implementation. The existing superseded test only asserted that nothing sent, which this bug satisfied trivially; it now also asserts the message survives. Confirmed red without the fix.
1 parent 39e2a84 commit 45013a4

2 files changed

Lines changed: 52 additions & 16 deletions

File tree

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

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -519,6 +519,13 @@ describe('useChat remount send recovery', () => {
519519
await sleep(3000)
520520
})
521521
expect(state.postCalls).toBe(postsBeforeUnmount)
522+
523+
/* Not sending is only half of it — the message must still be
524+
recoverable. A chatless surface's `pending::` key is regenerated per
525+
mount, so leaving the entry there would strand it just as surely as
526+
re-sending would have duplicated it. */
527+
expect(MothershipHandoffStorage.consume('ws-1')?.message).toBe('do not zombie me')
528+
expect(allQueuedMessages()).toHaveLength(0)
522529
})
523530

524531
it('re-sends when the server has no stream for it', async () => {

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

Lines changed: 45 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -4586,6 +4586,30 @@ export function useChat(
45864586
useMothershipQueueStore.getState().remove(dispatchChatKey, msg.id)
45874587
}
45884588

4589+
/**
4590+
* Hands a chatless send to whatever surface comes next, because its
4591+
* `pending::` queue key is regenerated per mount and anything left under
4592+
* this one is unreachable. Prefers the live replacement surface's
4593+
* listener and falls back to a one-shot stored handoff for a real
4594+
* navigation away. Both lanes carry attachments and the stream id, so the
4595+
* next surface probes before it sends.
4596+
*/
4597+
const handOffChatlessRecovery = (recoverStreamId?: string) => {
4598+
if (
4599+
!sendMothershipMessage(msg.content, msg.contexts, msg.fileAttachments, recoverStreamId)
4600+
) {
4601+
MothershipHandoffStorage.store(
4602+
{
4603+
message: msg.content,
4604+
...(msg.contexts?.length ? { contexts: msg.contexts } : {}),
4605+
...(msg.fileAttachments?.length ? { fileAttachments: msg.fileAttachments } : {}),
4606+
...(recoverStreamId ? { recoverStreamId } : {}),
4607+
},
4608+
workspaceId
4609+
)
4610+
}
4611+
}
4612+
45894613
const restoreQueuedMessage = (handoff?: QueuedSendHandoffSeed, recoverStreamId?: string) => {
45904614
const recoverableCleanupAbort = recoverStreamId !== undefined
45914615
if (!handoff) {
@@ -4613,19 +4637,7 @@ export function useChat(
46134637
Chat-bound sends keep the queue restore — their key is the stable
46144638
chat id — and carry the stream id on the restored entry. */
46154639
if (recoverableCleanupAbort && dispatchChatKey.startsWith(PENDING_CHAT_KEY_PREFIX)) {
4616-
if (
4617-
!sendMothershipMessage(msg.content, msg.contexts, msg.fileAttachments, recoverStreamId)
4618-
) {
4619-
MothershipHandoffStorage.store(
4620-
{
4621-
message: msg.content,
4622-
...(msg.contexts?.length ? { contexts: msg.contexts } : {}),
4623-
...(msg.fileAttachments?.length ? { fileAttachments: msg.fileAttachments } : {}),
4624-
...(recoverStreamId ? { recoverStreamId } : {}),
4625-
},
4626-
workspaceId
4627-
)
4628-
}
4640+
handOffChatlessRecovery(recoverStreamId)
46294641
return
46304642
}
46314643
useMothershipQueueStore.getState().insertAt(dispatchChatKey, originalIndex, {
@@ -4658,9 +4670,26 @@ export function useChat(
46584670
adopt its chat instead when it exists. */
46594671
if (liveMsg.recoverStreamId) {
46604672
const probe = await resolveRecoveredSendChatId(liveMsg.recoverStreamId, options.epoch)
4661-
/* Unknown, not "safe to send" — leave the entry queued (it keeps its
4662-
`recoverStreamId`) so the next mount's drain probes again. */
4663-
if (probe.status === 'superseded') return
4673+
/* Unknown, not "safe to send". A chat-bound key is the stable chat
4674+
id, so leaving the entry queued IS the retry — the next mount's
4675+
drain probes again. A chatless `pending::` key is regenerated per
4676+
mount, so the same move would strand the message under a dead key;
4677+
hand it to the recovery lanes instead, still carrying the stream
4678+
id. Skipped when the entry is no longer under this key (adoption
4679+
migrated it to a live chat), where it is already recoverable. */
4680+
if (probe.status === 'superseded') {
4681+
if (dispatchChatKey.startsWith(PENDING_CHAT_KEY_PREFIX)) {
4682+
const queueStore = useMothershipQueueStore.getState()
4683+
const stillUnderDeadKey = (queueStore.queues[dispatchChatKey] ?? []).some(
4684+
(queued) => queued.id === msg.id
4685+
)
4686+
if (stillUnderDeadKey) {
4687+
queueStore.remove(dispatchChatKey, msg.id)
4688+
handOffChatlessRecovery(liveMsg.recoverStreamId)
4689+
}
4690+
}
4691+
return
4692+
}
46644693
if (probe.status === 'adopted') {
46654694
removeQueuedMessage()
46664695
adoptResolvedChatId(probe.chatId, {

0 commit comments

Comments
 (0)