Skip to content

Commit ebfd384

Browse files
mrubensroomote
andauthored
More tolerant search/replace match (#6537)
Co-authored-by: Roo Code <[email protected]>
1 parent 305a5da commit ebfd384

File tree

3 files changed

+64
-13
lines changed

3 files changed

+64
-13
lines changed

src/core/diff/strategies/__tests__/multi-search-replace.spec.ts

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,51 @@ describe("MultiSearchReplaceDiffStrategy", () => {
1313
expect(strategy["validateMarkerSequencing"](diff).success).toBe(true)
1414
})
1515

16+
it("validates correct marker sequence with extra > in SEARCH", () => {
17+
const diff = "<<<<<<< SEARCH>\n" + "some content\n" + "=======\n" + "new content\n" + ">>>>>>> REPLACE"
18+
expect(strategy["validateMarkerSequencing"](diff).success).toBe(true)
19+
})
20+
21+
it("validates correct marker sequence with multiple > in SEARCH", () => {
22+
const diff = "<<<<<<< SEARCH>>\n" + "some content\n" + "=======\n" + "new content\n" + ">>>>>>> REPLACE"
23+
expect(strategy["validateMarkerSequencing"](diff).success).toBe(false)
24+
})
25+
26+
it("validates mixed cases with and without extra > in the same diff", () => {
27+
const diff =
28+
"<<<<<<< SEARCH>\n" +
29+
"content1\n" +
30+
"=======\n" +
31+
"new1\n" +
32+
">>>>>>> REPLACE\n\n" +
33+
"<<<<<<< SEARCH\n" +
34+
"content2\n" +
35+
"=======\n" +
36+
"new2\n" +
37+
">>>>>>> REPLACE"
38+
expect(strategy["validateMarkerSequencing"](diff).success).toBe(true)
39+
})
40+
41+
it("validates extra > with whitespace variations", () => {
42+
const diff1 = "<<<<<<< SEARCH> \n" + "some content\n" + "=======\n" + "new content\n" + ">>>>>>> REPLACE"
43+
expect(strategy["validateMarkerSequencing"](diff1).success).toBe(true)
44+
45+
const diff2 = "<<<<<<< SEARCH >\n" + "some content\n" + "=======\n" + "new content\n" + ">>>>>>> REPLACE"
46+
expect(strategy["validateMarkerSequencing"](diff2).success).toBe(false)
47+
})
48+
49+
it("validates extra > with line numbers", () => {
50+
const diff =
51+
"<<<<<<< SEARCH>\n" +
52+
":start_line:10\n" +
53+
"-------\n" +
54+
"content1\n" +
55+
"=======\n" +
56+
"new1\n" +
57+
">>>>>>> REPLACE"
58+
expect(strategy["validateMarkerSequencing"](diff).success).toBe(true)
59+
})
60+
1661
it("validates multiple correct marker sequences", () => {
1762
const diff =
1863
"<<<<<<< SEARCH\n" +

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

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -259,7 +259,10 @@ Each file requires its own path, start_line, and diff elements.
259259

260260
const state = { current: State.START, line: 0 }
261261

262-
const SEARCH = "<<<<<<< SEARCH"
262+
// Pattern allows optional '>' after SEARCH to handle AI-generated diffs
263+
// (e.g., Sonnet 4 sometimes adds an extra '>')
264+
const SEARCH_PATTERN = /^<<<<<<< SEARCH>?$/
265+
const SEARCH = SEARCH_PATTERN.source.replace(/[\^$]/g, "") // Remove regex anchors for display
263266
const SEP = "======="
264267
const REPLACE = ">>>>>>> REPLACE"
265268
const SEARCH_PREFIX = "<<<<<<< "
@@ -329,7 +332,7 @@ Each file requires its own path, start_line, and diff elements.
329332
})
330333

331334
const lines = diffContent.split("\n")
332-
const searchCount = lines.filter((l) => l.trim() === SEARCH).length
335+
const searchCount = lines.filter((l) => SEARCH_PATTERN.test(l.trim())).length
333336
const sepCount = lines.filter((l) => l.trim() === SEP).length
334337
const replaceCount = lines.filter((l) => l.trim() === REPLACE).length
335338

@@ -357,20 +360,20 @@ Each file requires its own path, start_line, and diff elements.
357360
: reportMergeConflictError(SEP, SEARCH)
358361
if (marker === REPLACE) return reportInvalidDiffError(REPLACE, SEARCH)
359362
if (marker.startsWith(REPLACE_PREFIX)) return reportMergeConflictError(marker, SEARCH)
360-
if (marker === SEARCH) state.current = State.AFTER_SEARCH
363+
if (SEARCH_PATTERN.test(marker)) state.current = State.AFTER_SEARCH
361364
else if (marker.startsWith(SEARCH_PREFIX)) return reportMergeConflictError(marker, SEARCH)
362365
break
363366

364367
case State.AFTER_SEARCH:
365-
if (marker === SEARCH) return reportInvalidDiffError(SEARCH, SEP)
368+
if (SEARCH_PATTERN.test(marker)) return reportInvalidDiffError(SEARCH_PATTERN.source, SEP)
366369
if (marker.startsWith(SEARCH_PREFIX)) return reportMergeConflictError(marker, SEARCH)
367370
if (marker === REPLACE) return reportInvalidDiffError(REPLACE, SEP)
368371
if (marker.startsWith(REPLACE_PREFIX)) return reportMergeConflictError(marker, SEARCH)
369372
if (marker === SEP) state.current = State.AFTER_SEPARATOR
370373
break
371374

372375
case State.AFTER_SEPARATOR:
373-
if (marker === SEARCH) return reportInvalidDiffError(SEARCH, REPLACE)
376+
if (SEARCH_PATTERN.test(marker)) return reportInvalidDiffError(SEARCH_PATTERN.source, REPLACE)
374377
if (marker.startsWith(SEARCH_PREFIX)) return reportMergeConflictError(marker, REPLACE)
375378
if (marker === SEP)
376379
return likelyBadStructure
@@ -456,7 +459,7 @@ Each file requires its own path, start_line, and diff elements.
456459

457460
/* Regex parts:
458461
1. (?:^|\n) Ensures the first marker starts at the beginning of the file or right after a newline.
459-
2. (?<!\\)<<<<<<< SEARCH\s*\n Matches the line "<<<<<<< SEARCH" (ignoring any trailing spaces) – the negative lookbehind makes sure it isn't escaped.
462+
2. (?<!\\)<<<<<<< SEARCH>?\s*\n Matches the line "<<<<<<< SEARCH" with optional '>' (ignoring any trailing spaces) – the negative lookbehind makes sure it isn't escaped.
460463
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.
461464
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.
462465
5. ((?<!\\)-------\s*\n)? Optionally matches the "-------" marker line (group 5).
@@ -467,7 +470,7 @@ Each file requires its own path, start_line, and diff elements.
467470
*/
468471
let matches = [
469472
...diffContent.matchAll(
470-
/(?:^|\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,
473+
/(?:^|\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,
471474
),
472475
]
473476

src/core/diff/strategies/multi-search-replace.ts

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -198,7 +198,10 @@ Only use a single line of '=======' between search and replacement content, beca
198198
}
199199
const state = { current: State.START, line: 0 }
200200

201-
const SEARCH = "<<<<<<< SEARCH"
201+
// Pattern allows optional '>' after SEARCH to handle AI-generated diffs
202+
// (e.g., Sonnet 4 sometimes adds an extra '>')
203+
const SEARCH_PATTERN = /^<<<<<<< SEARCH>?$/
204+
const SEARCH = SEARCH_PATTERN.source.replace(/[\^$]/g, "") // Remove regex anchors for display
202205
const SEP = "======="
203206
const REPLACE = ">>>>>>> REPLACE"
204207
const SEARCH_PREFIX = "<<<<<<<"
@@ -268,7 +271,7 @@ Only use a single line of '=======' between search and replacement content, beca
268271
})
269272

