test(core): cover patch edge cases

This commit is contained in:
Aiden Cline
2026-07-22 14:19:34 -05:00
parent 4de6a5f60a
commit 4abfeb4964
4 changed files with 75 additions and 16 deletions
+6 -7
View File
@@ -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"
@@ -89,7 +90,7 @@ export const Plugin = {
return new ToolFailure({ message: `Unable to apply patch at ${path}${detail}`, error })
}
return new ToolFailure({
message: `Patch partially applied before failing at ${path}${detail}. Applied: ${applied.map((item) => item.resource).join(", ")}`,
message: `Patch partially applied before failing at ${path}${detail}. Completed before failure: ${applied.map((item) => item.resource).join(", ")}`,
error,
})
}
@@ -97,7 +98,7 @@ export const Plugin = {
const previous =
applied.length === 0
? ""
: `. Applied before move: ${applied.map((item) => item.resource).join(", ")}`
: `. 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,
@@ -112,7 +113,7 @@ export const Plugin = {
if (!input.patchText) return yield* new ToolFailure({ message: "patchText is required" })
const hunks = yield* Effect.fromResult(Patch.parse(input.patchText)).pipe(
Effect.mapError(
(error) => new ToolFailure({ message: `patch verification failed: ${error.message}`, error }),
(error) => new ToolFailure({ message: `patch verification failed: ${error.message}` }),
),
)
if (hunks.length === 0) {
@@ -157,7 +158,6 @@ export const Plugin = {
(error) =>
new ToolFailure({
message: `patch verification failed: Failed to read file to delete ${target.canonical}: ${errorMessage(error)}`,
error,
}),
),
)
@@ -174,7 +174,6 @@ export const Plugin = {
(error) =>
new ToolFailure({
message: `patch verification failed: Failed to read file to update ${target.canonical}: ${errorMessage(error)}`,
error,
}),
),
)
@@ -189,7 +188,6 @@ export const Plugin = {
(error) =>
new ToolFailure({
message: `patch verification failed: Failed to read file to update ${target.canonical}: ${errorMessage(error)}`,
error,
}),
),
),
@@ -199,7 +197,7 @@ export const Plugin = {
const update = yield* Effect.try({
try: () => Patch.derive(hunk.path, hunk.chunks, original),
catch: (error) =>
new ToolFailure({ message: `patch verification failed: ${errorMessage(error)}`, error }),
new ToolFailure({ message: `patch verification failed: ${errorMessage(error)}` }),
})
const moveTarget = hunk.movePath ? resolveTarget(location, hunk.movePath) : undefined
if (moveTarget) targets.push(moveTarget)
@@ -328,6 +326,7 @@ 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)
}
+32
View File
@@ -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")
@@ -424,6 +453,9 @@ describe("Patch", () => {
expect(() => parse("*** Begin Patch\n*** Delete File: file.txt\nbad\n*** End Patch")).toThrow(
"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", () => {
+34 -9
View File
@@ -333,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* () {
@@ -712,15 +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"),
),
).toEqual({
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")
}),
),
@@ -787,7 +810,7 @@ describe("PatchTool", () => {
),
).toEqual({
type: "error",
value: `Unable to apply patch at new.txt: Unknown: FileSystem.writeWithDirs (${path.join(directory, "new.txt")}): forced write failure`,
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)
@@ -808,7 +831,8 @@ describe("PatchTool", () => {
),
).toEqual({
type: "error",
value: `Patch partially applied before failing at second.txt: Unknown: FileSystem.writeWithDirs (${path.join(directory, "second.txt")}): forced write failure. Applied: first.txt`,
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)
@@ -830,7 +854,8 @@ describe("PatchTool", () => {
),
).toEqual({
type: "error",
value: `Patch partially applied while moving old.txt to new.txt: wrote new.txt but failed to remove old.txt: Unknown: FileSystem.remove (${path.join(directory, "old.txt")}): forced remove failure`,
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")
+3
View File
@@ -79,6 +79,9 @@ export function parse(patchText: string): Result.Result<ReadonlyArray<Hunk>, Par
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,