From dc21f528ac2125f594bc4bdca7bff9f3fdfb06f5 Mon Sep 17 00:00:00 2001 From: Kit Langton Date: Mon, 26 Oct 2026 10:09:29 -0700 Subject: [PATCH] fix(core): stop GPT patch edits from deleting a file moved onto itself (#59197) --- packages/core/src/tool/plugin/patch.ts | 51 +++++++++++++++++++------- packages/core/test/tool-patch.test.ts | 42 +++++++++++++++++++++ 2 files changed, 79 insertions(+), 14 deletions(-) diff --git a/packages/core/src/tool/plugin/patch.ts b/packages/core/src/tool/plugin/patch.ts index 54eaa3950d1..554e7a9d3eb 100644 --- a/packages/core/src/tool/plugin/patch.ts +++ b/packages/core/src/tool/plugin/patch.ts @@ -113,7 +113,7 @@ export const Plugin = { return yield* new ToolFailure({ message: "patch rejected: empty patch" }) } const prepared: Prepared[] = [] - const updates = new Map() + const updates = new Map() 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() 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( diff --git a/packages/core/test/tool-patch.test.ts b/packages/core/test/tool-patch.test.ts index 8a392721ef7..209b97b5bb8 100644 --- a/packages/core/test/tool-patch.test.ts +++ b/packages/core/test/tool-patch.test.ts @@ -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* () {