mirror of
https://github.com/RooVetGit/Roo-Code.git
synced 2026-09-05 08:10:14 +00:00
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
This commit is contained in:
parent
714fafd328
commit
266ee8ef58
2 changed files with 176 additions and 12 deletions
|
|
@ -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", () => {
|
||||
|
|
|
|||
|
|
@ -57,19 +57,70 @@ async function validateParams(
|
|||
let parsedArguments: Record<string, unknown> | 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 }))
|
||||
// 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
|
||||
|
||||
pushToolResult(
|
||||
formatResponse.toolError(
|
||||
formatResponse.invalidMcpToolArgumentError(params.server_name, params.tool_name),
|
||||
),
|
||||
)
|
||||
return { isValid: false }
|
||||
// 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 }
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue