diff --git a/src/core/assistant-message/__tests__/parseAssistantMessage.spec.ts b/src/core/assistant-message/__tests__/parseAssistantMessage.spec.ts
index f5ae600bee..f6bc26d7cc 100644
--- a/src/core/assistant-message/__tests__/parseAssistantMessage.spec.ts
+++ b/src/core/assistant-message/__tests__/parseAssistantMessage.spec.ts
@@ -271,6 +271,48 @@ const isEmptyTextContent = (block: AssistantMessageContent) =>
expect(toolUse.partial).toBe(false)
})
+ it("should handle whitespace and newlines between XML tags (Grok/Qwen format)", () => {
+ // This is the format that Grok and Qwen3-Coder generate that was causing issues
+ const message = `
+
+
+src/shared/infrastructure/supabase/factory.py
+
+
+`
+ const result = parser(message).filter((block) => !isEmptyTextContent(block))
+
+ expect(result).toHaveLength(1)
+ const toolUse = result[0] as ToolUse
+ expect(toolUse.type).toBe("tool_use")
+ expect(toolUse.name).toBe("read_file")
+ // The args should be captured as a parameter
+ expect(toolUse.params.args).toBeDefined()
+ expect(toolUse.params.args).toContain("")
+ expect(toolUse.params.args).toContain("src/shared/infrastructure/supabase/factory.py")
+ expect(toolUse.partial).toBe(false)
+ })
+
+ it("should handle whitespace-only path values", () => {
+ const message = `
+
+
+
+
+
+`
+ const result = parser(message).filter((block) => !isEmptyTextContent(block))
+
+ expect(result).toHaveLength(1)
+ const toolUse = result[0] as ToolUse
+ expect(toolUse.type).toBe("tool_use")
+ expect(toolUse.name).toBe("read_file")
+ expect(toolUse.params.args).toBeDefined()
+ // The whitespace-only path should be preserved in the args
+ expect(toolUse.params.args).toContain(" ")
+ expect(toolUse.partial).toBe(false)
+ })
+
it("should handle multi-line parameters", () => {
const message = `file.ts
line 1
diff --git a/src/core/tools/__tests__/readFileTool.spec.ts b/src/core/tools/__tests__/readFileTool.spec.ts
index 7ba822dce0..a8a4b19ea1 100644
--- a/src/core/tools/__tests__/readFileTool.spec.ts
+++ b/src/core/tools/__tests__/readFileTool.spec.ts
@@ -1325,6 +1325,250 @@ describe("read_file tool XML output structure", () => {
`\n${testFilePath}Access to ${testFilePath} is blocked by the .rooignore file settings. You must try to continue in the task without using this file, or ask the user to update the .rooignore file.\n`,
)
})
+
+ it("should handle XML args with whitespace and newlines (Grok/Qwen format)", async () => {
+ // This test reproduces the exact issue reported with Grok and Qwen3-Coder
+ // where XML has newlines and spaces between tags
+ const argsWithWhitespace = `
+
+ test/file.txt
+
+`
+ const toolUse: ReadFileToolUse = {
+ type: "tool_use",
+ name: "read_file",
+ params: { args: argsWithWhitespace },
+ partial: false,
+ }
+
+ // Setup mocks
+ mockedCountFileLines.mockResolvedValue(5)
+ mockedExtractTextFromFile.mockResolvedValue("1 | Line 1\n2 | Line 2\n3 | Line 3\n4 | Line 4\n5 | Line 5")
+ mockProvider.getState.mockResolvedValue({
+ maxReadFileLine: -1,
+ maxImageFileSize: 20,
+ maxTotalImageSize: 20,
+ })
+
+ // Execute
+ await readFileTool(
+ mockCline,
+ toolUse,
+ mockCline.ask,
+ vi.fn(),
+ (result: ToolResponse) => {
+ toolResult = result
+ },
+ (param: ToolParamName, content?: string) => content ?? "",
+ )
+
+ // Verify the file was read successfully (path was trimmed)
+ expect(toolResult).toContain("test/file.txt")
+ expect(toolResult).toContain('')
+ expect(toolResult).not.toContain("")
+ })
+
+ it("should handle multiple files with whitespace in XML", async () => {
+ // Test multiple files with varying whitespace
+ const argsWithMultipleFiles = `
+
+
+ test/file1.txt
+
+
+
+ test/file2.txt
+
+
+ test/file3.txt
+
+`
+ const toolUse: ReadFileToolUse = {
+ type: "tool_use",
+ name: "read_file",
+ params: { args: argsWithMultipleFiles },
+ partial: false,
+ }
+
+ // Setup mocks
+ mockedCountFileLines.mockResolvedValue(3)
+ mockedExtractTextFromFile.mockResolvedValue("1 | Content")
+ mockProvider.getState.mockResolvedValue({
+ maxReadFileLine: -1,
+ maxImageFileSize: 20,
+ maxTotalImageSize: 20,
+ })
+
+ // Mock path.resolve for each file
+ mockedPathResolve.mockImplementation((cwd, relPath) => `/${relPath.trim()}`)
+
+ // Execute
+ await readFileTool(
+ mockCline,
+ toolUse,
+ mockCline.ask,
+ vi.fn(),
+ (result: ToolResponse) => {
+ toolResult = result
+ },
+ (param: ToolParamName, content?: string) => content ?? "",
+ )
+
+ // Verify all files were processed
+ expect(toolResult).toContain("test/file1.txt")
+ expect(toolResult).toContain("test/file2.txt")
+ expect(toolResult).toContain("test/file3.txt")
+ expect(toolResult).not.toContain("")
+ })
+
+ it("should handle empty path after trimming whitespace", async () => {
+ // Test case where path is only whitespace
+ const argsWithEmptyPath = `
+
+
+
+`
+ const toolUse: ReadFileToolUse = {
+ type: "tool_use",
+ name: "read_file",
+ params: { args: argsWithEmptyPath },
+ partial: false,
+ }
+
+ // Create a spy for handleError
+ const handleErrorSpy = vi.fn()
+
+ // Execute
+ await readFileTool(
+ mockCline,
+ toolUse,
+ mockCline.ask,
+ handleErrorSpy,
+ (result: ToolResponse) => {
+ toolResult = result
+ },
+ (param: ToolParamName, content?: string) => content ?? "",
+ )
+
+ // Verify error is returned for empty path
+ expect(toolResult).toContain("")
+ expect(toolResult).toContain("No valid file paths found")
+ expect(toolResult).toContain("Ensure each element contains a non-empty element")
+ expect(handleErrorSpy).toHaveBeenCalled()
+ })
+
+ it("should work without line_range parameter (line_range is optional)", async () => {
+ // Test that line_range is truly optional
+ const argsWithoutLineRange = `
+
+test/file.txt
+
+`
+ const toolUse: ReadFileToolUse = {
+ type: "tool_use",
+ name: "read_file",
+ params: { args: argsWithoutLineRange },
+ partial: false,
+ }
+
+ // Setup mocks
+ mockedCountFileLines.mockResolvedValue(5)
+ mockedExtractTextFromFile.mockResolvedValue("1 | Line 1\n2 | Line 2\n3 | Line 3\n4 | Line 4\n5 | Line 5")
+ mockProvider.getState.mockResolvedValue({
+ maxReadFileLine: -1,
+ maxImageFileSize: 20,
+ maxTotalImageSize: 20,
+ })
+
+ // Execute
+ await readFileTool(
+ mockCline,
+ toolUse,
+ mockCline.ask,
+ vi.fn(),
+ (result: ToolResponse) => {
+ toolResult = result
+ },
+ (param: ToolParamName, content?: string) => content ?? "",
+ )
+
+ // Verify the file was read successfully without line_range
+ expect(toolResult).toContain("test/file.txt")
+ expect(toolResult).toContain('')
+ expect(toolResult).not.toContain("")
+ expect(toolResult).not.toContain("line_range")
+ })
+
+ it("should provide helpful error for malformed XML missing path element", async () => {
+ // Test case simulating what Grok might send - file element with missing path
+ const malformedArgs = `
+
+
+
+`
+ const toolUse: ReadFileToolUse = {
+ type: "tool_use",
+ name: "read_file",
+ params: { args: malformedArgs },
+ partial: false,
+ }
+
+ // Create a spy for handleError
+ const handleErrorSpy = vi.fn()
+
+ // Execute
+ await readFileTool(
+ mockCline,
+ toolUse,
+ mockCline.ask,
+ handleErrorSpy,
+ (result: ToolResponse) => {
+ toolResult = result
+ },
+ (param: ToolParamName, content?: string) => content ?? "",
+ )
+
+ // Verify helpful error is returned
+ expect(toolResult).toContain("")
+ expect(toolResult).toContain("No valid file paths found")
+ expect(toolResult).toContain("Ensure each element contains a non-empty element")
+ expect(handleErrorSpy).toHaveBeenCalled()
+ })
+
+ it("should provide error when file element has no path child at all", async () => {
+ // Test case where file element exists but has no path child element
+ const malformedArgs = `
+
+
+`
+ const toolUse: ReadFileToolUse = {
+ type: "tool_use",
+ name: "read_file",
+ params: { args: malformedArgs },
+ partial: false,
+ }
+
+ // Execute
+ await readFileTool(
+ mockCline,
+ toolUse,
+ mockCline.ask,
+ vi.fn(),
+ (result: ToolResponse) => {
+ toolResult = result
+ },
+ (param: ToolParamName, content?: string) => content ?? "",
+ )
+
+ // When file element has no path child, it falls through to the general error
+ expect(toolResult).toContain("")
+ expect(toolResult).toContain("Missing required parameter")
+ // This is handled by sayAndCreateMissingParamError
+ expect(mockCline.sayAndCreateMissingParamError).toHaveBeenCalledWith(
+ "read_file",
+ "args with valid file paths. Expected format: filepath",
+ )
+ })
})
})
diff --git a/src/core/tools/readFileTool.ts b/src/core/tools/readFileTool.ts
index 01427f4d9d..1e4807c879 100644
--- a/src/core/tools/readFileTool.ts
+++ b/src/core/tools/readFileTool.ts
@@ -131,11 +131,21 @@ export async function readFileTool(
const parsed = parseXml(argsXmlTag) as any
const files = Array.isArray(parsed.file) ? parsed.file : [parsed.file].filter(Boolean)
+ // Track invalid entries for better error reporting
+ const invalidEntries: string[] = []
+
for (const file of files) {
- if (!file.path) continue // Skip if no path in a file entry
+ // Check if path exists and is not empty after trimming
+ const filePath = typeof file.path === "string" ? file.path.trim() : file.path
+
+ if (!filePath) {
+ // Track this invalid entry
+ invalidEntries.push(JSON.stringify(file).substring(0, 100))
+ continue // Skip if no path in a file entry
+ }
const fileEntry: FileEntry = {
- path: file.path,
+ path: filePath,
lineRanges: [],
}
@@ -153,8 +163,16 @@ export async function readFileTool(
}
fileEntries.push(fileEntry)
}
+
+ // If we had invalid entries but no valid ones, provide a helpful error
+ if (fileEntries.length === 0 && invalidEntries.length > 0) {
+ const errorMessage = `No valid file paths found in read_file args. Invalid entries: ${invalidEntries.join(", ")}. Ensure each element contains a non-empty element.`
+ await handleError("parsing read_file args", new Error(errorMessage))
+ pushToolResult(`${errorMessage}`)
+ return
+ }
} catch (error) {
- const errorMessage = `Failed to parse read_file XML args: ${error instanceof Error ? error.message : String(error)}`
+ const errorMessage = `Failed to parse read_file XML args: ${error instanceof Error ? error.message : String(error)}. Expected format: filepath`
await handleError("parsing read_file args", new Error(errorMessage))
pushToolResult(`${errorMessage}`)
return
@@ -186,7 +204,10 @@ export async function readFileTool(
if (fileEntries.length === 0) {
cline.consecutiveMistakeCount++
cline.recordToolError("read_file")
- const errorMsg = await cline.sayAndCreateMissingParamError("read_file", "args (containing valid file paths)")
+ const errorMsg = await cline.sayAndCreateMissingParamError(
+ "read_file",
+ "args with valid file paths. Expected format: filepath",
+ )
pushToolResult(`${errorMsg}`)
return
}
diff --git a/src/utils/__tests__/xml.spec.ts b/src/utils/__tests__/xml.spec.ts
index f7a282b0c0..1073d0c7cf 100644
--- a/src/utils/__tests__/xml.spec.ts
+++ b/src/utils/__tests__/xml.spec.ts
@@ -114,6 +114,120 @@ describe("parseXml", () => {
expect(result.root.data.nestedXml).toHaveProperty("item", "Should not parse this")
})
})
+
+ describe("whitespace handling", () => {
+ it("should handle spaces within path tags", () => {
+ const xml = `
+
+
+ ./test/file.ts
+
+
+ `
+
+ const result = parseXml(xml) as any
+
+ // The path should be trimmed
+ expect(result.args.file.path).toBe("./test/file.ts")
+ })
+
+ it("should handle newlines and spaces in nested tags", () => {
+ const xml = `
+
+
+
+ src/shared/infrastructure/supabase/factory.py
+
+
+
+ `
+
+ const result = parseXml(xml) as any
+
+ // The path should be trimmed
+ expect(result.args.file.path).toBe("src/shared/infrastructure/supabase/factory.py")
+ })
+
+ it("should handle multiple files with varying whitespace", () => {
+ const xml = `
+
+
+ file1.ts
+
+
+
+ file2.ts
+
+
+
+ file3.ts
+
+
+ `
+
+ const result = parseXml(xml) as any
+
+ // All paths should be trimmed
+ expect(Array.isArray(result.args.file)).toBe(true)
+ expect(result.args.file[0].path).toBe("file1.ts")
+ expect(result.args.file[1].path).toBe("file2.ts")
+ expect(result.args.file[2].path).toBe("file3.ts")
+ })
+
+ it("should handle empty or whitespace-only path tags", () => {
+ const xml = `
+
+
+
+
+
+ `
+
+ const result = parseXml(xml) as any
+
+ // Empty string after trimming
+ expect(result.args.file.path).toBe("")
+ })
+
+ it("should handle tabs and mixed whitespace", () => {
+ const xml = `
+
+
+
+ ./path/with/tabs.ts
+
+
+
+ `
+
+ const result = parseXml(xml) as any
+
+ // Should trim tabs and newlines
+ expect(result.args.file.path).toBe("./path/with/tabs.ts")
+ })
+
+ it("should handle the exact format from Grok that was failing", () => {
+ // This is the exact format that was causing issues with Grok
+ const xml = `
+
+
+src/shared/infrastructure/supabase/factory.py
+
+
+`
+
+ // First extract just the args portion
+ const argsMatch = xml.match(/([\s\S]*?)<\/args>/)
+ expect(argsMatch).toBeTruthy()
+
+ if (argsMatch) {
+ const argsXml = `${argsMatch[1]}`
+ const result = parseXml(argsXml) as any
+
+ expect(result.args.file.path).toBe("src/shared/infrastructure/supabase/factory.py")
+ }
+ })
+ })
})
describe("parseXmlForDiff", () => {