diff --git a/docs/tools/lsp.md b/docs/tools/lsp.md index 5bf1f5f22..6ed09cb7c 100644 --- a/docs/tools/lsp.md +++ b/docs/tools/lsp.md @@ -211,7 +211,7 @@ Uses the same location normalization and output shape as `definition`, but sends **Execution** - Workspace mode first invalidates the per-cwd configuration cache, reloads configuration from disk, and then reloads every newly configured non-custom LSP server. - Single-file mode keeps the cached configuration and reloads the primary server for that file. -- Both modes clear matching recent initialization failures before starting a server. `reloadServer()` then tries the `rust-analyzer/reloadWorkspace` request, falls back to a `workspace/didChangeConfiguration` notification with `{ settings: {} }`, and finally tears down the client so the next request cold-starts it. For a shared-mux client, teardown first sends the mux restart notification so the shared server—not only this session's link—is replaced. +- Both modes clear matching recent initialization failures before starting a server. For rust-analyzer servers, `reloadServer()` first tries the `rust-analyzer/reloadWorkspace` request (only rust-analyzer implements it; sending it to other servers such as Roslyn can crash them, so it is gated on the server binary/name). Every server then falls back to a `workspace/didChangeConfiguration` notification with `{ settings: {} }`, and finally tears down the client so the next request cold-starts it. For a shared-mux client, teardown first sends the mux restart notification so the shared server—not only this session's link—is replaced. **Output text** - One line per server: `Reloaded `, `Restarted `, or `Failed to reload : ...`. diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index 402af444b..09d059400 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -75,6 +75,9 @@ ### Fixed - Fixed `always-ask` approval prompts bypassing edit preview readiness when a built-in tool executes under its wire-level alias, such as `edit` running as `apply_patch` ([#8607](https://github.com/can1357/oh-my-pi/issues/8607)). +### Fixed + +- Fixed `lsp reload` crashing non-rust-analyzer language servers (e.g. Roslyn/`roslyn-language-server`) by sending the rust-analyzer-specific `rust-analyzer/reloadWorkspace` request to every server; the request is now gated on the server being rust-analyzer, and all other servers reload via `workspace/didChangeConfiguration` directly ([#8571](https://github.com/can1357/oh-my-pi/issues/8571)). ## [17.3.4] - 2026-08-14 diff --git a/packages/coding-agent/src/lsp/client.ts b/packages/coding-agent/src/lsp/client.ts index 80c2d452f..9a5950083 100644 --- a/packages/coding-agent/src/lsp/client.ts +++ b/packages/coding-agent/src/lsp/client.ts @@ -746,7 +746,13 @@ function commandBasename(command: string): string { return separator === -1 ? command : command.slice(separator + 1); } -function isRustAnalyzerClient(client: LspClient): boolean { +/** + * True when this client speaks the rust-analyzer protocol, detected by the + * command basename (`rust-analyzer[.exe]`) of the configured or resolved + * binary. Callers use it to gate rust-analyzer-only requests such as + * `rust-analyzer/reloadWorkspace` (see {@link reloadServer}). + */ +export function isRustAnalyzerClient(client: LspClient): boolean { return ( commandBasename(client.config.command) === "rust-analyzer" || (client.config.resolvedCommand ? commandBasename(client.config.resolvedCommand) === "rust-analyzer" : false) diff --git a/packages/coding-agent/src/lsp/servers.ts b/packages/coding-agent/src/lsp/servers.ts index ef14e8350..e71133dd5 100644 --- a/packages/coding-agent/src/lsp/servers.ts +++ b/packages/coding-agent/src/lsp/servers.ts @@ -4,6 +4,7 @@ import { getActiveClients, getActiveOrPendingClient, getOrCreateClient, + isRustAnalyzerClient, type LspServerStatus, notifySaved, sendNotification, @@ -256,17 +257,25 @@ export function isMethodNotFoundError(err: unknown): boolean { export async function reloadServer(client: LspClient, serverName: string, signal?: AbortSignal): Promise { throwIfAborted(signal); - // rust-analyzer exposes a real reload request. Every other server rejects it - // with method-not-found — that alone justifies the generic fallback. A caller - // cancel or tool timeout must propagate, never be mistaken for an unsupported - // method and swallowed into a bogus "Restarted" (issue #6369). - try { - await sendRequest(client, "rust-analyzer/reloadWorkspace", null, signal); - return `Reloaded ${serverName}`; - } catch (err) { - throwIfAborted(signal); - if (!isMethodNotFoundError(err)) throw err; - // Method not supported — fall through to the generic reload. + // rust-analyzer exposes a real reload request. Only rust-analyzer implements + // it, so gate the request on the binary (or registered name) rather than + // probing every server: some servers (Roslyn) crash the whole process on an + // unknown method instead of replying with method-not-found — killing the + // server `lsp reload` was meant to refresh (issue #8571, dotnet/roslyn#84890). + // A caller cancel or tool timeout must propagate, never be mistaken for an + // unsupported method and swallowed into a bogus "Restarted" (issue #6369). + if (isRustAnalyzerClient(client) || serverName === "rust-analyzer") { + try { + // Pass an empty object, not null: JSON-RPC structured params must be an + // object or array, and servers that validate this (Roslyn) reject a null + // before returning method-not-found (dotnet/roslyn#84890). + await sendRequest(client, "rust-analyzer/reloadWorkspace", {}, signal); + return `Reloaded ${serverName}`; + } catch (err) { + throwIfAborted(signal); + if (!isMethodNotFoundError(err)) throw err; + // Method not supported — fall through to the generic reload. + } } // workspace/didChangeConfiguration is a notification per spec; sending it // as a request hangs until the tool deadline on servers that route it to diff --git a/packages/coding-agent/src/lsp/tool.ts b/packages/coding-agent/src/lsp/tool.ts index e4daa01e7..84357c100 100644 --- a/packages/coding-agent/src/lsp/tool.ts +++ b/packages/coding-agent/src/lsp/tool.ts @@ -21,6 +21,7 @@ import { ensureFileOpen, getActiveClients, getOrCreateClient, + isRustAnalyzerClient, type LspServerStatus, refreshFile, sendNotification, @@ -1075,10 +1076,7 @@ export class LspTool implements AgentTool { const loadConfigSpy = vi .spyOn(lspConfig, "loadConfig") .mockImplementation(() => configs.shift() ?? configs[0]); - const client = { proc: { kill: vi.fn() } } as unknown as LspClient; + const client = { proc: { kill: vi.fn() }, config: server } as unknown as LspClient; vi.spyOn(lspClient, "getOrCreateClient").mockResolvedValue(client); - vi.spyOn(lspClient, "sendRequest").mockResolvedValue(null); + vi.spyOn(lspClient, "sendNotification").mockResolvedValue(undefined); const tool = new LspTool(makeLspSession(tempDir.path())); const initial = await tool.execute("reload-redetect-status", { action: "status" }); @@ -3640,8 +3640,8 @@ describe("lsp regressions", () => { describe("reload cancellation and truthful teardown (#6369)", () => { // A JSON-RPC error response the client maps to isMethodNotFoundError, so a - // non-rust server falls through from `rust-analyzer/reloadWorkspace` to the - // generic `workspace/didChangeConfiguration` reload. + // rust-analyzer server whose reloadWorkspace is unsupported falls through + // to the generic `workspace/didChangeConfiguration` reload. const methodNotFound = (id: RpcMessage["id"]): RpcMessage => ({ jsonrpc: "2.0", id, @@ -3662,7 +3662,7 @@ describe("lsp regressions", () => { // rust-analyzer/reloadWorkspace is left pending: only the caller // signal decides its fate. }); - const config: ServerConfig = { command: "fake-reload-cancel-req", fileTypes: [".ts"], rootMarkers: [] }; + const config: ServerConfig = { command: "rust-analyzer", fileTypes: [".ts"], rootMarkers: [] }; vi.spyOn(lspConfig, "loadConfig").mockReturnValue({ servers: { fake: config }, idleTimeoutMs: undefined }); const tool = new LspTool(makeLspSession(tempDir.path())); @@ -3699,7 +3699,7 @@ describe("lsp regressions", () => { srv.exit(0); } }); - const config: ServerConfig = { command: "fake-reload-cancel-fb", fileTypes: [".ts"], rootMarkers: [] }; + const config: ServerConfig = { command: "rust-analyzer", fileTypes: [".ts"], rootMarkers: [] }; vi.spyOn(lspConfig, "loadConfig").mockReturnValue({ servers: { fake: config }, idleTimeoutMs: undefined }); const tool = new LspTool(makeLspSession(tempDir.path())); @@ -3727,7 +3727,7 @@ describe("lsp regressions", () => { srv.exit(0); } }); - const config: ServerConfig = { command: "fake-reload-fallback", fileTypes: [".ts"], rootMarkers: [] }; + const config: ServerConfig = { command: "rust-analyzer", fileTypes: [".ts"], rootMarkers: [] }; vi.spyOn(lspConfig, "loadConfig").mockReturnValue({ servers: { fake: config }, idleTimeoutMs: undefined }); const tool = new LspTool(makeLspSession(tempDir.path())); @@ -3758,7 +3758,7 @@ describe("lsp regressions", () => { srv.exit(0); } }); - const config: ServerConfig = { command: "fake-reload-fallback-code", fileTypes: [".ts"], rootMarkers: [] }; + const config: ServerConfig = { command: "rust-analyzer", fileTypes: [".ts"], rootMarkers: [] }; vi.spyOn(lspConfig, "loadConfig").mockReturnValue({ servers: { fake: config }, idleTimeoutMs: undefined }); const tool = new LspTool(makeLspSession(tempDir.path())); @@ -3773,6 +3773,43 @@ describe("lsp regressions", () => { } }); + it("does not send rust-analyzer/reloadWorkspace to a non-rust server that crashes on it (#8571)", async () => { + const tempDir = TempDir.createSync("@omp-lsp-reload-non-rust-"); + try { + let sawRustReload = false; + const server = installFakeLsp((message, srv) => { + if (message.method === "initialize") { + srv.send({ jsonrpc: "2.0", id: message.id, result: { capabilities: {} } }); + } else if (message.method === "rust-analyzer/reloadWorkspace") { + // Roslyn crashes the whole process instead of replying -32601 + // (dotnet/roslyn#84890); the gate must never let us send this. + sawRustReload = true; + srv.exit(82); + } 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: "roslyn-language-server", fileTypes: [".cs"], rootMarkers: [] }; + vi.spyOn(lspConfig, "loadConfig").mockReturnValue({ + servers: { csharp: config }, + idleTimeoutMs: undefined, + }); + + const tool = new LspTool(makeLspSession(tempDir.path())); + const result = await tool.execute("reload-non-rust", { action: "reload", file: "*" }); + + expect(sawRustReload).toBe(false); + expect(textResult(result)).toContain("Reloaded csharp"); + expect(server.killed).toBe(false); + } finally { + vi.restoreAllMocks(); + await lspClient.shutdownAll(); + tempDir.removeSync(); + } + }); + it("shutdownClientInstance removes the client by identity and confirms process exit", async () => { const tempDir = TempDir.createSync("@omp-lsp-teardown-confirm-"); try {