From 79630149350b2309388a090d83db9004b3103b5b Mon Sep 17 00:00:00 2001 From: Janni Turunen Date: Mon, 23 Feb 2026 18:48:15 +0200 Subject: [PATCH] fix(hashline): enforce hashline_read before hashline_edit (#321) --- packages/opencode/src/file/time.ts | 29 ++++ packages/opencode/src/tool/hashline_edit.ts | 2 +- packages/opencode/src/tool/hashline_read.ts | 1 + .../opencode/test/tool/hashline_edit.test.ts | 152 ++++++++++++++++-- 4 files changed, 173 insertions(+), 11 deletions(-) diff --git a/packages/opencode/src/file/time.ts b/packages/opencode/src/file/time.ts index 35c780fbdd5..ab1e56f583e 100644 --- a/packages/opencode/src/file/time.ts +++ b/packages/opencode/src/file/time.ts @@ -14,9 +14,15 @@ export namespace FileTime { [path: string]: Date | undefined } } = {} + const hashlineRead: { + [sessionID: string]: { + [path: string]: Date | undefined + } + } = {} const locks = new Map>() return { read, + hashlineRead, locks, } }) @@ -28,6 +34,13 @@ export namespace FileTime { read[sessionID][file] = new Date() } + export function hashlineRead(sessionID: string, file: string) { + log.info("hashlineRead", { sessionID, file }) + const { hashlineRead: map } = state() + map[sessionID] = map[sessionID] || {} + map[sessionID][file] = new Date() + } + export function get(sessionID: string, file: string) { return state().read[sessionID]?.[file] } @@ -66,4 +79,20 @@ export namespace FileTime { ) } } + + export async function assertHashlineRead(sessionID: string, filepath: string) { + if (Flag.OPENCODE_DISABLE_FILETIME_CHECK === true) return + + const { hashlineRead: map } = state() + const time = map[sessionID]?.[filepath] + if (!time) + throw new Error( + `You must use hashline_read before hashline_edit on this file. The regular read tool does not provide hashline anchors.` + ) + const stats = await Bun.file(filepath).stat() + if (stats.mtime.getTime() > time.getTime()) + throw new Error( + `File ${filepath} has been modified since it was last read with hashline_read.\nLast modification: ${stats.mtime.toISOString()}\nLast read: ${time.toISOString()}\n\nPlease use hashline_read again before modifying it.` + ) + } } diff --git a/packages/opencode/src/tool/hashline_edit.ts b/packages/opencode/src/tool/hashline_edit.ts index 9fe4681e22d..839ee5db62d 100644 --- a/packages/opencode/src/tool/hashline_edit.ts +++ b/packages/opencode/src/tool/hashline_edit.ts @@ -69,7 +69,7 @@ export const HashlineEditTool = Tool.define("hashline_edit", { const stats = await file.stat().catch(() => {}) if (!stats) throw new Error(`File not found: ${filepath}`) if (stats.isDirectory()) throw new Error(`Path is a directory: ${filepath}`) - await FileTime.assert(ctx.sessionID, filepath) + await FileTime.assertHashlineRead(ctx.sessionID, filepath) const contentOld = await file.text() const contentNew = applyHashlineEdits(contentOld, parsedEdits) diff --git a/packages/opencode/src/tool/hashline_read.ts b/packages/opencode/src/tool/hashline_read.ts index 4f357fd1033..beece02d25d 100644 --- a/packages/opencode/src/tool/hashline_read.ts +++ b/packages/opencode/src/tool/hashline_read.ts @@ -121,6 +121,7 @@ export const HashlineReadTool = Tool.define("hashline_read", { LSP.touchFile(filepath, false) FileTime.read(ctx.sessionID, filepath) + FileTime.hashlineRead(ctx.sessionID, filepath) if (instructions.length > 0) { output += `\n\n\n${instructions.map((i) => i.content).join("\n\n")}\n` diff --git a/packages/opencode/test/tool/hashline_edit.test.ts b/packages/opencode/test/tool/hashline_edit.test.ts index 5357581cdc1..2c0958f49df 100644 --- a/packages/opencode/test/tool/hashline_edit.test.ts +++ b/packages/opencode/test/tool/hashline_edit.test.ts @@ -28,7 +28,7 @@ describe("tool.hashline_edit", () => { directory: tmp.path, fn: async () => { const tool = await HashlineEditTool.init() - FileTime.read(ctx.sessionID, path.join(tmp.path, "test.txt")) + FileTime.hashlineRead(ctx.sessionID, path.join(tmp.path, "test.txt")) const result = await tool.execute( { filePath: path.join(tmp.path, "test.txt"), @@ -53,7 +53,7 @@ describe("tool.hashline_edit", () => { directory: tmp.path, fn: async () => { const tool = await HashlineEditTool.init() - FileTime.read(ctx.sessionID, path.join(tmp.path, "test.txt")) + FileTime.hashlineRead(ctx.sessionID, path.join(tmp.path, "test.txt")) const result = await tool.execute( { filePath: path.join(tmp.path, "test.txt"), @@ -78,7 +78,7 @@ describe("tool.hashline_edit", () => { directory: tmp.path, fn: async () => { const tool = await HashlineEditTool.init() - FileTime.read(ctx.sessionID, path.join(tmp.path, "test.txt")) + FileTime.hashlineRead(ctx.sessionID, path.join(tmp.path, "test.txt")) const result = await tool.execute( { filePath: path.join(tmp.path, "test.txt"), @@ -110,7 +110,7 @@ describe("tool.hashline_edit", () => { directory: tmp.path, fn: async () => { const tool = await HashlineEditTool.init() - FileTime.read(ctx.sessionID, path.join(tmp.path, "test.txt")) + FileTime.hashlineRead(ctx.sessionID, path.join(tmp.path, "test.txt")) const result = await tool.execute( { filePath: path.join(tmp.path, "test.txt"), @@ -142,7 +142,7 @@ describe("tool.hashline_edit", () => { directory: tmp.path, fn: async () => { const tool = await HashlineEditTool.init() - FileTime.read(ctx.sessionID, path.join(tmp.path, "test.txt")) + FileTime.hashlineRead(ctx.sessionID, path.join(tmp.path, "test.txt")) const result = await tool.execute( { filePath: path.join(tmp.path, "test.txt"), @@ -173,7 +173,7 @@ describe("tool.hashline_edit", () => { directory: tmp.path, fn: async () => { const tool = await HashlineEditTool.init() - FileTime.read(ctx.sessionID, path.join(tmp.path, "test.txt")) + FileTime.hashlineRead(ctx.sessionID, path.join(tmp.path, "test.txt")) const result = await tool.execute( { filePath: path.join(tmp.path, "test.txt"), @@ -199,7 +199,7 @@ describe("tool.hashline_edit", () => { directory: tmp.path, fn: async () => { const tool = await HashlineEditTool.init() - FileTime.read(ctx.sessionID, path.join(tmp.path, "test.txt")) + FileTime.hashlineRead(ctx.sessionID, path.join(tmp.path, "test.txt")) const result = await tool.execute( { filePath: path.join(tmp.path, "test.txt"), @@ -224,7 +224,7 @@ describe("tool.hashline_edit", () => { directory: tmp.path, fn: async () => { const tool = await HashlineEditTool.init() - FileTime.read(ctx.sessionID, path.join(tmp.path, "test.txt")) + FileTime.hashlineRead(ctx.sessionID, path.join(tmp.path, "test.txt")) const result = await tool.execute( { filePath: path.join(tmp.path, "test.txt"), @@ -252,7 +252,7 @@ describe("tool.hashline_edit", () => { directory: tmp.path, fn: async () => { const tool = await HashlineEditTool.init() - FileTime.read(ctx.sessionID, path.join(tmp.path, "test.txt")) + FileTime.hashlineRead(ctx.sessionID, path.join(tmp.path, "test.txt")) const result = await tool.execute( { filePath: path.join(tmp.path, "test.txt"), @@ -281,4 +281,136 @@ describe("tool.hashline_edit", () => { delete process.env.OPENCODE_EXPERIMENTAL_HASHLINE } }) -}) \ No newline at end of file + + test("hashline_edit fails if called after regular read instead of hashline_read", async () => { + await using tmp = await tmpdir({ + init: async (dir) => { + await Bun.write(path.join(dir, "test.txt"), "line1\nline2\nline3") + }, + }) + await Instance.provide({ + directory: tmp.path, + fn: async () => { + const tool = await HashlineEditTool.init() + // Use regular read, NOT hashline_read + FileTime.read(ctx.sessionID, path.join(tmp.path, "test.txt")) + const result = await tool.execute( + { + filePath: path.join(tmp.path, "test.txt"), + edits: [{ op: "set_line", anchor: "2咲", new_text: "new line 2" }], + }, + ctx + ).catch((e) => e) + expect(result.message).toContain("You must use hashline_read before hashline_edit") + expect(result.message).toContain("The regular read tool does not provide hashline anchors") + const content = await Bun.file(path.join(tmp.path, "test.txt")).text() + expect(content).toBe("line1\nline2\nline3") + }, + }) + }) + + test("hashline_edit succeeds after hashline_read but fails after only regular read", async () => { + await using tmp = await tmpdir({ + init: async (dir) => { + await Bun.write(path.join(dir, "file1.txt"), "line1\nline2\nline3") + await Bun.write(path.join(dir, "file2.txt"), "line1\nline2\nline3") + }, + }) + await Instance.provide({ + directory: tmp.path, + fn: async () => { + const tool = await HashlineEditTool.init() + const filepath1 = path.join(tmp.path, "file1.txt") + const filepath2 = path.join(tmp.path, "file2.txt") + + // file1: use hashline_read - should succeed + FileTime.hashlineRead(ctx.sessionID, filepath1) + const result1 = await tool.execute( + { + filePath: filepath1, + edits: [{ op: "set_line", anchor: "2咲", new_text: "modified" }], + }, + ctx + ) + expect(result1.output).toContain("Edit applied successfully") + + // file2: use only regular read, NOT hashline_read - should fail + FileTime.read(ctx.sessionID, filepath2) + const result2 = await tool.execute( + { + filePath: filepath2, + edits: [{ op: "set_line", anchor: "2咲", new_text: "modified" }], + }, + ctx + ).catch((e) => e) + expect(result2.message).toContain("You must use hashline_read before hashline_edit") + expect(result2.message).toContain("The regular read tool does not provide hashline anchors") + }, + }) + }) + + test("hashline_edit fails when file modified on disk after hashline_read", async () => { + await using tmp = await tmpdir({ + init: async (dir) => { + await Bun.write(path.join(dir, "test.txt"), "line1\nline2\nline3") + }, + }) + await Instance.provide({ + directory: tmp.path, + fn: async () => { + const tool = await HashlineEditTool.init() + const filepath = path.join(tmp.path, "test.txt") + + // hashline_read the file + FileTime.hashlineRead(ctx.sessionID, filepath) + + // Modify file externally to trigger staleness error + await new Promise((r) => setTimeout(r, 10)) // Small delay to ensure mtime changes + await Bun.write(filepath, "line1\nmodified\nline3") + + const result = await tool.execute( + { + filePath: filepath, + edits: [{ op: "set_line", anchor: "2咲", new_text: "new" }], + }, + ctx + ).catch((e) => e) + expect(result.message).toContain("File") + expect(result.message).toContain("has been modified since it was last read with hashline_read") + }, + }) + }) + + test("hashline_edit rejects edit from different session even if another session used hashline_read", async () => { + await using tmp = await tmpdir({ + init: async (dir) => { + await Bun.write(path.join(dir, "test.txt"), "line1\nline2\nline3") + }, + }) + await Instance.provide({ + directory: tmp.path, + fn: async () => { + const tool = await HashlineEditTool.init() + const filepath = path.join(tmp.path, "test.txt") + + // Session A reads with hashline_read + FileTime.hashlineRead("session-a", filepath) + + // Session B (different sessionID) tries to edit — should fail + const ctxB = { ...ctx, sessionID: "session-b" } + const result = await tool.execute( + { + filePath: filepath, + edits: [{ op: "set_line", anchor: "2咲", new_text: "modified" }], + }, + ctxB + ).catch((e) => e) + expect(result.message).toContain("You must use hashline_read before hashline_edit") + + // File should be unchanged + const content = await Bun.file(filepath).text() + expect(content).toBe("line1\nline2\nline3") + }, + }) + }) +})