diff --git a/packages/core/src/tool/patch.ts b/packages/core/src/tool/patch.ts index 8e09354922..84a027b724 100644 --- a/packages/core/src/tool/patch.ts +++ b/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" @@ -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) } diff --git a/packages/core/test/patch.test.ts b/packages/core/test/patch.test.ts index 3ac8154b3b..bd75b26647 100644 --- a/packages/core/test/patch.test.ts +++ b/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") @@ -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", () => { diff --git a/packages/core/test/tool-patch.test.ts b/packages/core/test/tool-patch.test.ts index e63468526e..19f96f1b88 100644 --- a/packages/core/test/tool-patch.test.ts +++ b/packages/core/test/tool-patch.test.ts @@ -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") diff --git a/packages/util/src/patch.ts b/packages/util/src/patch.ts index 8a94a1948e..54766c5a61 100644 --- a/packages/util/src/patch.ts +++ b/packages/util/src/patch.ts @@ -79,6 +79,9 @@ export function parse(patchText: string): Result.Result, 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,