diff --git a/packages/cli/src/commands/mcp/list.test.ts b/packages/cli/src/commands/mcp/list.test.ts index 578894845e..b75556f1cf 100644 --- a/packages/cli/src/commands/mcp/list.test.ts +++ b/packages/cli/src/commands/mcp/list.test.ts @@ -217,6 +217,78 @@ describe('mcp list command', () => { ); }); + it('should display connected status even if ping fails', async () => { + const defaultMergedSettings = mergeSettings({}, {}, {}, {}, true); + mockedLoadSettings.mockReturnValue({ + merged: { + ...defaultMergedSettings, + mcpServers: { + 'test-server': { command: '/test/server' }, + }, + }, + isTrusted: true, + }); + + mockClient.connect.mockResolvedValue(undefined); + mockClient.ping.mockRejectedValue(new Error('Ping failed')); + + await listMcpServers(); + + expect(debugLogger.log).toHaveBeenCalledWith( + expect.stringContaining('test-server: /test/server (stdio) - Connected'), + ); + }); + + it('should use configured timeout for connection', async () => { + const defaultMergedSettings = mergeSettings({}, {}, {}, {}, true); + mockedLoadSettings.mockReturnValue({ + merged: { + ...defaultMergedSettings, + mcpServers: { + 'test-server': { command: '/test/server', timeout: 12345 }, + }, + }, + isTrusted: true, + }); + + mockClient.connect.mockResolvedValue(undefined); + + await listMcpServers(); + + expect(mockClient.connect).toHaveBeenCalledWith( + expect.anything(), + expect.objectContaining({ timeout: 12345 }), + ); + expect(mockClient.ping).toHaveBeenCalledWith( + expect.objectContaining({ timeout: 12345 }), + ); + }); + + it('should use default timeout for connection when not configured', async () => { + const defaultMergedSettings = mergeSettings({}, {}, {}, {}, true); + mockedLoadSettings.mockReturnValue({ + merged: { + ...defaultMergedSettings, + mcpServers: { + 'test-server': { command: '/test/server' }, + }, + }, + isTrusted: true, + }); + + mockClient.connect.mockResolvedValue(undefined); + + await listMcpServers(); + + expect(mockClient.connect).toHaveBeenCalledWith( + expect.anything(), + expect.objectContaining({ timeout: 5000 }), + ); + expect(mockClient.ping).toHaveBeenCalledWith( + expect.objectContaining({ timeout: 5000 }), + ); + }); + it('should merge extension servers with config servers', async () => { const defaultMergedSettings = mergeSettings({}, {}, {}, {}, true); mockedLoadSettings.mockReturnValue({ diff --git a/packages/cli/src/commands/mcp/list.ts b/packages/cli/src/commands/mcp/list.ts index 2747c77f00..6e57789321 100644 --- a/packages/cli/src/commands/mcp/list.ts +++ b/packages/cli/src/commands/mcp/list.ts @@ -67,6 +67,8 @@ export async function getMcpServersFromConfig( return filteredResult; } +const MCP_LIST_DEFAULT_TIMEOUT_MSEC = 5000; + async function testMCPConnection( serverName: string, config: MCPServerConfig, @@ -127,11 +129,22 @@ async function testMCPConnection( } try { - // Attempt actual MCP connection with short timeout - await client.connect(transport, { timeout: 5000 }); // 5s timeout + // Attempt actual MCP connection with timeout from config or default to 5s. + // We use a short default for the list command to keep it responsive. + const timeout = config.timeout ?? MCP_LIST_DEFAULT_TIMEOUT_MSEC; + await client.connect(transport, { timeout }); - // Test basic MCP protocol by pinging the server - await client.ping(); + // Test basic MCP protocol by pinging the server. + // Ping is optional per MCP spec - some servers (e.g. Google first-party) + // don't implement it. A successful connect() is sufficient proof of connectivity. + try { + await client.ping({ timeout }); + } catch (e) { + debugLogger.debug( + `MCP ping failed for ${serverName}, but connect succeeded:`, + e, + ); + } await client.close(); return MCPServerStatus.CONNECTED;