Skip to content

Commit bb69648

Browse files
authored
fix: preserve BOM in text tool round-trips (#23797)
1 parent ed3d364 commit bb69648

10 files changed

Lines changed: 276 additions & 39 deletions

File tree

packages/opencode/src/format/index.ts

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ export type Status = z.infer<typeof Status>
2525
export interface Interface {
2626
readonly init: () => Effect.Effect<void>
2727
readonly status: () => Effect.Effect<Status[]>
28-
readonly file: (filepath: string) => Effect.Effect<void>
28+
readonly file: (filepath: string) => Effect.Effect<boolean>
2929
}
3030

3131
export class Service extends Context.Service<Service, Interface>()("@opencode/Format") {}
@@ -70,16 +70,19 @@ export const layer = Layer.effect(
7070
}
7171
}),
7272
)
73-
return checks.filter((x) => x.cmd).map((x) => ({ item: x.item, cmd: x.cmd! }))
73+
return checks
74+
.filter((x): x is { item: Formatter.Info; cmd: string[] } => x.cmd !== false)
75+
.map((x) => ({ item: x.item, cmd: x.cmd }))
7476
}
7577

7678
function formatFile(filepath: string) {
7779
return Effect.gen(function* () {
7880
log.info("formatting", { file: filepath })
79-
const ext = path.extname(filepath)
81+
const formatters = yield* Effect.promise(() => getFormatter(path.extname(filepath)))
8082

81-
for (const { item, cmd } of yield* Effect.promise(() => getFormatter(ext))) {
82-
if (cmd === false) continue
83+
if (!formatters.length) return false
84+
85+
for (const { item, cmd } of formatters) {
8386
log.info("running", { command: cmd })
8487
const replaced = cmd.map((x) => x.replace("$FILE", filepath))
8588
const dir = yield* InstanceState.directory
@@ -113,6 +116,8 @@ export const layer = Layer.effect(
113116
})
114117
}
115118
}
119+
120+
return true
116121
})
117122
}
118123

