diff --git a/src/core/tools/__tests__/useMcpToolTool.spec.ts b/src/core/tools/__tests__/useMcpToolTool.spec.ts index 97893b3a97b..6529505d8a2 100644 --- a/src/core/tools/__tests__/useMcpToolTool.spec.ts +++ b/src/core/tools/__tests__/useMcpToolTool.spec.ts @@ -136,8 +136,13 @@ describe("useMcpToolTool", () => { expect(mockTask.consecutiveMistakeCount).toBe(1) expect(mockTask.recordToolError).toHaveBeenCalledWith("use_mcp_tool") - expect(mockTask.say).toHaveBeenCalledWith("error", expect.stringContaining("invalid JSON argument")) - expect(mockPushToolResult).toHaveBeenCalledWith("Tool error: Invalid args for test_server:test_tool") + expect(mockTask.say).toHaveBeenCalledWith( + "error", + expect.stringContaining("Invalid JSON arguments for tool 'test_tool'"), + ) + expect(mockPushToolResult).toHaveBeenCalledWith( + expect.stringContaining("Invalid JSON arguments for test_server.test_tool"), + ) }) }) diff --git a/src/core/tools/useMcpToolTool.ts b/src/core/tools/useMcpToolTool.ts index 30dff5ce4fa..8dd4c334eea 100644 --- a/src/core/tools/useMcpToolTool.ts +++ b/src/core/tools/useMcpToolTool.ts @@ -54,20 +54,45 @@ async function validateParams( return { isValid: false } } + // Validate server name format + const serverName = params.server_name.trim() + if (!serverName) { + cline.consecutiveMistakeCount++ + cline.recordToolError("use_mcp_tool") + await cline.say("error", "Server name cannot be empty or contain only whitespace") + pushToolResult(formatResponse.toolError("Invalid server name: cannot be empty")) + return { isValid: false } + } + + // Validate tool name format + const toolName = params.tool_name.trim() + if (!toolName) { + cline.consecutiveMistakeCount++ + cline.recordToolError("use_mcp_tool") + await cline.say("error", "Tool name cannot be empty or contain only whitespace") + pushToolResult(formatResponse.toolError("Invalid tool name: cannot be empty")) + return { isValid: false } + } + let parsedArguments: Record | undefined if (params.arguments) { try { parsedArguments = JSON.parse(params.arguments) + + // Validate that arguments is an object (not array or primitive) + if ((parsedArguments !== null && typeof parsedArguments !== "object") || Array.isArray(parsedArguments)) { + throw new Error("Arguments must be a JSON object, not an array or primitive value") + } } catch (error) { cline.consecutiveMistakeCount++ cline.recordToolError("use_mcp_tool") - await cline.say("error", t("mcp:errors.invalidJsonArgument", { toolName: params.tool_name })) + + const errorMessage = error instanceof Error ? error.message : "Invalid JSON" + await cline.say("error", `Invalid JSON arguments for tool '${toolName}': ${errorMessage}`) pushToolResult( - formatResponse.toolError( - formatResponse.invalidMcpToolArgumentError(params.server_name, params.tool_name), - ), + formatResponse.toolError(`Invalid JSON arguments for ${serverName}.${toolName}: ${errorMessage}`), ) return { isValid: false } } @@ -75,8 +100,8 @@ async function validateParams( return { isValid: true, - serverName: params.server_name, - toolName: params.tool_name, + serverName, + toolName, parsedArguments, } } @@ -127,41 +152,74 @@ async function executeToolAndProcessResult( toolName, }) - const toolResult = await cline.providerRef.deref()?.getMcpHub()?.callTool(serverName, toolName, parsedArguments) + let retryCount = 0 + const maxRetries = 3 + let lastError: Error | null = null - let toolResultPretty = "(No response)" + while (retryCount <= maxRetries) { + try { + const mcpHub = cline.providerRef.deref()?.getMcpHub() + if (!mcpHub) { + throw new Error("MCP Hub is not available. Please ensure MCP servers are properly configured.") + } - if (toolResult) { - const outputText = processToolContent(toolResult) + const toolResult = await mcpHub.callTool(serverName, toolName, parsedArguments) - if (outputText) { - await sendExecutionStatus(cline, { - executionId, - status: "output", - response: outputText, - }) + if (toolResult) { + const outputText = processToolContent(toolResult) - toolResultPretty = (toolResult.isError ? "Error:\n" : "") + outputText - } + if (outputText) { + await sendExecutionStatus(cline, { + executionId, + status: "output", + response: outputText, + }) - // Send completion status - await sendExecutionStatus(cline, { - executionId, - status: toolResult.isError ? "error" : "completed", - response: toolResultPretty, - error: toolResult.isError ? "Error executing MCP tool" : undefined, - }) - } else { - // Send error status if no result - await sendExecutionStatus(cline, { - executionId, - status: "error", - error: "No response from MCP server", - }) + const toolResultPretty = (toolResult.isError ? "Error:\n" : "") + outputText + + // Send completion status + await sendExecutionStatus(cline, { + executionId, + status: toolResult.isError ? "error" : "completed", + response: toolResultPretty, + error: toolResult.isError ? outputText : undefined, + }) + + await cline.say("mcp_server_response", toolResultPretty) + pushToolResult(formatResponse.toolResult(toolResultPretty)) + return + } + } + + // If we get here, toolResult was null/undefined + throw new Error(`No response received from MCP server '${serverName}' for tool '${toolName}'`) + } catch (error) { + lastError = error instanceof Error ? error : new Error(String(error)) + retryCount++ + + if (retryCount <= maxRetries) { + const delay = Math.min(1000 * Math.pow(2, retryCount - 1), 5000) // Exponential backoff with max 5s + await cline.say( + "error", + `MCP tool execution failed (attempt ${retryCount}/${maxRetries}). Retrying in ${delay / 1000}s...`, + ) + await new Promise((resolve) => setTimeout(resolve, delay)) + } + } } - await cline.say("mcp_server_response", toolResultPretty) - pushToolResult(formatResponse.toolResult(toolResultPretty)) + // All retries failed + const errorMessage = lastError?.message || "Unknown error occurred" + const userFriendlyError = `Failed to execute MCP tool '${toolName}' on server '${serverName}' after ${maxRetries} attempts. ${errorMessage}` + + await sendExecutionStatus(cline, { + executionId, + status: "error", + error: userFriendlyError, + }) + + await cline.say("mcp_server_response", `Error: ${userFriendlyError}`) + pushToolResult(formatResponse.toolError(userFriendlyError)) } export async function useMcpToolTool( diff --git a/src/i18n/locales/en/mcp.json b/src/i18n/locales/en/mcp.json index c34b2c9c08d..132cbcbdaa8 100644 --- a/src/i18n/locales/en/mcp.json +++ b/src/i18n/locales/en/mcp.json @@ -5,7 +5,9 @@ "invalid_settings_validation": "Invalid MCP settings format: {{errorMessages}}", "create_json": "Failed to create or open .roo/mcp.json: {{error}}", "failed_update_project": "Failed to update project MCP servers", - "invalidJsonArgument": "Roo tried to use {{toolName}} with an invalid JSON argument. Retrying..." + "invalidJsonArgument": "Roo tried to use {{toolName}} with an invalid JSON argument. Retrying...", + "connection_failed": "Failed to connect to MCP server '{{serverName}}'. Please check your server configuration and ensure the server is running.", + "tool_execution_failed": "Failed to execute MCP tool '{{toolName}}' on server '{{serverName}}': {{error}}" }, "info": { "server_restarting": "Restarting {{serverName}} MCP server...", diff --git a/src/services/mcp/McpHub.ts b/src/services/mcp/McpHub.ts index 10a74712ef0..2f3a83bc363 100644 --- a/src/services/mcp/McpHub.ts +++ b/src/services/mcp/McpHub.ts @@ -1464,12 +1464,39 @@ export class McpHub { ): Promise { const connection = this.findConnection(serverName, source) if (!connection) { + // Provide helpful suggestions + const availableServers = this.getAllServers() + .filter((s) => !s.disabled) + .map((s) => s.name) + .join(", ") + + const errorMsg = + availableServers.length > 0 + ? `MCP server '${serverName}' not found. Available servers: ${availableServers}` + : `No MCP servers are currently connected. Please configure MCP servers in your settings.` + + throw new Error(errorMsg) + } + + if (connection.server.disabled) { throw new Error( - `No connection found for server: ${serverName}${source ? ` with source ${source}` : ""}. Please make sure to use MCP servers available under 'Connected MCP Servers'.`, + `MCP server '${serverName}' is currently disabled. Please enable it in the MCP settings to use it.`, ) } - if (connection.server.disabled) { - throw new Error(`Server "${serverName}" is disabled and cannot be used`) + + // Check if server is connected + if (connection.server.status !== "connected") { + throw new Error( + `MCP server '${serverName}' is not connected (status: ${connection.server.status}). Please check the server configuration and logs.`, + ) + } + + // Validate tool exists + const availableTools = connection.server.tools || [] + const toolExists = availableTools.some((t) => t.name === toolName) + if (!toolExists && availableTools.length > 0) { + const toolNames = availableTools.map((t) => t.name).join(", ") + throw new Error(`Tool '${toolName}' not found on server '${serverName}'. Available tools: ${toolNames}`) } let timeout: number @@ -1482,19 +1509,44 @@ export class McpHub { timeout = 60 * 1000 } - return await connection.client.request( - { - method: "tools/call", - params: { - name: toolName, - arguments: toolArguments, + try { + const result = await connection.client.request( + { + method: "tools/call", + params: { + name: toolName, + arguments: toolArguments, + }, }, - }, - CallToolResultSchema, - { - timeout, - }, - ) + CallToolResultSchema, + { + timeout, + }, + ) + + return result + } catch (error) { + // Enhance error messages for common issues + if (error instanceof Error) { + if (error.message.includes("timeout")) { + throw new Error( + `MCP tool '${toolName}' on server '${serverName}' timed out after ${timeout / 1000} seconds. The tool may be taking too long to respond or the server may be unresponsive.`, + ) + } + if (error.message.includes("ECONNREFUSED")) { + throw new Error( + `Cannot connect to MCP server '${serverName}'. The server may not be running or may be configured incorrectly.`, + ) + } + if (error.message.includes("EPIPE") || error.message.includes("ENOTCONN")) { + throw new Error( + `Lost connection to MCP server '${serverName}'. The server may have crashed or been terminated.`, + ) + } + } + // Re-throw with original error if not a known issue + throw error + } } /** diff --git a/src/services/mcp/__tests__/McpHub.spec.ts b/src/services/mcp/__tests__/McpHub.spec.ts index 7dc7f00c045..846f620c6e9 100644 --- a/src/services/mcp/__tests__/McpHub.spec.ts +++ b/src/services/mcp/__tests__/McpHub.spec.ts @@ -545,7 +545,7 @@ describe("McpHub", () => { mcpHub.connections = [mockConnection] await expect(mcpHub.callTool("disabled-server", "some-tool", {})).rejects.toThrow( - 'Server "disabled-server" is disabled and cannot be used', + "MCP server 'disabled-server' is currently disabled. Please enable it in the MCP settings to use it.", ) }) @@ -610,7 +610,7 @@ describe("McpHub", () => { it("should throw error if server not found", async () => { await expect(mcpHub.callTool("non-existent-server", "some-tool", {})).rejects.toThrow( - "No connection found for server: non-existent-server", + "No MCP servers are currently connected. Please configure MCP servers in your settings.", ) })