فهرست منبع

fix: resolve apply_diff performance regression from PR #9456 (#9474)

Daniel 1 ماه پیش
والد
کامیت
999ea686ab

+ 189 - 0
src/core/diff/strategies/__tests__/multi-file-search-replace-8char.spec.ts

@@ -0,0 +1,189 @@
+import { describe, it, expect } from "vitest"
+import { MultiFileSearchReplaceDiffStrategy } from "../multi-file-search-replace"
+
+describe("MultiFileSearchReplaceDiffStrategy - 8-character marker support", () => {
+	it("should handle 8 '<' characters in SEARCH marker (PR #9456 use case)", async () => {
+		const strategy = new MultiFileSearchReplaceDiffStrategy()
+		const originalContent = "line 1\nline 2\nline 3"
+
+		const diff = `<<<<<<<< SEARCH
+:start_line:1
+-------
+line 1
+=======
+modified line 1
+>>>>>>>> REPLACE`
+
+		const result = await strategy.applyDiff(originalContent, diff)
+
+		expect(result.success).toBe(true)
+		if (result.success) {
+			expect(result.content).toBe("modified line 1\nline 2\nline 3")
+		}
+	})
+
+	it("should handle 7 '<' characters in SEARCH marker (standard)", async () => {
+		const strategy = new MultiFileSearchReplaceDiffStrategy()
+		const originalContent = "line 1\nline 2\nline 3"
+
+		const diff = `<<<<<<< SEARCH
+:start_line:1
+-------
+line 1
+=======
+modified line 1
+>>>>>>> REPLACE`
+
+		const result = await strategy.applyDiff(originalContent, diff)
+
+		expect(result.success).toBe(true)
+		if (result.success) {
+			expect(result.content).toBe("modified line 1\nline 2\nline 3")
+		}
+	})
+
+	it("should handle 8 '>' characters in REPLACE marker", async () => {
+		const strategy = new MultiFileSearchReplaceDiffStrategy()
+		const originalContent = "line 1\nline 2\nline 3"
+
+		const diff = `<<<<<<< SEARCH
+:start_line:2
+-------
+line 2
+=======
+modified line 2
+>>>>>>>> REPLACE`
+
+		const result = await strategy.applyDiff(originalContent, diff)
+
+		expect(result.success).toBe(true)
+		if (result.success) {
+			expect(result.content).toBe("line 1\nmodified line 2\nline 3")
+		}
+	})
+
+	it("should handle optional '<' at end of REPLACE marker", async () => {
+		const strategy = new MultiFileSearchReplaceDiffStrategy()
+		const originalContent = "line 1\nline 2\nline 3"
+
+		const diff = `<<<<<<< SEARCH
+:start_line:3
+-------
+line 3
+=======
+modified line 3
+>>>>>>> REPLACE<`
+
+		const result = await strategy.applyDiff(originalContent, diff)
+
+		expect(result.success).toBe(true)
+		if (result.success) {
+			expect(result.content).toBe("line 1\nline 2\nmodified line 3")
+		}
+	})
+
+	it("should handle mixed 7 and 8 character markers in same diff", async () => {
+		const strategy = new MultiFileSearchReplaceDiffStrategy()
+		const originalContent = "line 1\nline 2\nline 3"
+
+		const diff = `<<<<<<<< SEARCH
+:start_line:1
+-------
+line 1
+=======
+modified line 1
+>>>>>>> REPLACE
+
+<<<<<<< SEARCH
+:start_line:3
+-------
+line 3
+=======
+modified line 3
+>>>>>>>> REPLACE`
+
+		const result = await strategy.applyDiff(originalContent, diff)
+
+		expect(result.success).toBe(true)
+		if (result.success) {
+			expect(result.content).toBe("modified line 1\nline 2\nmodified line 3")
+		}
+	})
+
+	it("should reject markers with too many characters (9+)", async () => {
+		const strategy = new MultiFileSearchReplaceDiffStrategy()
+		const originalContent = "line 1\nline 2\nline 3"
+
+		const diff = `<<<<<<<<< SEARCH
+:start_line:1
+-------
+line 1
+=======
+modified line 1
+>>>>>>> REPLACE`
+
+		const result = await strategy.applyDiff(originalContent, diff)
+
+		expect(result.success).toBe(false)
+		if (!result.success) {
+			expect(result.error).toContain("Diff block is malformed")
+		}
+	})
+
+	it("should reject markers with too few characters (6-)", async () => {
+		const strategy = new MultiFileSearchReplaceDiffStrategy()
+		const originalContent = "line 1\nline 2\nline 3"
+
+		const diff = `<<<<<< SEARCH
+:start_line:1
+-------
+line 1
+=======
+modified line 1
+>>>>>>> REPLACE`
+
+		const result = await strategy.applyDiff(originalContent, diff)
+
+		expect(result.success).toBe(false)
+		if (!result.success) {
+			expect(result.error).toContain("Diff block is malformed")
+		}
+	})
+
+	it("should handle validation with 8 character markers", () => {
+		const strategy = new MultiFileSearchReplaceDiffStrategy()
+
+		const diff = `<<<<<<<< SEARCH
+:start_line:1
+-------
+content
+=======
+new content
+>>>>>>>> REPLACE`
+
+		const result = strategy["validateMarkerSequencing"](diff)
+
+		expect(result.success).toBe(true)
+	})
+
+	it("should detect merge conflict with 8 character prefix", () => {
+		const strategy = new MultiFileSearchReplaceDiffStrategy()
+
+		const diff = `<<<<<<<< SEARCH
+:start_line:1
+-------
+content
+<<<<<<<< HEAD
+conflict content
+=======
+new content
+>>>>>>>> REPLACE`
+
+		const result = strategy["validateMarkerSequencing"](diff)
+
+		expect(result.success).toBe(false)
+		if (!result.success) {
+			expect(result.error).toContain("merge conflict")
+		}
+	})
+})

