ceos/opencode
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

fix(core): improve patch errors (#38016)

Browse source
This commit is contained in:
Aiden Cline 2026-07-20 22:49:04 -05:00 • committed by GitHub
commit 63e2054f50
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
4 changed files with 102 additions and 27 deletions
Download patch file Download diff file Expand all files Collapse all files

36
packages/core/src/patch.ts
View file

@ -1,5 +1,26 @@
export * as Patch from "./patch"
import { Result, Schema } from "effect"
export class BoundaryError extends Schema.TaggedErrorClass<BoundaryError>()("Patch.BoundaryError", {
boundary: Schema.Literals(["first", "last"]),
}) {
override get message() {
return `The ${this.boundary} line of the patch must be '${this.boundary === "first" ? "*** Begin Patch" : "*** End Patch"}'`
}
}
export class InvalidHunkError extends Schema.TaggedErrorClass<InvalidHunkError>()("Patch.InvalidHunkError", {
line: Schema.String,
lineNumber: Schema.Number,
}) {
override get message() {
return `Invalid hunk at line ${this.lineNumber}: '${this.line}' is not a valid hunk header. Valid hunk headers: '*** Add File: {path}', '*** Delete File: {path}', '*** Update File: {path}'`
}
}
export type ParseError = BoundaryError | InvalidHunkError
export type Hunk =
| { readonly type: "add"; readonly path: string; readonly contents: string }
| { readonly type: "delete"; readonly path: string }
@ -22,11 +43,12 @@ export interface FileUpdate {
readonly bom: boolean
}
export function parse(patchText: string): ReadonlyArray<Hunk> {
export function parse(patchText: string): Result.Result<ReadonlyArray<Hunk>, ParseError> {
const lines = stripHeredoc(patchText.trim()).split("\n")
const begin = lines.findIndex((line) => line.trim() === "*** Begin Patch")
const end = lines.findIndex((line) => line.trim() === "*** End Patch")
if (begin === -1 || end === -1 || begin >= end) throw new Error("Invalid patch format: missing Begin/End markers")
if (begin === -1) return Result.fail(new BoundaryError({ boundary: "first" }))
if (end === -1 || begin >= end) return Result.fail(new BoundaryError({ boundary: "last" }))
const hunks: Hunk[] = []
let index = begin + 1
@ -72,7 +94,15 @@ export function parse(patchText: string): ReadonlyArray<Hunk> {
}
index++
}
return hunks
if (hunks.length === 0) {
const invalid = lines.findIndex((line, index) => index > begin && index < end && line.trim() !== "")
if (invalid !== -1) {
return Result.fail(
new InvalidHunkError({ line: lines[invalid]!.trim(), lineNumber: invalid + 1 }),
)
}
}
return Result.succeed(hunks)
}
export function derive(path: string, chunks: ReadonlyArray<UpdateFileChunk>, original: string): FileUpdate {

33
packages/core/src/tool/patch.ts
View file

@ -97,10 +97,11 @@ export const Plugin = {
callID: context.callID,
}
if (!input.patchText) return yield* new ToolFailure({ message: "patchText is required" })
const hunks = yield* Effect.try({
try: () => Patch.parse(input.patchText),
catch: (cause) => new ToolFailure({ message: `patch verification failed: ${String(cause)}` }),
})
const hunks = yield* Effect.fromResult(Patch.parse(input.patchText)).pipe(
Effect.mapError(
(error) => new ToolFailure({ message: `patch verification failed: ${error.message}` }),
),
)
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") {
@ -153,15 +154,27 @@ export const Plugin = {
prepared.push({ ...hunk, target, before: original.replace(/^\uFEFF/, ""), after: "" })
return
}
const stats = yield* fs
.stat(target.canonical)
.pipe(Effect.catch(() => Effect.succeed(undefined)))
if (!stats || stats.type === "Directory") {
const stats = yield* fs.stat(target.canonical).pipe(
Effect.mapError(
(error) =>
new ToolFailure({
message: `patch verification failed: Failed to read file to update ${target.canonical}: ${error instanceof Error ? error.message : String(error)}`,
}),
),
)
if (stats.type === "Directory") {
return yield* new ToolFailure({
message: `patch verification failed: Failed to read file to update: ${target.canonical}`,
message: `patch verification failed: Failed to read file to update ${target.canonical}: path is a directory`,
})
}
const content = yield* fs.readFile(target.canonical)
const content = yield* fs.readFile(target.canonical).pipe(
Effect.mapError(
(error) =>
new ToolFailure({
message: `patch verification failed: Failed to read file to update ${target.canonical}: ${error instanceof Error ? error.message : String(error)}`,
}),
),
)
const original = new TextDecoder("utf-8", { ignoreBOM: true }).decode(content)
const before = original.replace(/^\uFEFF/, "")
const update = yield* Effect.try({

26
packages/core/test/patch.test.ts
View file

@ -1,10 +1,13 @@
import { describe, expect, test } from "bun:test"
import { Patch } from "@opencode-ai/core/patch"
import { Result } from "effect"
const parse = (input: string) => Result.getOrThrow(Patch.parse(input))
describe("Patch", () => {
test("parses add, update, and delete hunks", () => {
expect(
Patch.parse(
parse(
"*** Begin Patch\n*** Add File: add.txt\n+added\n*** Update File: update.txt\n@@ section\n-old\n+new\n*** Delete File: delete.txt\n*** End Patch",
),
).toEqual([
@ -21,7 +24,7 @@ describe("Patch", () => {
test("parses a file move", () => {
expect(
Patch.parse(
parse(
"*** Begin Patch\n*** Update File: old.txt\n*** Move to: new.txt\n@@\n-old\n+new\n*** End Patch",
),
).toEqual([
@ -34,18 +37,23 @@ describe("Patch", () => {
])
})
test("rejects invalid patch format", () => {
expect(() => Patch.parse("This is not a valid patch")).toThrow("Invalid patch format")
test("identifies the missing patch boundary", () => {
expect(() => parse("This is not a valid patch")).toThrow(
"The first line of the patch must be '*** Begin Patch'",
)
expect(() => parse("*** Begin Patch\n*** Add File: add.txt\n+added")).toThrow(
"The last line of the patch must be '*** End Patch'",
)
})
test("strips a heredoc wrapper", () => {
expect(Patch.parse("cat <<'EOF'\n*** Begin Patch\n*** Add File: add.txt\n+added\n*** End Patch\nEOF")).toEqual([
expect(parse("cat <<'EOF'\n*** Begin Patch\n*** Add File: add.txt\n+added\n*** End Patch\nEOF")).toEqual([
{ type: "add", path: "add.txt", contents: "added" },
])
})
test("strips a heredoc wrapper without cat", () => {
expect(Patch.parse("<<EOF\n*** Begin Patch\n*** Add File: add.txt\n+added\n*** End Patch\nEOF")).toEqual([
expect(parse("<<EOF\n*** Begin Patch\n*** Add File: add.txt\n+added\n*** End Patch\nEOF")).toEqual([
{ type: "add", path: "add.txt", contents: "added" },
])
})
@ -112,13 +120,13 @@ describe("Patch", () => {
})
test("matches V1 lenient parsing of malformed hunk bodies", () => {
expect(Patch.parse("*** Begin Patch\n*** Add File: add.txt\nmissing plus\n*** End Patch")).toEqual([
expect(parse("*** Begin Patch\n*** Add File: add.txt\nmissing plus\n*** End Patch")).toEqual([
{ type: "add", path: "add.txt", contents: "" },
])
expect(Patch.parse("*** Begin Patch\n*** Update File: update.txt\n*** End Patch")).toEqual([
expect(parse("*** Begin Patch\n*** Update File: update.txt\n*** End Patch")).toEqual([
{ type: "update", path: "update.txt", movePath: undefined, chunks: [] },
])
expect(Patch.parse("*** Begin Patch\n*** Delete File: delete.txt\nunexpected body\n*** End Patch")).toEqual([
expect(parse("*** Begin Patch\n*** Delete File: delete.txt\nunexpected body\n*** End Patch")).toEqual([
{ type: "delete", path: "delete.txt" },
])
})

34
packages/core/test/tool-patch.test.ts
View file

@ -433,9 +433,13 @@ describe("PatchTool", () => {
it.live("rejects invalid patch format", () =>
withTempTool((_directory, registry) =>
Effect.gen(function* () {
expect(yield* executeTool(registry, call("invalid patch"))).toMatchObject({
expect(yield* executeTool(registry, call("invalid patch"))).toEqual({
type: "error",
value: expect.stringContaining("patch verification failed"),
value: "patch verification failed: The first line of the patch must be '*** Begin Patch'",
})
expect(yield* executeTool(registry, call("*** Begin Patch\n*** Add File: foo\n+hello"))).toEqual({
type: "error",
value: "patch verification failed: The last line of the patch must be '*** End Patch'",
})
}),
),
@ -460,7 +464,11 @@ describe("PatchTool", () => {
registry,
call("*** Begin Patch\n*** Frobnicate File: foo\n*** End Patch"),
),
).toEqual({ type: "error", value: "patch verification failed: no hunks found" })
).toEqual({
type: "error",
value:
"patch verification failed: Invalid hunk at line 2: '*** Frobnicate File: foo' is not a valid hunk header. Valid hunk headers: '*** Add File: {path}', '*** Delete File: {path}', '*** Update File: {path}'",
})
}),
),
)
@ -623,7 +631,7 @@ describe("PatchTool", () => {
)
it.live("rejects an update when the target file is missing", () =>
withTempTool((_directory, registry) =>
withTempTool((directory, registry) =>
Effect.gen(function* () {
expect(
yield* executeTool(
@ -632,7 +640,23 @@ describe("PatchTool", () => {
),
).toMatchObject({
type: "error",
value: expect.stringContaining("patch verification failed: Failed to read file to update"),
value: expect.stringContaining(
`patch verification failed: Failed to read file to update ${path.join(directory, "missing.txt")}: `,
),
})
}),
),
)
it.live("identifies a directory used as an update target", () =>
withTempTool((directory, registry) =>
Effect.gen(function* () {
yield* Effect.promise(() => fs.mkdir(path.join(directory, "nested")))
expect(
yield* executeTool(registry, call("*** Begin Patch\n*** Update File: nested\n@@\n-old\n+new\n*** End Patch")),
).toEqual({
type: "error",
value: `patch verification failed: Failed to read file to update ${path.join(directory, "nested")}: path is a directory`,
})
}),
),