fix: improve error messages for empty path elements in read_file tool

src/core/assistant-message/__tests__/parseAssistantMessageV2-issue7664.spec.ts

@@ -0,0 +1,183 @@
+import { parseAssistantMessageV2 } from "../parseAssistantMessageV2"
+import { ToolUse } from "../../../shared/tools"
+
+describe("parseAssistantMessageV2 - Issue #7664", () => {
+	describe("handling malformed XML structure from LLMs", () => {
+		it("should handle the exact structure from juliettefournier-econ's example", () => {
+			// This is the exact structure that was causing issues
+			const message = `I'll read that file for you.
+
+<read_file>
+<args>
+<file>
+<path>src/shared/infrastructure/supabase/factory.py</path>
+<line_range>10-20</line_range>
+</file>
+</args>
+</read_file>`
+
+			const result = parseAssistantMessageV2(message)
+
+			// Should have text and tool use
+			expect(result).toHaveLength(2)
+			expect(result[0].type).toBe("text")
+			expect(result[1].type).toBe("tool_use")
+
+			const toolUse = result[1] as ToolUse
+			expect(toolUse.name).toBe("read_file")
+			expect(toolUse.params.args).toBeDefined()
+			expect(toolUse.params.args).toContain("<file>")
+			expect(toolUse.params.args).toContain("<path>src/shared/infrastructure/supabase/factory.py</path>")
+			expect(toolUse.params.args).toContain("<line_range>10-20</line_range>")
+		})
+
+		it("should handle structure with no spaces between XML tags", () => {
+			const message = `<read_file><args><file><path>test.py</path><line_range>1-10</line_range></file></args></read_file>`
+
+			const result = parseAssistantMessageV2(message)
+
+			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()
+			expect(toolUse.params.args).toContain("<path>test.py</path>")
+			expect(toolUse.params.args).toContain("<line_range>1-10</line_range>")
+		})
+
+		it("should handle structure with mixed spacing and newlines", () => {
+			const message = `<read_file>
+  <args>
+    <file><path>  src/test.py  </path>
+      <line_range>1-10</line_range>
+    </file>
+  </args>
+</read_file>`
+
+			const result = parseAssistantMessageV2(message)
+
+			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 args should preserve the internal structure
+			expect(toolUse.params.args).toContain("<file>")
+			expect(toolUse.params.args).toContain("<path>")
+			expect(toolUse.params.args).toContain("src/test.py")
+			expect(toolUse.params.args).toContain("</path>")
+			expect(toolUse.params.args).toContain("<line_range>1-10</line_range>")
+		})
+
+		it("should handle empty path element", () => {
+			const message = `<read_file>
+<args>
+<file>
+<path></path>
+<line_range>10-20</line_range>
+</file>
+</args>
+</read_file>`
+
+			const result = parseAssistantMessageV2(message)
+
+			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()
+			expect(toolUse.params.args).toContain("<path></path>")
+		})
+
+		it("should handle self-closing path element", () => {
+			const message = `<read_file>
+<args>
+<file>
+<path/>
+<line_range>10-20</line_range>
+</file>
+</args>
+</read_file>`
+
+			const result = parseAssistantMessageV2(message)
+
+			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()
+			expect(toolUse.params.args).toContain("<path/>")
+		})
+
+		it("should handle multiple files with varying structures", () => {
+			const message = `<read_file>
+<args>
+<file>
+<path> ./file1.ts </path>
+</file>
+<file>
+<path>
+  ./file2.ts
+</path>
+<line_range>10-20</line_range>
+</file>
+</args>
+</read_file>`
+
+			const result = parseAssistantMessageV2(message)
+
+			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()
+			// Check that both files are present
+			expect(toolUse.params.args).toContain("./file1.ts")
+			expect(toolUse.params.args).toContain("./file2.ts")
+			expect(toolUse.params.args).toContain("<line_range>10-20</line_range>")
+		})
+
+		it("should handle partial/incomplete tool use", () => {
+			const message = `<read_file>
+<args>
+<file>
+<path>test.py</path>`
+			// Message ends abruptly
+
+			const result = parseAssistantMessageV2(message)
+
+			expect(result).toHaveLength(1)
+			const toolUse = result[0] as ToolUse
+			expect(toolUse.type).toBe("tool_use")
+			expect(toolUse.name).toBe("read_file")
+			expect(toolUse.partial).toBe(true)
+			expect(toolUse.params.args).toBeDefined()
+			expect(toolUse.params.args).toContain("<path>test.py</path>")
+		})
+	})
+
+	describe("args parameter trimming behavior", () => {
+		it("should trim args parameter content", () => {
+			const message = `<read_file>
+<args>
+
+  <file>
+    <path>test.py</path>
+  </file>
+
+</args>
+</read_file>`
+
+			const result = parseAssistantMessageV2(message)
+
+			expect(result).toHaveLength(1)
+			const toolUse = result[0] as ToolUse
+			expect(toolUse.params.args).toBeDefined()
+			// args should be trimmed
+			expect(toolUse.params.args).not.toMatch(/^\s+/)
+			expect(toolUse.params.args).not.toMatch(/\s+$/)
+			expect(toolUse.params.args).toMatch(/^<file>/)
+			expect(toolUse.params.args).toMatch(/<\/file>$/)
+		})
+	})
+})

src/core/tools/__tests__/readFileTool-empty-path.spec.ts

