Skip to content

Commit 53f94a7

Browse files
KJ7LNWEric Wheeler
andauthored
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 <[email protected]> Co-authored-by: Eric Wheeler <[email protected]>
1 parent ff422b1 commit 53f94a7

File tree

2 files changed

+198
-0
lines changed

2 files changed

+198
-0
lines changed

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

Lines changed: 164 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2407,4 +2407,168 @@ function two() {
24072407
expect(description).toContain("</apply_diff>")
24082408
})
24092409
})
2410+
2411+
describe("line marker validation in REPLACE sections", () => {
2412+
let strategy: MultiSearchReplaceDiffStrategy
2413+
2414+
beforeEach(() => {
2415+
strategy = new MultiSearchReplaceDiffStrategy()
2416+
})
2417+
2418+
it("should reject start_line marker in REPLACE section", () => {
2419+
const diff =
2420+
"<<<<<<< SEARCH\n" +
2421+
"content to find\n" +
2422+
"=======\n" +
2423+
":start_line:5\n" +
2424+
"replacement content\n" +
2425+
">>>>>>> REPLACE"
2426+
const result = strategy["validateMarkerSequencing"](diff)
2427+
expect(result.success).toBe(false)
2428+
expect(result.error).toContain("Invalid line marker ':start_line:' found in REPLACE section")
2429+
expect(result.error).toContain(
2430+
"Line markers (:start_line: and :end_line:) are only allowed in SEARCH sections",
2431+
)
2432+
})
2433+
2434+
it("should reject end_line marker in REPLACE section", () => {
2435+
const diff =
2436+
"<<<<<<< SEARCH\n" +
2437+
"content to find\n" +
2438+
"=======\n" +
2439+
":end_line:10\n" +
2440+
"replacement content\n" +
2441+
">>>>>>> REPLACE"
2442+
const result = strategy["validateMarkerSequencing"](diff)
2443+
expect(result.success).toBe(false)
2444+
expect(result.error).toContain("Invalid line marker ':end_line:' found in REPLACE section")
2445+
expect(result.error).toContain(
2446+
"Line markers (:start_line: and :end_line:) are only allowed in SEARCH sections",
2447+
)
2448+
})
2449+
2450+
it("should reject both line markers in REPLACE section", () => {
2451+
const diff =
2452+
"<<<<<<< SEARCH\n" +
2453+
"content to find\n" +
2454+
"=======\n" +
2455+
":start_line:5\n" +
2456+
":end_line:10\n" +
2457+
"replacement content\n" +
2458+
">>>>>>> REPLACE"
2459+
const result = strategy["validateMarkerSequencing"](diff)
2460+
expect(result.success).toBe(false)
2461+
expect(result.error).toContain("Invalid line marker ':start_line:' found in REPLACE section")
2462+
})
2463+
2464+
it("should reject line markers in multiple diff blocks where one has invalid markers", () => {
2465+
const diff =
2466+
"<<<<<<< SEARCH\n" +
2467+
":start_line:1\n" +
2468+
"content1\n" +
2469+
"=======\n" +
2470+
"replacement1\n" +
2471+
">>>>>>> REPLACE\n\n" +
2472+
"<<<<<<< SEARCH\n" +
2473+
"content2\n" +
2474+
"=======\n" +
2475+
":start_line:5\n" +
2476+
"replacement2\n" +
2477+
">>>>>>> REPLACE"
2478+
const result = strategy["validateMarkerSequencing"](diff)
2479+
expect(result.success).toBe(false)
2480+
expect(result.error).toContain("Invalid line marker ':start_line:' found in REPLACE section")
2481+
})
2482+
2483+
it("should allow valid markers in SEARCH section with content in REPLACE", () => {
2484+
const diff =
2485+
"<<<<<<< SEARCH\n" +
2486+
":start_line:5\n" +
2487+
":end_line:10\n" +
2488+
"-------\n" +
2489+
"content to find\n" +
2490+
"=======\n" +
2491+
"replacement content\n" +
2492+
">>>>>>> REPLACE"
2493+
const result = strategy["validateMarkerSequencing"](diff)
2494+
expect(result.success).toBe(true)
2495+
})
2496+
2497+
it("should allow escaped line markers in REPLACE content", () => {
2498+
const diff =
2499+
"<<<<<<< SEARCH\n" +
2500+
"content to find\n" +
2501+
"=======\n" +
2502+
"replacement content\n" +
2503+
"\\:start_line:5\n" +
2504+
"more content\n" +
2505+
">>>>>>> REPLACE"
2506+
const result = strategy["validateMarkerSequencing"](diff)
2507+
expect(result.success).toBe(true)
2508+
})
2509+
2510+
it("should allow escaped end_line markers in REPLACE content", () => {
2511+
const diff =
2512+
"<<<<<<< SEARCH\n" +
2513+
"content to find\n" +
2514+
"=======\n" +
2515+
"replacement content\n" +
2516+
"\\:end_line:10\n" +
2517+
"more content\n" +
2518+
">>>>>>> REPLACE"
2519+
const result = strategy["validateMarkerSequencing"](diff)
2520+
expect(result.success).toBe(true)
2521+
})
2522+
2523+
it("should allow both escaped line markers in REPLACE content", () => {
2524+
const diff =
2525+
"<<<<<<< SEARCH\n" +
2526+
"content to find\n" +
2527+
"=======\n" +
2528+
"replacement content\n" +
2529+
"\\:start_line:5\n" +
2530+
"\\:end_line:10\n" +
2531+
"more content\n" +
2532+
">>>>>>> REPLACE"
2533+
const result = strategy["validateMarkerSequencing"](diff)
2534+
expect(result.success).toBe(true)
2535+
})
2536+
2537+
it("should reject line markers with whitespace in REPLACE section", () => {
2538+
const diff =
2539+
"<<<<<<< SEARCH\n" +
2540+
"content to find\n" +
2541+
"=======\n" +
2542+
" :start_line:5 \n" +
2543+
"replacement content\n" +
2544+
">>>>>>> REPLACE"
2545+
const result = strategy["validateMarkerSequencing"](diff)
2546+
expect(result.success).toBe(false)
2547+
expect(result.error).toContain("Invalid line marker ':start_line:' found in REPLACE section")
2548+
})
2549+
2550+
it("should reject line markers in middle of REPLACE content", () => {
2551+
const diff =
2552+
"<<<<<<< SEARCH\n" +
2553+
"content to find\n" +
2554+
"=======\n" +
2555+
"some replacement\n" +
2556+
":end_line:15\n" +
2557+
"more replacement\n" +
2558+
">>>>>>> REPLACE"
2559+
const result = strategy["validateMarkerSequencing"](diff)
2560+
expect(result.success).toBe(false)
2561+
expect(result.error).toContain("Invalid line marker ':end_line:' found in REPLACE section")
2562+
})
2563+
2564+
it("should provide helpful error message format", () => {
2565+
const diff =
2566+
"<<<<<<< SEARCH\n" + "content\n" + "=======\n" + ":start_line:5\n" + "replacement\n" + ">>>>>>> REPLACE"
2567+
const result = strategy["validateMarkerSequencing"](diff)
2568+
expect(result.success).toBe(false)
2569+
expect(result.error).toContain("CORRECT FORMAT:")
2570+
expect(result.error).toContain("INCORRECT FORMAT:")
2571+
expect(result.error).toContain(":start_line:5 <-- Invalid location")
2572+
})
2573+
})
24102574
})

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

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -243,6 +243,30 @@ Only use a single line of '=======' between search and replacement content, beca
243243
">>>>>>> REPLACE\n",
244244
})
245245

