From d70394458353eba8c43edb126762dc9da8da3e51 Mon Sep 17 00:00:00 2001 From: spoons-and-mirrors <212802214+spoons-and-mirrors@users.noreply.github.com> Date: Sun, 16 Nov 2025 21:31:36 +0100 Subject: [PATCH 1/3] fix(edit): add per-file lock to prevent read-before-write race --- packages/opencode/src/file/time.ts | 29 +++++++++++++++++++++++++++++ packages/opencode/src/tool/edit.ts | 8 ++++---- 2 files changed, 33 insertions(+), 4 deletions(-) diff --git a/packages/opencode/src/file/time.ts b/packages/opencode/src/file/time.ts index 5cba5e82067..4e4bbb46a32 100644 --- a/packages/opencode/src/file/time.ts +++ b/packages/opencode/src/file/time.ts @@ -3,14 +3,20 @@ import { Log } from "../util/log" export namespace FileTime { const log = Log.create({ service: "file.time" }) + // Per-session read times plus per-file write locks. + // All tools that overwrite existing files should run their + // assert/read/write/update sequence inside withLock(filepath, ...) + // so concurrent writes to the same file are serialized. export const state = Instance.state(() => { const read: { [sessionID: string]: { [path: string]: Date | undefined } } = {} + const locks = new Map>() return { read, + locks, } }) @@ -25,6 +31,29 @@ export namespace FileTime { return state().read[sessionID]?.[file] } + export async function withLock(filepath: string, fn: () => Promise): Promise { + const current = state() + const currentLock = current.locks.get(filepath) ?? Promise.resolve() + let release: () => void = () => {} + const nextLock = new Promise((resolve) => { + release = resolve + }) + current.locks.set( + filepath, + currentLock.then(() => nextLock), + ) + await currentLock + try { + const result = await fn() + return result + } finally { + release() + if (current.locks.get(filepath) === nextLock) { + current.locks.delete(filepath) + } + } + } + export async function assert(sessionID: string, filepath: string) { const time = get(sessionID, filepath) if (!time) throw new Error(`You must read the file ${filepath} before overwriting it. Use the Read tool first`) diff --git a/packages/opencode/src/tool/edit.ts b/packages/opencode/src/tool/edit.ts index a5d34c949ff..0659236e982 100644 --- a/packages/opencode/src/tool/edit.ts +++ b/packages/opencode/src/tool/edit.ts @@ -74,7 +74,7 @@ export const EditTool = Tool.define("edit", { let diff = "" let contentOld = "" let contentNew = "" - await (async () => { + await FileTime.withLock(filePath, async () => { if (params.oldString === "") { contentNew = params.newString diff = trimDiff(createTwoFilesPatch(filePath, filePath, contentOld, contentNew)) @@ -95,6 +95,7 @@ export const EditTool = Tool.define("edit", { await Bus.publish(File.Event.Edited, { file: filePath, }) + FileTime.read(ctx.sessionID, filePath) return } @@ -131,9 +132,8 @@ export const EditTool = Tool.define("edit", { diff = trimDiff( createTwoFilesPatch(filePath, filePath, normalizeLineEndings(contentOld), normalizeLineEndings(contentNew)), ) - })() - - FileTime.read(ctx.sessionID, filePath) + FileTime.read(ctx.sessionID, filePath) + }) let output = "" await LSP.touchFile(filePath, true) From 190ded858c52e738bee9193d59f459f9d35a9532 Mon Sep 17 00:00:00 2001 From: spoons-and-mirrors <212802214+spoons-and-mirrors@users.noreply.github.com> Date: Sat, 13 Dec 2025 22:49:34 +0100 Subject: [PATCH 2/3] fix: lock entries accumation --- packages/opencode/src/file/time.ts | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/packages/opencode/src/file/time.ts b/packages/opencode/src/file/time.ts index 4e4bbb46a32..17eaaa58eb5 100644 --- a/packages/opencode/src/file/time.ts +++ b/packages/opencode/src/file/time.ts @@ -38,17 +38,15 @@ export namespace FileTime { const nextLock = new Promise((resolve) => { release = resolve }) - current.locks.set( - filepath, - currentLock.then(() => nextLock), - ) + const chained = currentLock.then(() => nextLock) + current.locks.set(filepath, chained) await currentLock try { const result = await fn() return result } finally { release() - if (current.locks.get(filepath) === nextLock) { + if (current.locks.get(filepath) === chained) { current.locks.delete(filepath) } } From 32e88df61b61fc3eee039f7ecc51de2854134065 Mon Sep 17 00:00:00 2001 From: spoons-and-mirrors <212802214+spoons-and-mirrors@users.noreply.github.com> Date: Sun, 14 Dec 2025 14:22:49 +0100 Subject: [PATCH 3/3] simplified to return await fn(() --- packages/opencode/src/file/time.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/packages/opencode/src/file/time.ts b/packages/opencode/src/file/time.ts index 17eaaa58eb5..770427abe96 100644 --- a/packages/opencode/src/file/time.ts +++ b/packages/opencode/src/file/time.ts @@ -42,8 +42,7 @@ export namespace FileTime { current.locks.set(filepath, chained) await currentLock try { - const result = await fn() - return result + return await fn() } finally { release() if (current.locks.get(filepath) === chained) {