diff --git a/packages/coding-agent/src/lsp/client.ts b/packages/coding-agent/src/lsp/client.ts index ae9886d79..80c2d452f 100644 --- a/packages/coding-agent/src/lsp/client.ts +++ b/packages/coding-agent/src/lsp/client.ts @@ -525,18 +525,13 @@ function uriIsWithin(uri: string, root: string): boolean { return uri === root || uri.startsWith(root.endsWith("/") ? root : `${root}/`); } -/** - * Apply a server-provided workspace edit and reconcile every affected open LSP document. - * Runtime callers use this wrapper so later semantic requests observe the committed files. - * Reconciliation is derived from the ops that actually ran — an op skipped via - * `ignoreIfExists`/`ignoreIfNotExists` neither closes overlays nor notifies watchers. - */ -export async function applyWorkspaceEditWithLsp( - edit: WorkspaceEdit, +/** Reconcile open overlays and file watchers with the ops a workspace edit actually performed. */ +async function reconcileExecutedChanges( + executed: ExecutedWorkspaceChange[], cwd: string, signal?: AbortSignal, -): Promise { - const { applied, executed } = await applyWorkspaceEdit(edit, cwd); +): Promise { + if (executed.length === 0) return; const { finalUris, deletedRoots, watchedFiles } = workspaceEditChanges(executed); const workspace = path.resolve(cwd); const activeClients = Array.from(clients.values()).filter( @@ -563,6 +558,38 @@ export async function applyWorkspaceEditWithLsp( } } await notifyWorkspaceWatchedFiles(cwd, watchedFiles, signal); +} + +/** + * Apply a server-provided workspace edit and reconcile every affected open LSP document. + * Runtime callers use this wrapper so later semantic requests observe the committed files. + * Reconciliation is derived from the ops that actually ran — an op skipped via + * `ignoreIfExists`/`ignoreIfNotExists` neither closes overlays nor notifies watchers, and + * when the edit fails partway the already-executed prefix is still reconciled before the + * error propagates so mutated files never keep stale overlays. + */ +export async function applyWorkspaceEditWithLsp( + edit: WorkspaceEdit, + cwd: string, + signal?: AbortSignal, +): Promise { + const executed: ExecutedWorkspaceChange[] = []; + let applied: string[]; + try { + ({ applied } = await applyWorkspaceEdit(edit, cwd, change => executed.push(change))); + } catch (err) { + // Best-effort: overlays for the mutated prefix must not stay stale, but + // reconciliation problems must not mask the original apply failure. + try { + await reconcileExecutedChanges(executed, cwd, signal); + } catch (reconcileErr) { + logger.warn("LSP overlay reconciliation after failed workspace edit failed", { + error: reconcileErr instanceof Error ? reconcileErr.message : String(reconcileErr), + }); + } + throw err; + } + await reconcileExecutedChanges(executed, cwd, signal); return applied; } diff --git a/packages/coding-agent/src/lsp/edits.ts b/packages/coding-agent/src/lsp/edits.ts index 4f9526b66..8bb98579a 100644 --- a/packages/coding-agent/src/lsp/edits.ts +++ b/packages/coding-agent/src/lsp/edits.ts @@ -307,10 +307,23 @@ export interface WorkspaceEditResult { * Apply a workspace edit (collection of file changes). * All text-edit batches are overlap-validated before anything is written so a * conflict throws without leaving the workspace half-applied. + * + * `onExecuted` fires after each filesystem mutation. When a later op throws, + * the callback has already reported the executed prefix — callers that must + * reconcile external state (e.g. LSP overlays) rely on this because the + * returned {@link WorkspaceEditResult} is lost on failure. */ -export async function applyWorkspaceEdit(edit: WorkspaceEdit, cwd: string): Promise { +export async function applyWorkspaceEdit( + edit: WorkspaceEdit, + cwd: string, + onExecuted?: (change: ExecutedWorkspaceChange) => void, +): Promise { const applied: string[] = []; const executed: ExecutedWorkspaceChange[] = []; + const record = (change: ExecutedWorkspaceChange) => { + executed.push(change); + onExecuted?.(change); + }; if (edit.documentChanges) { const ops = planDocumentChanges(edit.documentChanges); @@ -322,7 +335,7 @@ export async function applyWorkspaceEdit(edit: WorkspaceEdit, cwd: string): Prom const filePath = uriToFile(op.uri); await applyTextEdits(filePath, op.edits); applied.push(`Applied ${op.edits.length} edit(s) to ${formatPathRelativeToCwd(filePath, cwd)}`); - executed.push({ kind: "edit", uri: op.uri }); + record({ kind: "edit", uri: op.uri }); } else if (op.kind === "create") { const filePath = uriToFile(op.uri); await fs.mkdir(path.dirname(filePath), { recursive: true }); @@ -340,7 +353,7 @@ export async function applyWorkspaceEdit(edit: WorkspaceEdit, cwd: string): Prom continue; } applied.push(`Created ${formatPathRelativeToCwd(filePath, cwd)}`); - executed.push({ kind: "create", uri: op.uri }); + record({ kind: "create", uri: op.uri }); } else if (op.kind === "rename") { const oldPath = uriToFile(op.oldUri); const newPath = uriToFile(op.newUri); @@ -366,7 +379,7 @@ export async function applyWorkspaceEdit(edit: WorkspaceEdit, cwd: string): Prom await fs.rename(oldPath, newPath); } applied.push(`Renamed ${formatPathRelativeToCwd(oldPath, cwd)} → ${formatPathRelativeToCwd(newPath, cwd)}`); - executed.push({ kind: "rename", oldUri: op.oldUri, newUri: op.newUri }); + record({ kind: "rename", oldUri: op.oldUri, newUri: op.newUri }); } else { const filePath = uriToFile(op.uri); try { @@ -381,7 +394,7 @@ export async function applyWorkspaceEdit(edit: WorkspaceEdit, cwd: string): Prom continue; } applied.push(`Deleted ${formatPathRelativeToCwd(filePath, cwd)}`); - executed.push({ kind: "delete", uri: op.uri }); + record({ kind: "delete", uri: op.uri }); } } } else if (edit.changes) { @@ -396,7 +409,7 @@ export async function applyWorkspaceEdit(edit: WorkspaceEdit, cwd: string): Prom const filePath = uriToFile(uri); await applyTextEdits(filePath, textEdits); applied.push(`Applied ${textEdits.length} edit(s) to ${formatPathRelativeToCwd(filePath, cwd)}`); - executed.push({ kind: "edit", uri }); + record({ kind: "edit", uri }); } } diff --git a/packages/coding-agent/test/tools/lsp-regressions.test.ts b/packages/coding-agent/test/tools/lsp-regressions.test.ts index af421b230..e266b5fc3 100644 --- a/packages/coding-agent/test/tools/lsp-regressions.test.ts +++ b/packages/coding-agent/test/tools/lsp-regressions.test.ts @@ -3158,6 +3158,69 @@ describe("lsp regressions", () => { } }); + it("reconciles the executed prefix when a workspace edit fails partway", async () => { + // A text edit to an open file inside `src/` is flushed to disk before the + // non-recursive delete of `src/` runs (subtree flush), and that delete + // throws on the non-empty directory. The error must propagate, but the + // already-mutated file's overlay must be refreshed — not left stale. + const tempDir = TempDir.createSync("@omp-lsp-partial-edit-overlay-"); + try { + const srcDir = path.join(tempDir.path(), "src"); + fs.mkdirSync(srcDir); + const filePath = path.join(srcDir, "a.ts"); + await Bun.write(filePath, "export const a = 1;\n"); + + const server = installFakeLsp((message, srv) => { + if (message.method === "initialize") { + srv.send({ jsonrpc: "2.0", id: message.id, result: { capabilities: {} } }); + } else if (message.method === "shutdown") { + srv.send({ jsonrpc: "2.0", id: message.id, result: null }); + } else if (message.method === "exit") { + srv.exit(0); + } + }); + const config: ServerConfig = { command: "fake-lsp", fileTypes: ["ts"], rootMarkers: [] }; + const client = await lspClient.getOrCreateClient(config, tempDir.path(), 1_000); + await lspClient.ensureFileOpen(client, filePath); + await server.waitFor(message => message.method === "textDocument/didOpen"); + + const uri = fileToUri(filePath); + await expect( + lspClient.applyWorkspaceEditWithLsp( + { + documentChanges: [ + { + textDocument: { uri, version: null }, + edits: [ + { + range: { start: { line: 0, character: 17 }, end: { line: 0, character: 18 } }, + newText: "2", + }, + ], + } satisfies TextDocumentEdit, + { kind: "delete", uri: fileToUri(srcDir), options: { recursive: false } } satisfies DeleteFile, + ], + }, + tempDir.path(), + ), + ).rejects.toThrow(); + + // Disk carries the executed edit; the failing delete never ran. + expect(fs.readFileSync(filePath, "utf8")).toBe("export const a = 2;\n"); + expect(fs.existsSync(srcDir)).toBe(true); + + // The overlay was refreshed to the committed content despite the failure. + const didChange = await server.waitFor(message => message.method === "textDocument/didChange"); + expect(didChange.params).toMatchObject({ + textDocument: { uri }, + contentChanges: [{ text: "export const a = 2;\n" }], + }); + } finally { + await lspClient.shutdownAll(); + tempDir.removeSync(); + } + }); + it("honors DeleteFile recursive and ignoreIfNotExists options", async () => { const tempDir = TempDir.createSync("@omp-lsp-delete-options-"); try {