Skip to content

Commit 0362f9f

Browse files
committed
fix(mcp): retire in-flight creates evicted mid-connect via server generation
1 parent b86cfa3 commit 0362f9f

2 files changed

Lines changed: 38 additions & 2 deletions

File tree

apps/sim/lib/mcp/connection-pool.test.ts

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -252,6 +252,31 @@ describe('McpConnectionPool', () => {
252252
expect(create).toHaveBeenCalledTimes(2)
253253
})
254254

255+
it('does not pool a connection whose create finished after the server was evicted', async () => {
256+
let resolveCreate: ((client: McpClient) => void) | undefined
257+
const created = new Promise<McpClient>((resolve) => {
258+
resolveCreate = resolve
259+
})
260+
const staleClient = makeFakeClient()
261+
const freshClient = makeFakeClient()
262+
const create = vi
263+
.fn<() => Promise<McpClient>>()
264+
.mockImplementationOnce(() => created)
265+
.mockImplementation(async () => freshClient)
266+
267+
const acquireP = pool.acquire(params('s1:w1:u1', create))
268+
// Server config changes while the connect is still in flight.
269+
await pool.evictServer('s1', 'config changed')
270+
resolveCreate?.(staleClient)
271+
const lease = await acquireP
272+
273+
// The stale-config connection is disconnected, not pooled; a fresh one is built.
274+
expect(staleClient.disconnect).toHaveBeenCalledTimes(1)
275+
expect(lease.client).toBe(freshClient)
276+
expect(create).toHaveBeenCalledTimes(2)
277+
await lease.release()
278+
})
279+
255280
it('bypasses the pool once disposed (connects without caching)', async () => {
256281
const client = makeFakeClient()
257282
const create = vi.fn(async () => client)

apps/sim/lib/mcp/connection-pool.ts

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,8 @@ interface PoolEntry {
6666
export class McpConnectionPool {
6767
private entries = new Map<string, PoolEntry>()
6868
private pending = new Map<string, Promise<PoolEntry>>()
69+
/** Per-server counter bumped by `evictServer`; an in-flight create built against an older value is retired on completion. */
70+
private serverGenerations = new Map<string, number>()
6971
private idleCheckTimer: ReturnType<typeof setInterval> | null = null
7072
private disposed = false
7173

@@ -94,7 +96,10 @@ export class McpConnectionPool {
9496
const reusable = await this.tryReuse(current)
9597
if (reusable) return reusable
9698
}
97-
return this.createEntry(params)
99+
const created = await this.createEntry(params)
100+
// Evicted (config edit/delete) mid-create → don't borrow the stale connection.
101+
if (created.retired && !this.disposed) return this.resolveEntry(params)
102+
return created
98103
}
99104

100105
/** Return `entry` if in-age, connected, and live; else retire it and return null. */
@@ -132,6 +137,7 @@ export class McpConnectionPool {
132137
if (inFlight) return inFlight
133138

134139
const creation = (async () => {
140+
const generation = this.serverGenerations.get(params.serverId) ?? 0
135141
const client = await params.create()
136142
const now = Date.now()
137143
const entry: PoolEntry = {
@@ -145,7 +151,9 @@ export class McpConnectionPool {
145151
retired: false,
146152
closing: false,
147153
}
148-
if (this.disposed) {
154+
// Disposed, or the server was evicted (config edit/delete) while connecting:
155+
// don't pool a connection built against the old config.
156+
if (this.disposed || (this.serverGenerations.get(params.serverId) ?? 0) !== generation) {
149157
entry.retired = true
150158
void client.disconnect().catch(() => {})
151159
return entry
@@ -197,6 +205,9 @@ export class McpConnectionPool {
197205

198206
/** Retire every connection for a server (all users) — config changed or deleted. */
199207
async evictServer(serverId: string, reason: string): Promise<void> {
208+
// Bump first so an in-flight create for this server retires on completion
209+
// instead of pooling a connection built against the now-stale config.
210+
this.serverGenerations.set(serverId, (this.serverGenerations.get(serverId) ?? 0) + 1)
200211
for (const entry of this.entries.values()) {
201212
if (entry.serverId === serverId) {
202213
logger.info(`Evicting pooled MCP connection ${entry.key}: ${reason}`)

0 commit comments

Comments
 (0)