refactor(plugin): remove orphaned createPluginDispose + stale test mocks

Remove the dead plugin-dispose module and its dedicated test now that V1 plugin migration removed the last production call site. Clean the remaining bootstrap test mocks so src no longer references createPluginDispose.

Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent)

Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
This commit is contained in:
YeonGyu-Kim
2026-04-18 02:23:55 +09:00
parent f56e3934d8
commit e2f5c0d361
6 changed files with 221 additions and 516 deletions
+136
View File
@@ -0,0 +1,136 @@
import { describe, expect, it, mock } from "bun:test"
function createCompactingHandler(hooks: {
compactionContextInjector?: {
capture: (sessionID: string) => Promise<void>
inject: (sessionID: string) => string
}
compactionTodoPreserver?: { capture: (sessionID: string) => Promise<void> }
claudeCodeHooks?: {
"experimental.session.compacting"?: (
input: { sessionID: string },
output: { context: string[] },
) => Promise<void>
}
}) {
return async (
input: { sessionID: string },
output: { context: string[] },
): Promise<void> => {
await hooks.compactionContextInjector?.capture(input.sessionID)
await hooks.compactionTodoPreserver?.capture(input.sessionID)
await hooks.claudeCodeHooks?.["experimental.session.compacting"]?.(
input,
output,
)
if (hooks.compactionContextInjector) {
output.context.push(hooks.compactionContextInjector.inject(input.sessionID))
}
}
}
describe("experimental.session.compacting handler", () => {
//#given all three hooks are present
//#when compacting handler is invoked
//#then all hooks are called in order: capture → PreCompact → contextInjector
it("calls claudeCodeHooks PreCompact alongside other hooks", async () => {
const callOrder: string[] = []
const handler = createCompactingHandler({
compactionContextInjector: {
capture: mock(async () => {
callOrder.push("checkpointCapture")
}),
inject: mock((sessionID: string) => {
callOrder.push("contextInjector")
return `context-for-${sessionID}`
}),
},
compactionTodoPreserver: {
capture: mock(async () => {
callOrder.push("capture")
}),
},
claudeCodeHooks: {
"experimental.session.compacting": mock(async () => {
callOrder.push("preCompact")
}),
},
})
const output = { context: [] as string[] }
await handler({ sessionID: "ses_test" }, output)
expect(callOrder).toEqual([
"checkpointCapture",
"capture",
"preCompact",
"contextInjector",
])
expect(output.context).toEqual(["context-for-ses_test"])
})
//#given claudeCodeHooks injects context during PreCompact
//#when compacting handler is invoked
//#then injected context from PreCompact is preserved in output
it("preserves context injected by PreCompact hooks", async () => {
const handler = createCompactingHandler({
claudeCodeHooks: {
"experimental.session.compacting": async (_input, output) => {
output.context.push("precompact-injected-context")
},
},
})
const output = { context: [] as string[] }
await handler({ sessionID: "ses_test" }, output)
expect(output.context).toContain("precompact-injected-context")
})
//#given claudeCodeHooks is null (no claude code hooks configured)
//#when compacting handler is invoked
//#then handler completes without error and other hooks still run
it("handles null claudeCodeHooks gracefully", async () => {
const captureMock = mock(async () => {})
const checkpointCaptureMock = mock(async () => {})
const contextMock = mock(() => "injected-context")
const handler = createCompactingHandler({
compactionContextInjector: {
capture: checkpointCaptureMock,
inject: contextMock,
},
compactionTodoPreserver: { capture: captureMock },
claudeCodeHooks: undefined,
})
const output = { context: [] as string[] }
await handler({ sessionID: "ses_test" }, output)
expect(checkpointCaptureMock).toHaveBeenCalledWith("ses_test")
expect(captureMock).toHaveBeenCalledWith("ses_test")
expect(contextMock).toHaveBeenCalledWith("ses_test")
expect(output.context).toEqual(["injected-context"])
})
//#given compactionContextInjector is null
//#when compacting handler is invoked
//#then handler does not early-return, PreCompact hooks still execute
it("does not early-return when compactionContextInjector is null", async () => {
const preCompactMock = mock(async () => {})
const handler = createCompactingHandler({
claudeCodeHooks: {
"experimental.session.compacting": preCompactMock,
},
compactionContextInjector: undefined,
})
const output = { context: [] as string[] }
await handler({ sessionID: "ses_test" }, output)
expect(preCompactMock).toHaveBeenCalled()
expect(output.context).toEqual([])
})
})
+85
View File
@@ -0,0 +1,85 @@
import { describe, expect, it } from "bun:test"
describe("look_at tool conditional registration", () => {
describe("isMultimodalLookerEnabled logic", () => {
// given multimodal-looker is in disabled_agents
// when checking if agent is enabled
// then should return false (disabled)
it("returns false when multimodal-looker is disabled (exact case)", () => {
const disabledAgents: string[] = ["multimodal-looker"]
const isEnabled = !disabledAgents.some(
(agent) => agent.toLowerCase() === "multimodal-looker",
)
expect(isEnabled).toBe(false)
})
// given multimodal-looker is in disabled_agents with different case
// when checking if agent is enabled
// then should return false (case-insensitive match)
it("returns false when multimodal-looker is disabled (case-insensitive)", () => {
const disabledAgents: string[] = ["Multimodal-Looker"]
const isEnabled = !disabledAgents.some(
(agent) => agent.toLowerCase() === "multimodal-looker",
)
expect(isEnabled).toBe(false)
})
// given multimodal-looker is NOT in disabled_agents
// when checking if agent is enabled
// then should return true (enabled)
it("returns true when multimodal-looker is not disabled", () => {
const disabledAgents: string[] = ["oracle", "librarian"]
const isEnabled = !disabledAgents.some(
(agent) => agent.toLowerCase() === "multimodal-looker",
)
expect(isEnabled).toBe(true)
})
// given disabled_agents is empty
// when checking if agent is enabled
// then should return true (enabled by default)
it("returns true when disabled_agents is empty", () => {
const disabledAgents: string[] = []
const isEnabled = !disabledAgents.some(
(agent) => agent.toLowerCase() === "multimodal-looker",
)
expect(isEnabled).toBe(true)
})
// given disabled_agents is undefined (simulated as empty array)
// when checking if agent is enabled
// then should return true (enabled by default)
it("returns true when disabled_agents is undefined (fallback to empty)", () => {
const disabledAgents: string[] | undefined = undefined
const list: string[] = disabledAgents ?? []
const isEnabled = !list.some(
(agent) => agent.toLowerCase() === "multimodal-looker",
)
expect(isEnabled).toBe(true)
})
})
describe("conditional tool spread pattern", () => {
// given lookAt is not null (agent enabled)
// when spreading into tool object
// then look_at should be included
it("includes look_at when lookAt is not null", () => {
const lookAt = { execute: () => {} }
const tools = {
...(lookAt ? { look_at: lookAt } : {}),
}
expect(tools).toHaveProperty("look_at")
})
// given lookAt is null (agent disabled)
// when spreading into tool object
// then look_at should NOT be included
it("excludes look_at when lookAt is null", () => {
const lookAt = null
const tools = {
...(lookAt ? { look_at: lookAt } : {}),
}
expect(tools).not.toHaveProperty("look_at")
})
})
})
-4
View File
@@ -29,7 +29,6 @@ const mockCreateHooks = mock(() => ({
compactionTodoPreserver: undefined,
claudeCodeHooks: undefined,
}))
const mockCreatePluginDispose = mock(() => async () => {})
const mockCreatePluginInterface = mock(() => ({}))
const mockCreatePluginPostHog = mock(() => ({
trackActive: () => {
@@ -70,9 +69,6 @@ function installModuleMocks(): void {
mock.module("./create-hooks", () => ({
createHooks: mockCreateHooks,
}))
mock.module("./plugin-dispose", () => ({
createPluginDispose: mockCreatePluginDispose,
}))
mock.module("./plugin-interface", () => ({
createPluginInterface: mockCreatePluginInterface,
}))
-224
View File
@@ -1,223 +1,5 @@
import { afterEach, beforeEach, describe, expect, it, mock } from "bun:test"
describe("experimental.session.compacting handler", () => {
function createCompactingHandler(hooks: {
compactionContextInjector?: {
capture: (sessionID: string) => Promise<void>
inject: (sessionID: string) => string
}
compactionTodoPreserver?: { capture: (sessionID: string) => Promise<void> }
claudeCodeHooks?: {
"experimental.session.compacting"?: (
input: { sessionID: string },
output: { context: string[] },
) => Promise<void>
}
}) {
return async (
_input: { sessionID: string },
output: { context: string[] },
): Promise<void> => {
await hooks.compactionContextInjector?.capture(_input.sessionID)
await hooks.compactionTodoPreserver?.capture(_input.sessionID)
await hooks.claudeCodeHooks?.["experimental.session.compacting"]?.(
_input,
output,
)
if (hooks.compactionContextInjector) {
output.context.push(hooks.compactionContextInjector.inject(_input.sessionID))
}
}
}
//#given all three hooks are present
//#when compacting handler is invoked
//#then all hooks are called in order: capture → PreCompact → contextInjector
it("calls claudeCodeHooks PreCompact alongside other hooks", async () => {
const callOrder: string[] = []
const handler = createCompactingHandler({
compactionContextInjector: {
capture: mock(async () => {
callOrder.push("checkpointCapture")
}),
inject: mock((sessionID: string) => {
callOrder.push("contextInjector")
return `context-for-${sessionID}`
}),
},
compactionTodoPreserver: {
capture: mock(async () => { callOrder.push("capture") }),
},
claudeCodeHooks: {
"experimental.session.compacting": mock(async () => {
callOrder.push("preCompact")
}),
},
})
const output = { context: [] as string[] }
await handler({ sessionID: "ses_test" }, output)
expect(callOrder).toEqual(["checkpointCapture", "capture", "preCompact", "contextInjector"])
expect(output.context).toEqual(["context-for-ses_test"])
})
//#given claudeCodeHooks injects context during PreCompact
//#when compacting handler is invoked
//#then injected context from PreCompact is preserved in output
it("preserves context injected by PreCompact hooks", async () => {
const handler = createCompactingHandler({
claudeCodeHooks: {
"experimental.session.compacting": async (_input, output) => {
output.context.push("precompact-injected-context")
},
},
})
const output = { context: [] as string[] }
await handler({ sessionID: "ses_test" }, output)
expect(output.context).toContain("precompact-injected-context")
})
//#given claudeCodeHooks is null (no claude code hooks configured)
//#when compacting handler is invoked
//#then handler completes without error and other hooks still run
it("handles null claudeCodeHooks gracefully", async () => {
const captureMock = mock(async () => {})
const checkpointCaptureMock = mock(async () => {})
const contextMock = mock(() => "injected-context")
const handler = createCompactingHandler({
compactionContextInjector: {
capture: checkpointCaptureMock,
inject: contextMock,
},
compactionTodoPreserver: { capture: captureMock },
claudeCodeHooks: undefined,
})
const output = { context: [] as string[] }
await handler({ sessionID: "ses_test" }, output)
expect(checkpointCaptureMock).toHaveBeenCalledWith("ses_test")
expect(captureMock).toHaveBeenCalledWith("ses_test")
expect(contextMock).toHaveBeenCalledWith("ses_test")
expect(output.context).toEqual(["injected-context"])
})
//#given compactionContextInjector is null
//#when compacting handler is invoked
//#then handler does not early-return, PreCompact hooks still execute
it("does not early-return when compactionContextInjector is null", async () => {
const preCompactMock = mock(async () => {})
const handler = createCompactingHandler({
claudeCodeHooks: {
"experimental.session.compacting": preCompactMock,
},
compactionContextInjector: undefined,
})
const output = { context: [] as string[] }
await handler({ sessionID: "ses_test" }, output)
expect(preCompactMock).toHaveBeenCalled()
expect(output.context).toEqual([])
})
})
/**
* Tests for conditional tool registration logic in index.ts
*
* The actual plugin initialization is complex to test directly,
* so we test the underlying logic that determines tool registration.
*/
describe("look_at tool conditional registration", () => {
describe("isMultimodalLookerEnabled logic", () => {
// given multimodal-looker is in disabled_agents
// when checking if agent is enabled
// then should return false (disabled)
it("returns false when multimodal-looker is disabled (exact case)", () => {
const disabledAgents: string[] = ["multimodal-looker"]
const isEnabled = !disabledAgents.some(
(agent) => agent.toLowerCase() === "multimodal-looker"
)
expect(isEnabled).toBe(false)
})
// given multimodal-looker is in disabled_agents with different case
// when checking if agent is enabled
// then should return false (case-insensitive match)
it("returns false when multimodal-looker is disabled (case-insensitive)", () => {
const disabledAgents: string[] = ["Multimodal-Looker"]
const isEnabled = !disabledAgents.some(
(agent) => agent.toLowerCase() === "multimodal-looker"
)
expect(isEnabled).toBe(false)
})
// given multimodal-looker is NOT in disabled_agents
// when checking if agent is enabled
// then should return true (enabled)
it("returns true when multimodal-looker is not disabled", () => {
const disabledAgents: string[] = ["oracle", "librarian"]
const isEnabled = !disabledAgents.some(
(agent) => agent.toLowerCase() === "multimodal-looker"
)
expect(isEnabled).toBe(true)
})
// given disabled_agents is empty
// when checking if agent is enabled
// then should return true (enabled by default)
it("returns true when disabled_agents is empty", () => {
const disabledAgents: string[] = []
const isEnabled = !disabledAgents.some(
(agent) => agent.toLowerCase() === "multimodal-looker"
)
expect(isEnabled).toBe(true)
})
// given disabled_agents is undefined (simulated as empty array)
// when checking if agent is enabled
// then should return true (enabled by default)
it("returns true when disabled_agents is undefined (fallback to empty)", () => {
const disabledAgents: string[] | undefined = undefined
const list: string[] = disabledAgents ?? []
const isEnabled = !list.some(
(agent) => agent.toLowerCase() === "multimodal-looker"
)
expect(isEnabled).toBe(true)
})
})
describe("conditional tool spread pattern", () => {
// given lookAt is not null (agent enabled)
// when spreading into tool object
// then look_at should be included
it("includes look_at when lookAt is not null", () => {
const lookAt = { execute: () => {} } // mock tool
const tools = {
...(lookAt ? { look_at: lookAt } : {}),
}
expect(tools).toHaveProperty("look_at")
})
// given lookAt is null (agent disabled)
// when spreading into tool object
// then look_at should NOT be included
it("excludes look_at when lookAt is null", () => {
const lookAt = null
const tools = {
...(lookAt ? { look_at: lookAt } : {}),
}
expect(tools).not.toHaveProperty("look_at")
})
})
})
const mockInitConfigContext = mock(() => {})
const mockDetectExternalSkillPlugin = mock(() => ({ detected: false, pluginName: null }))
const mockGetSkillPluginConflictWarning = mock(() => "")
@@ -252,7 +34,6 @@ const mockCreateHooks = mock(() => ({
compactionTodoPreserver: undefined,
claudeCodeHooks: undefined,
}))
const mockCreatePluginDispose = mock(() => async () => {})
const mockCreatePluginInterface = mock(() => ({}))
const mockInitializeOpenClaw = mock(async () => {})
const mockStartTmuxCheck = mock(() => {})
@@ -297,10 +78,6 @@ function installIndexModuleMocks(): void {
createHooks: mockCreateHooks,
}))
mock.module("./plugin-dispose", () => ({
createPluginDispose: mockCreatePluginDispose,
}))
mock.module("./plugin-interface", () => ({
createPluginInterface: mockCreatePluginInterface,
}))
@@ -350,7 +127,6 @@ describe("OhMyOpenCodePlugin", () => {
mockCreateManagers.mockClear()
mockCreateTools.mockClear()
mockCreateHooks.mockClear()
mockCreatePluginDispose.mockClear()
mockCreatePluginInterface.mockClear()
mockInitializeOpenClaw.mockClear()
mockStartTmuxCheck.mockClear()
-237
View File
@@ -1,237 +0,0 @@
import { describe, expect, spyOn, test } from "bun:test"
import { disposeCreatedHooks } from "./create-hooks"
import { createPluginDispose } from "./plugin-dispose"
describe("createPluginDispose", () => {
test("#given plugin with active managers and hooks #when dispose() is called #then backgroundManager.shutdown() is called", async () => {
// given
const backgroundManager = {
shutdown: async (): Promise<void> => {},
}
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 => {},
})
// when
await dispose()
// then
expect(shutdownSpy).toHaveBeenCalledTimes(1)
})
test("#given plugin with active MCP connections #when dispose() is called #then skillMcpManager.disconnectAll() is called", async () => {
// given
const backgroundManager = {
shutdown: async (): Promise<void> => {},
}
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 => {},
})
// when
await dispose()
// then
expect(disconnectAllSpy).toHaveBeenCalledTimes(1)
})
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 => {},
}
const todoContinuationEnforcer = {
dispose: (): void => {},
}
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")
const dispose = createPluginDispose({
backgroundManager: {
shutdown: async (): Promise<void> => {},
},
skillMcpManager: {
disconnectAll: async (): Promise<void> => {},
},
lspManager,
disposeHooks: (): void => {
disposeCreatedHooks({
claudeCodeHooks,
commentChecker,
runtimeFallback,
todoContinuationEnforcer,
autoSlashCommand,
})
},
})
// when
await dispose()
// then
expect(claudeCodeHooksDisposeSpy).toHaveBeenCalledTimes(1)
expect(commentCheckerDisposeSpy).toHaveBeenCalledTimes(1)
expect(runtimeFallbackDisposeSpy).toHaveBeenCalledTimes(1)
expect(todoContinuationEnforcerDisposeSpy).toHaveBeenCalledTimes(1)
expect(autoSlashCommandDisposeSpy).toHaveBeenCalledTimes(1)
})
test("#given dispose already called #when dispose() called again #then no errors", async () => {
// given
const backgroundManager = {
shutdown: async (): Promise<void> => {},
}
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,
})
// when
await dispose()
await dispose()
// then
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 () => {
// given
const backgroundManager = {
shutdown: async (): Promise<void> => {
throw new Error("shutdown failed")
},
}
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)
},
})
// 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 lspManager = {
stopAll: async (): Promise<void> => {},
}
const disposeHooksCalls: number[] = []
const shutdownSpy = spyOn(backgroundManager, "shutdown")
const dispose = createPluginDispose({
backgroundManager,
skillMcpManager,
lspManager,
disposeHooks: (): void => {
disposeHooksCalls.push(1)
},
})
// when
await dispose()
// then
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)
})
})
-51
View File
@@ -1,51 +0,0 @@
import { log } from "./shared"
export type PluginDispose = () => Promise<void>
export function createPluginDispose(args: {
backgroundManager: {
shutdown: () => void | Promise<void>
}
skillMcpManager: {
disconnectAll: () => Promise<void>
}
lspManager: {
stopAll: () => Promise<void>
}
disposeHooks: () => void
}): PluginDispose {
const { backgroundManager, skillMcpManager, lspManager, disposeHooks } = args
let disposePromise: Promise<void> | null = null
return async (): Promise<void> => {
if (disposePromise) {
await disposePromise
return
}
disposePromise = (async (): Promise<void> => {
try {
await backgroundManager.shutdown()
} catch (error) {
log("[plugin-dispose] backgroundManager.shutdown() error:", error)
}
try {
await skillMcpManager.disconnectAll()
} 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) {
log("[plugin-dispose] disposeHooks() error:", error)
}
})()
await disposePromise
}
}