fix(lsp): reset safety block on server restart to prevent permanent blocks (#1366)
This commit is contained in:
parent
6ce482668b
commit
4738379ad7
@ -8,6 +8,17 @@ const TEST_DIR = join(tmpdir(), "mcp-loader-test-" + Date.now())
|
|||||||
describe("getSystemMcpServerNames", () => {
|
describe("getSystemMcpServerNames", () => {
|
||||||
beforeEach(() => {
|
beforeEach(() => {
|
||||||
mkdirSync(TEST_DIR, { recursive: true })
|
mkdirSync(TEST_DIR, { recursive: true })
|
||||||
|
|
||||||
|
// Isolate tests from real user environment (e.g., ~/.claude.json).
|
||||||
|
// loader.ts reads user-level config via os.homedir() + getClaudeConfigDir().
|
||||||
|
mock.module("os", () => ({
|
||||||
|
homedir: () => TEST_DIR,
|
||||||
|
tmpdir,
|
||||||
|
}))
|
||||||
|
|
||||||
|
mock.module("../../shared", () => ({
|
||||||
|
getClaudeConfigDir: () => join(TEST_DIR, ".claude"),
|
||||||
|
}))
|
||||||
})
|
})
|
||||||
|
|
||||||
afterEach(() => {
|
afterEach(() => {
|
||||||
|
|||||||
@ -1,8 +1,9 @@
|
|||||||
|
import { afterEach, describe, expect, test } from "bun:test"
|
||||||
import { _resetForTesting, setMainSession } from "../../features/claude-code-session-state"
|
import { _resetForTesting, setMainSession } from "../../features/claude-code-session-state"
|
||||||
import type { BackgroundTask } from "../../features/background-agent"
|
import type { BackgroundTask } from "../../features/background-agent"
|
||||||
import { createUnstableAgentBabysitterHook } from "./index"
|
import { createUnstableAgentBabysitterHook } from "./index"
|
||||||
|
|
||||||
const projectDir = "/Users/yeongyu/local-workspaces/oh-my-opencode"
|
const projectDir = process.cwd()
|
||||||
|
|
||||||
type BabysitterContext = Parameters<typeof createUnstableAgentBabysitterHook>[0]
|
type BabysitterContext = Parameters<typeof createUnstableAgentBabysitterHook>[0]
|
||||||
|
|
||||||
@ -21,6 +22,9 @@ function createMockPluginInput(options: {
|
|||||||
prompt: async (input: unknown) => {
|
prompt: async (input: unknown) => {
|
||||||
promptCalls.push({ input })
|
promptCalls.push({ input })
|
||||||
},
|
},
|
||||||
|
promptAsync: async (input: unknown) => {
|
||||||
|
promptCalls.push({ input })
|
||||||
|
},
|
||||||
},
|
},
|
||||||
},
|
},
|
||||||
}
|
}
|
||||||
|
|||||||
@ -2,7 +2,7 @@ import { mkdtempSync, rmSync, writeFileSync } from "node:fs"
|
|||||||
import { join } from "node:path"
|
import { join } from "node:path"
|
||||||
import { tmpdir } from "node:os"
|
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", () => ({
|
mock.module("vscode-jsonrpc/node", () => ({
|
||||||
createMessageConnection: () => {
|
createMessageConnection: () => {
|
||||||
@ -12,10 +12,18 @@ mock.module("vscode-jsonrpc/node", () => ({
|
|||||||
StreamMessageWriter: function StreamMessageWriter() {},
|
StreamMessageWriter: function StreamMessageWriter() {},
|
||||||
}))
|
}))
|
||||||
|
|
||||||
import { LSPClient, validateCwd } from "./client"
|
import { LSPClient, lspManager, validateCwd } from "./client"
|
||||||
import type { ResolvedServer } from "./types"
|
import type { ResolvedServer } from "./types"
|
||||||
|
|
||||||
describe("LSPClient", () => {
|
describe("LSPClient", () => {
|
||||||
|
beforeEach(async () => {
|
||||||
|
await lspManager.stopAll()
|
||||||
|
})
|
||||||
|
|
||||||
|
afterEach(async () => {
|
||||||
|
await lspManager.stopAll()
|
||||||
|
})
|
||||||
|
|
||||||
describe("openFile", () => {
|
describe("openFile", () => {
|
||||||
it("sends didChange when a previously opened file changes on disk", async () => {
|
it("sends didChange when a previously opened file changes on disk", async () => {
|
||||||
// #given
|
// #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", () => {
|
describe("validateCwd", () => {
|
||||||
it("returns valid for existing directory", () => {
|
it("returns valid for existing directory", () => {
|
||||||
// #given
|
// #given
|
||||||
|
|||||||
@ -221,6 +221,7 @@ interface ManagedClient {
|
|||||||
refCount: number
|
refCount: number
|
||||||
initPromise?: Promise<void>
|
initPromise?: Promise<void>
|
||||||
isInitializing: boolean
|
isInitializing: boolean
|
||||||
|
initializingSince?: number
|
||||||
}
|
}
|
||||||
|
|
||||||
class LSPServerManager {
|
class LSPServerManager {
|
||||||
@ -228,6 +229,7 @@ class LSPServerManager {
|
|||||||
private clients = new Map<string, ManagedClient>()
|
private clients = new Map<string, ManagedClient>()
|
||||||
private cleanupInterval: ReturnType<typeof setInterval> | null = null
|
private cleanupInterval: ReturnType<typeof setInterval> | null = null
|
||||||
private readonly IDLE_TIMEOUT = 5 * 60 * 1000
|
private readonly IDLE_TIMEOUT = 5 * 60 * 1000
|
||||||
|
private readonly INIT_TIMEOUT = 60 * 1000
|
||||||
|
|
||||||
private constructor() {
|
private constructor() {
|
||||||
this.startCleanupTimer()
|
this.startCleanupTimer()
|
||||||
@ -309,17 +311,43 @@ class LSPServerManager {
|
|||||||
const key = this.getKey(root, server.id)
|
const key = this.getKey(root, server.id)
|
||||||
|
|
||||||
let managed = this.clients.get(key)
|
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) {
|
||||||
if (managed.initPromise) {
|
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++
|
if (managed) {
|
||||||
managed.lastUsedAt = Date.now()
|
if (managed.client.isAlive()) {
|
||||||
return managed.client
|
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)
|
const client = new LSPClient(root, server)
|
||||||
@ -328,19 +356,30 @@ class LSPServerManager {
|
|||||||
await client.initialize()
|
await client.initialize()
|
||||||
})()
|
})()
|
||||||
|
|
||||||
|
const initStartedAt = Date.now()
|
||||||
this.clients.set(key, {
|
this.clients.set(key, {
|
||||||
client,
|
client,
|
||||||
lastUsedAt: Date.now(),
|
lastUsedAt: initStartedAt,
|
||||||
refCount: 1,
|
refCount: 1,
|
||||||
initPromise,
|
initPromise,
|
||||||
isInitializing: true,
|
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)
|
const m = this.clients.get(key)
|
||||||
if (m) {
|
if (m) {
|
||||||
m.initPromise = undefined
|
m.initPromise = undefined
|
||||||
m.isInitializing = false
|
m.isInitializing = false
|
||||||
|
m.initializingSince = undefined
|
||||||
}
|
}
|
||||||
|
|
||||||
return client
|
return client
|
||||||
@ -356,21 +395,30 @@ class LSPServerManager {
|
|||||||
await client.initialize()
|
await client.initialize()
|
||||||
})()
|
})()
|
||||||
|
|
||||||
|
const initStartedAt = Date.now()
|
||||||
this.clients.set(key, {
|
this.clients.set(key, {
|
||||||
client,
|
client,
|
||||||
lastUsedAt: Date.now(),
|
lastUsedAt: initStartedAt,
|
||||||
refCount: 0,
|
refCount: 0,
|
||||||
initPromise,
|
initPromise,
|
||||||
isInitializing: true,
|
isInitializing: true,
|
||||||
|
initializingSince: initStartedAt,
|
||||||
})
|
})
|
||||||
|
|
||||||
initPromise.then(() => {
|
initPromise
|
||||||
const m = this.clients.get(key)
|
.then(() => {
|
||||||
if (m) {
|
const m = this.clients.get(key)
|
||||||
m.initPromise = undefined
|
if (m) {
|
||||||
m.isInitializing = false
|
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 {
|
releaseClient(root: string, serverId: string): void {
|
||||||
|
|||||||
Loading…
x
Reference in New Issue
Block a user