fix(core): match dev patch behavior (#37709)

This commit is contained in:
Aiden Cline 2026-07-20 16:46:11 -05:00 committed by GitHub
commit 455b5d3165
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
8 changed files with 886 additions and 188 deletions

View file

@ -34,7 +34,10 @@ export function parse(patchText: string): ReadonlyArray<Hunk> {
const line = lines[index]! const line = lines[index]!
if (line.startsWith("*** Add File:")) { if (line.startsWith("*** Add File:")) {
const path = line.slice("*** Add File:".length).trim() const path = line.slice("*** Add File:".length).trim()
if (!path) throw new Error("Invalid add file path") if (!path) {
index++
continue
}
const parsed = parseAdd(lines, index + 1) const parsed = parseAdd(lines, index + 1)
hunks.push({ type: "add", path, contents: parsed.content }) hunks.push({ type: "add", path, contents: parsed.content })
index = parsed.next index = parsed.next
@ -42,28 +45,32 @@ export function parse(patchText: string): ReadonlyArray<Hunk> {
} }
if (line.startsWith("*** Delete File:")) { if (line.startsWith("*** Delete File:")) {
const path = line.slice("*** Delete File:".length).trim() const path = line.slice("*** Delete File:".length).trim()
if (!path) throw new Error("Invalid delete file path") if (!path) {
index++
continue
}
hunks.push({ type: "delete", path }) hunks.push({ type: "delete", path })
index++ index++
continue continue
} }
if (line.startsWith("*** Update File:")) { if (line.startsWith("*** Update File:")) {
const path = line.slice("*** Update File:".length).trim() const path = line.slice("*** Update File:".length).trim()
if (!path) throw new Error("Invalid update file path") if (!path) {
index++
continue
}
let next = index + 1 let next = index + 1
let movePath: string | undefined let movePath: string | undefined
if (lines[next]?.startsWith("*** Move to:")) { if (lines[next]?.startsWith("*** Move to:")) {
movePath = lines[next]!.slice("*** Move to:".length).trim() movePath = lines[next]!.slice("*** Move to:".length).trim()
if (!movePath) throw new Error("Invalid move file path")
next++ next++
} }
const parsed = parseUpdate(lines, next) const parsed = parseUpdate(lines, next)
if (parsed.chunks.length === 0) throw new Error(`Invalid update hunk for ${path}: expected at least one @@ chunk`)
hunks.push({ type: "update", path, movePath, chunks: parsed.chunks }) hunks.push({ type: "update", path, movePath, chunks: parsed.chunks })
index = parsed.next index = parsed.next
continue continue
} }
throw new Error(`Invalid patch line: ${line}`) index++
} }
return hunks return hunks
} }
@ -89,8 +96,7 @@ function parseAdd(lines: ReadonlyArray<string>, start: number) {
const content: string[] = [] const content: string[] = []
let index = start let index = start
while (index < lines.length && !lines[index]!.startsWith("***")) { while (index < lines.length && !lines[index]!.startsWith("***")) {
if (!lines[index]!.startsWith("+")) throw new Error(`Invalid add file line: ${lines[index]}`) if (lines[index]!.startsWith("+")) content.push(lines[index]!.slice(1))
content.push(lines[index]!.slice(1))
index++ index++
} }
return { content: content.join("\n"), next: index } return { content: content.join("\n"), next: index }
@ -101,27 +107,21 @@ function parseUpdate(lines: ReadonlyArray<string>, start: number) {
let index = start let index = start
while (index < lines.length && !lines[index]!.startsWith("***")) { while (index < lines.length && !lines[index]!.startsWith("***")) {
if (!lines[index]!.startsWith("@@")) { if (!lines[index]!.startsWith("@@")) {
throw new Error(`Invalid update file line: ${lines[index]}`) index++
continue
} }
const changeContext = lines[index]!.slice(2).trim() || undefined const changeContext = lines[index]!.slice(2).trim() || undefined
const oldLines: string[] = [] const oldLines: string[] = []
const newLines: string[] = [] const newLines: string[] = []
let endOfFile = false let endOfFile = false
index++ index++
while (index < lines.length && !lines[index]!.startsWith("@@")) { while (index < lines.length && !lines[index]!.startsWith("@@") && !lines[index]!.startsWith("***")) {
const line = lines[index]! const line = lines[index]!
if (line === "*** End of File") {
endOfFile = true
index++
break
}
if (line.startsWith("***")) break
if (line.startsWith(" ")) { if (line.startsWith(" ")) {
oldLines.push(line.slice(1)) oldLines.push(line.slice(1))
newLines.push(line.slice(1)) newLines.push(line.slice(1))
} else if (line.startsWith("-")) oldLines.push(line.slice(1)) } else if (line.startsWith("-")) oldLines.push(line.slice(1))
else if (line.startsWith("+")) newLines.push(line.slice(1)) else if (line.startsWith("+")) newLines.push(line.slice(1))
else throw new Error(`Invalid update chunk line: ${line}`)
index++ index++
} }
chunks.push({ oldLines, newLines, changeContext, endOfFile: endOfFile || undefined }) chunks.push({ oldLines, newLines, changeContext, endOfFile: endOfFile || undefined })

View file

@ -5,7 +5,7 @@ You are an interactive CLI tool that helps users with software engineering tasks
## Editing constraints ## Editing constraints
- Default to ASCII when editing or creating files. Only introduce non-ASCII or other Unicode characters when there is a clear justification and the file already uses them. - Default to ASCII when editing or creating files. Only introduce non-ASCII or other Unicode characters when there is a clear justification and the file already uses them.
- Only add comments if they are necessary to make a non-obvious block easier to understand. - Only add comments if they are necessary to make a non-obvious block easier to understand.
- Try to use apply_patch for single file edits, but it is fine to explore other options to make the edit if it does not work well. Do not use apply_patch for changes that are auto-generated (i.e. generating package.json or running a lint or format command like gofmt) or when scripting is more efficient (such as search and replacing a string across a codebase). - Try to use patch for single file edits, but it is fine to explore other options to make the edit if it does not work well. Do not use patch for changes that are auto-generated (i.e. generating package.json or running a lint or format command like gofmt) or when scripting is more efficient (such as search and replacing a string across a codebase).
## Tool usage ## Tool usage
- Prefer specialized tools over shell for file operations: - Prefer specialized tools over shell for file operations:

View file

@ -24,8 +24,8 @@ If you notice unexpected changes in the worktree or staging area that you did no
- Default to ASCII when editing or creating files. Only introduce non-ASCII or other Unicode characters when there is a clear justification and the file already uses them. - Default to ASCII when editing or creating files. Only introduce non-ASCII or other Unicode characters when there is a clear justification and the file already uses them.
- Add succinct code comments that explain what is going on if code is not self-explanatory. You should not add comments like "Assigns the value to the variable", but a brief comment might be useful ahead of a complex code block that the user would otherwise have to spend time parsing out. Usage of these comments should be rare. - Add succinct code comments that explain what is going on if code is not self-explanatory. You should not add comments like "Assigns the value to the variable", but a brief comment might be useful ahead of a complex code block that the user would otherwise have to spend time parsing out. Usage of these comments should be rare.
- Always use apply_patch for manual code edits. Do not use cat or any other commands when creating or editing files. Formatting commands or bulk edits don't need to be done with apply_patch. - Always use patch for manual code edits. Do not use cat or any other commands when creating or editing files. Formatting commands or bulk edits don't need to be done with patch.
- Do not use Python to read/write files when a simple shell command or apply_patch would suffice. - Do not use Python to read/write files when a simple shell command or patch would suffice.
- You may be in a dirty git worktree. - You may be in a dirty git worktree.
* NEVER revert existing changes you did not make unless explicitly requested, since these changes were made by the user. * NEVER revert existing changes you did not make unless explicitly requested, since these changes were made by the user.
* If asked to make a commit or code edits and there are unrelated changes to your work or changes that you didn't make in those files, don't revert those changes. * If asked to make a commit or code edits and there are unrelated changes to your work or changes that you didn't make in those files, don't revert those changes.

View file

@ -5,12 +5,13 @@ import { ToolFailure } from "@opencode-ai/ai"
import { FileDiff } from "@opencode-ai/schema/file-diff" import { FileDiff } from "@opencode-ai/schema/file-diff"
import { createTwoFilesPatch, diffLines } from "diff" import { createTwoFilesPatch, diffLines } from "diff"
import { Effect, Schema } from "effect" import { Effect, Schema } from "effect"
import { FileMutation } from "../file-mutation" import path from "path"
import { FSUtil } from "../fs-util" import { FSUtil } from "../fs-util"
import { LocationMutation } from "../location-mutation" import { Location } from "../location"
import { Patch } from "../patch" import { Patch } from "../patch"
import { PermissionV2 } from "../permission" import { PermissionV2 } from "../permission"
import { Tool } from "./tool" import { Tool } from "./tool"
import DESCRIPTION from "./patch.txt"
export const name = "patch" export const name = "patch"
@ -34,7 +35,7 @@ export type Output = typeof Output.Type
export const toModelOutput = (output: Output) => export const toModelOutput = (output: Output) =>
[ [
"Applied patch sequentially:", "Success. Updated the following files:",
...output.applied.map( ...output.applied.map(
(item) => `${item.type === "add" ? "A" : item.type === "delete" ? "D" : "M"} ${item.resource}`, (item) => `${item.type === "add" ? "A" : item.type === "delete" ? "D" : "M"} ${item.resource}`,
), ),
@ -42,24 +43,32 @@ export const toModelOutput = (output: Output) =>
type Prepared = type Prepared =
| (Extract<Patch.Hunk, { readonly type: "add" | "delete" }> & { | (Extract<Patch.Hunk, { readonly type: "add" | "delete" }> & {
readonly target: LocationMutation.Target readonly target: Target
readonly before: string readonly before: string
readonly after: string readonly after: string
}) })
| (Extract<Patch.Hunk, { readonly type: "update" }> & { | (Extract<Patch.Hunk, { readonly type: "update" }> & {
readonly target: LocationMutation.Target readonly target: Target
readonly source: Uint8Array
readonly content: string readonly content: string
readonly before: string readonly before: string
readonly after: string readonly after: string
readonly moveTarget?: Target
}) })
interface Target {
readonly canonical: string
readonly resource: string
readonly externalDirectory?: {
readonly directory: string
readonly resource: string
}
}
export const Plugin = { export const Plugin = {
id: "opencode.tool.patch", id: "opencode.tool.patch",
effect: Effect.fn("PatchTool.Plugin")(function* (ctx: PluginContext) { effect: Effect.fn("PatchTool.Plugin")(function* (ctx: PluginContext) {
const mutation = yield* LocationMutation.Service
const files = yield* FileMutation.Service
const fs = yield* FSUtil.Service const fs = yield* FSUtil.Service
const location = yield* Location.Service
const permission = yield* PermissionV2.Service const permission = yield* PermissionV2.Service
yield* ctx.tool yield* ctx.tool
@ -68,8 +77,7 @@ export const Plugin = {
name, name,
Tool.withPermission( Tool.withPermission(
Tool.make({ Tool.make({
description: description: DESCRIPTION,
"Apply one patch containing add, update, and delete file operations. All targets are resolved and approved before target contents are read. Operations apply sequentially; if a later operation fails, earlier operations remain applied and the failure reports them explicitly. Moves and atomic rollback are not supported yet.",
input: Input, input: Input,
output: Output, output: Output,
toModelOutput: ({ output }) => [{ type: "text", text: toModelOutput(output) }], toModelOutput: ({ output }) => [{ type: "text", text: toModelOutput(output) }],
@ -88,100 +96,163 @@ export const Plugin = {
messageID: context.messageID, messageID: context.messageID,
callID: context.callID, callID: context.callID,
} }
if (!input.patchText.trim()) return yield* new ToolFailure({ message: "patchText is required" }) if (!input.patchText) return yield* new ToolFailure({ message: "patchText is required" })
const hunks = yield* Effect.try({ const hunks = yield* Effect.try({
try: () => Patch.parse(input.patchText), try: () => Patch.parse(input.patchText),
catch: (cause) => new ToolFailure({ message: `patch verification failed: ${String(cause)}` }), catch: (cause) => new ToolFailure({ message: `patch verification failed: ${String(cause)}` }),
}) })
if (hunks.length === 0) return yield* new ToolFailure({ message: "patch rejected: empty patch" }) if (hunks.length === 0) {
const move = hunks.find((hunk) => hunk.type === "update" && hunk.movePath !== undefined) const normalized = input.patchText.replace(/\r\n/g, "\n").replace(/\r/g, "\n").trim()
if (move) return yield* new ToolFailure({ message: "patch moves are not supported yet" }) if (normalized === "*** Begin Patch\n*** End Patch") {
return yield* new ToolFailure({ message: "patch rejected: empty patch" })
const targets: Array<{ readonly hunk: Patch.Hunk; readonly target: LocationMutation.Target }> = [] }
for (const hunk of hunks) return yield* new ToolFailure({ message: "patch verification failed: no hunks found" })
targets.push({ hunk, target: yield* mutation.resolve({ path: hunk.path, kind: "file" }) })
const externalDirectories = new Map<string, LocationMutation.ExternalDirectoryAuthorization>()
for (const { target } of targets) {
const external = target.externalDirectory
if (external) externalDirectories.set(external.resource, external)
} }
for (const external of externalDirectories.values()) {
yield* permission.assert({
...LocationMutation.externalDirectoryPermission(external),
sessionID: context.sessionID,
agent: context.agent,
source,
})
}
yield* permission.assert({
action: "edit",
resources: [...new Set(targets.map(({ target }) => target.resource))],
save: ["*"],
sessionID: context.sessionID,
agent: context.agent,
source,
})
const prepared: Prepared[] = [] const prepared: Prepared[] = []
for (const { hunk, target } of targets) { const targets: Target[] = []
for (const hunk of hunks) {
yield* Effect.gen(function* () { 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.canonical,
parentDir: target.externalDirectory.directory,
},
sessionID: context.sessionID,
agent: context.agent,
source,
})
}
if (hunk.type === "add") { if (hunk.type === "add") {
prepared.push({ prepared.push({
...hunk, ...hunk,
target, target,
before: "", before: "",
after: after: (hunk.contents.endsWith("\n") || hunk.contents === ""
hunk.contents.endsWith("\n") || hunk.contents === "" ? hunk.contents : `${hunk.contents}\n`, ? hunk.contents
: `${hunk.contents}\n`
).replace(/^\uFEFF/, ""),
}) })
return return
} }
if ((yield* fs.stat(target.canonical)).type !== "File") yield* fail(hunk.path)
const source = yield* fs.readFile(target.canonical)
const original = new TextDecoder("utf-8", { ignoreBOM: true }).decode(source)
const before = original.replace(/^\uFEFF/, "")
if (hunk.type === "delete") { if (hunk.type === "delete") {
prepared.push({ ...hunk, target, before, after: "" }) const content = yield* fs.readFile(target.canonical).pipe(
Effect.mapError(
(error) =>
new ToolFailure({
message: `patch verification failed: ${error instanceof Error ? error.message : String(error)}`,
}),
),
)
const original = new TextDecoder("utf-8", { ignoreBOM: true }).decode(content)
prepared.push({ ...hunk, target, before: original.replace(/^\uFEFF/, ""), after: "" })
return return
} }
const update = Patch.derive(hunk.path, hunk.chunks, original) const stats = yield* fs
.stat(target.canonical)
.pipe(Effect.catch(() => Effect.succeed(undefined)))
if (!stats || stats.type === "Directory") {
return yield* new ToolFailure({
message: `patch verification failed: Failed to read file to update: ${target.canonical}`,
})
}
const content = yield* fs.readFile(target.canonical)
const original = new TextDecoder("utf-8", { ignoreBOM: true }).decode(content)
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)}` }),
})
const moveTarget = hunk.movePath ? resolveTarget(location, hunk.movePath) : undefined
if (moveTarget?.externalDirectory) {
yield* permission.assert({
action: "external_directory",
resources: [moveTarget.externalDirectory.resource],
save: [moveTarget.externalDirectory.resource],
metadata: {
filepath: moveTarget.canonical,
parentDir: moveTarget.externalDirectory.directory,
},
sessionID: context.sessionID,
agent: context.agent,
source,
})
}
prepared.push({ prepared.push({
...hunk, ...hunk,
target, target,
source,
content: Patch.joinBom(update.content, update.bom), content: Patch.joinBom(update.content, update.bom),
before, before,
after: update.content, after: update.content,
moveTarget,
}) })
}).pipe(Effect.mapError((error) => fail(hunk.path, error))) }).pipe(Effect.mapError((error) => (error instanceof ToolFailure ? error : fail(hunk.path, error))))
} }
const patchFiles = prepared.map(patchFile) const patchFiles = prepared.map(patchFile)
yield* permission.assert({
action: "edit",
resources: [...new Set(targets.map((target) => target.resource))],
save: ["*"],
metadata: {
filepath: targets.map((target) => target.resource).join(", "),
diff: patchFiles.map((file) => `${file.patch}\n`).join(""),
files: patchFiles,
},
sessionID: context.sessionID,
agent: context.agent,
source,
})
yield* Effect.forEach( yield* Effect.forEach(
prepared, prepared,
(change) => (change) =>
Effect.gen(function* () { Effect.gen(function* () {
if (change.type === "add") { if (change.type === "add") {
const result = yield* files.create({ yield* fs.writeWithDirs(
target: change.target, change.target.canonical,
content: change.contents.endsWith("\n") || change.contents === ""
change.contents.endsWith("\n") || change.contents === "" ? change.contents
? change.contents : `${change.contents}\n`,
: `${change.contents}\n`, )
applied.push({
type: change.type,
resource: change.target.resource,
target: change.target.canonical,
}) })
applied.push({ type: change.type, resource: result.resource, target: result.target })
return return
} }
if (change.type === "delete") { if (change.type === "delete") {
const result = yield* files.remove({ target: change.target }) yield* fs.remove(change.target.canonical)
applied.push({ type: change.type, resource: result.resource, target: result.target }) applied.push({
type: change.type,
resource: change.target.resource,
target: change.target.canonical,
})
return return
} }
const result = yield* files.writeIfUnchanged({ if (change.moveTarget) {
target: change.target, yield* fs.writeWithDirs(change.moveTarget.canonical, change.content)
expected: change.source, yield* fs.remove(change.target.canonical)
content: change.content, applied.push({
type: change.type,
resource: change.moveTarget.resource,
target: change.moveTarget.canonical,
})
return
}
yield* fs.writeWithDirs(change.target.canonical, change.content)
applied.push({
type: change.type,
resource: change.target.resource,
target: change.target.canonical,
}) })
applied.push({ type: change.type, resource: result.resource, target: result.target })
}).pipe(Effect.mapError((error) => fail(change.path, error))), }).pipe(Effect.mapError((error) => fail(change.path, error))),
{ discard: true }, { discard: true },
) )
@ -199,7 +270,7 @@ export const Plugin = {
yield* ctx.session.hook("context", (event) => yield* ctx.session.hook("context", (event) =>
Effect.sync(() => { Effect.sync(() => {
const usePatch = const usePatch =
event.model.providerID.toLowerCase() === "openai" || event.model.id.toLowerCase().includes("gpt") event.model.id.includes("gpt-") && !event.model.id.includes("oss") && !event.model.id.includes("gpt-4")
if (usePatch) { if (usePatch) {
delete event.tools.edit delete event.tools.edit
delete event.tools.write delete event.tools.write
@ -212,17 +283,74 @@ export const Plugin = {
} }
function patchFile(change: Prepared): typeof FileDiff.Info.Type { function patchFile(change: Prepared): typeof FileDiff.Info.Type {
const counts = diffLines(change.before, change.after).reduce( const target = (change.type === "update" ? change.moveTarget : undefined)?.resource ?? change.target.resource
(result, item) => ({ const patch = trimDiff(
additions: result.additions + (item.added ? (item.count ?? 0) : 0), createTwoFilesPatch(change.target.canonical, change.target.canonical, change.before, change.after),
deletions: result.deletions + (item.removed ? (item.count ?? 0) : 0),
}),
{ additions: 0, deletions: 0 },
) )
const counts =
change.type === "delete"
? { additions: 0, deletions: change.before.split("\n").length }
: diffLines(change.before, change.after).reduce(
(result, item) => ({
additions: result.additions + (item.added ? (item.count ?? 0) : 0),
deletions: result.deletions + (item.removed ? (item.count ?? 0) : 0),
}),
{ additions: 0, deletions: 0 },
)
return { return {
file: change.target.resource, file: target,
patch: createTwoFilesPatch(change.target.resource, change.target.resource, change.before, change.after), patch,
status: change.type === "add" ? "added" : change.type === "delete" ? "deleted" : "modified", status: change.type === "add" ? "added" : change.type === "delete" ? "deleted" : "modified",
...counts, ...counts,
} }
} }
function trimDiff(diff: string) {
const lines = diff.split("\n")
const content = lines.filter(
(line) =>
(line.startsWith("+") || line.startsWith("-") || line.startsWith(" ")) &&
!line.startsWith("---") &&
!line.startsWith("+++"),
)
if (content.length === 0) return diff
const indent = content.reduce((result, line) => {
const value = line.slice(1)
if (value.trim().length === 0) return result
return Math.min(result, value.match(/^(\s*)/)?.[1].length ?? result)
}, Infinity)
if (indent === Infinity || indent === 0) return diff
return lines
.map((line) => {
if (
(line.startsWith("+") || line.startsWith("-") || line.startsWith(" ")) &&
!line.startsWith("---") &&
!line.startsWith("+++")
) {
return line[0] + line.slice(1 + indent)
}
return line
})
.join("\n")
}
function resolveTarget(location: Location.Interface, value: string): Target {
const canonical =
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, canonical) &&
(location.project.directory === projectRoot || !FSUtil.contains(location.project.directory, canonical))
const directory = path.dirname(canonical)
const resource =
process.platform === "win32"
? FSUtil.normalizePathPattern(path.join(directory, "*"))
: path.join(directory, "*").replaceAll("\\", "/")
return {
canonical,
resource: path.relative(location.project.directory, canonical).replaceAll("\\", "/") || ".",
externalDirectory: external ? { directory, resource } : undefined,
}
}

View file

@ -0,0 +1,33 @@
Use the `patch` tool to edit files. Your patch language is a strippeddown, fileoriented diff format designed to be easy to parse and safe to apply. You can think of it as a highlevel envelope:
*** Begin Patch
[ one or more file sections ]
*** End Patch
Within that envelope, you get a sequence of file operations.
You MUST include a header to specify the action you are taking.
Each operation starts with one of three headers:
*** Add File: <path> - create a new file. Every following line is a + line (the initial contents).
*** Delete File: <path> - remove an existing file. Nothing follows.
*** Update File: <path> - patch an existing file in place (optionally with a rename).
Example patch:
```
*** Begin Patch
*** Add File: hello.txt
+Hello world
*** Update File: src/app.py
*** Move to: src/main.py
@@ def greet():
-print("Hi")
+print("Hello, world!")
*** Delete File: obsolete.txt
*** End Patch
```
It is important to remember:
- You must include a header with your intended action (Add/Delete/Update)
- You must prefix new lines with `+` even when creating a new file

File diff suppressed because one or more lines are too long

View file

@ -19,18 +19,88 @@ describe("Patch", () => {
]) ])
}) })
test("parses a file move", () => {
expect(
Patch.parse(
"*** Begin Patch\n*** Update File: old.txt\n*** Move to: new.txt\n@@\n-old\n+new\n*** End Patch",
),
).toEqual([
{
type: "update",
path: "old.txt",
movePath: "new.txt",
chunks: [{ oldLines: ["old"], newLines: ["new"], changeContext: undefined, endOfFile: undefined }],
},
])
})
test("rejects invalid patch format", () => {
expect(() => Patch.parse("This is not a valid patch")).toThrow("Invalid patch format")
})
test("strips a heredoc wrapper", () => { 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(Patch.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" }, { 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([
{ type: "add", path: "add.txt", contents: "added" },
])
})
test("derives fuzzy line updates while preserving BOM", () => { test("derives fuzzy line updates while preserving BOM", () => {
const update = Patch.derive("update.txt", [{ oldLines: [" old "], newLines: ["new"] }], "\uFEFFold\n") const update = Patch.derive("update.txt", [{ oldLines: [" old "], newLines: ["new"] }], "\uFEFFold\n")
expect(update).toEqual({ content: "new\n", bom: true }) expect(update).toEqual({ content: "new\n", bom: true })
expect(Patch.joinBom(update.content, update.bom)).toBe("\uFEFFnew\n") expect(Patch.joinBom(update.content, update.bom)).toBe("\uFEFFnew\n")
}) })
test("derives multiple update chunks", () => {
expect(
Patch.derive(
"update.txt",
[
{ oldLines: ["line 2"], newLines: ["LINE 2"] },
{ oldLines: ["line 4"], newLines: ["LINE 4"] },
],
"line 1\nline 2\nline 3\nline 4\n",
).content,
).toBe("line 1\nLINE 2\nline 3\nLINE 4\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",
)
})
test("disambiguates updates with change context", () => {
expect(
Patch.derive(
"update.txt",
[{ oldLines: ["x=10"], newLines: ["x=11"], changeContext: "fn b" }],
"fn a\nx=10\nfn b\nx=10\n",
).content,
).toBe("fn a\nx=10\nfn b\nx=11\n")
})
test("matches leading, trailing, and Unicode punctuation differences", () => {
expect(Patch.derive("leading.txt", [{ oldLines: ["line"], newLines: ["next"] }], " line\n").content).toBe(
"next\n",
)
expect(Patch.derive("trailing.txt", [{ oldLines: ["line"], newLines: ["next"] }], "line \n").content).toBe(
"next\n",
)
expect(
Patch.derive('unicode.txt', [{ oldLines: ['He said "hello"'], newLines: ['He said "hi"'] }], 'He said “hello”\n')
.content,
).toBe('He said "hi"\n')
})
test("matches EOF-anchored chunks from the end", () => { test("matches EOF-anchored chunks from the end", () => {
expect( expect(
Patch.derive( Patch.derive(
@ -41,28 +111,15 @@ describe("Patch", () => {
).toBe("marker\nmiddle\nmarker changed\nend\n") ).toBe("marker\nmiddle\nmarker changed\nend\n")
}) })
test("parses the EOF marker inside update chunks", () => { test("matches V1 lenient parsing of malformed hunk bodies", () => {
expect( expect(Patch.parse("*** Begin Patch\n*** Add File: add.txt\nmissing plus\n*** End Patch")).toEqual([
Patch.parse("*** Begin Patch\n*** Update File: update.txt\n@@\n-last\n+end\n*** End of File\n*** End Patch"), { type: "add", path: "add.txt", contents: "" },
).toEqual([ ])
{ expect(Patch.parse("*** Begin Patch\n*** Update File: update.txt\n*** End Patch")).toEqual([
type: "update", { type: "update", path: "update.txt", movePath: undefined, chunks: [] },
path: "update.txt", ])
movePath: undefined, expect(Patch.parse("*** Begin Patch\n*** Delete File: delete.txt\nunexpected body\n*** End Patch")).toEqual([
chunks: [{ oldLines: ["last"], newLines: ["end"], changeContext: undefined, endOfFile: true }], { type: "delete", path: "delete.txt" },
},
]) ])
}) })
test("rejects malformed hunk bodies", () => {
expect(() => Patch.parse("*** Begin Patch\n*** Add File: add.txt\nmissing plus\n*** End Patch")).toThrow(
"Invalid add file line",
)
expect(() => Patch.parse("*** Begin Patch\n*** Update File: update.txt\n*** End Patch")).toThrow(
"expected at least one @@ chunk",
)
expect(() => Patch.parse("*** Begin Patch\n*** Delete File: delete.txt\nunexpected body\n*** End Patch")).toThrow(
"Invalid patch line",
)
})
}) })

View file

@ -1,13 +1,11 @@
import fs from "fs/promises" import fs from "fs/promises"
import path from "path" import path from "path"
import { describe, expect } from "bun:test" import { describe, expect } from "bun:test"
import { Deferred, Effect, Exit, Fiber, Layer } from "effect" import { Effect, Exit, Layer, Schema } from "effect"
import { AppNodeBuilder } from "@opencode-ai/core/effect/app-node-builder" import { AppNodeBuilder } from "@opencode-ai/core/effect/app-node-builder"
import { LayerNode } from "@opencode-ai/core/effect/layer-node" import { LayerNode } from "@opencode-ai/core/effect/layer-node"
import { FileMutation } from "@opencode-ai/core/file-mutation"
import { FSUtil } from "@opencode-ai/core/fs-util" import { FSUtil } from "@opencode-ai/core/fs-util"
import { Location } from "@opencode-ai/core/location" import { Location } from "@opencode-ai/core/location"
import { LocationMutation } from "@opencode-ai/core/location-mutation"
import { PermissionV2 } from "@opencode-ai/core/permission" import { PermissionV2 } from "@opencode-ai/core/permission"
import { AbsolutePath } from "@opencode-ai/core/schema" import { AbsolutePath } from "@opencode-ai/core/schema"
import { SessionV2 } from "@opencode-ai/core/session" import { SessionV2 } from "@opencode-ai/core/session"
@ -23,7 +21,7 @@ import { toolIdentity, executeTool, registerToolPlugin, settleTool, toolDefiniti
const patchToolNode = makeLocationNode({ const patchToolNode = makeLocationNode({
name: "test/patch-tool-plugin", name: "test/patch-tool-plugin",
layer: Layer.effectDiscard(registerToolPlugin(PatchTool.Plugin)), layer: Layer.effectDiscard(registerToolPlugin(PatchTool.Plugin)),
deps: [ToolRegistry.toolsNode, LocationMutation.node, FileMutation.node, FSUtil.node, PermissionV2.node], deps: [ToolRegistry.toolsNode, FSUtil.node, Location.node, PermissionV2.node],
}) })
const sessionID = SessionV2.ID.make("ses_patch_tool_test") const sessionID = SessionV2.ID.make("ses_patch_tool_test")
@ -32,9 +30,6 @@ let denyAction: string | undefined
let failRemoveTarget: string | undefined let failRemoveTarget: string | undefined
let readsBeforeEditApproval = 0 let readsBeforeEditApproval = 0
let editApproved = false let editApproved = false
let blockRemoveTarget: string | undefined
let removeStarted: Deferred.Deferred<void> | undefined
let releaseRemove: Deferred.Deferred<void> | undefined
let afterEditApproval = (): Effect.Effect<void> => Effect.void let afterEditApproval = (): Effect.Effect<void> => Effect.void
const permission = Layer.succeed( const permission = Layer.succeed(
@ -72,9 +67,6 @@ const reset = () => {
failRemoveTarget = undefined failRemoveTarget = undefined
readsBeforeEditApproval = 0 readsBeforeEditApproval = 0
editApproved = false editApproved = false
blockRemoveTarget = undefined
removeStarted = undefined
releaseRemove = undefined
afterEditApproval = () => Effect.void afterEditApproval = () => Effect.void
} }
@ -90,21 +82,25 @@ const filesystem = Layer.effect(
}).pipe(Effect.andThen(fs.readFile(target))), }).pipe(Effect.andThen(fs.readFile(target))),
remove: (target, options) => { remove: (target, options) => {
if (failRemoveTarget && path.basename(target) === failRemoveTarget) return Effect.die("forced remove failure") if (failRemoveTarget && path.basename(target) === failRemoveTarget) return Effect.die("forced remove failure")
if (blockRemoveTarget && path.basename(target) === blockRemoveTarget && removeStarted && releaseRemove)
return Deferred.succeed(removeStarted, undefined).pipe(
Effect.andThen(Deferred.await(releaseRemove)),
Effect.andThen(fs.remove(target, options)),
)
return fs.remove(target, options) return fs.remove(target, options)
}, },
}) })
}), }),
).pipe(Layer.provide(LayerNode.compile(FSUtil.node))) ).pipe(Layer.provide(LayerNode.compile(FSUtil.node)))
const withTool = <A, E, R>(directory: string, body: (registry: ToolRegistry.Interface) => Effect.Effect<A, E, R>) => { const withTool = <A, E, R>(
directory: string,
body: (registry: ToolRegistry.Interface) => Effect.Effect<A, E, R>,
projectDirectory = directory,
) => {
const activeLocation = Layer.succeed( const activeLocation = Layer.succeed(
Location.Service, Location.Service,
Location.Service.of(location({ directory: AbsolutePath.make(directory) })), Location.Service.of(
location(
{ directory: AbsolutePath.make(directory) },
{ projectDirectory: AbsolutePath.make(projectDirectory) },
),
),
) )
return Effect.gen(function* () { return Effect.gen(function* () {
return yield* body(yield* ToolRegistry.Service) return yield* body(yield* ToolRegistry.Service)
@ -114,8 +110,6 @@ const withTool = <A, E, R>(directory: string, body: (registry: ToolRegistry.Inte
LayerNode.group([ LayerNode.group([
ToolRegistry.node, ToolRegistry.node,
ToolRegistry.toolsNode, ToolRegistry.toolsNode,
LocationMutation.node,
FileMutation.node,
patchToolNode, patchToolNode,
]), ]),
[ [
@ -143,6 +137,15 @@ const exists = (target: string) =>
), ),
) )
const it = testEffect(Layer.empty) const it = testEffect(Layer.empty)
const withTempTool = <A, E, R>(body: (directory: string, registry: ToolRegistry.Interface) => Effect.Effect<A, E, R>) =>
Effect.acquireUseRelease(
Effect.promise(() => tmpdir()),
(tmp) => {
reset()
return withTool(tmp.path, (registry) => body(tmp.path, registry))
},
(tmp) => Effect.promise(() => tmp[Symbol.asyncDispose]()),
)
describe("PatchTool", () => { describe("PatchTool", () => {
it.live("registers and sequentially applies add, update, and delete hunks", () => it.live("registers and sequentially applies add, update, and delete hunks", () =>
@ -167,8 +170,9 @@ describe("PatchTool", () => {
) )
expect(settled.result).toEqual({ expect(settled.result).toEqual({
type: "text", type: "text",
value: "Applied patch sequentially:\nA nested/new.txt\nM update.txt\nD remove.txt", value: "Success. Updated the following files:\nA nested/new.txt\nM update.txt\nD remove.txt",
}) })
if (process.platform === "win32") expect(settled.result.value).not.toContain("\\")
expect(settled.output?.structured).toMatchObject({ expect(settled.output?.structured).toMatchObject({
applied: [ applied: [
{ type: "add", resource: "nested/new.txt" }, { type: "add", resource: "nested/new.txt" },
@ -194,15 +198,25 @@ describe("PatchTool", () => {
file: "remove.txt", file: "remove.txt",
status: "deleted", status: "deleted",
additions: 0, additions: 0,
deletions: 1, deletions: 2,
patch: expect.stringContaining("-remove"), patch: expect.stringContaining("-remove"),
}, },
], ],
}) })
expect(assertions).toMatchObject([ expect(assertions).toMatchObject([
{ sessionID, action: "edit", resources: ["nested/new.txt", "update.txt", "remove.txt"], save: ["*"] }, {
sessionID,
action: "edit",
resources: ["nested/new.txt", "update.txt", "remove.txt"],
save: ["*"],
metadata: {
filepath: "nested/new.txt, update.txt, remove.txt",
diff: expect.stringContaining("Index:"),
files: expect.any(Array),
},
},
]) ])
expect(readsBeforeEditApproval).toBe(0) expect(readsBeforeEditApproval).toBe(2)
expect(yield* Effect.promise(() => fs.readFile(path.join(tmp.path, "nested/new.txt"), "utf8"))).toBe( expect(yield* Effect.promise(() => fs.readFile(path.join(tmp.path, "nested/new.txt"), "utf8"))).toBe(
"created\n", "created\n",
) )
@ -217,7 +231,7 @@ describe("PatchTool", () => {
), ),
) )
it.live("rejects moves before applying any hunk", () => it.live("moves and updates a file", () =>
Effect.acquireUseRelease( Effect.acquireUseRelease(
Effect.promise(() => tmpdir()), Effect.promise(() => tmpdir()),
(tmp) => { (tmp) => {
@ -234,9 +248,17 @@ describe("PatchTool", () => {
"*** Begin Patch\n*** Add File: created.txt\n+created\n*** Update File: old.txt\n*** Move to: moved.txt\n@@\n-before\n+after\n*** End Patch", "*** Begin Patch\n*** Add File: created.txt\n+created\n*** Update File: old.txt\n*** Move to: moved.txt\n@@\n-before\n+after\n*** End Patch",
), ),
), ),
).toEqual({ type: "error", value: "patch moves are not supported yet" }) ).toEqual({
expect(yield* exists(path.join(tmp.path, "created.txt"))).toBe(false) type: "text",
expect(assertions).toEqual([]) value: "Success. Updated the following files:\nA created.txt\nM moved.txt",
})
expect(yield* exists(source)).toBe(false)
expect(yield* Effect.promise(() => fs.readFile(path.join(tmp.path, "moved.txt"), "utf8"))).toBe(
"after\n",
)
expect(yield* Effect.promise(() => fs.readFile(path.join(tmp.path, "created.txt"), "utf8"))).toBe(
"created\n",
)
}), }),
), ),
), ),
@ -246,7 +268,387 @@ describe("PatchTool", () => {
), ),
) )
it.live("approves an external directory and the batch before reading external update content", () => it.live("moves a file over an existing destination", () =>
Effect.acquireUseRelease(
Effect.promise(() => tmpdir()),
(tmp) => {
reset()
const source = path.join(tmp.path, "old.txt")
const destination = path.join(tmp.path, "nested", "moved.txt")
return Effect.promise(() =>
Promise.all([
fs.writeFile(source, "before\n"),
fs.mkdir(path.dirname(destination), { recursive: true }).then(() => fs.writeFile(destination, "existing\n")),
]),
).pipe(
Effect.andThen(
withTool(tmp.path, (registry) =>
Effect.gen(function* () {
expect(
yield* executeTool(
registry,
call(
"*** Begin Patch\n*** Update File: old.txt\n*** Move to: nested/moved.txt\n@@\n-before\n+after\n*** End Patch",
),
),
).toMatchObject({ type: "text" })
expect(yield* exists(source)).toBe(false)
expect(yield* Effect.promise(() => fs.readFile(destination, "utf8"))).toBe("after\n")
}),
),
),
)
},
(tmp) => Effect.promise(() => tmp[Symbol.asyncDispose]()),
),
)
it.live("moves a symlink without deleting its target", () =>
withTempTool((directory, registry) =>
Effect.gen(function* () {
if (process.platform === "win32") return
const target = path.join(directory, "target.txt")
const source = path.join(directory, "link.txt")
const moved = path.join(directory, "moved.txt")
yield* Effect.promise(() => fs.writeFile(target, "before\n"))
yield* Effect.promise(() => fs.symlink(target, source))
yield* executeTool(
registry,
call(
"*** Begin Patch\n*** Update File: link.txt\n*** Move to: moved.txt\n@@\n-before\n+after\n*** End Patch",
),
)
expect(yield* exists(source)).toBe(false)
expect(yield* Effect.promise(() => fs.readFile(target, "utf8"))).toBe("before\n")
expect(yield* Effect.promise(() => fs.readFile(moved, "utf8"))).toBe("after\n")
}),
),
)
it.live("includes move file info in structured output", () =>
withTempTool((directory, registry) =>
Effect.gen(function* () {
const source = path.join(directory, "old", "name.txt")
yield* Effect.promise(() => fs.mkdir(path.dirname(source), { recursive: true }))
yield* Effect.promise(() => fs.writeFile(source, "old content\n"))
const settled = yield* settleTool(
registry,
call(
"*** Begin Patch\n*** Update File: old/name.txt\n*** Move to: renamed/dir/name.txt\n@@\n-old content\n+new content\n*** End Patch",
),
)
expect(settled.output?.structured).toMatchObject({
applied: [{ type: "update", resource: "renamed/dir/name.txt" }],
files: [
{
file: "renamed/dir/name.txt",
status: "modified",
patch: expect.stringContaining("-old content\n+new content"),
},
],
})
}),
),
)
it.live("inserts lines with an insert-only hunk", () =>
withTempTool((directory, registry) =>
Effect.gen(function* () {
const target = path.join(directory, "insert-only.txt")
yield* Effect.promise(() => fs.writeFile(target, "alpha\nomega\n"))
yield* executeTool(
registry,
call("*** Begin Patch\n*** Update File: insert-only.txt\n@@\n alpha\n+beta\n omega\n*** End Patch"),
)
expect(yield* Effect.promise(() => fs.readFile(target, "utf8"))).toBe("alpha\nbeta\nomega\n")
}),
),
)
it.live("updates an empty file", () =>
withTempTool((directory, registry) =>
Effect.gen(function* () {
const target = path.join(directory, "empty.txt")
yield* Effect.promise(() => fs.writeFile(target, ""))
yield* executeTool(registry, call("*** Begin Patch\n*** Update File: empty.txt\n@@\n+First line\n*** End Patch"))
expect(yield* Effect.promise(() => fs.readFile(target, "utf8"))).toBe("First line\n")
}),
),
)
it.live("rejects deleting a directory", () =>
withTempTool((directory, registry) =>
Effect.gen(function* () {
yield* Effect.promise(() => fs.mkdir(path.join(directory, "dir")))
expect(
yield* executeTool(registry, call("*** Begin Patch\n*** Delete File: dir\n*** End Patch")),
).toMatchObject({ type: "error" })
expect(yield* exists(path.join(directory, "dir"))).toBe(true)
}),
),
)
it.live("supports an end-of-file anchor", () =>
withTempTool((directory, registry) =>
Effect.gen(function* () {
const target = path.join(directory, "tail.txt")
yield* Effect.promise(() => fs.writeFile(target, "alpha\nlast\n"))
yield* executeTool(
registry,
call(
"*** Begin Patch\n*** Update File: tail.txt\n@@\n-last\n+end\n*** End of File\n*** End Patch",
),
)
expect(yield* Effect.promise(() => fs.readFile(target, "utf8"))).toBe("alpha\nend\n")
}),
),
)
it.live("rejects a missing second chunk context", () =>
withTempTool((directory, registry) =>
Effect.gen(function* () {
const target = path.join(directory, "two-chunks.txt")
yield* Effect.promise(() => fs.writeFile(target, "a\nb\nc\nd\n"))
expect(
yield* executeTool(
registry,
call(
"*** Begin Patch\n*** Update File: two-chunks.txt\n@@\n-b\n+B\n\n-d\n+D\n*** End Patch",
),
),
).toMatchObject({ type: "error" })
expect(yield* Effect.promise(() => fs.readFile(target, "utf8"))).toBe("a\nb\nc\nd\n")
}),
),
)
it.live("requires patchText", () =>
withTempTool((_directory, registry) =>
Effect.gen(function* () {
expect(yield* executeTool(registry, call(""))).toEqual({ type: "error", value: "patchText is required" })
}),
),
)
it.live("rejects invalid patch format", () =>
withTempTool((_directory, registry) =>
Effect.gen(function* () {
expect(yield* executeTool(registry, call("invalid patch"))).toMatchObject({
type: "error",
value: expect.stringContaining("patch verification failed"),
})
}),
),
)
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",
})
}),
),
)
it.live("rejects an invalid hunk header", () =>
withTempTool((_directory, registry) =>
Effect.gen(function* () {
expect(
yield* executeTool(
registry,
call("*** Begin Patch\n*** Frobnicate File: foo\n*** End Patch"),
),
).toEqual({ type: "error", value: "patch verification failed: no hunks found" })
}),
),
)
it.live("applies multiple hunks to one file", () =>
withTempTool((directory, registry) =>
Effect.gen(function* () {
const target = path.join(directory, "multi.txt")
yield* Effect.promise(() => fs.writeFile(target, "a\nb\nc\nd\n"))
yield* executeTool(
registry,
call("*** Begin Patch\n*** Update File: multi.txt\n@@\n-b\n+B\n@@\n-d\n+D\n*** End Patch"),
)
expect(yield* Effect.promise(() => fs.readFile(target, "utf8"))).toBe("a\nB\nc\nD\n")
}),
),
)
it.live("does not invent a first-line diff for BOM files", () =>
withTempTool((directory, registry) =>
Effect.gen(function* () {
const bom = "\uFEFF"
const target = path.join(directory, "example.cs")
yield* Effect.promise(() => fs.writeFile(target, `${bom}using System;\n\nclass Test {}\n`))
const settled = yield* settleTool(
registry,
call(
"*** Begin Patch\n*** Update File: example.cs\n@@\n class Test {}\n+class Next {}\n*** End Patch",
),
)
const output = Schema.decodeUnknownSync(PatchTool.Output)(settled.output?.structured)
expect(output.files[0]?.patch).not.toContain(bom)
expect(output.files[0]?.patch).not.toContain("-using System;")
expect(output.files[0]?.patch).not.toContain("+using System;")
expect(yield* Effect.promise(() => fs.readFile(target, "utf8"))).toBe(
`${bom}using System;\n\nclass Test {}\nclass Next {}\n`,
)
}),
),
)
it.live("appends a trailing newline on update", () =>
withTempTool((directory, registry) =>
Effect.gen(function* () {
const target = path.join(directory, "no-newline.txt")
yield* Effect.promise(() => fs.writeFile(target, "no newline at end"))
yield* executeTool(
registry,
call(
"*** Begin Patch\n*** Update File: no-newline.txt\n@@\n-no newline at end\n+first line\n+second line\n*** End Patch",
),
)
expect(yield* Effect.promise(() => fs.readFile(target, "utf8"))).toBe("first line\nsecond line\n")
}),
),
)
it.live("disambiguates change context with an @@ header", () =>
withTempTool((directory, registry) =>
Effect.gen(function* () {
const target = path.join(directory, "context.txt")
yield* Effect.promise(() => fs.writeFile(target, "fn a\nx=10\ny=2\nfn b\nx=10\ny=20\n"))
yield* executeTool(
registry,
call("*** Begin Patch\n*** Update File: context.txt\n@@ fn b\n-x=10\n+x=11\n*** End Patch"),
)
expect(yield* Effect.promise(() => fs.readFile(target, "utf8"))).toBe(
"fn a\nx=10\ny=2\nfn b\nx=11\ny=20\n",
)
}),
),
)
it.live("parses a heredoc-wrapped patch", () =>
withTempTool((directory, registry) =>
Effect.gen(function* () {
yield* executeTool(
registry,
call("cat <<'EOF'\n*** Begin Patch\n*** Add File: heredoc.txt\n+with cat\n*** End Patch\nEOF"),
)
expect(yield* Effect.promise(() => fs.readFile(path.join(directory, "heredoc.txt"), "utf8"))).toBe(
"with cat\n",
)
}),
),
)
it.live("parses a heredoc-wrapped patch without cat", () =>
withTempTool((directory, registry) =>
Effect.gen(function* () {
yield* executeTool(
registry,
call("<<EOF\n*** Begin Patch\n*** Add File: heredoc.txt\n+without cat\n*** End Patch\nEOF"),
)
expect(yield* Effect.promise(() => fs.readFile(path.join(directory, "heredoc.txt"), "utf8"))).toBe(
"without cat\n",
)
}),
),
)
it.live("matches with trailing whitespace differences", () =>
withTempTool((directory, registry) =>
Effect.gen(function* () {
const target = path.join(directory, "trailing.txt")
yield* Effect.promise(() => fs.writeFile(target, "line1 \nline2\nline3 \n"))
yield* executeTool(
registry,
call("*** Begin Patch\n*** Update File: trailing.txt\n@@\n-line2\n+changed\n*** End Patch"),
)
expect(yield* Effect.promise(() => fs.readFile(target, "utf8"))).toBe("line1 \nchanged\nline3 \n")
}),
),
)
it.live("matches with leading whitespace differences", () =>
withTempTool((directory, registry) =>
Effect.gen(function* () {
const target = path.join(directory, "leading.txt")
yield* Effect.promise(() => fs.writeFile(target, " line1\nline2\n line3\n"))
yield* executeTool(
registry,
call("*** Begin Patch\n*** Update File: leading.txt\n@@\n-line2\n+changed\n*** End Patch"),
)
expect(yield* Effect.promise(() => fs.readFile(target, "utf8"))).toBe(" line1\nchanged\n line3\n")
}),
),
)
it.live("matches with Unicode punctuation differences", () =>
withTempTool((directory, registry) =>
Effect.gen(function* () {
const target = path.join(directory, "unicode.txt")
yield* Effect.promise(() => fs.writeFile(target, "He said “hello”\nsome—dash\nend\n"))
yield* executeTool(
registry,
call(
'*** Begin Patch\n*** Update File: unicode.txt\n@@\n-He said "hello"\n+He said "hi"\n*** End Patch',
),
)
expect(yield* Effect.promise(() => fs.readFile(target, "utf8"))).toBe('He said "hi"\nsome—dash\nend\n')
}),
),
)
it.live("rejects an update with missing context", () =>
withTempTool((directory, registry) =>
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") })
expect(yield* Effect.promise(() => fs.readFile(target, "utf8"))).toBe("line1\nline2\n")
}),
),
)
it.live("rejects an update when the target file is missing", () =>
withTempTool((_directory, registry) =>
Effect.gen(function* () {
expect(
yield* executeTool(
registry,
call("*** Begin Patch\n*** Update File: missing.txt\n@@\n-old\n+new\n*** End Patch"),
),
).toMatchObject({
type: "error",
value: expect.stringContaining("patch verification failed: Failed to read file to update"),
})
}),
),
)
it.live("rejects a delete when the target file is missing", () =>
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") })
}),
),
)
it.live("approves an external directory before reading and requests edit permission afterward", () =>
Effect.acquireUseRelease( Effect.acquireUseRelease(
Effect.promise(() => Promise.all([tmpdir(), tmpdir()])), Effect.promise(() => Promise.all([tmpdir(), tmpdir()])),
([active, outside]) => { ([active, outside]) => {
@ -263,7 +665,7 @@ describe("PatchTool", () => {
), ),
).toMatchObject({ type: "text" }) ).toMatchObject({ type: "text" })
expect(assertions.map((input) => input.action)).toEqual(["external_directory", "edit"]) expect(assertions.map((input) => input.action)).toEqual(["external_directory", "edit"])
expect(readsBeforeEditApproval).toBe(0) expect(readsBeforeEditApproval).toBe(1)
expect(yield* Effect.promise(() => fs.readFile(target, "utf8"))).toBe("after\n") expect(yield* Effect.promise(() => fs.readFile(target, "utf8"))).toBe("after\n")
}), }),
), ),
@ -277,7 +679,106 @@ describe("PatchTool", () => {
), ),
) )
it.live("approves a relative external target before reading update content", () => it.live("does not inspect an external file when external permission is denied", () =>
Effect.acquireUseRelease(
Effect.promise(() => Promise.all([tmpdir(), tmpdir()])),
([active, outside]) => {
reset()
denyAction = "external_directory"
const target = path.join(outside.path, "external.txt")
return Effect.promise(() => fs.writeFile(target, "before\n")).pipe(
Effect.andThen(
withTool(
active.path,
(registry) =>
Effect.gen(function* () {
expect(
yield* executeTool(
registry,
call(`*** Begin Patch\n*** Update File: ${target}\n@@\n-before\n+after\n*** End Patch`),
),
).toMatchObject({ type: "error" })
expect(assertions.map((input) => input.action)).toEqual(["external_directory"])
expect(readsBeforeEditApproval).toBe(0)
expect(yield* Effect.promise(() => fs.readFile(target, "utf8"))).toBe("before\n")
}),
path.parse(active.path).root,
),
),
)
},
([active, outside]) =>
Effect.promise(() =>
Promise.all([active[Symbol.asyncDispose](), outside[Symbol.asyncDispose]()]).then(() => undefined),
),
),
)
it.live("treats a sibling path inside the project worktree as internal", () =>
Effect.acquireUseRelease(
Effect.promise(() => tmpdir()),
(tmp) => {
reset()
const active = path.join(tmp.path, "active")
const target = path.join(tmp.path, "sibling.txt")
return Effect.promise(() => Promise.all([fs.mkdir(active), fs.writeFile(target, "before\n")])).pipe(
Effect.andThen(
withTool(
active,
(registry) =>
Effect.gen(function* () {
expect(
yield* executeTool(
registry,
call("*** Begin Patch\n*** Update File: ../sibling.txt\n@@\n-before\n+after\n*** End Patch"),
),
).toMatchObject({ type: "text" })
expect(assertions.map((input) => input.action)).toEqual(["edit"])
expect(yield* Effect.promise(() => fs.readFile(target, "utf8"))).toBe("after\n")
}),
tmp.path,
),
),
)
},
(tmp) => Effect.promise(() => tmp[Symbol.asyncDispose]()),
),
)
it.live("follows an internal symlink to an external file without external permission", () =>
Effect.acquireUseRelease(
Effect.promise(() => Promise.all([tmpdir(), tmpdir()])),
([active, outside]) => {
reset()
if (process.platform === "win32") return Effect.void
const target = path.join(outside.path, "external.txt")
const link = path.join(active.path, "link.txt")
return Effect.promise(() => fs.writeFile(target, "before\n")).pipe(
Effect.andThen(Effect.promise(() => fs.symlink(target, link))),
Effect.andThen(
withTool(active.path, (registry) =>
Effect.gen(function* () {
expect(
yield* executeTool(
registry,
call("*** Begin Patch\n*** Update File: link.txt\n@@\n-before\n+after\n*** End Patch"),
),
).toMatchObject({ type: "text" })
expect(assertions.map((input) => input.action)).toEqual(["edit"])
expect(yield* Effect.promise(() => fs.readFile(target, "utf8"))).toBe("after\n")
}),
),
),
)
},
([active, outside]) =>
Effect.promise(() =>
Promise.all([active[Symbol.asyncDispose](), outside[Symbol.asyncDispose]()]).then(() => undefined),
),
),
)
it.live("approves a relative external target before reading and requests edit permission afterward", () =>
Effect.acquireUseRelease( Effect.acquireUseRelease(
Effect.promise(() => Promise.all([tmpdir(), tmpdir()])), Effect.promise(() => Promise.all([tmpdir(), tmpdir()])),
([active, outside]) => { ([active, outside]) => {
@ -295,7 +796,7 @@ describe("PatchTool", () => {
), ),
).toMatchObject({ type: "text" }) ).toMatchObject({ type: "text" })
expect(assertions.map((input) => input.action)).toEqual(["external_directory", "edit"]) expect(assertions.map((input) => input.action)).toEqual(["external_directory", "edit"])
expect(readsBeforeEditApproval).toBe(0) expect(readsBeforeEditApproval).toBe(1)
expect(yield* Effect.promise(() => fs.readFile(target, "utf8"))).toBe("after\n") expect(yield* Effect.promise(() => fs.readFile(target, "utf8"))).toBe("after\n")
}), }),
), ),
@ -309,7 +810,7 @@ describe("PatchTool", () => {
), ),
) )
it.live("approves one external directory scope for multiple files under the same parent", () => it.live("approves each external file under the same parent", () =>
Effect.acquireUseRelease( Effect.acquireUseRelease(
Effect.promise(() => Promise.all([tmpdir(), tmpdir()])), Effect.promise(() => Promise.all([tmpdir(), tmpdir()])),
([active, outside]) => { ([active, outside]) => {
@ -330,10 +831,17 @@ describe("PatchTool", () => {
), ),
), ),
).toMatchObject({ type: "text" }) ).toMatchObject({ type: "text" })
expect(assertions.map((input) => input.action)).toEqual(["external_directory", "edit"]) expect(assertions.map((input) => input.action)).toEqual([
expect(assertions[0]?.resources).toEqual([ "external_directory",
path.join(yield* Effect.promise(() => fs.realpath(outside.path)), "*").replaceAll("\\", "/"), "external_directory",
"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("\\", "/"),
])
expect(assertions[1]?.resources).toEqual(assertions[0]?.resources)
}), }),
), ),
), ),
@ -360,7 +868,10 @@ describe("PatchTool", () => {
"*** Begin Patch\n*** Add File: created.txt\n+created\n*** Update File: missing.txt\n@@\n-before\n+after\n*** End Patch", "*** Begin Patch\n*** Add File: created.txt\n+created\n*** Update File: missing.txt\n@@\n-before\n+after\n*** End Patch",
), ),
), ),
).toEqual({ type: "error", value: "Unable to apply patch at missing.txt" }) ).toMatchObject({
type: "error",
value: expect.stringContaining("patch verification failed: Failed to read file to update"),
})
expect(yield* exists(path.join(tmp.path, "created.txt"))).toBe(false) expect(yield* exists(path.join(tmp.path, "created.txt"))).toBe(false)
}), }),
) )
@ -369,7 +880,7 @@ describe("PatchTool", () => {
), ),
) )
it.live("rejects add hunks targeting an existing file without replacing it", () => it.live("adds files by overwriting existing targets", () =>
Effect.acquireUseRelease( Effect.acquireUseRelease(
Effect.promise(() => tmpdir()), Effect.promise(() => tmpdir()),
(tmp) => { (tmp) => {
@ -384,8 +895,8 @@ describe("PatchTool", () => {
registry, registry,
call("*** Begin Patch\n*** Add File: existing.txt\n+replacement\n*** End Patch"), call("*** Begin Patch\n*** Add File: existing.txt\n+replacement\n*** End Patch"),
), ),
).toEqual({ type: "error", value: "Unable to apply patch at existing.txt" }) ).toMatchObject({ type: "text" })
expect(yield* Effect.promise(() => fs.readFile(target, "utf8"))).toBe("sentinel\n") expect(yield* Effect.promise(() => fs.readFile(target, "utf8"))).toBe("replacement\n")
}), }),
), ),
), ),
@ -395,7 +906,7 @@ describe("PatchTool", () => {
), ),
) )
it.live("rejects an add target that appears during permission approval", () => it.live("overwrites an add target that appears during permission approval", () =>
Effect.acquireUseRelease( Effect.acquireUseRelease(
Effect.promise(() => tmpdir()), Effect.promise(() => tmpdir()),
(tmp) => { (tmp) => {
@ -409,8 +920,8 @@ describe("PatchTool", () => {
registry, registry,
call("*** Begin Patch\n*** Add File: appeared.txt\n+replacement\n*** End Patch"), call("*** Begin Patch\n*** Add File: appeared.txt\n+replacement\n*** End Patch"),
), ),
).toEqual({ type: "error", value: "Unable to apply patch at appeared.txt" }) ).toMatchObject({ type: "text" })
expect(yield* Effect.promise(() => fs.readFile(target, "utf8"))).toBe("winner\n") expect(yield* Effect.promise(() => fs.readFile(target, "utf8"))).toBe("replacement\n")
}), }),
) )
}, },
@ -449,35 +960,4 @@ describe("PatchTool", () => {
), ),
) )
it.live("finishes the sequential commit phase when interrupted after the first mutation", () =>
Effect.acquireUseRelease(
Effect.promise(() => tmpdir()),
(tmp) => {
reset()
const first = path.join(tmp.path, "first.txt")
const second = path.join(tmp.path, "second.txt")
blockRemoveTarget = path.basename(second)
return Effect.gen(function* () {
removeStarted = yield* Deferred.make<void>()
releaseRemove = yield* Deferred.make<void>()
yield* Effect.promise(() => Promise.all([fs.writeFile(first, "first"), fs.writeFile(second, "second")]))
yield* withTool(tmp.path, (registry) =>
Effect.gen(function* () {
const run = yield* executeTool(
registry,
call("*** Begin Patch\n*** Delete File: first.txt\n*** Delete File: second.txt\n*** End Patch"),
).pipe(Effect.forkChild)
yield* Deferred.await(removeStarted!)
const interrupt = yield* Fiber.interrupt(run).pipe(Effect.forkChild)
yield* Deferred.succeed(releaseRemove!, undefined)
yield* Fiber.join(interrupt)
expect(yield* exists(first)).toBe(false)
expect(yield* exists(second)).toBe(false)
}),
)
})
},
(tmp) => Effect.promise(() => tmp[Symbol.asyncDispose]()),
),
)
}) })