fix(cli): make MCP ping optional in list command and use configured timeout (#26068)

This commit is contained in:
Coco Sheng
2026-04-27 16:36:50 -04:00
committed by GitHub
parent 31337b9269
commit 7d08f84305
2 changed files with 89 additions and 4 deletions
@@ -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({
+17 -4
View File
@@ -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;