From 6acca09bd09abd09762148492d0d19bbe1b2934b Mon Sep 17 00:00:00 2001 From: YeonGyu-Kim Date: Sat, 4 Apr 2026 02:35:23 +0900 Subject: [PATCH] fix(ci): resolve mock.module() cross-file leakage in test suite Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus --- .../abort-with-timeout.test.ts | 4 +- .../fallback-retry-handler.test.ts | 9 ++-- src/features/background-agent/manager.test.ts | 1 + .../session-status-classifier.test.ts | 4 +- .../message-builder.test.ts | 4 +- .../executor-resolution.test.ts | 44 ++++++++++++------- src/hooks/legacy-plugin-toast/hook.test.ts | 4 +- src/hooks/model-fallback/hook.test.ts | 7 ++- src/hooks/read-image-resizer/hook.test.ts | 41 +++++++++++------ .../recover-tool-result-missing.test.ts | 1 + src/plugin/event.model-fallback.test.ts | 25 +++++++---- .../fallback.cliproxyapi-matrix.test.ts | 34 +++++++++----- .../log-legacy-plugin-startup-warning.test.ts | 5 ++- src/shared/model-capabilities.test.ts | 8 ++-- src/shared/model-error-classifier.test.ts | 5 ++- 15 files changed, 130 insertions(+), 66 deletions(-) diff --git a/src/features/background-agent/abort-with-timeout.test.ts b/src/features/background-agent/abort-with-timeout.test.ts index c41290f96..cd1aa82d9 100644 --- a/src/features/background-agent/abort-with-timeout.test.ts +++ b/src/features/background-agent/abort-with-timeout.test.ts @@ -6,9 +6,11 @@ mock.module("../../shared", () => ({ log: logMock, })) -import { abortWithTimeout } from "./abort-with-timeout" import type { OpencodeClient } from "./opencode-client" +const { abortWithTimeout } = await import("./abort-with-timeout") +mock.restore() + function createClient(abort: (...args: Array) => Promise): OpencodeClient { return { session: { diff --git a/src/features/background-agent/fallback-retry-handler.test.ts b/src/features/background-agent/fallback-retry-handler.test.ts index 7309bb526..4932093b7 100644 --- a/src/features/background-agent/fallback-retry-handler.test.ts +++ b/src/features/background-agent/fallback-retry-handler.test.ts @@ -17,14 +17,15 @@ mock.module("../../shared/provider-model-id-transform", () => ({ transformModelForProvider: mock((_provider: string, model: string) => model), })) -import { tryFallbackRetry } from "./fallback-retry-handler" -import { shouldRetryError } from "../../shared/model-error-classifier" -import { selectFallbackProvider } from "../../shared/model-error-classifier" -import { readProviderModelsCache } from "../../shared" import type { BackgroundTask } from "./types" import type { ConcurrencyManager } from "./concurrency" import type { OpencodeClient, QueueItem } from "./constants" +const { tryFallbackRetry } = await import("./fallback-retry-handler") +const { shouldRetryError, selectFallbackProvider } = await import("../../shared/model-error-classifier") +const { readProviderModelsCache } = await import("../../shared") +mock.restore() + function createDeferredPromise(): { promise: Promise resolve: () => void diff --git a/src/features/background-agent/manager.test.ts b/src/features/background-agent/manager.test.ts index 35766f368..78b3904fa 100644 --- a/src/features/background-agent/manager.test.ts +++ b/src/features/background-agent/manager.test.ts @@ -20,6 +20,7 @@ import { MIN_IDLE_TIME_MS } from "./constants" import { BackgroundManager } from "./manager" import { ConcurrencyManager } from "./concurrency" import { initTaskToastManager, _resetTaskToastManagerForTesting } from "../task-toast-manager/manager" +mock.restore() const TASK_TTL_MS = 30 * 60 * 1000 diff --git a/src/features/background-agent/session-status-classifier.test.ts b/src/features/background-agent/session-status-classifier.test.ts index 45cc394e2..d3dc36aa7 100644 --- a/src/features/background-agent/session-status-classifier.test.ts +++ b/src/features/background-agent/session-status-classifier.test.ts @@ -1,11 +1,13 @@ import { describe, test, expect, mock, afterAll } from "bun:test" -import { isActiveSessionStatus, isTerminalSessionStatus } from "./session-status-classifier" const mockLog = mock() mock.module("../../shared", () => ({ log: mockLog })) afterAll(() => { mock.restore() }) +const { isActiveSessionStatus, isTerminalSessionStatus } = await import("./session-status-classifier") +mock.restore() + describe("isActiveSessionStatus", () => { describe("#given a known active session status", () => { test('#when type is "busy" #then returns true', () => { diff --git a/src/hooks/anthropic-context-window-limit-recovery/message-builder.test.ts b/src/hooks/anthropic-context-window-limit-recovery/message-builder.test.ts index e107aed39..9d271c3bb 100644 --- a/src/hooks/anthropic-context-window-limit-recovery/message-builder.test.ts +++ b/src/hooks/anthropic-context-window-limit-recovery/message-builder.test.ts @@ -33,7 +33,9 @@ mock.module("../session-recovery/storage/text-part-injector", () => ({ })) async function importFreshMessageBuilder(): Promise { - return import(`./message-builder?test=${Date.now()}-${Math.random()}`) + const module = await import(`./message-builder?test=${Date.now()}-${Math.random()}`) + mock.restore() + return module } afterAll(() => { diff --git a/src/hooks/auto-slash-command/executor-resolution.test.ts b/src/hooks/auto-slash-command/executor-resolution.test.ts index 5fd8df584..45c905467 100644 --- a/src/hooks/auto-slash-command/executor-resolution.test.ts +++ b/src/hooks/auto-slash-command/executor-resolution.test.ts @@ -1,13 +1,19 @@ -import { afterAll, describe, expect, it, mock } from "bun:test" +import { afterEach, describe, expect, it, spyOn } from "bun:test" import type { LoadedSkill } from "../../features/opencode-skill-loader" +import * as shared from "../../shared" +import * as slashcommand from "../../tools/slashcommand" +import { executeSlashCommand } from "./executor" -mock.module("../../shared", () => ({ - resolveCommandsInText: async (content: string) => content, - resolveFileReferencesInText: async (content: string) => content, -})) +let resolveCommandsInTextSpy: { mockRestore: () => void } | undefined +let resolveFileReferencesInTextSpy: { mockRestore: () => void } | undefined +let discoverCommandsSyncSpy: { mockRestore: () => void } | undefined -mock.module("../../tools/slashcommand", () => ({ - discoverCommandsSync: () => [ +function setupExecutorSpies(): void { + resolveCommandsInTextSpy = spyOn(shared, "resolveCommandsInText") + .mockImplementation(async (content: string) => content) + resolveFileReferencesInTextSpy = spyOn(shared, "resolveFileReferencesInText") + .mockImplementation(async (content: string) => content) + discoverCommandsSyncSpy = spyOn(slashcommand, "discoverCommandsSync").mockReturnValue([ { name: "shadowed", metadata: { name: "shadowed", description: "builtin" }, @@ -20,18 +26,19 @@ mock.module("../../tools/slashcommand", () => ({ content: "project template", scope: "project", }, - ], -})) + ]) +} -mock.module("../../features/opencode-skill-loader", () => ({ - discoverAllSkills: async (): Promise => [], -})) +function restoreExecutorSpies(): void { + resolveCommandsInTextSpy?.mockRestore() + resolveFileReferencesInTextSpy?.mockRestore() + discoverCommandsSyncSpy?.mockRestore() + resolveCommandsInTextSpy = undefined + resolveFileReferencesInTextSpy = undefined + discoverCommandsSyncSpy = undefined +} -afterAll(() => { - mock.restore() -}) - -const { executeSlashCommand } = await import("./executor") +afterEach(restoreExecutorSpies) function createRestrictedSkill(): LoadedSkill { return { @@ -49,6 +56,7 @@ function createRestrictedSkill(): LoadedSkill { describe("executeSlashCommand resolution semantics", () => { it("returns project command when project and builtin names collide", async () => { //#given + setupExecutorSpies() const parsed = { command: "shadowed", args: "", @@ -67,6 +75,7 @@ describe("executeSlashCommand resolution semantics", () => { it("blocks slash skill invocation when invoking agent is missing", async () => { //#given + setupExecutorSpies() const parsed = { command: "restricted-skill", args: "", @@ -83,6 +92,7 @@ describe("executeSlashCommand resolution semantics", () => { it("allows slash skill invocation when invoking agent matches restriction", async () => { //#given + setupExecutorSpies() const parsed = { command: "restricted-skill", args: "", diff --git a/src/hooks/legacy-plugin-toast/hook.test.ts b/src/hooks/legacy-plugin-toast/hook.test.ts index c355587fc..e44811be5 100644 --- a/src/hooks/legacy-plugin-toast/hook.test.ts +++ b/src/hooks/legacy-plugin-toast/hook.test.ts @@ -53,7 +53,9 @@ function createEvent(type: string, parentID?: string) { } async function importFreshModule() { - return import(`./hook?t=${Date.now()}-${Math.random()}`) + const module = await import(`./hook?t=${Date.now()}-${Math.random()}`) + mock.restore() + return module } describe("createLegacyPluginToastHook", () => { diff --git a/src/hooks/model-fallback/hook.test.ts b/src/hooks/model-fallback/hook.test.ts index ced094a9e..5d89448c8 100644 --- a/src/hooks/model-fallback/hook.test.ts +++ b/src/hooks/model-fallback/hook.test.ts @@ -57,12 +57,13 @@ afterAll(() => { mock.restore() }) -import { +const { clearPendingModelFallback, createModelFallbackHook, setSessionFallbackChain, setPendingModelFallback, -} from "./hook" +} = await import("./hook") +mock.restore() describe("model fallback hook", () => { beforeEach(() => { @@ -452,3 +453,5 @@ describe("model fallback hook", () => { clearPendingModelFallback(sessionID) }) }) + +export {} diff --git a/src/hooks/read-image-resizer/hook.test.ts b/src/hooks/read-image-resizer/hook.test.ts index 0b55b885d..548b44a43 100644 --- a/src/hooks/read-image-resizer/hook.test.ts +++ b/src/hooks/read-image-resizer/hook.test.ts @@ -1,9 +1,13 @@ /// -import { beforeEach, describe, expect, it, mock } from "bun:test" +import { afterEach, beforeEach, describe, expect, it, mock, spyOn } from "bun:test" import type { PluginInput } from "@opencode-ai/plugin" import type { ImageDimensions, ResizeResult } from "./types" +import * as imageDimensions from "./image-dimensions" +import * as imageResizer from "./image-resizer" +import * as sessionModelState from "../../shared/session-model-state" +import { createReadImageResizerHook } from "./hook" const mockParseImageDimensions = mock((): ImageDimensions | null => null) const mockCalculateTargetDimensions = mock((): ImageDimensions | null => null) @@ -13,20 +17,17 @@ const mockGetSessionModel = mock((_sessionID: string) => ({ modelID: "claude-sonnet-4-6", } as { providerID: string; modelID: string } | undefined)) -mock.module("./image-dimensions", () => ({ - parseImageDimensions: mockParseImageDimensions, -})) +let parseImageDimensionsSpy: { mockRestore: () => void } | undefined +let calculateTargetDimensionsSpy: { mockRestore: () => void } | undefined +let resizeImageSpy: { mockRestore: () => void } | undefined +let getSessionModelSpy: { mockRestore: () => void } | undefined -mock.module("./image-resizer", () => ({ - calculateTargetDimensions: mockCalculateTargetDimensions, - resizeImage: mockResizeImage, -})) - -mock.module("../../shared/session-model-state", () => ({ - getSessionModel: mockGetSessionModel, -})) - -import { createReadImageResizerHook } from "./hook" +function setupHookSpies(): void { + parseImageDimensionsSpy = spyOn(imageDimensions, "parseImageDimensions").mockImplementation(mockParseImageDimensions) + calculateTargetDimensionsSpy = spyOn(imageResizer, "calculateTargetDimensions").mockImplementation(mockCalculateTargetDimensions) + resizeImageSpy = spyOn(imageResizer, "resizeImage").mockImplementation(mockResizeImage) + getSessionModelSpy = spyOn(sessionModelState, "getSessionModel").mockImplementation(mockGetSessionModel) +} type ToolOutput = { title: string @@ -52,6 +53,7 @@ function createInput(tool: string): { tool: string; sessionID: string; callID: s describe("createReadImageResizerHook", () => { beforeEach(() => { + setupHookSpies() mockParseImageDimensions.mockReset() mockCalculateTargetDimensions.mockReset() mockResizeImage.mockReset() @@ -59,6 +61,17 @@ describe("createReadImageResizerHook", () => { mockGetSessionModel.mockReturnValue({ providerID: "anthropic", modelID: "claude-sonnet-4-6" }) }) + afterEach(() => { + parseImageDimensionsSpy?.mockRestore() + calculateTargetDimensionsSpy?.mockRestore() + resizeImageSpy?.mockRestore() + getSessionModelSpy?.mockRestore() + parseImageDimensionsSpy = undefined + calculateTargetDimensionsSpy = undefined + resizeImageSpy = undefined + getSessionModelSpy = undefined + }) + it("skips non-Read tools", async () => { //#given const hook = createReadImageResizerHook(createMockContext()) diff --git a/src/hooks/session-recovery/recover-tool-result-missing.test.ts b/src/hooks/session-recovery/recover-tool-result-missing.test.ts index d8a56f3a3..9a8aaed80 100644 --- a/src/hooks/session-recovery/recover-tool-result-missing.test.ts +++ b/src/hooks/session-recovery/recover-tool-result-missing.test.ts @@ -22,6 +22,7 @@ afterAll(() => { }) const { recoverToolResultMissing } = await import("./recover-tool-result-missing") +mock.restore() function createMockClient(messages: MessageData[] = []) { const promptAsync = mock(() => Promise.resolve({})) diff --git a/src/plugin/event.model-fallback.test.ts b/src/plugin/event.model-fallback.test.ts index a88edceaf..02c012070 100644 --- a/src/plugin/event.model-fallback.test.ts +++ b/src/plugin/event.model-fallback.test.ts @@ -1,19 +1,23 @@ declare const require: (name: string) => any -const { afterEach, afterAll, describe, expect, mock, test } = require("bun:test") - -mock.module("../shared/connected-providers-cache", () => ({ - readConnectedProvidersCache: () => null, - readProviderModelsCache: () => null, -})) - -afterAll(() => { mock.restore() }) +const { afterEach, describe, expect, spyOn, test } = require("bun:test") import { createEventHandler } from "./event" import { createChatMessageHandler } from "./chat-message" import { _resetForTesting, setMainSession } from "../features/claude-code-session-state" import { createModelFallbackHook, clearPendingModelFallback } from "../hooks/model-fallback/hook" +import * as connectedProvidersCache from "../shared/connected-providers-cache" + +let readConnectedProvidersCacheSpy: { mockRestore: () => void } | undefined +let readProviderModelsCacheSpy: { mockRestore: () => void } | undefined + +function setupConnectedProviderCacheMocks(): void { + readConnectedProvidersCacheSpy = spyOn(connectedProvidersCache, "readConnectedProvidersCache").mockReturnValue(null) + readProviderModelsCacheSpy = spyOn(connectedProvidersCache, "readProviderModelsCache").mockReturnValue(null) +} + describe("createEventHandler - model fallback", () => { const createHandler = (args?: { hooks?: any; pluginConfig?: any }) => { + setupConnectedProviderCacheMocks() const abortCalls: string[] = [] const promptCalls: string[] = [] @@ -54,6 +58,10 @@ describe("createEventHandler - model fallback", () => { } afterEach(() => { + readConnectedProvidersCacheSpy?.mockRestore() + readProviderModelsCacheSpy?.mockRestore() + readConnectedProvidersCacheSpy = undefined + readProviderModelsCacheSpy = undefined _resetForTesting() }) @@ -442,6 +450,7 @@ describe("createEventHandler - model fallback", () => { const modelFallback = createModelFallbackHook() + setupConnectedProviderCacheMocks() const eventHandler = createEventHandler({ ctx: { directory: "/tmp", diff --git a/src/plugin/fallback.cliproxyapi-matrix.test.ts b/src/plugin/fallback.cliproxyapi-matrix.test.ts index b04e865c5..8d8e3c6f4 100644 --- a/src/plugin/fallback.cliproxyapi-matrix.test.ts +++ b/src/plugin/fallback.cliproxyapi-matrix.test.ts @@ -1,17 +1,8 @@ declare const require: (name: string) => any -const { afterEach, afterAll, describe, expect, mock, test } = require("bun:test") +const { afterEach, describe, expect, spyOn, test } = require("bun:test") const PROVIDER_ID = "cliproxyapi" -mock.module("../shared/connected-providers-cache", () => ({ - readConnectedProvidersCache: () => [PROVIDER_ID], - readProviderModelsCache: () => ({ - connected: [PROVIDER_ID], - }), -})) - -afterAll(() => { mock.restore() }) - import { createEventHandler } from "./event" import { createChatMessageHandler } from "./chat-message" import { createModelFallbackHook } from "../hooks/model-fallback/hook" @@ -20,6 +11,7 @@ import type { RuntimeFallbackPluginInput } from "../hooks/runtime-fallback/types import { _resetForTesting } from "../features/claude-code-session-state" import { _resetForTesting as _resetModelFallbackForTesting } from "../hooks/model-fallback/hook" import { SessionCategoryRegistry } from "../shared/session-category-registry" +import * as connectedProvidersCache from "../shared/connected-providers-cache" type EventHandlerArgs = Parameters[0] type ChatMessageHandlerArgs = Parameters[0] @@ -92,6 +84,9 @@ type PromptAsyncCall = { parts?: Array<{ type?: string; text?: string }> } +let readConnectedProvidersCacheSpy: { mockRestore: () => void } | undefined +let readProviderModelsCacheSpy: { mockRestore: () => void } | undefined + function createPluginConfig(mode: HarnessMode) { return { agents: { @@ -114,6 +109,7 @@ function createHarness(args: { promptAsyncImpl?: (call: PromptAsyncCall) => Promise sessionTimeoutMs?: number }) { + setupConnectedProviderCacheMocks() const abortCalls: string[] = [] const promptCalls: string[] = [] const promptAsyncCalls: PromptAsyncCall[] = [] @@ -353,6 +349,24 @@ async function triggerAssistantMessageError( })) } +afterEach(() => { + readConnectedProvidersCacheSpy?.mockRestore() + readProviderModelsCacheSpy?.mockRestore() + readConnectedProvidersCacheSpy = undefined + readProviderModelsCacheSpy = undefined +}) + +function setupConnectedProviderCacheMocks(): void { + readConnectedProvidersCacheSpy = spyOn(connectedProvidersCache, "readConnectedProvidersCache").mockReturnValue([ + PROVIDER_ID, + ]) + readProviderModelsCacheSpy = spyOn(connectedProvidersCache, "readProviderModelsCache").mockReturnValue({ + connected: [PROVIDER_ID], + models: {}, + updatedAt: new Date(0).toISOString(), + }) +} + afterEach(() => { _resetForTesting() _resetModelFallbackForTesting() diff --git a/src/shared/log-legacy-plugin-startup-warning.test.ts b/src/shared/log-legacy-plugin-startup-warning.test.ts index d95f48254..41a034975 100644 --- a/src/shared/log-legacy-plugin-startup-warning.test.ts +++ b/src/shared/log-legacy-plugin-startup-warning.test.ts @@ -37,7 +37,10 @@ afterAll(() => { }) async function importFreshStartupWarningModule(): Promise { - return import(`./log-legacy-plugin-startup-warning?test=${Date.now()}-${Math.random()}`) + const module = await import(`./log-legacy-plugin-startup-warning?test=${Date.now()}-${Math.random()}`) + mock.restore() + consoleWarnSpy = spyOn(console, "warn").mockImplementation(() => {}) + return module } describe("logLegacyPluginStartupWarning", () => { diff --git a/src/shared/model-capabilities.test.ts b/src/shared/model-capabilities.test.ts index 80747e333..917b4518a 100644 --- a/src/shared/model-capabilities.test.ts +++ b/src/shared/model-capabilities.test.ts @@ -1,3 +1,4 @@ +import type { ModelCapabilitiesSnapshot } from "./model-capabilities" import { afterAll, describe, expect, test, mock } from "bun:test" // Mock connected-providers-cache to prevent local disk cache from polluting test results. @@ -14,11 +15,8 @@ afterAll(() => { mock.restore() }) -import { - getModelCapabilities, - getBundledModelCapabilitiesSnapshot, - type ModelCapabilitiesSnapshot, -} from "./model-capabilities" +const { getModelCapabilities, getBundledModelCapabilitiesSnapshot } = await import("./model-capabilities") +mock.restore() import { AGENT_MODEL_REQUIREMENTS, CATEGORY_MODEL_REQUIREMENTS } from "./model-requirements" describe("getModelCapabilities", () => { diff --git a/src/shared/model-error-classifier.test.ts b/src/shared/model-error-classifier.test.ts index c199d8145..4cb870803 100644 --- a/src/shared/model-error-classifier.test.ts +++ b/src/shared/model-error-classifier.test.ts @@ -9,7 +9,8 @@ mock.module("./connected-providers-cache", () => ({ afterAll(() => { mock.restore() }) -import { shouldRetryError, selectFallbackProvider } from "./model-error-classifier" +const { shouldRetryError, selectFallbackProvider } = await import("./model-error-classifier") +mock.restore() describe("model-error-classifier", () => { beforeEach(() => { @@ -107,3 +108,5 @@ describe("model-error-classifier", () => { expect(result).toBe(true) }) }) + +export {}