Address code review feedback - improve error handling and add clarifying comments

Co-authored-by: samanhappy <2755122+samanhappy@users.noreply.github.com>

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:

docs/configuration/mcp-settings.mdx

@@ -72,9 +72,13 @@ MCPHub uses several configuration files:
 
 ### Optional Fields
 
-| Field          | Type    | Default         | Description                 |
+| Field            | Type    | Default         | Description                                                           |
-| -------------- | ------- | --------------- | --------------------------- |
+| ---------------- | ------- | --------------- | --------------------------------------------------------------------- |
-| `env`          | object  | `{}`            | Environment variables       |
+| `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

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
+    }
+  ]
+}

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<void> => {
+  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,
+      try {
-        {
+        const result = await callToolWithReconnect(
-          name: toolName,
+          targetServerInfo,
-          arguments: finalArgs,
+          {
-        },
+            name: toolName,
-        targetServerInfo.options || {},
+            arguments: finalArgs,
-      );
+          },
+          targetServerInfo.options || {},
+        );
 
-      console.log(`Tool invocation result: ${JSON.stringify(result)}`);
+        console.log(`Tool invocation result: ${JSON.stringify(result)}`);
-      return 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,
+    try {
-      request.params,
+      const result = await callToolWithReconnect(
-      serverInfo.options || {},
+        serverInfo,
-    );
+        request.params,
-    console.log(`Tool call result: ${JSON.stringify(result)}`);
+        serverInfo.options || {},
-    return result;
+      );
+      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 {

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<string, { enabled: boolean; description?: string }>; // Tool-specific configurations with enable/disable state and custom descriptions
   prompts?: Record<string, { enabled: boolean; description?: string }>; // Prompt-specific configurations with enable/disable state and custom descriptions
   options?: Partial<Pick<RequestOptions, 'timeout' | 'resetTimeoutOnProgress' | 'maxTotalTimeout'>>; // 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?: {

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');
+    });
+  });
+});