fix: validate separator newline in apply_diff to catch malformed ------- markers

When the LLM omits the newline after the ------- separator (e.g. "-------import"),
the parser silently absorbs the separator into the search content, producing a
confusing "No sufficiently similar match found" error. This causes smaller models
to loop without recovering.

Add detection in validateMarkerSequencing for lines starting with "-------" that
have non-whitespace content appended, returning a clear actionable error message
that tells the model to place the separator on its own line.

Closes #12210
This commit is contained in:
Roo Code 2026-04-28 11:32:39 +00:00
parent ad25634905
commit 347485717c
2 changed files with 58 additions and 0 deletions

View file

@ -131,6 +131,43 @@ describe("MultiSearchReplaceDiffStrategy", () => {
expect(result.error).toContain("Expected '>>>>>>> REPLACE' was not found")
})
it("detects malformed separator with content on the same line (no newline after -------)", () => {
const diff =
"<<<<<<< SEARCH\n" +
":start_line:7\n" +
"-------import { useTranslate } from '../../i18n/I18nContext';\n" +
"=======\n" +
"new content\n" +
">>>>>>> REPLACE"
const result = strategy["validateMarkerSequencing"](diff)
expect(result.success).toBe(false)
expect(result.error).toContain("separator line '-------'")
expect(result.error).toContain("must be on its own line")
expect(result.error).toContain("import { useTranslate }")
})
it("detects malformed separator without start_line", () => {
const diff =
"<<<<<<< SEARCH\n" + "-------some content here\n" + "=======\n" + "new content\n" + ">>>>>>> REPLACE"
const result = strategy["validateMarkerSequencing"](diff)
expect(result.success).toBe(false)
expect(result.error).toContain("separator line '-------'")
expect(result.error).toContain("some content here")
})
it("allows properly formatted separator on its own line", () => {
const diff =
"<<<<<<< SEARCH\n" +
":start_line:7\n" +
"-------\n" +
"import { useTranslate } from '../../i18n/I18nContext';\n" +
"=======\n" +
"new content\n" +
">>>>>>> REPLACE"
const result = strategy["validateMarkerSequencing"](diff)
expect(result.success).toBe(true)
})
describe("exact matching", () => {
let strategy: MultiSearchReplaceDiffStrategy

View file

@ -216,6 +216,27 @@ export class MultiSearchReplaceDiffStrategy implements DiffStrategy {
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)
// Detect malformed separator: `-------` with content on the same line (missing newline)
if (/^-------\S/.test(marker)) {
const trailingContent = marker.slice(7)
return {
success: false,
error:
`ERROR: The separator line '-------' at line ${state.line} must be on its own line followed by a newline. ` +
`Found content '${trailingContent.length > 40 ? trailingContent.slice(0, 40) + "..." : trailingContent}' on the same line.\n` +
"\n" +
"CORRECT FORMAT:\n\n" +
"<<<<<<< SEARCH\n" +
":start_line:5\n" +
"-------\n" +
"search content here\n" +
"=======\n" +
"replacement content\n" +
">>>>>>> REPLACE\n" +
"\n" +
"The '-------' separator must be on its own line, with the search content starting on the next line.",
}
}
if (marker === SEP) state.current = State.AFTER_SEPARATOR
break