mirror of
https://github.com/RooVetGit/Roo-Code.git
synced 2026-10-07 02:58:15 +00:00
fix: prevent start_line/end_line in apply_diff REPLACE (#4015)
Adds validation to ensure that `:start_line:` and `:end_line:` markers do not appear in the REPLACE section of an apply_diff operation. These markers are only valid within the SEARCH section. This change prevents potential errors and confusion when users might inadvertently include these markers in the replacement content. New tests have been added to verify this validation. Fixes: #4013 Signed-off-by: Eric Wheeler <roo-code@z.ewheeler.org> Co-authored-by: Eric Wheeler <roo-code@z.ewheeler.org>
This commit is contained in:
KJ7LNW
GitHub
parent
ff422b12f7
commit
53f94a7852
2 changed files with 198 additions and 0 deletions
|
|
@ -2407,4 +2407,168 @@ function two() {
|
|||
expect(description).toContain("</apply_diff>")
|
||||
})
|
||||
})
|
||||
|
||||
describe("line marker validation in REPLACE sections", () => {
|
||||
let strategy: MultiSearchReplaceDiffStrategy
|
||||
|
||||
beforeEach(() => {
|
||||
strategy = new MultiSearchReplaceDiffStrategy()
|
||||
})
|
||||
|
||||
it("should reject start_line marker in REPLACE section", () => {
|
||||
const diff =
|
||||
"<<<<<<< SEARCH\n" +
|
||||
"content to find\n" +
|
||||
"=======\n" +
|
||||
":start_line:5\n" +
|
||||
"replacement content\n" +
|
||||
">>>>>>> REPLACE"
|
||||
const result = strategy["validateMarkerSequencing"](diff)
|
||||
expect(result.success).toBe(false)
|
||||
expect(result.error).toContain("Invalid line marker ':start_line:' found in REPLACE section")
|
||||
expect(result.error).toContain(
|
||||
"Line markers (:start_line: and :end_line:) are only allowed in SEARCH sections",
|
||||
)
|
||||
})
|
||||
|
||||
it("should reject end_line marker in REPLACE section", () => {
|
||||
const diff =
|
||||
"<<<<<<< SEARCH\n" +
|
||||
"content to find\n" +
|
||||
"=======\n" +
|
||||
":end_line:10\n" +
|
||||
"replacement content\n" +
|
||||
">>>>>>> REPLACE"
|
||||
const result = strategy["validateMarkerSequencing"](diff)
|
||||
expect(result.success).toBe(false)
|
||||
expect(result.error).toContain("Invalid line marker ':end_line:' found in REPLACE section")
|
||||
expect(result.error).toContain(
|
||||
"Line markers (:start_line: and :end_line:) are only allowed in SEARCH sections",
|
||||
)
|
||||
})
|
||||
|
||||
it("should reject both line markers in REPLACE section", () => {
|
||||
const diff =
|
||||
"<<<<<<< SEARCH\n" +
|
||||
"content to find\n" +
|
||||
"=======\n" +
|
||||
":start_line:5\n" +
|
||||
":end_line:10\n" +
|
||||
"replacement content\n" +
|
||||
">>>>>>> REPLACE"
|
||||
const result = strategy["validateMarkerSequencing"](diff)
|
||||
expect(result.success).toBe(false)
|
||||
expect(result.error).toContain("Invalid line marker ':start_line:' found in REPLACE section")
|
||||
})
|
||||
|
||||
it("should reject line markers in multiple diff blocks where one has invalid markers", () => {
|
||||
const diff =
|
||||
"<<<<<<< SEARCH\n" +
|
||||
":start_line:1\n" +
|
||||
"content1\n" +
|
||||
"=======\n" +
|
||||
"replacement1\n" +
|
||||
">>>>>>> REPLACE\n\n" +
|
||||
"<<<<<<< SEARCH\n" +
|
||||
"content2\n" +
|
||||
"=======\n" +
|
||||
":start_line:5\n" +
|
||||
"replacement2\n" +
|
||||
">>>>>>> REPLACE"
|
||||
const result = strategy["validateMarkerSequencing"](diff)
|
||||
expect(result.success).toBe(false)
|
||||
expect(result.error).toContain("Invalid line marker ':start_line:' found in REPLACE section")
|
||||
})
|
||||
|
||||
it("should allow valid markers in SEARCH section with content in REPLACE", () => {
|
||||
const diff =
|
||||
"<<<<<<< SEARCH\n" +
|
||||
":start_line:5\n" +
|
||||
":end_line:10\n" +
|
||||
"-------\n" +
|
||||
"content to find\n" +
|
||||
"=======\n" +
|
||||
"replacement content\n" +
|
||||
">>>>>>> REPLACE"
|
||||
const result = strategy["validateMarkerSequencing"](diff)
|
||||
expect(result.success).toBe(true)
|
||||
})
|
||||
|
||||
it("should allow escaped line markers in REPLACE content", () => {
|
||||
const diff =
|
||||
"<<<<<<< SEARCH\n" +
|
||||
"content to find\n" +
|
||||
"=======\n" +
|
||||
"replacement content\n" +
|
||||
"\\:start_line:5\n" +
|
||||
"more content\n" +
|
||||
">>>>>>> REPLACE"
|
||||
const result = strategy["validateMarkerSequencing"](diff)
|
||||
expect(result.success).toBe(true)
|
||||
})
|
||||
|
||||
it("should allow escaped end_line markers in REPLACE content", () => {
|
||||
const diff =
|
||||
"<<<<<<< SEARCH\n" +
|
||||
"content to find\n" +
|
||||
"=======\n" +
|
||||
"replacement content\n" +
|
||||
"\\:end_line:10\n" +
|
||||
"more content\n" +
|
||||
">>>>>>> REPLACE"
|
||||
const result = strategy["validateMarkerSequencing"](diff)
|
||||
expect(result.success).toBe(true)
|
||||
})
|
||||
|
||||
it("should allow both escaped line markers in REPLACE content", () => {
|
||||
const diff =
|
||||
"<<<<<<< SEARCH\n" +
|
||||
"content to find\n" +
|
||||
"=======\n" +
|
||||
"replacement content\n" +
|
||||
"\\:start_line:5\n" +
|
||||
"\\:end_line:10\n" +
|
||||
"more content\n" +
|
||||
">>>>>>> REPLACE"
|
||||
const result = strategy["validateMarkerSequencing"](diff)
|
||||
expect(result.success).toBe(true)
|
||||
})
|
||||
|
||||
it("should reject line markers with whitespace in REPLACE section", () => {
|
||||
const diff =
|
||||
"<<<<<<< SEARCH\n" +
|
||||
"content to find\n" +
|
||||
"=======\n" +
|
||||
" :start_line:5 \n" +
|
||||
"replacement content\n" +
|
||||
">>>>>>> REPLACE"
|
||||
const result = strategy["validateMarkerSequencing"](diff)
|
||||
expect(result.success).toBe(false)
|
||||
expect(result.error).toContain("Invalid line marker ':start_line:' found in REPLACE section")
|
||||
})
|
||||
|
||||
it("should reject line markers in middle of REPLACE content", () => {
|
||||
const diff =
|
||||
"<<<<<<< SEARCH\n" +
|
||||
"content to find\n" +
|
||||
"=======\n" +
|
||||
"some replacement\n" +
|
||||
":end_line:15\n" +
|
||||
"more replacement\n" +
|
||||
">>>>>>> REPLACE"
|
||||
const result = strategy["validateMarkerSequencing"](diff)
|
||||
expect(result.success).toBe(false)
|
||||
expect(result.error).toContain("Invalid line marker ':end_line:' found in REPLACE section")
|
||||
})
|
||||
|
||||
it("should provide helpful error message format", () => {
|
||||
const diff =
|
||||
"<<<<<<< SEARCH\n" + "content\n" + "=======\n" + ":start_line:5\n" + "replacement\n" + ">>>>>>> REPLACE"
|
||||
const result = strategy["validateMarkerSequencing"](diff)
|
||||
expect(result.success).toBe(false)
|
||||
expect(result.error).toContain("CORRECT FORMAT:")
|
||||
expect(result.error).toContain("INCORRECT FORMAT:")
|
||||
expect(result.error).toContain(":start_line:5 <-- Invalid location")
|
||||
})
|
||||
})
|
||||
})
|
||||
|
|
|
|||
|
|
@ -243,6 +243,30 @@ Only use a single line of '=======' between search and replacement content, beca
|
|||
">>>>>>> REPLACE\n",
|
||||
})
|
||||
|
||||
const reportLineMarkerInReplaceError = (marker: string) => ({
|
||||
success: false,
|
||||
error:
|
||||
`ERROR: Invalid line marker '${marker}' found in REPLACE section at line ${state.line}\n` +
|
||||
"\n" +
|
||||
"Line markers (:start_line: and :end_line:) are only allowed in SEARCH sections.\n" +
|
||||
"\n" +
|
||||
"CORRECT FORMAT:\n" +
|
||||
"<<<<<<< SEARCH\n" +
|
||||
":start_line:5\n" +
|
||||
"content to find\n" +
|
||||
"=======\n" +
|
||||
"replacement content\n" +
|
||||
">>>>>>> REPLACE\n" +
|
||||
"\n" +
|
||||
"INCORRECT FORMAT:\n" +
|
||||
"<<<<<<< SEARCH\n" +
|
||||
"content to find\n" +
|
||||
"=======\n" +
|
||||
":start_line:5 <-- Invalid location\n" +
|
||||
"replacement content\n" +
|
||||
">>>>>>> REPLACE\n",
|
||||
})
|
||||
|
||||
const lines = diffContent.split("\n")
|
||||
const searchCount = lines.filter((l) => l.trim() === SEARCH).length
|
||||
const sepCount = lines.filter((l) => l.trim() === SEP).length
|
||||
|
|
@ -254,6 +278,16 @@ Only use a single line of '=======' between search and replacement content, beca
|
|||
state.line++
|
||||
const marker = line.trim()
|
||||
|
||||
// Check for line markers in REPLACE sections (but allow escaped ones)
|
||||
if (state.current === State.AFTER_SEPARATOR) {
|
||||
if (marker.startsWith(":start_line:") && !line.trim().startsWith("\\:start_line:")) {
|
||||
return reportLineMarkerInReplaceError(":start_line:")
|
||||
}
|
||||
if (marker.startsWith(":end_line:") && !line.trim().startsWith("\\:end_line:")) {
|
||||
return reportLineMarkerInReplaceError(":end_line:")
|
||||
}
|
||||
}
|
||||
|
||||
switch (state.current) {
|
||||
case State.START:
|
||||
if (marker === SEP)
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue