fix: address review-work round 6 findings (dispose isolation, event dispatch, disconnectedSessions ref-counting)

src/features/skill-mcp-manager/cleanup.ts

@@ -134,6 +134,7 @@ export async function disconnectAll(state: SkillMcpManagerState): Promise<void>
   state.clients.clear()
   state.pendingConnections.clear()
   state.disconnectedSessions.clear()
+  state.inFlightConnections.clear()
   state.authProviders.clear()
 
   for (const managed of clients) {

src/features/skill-mcp-manager/connection-race.test.ts

@@ -80,6 +80,7 @@ function createState(): SkillMcpManagerState {
     cleanupHandlers: [],
     idleTimeoutMs: 5 * 60 * 1000,
     shutdownGeneration: 0,
+    inFlightConnections: new Map(),
   }
 
   trackedStates.push(state)
@@ -136,13 +137,13 @@ describe("getOrCreateClient disconnect race", () => {
     await expect(clientPromise).rejects.toThrow(/disconnected during MCP connection setup/)
     expect(state.clients.has(clientKey)).toBe(false)
     expect(state.pendingConnections.has(clientKey)).toBe(false)
-    expect(state.disconnectedSessions.has(info.sessionID)).toBe(true)
+    expect(state.disconnectedSessions.has(info.sessionID)).toBe(false)
     expect(createdClients).toHaveLength(1)
     expect(createdClients[0]?.close).toHaveBeenCalledTimes(1)
     expect(createdTransports[0]?.close).toHaveBeenCalledTimes(1)
   })
 
-  it("#given session A in disconnectedSessions #when new connection is requested for session A #then connection proceeds normally and disconnectedSessions entry is retained for pending race protection", async () => {
+  it("#given session A in disconnectedSessions #when new connection completes with no remaining pending #then disconnectedSessions entry is cleaned up", async () => {
     const state = createState()
     const info = createClientInfo("session-a")
     const clientKey = createClientKey(info)
@@ -150,7 +151,7 @@ describe("getOrCreateClient disconnect race", () => {
 
     const client = await getOrCreateClient({ state, clientKey, info, config: stdioConfig })
 
-    expect(state.disconnectedSessions.has(info.sessionID)).toBe(true)
+    expect(state.disconnectedSessions.has(info.sessionID)).toBe(false)
     expect(state.clients.get(clientKey)?.client).toBe(client)
     expect(createdClients[0]?.close).not.toHaveBeenCalled()
   })
@@ -210,5 +211,6 @@ describe("getOrCreateClient multi-key disconnect race", () => {
 
     expect(state.clients.has(clientKey1)).toBe(false)
     expect(state.clients.has(clientKey2)).toBe(false)
+    expect(state.disconnectedSessions.has("session-a")).toBe(false)
   })
 })

src/features/skill-mcp-manager/connection.ts

@@ -29,6 +29,7 @@ export async function getOrCreateClient(params: {
 
   const expandedConfig = expandEnvVarsInObject(config)
   let currentConnectionPromise!: Promise<Client>
+  state.inFlightConnections.set(info.sessionID, (state.inFlightConnections.get(info.sessionID) ?? 0) + 1)
   currentConnectionPromise = (async () => {
     const disconnectGenAtStart = state.disconnectedSessions.get(info.sessionID) ?? 0
     const shutdownGenAtStart = state.shutdownGeneration
@@ -64,6 +65,13 @@ export async function getOrCreateClient(params: {
     if (state.pendingConnections.get(clientKey) === currentConnectionPromise) {
       state.pendingConnections.delete(clientKey)
     }
+    const remaining = (state.inFlightConnections.get(info.sessionID) ?? 1) - 1
+    if (remaining <= 0) {
+      state.inFlightConnections.delete(info.sessionID)
+      state.disconnectedSessions.delete(info.sessionID)
+    } else {
+      state.inFlightConnections.set(info.sessionID, remaining)
+    }
   }
 }
 

src/features/skill-mcp-manager/disconnect-cleanup.test.ts

@@ -25,6 +25,7 @@ function createState(): SkillMcpManagerState {
     cleanupHandlers: [],
     idleTimeoutMs: 5 * 60 * 1000,
     shutdownGeneration: 0,
+    inFlightConnections: new Map(),
   }
 
   trackedStates.push(state)

src/features/skill-mcp-manager/manager.ts

@@ -17,6 +17,7 @@ export class SkillMcpManager {
     cleanupHandlers: [],
     idleTimeoutMs: 5 * 60 * 1000,
     shutdownGeneration: 0,
+    inFlightConnections: new Map(),
   }
 
   private getClientKey(info: SkillMcpClientInfo): string {

src/features/skill-mcp-manager/types.ts

@@ -58,6 +58,7 @@ export interface SkillMcpManagerState {
   cleanupHandlers: ProcessCleanupHandler[]
   idleTimeoutMs: number
   shutdownGeneration: number
+  inFlightConnections: Map<string, number>
 }
 
 export interface SkillMcpClientConnectionParams {

src/plugin-dispose.test.ts

@@ -116,4 +116,60 @@ describe("createPluginDispose", () => {
     expect(disconnectAllSpy).toHaveBeenCalledTimes(1)
     expect(disposeHooksSpy).toHaveBeenCalledTimes(1)
   })
+
+  test("#given backgroundManager.shutdown() throws #when dispose() is called #then skillMcpManager.disconnectAll() and disposeHooks() are still called", async () => {
+    // given
+    const backgroundManager = {
+      shutdown: async (): Promise<void> => {
+        throw new Error("shutdown failed")
+      },
+    }
+    const skillMcpManager = {
+      disconnectAll: async (): Promise<void> => {},
+    }
+    const disposeHooksCalls: number[] = []
+    const disconnectAllSpy = spyOn(skillMcpManager, "disconnectAll")
+    const dispose = createPluginDispose({
+      backgroundManager,
+      skillMcpManager,
+      disposeHooks: (): void => {
+        disposeHooksCalls.push(1)
+      },
+    })
+
+    // when
+    await dispose()
+
+    // then
+    expect(disconnectAllSpy).toHaveBeenCalledTimes(1)
+    expect(disposeHooksCalls).toHaveLength(1)
+  })
+
+  test("#given skillMcpManager.disconnectAll() throws #when dispose() is called #then disposeHooks() is still called", async () => {
+    // given
+    const backgroundManager = {
+      shutdown: async (): Promise<void> => {},
+    }
+    const skillMcpManager = {
+      disconnectAll: async (): Promise<void> => {
+        throw new Error("disconnectAll failed")
+      },
+    }
+    const disposeHooksCalls: number[] = []
+    const shutdownSpy = spyOn(backgroundManager, "shutdown")
+    const dispose = createPluginDispose({
+      backgroundManager,
+      skillMcpManager,
+      disposeHooks: (): void => {
+        disposeHooksCalls.push(1)
+      },
+    })
+
+    // when
+    await dispose()
+
+    // then
+    expect(shutdownSpy).toHaveBeenCalledTimes(1)
+    expect(disposeHooksCalls).toHaveLength(1)
+  })
 })

src/plugin-dispose.ts

@@ -1,3 +1,5 @@
+import { log } from "./shared"
+
 export type PluginDispose = () => Promise<void>
 
 export function createPluginDispose(args: {
@@ -19,9 +21,21 @@ export function createPluginDispose(args: {
     }
 
     disposePromise = (async (): Promise<void> => {
-      await backgroundManager.shutdown()
+      try {
-      await skillMcpManager.disconnectAll()
+        await backgroundManager.shutdown()
-      disposeHooks()
+      } catch (error) {
+        log("[plugin-dispose] backgroundManager.shutdown() error:", error)
+      }
+      try {
+        await skillMcpManager.disconnectAll()
+      } catch (error) {
+        log("[plugin-dispose] skillMcpManager.disconnectAll() error:", error)
+      }
+      try {
+        disposeHooks()
+      } catch (error) {
+        log("[plugin-dispose] disposeHooks() error:", error)
+      }
     })()
 
     await disposePromise

src/plugin/event.ts

@@ -190,6 +190,7 @@ export function createEventHandler(args: {
     await Promise.resolve(hooks.compactionTodoPreserver?.event?.(input));
     await Promise.resolve(hooks.writeExistingFileGuard?.event?.(input));
     await Promise.resolve(hooks.atlasHook?.handler?.(input));
+    await Promise.resolve(hooks.autoSlashCommand?.event?.(input));
   };
 
   const recentSyntheticIdles = new Map<string, number>();