@@ -188,7 +193,7 @@ export const layer = Layer.effect(
188193

189194
const file = Effect.fn("Format.file")(function* (filepath: string) {
190195
const { formatFile } = yield* InstanceState.get(state)
191-
yield* formatFile(filepath)
196+
return yield* formatFile(filepath)
192197
})
193198

194199
return Service.of({ init, status, file })

packages/opencode/src/patch/index.ts

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import * as path from "path"
33
import * as fs from "fs/promises"
44
import { readFileSync } from "fs"
55
import { Log } from "../util"
6+
import * as Bom from "../util/bom"
67

78
const log = Log.create({ service: "patch" })
89

@@ -305,18 +306,19 @@ export function maybeParseApplyPatch(
305306
interface ApplyPatchFileUpdate {
306307
unified_diff: string
307308
content: string
309+
bom: boolean
308310
}
309311

310312
export function deriveNewContentsFromChunks(filePath: string, chunks: UpdateFileChunk[]): ApplyPatchFileUpdate {
311313
// Read original file content
312-
let originalContent: string
314+
let originalContent: ReturnType<typeof Bom.split>
313315
try {
314-
originalContent = readFileSync(filePath, "utf-8")
316+
originalContent = Bom.split(readFileSync(filePath, "utf-8"))
315317
} catch (error) {
316318
throw new Error(`Failed to read file ${filePath}: ${error}`, { cause: error })
317319
}
318320

319-
let originalLines = originalContent.split("\n")
321+
let originalLines = originalContent.text.split("\n")
320322

321323
// Drop trailing empty element for consistent line counting
322324
if (originalLines.length > 0 && originalLines[originalLines.length - 1] === "") {
@@ -331,14 +333,16 @@ export function deriveNewContentsFromChunks(filePath: string, chunks: UpdateFile
331333
newLines.push("")
332334
}
333335

334-
const newContent = newLines.join("\n")
336+
const next = Bom.split(newLines.join("\n"))
337+
const newContent = next.text
335338

336339
// Generate unified diff
337-
const unifiedDiff = generateUnifiedDiff(originalContent, newContent)
340+
const unifiedDiff = generateUnifiedDiff(originalContent.text, newContent)
338341

339342
return {
340343
unified_diff: unifiedDiff,
341344
content: newContent,
345+
bom: originalContent.bom || next.bom,
342346
}
343347
}
344348

@@ -553,13 +557,13 @@ export async function applyHunksToFiles(hunks: Hunk[]): Promise<AffectedPaths> {
553557
await fs.mkdir(moveDir, { recursive: true })
554558
}
555559

556-
await fs.writeFile(hunk.move_path, fileUpdate.content, "utf-8")
560+
await fs.writeFile(hunk.move_path, Bom.join(fileUpdate.content, fileUpdate.bom), "utf-8")
557561
await fs.unlink(hunk.path)
558562
modified.push(hunk.move_path)
559563
log.info(`Moved file: ${hunk.path} -> ${hunk.move_path}`)
560564
} else {
561565
// Regular update
562-
await fs.writeFile(hunk.path, fileUpdate.content, "utf-8")
566+
await fs.writeFile(hunk.path, Bom.join(fileUpdate.content, fileUpdate.bom), "utf-8")
563567
modified.push(hunk.path)
564568
log.info(`Updated file: ${hunk.path}`)
565569
}

packages/opencode/src/tool/apply_patch.ts

Lines changed: 22 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ import { AppFileSystem } from "@opencode-ai/shared/filesystem"
1414
import DESCRIPTION from "./apply_patch.txt"
1515
import { File } from "../file"
1616
import { Format } from "../format"
17+
import * as Bom from "@/util/bom"
1718

1819
const PatchParams = z.object({
1920
patchText: z.string().describe("The full patch text that describes all changes to be made"),
@@ -59,6 +60,7 @@ export const ApplyPatchTool = Tool.define(
5960
diff: string
6061
additions: number
6162
deletions: number
63+
bom: boolean
6264
}> = []
6365

6466
let totalDiff = ""
@@ -72,23 +74,25 @@ export const ApplyPatchTool = Tool.define(
7274
const oldContent = ""
7375
const newContent =
7476
hunk.contents.length === 0 || hunk.contents.endsWith("\n") ? hunk.contents : `${hunk.contents}\n`
75-
const diff = trimDiff(createTwoFilesPatch(filePath, filePath, oldContent, newContent))
77+
const next = Bom.split(newContent)
78+
const diff = trimDiff(createTwoFilesPatch(filePath, filePath, oldContent, next.text))
7679

7780
let additions = 0
7881
let deletions = 0
79-
for (const change of diffLines(oldContent, newContent)) {
82+
for (const change of diffLines(oldContent, next.text)) {
8083
if (change.added) additions += change.count || 0
8184
if (change.removed) deletions += change.count || 0
8285
}
8386

8487
fileChanges.push({
8588
filePath,
8689
oldContent,
87-
newContent,
90+
newContent: next.text,
8891
type: "add",
8992
diff,
9093
additions,
9194
deletions,
95+
bom: next.bom,
9296
})
9397

9498
totalDiff += diff + "\n"
@@ -104,13 +108,16 @@ export const ApplyPatchTool = Tool.define(
104108
)
105109
}
106110

107-
const oldContent = yield* afs.readFileString(filePath)
111+
const source = yield* Bom.readFile(afs, filePath)
112+
const oldContent = source.text
108113
let newContent = oldContent
114+
let bom = source.bom
109115

110116
// Apply the update chunks to get new content
111117
try {
112118
const fileUpdate = Patch.deriveNewContentsFromChunks(filePath, hunk.chunks)
113119
newContent = fileUpdate.content
120+
bom = fileUpdate.bom
114121
} catch (error) {
115122
return yield* Effect.fail(new Error(`apply_patch verification failed: ${error}`))
116123
}
@@ -136,15 +143,16 @@ export const ApplyPatchTool = Tool.define(
136143
diff,
137144
additions,
138145
deletions,
146+
bom,
139147
})
140148

141149
totalDiff += diff + "\n"
142150
break
143151
}
144152

145153
case "delete": {
146-
const contentToDelete = yield* afs
147-
.readFileString(filePath)
154+
const source = yield* Bom
155+
.readFile(afs, filePath)
148156
.pipe(
149157
Effect.catch((error) =>
150158
Effect.fail(
@@ -154,6 +162,7 @@ export const ApplyPatchTool = Tool.define(
154162
),
155163
),
156164
)
165+
const contentToDelete = source.text
157166
const deleteDiff = trimDiff(createTwoFilesPatch(filePath, filePath, contentToDelete, ""))
158167

159168
const deletions = contentToDelete.split("\n").length
@@ -166,6 +175,7 @@ export const ApplyPatchTool = Tool.define(
166175
diff: deleteDiff,
167176
additions: 0,
168177
deletions,
178+
bom: source.bom,
169179
})
170180

171181
totalDiff += deleteDiff + "\n"
@@ -207,20 +217,20 @@ export const ApplyPatchTool = Tool.define(
207217
case "add":
208218
// Create parent directories (recursive: true is safe on existing/root dirs)
209219

210-
yield* afs.writeWithDirs(change.filePath, change.newContent)
220+
yield* afs.writeWithDirs(change.filePath, Bom.join(change.newContent, change.bom))
211221
updates.push({ file: change.filePath, event: "add" })
212222
break
213223

214224
case "update":
215-
yield* afs.writeWithDirs(change.filePath, change.newContent)
225+
yield* afs.writeWithDirs(change.filePath, Bom.join(change.newContent, change.bom))
216226
updates.push({ file: change.filePath, event: "change" })
217227
break
218228

219229
case "move":
220230
if (change.movePath) {
221231
// Create parent directories (recursive: true is safe on existing/root dirs)
222232

223-
yield* afs.writeWithDirs(change.movePath!, change.newContent)
233+
yield* afs.writeWithDirs(change.movePath!, Bom.join(change.newContent, change.bom))
224234
yield* afs.remove(change.filePath)
225235
updates.push({ file: change.filePath, event: "unlink" })
226236
updates.push({ file: change.movePath, event: "add" })
@@ -234,7 +244,9 @@ export const ApplyPatchTool = Tool.define(
234244
}
235245

236246
if (edited) {
237-
yield* format.file(edited)
247+
if (yield* format.file(edited)) {
248+
yield* Bom.syncFile(afs, edited, change.bom)
249+
}
238250
yield* bus.publish(File.Event.Edited, { file: edited })
239251
}
240252
}

packages/opencode/src/tool/edit.ts

Lines changed: 20 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ import { Instance } from "../project/instance"
1818
import { Snapshot } from "@/snapshot"
1919
import { assertExternalDirectoryEffect } from "./external-directory"
2020
import { AppFileSystem } from "@opencode-ai/shared/filesystem"
21+
import * as Bom from "@/util/bom"
2122

2223
function normalizeLineEndings(text: string): string {
2324
return text.replaceAll("\r\n", "\n")
@@ -84,7 +85,11 @@ export const EditTool = Tool.define(
8485
Effect.gen(function* () {
8586
if (params.oldString === "") {
8687
const existed = yield* afs.existsSafe(filePath)
87-
contentNew = params.newString
88+
const source = existed ? yield* Bom.readFile(afs, filePath) : { bom: false, text: "" }
89+
const next = Bom.split(params.newString)
90+
const desiredBom = source.bom || next.bom
91+
contentOld = source.text
92+
contentNew = next.text
8893
diff = trimDiff(createTwoFilesPatch(filePath, filePath, contentOld, contentNew))
8994
yield* ctx.ask({
9095
permission: "edit",
@@ -95,8 +100,10 @@ export const EditTool = Tool.define(
95100
diff,
96101
},
97102
})
98-
yield* afs.writeWithDirs(filePath, params.newString)
99-
yield* format.file(filePath)
103+
yield* afs.writeWithDirs(filePath, Bom.join(contentNew, desiredBom))
104+
if (yield* format.file(filePath)) {
105+
contentNew = yield* Bom.syncFile(afs, filePath, desiredBom)
106+
}
100107
yield* bus.publish(File.Event.Edited, { file: filePath })
101108
yield* bus.publish(FileWatcher.Event.Updated, {
102109
file: filePath,
@@ -108,13 +115,16 @@ export const EditTool = Tool.define(
108115
const info = yield* afs.stat(filePath).pipe(Effect.catch(() => Effect.succeed(undefined)))
109116
if (!info) throw new Error(`File ${filePath} not found`)
110117
if (info.type === "Directory") throw new Error(`Path is a directory, not a file: ${filePath}`)
111-
contentOld = yield* afs.readFileString(filePath)
118+
const source = yield* Bom.readFile(afs, filePath)
119+
contentOld = source.text
112120

113121
const ending = detectLineEnding(contentOld)
114122
const old = convertToLineEnding(normalizeLineEndings(params.oldString), ending)
115-
const next = convertToLineEnding(normalizeLineEndings(params.newString), ending)
123+
const replacement = convertToLineEnding(normalizeLineEndings(params.newString), ending)
116124

117-
contentNew = replace(contentOld, old, next, params.replaceAll)
125+
const next = Bom.split(replace(contentOld, old, replacement, params.replaceAll))
126+
const desiredBom = source.bom || next.bom
127+
contentNew = next.text
118128

119129
diff = trimDiff(
120130
createTwoFilesPatch(
@@ -134,14 +144,15 @@ export const EditTool = Tool.define(
134144
},
135145
})
136146

137-
yield* afs.writeWithDirs(filePath, contentNew)
138-
yield* format.file(filePath)
147+
yield* afs.writeWithDirs(filePath, Bom.join(contentNew, desiredBom))
148+
if (yield* format.file(filePath)) {
149+
contentNew = yield* Bom.syncFile(afs, filePath, desiredBom)
150+
}
139151
yield* bus.publish(File.Event.Edited, { file: filePath })
140152
yield* bus.publish(FileWatcher.Event.Updated, {
141153
file: filePath,
142154
event: "change",
143155
})
144-
contentNew = yield* afs.readFileString(filePath)
145156
diff = trimDiff(
146157
createTwoFilesPatch(
147158
filePath,

packages/opencode/src/tool/write.ts

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import { AppFileSystem } from "@opencode-ai/shared/filesystem"
1313
import { Instance } from "../project/instance"
1414
import { trimDiff } from "./edit"
1515
import { assertExternalDirectoryEffect } from "./external-directory"
16+
import * as Bom from "@/util/bom"
1617

1718
const MAX_PROJECT_DIAGNOSTICS_FILES = 5
1819

@@ -38,9 +39,13 @@ export const WriteTool = Tool.define(
3839
yield* assertExternalDirectoryEffect(ctx, filepath)
3940

4041
const exists = yield* fs.existsSafe(filepath)
41-
const contentOld = exists ? yield* fs.readFileString(filepath) : ""
42+
const source = exists ? yield* Bom.readFile(fs, filepath) : { bom: false, text: "" }
43+
const next = Bom.split(params.content)
44+
const desiredBom = source.bom || next.bom
45+
const contentOld = source.text
46+
const contentNew = next.text
4247

43-
const diff = trimDiff(createTwoFilesPatch(filepath, filepath, contentOld, params.content))
48+
const diff = trimDiff(createTwoFilesPatch(filepath, filepath, contentOld, contentNew))
4449
yield* ctx.ask({
4550
permission: "edit",
4651
patterns: [path.relative(Instance.worktree, filepath)],
@@ -51,8 +56,10 @@ export const WriteTool = Tool.define(
5156
},
5257
})
5358

54-
yield* fs.writeWithDirs(filepath, params.content)
55-
yield* format.file(filepath)
59+
yield* fs.writeWithDirs(filepath, Bom.join(contentNew, desiredBom))
60+
if (yield* format.file(filepath)) {
61+
yield* Bom.syncFile(fs, filepath, desiredBom)
62+
}
5663
yield* bus.publish(File.Event.Edited, { file: filepath })
5764
yield* bus.publish(FileWatcher.Event.Updated, {
5865
file: filepath,

packages/opencode/src/util/bom.ts

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
import { Effect } from "effect"
2+
import { AppFileSystem } from "@opencode-ai/shared/filesystem"
3+
4+
const BOM_CODE = 0xfeff
5+
const BOM = String.fromCharCode(BOM_CODE)
6+
7+
export function split(text: string) {
8+
if (text.charCodeAt(0) !== BOM_CODE) return { bom: false, text }
9+
return { bom: true, text: text.slice(1) }
10+
}
11+
12+
export function join(text: string, bom: boolean) {
13+
const stripped = split(text).text
14+
if (!bom) return stripped
15+
return BOM + stripped
16+
}
17+
18+
export const readFile = Effect.fn("Bom.readFile")(function* (fs: AppFileSystem.Interface, filePath: string) {
19+
return split(new TextDecoder("utf-8", { ignoreBOM: true }).decode(yield* fs.readFile(filePath)))
20+
})
21+
22+
export const syncFile = Effect.fn("Bom.syncFile")(function* (
23+
fs: AppFileSystem.Interface,
24+
filePath: string,
25+
bom: boolean,
26+
) {
27+
const current = yield* readFile(fs, filePath)
28+
if (current.bom === bom) return current.text
29+
yield* fs.writeWithDirs(filePath, join(current.text, bom))
30+
return current.text
31+
})

0 commit comments

Comments
 (0)