Przeglądaj źródła

fix(core): add mutation permission previews (#39578)

Aiden Cline 1 tydzień temu
rodzic
commit
0ece10af43

+ 37 - 31
packages/core/src/tool/plugin/edit.ts

@@ -153,14 +153,6 @@ export const Plugin = {
                     })
                   }
 
-                  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}` })),
@@ -184,6 +176,26 @@ export const Plugin = {
                       : findLineOccurrences(source, oldString)
                   const matches = exact.length > 0 ? exact : unicode.length > 0 ? unicode : trailing
                   const replacements = matches.length
+                  const replaced = (input.replaceAll === true ? matches : matches.slice(0, 1))
+                    .toReversed()
+                    .reduce(
+                      (content, match) =>
+                        `${content.slice(0, match.start)}${newString}${content.slice(match.end)}`,
+                      source,
+                    )
+                  const preview =
+                    replacements > 0 && (replacements === 1 || input.replaceAll === true)
+                      ? fileDiff(target.resource, source, replaced)
+                      : undefined
+                  yield* permission.assert({
+                    action: "edit",
+                    resources: [target.resource],
+                    save: ["*"],
+                    metadata: preview ? { files: [preview] } : undefined,
+                    sessionID: context.sessionID,
+                    agent: context.agent,
+                    source: permissionSource,
+                  })
                   if (replacements === 0) {
                     return yield* new ToolFailure({
                       message: `Could not find oldString in ${input.path}. It must match exactly, including whitespace and indentation.`,
@@ -194,14 +206,6 @@ export const Plugin = {
                       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.`,
                     })
                   }
-
-                  const replaced = (input.replaceAll === true ? matches : matches.slice(0, 1))
-                    .toReversed()
-                    .reduce(
-                      (content, match) =>
-                        `${content.slice(0, match.start)}${newString}${content.slice(match.end)}`,
-                      source,
-                    )
                   const replacementBom = replaced.startsWith("\uFEFF")
                   const result = yield* files.write({
                     target,
@@ -211,22 +215,8 @@ export const Plugin = {
                   const formatted = (yield* formatter.file(target.canonical))
                     ? yield* Bom.syncFile(fs, target.canonical, bom)
                     : (yield* Bom.readFile(fs, target.canonical)).text
-                  const counts = diffLines(source, formatted).reduce(
-                    (result, item) => ({
-                      additions: result.additions + (item.added ? (item.count ?? 0) : 0),
-                      deletions: result.deletions + (item.removed ? (item.count ?? 0) : 0),
-                    }),
-                    { additions: 0, deletions: 0 },
-                  )
                   return {
-                    files: [
-                      {
-                        file: result.resource,
-                        patch: createTwoFilesPatch(result.resource, result.resource, source, formatted),
-                        status: "modified" as const,
-                        ...counts,
-                      },
-                    ],
+                    files: [fileDiff(result.resource, source, formatted)],
                     replacements,
                   } satisfies Output
                 }).pipe(
@@ -248,3 +238,19 @@ export const Plugin = {
       .pipe(Effect.orDie)
   }),
 }
+
+function fileDiff(file: string, before: string, after: string): typeof FileDiff.Info.Type {
+  const counts = diffLines(before, after).reduce(
+    (result, item) => ({
+      additions: result.additions + (item.added ? (item.count ?? 0) : 0),
+      deletions: result.deletions + (item.removed ? (item.count ?? 0) : 0),
+    }),
+    { additions: 0, deletions: 0 },
+  )
+  return {
+    file,
+    patch: createTwoFilesPatch(file, file, before, after),
+    status: "modified",
+    ...counts,
+  }
+}

+ 22 - 3
packages/core/src/tool/plugin/write.ts

@@ -8,7 +8,9 @@ export * as WriteTool from "./write"
 
 import type { Context as PluginContext } from "@opencode-ai/plugin/effect/plugin"
 import { ToolFailure } from "@opencode-ai/ai"
+import { FileDiff } from "@opencode-ai/schema/file-diff"
 import { Effect, Schema } from "effect"
+import { createTwoFilesPatch, diffLines } from "diff"
 import { Bom } from "@opencode-ai/util/bom"
 import { FSUtil } from "@opencode-ai/util/fs-util"
 import { FileMutation } from "../../file-mutation"
@@ -21,8 +23,7 @@ export const name = "write"
 // TODO: Revisit whether model-facing mutation schemas should prefer absolute `filePath` naming for trained-in compatibility after evaluating model behavior.
 export const Input = Schema.Struct({
   path: Schema.String.annotate({
-    description:
-      "File path to write. Relative paths resolve within the active Location. Absolute paths inside that Location are accepted; external absolute paths require external_directory approval.",
+    description: "Path to the file to write to",
   }),
   content: Schema.String.annotate({ description: "Content to write to the file" }),
 })