270273
const lines = diffContent.split("\n")
271-
const searchCount = lines.filter((l) => l.trim() === SEARCH).length
274+
const searchCount = lines.filter((l) => SEARCH_PATTERN.test(l.trim())).length
272275
const sepCount = lines.filter((l) => l.trim() === SEP).length
273276
const replaceCount = lines.filter((l) => l.trim() === REPLACE).length
274277

@@ -296,20 +299,20 @@ Only use a single line of '=======' between search and replacement content, beca
296299
: reportMergeConflictError(SEP, SEARCH)
297300
if (marker === REPLACE) return reportInvalidDiffError(REPLACE, SEARCH)
298301
if (marker.startsWith(REPLACE_PREFIX)) return reportMergeConflictError(marker, SEARCH)
299-
if (marker === SEARCH) state.current = State.AFTER_SEARCH
302+
if (SEARCH_PATTERN.test(marker)) state.current = State.AFTER_SEARCH
300303
else if (marker.startsWith(SEARCH_PREFIX)) return reportMergeConflictError(marker, SEARCH)
301304
break
302305

303306
case State.AFTER_SEARCH:
304-
if (marker === SEARCH) return reportInvalidDiffError(SEARCH, SEP)
307+
if (SEARCH_PATTERN.test(marker)) return reportInvalidDiffError(SEARCH_PATTERN.source, SEP)
305308
if (marker.startsWith(SEARCH_PREFIX)) return reportMergeConflictError(marker, SEARCH)
306309
if (marker === REPLACE) return reportInvalidDiffError(REPLACE, SEP)
307310
if (marker.startsWith(REPLACE_PREFIX)) return reportMergeConflictError(marker, SEARCH)
308311
if (marker === SEP) state.current = State.AFTER_SEPARATOR
309312
break
310313

311314
case State.AFTER_SEPARATOR:
312-
if (marker === SEARCH) return reportInvalidDiffError(SEARCH, REPLACE)
315+
if (SEARCH_PATTERN.test(marker)) return reportInvalidDiffError(SEARCH_PATTERN.source, REPLACE)
313316
if (marker.startsWith(SEARCH_PREFIX)) return reportMergeConflictError(marker, REPLACE)
314317
if (marker === SEP)
315318
return likelyBadStructure
@@ -378,7 +381,7 @@ Only use a single line of '=======' between search and replacement content, beca
378381

379382
let matches = [
380383
...diffContent.matchAll(
381-
/(?:^|\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,
384+
/(?:^|\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,
382385
),
383386
]
384387

0 commit comments

Comments
 (0)