Skip to content

improvement(mcp): bound pool create-invalidation + document DCR match #5777

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
waleedlatif1 merged 1 commit into from
Jul 20, 2026
Merged

improvement(mcp): bound pool create-invalidation + document DCR match #5777

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions apps/sim/app/api/mcp/oauth/start/route.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,13 @@ const MAX_SURFACED_ERROR_LENGTH = 250
const DCR_UNSUPPORTED_MESSAGE =
"This server doesn't support automatic OAuth client registration. Add a pre-registered OAuth client ID and secret, or configure a token instead."

/**
* The MCP SDK throws a plain `Error` (no typed class or code) when an auth server lacks a
* `registration_endpoint`, so this string-match is the only available signal. Pinned to
* `@modelcontextprotocol/sdk` v1.29.0 `registerClient` (client/auth.js): "Incompatible auth
* server: does not support dynamic client registration". If a version bump rephrases this, the
* 422 branch silently stops firing (users get a generic 500) — update the substring here.
*/
function isDynamicClientRegistrationUnsupported(error: unknown): boolean {
return getErrorMessage(error, '')
.toLowerCase()
Expand Down
48 changes: 33 additions & 15 deletions apps/sim/lib/mcp/connection-pool.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,11 +64,21 @@ interface PoolEntry {
closing: boolean
}

/**
* An in-flight `createEntry`. `evictServer` sets `invalidated` on the records for its
* server so a connect racing a config edit/delete is one-shot instead of pooled stale.
* Lives only until the create settles (cleared in `finally`), so it's self-bounded.
*/
interface PendingCreate {
serverId: string
/** `flag.invalidated` is set by `evictServer` racing this create → it one-shots instead of pooling. */
flag: { invalidated: boolean }
promise: Promise<PoolEntry>
}

export class McpConnectionPool {
private entries = new Map<string, PoolEntry>()
private pending = new Map<string, Promise<PoolEntry>>()
/** Per-server counter bumped by `evictServer`; an in-flight create built against an older value is retired on completion. */
private serverGenerations = new Map<string, number>()
private pending = new Map<string, PendingCreate>()
private idleCheckTimer: ReturnType<typeof setInterval> | null = null
private disposed = false

Expand All @@ -91,7 +101,7 @@ export class McpConnectionPool {
private async resolveEntry(params: AcquireParams): Promise<PoolEntry> {
while (true) {
const pending = this.pending.get(params.key)
if (pending) return pending
if (pending) return pending.promise

const current = this.entries.get(params.key)
if (!current) return this.createEntry(params)
Expand Down Expand Up @@ -136,10 +146,12 @@ export class McpConnectionPool {
// Re-check: the `await` in `resolveEntry` yields, so two acquires can both
// reach here; the first registers the pending create, the rest join.
const inFlight = this.pending.get(params.key)
if (inFlight) return inFlight
if (inFlight) return inFlight.promise

// Declared before the create closure so `evictServer` can invalidate a create
// that's racing a config edit/delete (the closure reads it after its `await`).
const flag = { invalidated: false }
const creation = (async () => {
const generation = this.serverGenerations.get(params.serverId) ?? 0
const client = await params.create()
const now = Date.now()
const entry: PoolEntry = {
Expand All @@ -157,7 +169,7 @@ export class McpConnectionPool {
// don't pool a connection built against the now-stale config. The current
// borrower still uses it once (as connect-per-op would for an in-flight
// request); it's disconnected on release, and the next acquire builds fresh.
if (this.disposed || (this.serverGenerations.get(params.serverId) ?? 0) !== generation) {
if (this.disposed || flag.invalidated) {
entry.retired = true
return entry
}
Expand All @@ -171,10 +183,15 @@ export class McpConnectionPool {
return entry
})()

this.pending.set(params.key, creation)
return creation.finally(() => {
if (this.pending.get(params.key) === creation) this.pending.delete(params.key)
})
const record: PendingCreate = {
serverId: params.serverId,
flag,
promise: creation.finally(() => {
if (this.pending.get(params.key) === record) this.pending.delete(params.key)
}),
}
this.pending.set(params.key, record)
return record.promise
}

private async release(entry: PoolEntry, poison: boolean): Promise<void> {
Expand Down Expand Up @@ -215,9 +232,11 @@ export class McpConnectionPool {

/** Retire every connection for a server (all users) — config changed or deleted. */
async evictServer(serverId: string, reason: string): Promise<void> {
// Bump first so an in-flight create for this server retires on completion
// instead of pooling a connection built against the now-stale config.
this.serverGenerations.set(serverId, (this.serverGenerations.get(serverId) ?? 0) + 1)
// Flag in-flight creates first so one racing this eviction is one-shot on
// completion instead of pooling a connection built against the now-stale config.
for (const record of this.pending.values()) {
if (record.serverId === serverId) record.flag.invalidated = true
}
for (const entry of this.entries.values()) {
if (entry.serverId === serverId) this.retire(entry, reason)
}
Expand Down Expand Up @@ -262,7 +281,6 @@ export class McpConnectionPool {
for (const entry of entries) entry.retired = true
this.entries.clear()
this.pending.clear()
this.serverGenerations.clear()
void Promise.allSettled(entries.map((entry) => entry.client.disconnect()))
}
}
Expand Down
Loading