fix(core): unify patch path resolution (#42667)

packages/core/src/location-mutation.ts

@@ -76,9 +76,11 @@ const layer = Layer.effect(
       const type =
         input.kind === "directory"
           ? "Directory"
-          : (yield* fs
-              .stat(absolute)
-              .pipe(Effect.catchReason("PlatformError", "NotFound", () => Effect.succeed(undefined))))?.type
+          : input.kind === "file"
+            ? "File"
+            : (yield* fs
+                .stat(absolute)
+                .pipe(Effect.catchReason("PlatformError", "NotFound", () => Effect.succeed(undefined))))?.type
       const externalDirectory = type === "Directory" ? absolute : path.dirname(absolute)
       const externalResource = slash(path.join(externalDirectory, "*"))
       return {

packages/core/src/tool/plugin/patch.ts

@@ -6,11 +6,11 @@ import { FileDiff } from "@opencode-ai/schema/file-diff"
 import { Effect, Result, Schema } from "effect"
 import path from "path"
 import { Bom } from "@opencode-ai/util/bom"
-import { FSUtil } from "@opencode-ai/util/fs-util"
 import { Environment } from "../../environment/index.js"
 import { Formatter } from "../../formatter.js"
 import { FileMutation } from "../../file-mutation.js"
 import { Location } from "../../location.js"
+import { LocationMutation } from "../../location-mutation.js"
 import { Patch } from "@opencode-ai/util/patch"
 import { Permission } from "../../permission.js"
 import DESCRIPTION from "../patch.txt"
@@ -46,38 +46,30 @@ export const toModelOutput = (output: Output) =>
 
 type Prepared =
   | (Extract<Patch.Hunk, { readonly type: "add" }> & {
-      readonly target: Target
+      readonly target: LocationMutation.Target
       readonly content: string
       readonly before: string
       readonly after: string
     })
   | (Extract<Patch.Hunk, { readonly type: "delete" }> & {
-      readonly target: Target
+      readonly target: LocationMutation.Target
       readonly before: string
       readonly after: string
     })
   | (Extract<Patch.Hunk, { readonly type: "update" }> & {
-      readonly target: Target
+      readonly target: LocationMutation.Target
       readonly content: string
       readonly before: string
       readonly after: string
-      readonly moveTarget?: Target
+      readonly moveTarget?: LocationMutation.Target
     })
 
-interface Target {
-  readonly absolute: string
-  readonly resource: string
-  readonly externalDirectory?: {
-    readonly directory: string
-    readonly resource: string
-  }
-}
-
 export const Plugin = {
   id: "opencode.tool.patch",
   effect: Effect.fn("PatchTool.Plugin")(function* (ctx: PluginContext) {
     const environment = yield* Environment.Service
-    const mutation = yield* FileMutation.Service
+    const mutation = yield* LocationMutation.Service
+    const fileMutation = yield* FileMutation.Service
     const formatter = yield* Formatter.Service
     const location = yield* Location.Service
     const permission = yield* Permission.Service
@@ -119,26 +111,25 @@ export const Plugin = {
                 return yield* new ToolFailure({ message: "patch rejected: empty patch" })
               }
               const prepared: Prepared[] = []
-              const targets: Target[] = []
               const updates = new Map<string, string>()
+              const resolveTarget = Effect.fnUntraced(function* (value: string) {
+                const target = yield* mutation.resolve({ path: value, kind: "file" })
+                if (!target.externalDirectory) return target
+                yield* permission.assert({
+                  ...LocationMutation.externalDirectoryPermission(target.externalDirectory),
+                  metadata: {
+                    filepath: target.absolute,
+                    parentDir: target.externalDirectory.directory,
+                  },
+                  sessionID: context.sessionID,
+                  agent: context.agent,
+                  source,
+                })
+                return target
+              })
               for (const hunk of hunks) {
                 yield* Effect.gen(function* () {
-                  const target = resolveTarget(location, hunk.path)
-                  targets.push(target)
-                  if (target.externalDirectory) {
-                    yield* permission.assert({
-                      action: "external_directory",
-                      resources: [target.externalDirectory.resource],
-                      save: [target.externalDirectory.resource],
-                      metadata: {
-                        filepath: target.absolute,
-                        parentDir: target.externalDirectory.directory,
-                      },
-                      sessionID: context.sessionID,
-                      agent: context.agent,
-                      source,
-                    })
-                  }
+                  const target = yield* resolveTarget(hunk.path)
                   if (hunk.type === "add") {
                     const content =
                       hunk.contents.endsWith("\n") || hunk.contents === "" ? hunk.contents : `${hunk.contents}\n`
@@ -182,22 +173,7 @@ export const Plugin = {
                     try: () => Patch.derive(hunk.path, hunk.chunks, original),
                     catch: (error) => new ToolFailure({ message: `patch verification failed: ${errorMessage(error)}` }),
                   })
-                  const moveTarget = hunk.movePath ? resolveTarget(location, hunk.movePath) : undefined
-                  if (moveTarget) targets.push(moveTarget)
-                  if (moveTarget?.externalDirectory) {
-                    yield* permission.assert({
-                      action: "external_directory",
-                      resources: [moveTarget.externalDirectory.resource],
-                      save: [moveTarget.externalDirectory.resource],
-                      metadata: {
-                        filepath: moveTarget.absolute,
-                        parentDir: moveTarget.externalDirectory.directory,
-                      },
-                      sessionID: context.sessionID,
-                      agent: context.agent,
-                      source,
-                    })
-                  }
+                  const moveTarget = hunk.movePath ? yield* resolveTarget(hunk.movePath) : undefined
                   prepared.push({
                     ...hunk,
                     target,
@@ -217,6 +193,10 @@ export const Plugin = {
               }
 
               const patchFiles = prepared.map((change) => patchFile(change))
+              const targets = prepared.flatMap((change) => [
+                change.target,
+                ...(change.type === "update" && change.moveTarget ? [change.moveTarget] : []),
+              ])
               yield* permission.assert({
                 action: "edit",
                 resources: [...new Set(targets.map((target) => target.resource))],
@@ -313,7 +293,7 @@ export const Plugin = {
               })
               return { applied, files }
             }).pipe(
-              mutation.withLock(lockTargets),
+              fileMutation.withLock(lockTargets),
               Effect.map((output) => ({
                 output,
                 content: toModelOutput(output),
@@ -394,24 +374,3 @@ function trimDiff(diff: string) {
     })
     .join("\n")
 }
-
-function resolveTarget(location: Location.Interface, value: string): Target {
-  const absolute =
-    process.platform === "win32"
-      ? FSUtil.normalizePath(path.resolve(location.directory, value))
-      : path.resolve(location.directory, value)
-  const projectRoot = path.parse(location.project.directory).root
-  const external =
-    !FSUtil.contains(location.directory, absolute) &&
-    (location.project.directory === projectRoot || !FSUtil.contains(location.project.directory, absolute))
-  const directory = path.dirname(absolute)
-  const resource =
-    process.platform === "win32"
-      ? FSUtil.normalizePathPattern(path.join(directory, "*"))
-      : path.join(directory, "*").replaceAll("\\", "/")
-  return {
-    absolute,
-    resource: path.relative(location.project.directory, absolute).replaceAll("\\", "/") || ".",
-    externalDirectory: external ? { directory, resource } : undefined,
-  }
-}

packages/core/test/location-mutation.test.ts

@@ -160,6 +160,20 @@ describe("LocationMutation", () => {
     ),
   )
 
+  it.live("uses an explicit file kind without treating an existing directory as the target boundary", () =>
+    withTmp((directory) =>
+      withTmp((outside) =>
+        Effect.gen(function* () {
+          const target = yield* (yield* LocationMutation.Service).resolve({ path: outside, kind: "file" })
+          expect(target.externalDirectory).toMatchObject({
+            directory: path.dirname(outside),
+            resource: path.join(path.dirname(outside), "*").replaceAll("\\", "/"),
+          })
+        }).pipe(provide(directory)),
+      ),
+    ),
+  )
+
   it.live("authorizes prospective external descendants at their lexical parent", () =>
     withTmp((directory) =>
       withTmp((outside) =>

packages/core/test/tool-patch.test.ts

@@ -5,10 +5,10 @@ import { Effect, Exit, Layer, Schema } from "effect"
 import { AppNodeBuilder } from "@opencode-ai/core/effect/app-node-builder"
 import { LayerNode } from "@opencode-ai/util/effect/layer-node"
 import { Environment } from "@opencode-ai/core/environment/index"
-import { FSUtil } from "@opencode-ai/util/fs-util"
 import { Formatter } from "@opencode-ai/core/formatter"
 import { FileMutation } from "@opencode-ai/core/file-mutation"
 import { Location } from "@opencode-ai/core/location"
+import { LocationMutation } from "@opencode-ai/core/location-mutation"
 import { Permission } from "@opencode-ai/core/permission"
 import { AbsolutePath } from "@opencode-ai/core/schema"
 import { Session } from "@opencode-ai/core/session"
@@ -25,7 +25,15 @@ import { toolIdentity, executeTool, registerToolPlugin, toolDefinitions } from "
 const patchToolNode = makeLocationNode({
   name: "test/patch-tool-plugin",
   layer: Layer.effectDiscard(registerToolPlugin(PatchTool.Plugin)),
-  deps: [Tool.node, FileMutation.node, Environment.node, Formatter.node, Location.node, Permission.node],
+  deps: [
+    Tool.node,
+    LocationMutation.node,
+    FileMutation.node,
+    Environment.node,
+    Formatter.node,
+    Location.node,
+    Permission.node,
+  ],
 })
 
 const sessionID = Session.ID.make("ses_patch_tool_test")
@@ -91,7 +99,7 @@ const withTool = <A, E, R>(
     return yield* body(yield* Tool.Service)
   }).pipe(
     Effect.provide(
-      AppNodeBuilder.build(LayerNode.group([Tool.node, FileMutation.node, patchToolNode]), [
+      AppNodeBuilder.build(LayerNode.group([Tool.node, LocationMutation.node, FileMutation.node, patchToolNode]), [
         [
           Environment.node,
           transformEnvironmentFiles(activeLocation, (files) => ({
@@ -485,6 +493,42 @@ describe("PatchTool", () => {
     ),
   )
 
+  it.live("uses Location-relative resources for move targets in a nested Location", () =>
+    Effect.acquireUseRelease(
+      Effect.promise(() => tmpdir()),
+      (tmp) => {
+        reset()
+        const active = path.join(tmp.path, "nested", "location")
+        const source = path.join(active, "old.txt")
+        return Effect.promise(() =>
+          fs.mkdir(active, { recursive: true }).then(() => fs.writeFile(source, "before\n")),
+        ).pipe(
+          Effect.andThen(
+            withTool(
+              active,
+              (registry) =>
+                Effect.gen(function* () {
+                  const settled = yield* executeTool(
+                    registry,
+                    call(
+                      "*** Begin Patch\n*** Update File: old.txt\n*** Move to: moved.txt\n@@\n-before\n+after\n*** End Patch",
+                    ),
+                  )
+                  expect(settled).toMatchObject({
+                    status: "completed",
+                    output: { applied: [{ resource: "moved.txt" }] },
+                  })
+                  expect(assertions).toMatchObject([{ action: "edit", resources: ["old.txt", "moved.txt"] }])
+                }),
+              tmp.path,
+            ),
+          ),
+        )
+      },
+      (tmp) => Effect.promise(() => tmp[Symbol.asyncDispose]()),
+    ),
+  )
+
   it.live("inserts lines with an insert-only hunk", () =>
     withTempTool((directory, registry) =>
       Effect.gen(function* () {
@@ -781,8 +825,15 @@ describe("PatchTool", () => {
       Effect.promise(() => Promise.all([tmpdir(), tmpdir()])),
       ([active, outside]) => {
         reset()
-        const target = path.join(outside.path, "external.txt")
-        return Effect.promise(() => fs.writeFile(target, "before\n")).pipe(
+        const repository = path.join(outside.path, "repository")
+        const directory = path.join(repository, "nested")
+        const target = path.join(directory, "external.txt")
+        return Effect.promise(() =>
+          Promise.all([
+            fs.mkdir(path.join(repository, ".git"), { recursive: true }),
+            fs.mkdir(directory, { recursive: true }).then(() => fs.writeFile(target, "before\n")),
+          ]),
+        ).pipe(
           Effect.andThen(
             withTool(active.path, (registry) =>
               Effect.gen(function* () {
@@ -793,6 +844,15 @@ describe("PatchTool", () => {
                   ),
                 ).toMatchObject({ status: "completed" })
                 expect(assertions.map((input) => input.action)).toEqual(["external_directory", "edit"])
+                expect(assertions[0]).toMatchObject({
+                  resources: [path.join(directory, "*").replaceAll("\\", "/")],
+                  save: [path.join(repository, "*").replaceAll("\\", "/")],
+                  metadata: {
+                    filepath: target,
+                    parentDir: directory,
+                  },
+                })
+                expect(assertions[1]?.resources).toEqual([target.replaceAll("\\", "/")])
                 expect(readsBeforeEditApproval).toBe(1)
                 expect(yield* Effect.promise(() => fs.readFile(target, "utf8"))).toBe("after\n")
               }),
@@ -860,7 +920,7 @@ describe("PatchTool", () => {
     ),
   )
 
-  it.live("treats a sibling path inside the project worktree as internal", () =>
+  it.live("treats a sibling path inside the project worktree as external to the Location", () =>
     Effect.acquireUseRelease(
       Effect.promise(() => tmpdir()),
       (tmp) => {
@@ -879,7 +939,9 @@ describe("PatchTool", () => {
                       call("*** Begin Patch\n*** Update File: ../sibling.txt\n@@\n-before\n+after\n*** End Patch"),
                     ),
                   ).toMatchObject({ status: "completed" })
-                  expect(assertions.map((input) => input.action)).toEqual(["edit"])
+                  expect(assertions.map((input) => input.action)).toEqual(["external_directory", "edit"])
+                  expect(assertions[0]?.resources).toEqual([path.join(tmp.path, "*").replaceAll("\\", "/")])
+                  expect(assertions[1]?.resources).toEqual([target.replaceAll("\\", "/")])
                   expect(yield* Effect.promise(() => fs.readFile(target, "utf8"))).toBe("after\n")
                 }),
               tmp.path,
@@ -956,6 +1018,53 @@ describe("PatchTool", () => {
     ),
   )
 
+  it.live("uses canonical external permissions and resources for a move destination", () =>
+    Effect.acquireUseRelease(
+      Effect.promise(() => Promise.all([tmpdir(), tmpdir()])),
+      ([active, outside]) => {
+        reset()
+        const source = path.join(active.path, "source.txt")
+        const destination = path.join(outside.path, "moved.txt")
+        return Effect.promise(() => fs.writeFile(source, "before\n")).pipe(
+          Effect.andThen(
+            withTool(active.path, (registry) =>
+              Effect.gen(function* () {
+                const settled = yield* executeTool(
+                  registry,
+                  call(
+                    `*** Begin Patch\n*** Update File: source.txt\n*** Move to: ${destination}\n@@\n-before\n+after\n*** End Patch`,
+                  ),
+                )
+                expect(settled).toMatchObject({
+                  status: "completed",
+                  output: { applied: [{ resource: destination.replaceAll("\\", "/") }] },
+                })
+                expect(assertions).toMatchObject([
+                  {
+                    action: "external_directory",
+                    resources: [path.join(outside.path, "*").replaceAll("\\", "/")],
+                    save: [path.join(outside.path, "*").replaceAll("\\", "/")],
+                    metadata: { filepath: destination, parentDir: outside.path },
+                  },
+                  {
+                    action: "edit",
+                    resources: ["source.txt", destination.replaceAll("\\", "/")],
+                  },
+                ])
+                expect(yield* exists(source)).toBe(false)
+                expect(yield* Effect.promise(() => fs.readFile(destination, "utf8"))).toBe("after\n")
+              }),
+            ),
+          ),
+        )
+      },
+      ([active, outside]) =>
+        Effect.promise(() =>
+          Promise.all([active[Symbol.asyncDispose](), outside[Symbol.asyncDispose]()]).then(() => undefined),
+        ),
+    ),
+  )
+
   it.live("approves each external file under the same parent", () =>
     Effect.acquireUseRelease(
       Effect.promise(() => Promise.all([tmpdir(), tmpdir()])),
@@ -983,9 +1092,7 @@ describe("PatchTool", () => {
                   "edit",
                 ])
                 expect(assertions[0]?.resources).toEqual([
-                  process.platform === "win32"
-                    ? FSUtil.normalizePathPattern(path.join(outside.path, "*"))
-                    : path.join(yield* Effect.promise(() => fs.realpath(outside.path)), "*").replaceAll("\\", "/"),
+                  path.join(yield* Effect.promise(() => fs.realpath(outside.path)), "*").replaceAll("\\", "/"),
                 ])
                 expect(assertions[1]?.resources).toEqual(assertions[0]?.resources)
               }),