@@ -59,7 +60,7 @@ export const Plugin = {
               name,
               options: { codemode: false, permission: "edit" },
               description:
-                "Write content to one file. Relative paths resolve within the active Location. Absolute paths inside the Location are accepted. Explicit external absolute paths require external_directory approval before edit approval.",
+                "Writes a file to the local filesystem, overwriting if one exists.\n\nMissing parent directories are created automatically.\n\nUse this tool to create new files or overwrite existing files. For partial changes, use the edit tool instead.",
               input: Input,
               output: Output,
               execute: (input, context) =>
@@ -78,10 +79,28 @@ export const Plugin = {
                       agent: context.agent,
                       source,
                     })
+                  const current = yield* Bom.readFile(fs, target.canonical).pipe(
+                    Effect.catchReason("PlatformError", "NotFound", () => Effect.succeed(undefined)),
+                  )
+                  const next = Bom.split(input.content)
+                  const counts = diffLines(current?.text ?? "", next.text).reduce(
+                    (result, item) => ({
+                      additions: result.additions + (item.added ? (item.count ?? 0) : 0),
+                      deletions: result.deletions + (item.removed ? (item.count ?? 0) : 0),
+                    }),
+                    { additions: 0, deletions: 0 },
+                  )
+                  const preview: typeof FileDiff.Info.Type = {
+                    file: target.resource,
+                    patch: createTwoFilesPatch(target.resource, target.resource, current?.text ?? "", next.text),
+                    status: current ? "modified" : "added",
+                    ...counts,
+                  }
                   yield* permission.assert({
                     action: "edit",
                     resources: [target.resource],
                     save: ["*"],
+                    metadata: { files: [preview] },
                     sessionID: context.sessionID,
                     agent: context.agent,
                     source,

+ 14 - 3
packages/core/test/tool-edit.test.ts

@@ -179,6 +179,17 @@ describe("EditTool", () => {
                 })
                 expect(yield* Effect.promise(() => fs.readFile(target, "utf8"))).toBe("after\nrest\n")
                 expect(assertions).toMatchObject([{ sessionID, action: "edit", resources: ["hello.txt"], save: ["*"] }])
+                expect(assertions[0]?.metadata).toMatchObject({
+                  files: [
+                    {
+                      file: "hello.txt",
+                      status: "modified",
+                      additions: 1,
+                      deletions: 1,
+                      patch: expect.stringContaining("-before\n+after"),
+                    },
+                  ],
+                })
                 expect(writes).toEqual([yield* Effect.promise(() => fs.realpath(target))])
               }),
             ),
@@ -343,7 +354,7 @@ describe("EditTool", () => {
             error: { type: "permission.rejected", message: "Permission denied: edit" },
           })
           expect(assertions.map((input) => input.action)).toEqual(["external_directory", "edit"])
-          expect(reads).toBe(0)
+          expect(reads).toBe(1)
           expect(writes).toEqual([])
           expect(yield* Effect.promise(() => fs.readFile(external, "utf8"))).toBe("before")
         }),
@@ -354,7 +365,7 @@ describe("EditTool", () => {
     ),
   )
 
-  it.live("denied edit reads no target content and does not disclose whether oldString matches", () =>
+  it.live("denied edit does not disclose whether oldString matches", () =>
     Effect.acquireUseRelease(
       Effect.promise(() => tmpdir()),
       (tmp) => {
@@ -380,7 +391,7 @@ describe("EditTool", () => {
                 })
                 expect(missing).toEqual(matching)
                 expect(assertions.map((input) => input.action)).toEqual(["edit", "edit"])
-                expect(reads).toBe(0)
+                expect(reads).toBe(2)
                 expect(writes).toEqual([])
               }),
             ),

+ 22 - 0
packages/core/test/tool-write.test.ts

@@ -140,6 +140,17 @@ describe("WriteTool", () => {
               "created",
             )
             expect(assertions).toMatchObject([{ sessionID, action: "edit", resources: ["src/new.txt"], save: ["*"] }])
+            expect(assertions[0]?.metadata).toMatchObject({
+              files: [
+                {
+                  file: "src/new.txt",
+                  status: "added",
+                  additions: 1,
+                  deletions: 0,
+                  patch: expect.stringContaining("+created"),
+                },
+              ],
+            })
             expect(writes).toEqual([path.join(yield* Effect.promise(() => fs.realpath(tmp.path)), "src", "new.txt")])
           }),
         )
@@ -187,6 +198,17 @@ describe("WriteTool", () => {
               if (settled.status !== "completed") return
               expect(settled.content).toEqual([{ type: "text", text: "Wrote file successfully: existing.txt" }])
               expect(settled.output).toMatchObject({ resource: "existing.txt", existed: true })
+              expect(assertions[0]?.metadata).toMatchObject({
+                files: [
+                  {
+                    file: "existing.txt",
+                    status: "modified",
+                    additions: 1,
+                    deletions: 1,
+                    patch: expect.stringMatching(/-before[\s\S]*\+after/),
+                  },
+                ],
+              })
               expect(yield* Effect.promise(() => fs.readFile(path.join(tmp.path, "existing.txt"), "utf8"))).toBe(
                 "after",
               )