diff --git a/packages/opencode/src/format/index.ts b/packages/opencode/src/format/index.ts index 85934ce9c9..53a2c10119 100644 --- a/packages/opencode/src/format/index.ts +++ b/packages/opencode/src/format/index.ts @@ -25,7 +25,7 @@ export type Status = z.infer export interface Interface { readonly init: () => Effect.Effect readonly status: () => Effect.Effect - readonly file: (filepath: string) => Effect.Effect + readonly file: (filepath: string) => Effect.Effect } export class Service extends Context.Service()("@opencode/Format") {} @@ -70,16 +70,19 @@ export const layer = Layer.effect( } }), ) - return checks.filter((x) => x.cmd).map((x) => ({ item: x.item, cmd: x.cmd! })) + return checks + .filter((x): x is { item: Formatter.Info; cmd: string[] } => x.cmd !== false) + .map((x) => ({ item: x.item, cmd: x.cmd })) } function formatFile(filepath: string) { return Effect.gen(function* () { log.info("formatting", { file: filepath }) - const ext = path.extname(filepath) + const formatters = yield* Effect.promise(() => getFormatter(path.extname(filepath))) - for (const { item, cmd } of yield* Effect.promise(() => getFormatter(ext))) { - if (cmd === false) continue + if (!formatters.length) return false + + for (const { item, cmd } of formatters) { log.info("running", { command: cmd }) const replaced = cmd.map((x) => x.replace("$FILE", filepath)) const dir = yield* InstanceState.directory @@ -113,6 +116,8 @@ export const layer = Layer.effect( }) } } + + return true }) } @@ -188,7 +193,7 @@ export const layer = Layer.effect( const file = Effect.fn("Format.file")(function* (filepath: string) { const { formatFile } = yield* InstanceState.get(state) - yield* formatFile(filepath) + return yield* formatFile(filepath) }) return Service.of({ init, status, file }) diff --git a/packages/opencode/src/patch/index.ts b/packages/opencode/src/patch/index.ts index 19e1d7555b..3662f9e908 100644 --- a/packages/opencode/src/patch/index.ts +++ b/packages/opencode/src/patch/index.ts @@ -3,6 +3,7 @@ import * as path from "path" import * as fs from "fs/promises" import { readFileSync } from "fs" import { Log } from "../util" +import * as Bom from "../util/bom" const log = Log.create({ service: "patch" }) @@ -305,18 +306,19 @@ export function maybeParseApplyPatch( interface ApplyPatchFileUpdate { unified_diff: string content: string + bom: boolean } export function deriveNewContentsFromChunks(filePath: string, chunks: UpdateFileChunk[]): ApplyPatchFileUpdate { // Read original file content - let originalContent: string + let originalContent: ReturnType try { - originalContent = readFileSync(filePath, "utf-8") + originalContent = Bom.split(readFileSync(filePath, "utf-8")) } catch (error) { throw new Error(`Failed to read file ${filePath}: ${error}`, { cause: error }) } - let originalLines = originalContent.split("\n") + let originalLines = originalContent.text.split("\n") // Drop trailing empty element for consistent line counting if (originalLines.length > 0 && originalLines[originalLines.length - 1] === "") { @@ -331,14 +333,16 @@ export function deriveNewContentsFromChunks(filePath: string, chunks: UpdateFile newLines.push("") } - const newContent = newLines.join("\n") + const next = Bom.split(newLines.join("\n")) + const newContent = next.text // Generate unified diff - const unifiedDiff = generateUnifiedDiff(originalContent, newContent) + const unifiedDiff = generateUnifiedDiff(originalContent.text, newContent) return { unified_diff: unifiedDiff, content: newContent, + bom: originalContent.bom || next.bom, } } @@ -553,13 +557,13 @@ export async function applyHunksToFiles(hunks: Hunk[]): Promise { await fs.mkdir(moveDir, { recursive: true }) } - await fs.writeFile(hunk.move_path, fileUpdate.content, "utf-8") + await fs.writeFile(hunk.move_path, Bom.join(fileUpdate.content, fileUpdate.bom), "utf-8") await fs.unlink(hunk.path) modified.push(hunk.move_path) log.info(`Moved file: ${hunk.path} -> ${hunk.move_path}`) } else { // Regular update - await fs.writeFile(hunk.path, fileUpdate.content, "utf-8") + await fs.writeFile(hunk.path, Bom.join(fileUpdate.content, fileUpdate.bom), "utf-8") modified.push(hunk.path) log.info(`Updated file: ${hunk.path}`) } diff --git a/packages/opencode/src/tool/apply_patch.ts b/packages/opencode/src/tool/apply_patch.ts index 7da7dd255c..e36d5a65d8 100644 --- a/packages/opencode/src/tool/apply_patch.ts +++ b/packages/opencode/src/tool/apply_patch.ts @@ -14,6 +14,7 @@ import { AppFileSystem } from "@opencode-ai/shared/filesystem" import DESCRIPTION from "./apply_patch.txt" import { File } from "../file" import { Format } from "../format" +import * as Bom from "@/util/bom" const PatchParams = z.object({ patchText: z.string().describe("The full patch text that describes all changes to be made"), @@ -59,6 +60,7 @@ export const ApplyPatchTool = Tool.define( diff: string additions: number deletions: number + bom: boolean }> = [] let totalDiff = "" @@ -72,11 +74,12 @@ export const ApplyPatchTool = Tool.define( const oldContent = "" const newContent = hunk.contents.length === 0 || hunk.contents.endsWith("\n") ? hunk.contents : `${hunk.contents}\n` - const diff = trimDiff(createTwoFilesPatch(filePath, filePath, oldContent, newContent)) + const next = Bom.split(newContent) + const diff = trimDiff(createTwoFilesPatch(filePath, filePath, oldContent, next.text)) let additions = 0 let deletions = 0 - for (const change of diffLines(oldContent, newContent)) { + for (const change of diffLines(oldContent, next.text)) { if (change.added) additions += change.count || 0 if (change.removed) deletions += change.count || 0 } @@ -84,11 +87,12 @@ export const ApplyPatchTool = Tool.define( fileChanges.push({ filePath, oldContent, - newContent, + newContent: next.text, type: "add", diff, additions, deletions, + bom: next.bom, }) totalDiff += diff + "\n" @@ -104,13 +108,16 @@ export const ApplyPatchTool = Tool.define( ) } - const oldContent = yield* afs.readFileString(filePath) + const source = yield* Bom.readFile(afs, filePath) + const oldContent = source.text let newContent = oldContent + let bom = source.bom // Apply the update chunks to get new content try { const fileUpdate = Patch.deriveNewContentsFromChunks(filePath, hunk.chunks) newContent = fileUpdate.content + bom = fileUpdate.bom } catch (error) { return yield* Effect.fail(new Error(`apply_patch verification failed: ${error}`)) } @@ -136,6 +143,7 @@ export const ApplyPatchTool = Tool.define( diff, additions, deletions, + bom, }) totalDiff += diff + "\n" @@ -143,8 +151,8 @@ export const ApplyPatchTool = Tool.define( } case "delete": { - const contentToDelete = yield* afs - .readFileString(filePath) + const source = yield* Bom + .readFile(afs, filePath) .pipe( Effect.catch((error) => Effect.fail( @@ -154,6 +162,7 @@ export const ApplyPatchTool = Tool.define( ), ), ) + const contentToDelete = source.text const deleteDiff = trimDiff(createTwoFilesPatch(filePath, filePath, contentToDelete, "")) const deletions = contentToDelete.split("\n").length @@ -166,6 +175,7 @@ export const ApplyPatchTool = Tool.define( diff: deleteDiff, additions: 0, deletions, + bom: source.bom, }) totalDiff += deleteDiff + "\n" @@ -207,12 +217,12 @@ export const ApplyPatchTool = Tool.define( case "add": // Create parent directories (recursive: true is safe on existing/root dirs) - yield* afs.writeWithDirs(change.filePath, change.newContent) + yield* afs.writeWithDirs(change.filePath, Bom.join(change.newContent, change.bom)) updates.push({ file: change.filePath, event: "add" }) break case "update": - yield* afs.writeWithDirs(change.filePath, change.newContent) + yield* afs.writeWithDirs(change.filePath, Bom.join(change.newContent, change.bom)) updates.push({ file: change.filePath, event: "change" }) break @@ -220,7 +230,7 @@ export const ApplyPatchTool = Tool.define( if (change.movePath) { // Create parent directories (recursive: true is safe on existing/root dirs) - yield* afs.writeWithDirs(change.movePath!, change.newContent) + yield* afs.writeWithDirs(change.movePath!, Bom.join(change.newContent, change.bom)) yield* afs.remove(change.filePath) updates.push({ file: change.filePath, event: "unlink" }) updates.push({ file: change.movePath, event: "add" }) @@ -234,7 +244,9 @@ export const ApplyPatchTool = Tool.define( } if (edited) { - yield* format.file(edited) + if (yield* format.file(edited)) { + yield* Bom.syncFile(afs, edited, change.bom) + } yield* bus.publish(File.Event.Edited, { file: edited }) } } diff --git a/packages/opencode/src/tool/edit.ts b/packages/opencode/src/tool/edit.ts index 2c6c2c1308..858d14e043 100644 --- a/packages/opencode/src/tool/edit.ts +++ b/packages/opencode/src/tool/edit.ts @@ -18,6 +18,7 @@ import { Instance } from "../project/instance" import { Snapshot } from "@/snapshot" import { assertExternalDirectoryEffect } from "./external-directory" import { AppFileSystem } from "@opencode-ai/shared/filesystem" +import * as Bom from "@/util/bom" function normalizeLineEndings(text: string): string { return text.replaceAll("\r\n", "\n") @@ -84,7 +85,11 @@ export const EditTool = Tool.define( Effect.gen(function* () { if (params.oldString === "") { const existed = yield* afs.existsSafe(filePath) - contentNew = params.newString + const source = existed ? yield* Bom.readFile(afs, filePath) : { bom: false, text: "" } + const next = Bom.split(params.newString) + const desiredBom = source.bom || next.bom + contentOld = source.text + contentNew = next.text diff = trimDiff(createTwoFilesPatch(filePath, filePath, contentOld, contentNew)) yield* ctx.ask({ permission: "edit", @@ -95,8 +100,10 @@ export const EditTool = Tool.define( diff, }, }) - yield* afs.writeWithDirs(filePath, params.newString) - yield* format.file(filePath) + yield* afs.writeWithDirs(filePath, Bom.join(contentNew, desiredBom)) + if (yield* format.file(filePath)) { + contentNew = yield* Bom.syncFile(afs, filePath, desiredBom) + } yield* bus.publish(File.Event.Edited, { file: filePath }) yield* bus.publish(FileWatcher.Event.Updated, { file: filePath, @@ -108,13 +115,16 @@ export const EditTool = Tool.define( const info = yield* afs.stat(filePath).pipe(Effect.catch(() => Effect.succeed(undefined))) if (!info) throw new Error(`File ${filePath} not found`) if (info.type === "Directory") throw new Error(`Path is a directory, not a file: ${filePath}`) - contentOld = yield* afs.readFileString(filePath) + const source = yield* Bom.readFile(afs, filePath) + contentOld = source.text const ending = detectLineEnding(contentOld) const old = convertToLineEnding(normalizeLineEndings(params.oldString), ending) - const next = convertToLineEnding(normalizeLineEndings(params.newString), ending) + const replacement = convertToLineEnding(normalizeLineEndings(params.newString), ending) - contentNew = replace(contentOld, old, next, params.replaceAll) + const next = Bom.split(replace(contentOld, old, replacement, params.replaceAll)) + const desiredBom = source.bom || next.bom + contentNew = next.text diff = trimDiff( createTwoFilesPatch( @@ -134,14 +144,15 @@ export const EditTool = Tool.define( }, }) - yield* afs.writeWithDirs(filePath, contentNew) - yield* format.file(filePath) + yield* afs.writeWithDirs(filePath, Bom.join(contentNew, desiredBom)) + if (yield* format.file(filePath)) { + contentNew = yield* Bom.syncFile(afs, filePath, desiredBom) + } yield* bus.publish(File.Event.Edited, { file: filePath }) yield* bus.publish(FileWatcher.Event.Updated, { file: filePath, event: "change", }) - contentNew = yield* afs.readFileString(filePath) diff = trimDiff( createTwoFilesPatch( filePath, diff --git a/packages/opencode/src/tool/write.ts b/packages/opencode/src/tool/write.ts index 741091b21d..79ed585198 100644 --- a/packages/opencode/src/tool/write.ts +++ b/packages/opencode/src/tool/write.ts @@ -13,6 +13,7 @@ import { AppFileSystem } from "@opencode-ai/shared/filesystem" import { Instance } from "../project/instance" import { trimDiff } from "./edit" import { assertExternalDirectoryEffect } from "./external-directory" +import * as Bom from "@/util/bom" const MAX_PROJECT_DIAGNOSTICS_FILES = 5 @@ -38,9 +39,13 @@ export const WriteTool = Tool.define( yield* assertExternalDirectoryEffect(ctx, filepath) const exists = yield* fs.existsSafe(filepath) - const contentOld = exists ? yield* fs.readFileString(filepath) : "" + const source = exists ? yield* Bom.readFile(fs, filepath) : { bom: false, text: "" } + const next = Bom.split(params.content) + const desiredBom = source.bom || next.bom + const contentOld = source.text + const contentNew = next.text - const diff = trimDiff(createTwoFilesPatch(filepath, filepath, contentOld, params.content)) + const diff = trimDiff(createTwoFilesPatch(filepath, filepath, contentOld, contentNew)) yield* ctx.ask({ permission: "edit", patterns: [path.relative(Instance.worktree, filepath)], @@ -51,8 +56,10 @@ export const WriteTool = Tool.define( }, }) - yield* fs.writeWithDirs(filepath, params.content) - yield* format.file(filepath) + yield* fs.writeWithDirs(filepath, Bom.join(contentNew, desiredBom)) + if (yield* format.file(filepath)) { + yield* Bom.syncFile(fs, filepath, desiredBom) + } yield* bus.publish(File.Event.Edited, { file: filepath }) yield* bus.publish(FileWatcher.Event.Updated, { file: filepath, diff --git a/packages/opencode/src/util/bom.ts b/packages/opencode/src/util/bom.ts new file mode 100644 index 0000000000..484228f3d4 --- /dev/null +++ b/packages/opencode/src/util/bom.ts @@ -0,0 +1,31 @@ +import { Effect } from "effect" +import { AppFileSystem } from "@opencode-ai/shared/filesystem" + +const BOM_CODE = 0xfeff +const BOM = String.fromCharCode(BOM_CODE) + +export function split(text: string) { + if (text.charCodeAt(0) !== BOM_CODE) return { bom: false, text } + return { bom: true, text: text.slice(1) } +} + +export function join(text: string, bom: boolean) { + const stripped = split(text).text + if (!bom) return stripped + return BOM + stripped +} + +export const readFile = Effect.fn("Bom.readFile")(function* (fs: AppFileSystem.Interface, filePath: string) { + return split(new TextDecoder("utf-8", { ignoreBOM: true }).decode(yield* fs.readFile(filePath))) +}) + +export const syncFile = Effect.fn("Bom.syncFile")(function* ( + fs: AppFileSystem.Interface, + filePath: string, + bom: boolean, +) { + const current = yield* readFile(fs, filePath) + if (current.bom === bom) return current.text + yield* fs.writeWithDirs(filePath, join(current.text, bom)) + return current.text +}) diff --git a/packages/opencode/test/format/format.test.ts b/packages/opencode/test/format/format.test.ts index 5530e195b2..2f6f235aa1 100644 --- a/packages/opencode/test/format/format.test.ts +++ b/packages/opencode/test/format/format.test.ts @@ -126,6 +126,24 @@ describe("Format", () => { it.live("service initializes without error", () => provideTmpdirInstance(() => Format.Service.use(() => Effect.void))) + it.live("file() returns false when no formatter runs", () => + provideTmpdirInstance( + (dir) => + Effect.gen(function* () { + const file = `${dir}/test.txt` + yield* Effect.promise(() => Bun.write(file, "x")) + + const formatted = yield* Format.Service.use((fmt) => fmt.file(file)) + expect(formatted).toBe(false) + }), + { + config: { + formatter: false, + }, + }, + ), + ) + it.live("status() initializes formatter state per directory", () => Effect.gen(function* () { const a = yield* provideTmpdirInstance(() => Format.Service.use((fmt) => fmt.status()), { @@ -219,7 +237,7 @@ describe("Format", () => { yield* Format.Service.use((fmt) => Effect.gen(function* () { yield* fmt.init() - yield* fmt.file(file) + expect(yield* fmt.file(file)).toBe(true) }), ) @@ -229,11 +247,21 @@ describe("Format", () => { config: { formatter: { first: { - command: ["sh", "-c", 'sleep 0.05; v=$(cat "$1"); printf \'%sA\' "$v" > "$1"', "sh", "$FILE"], + command: [ + "node", + "-e", + "const fs = require('fs'); const file = process.argv[1]; fs.writeFileSync(file, fs.readFileSync(file, 'utf8') + 'A')", + "$FILE", + ], extensions: [".seq"], }, second: { - command: ["sh", "-c", 'v=$(cat "$1"); printf \'%sB\' "$v" > "$1"', "sh", "$FILE"], + command: [ + "node", + "-e", + "const fs = require('fs'); const file = process.argv[1]; fs.writeFileSync(file, fs.readFileSync(file, 'utf8') + 'B')", + "$FILE", + ], extensions: [".seq"], }, }, diff --git a/packages/opencode/test/tool/apply_patch.test.ts b/packages/opencode/test/tool/apply_patch.test.ts index ebfa9a531e..7ce483726b 100644 --- a/packages/opencode/test/tool/apply_patch.test.ts +++ b/packages/opencode/test/tool/apply_patch.test.ts @@ -195,6 +195,34 @@ describe("tool.apply_patch freeform", () => { }) }) + test("does not invent a first-line diff for BOM files", async () => { + await using fixture = await tmpdir() + const { ctx, calls } = makeCtx() + + await Instance.provide({ + directory: fixture.path, + fn: async () => { + const bom = String.fromCharCode(0xfeff) + const target = path.join(fixture.path, "example.cs") + await fs.writeFile(target, `${bom}using System;\n\nclass Test {}\n`, "utf-8") + + const patchText = "*** Begin Patch\n*** Update File: example.cs\n@@\n class Test {}\n+class Next {}\n*** End Patch" + + await execute({ patchText }, ctx) + + expect(calls.length).toBe(1) + const shown = calls[0].metadata.files[0]?.patch ?? "" + expect(shown).not.toContain(bom) + expect(shown).not.toContain("-using System;") + expect(shown).not.toContain("+using System;") + + const content = await fs.readFile(target, "utf-8") + expect(content.charCodeAt(0)).toBe(0xfeff) + expect(content.slice(1)).toBe("using System;\n\nclass Test {}\nclass Next {}\n") + }, + }) + }) + test("inserts lines with insert-only hunk", async () => { await using fixture = await tmpdir() const { ctx } = makeCtx() diff --git a/packages/opencode/test/tool/edit.test.ts b/packages/opencode/test/tool/edit.test.ts index b5fbc0a67d..82e1b4a7fd 100644 --- a/packages/opencode/test/tool/edit.test.ts +++ b/packages/opencode/test/tool/edit.test.ts @@ -96,6 +96,37 @@ describe("tool.edit", () => { }) }) + test("preserves BOM when oldString is empty on existing files", async () => { + await using tmp = await tmpdir() + const filepath = path.join(tmp.path, "existing.cs") + const bom = String.fromCharCode(0xfeff) + await fs.writeFile(filepath, `${bom}using System;\n`, "utf-8") + + await Instance.provide({ + directory: tmp.path, + fn: async () => { + const edit = await resolve() + const result = await Effect.runPromise( + edit.execute( + { + filePath: filepath, + oldString: "", + newString: "using Up;\n", + }, + ctx, + ), + ) + + expect(result.metadata.diff).toContain("-using System;") + expect(result.metadata.diff).toContain("+using Up;") + + const content = await fs.readFile(filepath, "utf-8") + expect(content.charCodeAt(0)).toBe(0xfeff) + expect(content.slice(1)).toBe("using Up;\n") + }, + }) + }) + test("creates new file with nested directories", async () => { await using tmp = await tmpdir() const filepath = path.join(tmp.path, "nested", "dir", "file.txt") @@ -183,6 +214,38 @@ describe("tool.edit", () => { }) }) + test("replaces the first visible line in BOM files", async () => { + await using tmp = await tmpdir() + const filepath = path.join(tmp.path, "existing.cs") + const bom = String.fromCharCode(0xfeff) + await fs.writeFile(filepath, `${bom}using System;\nclass Test {}\n`, "utf-8") + + await Instance.provide({ + directory: tmp.path, + fn: async () => { + const edit = await resolve() + const result = await Effect.runPromise( + edit.execute( + { + filePath: filepath, + oldString: "using System;", + newString: "using Up;", + }, + ctx, + ), + ) + + expect(result.metadata.diff).toContain("-using System;") + expect(result.metadata.diff).toContain("+using Up;") + expect(result.metadata.diff).not.toContain(bom) + + const content = await fs.readFile(filepath, "utf-8") + expect(content.charCodeAt(0)).toBe(0xfeff) + expect(content.slice(1)).toBe("using Up;\nclass Test {}\n") + }, + }) + }) + test("throws error when file does not exist", async () => { await using tmp = await tmpdir() const filepath = path.join(tmp.path, "nonexistent.txt") diff --git a/packages/opencode/test/tool/write.test.ts b/packages/opencode/test/tool/write.test.ts index 50d3b57527..36131f9596 100644 --- a/packages/opencode/test/tool/write.test.ts +++ b/packages/opencode/test/tool/write.test.ts @@ -114,6 +114,54 @@ describe("tool.write", () => { ), ) + it.live("preserves BOM when overwriting existing files", () => + provideTmpdirInstance((dir) => + Effect.gen(function* () { + const filepath = path.join(dir, "existing.cs") + const bom = String.fromCharCode(0xfeff) + yield* Effect.promise(() => fs.writeFile(filepath, `${bom}using System;\n`, "utf-8")) + + yield* run({ filePath: filepath, content: "using Up;\n" }) + + const content = yield* Effect.promise(() => fs.readFile(filepath, "utf-8")) + expect(content.charCodeAt(0)).toBe(0xfeff) + expect(content.slice(1)).toBe("using Up;\n") + }), + ), + ) + + it.live("restores BOM after formatter strips it", () => + provideTmpdirInstance( + (dir) => + Effect.gen(function* () { + const filepath = path.join(dir, "formatted.cs") + const bom = String.fromCharCode(0xfeff) + yield* Effect.promise(() => fs.writeFile(filepath, `${bom}using System;\n`, "utf-8")) + + yield* run({ filePath: filepath, content: "using Up;\n" }) + + const content = yield* Effect.promise(() => fs.readFile(filepath, "utf-8")) + expect(content.charCodeAt(0)).toBe(0xfeff) + expect(content.slice(1)).toBe("using Up;\n") + }), + { + config: { + formatter: { + stripbom: { + extensions: [".cs"], + command: [ + "node", + "-e", + "const fs = require('fs'); const file = process.argv[1]; let text = fs.readFileSync(file, 'utf8'); if (text.charCodeAt(0) === 0xfeff) text = text.slice(1); fs.writeFileSync(file, text, 'utf8')", + "$FILE", + ], + }, + }, + }, + }, + ), + ) + it.live("returns diff in metadata for existing files", () => provideTmpdirInstance((dir) => Effect.gen(function* () {