diff --git a/packages/core/src/tool/plugin/edit.ts b/packages/core/src/tool/plugin/edit.ts index c55eb2e72b6..19123d7ea2c 100644 --- a/packages/core/src/tool/plugin/edit.ts +++ b/packages/core/src/tool/plugin/edit.ts @@ -60,23 +60,6 @@ const countOccurrences = (content: string, search: string) => { return count } -const previewLines = (value: string, prefix: "+" | "-") => { - const lines = normalizeLineEndings(value).split("\n") - const shown = lines.slice(0, 6).map((line) => `${prefix}${line.length > 240 ? `${line.slice(0, 240)}...` : line}`) - if (lines.length > shown.length) shown.push(`${prefix}...`) - return shown -} - -export const toModelOutput = (output: Output, oldString: string, newString: string) => - [ - `Edited file successfully: ${output.files[0]?.file}`, - `Replacements: ${output.replacements}`, - "```diff", - ...previewLines(oldString, "-"), - ...previewLines(newString, "+"), - "```", - ].join("\n") - /** Deferred edit behavior and UX integrations remain visible at the model-facing seam. */ // TODO: Port V1 fuzzy correction strategies only after exact-edit behavior is established: line-trimmed matching, block-anchor fallback, indentation correction, and similarity-threshold review. // TODO: Add formatter integration after formatter runtime exists. @@ -103,11 +86,6 @@ export const Plugin = { input: Input, output: Output, execute: (input, context) => { - const unableToEdit = (effect: Effect.Effect) => - effect.pipe( - Effect.mapError((error) => new ToolFailure({ message: `Unable to edit ${input.path}`, error })), - ) - return Effect.gen(function* () { const permissionSource = { type: "tool" as const, @@ -125,44 +103,46 @@ export const Plugin = { }) } - const target = yield* unableToEdit(mutation.resolve({ path: input.path, kind: "file" })) + const target = yield* mutation.resolve({ path: input.path, kind: "file" }) const external = target.externalDirectory if (external) { - yield* unableToEdit( - permission.assert({ - ...LocationMutation.externalDirectoryPermission(external), - sessionID: context.sessionID, - agent: context.agent, - source: permissionSource, - }), - ) - } - - yield* unableToEdit( - permission.assert({ - action: "edit", - resources: [target.resource], - save: ["*"], + yield* permission.assert({ + ...LocationMutation.externalDirectoryPermission(external), sessionID: context.sessionID, agent: context.agent, source: permissionSource, - }), + }) + } + + yield* permission.assert({ + action: "edit", + resources: [target.resource], + save: ["*"], + sessionID: context.sessionID, + agent: context.agent, + source: permissionSource, + }) + const info = yield* fs.stat(target.canonical).pipe( + Effect.catchReason("PlatformError", "NotFound", () => + Effect.fail(new ToolFailure({ message: `File not found: ${input.path}` })), + ), ) - const source = decodeUtf8(yield* unableToEdit(fs.readFile(target.canonical))) + if (info.type === "Directory") { + return yield* new ToolFailure({ message: `Path is a directory, not a file: ${input.path}` }) + } + const source = decodeUtf8(yield* fs.readFile(target.canonical)) const ending = detectLineEnding(source.text) const oldString = convertToLineEnding(input.oldString, ending) const newString = convertToLineEnding(input.newString, ending) const replacements = countOccurrences(source.text, oldString) if (replacements === 0) { return yield* new ToolFailure({ - message: - "Could not find oldString in the file. It must match exactly, including whitespace and indentation.", + message: `Could not find oldString in ${input.path}. It must match exactly, including whitespace and indentation.`, }) } if (replacements > 1 && input.replaceAll !== true) { return yield* new ToolFailure({ - message: - "Found multiple exact matches for oldString. Provide more surrounding context or set replaceAll to true.", + message: `Found ${replacements} matches for oldString, but expected exactly one. Add more surrounding context to make oldString unique, or set replaceAll to true to replace every occurrence.`, }) } @@ -178,12 +158,10 @@ export const Plugin = { { additions: 0, deletions: 0 }, ) const next = splitBom(replaced) - const result = yield* unableToEdit( - files.write({ - target, - content: joinBom(next.text, source.bom || next.bom), - }), - ) + const result = yield* files.write({ + target, + content: joinBom(next.text, source.bom || next.bom), + }) return { files: [ { @@ -198,9 +176,14 @@ export const Plugin = { }).pipe( Effect.map((output) => ({ output, - content: toModelOutput(output, input.oldString, input.newString), + content: `Edited ${output.files[0]?.file} (${output.replacements} replacement${output.replacements === 1 ? "" : "s"})`, metadata: { files: output.files }, })), + Effect.mapError((error) => + error instanceof ToolFailure + ? error + : new ToolFailure({ message: `Unable to edit ${input.path}`, error }), + ), ) }, }), diff --git a/packages/core/test/tool-edit.test.ts b/packages/core/test/tool-edit.test.ts index 97c3578839b..8116cb64a59 100644 --- a/packages/core/test/tool-edit.test.ts +++ b/packages/core/test/tool-edit.test.ts @@ -150,7 +150,7 @@ describe("EditTool", () => { expect(settled.content).toEqual([ { type: "text", - text: "Edited file successfully: hello.txt\nReplacements: 1\n```diff\n-before\n+after\n```", + text: "Edited hello.txt (1 replacement)", }, ]) // Compact UI metadata carries the file diffs the TUI renders. @@ -385,7 +385,7 @@ describe("EditTool", () => { error: { type: "tool.execution", message: - "Could not find oldString in the file. It must match exactly, including whitespace and indentation.", + "Could not find oldString in matches.txt. It must match exactly, including whitespace and indentation.", }, }) expect( @@ -395,7 +395,7 @@ describe("EditTool", () => { error: { type: "tool.execution", message: - "Found multiple exact matches for oldString. Provide more surrounding context or set replaceAll to true.", + "Found 2 matches for oldString, but expected exactly one. Add more surrounding context to make oldString unique, or set replaceAll to true to replace every occurrence.", }, }) expect(writes).toEqual([]) @@ -408,6 +408,41 @@ describe("EditTool", () => { ), ) + it.live("returns specific missing file and directory errors", () => + Effect.acquireUseRelease( + Effect.promise(() => tmpdir()), + (tmp) => { + reset() + const directory = path.join(tmp.path, "src") + return Effect.promise(() => fs.mkdir(directory)).pipe( + Effect.andThen( + withTool(tmp.path, (registry) => + Effect.gen(function* () { + expect( + yield* executeTool( + registry, + call({ path: "missing.ts", oldString: "before", newString: "after" }), + ), + ).toEqual({ + status: "error", + error: { type: "tool.execution", message: "File not found: missing.ts" }, + }) + expect( + yield* executeTool(registry, call({ path: "src", oldString: "before", newString: "after" })), + ).toEqual({ + status: "error", + error: { type: "tool.execution", message: "Path is a directory, not a file: src" }, + }) + expect(writes).toEqual([]) + }), + ), + ), + ) + }, + (tmp) => Effect.promise(() => tmp[Symbol.asyncDispose]()), + ), + ) + it.live("replaces every exact occurrence when replaceAll is true", () => Effect.acquireUseRelease( Effect.promise(() => tmpdir()), @@ -425,6 +460,7 @@ describe("EditTool", () => { expect(settled.status).toBe("completed") if (settled.status !== "completed") return expect(settled.output).toMatchObject({ replacements: 3 }) + expect(settled.content).toEqual([{ type: "text", text: "Edited all.txt (3 replacements)" }]) expect(yield* Effect.promise(() => fs.readFile(target, "utf8"))).toBe("after after after") expect(writes).toHaveLength(1) }),