From b41fa8e318a6d67acd52dc746500d415f923afc6 Mon Sep 17 00:00:00 2001 From: Kit Langton Date: Fri, 10 Apr 2026 17:10:28 -0400 Subject: [PATCH] refactor: convert edit tool to Tool.defineEffect (#21904) --- packages/opencode/src/tool/edit.ts | 280 +++++++++++++---------- packages/opencode/src/tool/multiedit.ts | 85 ++++--- packages/opencode/src/tool/registry.ts | 3 +- packages/opencode/test/tool/edit.test.ts | 56 +++-- 4 files changed, 240 insertions(+), 184 deletions(-) diff --git a/packages/opencode/src/tool/edit.ts b/packages/opencode/src/tool/edit.ts index 9505dd9eab..8ead6c9eea 100644 --- a/packages/opencode/src/tool/edit.ts +++ b/packages/opencode/src/tool/edit.ts @@ -5,6 +5,7 @@ import z from "zod" import * as path from "path" +import { Effect } from "effect" import { Tool } from "./tool" import { LSP } from "../lsp" import { createTwoFilesPatch, diffLines } from "diff" @@ -17,7 +18,7 @@ import { FileTime } from "../file/time" import { Filesystem } from "../util/filesystem" import { Instance } from "../project/instance" import { Snapshot } from "@/snapshot" -import { assertExternalDirectory } from "./external-directory" +import { assertExternalDirectoryEffect } from "./external-directory" const MAX_DIAGNOSTICS_PER_FILE = 20 @@ -34,136 +35,161 @@ function convertToLineEnding(text: string, ending: "\n" | "\r\n"): string { return text.replaceAll("\n", "\r\n") } -export const EditTool = Tool.define("edit", { - description: DESCRIPTION, - parameters: z.object({ - filePath: z.string().describe("The absolute path to the file to modify"), - oldString: z.string().describe("The text to replace"), - newString: z.string().describe("The text to replace it with (must be different from oldString)"), - replaceAll: z.boolean().optional().describe("Replace all occurrences of oldString (default false)"), - }), - async execute(params, ctx) { - if (!params.filePath) { - throw new Error("filePath is required") - } +const Parameters = z.object({ + filePath: z.string().describe("The absolute path to the file to modify"), + oldString: z.string().describe("The text to replace"), + newString: z.string().describe("The text to replace it with (must be different from oldString)"), + replaceAll: z.boolean().optional().describe("Replace all occurrences of oldString (default false)"), +}) - if (params.oldString === params.newString) { - throw new Error("No changes to apply: oldString and newString are identical.") - } - - const filePath = path.isAbsolute(params.filePath) ? params.filePath : path.join(Instance.directory, params.filePath) - await assertExternalDirectory(ctx, filePath) - - let diff = "" - let contentOld = "" - let contentNew = "" - await FileTime.withLock(filePath, async () => { - if (params.oldString === "") { - const existed = await Filesystem.exists(filePath) - contentNew = params.newString - diff = trimDiff(createTwoFilesPatch(filePath, filePath, contentOld, contentNew)) - await ctx.ask({ - permission: "edit", - patterns: [path.relative(Instance.worktree, filePath)], - always: ["*"], - metadata: { - filepath: filePath, - diff, - }, - }) - await Filesystem.write(filePath, params.newString) - await Format.file(filePath) - Bus.publish(File.Event.Edited, { file: filePath }) - await Bus.publish(FileWatcher.Event.Updated, { - file: filePath, - event: existed ? "change" : "add", - }) - await FileTime.read(ctx.sessionID, filePath) - return - } - - const stats = Filesystem.stat(filePath) - if (!stats) throw new Error(`File ${filePath} not found`) - 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) - - 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)), - ) - await ctx.ask({ - permission: "edit", - patterns: [path.relative(Instance.worktree, filePath)], - always: ["*"], - metadata: { - filepath: filePath, - diff, - }, - }) - - await Filesystem.write(filePath, contentNew) - await Format.file(filePath) - Bus.publish(File.Event.Edited, { file: filePath }) - await Bus.publish(FileWatcher.Event.Updated, { - file: filePath, - event: "change", - }) - contentNew = await Filesystem.readText(filePath) - diff = trimDiff( - createTwoFilesPatch(filePath, filePath, normalizeLineEndings(contentOld), normalizeLineEndings(contentNew)), - ) - await FileTime.read(ctx.sessionID, filePath) - }) - - const filediff: Snapshot.FileDiff = { - file: filePath, - patch: diff, - additions: 0, - deletions: 0, - } - for (const change of diffLines(contentOld, contentNew)) { - if (change.added) filediff.additions += change.count || 0 - if (change.removed) filediff.deletions += change.count || 0 - } - - ctx.metadata({ - metadata: { - diff, - filediff, - diagnostics: {}, - }, - }) - - let output = "Edit applied successfully." - await LSP.touchFile(filePath, true) - const diagnostics = await LSP.diagnostics() - const normalizedFilePath = Filesystem.normalizePath(filePath) - const issues = diagnostics[normalizedFilePath] ?? [] - const errors = issues.filter((item) => item.severity === 1) - if (errors.length > 0) { - const limited = errors.slice(0, MAX_DIAGNOSTICS_PER_FILE) - const suffix = - errors.length > MAX_DIAGNOSTICS_PER_FILE ? `\n... and ${errors.length - MAX_DIAGNOSTICS_PER_FILE} more` : "" - output += `\n\nLSP errors detected in this file, please fix:\n\n${limited.map(LSP.Diagnostic.pretty).join("\n")}${suffix}\n` - } +export const EditTool = Tool.defineEffect( + "edit", + Effect.gen(function* () { + const lsp = yield* LSP.Service + const filetime = yield* FileTime.Service return { - metadata: { - diagnostics, - diff, - filediff, - }, - title: `${path.relative(Instance.worktree, filePath)}`, - output, + description: DESCRIPTION, + parameters: Parameters, + execute: (params: z.infer, ctx: Tool.Context) => + Effect.gen(function* () { + if (!params.filePath) { + throw new Error("filePath is required") + } + + if (params.oldString === params.newString) { + throw new Error("No changes to apply: oldString and newString are identical.") + } + + const filePath = path.isAbsolute(params.filePath) + ? params.filePath + : path.join(Instance.directory, params.filePath) + yield* assertExternalDirectoryEffect(ctx, filePath) + + let diff = "" + let contentOld = "" + let contentNew = "" + yield* filetime.withLock(filePath, async () => { + if (params.oldString === "") { + const existed = await Filesystem.exists(filePath) + contentNew = params.newString + diff = trimDiff(createTwoFilesPatch(filePath, filePath, contentOld, contentNew)) + await ctx.ask({ + permission: "edit", + patterns: [path.relative(Instance.worktree, filePath)], + always: ["*"], + metadata: { + filepath: filePath, + diff, + }, + }) + await Filesystem.write(filePath, params.newString) + await Format.file(filePath) + Bus.publish(File.Event.Edited, { file: filePath }) + await Bus.publish(FileWatcher.Event.Updated, { + file: filePath, + event: existed ? "change" : "add", + }) + await FileTime.read(ctx.sessionID, filePath) + return + } + + const stats = Filesystem.stat(filePath) + if (!stats) throw new Error(`File ${filePath} not found`) + 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) + + 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), + ), + ) + await ctx.ask({ + permission: "edit", + patterns: [path.relative(Instance.worktree, filePath)], + always: ["*"], + metadata: { + filepath: filePath, + diff, + }, + }) + + await Filesystem.write(filePath, contentNew) + await Format.file(filePath) + Bus.publish(File.Event.Edited, { file: filePath }) + await Bus.publish(FileWatcher.Event.Updated, { + file: filePath, + event: "change", + }) + contentNew = await Filesystem.readText(filePath) + diff = trimDiff( + createTwoFilesPatch( + filePath, + filePath, + normalizeLineEndings(contentOld), + normalizeLineEndings(contentNew), + ), + ) + await FileTime.read(ctx.sessionID, filePath) + }) + + const filediff: Snapshot.FileDiff = { + file: filePath, + patch: diff, + additions: 0, + deletions: 0, + } + for (const change of diffLines(contentOld, contentNew)) { + if (change.added) filediff.additions += change.count || 0 + if (change.removed) filediff.deletions += change.count || 0 + } + + ctx.metadata({ + metadata: { + diff, + filediff, + diagnostics: {}, + }, + }) + + let output = "Edit applied successfully." + yield* lsp.touchFile(filePath, true) + const diagnostics = yield* lsp.diagnostics() + const normalizedFilePath = Filesystem.normalizePath(filePath) + const issues = diagnostics[normalizedFilePath] ?? [] + const errors = issues.filter((item) => item.severity === 1) + if (errors.length > 0) { + const limited = errors.slice(0, MAX_DIAGNOSTICS_PER_FILE) + const suffix = + errors.length > MAX_DIAGNOSTICS_PER_FILE + ? `\n... and ${errors.length - MAX_DIAGNOSTICS_PER_FILE} more` + : "" + output += `\n\nLSP errors detected in this file, please fix:\n\n${limited.map(LSP.Diagnostic.pretty).join("\n")}${suffix}\n` + } + + return { + metadata: { + diagnostics, + diff, + filediff, + }, + title: `${path.relative(Instance.worktree, filePath)}`, + output, + } + }).pipe(Effect.orDie, Effect.runPromise), } - }, -}) + }), +) export type Replacer = (content: string, find: string) => Generator diff --git a/packages/opencode/src/tool/multiedit.ts b/packages/opencode/src/tool/multiedit.ts index 7f562f4737..5a52d0f0af 100644 --- a/packages/opencode/src/tool/multiedit.ts +++ b/packages/opencode/src/tool/multiedit.ts @@ -1,46 +1,57 @@ import z from "zod" +import { Effect } from "effect" import { Tool } from "./tool" import { EditTool } from "./edit" import DESCRIPTION from "./multiedit.txt" import path from "path" import { Instance } from "../project/instance" -export const MultiEditTool = Tool.define("multiedit", { - description: DESCRIPTION, - parameters: z.object({ - filePath: z.string().describe("The absolute path to the file to modify"), - edits: z - .array( - z.object({ - filePath: z.string().describe("The absolute path to the file to modify"), - oldString: z.string().describe("The text to replace"), - newString: z.string().describe("The text to replace it with (must be different from oldString)"), - replaceAll: z.boolean().optional().describe("Replace all occurrences of oldString (default false)"), - }), - ) - .describe("Array of edit operations to perform sequentially on the file"), - }), - async execute(params, ctx) { - const tool = await EditTool.init() - const results = [] - for (const [, edit] of params.edits.entries()) { - const result = await tool.execute( - { - filePath: params.filePath, - oldString: edit.oldString, - newString: edit.newString, - replaceAll: edit.replaceAll, - }, - ctx, - ) - results.push(result) - } +export const MultiEditTool = Tool.defineEffect( + "multiedit", + Effect.gen(function* () { + const editInfo = yield* EditTool + const edit = yield* Effect.promise(() => editInfo.init()) + return { - title: path.relative(Instance.worktree, params.filePath), - metadata: { - results: results.map((r) => r.metadata), - }, - output: results.at(-1)!.output, + description: DESCRIPTION, + parameters: z.object({ + filePath: z.string().describe("The absolute path to the file to modify"), + edits: z + .array( + z.object({ + filePath: z.string().describe("The absolute path to the file to modify"), + oldString: z.string().describe("The text to replace"), + newString: z.string().describe("The text to replace it with (must be different from oldString)"), + replaceAll: z.boolean().optional().describe("Replace all occurrences of oldString (default false)"), + }), + ) + .describe("Array of edit operations to perform sequentially on the file"), + }), + execute: (params: { filePath: string; edits: Array<{ filePath: string; oldString: string; newString: string; replaceAll?: boolean }> }, ctx: Tool.Context) => + Effect.gen(function* () { + const results = [] + for (const [, entry] of params.edits.entries()) { + const result = yield* Effect.promise(() => + edit.execute( + { + filePath: params.filePath, + oldString: entry.oldString, + newString: entry.newString, + replaceAll: entry.replaceAll, + }, + ctx, + ), + ) + results.push(result) + } + return { + title: path.relative(Instance.worktree, params.filePath), + metadata: { + results: results.map((r) => r.metadata), + }, + output: results.at(-1)!.output, + } + }).pipe(Effect.orDie, Effect.runPromise), } - }, -}) + }), +) diff --git a/packages/opencode/src/tool/registry.ts b/packages/opencode/src/tool/registry.ts index bd27802ff8..bbc154371c 100644 --- a/packages/opencode/src/tool/registry.ts +++ b/packages/opencode/src/tool/registry.ts @@ -111,6 +111,7 @@ export namespace ToolRegistry { const codesearch = yield* CodeSearchTool const globtool = yield* GlobTool const writetool = yield* WriteTool + const edit = yield* EditTool const state = yield* InstanceState.make( Effect.fn("ToolRegistry.state")(function* (ctx) { @@ -173,7 +174,7 @@ export namespace ToolRegistry { read: Tool.init(read), glob: Tool.init(globtool), grep: Tool.init(GrepTool), - edit: Tool.init(EditTool), + edit: Tool.init(edit), write: Tool.init(writetool), task: Tool.init(task), fetch: Tool.init(webfetch), diff --git a/packages/opencode/test/tool/edit.test.ts b/packages/opencode/test/tool/edit.test.ts index 96d41400e3..220fe299b3 100644 --- a/packages/opencode/test/tool/edit.test.ts +++ b/packages/opencode/test/tool/edit.test.ts @@ -1,10 +1,12 @@ -import { afterEach, describe, test, expect } from "bun:test" +import { afterAll, afterEach, describe, test, expect } from "bun:test" import path from "path" import fs from "fs/promises" +import { Effect, Layer, ManagedRuntime } from "effect" import { EditTool } from "../../src/tool/edit" import { Instance } from "../../src/project/instance" import { tmpdir } from "../fixture/fixture" import { FileTime } from "../../src/file/time" +import { LSP } from "../../src/lsp" import { SessionID, MessageID } from "../../src/session/schema" const ctx = { @@ -27,6 +29,22 @@ async function touch(file: string, time: number) { await fs.utimes(file, date, date) } +const runtime = ManagedRuntime.make( + Layer.mergeAll(LSP.defaultLayer, FileTime.defaultLayer), +) + +afterAll(async () => { + await runtime.dispose() +}) + +const resolve = () => + runtime.runPromise( + Effect.gen(function* () { + const info = yield* EditTool + return yield* Effect.promise(() => info.init()) + }), + ) + describe("tool.edit", () => { describe("creating new files", () => { test("creates new file when oldString is empty", async () => { @@ -36,7 +54,7 @@ describe("tool.edit", () => { await Instance.provide({ directory: tmp.path, fn: async () => { - const edit = await EditTool.init() + const edit = await resolve() const result = await edit.execute( { filePath: filepath, @@ -61,7 +79,7 @@ describe("tool.edit", () => { await Instance.provide({ directory: tmp.path, fn: async () => { - const edit = await EditTool.init() + const edit = await resolve() await edit.execute( { filePath: filepath, @@ -91,7 +109,7 @@ describe("tool.edit", () => { const events: string[] = [] const unsubUpdated = Bus.subscribe(FileWatcher.Event.Updated, () => events.push("updated")) - const edit = await EditTool.init() + const edit = await resolve() await edit.execute( { filePath: filepath, @@ -119,7 +137,7 @@ describe("tool.edit", () => { fn: async () => { await FileTime.read(ctx.sessionID, filepath) - const edit = await EditTool.init() + const edit = await resolve() const result = await edit.execute( { filePath: filepath, @@ -146,7 +164,7 @@ describe("tool.edit", () => { fn: async () => { await FileTime.read(ctx.sessionID, filepath) - const edit = await EditTool.init() + const edit = await resolve() await expect( edit.execute( { @@ -169,7 +187,7 @@ describe("tool.edit", () => { await Instance.provide({ directory: tmp.path, fn: async () => { - const edit = await EditTool.init() + const edit = await resolve() await expect( edit.execute( { @@ -194,7 +212,7 @@ describe("tool.edit", () => { fn: async () => { await FileTime.read(ctx.sessionID, filepath) - const edit = await EditTool.init() + const edit = await resolve() await expect( edit.execute( { @@ -217,7 +235,7 @@ describe("tool.edit", () => { await Instance.provide({ directory: tmp.path, fn: async () => { - const edit = await EditTool.init() + const edit = await resolve() await expect( edit.execute( { @@ -249,7 +267,7 @@ describe("tool.edit", () => { await touch(filepath, 2_000) // Try to edit with the new content - const edit = await EditTool.init() + const edit = await resolve() await expect( edit.execute( { @@ -274,7 +292,7 @@ describe("tool.edit", () => { fn: async () => { await FileTime.read(ctx.sessionID, filepath) - const edit = await EditTool.init() + const edit = await resolve() await edit.execute( { filePath: filepath, @@ -307,7 +325,7 @@ describe("tool.edit", () => { const events: string[] = [] const unsubUpdated = Bus.subscribe(FileWatcher.Event.Updated, () => events.push("updated")) - const edit = await EditTool.init() + const edit = await resolve() await edit.execute( { filePath: filepath, @@ -335,7 +353,7 @@ describe("tool.edit", () => { fn: async () => { await FileTime.read(ctx.sessionID, filepath) - const edit = await EditTool.init() + const edit = await resolve() await edit.execute( { filePath: filepath, @@ -361,7 +379,7 @@ describe("tool.edit", () => { fn: async () => { await FileTime.read(ctx.sessionID, filepath) - const edit = await EditTool.init() + const edit = await resolve() await edit.execute( { filePath: filepath, @@ -385,7 +403,7 @@ describe("tool.edit", () => { await Instance.provide({ directory: tmp.path, fn: async () => { - const edit = await EditTool.init() + const edit = await resolve() await expect( edit.execute( { @@ -410,7 +428,7 @@ describe("tool.edit", () => { fn: async () => { await FileTime.read(ctx.sessionID, dirpath) - const edit = await EditTool.init() + const edit = await resolve() await expect( edit.execute( { @@ -435,7 +453,7 @@ describe("tool.edit", () => { fn: async () => { await FileTime.read(ctx.sessionID, filepath) - const edit = await EditTool.init() + const edit = await resolve() const result = await edit.execute( { filePath: filepath, @@ -502,7 +520,7 @@ describe("tool.edit", () => { return await Instance.provide({ directory: tmp.path, fn: async () => { - const edit = await EditTool.init() + const edit = await resolve() const filePath = path.join(tmp.path, "test.txt") await FileTime.read(ctx.sessionID, filePath) await edit.execute( @@ -647,7 +665,7 @@ describe("tool.edit", () => { fn: async () => { await FileTime.read(ctx.sessionID, filepath) - const edit = await EditTool.init() + const edit = await resolve() // Two concurrent edits const promise1 = edit.execute(