From 1251b424e22f0140c1bd531f99e748ba8a20fa78 Mon Sep 17 00:00:00 2001 From: waleed Date: Thu, 4 Dec 2025 08:32:31 -0800 Subject: [PATCH 1/2] fix(mcp): remove client-side cache and reduce server cache from 5m to 30s --- apps/sim/lib/mcp/client.ts | 8 ++-- apps/sim/lib/mcp/service.ts | 86 ++++++------------------------------- apps/sim/lib/mcp/utils.ts | 2 +- 3 files changed, 20 insertions(+), 76 deletions(-) diff --git a/apps/sim/lib/mcp/client.ts b/apps/sim/lib/mcp/client.ts index 812c3c96e5d..c6675c958c3 100644 --- a/apps/sim/lib/mcp/client.ts +++ b/apps/sim/lib/mcp/client.ts @@ -44,11 +44,14 @@ export class McpClient { /** * Creates a new MCP client + * + * No session ID parameter (we disconnect after each operation). + * The SDK handles session management automatically via Mcp-Session-Id header. + * * @param config - Server configuration * @param securityPolicy - Optional security policy - * @param sessionId - Optional session ID for session restoration (from previous connection) */ - constructor(config: McpServerConfig, securityPolicy?: McpSecurityPolicy, sessionId?: string) { + constructor(config: McpServerConfig, securityPolicy?: McpSecurityPolicy) { this.config = config this.connectionStatus = { connected: false } this.securityPolicy = securityPolicy ?? { @@ -65,7 +68,6 @@ export class McpClient { requestInit: { headers: this.config.headers, }, - sessionId, }) this.client = new Client( diff --git a/apps/sim/lib/mcp/service.ts b/apps/sim/lib/mcp/service.ts index 64e9afb5d1a..650c13ad2dd 100644 --- a/apps/sim/lib/mcp/service.ts +++ b/apps/sim/lib/mcp/service.ts @@ -18,7 +18,6 @@ import type { McpToolResult, McpTransport, } from '@/lib/mcp/types' -import { MCP_CONSTANTS } from '@/lib/mcp/utils' const logger = createLogger('McpService') @@ -42,42 +41,17 @@ interface CacheStats { class McpService { private toolCache = new Map() - private readonly cacheTimeout = MCP_CONSTANTS.CACHE_TIMEOUT + private readonly cacheTimeout = 30 * 1000 // 30 seconds private readonly maxCacheSize = 1000 private cleanupInterval: NodeJS.Timeout | null = null private cacheHits = 0 private cacheMisses = 0 private entriesEvicted = 0 - private sessionCache = new Map() - constructor() { this.startPeriodicCleanup() } - /** - * Get cached session ID for a server - */ - private getCachedSessionId(serverId: string): string | undefined { - return this.sessionCache.get(serverId) - } - - /** - * Cache session ID for a server - */ - private cacheSessionId(serverId: string, sessionId: string): void { - this.sessionCache.set(serverId, sessionId) - logger.debug(`Cached session ID for server ${serverId}`) - } - - /** - * Clear cached session ID for a server - */ - private clearCachedSessionId(serverId: string): void { - this.sessionCache.delete(serverId) - logger.debug(`Cleared cached session ID for server ${serverId}`) - } - /** * Start periodic cleanup of expired cache entries */ @@ -341,49 +315,9 @@ class McpService { allowedOrigins: config.url ? [new URL(config.url).origin] : undefined, } - const cachedSessionId = this.getCachedSessionId(config.id) - - const client = new McpClient(config, securityPolicy, cachedSessionId) - - try { - await client.connect() - - const newSessionId = client.getSessionId() - if (newSessionId) { - this.cacheSessionId(config.id, newSessionId) - } - - return client - } catch (error) { - if (cachedSessionId && this.isSessionError(error)) { - logger.debug(`Session restoration failed for server ${config.id}, retrying fresh`) - this.clearCachedSessionId(config.id) - - const freshClient = new McpClient(config, securityPolicy) - await freshClient.connect() - - const freshSessionId = freshClient.getSessionId() - if (freshSessionId) { - this.cacheSessionId(config.id, freshSessionId) - } - - return freshClient - } - - throw error - } - } - - private isSessionError(error: unknown): boolean { - if (error instanceof Error) { - const message = error.message.toLowerCase() - return ( - message.includes('no valid session') || - message.includes('invalid session') || - message.includes('session expired') - ) - } - return false + const client = new McpClient(config, securityPolicy) + await client.connect() + return client } /** @@ -466,10 +400,12 @@ class McpService { }) ) + let failedCount = 0 results.forEach((result, index) => { if (result.status === 'fulfilled') { allTools.push(...result.value) } else { + failedCount++ logger.warn( `[${requestId}] Failed to discover tools from server ${servers[index].name}:`, result.reason @@ -477,10 +413,16 @@ class McpService { } }) - this.setCacheEntry(cacheKey, allTools) + if (failedCount === 0) { + this.setCacheEntry(cacheKey, allTools) + } else { + logger.warn( + `[${requestId}] Skipping cache due to ${failedCount} failed server(s) - will retry on next request` + ) + } logger.info( - `[${requestId}] Discovered ${allTools.length} tools from ${servers.length} servers` + `[${requestId}] Discovered ${allTools.length} tools from ${servers.length - failedCount}/${servers.length} servers` ) return allTools } catch (error) { diff --git a/apps/sim/lib/mcp/utils.ts b/apps/sim/lib/mcp/utils.ts index 55ef118ae8c..efdc9056a71 100644 --- a/apps/sim/lib/mcp/utils.ts +++ b/apps/sim/lib/mcp/utils.ts @@ -6,7 +6,7 @@ import type { McpApiResponse } from '@/lib/mcp/types' */ export const MCP_CONSTANTS = { EXECUTION_TIMEOUT: 60000, - CACHE_TIMEOUT: 5 * 60 * 1000, + CACHE_TIMEOUT: 30 * 1000, DEFAULT_RETRIES: 3, DEFAULT_CONNECTION_TIMEOUT: 30000, } as const From 028e8090a9798d0fd17c089f67ba303669ac6d41 Mon Sep 17 00:00:00 2001 From: waleed Date: Thu, 4 Dec 2025 08:50:44 -0800 Subject: [PATCH 2/2] ack PR comments --- apps/sim/lib/mcp/service.ts | 5 +++-- apps/sim/lib/mcp/utils.ts | 1 + 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/apps/sim/lib/mcp/service.ts b/apps/sim/lib/mcp/service.ts index 650c13ad2dd..5f2bf7fc4e0 100644 --- a/apps/sim/lib/mcp/service.ts +++ b/apps/sim/lib/mcp/service.ts @@ -18,6 +18,7 @@ import type { McpToolResult, McpTransport, } from '@/lib/mcp/types' +import { MCP_CONSTANTS } from '@/lib/mcp/utils' const logger = createLogger('McpService') @@ -41,8 +42,8 @@ interface CacheStats { class McpService { private toolCache = new Map() - private readonly cacheTimeout = 30 * 1000 // 30 seconds - private readonly maxCacheSize = 1000 + private readonly cacheTimeout = MCP_CONSTANTS.CACHE_TIMEOUT // 30 seconds + private readonly maxCacheSize = MCP_CONSTANTS.MAX_CACHE_SIZE // 1000 private cleanupInterval: NodeJS.Timeout | null = null private cacheHits = 0 private cacheMisses = 0 diff --git a/apps/sim/lib/mcp/utils.ts b/apps/sim/lib/mcp/utils.ts index efdc9056a71..107b32aa9e3 100644 --- a/apps/sim/lib/mcp/utils.ts +++ b/apps/sim/lib/mcp/utils.ts @@ -9,6 +9,7 @@ export const MCP_CONSTANTS = { CACHE_TIMEOUT: 30 * 1000, DEFAULT_RETRIES: 3, DEFAULT_CONNECTION_TIMEOUT: 30000, + MAX_CACHE_SIZE: 1000, } as const /**