+ 8 - 7
src/core/diff/strategies/multi-file-search-replace.ts

@@ -253,14 +253,15 @@ Each file requires its own path, start_line, and diff elements.
 
 		// Pattern allows optional extra '<' or '>' for SEARCH to handle AI-generated diffs
 		// (e.g., Sonnet 4 sometimes adds extra markers)
-		const SEARCH_PATTERN = /^<{7,8} SEARCH>?$/
+		// Using explicit alternation instead of quantifiers to avoid regex backtracking
+		const SEARCH_PATTERN = /^(?:<<<<<<< |<<<<<<<< )SEARCH>?$/
 		const SEARCH = "<<<<<<< SEARCH" // Simplified for display
 		const SEP = "======="
 		// Pattern allows optional extra '>' or '<' for REPLACE
-		const REPLACE_PATTERN = /^>{7,8} REPLACE<?$/
+		const REPLACE_PATTERN = /^(?:>>>>>>> |>>>>>>>> )REPLACE<?$/
 		const REPLACE = ">>>>>>> REPLACE" // Simplified for display
-		const SEARCH_PREFIX_PATTERN = /^<{7,8} /
-		const REPLACE_PREFIX_PATTERN = /^>{7,8} /
+		const SEARCH_PREFIX_PATTERN = /^(?:<<<<<<< |<<<<<<<< )/
+		const REPLACE_PREFIX_PATTERN = /^(?:>>>>>>> |>>>>>>>> )/
 
 		const reportMergeConflictError = (found: string, _expected: string) => ({
 			success: false,
@@ -453,18 +454,18 @@ Each file requires its own path, start_line, and diff elements.
 
 		/* Regex parts:
 		1. (?:^|\n)   Ensures the first marker starts at the beginning of the file or right after a newline.
-		2. (?<!\\)<{7,8} SEARCH>?\s*\n   Matches "<<<<<<< SEARCH" or "<<<<<<< SEARCH>" or "<<<<<<<<" with 7-8 '<' chars (ignoring any trailing spaces) – the negative lookbehind makes sure it isn't escaped.
+		2. (?<!\\)(?:<<<<<<< |<<<<<<<< )SEARCH>?\s*\n   Matches "<<<<<<< SEARCH" or "<<<<<<< SEARCH>" or "<<<<<<<< SEARCH" (7 or 8 '<' chars) (ignoring any trailing spaces) – the negative lookbehind makes sure it isn't escaped. Uses explicit alternation to avoid backtracking.
 		3. ((?:\:start_line:\s*(\d+)\s*\n))?   Optionally matches a ":start_line:" line. The outer capturing group is group 1 and the inner (\d+) is group 2.
 		4. ((?:\:end_line:\s*(\d+)\s*\n))?   Optionally matches a ":end_line:" line. Group 3 is the whole match and group 4 is the digits.
 		5. ((?<!\\)-------\s*\n)?   Optionally matches the "-------" marker line (group 5).
 		6. ([\s\S]*?)(?:\n)?   Non‐greedy match for the "search content" (group 6) up to the next marker.
 		7. (?:(?<=\n)(?<!\\)=======\s*\n)   Matches the "=======" marker on its own line.
 		8. ([\s\S]*?)(?:\n)?   Non‐greedy match for the "replace content" (group 7).
-		9. (?:(?<=\n)(?<!\\)>{7,8} REPLACE<?)(?=\n|$)   Matches ">>>>>>> REPLACE" or ">>>>>>> REPLACE<" or ">>>>>>>>" with 7-8 '>' chars on its own line (and requires a following newline or the end of file).
+		9. (?:(?<=\n)(?<!\\)(?:>>>>>>> |>>>>>>>> )REPLACE<?)(?=\n|$)   Matches ">>>>>>> REPLACE" or ">>>>>>> REPLACE<" or ">>>>>>>> REPLACE" (7 or 8 '>' chars) on its own line (and requires a following newline or the end of file). Uses explicit alternation to avoid backtracking.
 		*/
 		let matches = [
 			...diffContent.matchAll(
-				/(?:^|\n)(?<!\\)<{7,8} SEARCH>?\s*\n((?:\:start_line:\s*(\d+)\s*\n))?((?:\:end_line:\s*(\d+)\s*\n))?((?<!\\)-------\s*\n)?([\s\S]*?)(?:\n)?(?:(?<=\n)(?<!\\)=======\s*\n)([\s\S]*?)(?:\n)?(?:(?<=\n)(?<!\\)>{7,8} REPLACE<?)(?=\n|$)/g,
+				/(?:^|\n)(?<!\\)(?:<<<<<<< |<<<<<<<< )SEARCH>?\s*\n((?:\:start_line:\s*(\d+)\s*\n))?((?:\:end_line:\s*(\d+)\s*\n))?((?<!\\)-------\s*\n)?([\s\S]*?)(?:\n)?(?:(?<=\n)(?<!\\)=======\s*\n)([\s\S]*?)(?:\n)?(?:(?<=\n)(?<!\\)(?:>>>>>>> |>>>>>>>> )REPLACE<?)(?=\n|$)/g,
 			),
 		]