Skip to content

Commit b86cfa3

Browse files
committed
improvement(mcp): defer bulk-discovery env resolution to the pool miss path
1 parent 252a4a0 commit b86cfa3

1 file changed

Lines changed: 30 additions & 29 deletions

File tree

apps/sim/lib/mcp/service.ts

Lines changed: 30 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -57,12 +57,7 @@ const FAILURE_CACHE_SENTINEL: McpTool[] = []
5757

5858
type DiscoveryOutcome =
5959
| { kind: 'cached'; tools: McpTool[] }
60-
| {
61-
kind: 'fetched'
62-
tools: McpTool[]
63-
resolvedConfig: McpServerConfig
64-
resolvedIP: string | null
65-
}
60+
| { kind: 'fetched'; tools: McpTool[] }
6661
| { kind: 'oauth-pending' }
6762
| { kind: 'unhealthy' }
6863
// originalError preserves the type so markServerUnhealthy's instanceof
@@ -654,24 +649,27 @@ class McpService {
654649
}
655650

656651
try {
657-
const { config: resolvedConfig, resolvedIP } = await this.resolveConfigEnvVars(
658-
config,
659-
userId,
660-
workspaceId
661-
)
652+
const create = async () => {
653+
const { config: resolvedConfig, resolvedIP } = await this.resolveConfigEnvVars(
654+
config,
655+
userId,
656+
workspaceId
657+
)
658+
return this.createClient(resolvedConfig, resolvedIP, userId)
659+
}
662660
const tools = await this.withServerClient(
663661
{
664662
key: this.poolKey(config.id, workspaceId, userId),
665663
serverId: config.id,
666664
allowPool: true,
667665
},
668-
() => this.createClient(resolvedConfig, resolvedIP, userId),
666+
create,
669667
(client) => client.listTools()
670668
)
671669
logger.debug(
672670
`[${requestId}] Discovered ${tools.length} tools from server ${config.name}`
673671
)
674-
return { kind: 'fetched', tools, resolvedConfig, resolvedIP }
672+
return { kind: 'fetched', tools }
675673
} catch (error) {
676674
if (isOauthAuthorizationError(error, config.authType)) {
677675
return { kind: 'oauth-pending' }
@@ -688,10 +686,7 @@ class McpService {
688686
const allTools: McpTool[] = []
689687
const cacheWrites: Promise<unknown>[] = []
690688
const deferredSideEffects: Promise<unknown>[] = []
691-
const liveConnections: Array<{
692-
resolvedConfig: McpServerConfig
693-
resolvedIP: string | null
694-
}> = []
689+
const liveConnections: McpServerConfig[] = []
695690
let cachedCount = 0
696691
let fetchedCount = 0
697692
let failedCount = 0
@@ -720,10 +715,7 @@ class McpService {
720715
)
721716
)
722717
deferredSideEffects.push(this.clearServerFailure(workspaceId, server.id))
723-
liveConnections.push({
724-
resolvedConfig: outcome.resolvedConfig,
725-
resolvedIP: outcome.resolvedIP,
726-
})
718+
liveConnections.push(server)
727719
return
728720
}
729721
if (outcome.kind === 'oauth-pending') {
@@ -776,15 +768,24 @@ class McpService {
776768
for (const p of deferredSideEffects) p.catch(() => {})
777769

778770
if (mcpConnectionManager) {
779-
for (const conn of liveConnections) {
780-
mcpConnectionManager
781-
.connect(conn.resolvedConfig, userId, workspaceId, conn.resolvedIP)
782-
.catch((err) => {
783-
logger.warn(
784-
`[${requestId}] Persistent connection failed for ${conn.resolvedConfig.name}:`,
785-
err
771+
const manager = mcpConnectionManager
772+
for (const config of liveConnections) {
773+
// Resolve only for servers the manager isn't already monitoring — a
774+
// pooled `listTools` hit above no longer resolves, so this is the sole
775+
// remaining resolution cost, and it's skipped in the steady state.
776+
if (manager.hasConnection(config.id)) continue
777+
void (async () => {
778+
try {
779+
const { config: resolvedConfig, resolvedIP } = await this.resolveConfigEnvVars(
780+
config,
781+
userId,
782+
workspaceId
786783
)
787-
})
784+
await manager.connect(resolvedConfig, userId, workspaceId, resolvedIP)
785+
} catch (err) {
786+
logger.warn(`[${requestId}] Persistent connection failed for ${config.name}:`, err)
787+
}
788+
})()
788789
}
789790
}
790791

0 commit comments

Comments
 (0)