Merge pull request #1657 from code-yeongyu/fix-1366-lsp-unblock

fix(lsp): reset safety block on server restart (#1366)

src/tools/lsp/client.test.ts

@@ -2,7 +2,7 @@ import { mkdtempSync, rmSync, writeFileSync } from "node:fs"
 import { join } from "node:path"
 import { tmpdir } from "node:os"
 
-import { describe, it, expect, spyOn, mock } from "bun:test"
+import { describe, it, expect, spyOn, mock, beforeEach, afterEach } from "bun:test"
 
 mock.module("vscode-jsonrpc/node", () => ({
   createMessageConnection: () => {
@@ -12,10 +12,18 @@ mock.module("vscode-jsonrpc/node", () => ({
   StreamMessageWriter: function StreamMessageWriter() {},
 }))
 
-import { LSPClient, validateCwd } from "./client"
+import { LSPClient, lspManager, validateCwd } from "./client"
 import type { ResolvedServer } from "./types"
 
 describe("LSPClient", () => {
+  beforeEach(async () => {
+    await lspManager.stopAll()
+  })
+
+  afterEach(async () => {
+    await lspManager.stopAll()
+  })
+
   describe("openFile", () => {
     it("sends didChange when a previously opened file changes on disk", async () => {
       // #given
@@ -61,6 +69,108 @@ describe("LSPClient", () => {
     })
   })
 
+  describe("LSPServerManager", () => {
+    it("recreates client after init failure instead of staying permanently blocked", async () => {
+      //#given
+      const dir = mkdtempSync(join(tmpdir(), "lsp-manager-test-"))
+
+      const server: ResolvedServer = {
+        id: "typescript",
+        command: ["typescript-language-server", "--stdio"],
+        extensions: [".ts"],
+        priority: 0,
+      }
+
+      const startSpy = spyOn(LSPClient.prototype, "start")
+      const initializeSpy = spyOn(LSPClient.prototype, "initialize")
+      const isAliveSpy = spyOn(LSPClient.prototype, "isAlive")
+      const stopSpy = spyOn(LSPClient.prototype, "stop")
+
+      startSpy.mockImplementationOnce(async () => {
+        throw new Error("boom")
+      })
+      startSpy.mockImplementation(async () => {})
+      initializeSpy.mockImplementation(async () => {})
+      isAliveSpy.mockImplementation(() => true)
+      stopSpy.mockImplementation(async () => {})
+
+      try {
+        //#when
+        await expect(lspManager.getClient(dir, server)).rejects.toThrow("boom")
+
+        const client = await lspManager.getClient(dir, server)
+
+        //#then
+        expect(client).toBeInstanceOf(LSPClient)
+        expect(startSpy).toHaveBeenCalledTimes(2)
+        expect(stopSpy).toHaveBeenCalled()
+      } finally {
+        startSpy.mockRestore()
+        initializeSpy.mockRestore()
+        isAliveSpy.mockRestore()
+        stopSpy.mockRestore()
+        rmSync(dir, { recursive: true, force: true })
+      }
+    })
+
+    it("resets stale initializing entry so a hung init does not permanently block future clients", async () => {
+      //#given
+      const dir = mkdtempSync(join(tmpdir(), "lsp-manager-stale-test-"))
+
+      const server: ResolvedServer = {
+        id: "typescript",
+        command: ["typescript-language-server", "--stdio"],
+        extensions: [".ts"],
+        priority: 0,
+      }
+
+      const dateNowSpy = spyOn(Date, "now")
+
+      const startSpy = spyOn(LSPClient.prototype, "start")
+      const initializeSpy = spyOn(LSPClient.prototype, "initialize")
+      const isAliveSpy = spyOn(LSPClient.prototype, "isAlive")
+      const stopSpy = spyOn(LSPClient.prototype, "stop")
+
+      // First client init hangs forever.
+      const never = new Promise<void>(() => {})
+      startSpy.mockImplementationOnce(async () => {
+        await never
+      })
+
+      // Second attempt should be allowed after stale reset.
+      startSpy.mockImplementationOnce(async () => {})
+      startSpy.mockImplementation(async () => {})
+      initializeSpy.mockImplementation(async () => {})
+      isAliveSpy.mockImplementation(() => true)
+      stopSpy.mockImplementation(async () => {})
+
+      try {
+        //#when
+        dateNowSpy.mockReturnValueOnce(0)
+        lspManager.warmupClient(dir, server)
+
+        dateNowSpy.mockReturnValueOnce(60_000)
+
+        const client = await Promise.race([
+          lspManager.getClient(dir, server),
+          new Promise<never>((_, reject) => setTimeout(() => reject(new Error("test-timeout")), 50)),
+        ])
+
+        //#then
+        expect(client).toBeInstanceOf(LSPClient)
+        expect(startSpy).toHaveBeenCalledTimes(2)
+        expect(stopSpy).toHaveBeenCalled()
+      } finally {
+        dateNowSpy.mockRestore()
+        startSpy.mockRestore()
+        initializeSpy.mockRestore()
+        isAliveSpy.mockRestore()
+        stopSpy.mockRestore()
+        rmSync(dir, { recursive: true, force: true })
+      }
+    })
+  })
+
   describe("validateCwd", () => {
     it("returns valid for existing directory", () => {
       // #given

src/tools/lsp/client.ts

@@ -221,6 +221,7 @@ interface ManagedClient {
   refCount: number
   initPromise?: Promise<void>
   isInitializing: boolean
+  initializingSince?: number
 }
 
 class LSPServerManager {
@@ -228,6 +229,7 @@ class LSPServerManager {
   private clients = new Map<string, ManagedClient>()
   private cleanupInterval: ReturnType<typeof setInterval> | null = null
   private readonly IDLE_TIMEOUT = 5 * 60 * 1000
+  private readonly INIT_TIMEOUT = 60 * 1000
 
   private constructor() {
     this.startCleanupTimer()
@@ -309,17 +311,43 @@ class LSPServerManager {
     const key = this.getKey(root, server.id)
 
     let managed = this.clients.get(key)
+    if (managed) {
+      const now = Date.now()
+      if (managed.isInitializing && managed.initializingSince !== undefined && now - managed.initializingSince >= this.INIT_TIMEOUT) {
+        // Stale init can permanently block subsequent calls (e.g., LSP process hang)
+        try {
+          await managed.client.stop()
+        } catch {}
+        this.clients.delete(key)
+        managed = undefined
+      }
+    }
+
     if (managed) {
       if (managed.initPromise) {
-        await managed.initPromise
+        try {
+          await managed.initPromise
+        } catch {
+          // Failed init should not keep the key blocked forever.
+          try {
+            await managed.client.stop()
+          } catch {}
+          this.clients.delete(key)
+          managed = undefined
+        }
       }
-      if (managed.client.isAlive()) {
-        managed.refCount++
-        managed.lastUsedAt = Date.now()
-        return managed.client
+
+      if (managed) {
+        if (managed.client.isAlive()) {
+          managed.refCount++
+          managed.lastUsedAt = Date.now()
+          return managed.client
+        }
+        try {
+          await managed.client.stop()
+        } catch {}
+        this.clients.delete(key)
       }
-      await managed.client.stop()
-      this.clients.delete(key)
     }
 
     const client = new LSPClient(root, server)
@@ -328,19 +356,30 @@ class LSPServerManager {
       await client.initialize()
     })()
 
+    const initStartedAt = Date.now()
     this.clients.set(key, {
       client,
-      lastUsedAt: Date.now(),
+      lastUsedAt: initStartedAt,
       refCount: 1,
       initPromise,
       isInitializing: true,
+      initializingSince: initStartedAt,
     })
 
-    await initPromise
+    try {
+      await initPromise
+    } catch (error) {
+      this.clients.delete(key)
+      try {
+        await client.stop()
+      } catch {}
+      throw error
+    }
     const m = this.clients.get(key)
     if (m) {
       m.initPromise = undefined
       m.isInitializing = false
+      m.initializingSince = undefined
     }
 
     return client
@@ -356,21 +395,30 @@ class LSPServerManager {
       await client.initialize()
     })()
 
+    const initStartedAt = Date.now()
     this.clients.set(key, {
       client,
-      lastUsedAt: Date.now(),
+      lastUsedAt: initStartedAt,
       refCount: 0,
       initPromise,
       isInitializing: true,
+      initializingSince: initStartedAt,
     })
 
-    initPromise.then(() => {
-      const m = this.clients.get(key)
-      if (m) {
-        m.initPromise = undefined
-        m.isInitializing = false
-      }
-    })
+    initPromise
+      .then(() => {
+        const m = this.clients.get(key)
+        if (m) {
+          m.initPromise = undefined
+          m.isInitializing = false
+          m.initializingSince = undefined
+        }
+      })
+      .catch(() => {
+        // Warmup failures must not permanently block future initialization.
+        this.clients.delete(key)
+        void client.stop().catch(() => {})
+      })
   }
 
   releaseClient(root: string, serverId: string): void {