test: add comprehensive tests for XML parsing with whitespace

- 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
This commit is contained in:
Roo Code 2025-09-09 03:00:33 +00:00
parent 195f4eb245
commit 593292c98c
3 changed files with 282 additions and 0 deletions

View file

@ -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

View file

@ -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()
})
})
})

View file

@ -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", () => {