diff --git a/README.md b/README.md index b1017dfa..dd12a190 100644 --- a/README.md +++ b/README.md @@ -98,6 +98,34 @@ Manual registration example: For manual providers, create the OAuth App in the upstream console, set the redirect URI to `http://localhost:3000/oauth/callback` (or your deployed domain), and then plug the credentials into the dashboard or config file. +#### Connection Modes (Optional) + +MCPHub supports two connection strategies: + +- **`persistent` (default)**: Maintains long-running connections for stateful servers +- **`on-demand`**: Connects only when needed, ideal for ephemeral servers that exit after operations + +Example for one-time use servers: + +```json +{ + "mcpServers": { + "pdf-reader": { + "command": "npx", + "args": ["-y", "pdf-mcp-server"], + "connectionMode": "on-demand" + } + } +} +``` + +Use `on-demand` mode for servers that: +- Don't support long-running connections +- Exit automatically after handling requests +- Experience "Connection closed" errors + +See the [Configuration Guide](docs/configuration/mcp-settings.mdx) for more details. + ### Docker Deployment **Recommended**: Mount your custom config: diff --git a/docs/configuration/mcp-settings.mdx b/docs/configuration/mcp-settings.mdx index 49cc9c69..32b41fce 100644 --- a/docs/configuration/mcp-settings.mdx +++ b/docs/configuration/mcp-settings.mdx @@ -72,9 +72,13 @@ MCPHub uses several configuration files: ### Optional Fields -| Field | Type | Default | Description | -| -------------- | ------- | --------------- | --------------------------- | -| `env` | object | `{}` | Environment variables | +| Field | Type | Default | Description | +| ---------------- | ------- | --------------- | --------------------------------------------------------------------- | +| `env` | object | `{}` | Environment variables | +| `connectionMode` | string | `"persistent"` | Connection strategy: `"persistent"` or `"on-demand"` | +| `enabled` | boolean | `true` | Enable/disable the server | +| `keepAliveInterval` | number | `60000` | Keep-alive ping interval for SSE connections (milliseconds) | +| `options` | object | `{}` | MCP request options (timeout, resetTimeoutOnProgress, maxTotalTimeout)| ## Common MCP Server Examples @@ -238,6 +242,68 @@ MCPHub uses several configuration files: } ``` +## Connection Modes + +MCPHub supports two connection strategies for MCP servers: + +### Persistent Connection (Default) + +Persistent mode maintains a long-running connection to the MCP server. This is the default and recommended mode for most servers. + +**Use cases:** +- Servers that maintain state between requests +- Servers with slow startup times +- Servers designed for long-running connections + +**Example:** +```json +{ + "github": { + "command": "npx", + "args": ["-y", "@modelcontextprotocol/server-github"], + "connectionMode": "persistent", + "env": { + "GITHUB_PERSONAL_ACCESS_TOKEN": "${GITHUB_TOKEN}" + } + } +} +``` + +### On-Demand Connection + +On-demand mode connects only when a tool is invoked, then disconnects immediately after. This is ideal for servers that: +- Don't support long-running connections +- Are designed for one-time use +- Exit automatically after handling requests + +**Use cases:** +- PDF processing tools that exit after each operation +- One-time command-line utilities +- Servers with connection stability issues +- Resource-intensive servers that shouldn't run continuously + +**Example:** +```json +{ + "pdf-reader": { + "command": "npx", + "args": ["-y", "pdf-mcp-server"], + "connectionMode": "on-demand", + "env": { + "PDF_CACHE_DIR": "/tmp/pdf-cache" + } + } +} +``` + +**Benefits of on-demand mode:** +- Avoids "Connection closed" errors for ephemeral services +- Reduces resource usage for infrequently used tools +- Better suited for stateless operations +- Handles servers that automatically exit after operations + +**Note:** On-demand servers briefly connect during initialization to discover available tools, then disconnect. The connection is re-established only when a tool from that server is actually invoked. + ## Advanced Configuration ### Environment Variable Substitution diff --git a/examples/mcp_settings_with_connection_modes.json b/examples/mcp_settings_with_connection_modes.json new file mode 100644 index 00000000..5189f6b3 --- /dev/null +++ b/examples/mcp_settings_with_connection_modes.json @@ -0,0 +1,61 @@ +{ + "$schema": "https://json-schema.org/draft-07/schema", + "description": "Example MCP settings showing different connection modes", + "mcpServers": { + "github": { + "command": "npx", + "args": ["-y", "@modelcontextprotocol/server-github"], + "connectionMode": "persistent", + "env": { + "GITHUB_PERSONAL_ACCESS_TOKEN": "${GITHUB_TOKEN}" + }, + "enabled": true + }, + "playwright": { + "command": "npx", + "args": ["@playwright/mcp@latest", "--headless"], + "connectionMode": "persistent", + "enabled": true + }, + "pdf-reader": { + "command": "npx", + "args": ["-y", "pdf-mcp-server"], + "connectionMode": "on-demand", + "env": { + "PDF_CACHE_DIR": "/tmp/pdf-cache" + }, + "enabled": true + }, + "image-processor": { + "command": "python", + "args": ["-m", "image_mcp_server"], + "connectionMode": "on-demand", + "env": { + "IMAGE_OUTPUT_DIR": "/tmp/images" + }, + "enabled": true + }, + "fetch": { + "command": "uvx", + "args": ["mcp-server-fetch"], + "enabled": true + }, + "slack": { + "command": "npx", + "args": ["-y", "@modelcontextprotocol/server-slack"], + "connectionMode": "persistent", + "env": { + "SLACK_BOT_TOKEN": "${SLACK_BOT_TOKEN}", + "SLACK_TEAM_ID": "${SLACK_TEAM_ID}" + }, + "enabled": true + } + }, + "users": [ + { + "username": "admin", + "password": "$2b$10$Vt7krIvjNgyN67LXqly0uOcTpN0LI55cYRbcKC71pUDAP0nJ7RPa.", + "isAdmin": true + } + ] +} diff --git a/src/services/mcpService.ts b/src/services/mcpService.ts index 801e4dad..cbd71a04 100644 --- a/src/services/mcpService.ts +++ b/src/services/mcpService.ts @@ -369,6 +369,118 @@ export const createTransportFromConfig = async (name: string, conf: ServerConfig return transport; }; +// Helper function to connect an on-demand server temporarily +const connectOnDemandServer = async (serverInfo: ServerInfo): Promise => { + if (!serverInfo.config) { + throw new Error(`Server configuration not found for on-demand server: ${serverInfo.name}`); + } + + console.log(`Connecting on-demand server: ${serverInfo.name}`); + + // Create transport + const transport = await createTransportFromConfig(serverInfo.name, serverInfo.config); + + // Create client + const client = new Client( + { + name: `mcp-client-${serverInfo.name}`, + version: '1.0.0', + }, + { + capabilities: { + prompts: {}, + resources: {}, + tools: {}, + }, + }, + ); + + // Get request options from server configuration + const serverRequestOptions = serverInfo.config.options || {}; + const requestOptions = { + timeout: serverRequestOptions.timeout || 60000, + resetTimeoutOnProgress: serverRequestOptions.resetTimeoutOnProgress || false, + maxTotalTimeout: serverRequestOptions.maxTotalTimeout, + }; + + // Connect the client + await client.connect(transport, requestOptions); + + // Update server info with client and transport + serverInfo.client = client; + serverInfo.transport = transport; + serverInfo.options = requestOptions; + serverInfo.status = 'connected'; + + console.log(`Successfully connected on-demand server: ${serverInfo.name}`); + + // List tools if not already loaded + if (serverInfo.tools.length === 0) { + const capabilities = client.getServerCapabilities(); + if (capabilities?.tools) { + try { + const tools = await client.listTools({}, requestOptions); + serverInfo.tools = tools.tools.map((tool) => ({ + name: `${serverInfo.name}${getNameSeparator()}${tool.name}`, + description: tool.description || '', + inputSchema: cleanInputSchema(tool.inputSchema || {}), + })); + // Save tools as vector embeddings for search + saveToolsAsVectorEmbeddings(serverInfo.name, serverInfo.tools); + console.log(`Loaded ${serverInfo.tools.length} tools for on-demand server: ${serverInfo.name}`); + } catch (error) { + console.warn(`Failed to list tools for on-demand server ${serverInfo.name}:`, error); + } + } + + // List prompts if available + if (capabilities?.prompts) { + try { + const prompts = await client.listPrompts({}, requestOptions); + serverInfo.prompts = prompts.prompts.map((prompt) => ({ + name: `${serverInfo.name}${getNameSeparator()}${prompt.name}`, + title: prompt.title, + description: prompt.description, + arguments: prompt.arguments, + })); + console.log(`Loaded ${serverInfo.prompts.length} prompts for on-demand server: ${serverInfo.name}`); + } catch (error) { + console.warn(`Failed to list prompts for on-demand server ${serverInfo.name}:`, error); + } + } + } +}; + +// Helper function to disconnect an on-demand server +const disconnectOnDemandServer = (serverInfo: ServerInfo): void => { + if (serverInfo.connectionMode !== 'on-demand') { + return; + } + + console.log(`Disconnecting on-demand server: ${serverInfo.name}`); + + try { + if (serverInfo.client) { + serverInfo.client.close(); + serverInfo.client = undefined; + } + if (serverInfo.transport) { + serverInfo.transport.close(); + serverInfo.transport = undefined; + } + serverInfo.status = 'disconnected'; + console.log(`Successfully disconnected on-demand server: ${serverInfo.name}`); + } catch (error) { + // Log disconnect errors but don't throw - this is cleanup code that shouldn't fail the request + // The connection is likely already closed if we get an error here + console.warn(`Error disconnecting on-demand server ${serverInfo.name}:`, error); + // Force status to disconnected even if cleanup had errors + serverInfo.status = 'disconnected'; + serverInfo.client = undefined; + serverInfo.transport = undefined; + } +}; + // Helper function to handle client.callTool with reconnection logic const callToolWithReconnect = async ( serverInfo: ServerInfo, @@ -529,7 +641,6 @@ export const initializeClientsFromSettings = async ( continue; } - let transport; let openApiClient; if (expandedConf.type === 'openapi') { // Handle OpenAPI type servers @@ -600,10 +711,43 @@ export const initializeClientsFromSettings = async ( serverInfo.error = `Failed to initialize OpenAPI server: ${error}`; continue; } - } else { - transport = await createTransportFromConfig(name, expandedConf); } + // Handle on-demand connection mode servers + // These servers connect briefly to get tools list, then disconnect + const connectionMode = expandedConf.connectionMode || 'persistent'; + if (connectionMode === 'on-demand') { + console.log(`Initializing on-demand server: ${name}`); + const serverInfo: ServerInfo = { + name, + owner: expandedConf.owner, + status: 'disconnected', + error: null, + tools: [], + prompts: [], + createTime: Date.now(), + enabled: expandedConf.enabled === undefined ? true : expandedConf.enabled, + connectionMode: 'on-demand', + config: expandedConf, + }; + nextServerInfos.push(serverInfo); + + // Connect briefly to get tools list, then disconnect + try { + await connectOnDemandServer(serverInfo); + console.log(`Successfully initialized on-demand server: ${name} with ${serverInfo.tools.length} tools`); + // Disconnect immediately after getting tools + disconnectOnDemandServer(serverInfo); + } catch (error) { + console.error(`Failed to initialize on-demand server ${name}:`, error); + serverInfo.error = `Failed to initialize: ${error}`; + } + continue; + } + + // Create transport for persistent connection mode servers (not OpenAPI, already handled above) + const transport = await createTransportFromConfig(name, expandedConf); + const client = new Client( { name: `mcp-client-${name}`, @@ -644,6 +788,7 @@ export const initializeClientsFromSettings = async ( transport, options: requestOptions, createTime: Date.now(), + connectionMode: connectionMode, config: expandedConf, // Store reference to expanded config }; @@ -1011,8 +1156,11 @@ export const handleListToolsRequest = async (_: any, extra: any) => { const targetGroup = group?.startsWith('$smart/') ? group.substring(7) : undefined; // Get info about available servers, filtered by target group if specified + // Include both connected persistent servers and on-demand servers (even if disconnected) let availableServers = serverInfos.filter( - (server) => server.status === 'connected' && server.enabled !== false, + (server) => + server.enabled !== false && + (server.status === 'connected' || server.connectionMode === 'on-demand'), ); // If a target group is specified, filter servers to only those in the group @@ -1139,6 +1287,10 @@ Available servers: ${serversList}`, export const handleCallToolRequest = async (request: any, extra: any) => { console.log(`Handling CallToolRequest for tool: ${JSON.stringify(request.params)}`); try { + // Note: On-demand server connection and disconnection are handled in the specific + // code paths below (call_tool and regular tool handling) with try-finally blocks. + // This outer try-catch only handles errors from operations that don't connect servers. + // Special handling for agent group tools if (request.params.name === 'search_tools') { const { query, limit = 10 } = request.params.arguments || {}; @@ -1284,10 +1436,11 @@ export const handleCallToolRequest = async (request: any, extra: any) => { targetServerInfo = getServerByName(extra.server); } else { // Find the first server that has this tool + // Include both connected servers and on-demand servers (even if disconnected) targetServerInfo = serverInfos.find( (serverInfo) => - serverInfo.status === 'connected' && serverInfo.enabled !== false && + (serverInfo.status === 'connected' || serverInfo.connectionMode === 'on-demand') && serverInfo.tools.some((tool) => tool.name === toolName), ); } @@ -1363,6 +1516,11 @@ export const handleCallToolRequest = async (request: any, extra: any) => { } // Call the tool on the target server (MCP servers) + // Connect on-demand server if needed + if (targetServerInfo.connectionMode === 'on-demand' && !targetServerInfo.client) { + await connectOnDemandServer(targetServerInfo); + } + const client = targetServerInfo.client; if (!client) { throw new Error(`Client not found for server: ${targetServerInfo.name}`); @@ -1379,17 +1537,23 @@ export const handleCallToolRequest = async (request: any, extra: any) => { const separator = getNameSeparator(); const prefix = `${targetServerInfo.name}${separator}`; toolName = toolName.startsWith(prefix) ? toolName.substring(prefix.length) : toolName; - const result = await callToolWithReconnect( - targetServerInfo, - { - name: toolName, - arguments: finalArgs, - }, - targetServerInfo.options || {}, - ); + + try { + const result = await callToolWithReconnect( + targetServerInfo, + { + name: toolName, + arguments: finalArgs, + }, + targetServerInfo.options || {}, + ); - console.log(`Tool invocation result: ${JSON.stringify(result)}`); - return result; + console.log(`Tool invocation result: ${JSON.stringify(result)}`); + return result; + } finally { + // Disconnect on-demand server after tool call + disconnectOnDemandServer(targetServerInfo); + } } // Regular tool handling @@ -1459,6 +1623,11 @@ export const handleCallToolRequest = async (request: any, extra: any) => { } // Handle MCP servers + // Connect on-demand server if needed + if (serverInfo.connectionMode === 'on-demand' && !serverInfo.client) { + await connectOnDemandServer(serverInfo); + } + const client = serverInfo.client; if (!client) { throw new Error(`Client not found for server: ${serverInfo.name}`); @@ -1469,13 +1638,19 @@ export const handleCallToolRequest = async (request: any, extra: any) => { request.params.name = request.params.name.startsWith(prefix) ? request.params.name.substring(prefix.length) : request.params.name; - const result = await callToolWithReconnect( - serverInfo, - request.params, - serverInfo.options || {}, - ); - console.log(`Tool call result: ${JSON.stringify(result)}`); - return result; + + try { + const result = await callToolWithReconnect( + serverInfo, + request.params, + serverInfo.options || {}, + ); + console.log(`Tool call result: ${JSON.stringify(result)}`); + return result; + } finally { + // Disconnect on-demand server after tool call + disconnectOnDemandServer(serverInfo); + } } catch (error) { console.error(`Error handling CallToolRequest: ${error}`); return { diff --git a/src/types/index.ts b/src/types/index.ts index 44baf23c..2529f9ea 100644 --- a/src/types/index.ts +++ b/src/types/index.ts @@ -204,6 +204,7 @@ export interface ServerConfig { enabled?: boolean; // Flag to enable/disable the server owner?: string; // Owner of the server, defaults to 'admin' user keepAliveInterval?: number; // Keep-alive ping interval in milliseconds (default: 60000ms for SSE servers) + connectionMode?: 'persistent' | 'on-demand'; // Connection strategy: 'persistent' maintains long-running connections (default), 'on-demand' connects only when tools are called tools?: Record; // Tool-specific configurations with enable/disable state and custom descriptions prompts?: Record; // Prompt-specific configurations with enable/disable state and custom descriptions options?: Partial>; // MCP request options configuration @@ -312,6 +313,7 @@ export interface ServerInfo { options?: RequestOptions; // Options for requests createTime: number; // Timestamp of when the server was created enabled?: boolean; // Flag to indicate if the server is enabled + connectionMode?: 'persistent' | 'on-demand'; // Connection strategy for this server keepAliveIntervalId?: NodeJS.Timeout; // Timer ID for keep-alive ping interval config?: ServerConfig; // Reference to the original server configuration for OpenAPI passthrough headers oauth?: { diff --git a/tests/services/mcpService-on-demand.test.ts b/tests/services/mcpService-on-demand.test.ts new file mode 100644 index 00000000..e4014187 --- /dev/null +++ b/tests/services/mcpService-on-demand.test.ts @@ -0,0 +1,340 @@ +import { describe, it, expect, jest, beforeEach, afterEach } from '@jest/globals'; + +// Mock dependencies before importing mcpService +jest.mock('../../src/services/oauthService.js', () => ({ + initializeAllOAuthClients: jest.fn(), +})); + +jest.mock('../../src/services/oauthClientRegistration.js', () => ({ + registerOAuthClient: jest.fn(), +})); + +jest.mock('../../src/services/mcpOAuthProvider.js', () => ({ + createOAuthProvider: jest.fn(), +})); + +jest.mock('../../src/services/groupService.js', () => ({ + getServersInGroup: jest.fn(), + getServerConfigInGroup: jest.fn(), +})); + +jest.mock('../../src/services/sseService.js', () => ({ + getGroup: jest.fn(), +})); + +jest.mock('../../src/services/vectorSearchService.js', () => ({ + saveToolsAsVectorEmbeddings: jest.fn(), + searchToolsByVector: jest.fn(() => Promise.resolve([])), +})); + +jest.mock('../../src/services/services.js', () => ({ + getDataService: jest.fn(() => ({ + filterData: (data: any) => data, + })), +})); + +jest.mock('../../src/config/index.js', () => ({ + default: { + mcpHubName: 'test-hub', + mcpHubVersion: '1.0.0', + initTimeout: 60000, + }, + loadSettings: jest.fn(() => ({})), + expandEnvVars: jest.fn((val: string) => val), + replaceEnvVars: jest.fn((obj: any) => obj), + getNameSeparator: jest.fn(() => '-'), +})); + +// Mock Client +const mockClient = { + connect: jest.fn(), + close: jest.fn(), + listTools: jest.fn(), + listPrompts: jest.fn(), + getServerCapabilities: jest.fn(() => ({ + tools: {}, + prompts: {}, + })), + callTool: jest.fn(), +}; + +jest.mock('@modelcontextprotocol/sdk/client/index.js', () => ({ + Client: jest.fn(() => mockClient), +})); + +// Mock StdioClientTransport +const mockTransport = { + close: jest.fn(), + stderr: null, +}; + +jest.mock('@modelcontextprotocol/sdk/client/stdio.js', () => ({ + StdioClientTransport: jest.fn(() => mockTransport), +})); + +// Mock DAO +const mockServerDao = { + findAll: jest.fn(), + findById: jest.fn(), + create: jest.fn(), + update: jest.fn(), + delete: jest.fn(), + exists: jest.fn(), + setEnabled: jest.fn(), +}; + +jest.mock('../../src/dao/index.js', () => ({ + getServerDao: jest.fn(() => mockServerDao), +})); + +import { initializeClientsFromSettings, handleCallToolRequest } from '../../src/services/mcpService.js'; + +describe('On-Demand MCP Server Connection Mode', () => { + beforeEach(() => { + jest.clearAllMocks(); + mockClient.connect.mockResolvedValue(undefined); + mockClient.close.mockReturnValue(undefined); + mockClient.listTools.mockResolvedValue({ + tools: [ + { + name: 'test-tool', + description: 'Test tool', + inputSchema: { type: 'object' }, + }, + ], + }); + mockClient.listPrompts.mockResolvedValue({ + prompts: [], + }); + mockClient.callTool.mockResolvedValue({ + content: [{ type: 'text', text: 'Success' }], + }); + mockTransport.close.mockReturnValue(undefined); + }); + + afterEach(() => { + jest.restoreAllMocks(); + }); + + describe('Server Initialization', () => { + it('should not maintain persistent connection for on-demand servers', async () => { + mockServerDao.findAll.mockResolvedValue([ + { + name: 'on-demand-server', + command: 'node', + args: ['test.js'], + connectionMode: 'on-demand', + enabled: true, + }, + ]); + + const serverInfos = await initializeClientsFromSettings(true); + + expect(serverInfos).toHaveLength(1); + expect(serverInfos[0].name).toBe('on-demand-server'); + expect(serverInfos[0].connectionMode).toBe('on-demand'); + expect(serverInfos[0].status).toBe('disconnected'); + // Should connect once to get tools, then disconnect + expect(mockClient.connect).toHaveBeenCalledTimes(1); + expect(mockTransport.close).toHaveBeenCalledTimes(1); + }); + + it('should load tools during initialization for on-demand servers', async () => { + mockServerDao.findAll.mockResolvedValue([ + { + name: 'on-demand-server', + command: 'node', + args: ['test.js'], + connectionMode: 'on-demand', + enabled: true, + }, + ]); + + const serverInfos = await initializeClientsFromSettings(true); + + expect(serverInfos[0].tools).toHaveLength(1); + expect(serverInfos[0].tools[0].name).toBe('on-demand-server-test-tool'); + expect(mockClient.listTools).toHaveBeenCalled(); + }); + + it('should maintain persistent connection for default connection mode', async () => { + mockServerDao.findAll.mockResolvedValue([ + { + name: 'persistent-server', + command: 'node', + args: ['test.js'], + enabled: true, + }, + ]); + + const serverInfos = await initializeClientsFromSettings(true); + + expect(serverInfos).toHaveLength(1); + expect(serverInfos[0].connectionMode).toBe('persistent'); + expect(mockClient.connect).toHaveBeenCalledTimes(1); + // Should not disconnect immediately + expect(mockTransport.close).not.toHaveBeenCalled(); + }); + + it('should handle initialization errors for on-demand servers gracefully', async () => { + mockClient.connect.mockRejectedValueOnce(new Error('Connection failed')); + mockServerDao.findAll.mockResolvedValue([ + { + name: 'failing-server', + command: 'node', + args: ['test.js'], + connectionMode: 'on-demand', + enabled: true, + }, + ]); + + const serverInfos = await initializeClientsFromSettings(true); + + expect(serverInfos).toHaveLength(1); + expect(serverInfos[0].status).toBe('disconnected'); + expect(serverInfos[0].error).toContain('Failed to initialize'); + }); + }); + + describe('Tool Invocation with On-Demand Servers', () => { + beforeEach(async () => { + // Set up server infos with an on-demand server that's disconnected + mockServerDao.findAll.mockResolvedValue([ + { + name: 'on-demand-server', + command: 'node', + args: ['test.js'], + connectionMode: 'on-demand', + enabled: true, + }, + ]); + + // Initialize to get the server set up + await initializeClientsFromSettings(true); + + // Clear mocks after initialization + jest.clearAllMocks(); + + // Reset mock implementations + mockClient.connect.mockResolvedValue(undefined); + mockClient.listTools.mockResolvedValue({ + tools: [ + { + name: 'test-tool', + description: 'Test tool', + inputSchema: { type: 'object' }, + }, + ], + }); + mockClient.callTool.mockResolvedValue({ + content: [{ type: 'text', text: 'Success' }], + }); + }); + + it('should connect on-demand server before tool invocation', async () => { + const request = { + params: { + name: 'on-demand-server-test-tool', + arguments: { arg1: 'value1' }, + }, + }; + + await handleCallToolRequest(request, {}); + + // Should connect before calling the tool + expect(mockClient.connect).toHaveBeenCalledTimes(1); + expect(mockClient.callTool).toHaveBeenCalledWith( + { + name: 'test-tool', + arguments: { arg1: 'value1' }, + }, + undefined, + expect.any(Object), + ); + }); + + it('should disconnect on-demand server after tool invocation', async () => { + const request = { + params: { + name: 'on-demand-server-test-tool', + arguments: {}, + }, + }; + + await handleCallToolRequest(request, {}); + + // Should disconnect after calling the tool + expect(mockTransport.close).toHaveBeenCalledTimes(1); + expect(mockClient.close).toHaveBeenCalledTimes(1); + }); + + it('should disconnect on-demand server even if tool invocation fails', async () => { + mockClient.callTool.mockRejectedValueOnce(new Error('Tool execution failed')); + + const request = { + params: { + name: 'on-demand-server-test-tool', + arguments: {}, + }, + }; + + try { + await handleCallToolRequest(request, {}); + } catch (error) { + // Expected to fail + } + + // Should still disconnect after error + expect(mockTransport.close).toHaveBeenCalledTimes(1); + expect(mockClient.close).toHaveBeenCalledTimes(1); + }); + + it('should return error for call_tool if server not found', async () => { + const request = { + params: { + name: 'call_tool', + arguments: { + toolName: 'nonexistent-server-tool', + arguments: {}, + }, + }, + }; + + const result = await handleCallToolRequest(request, {}); + + expect(result.isError).toBe(true); + expect(result.content[0].text).toContain('No available servers found'); + }); + }); + + describe('Mixed Server Modes', () => { + it('should handle both persistent and on-demand servers together', async () => { + mockServerDao.findAll.mockResolvedValue([ + { + name: 'persistent-server', + command: 'node', + args: ['persistent.js'], + enabled: true, + }, + { + name: 'on-demand-server', + command: 'node', + args: ['on-demand.js'], + connectionMode: 'on-demand', + enabled: true, + }, + ]); + + const serverInfos = await initializeClientsFromSettings(true); + + expect(serverInfos).toHaveLength(2); + + const persistentServer = serverInfos.find(s => s.name === 'persistent-server'); + const onDemandServer = serverInfos.find(s => s.name === 'on-demand-server'); + + expect(persistentServer?.connectionMode).toBe('persistent'); + expect(onDemandServer?.connectionMode).toBe('on-demand'); + expect(onDemandServer?.status).toBe('disconnected'); + }); + }); +});