diff --git a/src/mcp/tools/fileSystemReadDirectoryTool.ts b/src/mcp/tools/fileSystemReadDirectoryTool.ts index e1be2a2af..242da5191 100644 --- a/src/mcp/tools/fileSystemReadDirectoryTool.ts +++ b/src/mcp/tools/fileSystemReadDirectoryTool.ts @@ -4,6 +4,7 @@ import type { McpServer } from '@modelcontextprotocol/sdk/server/mcp.js'; import type { CallToolResult } from '@modelcontextprotocol/sdk/types.js'; import { z } from 'zod'; import { logger } from '../../shared/logger.js'; +import { buildMcpToolErrorResponse, buildMcpToolSuccessResponse } from './mcpToolRuntime.js'; /** * Register file system directory listing tool @@ -28,41 +29,19 @@ export const registerFileSystemReadDirectoryTool = (mcpServer: McpServer) => { // Ensure path is absolute if (!path.isAbsolute(directoryPath)) { - return { - isError: true, - content: [ - { - type: 'text', - text: `Error: Path must be absolute. Received: ${directoryPath}`, - }, - ], - }; + return buildMcpToolErrorResponse([`Error: Path must be absolute. Received: ${directoryPath}`]); } // Check if directory exists try { const stats = await fs.stat(directoryPath); if (!stats.isDirectory()) { - return { - isError: true, - content: [ - { - type: 'text', - text: `Error: The specified path is not a directory: ${directoryPath}. Use file_system_read_file for files.`, - }, - ], - }; + return buildMcpToolErrorResponse([ + `Error: The specified path is not a directory: ${directoryPath}. Use file_system_read_file for files.`, + ]); } } catch { - return { - isError: true, - content: [ - { - type: 'text', - text: `Error: Directory not found at path: ${directoryPath}`, - }, - ], - }; + return buildMcpToolErrorResponse([`Error: Directory not found at path: ${directoryPath}`]); } // Read directory contents @@ -71,29 +50,12 @@ export const registerFileSystemReadDirectoryTool = (mcpServer: McpServer) => { .map((entry) => `${entry.isDirectory() ? '[DIR]' : '[FILE]'} ${entry.name}`) .join('\n'); - return { - content: [ - { - type: 'text', - text: `Contents of ${directoryPath}:`, - }, - { - type: 'text', - text: formatted || '(empty directory)', - }, - ], - }; + return buildMcpToolSuccessResponse([`Contents of ${directoryPath}:`, formatted || '(empty directory)']); } catch (error) { logger.error(`Error in file_system_read_directory tool: ${error}`); - return { - isError: true, - content: [ - { - type: 'text', - text: `Error listing directory: ${error instanceof Error ? error.message : String(error)}`, - }, - ], - }; + return buildMcpToolErrorResponse([ + `Error listing directory: ${error instanceof Error ? error.message : String(error)}`, + ]); } }, ); diff --git a/src/mcp/tools/fileSystemReadFileTool.ts b/src/mcp/tools/fileSystemReadFileTool.ts index 4101aa10e..e6f331086 100644 --- a/src/mcp/tools/fileSystemReadFileTool.ts +++ b/src/mcp/tools/fileSystemReadFileTool.ts @@ -6,6 +6,7 @@ import { z } from 'zod'; import type { SuspiciousFileResult } from '../../core/security/securityCheck.js'; import { createSecretLintConfig, runSecretLint } from '../../core/security/workers/securityCheckWorker.js'; import { logger } from '../../shared/logger.js'; +import { buildMcpToolErrorResponse, buildMcpToolSuccessResponse } from './mcpToolRuntime.js'; /** * Register file system read file tool with security checks @@ -30,44 +31,22 @@ export const registerFileSystemReadFileTool = (mcpServer: McpServer) => { // Ensure path is absolute if (!path.isAbsolute(filePath)) { - return { - isError: true, - content: [ - { - type: 'text', - text: `Error: Path must be absolute. Received: ${filePath}`, - }, - ], - }; + return buildMcpToolErrorResponse([`Error: Path must be absolute. Received: ${filePath}`]); } // Check if file exists try { await fs.access(filePath); } catch { - return { - isError: true, - content: [ - { - type: 'text', - text: `Error: File not found at path: ${filePath}`, - }, - ], - }; + return buildMcpToolErrorResponse([`Error: File not found at path: ${filePath}`]); } // Check if it's a directory const stats = await fs.stat(filePath); if (stats.isDirectory()) { - return { - isError: true, - content: [ - { - type: 'text', - text: `Error: The specified path is a directory, not a file: ${filePath}. Use file_system_read_directory for directories.`, - }, - ], - }; + return buildMcpToolErrorResponse([ + `Error: The specified path is a directory, not a file: ${filePath}. Use file_system_read_directory for directories.`, + ]); } // Read file content @@ -79,40 +58,17 @@ export const registerFileSystemReadFileTool = (mcpServer: McpServer) => { // If security check found issues, block the file if (securityCheckResult !== null) { - return { - isError: true, - content: [ - { - type: 'text', - text: `Error: Security check failed. The file at ${filePath} may contain sensitive information.`, - }, - ], - }; + return buildMcpToolErrorResponse([ + `Error: Security check failed. The file at ${filePath} may contain sensitive information.`, + ]); } - return { - content: [ - { - type: 'text', - text: `Content of ${filePath}:`, - }, - { - type: 'text', - text: fileContent, - }, - ], - }; + return buildMcpToolSuccessResponse([`Content of ${filePath}:`, fileContent]); } catch (error) { logger.error(`Error in file_system_read_file tool: ${error}`); - return { - isError: true, - content: [ - { - type: 'text', - text: `Error reading file: ${error instanceof Error ? error.message : String(error)}`, - }, - ], - }; + return buildMcpToolErrorResponse([ + `Error reading file: ${error instanceof Error ? error.message : String(error)}`, + ]); } }, ); diff --git a/src/mcp/tools/grepRepomixOutputTool.ts b/src/mcp/tools/grepRepomixOutputTool.ts index 20ff89cc4..27f37a6a8 100644 --- a/src/mcp/tools/grepRepomixOutputTool.ts +++ b/src/mcp/tools/grepRepomixOutputTool.ts @@ -3,7 +3,7 @@ import type { McpServer } from '@modelcontextprotocol/sdk/server/mcp.js'; import type { CallToolResult } from '@modelcontextprotocol/sdk/types.js'; import { z } from 'zod'; import { logger } from '../../shared/logger.js'; -import { getOutputFilePath } from './mcpToolRuntime.js'; +import { buildMcpToolErrorResponse, buildMcpToolSuccessResponse, getOutputFilePath } from './mcpToolRuntime.js'; /** * Search options for grep functionality @@ -83,29 +83,17 @@ export const registerGrepRepomixOutputTool = (mcpServer: McpServer) => { const filePath = getOutputFilePath(outputId); if (!filePath) { - return { - isError: true, - content: [ - { - type: 'text', - text: `Error: Output file with ID ${outputId} not found. The output file may have been deleted or the ID is invalid.`, - }, - ], - }; + return buildMcpToolErrorResponse([ + `Error: Output file with ID ${outputId} not found. The output file may have been deleted or the ID is invalid.`, + ]); } try { await fs.access(filePath); } catch (error) { - return { - isError: true, - content: [ - { - type: 'text', - text: `Error: Output file does not exist at path: ${filePath}. The temporary file may have been cleaned up.`, - }, - ], - }; + return buildMcpToolErrorResponse([ + `Error: Output file does not exist at path: ${filePath}. The temporary file may have been cleaned up.`, + ]); } const content = await fs.readFile(filePath, 'utf8'); @@ -125,51 +113,24 @@ export const registerGrepRepomixOutputTool = (mcpServer: McpServer) => { ignoreCase, }); } catch (error) { - return { - isError: true, - content: [ - { - type: 'text', - text: `Error: ${error instanceof Error ? error.message : String(error)}`, - }, - ], - }; + return buildMcpToolErrorResponse([`Error: ${error instanceof Error ? error.message : String(error)}`]); } if (searchResult.matches.length === 0) { - return { - content: [ - { - type: 'text', - text: `No matches found for pattern "${pattern}" in Repomix output file (ID: ${outputId}).`, - }, - ], - }; + return buildMcpToolSuccessResponse([ + `No matches found for pattern "${pattern}" in Repomix output file (ID: ${outputId}).`, + ]); } - return { - content: [ - { - type: 'text', - text: `Found ${searchResult.matches.length} match(es) for pattern "${pattern}" in Repomix output file (ID: ${outputId}):`, - }, - { - type: 'text', - text: searchResult.formattedOutput.join('\n'), - }, - ], - }; + return buildMcpToolSuccessResponse([ + `Found ${searchResult.matches.length} match(es) for pattern "${pattern}" in Repomix output file (ID: ${outputId}):`, + searchResult.formattedOutput.join('\n'), + ]); } catch (error) { logger.error(`Error in grep_repomix_output: ${error}`); - return { - isError: true, - content: [ - { - type: 'text', - text: `Error searching Repomix output: ${error instanceof Error ? error.message : String(error)}`, - }, - ], - }; + return buildMcpToolErrorResponse([ + `Error searching Repomix output: ${error instanceof Error ? error.message : String(error)}`, + ]); } }, ); diff --git a/src/mcp/tools/mcpToolRuntime.ts b/src/mcp/tools/mcpToolRuntime.ts index cd482631b..954dbff68 100644 --- a/src/mcp/tools/mcpToolRuntime.ts +++ b/src/mcp/tools/mcpToolRuntime.ts @@ -190,3 +190,28 @@ export const formatToolError = (error: unknown): CallToolResult => { ], }; }; + +/** + * Creates a successful MCP tool response with type safety + */ +export const buildMcpToolSuccessResponse = (messages: string[]): CallToolResult => { + return { + content: messages.map((message) => ({ + type: 'text' as const, + text: message, + })), + }; +}; + +/** + * Creates an error MCP tool response with type safety + */ +export const buildMcpToolErrorResponse = (errorMessages: string[]): CallToolResult => { + return { + isError: true, + content: errorMessages.map((message) => ({ + type: 'text' as const, + text: message, + })), + }; +}; diff --git a/src/mcp/tools/packCodebaseTool.ts b/src/mcp/tools/packCodebaseTool.ts index 36d35a60a..703d55b30 100644 --- a/src/mcp/tools/packCodebaseTool.ts +++ b/src/mcp/tools/packCodebaseTool.ts @@ -4,7 +4,12 @@ import type { CallToolResult } from '@modelcontextprotocol/sdk/types.js'; import { z } from 'zod'; import { runCli } from '../../cli/cliRun.js'; import type { CliOptions } from '../../cli/types.js'; -import { createToolWorkspace, formatToolError, formatToolResponse } from './mcpToolRuntime.js'; +import { + buildMcpToolErrorResponse, + createToolWorkspace, + formatToolError, + formatToolResponse, +} from './mcpToolRuntime.js'; export const registerPackCodebaseTool = (mcpServer: McpServer) => { mcpServer.tool( @@ -65,22 +70,7 @@ export const registerPackCodebaseTool = (mcpServer: McpServer) => { const result = await runCli(['.'], directory, cliOptions); if (!result) { - return { - isError: true, - content: [ - { - type: 'text', - text: JSON.stringify( - { - success: false, - error: 'Failed to return a result', - }, - null, - 2, - ), - }, - ], - }; + return buildMcpToolErrorResponse(['Failed to return a result']); } // Extract metrics information from the pack result diff --git a/src/mcp/tools/packRemoteRepositoryTool.ts b/src/mcp/tools/packRemoteRepositoryTool.ts index 620904ebb..d01db675b 100644 --- a/src/mcp/tools/packRemoteRepositoryTool.ts +++ b/src/mcp/tools/packRemoteRepositoryTool.ts @@ -4,7 +4,12 @@ import type { CallToolResult } from '@modelcontextprotocol/sdk/types.js'; import { z } from 'zod'; import { runCli } from '../../cli/cliRun.js'; import type { CliOptions } from '../../cli/types.js'; -import { createToolWorkspace, formatToolError, formatToolResponse } from './mcpToolRuntime.js'; +import { + buildMcpToolErrorResponse, + createToolWorkspace, + formatToolError, + formatToolResponse, +} from './mcpToolRuntime.js'; export const registerPackRemoteRepositoryTool = (mcpServer: McpServer) => { mcpServer.tool( @@ -70,15 +75,7 @@ export const registerPackRemoteRepositoryTool = (mcpServer: McpServer) => { const result = await runCli(['.'], process.cwd(), cliOptions); if (!result) { - return { - isError: true, - content: [ - { - type: 'text', - text: 'Failed to return a result', - }, - ], - }; + return buildMcpToolErrorResponse(['Failed to return a result']); } // Extract metrics information from the pack result diff --git a/src/mcp/tools/readRepomixOutputTool.ts b/src/mcp/tools/readRepomixOutputTool.ts index 21c717ef6..bc5b0450f 100644 --- a/src/mcp/tools/readRepomixOutputTool.ts +++ b/src/mcp/tools/readRepomixOutputTool.ts @@ -3,7 +3,7 @@ import type { McpServer } from '@modelcontextprotocol/sdk/server/mcp.js'; import type { CallToolResult } from '@modelcontextprotocol/sdk/types.js'; import { z } from 'zod'; import { logger } from '../../shared/logger.js'; -import { getOutputFilePath } from './mcpToolRuntime.js'; +import { buildMcpToolErrorResponse, buildMcpToolSuccessResponse, getOutputFilePath } from './mcpToolRuntime.js'; /** * Register the tool to read Repomix output files @@ -37,30 +37,18 @@ export const registerReadRepomixOutputTool = (mcpServer: McpServer) => { // Get the file path from the registry const filePath = getOutputFilePath(outputId); if (!filePath) { - return { - isError: true, - content: [ - { - type: 'text', - text: `Error: Output file with ID ${outputId} not found. The output file may have been deleted or the ID is invalid.`, - }, - ], - }; + return buildMcpToolErrorResponse([ + `Error: Output file with ID ${outputId} not found. The output file may have been deleted or the ID is invalid.`, + ]); } // Check if the file exists try { await fs.access(filePath); } catch (error) { - return { - isError: true, - content: [ - { - type: 'text', - text: `Error: Output file does not exist at path: ${filePath}. The temporary file may have been cleaned up.`, - }, - ], - }; + return buildMcpToolErrorResponse([ + `Error: Output file does not exist at path: ${filePath}. The temporary file may have been cleaned up.`, + ]); } // Read the file content @@ -70,15 +58,9 @@ export const registerReadRepomixOutputTool = (mcpServer: McpServer) => { if (startLine !== undefined || endLine !== undefined) { // Validate that startLine is less than or equal to endLine when both are provided if (startLine !== undefined && endLine !== undefined && startLine > endLine) { - return { - isError: true, - content: [ - { - type: 'text', - text: `Error: Start line (${startLine}) cannot be greater than end line (${endLine}).`, - }, - ], - }; + return buildMcpToolErrorResponse([ + `Error: Start line (${startLine}) cannot be greater than end line (${endLine}).`, + ]); } const lines = content.split('\n'); @@ -86,43 +68,23 @@ export const registerReadRepomixOutputTool = (mcpServer: McpServer) => { const end = endLine ? Math.min(lines.length, endLine) : lines.length; if (start >= lines.length) { - return { - isError: true, - content: [ - { - type: 'text', - text: `Error: Start line ${startLine} exceeds total lines (${lines.length}) in the file.`, - }, - ], - }; + return buildMcpToolErrorResponse([ + `Error: Start line ${startLine} exceeds total lines (${lines.length}) in the file.`, + ]); } processedContent = lines.slice(start, end).join('\n'); } - return { - content: [ - { - type: 'text', - text: `Content of Repomix output file (ID: ${outputId})${startLine || endLine ? ` (lines ${startLine || 1}-${endLine || 'end'})` : ''}:`, - }, - { - type: 'text', - text: processedContent, - }, - ], - }; + return buildMcpToolSuccessResponse([ + `Content of Repomix output file (ID: ${outputId})${startLine || endLine ? ` (lines ${startLine || 1}-${endLine || 'end'})` : ''}:`, + processedContent, + ]); } catch (error) { logger.error(`Error reading Repomix output: ${error}`); - return { - isError: true, - content: [ - { - type: 'text', - text: `Error reading Repomix output: ${error instanceof Error ? error.message : String(error)}`, - }, - ], - }; + return buildMcpToolErrorResponse([ + `Error reading Repomix output: ${error instanceof Error ? error.message : String(error)}`, + ]); } }, ); diff --git a/tests/mcp/tools/grepRepomixOutputTool.test.ts b/tests/mcp/tools/grepRepomixOutputTool.test.ts index 11598cc65..ae1b5ab13 100644 --- a/tests/mcp/tools/grepRepomixOutputTool.test.ts +++ b/tests/mcp/tools/grepRepomixOutputTool.test.ts @@ -11,13 +11,14 @@ import { import * as mcpToolRuntime from '../../../src/mcp/tools/mcpToolRuntime.js'; vi.mock('node:fs/promises'); -vi.mock('../../../src/mcp/tools/mcpToolRuntime.js'); -vi.mock('../../../src/shared/logger.js', () => ({ - logger: { - trace: vi.fn(), - error: vi.fn(), - }, -})); +vi.mock('../../../src/shared/logger.js'); +vi.mock('../../../src/mcp/tools/mcpToolRuntime.js', async () => { + const actual = await vi.importActual('../../../src/mcp/tools/mcpToolRuntime.js'); + return { + ...actual, + getOutputFilePath: vi.fn(), + }; +}); /** * Search options for grep functionality diff --git a/tests/mcp/tools/mcpToolRuntime.test.ts b/tests/mcp/tools/mcpToolRuntime.test.ts index 402d663ae..f619ab78e 100644 --- a/tests/mcp/tools/mcpToolRuntime.test.ts +++ b/tests/mcp/tools/mcpToolRuntime.test.ts @@ -4,6 +4,8 @@ import os from 'node:os'; import path from 'node:path'; import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; import { + buildMcpToolErrorResponse, + buildMcpToolSuccessResponse, createToolWorkspace, formatToolError, formatToolResponse, @@ -188,4 +190,104 @@ describe('mcpToolRuntime', () => { expect(jsonContent.metrics.totalLines).toBe(5); }); }); + + describe('buildMcpToolSuccessResponse', () => { + it('should create a successful response with single message', () => { + const messages = ['Operation completed successfully']; + const response = buildMcpToolSuccessResponse(messages); + + expect(response).toEqual({ + content: [ + { + type: 'text', + text: 'Operation completed successfully', + }, + ], + }); + expect(response.isError).toBeUndefined(); + }); + + it('should create a successful response with multiple messages', () => { + const messages = ['First message', 'Second message', 'Third message']; + const response = buildMcpToolSuccessResponse(messages); + + expect(response).toEqual({ + content: [ + { + type: 'text', + text: 'First message', + }, + { + type: 'text', + text: 'Second message', + }, + { + type: 'text', + text: 'Third message', + }, + ], + }); + expect(response.isError).toBeUndefined(); + }); + + it('should create a successful response with empty messages array', () => { + const messages: string[] = []; + const response = buildMcpToolSuccessResponse(messages); + + expect(response).toEqual({ + content: [], + }); + expect(response.isError).toBeUndefined(); + }); + }); + + describe('buildMcpToolErrorResponse', () => { + it('should create an error response with single message', () => { + const errorMessages = ['Something went wrong']; + const response = buildMcpToolErrorResponse(errorMessages); + + expect(response).toEqual({ + isError: true, + content: [ + { + type: 'text', + text: 'Something went wrong', + }, + ], + }); + }); + + it('should create an error response with multiple messages', () => { + const errorMessages = ['Error 1', 'Error 2', 'Error 3']; + const response = buildMcpToolErrorResponse(errorMessages); + + expect(response).toEqual({ + isError: true, + content: [ + { + type: 'text', + text: 'Error 1', + }, + { + type: 'text', + text: 'Error 2', + }, + { + type: 'text', + text: 'Error 3', + }, + ], + }); + }); + + it('should create an error response with empty messages array', () => { + const errorMessages: string[] = []; + const response = buildMcpToolErrorResponse(errorMessages); + + expect(response).toEqual({ + isError: true, + content: [], + }); + }); + }); }); diff --git a/tests/mcp/tools/packCodebaseTool.test.ts b/tests/mcp/tools/packCodebaseTool.test.ts index de868a1da..e437ccd4a 100644 --- a/tests/mcp/tools/packCodebaseTool.test.ts +++ b/tests/mcp/tools/packCodebaseTool.test.ts @@ -8,7 +8,15 @@ import { registerPackCodebaseTool } from '../../../src/mcp/tools/packCodebaseToo vi.mock('node:path'); vi.mock('../../../src/cli/cliRun.js'); -vi.mock('../../../src/mcp/tools/mcpToolRuntime.js'); +vi.mock('../../../src/mcp/tools/mcpToolRuntime.js', async () => { + const actual = await vi.importActual('../../../src/mcp/tools/mcpToolRuntime.js'); + return { + ...actual, + createToolWorkspace: vi.fn(), + formatToolError: vi.fn(), + formatToolResponse: vi.fn(), + }; +}); describe('PackCodebaseTool', () => { const mockServer = { @@ -144,14 +152,7 @@ describe('PackCodebaseTool', () => { content: [ { type: 'text', - text: JSON.stringify( - { - success: false, - error: 'Failed to return a result', - }, - null, - 2, - ), + text: 'Failed to return a result', }, ], }); diff --git a/tests/mcp/tools/readRepomixOutputTool.test.ts b/tests/mcp/tools/readRepomixOutputTool.test.ts index 07bf65abd..81db22ea9 100644 --- a/tests/mcp/tools/readRepomixOutputTool.test.ts +++ b/tests/mcp/tools/readRepomixOutputTool.test.ts @@ -5,7 +5,13 @@ import * as mcpToolRuntime from '../../../src/mcp/tools/mcpToolRuntime.js'; import { registerReadRepomixOutputTool } from '../../../src/mcp/tools/readRepomixOutputTool.js'; vi.mock('node:fs/promises'); -vi.mock('../../../src/mcp/tools/mcpToolRuntime.js'); +vi.mock('../../../src/mcp/tools/mcpToolRuntime.js', async () => { + const actual = await vi.importActual('../../../src/mcp/tools/mcpToolRuntime.js'); + return { + ...actual, + getOutputFilePath: vi.fn(), + }; +}); vi.mock('../../../src/shared/logger.js', () => ({ logger: { trace: vi.fn(),