From 42344c266646dc96766164e8ac364941519ab847 Mon Sep 17 00:00:00 2001 From: Taylor Mullen Date: Mon, 23 Mar 2026 20:18:27 -0700 Subject: [PATCH] feat(cli): support Open Plugins MCP servers - Implement discovery of .mcp.json at the plugin root - Support explicit mcpServers path or record in plugin.json - Add variable expansion for PLUGIN_ROOT in command, args, env, and cwd - Align MCP server naming with the pluginName:mcpServerName format - Ensure MCP tool names use standard mcp_ prefix for API compatibility - Enable settings and mcpServers for Open Plugins - Refactor MCP server resolution into dedicated method - Improve type safety in tests and configuration loading Fixes https://github.com/google-gemini/maintainers-gemini-cli/issues/1595 --- .../cli/src/config/extensions/variables.ts | 5 + .../cli/src/config/open-plugin-mcp.test.ts | 133 +++++++++++++++++ packages/cli/src/config/plugin.ts | 92 +++++++++++- packages/core/src/tools/mcp-client-manager.ts | 14 +- packages/core/src/tools/mcp-client.ts | 20 ++- .../src/tools/mcp-plugin-expansion.test.ts | 97 ++++++++++++ .../src/tools/mcp-plugin-namespacing.test.ts | 138 ++++++++++++++++++ .../core/src/tools/mcp-plugin-naming.test.ts | 60 ++++++++ 8 files changed, 548 insertions(+), 11 deletions(-) create mode 100644 packages/cli/src/config/open-plugin-mcp.test.ts create mode 100644 packages/core/src/tools/mcp-plugin-expansion.test.ts create mode 100644 packages/core/src/tools/mcp-plugin-namespacing.test.ts create mode 100644 packages/core/src/tools/mcp-plugin-naming.test.ts diff --git a/packages/cli/src/config/extensions/variables.ts b/packages/cli/src/config/extensions/variables.ts index 7b45611c03..2a120d4dab 100644 --- a/packages/cli/src/config/extensions/variables.ts +++ b/packages/cli/src/config/extensions/variables.ts @@ -25,6 +25,11 @@ export const HIDDEN_OPEN_PLUGIN_CONFIG_FILENAME = path.join( '.plugin', 'plugin.json', ); +export const OPEN_PLUGIN_MCP_CONFIG_FILENAME = '.mcp.json'; +export const HIDDEN_OPEN_PLUGIN_MCP_CONFIG_FILENAME = path.join( + '.plugin', + '.mcp.json', +); export const INSTALL_METADATA_FILENAME = '.gemini-extension-install.json'; export const EXTENSION_SETTINGS_FILENAME = '.env'; diff --git a/packages/cli/src/config/open-plugin-mcp.test.ts b/packages/cli/src/config/open-plugin-mcp.test.ts new file mode 100644 index 0000000000..a73a9afe7e --- /dev/null +++ b/packages/cli/src/config/open-plugin-mcp.test.ts @@ -0,0 +1,133 @@ +/** + * @license + * Copyright 2026 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +import { describe, it, expect, beforeEach, vi } from 'vitest'; +import * as fs from 'node:fs'; +import * as os from 'node:os'; +import * as path from 'node:path'; +import { ExtensionManager } from './extension-manager.js'; +import { createTestMergedSettings } from './settings.js'; +import { EXTENSIONS_DIRECTORY_NAME } from './extensions/variables.js'; + +const mockHomedir = vi.hoisted(() => vi.fn(() => '/tmp/mock-home')); +const mockIntegrityManager = vi.hoisted(() => ({ + verify: vi.fn().mockResolvedValue('verified'), + store: vi.fn().mockResolvedValue(undefined), +})); + +vi.mock('os', async (importOriginal) => { + const mockedOs = await importOriginal(); + return { + ...mockedOs, + homedir: mockHomedir, + }; +}); + +vi.mock('@google/gemini-cli-core', async (importOriginal) => { + const actual = + await importOriginal(); + return { + ...actual, + homedir: mockHomedir, + ExtensionIntegrityManager: vi + .fn() + .mockImplementation(() => mockIntegrityManager), + }; +}); + +describe('Open Plugin MCP Support', () => { + let tempHomeDir: string; + let tempWorkspaceDir: string; + let userExtensionsDir: string; + let extensionManager: ExtensionManager; + + beforeEach(() => { + vi.clearAllMocks(); + tempHomeDir = fs.mkdtempSync( + path.join(os.tmpdir(), 'gemini-cli-test-home-mcp-'), + ); + tempWorkspaceDir = fs.mkdtempSync( + path.join(tempHomeDir, 'gemini-cli-test-workspace-'), + ); + mockHomedir.mockReturnValue(tempHomeDir); + userExtensionsDir = path.join(tempHomeDir, EXTENSIONS_DIRECTORY_NAME); + fs.mkdirSync(userExtensionsDir, { recursive: true }); + + extensionManager = new ExtensionManager({ + settings: createTestMergedSettings(), + workspaceDir: tempWorkspaceDir, + requestConsent: vi.fn().mockResolvedValue(true), + requestSetting: null, + integrityManager: mockIntegrityManager, + }); + }); + + it('should discover MCP servers from .mcp.json', async () => { + const pluginDir = path.join(userExtensionsDir, 'mcp-plugin'); + fs.mkdirSync(pluginDir, { recursive: true }); + + fs.writeFileSync( + path.join(pluginDir, 'plugin.json'), + JSON.stringify({ + name: 'mcp-plugin', + version: '1.0.0', + }), + ); + + fs.writeFileSync( + path.join(pluginDir, '.mcp.json'), + JSON.stringify({ + mcpServers: { + 'test-server': { + command: 'node', + args: ['server.js'], + }, + }, + }), + ); + + const extensions = await extensionManager.loadExtensions(); + const plugin = extensions.find((ext) => ext.name === 'mcp-plugin'); + + expect(plugin).toBeDefined(); + expect(plugin?.mcpServers).toBeDefined(); + expect(plugin?.mcpServers?.['test-server']).toBeDefined(); + expect(plugin?.mcpServers?.['test-server'].command).toBe('node'); + }); + + it('should support explicit mcpServers path in plugin.json', async () => { + const pluginDir = path.join(userExtensionsDir, 'explicit-mcp-plugin'); + fs.mkdirSync(pluginDir, { recursive: true }); + + fs.writeFileSync( + path.join(pluginDir, 'plugin.json'), + JSON.stringify({ + name: 'explicit-mcp-plugin', + version: '1.0.0', + mcpServers: 'custom-mcp.json', + }), + ); + + fs.writeFileSync( + path.join(pluginDir, 'custom-mcp.json'), + JSON.stringify({ + mcpServers: { + 'custom-server': { + command: 'node', + args: ['custom.js'], + }, + }, + }), + ); + + const extensions = await extensionManager.loadExtensions(); + const plugin = extensions.find((ext) => ext.name === 'explicit-mcp-plugin'); + + expect(plugin).toBeDefined(); + expect(plugin?.mcpServers).toBeDefined(); + expect(plugin?.mcpServers?.['custom-server']).toBeDefined(); + }); +}); diff --git a/packages/cli/src/config/plugin.ts b/packages/cli/src/config/plugin.ts index 18386cf9f3..f08dc67d88 100644 --- a/packages/cli/src/config/plugin.ts +++ b/packages/cli/src/config/plugin.ts @@ -11,11 +11,14 @@ import { loadSkillsFromDir, type ExtensionInstallMetadata, type GeminiCLIExtension, + type MCPServerConfig, } from '@google/gemini-cli-core'; import { EXTENSIONS_CONFIG_FILENAME, HIDDEN_OPEN_PLUGIN_CONFIG_FILENAME, OPEN_PLUGIN_CONFIG_FILENAME, + OPEN_PLUGIN_MCP_CONFIG_FILENAME, + HIDDEN_OPEN_PLUGIN_MCP_CONFIG_FILENAME, recursivelyHydrateStrings, type JsonObject, } from './extensions/variables.js'; @@ -35,7 +38,7 @@ export interface OpenPluginConfig { skills?: string[] | Record; agents?: string[] | Record; hooks?: string[] | Record; - mcpServers?: string[] | Record; + mcpServers?: string | string[] | Record; } export const OPEN_PLUGIN_NAME_REGEX = /^[a-z0-9]([a-z0-9.-]*[a-z0-9])?$/; @@ -58,7 +61,13 @@ export const openPluginSchema = z.object({ skills: z.union([z.array(z.string()), z.record(z.any())]).optional(), agents: z.union([z.array(z.string()), z.record(z.any())]).optional(), hooks: z.union([z.array(z.string()), z.record(z.any())]).optional(), - mcpServers: z.union([z.array(z.string()), z.record(z.any())]).optional(), + mcpServers: z + .union([z.string(), z.array(z.string()), z.record(z.any())]) + .optional(), +}); + +export const openPluginMcpSchema = z.object({ + mcpServers: z.record(z.any()), }); export interface ManifestInfo { @@ -119,6 +128,8 @@ export async function loadOpenPluginConfig( }, ) as unknown as OpenPluginConfig; + const mcpServers = await resolveMcpServers(hydratedConfig, extensionDir); + return { name: hydratedConfig.name, version: hydratedConfig.version ?? '0.0.0', @@ -126,10 +137,83 @@ export async function loadOpenPluginConfig( description: hydratedConfig.description, author: hydratedConfig.author, license: hydratedConfig.license, - // Features are explicitly NOT mapped here for v1 plugins + mcpServers, }; } +/** + * Resolves MCP server configurations for an Open Plugin by checking the manifest + * and falling back to default filesystem locations. + */ +async function resolveMcpServers( + hydratedConfig: OpenPluginConfig, + extensionDir: string, +): Promise | undefined> { + let mcpServers: Record | undefined; + + // 1. Explicit mcpServers in plugin.json + if (hydratedConfig.mcpServers) { + if (typeof hydratedConfig.mcpServers === 'string') { + const mcpPath = path.resolve(extensionDir, hydratedConfig.mcpServers); + mcpServers = await loadMcpConfigFile(mcpPath); + } else if (Array.isArray(hydratedConfig.mcpServers)) { + const mcpServersValue = hydratedConfig.mcpServers; + if (mcpServersValue.length > 0) { + const first = mcpServersValue[0]; + if (typeof first === 'string') { + // Support array of paths + mcpServers = {}; + for (const p of mcpServersValue) { + const mcpPath = path.resolve(extensionDir, p); + const servers = await loadMcpConfigFile(mcpPath); + if (servers) { + Object.assign(mcpServers, servers); + } + } + } + } + } else { + // It's a Record + // eslint-disable-next-line @typescript-eslint/no-unsafe-type-assertion + mcpServers = hydratedConfig.mcpServers as Record; + } + } + + // 2. Fallback to .mcp.json at plugin root if no servers found yet + if (!mcpServers) { + const mcpPath = path.join(extensionDir, OPEN_PLUGIN_MCP_CONFIG_FILENAME); + const hiddenMcpPath = path.join( + extensionDir, + HIDDEN_OPEN_PLUGIN_MCP_CONFIG_FILENAME, + ); + + if (fs.existsSync(mcpPath)) { + mcpServers = await loadMcpConfigFile(mcpPath); + } else if (fs.existsSync(hiddenMcpPath)) { + mcpServers = await loadMcpConfigFile(hiddenMcpPath); + } + } + + return mcpServers; +} + +async function loadMcpConfigFile( + mcpPath: string, +): Promise | undefined> { + try { + const content = await fs.promises.readFile(mcpPath, 'utf-8'); + const json = JSON.parse(content) as unknown; + const result = openPluginMcpSchema.safeParse(json); + if (result.success) { + // eslint-disable-next-line @typescript-eslint/no-unsafe-type-assertion + return result.data.mcpServers as Record; + } + } catch (_e) { + // Ignore errors loading fallback file + } + return undefined; +} + /** * Creates a GeminiCLIExtension from an Open Plugin directory. */ @@ -174,7 +258,7 @@ export async function createOpenPlugin( author: config.author, license: config.license, contextFiles: [], - mcpServers: undefined, + mcpServers: config.mcpServers, excludeTools: undefined, settings: undefined, resolvedSettings: undefined, diff --git a/packages/core/src/tools/mcp-client-manager.ts b/packages/core/src/tools/mcp-client-manager.ts index 666b6d5321..9bda3d0dc8 100644 --- a/packages/core/src/tools/mcp-client-manager.ts +++ b/packages/core/src/tools/mcp-client-manager.ts @@ -212,14 +212,20 @@ export class McpClientManager { */ async startExtension(extension: GeminiCLIExtension) { debugLogger.log(`Loading extension: ${extension.name}`); + const mcpServers = Object.entries(extension.mcpServers ?? {}); await Promise.all( - Object.entries(extension.mcpServers ?? {}).map(([name, config]) => - this.maybeDiscoverMcpServer(name, { + mcpServers.map(([name, config]) => { + let serverName = name; + if (extension.manifestType === 'open-plugin') { + // Open Plugin MCP servers are prefixed with the plugin name using a colon. + serverName = `${extension.name}:${name}`; + } + return this.maybeDiscoverMcpServer(serverName, { // eslint-disable-next-line @typescript-eslint/no-misused-spread ...config, extension, - }), - ), + }); + }), ); await this.scheduleMcpContextRefresh(); } diff --git a/packages/core/src/tools/mcp-client.ts b/packages/core/src/tools/mcp-client.ts index 58b7b6c8e2..4a29028ccf 100644 --- a/packages/core/src/tools/mcp-client.ts +++ b/packages/core/src/tools/mcp-client.ts @@ -2271,11 +2271,22 @@ export async function createTransport( } } + const expandedCommand = expandEnvVars( + mcpServerConfig.command, + expansionEnv, + ); + const expandedArgs = (mcpServerConfig.args || []).map((arg) => + expandEnvVars(arg, expansionEnv), + ); + const expandedCwd = mcpServerConfig.cwd + ? expandEnvVars(mcpServerConfig.cwd, expansionEnv) + : undefined; + let transport: Transport = new StdioClientTransport({ - command: mcpServerConfig.command, - args: mcpServerConfig.args || [], + command: expandedCommand, + args: expandedArgs, env: finalEnv, - cwd: mcpServerConfig.cwd, + cwd: expandedCwd, stderr: 'pipe', }); @@ -2329,6 +2340,9 @@ function getExtensionEnvironment( extension?: GeminiCLIExtension, ): Record { const env: Record = {}; + if (extension?.path) { + env['PLUGIN_ROOT'] = extension.path; + } if (extension?.resolvedSettings) { for (const setting of extension.resolvedSettings) { if (setting.value !== undefined) { diff --git a/packages/core/src/tools/mcp-plugin-expansion.test.ts b/packages/core/src/tools/mcp-plugin-expansion.test.ts new file mode 100644 index 0000000000..9ebe2257d1 --- /dev/null +++ b/packages/core/src/tools/mcp-plugin-expansion.test.ts @@ -0,0 +1,97 @@ +/** + * @license + * Copyright 2026 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +import { describe, it, expect, vi, beforeEach } from 'vitest'; +import * as SdkClientStdioLib from '@modelcontextprotocol/sdk/client/stdio.js'; +import { createTransport, type McpContext } from './mcp-client.js'; +import type { GeminiCLIExtension, MCPServerConfig } from '../config/config.js'; + +vi.mock('@modelcontextprotocol/sdk/client/stdio.js'); + +describe('MCP Plugin Variable Expansion', () => { + beforeEach(() => { + vi.clearAllMocks(); + }); + + const mockMcpContext: Partial = { + sanitizationConfig: { + allowedEnvironmentVariables: [], + blockedEnvironmentVariables: [], + }, + isTrustedFolder: () => true, + emitMcpDiagnostic: () => {}, + }; + + it('should expand ${PLUGIN_ROOT} in command, args, and cwd', async () => { + const mockExtension: GeminiCLIExtension = { + name: 'test-plugin', + path: '/path/to/plugin', + isActive: true, + id: 'test-id', + version: '1.0.0', + } as GeminiCLIExtension; + + const config: MCPServerConfig = { + command: 'node', + args: ['${PLUGIN_ROOT}/index.js'], + cwd: '${PLUGIN_ROOT}/src', + extension: mockExtension, + }; + + await createTransport( + 'test-server', + config, + false, + mockMcpContext as McpContext, + ); + + expect(SdkClientStdioLib.StdioClientTransport).toHaveBeenCalledWith( + expect.objectContaining({ + command: 'node', + args: ['/path/to/plugin/index.js'], + cwd: '/path/to/plugin/src', + env: expect.objectContaining({ + PLUGIN_ROOT: '/path/to/plugin', + }), + }), + ); + }); + + it('should expand ${PLUGIN_ROOT} in env values', async () => { + const mockExtension: GeminiCLIExtension = { + name: 'test-plugin', + path: '/path/to/plugin', + isActive: true, + id: 'test-id', + version: '1.0.0', + } as GeminiCLIExtension; + + const config: MCPServerConfig = { + command: 'node', + args: [], + env: { + PLUGIN_PATH: '${PLUGIN_ROOT}/lib', + }, + extension: mockExtension, + }; + + await createTransport( + 'test-server', + config, + false, + mockMcpContext as McpContext, + ); + + expect(SdkClientStdioLib.StdioClientTransport).toHaveBeenCalledWith( + expect.objectContaining({ + env: expect.objectContaining({ + PLUGIN_ROOT: '/path/to/plugin', + PLUGIN_PATH: '/path/to/plugin/lib', + }), + }), + ); + }); +}); diff --git a/packages/core/src/tools/mcp-plugin-namespacing.test.ts b/packages/core/src/tools/mcp-plugin-namespacing.test.ts new file mode 100644 index 0000000000..4fd4153633 --- /dev/null +++ b/packages/core/src/tools/mcp-plugin-namespacing.test.ts @@ -0,0 +1,138 @@ +/** + * @license + * Copyright 2026 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +import { describe, it, expect, vi, beforeEach } from 'vitest'; +import { McpClientManager } from './mcp-client-manager.js'; +import type { + Config, + GeminiCLIExtension, + MCPServerConfig, +} from '../config/config.js'; + +interface McpClientManagerInternals { + maybeDiscoverMcpServer(name: string, config: MCPServerConfig): Promise; +} + +describe('MCP Plugin Namespacing', () => { + let mcpClientManager: McpClientManager; + let mockConfig: Config; + + beforeEach(() => { + vi.clearAllMocks(); + mockConfig = { + isTrustedFolder: () => true, + getMcpServers: () => ({}), + getMcpServerCommand: () => undefined, + getMcpEnablementCallbacks: () => undefined, + getAllowedMcpServers: () => [], + getBlockedMcpServers: () => [], + getDebugMode: () => false, + getWorkspaceContext: () => ({}), + } as unknown as Config; + mcpClientManager = new McpClientManager('1.0.0', mockConfig); + }); + + it('should use pluginName:mcpServerName for single MCP server in Open Plugin', async () => { + const extension: GeminiCLIExtension = { + name: 'my-plugin', + manifestType: 'open-plugin', + mcpServers: { + 'original-name': { + command: 'node', + args: ['index.js'], + }, + }, + id: 'test-id', + isActive: true, + } as unknown as GeminiCLIExtension; + + const maybeDiscoverSpy = vi + .spyOn( + mcpClientManager as unknown as McpClientManagerInternals, + 'maybeDiscoverMcpServer', + ) + .mockResolvedValue(undefined); + + await mcpClientManager.startExtension(extension); + + expect(maybeDiscoverSpy).toHaveBeenCalledWith( + 'my-plugin:original-name', + expect.objectContaining({ + command: 'node', + extension, + }), + ); + }); + + it('should use pluginName:mcpServerName for multiple MCP servers in Open Plugin', async () => { + const extension: GeminiCLIExtension = { + name: 'multi-plugin', + manifestType: 'open-plugin', + mcpServers: { + s1: { command: 'node', args: ['s1.js'] }, + s2: { command: 'node', args: ['s2.js'] }, + }, + id: 'test-id', + isActive: true, + } as unknown as GeminiCLIExtension; + + const maybeDiscoverSpy = vi + .spyOn( + mcpClientManager as unknown as McpClientManagerInternals, + 'maybeDiscoverMcpServer', + ) + .mockResolvedValue(undefined); + + await mcpClientManager.startExtension(extension); + + expect(maybeDiscoverSpy).toHaveBeenCalledWith( + 'multi-plugin:s1', + expect.objectContaining({ + command: 'node', + extension, + }), + ); + expect(maybeDiscoverSpy).toHaveBeenCalledWith( + 'multi-plugin:s2', + expect.objectContaining({ + command: 'node', + extension, + }), + ); + }); + + it('should NOT use plugin name for Gemini extensions', async () => { + const extension: GeminiCLIExtension = { + name: 'gemini-extension', + manifestType: 'gemini', + mcpServers: { + 'original-name': { + command: 'node', + args: ['index.js'], + }, + }, + id: 'test-id', + isActive: true, + } as unknown as GeminiCLIExtension; + + const maybeDiscoverSpy = vi + .spyOn( + mcpClientManager as unknown as McpClientManagerInternals, + 'maybeDiscoverMcpServer', + ) + .mockResolvedValue(undefined); + + await mcpClientManager.startExtension(extension); + + expect(maybeDiscoverSpy).toHaveBeenCalledWith( + 'original-name', + expect.objectContaining({ + command: 'node', + extension, + }), + ); + }); +}); diff --git a/packages/core/src/tools/mcp-plugin-naming.test.ts b/packages/core/src/tools/mcp-plugin-naming.test.ts new file mode 100644 index 0000000000..5d6d9d17d2 --- /dev/null +++ b/packages/core/src/tools/mcp-plugin-naming.test.ts @@ -0,0 +1,60 @@ +/** + * @license + * Copyright 2026 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +import { describe, it, expect } from 'vitest'; +import { + isMcpToolName, + parseMcpToolName, + generateValidName, + formatMcpToolName, +} from './mcp-tool.js'; + +describe('MCP Plugin Naming', () => { + describe('isMcpToolName', () => { + it('should identify standard MCP tool names', () => { + expect(isMcpToolName('mcp_server_tool')).toBe(true); + }); + }); + + describe('parseMcpToolName', () => { + it('should parse standard MCP tool names', () => { + const result = parseMcpToolName('mcp_server_tool'); + expect(result.serverName).toBe('server'); + expect(result.toolName).toBe('tool'); + }); + + it('should parse MCP names with colons in the server name', () => { + // This is the format for Open Plugins: mcp_plugin:server_tool + const result = parseMcpToolName('mcp_demo-plugin:demo-server_demo_tool'); + expect(result.serverName).toBe('demo-plugin:demo-server'); + expect(result.toolName).toBe('demo_tool'); + }); + }); + + describe('generateValidName', () => { + it('should add mcp_ prefix to standard names', () => { + expect(generateValidName('server_tool')).toBe('mcp_server_tool'); + }); + + it('should include colons if they are in the input', () => { + expect(generateValidName('demo-plugin:demo-server_demo_tool')).toBe( + 'mcp_demo-plugin:demo-server_demo_tool', + ); + }); + }); + + describe('formatMcpToolName', () => { + it('should format standard MCP tool names', () => { + expect(formatMcpToolName('server', 'tool')).toBe('mcp_server_tool'); + }); + + it('should format Open Plugin namespaced tool names using underscores for the tool portion', () => { + expect(formatMcpToolName('plugin:server', 'tool')).toBe( + 'mcp_plugin:server_tool', + ); + }); + }); +});