@@ -0,0 +1,130 @@
+import { vi, describe, it, expect, beforeEach } from "vitest"
+import { readFileTool } from "../readFileTool"
+import { Task } from "../../task/Task"
+import { ReadFileToolUse } from "../../../shared/tools"
+
+describe("readFileTool - empty path handling", () => {
+	let mockCline: any
+	let mockAskApproval: any
+	let mockHandleError: any
+	let mockPushToolResult: any
+	let mockRemoveClosingTag: any
+
+	beforeEach(() => {
+		mockCline = {
+			cwd: "/test",
+			consecutiveMistakeCount: 0,
+			recordToolError: vi.fn(),
+			sayAndCreateMissingParamError: vi.fn().mockResolvedValue("Missing parameter error"),
+			say: vi.fn(),
+			rooIgnoreController: undefined,
+			api: {
+				getModel: () => ({ info: { supportsImages: false } }),
+			},
+		}
+
+		mockAskApproval = vi.fn().mockResolvedValue(true)
+		mockHandleError = vi.fn()
+		mockPushToolResult = vi.fn()
+		mockRemoveClosingTag = vi.fn()
+	})
+
+	it("should provide clear error message for empty path elements", async () => {
+		const toolUse: ReadFileToolUse = {
+			type: "tool_use",
+			name: "read_file",
+			params: {
+				args: `<file><path></path></file>`,
+			},
+			partial: false,
+		}
+
+		await readFileTool(
+			mockCline as unknown as Task,
+			toolUse,
+			mockAskApproval,
+			mockHandleError,
+			mockPushToolResult,
+			mockRemoveClosingTag,
+		)
+
+		// Should handle the error with a clear message
+		expect(mockHandleError).toHaveBeenCalledWith(
+			"parsing read_file args",
+			expect.objectContaining({
+				message: "All file paths are empty or missing. Please provide valid file paths in the <path> elements.",
+			}),
+		)
+
+		// Should push the error result
+		expect(mockPushToolResult).toHaveBeenCalledWith(
+			`<files><error>All file paths are empty or missing. Please provide valid file paths in the <path> elements.</error></files>`,
+		)
+	})
+
+	it("should provide clear error message for whitespace-only path elements", async () => {
+		const toolUse: ReadFileToolUse = {
+			type: "tool_use",
+			name: "read_file",
+			params: {
+				args: `<file><path>   </path></file>`,
+			},
+			partial: false,
+		}
+
+		await readFileTool(
+			mockCline as unknown as Task,
+			toolUse,
+			mockAskApproval,
+			mockHandleError,
+			mockPushToolResult,
+			mockRemoveClosingTag,
+		)
+
+		// Should handle the error with a clear message
+		expect(mockHandleError).toHaveBeenCalledWith(
+			"parsing read_file args",
+			expect.objectContaining({
+				message: "All file paths are empty or missing. Please provide valid file paths in the <path> elements.",
+			}),
+		)
+
+		// Should push the error result
+		expect(mockPushToolResult).toHaveBeenCalledWith(
+			`<files><error>All file paths are empty or missing. Please provide valid file paths in the <path> elements.</error></files>`,
+		)
+	})
+
+	// Note: Testing the case where some paths are empty but others are valid
+	// would require extensive mocking of file system operations.
+	// The core functionality is tested by the other tests which verify
+	// that empty paths are properly detected and reported.
+
+	it("should handle multiple empty paths", async () => {
+		const toolUse: ReadFileToolUse = {
+			type: "tool_use",
+			name: "read_file",
+			params: {
+				args: `<file><path></path></file><file><path>   </path></file><file><path/></file>`,
+			},
+			partial: false,
+		}
+
+		await readFileTool(
+			mockCline as unknown as Task,
+			toolUse,
+			mockAskApproval,
+			mockHandleError,
+			mockPushToolResult,
+			mockRemoveClosingTag,
+		)
+
+		// Should handle the error with a clear message
+		expect(mockHandleError).toHaveBeenCalledWith(
+			"parsing read_file args",
+			expect.objectContaining({
+				message: "All file paths are empty or missing. Please provide valid file paths in the <path> elements.",
+			}),
+		)
+	})
+})

src/core/tools/readFileTool.ts

@@ -131,8 +131,18 @@ export async function readFileTool(
 			const parsed = parseXml(argsXmlTag) as any
 			const files = Array.isArray(parsed.file) ? parsed.file : [parsed.file].filter(Boolean)
 
+			// Track empty paths for better error reporting
+			let emptyPathCount = 0
+			let totalFileCount = 0
+
 			for (const file of files) {
-				if (!file.path) continue // Skip if no path in a file entry
+				totalFileCount++
+
+				// Check for empty or whitespace-only paths
+				if (!file.path || (typeof file.path === "string" && file.path.trim() === "")) {
+					emptyPathCount++
+					continue // Skip if no path or empty path in a file entry
+				}
 
 				const fileEntry: FileEntry = {
 					path: file.path,
@@ -153,6 +163,14 @@ export async function readFileTool(
 				}
 				fileEntries.push(fileEntry)
 			}
+
+			// If all paths were empty, provide a more specific error
+			if (emptyPathCount > 0 && fileEntries.length === 0) {
+				const errorMessage = `All file paths are empty or missing. Please provide valid file paths in the <path> elements.`
+				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)}`
 			await handleError("parsing read_file args", new Error(errorMessage))