From 98af4564e83e0863f72a23739770d0debc7806f6 Mon Sep 17 00:00:00 2001 From: Danny Avila Date: Thu, 18 Sep 2025 21:03:21 -0400 Subject: [PATCH] =?UTF-8?q?=F0=9F=8E=AF=20refactor:=20MCP=20Registry=20To?= =?UTF-8?q?=20Handle=20`serverInstructions`=20As=20String=20"true"=20(#970?= =?UTF-8?q?3)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Updated MCPServersRegistry to correctly process serverInstructions when provided as a string "true", allowing it to fetch instructions from the server. - Added a new utility function `isEnabled` to determine if serverInstructions should trigger a fetch. - Introduced comprehensive tests to validate the behavior for different serverInstructions configurations, ensuring both string "true" and boolean true fetch from the server while custom strings remain unchanged. 🎯 This enhancement improves the flexibility and correctness of server instruction handling in the MCPServersRegistry. --- packages/api/src/mcp/MCPServersRegistry.ts | 9 ++- .../mcp/__tests__/MCPServersRegistry.test.ts | 69 +++++++++++++++++++ 2 files changed, 76 insertions(+), 2 deletions(-) diff --git a/packages/api/src/mcp/MCPServersRegistry.ts b/packages/api/src/mcp/MCPServersRegistry.ts index d5ecd990bd..23850358d0 100644 --- a/packages/api/src/mcp/MCPServersRegistry.ts +++ b/packages/api/src/mcp/MCPServersRegistry.ts @@ -8,7 +8,7 @@ import type * as t from '~/mcp/types'; import { ConnectionsRepository } from '~/mcp/ConnectionsRepository'; import { detectOAuthRequirement } from '~/mcp/oauth'; import { sanitizeUrlForLogging } from '~/mcp/utils'; -import { processMCPEnv } from '~/utils'; +import { processMCPEnv, isEnabled } from '~/utils'; import { CONSTANTS } from '~/mcp/enum'; /** @@ -158,8 +158,13 @@ export class MCPServersRegistry { private async fetchServerInstructions(serverName: string): Promise { const config = this.parsedConfigs[serverName]; if (!config.serverInstructions) return; - if (typeof config.serverInstructions === 'string') return; + // If it's a string that's not "true", it's a custom instruction + if (typeof config.serverInstructions === 'string' && !isEnabled(config.serverInstructions)) { + return; + } + + // Fetch from server if true (boolean) or "true" (string) const conn = await this.connections.get(serverName); config.serverInstructions = conn.client.getInstructions(); if (!config.serverInstructions) { diff --git a/packages/api/src/mcp/__tests__/MCPServersRegistry.test.ts b/packages/api/src/mcp/__tests__/MCPServersRegistry.test.ts index 5d58073edf..9a276e3713 100644 --- a/packages/api/src/mcp/__tests__/MCPServersRegistry.test.ts +++ b/packages/api/src/mcp/__tests__/MCPServersRegistry.test.ts @@ -288,5 +288,74 @@ describe('MCPServersRegistry - Initialize Function', () => { // Compare the actual parsedConfigs against the expected fixture expect(registry.parsedConfigs).toEqual(expectedParsedConfigs); }); + + it('should handle serverInstructions as string "true" correctly and fetch from server', async () => { + // Create test config with serverInstructions as string "true" + const testConfig: t.MCPServers = { + test_server_string_true: { + type: 'stdio', + args: [], + command: 'test-command', + serverInstructions: 'true', // Simulating string "true" from YAML parsing + }, + test_server_custom_string: { + type: 'stdio', + args: [], + command: 'test-command', + serverInstructions: 'Custom instructions here', + }, + test_server_bool_true: { + type: 'stdio', + args: [], + command: 'test-command', + serverInstructions: true, + }, + }; + + const registry = new MCPServersRegistry(testConfig); + + // Setup mock connection for servers that should fetch + const mockClient = { + listTools: jest.fn().mockResolvedValue({ tools: [] }), + getInstructions: jest.fn().mockReturnValue('Fetched instructions from server'), + getServerCapabilities: jest.fn().mockReturnValue({ tools: {} }), + }; + const mockConnection = { + client: mockClient, + } as unknown as jest.Mocked; + + mockConnectionsRepo.get.mockResolvedValue(mockConnection); + mockConnectionsRepo.getLoaded.mockResolvedValue( + new Map([ + ['test_server_string_true', mockConnection], + ['test_server_bool_true', mockConnection], + ]), + ); + mockDetectOAuthRequirement.mockResolvedValue({ + requiresOAuth: false, + method: 'no-metadata-found', + metadata: null, + }); + + await registry.initialize(); + + // Verify that string "true" was treated as fetch-from-server + expect(registry.parsedConfigs['test_server_string_true'].serverInstructions).toBe( + 'Fetched instructions from server', + ); + + // Verify that custom string was kept as-is + expect(registry.parsedConfigs['test_server_custom_string'].serverInstructions).toBe( + 'Custom instructions here', + ); + + // Verify that boolean true also fetched from server + expect(registry.parsedConfigs['test_server_bool_true'].serverInstructions).toBe( + 'Fetched instructions from server', + ); + + // Verify getInstructions was called for both "true" cases + expect(mockClient.getInstructions).toHaveBeenCalledTimes(2); + }); }); });