diff --git a/src/features/skill-mcp-manager/manager.test.ts b/src/features/skill-mcp-manager/manager.test.ts index 4a315d442..f65aa5c55 100644 --- a/src/features/skill-mcp-manager/manager.test.ts +++ b/src/features/skill-mcp-manager/manager.test.ts @@ -395,6 +395,35 @@ describe("SkillMcpManager", () => { // then expect(manager.getConnectedServers()).toEqual([]) }) + + it("unregisters signal handlers after disconnectAll", async () => { + // given + const info: SkillMcpClientInfo = { + serverName: "signal-server", + skillName: "signal-skill", + sessionID: "session-1", + } + const config: ClaudeCodeMcpServer = { + url: "https://example.com/mcp", + } + + const before = process.listenerCount("SIGINT") + + // when + try { + await manager.getOrCreateClient(info, config) + } catch { + // Expected to fail connection, still registers cleanup handlers + } + const afterRegister = process.listenerCount("SIGINT") + + await manager.disconnectAll() + const afterDisconnect = process.listenerCount("SIGINT") + + // then + expect(afterRegister).toBe(before + 1) + expect(afterDisconnect).toBe(before) + }) }) describe("isConnected", () => { diff --git a/src/features/skill-mcp-manager/manager.ts b/src/features/skill-mcp-manager/manager.ts index 4173f1d77..43cb3dd89 100644 --- a/src/features/skill-mcp-manager/manager.ts +++ b/src/features/skill-mcp-manager/manager.ts @@ -65,6 +65,7 @@ export class SkillMcpManager { private authProviders: Map = new Map() private cleanupRegistered = false private cleanupInterval: ReturnType | null = null + private cleanupHandlers: Array<{ signal: NodeJS.Signals; listener: () => void }> = [] private readonly IDLE_TIMEOUT = 5 * 60 * 1000 private getClientKey(info: SkillMcpClientInfo): string { @@ -119,11 +120,26 @@ export class SkillMcpManager { // Don't call process.exit() here - let the background-agent manager handle the final process exit. // Use void + catch to trigger async cleanup without awaiting it in the signal handler. - process.on("SIGINT", () => void cleanup().catch(() => {})) - process.on("SIGTERM", () => void cleanup().catch(() => {})) - if (process.platform === "win32") { - process.on("SIGBREAK", () => void cleanup().catch(() => {})) + const register = (signal: NodeJS.Signals) => { + const listener = () => void cleanup().catch(() => {}) + this.cleanupHandlers.push({ signal, listener }) + process.on(signal, listener) } + + register("SIGINT") + register("SIGTERM") + if (process.platform === "win32") { + register("SIGBREAK") + } + } + + private unregisterProcessCleanup(): void { + if (!this.cleanupRegistered) return + for (const { signal, listener } of this.cleanupHandlers) { + process.off(signal, listener) + } + this.cleanupHandlers = [] + this.cleanupRegistered = false } async getOrCreateClient( @@ -376,12 +392,23 @@ export class SkillMcpManager { } } } + + for (const key of keysToRemove) { + this.pendingConnections.delete(key) + } + + if (this.clients.size === 0) { + this.stopCleanupTimer() + } } async disconnectAll(): Promise { this.stopCleanupTimer() + this.unregisterProcessCleanup() const clients = Array.from(this.clients.values()) this.clients.clear() + this.pendingConnections.clear() + this.authProviders.clear() for (const managed of clients) { try { await managed.client.close() @@ -420,6 +447,10 @@ export class SkillMcpManager { } catch { /* transport may already be terminated */ } } } + + if (this.clients.size === 0) { + this.stopCleanupTimer() + } } async listTools( diff --git a/src/tools/delegate-task/tools.test.ts b/src/tools/delegate-task/tools.test.ts index 46736727f..e72dc25e3 100644 --- a/src/tools/delegate-task/tools.test.ts +++ b/src/tools/delegate-task/tools.test.ts @@ -237,7 +237,7 @@ describe("sisyphus-task", () => { // then proceeds without error - uses fallback chain expect(result).not.toContain("oh-my-opencode requires a default model") - }) + }, { timeout: 10000 }) test("returns clear error when no model can be resolved", async () => { // given - custom category with no model, no systemDefaultModel, no available models