From 28d8ba2e348e4dde085b41c5683183b18b1f347c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fortissolu=C3=A7=C3=B5es?= Date: Fri, 17 Jul 2026 13:57:21 -0300 Subject: [PATCH 1/3] feat(cli): add read-only /mcp status command --- README.md | 1 + .../commands/__tests__/mcp-command.test.ts | 317 ++++++++++++++++++ cli/src/commands/command-registry.ts | 11 + cli/src/commands/mcp-command.ts | 157 +++++++++ cli/src/data/slash-commands.ts | 6 + common/src/mcp/client.ts | 50 ++- 6 files changed, 541 insertions(+), 1 deletion(-) create mode 100644 cli/src/commands/__tests__/mcp-command.test.ts create mode 100644 cli/src/commands/mcp-command.ts diff --git a/README.md b/README.md index 3f6a7b24ef..0896ee5f8a 100644 --- a/README.md +++ b/README.md @@ -57,6 +57,7 @@ Then tell Freebuff what you want — it finds the right files, makes the changes | `/init` | Create a starter knowledge.md | | `/feedback` | Share feedback | | `/theme:toggle` | Toggle light/dark mode | +| `/mcp` | Show configured MCP servers | | `/logout` | Sign out | | `/exit` | Quit | diff --git a/cli/src/commands/__tests__/mcp-command.test.ts b/cli/src/commands/__tests__/mcp-command.test.ts new file mode 100644 index 0000000000..5045f6d95f --- /dev/null +++ b/cli/src/commands/__tests__/mcp-command.test.ts @@ -0,0 +1,317 @@ +import { describe, test, expect, beforeEach, afterEach, mock } from 'bun:test' +import fs from 'fs' +import os from 'os' +import path from 'path' + +import { findCommand } from '../command-registry' +import { + buildMcpStatusReport, + formatMcpStatusForCli, + handleMcpCommand, +} from '../mcp-command' + +import type { RouterParams } from '../command-registry' +import type { ChatMessage } from '../../types/chat' + +// ============================================================================ +// Helmock module for loadMCPConfigSync +// ============================================================================ +let mockMcpConfig: any = { mcpServers: {}, _sourceFilePath: '' } + +mock.module('@codebuff/sdk', () => { + const actual = require('@codebuff/sdk') as any + return { + ...actual, + loadMCPConfigSync: () => mockMcpConfig, + } +}) + +// ============================================================================ +// Tests +// ============================================================================ + +describe('/mcp command registration', () => { + test('findCommand finds /mcp', () => { + const command = findCommand('mcp') + expect(command).toBeDefined() + expect(command!.name).toBe('mcp') + }) + + test('findCommand finds mcp via alias', () => { + const command = findCommand('mcp-servers') + expect(command).toBeDefined() + expect(command!.name).toBe('mcp') + }) + + test('command is defined with defineCommand (no args)', () => { + const command = findCommand('mcp') + expect(command!.acceptsArgs).toBe(false) + }) + + test('command handler renders a system message via handleMcpCommand', () => { + mockMcpConfig = { mcpServers: {}, _sourceFilePath: '' } + const { postUserMessage } = handleMcpCommand() + const prev: any[] = [{ id: '1', variant: 'user', content: '/mcp', timestamp: '' }] + const result = postUserMessage(prev) + + expect(result).toHaveLength(2) + const systemMsg = result[1] + expect(systemMsg.variant).toBe('ai') + expect(systemMsg.content).toContain('MCP') + }) +}) + +describe('buildMcpStatusReport', () => { + beforeEach(() => { + mockMcpConfig = { mcpServers: {}, _sourceFilePath: '' } + }) + + test('no servers configured', () => { + mockMcpConfig = { mcpServers: {}, _sourceFilePath: '' } + const report = buildMcpStatusReport() + expect(report.servers).toEqual([]) + expect(report.configPath).toBe('') + }) + + test('one stdio server configured (not connected)', () => { + mockMcpConfig = { + mcpServers: { + github: { + type: 'stdio', + command: 'npx', + args: ['-y', '@modelcontextprotocol/server-github'], + env: {}, + }, + }, + _sourceFilePath: '/project/.agents/mcp.json', + } + + const report = buildMcpStatusReport() + expect(report.servers).toHaveLength(1) + expect(report.servers[0].name).toBe('github') + expect(report.servers[0].transport).toBe('stdio') + expect(report.servers[0].connected).toBe(false) + expect(report.servers[0].toolCount).toBeNull() + expect(report.servers[0].errorLabel).toBeNull() + }) + + test('multiple servers configured', () => { + mockMcpConfig = { + mcpServers: { + github: { + type: 'stdio', + command: 'npx', + args: ['-y', '@modelcontextprotocol/server-github'], + env: {}, + }, + postgres: { + type: 'stdio', + command: 'uvx', + args: ['mcp-server-postgres'], + env: {}, + }, + }, + _sourceFilePath: '/project/.agents/mcp.json', + } + + const report = buildMcpStatusReport() + expect(report.servers).toHaveLength(2) + + const github = report.servers.find((s) => s.name === 'github')! + expect(github).toBeDefined() + expect(github.transport).toBe('stdio') + + const postgres = report.servers.find((s) => s.name === 'postgres')! + expect(postgres).toBeDefined() + expect(postgres.transport).toBe('stdio') + }) + + test('config file with http transport', () => { + mockMcpConfig = { + mcpServers: { + remote: { + type: 'http', + url: 'https://example.com/mcp', + headers: { Authorization: 'Bearer token' }, + params: {}, + }, + }, + _sourceFilePath: '/project/.agents/mcp.json', + } + + const report = buildMcpStatusReport() + expect(report.servers).toHaveLength(1) + expect(report.servers[0].transport).toBe('Streamable HTTP') + }) + + test('config file with sse transport', () => { + mockMcpConfig = { + mcpServers: { + events: { + type: 'sse', + url: 'https://events.example.com/mcp', + params: {}, + headers: {}, + }, + }, + _sourceFilePath: '/project/.agents/mcp.json', + } + + const report = buildMcpStatusReport() + expect(report.servers).toHaveLength(1) + expect(report.servers[0].transport).toBe('SSE') + }) +}) + +describe('formatMcpStatusForCli', () => { + test('empty config shows helpful message', () => { + const output = formatMcpStatusForCli({ servers: [], configPath: '' }) + expect(output).toContain('No MCP servers configured') + expect(output).toContain('.agents/mcp.json') + expect(output).not.toContain('✓') + expect(output).not.toContain('✗') + }) + + test('single connected server', () => { + const output = formatMcpStatusForCli({ + servers: [ + { + name: 'github', + transport: 'stdio', + connected: true, + toolCount: 18, + configPath: '/project/.agents/mcp.json', + errorLabel: null, + }, + ], + configPath: '/project/.agents/mcp.json', + }) + expect(output).toContain('✓') + expect(output).toContain('github') + expect(output).toContain('connected') + expect(output).toContain('stdio') + expect(output).toContain('18') + expect(output).toContain('Config path') + }) + + test('multiple servers with mixed status', () => { + const output = formatMcpStatusForCli({ + servers: [ + { + name: 'github', + transport: 'stdio', + connected: true, + toolCount: 18, + configPath: '/project/.agents/mcp.json', + errorLabel: null, + }, + { + name: 'postgres', + transport: 'stdio', + connected: false, + toolCount: null, + configPath: '/project/.agents/mcp.json', + errorLabel: null, + }, + ], + configPath: '/project/.agents/mcp.json', + }) + expect(output).toContain('✓ github') + expect(output).toContain('○ postgres') + expect(output).toContain('not connected') + expect(output).toContain('will connect when the agent uses its tools') + }) + + test('failed server with error', () => { + const output = formatMcpStatusForCli({ + servers: [ + { + name: 'broken', + transport: 'stdio', + connected: false, + toolCount: null, + configPath: '/project/.agents/mcp.json', + errorLabel: 'process exited before initialization', + }, + ], + configPath: '/project/.agents/mcp.json', + }) + expect(output).toContain('✗') + expect(output).toContain('broken') + expect(output).toContain('failed') + expect(output).toContain('process exited before initialization') + }) + + test('unknown transport', () => { + const output = formatMcpStatusForCli({ + servers: [ + { + name: 'weird', + transport: 'unknown', + connected: false, + toolCount: null, + configPath: '/project/.agents/mcp.json', + errorLabel: null, + }, + ], + configPath: '/project/.agents/mcp.json', + }) + expect(output).toContain('weird') + expect(output).toContain('unknown') + }) + + test('tool count shows ellipsis when null', () => { + const output = formatMcpStatusForCli({ + servers: [ + { + name: 'github', + transport: 'stdio', + connected: true, + toolCount: null, + configPath: '/project/.agents/mcp.json', + errorLabel: null, + }, + ], + configPath: '/project/.agents/mcp.json', + }) + expect(output).toContain('Tools: …') + }) +}) + +describe('/mcp command does not break other commands', () => { + test('help command still works', () => { + const help = findCommand('help') + expect(help).toBeDefined() + expect(help!.name).toBe('help') + }) + + test('bash command still works', () => { + const bash = findCommand('bash') + expect(bash).toBeDefined() + expect(bash!.name).toBe('bash') + }) + + test('history command still works', () => { + const history = findCommand('history') + expect(history).toBeDefined() + expect(history!.name).toBe('history') + }) + + test('init command still works', () => { + const init = findCommand('init') + expect(init).toBeDefined() + expect(init!.name).toBe('init') + }) + + test('theme:toggle command still works', () => { + const theme = findCommand('theme:toggle') + expect(theme).toBeDefined() + expect(theme!.name).toBe('theme:toggle') + }) + + test('feedback command still works', () => { + const fb = findCommand('feedback') + expect(fb).toBeDefined() + expect(fb!.name).toBe('feedback') + }) +}) diff --git a/cli/src/commands/command-registry.ts b/cli/src/commands/command-registry.ts index 6738caed9a..8a32241119 100644 --- a/cli/src/commands/command-registry.ts +++ b/cli/src/commands/command-registry.ts @@ -6,6 +6,7 @@ import { handleCopyConversationCommand } from './copy-conversation' import { handleHelpCommand } from './help' import { handleImageCommand } from './image' import { handleInitializationFlowLocally } from './init' +import { handleMcpCommand } from './mcp-command' import { collectProcessDiagnostics, formatProcessDiagnostics, @@ -223,6 +224,16 @@ const ALL_COMMANDS: CommandDefinition[] = [ clearInput(params) }, }), + defineCommand({ + name: 'mcp', + aliases: ['mcp-servers'], + handler: (params) => { + const { postUserMessage } = handleMcpCommand() + params.setMessages((prev) => postUserMessage(prev)) + params.saveToHistory(params.inputValue.trim()) + clearInput(params) + }, + }), defineCommand({ name: 'copy', aliases: ['copy-chat', 'export'], diff --git a/cli/src/commands/mcp-command.ts b/cli/src/commands/mcp-command.ts new file mode 100644 index 0000000000..e26d94b8b5 --- /dev/null +++ b/cli/src/commands/mcp-command.ts @@ -0,0 +1,157 @@ +import { loadMCPConfigSync } from '@codebuff/sdk' +import { + getMCPClientConnectionInfo, + hashMcpConfig, +} from '@codebuff/common/mcp/client' + +import { getSystemMessage } from '../utils/message-history' + +import type { MCPConfig } from '@codebuff/common/types/mcp' + +// ============================================================================ +// Public API +// ============================================================================ + +export type McpServerStatus = { + name: string + transport: string + connected: boolean + toolCount: number | null + configPath: string + errorLabel: string | null +} + +/** + * Load configured MCP servers and enrich each with connection status. + * This is the single-entry point that the /mcp command handler calls. + */ +export function buildMcpStatusReport(): { + servers: McpServerStatus[] + configPath: string +} { + const loaded = loadMCPConfigSync({ verbose: false }) + const servers: McpServerStatus[] = [] + + for (const [name, config] of Object.entries(loaded.mcpServers)) { + const info = getMCPClientConnectionInfo(config) + const hash = hashMcpConfig(config) + + servers.push({ + name, + transport: describeTransport(config), + connected: info.connected, + toolCount: info.toolCount, + configPath: loaded._sourceFilePath, + errorLabel: null, // Runtime errors are logged server-side; the client + // knows a failed connection only when the agent + // attempted to use the server. We still show it as + // connected=false with no error in that case. + }) + } + + return { servers, configPath: loaded._sourceFilePath } +} + +/** + * Render the status report as a system message for the chat. + */ +export function formatMcpStatusForCli(report: { + servers: McpServerStatus[] + configPath: string +}): string { + const { servers, configPath } = report + + if (servers.length === 0) { + return [ + '### MCP Servers', + '', + 'No MCP servers configured.', + '', + 'Create an `.agents/mcp.json` file in your project with server definitions,', + 'then run /mcp again to see their status.', + '', + 'Example `.agents/mcp.json`:', + '```json', + '{', + ' "mcpServers": {', + ' "my-server": {', + ' "command": "npx",', + ' "args": ["-y", "@modelcontextprotocol/server-my-server"]', + ' }', + ' }', + '}', + '```', + '', + `Config path: ${sanitizePath(configPath) || '(not found)'}`, + ].join('\n') + } + + const lines: string[] = ['### MCP Servers', ''] + + for (const server of servers) { + const icon = server.connected ? '✓' : server.errorLabel ? '✗' : '○' + const statusLabel = server.connected + ? 'connected' + : server.errorLabel + ? 'failed' + : 'not connected' + const colorTag = server.connected ? '' : ' (will connect when the agent uses its tools)' + + lines.push(`**${icon} ${server.name}**`) + lines.push(` Status: ${statusLabel}${!server.connected && !server.errorLabel ? colorTag : ''}`) + lines.push(` Transport: ${server.transport}`) + lines.push( + ` Tools: ${server.toolCount !== null ? String(server.toolCount) : '…'}`, + ) + if (server.errorLabel) { + lines.push(` Error: ${server.errorLabel}`) + } + lines.push('') + } + + if (configPath) { + lines.push(`Config path: ${sanitizePath(configPath)}`) + } + + return lines.join('\n') +} + +/** + * The /mcp command handler — builds a system message and returns it. + */ +export function handleMcpCommand(): { postUserMessage: (prev: any[]) => any[] } { + const report = buildMcpStatusReport() + const formatted = formatMcpStatusForCli(report) + + const postUserMessage = (prev: any[]): any[] => [ + ...prev, + getSystemMessage(formatted), + ] + + return { postUserMessage } +} + +// ============================================================================ +// Private helpers +// ============================================================================ + +/** + * Describe the transport type from an MCP config object. + */ +function describeTransport(config: MCPConfig): string { + if ('command' in config) return 'stdio' + if (config.type === 'http') return 'Streamable HTTP' + if (config.type === 'sse') return 'SSE' + return 'unknown' +} + +/** + * Sanitize a file-system path for display, redacting user-home segments on + * platforms where that isn't already handled by the resolved path. + */ +function sanitizePath(filePath: string): string { + if (!filePath) return '' + // On Unix, the resolved path already uses the actual home dir value, so + // it's safe to display as-is (no $HOME leak). Just return the path. + return filePath +} diff --git a/cli/src/data/slash-commands.ts b/cli/src/data/slash-commands.ts index b1eccce5bf..21ad871d3e 100644 --- a/cli/src/data/slash-commands.ts +++ b/cli/src/data/slash-commands.ts @@ -62,6 +62,12 @@ const ALL_SLASH_COMMANDS: SlashCommand[] = [ description: 'Show local CLI resource usage and terminal tool process IDs', aliases: ['diag', 'processes'], }, + { + id: 'mcp', + label: 'mcp', + description: 'Show configured MCP servers and their connection status', + aliases: ['mcp-servers'], + }, ...(CHATGPT_OAUTH_ENABLED ? [ { diff --git a/common/src/mcp/client.ts b/common/src/mcp/client.ts index 5a5608d57f..fea3590676 100644 --- a/common/src/mcp/client.ts +++ b/common/src/mcp/client.ts @@ -24,6 +24,13 @@ const listToolsCache: Record< ReturnType > = {} +/** + * Synchronously populated map from client ID to tool count, updated when + * listMCPTools resolves. Used by the status API to avoid awaiting a promise + * that may already be cached. + */ +const resolvedToolCounts: Record = {} + /** * Substitutes environment variable references ($VAR_NAME) in a string with their values. * Supports both simple replacement ("$VAR_NAME") and interpolation ("Bearer $VAR_NAME"). @@ -168,7 +175,11 @@ export function listMCPTools( throw new Error(`listTools: client not found with id: ${clientId}`) } if (!listToolsCache[clientId]) { - listToolsCache[clientId] = client.listTools(...args) + const promise = client.listTools(...args) + listToolsCache[clientId] = promise.then((result) => { + resolvedToolCounts[clientId] = result.tools.length + return result + }) } return listToolsCache[clientId] } @@ -231,3 +242,40 @@ export async function callMCPTool( } satisfies ToolResultOutput }) } + +// --------------------------------------------------------------------------- +// Public status API — used by the /mcp CLI command +// --------------------------------------------------------------------------- + +export type McpClientConnectionInfo = { + connected: boolean + /** Number of tools discovered, or null if not yet resolved */ + toolCount: number | null +} + +/** + * Returns the client-id key for a given MCP config, so callers can match + * configured servers against the running-clients map. + */ +export function hashMcpConfig(config: MCPConfig): string { + return hashConfig(config) +} + +/** + * Returns connection info for an MCP server config without initiating any + * new connection. The tool count is populated synchronously only when + * listMCPTools has already resolved for this server. + */ +export function getMCPClientConnectionInfo(config: MCPConfig): McpClientConnectionInfo { + const key = hashConfig(config) + const connected = key in runningClients + const toolCount = key in resolvedToolCounts ? resolvedToolCounts[key] : null + return { connected, toolCount } +} + +/** + * Returns all client-ids that are currently connected. + */ +export function getConnectedMCPClientKeys(): string[] { + return Object.keys(runningClients) +} From 6520c1cc2ebc463a127019e4c9211333ec80e6f3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fortissolu=C3=A7=C3=B5es?= Date: Fri, 17 Jul 2026 14:04:45 -0300 Subject: [PATCH 2/3] feat(cli): add sanitization, truncation, and error tracking for /mcp --- .../commands/__tests__/mcp-command.test.ts | 131 ++++++++++++++++++ cli/src/commands/mcp-command.ts | 12 +- common/src/mcp/client.ts | 95 +++++++++++-- 3 files changed, 221 insertions(+), 17 deletions(-) diff --git a/cli/src/commands/__tests__/mcp-command.test.ts b/cli/src/commands/__tests__/mcp-command.test.ts index 5045f6d95f..468b607e6c 100644 --- a/cli/src/commands/__tests__/mcp-command.test.ts +++ b/cli/src/commands/__tests__/mcp-command.test.ts @@ -9,6 +9,10 @@ import { formatMcpStatusForCli, handleMcpCommand, } from '../mcp-command' +import { + sanitizeErrorForDisplay, + truncateError, +} from '@codebuff/common/mcp/client' import type { RouterParams } from '../command-registry' import type { ChatMessage } from '../../types/chat' @@ -315,3 +319,130 @@ describe('/mcp command does not break other commands', () => { expect(fb!.name).toBe('feedback') }) }) + +// ============================================================================ +// Sanitization and truncation +// ============================================================================ + +describe('sanitizeErrorForDisplay', () => { + test('redacts OpenAI-style secret keys', () => { + const input = 'Error: Invalid API key sk-proj-abcdefghijklmnopqrstuvwxyZ1234567890abcd' + expect(sanitizeErrorForDisplay(input)).not.toContain('sk-proj-abcdefghijklmnopqrstuvwxyZ1234567890abcd') + expect(sanitizeErrorForDisplay(input)).toContain('') + }) + + test('redacts Bearer tokens', () => { + const input = 'Authorization: Bearer abcdefghijklmnopqrstuvwxyz1234567890abcdefghij' + expect(sanitizeErrorForDisplay(input)).not.toContain('Bearer abcdefghijklmnopqrstuvwxyz1234567890abcdefghij') + expect(sanitizeErrorForDisplay(input)).toContain('') + }) + + test('redacts GitHub tokens', () => { + const input = 'Failed to clone: ghp_abcdefghijklmnopqrstuvwxyz1234567890abcdef' + expect(sanitizeErrorForDisplay(input)).not.toContain('ghp_abcdefghijklmnopqrstuvwxyz1234567890abcdef') + expect(sanitizeErrorForDisplay(input)).toContain('') + }) + + test('redacts api_key patterns', () => { + const input = 'Using api_key=abcdefghijklmnopqrstuvwxyz1234567890' + expect(sanitizeErrorForDisplay(input)).not.toContain('api_key=') + expect(sanitizeErrorForDisplay(input)).toContain('') + }) + + test('redacts token= patterns', () => { + const input = 'Setting token=mysecrettokenvalue123456789' + expect(sanitizeErrorForDisplay(input)).not.toContain('token=mysecrettokenvalue123456789') + expect(sanitizeErrorForDisplay(input)).toContain('') + }) + + test('redacts password= patterns', () => { + const input = 'password=hunter2!@#$%^&*' + expect(sanitizeErrorForDisplay(input)).not.toContain('password=hunter2') + expect(sanitizeErrorForDisplay(input)).toContain('') + }) + + test('passes through safe messages unchanged', () => { + const input = 'Process exited with code 1. Failed to start MCP server.' + expect(sanitizeErrorForDisplay(input)).toBe(input) + }) + + test('redacts multiple occurrences', () => { + const input = 'sk-abcdefghijklmnopqrstuvwxyz1 failed, then sk-abcdefghijklmnopqrstuvwxyz2 also failed' + const result = sanitizeErrorForDisplay(input) + expect(result).not.toContain('sk-abcdefghijklmnopqrstuvwxyz1') + expect(result).not.toContain('sk-abcdefghijklmnopqrstuvwxyz2') + // Both should be replaced with the placeholder + expect((result.match(//g) || []).length).toBe(2) + }) +}) + +describe('truncateError', () => { + test('short messages pass through', () => { + const input = 'short error' + expect(truncateError(input, 2000)).toBe(input) + }) + + test('truncates long messages at newline boundary', () => { + const input = 'line one\nline two\nline three\nline four\nline five' + const result = truncateError(input, 20) + expect(result).toContain('… (truncated)') + // Should break at newline boundary + expect(result).toContain('line one') + expect(result).not.toContain('line five') + }) + + test('truncates at space boundary when no newline found', () => { + const input = 'word1 word2 word3 word4 word5 word6' + const result = truncateError(input, 15) + expect(result).toContain('… (truncated)') + }) + + test('truncates at hard limit when no boundary found', () => { + const veryLongWord = 'x'.repeat(5000) + const result = truncateError(veryLongWord, 100) + expect(result.length).toBeLessThan(5000) + expect(result).toContain('… (truncated)') + }) +}) + +describe('formatMcpStatusForCli with error labels', () => { + test('shows failed server with error label', () => { + const output = formatMcpStatusForCli({ + servers: [ + { + name: 'broken', + transport: 'stdio', + connected: false, + toolCount: null, + configPath: '/project/.agents/mcp.json', + errorLabel: 'Connection refused: process exited before initialization', + }, + ], + configPath: '/project/.agents/mcp.json', + }) + expect(output).toContain('✗') + expect(output).toContain('broken') + expect(output).toContain('failed') + expect(output).toContain('Connection refused') + }) + + test('shows sanitized error when error contains credentials', () => { + // The buildMcpStatusReport already applies sanitizeErrorForDisplay + // through getMCPClientConnectionInfo. This test verifies the format + // layer handles sanitized content. + const output = formatMcpStatusForCli({ + servers: [ + { + name: 'leaky', + transport: 'http', + connected: false, + toolCount: null, + configPath: '/project/.agents/mcp.json', + errorLabel: 'Unauthorized: invalid ', + }, + ], + configPath: '/project/.agents/mcp.json', + }) + expect(output).toContain('') + }) +}) diff --git a/cli/src/commands/mcp-command.ts b/cli/src/commands/mcp-command.ts index e26d94b8b5..8039030492 100644 --- a/cli/src/commands/mcp-command.ts +++ b/cli/src/commands/mcp-command.ts @@ -2,12 +2,16 @@ import { loadMCPConfigSync } from '@codebuff/sdk' import { getMCPClientConnectionInfo, hashMcpConfig, + sanitizeErrorForDisplay, + truncateError, } from '@codebuff/common/mcp/client' import { getSystemMessage } from '../utils/message-history' import type { MCPConfig } from '@codebuff/common/types/mcp' +const MAX_ERROR_LENGTH = 2000 + // ============================================================================ // Public API // ============================================================================ @@ -42,10 +46,10 @@ export function buildMcpStatusReport(): { connected: info.connected, toolCount: info.toolCount, configPath: loaded._sourceFilePath, - errorLabel: null, // Runtime errors are logged server-side; the client - // knows a failed connection only when the agent - // attempted to use the server. We still show it as - // connected=false with no error in that case. + errorLabel: + info.errorLabel !== null + ? truncateError(info.errorLabel, MAX_ERROR_LENGTH) + : null, }) } diff --git a/common/src/mcp/client.ts b/common/src/mcp/client.ts index fea3590676..233b2362d7 100644 --- a/common/src/mcp/client.ts +++ b/common/src/mcp/client.ts @@ -31,6 +31,13 @@ const listToolsCache: Record< */ const resolvedToolCounts: Record = {} +/** + * Maps client config hashes to sanitized error messages when a connection + * attempt fails. Cleared on the next successful connection for that key. + * Used by the /mcp CLI command to surface failures without reconnecting. + */ +const connectionErrors: Record = {} + /** * Substitutes environment variable references ($VAR_NAME) in a string with their values. * Supports both simple replacement ("$VAR_NAME") and interpolation ("Bearer $VAR_NAME"). @@ -87,6 +94,58 @@ function hashConfig(config: MCPConfig): string { ) } +// Patterns whose presence in an error string indicates a credential/token +// value that should be redacted. +// +// This is intentionally conservative — patterns known to be safe are +// excluded; everything else that looks like a credential is replaced. +const SENSITIVE_PATTERNS = [ + /sk-[A-Za-z0-9_-]{20,}/g, // OpenAI-style secret keys (sk-proj-xxx, sk-user-xxx) + /ghp_[A-Za-z0-9]{36,}/g, // GitHub personal access tokens + /gho_[A-Za-z0-9]{36,}/g, // GitHub OAuth tokens + /Bearer\s+[A-Za-z0-9._~+/=-]{8,}/g, // Bearer tokens in headers + /api[_-]?key['"]?\s*[:=]\s*['"]?[A-Za-z0-9_/-]{16,}/gi, // api_key=... patterns + /token['"]?\s*[:=]\s*['"]?[A-Za-z0-9_./-]{8,}/gi, // token=... patterns + /secret['"]?\s*[:=]\s*['"]?[A-Za-z0-9_./-]{8,}/gi, // secret=... patterns + /password['"]?\s*[:=]\s*['"]?[A-Za-z0-9_@!$%&*+-]{4,}/gi, // password=... patterns +] + +export const REDACTED_PLACEHOLDER = '' + +/** + * Sanitize a string for display by redacting any value that matches a + * known credential pattern. Safe to call on user-facing error messages + * and config values. + * + * This is a best-effort heuristic, not a cryptographic guarantee. If no + * pattern matches, the value passes through unchanged. + */ +export function sanitizeErrorForDisplay(input: string): string { + let result = input + for (const pattern of SENSITIVE_PATTERNS) { + result = result.replace(pattern, REDACTED_PLACEHOLDER) + } + return result +} + +/** + * Truncate a string at the last safe boundary before `maxLength` chars. + * Appends a truncation indicator. + */ +export const TRUNCATION_INDICATOR = '… (truncated)' + +export function truncateError( + input: string, + maxLength: number = 2000, +): string { + if (input.length <= maxLength) return input + // Try to break at a newline or space boundary before maxLength + const cutoff = input.lastIndexOf('\n', maxLength) + const breakAt = cutoff > 0 ? cutoff : input.lastIndexOf(' ', maxLength) + const endAt = breakAt > 0 ? breakAt : maxLength + return input.slice(0, endAt) + '\n' + TRUNCATION_INDICATOR +} + export async function getMCPClient(config: MCPConfig): Promise { let key = hashConfig(config) if (key in runningClients) { @@ -147,19 +206,26 @@ export async function getMCPClient(config: MCPConfig): Promise { await client.connect(transport) } catch (error) { const baseMessage = getErrorObject(error).message - if (config.type === 'stdio') { - const commandStr = [config.command, ...(config.args ?? [])].join(' ') - const detail = stderrBuffer.trim() - throw new Error( - `${baseMessage}. Failed to start MCP server via \`${commandStr}\`. ` + - `Ensure the command is installed and runnable (e.g. an up-to-date ` + - `node/npm/npx, or python/uvx) and that any required env vars are set.` + - (detail ? `\nServer stderr:\n${detail}` : ''), + const enrichedError = (() => { + if (config.type === 'stdio') { + const commandStr = [config.command, ...(config.args ?? [])].join(' ') + const detail = stderrBuffer.trim() + return new Error( + `${baseMessage}. Failed to start MCP server via \`${commandStr}\`. ` + + `Ensure the command is installed and runnable (e.g. an up-to-date ` + + `node/npm/npx, or python/uvx) and that any required env vars are set.` + + (detail ? `\nServer stderr:\n${detail}` : ''), + ) + } + return new Error( + `${baseMessage}. Failed to connect to MCP server at ${config.url}.`, ) - } - throw new Error( - `${baseMessage}. Failed to connect to MCP server at ${config.url}.`, - ) + })() + + // Store a sanitized version for the /mcp status command without + // breaking the existing throw contract + connectionErrors[key] = sanitizeErrorForDisplay(enrichedError.message) + throw enrichedError } runningClients[key] = client @@ -251,6 +317,8 @@ export type McpClientConnectionInfo = { connected: boolean /** Number of tools discovered, or null if not yet resolved */ toolCount: number | null + /** Sanitized error message from the last connection attempt, or null */ + errorLabel: string | null } /** @@ -270,7 +338,8 @@ export function getMCPClientConnectionInfo(config: MCPConfig): McpClientConnecti const key = hashConfig(config) const connected = key in runningClients const toolCount = key in resolvedToolCounts ? resolvedToolCounts[key] : null - return { connected, toolCount } + const errorLabel = connected ? null : (key in connectionErrors ? connectionErrors[key] : null) + return { connected, toolCount, errorLabel } } /** From 4da1189dc4ff7dc6a023453c45c33bf9e01f515b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fortissolu=C3=A7=C3=B5es?= Date: Fri, 17 Jul 2026 14:23:29 -0300 Subject: [PATCH 3/3] feat(cli): add /mcp list parser, /mcp unknown help, cross-platform path sanitization, and expanded credential redaction - Switch /mcp from defineCommand to defineCommandWithArgs for subcommand parsing - Add /mcp unknown subcommand handling that shows inline help with usage - Add /mcp list to help banner (Tips section) - Implement proper cross-platform sanitizePath replacing home dir with ~ - Add URL credentials, Basic auth, github_pat_, access_token, refresh_token, client_secret, passwd, and Authorization header sanitization patterns - Remove unused hashMcpConfig and getConnectedMCPClientKeys exports - Add parser integration tests, path sanitization tests, lifecycle tests, and expanded sanitization pattern tests - Improve JSDoc for McpClientConnectionInfo and getMCPClientConnectionInfo --- .../commands/__tests__/mcp-command.test.ts | 189 +++++++++++++++++- cli/src/commands/command-registry.ts | 6 +- cli/src/commands/mcp-command.ts | 72 ++++++- cli/src/components/help-banner.tsx | 5 +- common/src/mcp/client.ts | 49 +++-- 5 files changed, 285 insertions(+), 36 deletions(-) diff --git a/cli/src/commands/__tests__/mcp-command.test.ts b/cli/src/commands/__tests__/mcp-command.test.ts index 468b607e6c..49473c04b3 100644 --- a/cli/src/commands/__tests__/mcp-command.test.ts +++ b/cli/src/commands/__tests__/mcp-command.test.ts @@ -1,7 +1,5 @@ import { describe, test, expect, beforeEach, afterEach, mock } from 'bun:test' -import fs from 'fs' import os from 'os' -import path from 'path' import { findCommand } from '../command-registry' import { @@ -47,9 +45,9 @@ describe('/mcp command registration', () => { expect(command!.name).toBe('mcp') }) - test('command is defined with defineCommand (no args)', () => { + test('command is defined with defineCommandWithArgs (accepts args)', () => { const command = findCommand('mcp') - expect(command!.acceptsArgs).toBe(false) + expect(command!.acceptsArgs).toBe(true) }) test('command handler renders a system message via handleMcpCommand', () => { @@ -65,6 +63,67 @@ describe('/mcp command registration', () => { }) }) +describe('/mcp parser integration', () => { + test('/mcp triggers handler via parseCommandInput', () => { + const { parseCommandInput } = require('../router-utils') + const result = parseCommandInput('/mcp') + expect(result).not.toBeNull() + expect(result!.command).toBe('mcp') + expect(result!.args).toBe('') + }) + + test('/mcp list triggers handler with args="list"', () => { + const { parseCommandInput } = require('../router-utils') + const result = parseCommandInput('/mcp list') + expect(result).not.toBeNull() + expect(result!.command).toBe('mcp') + expect(result!.args).toBe('list') + }) + + test('/mcp unknown triggers handler with args="unknown"', () => { + const { parseCommandInput } = require('../router-utils') + const result = parseCommandInput('/mcp unknown') + expect(result).not.toBeNull() + expect(result!.command).toBe('mcp') + expect(result!.args).toBe('unknown') + }) + + test('/mcp list shows same report as /mcp', () => { + mockMcpConfig = { + mcpServers: { + github: { + type: 'stdio', + command: 'npx', + args: ['-y', '@modelcontextprotocol/server-github'], + env: {}, + }, + }, + _sourceFilePath: '/home/user/project/.agents/mcp.json', + } + + const noArgs = handleMcpCommand() + const withList = handleMcpCommand('list') + + const prev: any[] = [{ id: '1', variant: 'user', content: '/mcp', timestamp: '' }] + const resultNoArgs = noArgs.postUserMessage([...prev]) + const resultWithList = withList.postUserMessage([...prev]) + + // Both should contain the server info, not help text + expect(resultNoArgs[1].content).toContain('github') + expect(resultWithList[1].content).toContain('github') + }) + + test('/mcp unknown shows help text', () => { + const { postUserMessage } = handleMcpCommand('unknown') + const prev: any[] = [{ id: '1', variant: 'user', content: '/mcp unknown', timestamp: '' }] + const result = postUserMessage([...prev]) + + expect(result[1].content).toContain('Usage:') + expect(result[1].content).toContain('/mcp') + expect(result[1].content).toContain('read-only') + }) +}) + describe('buildMcpStatusReport', () => { beforeEach(() => { mockMcpConfig = { mcpServers: {}, _sourceFilePath: '' } @@ -374,6 +433,42 @@ describe('sanitizeErrorForDisplay', () => { // Both should be replaced with the placeholder expect((result.match(//g) || []).length).toBe(2) }) + + test('redacts GitHub fine-grained PATs (github_pat_)', () => { + const input = 'Error: token github_pat_abcdefghijklmnopqrstuvwxyz1234567890abcdefghijklmnopqrstuvwxyz1 is invalid' + expect(sanitizeErrorForDisplay(input)).not.toContain('github_pat_') + expect(sanitizeErrorForDisplay(input)).toContain('') + }) + + test('redacts Basic auth tokens', () => { + const input = 'Authorization: Basic QWxhZGRpbjpvcGVuIHNlc2FtZQ==' + expect(sanitizeErrorForDisplay(input)).not.toContain('Basic QWxhZGRpbjpvcGVuIHNlc2FtZQ==') + expect(sanitizeErrorForDisplay(input)).toContain('') + }) + + test('redacts access_token patterns', () => { + const input = 'Using access_token=gho_abcdefghijklmnopqrstuvwxyz1234567890abcdef' + expect(sanitizeErrorForDisplay(input)).not.toContain('access_token=gho_abcdefghijklmnopqrstuvwxyz1234567890abcdef') + expect(sanitizeErrorForDisplay(input)).toContain('') + }) + + test('redacts refresh_token patterns', () => { + const input = 'refresh_token=rt_abcdefghijklmnopqrstuvwxyz1234567890abcdef' + expect(sanitizeErrorForDisplay(input)).not.toContain('refresh_token=rt_abcdefghijklmnopqrstuvwxyz1234567890abcdef') + expect(sanitizeErrorForDisplay(input)).toContain('') + }) + + test('redacts client_secret patterns', () => { + const input = 'client_secret=cs_abcdefghijklmnopqrstuvwxyz1234567890abcdef' + expect(sanitizeErrorForDisplay(input)).not.toContain('client_secret=cs_abcdefghijklmnopqrstuvwxyz1234567890abcdef') + expect(sanitizeErrorForDisplay(input)).toContain('') + }) + + test('redacts URL credentials (user:password@host)', () => { + const input = 'Error connecting to https://admin:supersecret@mysql.example.com:3306/db' + expect(sanitizeErrorForDisplay(input)).not.toContain('admin:supersecret@') + expect(sanitizeErrorForDisplay(input)).toContain('') + }) }) describe('truncateError', () => { @@ -446,3 +541,89 @@ describe('formatMcpStatusForCli with error labels', () => { expect(output).toContain('') }) }) + +// ============================================================================ +// Path sanitization +// ============================================================================ + +describe('sanitizePath (cross-platform home dir redaction)', () => { + // sanitizePath is private; we test it indirectly via formatMcpStatusForCli + // by verifying the config path is redacted when it contains the home dir. + + const originalHomedir = os.homedir() + + test('config path is redacted via formatMcpStatusForCli', () => { + const output = formatMcpStatusForCli({ + servers: [ + { + name: 'test', + transport: 'stdio', + connected: false, + toolCount: null, + configPath: originalHomedir + '/project/.agents/mcp.json', + errorLabel: null, + }, + ], + configPath: originalHomedir + '/project/.agents/mcp.json', + }) + expect(output).toContain('~/project/.agents/mcp.json') + expect(output).not.toContain(originalHomedir) + }) + + test('non-home paths appear as-is', () => { + const output = formatMcpStatusForCli({ + servers: [], + configPath: '/etc/project/mcp.json', + }) + expect(output).toContain('/etc/project/mcp.json') + }) +}) + +// ============================================================================ +// Connection lifecycle (connection/disconnection error tracking) +// ============================================================================ + +describe('MCP connection lifecycle error tracking', () => { + const { getMCPClientConnectionInfo } = require('@codebuff/common/mcp/client') + + test('getMCPClientConnectionInfo returns status for a server', () => { + const info = getMCPClientConnectionInfo({ + type: 'stdio' as const, + command: 'echo', + args: ['hello'], + env: {}, + }) + + // Returns a single info object (not an array) + expect(info).toHaveProperty('connected') + expect(info).toHaveProperty('toolCount') + expect(info).toHaveProperty('errorLabel') + }) + + test('getMCPClientConnectionInfo handles disconnected server', () => { + const info = getMCPClientConnectionInfo({ + type: 'stdio' as const, + command: 'never-connects', + args: [], + env: {}, + }) + + expect(info.connected).toBe(false) + // toolCount should be null since it was never resolved + expect(info.toolCount).toBeNull() + }) + + test('error labels are null for unknown servers', () => { + // A server that was never tried to connect should have no error + const info = getMCPClientConnectionInfo({ + type: 'stdio' as const, + command: 'unknown-command', + args: [], + env: {}, + }) + + // A server that was never attempted will have errorLabel as null + // (it's keyed by hashConfig, so if never connected, no error entry) + expect(info.errorLabel).toBeNull() + }) +}) diff --git a/cli/src/commands/command-registry.ts b/cli/src/commands/command-registry.ts index 8a32241119..ca1fb14057 100644 --- a/cli/src/commands/command-registry.ts +++ b/cli/src/commands/command-registry.ts @@ -224,11 +224,11 @@ const ALL_COMMANDS: CommandDefinition[] = [ clearInput(params) }, }), - defineCommand({ + defineCommandWithArgs({ name: 'mcp', aliases: ['mcp-servers'], - handler: (params) => { - const { postUserMessage } = handleMcpCommand() + handler: (params, args) => { + const { postUserMessage } = handleMcpCommand(args) params.setMessages((prev) => postUserMessage(prev)) params.saveToHistory(params.inputValue.trim()) clearInput(params) diff --git a/cli/src/commands/mcp-command.ts b/cli/src/commands/mcp-command.ts index 8039030492..b699187bc5 100644 --- a/cli/src/commands/mcp-command.ts +++ b/cli/src/commands/mcp-command.ts @@ -1,11 +1,13 @@ import { loadMCPConfigSync } from '@codebuff/sdk' import { getMCPClientConnectionInfo, - hashMcpConfig, sanitizeErrorForDisplay, truncateError, } from '@codebuff/common/mcp/client' +import os from 'os' +import path from 'path' + import { getSystemMessage } from '../utils/message-history' import type { MCPConfig } from '@codebuff/common/types/mcp' @@ -38,7 +40,6 @@ export function buildMcpStatusReport(): { for (const [name, config] of Object.entries(loaded.mcpServers)) { const info = getMCPClientConnectionInfo(config) - const hash = hashMcpConfig(config) servers.push({ name, @@ -122,8 +123,39 @@ export function formatMcpStatusForCli(report: { /** * The /mcp command handler — builds a system message and returns it. + * + * Accepts an optional `args` string from the parser: + * - `''` or `'list'` → show status report + * - anything else → show brief help with accepted subcommands */ -export function handleMcpCommand(): { postUserMessage: (prev: any[]) => any[] } { +export function handleMcpCommand(args?: string): { postUserMessage: (prev: any[]) => any[] } { + // '/mcp unknown' or other unknown subcommand → show help + if (args && args.trim() && args.trim() !== 'list') { + const helpText = [ + '### /mcp — MCP server status', + '', + '**Usage:**', + '', + ' `/mcp` Show configured MCP servers and connection status', + ' `/mcp list` Show the same status report', + '', + '**About:**', + '', + 'This command reads from `.agents/mcp.json` in your project, parent,', + 'or home directory. It shows which servers are configured, their', + 'transport, connection state, and discovered tools.', + '', + 'This is a read-only command. No connections are initiated just to', + 'render the status.', + ].join('\n') + + const postUserMessage = (prev: any[]): any[] => [ + ...prev, + getSystemMessage(helpText), + ] + return { postUserMessage } + } + const report = buildMcpStatusReport() const formatted = formatMcpStatusForCli(report) @@ -150,12 +182,36 @@ function describeTransport(config: MCPConfig): string { } /** - * Sanitize a file-system path for display, redacting user-home segments on - * platforms where that isn't already handled by the resolved path. + * Sanitize a file-system path for display by replacing the user's home + * directory with `~` on any platform (Linux, macOS, Windows). + * + * Examples: + * /home/alice/project/.agents/mcp.json → ~/project/.agents/mcp.json + * /Users/alice/project/.agents/mcp.json → ~/project/.agents/mcp.json + * C:\Users\Alice\project\.agents\mcp.json → ~\project\.agents\mcp.json */ function sanitizePath(filePath: string): string { if (!filePath) return '' - // On Unix, the resolved path already uses the actual home dir value, so - // it's safe to display as-is (no $HOME leak). Just return the path. - return filePath + + const homeDir = os.homedir() + + if (!homeDir) return filePath + + // Normalize the file path first so separators match + const normalizedPath = path.normalize(filePath) + const normalizedHome = path.normalize(homeDir) + + if (normalizedPath === normalizedHome) return '~' + + // Check if the path starts with the home directory + const prefixHome = normalizedHome.endsWith(path.sep) + ? normalizedHome + : normalizedHome + path.sep + + if (normalizedPath.startsWith(prefixHome)) { + const relativePart = normalizedPath.slice(prefixHome.length) + return '~' + path.sep + relativePart + } + + return normalizedPath } diff --git a/cli/src/components/help-banner.tsx b/cli/src/components/help-banner.tsx index 5d60f1aa87..f7c91982ef 100644 --- a/cli/src/components/help-banner.tsx +++ b/cli/src/components/help-banner.tsx @@ -84,7 +84,10 @@ export const HelpBanner = () => { )} - Use @ to reference agents to spawn or files to read + Use @ to reference agents to spawn or files to read + + + /mcp — check MCP server status and tool counts Drag to select text — it copies automatically (or click ⎘ on a diff --git a/common/src/mcp/client.ts b/common/src/mcp/client.ts index 233b2362d7..718d5a70dc 100644 --- a/common/src/mcp/client.ts +++ b/common/src/mcp/client.ts @@ -99,15 +99,26 @@ function hashConfig(config: MCPConfig): string { // // This is intentionally conservative — patterns known to be safe are // excluded; everything else that looks like a credential is replaced. +// URL regex for sanitizing user:password@host patterns +const URL_CREDENTIALS_PATTERN = /https?:\/\/[A-Za-z0-9_.~-]+:[A-Za-z0-9_.~!@#$%^&*()_+\-={}[\]\\|;:',.<>/?]+@/g + const SENSITIVE_PATTERNS = [ /sk-[A-Za-z0-9_-]{20,}/g, // OpenAI-style secret keys (sk-proj-xxx, sk-user-xxx) /ghp_[A-Za-z0-9]{36,}/g, // GitHub personal access tokens /gho_[A-Za-z0-9]{36,}/g, // GitHub OAuth tokens + /github_pat_[A-Za-z0-9]{50,}/g, // GitHub fine-grained PATs /Bearer\s+[A-Za-z0-9._~+/=-]{8,}/g, // Bearer tokens in headers + /(^|\n)\s*Authorization['"]?\s*[:=]\s*.+/gi, // Authorization header lines + /Basic\s+[A-Za-z0-9+/=]{8,}/g, // Basic auth tokens /api[_-]?key['"]?\s*[:=]\s*['"]?[A-Za-z0-9_/-]{16,}/gi, // api_key=... patterns + /access_token['"]?\s*[:=]\s*['"]?[A-Za-z0-9_./-]{8,}/gi, // access_token patterns + /refresh_token['"]?\s*[:=]\s*['"]?[A-Za-z0-9_./-]{8,}/gi, // refresh_token patterns + /client_secret['"]?\s*[:=]\s*['"]?[A-Za-z0-9_./-]{8,}/gi, // client_secret patterns /token['"]?\s*[:=]\s*['"]?[A-Za-z0-9_./-]{8,}/gi, // token=... patterns /secret['"]?\s*[:=]\s*['"]?[A-Za-z0-9_./-]{8,}/gi, // secret=... patterns /password['"]?\s*[:=]\s*['"]?[A-Za-z0-9_@!$%&*+-]{4,}/gi, // password=... patterns + /passwd['"]?\s*[:=]\s*['"]?[A-Za-z0-9_@!$%&*+-]{4,}/gi, // passwd=... patterns + URL_CREDENTIALS_PATTERN, // https://user:pass@host URLs ] export const REDACTED_PLACEHOLDER = '' @@ -313,26 +324,31 @@ export async function callMCPTool( // Public status API — used by the /mcp CLI command // --------------------------------------------------------------------------- +/** + * Runtime status snapshot for a single MCP server config. + * + * All fields are derived from module-level state that was already populated + * by normal agent operation — no new connections are initiated. + */ export type McpClientConnectionInfo = { + /** Whether the client is currently connected */ connected: boolean - /** Number of tools discovered, or null if not yet resolved */ + /** Number of discovered tools, or null if the tool list hasn't resolved yet */ toolCount: number | null - /** Sanitized error message from the last connection attempt, or null */ + /** Sanitized error message from the last failed connection, or null */ errorLabel: string | null } /** - * Returns the client-id key for a given MCP config, so callers can match - * configured servers against the running-clients map. - */ -export function hashMcpConfig(config: MCPConfig): string { - return hashConfig(config) -} - -/** - * Returns connection info for an MCP server config without initiating any - * new connection. The tool count is populated synchronously only when - * listMCPTools has already resolved for this server. + * Returns a readonly snapshot of the connection status for a single MCP + * server configuration, without initiating any new connection. + * + * - `connected` — checked against the live `runningClients` map + * - `toolCount` — populated lazily from the resolved `listTools` cache + * - `errorLabel` — populated from the last failed `getMCPClient` attempt + * + * The returned object is a plain value type. Callers cannot mutate internal + * module state through it. */ export function getMCPClientConnectionInfo(config: MCPConfig): McpClientConnectionInfo { const key = hashConfig(config) @@ -341,10 +357,3 @@ export function getMCPClientConnectionInfo(config: MCPConfig): McpClientConnecti const errorLabel = connected ? null : (key in connectionErrors ? connectionErrors[key] : null) return { connected, toolCount, errorLabel } } - -/** - * Returns all client-ids that are currently connected. - */ -export function getConnectedMCPClientKeys(): string[] { - return Object.keys(runningClients) -}