Skip to content

Commit cbe8898

Browse files
committed
fix(sse): raise the ceiling and narrow reconnect resync to the lists
Deciding from cache whether a chat is still streaming is not reliable — the optimistic markers outlive the turn, and each refinement of that predicate exposed another state where it answers wrongly. Drop it: the resync now invalidates only the workspace lists, which is always safe, and chat detail reconciliation stays as it is today rather than being half-solved here. Raise the ceiling to 4h, matching lib/realtime/event-stream-route.ts. A healthy client is drained and so is never unread; the unread check is what reclaims a vanished consumer, and it does so within minutes. A short ceiling would therefore only force reconnects on the connections that are working, and every reconnect is a window where a transient event can be missed. Retention stays bounded by the ceiling instead of by process uptime.
1 parent f76765f commit cbe8898

4 files changed

Lines changed: 32 additions & 84 deletions

File tree

apps/sim/hooks/use-mothership-chat-events.test.ts

Lines changed: 7 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -432,55 +432,20 @@ describe('resyncMothershipChatCaches', () => {
432432
vi.clearAllMocks()
433433
})
434434

435-
function detailPredicate() {
436-
resyncMothershipChatCaches(queryClient, 'ws-1')
437-
const predicate = queryClient.invalidateQueries.mock.calls
438-
.map(([arg]: [{ predicate?: (query: unknown) => boolean }]) => arg.predicate)
439-
.find(Boolean)
440-
if (!predicate) throw new Error('detail invalidation did not pass a predicate')
441-
return (data: unknown) => predicate({ state: { data } })
442-
}
443-
444-
it('invalidates the workspace lists and the chat details', () => {
435+
it('invalidates the workspace lists', () => {
445436
resyncMothershipChatCaches(queryClient, 'ws-1')
446437

447-
expect(queryClient.invalidateQueries).toHaveBeenCalledTimes(2)
438+
expect(queryClient.invalidateQueries).toHaveBeenCalledTimes(1)
448439
expect(queryClient.invalidateQueries).toHaveBeenCalledWith({
449440
queryKey: mothershipChatKeys.workspaceLists('ws-1'),
450441
})
451-
expect(queryClient.invalidateQueries).toHaveBeenCalledWith(
452-
expect.objectContaining({ queryKey: mothershipChatKeys.details() })
453-
)
454442
})
455443

456-
it('skips the detail of a chat this client is still streaming', () => {
457-
expect(
458-
detailPredicate()({
459-
messages: [{ id: 'new-stream' }, { id: 'live-assistant:new-stream' }],
460-
activeStreamId: 'new-stream',
461-
streamSnapshot: { events: [], previewSessions: [], status: 'streaming' },
462-
})
463-
).toBe(false)
464-
})
465-
466-
it('invalidates a chat whose stream finished but left its optimistic markers cached', () => {
467-
const predicate = detailPredicate()
468-
const finished = (status: string) => ({
469-
messages: [{ id: 'new-stream' }, { id: 'live-assistant:new-stream' }],
470-
activeStreamId: 'new-stream',
471-
streamSnapshot: { events: [], previewSessions: [], status },
472-
})
473-
474-
expect(predicate(finished('complete'))).toBe(true)
475-
expect(predicate(finished('error'))).toBe(true)
476-
expect(predicate(finished('cancelled'))).toBe(true)
477-
})
478-
479-
it('invalidates details with no active stream, and streams not rendered locally', () => {
480-
const predicate = detailPredicate()
444+
it('leaves chat details untouched so a mounted stream cannot be refetched mid-turn', () => {
445+
resyncMothershipChatCaches(queryClient, 'ws-1')
481446

482-
expect(predicate(undefined)).toBe(true)
483-
expect(predicate({ messages: [{ id: 'stream-1' }] })).toBe(true)
484-
expect(predicate({ messages: [{ id: 'stream-1' }], activeStreamId: 'stream-1' })).toBe(true)
447+
expect(queryClient.invalidateQueries).not.toHaveBeenCalledWith(
448+
expect.objectContaining({ queryKey: mothershipChatKeys.details() })
449+
)
485450
})
486451
})

apps/sim/hooks/use-mothership-chat-events.ts

Lines changed: 11 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -2,10 +2,7 @@ import { useEffect } from 'react'
22
import { createLogger } from '@sim/logger'
33
import type { QueryClient } from '@tanstack/react-query'
44
import { useQueryClient } from '@tanstack/react-query'
5-
import {
6-
getLiveAssistantMessageId,
7-
isTerminalStreamStatus,
8-
} from '@/lib/copilot/chat/effective-transcript'
5+
import { getLiveAssistantMessageId } from '@/lib/copilot/chat/effective-transcript'
96
import { isChatEnabled } from '@/lib/core/config/env-flags'
107
import { suspendDesktopChatScopes } from '@/lib/desktop/chat-scope'
118
import { type MothershipChatHistory, mothershipChatKeys } from '@/hooks/queries/mothership-chats'
@@ -41,23 +38,6 @@ function isLocalOptimisticActiveStream(current: MothershipChatHistory | undefine
4138
return current.messages.some((message) => message.id === liveAssistantId)
4239
}
4340

44-
/**
45-
* True while this client is still rendering a stream for the chat.
46-
*
47-
* The optimistic markers alone are not enough: a finished turn can leave
48-
* `activeStreamId` and its live-assistant message in the cache (finalization
49-
* skips detail invalidation when a follow-up is queued), and treating those as
50-
* live would exclude the chat from every future resync — permanently, since
51-
* only a refetch would clear them. Requiring a non-terminal snapshot status
52-
* keeps the skip to turns that are genuinely still streaming.
53-
*/
54-
function isStreamingLocally(current: MothershipChatHistory | undefined) {
55-
return (
56-
isLocalOptimisticActiveStream(current) &&
57-
!isTerminalStreamStatus(current?.streamSnapshot?.status)
58-
)
59-
}
60-
6141
/**
6242
* Returns true when the cached active stream is known to be later in the
6343
* chronological transcript than the stream that emitted this status event.
@@ -152,28 +132,25 @@ export function handleMothershipChatStatusEvent(
152132
}
153133

154134
/**
155-
* Re-syncs chat caches after a gap in the event stream.
135+
* Re-syncs the workspace chat lists after a gap in the event stream.
156136
*
157137
* `task_status` events are transient — nothing replays what was published while
158-
* no connection was open — so any reconnect may have missed a create, rename,
159-
* delete, or completion. Invalidating the workspace lists and every chat detail
160-
* reconciles from the server; only queries that are currently mounted refetch.
138+
* no connection was open — so a reconnect may have missed a create, rename, or
139+
* delete. The lists carry that workspace-level state, and refetching them is
140+
* always safe.
161141
*
162-
* A chat this client is still streaming is left alone, for the same reason
163-
* status events skip it: refetching mid-stream would replace the optimistic
164-
* transcript with a server copy that does not yet hold the in-flight message.
165-
* That chat reconciles when its own stream finishes.
142+
* Chat details are deliberately left alone. The only one that would refetch is
143+
* the mounted chat, which may be rendering an in-flight stream, and refetching
144+
* there replaces the optimistic transcript with a server copy that does not yet
145+
* hold the streaming message. Cached state cannot reliably say whether a turn is
146+
* still running — the optimistic markers outlive it — so detail reconciliation
147+
* stays as it is today and belongs with the streaming state that can answer it.
166148
*/
167149
export function resyncMothershipChatCaches(
168150
queryClient: Pick<QueryClient, 'invalidateQueries'>,
169151
workspaceId: string
170152
): void {
171153
queryClient.invalidateQueries({ queryKey: mothershipChatKeys.workspaceLists(workspaceId) })
172-
queryClient.invalidateQueries({
173-
queryKey: mothershipChatKeys.details(),
174-
predicate: (query) =>
175-
!isStreamingLocally(query.state.data as MothershipChatHistory | undefined),
176-
})
177154
}
178155

179156
/**

apps/sim/lib/copilot/chat/effective-transcript.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,7 @@ function asPayloadRecord(value: unknown): Record<string, unknown> | undefined {
5252
return isRecordLike(value) ? value : undefined
5353
}
5454

55-
export function isTerminalStreamStatus(status: string | null | undefined): boolean {
55+
function isTerminalStreamStatus(status: string | null | undefined): boolean {
5656
return (
5757
status === MothershipStreamV1CompletionStatus.complete ||
5858
status === MothershipStreamV1CompletionStatus.error ||

apps/sim/lib/events/sse-endpoint.ts

Lines changed: 13 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -29,16 +29,22 @@ const encoder = new TextEncoder()
2929
export const HEARTBEAT_INTERVAL_MS = 30_000
3030

3131
/**
32-
* Hard ceiling on one connection's lifetime.
32+
* Defensive ceiling on one connection's lifetime; `EventSource` reconnects past
33+
* this, so delivery continues across the boundary.
3334
*
3435
* `request.signal` abort and stream `cancel()` are the primary teardown paths,
35-
* but both fire only when the runtime reports the client disconnect. This
36-
* ceiling releases the connection without depending on that report, so a
37-
* missed disconnect costs one connection rather than accumulating for the life
38-
* of the process. `EventSource` reconnects on its own, so delivery continues
39-
* across the boundary.
36+
* but both fire only when the runtime reports the client disconnect, and the
37+
* unread check below only catches a consumer that has stopped draining. This
38+
* releases whatever both miss, so retention is bounded by the ceiling instead
39+
* of by process uptime.
40+
*
41+
* Matches the ceiling `lib/realtime/event-stream-route.ts` already uses for the
42+
* same purpose. It is deliberately far longer than the unread window: a healthy
43+
* client is drained and therefore never unread, so a short ceiling would only
44+
* force reconnects on the connections that are working, and every reconnect is
45+
* a window in which a transient event can be missed.
4046
*/
41-
export const MAX_CONNECTION_MS = 15 * 60 * 1000
47+
export const MAX_CONNECTION_MS = 4 * 60 * 60 * 1000
4248

4349
/** Spreads reconnects so connections opened together do not expire together. */
4450
export const MAX_CONNECTION_JITTER_MS = 60_000

0 commit comments

Comments
 (0)