diff --git a/src/core/tools/multiApplyDiffTool.ts b/src/core/tools/multiApplyDiffTool.ts index db514d2b64..4cce2d1777 100644 --- a/src/core/tools/multiApplyDiffTool.ts +++ b/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) { diff --git a/src/utils/__tests__/xml.spec.ts b/src/utils/__tests__/xml.spec.ts index 0e43cf04a1..f7a282b0c0 100644 --- a/src/utils/__tests__/xml.spec.ts +++ b/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 = ` + + Team Identity & Project Positioning + + ` + + const result = parseXmlForDiff(xml) as any + + // The & should remain as-is, not be decoded to & + expect(result.root.content).toBe("Team Identity & Project Positioning") + }) + + it("should preserve & character without encoding", () => { + const xml = ` + + Team Identity & Project Positioning + + ` + + 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 = ` + + <div> "Hello" 'World' + + ` + + const result = parseXmlForDiff(xml) as any + + // All HTML entities should remain as-is + expect(result.root.content).toBe("<div> "Hello" 'World'") + }) + + it("should handle mixed content with entities correctly", () => { + const xml = ` + + if (a < b && c > d) { return "test"; } + + ` + + const result = parseXmlForDiff(xml) as any + + // All entities should remain unchanged + expect(result.root.code).toBe("if (a < b && c > d) { return "test"; }") + }) + }) + + describe("basic functionality (same as parseXml)", () => { + it("should correctly parse a simple XML string", () => { + const xml = ` + + Test Name + Some description + + ` + + 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 = ` + + Item content + + ` + + 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 = ` + + + Should not parse this + + + ` + + 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 = ` + + + ./doc.md + + Team Identity & Project Positioning + + + + ` + + 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") + }) + }) +}) diff --git a/src/utils/xml.ts b/src/utils/xml.ts index 0fd6ef574c..f183309d49 100644 --- a/src/utils/xml.ts +++ b/src/utils/xml.ts @@ -1,13 +1,28 @@ 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, @@ -15,6 +30,7 @@ export function parseXml(xmlString: string, stopNodes?: string[]): unknown { 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 }) +}