Skip to content

Commit 50104c4

Browse files
committed
fix(sse): rotate workspace streams without gaps
1 parent 1e24a6a commit 50104c4

7 files changed

Lines changed: 476 additions & 140 deletions

File tree

apps/sim/hooks/queries/mcp.test.tsx

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -127,8 +127,8 @@ describe('useMcpToolsQuery', () => {
127127
// mcp.ts captured these Map/Set instances in module consts at import, so reassigning the
128128
// globalThis property wouldn't reset what the module uses — clear the shared instances.
129129
;(
130-
globalThis as unknown as { __mcp_sse_connections?: Map<string, unknown> }
131-
).__mcp_sse_connections?.clear()
130+
globalThis as unknown as { __mcp_rotating_sse_connections?: Map<string, unknown> }
131+
).__mcp_rotating_sse_connections?.clear()
132132
;(globalThis as unknown as { __mcp_sse_subscribed?: Set<string> }).__mcp_sse_subscribed?.clear()
133133
})
134134

apps/sim/hooks/queries/mcp.ts

Lines changed: 27 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,10 @@ import {
2727
testMcpServerConnectionContract,
2828
updateMcpServerContract,
2929
} from '@/lib/api/contracts/mcp'
30+
import {
31+
createRotatingEventSource,
32+
type RotatingEventSourceConnection,
33+
} from '@/lib/events/rotating-event-source'
3034
import { sanitizeForHttp, sanitizeHeaders } from '@/lib/mcp/shared'
3135
import type {
3236
McpAuthType,
@@ -539,9 +543,9 @@ export function useStoredMcpTools(workspaceId: string, options?: { enabled?: boo
539543
* Reference-counted so the connection is closed when the last consumer unmounts.
540544
* Attached to `globalThis` so connections survive HMR in development.
541545
*/
542-
const SSE_KEY = '__mcp_sse_connections' as const
546+
const SSE_KEY = '__mcp_rotating_sse_connections' as const
543547

544-
type SseEntry = { source: EventSource; refs: number }
548+
type SseEntry = { connection: RotatingEventSourceConnection; refs: number }
545549

546550
const sseConnections: Map<string, SseEntry> =
547551
((globalThis as Record<string, unknown>)[SSE_KEY] as Map<string, SseEntry>) ??
@@ -576,40 +580,29 @@ export function useMcpToolsEvents(workspaceId: string) {
576580
let entry = sseConnections.get(workspaceId)
577581

578582
if (!entry) {
579-
const source = new EventSource(`/api/mcp/events?workspaceId=${workspaceId}`)
580-
581-
source.addEventListener('tools_changed', (e) => {
582-
let serverId: string | undefined
583-
try {
584-
const parsed = JSON.parse((e as MessageEvent).data) as { serverId?: string }
585-
serverId = parsed.serverId
586-
} catch {
587-
// Non-JSON payload → workspace-wide fallback.
588-
}
589-
invalidate(serverId)
590-
})
591-
592-
// EventSource fires `onopen` on the initial connect and on every auto-reconnect. Re-sync
593-
// the workspace whenever we could have missed a `tools_changed` event: on any reconnect,
594-
// on the first open of a RE-subscription (leaving the tab tears the connection down), and
595-
// on a first open that only succeeded after an earlier connection error (the initial tools
596-
// query may have failed during that gap and won't retry itself). Skip only a clean first
597-
// subscription — the queries fetch fresh on their own initial mount.
598583
const isResubscribe = sseEverSubscribed.has(workspaceId)
599584
sseEverSubscribed.add(workspaceId)
600-
let opened = false
601-
let erroredBeforeOpen = false
602-
source.onopen = () => {
603-
if (opened || isResubscribe || erroredBeforeOpen) invalidate()
604-
opened = true
605-
}
606-
607-
source.onerror = () => {
608-
if (!opened) erroredBeforeOpen = true
609-
logger.warn(`SSE connection error for workspace ${workspaceId}`)
610-
}
585+
const connection = createRotatingEventSource({
586+
url: `/api/mcp/events?workspaceId=${encodeURIComponent(workspaceId)}`,
587+
events: {
588+
tools_changed: (event) => {
589+
let serverId: string | undefined
590+
try {
591+
const parsed = JSON.parse((event as MessageEvent).data) as { serverId?: string }
592+
serverId = parsed.serverId
593+
} catch {}
594+
invalidate(serverId)
595+
},
596+
},
597+
onOpen: (reason) => {
598+
if (reason === 'reconnect' || (reason === 'initial' && isResubscribe)) invalidate()
599+
},
600+
onError: () => {
601+
logger.warn(`SSE connection error for workspace ${workspaceId}`)
602+
},
603+
})
611604

612-
entry = { source, refs: 0 }
605+
entry = { connection, refs: 0 }
613606
sseConnections.set(workspaceId, entry)
614607
}
615608

@@ -621,7 +614,7 @@ export function useMcpToolsEvents(workspaceId: string) {
621614

622615
current.refs--
623616
if (current.refs <= 0) {
624-
current.source.close()
617+
current.connection.close()
625618
sseConnections.delete(workspaceId)
626619
}
627620
}

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

Lines changed: 22 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import { useQueryClient } from '@tanstack/react-query'
55
import { getLiveAssistantMessageId } from '@/lib/copilot/chat/effective-transcript'
66
import { isChatEnabled } from '@/lib/core/config/env-flags'
77
import { suspendDesktopChatScopes } from '@/lib/desktop/chat-scope'
8+
import { createRotatingEventSource } from '@/lib/events/rotating-event-source'
89
import { type MothershipChatHistory, mothershipChatKeys } from '@/hooks/queries/mothership-chats'
910

1011
const logger = createLogger('MothershipChatEvents')
@@ -167,45 +168,31 @@ export function useMothershipChatEvents(workspaceId: string | undefined) {
167168
useEffect(() => {
168169
if (!workspaceId || !isChatEnabled) return
169170

170-
const eventSource = new EventSource(
171-
`/api/mothership/events?workspaceId=${encodeURIComponent(workspaceId)}`
172-
)
173-
174-
eventSource.addEventListener('task_status', (event) => {
175-
handleMothershipChatStatusEvent(
176-
queryClient,
177-
workspaceId,
178-
event instanceof MessageEvent ? event.data : undefined
179-
)
180-
})
181-
182-
// `onopen` fires on the initial connect and on every auto-reconnect. Re-sync
183-
// whenever a gap could have swallowed an event: on any reconnect, on the
184-
// first open of a RE-subscription (switching workspace away and back tears
185-
// the connection down, and the list/detail queries remount inside their
186-
// stale times so they do not refetch on their own), and on a first open that
187-
// only succeeded after an error (the initial queries may have failed during
188-
// that gap and will not retry themselves). Skip only a clean first
189-
// subscription — those queries fetch fresh on their own initial mount.
190171
const isResubscribe = everSubscribed.has(workspaceId)
191172
everSubscribed.add(workspaceId)
192-
let opened = false
193-
let erroredBeforeOpen = false
194-
195-
eventSource.onopen = () => {
196-
if (opened || isResubscribe || erroredBeforeOpen) {
197-
resyncMothershipChatCaches(queryClient, workspaceId)
198-
}
199-
opened = true
200-
}
201-
202-
eventSource.onerror = () => {
203-
if (!opened) erroredBeforeOpen = true
204-
logger.warn(`SSE connection error for workspace ${workspaceId}`)
205-
}
173+
const connection = createRotatingEventSource({
174+
url: `/api/mothership/events?workspaceId=${encodeURIComponent(workspaceId)}`,
175+
events: {
176+
task_status: (event) => {
177+
handleMothershipChatStatusEvent(
178+
queryClient,
179+
workspaceId,
180+
event instanceof MessageEvent ? event.data : undefined
181+
)
182+
},
183+
},
184+
onOpen: (reason) => {
185+
if (reason === 'reconnect' || (reason === 'initial' && isResubscribe)) {
186+
resyncMothershipChatCaches(queryClient, workspaceId)
187+
}
188+
},
189+
onError: () => {
190+
logger.warn(`SSE connection error for workspace ${workspaceId}`)
191+
},
192+
})
206193

207194
return () => {
208-
eventSource.close()
195+
connection.close()
209196
}
210197
}, [workspaceId, queryClient])
211198
}
Lines changed: 154 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,154 @@
1+
/**
2+
* @vitest-environment node
3+
*/
4+
5+
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
6+
import {
7+
createRotatingEventSource,
8+
type EventSourceOpenReason,
9+
} from '@/lib/events/rotating-event-source'
10+
11+
class MockEventSource {
12+
static readonly CONNECTING = 0
13+
static readonly OPEN = 1
14+
static readonly CLOSED = 2
15+
static instances: MockEventSource[] = []
16+
static failNextConstruction = false
17+
18+
readonly listeners = new Map<string, Array<(event: Event) => void>>()
19+
readyState = MockEventSource.CONNECTING
20+
onopen: (() => void) | null = null
21+
onerror: (() => void) | null = null
22+
23+
constructor(readonly url: string) {
24+
if (MockEventSource.failNextConstruction) {
25+
MockEventSource.failNextConstruction = false
26+
throw new Error('construction failed')
27+
}
28+
MockEventSource.instances.push(this)
29+
}
30+
31+
addEventListener(eventName: string, listener: (event: Event) => void): void {
32+
const listeners = this.listeners.get(eventName) ?? []
33+
listeners.push(listener)
34+
this.listeners.set(eventName, listeners)
35+
}
36+
37+
close(): void {
38+
this.readyState = MockEventSource.CLOSED
39+
}
40+
41+
open(): void {
42+
this.readyState = MockEventSource.OPEN
43+
this.onopen?.()
44+
}
45+
46+
error(): void {
47+
this.readyState = MockEventSource.CONNECTING
48+
this.onerror?.()
49+
}
50+
51+
emit(eventName: string): void {
52+
for (const listener of this.listeners.get(eventName) ?? []) {
53+
listener(new Event(eventName))
54+
}
55+
}
56+
}
57+
58+
describe('createRotatingEventSource', () => {
59+
beforeEach(() => {
60+
MockEventSource.instances = []
61+
MockEventSource.failNextConstruction = false
62+
vi.stubGlobal('EventSource', MockEventSource)
63+
})
64+
65+
afterEach(() => {
66+
vi.unstubAllGlobals()
67+
})
68+
69+
it('keeps the current source open until its replacement connects', () => {
70+
const reasons: EventSourceOpenReason[] = []
71+
const connection = createRotatingEventSource({
72+
url: '/api/events',
73+
events: {},
74+
onOpen: (reason) => reasons.push(reason),
75+
})
76+
const first = MockEventSource.instances[0]
77+
first.open()
78+
79+
first.emit('rotate')
80+
81+
expect(MockEventSource.instances).toHaveLength(2)
82+
expect(first.readyState).toBe(MockEventSource.OPEN)
83+
84+
const second = MockEventSource.instances[1]
85+
second.open()
86+
87+
expect(first.readyState).toBe(MockEventSource.CLOSED)
88+
expect(second.readyState).toBe(MockEventSource.OPEN)
89+
expect(reasons).toEqual(['initial', 'rotation'])
90+
connection.close()
91+
})
92+
93+
it('classifies a replacement as reconnecting when the old source dropped first', () => {
94+
const reasons: EventSourceOpenReason[] = []
95+
const connection = createRotatingEventSource({
96+
url: '/api/events',
97+
events: {},
98+
onOpen: (reason) => reasons.push(reason),
99+
})
100+
const first = MockEventSource.instances[0]
101+
first.open()
102+
first.emit('rotate')
103+
first.error()
104+
105+
MockEventSource.instances[1].open()
106+
107+
expect(reasons).toEqual(['initial', 'reconnect'])
108+
connection.close()
109+
})
110+
111+
it('classifies an automatic EventSource recovery as a reconnect', () => {
112+
const reasons: EventSourceOpenReason[] = []
113+
const connection = createRotatingEventSource({
114+
url: '/api/events',
115+
events: {},
116+
onOpen: (reason) => reasons.push(reason),
117+
})
118+
const source = MockEventSource.instances[0]
119+
source.open()
120+
source.error()
121+
source.open()
122+
123+
expect(reasons).toEqual(['initial', 'reconnect'])
124+
connection.close()
125+
})
126+
127+
it('closes both sources when disposed during rotation', () => {
128+
const connection = createRotatingEventSource({ url: '/api/events', events: {} })
129+
const first = MockEventSource.instances[0]
130+
first.open()
131+
first.emit('rotate')
132+
const second = MockEventSource.instances[1]
133+
134+
connection.close()
135+
136+
expect(first.readyState).toBe(MockEventSource.CLOSED)
137+
expect(second.readyState).toBe(MockEventSource.CLOSED)
138+
})
139+
140+
it('keeps the current source when opening its replacement fails', () => {
141+
const onError = vi.fn()
142+
const connection = createRotatingEventSource({ url: '/api/events', events: {}, onError })
143+
const first = MockEventSource.instances[0]
144+
first.open()
145+
MockEventSource.failNextConstruction = true
146+
147+
first.emit('rotate')
148+
149+
expect(first.readyState).toBe(MockEventSource.OPEN)
150+
expect(MockEventSource.instances).toHaveLength(1)
151+
expect(onError).toHaveBeenCalledTimes(1)
152+
connection.close()
153+
})
154+
})

0 commit comments

Comments
 (0)