Merge pull request #1861 from KJ7LNW/roo-fix-apply-diff-collisions

Fix multiple `apply_diff` issues

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

@@ -73,6 +73,7 @@ Diff format:
 
 \`\`\`
 
+
 Example:
 
 Original file:
@@ -128,6 +129,7 @@ def calculate_sum(items):
 >>>>>>> REPLACE
 \`\`\`
 
+
 Usage:
 <apply_diff>
 <path>File path here</path>
@@ -139,22 +141,140 @@ Only use a single line of '=======' between search and replacement content, beca
 </apply_diff>`
 	}
 
+	private unescapeMarkers(content: string): string {
+		return content
+			.replace(/^\\<<<<<<< SEARCH/gm, "<<<<<<< SEARCH")
+			.replace(/^\\=======/gm, "=======")
+			.replace(/^\\>>>>>>> REPLACE/gm, ">>>>>>> REPLACE")
+			.replace(/^\\-------/gm, "-------")
+			.replace(/^\\:end_line:/gm, ":end_line:")
+			.replace(/^\\:start_line:/gm, ":start_line:")
+	}
+
+	private validateMarkerSequencing(diffContent: string): { success: boolean; error?: string } {
+		enum State {
+			START,
+			AFTER_SEARCH,
+			AFTER_SEPARATOR,
+		}
+		const state = { current: State.START, line: 0 }
+
+		const SEARCH = "<<<<<<< SEARCH"
+		const SEP = "======="
+		const REPLACE = ">>>>>>> REPLACE"
+
+		const reportError = (found: string, expected: string) => ({
+			success: false,
+			error:
+				`ERROR: Special marker '${found}' found in your diff content at line ${state.line}:\n` +
+				"\n" +
+				`When removing merge conflict markers like '${found}' from files, you MUST escape them\n` +
+				"in your SEARCH section by prepending a backslash (\\) at the beginning of the line:\n" +
+				"\n" +
+				"CORRECT FORMAT:\n\n" +
+				"<<<<<<< SEARCH\n" +
+				"content before\n" +
+				`\\${found}    <-- Note the backslash here in this example\n` +
+				"content after\n" +
+				"=======\n" +
+				"replacement content\n" +
+				">>>>>>> REPLACE\n" +
+				"\n" +
+				"Without escaping, the system confuses your content with diff syntax markers.\n" +
+				"You may use multiple diff blocks in a single diff request, but ANY of ONLY the following separators that occur within SEARCH or REPLACE content must be escaped, as follows:\n" +
+				`\\${SEARCH}\n` +
+				`\\${SEP}\n` +
+				`\\${REPLACE}\n`,
+		})
+
+		for (const line of diffContent.split("\n")) {
+			state.line++
+			const marker = line.trim()
+
+			switch (state.current) {
+				case State.START:
+					if (marker === SEP) return reportError(SEP, SEARCH)
+					if (marker === REPLACE) return reportError(REPLACE, SEARCH)
+					if (marker === SEARCH) state.current = State.AFTER_SEARCH
+					break
+
+				case State.AFTER_SEARCH:
+					if (marker === SEARCH) return reportError(SEARCH, SEP)
+					if (marker === REPLACE) return reportError(REPLACE, SEP)
+					if (marker === SEP) state.current = State.AFTER_SEPARATOR
+					break
+
+				case State.AFTER_SEPARATOR:
+					if (marker === SEARCH) return reportError(SEARCH, REPLACE)
+					if (marker === SEP) return reportError(SEP, REPLACE)
+					if (marker === REPLACE) state.current = State.START
+					break
+			}
+		}
+
+		return state.current === State.START
+			? { success: true }
+			: {
+					success: false,
+					error: `ERROR: Unexpected end of sequence: Expected '${state.current === State.AFTER_SEARCH ? SEP : REPLACE}' was not found.`,
+				}
+	}
+
 	async applyDiff(
 		originalContent: string,
 		diffContent: string,
 		_paramStartLine?: number,
 		_paramEndLine?: number,
 	): Promise<DiffResult> {
+		const validseq = this.validateMarkerSequencing(diffContent)
+		if (!validseq.success) {
+			return {
+				success: false,
+				error: validseq.error!,
+			}
+		}
+
+		/*
+			Regex parts:
+			
+			1. (?:^|\n)  
+			  Ensures the first marker starts at the beginning of the file or right after a newline.
+
+			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.
+
+			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.
+
+			5. ((?<!\\)-------\s*\n)?  
+			  Optionally matches the “-------” marker line (group 5).
+
+			6. ([\s\S]*?)(?:\n)?  
+			  Non‐greedy match for the “search content” (group 6) 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).
+
+			9. (?:(?<=\n)(?<!\\)>>>>>>> REPLACE)(?=\n|$)  
+			  Matches the final “>>>>>>> REPLACE” marker on its own line (and requires a following newline or the end of file).
+		*/
+
 		let matches = [
 			...diffContent.matchAll(
-				/<<<<<<< SEARCH\n(:start_line:\s*(\d+)\n){0,1}(:end_line:\s*(\d+)\n){0,1}(-------\n){0,1}([\s\S]*?)\n?=======\n([\s\S]*?)\n?>>>>>>> REPLACE/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:end_line: end line\\n-------\\n[search content]\\n=======\\n[replace content]\\n>>>>>>> REPLACE\n- Tip: Make sure to include start_line/end_line/SEARCH/REPLACE sections with correct markers`,
+				error: `Invalid diff format - missing required sections\n\nDebug Info:\n- Expected Format: <<<<<<< SEARCH\\n:start_line: start line\\n:end_line: end line\\n-------\\n[search content]\\n=======\\n[replace content]\\n>>>>>>> REPLACE\n- Tip: Make sure to include start_line/end_line/SEARCH/=======/REPLACE sections with correct markers on new lines`,
 			}
 		}
 		// Detect line ending from original content
@@ -176,12 +296,29 @@ Only use a single line of '=======' between search and replacement content, beca
 			startLine += startLine === 0 ? 0 : delta
 			endLine += delta
 
+			// First unescape any escaped markers in the content
+			searchContent = this.unescapeMarkers(searchContent)
+			replaceContent = this.unescapeMarkers(replaceContent)
+
 			// Strip line numbers from search and replace content if every line starts with a line number
 			if (everyLineHasLineNumbers(searchContent) && everyLineHasLineNumbers(replaceContent)) {
 				searchContent = stripLineNumbers(searchContent)
 				replaceContent = stripLineNumbers(replaceContent)
 			}
 
+			// Validate that search and replace content are not identical
+			if (searchContent === replaceContent) {
+				diffResults.push({
+					success: false,
+					error:
+						`Search and replace content are identical - no changes would be made\n\n` +
+						`Debug Info:\n` +
+						`- Search and replace must be different to make changes\n` +
+						`- Use read_file to verify the content you want to change`,
+				})
+				continue
+			}
+
 			// Split content into lines, handling both \n and \r\n
 			const searchLines = searchContent === "" ? [] : searchContent.split(/\r?\n/)
 			const replaceLines = replaceContent === "" ? [] : replaceContent.split(/\r?\n/)

src/core/webview/ClineProvider.ts

@@ -2046,6 +2046,7 @@ export class ClineProvider extends EventEmitter<ClineProviderEvents> implements
 				apiConfiguration.apiModelId || apiConfiguration.openRouterModelId || "",
 				fuzzyMatchThreshold,
 				Experiments.isEnabled(experiments, EXPERIMENT_IDS.DIFF_STRATEGY),
+				Experiments.isEnabled(experiments, EXPERIMENT_IDS.MULTI_SEARCH_AND_REPLACE),
 			)
 			const cwd = this.cwd