Merge PR #8394: fix(lsp): reject unsupported rust-analyzer snippet edits (@roboomp)

This commit is contained in:
can1357
2026-08-13 05:46:37 +02:00
6 changed files with 159 additions and 4 deletions
+3
View File
@@ -66,6 +66,9 @@
### Fixed
- Fixed LSP `diagnostics` reporting `OK`/`success: true` when every applicable language server failed; a run with zero successful server responses now fails, and partial failures surface diagnostics while naming the servers that failed ([#8377](https://github.com/can1357/oh-my-pi/issues/8377)).
### Fixed
- Fixed rust-analyzer code actions writing snippet placeholders such as `$0` into source by no longer advertising unsupported snippet text edits and rejecting any unexpected snippet edit before applying it ([#8376](https://github.com/can1357/oh-my-pi/issues/8376)).
## [17.2.15] - 2026-08-12
-3
View File
@@ -189,9 +189,6 @@ const CLIENT_CAPABILITIES = {
didDelete: false,
},
},
experimental: {
snippetTextEdit: true,
},
};
/** LSP `FileChangeType` values for workspace/didChangeWatchedFiles notifications. */
+9
View File
@@ -77,7 +77,16 @@ export function rangesOverlap(a: Range, b: Range): boolean {
* Byte-identical non-empty range edits are idempotent, so duplicate server
* output is collapsed before overlap validation.
*/
function rejectSnippetTextEdits(edits: TextEdit[]): void {
for (const edit of edits) {
if ("insertTextFormat" in edit && edit.insertTextFormat === 2) {
throw new ToolError("snippet-formatted LSP edits are unsupported");
}
}
}
export function sortAndValidateTextEdits(edits: TextEdit[]): TextEdit[] {
rejectSnippetTextEdits(edits);
const sorted = edits
.map((edit, index) => ({ edit, index }))
.sort((a, b) => {
+14 -1
View File
@@ -43,7 +43,13 @@ import {
WORKSPACE_SYMBOL_LIMIT,
waitForDiagnostics,
} from "./diagnostics";
import { applyTextEdits, applyWorkspaceEdit, flattenWorkspaceTextEdits, rangesOverlap } from "./edits";
import {
applyTextEdits,
applyWorkspaceEdit,
flattenWorkspaceTextEdits,
rangesOverlap,
sortAndValidateTextEdits,
} from "./edits";
import { detectLspmux } from "./lspmux";
import {
configCache,
@@ -662,6 +668,13 @@ export class LspTool implements AgentTool<typeof lspSchema, LspToolDetails, Them
}
}
// Validate every accepted bucket (overlap + snippet-format rejection)
// before writing any file, so a snippet edit in a later URI cannot
// leave earlier files half-applied.
for (const bucket of acceptedByUri.values()) {
sortAndValidateTextEdits(bucket.edits);
}
for (const [uri, bucket] of acceptedByUri) {
const filePath = uriToFile(uri);
await applyTextEdits(filePath, bucket.edits);
+1
View File
@@ -97,6 +97,7 @@ export interface PublishDiagnosticsParams {
export interface TextEdit {
range: Range;
newText: string;
insertTextFormat?: 1 | 2;
}
export interface AnnotatedTextEdit extends TextEdit {
@@ -545,6 +545,36 @@ describe("lsp regressions", () => {
}
});
it("does not advertise unsupported snippet text edits", async () => {
const tempDir = TempDir.createSync("@omp-lsp-snippet-capability-");
try {
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: ["rs"],
rootMarkers: [],
};
await lspClient.getOrCreateClient(config, tempDir.path(), 1_000);
const init = server.received.find(message => message.method === "initialize");
const params = init?.params as { capabilities?: { experimental?: { snippetTextEdit?: boolean } } };
expect(params.capabilities?.experimental?.snippetTextEdit).toBeUndefined();
} finally {
await lspClient.shutdownAll();
tempDir.removeSync();
}
});
it("answers workspace/workspaceFolders requests with the current folder set", async () => {
const tempDir = TempDir.createSync("@omp-lsp-workspace-folders-request-");
try {
@@ -2055,6 +2085,96 @@ describe("lsp regressions", () => {
}
});
it("rename_file rejects a snippet edit before writing any file", async () => {
const tempDir = TempDir.createSync("@omp-lsp-rename-snippet-");
try {
const sourceFile = path.join(tempDir.path(), "src", "old.ts");
const destFile = path.join(tempDir.path(), "src", "new.ts");
const plainFile = path.join(tempDir.path(), "src", "plain.ts");
const snippetFile = path.join(tempDir.path(), "src", "snippet.ts");
await Bun.write(sourceFile, "export const value = 42;\n");
await Bun.write(plainFile, "import { value } from './old';\n");
await Bun.write(snippetFile, "import { value } from './old';\n");
const plainUri = fileToUri(plainFile);
const snippetUri = fileToUri(snippetFile);
const server: ServerConfig = { command: "test-lsp", fileTypes: ["ts"], rootMarkers: [] };
const client: LspClient = {
name: "test-lsp",
cwd: tempDir.path(),
config: server,
proc: {
stdin: { write() {}, flush: async () => {} },
} as unknown as LspClient["proc"],
requestId: 0,
diagnostics: new Map(),
diagnosticsVersion: 0,
openFiles: new Map(),
pendingRequests: new Map(),
messageBuffer: new Uint8Array(),
isReading: false,
status: "ready",
lastActivity: Date.now(),
writeQueue: Promise.resolve(),
activeProgressTokens: new Set(),
projectLoaded: Promise.resolve(),
resolveProjectLoaded: () => {},
};
vi.spyOn(lspConfig, "loadConfig").mockReturnValue({
servers: { "test-lsp": server },
idleTimeoutMs: undefined,
});
vi.spyOn(lspClient, "getOrCreateClient").mockResolvedValue(client);
vi.spyOn(lspClient, "sendRequest").mockImplementation(async (_client, method) => {
if (method === "workspace/willRenameFiles") {
return {
// plainUri is a valid edit; snippetUri carries insertTextFormat 2.
// The plain bucket must NOT be written when the snippet bucket rejects.
changes: {
[plainUri]: [
{
range: { start: { line: 0, character: 22 }, end: { line: 0, character: 29 } },
newText: "'./new'",
},
],
[snippetUri]: [
{
range: { start: { line: 0, character: 22 }, end: { line: 0, character: 29 } },
newText: "'./new$0'",
insertTextFormat: 2,
},
],
},
};
}
return null;
});
vi.spyOn(lspClient, "sendNotification").mockResolvedValue();
const tool = new LspTool(makeLspSession(tempDir.path()));
await expect(
tool.execute("rename-snippet-test", {
action: "rename_file",
file: sourceFile,
new_name: destFile,
timeout: 5,
}),
).rejects.toThrow("snippet-formatted LSP edits are unsupported");
// Nothing was half-applied: the plain bucket is untouched and the
// rename never ran.
expect(await Bun.file(plainFile).text()).toBe("import { value } from './old';\n");
expect(await Bun.file(snippetFile).text()).toBe("import { value } from './old';\n");
expect(fs.existsSync(sourceFile)).toBe(true);
expect(fs.existsSync(destFile)).toBe(false);
} finally {
vi.restoreAllMocks();
tempDir.removeSync();
}
});
it("rename_file with apply:false previews edits without filesystem changes", async () => {
const tempDir = TempDir.createSync("@omp-lsp-rename-file-preview-");
try {
@@ -2507,6 +2627,18 @@ describe("lsp regressions", () => {
).toBe("import x from './megaMenu';\n");
});
it("rejects unexpected snippet text edits without writing their syntax", () => {
expect(() =>
applyTextEditsToString("struct S;", [
{
range: { start: { line: 0, character: 0 }, end: { line: 0, character: 0 } },
newText: "#[derive($0)] ",
insertTextFormat: 2,
},
]),
).toThrow("snippet-formatted LSP edits are unsupported");
});
it("keeps byte-identical zero-width inserts because they are not idempotent", () => {
const result = applyTextEditsToString("abc", [
{ range: { start: { line: 0, character: 1 }, end: { line: 0, character: 1 } }, newText: "X" },