test: add comprehensive tests for XML parsing with whitespace

roomote · roomote · commit 593292c98cd1 · 2025-09-09T03:00:33.000Z
- Add tests to verify XML parser handles whitespace correctly - Add tests for assistant message parser with whitespace in XML - Add tests for readFileTool handling XML args with whitespace - Tests confirm existing implementation already handles whitespace properly - Addresses issue #7664 where Grok/Qwen3-Coder generate XML with spaces
diff --git 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 = `<read_file>
+<args>
+<file>
+<path>src/shared/infrastructure/supabase/factory.py</path>
+</file>
+</args>
+</read_file>`
+				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("<file>")
+				expect(toolUse.params.args).toContain("<path>src/shared/infrastructure/supabase/factory.py</path>")
+				expect(toolUse.partial).toBe(false)
+			})
+
+			it("should handle whitespace-only path values", () => {
+				const message = `<read_file>
+<args>
+<file>
+<path>   </path>
+</file>
+</args>
+</read_file>`
+				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("<path>   </path>")
+				expect(toolUse.partial).toBe(false)
+			})
+
 			it("should handle multi-line parameters", () => {
 				const message = `<write_to_file><path>file.ts</path><content>
 	line 1
diff --git a/src/core/tools/__tests__/readFileTool.spec.ts b/src/core/tools/__tests__/readFileTool.spec.ts
@@ -1325,6 +1325,132 @@ describe("read_file tool XML output structure", () => {
 				`<files>\n<file><path>${testFilePath}</path><error>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.</error></file>\n</files>`,
 			)
 		})
+
+		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 = `
+<file>
+<path>   test/file.txt   </path>
+</file>
+`
+			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("<file><path>test/file.txt</path>")
+			expect(toolResult).toContain('<content lines="1-5">')
+			expect(toolResult).not.toContain("<error>")
+		})
+
+		it("should handle multiple files with whitespace in XML", async () => {
+			// Test multiple files with varying whitespace
+			const argsWithMultipleFiles = `
+<file>
+	<path>
+		test/file1.txt
+	</path>
+</file>
+<file>
+	<path>  test/file2.txt  </path>
+</file>
+<file>
+	<path>test/file3.txt</path>
+</file>
+`
+			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("<file><path>test/file1.txt</path>")
+			expect(toolResult).toContain("<file><path>test/file2.txt</path>")
+			expect(toolResult).toContain("<file><path>test/file3.txt</path>")
+			expect(toolResult).not.toContain("<error>")
+		})
+
+		it("should handle empty path after trimming whitespace", async () => {
+			// Test case where path is only whitespace
+			const argsWithEmptyPath = `
+<file>
+<path>     </path>
+</file>
+`
+			const toolUse: ReadFileToolUse = {
+				type: "tool_use",
+				name: "read_file",
+				params: { args: argsWithEmptyPath },
+				partial: false,
+			}
+
+			// Execute
+			await readFileTool(
+				mockCline,
+				toolUse,
+				mockCline.ask,
+				vi.fn(),
+				(result: ToolResponse) => {
+					toolResult = result
+				},
+				(param: ToolParamName, content?: string) => content ?? "",
+			)
+
+			// Verify error is returned for empty path
+			expect(toolResult).toContain("<error>")
+			expect(mockCline.sayAndCreateMissingParamError).toHaveBeenCalled()
+		})
 	})
 })
 
diff --git 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 = `
+				<args>
+					<file>
+						<path> ./test/file.ts </path>
+					</file>
+				</args>
+			`
+
+			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 = `
+				<args>
+					<file>
+						<path>
+							src/shared/infrastructure/supabase/factory.py
+						</path>
+					</file>
+				</args>
+			`
+
+			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 = `
+				<args>
+					<file>
+						<path> file1.ts </path>
+					</file>
+					<file>
+						<path>
+							file2.ts
+						</path>
+					</file>
+					<file>
+						<path>file3.ts</path>
+					</file>
+				</args>
+			`
+
+			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 = `
+				<args>
+					<file>
+						<path>   </path>
+					</file>
+				</args>
+			`
+
+			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 = `
+				<args>
+					<file>
+						<path>
+							./path/with/tabs.ts
+							</path>
+					</file>
+				</args>
+			`
+
+			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 = `<read_file>
+<args>
+<file>
+<path>src/shared/infrastructure/supabase/factory.py</path>
+</file>
+</args>
+</read_file>`
+
+			// First extract just the args portion
+			const argsMatch = xml.match(/<args>([\s\S]*?)<\/args>/)
+			expect(argsMatch).toBeTruthy()
+
+			if (argsMatch) {
+				const argsXml = `<args>${argsMatch[1]}</args>`
+				const result = parseXml(argsXml) as any
+
+				expect(result.args.file.path).toBe("src/shared/infrastructure/supabase/factory.py")
+			}
+		})
+	})
 })
 
 describe("parseXmlForDiff", () => {
