fix(core): stop GPT patch edits from deleting a file moved onto itself (#59197)

This commit is contained in:
Kit Langton
2026-10-26 10:09:29 -07:00
committed by GitHub
Unverified
parent 9f96a331e3
commit dc21f528ac
2 changed files with 79 additions and 14 deletions
+37 -14
View File
@@ -113,7 +113,7 @@ export const Plugin = {
return yield* new ToolFailure({ message: "patch rejected: empty patch" })
}
const prepared: Prepared[] = []
const updates = new Map<string, string>()
const updates = new Map<string, string | null>()
const resolveTarget = Effect.fnUntraced(function* (value: string) {
const target = yield* access.resolve({ path: value, kind: "file" })
if (!target.externalDirectory) return target
@@ -136,21 +136,37 @@ export const Plugin = {
before: "",
after: Bom.split(content).text,
})
updates.set(target.absolute, content)
return
}
if (hunk.type === "delete") {
const content = yield* FileMutation.readText(environment.files, target.absolute).pipe(
Effect.mapError(
(error) =>
new ToolFailure({
message: `patch verification failed: Failed to delete ${target.resource}: ${errorMessage(error)}`,
}),
),
)
prepared.push({ ...hunk, target, before: content.text, after: "" })
const previous = updates.get(target.absolute)
if (previous === null) {
return yield* new ToolFailure({
message: `patch verification failed: Failed to delete ${target.resource}: file does not exist`,
})
}
const before =
previous !== undefined
? Bom.split(previous).text
: (yield* FileMutation.readText(environment.files, target.absolute).pipe(
Effect.mapError(
(error) =>
new ToolFailure({
message: `patch verification failed: Failed to delete ${target.resource}: ${errorMessage(error)}`,
}),
),
)).text
prepared.push({ ...hunk, target, before, after: "" })
updates.set(target.absolute, null)
return
}
const previous = updates.get(target.absolute)
if (previous === null) {
return yield* new ToolFailure({
message: `patch verification failed: Failed to read file to update ${target.absolute}: file does not exist`,
})
}
const original =
previous ??
(yield* Effect.gen(function* () {
@@ -169,16 +185,23 @@ export const Plugin = {
try: () => Patch.derive(hunk.path, hunk.chunks, original),
catch: (error) => new ToolFailure({ message: `patch verification failed: ${errorMessage(error)}` }),
})
const moveTarget = hunk.movePath ? yield* resolveTarget(hunk.movePath) : undefined
const move = hunk.movePath ? yield* resolveTarget(hunk.movePath) : undefined
const moveTarget = move && move.absolute !== target.absolute ? move : undefined
const next = Patch.joinBom(update.content, update.bom)
prepared.push({
...hunk,
target,
content: Patch.joinBom(update.content, update.bom),
content: next,
before,
after: update.content,
moveTarget,
})
if (!moveTarget) updates.set(target.absolute, Patch.joinBom(update.content, update.bom))
if (moveTarget) {
updates.set(target.absolute, null)
updates.set(moveTarget.absolute, next)
} else {
updates.set(target.absolute, next)
}
}).pipe(
Effect.mapError((error) =>
error instanceof ToolFailure
@@ -254,7 +277,7 @@ export const Plugin = {
)
const formatted = new Map<string, string>()
yield* Effect.forEach(
[...new Set(applied.filter((item) => item.type !== "delete").map((item) => item.target))],
[...updates.entries()].flatMap(([target, value]) => (value === null ? [] : [target])),
(target) =>
Effect.gen(function* () {
const current = yield* FileMutation.readText(environment.files, target).pipe(
+42
View File
@@ -709,6 +709,48 @@ describe("PatchTool", () => {
),
)
it.live("preserves files across self-moves and multi-hunk moves in one patch", () =>
withTempTool((directory, registry) =>
Effect.gen(function* () {
const same = path.join(directory, "same.txt")
const source = path.join(directory, "old.txt")
const destination = path.join(directory, "moved.txt")
yield* Effect.promise(() => Promise.all([fs.writeFile(same, "before\n"), fs.writeFile(source, "a\nb\nc\n")]))
const settled = yield* executeTool(
registry,
call(
[
"*** Begin Patch",
"*** Update File: same.txt",
"*** Move to: ./same.txt",
"@@",
"-before",
"+after",
"*** Update File: old.txt",
"@@",
"-a",
"+A",
"*** Update File: old.txt",
"*** Move to: moved.txt",
"@@",
"-b",
"+B",
"*** Update File: moved.txt",
"@@",
"-c",
"+C",
"*** End Patch",
].join("\n"),
),
)
expect(settled).toMatchObject({ status: "completed" })
expect(yield* Effect.promise(() => fs.readFile(same, "utf8"))).toBe("after\n")
expect(yield* exists(source)).toBe(false)
expect(yield* Effect.promise(() => fs.readFile(destination, "utf8"))).toBe("A\nB\nC\n")
}),
),
)
it.live("does not invent a first-line diff for BOM files", () =>
withTempTool((directory, registry) =>
Effect.gen(function* () {