fix(dispose): improve hook disposal and plugin cleanup

🤖 Generated with assistance of OhMyOpenCode

src/create-hooks.ts

@@ -14,12 +14,16 @@ export type CreatedHooks = ReturnType<typeof createHooks>
 type DisposableHook = { dispose?: () => void } | null | undefined
 
 export type DisposableCreatedHooks = {
+  claudeCodeHooks?: DisposableHook
+  commentChecker?: DisposableHook
   runtimeFallback?: DisposableHook
   todoContinuationEnforcer?: DisposableHook
   autoSlashCommand?: DisposableHook
 }
 
 export function disposeCreatedHooks(hooks: DisposableCreatedHooks): void {
+	 hooks.claudeCodeHooks?.dispose?.()
+	 hooks.commentChecker?.dispose?.()
   hooks.runtimeFallback?.dispose?.()
   hooks.todoContinuationEnforcer?.dispose?.()
   hooks.autoSlashCommand?.dispose?.()

src/plugin-dispose.test.ts

@@ -12,10 +12,14 @@ describe("createPluginDispose", () => {
     const skillMcpManager = {
       disconnectAll: async (): Promise<void> => {},
     }
+    const lspManager = {
+      stopAll: async (): Promise<void> => {},
+    }
     const shutdownSpy = spyOn(backgroundManager, "shutdown")
     const dispose = createPluginDispose({
       backgroundManager,
       skillMcpManager,
+      lspManager,
       disposeHooks: (): void => {},
     })
 
@@ -34,10 +38,14 @@ describe("createPluginDispose", () => {
     const skillMcpManager = {
       disconnectAll: async (): Promise<void> => {},
     }
+    const lspManager = {
+      stopAll: async (): Promise<void> => {},
+    }
     const disconnectAllSpy = spyOn(skillMcpManager, "disconnectAll")
     const dispose = createPluginDispose({
       backgroundManager,
       skillMcpManager,
+      lspManager,
       disposeHooks: (): void => {},
     })
 
@@ -50,6 +58,12 @@ describe("createPluginDispose", () => {
 
   test("#given plugin with hooks that have dispose #when dispose() is called #then each hook's dispose is called", async () => {
     // given
+    const claudeCodeHooks = {
+      dispose: (): void => {},
+    }
+    const commentChecker = {
+      dispose: (): void => {},
+    }
     const runtimeFallback = {
       dispose: (): void => {},
     }
@@ -59,6 +73,11 @@ describe("createPluginDispose", () => {
     const autoSlashCommand = {
       dispose: (): void => {},
     }
+    const lspManager = {
+      stopAll: async (): Promise<void> => {},
+    }
+    const claudeCodeHooksDisposeSpy = spyOn(claudeCodeHooks, "dispose")
+    const commentCheckerDisposeSpy = spyOn(commentChecker, "dispose")
     const runtimeFallbackDisposeSpy = spyOn(runtimeFallback, "dispose")
     const todoContinuationEnforcerDisposeSpy = spyOn(todoContinuationEnforcer, "dispose")
     const autoSlashCommandDisposeSpy = spyOn(autoSlashCommand, "dispose")
@@ -69,8 +88,11 @@ describe("createPluginDispose", () => {
       skillMcpManager: {
         disconnectAll: async (): Promise<void> => {},
       },
+      lspManager,
       disposeHooks: (): void => {
         disposeCreatedHooks({
+          claudeCodeHooks,
+          commentChecker,
           runtimeFallback,
           todoContinuationEnforcer,
           autoSlashCommand,
@@ -82,6 +104,8 @@ describe("createPluginDispose", () => {
     await dispose()
 
     // then
+    expect(claudeCodeHooksDisposeSpy).toHaveBeenCalledTimes(1)
+    expect(commentCheckerDisposeSpy).toHaveBeenCalledTimes(1)
     expect(runtimeFallbackDisposeSpy).toHaveBeenCalledTimes(1)
     expect(todoContinuationEnforcerDisposeSpy).toHaveBeenCalledTimes(1)
     expect(autoSlashCommandDisposeSpy).toHaveBeenCalledTimes(1)
@@ -95,15 +119,20 @@ describe("createPluginDispose", () => {
     const skillMcpManager = {
       disconnectAll: async (): Promise<void> => {},
     }
+    const lspManager = {
+      stopAll: async (): Promise<void> => {},
+    }
     const disposeHooks = {
       run: (): void => {},
     }
     const shutdownSpy = spyOn(backgroundManager, "shutdown")
     const disconnectAllSpy = spyOn(skillMcpManager, "disconnectAll")
+    const stopAllSpy = spyOn(lspManager, "stopAll")
     const disposeHooksSpy = spyOn(disposeHooks, "run")
     const dispose = createPluginDispose({
       backgroundManager,
       skillMcpManager,
+      lspManager,
       disposeHooks: disposeHooks.run,
     })
 
@@ -112,9 +141,10 @@ describe("createPluginDispose", () => {
     await dispose()
 
     // then
-    expect(shutdownSpy).toHaveBeenCalledTimes(1)
-    expect(disconnectAllSpy).toHaveBeenCalledTimes(1)
-    expect(disposeHooksSpy).toHaveBeenCalledTimes(1)
+      expect(shutdownSpy).toHaveBeenCalledTimes(1)
+      expect(disconnectAllSpy).toHaveBeenCalledTimes(1)
+      expect(stopAllSpy).toHaveBeenCalledTimes(1)
+      expect(disposeHooksSpy).toHaveBeenCalledTimes(1)
   })
 
   test("#given backgroundManager.shutdown() throws #when dispose() is called #then skillMcpManager.disconnectAll() and disposeHooks() are still called", async () => {
@@ -127,11 +157,15 @@ describe("createPluginDispose", () => {
     const skillMcpManager = {
       disconnectAll: async (): Promise<void> => {},
     }
+    const lspManager = {
+      stopAll: async (): Promise<void> => {},
+    }
     const disposeHooksCalls: number[] = []
     const disconnectAllSpy = spyOn(skillMcpManager, "disconnectAll")
     const dispose = createPluginDispose({
       backgroundManager,
       skillMcpManager,
+      lspManager,
       disposeHooks: (): void => {
         disposeHooksCalls.push(1)
       },
@@ -155,11 +189,15 @@ describe("createPluginDispose", () => {
         throw new Error("disconnectAll failed")
       },
     }
+    const lspManager = {
+      stopAll: async (): Promise<void> => {},
+    }
     const disposeHooksCalls: number[] = []
     const shutdownSpy = spyOn(backgroundManager, "shutdown")
     const dispose = createPluginDispose({
       backgroundManager,
       skillMcpManager,
+      lspManager,
       disposeHooks: (): void => {
         disposeHooksCalls.push(1)
       },
@@ -172,4 +210,28 @@ describe("createPluginDispose", () => {
     expect(shutdownSpy).toHaveBeenCalledTimes(1)
     expect(disposeHooksCalls).toHaveLength(1)
   })
+
+  test("#given active LSP clients #when dispose runs #then lsp manager is stopped", async () => {
+    // given
+    const lspManager = {
+      stopAll: async (): Promise<void> => {},
+    }
+    const stopAllSpy = spyOn(lspManager, "stopAll")
+    const dispose = createPluginDispose({
+      backgroundManager: {
+        shutdown: async (): Promise<void> => {},
+      },
+      skillMcpManager: {
+        disconnectAll: async (): Promise<void> => {},
+      },
+      lspManager,
+      disposeHooks: (): void => {},
+    })
+
+    // when
+    await dispose()
+
+    // then
+    expect(stopAllSpy).toHaveBeenCalledTimes(1)
+  })
 })

src/plugin-dispose.ts

@@ -9,9 +9,12 @@ export function createPluginDispose(args: {
   skillMcpManager: {
     disconnectAll: () => Promise<void>
   }
+  lspManager: {
+    stopAll: () => Promise<void>
+  }
   disposeHooks: () => void
 }): PluginDispose {
-  const { backgroundManager, skillMcpManager, disposeHooks } = args
+  const { backgroundManager, skillMcpManager, lspManager, disposeHooks } = args
   let disposePromise: Promise<void> | null = null
 
   return async (): Promise<void> => {
@@ -31,6 +34,11 @@ export function createPluginDispose(args: {
       } catch (error) {
         log("[plugin-dispose] skillMcpManager.disconnectAll() error:", error)
       }
+      try {
+        await lspManager.stopAll()
+      } catch (error) {
+        log("[plugin-dispose] lspManager.stopAll() error:", error)
+      }
       try {
         disposeHooks()
       } catch (error) {