From be9b4d1bcde39341c7813b66c5c19150a01bb8c2 Mon Sep 17 00:00:00 2001 From: Quan Ran Date: Sat, 7 Mar 2026 16:42:54 +0900 Subject: [PATCH] fix(opencode): preserve original line endings in 'edit' tool (#9443) Co-authored-by: LukeParkerDev <10430890+Hona@users.noreply.github.com> --- packages/opencode/src/tool/edit.ts | 16 +- packages/opencode/test/tool/edit.test.ts | 183 +++++++++++++++++++++++ 2 files changed, 198 insertions(+), 1 deletion(-) diff --git a/packages/opencode/src/tool/edit.ts b/packages/opencode/src/tool/edit.ts index 7a097d3fe..c7b12378e 100644 --- a/packages/opencode/src/tool/edit.ts +++ b/packages/opencode/src/tool/edit.ts @@ -24,6 +24,15 @@ function normalizeLineEndings(text: string): string { return text.replaceAll("\r\n", "\n") } +function detectLineEnding(text: string): "\n" | "\r\n" { + return text.includes("\r\n") ? "\r\n" : "\n" +} + +function convertToLineEnding(text: string, ending: "\n" | "\r\n"): string { + if (ending === "\n") return text + return text.replaceAll("\n", "\r\n") +} + export const EditTool = Tool.define("edit", { description: DESCRIPTION, parameters: z.object({ @@ -78,7 +87,12 @@ export const EditTool = Tool.define("edit", { if (stats.isDirectory()) throw new Error(`Path is a directory, not a file: ${filePath}`) await FileTime.assert(ctx.sessionID, filePath) contentOld = await Filesystem.readText(filePath) - contentNew = replace(contentOld, params.oldString, params.newString, params.replaceAll) + + const ending = detectLineEnding(contentOld) + const old = convertToLineEnding(normalizeLineEndings(params.oldString), ending) + const next = convertToLineEnding(normalizeLineEndings(params.newString), ending) + + contentNew = replace(contentOld, old, next, params.replaceAll) diff = trimDiff( createTwoFilesPatch(filePath, filePath, normalizeLineEndings(contentOld), normalizeLineEndings(contentNew)), diff --git a/packages/opencode/test/tool/edit.test.ts b/packages/opencode/test/tool/edit.test.ts index c3cf0404b..bcf75da05 100644 --- a/packages/opencode/test/tool/edit.test.ts +++ b/packages/opencode/test/tool/edit.test.ts @@ -451,6 +451,189 @@ describe("tool.edit", () => { }) }) + describe("line endings", () => { + const old = "alpha\nbeta\ngamma" + const next = "alpha\nbeta-updated\ngamma" + const alt = "alpha\nbeta\nomega" + + const normalize = (text: string, ending: "\n" | "\r\n") => { + const normalized = text.replaceAll("\r\n", "\n") + if (ending === "\n") return normalized + return normalized.replaceAll("\n", "\r\n") + } + + const count = (content: string) => { + const crlf = content.match(/\r\n/g)?.length ?? 0 + const lf = content.match(/\n/g)?.length ?? 0 + return { + crlf, + lf: lf - crlf, + } + } + + const expectLf = (content: string) => { + const counts = count(content) + expect(counts.crlf).toBe(0) + expect(counts.lf).toBeGreaterThan(0) + } + + const expectCrlf = (content: string) => { + const counts = count(content) + expect(counts.lf).toBe(0) + expect(counts.crlf).toBeGreaterThan(0) + } + + type Input = { + content: string + oldString: string + newString: string + replaceAll?: boolean + } + + const apply = async (input: Input) => { + await using tmp = await tmpdir({ + init: async (dir) => { + await Bun.write(path.join(dir, "test.txt"), input.content) + }, + }) + + return await Instance.provide({ + directory: tmp.path, + fn: async () => { + const edit = await EditTool.init() + const filePath = path.join(tmp.path, "test.txt") + FileTime.read(ctx.sessionID, filePath) + await edit.execute( + { + filePath, + oldString: input.oldString, + newString: input.newString, + replaceAll: input.replaceAll, + }, + ctx, + ) + return await Bun.file(filePath).text() + }, + }) + } + + test("preserves LF with LF multi-line strings", async () => { + const content = normalize(old + "\n", "\n") + const output = await apply({ + content, + oldString: normalize(old, "\n"), + newString: normalize(next, "\n"), + }) + expect(output).toBe(normalize(next + "\n", "\n")) + expectLf(output) + }) + + test("preserves CRLF with CRLF multi-line strings", async () => { + const content = normalize(old + "\n", "\r\n") + const output = await apply({ + content, + oldString: normalize(old, "\r\n"), + newString: normalize(next, "\r\n"), + }) + expect(output).toBe(normalize(next + "\n", "\r\n")) + expectCrlf(output) + }) + + test("preserves LF when old/new use CRLF", async () => { + const content = normalize(old + "\n", "\n") + const output = await apply({ + content, + oldString: normalize(old, "\r\n"), + newString: normalize(next, "\r\n"), + }) + expect(output).toBe(normalize(next + "\n", "\n")) + expectLf(output) + }) + + test("preserves CRLF when old/new use LF", async () => { + const content = normalize(old + "\n", "\r\n") + const output = await apply({ + content, + oldString: normalize(old, "\n"), + newString: normalize(next, "\n"), + }) + expect(output).toBe(normalize(next + "\n", "\r\n")) + expectCrlf(output) + }) + + test("preserves LF when newString uses CRLF", async () => { + const content = normalize(old + "\n", "\n") + const output = await apply({ + content, + oldString: normalize(old, "\n"), + newString: normalize(next, "\r\n"), + }) + expect(output).toBe(normalize(next + "\n", "\n")) + expectLf(output) + }) + + test("preserves CRLF when newString uses LF", async () => { + const content = normalize(old + "\n", "\r\n") + const output = await apply({ + content, + oldString: normalize(old, "\r\n"), + newString: normalize(next, "\n"), + }) + expect(output).toBe(normalize(next + "\n", "\r\n")) + expectCrlf(output) + }) + + test("preserves LF with mixed old/new line endings", async () => { + const content = normalize(old + "\n", "\n") + const output = await apply({ + content, + oldString: "alpha\nbeta\r\ngamma", + newString: "alpha\r\nbeta\nomega", + }) + expect(output).toBe(normalize(alt + "\n", "\n")) + expectLf(output) + }) + + test("preserves CRLF with mixed old/new line endings", async () => { + const content = normalize(old + "\n", "\r\n") + const output = await apply({ + content, + oldString: "alpha\r\nbeta\ngamma", + newString: "alpha\nbeta\r\nomega", + }) + expect(output).toBe(normalize(alt + "\n", "\r\n")) + expectCrlf(output) + }) + + test("replaceAll preserves LF for multi-line blocks", async () => { + const blockOld = "alpha\nbeta" + const blockNew = "alpha\nbeta-updated" + const content = normalize(blockOld + "\n" + blockOld + "\n", "\n") + const output = await apply({ + content, + oldString: normalize(blockOld, "\n"), + newString: normalize(blockNew, "\n"), + replaceAll: true, + }) + expect(output).toBe(normalize(blockNew + "\n" + blockNew + "\n", "\n")) + expectLf(output) + }) + + test("replaceAll preserves CRLF for multi-line blocks", async () => { + const blockOld = "alpha\nbeta" + const blockNew = "alpha\nbeta-updated" + const content = normalize(blockOld + "\n" + blockOld + "\n", "\r\n") + const output = await apply({ + content, + oldString: normalize(blockOld, "\r\n"), + newString: normalize(blockNew, "\r\n"), + replaceAll: true, + }) + expect(output).toBe(normalize(blockNew + "\n" + blockNew + "\n", "\r\n")) + expectCrlf(output) + }) + }) + describe("concurrent editing", () => { test("serializes concurrent edits to same file", async () => { await using tmp = await tmpdir()