246+
const reportLineMarkerInReplaceError = (marker: string) => ({
247+
success: false,
248+
error:
249+
`ERROR: Invalid line marker '${marker}' found in REPLACE section at line ${state.line}\n` +
250+
"\n" +
251+
"Line markers (:start_line: and :end_line:) are only allowed in SEARCH sections.\n" +
252+
"\n" +
253+
"CORRECT FORMAT:\n" +
254+
"<<<<<<< SEARCH\n" +
255+
":start_line:5\n" +
256+
"content to find\n" +
257+
"=======\n" +
258+
"replacement content\n" +
259+
">>>>>>> REPLACE\n" +
260+
"\n" +
261+
"INCORRECT FORMAT:\n" +
262+
"<<<<<<< SEARCH\n" +
263+
"content to find\n" +
264+
"=======\n" +
265+
":start_line:5 <-- Invalid location\n" +
266+
"replacement content\n" +
267+
">>>>>>> REPLACE\n",
268+
})
269+
246270
const lines = diffContent.split("\n")
247271
const searchCount = lines.filter((l) => l.trim() === SEARCH).length
248272
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
254278
state.line++
255279
const marker = line.trim()
256280

281+
// Check for line markers in REPLACE sections (but allow escaped ones)
282+
if (state.current === State.AFTER_SEPARATOR) {
283+
if (marker.startsWith(":start_line:") && !line.trim().startsWith("\\:start_line:")) {
284+
return reportLineMarkerInReplaceError(":start_line:")
285+
}
286+
if (marker.startsWith(":end_line:") && !line.trim().startsWith("\\:end_line:")) {
287+
return reportLineMarkerInReplaceError(":end_line:")
288+
}
289+
}
290+
257291
switch (state.current) {
258292
case State.START:
259293
if (marker === SEP)

0 commit comments

Comments
 (0)