From 266ee8ef58baac3cc06f5df637ff3c7352344e82 Mon Sep 17 00:00:00 2001 From: Roo Code Date: Wed, 23 Jul 2025 13:30:33 +0000 Subject: [PATCH] fix: add validation and warnings for large MCP tool arguments - Add detection for potentially truncated MCP tool arguments (around 4000 chars) - Show helpful error messages when truncation is detected - Warn users when arguments are large but still valid JSON - Add comprehensive tests for the new validation logic Fixes #6108 --- .../tools/__tests__/useMcpToolTool.spec.ts | 113 ++++++++++++++++++ src/core/tools/useMcpToolTool.ts | 77 ++++++++++-- 2 files changed, 177 insertions(+), 13 deletions(-) diff --git a/src/core/tools/__tests__/useMcpToolTool.spec.ts b/src/core/tools/__tests__/useMcpToolTool.spec.ts index 97893b3a97b..6c1806178c1 100644 --- a/src/core/tools/__tests__/useMcpToolTool.spec.ts +++ b/src/core/tools/__tests__/useMcpToolTool.spec.ts @@ -139,6 +139,119 @@ describe("useMcpToolTool", () => { expect(mockTask.say).toHaveBeenCalledWith("error", expect.stringContaining("invalid JSON argument")) expect(mockPushToolResult).toHaveBeenCalledWith("Tool error: Invalid args for test_server:test_tool") }) + + it("should warn about large but valid JSON arguments", async () => { + // Create a large JSON string (over 3800 characters) + const largeData = { data: "x".repeat(3850) } + const largeJson = JSON.stringify(largeData) + + const block: ToolUse = { + type: "tool_use", + name: "use_mcp_tool", + params: { + server_name: "test_server", + tool_name: "test_tool", + arguments: largeJson, + }, + partial: false, + } + + mockAskApproval.mockResolvedValue(true) + const mockToolResult = { + content: [{ type: "text", text: "Tool executed successfully" }], + isError: false, + } + mockProviderRef.deref.mockReturnValue({ + getMcpHub: () => ({ + callTool: vi.fn().mockResolvedValue(mockToolResult), + }), + postMessageToWebview: vi.fn(), + }) + + await useMcpToolTool( + mockTask as Task, + block, + mockAskApproval, + mockHandleError, + mockPushToolResult, + mockRemoveClosingTag, + ) + + // Should warn about large arguments + expect(mockTask.say).toHaveBeenCalledWith("error", expect.stringContaining("Warning:")) + expect(mockTask.say).toHaveBeenCalledWith("error", expect.stringContaining("very large")) + expect(mockTask.say).toHaveBeenCalledWith("error", expect.stringContaining("4000 characters")) + + // But should still execute successfully + expect(mockTask.consecutiveMistakeCount).toBe(0) + expect(mockAskApproval).toHaveBeenCalled() + expect(mockPushToolResult).toHaveBeenCalledWith("Tool result: Tool executed successfully") + }) + + it("should detect truncated JSON arguments", async () => { + // Create a truncated JSON string that looks like it was cut off + const truncatedJson = '{"data": "' + "x".repeat(3950) // No closing quote or brace + + const block: ToolUse = { + type: "tool_use", + name: "use_mcp_tool", + params: { + server_name: "test_server", + tool_name: "test_tool", + arguments: truncatedJson, + }, + partial: false, + } + + await useMcpToolTool( + mockTask as Task, + block, + mockAskApproval, + mockHandleError, + mockPushToolResult, + mockRemoveClosingTag, + ) + + expect(mockTask.consecutiveMistakeCount).toBe(1) + expect(mockTask.recordToolError).toHaveBeenCalledWith("use_mcp_tool") + + // Should show truncation-specific error message + expect(mockTask.say).toHaveBeenCalledWith("error", expect.stringContaining("truncated")) + expect(mockTask.say).toHaveBeenCalledWith("error", expect.stringContaining("4000 characters")) + expect(mockTask.say).toHaveBeenCalledWith("error", expect.stringContaining("smaller chunks")) + + expect(mockPushToolResult).toHaveBeenCalledWith(expect.stringContaining("truncated by the language model")) + }) + + it("should handle JSON ending with incomplete array", async () => { + // Create a JSON string that ends with an incomplete array + const truncatedJson = '{"nodes": [{"id": 1}, {"id": 2}, {"id"' + "x".repeat(3920) + + const block: ToolUse = { + type: "tool_use", + name: "use_mcp_tool", + params: { + server_name: "test_server", + tool_name: "test_tool", + arguments: truncatedJson, + }, + partial: false, + } + + await useMcpToolTool( + mockTask as Task, + block, + mockAskApproval, + mockHandleError, + mockPushToolResult, + mockRemoveClosingTag, + ) + + expect(mockTask.consecutiveMistakeCount).toBe(1) + expect(mockTask.recordToolError).toHaveBeenCalledWith("use_mcp_tool") + expect(mockTask.say).toHaveBeenCalledWith("error", expect.stringContaining("truncated")) + expect(mockPushToolResult).toHaveBeenCalledWith(expect.stringContaining("truncated by the language model")) + }) }) describe("partial requests", () => { diff --git a/src/core/tools/useMcpToolTool.ts b/src/core/tools/useMcpToolTool.ts index 30dff5ce4fa..a20764a71a0 100644 --- a/src/core/tools/useMcpToolTool.ts +++ b/src/core/tools/useMcpToolTool.ts @@ -57,19 +57,70 @@ async function validateParams( let parsedArguments: Record | undefined if (params.arguments) { - try { - parsedArguments = JSON.parse(params.arguments) - } catch (error) { - cline.consecutiveMistakeCount++ - cline.recordToolError("use_mcp_tool") - await cline.say("error", t("mcp:errors.invalidJsonArgument", { toolName: params.tool_name })) - - pushToolResult( - formatResponse.toolError( - formatResponse.invalidMcpToolArgumentError(params.server_name, params.tool_name), - ), - ) - return { isValid: false } + // Check if arguments appear to be truncated (common around 4000 characters) + const argLength = params.arguments.length + const TRUNCATION_WARNING_THRESHOLD = 3800 // Warn when close to 4000 chars + const LIKELY_TRUNCATED_THRESHOLD = 3900 // Very likely truncated if over this + + // Check for signs of truncation + const lastChar = params.arguments[params.arguments.length - 1] + const endsWithIncompleteJSON = + argLength > LIKELY_TRUNCATED_THRESHOLD && lastChar !== "}" && lastChar !== "]" && lastChar !== '"' + + if (endsWithIncompleteJSON || argLength > TRUNCATION_WARNING_THRESHOLD) { + // Try to parse anyway to see if it's valid JSON + try { + parsedArguments = JSON.parse(params.arguments) + + // Valid JSON but very large - warn the user + if (argLength > TRUNCATION_WARNING_THRESHOLD) { + await cline.say( + "error", + `⚠️ Warning: The MCP tool arguments are very large (${argLength} characters). ` + + `Some language models may truncate tool calls around 4000 characters. ` + + `Consider breaking this into smaller operations if the tool fails.`, + ) + } + } catch (error) { + // Invalid JSON and likely truncated + cline.consecutiveMistakeCount++ + cline.recordToolError("use_mcp_tool") + + const errorMessage = + argLength > LIKELY_TRUNCATED_THRESHOLD + ? `The MCP tool arguments appear to be truncated (${argLength} characters). ` + + `The JSON is incomplete and cannot be parsed. ` + + `This is a known limitation where some language models truncate tool calls around 4000 characters. ` + + `Please try breaking this operation into smaller chunks or reducing the data size.` + : t("mcp:errors.invalidJsonArgument", { toolName: params.tool_name }) + + await cline.say("error", errorMessage) + + pushToolResult( + formatResponse.toolError( + argLength > LIKELY_TRUNCATED_THRESHOLD + ? `Tool arguments were truncated by the language model. The JSON is incomplete (${argLength} characters). Please use smaller data chunks.` + : formatResponse.invalidMcpToolArgumentError(params.server_name, params.tool_name), + ), + ) + return { isValid: false } + } + } else { + // Normal JSON parsing for smaller arguments + try { + parsedArguments = JSON.parse(params.arguments) + } catch (error) { + cline.consecutiveMistakeCount++ + cline.recordToolError("use_mcp_tool") + await cline.say("error", t("mcp:errors.invalidJsonArgument", { toolName: params.tool_name })) + + pushToolResult( + formatResponse.toolError( + formatResponse.invalidMcpToolArgumentError(params.server_name, params.tool_name), + ), + ) + return { isValid: false } + } } }