Merge bf92af93df into a38e2f0048

2026-04-21 13:37:17 +00:00 · 2026-04-21 12:55:14 +05:30 · 2026-04-21 12:55:14 +05:30 · 815686e509
commit 815686e509
parent a38e2f0048 bf92af93df
2 changed files with 118 additions and 87 deletions
--- a/packages/cli/src/commands/mcp/list.test.ts
+++ b/packages/cli/src/commands/mcp/list.test.ts
@ -138,7 +138,7 @@ describe('mcp list command', () => {
      merged: { ...defaultMergedSettings, mcpServers: {} },
    });

-    await listMcpServers();
+    await listMcpServers(undefined, true);

    expect(debugLogger.log).toHaveBeenCalledWith('No MCP servers configured.');
  });
@ -165,7 +165,7 @@ describe('mcp list command', () => {
    mockClient.connect.mockResolvedValue(undefined);
    mockClient.ping.mockResolvedValue(undefined);

-    await listMcpServers();
+    await listMcpServers(undefined, true);

    expect(debugLogger.log).toHaveBeenCalledWith('Configured MCP servers:\n');
    expect(debugLogger.log).toHaveBeenCalledWith(
@ -208,7 +208,7 @@ describe('mcp list command', () => {

    mockClient.connect.mockRejectedValue(new Error('Connection failed'));

-    await listMcpServers();
+    await listMcpServers(undefined, true);

    expect(debugLogger.log).toHaveBeenCalledWith(
      expect.stringContaining(
@ -239,7 +239,7 @@ describe('mcp list command', () => {
    mockClient.connect.mockResolvedValue(undefined);
    mockClient.ping.mockResolvedValue(undefined);

-    await listMcpServers();
+    await listMcpServers(undefined, true);

    expect(debugLogger.log).toHaveBeenCalledWith(
      expect.stringContaining(
@ -280,10 +280,13 @@ describe('mcp list command', () => {
    mockClient.connect.mockResolvedValue(undefined);
    mockClient.ping.mockResolvedValue(undefined);

-    await listMcpServers({
-      merged: settingsWithAllowlist,
-      isTrusted: true,
-    } as unknown as LoadedSettings);
+    await listMcpServers(
+      {
+        merged: settingsWithAllowlist,
+        isTrusted: true,
+      } as unknown as LoadedSettings,
+      true,
+    );

    expect(debugLogger.log).toHaveBeenCalledWith(
      expect.stringContaining('allowed-server'),
@ -314,7 +317,7 @@ describe('mcp list command', () => {
    // createTransport will throw in core if not trusted
    mockedCreateTransport.mockRejectedValue(new Error('Folder not trusted'));

-    await listMcpServers();
+    await listMcpServers(undefined, true);

    expect(debugLogger.log).toHaveBeenCalledWith(
      expect.stringContaining(
@ -338,7 +341,7 @@ describe('mcp list command', () => {
      isTrusted: true,
    });

-    await listMcpServers();
+    await listMcpServers(undefined, true);

    expect(debugLogger.log).toHaveBeenCalledWith(
      expect.stringContaining(
@ -365,7 +368,7 @@ describe('mcp list command', () => {
      'isFileEnabled',
    ).mockResolvedValue(false);

-    await listMcpServers();
+    await listMcpServers(undefined, true);

    expect(debugLogger.log).toHaveBeenCalledWith(
      expect.stringContaining(
@ -374,4 +377,25 @@ describe('mcp list command', () => {
    );
    expect(mockedCreateTransport).not.toHaveBeenCalled();
  });
+
+  it('should display fast list (bullets) by default without connecting', async () => {
+    const defaultMergedSettings = mergeSettings({}, {}, {}, {}, true);
+    mockedLoadSettings.mockReturnValue({
+      merged: {
+        ...defaultMergedSettings,
+        mcpServers: {
+          'fast-server': { command: '/path/to/server' },
+        },
+      },
+      isTrusted: true,
+    });
+
+    await listMcpServers(); // Default checkConnections = false
+
+    expect(debugLogger.log).toHaveBeenCalledWith('Configured MCP servers:\n');
+    expect(debugLogger.log).toHaveBeenCalledWith(
+      expect.stringContaining('• fast-server: /path/to/server  (stdio)'),
+    );
+    expect(mockedCreateTransport).not.toHaveBeenCalled(); // Fast list should not connect
+  });
 });
--- a/packages/cli/src/commands/mcp/list.ts
+++ b/packages/cli/src/commands/mcp/list.ts
@ -46,13 +46,14 @@ export async function getMcpServersFromConfig(
    requestConsent: requestConsentNonInteractive,
    requestSetting: promptForSetting,
  });
+
  const extensions = await extensionManager.loadExtensions();
  const mcpServers = { ...settings.mcpServers };
+
  for (const extension of extensions) {
    Object.entries(extension.mcpServers || {}).forEach(([key, server]) => {
-      if (mcpServers[key]) {
-        return;
-      }
+      if (mcpServers[key]) return;
+
      mcpServers[key] = {
        // eslint-disable-next-line @typescript-eslint/no-misused-spread
        ...server,
@ -62,9 +63,7 @@ export async function getMcpServersFromConfig(
  }

  const adminAllowlist = settings.admin?.mcp?.config;
-  const filteredResult = applyAdminAllowlist(mcpServers, adminAllowlist);
-
-  return filteredResult;
+  return applyAdminAllowlist(mcpServers, adminAllowlist);
 }

 async function testMCPConnection(
@ -73,9 +72,8 @@ async function testMCPConnection(
  isTrusted: boolean,
  activeSettings: MergedSettings,
 ): Promise<MCPServerStatus> {
-  // SECURITY: Only test connection if workspace is trusted or if it's a remote server.
-  // stdio servers execute local commands and must never run in untrusted workspaces.
  const isStdio = !!config.command;
+
  if (isStdio && !isTrusted) {
    return MCPServerStatus.DISCONNECTED;
  }
@ -97,7 +95,6 @@ async function testMCPConnection(
      error?: unknown,
      serverName?: string,
    ) => {
-      // In non-interactive list, we log everything through debugLogger for consistency
      if (severity === 'error') {
        debugLogger.error(
          chalk.red(`Error${serverName ? ` (${serverName})` : ''}: ${message}`),
@ -119,7 +116,6 @@ async function testMCPConnection(

  let transport;
  try {
-    // Use the same transport creation logic as core
    transport = await createTransport(serverName, config, false, mcpContext);
  } catch {
    await client.close();
@ -127,10 +123,7 @@ async function testMCPConnection(
  }

  try {
-    // Attempt actual MCP connection with short timeout
-    await client.connect(transport, { timeout: 5000 }); // 5s timeout
-
-    // Test basic MCP protocol by pinging the server
+    await client.connect(transport, { timeout: 5000 });
    await client.ping();

    await client.close();
@ -141,43 +134,16 @@ async function testMCPConnection(
  }
 }

-async function getServerStatus(
-  serverName: string,
-  server: MCPServerConfig,
-  isTrusted: boolean,
-  activeSettings: MergedSettings,
-): Promise<MCPServerStatus> {
-  const mcpEnablementManager = McpServerEnablementManager.getInstance();
-  const loadResult = await canLoadServer(serverName, {
-    adminMcpEnabled: activeSettings.admin?.mcp?.enabled ?? true,
-    allowedList: activeSettings.mcp?.allowed,
-    excludedList: activeSettings.mcp?.excluded,
-    enablement: mcpEnablementManager.getEnablementCallbacks(),
-  });
-
-  if (!loadResult.allowed) {
-    if (
-      loadResult.blockType === 'admin' ||
-      loadResult.blockType === 'allowlist' ||
-      loadResult.blockType === 'excludelist'
-    ) {
-      return MCPServerStatus.BLOCKED;
-    }
-    return MCPServerStatus.DISABLED;
-  }
-
-  // Test all server types by attempting actual connection
-  return testMCPConnection(serverName, server, isTrusted, activeSettings);
-}
-
 export async function listMcpServers(
  loadedSettingsArg?: LoadedSettings,
+  checkConnections = false,
 ): Promise<void> {
  const loadedSettings = loadedSettingsArg ?? loadSettings();
  const activeSettings = loadedSettings.merged;

  const { mcpServers, blockedServerNames } =
    await getMcpServersFromConfig(activeSettings);
+
  const serverNames = Object.keys(mcpServers);

  if (blockedServerNames.length > 0) {
@ -197,46 +163,50 @@ export async function listMcpServers(

  debugLogger.log('Configured MCP servers:\n');

+  // ✅ Optimization: compute once
+  const enablementCallbacks =
+    McpServerEnablementManager.getInstance().getEnablementCallbacks();
+
  for (const serverName of serverNames) {
    const server = mcpServers[serverName];

-    const status = await getServerStatus(
-      serverName,
-      server,
-      loadedSettings.isTrusted,
-      activeSettings,
-    );
+    let status: MCPServerStatus;

-    let statusIndicator = '';
-    let statusText = '';
-    switch (status) {
-      case MCPServerStatus.CONNECTED:
-        statusIndicator = chalk.green('✓');
-        statusText = 'Connected';
-        break;
-      case MCPServerStatus.CONNECTING:
-        statusIndicator = chalk.yellow('…');
-        statusText = 'Connecting';
-        break;
-      case MCPServerStatus.BLOCKED:
-        statusIndicator = chalk.red('⛔');
-        statusText = 'Blocked';
-        break;
-      case MCPServerStatus.DISABLED:
-        statusIndicator = chalk.gray('○');
-        statusText = 'Disabled';
-        break;
-      case MCPServerStatus.DISCONNECTED:
-      default:
-        statusIndicator = chalk.red('✗');
-        statusText = 'Disconnected';
-        break;
+    // ✅ Always perform local checks (fast)
+    const loadResult = await canLoadServer(serverName, {
+      adminMcpEnabled: activeSettings.admin?.mcp?.enabled ?? true,
+      allowedList: activeSettings.mcp?.allowed,
+      excludedList: activeSettings.mcp?.excluded,
+      enablement: enablementCallbacks,
+    });
+
+    if (!loadResult.allowed) {
+      if (
+        loadResult.blockType === 'admin' ||
+        loadResult.blockType === 'allowlist' ||
+        loadResult.blockType === 'excludelist'
+      ) {
+        status = MCPServerStatus.BLOCKED;
+      } else {
+        status = MCPServerStatus.DISABLED;
+      }
+    } else if (checkConnections) {
+      // ✅ Only slow check when explicitly requested
+      status = await testMCPConnection(
+        serverName,
+        server,
+        loadedSettings.isTrusted,
+        activeSettings,
+      );
+    } else {
+      status = MCPServerStatus.DISCONNECTED;
    }

    let serverInfo =
      serverName +
      (server.extension?.name ? ` (from ${server.extension.name})` : '') +
      ': ';
+
    if (server.httpUrl) {
      serverInfo += `${server.httpUrl} (http)`;
    } else if (server.url) {
@ -246,19 +216,56 @@ export async function listMcpServers(
      serverInfo += `${server.command} ${server.args?.join(' ') || ''} (stdio)`;
    }

-    debugLogger.log(`${statusIndicator} ${serverInfo} - ${statusText}`);
+    if (checkConnections) {
+      let statusIndicator = '';
+      let statusText = '';
+
+      switch (status) {
+        case MCPServerStatus.CONNECTED:
+          statusIndicator = chalk.green('✓');
+          statusText = 'Connected';
+          break;
+        case MCPServerStatus.CONNECTING:
+          statusIndicator = chalk.yellow('…');
+          statusText = 'Connecting';
+          break;
+        case MCPServerStatus.BLOCKED:
+          statusIndicator = chalk.red('⛔');
+          statusText = 'Blocked';
+          break;
+        case MCPServerStatus.DISABLED:
+          statusIndicator = chalk.gray('○');
+          statusText = 'Disabled';
+          break;
+        case MCPServerStatus.DISCONNECTED:
+        default:
+          statusIndicator = chalk.red('✗');
+          statusText = 'Disconnected';
+          break;
+      }
+
+      debugLogger.log(`${statusIndicator} ${serverInfo} - ${statusText}`);
+    } else {
+      debugLogger.log(`• ${serverInfo}`);
+    }
  }
 }

 interface ListArgs {
-  loadedSettings?: LoadedSettings;
+  check?: boolean;
 }

 export const listCommand: CommandModule<object, ListArgs> = {
  command: 'list',
  describe: 'List all configured MCP servers',
+  builder: (yargs) =>
+    yargs.option('check', {
+      type: 'boolean',
+      default: false,
+      describe: 'Test connection to each MCP server',
+    }),
  handler: async (argv) => {
-    await listMcpServers(argv.loadedSettings);
+    await listMcpServers(undefined, argv.check);
    await exitCli();
  },
 };