diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index b857f83da..223e6f008 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/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 diff --git a/packages/coding-agent/src/lsp/edits.ts b/packages/coding-agent/src/lsp/edits.ts index 75bff26d9..c9987b62b 100644 --- a/packages/coding-agent/src/lsp/edits.ts +++ b/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 { + 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 // ============================================================================= diff --git a/packages/coding-agent/src/lsp/tool.ts b/packages/coding-agent/src/lsp/tool.ts index d17b6f324..576d5fd23 100644 --- a/packages/coding-agent/src/lsp/tool.ts +++ b/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 0) { @@ -734,8 +736,10 @@ export class LspTool implements AgentTool { + 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); + }); +});