From e95a6086fc89191888f2a4e41a3c875273311d96 Mon Sep 17 00:00:00 2001 From: Bryan Morgan Date: Sat, 7 Jun 2025 18:30:56 -0400 Subject: [PATCH] Bryanmorgan/add mcp description support (#825) --- docs/cli/commands.md | 13 +++ packages/cli/src/ui/App.tsx | 102 ++++++++++-------- .../ui/components/ContextSummaryDisplay.tsx | 10 ++ .../ui/hooks/slashCommandProcessor.test.ts | 90 +++++++++++++--- .../cli/src/ui/hooks/slashCommandProcessor.ts | 82 +++++++++++++- packages/core/src/config/config.ts | 2 + packages/core/src/tools/mcp-tool.test.ts | 10 +- packages/core/src/tools/mcp-tool.ts | 11 -- 8 files changed, 246 insertions(+), 74 deletions(-) diff --git a/docs/cli/commands.md b/docs/cli/commands.md index c0952129..859ebd3c 100644 --- a/docs/cli/commands.md +++ b/docs/cli/commands.md @@ -11,6 +11,19 @@ Slash commands provide meta-level control over the CLI itself. They can typicall - **Description:** Displays help information about the Gemini CLI, including available commands and their usage. - **Action:** Opens a help dialog or section within the CLI. +- **`/mcp`** (Toggle descriptions: **Ctrl+T**) + + - **Description:** Lists configured Model Context Protocol (MCP) servers and their available tools. + - **Action:** Displays a formatted list of MCP servers with connection status indicators, server details, and available tools. + - **Sub-commands:** + - **`desc`** or **`descriptions`**: + - **Description:** Shows detailed descriptions for MCP servers and tools. + - **Action:** Displays each tool's name with its full description, formatted for readability. + - **`nodesc`** or **`nodescriptions`**: + - **Description:** Hides tool descriptions, showing only the tool names. + - **Action:** Displays a compact list with only tool names. + - **Keyboard Shortcut:** Press **Ctrl+T** at any time to toggle between showing and hiding tool descriptions. + - **`/clear`** (Shortcut: **Ctrl+L**) - **Description:** Clears the entire terminal screen, including the visible session history and scrollback within the CLI. diff --git a/packages/cli/src/ui/App.tsx b/packages/cli/src/ui/App.tsx index c3a2b8af..d0e9efb7 100644 --- a/packages/cli/src/ui/App.tsx +++ b/packages/cli/src/ui/App.tsx @@ -79,6 +79,8 @@ export const App = ({ config, settings, startupWarnings = [] }: AppProps) => { const [corgiMode, setCorgiMode] = useState(false); const [shellModeActive, setShellModeActive] = useState(false); const [showErrorDetails, setShowErrorDetails] = useState(false); + const [showToolDescriptions, setShowToolDescriptions] = + useState(false); const [ctrlCPressedOnce, setCtrlCPressedOnce] = useState(false); const ctrlCTimerRef = useRef(null); @@ -87,44 +89,6 @@ export const App = ({ config, settings, startupWarnings = [] }: AppProps) => { [consoleMessages], ); - useInput((input: string, key: InkKeyType) => { - if (key.ctrl && input === 'o') { - setShowErrorDetails((prev) => !prev); - refreshStatic(); - } else if (key.ctrl && (input === 'c' || input === 'C')) { - if (ctrlCPressedOnce) { - if (ctrlCTimerRef.current) { - clearTimeout(ctrlCTimerRef.current); - } - process.exit(0); - } else { - setCtrlCPressedOnce(true); - ctrlCTimerRef.current = setTimeout(() => { - setCtrlCPressedOnce(false); - ctrlCTimerRef.current = null; - }, CTRL_C_PROMPT_DURATION_MS); - } - } - }); - - useEffect( - () => () => { - if (ctrlCTimerRef.current) { - clearTimeout(ctrlCTimerRef.current); - } - }, - [], - ); - - useConsolePatcher({ - onNewMessage: handleNewMessage, - debugMode: config.getDebugMode(), - }); - - const toggleCorgiMode = useCallback(() => { - setCorgiMode((prev) => !prev); - }, []); - const { isThemeDialogOpen, openThemeDialog, @@ -132,11 +96,9 @@ export const App = ({ config, settings, startupWarnings = [] }: AppProps) => { handleThemeHighlight, } = useThemeCommand(settings, setThemeError, addItem); - useEffect(() => { - if (config) { - setGeminiMdFileCount(config.getGeminiMdFileCount()); - } - }, [config]); + const toggleCorgiMode = useCallback(() => { + setCorgiMode((prev) => !prev); + }, []); const performMemoryRefresh = useCallback(async () => { addItem( @@ -190,8 +152,61 @@ export const App = ({ config, settings, startupWarnings = [] }: AppProps) => { openThemeDialog, performMemoryRefresh, toggleCorgiMode, + showToolDescriptions, ); + useInput((input: string, key: InkKeyType) => { + if (key.ctrl && input === 'o') { + setShowErrorDetails((prev) => !prev); + refreshStatic(); + } else if (key.ctrl && input === 't') { + // Toggle showing tool descriptions + const newValue = !showToolDescriptions; + setShowToolDescriptions(newValue); + refreshStatic(); + + // Re-execute the MCP command to show/hide descriptions + const mcpServers = config.getMcpServers(); + if (Object.keys(mcpServers || {}).length > 0) { + // Pass description flag based on the new value + handleSlashCommand(newValue ? '/mcp desc' : '/mcp nodesc'); + } + } else if (key.ctrl && (input === 'c' || input === 'C')) { + if (ctrlCPressedOnce) { + if (ctrlCTimerRef.current) { + clearTimeout(ctrlCTimerRef.current); + } + process.exit(0); + } else { + setCtrlCPressedOnce(true); + ctrlCTimerRef.current = setTimeout(() => { + setCtrlCPressedOnce(false); + ctrlCTimerRef.current = null; + }, CTRL_C_PROMPT_DURATION_MS); + } + } + }); + + useEffect( + () => () => { + if (ctrlCTimerRef.current) { + clearTimeout(ctrlCTimerRef.current); + } + }, + [], + ); + + useConsolePatcher({ + onNewMessage: handleNewMessage, + debugMode: config.getDebugMode(), + }); + + useEffect(() => { + if (config) { + setGeminiMdFileCount(config.getGeminiMdFileCount()); + } + }, [config]); + const { streamingState, submitQuery, initError, pendingHistoryItems } = useGeminiStream( config.getGeminiClient(), @@ -422,6 +437,7 @@ export const App = ({ config, settings, startupWarnings = [] }: AppProps) => { getCurrentGeminiMdFilename() } mcpServers={config.getMcpServers()} + showToolDescriptions={showToolDescriptions} /> )} diff --git a/packages/cli/src/ui/components/ContextSummaryDisplay.tsx b/packages/cli/src/ui/components/ContextSummaryDisplay.tsx index c19a8172..c4527066 100644 --- a/packages/cli/src/ui/components/ContextSummaryDisplay.tsx +++ b/packages/cli/src/ui/components/ContextSummaryDisplay.tsx @@ -13,12 +13,14 @@ interface ContextSummaryDisplayProps { geminiMdFileCount: number; contextFileName: string; mcpServers?: Record; + showToolDescriptions?: boolean; } export const ContextSummaryDisplay: React.FC = ({ geminiMdFileCount, contextFileName, mcpServers, + showToolDescriptions, }) => { const mcpServerCount = Object.keys(mcpServers || {}).length; @@ -45,6 +47,14 @@ export const ContextSummaryDisplay: React.FC = ({ } if (mcpText) { summaryText += mcpText; + // Add Ctrl+T hint when MCP servers are available + if (mcpServers && Object.keys(mcpServers).length > 0) { + if (showToolDescriptions) { + summaryText += ' (Ctrl+T to hide descriptions)'; + } else { + summaryText += ' (Ctrl+T to view descriptions)'; + } + } } return {summaryText}; diff --git a/packages/cli/src/ui/hooks/slashCommandProcessor.test.ts b/packages/cli/src/ui/hooks/slashCommandProcessor.test.ts index c279da8c..221893a2 100644 --- a/packages/cli/src/ui/hooks/slashCommandProcessor.test.ts +++ b/packages/cli/src/ui/hooks/slashCommandProcessor.test.ts @@ -49,7 +49,7 @@ vi.mock('node:fs/promises', () => ({ })); import { act, renderHook } from '@testing-library/react'; -import { vi, describe, it, expect, beforeEach, Mock } from 'vitest'; +import { vi, describe, it, expect, beforeEach, afterEach, Mock } from 'vitest'; import open from 'open'; import { useSlashCommandProcessor, @@ -107,7 +107,7 @@ describe('useSlashCommandProcessor', () => { process.env = { ...globalThis.process.env }; }); - const getProcessor = () => { + const getProcessor = (showToolDescriptions: boolean = false) => { const { result } = renderHook(() => useSlashCommandProcessor( mockConfig, @@ -119,6 +119,7 @@ describe('useSlashCommandProcessor', () => { mockOpenThemeDialog, mockPerformMemoryRefresh, mockCorgiMode, + showToolDescriptions, ), ); return result.current; @@ -571,17 +572,82 @@ Add any other context about the problem here. // Check that the message contains details about both servers and their tools const message = mockAddItem.mock.calls[1][0].text; // Server 1 - Connected (green dot) - expect(message).toContain('🟢 server1 (2 tools):'); - expect(message).toContain('server1_tool1'); - expect(message).toContain('server1_tool2'); + expect(message).toContain('🟢 \u001b[1mserver1\u001b[0m (2 tools)'); + expect(message).toContain('\u001b[36mserver1_tool1\u001b[0m'); + expect(message).toContain('\u001b[36mserver1_tool2\u001b[0m'); // Server 2 - Connecting (yellow dot) - expect(message).toContain('🟡 server2 (1 tools):'); - expect(message).toContain('server2_tool1'); + expect(message).toContain('🟡 \u001b[1mserver2\u001b[0m (1 tools)'); + expect(message).toContain('\u001b[36mserver2_tool1\u001b[0m'); // Server 3 - No status, should default to Disconnected (red dot) - expect(message).toContain('🔴 server3 (1 tools):'); - expect(message).toContain('server3_tool1'); + expect(message).toContain('🔴 \u001b[1mserver3\u001b[0m (1 tools)'); + expect(message).toContain('\u001b[36mserver3_tool1\u001b[0m'); + + expect(commandResult).toBe(true); + }); + + it('should display tool descriptions when showToolDescriptions is true', async () => { + // Mock MCP servers configuration with server description + const mockMcpServers = { + server1: { + command: 'cmd1', + description: 'This is a server description', + }, + }; + + // Setup getMCPServerStatus mock implementation + vi.mocked(getMCPServerStatus).mockImplementation((serverName) => { + if (serverName === 'server1') return MCPServerStatus.CONNECTED; + return MCPServerStatus.DISCONNECTED; + }); + + // Mock tools from server with descriptions + const mockServerTools = [ + { name: 'tool1', description: 'This is tool 1 description' }, + { name: 'tool2', description: 'This is tool 2 description' }, + ]; + + mockConfig = { + ...mockConfig, + getToolRegistry: vi.fn().mockResolvedValue({ + getToolsByServer: vi.fn().mockReturnValue(mockServerTools), + }), + getMcpServers: vi.fn().mockReturnValue(mockMcpServers), + } as unknown as Config; + + const { handleSlashCommand } = getProcessor(true); + let commandResult: SlashCommandActionReturn | boolean = false; + await act(async () => { + commandResult = handleSlashCommand('/mcp'); + }); + + expect(mockAddItem).toHaveBeenNthCalledWith( + 2, + expect.objectContaining({ + type: MessageType.INFO, + text: expect.stringContaining('Configured MCP servers and tools:'), + }), + expect.any(Number), + ); + + const message = mockAddItem.mock.calls[1][0].text; + + // Check that server description is included (with ANSI color codes) + expect(message).toContain('\u001b[1mserver1\u001b[0m (2 tools)'); + expect(message).toContain( + '\u001b[32mThis is a server description\u001b[0m', + ); + + // Check that tool descriptions are included (with ANSI color codes) + expect(message).toContain('\u001b[36mtool1\u001b[0m'); + expect(message).toContain( + '\u001b[32mThis is tool 1 description\u001b[0m', + ); + expect(message).toContain('\u001b[36mtool2\u001b[0m'); + expect(message).toContain( + '\u001b[32mThis is tool 2 description\u001b[0m', + ); expect(commandResult).toBe(true); }); @@ -636,9 +702,9 @@ Add any other context about the problem here. // Check that the message contains details about both servers and their tools const message = mockAddItem.mock.calls[1][0].text; - expect(message).toContain('🟢 server1 (1 tools):'); - expect(message).toContain('server1_tool1'); - expect(message).toContain('🔴 server2 (0 tools):'); + expect(message).toContain('🟢 \u001b[1mserver1\u001b[0m (1 tools)'); + expect(message).toContain('\u001b[36mserver1_tool1\u001b[0m'); + expect(message).toContain('🔴 \u001b[1mserver2\u001b[0m (0 tools)'); expect(message).toContain('No tools available'); expect(commandResult).toBe(true); diff --git a/packages/cli/src/ui/hooks/slashCommandProcessor.ts b/packages/cli/src/ui/hooks/slashCommandProcessor.ts index 39ccf0e8..38fdddba 100644 --- a/packages/cli/src/ui/hooks/slashCommandProcessor.ts +++ b/packages/cli/src/ui/hooks/slashCommandProcessor.ts @@ -47,6 +47,7 @@ export const useSlashCommandProcessor = ( openThemeDialog: () => void, performMemoryRefresh: () => Promise, toggleCorgiMode: () => void, + showToolDescriptions: boolean = false, ) => { const addMessage = useCallback( (message: Message) => { @@ -141,6 +142,21 @@ export const useSlashCommandProcessor = ( name: 'mcp', description: 'list configured MCP servers and tools', action: async (_mainCommand, _subCommand, _args) => { + // Check if the _subCommand includes a specific flag to control description visibility + let useShowDescriptions = showToolDescriptions; + if (_subCommand === 'desc' || _subCommand === 'descriptions') { + useShowDescriptions = true; + } else if ( + _subCommand === 'nodesc' || + _subCommand === 'nodescriptions' + ) { + useShowDescriptions = false; + } else if (_args === 'desc' || _args === 'descriptions') { + useShowDescriptions = true; + } else if (_args === 'nodesc' || _args === 'nodescriptions') { + useShowDescriptions = false; + } + const toolRegistry = await config?.getToolRegistry(); if (!toolRegistry) { addMessage({ @@ -184,10 +200,68 @@ export const useSlashCommandProcessor = ( break; } - message += `${statusDot} ${serverName} (${serverTools.length} tools):\n`; + // Get server description if available + const server = mcpServers[serverName]; + + // Format server header with bold formatting + message += `${statusDot} \u001b[1m${serverName}\u001b[0m (${serverTools.length} tools)`; + + // Add server description with proper handling of multi-line descriptions + if (useShowDescriptions && server?.description) { + const greenColor = '\u001b[32m'; + const resetColor = '\u001b[0m'; + + const descLines = server.description.split('\n'); + message += `: ${greenColor}${descLines[0]}${resetColor}`; + message += '\n'; + + // If there are multiple lines, add proper indentation for each line + if (descLines.length > 1) { + for (let i = 1; i < descLines.length; i++) { + // Skip empty lines at the end + if (i === descLines.length - 1 && descLines[i].trim() === '') + continue; + message += ` ${greenColor}${descLines[i]}${resetColor}\n`; + } + } + } else { + message += '\n'; + } + + // Reset formatting after server entry + message += '\u001b[0m'; + if (serverTools.length > 0) { serverTools.forEach((tool) => { - message += ` - ${tool.name}\n`; + if (useShowDescriptions && tool.description) { + // Format tool name in cyan using simple ANSI cyan color + message += ` - \u001b[36m${tool.name}\u001b[0m: `; + + // Apply green color to the description text + const greenColor = '\u001b[32m'; + const resetColor = '\u001b[0m'; + + // Handle multi-line descriptions by properly indenting and preserving formatting + const descLines = tool.description.split('\n'); + message += `${greenColor}${descLines[0]}${resetColor}\n`; + + // If there are multiple lines, add proper indentation for each line + if (descLines.length > 1) { + for (let i = 1; i < descLines.length; i++) { + // Skip empty lines at the end + if ( + i === descLines.length - 1 && + descLines[i].trim() === '' + ) + continue; + message += ` ${greenColor}${descLines[i]}${resetColor}\n`; + } + } + // Reset is handled inline with each line now + } else { + // Use cyan color for the tool name even when not showing descriptions + message += ` - \u001b[36m${tool.name}\u001b[0m\n`; + } }); } else { message += ' No tools available\n'; @@ -195,6 +269,9 @@ export const useSlashCommandProcessor = ( message += '\n'; } + // Make sure to reset any ANSI formatting at the end to prevent it from affecting the terminal + message += '\u001b[0m'; + addMessage({ type: MessageType.INFO, content: message, @@ -369,6 +446,7 @@ Add any other context about the problem here. addMessage, toggleCorgiMode, config, + showToolDescriptions, ], ); diff --git a/packages/core/src/config/config.ts b/packages/core/src/config/config.ts index 3eb6ecf1..7d25c1bb 100644 --- a/packages/core/src/config/config.ts +++ b/packages/core/src/config/config.ts @@ -49,6 +49,8 @@ export class MCPServerConfig { // Common readonly timeout?: number, readonly trust?: boolean, + // Metadata + readonly description?: string, ) {} } diff --git a/packages/core/src/tools/mcp-tool.test.ts b/packages/core/src/tools/mcp-tool.test.ts index fc6ce6be..d972efdb 100644 --- a/packages/core/src/tools/mcp-tool.test.ts +++ b/packages/core/src/tools/mcp-tool.test.ts @@ -52,7 +52,7 @@ describe('DiscoveredMCPTool', () => { }); describe('constructor', () => { - it('should set properties correctly and augment description (non-generic server)', () => { + it('should set properties correctly (non-generic server)', () => { const tool = new DiscoveredMCPTool( mockCallableToolInstance, serverName, // serverName is 'mock-mcp-server', not 'mcp' @@ -64,14 +64,13 @@ describe('DiscoveredMCPTool', () => { expect(tool.name).toBe(toolNameForModel); expect(tool.schema.name).toBe(toolNameForModel); - const expectedDescription = `${baseDescription}\n\nThis MCP tool named '${serverToolName}' was discovered from an MCP server.`; - expect(tool.schema.description).toBe(expectedDescription); + expect(tool.schema.description).toBe(baseDescription); expect(tool.schema.parameters).toEqual(inputSchema); expect(tool.serverToolName).toBe(serverToolName); expect(tool.timeout).toBeUndefined(); }); - it('should set properties correctly and augment description (generic "mcp" server)', () => { + it('should set properties correctly (generic "mcp" server)', () => { const genericServerName = 'mcp'; const tool = new DiscoveredMCPTool( mockCallableToolInstance, @@ -81,8 +80,7 @@ describe('DiscoveredMCPTool', () => { inputSchema, serverToolName, ); - const expectedDescription = `${baseDescription}\n\nThis MCP tool named '${serverToolName}' was discovered from '${genericServerName}' MCP server.`; - expect(tool.schema.description).toBe(expectedDescription); + expect(tool.schema.description).toBe(baseDescription); }); it('should accept and store a custom timeout', () => { diff --git a/packages/core/src/tools/mcp-tool.ts b/packages/core/src/tools/mcp-tool.ts index 8a7694d8..ffe12cf7 100644 --- a/packages/core/src/tools/mcp-tool.ts +++ b/packages/core/src/tools/mcp-tool.ts @@ -30,17 +30,6 @@ export class DiscoveredMCPTool extends BaseTool { readonly timeout?: number, readonly trust?: boolean, ) { - if (serverName !== 'mcp') { - // Add server name if not the generic 'mcp' - description += ` - -This MCP tool named '${serverToolName}' was discovered from an MCP server.`; - } else { - description += ` - -This MCP tool named '${serverToolName}' was discovered from '${serverName}' MCP server.`; - } - super( name, name,