mirror of
https://github.com/RooVetGit/Roo-Code.git
synced 2026-08-28 05:27:24 +00:00
fix: enforce mandatory :start_line: in apply_diff tool
- Make :start_line: mandatory in the regex pattern (was optional) - Add :start_line: presence tracking in validateMarkerSequencing - Add early validation in ApplyDiffTool before calling applyDiff - Add sayAndCreateMissingStartLineError helper method on Task - Update error messages to reflect :start_line: requirement - Update all existing tests to include :start_line: directive - Add new test for rejecting diffs without :start_line: Closes #12199
This commit is contained in:
Roo Code
parent
ad25634905
commit
68299b68e8
5 changed files with 186 additions and 22 deletions
|
|
@ -16,6 +16,8 @@ describe("MultiSearchReplaceDiffStrategy - trailing newline preservation", () =>
|
|||
}
|
||||
}`
|
||||
const diffContent = `<<<<<<< SEARCH
|
||||
:start_line:1
|
||||
-------
|
||||
1 | class Example {
|
||||
2 | constructor() {
|
||||
3 | this.value = 0;
|
||||
|
|
@ -43,6 +45,8 @@ class Example {
|
|||
it("should handle Windows line endings with trailing newlines and line numbers", async () => {
|
||||
const originalContent = "function test() {\r\n return true;\r\n}\r\n"
|
||||
const diffContent = `<<<<<<< SEARCH
|
||||
:start_line:1
|
||||
-------
|
||||
1 | function test() {
|
||||
2 | return true;
|
||||
3 | }
|
||||
|
|
@ -69,6 +73,8 @@ function two() {
|
|||
return 2;
|
||||
}`
|
||||
const diffContent = `<<<<<<< SEARCH
|
||||
:start_line:1
|
||||
-------
|
||||
1 | function one() {
|
||||
2 | return 1;
|
||||
3 | }
|
||||
|
|
@ -79,6 +85,8 @@ function one() {
|
|||
>>>>>>> REPLACE
|
||||
|
||||
<<<<<<< SEARCH
|
||||
:start_line:5
|
||||
-------
|
||||
5 | function two() {
|
||||
6 | return 2;
|
||||
7 | }
|
||||
|
|
@ -109,6 +117,8 @@ function two() {
|
|||
+ CollectionUtils.size(personIdentityInfoList));`
|
||||
|
||||
const diffContent = `<<<<<<< SEARCH
|
||||
:start_line:1
|
||||
-------
|
||||
1476 | List<ContactInfoItemResp> addressInfoList = new ArrayList<>(CollectionUtils.size(repairInfoList) > 10 ? 10
|
||||
1477 | : CollectionUtils.size(repairInfoList) + CollectionUtils.size(homeAddressInfoList)
|
||||
1478 | + CollectionUtils.size(idNoAddressInfoList) + CollectionUtils.size(workAddressInfoList)
|
||||
|
|
@ -143,6 +153,8 @@ function two() {
|
|||
it("should correctly strip line numbers even when last line has no trailing newline", async () => {
|
||||
const originalContent = "line 1\nline 2\nline 3" // No trailing newline
|
||||
const diffContent = `<<<<<<< SEARCH
|
||||
:start_line:1
|
||||
-------
|
||||
1 | line 1
|
||||
2 | line 2
|
||||
3 | line 3
|
||||
|
|
|
|||
|
|
@ -9,12 +9,24 @@ describe("MultiSearchReplaceDiffStrategy", () => {
|
|||
})
|
||||
|
||||
it("validates correct marker sequence", () => {
|
||||
const diff = "<<<<<<< SEARCH\n" + "some content\n" + "=======\n" + "new content\n" + ">>>>>>> REPLACE"
|
||||
const diff =
|
||||
"<<<<<<< SEARCH\n" +
|
||||
":start_line:1\n" +
|
||||
"some content\n" +
|
||||
"=======\n" +
|
||||
"new content\n" +
|
||||
">>>>>>> REPLACE"
|
||||
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"
|
||||
const diff =
|
||||
"<<<<<<< SEARCH>\n" +
|
||||
":start_line:1\n" +
|
||||
"some content\n" +
|
||||
"=======\n" +
|
||||
"new content\n" +
|
||||
">>>>>>> REPLACE"
|
||||
expect(strategy["validateMarkerSequencing"](diff).success).toBe(true)
|
||||
})
|
||||
|
||||
|
|
@ -26,11 +38,13 @@ describe("MultiSearchReplaceDiffStrategy", () => {
|
|||
it("validates mixed cases with and without extra > in the same diff", () => {
|
||||
const diff =
|
||||
"<<<<<<< SEARCH>\n" +
|
||||
":start_line:1\n" +
|
||||
"content1\n" +
|
||||
"=======\n" +
|
||||
"new1\n" +
|
||||
">>>>>>> REPLACE\n\n" +
|
||||
"<<<<<<< SEARCH\n" +
|
||||
":start_line:2\n" +
|
||||
"content2\n" +
|
||||
"=======\n" +
|
||||
"new2\n" +
|
||||
|
|
@ -39,7 +53,13 @@ describe("MultiSearchReplaceDiffStrategy", () => {
|
|||
})
|
||||
|
||||
it("validates extra > with whitespace variations", () => {
|
||||
const diff1 = "<<<<<<< SEARCH> \n" + "some content\n" + "=======\n" + "new content\n" + ">>>>>>> REPLACE"
|
||||
const diff1 =
|
||||
"<<<<<<< SEARCH> \n" +
|
||||
":start_line:1\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"
|
||||
|
|
@ -61,11 +81,13 @@ describe("MultiSearchReplaceDiffStrategy", () => {
|
|||
it("validates multiple correct marker sequences", () => {
|
||||
const diff =
|
||||
"<<<<<<< SEARCH\n" +
|
||||
":start_line:1\n" +
|
||||
"content1\n" +
|
||||
"=======\n" +
|
||||
"new1\n" +
|
||||
">>>>>>> REPLACE\n\n" +
|
||||
"<<<<<<< SEARCH\n" +
|
||||
":start_line:2\n" +
|
||||
"content2\n" +
|
||||
"=======\n" +
|
||||
"new2\n" +
|
||||
|
|
@ -101,7 +123,7 @@ describe("MultiSearchReplaceDiffStrategy", () => {
|
|||
})
|
||||
|
||||
it("detects missing separator", () => {
|
||||
const diff = "<<<<<<< SEARCH\n" + "content\n" + ">>>>>>> REPLACE"
|
||||
const diff = "<<<<<<< SEARCH\n" + ":start_line:1\n" + "content\n" + ">>>>>>> REPLACE"
|
||||
const result = strategy["validateMarkerSequencing"](diff)
|
||||
expect(result.success).toBe(false)
|
||||
expect(result.error).toContain("'>>>>>>> REPLACE' found in your diff content")
|
||||
|
|
@ -109,7 +131,8 @@ describe("MultiSearchReplaceDiffStrategy", () => {
|
|||
})
|
||||
|
||||
it("detects two separators", () => {
|
||||
const diff = "<<<<<<< SEARCH\n" + "content\n" + "=======\n" + "=======\n" + ">>>>>>> REPLACE"
|
||||
const diff =
|
||||
"<<<<<<< SEARCH\n" + ":start_line:1\n" + "content\n" + "=======\n" + "=======\n" + ">>>>>>> REPLACE"
|
||||
const result = strategy["validateMarkerSequencing"](diff)
|
||||
expect(result.success).toBe(false)
|
||||
expect(result.error).toContain("'=======' found in your diff content")
|
||||
|
|
@ -125,12 +148,19 @@ describe("MultiSearchReplaceDiffStrategy", () => {
|
|||
})
|
||||
|
||||
it("detects incomplete sequence", () => {
|
||||
const diff = "<<<<<<< SEARCH\n" + "content\n" + "=======\n" + "new content"
|
||||
const diff = "<<<<<<< SEARCH\n" + ":start_line:1\n" + "content\n" + "=======\n" + "new content"
|
||||
const result = strategy["validateMarkerSequencing"](diff)
|
||||
expect(result.success).toBe(false)
|
||||
expect(result.error).toContain("Expected '>>>>>>> REPLACE' was not found")
|
||||
})
|
||||
|
||||
it("rejects diff without :start_line:", () => {
|
||||
const diff = "<<<<<<< SEARCH\n" + "some content\n" + "=======\n" + "new content\n" + ">>>>>>> REPLACE"
|
||||
const result = strategy["validateMarkerSequencing"](diff)
|
||||
expect(result.success).toBe(false)
|
||||
expect(result.error).toContain("Expected: :start_line: <line_number>")
|
||||
})
|
||||
|
||||
describe("exact matching", () => {
|
||||
let strategy: MultiSearchReplaceDiffStrategy
|
||||
|
||||
|
|
@ -142,6 +172,8 @@ describe("MultiSearchReplaceDiffStrategy", () => {
|
|||
const originalContent = 'function hello() {\n console.log("hello")\n}\n'
|
||||
const diffContent = `test.ts
|
||||
<<<<<<< SEARCH
|
||||
:start_line:1
|
||||
-------
|
||||
function hello() {
|
||||
console.log("hello")
|
||||
}
|
||||
|
|
@ -162,11 +194,15 @@ function hello() {
|
|||
const originalContent = 'function hello() {\n console.log("hello")\n}\n'
|
||||
const diffContent = `test.ts
|
||||
<<<<<<< SEARCH
|
||||
:start_line:1
|
||||
-------
|
||||
function hello() {
|
||||
=======
|
||||
function helloWorld() {
|
||||
>>>>>>> REPLACE
|
||||
<<<<<<< SEARCH
|
||||
:start_line:2
|
||||
-------
|
||||
console.log("hello")
|
||||
=======
|
||||
console.log("hello world")
|
||||
|
|
@ -227,6 +263,8 @@ function helloWorld() {
|
|||
const originalContent = "\nfunction example() {\n return 42;\n}\n\n"
|
||||
const diffContent = `test.ts
|
||||
<<<<<<< SEARCH
|
||||
:start_line:2
|
||||
-------
|
||||
function example() {
|
||||
return 42;
|
||||
}
|
||||
|
|
@ -247,6 +285,8 @@ function example() {
|
|||
const originalContent = " function test() {\n return true;\n }\n"
|
||||
const diffContent = `test.ts
|
||||
<<<<<<< SEARCH
|
||||
:start_line:1
|
||||
-------
|
||||
function test() {
|
||||
return true;
|
||||
}
|
||||
|
|
@ -267,6 +307,8 @@ function test() {
|
|||
const originalContent = "function test() {\n\treturn true;\n}\n"
|
||||
const diffContent = `test.ts
|
||||
<<<<<<< SEARCH
|
||||
:start_line:1
|
||||
-------
|
||||
function test() {
|
||||
\treturn true;
|
||||
}
|
||||
|
|
@ -287,6 +329,8 @@ function test() {
|
|||
const originalContent = "\tclass Example {\n\t constructor() {\n\t\tthis.value = 0;\n\t }\n\t}"
|
||||
const diffContent = `test.ts
|
||||
<<<<<<< SEARCH
|
||||
:start_line:1
|
||||
-------
|
||||
\tclass Example {
|
||||
\t constructor() {
|
||||
\t\tthis.value = 0;
|
||||
|
|
@ -313,6 +357,8 @@ function test() {
|
|||
const originalContent = "\tfunction test() {\n\t\treturn true;\n\t}"
|
||||
const diffContent = `test.ts
|
||||
<<<<<<< SEARCH
|
||||
:start_line:1
|
||||
-------
|
||||
function test() {
|
||||
\treturn true;
|
||||
}
|
||||
|
|
@ -334,6 +380,8 @@ function test() {
|
|||
const originalContent = "\tfunction test() {\n\t\treturn true;\n\t}"
|
||||
const diffContent = `test.ts
|
||||
<<<<<<< SEARCH
|
||||
:start_line:1
|
||||
-------
|
||||
\tfunction test() {
|
||||
\t\treturn true;
|
||||
\t}
|
||||
|
|
@ -358,6 +406,8 @@ function test() {
|
|||
const originalContent = "function test() {\r\n return true;\r\n}\r\n"
|
||||
const diffContent = `test.ts
|
||||
<<<<<<< SEARCH
|
||||
:start_line:1
|
||||
-------
|
||||
function test() {
|
||||
return true;
|
||||
}
|
||||
|
|
@ -378,6 +428,8 @@ function test() {
|
|||
const originalContent = 'function hello() {\n console.log("hello")\n}\n'
|
||||
const diffContent = `test.ts
|
||||
<<<<<<< SEARCH
|
||||
:start_line:1
|
||||
-------
|
||||
function hello() {
|
||||
console.log("wrong")
|
||||
}
|
||||
|
|
@ -404,6 +456,8 @@ function hello() {
|
|||
"class Example {\n constructor() {\n this.value = 0\n }\n\n getValue() {\n return this.value\n }\n}\n"
|
||||
const diffContent = `test.ts
|
||||
<<<<<<< SEARCH
|
||||
:start_line:6
|
||||
-------
|
||||
getValue() {
|
||||
return this.value
|
||||
}
|
||||
|
|
@ -428,6 +482,8 @@ function hello() {
|
|||
const originalContent = " indented\n more indented\n back\n"
|
||||
const diffContent = `test.ts
|
||||
<<<<<<< SEARCH
|
||||
:start_line:1
|
||||
-------
|
||||
indented
|
||||
more indented
|
||||
back
|
||||
|
|
@ -448,6 +504,8 @@ function hello() {
|
|||
const originalContent = " onScroll={() => updateHighlights()}"
|
||||
const diffContent = `test.ts
|
||||
<<<<<<< SEARCH
|
||||
:start_line:1
|
||||
-------
|
||||
onScroll={() => updateHighlights()}
|
||||
=======
|
||||
onScroll={() => updateHighlights()}
|
||||
|
|
@ -479,6 +537,8 @@ class Example {
|
|||
|
||||
const diffContent = `test.ts
|
||||
<<<<<<< SEARCH
|
||||
:start_line:1
|
||||
-------
|
||||
class Example {
|
||||
constructor() {
|
||||
this.value = 0;
|
||||
|
|
@ -530,6 +590,8 @@ class Example {
|
|||
}`.trim()
|
||||
const diffContent = `test.ts
|
||||
<<<<<<< SEARCH
|
||||
:start_line:2
|
||||
-------
|
||||
constructor() {
|
||||
this.value = 0;
|
||||
if (true) {
|
||||
|
|
@ -570,6 +632,8 @@ class Example {
|
|||
return True`.trim()
|
||||
const diffContent = `test.ts
|
||||
<<<<<<< SEARCH
|
||||
:start_line:2
|
||||
-------
|
||||
if condition:
|
||||
do_something()
|
||||
for item in items:
|
||||
|
|
@ -605,6 +669,8 @@ class Example {
|
|||
}`.trim()
|
||||
const diffContent = `test.ts
|
||||
<<<<<<< SEARCH
|
||||
:start_line:2
|
||||
-------
|
||||
const x = 1;
|
||||
|
||||
if (x) {
|
||||
|
|
@ -639,6 +705,8 @@ class Example {
|
|||
}`.trim()
|
||||
const diffContent = `test.ts
|
||||
<<<<<<< SEARCH
|
||||
:start_line:2
|
||||
-------
|
||||
method() {
|
||||
if (true) {
|
||||
console.log("test");
|
||||
|
|
@ -684,6 +752,8 @@ class Example {
|
|||
}`.trim()
|
||||
const diffContent = `test.ts
|
||||
<<<<<<< SEARCH
|
||||
:start_line:4
|
||||
-------
|
||||
this.init();
|
||||
this.setup();
|
||||
=======
|
||||
|
|
@ -715,6 +785,8 @@ class Example {
|
|||
}`.trim()
|
||||
const diffContent = `test.ts
|
||||
<<<<<<< SEARCH
|
||||
:start_line:4
|
||||
-------
|
||||
this.init();
|
||||
=======
|
||||
this.init();
|
||||
|
|
@ -745,6 +817,8 @@ this.init();
|
|||
}`.trim()
|
||||
const diffContent = `test.ts
|
||||
<<<<<<< SEARCH
|
||||
:start_line:4
|
||||
-------
|
||||
this.init();
|
||||
this.setup();
|
||||
this.validate();
|
||||
|
|
@ -793,6 +867,8 @@ function five() {
|
|||
|
||||
const diffContent = `test.ts
|
||||
<<<<<<< SEARCH
|
||||
:start_line:9
|
||||
-------
|
||||
return "target";
|
||||
=======
|
||||
return "updated";
|
||||
|
|
@ -839,6 +915,8 @@ function five() {
|
|||
"function getData() {\n const results = fetchData();\n return results.filter(Boolean);\n}\n"
|
||||
const diffContent = `test.ts
|
||||
<<<<<<< SEARCH
|
||||
:start_line:1
|
||||
-------
|
||||
function getData() {
|
||||
const result = fetchData();
|
||||
return results.filter(Boolean);
|
||||
|
|
@ -865,6 +943,8 @@ function getData() {
|
|||
const originalContent = "function processUsers(data) {\n return data.map(user => user.name);\n}\n"
|
||||
const diffContent = `test.ts
|
||||
<<<<<<< SEARCH
|
||||
:start_line:1
|
||||
-------
|
||||
function handleItems(items) {
|
||||
return items.map(item => item.username);
|
||||
}
|
||||
|
|
@ -882,6 +962,8 @@ function processData(data) {
|
|||
const originalContent = "function sum(a, b) {\n return a + b;\n}"
|
||||
const diffContent = `test.ts
|
||||
<<<<<<< SEARCH
|
||||
:start_line:1
|
||||
-------
|
||||
function sum(a, b) {
|
||||
return a + b;
|
||||
}
|
||||
|
|
@ -900,9 +982,11 @@ function sum(a, b) {
|
|||
|
||||
it("should match content with smart quotes", async () => {
|
||||
const originalContent =
|
||||
"**Enjoy Roo Code!** Whether you keep it on a short leash or let it roam autonomously, we can't wait to see what you build. If you have questions or feature ideas, drop by our [Reddit community](https://www.reddit.com/r/RooCode/) or [Discord](https://discord.gg/roocode). Happy coding!"
|
||||
"**Enjoy Roo Code!** Whether you keep it on a short leash or let it roam autonomously, we can\u2019t wait to see what you build. If you have questions or feature ideas, drop by our [Reddit community](https://www.reddit.com/r/RooCode/) or [Discord](https://discord.gg/roocode). Happy coding!"
|
||||
const diffContent = `test.ts
|
||||
<<<<<<< SEARCH
|
||||
:start_line:1
|
||||
-------
|
||||
**Enjoy Roo Code!** Whether you keep it on a short leash or let it roam autonomously, we can't wait to see what you build. If you have questions or feature ideas, drop by our [Reddit community](https://www.reddit.com/r/RooCode/) or [Discord](https://discord.gg/roocode). Happy coding!
|
||||
=======
|
||||
**Enjoy Roo Code!** Whether you keep it on a short leash or let it roam autonomously, we can't wait to see what you build. If you have questions or feature ideas, drop by our [Reddit community](https://www.reddit.com/r/RooCode/) or [Discord](https://discord.gg/roocode). Happy coding!
|
||||
|
|
@ -923,6 +1007,8 @@ You're still here?
|
|||
const originalContent = "function sum(a, b) {\n\n return a + b;\n}"
|
||||
const diffContent = `test.ts
|
||||
<<<<<<< SEARCH
|
||||
:start_line:1
|
||||
-------
|
||||
function sum(a, b) {
|
||||
=======
|
||||
import { a } from "a";
|
||||
|
|
@ -952,6 +1038,8 @@ function sum(a, b) {
|
|||
}`
|
||||
const diffContent = `test.ts
|
||||
<<<<<<< SEARCH
|
||||
:start_line:3
|
||||
-------
|
||||
// Comment to remove
|
||||
=======
|
||||
>>>>>>> REPLACE`
|
||||
|
|
@ -978,6 +1066,8 @@ function sum(a, b) {
|
|||
}`
|
||||
const diffContent = `test.ts
|
||||
<<<<<<< SEARCH
|
||||
:start_line:3
|
||||
-------
|
||||
// Initialize
|
||||
this.value = 0;
|
||||
// Set defaults
|
||||
|
|
@ -1007,6 +1097,8 @@ function sum(a, b) {
|
|||
}`
|
||||
const diffContent = `test.ts
|
||||
<<<<<<< SEARCH
|
||||
:start_line:3
|
||||
-------
|
||||
// Remove this
|
||||
console.log("test");
|
||||
// And this
|
||||
|
|
@ -1051,6 +1143,7 @@ function sum(a, b) {
|
|||
it("should reject start_line marker in REPLACE section", () => {
|
||||
const diff =
|
||||
"<<<<<<< SEARCH\n" +
|
||||
":start_line:1\n" +
|
||||
"content to find\n" +
|
||||
"=======\n" +
|
||||
":start_line:5\n" +
|
||||
|
|
@ -1067,6 +1160,7 @@ function sum(a, b) {
|
|||
it("should reject end_line marker in REPLACE section", () => {
|
||||
const diff =
|
||||
"<<<<<<< SEARCH\n" +
|
||||
":start_line:1\n" +
|
||||
"content to find\n" +
|
||||
"=======\n" +
|
||||
":end_line:10\n" +
|
||||
|
|
@ -1083,6 +1177,7 @@ function sum(a, b) {
|
|||
it("should reject both line markers in REPLACE section", () => {
|
||||
const diff =
|
||||
"<<<<<<< SEARCH\n" +
|
||||
":start_line:1\n" +
|
||||
"content to find\n" +
|
||||
"=======\n" +
|
||||
":start_line:5\n" +
|
||||
|
|
@ -1103,6 +1198,7 @@ function sum(a, b) {
|
|||
"replacement1\n" +
|
||||
">>>>>>> REPLACE\n\n" +
|
||||
"<<<<<<< SEARCH\n" +
|
||||
":start_line:5\n" +
|
||||
"content2\n" +
|
||||
"=======\n" +
|
||||
":start_line:5\n" +
|
||||
|
|
@ -1130,6 +1226,7 @@ function sum(a, b) {
|
|||
it("should allow escaped line markers in REPLACE content", () => {
|
||||
const diff =
|
||||
"<<<<<<< SEARCH\n" +
|
||||
":start_line:1\n" +
|
||||
"content to find\n" +
|
||||
"=======\n" +
|
||||
"replacement content\n" +
|
||||
|
|
@ -1143,6 +1240,7 @@ function sum(a, b) {
|
|||
it("should allow escaped end_line markers in REPLACE content", () => {
|
||||
const diff =
|
||||
"<<<<<<< SEARCH\n" +
|
||||
":start_line:1\n" +
|
||||
"content to find\n" +
|
||||
"=======\n" +
|
||||
"replacement content\n" +
|
||||
|
|
@ -1156,6 +1254,7 @@ function sum(a, b) {
|
|||
it("should allow both escaped line markers in REPLACE content", () => {
|
||||
const diff =
|
||||
"<<<<<<< SEARCH\n" +
|
||||
":start_line:1\n" +
|
||||
"content to find\n" +
|
||||
"=======\n" +
|
||||
"replacement content\n" +
|
||||
|
|
@ -1170,6 +1269,7 @@ function sum(a, b) {
|
|||
it("should reject line markers with whitespace in REPLACE section", () => {
|
||||
const diff =
|
||||
"<<<<<<< SEARCH\n" +
|
||||
":start_line:1\n" +
|
||||
"content to find\n" +
|
||||
"=======\n" +
|
||||
" :start_line:5 \n" +
|
||||
|
|
@ -1183,6 +1283,7 @@ function sum(a, b) {
|
|||
it("should reject line markers in middle of REPLACE content", () => {
|
||||
const diff =
|
||||
"<<<<<<< SEARCH\n" +
|
||||
":start_line:1\n" +
|
||||
"content to find\n" +
|
||||
"=======\n" +
|
||||
"some replacement\n" +
|
||||
|
|
@ -1196,7 +1297,13 @@ function sum(a, b) {
|
|||
|
||||
it("should provide helpful error message format", () => {
|
||||
const diff =
|
||||
"<<<<<<< SEARCH\n" + "content\n" + "=======\n" + ":start_line:5\n" + "replacement\n" + ">>>>>>> REPLACE"
|
||||
"<<<<<<< SEARCH\n" +
|
||||
":start_line:1\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:")
|
||||
|
|
|
|||
|
|
@ -105,6 +105,7 @@ export class MultiSearchReplaceDiffStrategy implements DiffStrategy {
|
|||
AFTER_SEPARATOR,
|
||||
}
|
||||
const state = { current: State.START, line: 0 }
|
||||
let foundStartLine = false
|
||||
|
||||
// Pattern allows optional '>' after SEARCH to handle AI-generated diffs
|
||||
// (e.g., Sonnet 4 sometimes adds an extra '>')
|
||||
|
|
@ -207,8 +208,10 @@ export class MultiSearchReplaceDiffStrategy implements DiffStrategy {
|
|||
: reportMergeConflictError(SEP, SEARCH)
|
||||
if (marker === REPLACE) return reportInvalidDiffError(REPLACE, SEARCH)
|
||||
if (marker.startsWith(REPLACE_PREFIX)) return reportMergeConflictError(marker, SEARCH)
|
||||
if (SEARCH_PATTERN.test(marker)) state.current = State.AFTER_SEARCH
|
||||
else if (marker.startsWith(SEARCH_PREFIX)) return reportMergeConflictError(marker, SEARCH)
|
||||
if (SEARCH_PATTERN.test(marker)) {
|
||||
foundStartLine = false
|
||||
state.current = State.AFTER_SEARCH
|
||||
} else if (marker.startsWith(SEARCH_PREFIX)) return reportMergeConflictError(marker, SEARCH)
|
||||
break
|
||||
|
||||
case State.AFTER_SEARCH:
|
||||
|
|
@ -216,7 +219,13 @@ 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)
|
||||
if (marker === SEP) state.current = State.AFTER_SEPARATOR
|
||||
if (/^:start_line:\s*\d+/.test(marker)) foundStartLine = true
|
||||
if (marker === SEP) {
|
||||
if (!foundStartLine) {
|
||||
return reportInvalidDiffError(SEP, ":start_line: <line_number>")
|
||||
}
|
||||
state.current = State.AFTER_SEPARATOR
|
||||
}
|
||||
break
|
||||
|
||||
case State.AFTER_SEPARATOR:
|
||||
|
|
@ -265,23 +274,23 @@ export class MultiSearchReplaceDiffStrategy implements DiffStrategy {
|
|||
2. (?<!\\)<<<<<<< SEARCH\s*\n
|
||||
Matches the line "<<<<<<< SEARCH" (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.
|
||||
3. \:start_line:\s*(\d+)\s*\n
|
||||
Matches the required ":start_line:" line. Group 1 is the line number.
|
||||
|
||||
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.
|
||||
Optionally matches a ":end_line:" line. Group 2 is the whole match and group 3 is the digits.
|
||||
|
||||
5. ((?<!\\)-------\s*\n)?
|
||||
Optionally matches the "-------" marker line (group 5).
|
||||
Optionally matches the "-------" marker line (group 4).
|
||||
|
||||
6. ([\s\S]*?)(?:\n)?
|
||||
Non‐greedy match for the "search content" (group 6) up to the next marker.
|
||||
Non‐greedy match for the "search content" (group 5) 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).
|
||||
Non‐greedy match for the "replace content" (group 6).
|
||||
|
||||
9. (?:(?<=\n)(?<!\\)>>>>>>> REPLACE)(?=\n|$)
|
||||
Matches the final ">>>>>>> REPLACE" marker on its own line (and requires a following newline or the end of file).
|
||||
|
|
@ -289,14 +298,14 @@ export class MultiSearchReplaceDiffStrategy implements DiffStrategy {
|
|||
|
||||
let matches = [
|
||||
...diffContent.matchAll(
|
||||
/(?:^|\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,
|
||||
/(?:^|\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,
|
||||
),
|
||||
]
|
||||
|
||||
if (matches.length === 0) {
|
||||
return {
|
||||
success: false,
|
||||
error: `Invalid diff format - missing required sections\n\nDebug Info:\n- Expected Format: <<<<<<< SEARCH\\n:start_line: start line\\n-------\\n[search content]\\n=======\\n[replace content]\\n>>>>>>> REPLACE\n- Tip: Make sure to include start_line/SEARCH/=======/REPLACE sections with correct markers on new lines`,
|
||||
error: `Invalid diff format - missing required sections\n\nDebug Info:\n- Expected Format: <<<<<<< SEARCH\\n:start_line: <line_number>\\n-------\\n[search content]\\n=======\\n[replace content]\\n>>>>>>> REPLACE\n- Tip: Make sure to include :start_line:, SEARCH, -------, SEARCH content, =======, REPLACE content, and REPLACE sections with correct markers on new lines`,
|
||||
}
|
||||
}
|
||||
// Detect line ending from original content
|
||||
|
|
@ -307,9 +316,9 @@ export class MultiSearchReplaceDiffStrategy implements DiffStrategy {
|
|||
let appliedCount = 0
|
||||
const replacements = matches
|
||||
.map((match) => ({
|
||||
startLine: Number(match[2] ?? 0),
|
||||
searchContent: match[6],
|
||||
replaceContent: match[7],
|
||||
startLine: Number(match[1]),
|
||||
searchContent: match[5],
|
||||
replaceContent: match[6],
|
||||
}))
|
||||
.sort((a, b) => a.startLine - b.startLine)
|
||||
|
||||
|
|
|
|||
|
|
@ -1876,6 +1876,35 @@ export class Task extends EventEmitter<TaskEvents> implements TaskLike {
|
|||
return formatResponse.toolError(formatResponse.missingToolParameterError(paramName))
|
||||
}
|
||||
|
||||
async sayAndCreateMissingStartLineError(diffContent: string) {
|
||||
await this.say(
|
||||
"error",
|
||||
"Roo tried to use apply_diff without value for required parameter ':start_line:' in diff. Retrying...",
|
||||
)
|
||||
|
||||
const formattedError =
|
||||
`<error_details>\n:start_line: is required in your diff content.\n\n` +
|
||||
"The ':start_line:' directive specifies the exact line number in the original file where the search block starts.\n\n" +
|
||||
"CORRECT FORMAT:\n\n" +
|
||||
"<<<<<<< SEARCH\n" +
|
||||
":start_line:42\n" +
|
||||
"-------\n" +
|
||||
"[exact content to find including whitespace]\n" +
|
||||
"=======\n" +
|
||||
"[new content to replace with]\n" +
|
||||
">>>>>>> REPLACE\n" +
|
||||
"\n" +
|
||||
"Your diff content:\n" +
|
||||
"```\n" +
|
||||
diffContent.slice(0, 500) +
|
||||
(diffContent.length > 500 ? "..." : "") +
|
||||
"\n" +
|
||||
"```\n" +
|
||||
"\n</error_details>"
|
||||
|
||||
return formattedError
|
||||
}
|
||||
|
||||
// Lifecycle
|
||||
// Start / Resume / Abort / Dispose
|
||||
|
||||
|
|
|
|||
|
|
@ -55,6 +55,13 @@ export class ApplyDiffTool extends BaseTool<"apply_diff"> {
|
|||
return
|
||||
}
|
||||
|
||||
if (!/:start_line:\s*\d+/.test(diffContent)) {
|
||||
task.consecutiveMistakeCount++
|
||||
task.recordToolError("apply_diff")
|
||||
pushToolResult(await task.sayAndCreateMissingStartLineError(diffContent))
|
||||
return
|
||||
}
|
||||
|
||||
const absolutePath = path.resolve(task.cwd, relPath)
|
||||
const fileExists = await fileExistsAtPath(absolutePath)
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue