test(core): cover patch edge cases

packages/core/src/tool/patch.ts

@@ -5,6 +5,7 @@ import { ToolFailure } from "@opencode-ai/ai"
 import { FileDiff } from "@opencode-ai/schema/file-diff"
 import { createTwoFilesPatch, diffLines } from "diff"
 import { Effect, Schema } from "effect"
+import { PlatformError } from "effect/PlatformError"
 import path from "path"
 import { FSUtil } from "@opencode-ai/util/fs-util"
 import { Location } from "../location"
@@ -84,11 +85,24 @@ export const Plugin = {
               execute: (input, context) => {
                 const applied: Array<typeof Applied.Type> = []
                 const fail = (path: string, error?: unknown) => {
-                  const prefix =
+                  const detail = error === undefined ? "" : `: ${errorMessage(error)}`
+                  if (applied.length === 0) {
+                    return new ToolFailure({ message: `Unable to apply patch at ${path}${detail}`, error })
+                  }
+                  return new ToolFailure({
+                    message: `Patch partially applied before failing at ${path}${detail}. Completed before failure: ${applied.map((item) => item.resource).join(", ")}`,
+                    error,
+                  })
+                }
+                const failMoveRemoval = (source: string, destination: string, error: unknown) => {
+                  const previous =
                     applied.length === 0
-                      ? `Unable to apply patch at ${path}`
-                      : `Patch partially applied before failing at ${path}. Applied: ${applied.map((item) => item.resource).join(", ")}`
-                  return new ToolFailure({ message: prefix, error })
+                      ? ""
+                      : `. Completed before move: ${applied.map((item) => item.resource).join(", ")}`
+                  return new ToolFailure({
+                    message: `Patch partially applied while moving ${source} to ${destination}: wrote ${destination} but failed to remove ${source}: ${errorMessage(error)}${previous}`,
+                    error,
+                  })
                 }
                 return Effect.gen(function* () {
                   const source = {
@@ -103,11 +117,7 @@ export const Plugin = {
                     ),
                   )
                   if (hunks.length === 0) {
-                    const normalized = input.patchText.replace(/\r\n/g, "\n").replace(/\r/g, "\n").trim()
-                    if (normalized === "*** Begin Patch\n*** End Patch") {
-                      return yield* new ToolFailure({ message: "patch rejected: empty patch" })
-                    }
-                    return yield* new ToolFailure({ message: "patch verification failed: no hunks found" })
+                    return yield* new ToolFailure({ message: "patch rejected: empty patch" })
                   }
                   const prepared: Prepared[] = []
                   const targets: Target[] = []
@@ -147,7 +157,7 @@ export const Plugin = {
                           Effect.mapError(
                             (error) =>
                               new ToolFailure({
-                                message: `patch verification failed: ${error instanceof Error ? error.message : String(error)}`,
+                                message: `patch verification failed: Failed to read file to delete ${target.canonical}: ${errorMessage(error)}`,
                               }),
                           ),
                         )
@@ -163,7 +173,7 @@ export const Plugin = {
                             Effect.mapError(
                               (error) =>
                                 new ToolFailure({
-                                  message: `patch verification failed: Failed to read file to update ${target.canonical}: ${error instanceof Error ? error.message : String(error)}`,
+                                  message: `patch verification failed: Failed to read file to update ${target.canonical}: ${errorMessage(error)}`,
                                 }),
                             ),
                           )
@@ -177,7 +187,7 @@ export const Plugin = {
                               Effect.mapError(
                                 (error) =>
                                   new ToolFailure({
-                                    message: `patch verification failed: Failed to read file to update ${target.canonical}: ${error instanceof Error ? error.message : String(error)}`,
+                                    message: `patch verification failed: Failed to read file to update ${target.canonical}: ${errorMessage(error)}`,
                                   }),
                               ),
                             ),
@@ -186,7 +196,8 @@ export const Plugin = {
                       const before = original.replace(/^\uFEFF/, "")
                       const update = yield* Effect.try({
                         try: () => Patch.derive(hunk.path, hunk.chunks, original),
-                        catch: (error) => new ToolFailure({ message: `patch verification failed: ${String(error)}` }),
+                        catch: (error) =>
+                          new ToolFailure({ message: `patch verification failed: ${errorMessage(error)}` }),
                       })
                       const moveTarget = hunk.movePath ? resolveTarget(location, hunk.movePath) : undefined
                       if (moveTarget) targets.push(moveTarget)
@@ -241,7 +252,7 @@ export const Plugin = {
                             change.contents.endsWith("\n") || change.contents === ""
                               ? change.contents
                               : `${change.contents}\n`,
-                          )
+                          ).pipe(Effect.mapError((error) => fail(change.target.resource, error)))
                           applied.push({
                             type: change.type,
                             resource: change.target.resource,
@@ -250,7 +261,9 @@ export const Plugin = {
                           return
                         }
                         if (change.type === "delete") {
-                          yield* fs.remove(change.target.canonical)
+                          yield* fs
+                            .remove(change.target.canonical)
+                            .pipe(Effect.mapError((error) => fail(change.target.resource, error)))
                           applied.push({
                             type: change.type,
                             resource: change.target.resource,
@@ -259,8 +272,15 @@ export const Plugin = {
                           return
                         }
                         if (change.moveTarget) {
-                          yield* fs.writeWithDirs(change.moveTarget.canonical, change.content)
-                          yield* fs.remove(change.target.canonical)
+                          const moveTarget = change.moveTarget
+                          yield* fs
+                            .writeWithDirs(moveTarget.canonical, change.content)
+                            .pipe(Effect.mapError((error) => fail(moveTarget.resource, error)))
+                          yield* fs.remove(change.target.canonical).pipe(
+                            Effect.mapError((error) =>
+                              failMoveRemoval(change.target.resource, moveTarget.resource, error),
+                            ),
+                          )
                           applied.push({
                             type: change.type,
                             resource: change.moveTarget.resource,
@@ -268,13 +288,15 @@ export const Plugin = {
                           })
                           return
                         }
-                        yield* fs.writeWithDirs(change.target.canonical, change.content)
+                        yield* fs
+                          .writeWithDirs(change.target.canonical, change.content)
+                          .pipe(Effect.mapError((error) => fail(change.target.resource, error)))
                         applied.push({
                           type: change.type,
                           resource: change.target.resource,
                           target: change.target.canonical,
                         })
-                      }).pipe(Effect.mapError((error) => fail(change.path, error))),
+                      }),
                     { discard: true },
                   )
                   return { applied, files: patchFiles }
@@ -303,6 +325,11 @@ export const Plugin = {
   }),
 }
 
+function errorMessage(error: unknown) {
+  if (error instanceof PlatformError) return error.reason.description ?? error.reason.message
+  return error instanceof Error ? error.message : String(error)
+}
+
 function patchFile(change: Prepared): typeof FileDiff.Info.Type {
   const target = (change.type === "update" ? change.moveTarget : undefined)?.resource ?? change.target.resource
   const patch = trimDiff(

packages/core/test/patch.test.ts

@@ -246,6 +246,35 @@ describe("Patch", () => {
     ).toBe("line 1\nLINE 2\nline 3\nLINE 4\n")
   })
 
+  test("appends a pure-addition chunk to a nonempty file", () => {
+    expect(Patch.derive("update.txt", [{ oldLines: [], newLines: ["added 1", "added 2"] }], "line 1\nline 2\n").content).toBe(
+      "line 1\nline 2\nadded 1\nadded 2\n",
+    )
+  })
+
+  test("applies a pure-addition chunk after an earlier replacement", () => {
+    expect(
+      Patch.derive(
+        "update.txt",
+        [
+          { oldLines: [], newLines: ["after-context", "second-line"] },
+          { oldLines: ["line1", "line2", "line3"], newLines: ["line1", "line2-replacement"] },
+        ],
+        "line1\nline2\nline3\n",
+      ).content,
+    ).toBe("line1\nline2-replacement\nafter-context\nsecond-line\n")
+  })
+
+  test("applies a deletion-only update chunk", () => {
+    expect(
+      Patch.derive(
+        "update.txt",
+        [{ oldLines: ["line1", "line2", "line3"], newLines: ["line1", "line3"] }],
+        "line1\nline2\nline3\n",
+      ).content,
+    ).toBe("line1\nline3\n")
+  })
+
   test("updates empty files and adds a trailing newline", () => {
     expect(Patch.derive("empty.txt", [{ oldLines: [], newLines: ["First line"] }], "").content).toBe("First line\n")
     expect(Patch.derive("no-newline.txt", [{ oldLines: ["old"], newLines: ["new"] }], "old").content).toBe("new\n")
@@ -327,6 +356,12 @@ describe("Patch", () => {
     ).toThrow("Failed to find expected lines")
   })
 
+  test("identifies a missing blank line", () => {
+    expect(() =>
+      Patch.derive("update.txt", [{ oldLines: [""], newLines: ["added"] }], "content\n"),
+    ).toThrow("Failed to find an expected blank line in update.txt")
+  })
+
   test("parses an update without an explicit first chunk header", () => {
     expect(parse("*** Begin Patch\n*** Update File: file.txt\n import foo\n+bar\n*** End Patch")).toEqual([
       {
@@ -413,11 +448,14 @@ describe("Patch", () => {
 
   test("rejects invalid add and delete lines", () => {
     expect(() => parse("*** Begin Patch\n*** Add File: file.txt\nbad\n*** End Patch")).toThrow(
-      "Invalid hunk at line 3: 'bad' is not a valid hunk header",
+      "Invalid hunk at line 3: Invalid Add File line for 'file.txt': expected a line starting with '+', got 'bad'",
     )
     expect(() => parse("*** Begin Patch\n*** Delete File: file.txt\nbad\n*** End Patch")).toThrow(
-      "Invalid hunk at line 3: 'bad' is not a valid hunk header",
+      "Invalid hunk at line 3: Unexpected line after Delete File 'file.txt': 'bad'. Delete hunks do not contain body lines",
     )
+    expect(() =>
+      parse("*** Begin Patch\n*** Delete File: file.txt\n*** Frobnicate File: next.txt\n*** End Patch"),
+    ).toThrow("Invalid hunk at line 3: '*** Frobnicate File: next.txt' is not a valid hunk header")
   })
 
   test("rejects an empty update hunk", () => {
@@ -478,6 +516,6 @@ describe("Patch", () => {
     }
     expect(() =>
       parse("*** Begin Patch\n*** Update File: old.txt\n*** Move to: \n@@\n-old\n+new\n*** End Patch"),
-    ).toThrow("Invalid hunk at line 3: '*** Move to:' is not a valid hunk header")
+    ).toThrow("Invalid hunk at line 3: Move destination for 'old.txt' must not be empty")
   })
 })

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

@@ -2,6 +2,7 @@ import fs from "fs/promises"
 import path from "path"
 import { describe, expect } from "bun:test"
 import { Effect, Exit, Layer, Schema } from "effect"
+import { systemError } from "effect/PlatformError"
 import { AppNodeBuilder } from "@opencode-ai/core/effect/app-node-builder"
 import { LayerNode } from "@opencode-ai/util/effect/layer-node"
 import { FSUtil } from "@opencode-ai/util/fs-util"
@@ -28,6 +29,8 @@ const sessionID = SessionV2.ID.make("ses_patch_tool_test")
 const assertions: PermissionV2.AssertInput[] = []
 let denyAction: string | undefined
 let failRemoveTarget: string | undefined
+let failRemoveErrorTarget: string | undefined
+let failWriteTarget: string | undefined
 let readsBeforeEditApproval = 0
 let editApproved = false
 let afterEditApproval = (): Effect.Effect<void> => Effect.void
@@ -65,6 +68,8 @@ const reset = () => {
   assertions.length = 0
   denyAction = undefined
   failRemoveTarget = undefined
+  failRemoveErrorTarget = undefined
+  failWriteTarget = undefined
   readsBeforeEditApproval = 0
   editApproved = false
   afterEditApproval = () => Effect.void
@@ -82,8 +87,33 @@ const filesystem = Layer.effect(
         }).pipe(Effect.andThen(fs.readFile(target))),
       remove: (target, options) => {
         if (failRemoveTarget && path.basename(target) === failRemoveTarget) return Effect.die("forced remove failure")
+        if (failRemoveErrorTarget && path.basename(target) === failRemoveErrorTarget) {
+          return Effect.fail(
+            systemError({
+              _tag: "Unknown",
+              module: "FileSystem",
+              method: "remove",
+              description: "forced remove failure",
+              pathOrDescriptor: target,
+            }),
+          )
+        }
         return fs.remove(target, options)
       },
+      writeWithDirs: (target, content, mode) => {
+        if (failWriteTarget && path.basename(target) === failWriteTarget) {
+          return Effect.fail(
+            systemError({
+              _tag: "Unknown",
+              module: "FileSystem",
+              method: "writeWithDirs",
+              description: "forced write failure",
+              pathOrDescriptor: target,
+            }),
+          )
+        }
+        return fs.writeWithDirs(target, content, mode)
+      },
     })
   }),
 ).pipe(Layer.provide(LayerNode.compile(FSUtil.node)))
@@ -303,6 +333,26 @@ describe("PatchTool", () => {
     ),
   )
 
+  it.live("moves a file without changing its contents", () =>
+    withTempTool((directory, registry) =>
+      Effect.gen(function* () {
+        const source = path.join(directory, "old.txt")
+        const destination = path.join(directory, "moved.txt")
+        yield* Effect.promise(() => fs.writeFile(source, "same\n"))
+        expect(
+          yield* executeTool(
+            registry,
+            call(
+              "*** Begin Patch\n*** Update File: old.txt\n*** Move to: moved.txt\n@@\n same\n*** End Patch",
+            ),
+          ),
+        ).toEqual({ type: "text", value: "Success. Updated the following files:\nM moved.txt" })
+        expect(yield* exists(source)).toBe(false)
+        expect(yield* Effect.promise(() => fs.readFile(destination, "utf8"))).toBe("same\n")
+      }),
+    ),
+  )
+
   it.live("moves a symlink without deleting its target", () =>
     withTempTool((directory, registry) =>
       Effect.gen(function* () {
@@ -488,10 +538,17 @@ describe("PatchTool", () => {
   it.live("rejects an empty patch", () =>
     withTempTool((_directory, registry) =>
       Effect.gen(function* () {
-        expect(yield* executeTool(registry, call("*** Begin Patch\n*** End Patch"))).toEqual({
-          type: "error",
-          value: "patch rejected: empty patch",
-        })
+        for (const patchText of [
+          "*** Begin Patch\n*** End Patch",
+          " *** Begin Patch \n *** End Patch ",
+          "<<EOF\n*** Begin Patch\n*** End Patch\nEOF",
+          "*** Begin Patch\n*** Environment ID: remote\n*** End Patch",
+        ]) {
+          expect(yield* executeTool(registry, call(patchText))).toEqual({
+            type: "error",
+            value: "patch rejected: empty patch",
+          })
+        }
       }),
     ),
   )
@@ -675,12 +732,18 @@ describe("PatchTool", () => {
       Effect.gen(function* () {
         const target = path.join(directory, "unchanged.txt")
         yield* Effect.promise(() => fs.writeFile(target, "line1\nline2\n"))
-        expect(
-          yield* executeTool(
-            registry,
-            call("*** Begin Patch\n*** Update File: unchanged.txt\n@@\n-missing\n+changed\n*** End Patch"),
-          ),
-        ).toMatchObject({ type: "error", value: expect.stringContaining("Failed to find expected lines") })
+        const settled = yield* settleTool(
+          registry,
+          call("*** Begin Patch\n*** Update File: unchanged.txt\n@@\n-missing\n+changed\n*** End Patch"),
+        )
+        expect(settled.result).toEqual({
+          type: "error",
+          value: "patch verification failed: Failed to find expected lines in unchanged.txt:\nmissing",
+        })
+        expect(settled.error).toEqual({
+          type: "tool.execution",
+          message: "patch verification failed: Failed to find expected lines in unchanged.txt:\nmissing",
+        })
         expect(yield* Effect.promise(() => fs.readFile(target, "utf8"))).toBe("line1\nline2\n")
       }),
     ),
@@ -718,12 +781,84 @@ describe("PatchTool", () => {
     ),
   )
 
-  it.live("rejects a delete when the target file is missing", () =>
-    withTempTool((_directory, registry) =>
+  it.live("identifies a missing delete target", () =>
+    withTempTool((directory, registry) =>
       Effect.gen(function* () {
         expect(
           yield* executeTool(registry, call("*** Begin Patch\n*** Delete File: missing.txt\n*** End Patch")),
-        ).toMatchObject({ type: "error", value: expect.stringContaining("patch verification failed") })
+        ).toMatchObject({
+          type: "error",
+          value: expect.stringContaining(
+            `patch verification failed: Failed to read file to delete ${path.join(directory, "missing.txt")}: `,
+          ),
+        })
+      }),
+    ),
+  )
+
+  it.live("reports the failing destination and filesystem error", () =>
+    withTempTool((directory, registry) =>
+      Effect.gen(function* () {
+        yield* Effect.promise(() => fs.writeFile(path.join(directory, "old.txt"), "before\n"))
+        failWriteTarget = "new.txt"
+        expect(
+          yield* executeTool(
+            registry,
+            call(
+              "*** Begin Patch\n*** Update File: old.txt\n*** Move to: new.txt\n@@\n-before\n+after\n*** End Patch",
+            ),
+          ),
+        ).toEqual({
+          type: "error",
+          value: "Unable to apply patch at new.txt: forced write failure",
+        })
+        expect(yield* Effect.promise(() => fs.readFile(path.join(directory, "old.txt"), "utf8"))).toBe("before\n")
+        expect(yield* exists(path.join(directory, "new.txt"))).toBe(false)
+      }),
+    ),
+  )
+
+  it.live("reports the successful prefix and filesystem error", () =>
+    withTempTool((directory, registry) =>
+      Effect.gen(function* () {
+        failWriteTarget = "second.txt"
+        expect(
+          yield* executeTool(
+            registry,
+            call(
+              "*** Begin Patch\n*** Add File: first.txt\n+first\n*** Add File: second.txt\n+second\n*** End Patch",
+            ),
+          ),
+        ).toEqual({
+          type: "error",
+          value:
+            "Patch partially applied before failing at second.txt: forced write failure. Completed before failure: first.txt",
+        })
+        expect(yield* Effect.promise(() => fs.readFile(path.join(directory, "first.txt"), "utf8"))).toBe("first\n")
+        expect(yield* exists(path.join(directory, "second.txt"))).toBe(false)
+      }),
+    ),
+  )
+
+  it.live("reports a destination written before move removal fails", () =>
+    withTempTool((directory, registry) =>
+      Effect.gen(function* () {
+        yield* Effect.promise(() => fs.writeFile(path.join(directory, "old.txt"), "before\n"))
+        failRemoveErrorTarget = "old.txt"
+        expect(
+          yield* executeTool(
+            registry,
+            call(
+              "*** Begin Patch\n*** Update File: old.txt\n*** Move to: new.txt\n@@\n-before\n+after\n*** End Patch",
+            ),
+          ),
+        ).toEqual({
+          type: "error",
+          value:
+            "Patch partially applied while moving old.txt to new.txt: wrote new.txt but failed to remove old.txt: forced remove failure",
+        })
+        expect(yield* Effect.promise(() => fs.readFile(path.join(directory, "old.txt"), "utf8"))).toBe("before\n")
+        expect(yield* Effect.promise(() => fs.readFile(path.join(directory, "new.txt"), "utf8"))).toBe("after\n")
       }),
     ),
   )

packages/util/src/patch.ts

@@ -69,7 +69,7 @@ export function parse(patchText: string): Result.Result<ReadonlyArray<Hunk>, Par
     }
     if (header.startsWith("*** Add File: ")) {
       const path = header.slice("*** Add File: ".length).trim()
-      const parsed = parseAdd(lines, index + 1, end)
+      const parsed = parseAdd(lines, index + 1, end, path)
       if ("error" in parsed) return Result.fail(parsed.error)
       hunks.push({ type: "add", path, contents: parsed.content })
       index = parsed.next
@@ -77,6 +77,19 @@ export function parse(patchText: string): Result.Result<ReadonlyArray<Hunk>, Par
     }
     if (header.startsWith("*** Delete File: ")) {
       const path = header.slice("*** Delete File: ".length).trim()
+      const next = lines[index + 1]?.trim()
+      if (index + 1 < end && next !== undefined && !isBoundary(next)) {
+        if (next.startsWith("*** ")) {
+          return Result.fail(new InvalidHunkError({ line: next, lineNumber: index + 2 }))
+        }
+        return Result.fail(
+          new InvalidHunkError({
+            line: next,
+            lineNumber: index + 2,
+            reason: `Unexpected line after Delete File '${path}': '${next}'. Delete hunks do not contain body lines`,
+          }),
+        )
+      }
       hunks.push({ type: "delete", path })
       index++
       continue
@@ -90,7 +103,13 @@ export function parse(patchText: string): Result.Result<ReadonlyArray<Hunk>, Par
       if (move === "*** Move to:" || move?.startsWith("*** Move to: ")) {
         movePath = move.slice("*** Move to: ".length).trim()
         if (!movePath) {
-          return Result.fail(new InvalidHunkError({ line: lines[next]!.trim(), lineNumber: next + 1 }))
+          return Result.fail(
+            new InvalidHunkError({
+              line: lines[next]!.trim(),
+              lineNumber: next + 1,
+              reason: `Move destination for '${path}' must not be empty`,
+            }),
+          )
         }
         next++
       }
@@ -126,12 +145,20 @@ function parseAdd(
   lines: ReadonlyArray<string>,
   start: number,
   end: number,
+  path: string,
 ): { content: string; next: number } | { error: InvalidHunkError } {
   const content: string[] = []
   let index = start
   while (index < end && !isBoundary(lines[index]!.trim())) {
     if (!lines[index]!.startsWith("+")) {
-      return { error: new InvalidHunkError({ line: lines[index]!.trim(), lineNumber: index + 1 }) }
+      const line = lines[index]!.trim()
+      return {
+        error: new InvalidHunkError({
+          line,
+          lineNumber: index + 1,
+          reason: `Invalid Add File line for '${path}': expected a line starting with '+', got '${line}'`,
+        }),
+      }
     }
     content.push(lines[index]!.slice(1))
     index++
@@ -303,6 +330,11 @@ function computeReplacements(lines: ReadonlyArray<string>, path: string, chunks:
       if (newLines.at(-1) === "") newLines = newLines.slice(0, -1)
       found = seek(lines, oldLines, lineIndex, chunk.endOfFile)
     }
+    if (found === -1 && chunk.oldLines.every((line) => line === "")) {
+      const expected =
+        chunk.oldLines.length === 1 ? "an expected blank line" : `${chunk.oldLines.length} consecutive blank lines`
+      throw new Error(`Failed to find ${expected} in ${path}`)
+    }
     if (found === -1) throw new Error(`Failed to find expected lines in ${path}:\n${chunk.oldLines.join("\n")}`)
     replacements.push([found, oldLines.length, newLines])
     lineIndex = found + oldLines.length