feat(core): improve edit tool output (#39211)

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 = <A, E, R>(effect: Effect.Effect<A, E, R>) =>
-                  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 }),
+                  ),
                 )
               },
             }),

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