diff --git a/packages/cli/src/ui/hooks/slashCommandProcessor.test.ts b/packages/cli/src/ui/hooks/slashCommandProcessor.test.ts index cc6be49e..7466e2a6 100644 --- a/packages/cli/src/ui/hooks/slashCommandProcessor.test.ts +++ b/packages/cli/src/ui/hooks/slashCommandProcessor.test.ts @@ -60,6 +60,8 @@ import { type Config, MCPServerStatus, getMCPServerStatus, + MCPDiscoveryState, + getMCPDiscoveryState, } from '@gemini-cli/core'; import { useSessionStats } from '../contexts/SessionContext.js'; @@ -525,7 +527,7 @@ Add any other context about the problem here. describe('/mcp command', () => { beforeEach(() => { - // Mock the core module with getMCPServerStatus + // Mock the core module with getMCPServerStatus and getMCPDiscoveryState vi.mock('@gemini-cli/core', async (importOriginal) => { const actual = await importOriginal(); return { @@ -535,7 +537,13 @@ Add any other context about the problem here. CONNECTING: 'connecting', DISCONNECTED: 'disconnected', }, + MCPDiscoveryState: { + NOT_STARTED: 'not_started', + IN_PROGRESS: 'in_progress', + COMPLETED: 'completed', + }, getMCPServerStatus: vi.fn(), + getMCPDiscoveryState: vi.fn(), }; }); }); @@ -596,13 +604,18 @@ Add any other context about the problem here. server3: { command: 'cmd3' }, }; - // Setup getMCPServerStatus mock implementation + // Setup getMCPServerStatus mock implementation - use all CONNECTED to avoid startup message in this test vi.mocked(getMCPServerStatus).mockImplementation((serverName) => { if (serverName === 'server1') return MCPServerStatus.CONNECTED; - if (serverName === 'server2') return MCPServerStatus.CONNECTING; + if (serverName === 'server2') return MCPServerStatus.CONNECTED; return MCPServerStatus.DISCONNECTED; // Default for server3 and others }); + // Setup getMCPDiscoveryState mock to return completed so no startup message is shown + vi.mocked(getMCPDiscoveryState).mockReturnValue( + MCPDiscoveryState.COMPLETED, + ); + // Mock tools from each server const mockServer1Tools = [ { name: 'server1_tool1' }, @@ -638,24 +651,30 @@ Add any other context about the problem here. 2, expect.objectContaining({ type: MessageType.INFO, - text: expect.stringContaining('Configured MCP servers and tools:'), + text: expect.stringContaining('Configured MCP servers:'), }), expect.any(Number), ); - // Check that the message contains details about both servers and their tools + // Check that the message contains details about servers and their tools const message = mockAddItem.mock.calls[1][0].text; - // Server 1 - Connected (green dot) - expect(message).toContain('🟢 \u001b[1mserver1\u001b[0m (2 tools)'); + // Server 1 - Connected + expect(message).toContain( + '🟢 \u001b[1mserver1\u001b[0m - Ready (2 tools)', + ); expect(message).toContain('\u001b[36mserver1_tool1\u001b[0m'); expect(message).toContain('\u001b[36mserver1_tool2\u001b[0m'); - // Server 2 - Connecting (yellow dot) - expect(message).toContain('🟡 \u001b[1mserver2\u001b[0m (1 tools)'); + // Server 2 - Connected + expect(message).toContain( + '🟢 \u001b[1mserver2\u001b[0m - Ready (1 tools)', + ); expect(message).toContain('\u001b[36mserver2_tool1\u001b[0m'); - // Server 3 - No status, should default to Disconnected (red dot) - expect(message).toContain('🔴 \u001b[1mserver3\u001b[0m (1 tools)'); + // Server 3 - Disconnected + expect(message).toContain( + '🔴 \u001b[1mserver3\u001b[0m - Disconnected (1 tools cached)', + ); expect(message).toContain('\u001b[36mserver3_tool1\u001b[0m'); expect(commandResult).toBe(true); @@ -676,6 +695,11 @@ Add any other context about the problem here. return MCPServerStatus.DISCONNECTED; }); + // Setup getMCPDiscoveryState mock to return completed + vi.mocked(getMCPDiscoveryState).mockReturnValue( + MCPDiscoveryState.COMPLETED, + ); + // Mock tools from server with descriptions const mockServerTools = [ { name: 'tool1', description: 'This is tool 1 description' }, @@ -700,7 +724,7 @@ Add any other context about the problem here. 2, expect.objectContaining({ type: MessageType.INFO, - text: expect.stringContaining('Configured MCP servers and tools:'), + text: expect.stringContaining('Configured MCP servers:'), }), expect.any(Number), ); @@ -708,7 +732,7 @@ Add any other context about the problem here. const message = mockAddItem.mock.calls[1][0].text; // Check that server description is included (with ANSI color codes) - expect(message).toContain('\u001b[1mserver1\u001b[0m (2 tools)'); + expect(message).toContain('\u001b[1mserver1\u001b[0m - Ready (2 tools)'); expect(message).toContain( '\u001b[32mThis is a server description\u001b[0m', ); @@ -740,6 +764,11 @@ Add any other context about the problem here. return MCPServerStatus.DISCONNECTED; }); + // Setup getMCPDiscoveryState mock to return completed + vi.mocked(getMCPDiscoveryState).mockReturnValue( + MCPDiscoveryState.COMPLETED, + ); + // Mock tools from each server - server2 has no tools const mockServer1Tools = [{ name: 'server1_tool1' }]; @@ -769,19 +798,87 @@ Add any other context about the problem here. 2, expect.objectContaining({ type: MessageType.INFO, - text: expect.stringContaining('Configured MCP servers and tools:'), + text: expect.stringContaining('Configured MCP servers:'), }), expect.any(Number), ); // Check that the message contains details about both servers and their tools const message = mockAddItem.mock.calls[1][0].text; - expect(message).toContain('🟢 \u001b[1mserver1\u001b[0m (1 tools)'); + expect(message).toContain( + '🟢 \u001b[1mserver1\u001b[0m - Ready (1 tools)', + ); expect(message).toContain('\u001b[36mserver1_tool1\u001b[0m'); - expect(message).toContain('🔴 \u001b[1mserver2\u001b[0m (0 tools)'); + expect(message).toContain( + '🔴 \u001b[1mserver2\u001b[0m - Disconnected (0 tools cached)', + ); expect(message).toContain('No tools available'); expect(commandResult).toBe(true); }); + + it('should show startup indicator when servers are connecting', async () => { + // Mock MCP servers configuration + const mockMcpServers = { + server1: { command: 'cmd1' }, + server2: { command: 'cmd2' }, + }; + + // Setup getMCPServerStatus mock implementation with one server connecting + vi.mocked(getMCPServerStatus).mockImplementation((serverName) => { + if (serverName === 'server1') return MCPServerStatus.CONNECTED; + if (serverName === 'server2') return MCPServerStatus.CONNECTING; + return MCPServerStatus.DISCONNECTED; + }); + + // Setup getMCPDiscoveryState mock to return in progress + vi.mocked(getMCPDiscoveryState).mockReturnValue( + MCPDiscoveryState.IN_PROGRESS, + ); + + // Mock tools from each server + const mockServer1Tools = [{ name: 'server1_tool1' }]; + const mockServer2Tools = [{ name: 'server2_tool1' }]; + + const mockGetToolsByServer = vi.fn().mockImplementation((serverName) => { + if (serverName === 'server1') return mockServer1Tools; + if (serverName === 'server2') return mockServer2Tools; + return []; + }); + + mockConfig = { + ...mockConfig, + getToolRegistry: vi.fn().mockResolvedValue({ + getToolsByServer: mockGetToolsByServer, + }), + getMcpServers: vi.fn().mockReturnValue(mockMcpServers), + } as unknown as Config; + + const { handleSlashCommand } = getProcessor(); + let commandResult: SlashCommandActionReturn | boolean = false; + await act(async () => { + commandResult = handleSlashCommand('/mcp'); + }); + + const message = mockAddItem.mock.calls[1][0].text; + + // Check that startup indicator is shown + expect(message).toContain( + '⏳ MCP servers are starting up (1 initializing)...', + ); + expect(message).toContain( + 'Note: First startup may take longer. Tool availability will update automatically.', + ); + + // Check server statuses + expect(message).toContain( + '🟢 \u001b[1mserver1\u001b[0m - Ready (1 tools)', + ); + expect(message).toContain( + '🔄 \u001b[1mserver2\u001b[0m - Starting... (first startup may take longer) (tools will appear when ready)', + ); + + expect(commandResult).toBe(true); + }); }); }); diff --git a/packages/cli/src/ui/hooks/slashCommandProcessor.ts b/packages/cli/src/ui/hooks/slashCommandProcessor.ts index 6159fe89..fa1e4016 100644 --- a/packages/cli/src/ui/hooks/slashCommandProcessor.ts +++ b/packages/cli/src/ui/hooks/slashCommandProcessor.ts @@ -9,7 +9,13 @@ import { type PartListUnion } from '@google/genai'; import open from 'open'; import process from 'node:process'; import { UseHistoryManagerReturn } from './useHistoryManager.js'; -import { Config, MCPServerStatus, getMCPServerStatus } from '@gemini-cli/core'; +import { + Config, + MCPServerStatus, + getMCPServerStatus, + getMCPDiscoveryState, + MCPDiscoveryState, +} from '@gemini-cli/core'; import { Message, MessageType, HistoryItemWithoutId } from '../types.js'; import { useSessionStats } from '../contexts/SessionContext.js'; import { createShowMemoryAction } from './useShowMemoryCommand.js'; @@ -226,32 +232,62 @@ export const useSlashCommandProcessor = ( return; } - let message = 'Configured MCP servers and tools:\n\n'; + // Check if any servers are still connecting + const connectingServers = serverNames.filter( + (name) => getMCPServerStatus(name) === MCPServerStatus.CONNECTING, + ); + const discoveryState = getMCPDiscoveryState(); + + let message = ''; + + // Add overall discovery status message if needed + if ( + discoveryState === MCPDiscoveryState.IN_PROGRESS || + connectingServers.length > 0 + ) { + message += `\u001b[33m⏳ MCP servers are starting up (${connectingServers.length} initializing)...\u001b[0m\n`; + message += `\u001b[90mNote: First startup may take longer. Tool availability will update automatically.\u001b[0m\n\n`; + } + + message += 'Configured MCP servers:\n\n'; for (const serverName of serverNames) { const serverTools = toolRegistry.getToolsByServer(serverName); const status = getMCPServerStatus(serverName); - // Add status indicator - let statusDot = ''; + // Add status indicator with descriptive text + let statusIndicator = ''; + let statusText = ''; switch (status) { case MCPServerStatus.CONNECTED: - statusDot = '🟢'; // Green dot for connected + statusIndicator = '🟢'; + statusText = 'Ready'; break; case MCPServerStatus.CONNECTING: - statusDot = '🟡'; // Yellow dot for connecting + statusIndicator = '🔄'; + statusText = 'Starting... (first startup may take longer)'; break; case MCPServerStatus.DISCONNECTED: default: - statusDot = '🔴'; // Red dot for disconnected + statusIndicator = '🔴'; + statusText = 'Disconnected'; break; } // Get server description if available const server = mcpServers[serverName]; - // Format server header with bold formatting - message += `${statusDot} \u001b[1m${serverName}\u001b[0m (${serverTools.length} tools)`; + // Format server header with bold formatting and status + message += `${statusIndicator} \u001b[1m${serverName}\u001b[0m - ${statusText}`; + + // Add tool count with conditional messaging + if (status === MCPServerStatus.CONNECTED) { + message += ` (${serverTools.length} tools)`; + } else if (status === MCPServerStatus.CONNECTING) { + message += ` (tools will appear when ready)`; + } else { + message += ` (${serverTools.length} tools cached)`; + } // Add server description with proper handling of multi-line descriptions if (useShowDescriptions && server?.description) { diff --git a/packages/core/src/tools/mcp-client.ts b/packages/core/src/tools/mcp-client.ts index 1b797ba4..a7d6e00c 100644 --- a/packages/core/src/tools/mcp-client.ts +++ b/packages/core/src/tools/mcp-client.ts @@ -27,11 +27,28 @@ export enum MCPServerStatus { CONNECTED = 'connected', } +/** + * Enum representing the overall MCP discovery state + */ +export enum MCPDiscoveryState { + /** Discovery has not started yet */ + NOT_STARTED = 'not_started', + /** Discovery is currently in progress */ + IN_PROGRESS = 'in_progress', + /** Discovery has completed (with or without errors) */ + COMPLETED = 'completed', +} + /** * Map to track the status of each MCP server within the core package */ const mcpServerStatusesInternal: Map = new Map(); +/** + * Track the overall MCP discovery state + */ +let mcpDiscoveryState: MCPDiscoveryState = MCPDiscoveryState.NOT_STARTED; + /** * Event listeners for MCP server status changes */ @@ -92,29 +109,48 @@ export function getAllMCPServerStatuses(): Map { return new Map(mcpServerStatusesInternal); } +/** + * Get the current MCP discovery state + */ +export function getMCPDiscoveryState(): MCPDiscoveryState { + return mcpDiscoveryState; +} + export async function discoverMcpTools( mcpServers: Record, mcpServerCommand: string | undefined, toolRegistry: ToolRegistry, ): Promise { - if (mcpServerCommand) { - const cmd = mcpServerCommand; - const args = parse(cmd, process.env) as string[]; - if (args.some((arg) => typeof arg !== 'string')) { - throw new Error('failed to parse mcpServerCommand: ' + cmd); - } - // use generic server name 'mcp' - mcpServers['mcp'] = { - command: args[0], - args: args.slice(1), - }; - } + // Set discovery state to in progress + mcpDiscoveryState = MCPDiscoveryState.IN_PROGRESS; - const discoveryPromises = Object.entries(mcpServers).map( - ([mcpServerName, mcpServerConfig]) => - connectAndDiscover(mcpServerName, mcpServerConfig, toolRegistry), - ); - await Promise.all(discoveryPromises); + try { + if (mcpServerCommand) { + const cmd = mcpServerCommand; + const args = parse(cmd, process.env) as string[]; + if (args.some((arg) => typeof arg !== 'string')) { + throw new Error('failed to parse mcpServerCommand: ' + cmd); + } + // use generic server name 'mcp' + mcpServers['mcp'] = { + command: args[0], + args: args.slice(1), + }; + } + + const discoveryPromises = Object.entries(mcpServers).map( + ([mcpServerName, mcpServerConfig]) => + connectAndDiscover(mcpServerName, mcpServerConfig, toolRegistry), + ); + await Promise.all(discoveryPromises); + + // Mark discovery as completed + mcpDiscoveryState = MCPDiscoveryState.COMPLETED; + } catch (error) { + // Still mark as completed even with errors + mcpDiscoveryState = MCPDiscoveryState.COMPLETED; + throw error; + } } async function connectAndDiscover( @@ -172,9 +208,19 @@ async function connectAndDiscover( // Connection successful updateMCPServerStatus(mcpServerName, MCPServerStatus.CONNECTED); } catch (error) { + // Create a safe config object that excludes sensitive information + const safeConfig = { + command: mcpServerConfig.command, + url: mcpServerConfig.url, + cwd: mcpServerConfig.cwd, + timeout: mcpServerConfig.timeout, + trust: mcpServerConfig.trust, + // Exclude args and env which may contain sensitive data + }; + console.error( `failed to start or connect to MCP server '${mcpServerName}' ` + - `${JSON.stringify(mcpServerConfig)}; \n${error}`, + `${JSON.stringify(safeConfig)}; \n${error}`, ); // Update status to disconnected updateMCPServerStatus(mcpServerName, MCPServerStatus.DISCONNECTED);