From 20d884da2fe16f7b4b41e75c142d44e570cff144 Mon Sep 17 00:00:00 2001 From: Spencer Date: Fri, 27 Feb 2026 15:04:36 -0500 Subject: [PATCH] fix(core): reduce intrusive MCP errors and deduplicate diagnostics (#20232) --- docs/tools/mcp-server.md | 22 ++ packages/cli/src/commands/mcp/list.test.ts | 40 +++- packages/cli/src/commands/mcp/list.ts | 65 ++++-- packages/cli/src/test-utils/mockConfig.ts | 3 + .../cli/src/ui/commands/mcpCommand.test.ts | 16 +- packages/cli/src/ui/commands/mcpCommand.ts | 24 +- .../ui/components/views/McpStatus.test.tsx | 13 ++ .../cli/src/ui/components/views/McpStatus.tsx | 10 + .../__snapshots__/McpStatus.test.tsx.snap | 12 + packages/cli/src/ui/types.ts | 1 + packages/core/src/config/config.ts | 30 ++- .../tools/__snapshots__/shell.test.ts.snap | 8 - .../coreToolsModelSnapshots.test.ts.snap | 6 +- .../dynamic-declaration-helpers.ts | 8 +- .../definitions/model-family-sets/gemini-3.ts | 2 +- .../core/src/tools/mcp-client-manager.test.ts | 94 ++++++++ packages/core/src/tools/mcp-client-manager.ts | 91 +++++++- packages/core/src/tools/mcp-client.test.ts | 210 ++++++++++-------- packages/core/src/tools/mcp-client.ts | 155 +++++++++---- packages/core/src/tools/mcp-tool.ts | 7 +- 20 files changed, 626 insertions(+), 191 deletions(-) diff --git a/docs/tools/mcp-server.md b/docs/tools/mcp-server.md index 22ce748918..202325a83d 100644 --- a/docs/tools/mcp-server.md +++ b/docs/tools/mcp-server.md @@ -1066,6 +1066,11 @@ command has no flags. gemini mcp list ``` +> **Note on Trust:** For security, `stdio` MCP servers (those using the +> `command` property) are only tested and displayed as "Connected" if the +> current folder is trusted. If the folder is untrusted, they will show as +> "Disconnected". Use `gemini trust` to trust the current folder. + **Example output:** ```sh @@ -1074,6 +1079,23 @@ gemini mcp list ✗ sse-server: https://api.example.com/sse (sse) - Disconnected ``` +## Troubleshooting and Diagnostics + +To minimize noise during startup, MCP connection errors for background servers +are "silent by default." If issues are detected during startup, a single +informational hint will be shown: _"MCP issues detected. Run /mcp list for +status."_ + +Detailed, actionable diagnostics for a specific server are automatically +re-enabled when: + +1. You run an interactive command like `/mcp list`, `/mcp auth`, etc. +2. The model attempts to execute a tool from that server. +3. You invoke an MCP prompt from that server. + +You can also use `gemini mcp list` from your shell to see connection errors for +all configured servers. + ### Removing a server (`gemini mcp remove`) To delete a server from your configuration, use the `remove` command with the diff --git a/packages/cli/src/commands/mcp/list.test.ts b/packages/cli/src/commands/mcp/list.test.ts index 60912c51f5..aaaf667815 100644 --- a/packages/cli/src/commands/mcp/list.test.ts +++ b/packages/cli/src/commands/mcp/list.test.ts @@ -4,7 +4,15 @@ * SPDX-License-Identifier: Apache-2.0 */ -import { vi, describe, it, expect, beforeEach, type Mock } from 'vitest'; +import { + vi, + describe, + it, + expect, + beforeEach, + afterEach, + type Mock, +} from 'vitest'; import { listMcpServers } from './list.js'; import { loadSettings, mergeSettings } from '../../config/settings.js'; import { createTransport, debugLogger } from '@google/gemini-cli-core'; @@ -106,6 +114,10 @@ describe('mcp list command', () => { mockedGetUserExtensionsDir.mockReturnValue('/mocked/extensions/dir'); }); + afterEach(() => { + vi.restoreAllMocks(); + }); + it('should display message when no servers configured', async () => { const defaultMergedSettings = mergeSettings({}, {}, {}, {}, true); mockedLoadSettings.mockReturnValue({ @@ -133,6 +145,7 @@ describe('mcp list command', () => { }, }, }, + isTrusted: true, }); mockClient.connect.mockResolvedValue(undefined); @@ -199,6 +212,7 @@ describe('mcp list command', () => { 'config-server': { command: '/config/server' }, }, }, + isTrusted: true, }); mockExtensionManager.loadExtensions.mockReturnValue([ @@ -266,4 +280,28 @@ describe('mcp list command', () => { expect.anything(), ); }); + + it('should show stdio servers as disconnected in untrusted folders', async () => { + const defaultMergedSettings = mergeSettings({}, {}, {}, {}, true); + mockedLoadSettings.mockReturnValue({ + merged: { + ...defaultMergedSettings, + mcpServers: { + 'test-server': { command: '/test/server' }, + }, + }, + isTrusted: false, + }); + + // createTransport will throw in core if not trusted + mockedCreateTransport.mockRejectedValue(new Error('Folder not trusted')); + + await listMcpServers(); + + expect(debugLogger.log).toHaveBeenCalledWith( + expect.stringContaining( + 'test-server: /test/server (stdio) - Disconnected', + ), + ); + }); }); diff --git a/packages/cli/src/commands/mcp/list.ts b/packages/cli/src/commands/mcp/list.ts index d51093fbfa..421c822a55 100644 --- a/packages/cli/src/commands/mcp/list.ts +++ b/packages/cli/src/commands/mcp/list.ts @@ -20,11 +20,7 @@ import { ExtensionManager } from '../../config/extension-manager.js'; import { requestConsentNonInteractive } from '../../config/extensions/consent.js'; import { promptForSetting } from '../../config/extensions/extensionSettings.js'; import { exitCli } from '../utils.js'; - -const COLOR_GREEN = '\u001b[32m'; -const COLOR_YELLOW = '\u001b[33m'; -const COLOR_RED = '\u001b[31m'; -const RESET_COLOR = '\u001b[0m'; +import chalk from 'chalk'; export async function getMcpServersFromConfig( settings?: MergedSettings, @@ -66,27 +62,56 @@ async function testMCPConnection( serverName: string, config: MCPServerConfig, ): Promise { + const settings = loadSettings(); + + // 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 && !settings.isTrusted) { + return MCPServerStatus.DISCONNECTED; + } + const client = new Client({ name: 'mcp-test-client', version: '0.0.1', }); - const settings = loadSettings(); - const sanitizationConfig = { - enableEnvironmentVariableRedaction: true, - allowedEnvironmentVariables: [], - blockedEnvironmentVariables: settings.merged.advanced.excludedEnvVars, + const mcpContext = { + sanitizationConfig: { + enableEnvironmentVariableRedaction: true, + allowedEnvironmentVariables: [], + blockedEnvironmentVariables: settings.merged.advanced.excludedEnvVars, + }, + emitMcpDiagnostic: ( + severity: 'info' | 'warning' | 'error', + message: string, + 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}`), + error, + ); + } else if (severity === 'warning') { + debugLogger.warn( + chalk.yellow( + `Warning${serverName ? ` (${serverName})` : ''}: ${message}`, + ), + error, + ); + } else { + debugLogger.log(message, error); + } + }, + isTrustedFolder: () => settings.isTrusted, }; let transport; try { // Use the same transport creation logic as core - transport = await createTransport( - serverName, - config, - false, - sanitizationConfig, - ); + transport = await createTransport(serverName, config, false, mcpContext); } catch (_error) { await client.close(); return MCPServerStatus.DISCONNECTED; @@ -125,7 +150,7 @@ export async function listMcpServers(settings?: MergedSettings): Promise { blockedServerNames, undefined, ); - debugLogger.log(COLOR_YELLOW + message + RESET_COLOR + '\n'); + debugLogger.log(chalk.yellow(message + '\n')); } if (serverNames.length === 0) { @@ -146,16 +171,16 @@ export async function listMcpServers(settings?: MergedSettings): Promise { let statusText = ''; switch (status) { case MCPServerStatus.CONNECTED: - statusIndicator = COLOR_GREEN + '✓' + RESET_COLOR; + statusIndicator = chalk.green('✓'); statusText = 'Connected'; break; case MCPServerStatus.CONNECTING: - statusIndicator = COLOR_YELLOW + '…' + RESET_COLOR; + statusIndicator = chalk.yellow('…'); statusText = 'Connecting'; break; case MCPServerStatus.DISCONNECTED: default: - statusIndicator = COLOR_RED + '✗' + RESET_COLOR; + statusIndicator = chalk.red('✗'); statusText = 'Disconnected'; break; } diff --git a/packages/cli/src/test-utils/mockConfig.ts b/packages/cli/src/test-utils/mockConfig.ts index bae89d36c9..8b7c7c520d 100644 --- a/packages/cli/src/test-utils/mockConfig.ts +++ b/packages/cli/src/test-utils/mockConfig.ts @@ -141,7 +141,10 @@ export const createMockConfig = (overrides: Partial = {}): Config => getMcpClientManager: vi.fn().mockReturnValue({ getMcpInstructions: vi.fn().mockReturnValue(''), getMcpServers: vi.fn().mockReturnValue({}), + getLastError: vi.fn().mockReturnValue(undefined), }), + setUserInteractedWithMcp: vi.fn(), + emitMcpDiagnostic: vi.fn(), getEnableEventDrivenScheduler: vi.fn().mockReturnValue(false), getAdminSkillsEnabled: vi.fn().mockReturnValue(false), getDisabledSkills: vi.fn().mockReturnValue([]), diff --git a/packages/cli/src/ui/commands/mcpCommand.test.ts b/packages/cli/src/ui/commands/mcpCommand.test.ts index ecce5c9cd5..3acace0774 100644 --- a/packages/cli/src/ui/commands/mcpCommand.test.ts +++ b/packages/cli/src/ui/commands/mcpCommand.test.ts @@ -4,7 +4,7 @@ * SPDX-License-Identifier: Apache-2.0 */ -import { vi, describe, it, expect, beforeEach } from 'vitest'; +import { vi, describe, it, expect, beforeEach, afterEach } from 'vitest'; import { mcpCommand } from './mcpCommand.js'; import { createMockCommandContext } from '../../test-utils/mockCommandContext.js'; import { @@ -77,6 +77,8 @@ describe('mcpCommand', () => { getGeminiClient: ReturnType; getMcpClientManager: ReturnType; getResourceRegistry: ReturnType; + setUserInteractedWithMcp: ReturnType; + getLastMcpError: ReturnType; }; beforeEach(() => { @@ -104,12 +106,15 @@ describe('mcpCommand', () => { }), getGeminiClient: vi.fn(), getMcpClientManager: vi.fn().mockImplementation(() => ({ - getBlockedMcpServers: vi.fn(), - getMcpServers: vi.fn(), + getBlockedMcpServers: vi.fn().mockReturnValue([]), + getMcpServers: vi.fn().mockReturnValue({}), + getLastError: vi.fn().mockReturnValue(undefined), })), getResourceRegistry: vi.fn().mockReturnValue({ getAllResources: vi.fn().mockReturnValue([]), }), + setUserInteractedWithMcp: vi.fn(), + getLastMcpError: vi.fn().mockReturnValue(undefined), }; mockContext = createMockCommandContext({ @@ -119,6 +124,10 @@ describe('mcpCommand', () => { }); }); + afterEach(() => { + vi.restoreAllMocks(); + }); + describe('basic functionality', () => { it('should show an error if config is not available', async () => { const contextWithoutConfig = createMockCommandContext({ @@ -161,6 +170,7 @@ describe('mcpCommand', () => { mockConfig.getMcpClientManager = vi.fn().mockReturnValue({ getMcpServers: vi.fn().mockReturnValue(mockMcpServers), getBlockedMcpServers: vi.fn().mockReturnValue([]), + getLastError: vi.fn().mockReturnValue(undefined), }); }); diff --git a/packages/cli/src/ui/commands/mcpCommand.ts b/packages/cli/src/ui/commands/mcpCommand.ts index 6b5e7d120c..e488db780f 100644 --- a/packages/cli/src/ui/commands/mcpCommand.ts +++ b/packages/cli/src/ui/commands/mcpCommand.ts @@ -52,6 +52,8 @@ const authCommand: SlashCommand = { }; } + config.setUserInteractedWithMcp(); + const mcpServers = config.getMcpClientManager()?.getMcpServers() ?? {}; if (!serverName) { @@ -184,6 +186,8 @@ const listAction = async ( }; } + config.setUserInteractedWithMcp(); + const toolRegistry = config.getToolRegistry(); if (!toolRegistry) { return { @@ -250,6 +254,13 @@ const listAction = async ( enablementState[serverName] = await enablementManager.getDisplayState(serverName); } + const errors: Record = {}; + for (const serverName of serverNames) { + const error = config.getMcpClientManager()?.getLastError(serverName); + if (error) { + errors[serverName] = error; + } + } const mcpStatusItem: HistoryItemMcpStatus = { type: MessageType.MCP_STATUS, @@ -274,16 +285,19 @@ const listAction = async ( })), authStatus, enablementState, - blockedServers: blockedMcpServers, + errors, + blockedServers: blockedMcpServers.map((s) => ({ + name: s.name, + extensionName: s.extensionName, + })), discoveryInProgress, connectingServers, - showDescriptions, - showSchema, + showDescriptions: Boolean(showDescriptions), + showSchema: Boolean(showSchema), }; context.ui.addItem(mcpStatusItem); }; - const listCommand: SlashCommand = { name: 'list', altNames: ['ls', 'nodesc', 'nodescription'], @@ -372,6 +386,8 @@ async function handleEnableDisable( }; } + config.setUserInteractedWithMcp(); + const parts = args.trim().split(/\s+/); const isSession = parts.includes('--session'); const serverName = parts.filter((p) => p !== '--session')[0]; diff --git a/packages/cli/src/ui/components/views/McpStatus.test.tsx b/packages/cli/src/ui/components/views/McpStatus.test.tsx index 77dccc77fe..1c600069c1 100644 --- a/packages/cli/src/ui/components/views/McpStatus.test.tsx +++ b/packages/cli/src/ui/components/views/McpStatus.test.tsx @@ -47,6 +47,7 @@ describe('McpStatus', () => { isPersistentDisabled: false, }, }, + errors: {}, discoveryInProgress: false, connectingServers: [], showDescriptions: true, @@ -208,6 +209,18 @@ describe('McpStatus', () => { unmount(); }); + it('renders correctly with a server error', async () => { + const { lastFrame, unmount, waitUntilReady } = render( + , + ); + await waitUntilReady(); + expect(lastFrame()).toMatchSnapshot(); + unmount(); + }); + it('truncates resources when exceeding limit', async () => { const manyResources = Array.from({ length: 25 }, (_, i) => ({ serverName: 'server-1', diff --git a/packages/cli/src/ui/components/views/McpStatus.tsx b/packages/cli/src/ui/components/views/McpStatus.tsx index 14ff7bdfc6..473c3de547 100644 --- a/packages/cli/src/ui/components/views/McpStatus.tsx +++ b/packages/cli/src/ui/components/views/McpStatus.tsx @@ -26,6 +26,7 @@ interface McpStatusProps { serverStatus: (serverName: string) => MCPServerStatus; authStatus: HistoryItemMcpStatus['authStatus']; enablementState: HistoryItemMcpStatus['enablementState']; + errors: Record; discoveryInProgress: boolean; connectingServers: string[]; showDescriptions: boolean; @@ -41,6 +42,7 @@ export const McpStatus: React.FC = ({ serverStatus, authStatus, enablementState, + errors, discoveryInProgress, connectingServers, showDescriptions, @@ -195,6 +197,14 @@ export const McpStatus: React.FC = ({ ({toolCount} tools cached) )} + {errors[serverName] && ( + + + Error: {errors[serverName]} + + + )} + {showDescriptions && server?.description && ( {server.description.trim()} diff --git a/packages/cli/src/ui/components/views/__snapshots__/McpStatus.test.tsx.snap b/packages/cli/src/ui/components/views/__snapshots__/McpStatus.test.tsx.snap index e7b7af36f2..eb1d1de83c 100644 --- a/packages/cli/src/ui/components/views/__snapshots__/McpStatus.test.tsx.snap +++ b/packages/cli/src/ui/components/views/__snapshots__/McpStatus.test.tsx.snap @@ -60,6 +60,18 @@ A test server " `; +exports[`McpStatus > renders correctly with a server error 1`] = ` +"Configured MCP servers: + +🟢 server-1 - Ready (1 tool) + Error: Failed to connect to server +A test server + Tools: + - tool-1 + A test tool +" +`; + exports[`McpStatus > renders correctly with authenticated OAuth status 1`] = ` "Configured MCP servers: diff --git a/packages/cli/src/ui/types.ts b/packages/cli/src/ui/types.ts index 14bf5847d7..55048ef6bc 100644 --- a/packages/cli/src/ui/types.ts +++ b/packages/cli/src/ui/types.ts @@ -340,6 +340,7 @@ export type HistoryItemMcpStatus = HistoryItemBase & { isPersistentDisabled: boolean; } >; + errors: Record; blockedServers: Array<{ name: string; extensionName: string }>; discoveryInProgress: boolean; connectingServers: string[]; diff --git a/packages/core/src/config/config.ts b/packages/core/src/config/config.ts index d2a6fe3672..6262956c11 100644 --- a/packages/core/src/config/config.ts +++ b/packages/core/src/config/config.ts @@ -367,6 +367,7 @@ import { SimpleExtensionLoader, } from '../utils/extensionLoader.js'; import { McpClientManager } from '../tools/mcp-client-manager.js'; +import { type McpContext } from '../tools/mcp-client.js'; import type { EnvironmentSanitizationConfig } from '../services/environmentSanitization.js'; import { getErrorMessage } from '../utils/errors.js'; import { @@ -580,7 +581,7 @@ export interface ConfigParameters { }; } -export class Config { +export class Config implements McpContext { private toolRegistry!: ToolRegistry; private mcpClientManager?: McpClientManager; private allowedMcpServers: string[]; @@ -1769,6 +1770,33 @@ export class Config { return this.mcpClientManager; } + setUserInteractedWithMcp(): void { + this.mcpClientManager?.setUserInteractedWithMcp(); + } + + /** @deprecated Use getMcpClientManager().getLastError() directly */ + getLastMcpError(serverName: string): string | undefined { + return this.mcpClientManager?.getLastError(serverName); + } + + emitMcpDiagnostic( + severity: 'info' | 'warning' | 'error', + message: string, + error?: unknown, + serverName?: string, + ): void { + if (this.mcpClientManager) { + this.mcpClientManager.emitDiagnostic( + severity, + message, + error, + serverName, + ); + } else { + coreEvents.emitFeedback(severity, message, error); + } + } + getAllowedMcpServers(): string[] | undefined { return this.allowedMcpServers; } diff --git a/packages/core/src/tools/__snapshots__/shell.test.ts.snap b/packages/core/src/tools/__snapshots__/shell.test.ts.snap index b7101cb6b6..471ce45f6e 100644 --- a/packages/core/src/tools/__snapshots__/shell.test.ts.snap +++ b/packages/core/src/tools/__snapshots__/shell.test.ts.snap @@ -3,8 +3,6 @@ exports[`ShellTool > getDescription > should return the non-windows description when not on windows 1`] = ` "This tool executes a given shell command as \`bash -c \`. Command can start background processes using \`&\`. Command is executed as a subprocess that leads its own process group. Command process group can be terminated as \`kill -- -PGID\` or signaled as \`kill -s SIGNAL -- -PGID\`. - The CLI will automatically prompt the user for confirmation before executing any command provided by this tool, so you MUST NOT ask for permission or confirmation separately (e.g., using ask_user). - Efficiency Guidelines: - Quiet Flags: Always prefer silent or quiet flags (e.g., \`npm install --silent\`, \`git --no-pager\`) to reduce output volume while still capturing necessary information. - Pagination: Always disable terminal pagination to ensure commands terminate (e.g., use \`git --no-pager\`, \`systemctl --no-pager\`, or set \`PAGER=cat\`). @@ -22,8 +20,6 @@ exports[`ShellTool > getDescription > should return the non-windows description exports[`ShellTool > getDescription > should return the windows description when on windows 1`] = ` "This tool executes a given shell command as \`powershell.exe -NoProfile -Command \`. Command can start background processes using PowerShell constructs such as \`Start-Process -NoNewWindow\` or \`Start-Job\`. - The CLI will automatically prompt the user for confirmation before executing any command provided by this tool, so you MUST NOT ask for permission or confirmation separately (e.g., using ask_user). - Efficiency Guidelines: - Quiet Flags: Always prefer silent or quiet flags (e.g., \`npm install --silent\`, \`git --no-pager\`) to reduce output volume while still capturing necessary information. - Pagination: Always disable terminal pagination to ensure commands terminate (e.g., use \`git --no-pager\`, \`systemctl --no-pager\`, or set \`PAGER=cat\`). @@ -41,8 +37,6 @@ exports[`ShellTool > getDescription > should return the windows description when exports[`ShellTool > getSchema > should return the base schema when no modelId is provided 1`] = ` "This tool executes a given shell command as \`bash -c \`. Command can start background processes using \`&\`. Command is executed as a subprocess that leads its own process group. Command process group can be terminated as \`kill -- -PGID\` or signaled as \`kill -s SIGNAL -- -PGID\`. - The CLI will automatically prompt the user for confirmation before executing any command provided by this tool, so you MUST NOT ask for permission or confirmation separately (e.g., using ask_user). - Efficiency Guidelines: - Quiet Flags: Always prefer silent or quiet flags (e.g., \`npm install --silent\`, \`git --no-pager\`) to reduce output volume while still capturing necessary information. - Pagination: Always disable terminal pagination to ensure commands terminate (e.g., use \`git --no-pager\`, \`systemctl --no-pager\`, or set \`PAGER=cat\`). @@ -60,8 +54,6 @@ exports[`ShellTool > getSchema > should return the base schema when no modelId i exports[`ShellTool > getSchema > should return the schema from the resolver when modelId is provided 1`] = ` "This tool executes a given shell command as \`bash -c \`. Command can start background processes using \`&\`. Command is executed as a subprocess that leads its own process group. Command process group can be terminated as \`kill -- -PGID\` or signaled as \`kill -s SIGNAL -- -PGID\`. - The CLI will automatically prompt the user for confirmation before executing any command provided by this tool, so you MUST NOT ask for permission or confirmation separately (e.g., using ask_user). - Efficiency Guidelines: - Quiet Flags: Always prefer silent or quiet flags (e.g., \`npm install --silent\`, \`git --no-pager\`) to reduce output volume while still capturing necessary information. - Pagination: Always disable terminal pagination to ensure commands terminate (e.g., use \`git --no-pager\`, \`systemctl --no-pager\`, or set \`PAGER=cat\`). diff --git a/packages/core/src/tools/definitions/__snapshots__/coreToolsModelSnapshots.test.ts.snap b/packages/core/src/tools/definitions/__snapshots__/coreToolsModelSnapshots.test.ts.snap index 4700865d06..70cf828d86 100644 --- a/packages/core/src/tools/definitions/__snapshots__/coreToolsModelSnapshots.test.ts.snap +++ b/packages/core/src/tools/definitions/__snapshots__/coreToolsModelSnapshots.test.ts.snap @@ -568,8 +568,6 @@ exports[`coreTools snapshots for specific models > Model: gemini-2.5-pro > snaps { "description": "This tool executes a given shell command as \`bash -c \`. To run a command in the background, set the \`is_background\` parameter to true. Do NOT use \`&\` to background commands. Command is executed as a subprocess that leads its own process group. Command process group can be terminated as \`kill -- -PGID\` or signaled as \`kill -s SIGNAL -- -PGID\`. - The CLI will automatically prompt the user for confirmation before executing any command provided by this tool, so you MUST NOT ask for permission or confirmation separately (e.g., using ask_user). - Efficiency Guidelines: - Quiet Flags: Always prefer silent or quiet flags (e.g., \`npm install --silent\`, \`git --no-pager\`) to reduce output volume while still capturing necessary information. - Pagination: Always disable terminal pagination to ensure commands terminate (e.g., use \`git --no-pager\`, \`systemctl --no-pager\`, or set \`PAGER=cat\`). @@ -861,7 +859,7 @@ exports[`coreTools snapshots for specific models > Model: gemini-3-pro-preview > exports[`coreTools snapshots for specific models > Model: gemini-3-pro-preview > snapshot for tool: ask_user 1`] = ` { - "description": "Ask the user one or more questions to gather preferences, clarify requirements, or make decisions. DO NOT use this tool to ask for permission to run shell commands; the run_shell_command tool has built-in confirmation. When using this tool, prefer providing multiple-choice options with detailed descriptions and enable multi-select where appropriate to provide maximum flexibility.", + "description": "Ask the user one or more questions to gather preferences, clarify requirements, or make decisions. When using this tool, prefer providing multiple-choice options with detailed descriptions and enable multi-select where appropriate to provide maximum flexibility.", "name": "ask_user", "parametersJsonSchema": { "properties": { @@ -1333,8 +1331,6 @@ exports[`coreTools snapshots for specific models > Model: gemini-3-pro-preview > { "description": "This tool executes a given shell command as \`bash -c \`. To run a command in the background, set the \`is_background\` parameter to true. Do NOT use \`&\` to background commands. Command is executed as a subprocess that leads its own process group. Command process group can be terminated as \`kill -- -PGID\` or signaled as \`kill -s SIGNAL -- -PGID\`. - The CLI will automatically prompt the user for confirmation before executing any command provided by this tool, so you MUST NOT ask for permission or confirmation separately (e.g., using ask_user). - Efficiency Guidelines: - Quiet Flags: Always prefer silent or quiet flags (e.g., \`npm install --silent\`, \`git --no-pager\`) to reduce output volume while still capturing necessary information. - Pagination: Always disable terminal pagination to ensure commands terminate (e.g., use \`git --no-pager\`, \`systemctl --no-pager\`, or set \`PAGER=cat\`). diff --git a/packages/core/src/tools/definitions/dynamic-declaration-helpers.ts b/packages/core/src/tools/definitions/dynamic-declaration-helpers.ts index 562320e57b..83ed680ce7 100644 --- a/packages/core/src/tools/definitions/dynamic-declaration-helpers.ts +++ b/packages/core/src/tools/definitions/dynamic-declaration-helpers.ts @@ -45,20 +45,16 @@ export function getShellToolDescription( Background PIDs: Only included if background processes were started. Process Group PGID: Only included if available.`; - const confirmationNote = ` - - The CLI will automatically prompt the user for confirmation before executing any command provided by this tool, so you MUST NOT ask for permission or confirmation separately (e.g., using ask_user).`; - if (os.platform() === 'win32') { const backgroundInstructions = enableInteractiveShell ? 'To run a command in the background, set the `is_background` parameter to true. Do NOT use PowerShell background constructs.' : 'Command can start background processes using PowerShell constructs such as `Start-Process -NoNewWindow` or `Start-Job`.'; - return `This tool executes a given shell command as \`powershell.exe -NoProfile -Command \`. ${backgroundInstructions}${confirmationNote}${efficiencyGuidelines}${returnedInfo}`; + return `This tool executes a given shell command as \`powershell.exe -NoProfile -Command \`. ${backgroundInstructions}${efficiencyGuidelines}${returnedInfo}`; } else { const backgroundInstructions = enableInteractiveShell ? 'To run a command in the background, set the `is_background` parameter to true. Do NOT use `&` to background commands.' : 'Command can start background processes using `&`.'; - return `This tool executes a given shell command as \`bash -c \`. ${backgroundInstructions} Command is executed as a subprocess that leads its own process group. Command process group can be terminated as \`kill -- -PGID\` or signaled as \`kill -s SIGNAL -- -PGID\`.${confirmationNote}${efficiencyGuidelines}${returnedInfo}`; + return `This tool executes a given shell command as \`bash -c \`. ${backgroundInstructions} Command is executed as a subprocess that leads its own process group. Command process group can be terminated as \`kill -- -PGID\` or signaled as \`kill -s SIGNAL -- -PGID\`.${efficiencyGuidelines}${returnedInfo}`; } } diff --git a/packages/core/src/tools/definitions/model-family-sets/gemini-3.ts b/packages/core/src/tools/definitions/model-family-sets/gemini-3.ts index 7c1f171366..7c4fddc9f6 100644 --- a/packages/core/src/tools/definitions/model-family-sets/gemini-3.ts +++ b/packages/core/src/tools/definitions/model-family-sets/gemini-3.ts @@ -558,7 +558,7 @@ The agent did not use the todo list because this task could be completed by a ti ask_user: { name: ASK_USER_TOOL_NAME, description: - 'Ask the user one or more questions to gather preferences, clarify requirements, or make decisions. DO NOT use this tool to ask for permission to run shell commands; the run_shell_command tool has built-in confirmation. When using this tool, prefer providing multiple-choice options with detailed descriptions and enable multi-select where appropriate to provide maximum flexibility.', + 'Ask the user one or more questions to gather preferences, clarify requirements, or make decisions. When using this tool, prefer providing multiple-choice options with detailed descriptions and enable multi-select where appropriate to provide maximum flexibility.', parametersJsonSchema: { type: 'object', required: ['questions'], diff --git a/packages/core/src/tools/mcp-client-manager.test.ts b/packages/core/src/tools/mcp-client-manager.test.ts index 9aec6014b0..e436cea356 100644 --- a/packages/core/src/tools/mcp-client-manager.test.ts +++ b/packages/core/src/tools/mcp-client-manager.test.ts @@ -474,4 +474,98 @@ describe('McpClientManager', () => { }); }); }); + + describe('diagnostic reporting', () => { + let coreEventsMock: typeof import('../utils/events.js').coreEvents; + + beforeEach(async () => { + const eventsModule = await import('../utils/events.js'); + coreEventsMock = eventsModule.coreEvents; + vi.spyOn(coreEventsMock, 'emitFeedback').mockImplementation(() => {}); + }); + + it('should emit hint instead of full error when user has not interacted with MCP', () => { + const manager = new McpClientManager('0.0.1', toolRegistry, mockConfig); + manager.emitDiagnostic( + 'error', + 'Something went wrong', + new Error('boom'), + ); + + expect(coreEventsMock.emitFeedback).toHaveBeenCalledWith( + 'info', + 'MCP issues detected. Run /mcp list for status.', + ); + expect(coreEventsMock.emitFeedback).not.toHaveBeenCalledWith( + 'error', + 'Something went wrong', + expect.anything(), + ); + }); + + it('should emit full error when user has interacted with MCP', () => { + const manager = new McpClientManager('0.0.1', toolRegistry, mockConfig); + manager.setUserInteractedWithMcp(); + manager.emitDiagnostic( + 'error', + 'Something went wrong', + new Error('boom'), + ); + + expect(coreEventsMock.emitFeedback).toHaveBeenCalledWith( + 'error', + 'Something went wrong', + expect.any(Error), + ); + }); + + it('should still deduplicate diagnostic messages after user interaction', () => { + const manager = new McpClientManager('0.0.1', toolRegistry, mockConfig); + manager.setUserInteractedWithMcp(); + + manager.emitDiagnostic('error', 'Same error'); + manager.emitDiagnostic('error', 'Same error'); + + expect(coreEventsMock.emitFeedback).toHaveBeenCalledTimes(1); + }); + + it('should only show hint once per session', () => { + const manager = new McpClientManager('0.0.1', toolRegistry, mockConfig); + + manager.emitDiagnostic('error', 'Error 1'); + manager.emitDiagnostic('error', 'Error 2'); + + expect(coreEventsMock.emitFeedback).toHaveBeenCalledTimes(1); + expect(coreEventsMock.emitFeedback).toHaveBeenCalledWith( + 'info', + 'MCP issues detected. Run /mcp list for status.', + ); + }); + + it('should capture last error for a server even when silenced', () => { + const manager = new McpClientManager('0.0.1', toolRegistry, mockConfig); + + manager.emitDiagnostic( + 'error', + 'Error in server (test-server)', + undefined, + 'test-server', + ); + + expect(manager.getLastError('test-server')).toBe( + 'Error in server (test-server)', + ); + }); + + it('should show previously deduplicated errors after interaction clears state', () => { + const manager = new McpClientManager('0.0.1', toolRegistry, mockConfig); + + manager.emitDiagnostic('error', 'Same error'); + expect(coreEventsMock.emitFeedback).toHaveBeenCalledTimes(1); // The hint + + manager.setUserInteractedWithMcp(); + manager.emitDiagnostic('error', 'Same error'); + expect(coreEventsMock.emitFeedback).toHaveBeenCalledTimes(2); // Now the actual error + }); + }); }); diff --git a/packages/core/src/tools/mcp-client-manager.ts b/packages/core/src/tools/mcp-client-manager.ts index 38c391c352..96d7abf55c 100644 --- a/packages/core/src/tools/mcp-client-manager.ts +++ b/packages/core/src/tools/mcp-client-manager.ts @@ -42,6 +42,28 @@ export class McpClientManager { extensionName: string; }> = []; + /** + * Track whether the user has explicitly interacted with MCP in this session + * (e.g. by running an /mcp command). + */ + private userInteractedWithMcp: boolean = false; + + /** + * Track which MCP diagnostics have already been shown to the user this session + * and at what verbosity level. + */ + private shownDiagnostics: Map = new Map(); + + /** + * Track whether the MCP "hint" has been shown. + */ + private hintShown: boolean = false; + + /** + * Track the last error message for each server. + */ + private lastErrors: Map = new Map(); + constructor( clientVersion: string, toolRegistry: ToolRegistry, @@ -54,6 +76,69 @@ export class McpClientManager { this.eventEmitter = eventEmitter; } + setUserInteractedWithMcp() { + this.userInteractedWithMcp = true; + } + + getLastError(serverName: string): string | undefined { + return this.lastErrors.get(serverName); + } + + /** + * Emit an MCP diagnostic message, adhering to the user's intent and + * deduplication rules. + */ + emitDiagnostic( + severity: 'info' | 'warning' | 'error', + message: string, + error?: unknown, + serverName?: string, + ) { + // Capture error for later display if it's an error/warning + if (severity === 'error' || severity === 'warning') { + if (serverName) { + this.lastErrors.set(serverName, message); + } + } + + // Deduplicate + const diagnosticKey = `${severity}:${message}`; + const previousStatus = this.shownDiagnostics.get(diagnosticKey); + + // If user has interacted, show verbosely unless already shown verbosely + if (this.userInteractedWithMcp) { + if (previousStatus === 'verbose') { + debugLogger.debug( + `Deduplicated verbose MCP diagnostic: ${diagnosticKey}`, + ); + return; + } + this.shownDiagnostics.set(diagnosticKey, 'verbose'); + coreEvents.emitFeedback(severity, message, error); + return; + } + + // In silent mode, if it has been shown at all, skip + if (previousStatus) { + debugLogger.debug(`Deduplicated silent MCP diagnostic: ${diagnosticKey}`); + return; + } + this.shownDiagnostics.set(diagnosticKey, 'silent'); + + // Otherwise, be less annoying + debugLogger.log(`[MCP ${severity}] ${message}`, error); + + if (severity === 'error' || severity === 'warning') { + if (!this.hintShown) { + this.hintShown = true; + coreEvents.emitFeedback( + 'info', + 'MCP issues detected. Run /mcp list for status.', + ); + } + } + } + getBlockedMcpServers() { return this.blockedMcpServers; } @@ -253,7 +338,7 @@ export class McpClientManager { if (!isAuthenticationError(error)) { // Log the error but don't let a single failed server stop the others const errorMessage = getErrorMessage(error); - coreEvents.emitFeedback( + this.emitDiagnostic( 'error', `Error during discovery for MCP server '${name}': ${errorMessage}`, error, @@ -262,7 +347,7 @@ export class McpClientManager { } } catch (error) { const errorMessage = getErrorMessage(error); - coreEvents.emitFeedback( + this.emitDiagnostic( 'error', `Error initializing MCP server '${name}': ${errorMessage}`, error, @@ -391,7 +476,7 @@ export class McpClientManager { try { await client.disconnect(); } catch (error) { - coreEvents.emitFeedback( + this.emitDiagnostic( 'error', `Error stopping client '${name}':`, error, diff --git a/packages/core/src/tools/mcp-client.test.ts b/packages/core/src/tools/mcp-client.test.ts index df4c869c34..126fb7ce68 100644 --- a/packages/core/src/tools/mcp-client.test.ts +++ b/packages/core/src/tools/mcp-client.test.ts @@ -33,6 +33,7 @@ import { isEnabled, McpClient, populateMcpServerCommand, + type McpContext, } from './mcp-client.js'; import type { ToolRegistry } from './tool-registry.js'; import type { ResourceRegistry } from '../resources/resource-registry.js'; @@ -42,12 +43,28 @@ import * as path from 'node:path'; import { coreEvents } from '../utils/events.js'; import type { EnvironmentSanitizationConfig } from '../services/environmentSanitization.js'; +interface TestableTransport { + _authProvider?: GoogleCredentialProvider; + _requestInit?: { + headers?: Record; + }; +} + const EMPTY_CONFIG: EnvironmentSanitizationConfig = { enableEnvironmentVariableRedaction: true, allowedEnvironmentVariables: [], blockedEnvironmentVariables: [], }; +const MOCK_CONTEXT_DEFAULT = { + sanitizationConfig: EMPTY_CONFIG, + emitMcpDiagnostic: vi.fn(), + setUserInteractedWithMcp: vi.fn(), + isTrustedFolder: vi.fn().mockReturnValue(true), +}; + +let MOCK_CONTEXT: McpContext = MOCK_CONTEXT_DEFAULT; + vi.mock('@modelcontextprotocol/sdk/client/stdio.js'); vi.mock('@modelcontextprotocol/sdk/client/index.js'); vi.mock('@google/genai'); @@ -69,6 +86,12 @@ describe('mcp-client', () => { let testWorkspace: string; beforeEach(() => { + MOCK_CONTEXT = { + sanitizationConfig: EMPTY_CONFIG, + emitMcpDiagnostic: vi.fn(), + setUserInteractedWithMcp: vi.fn(), + isTrustedFolder: vi.fn().mockReturnValue(true), + }; // create a tmp dir for this test // Create a unique temporary directory for the workspace to avoid conflicts testWorkspace = fs.mkdtempSync( @@ -136,12 +159,12 @@ describe('mcp-client', () => { promptRegistry, resourceRegistry, workspaceContext, - { sanitizationConfig: EMPTY_CONFIG } as Config, + MOCK_CONTEXT, false, '0.0.1', ); await client.connect(); - await client.discover({} as Config); + await client.discover(MOCK_CONTEXT); expect(mockedClient.listTools).toHaveBeenCalledWith( {}, expect.objectContaining({ timeout: 600000, progressReporter: client }), @@ -217,12 +240,12 @@ describe('mcp-client', () => { promptRegistry, resourceRegistry, workspaceContext, - { sanitizationConfig: EMPTY_CONFIG } as Config, + MOCK_CONTEXT, false, '0.0.1', ); await client.connect(); - await client.discover({} as Config); + await client.discover(MOCK_CONTEXT); expect(mockedToolRegistry.registerTool).toHaveBeenCalledTimes(2); expect(consoleWarnSpy).not.toHaveBeenCalled(); consoleWarnSpy.mockRestore(); @@ -269,16 +292,17 @@ describe('mcp-client', () => { promptRegistry, resourceRegistry, workspaceContext, - { sanitizationConfig: EMPTY_CONFIG } as Config, + MOCK_CONTEXT, false, '0.0.1', ); await client.connect(); - await expect(client.discover({} as Config)).rejects.toThrow('Test error'); - expect(coreEvents.emitFeedback).toHaveBeenCalledWith( + await expect(client.discover(MOCK_CONTEXT)).rejects.toThrow('Test error'); + expect(MOCK_CONTEXT.emitMcpDiagnostic).toHaveBeenCalledWith( 'error', `Error discovering prompts from test-server: Test error`, expect.any(Error), + 'test-server', ); }); @@ -323,12 +347,12 @@ describe('mcp-client', () => { promptRegistry, resourceRegistry, workspaceContext, - { sanitizationConfig: EMPTY_CONFIG } as Config, + MOCK_CONTEXT, false, '0.0.1', ); await client.connect(); - await expect(client.discover({} as Config)).rejects.toThrow( + await expect(client.discover(MOCK_CONTEXT)).rejects.toThrow( 'No prompts, tools, or resources found on the server.', ); }); @@ -383,12 +407,12 @@ describe('mcp-client', () => { promptRegistry, resourceRegistry, workspaceContext, - { sanitizationConfig: EMPTY_CONFIG } as Config, + MOCK_CONTEXT, false, '0.0.1', ); await client.connect(); - await client.discover({} as Config); + await client.discover(MOCK_CONTEXT); expect(mockedToolRegistry.registerTool).toHaveBeenCalledOnce(); }); @@ -451,7 +475,7 @@ describe('mcp-client', () => { promptRegistry, resourceRegistry, workspaceContext, - { sanitizationConfig: EMPTY_CONFIG } as Config, + MOCK_CONTEXT, false, '0.0.1', ); @@ -532,7 +556,7 @@ describe('mcp-client', () => { promptRegistry, resourceRegistry, workspaceContext, - { sanitizationConfig: EMPTY_CONFIG } as Config, + MOCK_CONTEXT, false, '0.0.1', ); @@ -610,7 +634,7 @@ describe('mcp-client', () => { promptRegistry, resourceRegistry, workspaceContext, - { sanitizationConfig: EMPTY_CONFIG } as Config, + MOCK_CONTEXT, false, '0.0.1', ); @@ -696,12 +720,12 @@ describe('mcp-client', () => { promptRegistry, resourceRegistry, workspaceContext, - { sanitizationConfig: EMPTY_CONFIG } as Config, + MOCK_CONTEXT, false, '0.0.1', ); await client.connect(); - await client.discover({} as Config); + await client.discover(MOCK_CONTEXT); expect(mockedToolRegistry.registerTool).toHaveBeenCalledOnce(); const registeredTool = vi.mocked(mockedToolRegistry.registerTool).mock .calls[0][0]; @@ -773,12 +797,12 @@ describe('mcp-client', () => { promptRegistry, resourceRegistry, workspaceContext, - { sanitizationConfig: EMPTY_CONFIG } as Config, + MOCK_CONTEXT, false, '0.0.1', ); await client.connect(); - await client.discover({} as Config); + await client.discover(MOCK_CONTEXT); expect(resourceRegistry.setResourcesForServer).toHaveBeenCalledWith( 'test-server', [ @@ -859,12 +883,12 @@ describe('mcp-client', () => { promptRegistry, resourceRegistry, workspaceContext, - { sanitizationConfig: EMPTY_CONFIG } as Config, + MOCK_CONTEXT, false, '0.0.1', ); await client.connect(); - await client.discover({} as Config); + await client.discover(MOCK_CONTEXT); expect(mockedClient.setNotificationHandler).toHaveBeenCalledTimes(2); expect(resourceListHandler).toBeDefined(); @@ -878,9 +902,11 @@ describe('mcp-client', () => { [expect.objectContaining({ uri: 'file:///tmp/two.txt' })], ); - expect(coreEvents.emitFeedback).toHaveBeenCalledWith( + expect(MOCK_CONTEXT.emitMcpDiagnostic).toHaveBeenCalledWith( 'info', 'Resources updated for server: test-server', + undefined, + 'test-server', ); }); @@ -943,12 +969,12 @@ describe('mcp-client', () => { promptRegistry, resourceRegistry, workspaceContext, - { sanitizationConfig: EMPTY_CONFIG } as Config, + MOCK_CONTEXT, false, '0.0.1', ); await client.connect(); - await client.discover({ sanitizationConfig: EMPTY_CONFIG } as Config); + await client.discover(MOCK_CONTEXT); expect(mockedClient.setNotificationHandler).toHaveBeenCalledTimes(2); expect(promptListHandler).toBeDefined(); @@ -963,9 +989,11 @@ describe('mcp-client', () => { expect(promptRegistry.registerPrompt).toHaveBeenLastCalledWith( expect.objectContaining({ name: 'two' }), ); - expect(coreEvents.emitFeedback).toHaveBeenCalledWith( + expect(MOCK_CONTEXT.emitMcpDiagnostic).toHaveBeenCalledWith( 'info', 'Prompts updated for server: test-server', + undefined, + 'test-server', ); }); @@ -1025,12 +1053,12 @@ describe('mcp-client', () => { mockedPromptRegistry, resourceRegistry, workspaceContext, - { sanitizationConfig: EMPTY_CONFIG } as Config, + MOCK_CONTEXT, false, '0.0.1', ); await client.connect(); - await client.discover({} as Config); + await client.discover(MOCK_CONTEXT); expect(mockedToolRegistry.registerTool).toHaveBeenCalledOnce(); expect(mockedPromptRegistry.registerPrompt).toHaveBeenCalledOnce(); @@ -1075,7 +1103,7 @@ describe('mcp-client', () => { {} as PromptRegistry, {} as ResourceRegistry, workspaceContext, - { sanitizationConfig: EMPTY_CONFIG } as Config, + MOCK_CONTEXT, false, '0.0.1', ); @@ -1112,7 +1140,7 @@ describe('mcp-client', () => { {} as PromptRegistry, {} as ResourceRegistry, workspaceContext, - { sanitizationConfig: EMPTY_CONFIG } as Config, + MOCK_CONTEXT, false, '0.0.1', ); @@ -1168,7 +1196,7 @@ describe('mcp-client', () => { {} as PromptRegistry, {} as ResourceRegistry, workspaceContext, - { sanitizationConfig: EMPTY_CONFIG } as Config, + MOCK_CONTEXT, false, '0.0.1', onToolsUpdatedSpy, @@ -1200,9 +1228,11 @@ describe('mcp-client', () => { expect(onToolsUpdatedSpy).toHaveBeenCalled(); // It should emit feedback event - expect(coreEvents.emitFeedback).toHaveBeenCalledWith( + expect(MOCK_CONTEXT.emitMcpDiagnostic).toHaveBeenCalledWith( 'info', 'Tools updated for server: test-server', + undefined, + 'test-server', ); }); @@ -1239,7 +1269,7 @@ describe('mcp-client', () => { {} as PromptRegistry, {} as ResourceRegistry, workspaceContext, - { sanitizationConfig: EMPTY_CONFIG } as Config, + MOCK_CONTEXT, false, '0.0.1', ); @@ -1310,7 +1340,7 @@ describe('mcp-client', () => { {} as PromptRegistry, {} as ResourceRegistry, workspaceContext, - { sanitizationConfig: EMPTY_CONFIG } as Config, + MOCK_CONTEXT, false, '0.0.1', onToolsUpdatedSpy, @@ -1323,7 +1353,7 @@ describe('mcp-client', () => { {} as PromptRegistry, {} as ResourceRegistry, workspaceContext, - { sanitizationConfig: EMPTY_CONFIG } as Config, + MOCK_CONTEXT, false, '0.0.1', onToolsUpdatedSpy, @@ -1409,7 +1439,7 @@ describe('mcp-client', () => { {} as PromptRegistry, {} as ResourceRegistry, workspaceContext, - { sanitizationConfig: EMPTY_CONFIG } as Config, + MOCK_CONTEXT, false, '0.0.1', ); @@ -1474,7 +1504,7 @@ describe('mcp-client', () => { {} as PromptRegistry, {} as ResourceRegistry, workspaceContext, - { sanitizationConfig: EMPTY_CONFIG } as Config, + MOCK_CONTEXT, false, '0.0.1', onToolsUpdatedSpy, @@ -1526,7 +1556,7 @@ describe('mcp-client', () => { httpUrl: 'http://test-server', }, false, - EMPTY_CONFIG, + MOCK_CONTEXT, ); expect(transport).toBeInstanceOf(StreamableHTTPClientTransport); @@ -1544,7 +1574,7 @@ describe('mcp-client', () => { headers: { Authorization: 'derp' }, }, false, - EMPTY_CONFIG, + MOCK_CONTEXT, ); expect(transport).toBeInstanceOf(StreamableHTTPClientTransport); @@ -1565,7 +1595,7 @@ describe('mcp-client', () => { url: 'http://test-server', }, false, - EMPTY_CONFIG, + MOCK_CONTEXT, ); expect(transport).toBeInstanceOf(StreamableHTTPClientTransport); expect(transport).toMatchObject({ @@ -1582,7 +1612,7 @@ describe('mcp-client', () => { headers: { Authorization: 'derp' }, }, false, - EMPTY_CONFIG, + MOCK_CONTEXT, ); expect(transport).toBeInstanceOf(StreamableHTTPClientTransport); @@ -1602,7 +1632,7 @@ describe('mcp-client', () => { type: 'http', }, false, - EMPTY_CONFIG, + MOCK_CONTEXT, ); expect(transport).toBeInstanceOf(StreamableHTTPClientTransport); @@ -1620,7 +1650,7 @@ describe('mcp-client', () => { type: 'sse', }, false, - EMPTY_CONFIG, + MOCK_CONTEXT, ); expect(transport).toBeInstanceOf(SSEClientTransport); @@ -1637,7 +1667,7 @@ describe('mcp-client', () => { url: 'http://test-server', }, false, - EMPTY_CONFIG, + MOCK_CONTEXT, ); expect(transport).toBeInstanceOf(StreamableHTTPClientTransport); @@ -1656,7 +1686,7 @@ describe('mcp-client', () => { headers: { Authorization: 'Bearer token' }, }, false, - EMPTY_CONFIG, + MOCK_CONTEXT, ); expect(transport).toBeInstanceOf(StreamableHTTPClientTransport); @@ -1677,7 +1707,7 @@ describe('mcp-client', () => { headers: { 'X-API-Key': 'key123' }, }, false, - EMPTY_CONFIG, + MOCK_CONTEXT, ); expect(transport).toBeInstanceOf(SSEClientTransport); @@ -1697,7 +1727,7 @@ describe('mcp-client', () => { url: 'http://test-server-url', }, false, - EMPTY_CONFIG, + MOCK_CONTEXT, ); // httpUrl should take priority and create HTTP transport @@ -1723,7 +1753,7 @@ describe('mcp-client', () => { cwd: 'test/cwd', }, false, - EMPTY_CONFIG, + MOCK_CONTEXT, ); expect(mockedTransport).toHaveBeenCalledWith({ @@ -1749,7 +1779,7 @@ describe('mcp-client', () => { cwd: 'test/cwd', }, false, - EMPTY_CONFIG, + MOCK_CONTEXT, ); const callArgs = mockedTransport.mock.calls[0][0]; @@ -1784,7 +1814,7 @@ describe('mcp-client', () => { }, }, false, - EMPTY_CONFIG, + MOCK_CONTEXT, ); const callArgs = mockedTransport.mock.calls[0][0]; @@ -1819,7 +1849,7 @@ describe('mcp-client', () => { }, }, false, - EMPTY_CONFIG, + MOCK_CONTEXT, ); const callArgs = mockedTransport.mock.calls[0][0]; @@ -1857,7 +1887,7 @@ describe('mcp-client', () => { }, }, false, - EMPTY_CONFIG, + MOCK_CONTEXT, ); const callArgs = mockedTransport.mock.calls[0][0]; @@ -1888,7 +1918,7 @@ describe('mcp-client', () => { }, }, false, - EMPTY_CONFIG, + MOCK_CONTEXT, ); const callArgs = mockedTransport.mock.calls[0][0]; @@ -1925,17 +1955,15 @@ describe('mcp-client', () => { }, }, false, - EMPTY_CONFIG, + MOCK_CONTEXT, ); expect(transport).toBeInstanceOf(StreamableHTTPClientTransport); - // eslint-disable-next-line @typescript-eslint/no-explicit-any - const authProvider = (transport as any)._authProvider; + const testableTransport = transport as unknown as TestableTransport; + const authProvider = testableTransport._authProvider; expect(authProvider).toBeInstanceOf(GoogleCredentialProvider); - // eslint-disable-next-line @typescript-eslint/no-explicit-any - const googUserProject = (transport as any)._requestInit?.headers?.[ - 'X-Goog-User-Project' - ]; + const googUserProject = + testableTransport._requestInit?.headers?.['X-Goog-User-Project']; expect(googUserProject).toBe('myproject'); }); @@ -1958,14 +1986,14 @@ describe('mcp-client', () => { }, }, false, - EMPTY_CONFIG, + MOCK_CONTEXT, ); expect(transport).toBeInstanceOf(StreamableHTTPClientTransport); expect(mockGetRequestHeaders).toHaveBeenCalled(); - // eslint-disable-next-line @typescript-eslint/no-explicit-any - const headers = (transport as any)._requestInit?.headers; - expect(headers['X-Goog-User-Project']).toBe('provider-project'); + const testableTransport = transport as unknown as TestableTransport; + const headers = testableTransport._requestInit?.headers; + expect(headers?.['X-Goog-User-Project']).toBe('provider-project'); }); it('should prioritize provider headers over config headers', async () => { @@ -1990,13 +2018,13 @@ describe('mcp-client', () => { }, }, false, - EMPTY_CONFIG, + MOCK_CONTEXT, ); expect(transport).toBeInstanceOf(StreamableHTTPClientTransport); - // eslint-disable-next-line @typescript-eslint/no-explicit-any - const headers = (transport as any)._requestInit?.headers; - expect(headers['X-Goog-User-Project']).toBe('provider-project'); + const testableTransport = transport as unknown as TestableTransport; + const headers = testableTransport._requestInit?.headers; + expect(headers?.['X-Goog-User-Project']).toBe('provider-project'); }); it('should use GoogleCredentialProvider with SSE transport', async () => { @@ -2011,12 +2039,12 @@ describe('mcp-client', () => { }, }, false, - EMPTY_CONFIG, + MOCK_CONTEXT, ); expect(transport).toBeInstanceOf(SSEClientTransport); - // eslint-disable-next-line @typescript-eslint/no-explicit-any - const authProvider = (transport as any)._authProvider; + const testableTransport = transport as unknown as TestableTransport; + const authProvider = testableTransport._authProvider; expect(authProvider).toBeInstanceOf(GoogleCredentialProvider); }); @@ -2031,7 +2059,7 @@ describe('mcp-client', () => { }, }, false, - EMPTY_CONFIG, + MOCK_CONTEXT, ), ).rejects.toThrow( 'URL must be provided in the config for Google Credentials provider', @@ -2178,12 +2206,11 @@ describe('connectToMcpServer with OAuth', () => { scopes: ['test-scope'], }); - // We need this to be an any type because we dig into its private state. - // eslint-disable-next-line @typescript-eslint/no-explicit-any - let capturedTransport: any; + // We need this to be typed to dig into its private state. + let capturedTransport: TestableTransport | undefined; vi.mocked(mockedClient.connect).mockImplementationOnce( async (transport) => { - capturedTransport = transport; + capturedTransport = transport as unknown as TestableTransport; return Promise.resolve(); }, ); @@ -2194,15 +2221,15 @@ describe('connectToMcpServer with OAuth', () => { { httpUrl: serverUrl, oauth: { enabled: true } }, false, workspaceContext, - EMPTY_CONFIG, + MOCK_CONTEXT, ); expect(client).toBe(mockedClient); expect(mockedClient.connect).toHaveBeenCalledTimes(2); expect(mockAuthProvider.authenticate).toHaveBeenCalledOnce(); - const authHeader = - capturedTransport._requestInit?.headers?.['Authorization']; + const authHeader = (capturedTransport as TestableTransport)._requestInit + ?.headers?.['Authorization']; expect(authHeader).toBe('Bearer test-access-token'); }); @@ -2224,12 +2251,11 @@ describe('connectToMcpServer with OAuth', () => { 'test-access-token-from-discovery', ); - // We need this to be an any type because we dig into its private state. - // eslint-disable-next-line @typescript-eslint/no-explicit-any - let capturedTransport: any; + // We need this to be typed to dig into its private state. + let capturedTransport: TestableTransport | undefined; vi.mocked(mockedClient.connect).mockImplementationOnce( async (transport) => { - capturedTransport = transport; + capturedTransport = transport as unknown as TestableTransport; return Promise.resolve(); }, ); @@ -2240,7 +2266,7 @@ describe('connectToMcpServer with OAuth', () => { { httpUrl: serverUrl, oauth: { enabled: true } }, false, workspaceContext, - EMPTY_CONFIG, + MOCK_CONTEXT, ); expect(client).toBe(mockedClient); @@ -2248,8 +2274,8 @@ describe('connectToMcpServer with OAuth', () => { expect(mockAuthProvider.authenticate).toHaveBeenCalledOnce(); expect(OAuthUtils.discoverOAuthConfig).toHaveBeenCalledWith(serverUrl); - const authHeader = - capturedTransport._requestInit?.headers?.['Authorization']; + const authHeader = (capturedTransport as TestableTransport)._requestInit + ?.headers?.['Authorization']; expect(authHeader).toBe('Bearer test-access-token-from-discovery'); }); @@ -2280,7 +2306,7 @@ describe('connectToMcpServer with OAuth', () => { { httpUrl: serverUrl, oauth: { enabled: true } }, false, workspaceContext, - EMPTY_CONFIG, + MOCK_CONTEXT, ); expect(client).toBe(mockedClient); @@ -2324,7 +2350,7 @@ describe('connectToMcpServer with OAuth', () => { { httpUrl: serverUrl, oauth: { enabled: true } }, false, workspaceContext, - EMPTY_CONFIG, + MOCK_CONTEXT, ); expect(client).toBe(mockedClient); @@ -2380,7 +2406,7 @@ describe('connectToMcpServer - HTTP→SSE fallback', () => { { url: 'http://test-server', type: 'http' }, false, workspaceContext, - EMPTY_CONFIG, + MOCK_CONTEXT, ), ).rejects.toThrow('Connection failed'); @@ -2400,7 +2426,7 @@ describe('connectToMcpServer - HTTP→SSE fallback', () => { { url: 'http://test-server', type: 'sse' }, false, workspaceContext, - EMPTY_CONFIG, + MOCK_CONTEXT, ), ).rejects.toThrow('Connection failed'); @@ -2419,7 +2445,7 @@ describe('connectToMcpServer - HTTP→SSE fallback', () => { { url: 'http://test-server' }, false, workspaceContext, - EMPTY_CONFIG, + MOCK_CONTEXT, ); expect(client).toBe(mockedClient); @@ -2442,7 +2468,7 @@ describe('connectToMcpServer - HTTP→SSE fallback', () => { { url: 'http://test-server' }, false, workspaceContext, - EMPTY_CONFIG, + MOCK_CONTEXT, ), ).rejects.toThrow('Server error'); @@ -2460,7 +2486,7 @@ describe('connectToMcpServer - HTTP→SSE fallback', () => { { url: 'http://test-server' }, false, workspaceContext, - EMPTY_CONFIG, + MOCK_CONTEXT, ); expect(client).toBe(mockedClient); @@ -2545,7 +2571,7 @@ describe('connectToMcpServer - OAuth with transport fallback', () => { { url: 'http://test-server', oauth: { enabled: true } }, false, workspaceContext, - EMPTY_CONFIG, + MOCK_CONTEXT, ); expect(client).toBe(mockedClient); diff --git a/packages/core/src/tools/mcp-client.ts b/packages/core/src/tools/mcp-client.ts index c393273dbf..18c2029d9e 100644 --- a/packages/core/src/tools/mcp-client.ts +++ b/packages/core/src/tools/mcp-client.ts @@ -150,7 +150,7 @@ export class McpClient implements McpProgressReporter { private readonly promptRegistry: PromptRegistry, private readonly resourceRegistry: ResourceRegistry, private readonly workspaceContext: WorkspaceContext, - private readonly cliConfig: Config, + private readonly cliConfig: McpContext, private readonly debugMode: boolean, private readonly clientVersion: string, private readonly onToolsUpdated?: (signal?: AbortSignal) => Promise, @@ -173,7 +173,7 @@ export class McpClient implements McpProgressReporter { this.serverConfig, this.debugMode, this.workspaceContext, - this.cliConfig.sanitizationConfig, + this.cliConfig, ); this.registerNotificationHandlers(); @@ -184,10 +184,11 @@ export class McpClient implements McpProgressReporter { return; } if (originalOnError) originalOnError(error); - coreEvents.emitFeedback( + this.cliConfig.emitMcpDiagnostic( 'error', `MCP ERROR (${this.serverName})`, error, + this.serverName, ); this.updateStatus(MCPServerStatus.DISCONNECTED); }; @@ -201,7 +202,7 @@ export class McpClient implements McpProgressReporter { /** * Discovers tools and prompts from the MCP server. */ - async discover(cliConfig: Config): Promise { + async discover(cliConfig: McpContext): Promise { this.assertConnected(); const prompts = await this.fetchPrompts(); @@ -265,7 +266,7 @@ export class McpClient implements McpProgressReporter { } private async discoverTools( - cliConfig: Config, + cliConfig: McpContext, options?: { timeout?: number; signal?: AbortSignal }, ): Promise { this.assertConnected(); @@ -288,12 +289,17 @@ export class McpClient implements McpProgressReporter { signal?: AbortSignal; }): Promise { this.assertConnected(); - return discoverPrompts(this.serverName, this.client!, options); + return discoverPrompts( + this.serverName, + this.client!, + this.cliConfig, + options, + ); } private async discoverResources(): Promise { this.assertConnected(); - return discoverResources(this.serverName, this.client!); + return discoverResources(this.serverName, this.client!, this.cliConfig); } private updateResourceRegistry(resources: Resource[]): void { @@ -437,9 +443,11 @@ export class McpClient implements McpProgressReporter { clearTimeout(timeoutId); - coreEvents.emitFeedback( + this.cliConfig.emitMcpDiagnostic( 'info', `Resources updated for server: ${this.serverName}`, + undefined, + this.serverName, ); } while (this.pendingResourceRefresh); } catch (error) { @@ -508,9 +516,11 @@ export class McpClient implements McpProgressReporter { clearTimeout(timeoutId); - coreEvents.emitFeedback( + this.cliConfig.emitMcpDiagnostic( 'info', `Prompts updated for server: ${this.serverName}`, + undefined, + this.serverName, ); } while (this.pendingPromptRefresh); } catch (error) { @@ -585,9 +595,11 @@ export class McpClient implements McpProgressReporter { clearTimeout(timeoutId); - coreEvents.emitFeedback( + this.cliConfig.emitMcpDiagnostic( 'info', `Tools updated for server: ${this.serverName}`, + undefined, + this.serverName, ); } while (this.pendingToolRefresh); } catch (error) { @@ -719,6 +731,7 @@ async function handleAutomaticOAuth( mcpServerName: string, mcpServerConfig: MCPServerConfig, wwwAuthenticate: string, + cliConfig: McpContext, ): Promise { try { debugLogger.log(`🔐 '${mcpServerName}' requires OAuth authentication`); @@ -738,9 +751,11 @@ async function handleAutomaticOAuth( } if (!oauthConfig) { - coreEvents.emitFeedback( + cliConfig.emitMcpDiagnostic( 'error', `Could not configure OAuth for '${mcpServerName}' - please authenticate manually with /mcp auth ${mcpServerName}`, + undefined, + mcpServerName, ); return false; } @@ -768,10 +783,11 @@ async function handleAutomaticOAuth( ); return true; } catch (error) { - coreEvents.emitFeedback( + cliConfig.emitMcpDiagnostic( 'error', `Failed to handle automatic OAuth for server '${mcpServerName}': ${getErrorMessage(error)}`, error, + mcpServerName, ); return false; } @@ -840,14 +856,14 @@ function createAuthProvider( * @param mcpServerName The name of the MCP server * @param mcpServerConfig The MCP server configuration * @param accessToken The OAuth access token - * @param sanitizationConfig Configuration for environment sanitization + * @param cliConfig The CLI configuration providing sanitization and diagnostic reporting * @returns The transport with OAuth token, or null if creation fails */ async function createTransportWithOAuth( mcpServerName: string, mcpServerConfig: MCPServerConfig, accessToken: string, - sanitizationConfig: EnvironmentSanitizationConfig, + cliConfig: McpContext, ): Promise { try { const headers: Record = { @@ -859,16 +875,17 @@ async function createTransportWithOAuth( requestInit: createTransportRequestInit( mcpServerConfig, headers, - sanitizationConfig, + cliConfig.sanitizationConfig, ), }; return createUrlTransport(mcpServerName, mcpServerConfig, transportOptions); } catch (error) { - coreEvents.emitFeedback( + cliConfig.emitMcpDiagnostic( 'error', `Failed to create OAuth transport for server '${mcpServerName}': ${getErrorMessage(error)}`, error, + mcpServerName, ); return null; } @@ -990,7 +1007,7 @@ export async function connectAndDiscover( promptRegistry: PromptRegistry, debugMode: boolean, workspaceContext: WorkspaceContext, - cliConfig: Config, + cliConfig: McpContext, ): Promise { updateMCPServerStatus(mcpServerName, MCPServerStatus.CONNECTING); @@ -1002,16 +1019,21 @@ export async function connectAndDiscover( mcpServerConfig, debugMode, workspaceContext, - cliConfig.sanitizationConfig, + cliConfig, ); mcpClient.onerror = (error) => { - coreEvents.emitFeedback('error', `MCP ERROR (${mcpServerName}):`, error); + cliConfig.emitMcpDiagnostic( + 'error', + `MCP ERROR (${mcpServerName}):`, + error, + mcpServerName, + ); updateMCPServerStatus(mcpServerName, MCPServerStatus.DISCONNECTED); }; // Attempt to discover both prompts and tools - const prompts = await discoverPrompts(mcpServerName, mcpClient); + const prompts = await discoverPrompts(mcpServerName, mcpClient, cliConfig); const tools = await discoverTools( mcpServerName, mcpServerConfig, @@ -1042,12 +1064,13 @@ export async function connectAndDiscover( // eslint-disable-next-line @typescript-eslint/no-floating-promises mcpClient.close(); } - coreEvents.emitFeedback( + cliConfig.emitMcpDiagnostic( 'error', `Error connecting to MCP server '${mcpServerName}': ${getErrorMessage( error, )}`, error, + mcpServerName, ); updateMCPServerStatus(mcpServerName, MCPServerStatus.DISCONNECTED); } @@ -1070,7 +1093,7 @@ export async function discoverTools( mcpServerName: string, mcpServerConfig: MCPServerConfig, mcpClient: Client, - cliConfig: Config, + cliConfig: McpContext, messageBus: MessageBus, options?: { timeout?: number; @@ -1119,13 +1142,14 @@ export async function discoverTools( discoveredTools.push(tool); } catch (error) { - coreEvents.emitFeedback( + cliConfig.emitMcpDiagnostic( 'error', `Error discovering tool: '${ toolDef.name // eslint-disable-next-line @typescript-eslint/no-unsafe-type-assertion }' from MCP server '${mcpServerName}': ${(error as Error).message}`, error, + mcpServerName, ); } } @@ -1135,12 +1159,13 @@ export async function discoverTools( error instanceof Error && !error.message?.includes('Method not found') ) { - coreEvents.emitFeedback( + cliConfig.emitMcpDiagnostic( 'error', `Error discovering tools from ${mcpServerName}: ${getErrorMessage( error, )}`, error, + mcpServerName, ); } return []; @@ -1236,6 +1261,7 @@ class McpCallableTool implements CallableTool { export async function discoverPrompts( mcpServerName: string, mcpClient: Client, + cliConfig: McpContext, options?: { signal?: AbortSignal }, ): Promise { // Only request prompts if the server supports them. @@ -1247,19 +1273,26 @@ export async function discoverPrompts( ...prompt, serverName: mcpServerName, invoke: (params: Record) => - invokeMcpPrompt(mcpServerName, mcpClient, prompt.name, params), + invokeMcpPrompt( + mcpServerName, + mcpClient, + prompt.name, + params, + cliConfig, + ), })); } catch (error) { // It's okay if the method is not found, which is a common case. if (error instanceof Error && error.message?.includes('Method not found')) { return []; } - coreEvents.emitFeedback( + cliConfig.emitMcpDiagnostic( 'error', `Error discovering prompts from ${mcpServerName}: ${getErrorMessage( error, )}`, error, + mcpServerName, ); throw error; } @@ -1268,18 +1301,20 @@ export async function discoverPrompts( export async function discoverResources( mcpServerName: string, mcpClient: Client, + cliConfig: McpContext, ): Promise { if (mcpClient.getServerCapabilities()?.resources == null) { return []; } - const resources = await listResources(mcpServerName, mcpClient); + const resources = await listResources(mcpServerName, mcpClient, cliConfig); return resources; } async function listResources( mcpServerName: string, mcpClient: Client, + cliConfig: McpContext, ): Promise { const resources: Resource[] = []; let cursor: string | undefined; @@ -1299,12 +1334,13 @@ async function listResources( if (error instanceof Error && error.message?.includes('Method not found')) { return []; } - coreEvents.emitFeedback( + cliConfig.emitMcpDiagnostic( 'error', `Error discovering resources from ${mcpServerName}: ${getErrorMessage( error, )}`, error, + mcpServerName, ); throw error; } @@ -1325,7 +1361,9 @@ export async function invokeMcpPrompt( mcpClient: Client, promptName: string, promptParams: Record, + cliConfig: McpContext, ): Promise { + cliConfig.setUserInteractedWithMcp?.(); try { const sanitizedParams: Record = {}; for (const [key, value] of Object.entries(promptParams)) { @@ -1345,12 +1383,13 @@ export async function invokeMcpPrompt( error instanceof Error && !error.message?.includes('Method not found') ) { - coreEvents.emitFeedback( + cliConfig.emitMcpDiagnostic( 'error', `Error invoking prompt '${promptName}' from ${mcpServerName} ${promptParams}: ${getErrorMessage( error, )}`, error, + mcpServerName, ); } throw error; @@ -1435,14 +1474,17 @@ async function connectWithSSETransport( * @param serverName The name of the MCP server * @throws Always throws an error with authentication instructions */ -async function showAuthRequiredMessage(serverName: string): Promise { +async function showAuthRequiredMessage( + serverName: string, + cliConfig: McpContext, +): Promise { const hasRejectedToken = !!(await getStoredOAuthToken(serverName)); const message = hasRejectedToken ? `MCP server '${serverName}' rejected stored OAuth token. Please re-authenticate using: /mcp auth ${serverName}` : `MCP server '${serverName}' requires authentication using: /mcp auth ${serverName}`; - coreEvents.emitFeedback('info', message); + cliConfig.emitMcpDiagnostic('info', message, undefined, serverName); throw new UnauthorizedError(message); } @@ -1455,7 +1497,7 @@ async function showAuthRequiredMessage(serverName: string): Promise { * @param config The MCP server configuration * @param accessToken The OAuth access token to use * @param httpReturned404 Whether the HTTP transport returned 404 (indicating SSE-only server) - * @param sanitizationConfig Configuration for environment sanitization + * @param cliConfig The CLI configuration providing sanitization and diagnostic reporting */ async function retryWithOAuth( client: Client, @@ -1463,7 +1505,7 @@ async function retryWithOAuth( config: MCPServerConfig, accessToken: string, httpReturned404: boolean, - sanitizationConfig: EnvironmentSanitizationConfig, + cliConfig: McpContext, ): Promise { if (httpReturned404) { // HTTP returned 404, only try SSE @@ -1484,7 +1526,7 @@ async function retryWithOAuth( serverName, config, accessToken, - sanitizationConfig, + cliConfig, ); if (!httpTransport) { throw new Error( @@ -1520,6 +1562,23 @@ async function retryWithOAuth( } } +/** + * Interface for MCP operations that require configuration or diagnostic reporting. + * This is implemented by the central Config class and can be mocked for testing + * or used by the non-interactive CLI. + */ +export interface McpContext { + readonly sanitizationConfig: EnvironmentSanitizationConfig; + emitMcpDiagnostic( + severity: 'info' | 'warning' | 'error', + message: string, + error?: unknown, + serverName?: string, + ): void; + setUserInteractedWithMcp?(): void; + isTrustedFolder(): boolean; +} + /** * Creates and connects an MCP client to a server based on the provided configuration. * It determines the appropriate transport (Stdio, SSE, or Streamable HTTP) and @@ -1536,7 +1595,7 @@ export async function connectToMcpServer( mcpServerConfig: MCPServerConfig, debugMode: boolean, workspaceContext: WorkspaceContext, - sanitizationConfig: EnvironmentSanitizationConfig, + cliConfig: McpContext, ): Promise { const mcpClient = new Client( { @@ -1603,7 +1662,7 @@ export async function connectToMcpServer( mcpServerName, mcpServerConfig, debugMode, - sanitizationConfig, + cliConfig, ); try { await mcpClient.connect(transport, { @@ -1682,7 +1741,7 @@ export async function connectToMcpServer( const shouldTriggerOAuth = mcpServerConfig.oauth?.enabled; if (!shouldTriggerOAuth) { - await showAuthRequiredMessage(mcpServerName); + await showAuthRequiredMessage(mcpServerName, cliConfig); } // Try to extract www-authenticate header from the error @@ -1748,6 +1807,7 @@ export async function connectToMcpServer( mcpServerName, mcpServerConfig, wwwAuthenticate, + cliConfig, ); if (oauthSuccess) { // Retry connection with OAuth token @@ -1764,7 +1824,7 @@ export async function connectToMcpServer( mcpServerConfig, accessToken, httpReturned404, - sanitizationConfig, + cliConfig, ); return mcpClient; } else { @@ -1778,7 +1838,7 @@ export async function connectToMcpServer( const shouldTryDiscovery = mcpServerConfig.oauth?.enabled; if (!shouldTryDiscovery) { - await showAuthRequiredMessage(mcpServerName); + await showAuthRequiredMessage(mcpServerName, cliConfig); } // For SSE/HTTP servers, try to discover OAuth configuration from the base URL @@ -1837,7 +1897,7 @@ export async function connectToMcpServer( mcpServerName, mcpServerConfig, accessToken, - sanitizationConfig, + cliConfig, ); if (!oauthTransport) { throw new Error( @@ -1925,7 +1985,7 @@ export async function createTransport( mcpServerName: string, mcpServerConfig: MCPServerConfig, debugMode: boolean, - sanitizationConfig: EnvironmentSanitizationConfig, + cliConfig: McpContext, ): Promise { const noUrl = !mcpServerConfig.url && !mcpServerConfig.httpUrl; if (noUrl) { @@ -1963,9 +2023,11 @@ export async function createTransport( if (!accessToken) { // Emit info message (not error) since this is expected behavior - coreEvents.emitFeedback( + cliConfig.emitMcpDiagnostic( 'info', `MCP server '${mcpServerName}' requires authentication using: /mcp auth ${mcpServerName}`, + undefined, + mcpServerName, ); } } else { @@ -1988,7 +2050,7 @@ export async function createTransport( requestInit: createTransportRequestInit( mcpServerConfig, headers, - sanitizationConfig, + cliConfig.sanitizationConfig, ), authProvider, }; @@ -1997,12 +2059,17 @@ export async function createTransport( } if (mcpServerConfig.command) { + if (!cliConfig.isTrustedFolder()) { + throw new Error( + `MCP server '${mcpServerName}' uses stdio transport but current folder is not trusted. Use 'gemini trust' to enable it.`, + ); + } const extensionEnv = getExtensionEnvironment(mcpServerConfig.extension); const expansionEnv = { ...process.env, ...extensionEnv }; // 1. Sanitize the base process environment to prevent unintended leaks of system-wide secrets. const sanitizedEnv = sanitizeEnvironment(expansionEnv, { - ...sanitizationConfig, + ...cliConfig.sanitizationConfig, enableEnvironmentVariableRedaction: true, }); diff --git a/packages/core/src/tools/mcp-tool.ts b/packages/core/src/tools/mcp-tool.ts index 18c1638c95..3d492457f2 100644 --- a/packages/core/src/tools/mcp-tool.ts +++ b/packages/core/src/tools/mcp-tool.ts @@ -20,8 +20,8 @@ import { } from './tools.js'; import type { CallableTool, FunctionCall, Part } from '@google/genai'; import { ToolErrorType } from './tool-error.js'; -import type { Config } from '../config/config.js'; import type { MessageBus } from '../confirmation-bus/message-bus.js'; +import type { McpContext } from './mcp-client.js'; /** * The separator used to qualify MCP tool names with their server prefix. @@ -89,7 +89,7 @@ export class DiscoveredMCPToolInvocation extends BaseToolInvocation< messageBus: MessageBus, readonly trust?: boolean, params: ToolParams = {}, - private readonly cliConfig?: Config, + private readonly cliConfig?: McpContext, private readonly toolDescription?: string, private readonly toolParameterSchema?: unknown, toolAnnotationsData?: Record, @@ -184,6 +184,7 @@ export class DiscoveredMCPToolInvocation extends BaseToolInvocation< } async execute(signal: AbortSignal): Promise { + this.cliConfig?.setUserInteractedWithMcp?.(); const functionCalls: FunctionCall[] = [ { name: this.serverToolName, @@ -266,7 +267,7 @@ export class DiscoveredMCPTool extends BaseDeclarativeTool< readonly trust?: boolean, isReadOnly?: boolean, nameOverride?: string, - private readonly cliConfig?: Config, + private readonly cliConfig?: McpContext, override readonly extensionName?: string, override readonly extensionId?: string, private readonly _toolAnnotations?: Record,