diff --git a/src/index.compacting.test.ts b/src/index.compacting.test.ts new file mode 100644 index 000000000..46434d8cb --- /dev/null +++ b/src/index.compacting.test.ts @@ -0,0 +1,136 @@ +import { describe, expect, it, mock } from "bun:test" + +function createCompactingHandler(hooks: { + compactionContextInjector?: { + capture: (sessionID: string) => Promise + inject: (sessionID: string) => string + } + compactionTodoPreserver?: { capture: (sessionID: string) => Promise } + claudeCodeHooks?: { + "experimental.session.compacting"?: ( + input: { sessionID: string }, + output: { context: string[] }, + ) => Promise + } +}) { + return async ( + input: { sessionID: string }, + output: { context: string[] }, + ): Promise => { + 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([]) + }) +}) diff --git a/src/index.conditional-tools.test.ts b/src/index.conditional-tools.test.ts new file mode 100644 index 000000000..96c955c4b --- /dev/null +++ b/src/index.conditional-tools.test.ts @@ -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") + }) + }) +}) diff --git a/src/index.telemetry.test.ts b/src/index.telemetry.test.ts index 1f552f7eb..7f751f594 100644 --- a/src/index.telemetry.test.ts +++ b/src/index.telemetry.test.ts @@ -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, })) diff --git a/src/index.test.ts b/src/index.test.ts index 00af70bb9..335562cd0 100644 --- a/src/index.test.ts +++ b/src/index.test.ts @@ -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 - inject: (sessionID: string) => string - } - compactionTodoPreserver?: { capture: (sessionID: string) => Promise } - claudeCodeHooks?: { - "experimental.session.compacting"?: ( - input: { sessionID: string }, - output: { context: string[] }, - ) => Promise - } - }) { - return async ( - _input: { sessionID: string }, - output: { context: string[] }, - ): Promise => { - 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() diff --git a/src/plugin-dispose.test.ts b/src/plugin-dispose.test.ts deleted file mode 100644 index d0dd0285b..000000000 --- a/src/plugin-dispose.test.ts +++ /dev/null @@ -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 => {}, - } - const skillMcpManager = { - disconnectAll: async (): Promise => {}, - } - const lspManager = { - stopAll: async (): Promise => {}, - } - 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 => {}, - } - const skillMcpManager = { - disconnectAll: async (): Promise => {}, - } - const lspManager = { - stopAll: async (): Promise => {}, - } - 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 => {}, - } - 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 => {}, - }, - skillMcpManager: { - disconnectAll: async (): Promise => {}, - }, - 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 => {}, - } - const skillMcpManager = { - disconnectAll: async (): Promise => {}, - } - const lspManager = { - stopAll: async (): Promise => {}, - } - 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 => { - throw new Error("shutdown failed") - }, - } - const skillMcpManager = { - disconnectAll: async (): Promise => {}, - } - const lspManager = { - stopAll: async (): Promise => {}, - } - 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 => {}, - } - const skillMcpManager = { - disconnectAll: async (): Promise => { - throw new Error("disconnectAll failed") - }, - } - const lspManager = { - stopAll: async (): Promise => {}, - } - 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 => {}, - } - const stopAllSpy = spyOn(lspManager, "stopAll") - const dispose = createPluginDispose({ - backgroundManager: { - shutdown: async (): Promise => {}, - }, - skillMcpManager: { - disconnectAll: async (): Promise => {}, - }, - lspManager, - disposeHooks: (): void => {}, - }) - - // when - await dispose() - - // then - expect(stopAllSpy).toHaveBeenCalledTimes(1) - }) -}) diff --git a/src/plugin-dispose.ts b/src/plugin-dispose.ts deleted file mode 100644 index 998fd28eb..000000000 --- a/src/plugin-dispose.ts +++ /dev/null @@ -1,51 +0,0 @@ -import { log } from "./shared" - -export type PluginDispose = () => Promise - -export function createPluginDispose(args: { - backgroundManager: { - shutdown: () => void | Promise - } - skillMcpManager: { - disconnectAll: () => Promise - } - lspManager: { - stopAll: () => Promise - } - disposeHooks: () => void -}): PluginDispose { - const { backgroundManager, skillMcpManager, lspManager, disposeHooks } = args - let disposePromise: Promise | null = null - - return async (): Promise => { - if (disposePromise) { - await disposePromise - return - } - - disposePromise = (async (): Promise => { - 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 - } -}