From f4ed2698dd74a19a42196cc9361ca8aa6a29ecec Mon Sep 17 00:00:00 2001 From: roboomp Date: Sun, 31 May 2026 15:26:13 +0000 Subject: [PATCH] fix(lsp): shut down servers with exit notification Send the LSP exit notification after a successful shutdown response before falling back to process termination. Add a regression test that fails when a server receives shutdown but not exit.\n\nFixes #1593 --- packages/coding-agent/src/lsp/client.ts | 33 +++++-- .../test/tools/lsp-regressions.test.ts | 90 +++++++++++++++++++ 2 files changed, 118 insertions(+), 5 deletions(-) diff --git a/packages/coding-agent/src/lsp/client.ts b/packages/coding-agent/src/lsp/client.ts index 682c7eeb8..c93df3abe 100644 --- a/packages/coding-agent/src/lsp/client.ts +++ b/packages/coding-agent/src/lsp/client.ts @@ -415,6 +415,10 @@ export const WARMUP_TIMEOUT_MS = 5000; /** Max time to wait for the server to report project loading completion via $/progress */ const PROJECT_LOAD_TIMEOUT_MS = 15_000; +/** Max time to wait for graceful LSP shutdown and process exit. */ +const SHUTDOWN_TIMEOUT_MS = 5_000; +const EXIT_TIMEOUT_MS = 1_000; + /** * Get or create an LSP client for the given server configuration and working directory. * @param config - Server configuration @@ -768,8 +772,18 @@ export async function refreshFile(client: LspClient, filePath: string, signal?: } } +async function waitForExit(client: LspClient, timeoutMs: number): Promise { + return await Promise.race([ + client.proc.exited.then( + () => true, + () => true, + ), + Bun.sleep(timeoutMs).then(() => false), + ]); +} + /** - * Shutdown a specific client by key. + * Shutdown a specific client instance using the LSP shutdown/exit handshake. */ async function shutdownClientInstance(client: LspClient): Promise { const err = new Error("LSP client shutdown"); @@ -778,13 +792,22 @@ async function shutdownClientInstance(client: LspClient): Promise { } client.pendingRequests.clear(); - const timeout = Bun.sleep(5_000); - const shutdown = sendRequest(client, "shutdown", null).catch(() => {}); - await Promise.race([shutdown, timeout]); + const shutdownCompleted = await sendRequest(client, "shutdown", null, undefined, SHUTDOWN_TIMEOUT_MS).then( + () => true, + () => false, + ); + if (shutdownCompleted) { + await sendNotification(client, "exit", undefined).catch(() => {}); + if (await waitForExit(client, EXIT_TIMEOUT_MS)) return; + } + client.proc.kill(); - await Promise.race([client.proc.exited.catch(() => {}), Bun.sleep(1_000)]); + await waitForExit(client, EXIT_TIMEOUT_MS); } +/** + * Shutdown a specific client by key. + */ export async function shutdownClient(key: string): Promise { const client = clients.get(key); if (!client) return; diff --git a/packages/coding-agent/test/tools/lsp-regressions.test.ts b/packages/coding-agent/test/tools/lsp-regressions.test.ts index 447dc7594..b15bdbba4 100644 --- a/packages/coding-agent/test/tools/lsp-regressions.test.ts +++ b/packages/coding-agent/test/tools/lsp-regressions.test.ts @@ -55,6 +55,96 @@ describe("lsp regressions", () => { expect(clampTimeout("lsp", 1000)).toBe(60); }); + async function markerExists(filePath: string): Promise { + try { + await Bun.file(filePath).bytes(); + return true; + } catch (error) { + if (piUtils.isEnoent(error)) return false; + throw error; + } + } + + it("sends the LSP exit notification after shutdown completes", async () => { + const tempDir = TempDir.createSync("@omp-lsp-shutdown-"); + try { + const markerDir = tempDir.path(); + const serverPath = path.join(markerDir, "server.ts"); + await Bun.write( + serverPath, + ` +const markerDir = process.argv[2]; +const decoder = new TextDecoder(); +let buffer = ""; + +async function mark(name) { + await Bun.write(\`\${markerDir}/\${name}\`, "1\\n"); +} + +function send(message) { + const content = JSON.stringify(message); + process.stdout.write(\`Content-Length: \${Buffer.byteLength(content, "utf8")}\\r\\n\\r\\n\${content}\`); +} + +process.on("SIGTERM", () => { + void mark("sigterm").finally(() => process.abort()); +}); + +for await (const chunk of Bun.stdin.stream()) { + buffer += decoder.decode(chunk, { stream: true }); + while (true) { + const headerEnd = buffer.indexOf("\\r\\n\\r\\n"); + if (headerEnd === -1) break; + + const header = buffer.slice(0, headerEnd); + const match = /Content-Length: (\\d+)/i.exec(header); + if (!match) process.exit(2); + + const contentLength = Number(match[1]); + const contentStart = headerEnd + 4; + const contentEnd = contentStart + contentLength; + if (buffer.length < contentEnd) break; + + const message = JSON.parse(buffer.slice(contentStart, contentEnd)); + buffer = buffer.slice(contentEnd); + + if (message.method === "initialize") { + await mark("initialize"); + send({ jsonrpc: "2.0", id: message.id, result: { capabilities: {} } }); + } else if (message.method === "shutdown") { + await mark("shutdown"); + send({ jsonrpc: "2.0", id: message.id, result: null }); + } else if (message.method === "exit") { + await mark("exit"); + process.exit(0); + } + } +} + +await mark("stdin-closed"); +process.abort(); +`, + ); + + const server: ServerConfig = { + command: process.execPath, + args: [serverPath, markerDir], + fileTypes: ["ts"], + rootMarkers: [], + }; + + await lspClient.getOrCreateClient(server, tempDir.path(), 1_000); + await lspClient.shutdownAll(); + + expect(await markerExists(path.join(markerDir, "shutdown"))).toBe(true); + expect(await markerExists(path.join(markerDir, "exit"))).toBe(true); + expect(await markerExists(path.join(markerDir, "sigterm"))).toBe(false); + } finally { + await lspClient.shutdownAll(); + tempDir.removeSync(); + } + }); + it("limits glob collection to avoid large diagnostic stalls", async () => { const tempDir = TempDir.createSync("@omp-lsp-glob-"); try {