diff --git a/src/agents/sessions/tools/edit-diff.test.ts b/src/agents/sessions/tools/edit-diff.test.ts index 7bc2cfc95da5..11aae6d32f8e 100644 --- a/src/agents/sessions/tools/edit-diff.test.ts +++ b/src/agents/sessions/tools/edit-diff.test.ts @@ -1,13 +1,12 @@ import { describe, expect, it } from "vitest"; -import { normalizeToLF } from "../../line-endings.js"; -import { applyEditsToNormalizedContent, generateDiffString } from "./edit-diff.js"; +import { applyEditsPreservingLineEndings, generateDiffString } from "./edit-diff.js"; function getMismatchMessage( content: string, edits: Array<{ oldText: string; newText: string }>, ): string { try { - applyEditsToNormalizedContent(normalizeToLF(content), edits, "test.ts"); + applyEditsPreservingLineEndings(content, edits, "test.ts"); } catch (error) { return error instanceof Error ? error.message : String(error); } @@ -142,8 +141,8 @@ describe("applyEditsToNormalizedContent uniqueness", () => { it("replaces an exactly unique match when trailing whitespace makes a sibling line fuzzy-identical", () => { const content = "foo(); \nfoo();\nbar();\n"; - const result = applyEditsToNormalizedContent( - normalizeToLF(content), + const result = applyEditsPreservingLineEndings( + content, [{ oldText: "foo();\n", newText: "baz();\n" }], "test.ts", ); @@ -157,8 +156,8 @@ describe("applyEditsToNormalizedContent fuzzy uniqueness", () => { const content = "foo(); \nfoo();\t\nbar();\n"; expect(() => - applyEditsToNormalizedContent( - normalizeToLF(content), + applyEditsPreservingLineEndings( + content, [{ oldText: "foo();\n", newText: "baz();\n" }], "test.ts", ), diff --git a/src/agents/sessions/tools/edit-diff.ts b/src/agents/sessions/tools/edit-diff.ts index 0219e5a2bf5a..932e08106278 100644 --- a/src/agents/sessions/tools/edit-diff.ts +++ b/src/agents/sessions/tools/edit-diff.ts @@ -8,6 +8,12 @@ import { access, readFile } from "node:fs/promises"; import { createPatch, FILE_HEADERS_ONLY, structuredPatch } from "diff"; import { levenshteinDistance } from "../../../shared/levenshtein-distance.js"; import { normalizeToLF } from "../../line-endings.js"; +import { + applyReplacements, + applyReplacementsPreservingLineEndings, + applyReplacementsPreservingUnchangedLines, + type TextReplacement, +} from "./edit-replacements.js"; import { resolveToCwd } from "./path-utils.js"; /** @@ -68,122 +74,15 @@ export class EditNoChangeError extends Error { } } -interface MatchedEdit { +interface MatchedEdit extends TextReplacement { editIndex: number; - matchIndex: number; - matchLength: number; - newText: string; } -type TextReplacement = Pick; - -interface LineSpan { - start: number; - end: number; -} - -function splitLinesWithEndings(content: string): string[] { - return content.match(/[^\n]*\n|[^\n]+/g) ?? []; -} - -function getLineSpans(content: string): LineSpan[] { - let offset = 0; - return splitLinesWithEndings(content).map((line) => { - const span = { start: offset, end: offset + line.length }; - offset = span.end; - return span; - }); -} - -function getReplacementLineRange(lines: LineSpan[], replacement: TextReplacement) { - const replacementStart = replacement.matchIndex; - const replacementEnd = replacement.matchIndex + replacement.matchLength; - const startLine = lines.findIndex( - (line) => replacementStart >= line.start && replacementStart < line.end, - ); - if (startLine === -1) { - throw new Error("Replacement range is outside the base content."); - } - - let endLine = startLine; - while (endLine < lines.length) { - const line = lines.at(endLine); - if (!line || line.end >= replacementEnd) { - break; - } - endLine++; - } - if (endLine >= lines.length) { - throw new Error("Replacement range is outside the base content."); - } - return { startLine, endLine: endLine + 1 }; -} - -function applyReplacements(content: string, replacements: TextReplacement[], offset = 0): string { - let result = content; - for (const replacement of replacements.toReversed()) { - const matchIndex = replacement.matchIndex - offset; - result = - result.slice(0, matchIndex) + - replacement.newText + - result.slice(matchIndex + replacement.matchLength); - } - return result; -} - -/** - * Rewrite only lines touched by fuzzy replacements. Untouched lines retain - * their original bytes even though matching used normalized content. - */ -function applyReplacementsPreservingUnchangedLines( - originalContent: string, - baseContent: string, - replacements: TextReplacement[], -): string { - const originalLines = splitLinesWithEndings(originalContent); - const baseLines = getLineSpans(baseContent); - if (originalLines.length !== baseLines.length) { - throw new Error( - "Cannot preserve unchanged lines because the base content has a different line count.", - ); - } - - const groups: Array<{ - startLine: number; - endLine: number; - replacements: TextReplacement[]; - }> = []; - const sortedReplacements = replacements.toSorted((a, b) => a.matchIndex - b.matchIndex); - for (const replacement of sortedReplacements) { - const range = getReplacementLineRange(baseLines, replacement); - const current = groups.at(-1); - if (current && range.startLine < current.endLine) { - current.endLine = Math.max(current.endLine, range.endLine); - current.replacements.push(replacement); - } else { - groups.push({ ...range, replacements: [replacement] }); - } - } - - let originalLineIndex = 0; - let result = ""; - for (const group of groups) { - result += originalLines.slice(originalLineIndex, group.startLine).join(""); - const firstLine = baseLines.at(group.startLine); - const lastLine = baseLines.at(group.endLine - 1); - if (!firstLine || !lastLine) { - throw new Error("Replacement group is outside the base content."); - } - const groupStartOffset = firstLine.start; - const groupEndOffset = lastLine.end; - result += applyReplacements( - baseContent.slice(groupStartOffset, groupEndOffset), - group.replacements, - groupStartOffset, - ); - originalLineIndex = group.endLine; - } - return result + originalLines.slice(originalLineIndex).join(""); +interface AppliedEdits { + baseContent: string; + newContent: string; + replacementBaseContent: string; + replacements: MatchedEdit[]; } /** @@ -444,11 +343,7 @@ function getNoChangeError(path: string, totalEdits: number): EditNoChangeError { * then applied in reverse order so offsets remain stable. If any edit needs * fuzzy matching, only touched lines are rewritten from normalized content. */ -export function applyEditsToNormalizedContent( - normalizedContent: string, - edits: Edit[], - path: string, -): { baseContent: string; newContent: string } { +function applyEdits(normalizedContent: string, edits: Edit[], path: string): AppliedEdits { const normalizedEdits = edits.map((edit) => ({ oldText: normalizeToLF(edit.oldText), newText: normalizeToLF(edit.newText), @@ -520,9 +415,44 @@ export function applyEditsToNormalizedContent( throw getNoChangeError(path, normalizedEdits.length); } + return { + baseContent, + newContent, + replacementBaseContent, + replacements: matchedEdits, + }; +} + +function applyEditsToNormalizedContent( + normalizedContent: string, + edits: Edit[], + path: string, +): { baseContent: string; newContent: string } { + const { baseContent, newContent } = applyEdits(normalizedContent, edits, path); return { baseContent, newContent }; } +export function applyEditsPreservingLineEndings( + originalContent: string, + edits: Edit[], + path: string, +): { baseContent: string; newContent: string; finalContent: string } { + const applied = applyEdits(normalizeToLF(originalContent), edits, path); + const finalContent = applyReplacementsPreservingLineEndings( + originalContent, + applied.replacementBaseContent, + applied.replacements, + ); + if (normalizeToLF(finalContent) !== applied.newContent) { + throw new Error("Line-ending restoration changed the normalized edit result."); + } + return { + baseContent: applied.baseContent, + newContent: applied.newContent, + finalContent, + }; +} + /** Generate a standard unified patch. */ export function generateUnifiedPatch( path: string, diff --git a/src/agents/sessions/tools/edit-replacements.ts b/src/agents/sessions/tools/edit-replacements.ts new file mode 100644 index 000000000000..9d59e23b12af --- /dev/null +++ b/src/agents/sessions/tools/edit-replacements.ts @@ -0,0 +1,240 @@ +import { detectLineEnding } from "../../line-endings.js"; + +export interface TextReplacement { + matchIndex: number; + matchLength: number; + newText: string; +} + +interface LineSpan { + start: number; + end: number; +} + +interface ReplacementGroup { + startLine: number; + endLine: number; + replacements: TextReplacement[]; +} + +function splitLinesWithEndings(content: string): string[] { + return content.match(/[^\n]*\n|[^\n]+/g) ?? []; +} + +function getLineSpans(content: string): LineSpan[] { + let offset = 0; + return splitLinesWithEndings(content).map((line) => { + const span = { start: offset, end: offset + line.length }; + offset = span.end; + return span; + }); +} + +function getReplacementLineRange(lines: LineSpan[], replacement: TextReplacement) { + const replacementStart = replacement.matchIndex; + const replacementEnd = replacement.matchIndex + replacement.matchLength; + const startLine = lines.findIndex( + (line) => replacementStart >= line.start && replacementStart < line.end, + ); + if (startLine === -1) { + throw new Error("Replacement range is outside the base content."); + } + + let endLine = startLine; + while (endLine < lines.length) { + const line = lines.at(endLine); + if (!line || line.end >= replacementEnd) { + break; + } + endLine++; + } + if (endLine >= lines.length) { + throw new Error("Replacement range is outside the base content."); + } + return { startLine, endLine: endLine + 1 }; +} + +export function applyReplacements( + content: string, + replacements: TextReplacement[], + offset = 0, +): string { + let result = content; + for (const replacement of replacements.toReversed()) { + const matchIndex = replacement.matchIndex - offset; + result = + result.slice(0, matchIndex) + + replacement.newText + + result.slice(matchIndex + replacement.matchLength); + } + return result; +} + +function groupReplacementsByLine( + baseContent: string, + replacements: TextReplacement[], +): { lines: LineSpan[]; groups: ReplacementGroup[] } { + const lines = getLineSpans(baseContent); + const groups: ReplacementGroup[] = []; + const sortedReplacements = replacements.toSorted((a, b) => a.matchIndex - b.matchIndex); + for (const replacement of sortedReplacements) { + const range = getReplacementLineRange(lines, replacement); + const current = groups.at(-1); + if (current && range.startLine < current.endLine) { + current.endLine = Math.max(current.endLine, range.endLine); + current.replacements.push(replacement); + } else { + groups.push({ ...range, replacements: [replacement] }); + } + } + return { lines, groups }; +} + +/** + * Rewrite only lines touched by fuzzy replacements. Untouched lines retain + * their original bytes even though matching used normalized content. + */ +export function applyReplacementsPreservingUnchangedLines( + originalContent: string, + baseContent: string, + replacements: TextReplacement[], +): string { + const originalLines = splitLinesWithEndings(originalContent); + const { lines: baseLines, groups } = groupReplacementsByLine(baseContent, replacements); + if (originalLines.length !== baseLines.length) { + throw new Error( + "Cannot preserve unchanged lines because the base content has a different line count.", + ); + } + + let originalLineIndex = 0; + let result = ""; + for (const group of groups) { + result += originalLines.slice(originalLineIndex, group.startLine).join(""); + const firstLine = baseLines.at(group.startLine); + const lastLine = baseLines.at(group.endLine - 1); + if (!firstLine || !lastLine) { + throw new Error("Replacement group is outside the base content."); + } + const groupStartOffset = firstLine.start; + result += applyReplacements( + baseContent.slice(groupStartOffset, lastLine.end), + group.replacements, + groupStartOffset, + ); + originalLineIndex = group.endLine; + } + return result + originalLines.slice(originalLineIndex).join(""); +} + +function splitLinesWithTerminators(content: string): string[] { + return content.match(/[^\r\n]*(?:\r\n|\r|\n)|[^\r\n]+/g) ?? []; +} + +type LineTerminator = "\r\n" | "\r" | "\n"; + +function getLineTerminator(line: string | undefined): LineTerminator | undefined { + if (line === undefined) { + return undefined; + } + if (line.endsWith("\r\n")) { + return "\r\n"; + } + if (line.endsWith("\n")) { + return "\n"; + } + return line.endsWith("\r") ? "\r" : undefined; +} + +function restoreNormalizedLineEndings( + normalizedContent: string, + sourceLines: string[], + fallback: LineTerminator, +): string { + let sourceIndex = 0; + return normalizedContent.replace(/\n/g, () => { + const source = sourceLines[sourceIndex] ?? sourceLines.at(-1); + sourceIndex++; + return getLineTerminator(source) ?? fallback; + }); +} + +function countLineBreaks(content: string): number { + return content.match(/\n/g)?.length ?? 0; +} + +export function applyReplacementsPreservingLineEndings( + originalContent: string, + baseContent: string, + replacements: TextReplacement[], +): string { + const originalLines = splitLinesWithTerminators(originalContent); + const { lines: baseLines, groups } = groupReplacementsByLine(baseContent, replacements); + if (originalLines.length !== baseLines.length) { + throw new Error( + "Cannot preserve original line endings because the base content has a different line count.", + ); + } + + const fileFallback = detectLineEnding(originalContent); + let originalIndex = 0; + let result = ""; + for (const group of groups) { + result += originalLines.slice(originalIndex, group.startLine).join(""); + const firstLine = baseLines.at(group.startLine); + const lastLine = baseLines.at(group.endLine - 1); + if (!firstLine || !lastLine) { + throw new Error("Replacement group is outside the base content."); + } + + const groupStartOffset = firstLine.start; + const normalizedGroup = baseContent.slice(groupStartOffset, lastLine.end); + const sourceGroup = originalLines.slice(group.startLine, group.endLine); + const groupFallback = + getLineTerminator(sourceGroup[0]) ?? + getLineTerminator(originalLines[group.startLine - 1]) ?? + fileFallback; + const restoredGroup = restoreNormalizedLineEndings(normalizedGroup, sourceGroup, groupFallback); + const restoredReplacements = group.replacements.map((replacement) => { + const relativeStart = replacement.matchIndex - groupStartOffset; + const relativeEnd = relativeStart + replacement.matchLength; + const restoredStart = restoreNormalizedLineEndings( + normalizedGroup.slice(0, relativeStart), + sourceGroup, + groupFallback, + ).length; + const restoredEnd = restoreNormalizedLineEndings( + normalizedGroup.slice(0, relativeEnd), + sourceGroup, + groupFallback, + ).length; + const range = getReplacementLineRange(baseLines, replacement); + const replacementSource = originalLines.slice(range.startLine, range.endLine); + const replacementFallback = + getLineTerminator(replacementSource[0]) ?? + getLineTerminator(originalLines[range.startLine - 1]) ?? + fileFallback; + const consumedTerminatorCount = countLineBreaks( + normalizedGroup.slice(relativeStart, relativeEnd), + ); + const replacementTerminatorCount = countLineBreaks(replacement.newText); + const terminatorSources = + consumedTerminatorCount > 0 + ? replacementSource.slice(0, consumedTerminatorCount) + : replacementSource.slice(0, 1); + const sourceOffset = Math.max(0, terminatorSources.length - replacementTerminatorCount); + return { + matchIndex: restoredStart, + matchLength: restoredEnd - restoredStart, + newText: restoreNormalizedLineEndings( + replacement.newText, + terminatorSources.slice(sourceOffset), + replacementFallback, + ), + }; + }); + result += applyReplacements(restoredGroup, restoredReplacements); + originalIndex = group.endLine; + } + return result + originalLines.slice(originalIndex).join(""); +} diff --git a/src/agents/sessions/tools/edit.test.ts b/src/agents/sessions/tools/edit.test.ts index 935d5f27f7bf..1d7a622552fb 100644 --- a/src/agents/sessions/tools/edit.test.ts +++ b/src/agents/sessions/tools/edit.test.ts @@ -694,4 +694,159 @@ describe("edit tool", () => { expect("text" in tc1 ? tc1.text : "").toContain("Successfully replaced"); await expect(fs.readFile(filePath, "utf-8")).resolves.toBe("new content\n"); }); + + const lineEndingCases = [ + { + name: "keeps a lone carriage return that carries data", + original: "start\n10%\r50%\r100%\ndone\n", + edits: [{ oldText: "done", newText: "finished" }], + expected: "start\n10%\r50%\r100%\nfinished\n", + }, + { + name: "keeps a lone carriage return that terminates the edited line", + original: "prefix\rprogress\n", + edits: [{ oldText: "prefix", newText: "PREFIX" }], + expected: "PREFIX\rprogress\n", + }, + { + name: "keeps carriage return separators when a middle record is rewritten", + original: "id=1\rid=2\rid=3\nfooter\n", + edits: [{ oldText: "id=2", newText: "id=two" }], + expected: "id=1\rid=two\rid=3\nfooter\n", + }, + { + name: "expands a carriage return terminated line into carriage return terminated lines", + original: "id=1\rid=2\nfooter\n", + edits: [{ oldText: "id=1", newText: "id=1a\nid=1b" }], + expected: "id=1a\rid=1b\rid=2\nfooter\n", + }, + { + name: "keeps a lone carriage return at end of file on the edited line", + original: "only\r", + edits: [{ oldText: "only", newText: "ONLY" }], + expected: "ONLY\r", + }, + { + name: "uses the first carriage return for a leading inserted line", + original: "alpha\rbeta\r", + edits: [{ oldText: "alpha", newText: "prefix\nalpha" }], + expected: "prefix\ralpha\rbeta\r", + }, + { + name: "uses the retained target terminator for a prefixed line", + original: "alpha\r\nbeta\ngamma\n", + edits: [{ oldText: "beta", newText: "prefix\nbeta" }], + expected: "alpha\r\nprefix\nbeta\ngamma\n", + }, + { + name: "uses the edited target terminator for an appended line", + original: "alpha\r\nbeta\n", + edits: [{ oldText: "alpha", newText: "alpha\nsuffix" }], + expected: "alpha\r\nsuffix\r\nbeta\n", + }, + { + name: "keeps neighboring duplicate line terminators unchanged", + original: "A\r\nB\nC\r", + edits: [{ oldText: "A", newText: "B" }], + expected: "B\r\nB\nC\r", + }, + { + name: "preserves mixed terminators during a fuzzy multi-line edit", + original: "keep\r\ntarget \nafter\r\n", + edits: [{ oldText: "target\nafter", newText: "TARGET\nAFTER" }], + expected: "keep\r\nTARGET\nAFTER\r\n", + }, + { + name: "keeps the trailing boundary when replacement collapses mixed lines", + original: "alpha\r\nbeta\ngamma\n", + edits: [{ oldText: "alpha\nbeta", newText: "merged" }], + expected: "merged\ngamma\n", + }, + { + name: "uses the last consumed terminator when collapsing complete lines", + original: "alpha\r\nbeta\ngamma\n", + edits: [{ oldText: "alpha\nbeta\n", newText: "combined\n" }], + expected: "combined\ngamma\n", + }, + { + name: "aligns an unterminated replacement at end of file", + original: "h1\r\nh2\r\none\ntwo", + edits: [{ oldText: "one\ntwo", newText: "x\ny" }], + expected: "h1\r\nh2\r\nx\ny", + }, + { + name: "keeps trailing CRLF lines when the first line ends with LF", + original: "alpha\nbeta\r\ngamma\r\n", + edits: [{ oldText: "alpha", newText: "ALPHA" }], + expected: "ALPHA\nbeta\r\ngamma\r\n", + }, + { + name: "keeps trailing LF lines when the first line ends with CRLF", + original: "alpha\r\nbeta\ngamma\n", + edits: [{ oldText: "gamma", newText: "GAMMA" }], + expected: "alpha\r\nbeta\nGAMMA\n", + }, + { + name: "keeps untouched lines between two separate edits", + original: "one\r\ntwo\nthree\r75%\rfour\nfive\r\n", + edits: [ + { oldText: "one", newText: "ONE" }, + { oldText: "five", newText: "FIVE" }, + ], + expected: "ONE\r\ntwo\nthree\r75%\rfour\nFIVE\r\n", + }, + { + name: "writes inserted lines with the terminator of the replaced line", + original: "alpha\r\nbeta\r\ngamma\r\n", + edits: [{ oldText: "beta", newText: "beta1\nbeta2" }], + expected: "alpha\r\nbeta1\r\nbeta2\r\ngamma\r\n", + }, + { + name: "leaves a uniform CRLF file uniform", + original: "alpha\r\nbeta\r\ngamma\r\n", + edits: [{ oldText: "beta", newText: "BETA" }], + expected: "alpha\r\nBETA\r\ngamma\r\n", + }, + { + name: "leaves a uniform LF file uniform", + original: "alpha\nbeta\ngamma\n", + edits: [{ oldText: "beta", newText: "BETA" }], + expected: "alpha\nBETA\ngamma\n", + }, + ]; + + for (const testCase of lineEndingCases) { + it(testCase.name, async () => { + const filePath = await createTempFile(testCase.original); + const tool = createEditTool(tmpDir); + + const result = await tool.execute( + "call-line-endings", + { path: filePath, edits: testCase.edits }, + undefined, + ); + + const first = expectDefined(result.content[0], "result.content[0] test invariant"); + expect("text" in first ? first.text : "").toContain("Successfully replaced"); + await expect(fs.readFile(filePath, "utf-8")).resolves.toBe(testCase.expected); + }); + } + + it("preserves a lone carriage return on an edited line", async () => { + const filePath = await createTempFile("start\nprogress 10%\rprogress 50%\ndone\n"); + const tool = createEditTool(tmpDir); + + await tool.execute( + "call-cr-line", + { + path: filePath, + edits: [{ oldText: "progress 50%", newText: "progress 90%" }], + }, + undefined, + ); + + await expect(fs.readFile(filePath, "utf-8")).resolves.toBe( + "start\nprogress 10%\rprogress 90%\ndone\n", + ); + }); }); diff --git a/src/agents/sessions/tools/edit.ts b/src/agents/sessions/tools/edit.ts index b2f4c90c00f5..ef41623f2cf5 100644 --- a/src/agents/sessions/tools/edit.ts +++ b/src/agents/sessions/tools/edit.ts @@ -12,14 +12,14 @@ import { import { Box, Container, Spacer, Text } from "@earendil-works/pi-tui"; import { truncateUtf16Safe } from "@openclaw/normalization-core/utf16-slice"; import { Type } from "typebox"; -import { detectLineEnding, normalizeToLF, restoreLineEndings } from "../../line-endings.js"; +import { normalizeToLF } from "../../line-endings.js"; import { renderDiff } from "../../modes/interactive/components/diff.js"; import type { AgentTool } from "../../runtime/index.js"; import { textResult } from "../../tools/common.js"; import { decodeUtf8File } from "../../utf8-file.js"; import type { ToolDefinition } from "../extensions/types.js"; import { - applyEditsToNormalizedContent, + applyEditsPreservingLineEndings, computeEditsDiff, EditNoChangeError, type Edit, @@ -443,7 +443,6 @@ export function createEditToolDefinition( } const { bom, text: content } = stripBom(rawContent); - const originalEnding = detectLineEnding(content); const normalizedContent = normalizeToLF(content); const editSets = splitNoOpEdits(normalizedContent, originalEdits, path); const noOpEdits = editSets.noOpEdits; @@ -458,13 +457,12 @@ export function createEditToolDefinition( terminate: true, }; } - const { baseContent, newContent } = applyEditsToNormalizedContent( - normalizedContent, + const { baseContent, newContent, finalContent } = applyEditsPreservingLineEndings( + content, realEdits, path, ); - const finalContent = bom + restoreLineEndings(newContent, originalEnding); - await ops.writeFile(absolutePath, finalContent); + await ops.writeFile(absolutePath, bom + finalContent); if (signal?.aborted) { throw new Error("Operation aborted"); }