fix(lsp): reconciled executed prefix when workspace edit fails partway
- applyWorkspaceEdit takes an onExecuted callback fired after each filesystem mutation, so callers hold the executed prefix even when a later op throws. - applyWorkspaceEditWithLsp reconciles overlays/watchers for that prefix best-effort before rethrowing the original apply error.
This commit is contained in:
@@ -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<string[]> {
|
||||
const { applied, executed } = await applyWorkspaceEdit(edit, cwd);
|
||||
): Promise<void> {
|
||||
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<string[]> {
|
||||
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;
|
||||
}
|
||||
|
||||
|
||||
@@ -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<WorkspaceEditResult> {
|
||||
export async function applyWorkspaceEdit(
|
||||
edit: WorkspaceEdit,
|
||||
cwd: string,
|
||||
onExecuted?: (change: ExecutedWorkspaceChange) => void,
|
||||
): Promise<WorkspaceEditResult> {
|
||||
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 });
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -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 {
|
||||
|
||||
Reference in New Issue
Block a user