addressed b/423798481 (#887)

3 files changed, 221 insertions, 42 deletions

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
@@ -28,11 +28,28 @@ export enum MCPServerStatus {
 }
 
 /**
+ * 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<string, MCPServerStatus> = new Map();
 
 /**
+ * Track the overall MCP discovery state
+ */
+let mcpDiscoveryState: MCPDiscoveryState = MCPDiscoveryState.NOT_STARTED;
+
+/**
  * Event listeners for MCP server status changes
  */
 type StatusChangeListener = (
@@ -92,29 +109,48 @@ export function getAllMCPServerStatuses(): Map<string, MCPServerStatus> {
   return new Map(mcpServerStatusesInternal);
 }
 
+/**
+ * Get the current MCP discovery state
+ */
+export function getMCPDiscoveryState(): MCPDiscoveryState {
+  return mcpDiscoveryState;
+}
+
 export async function discoverMcpTools(
   mcpServers: Record<string, MCPServerConfig>,
   mcpServerCommand: string | undefined,
   toolRegistry: ToolRegistry,
 ): Promise<void> {
-  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);
+  // Set discovery state to in progress
+  mcpDiscoveryState = MCPDiscoveryState.IN_PROGRESS;
+
+  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),
+      };
     }
-    // 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);
+    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);