From 8db58907b0dcf4829805ab12cc9ffe6840175f3a Mon Sep 17 00:00:00 2001 From: Matt Rubens Date: Thu, 31 Jul 2025 23:58:49 -0400 Subject: [PATCH 1/2] More tolerant search/replace match --- .../strategies/__tests__/multi-search-replace.spec.ts | 5 +++++ src/core/diff/strategies/multi-file-search-replace.ts | 11 ++++++----- src/core/diff/strategies/multi-search-replace.ts | 11 ++++++----- 3 files changed, 17 insertions(+), 10 deletions(-) diff --git a/src/core/diff/strategies/__tests__/multi-search-replace.spec.ts b/src/core/diff/strategies/__tests__/multi-search-replace.spec.ts index 23900fc142..196ed4a7af 100644 --- a/src/core/diff/strategies/__tests__/multi-search-replace.spec.ts +++ b/src/core/diff/strategies/__tests__/multi-search-replace.spec.ts @@ -13,6 +13,11 @@ describe("MultiSearchReplaceDiffStrategy", () => { expect(strategy["validateMarkerSequencing"](diff).success).toBe(true) }) + it("validates correct marker sequence with extra > in SEARCH", () => { + const diff = "<<<<<<< SEARCH>\n" + "some content\n" + "=======\n" + "new content\n" + ">>>>>>> REPLACE" + expect(strategy["validateMarkerSequencing"](diff).success).toBe(true) + }) + it("validates multiple correct marker sequences", () => { const diff = "<<<<<<< SEARCH\n" + diff --git a/src/core/diff/strategies/multi-file-search-replace.ts b/src/core/diff/strategies/multi-file-search-replace.ts index 5ec223477c..e978182ae7 100644 --- a/src/core/diff/strategies/multi-file-search-replace.ts +++ b/src/core/diff/strategies/multi-file-search-replace.ts @@ -260,6 +260,7 @@ Each file requires its own path, start_line, and diff elements. const state = { current: State.START, line: 0 } const SEARCH = "<<<<<<< SEARCH" + const SEARCH_PATTERN = /^<<<<<<< SEARCH>?$/ const SEP = "=======" const REPLACE = ">>>>>>> REPLACE" const SEARCH_PREFIX = "<<<<<<< " @@ -329,7 +330,7 @@ Each file requires its own path, start_line, and diff elements. }) const lines = diffContent.split("\n") - const searchCount = lines.filter((l) => l.trim() === SEARCH).length + const searchCount = lines.filter((l) => SEARCH_PATTERN.test(l.trim())).length const sepCount = lines.filter((l) => l.trim() === SEP).length const replaceCount = lines.filter((l) => l.trim() === REPLACE).length @@ -357,12 +358,12 @@ Each file requires its own path, start_line, and diff elements. : reportMergeConflictError(SEP, SEARCH) if (marker === REPLACE) return reportInvalidDiffError(REPLACE, SEARCH) if (marker.startsWith(REPLACE_PREFIX)) return reportMergeConflictError(marker, SEARCH) - if (marker === SEARCH) state.current = State.AFTER_SEARCH + if (SEARCH_PATTERN.test(marker)) state.current = State.AFTER_SEARCH else if (marker.startsWith(SEARCH_PREFIX)) return reportMergeConflictError(marker, SEARCH) break case State.AFTER_SEARCH: - if (marker === SEARCH) return reportInvalidDiffError(SEARCH, SEP) + if (SEARCH_PATTERN.test(marker)) return reportInvalidDiffError(SEARCH, SEP) if (marker.startsWith(SEARCH_PREFIX)) return reportMergeConflictError(marker, SEARCH) if (marker === REPLACE) return reportInvalidDiffError(REPLACE, SEP) if (marker.startsWith(REPLACE_PREFIX)) return reportMergeConflictError(marker, SEARCH) @@ -370,7 +371,7 @@ Each file requires its own path, start_line, and diff elements. break case State.AFTER_SEPARATOR: - if (marker === SEARCH) return reportInvalidDiffError(SEARCH, REPLACE) + if (SEARCH_PATTERN.test(marker)) return reportInvalidDiffError(SEARCH, REPLACE) if (marker.startsWith(SEARCH_PREFIX)) return reportMergeConflictError(marker, REPLACE) if (marker === SEP) return likelyBadStructure @@ -467,7 +468,7 @@ Each file requires its own path, start_line, and diff elements. */ let matches = [ ...diffContent.matchAll( - /(?:^|\n)(?>>>>>> REPLACE)(?=\n|$)/g, + /(?:^|\n)(??\s*\n((?:\:start_line:\s*(\d+)\s*\n))?((?:\:end_line:\s*(\d+)\s*\n))?((?>>>>>> REPLACE)(?=\n|$)/g, ), ] diff --git a/src/core/diff/strategies/multi-search-replace.ts b/src/core/diff/strategies/multi-search-replace.ts index d4c14b169f..876b0e5abe 100644 --- a/src/core/diff/strategies/multi-search-replace.ts +++ b/src/core/diff/strategies/multi-search-replace.ts @@ -199,6 +199,7 @@ Only use a single line of '=======' between search and replacement content, beca const state = { current: State.START, line: 0 } const SEARCH = "<<<<<<< SEARCH" + const SEARCH_PATTERN = /^<<<<<<< SEARCH>?$/ const SEP = "=======" const REPLACE = ">>>>>>> REPLACE" const SEARCH_PREFIX = "<<<<<<<" @@ -268,7 +269,7 @@ Only use a single line of '=======' between search and replacement content, beca }) const lines = diffContent.split("\n") - const searchCount = lines.filter((l) => l.trim() === SEARCH).length + const searchCount = lines.filter((l) => SEARCH_PATTERN.test(l.trim())).length const sepCount = lines.filter((l) => l.trim() === SEP).length const replaceCount = lines.filter((l) => l.trim() === REPLACE).length @@ -296,12 +297,12 @@ Only use a single line of '=======' between search and replacement content, beca : reportMergeConflictError(SEP, SEARCH) if (marker === REPLACE) return reportInvalidDiffError(REPLACE, SEARCH) if (marker.startsWith(REPLACE_PREFIX)) return reportMergeConflictError(marker, SEARCH) - if (marker === SEARCH) state.current = State.AFTER_SEARCH + if (SEARCH_PATTERN.test(marker)) state.current = State.AFTER_SEARCH else if (marker.startsWith(SEARCH_PREFIX)) return reportMergeConflictError(marker, SEARCH) break case State.AFTER_SEARCH: - if (marker === SEARCH) return reportInvalidDiffError(SEARCH, SEP) + if (SEARCH_PATTERN.test(marker)) return reportInvalidDiffError(SEARCH, SEP) if (marker.startsWith(SEARCH_PREFIX)) return reportMergeConflictError(marker, SEARCH) if (marker === REPLACE) return reportInvalidDiffError(REPLACE, SEP) if (marker.startsWith(REPLACE_PREFIX)) return reportMergeConflictError(marker, SEARCH) @@ -309,7 +310,7 @@ Only use a single line of '=======' between search and replacement content, beca break case State.AFTER_SEPARATOR: - if (marker === SEARCH) return reportInvalidDiffError(SEARCH, REPLACE) + if (SEARCH_PATTERN.test(marker)) return reportInvalidDiffError(SEARCH, REPLACE) if (marker.startsWith(SEARCH_PREFIX)) return reportMergeConflictError(marker, REPLACE) if (marker === SEP) return likelyBadStructure @@ -378,7 +379,7 @@ Only use a single line of '=======' between search and replacement content, beca let matches = [ ...diffContent.matchAll( - /(?:^|\n)(?>>>>>> REPLACE)(?=\n|$)/g, + /(?:^|\n)(??\s*\n((?:\:start_line:\s*(\d+)\s*\n))?((?:\:end_line:\s*(\d+)\s*\n))?((?>>>>>> REPLACE)(?=\n|$)/g, ), ] From 0f504cf053ca663a9f34fc44b3b9d0818de778e9 Mon Sep 17 00:00:00 2001 From: Roo Code Date: Fri, 1 Aug 2025 04:17:17 +0000 Subject: [PATCH 2/2] feat: address PR feedback for more tolerant search/replace - Use SEARCH_PATTERN.source consistently in error messages - Add explanatory comment about Sonnet 4 behavior - Add edge case tests for multiple >, mixed cases, and whitespace variations - Update regex comment to mention optional > character --- .../__tests__/multi-search-replace.spec.ts | 40 +++++++++++++++++++ .../strategies/multi-file-search-replace.ts | 10 +++-- .../diff/strategies/multi-search-replace.ts | 8 ++-- 3 files changed, 51 insertions(+), 7 deletions(-) diff --git a/src/core/diff/strategies/__tests__/multi-search-replace.spec.ts b/src/core/diff/strategies/__tests__/multi-search-replace.spec.ts index 196ed4a7af..b25286f5fa 100644 --- a/src/core/diff/strategies/__tests__/multi-search-replace.spec.ts +++ b/src/core/diff/strategies/__tests__/multi-search-replace.spec.ts @@ -18,6 +18,46 @@ describe("MultiSearchReplaceDiffStrategy", () => { expect(strategy["validateMarkerSequencing"](diff).success).toBe(true) }) + it("validates correct marker sequence with multiple > in SEARCH", () => { + const diff = "<<<<<<< SEARCH>>\n" + "some content\n" + "=======\n" + "new content\n" + ">>>>>>> REPLACE" + expect(strategy["validateMarkerSequencing"](diff).success).toBe(false) + }) + + it("validates mixed cases with and without extra > in the same diff", () => { + const diff = + "<<<<<<< SEARCH>\n" + + "content1\n" + + "=======\n" + + "new1\n" + + ">>>>>>> REPLACE\n\n" + + "<<<<<<< SEARCH\n" + + "content2\n" + + "=======\n" + + "new2\n" + + ">>>>>>> REPLACE" + expect(strategy["validateMarkerSequencing"](diff).success).toBe(true) + }) + + it("validates extra > with whitespace variations", () => { + const diff1 = "<<<<<<< SEARCH> \n" + "some content\n" + "=======\n" + "new content\n" + ">>>>>>> REPLACE" + expect(strategy["validateMarkerSequencing"](diff1).success).toBe(true) + + const diff2 = "<<<<<<< SEARCH >\n" + "some content\n" + "=======\n" + "new content\n" + ">>>>>>> REPLACE" + expect(strategy["validateMarkerSequencing"](diff2).success).toBe(false) + }) + + it("validates extra > with line numbers", () => { + const diff = + "<<<<<<< SEARCH>\n" + + ":start_line:10\n" + + "-------\n" + + "content1\n" + + "=======\n" + + "new1\n" + + ">>>>>>> REPLACE" + expect(strategy["validateMarkerSequencing"](diff).success).toBe(true) + }) + it("validates multiple correct marker sequences", () => { const diff = "<<<<<<< SEARCH\n" + diff --git a/src/core/diff/strategies/multi-file-search-replace.ts b/src/core/diff/strategies/multi-file-search-replace.ts index e978182ae7..c71d3c3807 100644 --- a/src/core/diff/strategies/multi-file-search-replace.ts +++ b/src/core/diff/strategies/multi-file-search-replace.ts @@ -259,8 +259,10 @@ Each file requires its own path, start_line, and diff elements. const state = { current: State.START, line: 0 } - const SEARCH = "<<<<<<< SEARCH" + // Pattern allows optional '>' after SEARCH to handle AI-generated diffs + // (e.g., Sonnet 4 sometimes adds an extra '>') const SEARCH_PATTERN = /^<<<<<<< SEARCH>?$/ + const SEARCH = SEARCH_PATTERN.source.replace(/[\^$]/g, "") // Remove regex anchors for display const SEP = "=======" const REPLACE = ">>>>>>> REPLACE" const SEARCH_PREFIX = "<<<<<<< " @@ -363,7 +365,7 @@ Each file requires its own path, start_line, and diff elements. break case State.AFTER_SEARCH: - if (SEARCH_PATTERN.test(marker)) return reportInvalidDiffError(SEARCH, SEP) + if (SEARCH_PATTERN.test(marker)) return reportInvalidDiffError(SEARCH_PATTERN.source, SEP) if (marker.startsWith(SEARCH_PREFIX)) return reportMergeConflictError(marker, SEARCH) if (marker === REPLACE) return reportInvalidDiffError(REPLACE, SEP) if (marker.startsWith(REPLACE_PREFIX)) return reportMergeConflictError(marker, SEARCH) @@ -371,7 +373,7 @@ Each file requires its own path, start_line, and diff elements. break case State.AFTER_SEPARATOR: - if (SEARCH_PATTERN.test(marker)) return reportInvalidDiffError(SEARCH, REPLACE) + if (SEARCH_PATTERN.test(marker)) return reportInvalidDiffError(SEARCH_PATTERN.source, REPLACE) if (marker.startsWith(SEARCH_PREFIX)) return reportMergeConflictError(marker, REPLACE) if (marker === SEP) return likelyBadStructure @@ -457,7 +459,7 @@ 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. (??\s*\n Matches the line "<<<<<<< SEARCH" with optional '>' (ignoring any trailing spaces) – the negative lookbehind makes sure it isn't escaped. 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. ((?' after SEARCH to handle AI-generated diffs + // (e.g., Sonnet 4 sometimes adds an extra '>') const SEARCH_PATTERN = /^<<<<<<< SEARCH>?$/ + const SEARCH = SEARCH_PATTERN.source.replace(/[\^$]/g, "") // Remove regex anchors for display const SEP = "=======" const REPLACE = ">>>>>>> REPLACE" const SEARCH_PREFIX = "<<<<<<<" @@ -302,7 +304,7 @@ Only use a single line of '=======' between search and replacement content, beca break case State.AFTER_SEARCH: - if (SEARCH_PATTERN.test(marker)) return reportInvalidDiffError(SEARCH, SEP) + if (SEARCH_PATTERN.test(marker)) return reportInvalidDiffError(SEARCH_PATTERN.source, SEP) if (marker.startsWith(SEARCH_PREFIX)) return reportMergeConflictError(marker, SEARCH) if (marker === REPLACE) return reportInvalidDiffError(REPLACE, SEP) if (marker.startsWith(REPLACE_PREFIX)) return reportMergeConflictError(marker, SEARCH) @@ -310,7 +312,7 @@ Only use a single line of '=======' between search and replacement content, beca break case State.AFTER_SEPARATOR: - if (SEARCH_PATTERN.test(marker)) return reportInvalidDiffError(SEARCH, REPLACE) + if (SEARCH_PATTERN.test(marker)) return reportInvalidDiffError(SEARCH_PATTERN.source, REPLACE) if (marker.startsWith(SEARCH_PREFIX)) return reportMergeConflictError(marker, REPLACE) if (marker === SEP) return likelyBadStructure