fix: prevent XML entity decoding in diff tools (#7107) (#7108)

* fix: prevent XML entity decoding in diff tools

- Add parseXmlForDiff function with processEntities: false to preserve exact content
- Update multiApplyDiffTool to use parseXmlForDiff instead of parseXml
- Add comprehensive tests for entity handling in parseXmlForDiff

This fixes the issue where fast-xml-parser was decoding HTML entities like &
causing mismatches in diff tools when comparing against original file content.

Fixes #7107

* refactor: eliminate code duplication between parseXml and parseXmlForDiff

- Refactored parseXml to accept optional ParseXmlOptions parameter
- parseXmlForDiff now delegates to parseXml with processEntities: false
- Added explanatory comment in multiApplyDiffTool.ts about why parseXmlForDiff is used
- Improved JSDoc documentation with specific use cases for parseXmlForDiff

This maintains backward compatibility while eliminating code duplication.
parseXml continues to be used for general XML parsing (file reads, follow-up questions),
while parseXmlForDiff is specifically for diff operations where entity processing must be disabled.

---------

Co-authored-by: Roo Code <[email protected]>
Co-authored-by: daniel-lxs <[email protected]>

Author: 3 people

src/core/tools/multiApplyDiffTool.ts

@@ -12,7 +12,7 @@ import { formatResponse } from "../prompts/responses"
 import { fileExistsAtPath } from "../../utils/fs"
 import { RecordSource } from "../context-tracking/FileContextTrackerTypes"
 import { unescapeHtmlEntities } from "../../utils/text-normalization"
-import { parseXml } from "../../utils/xml"
+import { parseXmlForDiff } from "../../utils/xml"
 import { EXPERIMENT_IDS, experiments } from "../../shared/experiments"
 import { applyDiffToolLegacy } from "./applyDiffTool"
 
@@ -108,7 +108,10 @@ export async function applyDiffTool(
 	if (argsXmlTag) {
 		// Parse file entries from XML (new way)
 		try {
-			const parsed = parseXml(argsXmlTag, ["file.diff.content"]) as ParsedXmlResult
+			// IMPORTANT: We use parseXmlForDiff here instead of parseXml to prevent HTML entity decoding
+			// This ensures exact character matching when comparing parsed content against original file content
+			// Without this, special characters like & would be decoded to & causing diff mismatches
+			const parsed = parseXmlForDiff(argsXmlTag, ["file.diff.content"]) as ParsedXmlResult
 			const files = Array.isArray(parsed.file) ? parsed.file : [parsed.file].filter(Boolean)
 
 			for (const file of files) {

src/utils/__tests__/xml.spec.ts

@@ -1,4 +1,4 @@
-import { parseXml } from "../xml"
+import { parseXml, parseXmlForDiff } from "../xml"
 
 describe("parseXml", () => {
 	describe("type conversion", () => {
@@ -115,3 +115,126 @@ describe("parseXml", () => {
 		})
 	})
 })
+
+describe("parseXmlForDiff", () => {
+	describe("HTML entity handling", () => {
+		it("should NOT decode HTML entities like &", () => {
+			const xml = `
+        <root>
+          <content>Team Identity &amp; Project Positioning</content>
+        </root>
+      `
+
+			const result = parseXmlForDiff(xml) as any
+
+			// The &amp; should remain as-is, not be decoded to &
+			expect(result.root.content).toBe("Team Identity &amp; Project Positioning")
+		})
+
+		it("should preserve & character without encoding", () => {
+			const xml = `
+        <root>
+          <content>Team Identity & Project Positioning</content>
+        </root>
+      `
+
+			const result = parseXmlForDiff(xml) as any
+
+			// The & should remain as-is
+			expect(result.root.content).toBe("Team Identity & Project Positioning")
+		})
+
+		it("should NOT decode other HTML entities", () => {
+			const xml = `
+        <root>
+          <content>&lt;div&gt; &quot;Hello&quot; &apos;World&apos;</content>
+        </root>
+      `
+
+			const result = parseXmlForDiff(xml) as any
+
+			// All HTML entities should remain as-is
+			expect(result.root.content).toBe("&lt;div&gt; &quot;Hello&quot; &apos;World&apos;")
+		})
+
+		it("should handle mixed content with entities correctly", () => {
+			const xml = `
+        <root>
+          <code>if (a &lt; b &amp;&amp; c &gt; d) { return &quot;test&quot;; }</code>
+        </root>
+      `
+
+			const result = parseXmlForDiff(xml) as any
+
+			// All entities should remain unchanged
+			expect(result.root.code).toBe("if (a &lt; b &amp;&amp; c &gt; d) { return &quot;test&quot;; }")
+		})
+	})
+
+	describe("basic functionality (same as parseXml)", () => {
+		it("should correctly parse a simple XML string", () => {
+			const xml = `
+        <root>
+          <name>Test Name</name>
+          <description>Some description</description>
+        </root>
+      `
+
+			const result = parseXmlForDiff(xml) as any
+
+			expect(result).toHaveProperty("root")
+			expect(result.root).toHaveProperty("name", "Test Name")
+			expect(result.root).toHaveProperty("description", "Some description")
+		})
+
+		it("should handle attributes correctly", () => {
+			const xml = `
+        <root>
+          <item id="1" category="test">Item content</item>
+        </root>
+      `
+
+			const result = parseXmlForDiff(xml) as any
+
+			expect(result.root.item).toHaveProperty("@_id", "1")
+			expect(result.root.item).toHaveProperty("@_category", "test")
+			expect(result.root.item).toHaveProperty("#text", "Item content")
+		})
+
+		it("should support stopNodes parameter", () => {
+			const xml = `
+        <root>
+          <data>
+            <nestedXml><item>Should not parse this</item></nestedXml>
+          </data>
+        </root>
+      `
+
+			const result = parseXmlForDiff(xml, ["nestedXml"]) as any
+
+			expect(result.root.data.nestedXml).toBeTruthy()
+			expect(result.root.data.nestedXml).toHaveProperty("item", "Should not parse this")
+		})
+	})
+
+	describe("diff-specific use case", () => {
+		it("should preserve exact content for diff matching", () => {
+			// This simulates the actual use case from the issue
+			const xml = `
+        <args>
+          <file>
+            <path>./doc.md</path>
+            <diff>
+              <content>Team Identity & Project Positioning</content>
+            </diff>
+          </file>
+        </args>
+      `
+
+			const result = parseXmlForDiff(xml, ["file.diff.content"]) as any
+
+			// The & should remain as-is for exact matching with file content
+			expect(result.args.file.diff.content).toBe("Team Identity & Project Positioning")
+		})
+	})
+})

src/utils/xml.ts

@@ -1,20 +1,36 @@
 import { XMLParser } from "fast-xml-parser"
 
+/**
+ * Options for XML parsing
+ */
+interface ParseXmlOptions {
+	/**
+	 * Whether to process HTML entities (e.g., & to &).
+	 * Default: true for general parsing, false for diff operations
+	 */
+	processEntities?: boolean
+}
+
 /**
  * Parses an XML string into a JavaScript object
  * @param xmlString The XML string to parse
+ * @param stopNodes Optional array of node names to stop parsing at
+ * @param options Optional parsing options
  * @returns Parsed JavaScript object representation of the XML
  * @throws Error if the XML is invalid or parsing fails
  */
-export function parseXml(xmlString: string, stopNodes?: string[]): unknown {
+export function parseXml(xmlString: string, stopNodes?: string[], options?: ParseXmlOptions): unknown {
 	const _stopNodes = stopNodes ?? []
+	const processEntities = options?.processEntities ?? true
+
 	try {
 		const parser = new XMLParser({
 			ignoreAttributes: false,
 			attributeNamePrefix: "@_",
 			parseAttributeValue: false,
 			parseTagValue: false,
 			trimValues: true,
+			processEntities,
 			stopNodes: _stopNodes,
 		})
 
@@ -25,3 +41,23 @@ export function parseXml(xmlString: string, stopNodes?: string[]): unknown {
 		throw new Error(`Failed to parse XML: ${errorMessage}`)
 	}
 }
+
+/**
+ * Parses an XML string for diffing purposes, ensuring no HTML entities are decoded.
+ * This is a specialized version of parseXml to be used exclusively by diffing tools
+ * to prevent mismatches caused by entity processing.
+ *
+ * Use this instead of parseXml when:
+ * - Comparing parsed content against original file content
+ * - Performing diff operations where exact character matching is required
+ * - Processing XML that will be used in search/replace operations
+ *
+ * @param xmlString The XML string to parse
+ * @param stopNodes Optional array of node names to stop parsing at
+ * @returns Parsed JavaScript object representation of the XML
+ * @throws Error if the XML is invalid or parsing fails
+ */
+export function parseXmlForDiff(xmlString: string, stopNodes?: string[]): unknown {
+	// Delegate to parseXml with processEntities disabled
+	return parseXml(xmlString, stopNodes, { processEntities: false })
+}