refactor: convert edit tool to Tool.defineEffect (#21904)

This commit is contained in:
Kit Langton
2026-04-10 17:10:28 -04:00
committed by GitHub
parent 57b2e64345
commit b41fa8e318
4 changed files with 240 additions and 184 deletions

View File

@@ -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<diagnostics file="${filePath}">\n${limited.map(LSP.Diagnostic.pretty).join("\n")}${suffix}\n</diagnostics>`
}
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<typeof Parameters>, 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<diagnostics file="${filePath}">\n${limited.map(LSP.Diagnostic.pretty).join("\n")}${suffix}\n</diagnostics>`
}
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<string, void, unknown>

View File

@@ -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),
}
},
})
}),
)

View File

@@ -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<State>(
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),

View File

@@ -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(