Merge PR #8395: fix(lsp): roll back rename_file edits when the move fails (@roboomp)

packages/coding-agent/CHANGELOG.md

@@ -75,6 +75,9 @@
 ### Fixed
 
 - Fixed `lsp` `rename_file` moving the path even when a server that supports `workspace/willRenameFiles` failed the request, leaving references dangling; the rename now aborts before any filesystem mutation and reports the failure ([#8380](https://github.com/can1357/oh-my-pi/issues/8380)).
+### Fixed
+
+- Fixed `lsp` `rename_file` leaving reference edits applied when the file move fails; a failed move now rolls back every rewritten reference file so the source, destination, and references are left unchanged ([#8379](https://github.com/can1357/oh-my-pi/issues/8379)).
 
 ## [17.2.15] - 2026-08-12
 

packages/coding-agent/src/lsp/edits.ts

@@ -166,6 +166,43 @@ export async function applyTextEdits(filePath: string, edits: TextEdit[]): Promi
 	await Bun.write(filePath, result);
 }
 
+/** A reference file and the text edits a rename computed for it. */
+export interface RenameReferenceEdit {
+	filePath: string;
+	edits: TextEdit[];
+}
+
+/**
+ * Apply a rename's reference edits and then move `source` → `dest` as one unit.
+ *
+ * The reference edits (import/usage rewrites in other files) must be written
+ * before the move so their positions match the pre-move file contents, but a
+ * failed move must not leave those files half-rewritten: each edited file is
+ * snapshotted first, and if `mkdir`/`rename` throws, every snapshot is restored
+ * before the error propagates. A failed move therefore leaves the source,
+ * destination, and every reference file exactly as they were.
+ *
+ * @throws the original `mkdir`/`rename` error, after rolling back the edits.
+ */
+export async function applyEditsThenRename(
+	references: RenameReferenceEdit[],
+	source: string,
+	dest: string,
+): Promise<void> {
+	const backups: Array<{ filePath: string; original: string }> = [];
+	for (const { filePath, edits } of references) {
+		backups.push({ filePath, original: await Bun.file(filePath).text() });
+		await applyTextEdits(filePath, edits);
+	}
+	try {
+		await fs.mkdir(path.dirname(dest), { recursive: true });
+		await fs.rename(source, dest);
+	} catch (err) {
+		await Promise.all(backups.map(({ filePath, original }) => Bun.write(filePath, original)));
+		throw err;
+	}
+}
+
 // =============================================================================
 // Workspace Edit Application
 // =============================================================================

packages/coding-agent/src/lsp/tool.ts

@@ -44,9 +44,10 @@ import {
 	waitForDiagnostics,
 } from "./diagnostics";
 import {
-	applyTextEdits,
+	applyEditsThenRename,
 	applyWorkspaceEdit,
 	flattenWorkspaceTextEdits,
+	type RenameReferenceEdit,
 	rangesOverlap,
 	sortAndValidateTextEdits,
 } from "./edits";
@@ -718,9 +719,10 @@ export class LspTool implements AgentTool<typeof lspSchema, LspToolDetails, Them
 				sortAndValidateTextEdits(bucket.edits);
 			}
 
+			const referenceEdits: RenameReferenceEdit[] = [];
 			for (const [uri, bucket] of acceptedByUri) {
 				const filePath = uriToFile(uri);
-				await applyTextEdits(filePath, bucket.edits);
+				referenceEdits.push({ filePath, edits: bucket.edits });
 				const rel = formatPathRelativeToCwd(filePath, this.session.cwd);
 				summary.push(`  ${bucket.primaryServer}: applied ${bucket.edits.length} edit(s) to ${rel}`);
 				if (bucket.discarded > 0) {
@@ -734,8 +736,10 @@ export class LspTool implements AgentTool<typeof lspSchema, LspToolDetails, Them
 				}
 			}
 
-			await fs.promises.mkdir(path.dirname(dest), { recursive: true });
-			await fs.promises.rename(source, dest);
+			// Apply the reference edits and move as one unit: a failed move rolls
+			// the reference edits back so the source, destination, and every
+			// reference file are left unchanged.
+			await applyEditsThenRename(referenceEdits, source, dest);
 			summary.push(`  Renamed ${sourceLabel} → ${destLabel}`);
 
 			for (const [serverName, serverConfig] of servers) {

packages/coding-agent/test/lsp/edits.test.ts

@@ -0,0 +1,54 @@
+import { afterEach, beforeEach, describe, expect, it } from "bun:test";
+import * as fs from "node:fs/promises";
+import * as os from "node:os";
+import * as path from "node:path";
+import { applyEditsThenRename } from "@oh-my-pi/pi-coding-agent/lsp/edits";
+import type { TextEdit } from "@oh-my-pi/pi-coding-agent/lsp/types";
+
+// Rewrite `./moved` → `./renamed` on line 0 of the reference file below.
+const importEdit: TextEdit[] = [
+	{ range: { start: { line: 0, character: 19 }, end: { line: 0, character: 26 } }, newText: "./renamed" },
+];
+
+describe("applyEditsThenRename", () => {
+	let dir: string;
+	let source: string;
+	let ref: string;
+	const refBefore = 'import { x } from "./moved";\n';
+
+	beforeEach(async () => {
+		dir = await fs.mkdtemp(path.join(os.tmpdir(), "edits-rename-"));
+		source = path.join(dir, "moved.ts");
+		ref = path.join(dir, "ref.ts");
+		await Bun.write(source, "export const x = 1;\n");
+		await Bun.write(ref, refBefore);
+	});
+
+	afterEach(async () => {
+		await fs.rm(dir, { recursive: true, force: true });
+	});
+
+	it("applies reference edits and moves the source when the move succeeds", async () => {
+		const dest = path.join(dir, "nested", "renamed.ts");
+		await applyEditsThenRename([{ filePath: ref, edits: importEdit }], source, dest);
+
+		expect(await Bun.file(dest).text()).toBe("export const x = 1;\n");
+		expect(await Bun.file(source).exists()).toBe(false);
+		expect(await Bun.file(ref).text()).toBe('import { x } from "./renamed";\n');
+	});
+
+	it("rolls back reference edits when the move fails", async () => {
+		// A regular file stands where a dest-parent directory must be, so the
+		// recursive mkdir throws ENOTDIR before the rename runs.
+		const blocker = path.join(dir, "blocker");
+		await Bun.write(blocker, "not a dir");
+		const dest = path.join(blocker, "sub", "renamed.ts");
+
+		await expect(applyEditsThenRename([{ filePath: ref, edits: importEdit }], source, dest)).rejects.toThrow();
+
+		// Failed move must leave source, destination, and reference files untouched.
+		expect(await Bun.file(ref).text()).toBe(refBefore);
+		expect(await Bun.file(source).exists()).toBe(true);
+		expect(await Bun.file(dest).exists()).toBe(false);
+	});
+});