fix: improve XML parsing error handling for read_file tool

- Add proper trimming of path values to handle whitespace
- Provide clearer error messages when path is empty or missing
- Track invalid entries for better error reporting
- Add tests to confirm line_range is optional
- Add tests for malformed XML scenarios
- Fixes issue #7664 where Grok/Qwen3-Coder XML with whitespace caused errors

Author: roomote

src/core/tools/__tests__/readFileTool.spec.ts

@@ -1435,12 +1435,15 @@ describe("read_file tool XML output structure", () => {
 				partial: false,
 			}
 
+			// Create a spy for handleError
+			const handleErrorSpy = vi.fn()
+
 			// Execute
 			await readFileTool(
 				mockCline,
 				toolUse,
 				mockCline.ask,
-				vi.fn(),
+				handleErrorSpy,
 				(result: ToolResponse) => {
 					toolResult = result
 				},
@@ -1449,7 +1452,122 @@ describe("read_file tool XML output structure", () => {
 
 			// Verify error is returned for empty path
 			expect(toolResult).toContain("<error>")
-			expect(mockCline.sayAndCreateMissingParamError).toHaveBeenCalled()
+			expect(toolResult).toContain("No valid file paths found")
+			expect(toolResult).toContain("Ensure each <file> element contains a non-empty <path> 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 = `
+<file>
+<path>test/file.txt</path>
+</file>
+`
+			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("<file><path>test/file.txt</path>")
+			expect(toolResult).toContain('<content lines="1-5">')
+			expect(toolResult).not.toContain("<error>")
+			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 = `
+<file>
+<path></path>
+</file>
+`
+			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("<error>")
+			expect(toolResult).toContain("No valid file paths found")
+			expect(toolResult).toContain("Ensure each <file> element contains a non-empty <path> 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 = `
+<file>
+</file>
+`
+			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("<error>")
+			expect(toolResult).toContain("Missing required parameter")
+			// This is handled by sayAndCreateMissingParamError
+			expect(mockCline.sayAndCreateMissingParamError).toHaveBeenCalledWith(
+				"read_file",
+				"args with valid file paths. Expected format: <args><file><path>filepath</path></file></args>",
+			)
 		})
 	})
 })

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 <file> element contains a non-empty <path> element.`
+				await handleError("parsing read_file args", new Error(errorMessage))
+				pushToolResult(`<files><error>${errorMessage}</error></files>`)
+				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: <file><path>filepath</path></file>`
 			await handleError("parsing read_file args", new Error(errorMessage))
 			pushToolResult(`<files><error>${errorMessage}</error></files>`)
 			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: <args><file><path>filepath</path></file></args>",
+		)
 		pushToolResult(`<files><error>${errorMsg}</error></files>`)
 		return
 	}