From b6413b06bc47ad4c6941f6480fd990bd410ae6d0 Mon Sep 17 00:00:00 2001 From: "Theo N. Truong" <644650+nhtruong@users.noreply.github.com> Date: Wed, 13 Aug 2025 14:19:55 -0600 Subject: [PATCH] =?UTF-8?q?=F0=9F=90=9E=20fix:=20Update=20MCP=20server=20i?= =?UTF-8?q?nitialization=20to=20skip=20non-startup=20and=20oauth=20servers?= =?UTF-8?q?=20(#9049)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- packages/api/src/mcp/MCPServersRegistry.ts | 40 +++++++++++-------- .../mcp/__tests__/MCPServersRegistry.test.ts | 22 ++++------ .../MCPServersRegistry.parsedConfigs.yml | 13 ++---- 3 files changed, 33 insertions(+), 42 deletions(-) diff --git a/packages/api/src/mcp/MCPServersRegistry.ts b/packages/api/src/mcp/MCPServersRegistry.ts index b2ce2ed0ad..8115d52508 100644 --- a/packages/api/src/mcp/MCPServersRegistry.ts +++ b/packages/api/src/mcp/MCPServersRegistry.ts @@ -62,21 +62,23 @@ export class MCPServersRegistry { // Fetches all metadata for a single server in parallel private async gatherServerInfo(serverName: string) { try { - await Promise.allSettled([ - this.fetchOAuthRequirement(serverName).catch((error) => - logger.error(`${this.prefix(serverName)} Failed to fetch OAuth requirement:`, error), - ), - this.fetchServerInstructions(serverName).catch((error) => - logger.error(`${this.prefix(serverName)} Failed to fetch server instructions:`, error), - ), - this.fetchServerCapabilities(serverName).catch((error) => - logger.error(`${this.prefix(serverName)} Failed to fetch server capabilities:`, error), - ), - ]); + await this.fetchOAuthRequirement(serverName); + const config = this.parsedConfigs[serverName]; + + if (config.startup !== false && !config.requiresOAuth) { + await Promise.allSettled([ + this.fetchServerInstructions(serverName).catch((error) => + logger.warn(`${this.prefix(serverName)} Failed to fetch server instructions:`, error), + ), + this.fetchServerCapabilities(serverName).catch((error) => + logger.warn(`${this.prefix(serverName)} Failed to fetch server capabilities:`, error), + ), + ]); + } this.logUpdatedConfig(serverName); } catch (error) { - logger.error(`${this.prefix(serverName)} Failed to initialize server:`, error); + logger.warn(`${this.prefix(serverName)} Failed to initialize server:`, error); } } @@ -117,7 +119,7 @@ export class MCPServersRegistry { const toolFunctions = await this.getToolFunctions(serverName, conn); Object.assign(allToolFunctions, toolFunctions); } catch (error) { - logger.error(`${this.prefix(serverName)} Error fetching tool functions:`, error); + logger.warn(`${this.prefix(serverName)} Error fetching tool functions:`, error); } } this.toolFunctions = allToolFunctions; @@ -147,14 +149,16 @@ export class MCPServersRegistry { } // Determines if server requires OAuth if not already specified in the config - private async fetchOAuthRequirement(serverName: string) { + private async fetchOAuthRequirement(serverName: string): Promise { const config = this.parsedConfigs[serverName]; - if (config.requiresOAuth != null) return; + if (config.requiresOAuth != null) return config.requiresOAuth; if (config.url == null) return (config.requiresOAuth = false); + if (config.startup === false) return (config.requiresOAuth = false); const result = await detectOAuthRequirement(config.url); config.requiresOAuth = result.requiresOAuth; config.oauthMetadata = result.metadata; + return config.requiresOAuth; } // Retrieves server instructions from MCP server if enabled in the config @@ -186,11 +190,13 @@ export class MCPServersRegistry { private logUpdatedConfig(serverName: string) { const prefix = this.prefix(serverName); const config = this.parsedConfigs[serverName]; - logger.info(`${prefix} URL: ${config.url ?? 'N/A'}`); + logger.info(`${prefix} -------------------------------------------------┐`); + logger.info(`${prefix} URL: ${config.url}`); logger.info(`${prefix} OAuth Required: ${config.requiresOAuth}`); logger.info(`${prefix} Capabilities: ${config.capabilities}`); logger.info(`${prefix} Tools: ${config.tools}`); - logger.info(`${prefix} Server Instructions: ${config.serverInstructions ?? 'None'}`); + logger.info(`${prefix} Server Instructions: ${config.serverInstructions}`); + logger.info(`${prefix} -------------------------------------------------┘`); } // Returns formatted log prefix for server messages diff --git a/packages/api/src/mcp/__tests__/MCPServersRegistry.test.ts b/packages/api/src/mcp/__tests__/MCPServersRegistry.test.ts index 63fdc72ffb..033a18a72a 100644 --- a/packages/api/src/mcp/__tests__/MCPServersRegistry.test.ts +++ b/packages/api/src/mcp/__tests__/MCPServersRegistry.test.ts @@ -179,10 +179,10 @@ describe('MCPServersRegistry - Initialize Function', () => { new Set(['oauth_server', 'oauth_predefined', 'oauth_startup_enabled']), ); - // Test serverInstructions + // Test serverInstructions - OAuth servers keep their original boolean value, non-OAuth fetch actual strings expect(registry.serverInstructions).toEqual({ - oauth_server: 'GitHub MCP server instructions', stdio_server: 'Follow these instructions for stdio server', + oauth_server: true, non_oauth_server: 'Public API instructions', }); @@ -193,16 +193,8 @@ describe('MCPServersRegistry - Initialize Function', () => { non_oauth_server: rawConfigs.non_oauth_server, }); - // Test toolFunctions (only 2 servers have tools: oauth_server has 1, stdio_server has 2) + // Test toolFunctions (only non-OAuth servers get their tools fetched during initialization) const expectedToolFunctions = { - get_repository_mcp_oauth_server: { - type: 'function', - function: { - name: 'get_repository_mcp_oauth_server', - description: 'Description for get_repository', - parameters: { type: 'object', properties: { input: { type: 'string' } } }, - }, - }, file_read_mcp_stdio_server: { type: 'function', function: { @@ -235,9 +227,9 @@ describe('MCPServersRegistry - Initialize Function', () => { expect(registry.oauthServers).toBeInstanceOf(Set); expect(registry.toolFunctions).toBeDefined(); - // Error should be logged - expect(mockLogger.error).toHaveBeenCalledWith( - expect.stringContaining('[MCP][oauth_server] Failed to fetch OAuth requirement:'), + // Error should be logged as a warning at the higher level + expect(mockLogger.warn).toHaveBeenCalledWith( + expect.stringContaining('[MCP][oauth_server] Failed to initialize server:'), expect.any(Error), ); }); @@ -250,7 +242,7 @@ describe('MCPServersRegistry - Initialize Function', () => { expect(mockConnectionsRepo.disconnectAll).toHaveBeenCalledTimes(1); }); - it('should log configuration updates for each server', async () => { + it('should log configuration updates for each startup-enabled server', async () => { const registry = new MCPServersRegistry(rawConfigs); await registry.initialize(); diff --git a/packages/api/src/mcp/__tests__/fixtures/MCPServersRegistry.parsedConfigs.yml b/packages/api/src/mcp/__tests__/fixtures/MCPServersRegistry.parsedConfigs.yml index 55ddbf4ed5..71b3e01d22 100644 --- a/packages/api/src/mcp/__tests__/fixtures/MCPServersRegistry.parsedConfigs.yml +++ b/packages/api/src/mcp/__tests__/fixtures/MCPServersRegistry.parsedConfigs.yml @@ -7,13 +7,11 @@ oauth_server: url: "https://api.github.com/mcp" headers: Authorization: "Bearer {{GITHUB_TOKEN}}" - serverInstructions: "GitHub MCP server instructions" + serverInstructions: true requiresOAuth: true oauthMetadata: authorization_url: "https://github.com/login/oauth/authorize" token_url: "https://github.com/login/oauth/access_token" - capabilities: '{"tools":{"listChanged":true},"resources":{},"prompts":{}}' - tools: "get_repository" oauth_predefined: _processed: true @@ -23,8 +21,6 @@ oauth_predefined: oauthMetadata: authorization_url: "https://example.com/oauth/authorize" token_url: "https://example.com/oauth/token" - capabilities: '{"tools":{},"resources":{},"prompts":{}}' - tools: "" stdio_server: _processed: true @@ -50,11 +46,10 @@ websocket_server: disabled_server: _processed: true + requiresOAuth: false type: "streamable-http" url: "https://api.disabled.com/mcp" startup: false - requiresOAuth: false - oauthMetadata: null non_oauth_server: _processed: true @@ -69,6 +64,4 @@ oauth_startup_enabled: _processed: true type: "sse" url: "https://api.oauth-startup.com/sse" - requiresOAuth: true - capabilities: '{"tools":{},"resources":{},"prompts":{}}' - tools: "" \ No newline at end of file + requiresOAuth: true \ No newline at end of file