From a02686e729c43a74a1a4ec9dc0548b1c9bad7bf0 Mon Sep 17 00:00:00 2001 From: YeonGyu-Kim Date: Fri, 15 May 2026 18:21:04 +0900 Subject: [PATCH] test(ci): remove suite-order mock coupling --- .../fallback-retry-handler.test.ts | 65 ++---- .../fallback-retry-handler.ts | 48 +++- src/features/background-agent/manager.test.ts | 10 - .../team-runtime/session-cleanup.test.ts | 6 +- ...inal-wave-approval-gate-regression.test.ts | 10 +- ...ol-execute-after-background-launch.test.ts | 64 ++++-- .../tool-execute-after-task-timers.test.ts | 15 +- src/hooks/atlas/tool-execute-after.ts | 8 +- .../zauc-sync-mocks/sync-package-json.test.ts | 54 ++--- src/index.telemetry.test.ts | 99 +++----- src/index.test.ts | 117 +++------- src/index.ts | 215 +++++++++++------- .../git-worktree/collect-git-diff-stats.ts | 12 +- src/shared/migrate-legacy-plugin-entry.ts | 16 +- .../migrate-legacy-plugin-entry.test.ts | 33 +-- src/tools/delegate-task/available-models.ts | 6 +- src/tools/delegate-task/model-selection.ts | 10 +- 17 files changed, 387 insertions(+), 401 deletions(-) diff --git a/src/features/background-agent/fallback-retry-handler.test.ts b/src/features/background-agent/fallback-retry-handler.test.ts index f51ce2059..53b7b7ef8 100644 --- a/src/features/background-agent/fallback-retry-handler.test.ts +++ b/src/features/background-agent/fallback-retry-handler.test.ts @@ -1,11 +1,13 @@ import { afterAll, beforeEach, describe, expect, mock, test } from "bun:test" +import { tryFallbackRetry, type FallbackRetryHandlerDeps } from "./fallback-retry-handler" +import type { FallbackEntry } from "../../shared/model-requirements" const sharedLogMock = mock(() => {}) const readConnectedProvidersCacheMock = mock(() => null) const readProviderModelsCacheMock = mock((): { connected: string[] } | null => null) const shouldRetryErrorMock = mock(() => true) -const getNextFallbackMock = mock((chain: Array<{ model: string }>, attempt: number) => chain[attempt]) -const hasMoreFallbacksMock = mock((chain: Array<{ model: string }>, attempt: number) => attempt < chain.length) +const getNextFallbackMock = mock((chain: FallbackEntry[], attempt: number) => chain[attempt]) +const hasMoreFallbacksMock = mock((chain: FallbackEntry[], attempt: number) => attempt < chain.length) const selectFallbackProviderMock = mock((providers: string[]) => providers[0]) const transformModelForProviderMock = mock((_provider: string, model: string) => model) @@ -13,41 +15,17 @@ import type { BackgroundTask } from "./types" import type { ConcurrencyManager } from "./concurrency" import type { OpencodeClient, QueueItem } from "./constants" -async function importFreshFallbackRetryHandlerModule() { - mock.module("../../shared/logger", () => ({ - log: sharedLogMock, - })) - - mock.module("../../shared/connected-providers-cache", () => ({ - readConnectedProvidersCache: readConnectedProvidersCacheMock, - readProviderModelsCache: readProviderModelsCacheMock, - })) - - mock.module("../../shared/model-error-classifier", () => ({ - shouldRetryError: shouldRetryErrorMock, - getNextFallback: getNextFallbackMock, - hasMoreFallbacks: hasMoreFallbacksMock, - selectFallbackProvider: selectFallbackProviderMock, - })) - - mock.module("../../shared/provider-model-id-transform", () => ({ - transformModelForProvider: transformModelForProviderMock, - })) - - const retryHandlerModule = await import(`./fallback-retry-handler?test=${Date.now()}-${Math.random()}`) - mock.restore() - - return { - tryFallbackRetry: retryHandlerModule.tryFallbackRetry, - shouldRetryError: shouldRetryErrorMock, - selectFallbackProvider: selectFallbackProviderMock, - readProviderModelsCache: readProviderModelsCacheMock, - } +const retryHandlerDeps: Partial = { + log: sharedLogMock, + readConnectedProvidersCache: readConnectedProvidersCacheMock, + readProviderModelsCache: readProviderModelsCacheMock, + shouldRetryError: shouldRetryErrorMock, + getNextFallback: getNextFallbackMock, + hasMoreFallbacks: hasMoreFallbacksMock, + selectFallbackProvider: selectFallbackProviderMock, + transformModelForProvider: transformModelForProviderMock, } -const { tryFallbackRetry, shouldRetryError, selectFallbackProvider, readProviderModelsCache } = - await importFreshFallbackRetryHandlerModule() - function createDeferredPromise(): { promise: Promise resolve: () => void @@ -124,6 +102,7 @@ function createDefaultArgs(taskOverrides: Partial = {}) { idleDeferralTimers, queuesByKey, processKey: processKeyFn, + deps: retryHandlerDeps, } } @@ -133,9 +112,13 @@ describe("tryFallbackRetry", () => { }) beforeEach(() => { - shouldRetryError.mockImplementation(() => true) - selectFallbackProvider.mockImplementation((providers: string[]) => providers[0]) - readProviderModelsCache.mockReturnValue(null) + shouldRetryErrorMock.mockImplementation(() => true) + selectFallbackProviderMock.mockImplementation((providers: string[]) => providers[0]) + readProviderModelsCacheMock.mockReturnValue(null) + readConnectedProvidersCacheMock.mockReturnValue(null) + getNextFallbackMock.mockImplementation((chain: FallbackEntry[], attempt: number) => chain[attempt]) + hasMoreFallbacksMock.mockImplementation((chain: FallbackEntry[], attempt: number) => attempt < chain.length) + transformModelForProviderMock.mockImplementation((_provider: string, model: string) => model) }) describe("#given retryable error with fallback chain", () => { @@ -332,7 +315,7 @@ describe("tryFallbackRetry", () => { describe("#given non-retryable error", () => { test("returns false when shouldRetryError returns false", async () => { - shouldRetryError.mockImplementation(() => false) + shouldRetryErrorMock.mockImplementation(() => false) const args = createDefaultArgs() const result = await tryFallbackRetry(args) @@ -433,8 +416,8 @@ describe("tryFallbackRetry", () => { describe("#given disconnected fallback providers with connected preferred provider", () => { test("keeps fallback entry and selects connected preferred provider", async () => { - readProviderModelsCache.mockReturnValueOnce({ connected: ["provider-a"] }) - selectFallbackProvider.mockImplementationOnce( + readProviderModelsCacheMock.mockReturnValueOnce({ connected: ["provider-a"] }) + selectFallbackProviderMock.mockImplementationOnce( (_providers: string[], preferredProviderID?: string) => preferredProviderID ?? "provider-b", ) diff --git a/src/features/background-agent/fallback-retry-handler.ts b/src/features/background-agent/fallback-retry-handler.ts index d61a839b1..4cb981088 100644 --- a/src/features/background-agent/fallback-retry-handler.ts +++ b/src/features/background-agent/fallback-retry-handler.ts @@ -17,6 +17,28 @@ function canonicalizeModelID(modelID: string): string { return modelID.toLowerCase().replace(/\./g, "-") } +export type FallbackRetryHandlerDeps = { + log: typeof log + readProviderModelsCache: typeof readProviderModelsCache + readConnectedProvidersCache: typeof readConnectedProvidersCache + shouldRetryError: typeof shouldRetryError + getNextFallback: typeof getNextFallback + hasMoreFallbacks: typeof hasMoreFallbacks + selectFallbackProvider: typeof selectFallbackProvider + transformModelForProvider: typeof transformModelForProvider +} + +const defaultFallbackRetryHandlerDeps: FallbackRetryHandlerDeps = { + log, + readProviderModelsCache, + readConnectedProvidersCache, + shouldRetryError, + getNextFallback, + hasMoreFallbacks, + selectFallbackProvider, + transformModelForProvider, +} + export async function tryFallbackRetry(args: { task: BackgroundTask errorInfo: { name?: string; message?: string } @@ -34,20 +56,22 @@ export async function tryFallbackRetry(args: { failedError?: string nextModel: string }) => void + deps?: Partial }): Promise { const { task, errorInfo, source, concurrencyManager, client, idleDeferralTimers, queuesByKey, processKey, onRetrying } = args + const deps = { ...defaultFallbackRetryHandlerDeps, ...args.deps } const fallbackChain = task.fallbackChain const canRetry = - shouldRetryError(errorInfo) && + deps.shouldRetryError(errorInfo) && fallbackChain && fallbackChain.length > 0 && - hasMoreFallbacks(fallbackChain, task.attemptCount ?? 0) + deps.hasMoreFallbacks(fallbackChain, task.attemptCount ?? 0) if (!canRetry) return false const attemptCount = task.attemptCount ?? 0 - const providerModelsCache = readProviderModelsCache() - const connectedProviders = providerModelsCache?.connected ?? readConnectedProvidersCache() + const providerModelsCache = deps.readProviderModelsCache() + const connectedProviders = providerModelsCache?.connected ?? deps.readConnectedProvidersCache() const connectedSet = connectedProviders ? new Set(connectedProviders.map(p => p.toLowerCase())) : null const preferredProvider = task.model?.providerID?.toLowerCase() @@ -63,11 +87,11 @@ export async function tryFallbackRetry(args: { let nextFallback: FallbackEntry | undefined let nextProviderID: string | undefined while (fallbackChain && selectedAttemptCount < fallbackChain.length) { - const candidate = getNextFallback(fallbackChain, selectedAttemptCount) + const candidate = deps.getNextFallback(fallbackChain, selectedAttemptCount) if (!candidate) break selectedAttemptCount++ if (!isReachable(candidate)) { - log("[background-agent] Skipping unreachable fallback:", { + deps.log("[background-agent] Skipping unreachable fallback:", { taskId: task.id, source, model: candidate.model, @@ -75,17 +99,17 @@ export async function tryFallbackRetry(args: { }) continue } - const candidateProviderID = selectFallbackProvider( + const candidateProviderID = deps.selectFallbackProvider( candidate.providers, task.model?.providerID, ) - const candidateModelID = transformModelForProvider(candidateProviderID, candidate.model) + const candidateModelID = deps.transformModelForProvider(candidateProviderID, candidate.model) const isNoOpFallback = !!task.model && candidateProviderID.toLowerCase() === task.model.providerID.toLowerCase() && canonicalizeModelID(candidateModelID) === canonicalizeModelID(task.model.modelID) if (isNoOpFallback) { - log("[background-agent] Skipping no-op fallback:", { + deps.log("[background-agent] Skipping no-op fallback:", { taskId: task.id, source, model: candidate.model, @@ -99,12 +123,12 @@ export async function tryFallbackRetry(args: { } if (!nextFallback) return false - const providerID = nextProviderID ?? selectFallbackProvider( + const providerID = nextProviderID ?? deps.selectFallbackProvider( nextFallback.providers, task.model?.providerID, ) - log("[background-agent] Retryable error, attempting fallback:", { + deps.log("[background-agent] Retryable error, attempting fallback:", { taskId: task.id, source, errorName: errorInfo.name, @@ -127,7 +151,7 @@ export async function tryFallbackRetry(args: { const previousSessionID = task.sessionId const previousModel = task.model - const transformedModelId = transformModelForProvider(providerID, nextFallback.model) + const transformedModelId = deps.transformModelForProvider(providerID, nextFallback.model) const nextModel = { providerID, modelID: transformedModelId, diff --git a/src/features/background-agent/manager.test.ts b/src/features/background-agent/manager.test.ts index c9792e997..823da5435 100644 --- a/src/features/background-agent/manager.test.ts +++ b/src/features/background-agent/manager.test.ts @@ -16,16 +16,6 @@ import { promptAsyncAfterSessionIdle } from "../../shared/prompt-async-gate" import { initTaskToastManager, _resetTaskToastManagerForTesting } from "../task-toast-manager/manager" import { _resetForTesting as resetProcessCleanupState } from "./process-cleanup" -mock.module("../../shared/connected-providers-cache", () => ({ - readConnectedProvidersCache: () => null, - readProviderModelsCache: () => null, - hasConnectedProvidersCache: () => false, - hasProviderModelsCache: () => false, - writeProviderModelsCache: () => {}, - updateConnectedProvidersCache: () => {}, -})) -mock.restore() - const TASK_TTL_MS = 30 * 60 * 1000 type PendingParentWakeForTest = { diff --git a/src/features/team-mode/team-runtime/session-cleanup.test.ts b/src/features/team-mode/team-runtime/session-cleanup.test.ts index ae745f14e..4250ab993 100644 --- a/src/features/team-mode/team-runtime/session-cleanup.test.ts +++ b/src/features/team-mode/team-runtime/session-cleanup.test.ts @@ -1,6 +1,6 @@ /// -import { afterEach, describe, expect, mock, test } from "bun:test" +import { afterEach, beforeEach, describe, expect, mock, test } from "bun:test" import { TeamModeConfigSchema } from "../../../config/schema/team-mode" import type { BackgroundManager } from "../../background-agent/manager" @@ -14,6 +14,10 @@ import { } from "./session-cleanup" describe("session team cleanup", () => { + beforeEach(() => { + clearSessionTeamRunCleanupRegistry() + }) + afterEach(() => { clearSessionTeamRunCleanupRegistry() mock.restore() diff --git a/src/hooks/atlas/final-wave-approval-gate-regression.test.ts b/src/hooks/atlas/final-wave-approval-gate-regression.test.ts index d0ce73c67..761eca94d 100644 --- a/src/hooks/atlas/final-wave-approval-gate-regression.test.ts +++ b/src/hooks/atlas/final-wave-approval-gate-regression.test.ts @@ -149,7 +149,10 @@ describe("Atlas final-wave approval gate regressions", () => { - [ ] All tests pass `) - const hook = createAtlasHook(createMockPluginInput()) + const hook = createAtlasHook(createMockPluginInput(), { + directory: testDirectory, + isCallerOrchestrator: async () => true, + }) const toolOutput = { title: "Sisyphus Task", output: `Tasks [1/1 compliant] | Contamination [CLEAN] | Unaccounted [CLEAN] | VERDICT: APPROVE @@ -186,7 +189,10 @@ session_id: ses_nested_scope_review - [ ] F4. **Scope Fidelity Check** - \`deep\` `) - const hook = createAtlasHook(createMockPluginInput()) + const hook = createAtlasHook(createMockPluginInput(), { + directory: testDirectory, + isCallerOrchestrator: async () => true, + }) const firstThreeOutputs = [1, 2, 3].map((index) => ({ title: `Final review ${index}`, output: `Reviewer ${index} | VERDICT: APPROVE diff --git a/src/hooks/atlas/tool-execute-after-background-launch.test.ts b/src/hooks/atlas/tool-execute-after-background-launch.test.ts index 4bca3e302..36dd5141b 100644 --- a/src/hooks/atlas/tool-execute-after-background-launch.test.ts +++ b/src/hooks/atlas/tool-execute-after-background-launch.test.ts @@ -16,15 +16,7 @@ const collectGitDiffStatsMock = mock(() => ({ insertions: 0, deletions: 0, })) - -mock.module("../../shared/session-utils", () => ({ - isCallerOrchestrator: isCallerOrchestratorMock, -})) - -mock.module("../../shared/git-worktree", () => ({ - collectGitDiffStats: collectGitDiffStatsMock, - formatFileChanges: mock(() => "No file changes"), -})) +const formatFileChangesMock = mock(() => "No file changes") afterAll(() => { mock.restore() }) @@ -50,6 +42,7 @@ describe("createToolExecuteAfterHandler background launch detection", () => { isCallerOrchestratorMock.mockClear() collectGitDiffStatsMock.mockClear() + formatFileChangesMock.mockClear() }) afterEach(() => { @@ -108,6 +101,9 @@ describe("createToolExecuteAfterHandler background launch detection", () => { pendingTaskRefs: new Map(), autoCommit: true, getState: () => ({ promptFailureCount: 0 }), + isCallerOrchestrator: isCallerOrchestratorMock, + collectGitDiffStats: collectGitDiffStatsMock as never, + formatFileChanges: formatFileChangesMock as never, }) } @@ -175,13 +171,21 @@ describe("createToolExecuteAfterHandler background launch detection", () => { serverUrl: new URL("https://example.com"), $: Bun.$, } satisfies PluginInput - const beforeHandler = createToolExecuteBeforeHandler({ ctx, pendingFilePaths, pendingTaskRefs }) + const beforeHandler = createToolExecuteBeforeHandler({ + ctx, + pendingFilePaths, + pendingTaskRefs, + isCallerOrchestrator: isCallerOrchestratorMock, + }) const afterHandler = createToolExecuteAfterHandler({ ctx, pendingFilePaths, pendingTaskRefs, autoCommit: true, getState: () => ({ promptFailureCount: 0 }), + isCallerOrchestrator: isCallerOrchestratorMock, + collectGitDiffStats: collectGitDiffStatsMock as never, + formatFileChanges: formatFileChangesMock as never, }) await beforeHandler( @@ -252,13 +256,21 @@ describe("createToolExecuteAfterHandler background launch detection", () => { serverUrl: new URL("https://example.com"), $: Bun.$, } satisfies PluginInput - const beforeHandler = createToolExecuteBeforeHandler({ ctx, pendingFilePaths, pendingTaskRefs }) + const beforeHandler = createToolExecuteBeforeHandler({ + ctx, + pendingFilePaths, + pendingTaskRefs, + isCallerOrchestrator: isCallerOrchestratorMock, + }) const afterHandler = createToolExecuteAfterHandler({ ctx, pendingFilePaths, pendingTaskRefs, autoCommit: true, getState: () => ({ promptFailureCount: 0 }), + isCallerOrchestrator: isCallerOrchestratorMock, + collectGitDiffStats: collectGitDiffStatsMock as never, + formatFileChanges: formatFileChangesMock as never, }) await beforeHandler( @@ -322,13 +334,21 @@ describe("createToolExecuteAfterHandler background launch detection", () => { serverUrl: new URL("https://example.com"), $: Bun.$, } satisfies PluginInput - const beforeHandler = createToolExecuteBeforeHandler({ ctx, pendingFilePaths, pendingTaskRefs }) + const beforeHandler = createToolExecuteBeforeHandler({ + ctx, + pendingFilePaths, + pendingTaskRefs, + isCallerOrchestrator: isCallerOrchestratorMock, + }) const afterHandler = createToolExecuteAfterHandler({ ctx, pendingFilePaths, pendingTaskRefs, autoCommit: true, getState: () => ({ promptFailureCount: 0 }), + isCallerOrchestrator: isCallerOrchestratorMock, + collectGitDiffStats: collectGitDiffStatsMock as never, + formatFileChanges: formatFileChangesMock as never, }) await beforeHandler( @@ -393,13 +413,21 @@ describe("createToolExecuteAfterHandler background launch detection", () => { serverUrl: new URL("https://example.com"), $: Bun.$, } satisfies PluginInput - const beforeHandler = createToolExecuteBeforeHandler({ ctx, pendingFilePaths, pendingTaskRefs }) + const beforeHandler = createToolExecuteBeforeHandler({ + ctx, + pendingFilePaths, + pendingTaskRefs, + isCallerOrchestrator: isCallerOrchestratorMock, + }) const afterHandler = createToolExecuteAfterHandler({ ctx, pendingFilePaths, pendingTaskRefs, autoCommit: true, getState: () => ({ promptFailureCount: 0 }), + isCallerOrchestrator: isCallerOrchestratorMock, + collectGitDiffStats: collectGitDiffStatsMock as never, + formatFileChanges: formatFileChangesMock as never, }) await beforeHandler( @@ -482,13 +510,21 @@ describe("createToolExecuteAfterHandler background launch detection", () => { serverUrl: new URL("https://example.com"), $: Bun.$, } satisfies PluginInput - const beforeHandler = createToolExecuteBeforeHandler({ ctx, pendingFilePaths, pendingTaskRefs }) + const beforeHandler = createToolExecuteBeforeHandler({ + ctx, + pendingFilePaths, + pendingTaskRefs, + isCallerOrchestrator: isCallerOrchestratorMock, + }) const afterHandler = createToolExecuteAfterHandler({ ctx, pendingFilePaths, pendingTaskRefs, autoCommit: true, getState: () => ({ promptFailureCount: 0 }), + isCallerOrchestrator: isCallerOrchestratorMock, + collectGitDiffStats: collectGitDiffStatsMock as never, + formatFileChanges: formatFileChangesMock as never, }) await beforeHandler( diff --git a/src/hooks/atlas/tool-execute-after-task-timers.test.ts b/src/hooks/atlas/tool-execute-after-task-timers.test.ts index 095d9f4c3..eac5735a8 100644 --- a/src/hooks/atlas/tool-execute-after-task-timers.test.ts +++ b/src/hooks/atlas/tool-execute-after-task-timers.test.ts @@ -15,15 +15,7 @@ const collectGitDiffStatsMock = mock(() => ({ insertions: 0, deletions: 0, })) - -mock.module("../../shared/session-utils", () => ({ - isCallerOrchestrator: isCallerOrchestratorMock, -})) - -mock.module("../../shared/git-worktree", () => ({ - collectGitDiffStats: collectGitDiffStatsMock, - formatFileChanges: mock(() => "No file changes"), -})) +const formatFileChangesMock = mock(() => "No file changes") afterAll(() => { mock.restore() }) @@ -47,6 +39,7 @@ describe("createToolExecuteAfterHandler task timers", () => { } isCallerOrchestratorMock.mockClear() collectGitDiffStatsMock.mockClear() + formatFileChangesMock.mockClear() }) afterEach(() => { @@ -104,6 +97,7 @@ describe("createToolExecuteAfterHandler task timers", () => { pendingFilePaths, pendingTaskRefs, pendingPlanSnapshots, + isCallerOrchestrator: isCallerOrchestratorMock, }), afterHandler: createToolExecuteAfterHandler({ ctx, @@ -112,6 +106,9 @@ describe("createToolExecuteAfterHandler task timers", () => { pendingPlanSnapshots, autoCommit: true, getState: () => ({ promptFailureCount: 0 }), + isCallerOrchestrator: isCallerOrchestratorMock, + collectGitDiffStats: collectGitDiffStatsMock as never, + formatFileChanges: formatFileChangesMock as never, }), } } diff --git a/src/hooks/atlas/tool-execute-after.ts b/src/hooks/atlas/tool-execute-after.ts index 46928cb81..b9fb49a15 100644 --- a/src/hooks/atlas/tool-execute-after.ts +++ b/src/hooks/atlas/tool-execute-after.ts @@ -129,9 +129,13 @@ export function createToolExecuteAfterHandler(input: { autoCommit: boolean getState: (sessionID: string) => SessionState isCallerOrchestrator?: (sessionID: string | undefined) => Promise + collectGitDiffStats?: typeof collectGitDiffStats + formatFileChanges?: typeof formatFileChanges }): (toolInput: ToolExecuteAfterInput, toolOutput: ToolExecuteAfterOutput | undefined) => Promise { const { ctx, pendingFilePaths, pendingTaskRefs, pendingPlanSnapshots, autoCommit, getState } = input const resolveIsCallerOrchestrator = input.isCallerOrchestrator ?? ((sessionID) => isCallerOrchestrator(sessionID, ctx.client)) + const collectGitDiffStatsImpl = input.collectGitDiffStats ?? collectGitDiffStats + const formatFileChangesImpl = input.formatFileChanges ?? formatFileChanges return async (toolInput, toolOutput): Promise => { // Guard against undefined output (e.g., from /review command - see issue #1035) if (!toolOutput) { @@ -212,8 +216,8 @@ export function createToolExecuteAfterHandler(input: { if (toolOutput.output && typeof toolOutput.output === "string") { const worktreePath = boulderState?.worktree_path?.trim() const verificationDirectory = worktreePath ? worktreePath : ctx.directory - const gitStats = collectGitDiffStats(verificationDirectory) - const fileChanges = formatFileChanges(gitStats) + const gitStats = collectGitDiffStatsImpl(verificationDirectory) + const fileChanges = formatFileChangesImpl(gitStats) const extractedSessionId = metadataSessionId ?? extractSessionIdFromOutput(toolOutput.output) if (boulderState) { diff --git a/src/hooks/zauc-sync-mocks/sync-package-json.test.ts b/src/hooks/zauc-sync-mocks/sync-package-json.test.ts index acb2abefb..59ef4957a 100644 --- a/src/hooks/zauc-sync-mocks/sync-package-json.test.ts +++ b/src/hooks/zauc-sync-mocks/sync-package-json.test.ts @@ -1,4 +1,4 @@ -import { afterAll, afterEach, beforeEach, describe, expect, it, mock } from "bun:test" +import { afterAll, afterEach, beforeEach, describe, expect, it, mock, spyOn } from "bun:test" import { existsSync, mkdirSync, readFileSync, rmSync, writeFileSync } from "node:fs" import { join } from "node:path" import type { PluginEntryInfo } from "../auto-update-checker/checker/plugin-entry" @@ -13,10 +13,6 @@ const ORIGINAL_CACHE_PACKAGE_JSON = existsSync(CACHE_PACKAGE_JSON_PATH) let importCounter = 0 async function importFreshSyncPackageJsonModule(): Promise { - mock.module("../../shared/logger", () => ({ - log: () => {}, - })) - return import(`../auto-update-checker/checker/sync-package-json?test=${importCounter++}`) } @@ -253,16 +249,9 @@ describe("syncCachePackageJsonToIntent", () => { ) const fs = await import("node:fs") - const originalWriteFileSync = fs.writeFileSync - const originalRenameSync = fs.renameSync - - mock.module("node:fs", () => ({ - ...fs, - writeFileSync: mock(() => { - throw new Error("EACCES: permission denied") - }), - renameSync: fs.renameSync, - })) + const writeFileSyncSpy = spyOn(fs, "writeFileSync").mockImplementation(() => { + throw new Error("EACCES: permission denied") + }) try { const { syncCachePackageJsonToIntent } = await importFreshSyncPackageJsonModule() @@ -279,11 +268,7 @@ describe("syncCachePackageJsonToIntent", () => { expect(result.synced).toBe(false) expect(result.error).toBe("write_error") } finally { - mock.module("node:fs", () => ({ - ...fs, - writeFileSync: originalWriteFileSync, - renameSync: originalRenameSync, - })) + writeFileSyncSpy.mockRestore() } }) }) @@ -299,20 +284,20 @@ describe("syncCachePackageJsonToIntent", () => { const fs = await import("node:fs") const originalWriteFileSync = fs.writeFileSync - const originalRenameSync = fs.renameSync let tempFilePath: string | null = null - mock.module("node:fs", () => ({ - ...fs, - writeFileSync: mock((path: string, data: string) => { - tempFilePath = path - return originalWriteFileSync(path, data) - }), - renameSync: mock(() => { - throw new Error("EXDEV: cross-device link not permitted") - }), - })) + const writeFileSyncSpy = spyOn(fs, "writeFileSync").mockImplementation(( + (file: Parameters[0], + data: Parameters[1], + options?: Parameters[2]) => { + tempFilePath = String(file) + return originalWriteFileSync(file, data, options) + } + ) as typeof fs.writeFileSync) + const renameSyncSpy = spyOn(fs, "renameSync").mockImplementation(() => { + throw new Error("EXDEV: cross-device link not permitted") + }) try { const { syncCachePackageJsonToIntent } = await importFreshSyncPackageJsonModule() @@ -331,11 +316,8 @@ describe("syncCachePackageJsonToIntent", () => { expect(tempFilePath).not.toBeNull() expect(existsSync(tempFilePath!)).toBe(false) } finally { - mock.module("node:fs", () => ({ - ...fs, - writeFileSync: originalWriteFileSync, - renameSync: originalRenameSync, - })) + writeFileSyncSpy.mockRestore() + renameSyncSpy.mockRestore() } }) }) diff --git a/src/index.telemetry.test.ts b/src/index.telemetry.test.ts index 83e0f748a..a10f0f028 100644 --- a/src/index.telemetry.test.ts +++ b/src/index.telemetry.test.ts @@ -1,4 +1,5 @@ -import { afterEach, beforeEach, describe, expect, it, mock } from "bun:test" +import { beforeEach, describe, expect, it, mock } from "bun:test" +import { createPluginModule } from "./index" const mockInitConfigContext = mock(() => {}) const mockInjectServerAuthIntoClient = mock(() => {}) @@ -30,85 +31,55 @@ const mockCreateHooks = mock(() => ({ claudeCodeHooks: undefined, })) const mockCreatePluginInterface = mock(() => ({})) -function installModuleMocks(): void { - mock.module("./cli/config-manager/config-context", () => ({ +const mockLog = mock(() => {}) + +function createTestPluginModule(): ReturnType { + return createPluginModule({ initConfigContext: mockInitConfigContext, - })) - mock.module("./shared/external-plugin-detector", () => ({ + injectServerAuthIntoClient: mockInjectServerAuthIntoClient, + logLegacyPluginStartupWarning: mockLogLegacyPluginStartupWarning, + loadPluginConfig: mockLoadPluginConfig as never, + isTmuxIntegrationEnabled: mockIsTmuxIntegrationEnabled as never, + createRuntimeTmuxConfig: mockCreateRuntimeTmuxConfig as never, + createManagers: mockCreateManagers as never, + createTools: mockCreateTools as never, + createHooks: mockCreateHooks as never, + createPluginInterface: mockCreatePluginInterface as never, + log: mockLog, detectExternalSkillPlugin: mock(() => ({ detected: false, pluginName: null })), getSkillPluginConflictWarning: mock(() => ""), - })) - mock.module("./shared/logger", () => ({ - log: mock(() => {}), - })) - mock.module("./shared/log-legacy-plugin-startup-warning", () => ({ - logLegacyPluginStartupWarning: mockLogLegacyPluginStartupWarning, - })) - mock.module("./shared/opencode-server-auth", () => ({ - injectServerAuthIntoClient: mockInjectServerAuthIntoClient, - })) - mock.module("./plugin-config", () => ({ - loadPluginConfig: mockLoadPluginConfig, - })) - mock.module("./create-runtime-tmux-config", () => ({ - createRuntimeTmuxConfig: mockCreateRuntimeTmuxConfig, - isTmuxIntegrationEnabled: mockIsTmuxIntegrationEnabled, - })) - mock.module("./create-managers", () => ({ - createManagers: mockCreateManagers, - })) - mock.module("./create-tools", () => ({ - createTools: mockCreateTools, - })) - mock.module("./create-hooks", () => ({ - createHooks: mockCreateHooks, - })) - mock.module("./plugin-interface", () => ({ - createPluginInterface: mockCreatePluginInterface, - })) - mock.module("./plugin-state", () => ({ - createModelCacheState: mock(() => ({})), - })) - mock.module("./shared/first-message-variant", () => ({ + initializeOpenClaw: mock(async () => {}), + startTmuxCheck: mock(() => {}), + createModelCacheState: mock(() => ({})) as never, createFirstMessageVariantGate: mock(() => ({ shouldOverride: () => false, markApplied: () => {}, markSessionCreated: () => {}, clear: () => {}, - })), - })) - mock.module("./openclaw", () => ({ - initializeOpenClaw: mock(async () => {}), - })) - mock.module("./tools/interactive-bash", () => ({ - interactive_bash: {}, - startBackgroundCheck: mock(() => {}), - })) - mock.module("./tools/lsp/client", () => ({ - lspManager: { - getClient: mock(async () => ({ - diagnostics: mock(async () => ({ items: [] })), - })), - stopAll: mock(async () => {}), - releaseClient: mock(() => {}), - cleanupTempDirectoryClients: mock(async () => {}), - }, - })) + })) as never, + installAgentSortShim: mock(() => {}), + setAgentSortOrder: mock(() => {}), + }) } describe("oh-my-openagent telemetry isolation", () => { beforeEach(() => { - mock.restore() - installModuleMocks() - }) - - afterEach(() => { - mock.restore() + mockInitConfigContext.mockClear() + mockInjectServerAuthIntoClient.mockClear() + mockLogLegacyPluginStartupWarning.mockClear() + mockLoadPluginConfig.mockClear() + mockIsTmuxIntegrationEnabled.mockClear() + mockCreateRuntimeTmuxConfig.mockClear() + mockCreateManagers.mockClear() + mockCreateTools.mockClear() + mockCreateHooks.mockClear() + mockCreatePluginInterface.mockClear() + mockLog.mockClear() }) it("does not crash plugin load when telemetry throws", async () => { // given - const { default: plugin } = await import(`./index?telemetry=${Date.now()}-${Math.random()}`) + const plugin = createTestPluginModule() // when const result = await plugin.server({ diff --git a/src/index.test.ts b/src/index.test.ts index b1f5e2a73..7b5f4512e 100644 --- a/src/index.test.ts +++ b/src/index.test.ts @@ -1,4 +1,5 @@ -import { afterEach, beforeEach, describe, expect, it, mock } from "bun:test" +import { beforeEach, describe, expect, it, mock } from "bun:test" +import { createPluginModule } from "./index" const mockInitConfigContext = mock(() => {}) const mockDetectExternalSkillPlugin = mock(() => ({ detected: false, pluginName: null })) @@ -9,7 +10,6 @@ const mockLoadPluginConfig = mock(() => ({})) const mockIsTmuxIntegrationEnabled = mock( (pluginConfig: { tmux?: { enabled?: boolean } | undefined }) => pluginConfig.tmux?.enabled ?? false, ) -const mockIsInteractiveBashEnabled = mock(() => false) const mockCreateRuntimeTmuxConfig = mock(() => ({ enabled: false, layout: "tiled" as const, @@ -39,95 +39,43 @@ const mockInitializeOpenClaw = mock(async () => {}) const mockStartTmuxCheck = mock(() => {}) const mockInstallAgentSortShim = mock(() => {}) const mockSetAgentSortOrder = mock(() => {}) +const mockLog = mock(() => {}) +const mockCreateModelCacheState = mock(() => ({})) +const mockCreateFirstMessageVariantGate = mock(() => ({ + shouldOverride: () => false, + markApplied: () => {}, + markSessionCreated: () => {}, + clear: () => {}, +})) -let pluginModule: (typeof import("./index"))["default"] +let pluginModule: ReturnType -function installIndexModuleMocks(): void { - mock.module("./cli/config-manager/config-context", () => ({ +function createTestPluginModule(): ReturnType { + return createPluginModule({ initConfigContext: mockInitConfigContext, - })) - - mock.module("./shared/external-plugin-detector", () => ({ detectExternalSkillPlugin: mockDetectExternalSkillPlugin, getSkillPluginConflictWarning: mockGetSkillPluginConflictWarning, - })) - - mock.module("./shared/logger", () => ({ - log: mock(() => {}), - })) - - mock.module("./shared/log-legacy-plugin-startup-warning", () => ({ - logLegacyPluginStartupWarning: mockLogLegacyPluginStartupWarning, - })) - - mock.module("./shared/opencode-server-auth", () => ({ injectServerAuthIntoClient: mockInjectServerAuthIntoClient, - })) - - mock.module("./plugin-config", () => ({ - loadPluginConfig: mockLoadPluginConfig, - })) - - mock.module("./create-runtime-tmux-config", () => ({ - createRuntimeTmuxConfig: mockCreateRuntimeTmuxConfig, - isTmuxIntegrationEnabled: mockIsTmuxIntegrationEnabled, - isInteractiveBashEnabled: mockIsInteractiveBashEnabled, - })) - - mock.module("./create-managers", () => ({ - createManagers: mockCreateManagers, - })) - - mock.module("./create-tools", () => ({ - createTools: mockCreateTools, - })) - - mock.module("./create-hooks", () => ({ - createHooks: mockCreateHooks, - })) - - mock.module("./plugin-interface", () => ({ - createPluginInterface: mockCreatePluginInterface, - })) - - mock.module("./plugin-state", () => ({ - createModelCacheState: mock(() => ({})), - })) - - mock.module("./shared/first-message-variant", () => ({ - createFirstMessageVariantGate: mock(() => ({ - shouldOverride: () => false, - markApplied: () => {}, - markSessionCreated: () => {}, - clear: () => {}, - })), - })) - - mock.module("./shared/agent-sort-shim", () => ({ + logLegacyPluginStartupWarning: mockLogLegacyPluginStartupWarning, + loadPluginConfig: mockLoadPluginConfig as never, + isTmuxIntegrationEnabled: mockIsTmuxIntegrationEnabled as never, + createRuntimeTmuxConfig: mockCreateRuntimeTmuxConfig as never, + createManagers: mockCreateManagers as never, + createTools: mockCreateTools as never, + createHooks: mockCreateHooks as never, + createPluginInterface: mockCreatePluginInterface as never, + initializeOpenClaw: mockInitializeOpenClaw as never, + startTmuxCheck: mockStartTmuxCheck, installAgentSortShim: mockInstallAgentSortShim, setAgentSortOrder: mockSetAgentSortOrder, - })) - - mock.module("./openclaw", () => ({ - initializeOpenClaw: mockInitializeOpenClaw, - })) - - mock.module("./tools/interactive-bash", () => ({ - interactive_bash: {}, - startBackgroundCheck: mockStartTmuxCheck, - })) - -} - -async function importFreshIndexModule(): Promise { - return import(`./index?test=${Date.now()}-${Math.random()}`) + log: mockLog, + createModelCacheState: mockCreateModelCacheState as never, + createFirstMessageVariantGate: mockCreateFirstMessageVariantGate as never, + }) } describe("oh-my-openagent plugin module", () => { - beforeEach(async () => { - mock.restore() - installIndexModuleMocks() - ;({ default: pluginModule } = await importFreshIndexModule()) + beforeEach(() => { mockInitConfigContext.mockClear() mockDetectExternalSkillPlugin.mockClear() mockGetSkillPluginConflictWarning.mockClear() @@ -135,7 +83,6 @@ describe("oh-my-openagent plugin module", () => { mockLogLegacyPluginStartupWarning.mockClear() mockLoadPluginConfig.mockClear() mockIsTmuxIntegrationEnabled.mockClear() - mockIsInteractiveBashEnabled.mockClear() mockCreateRuntimeTmuxConfig.mockClear() mockCreateManagers.mockClear() mockCreateTools.mockClear() @@ -145,10 +92,10 @@ describe("oh-my-openagent plugin module", () => { mockStartTmuxCheck.mockClear() mockInstallAgentSortShim.mockClear() mockSetAgentSortOrder.mockClear() - }) - - afterEach(() => { - mock.restore() + mockLog.mockClear() + mockCreateModelCacheState.mockClear() + mockCreateFirstMessageVariantGate.mockClear() + pluginModule = createTestPluginModule() }) it("starts openclaw during plugin bootstrap when openclaw config exists", async () => { diff --git a/src/index.ts b/src/index.ts index 7d229a134..52478b4ef 100644 --- a/src/index.ts +++ b/src/index.ts @@ -29,108 +29,157 @@ type HooksWithCompactionAutocontinue = Hooks & { "experimental.compaction.autocontinue"?: CompactionAutocontinueHook } -const serverPlugin: Plugin = async (input, _options): Promise => { - installAgentSortShim() - initConfigContext("opencode", null) - log("[oh-my-openagent] ENTRY - plugin loading", { - directory: input.directory, - }) - logLegacyPluginStartupWarning() +type PluginModuleDeps = { + initConfigContext: typeof initConfigContext + installAgentSortShim: typeof installAgentSortShim + setAgentSortOrder: typeof setAgentSortOrder + log: typeof log + logLegacyPluginStartupWarning: typeof logLegacyPluginStartupWarning + detectExternalSkillPlugin: typeof detectExternalSkillPlugin + getSkillPluginConflictWarning: typeof getSkillPluginConflictWarning + injectServerAuthIntoClient: typeof injectServerAuthIntoClient + loadPluginConfig: typeof loadPluginConfig + initializeOpenClaw: typeof initializeOpenClaw + isTmuxIntegrationEnabled: typeof isTmuxIntegrationEnabled + startTmuxCheck: typeof startTmuxCheck + createFirstMessageVariantGate: typeof createFirstMessageVariantGate + createRuntimeTmuxConfig: typeof createRuntimeTmuxConfig + createModelCacheState: typeof createModelCacheState + createManagers: typeof createManagers + createTools: typeof createTools + createHooks: typeof createHooks + createPluginInterface: typeof createPluginInterface +} - const skillPluginCheck = detectExternalSkillPlugin(input.directory) - if (skillPluginCheck.detected && skillPluginCheck.pluginName) { - console.warn(getSkillPluginConflictWarning(skillPluginCheck.pluginName)) - } +const defaultPluginModuleDeps: PluginModuleDeps = { + initConfigContext, + installAgentSortShim, + setAgentSortOrder, + log, + logLegacyPluginStartupWarning, + detectExternalSkillPlugin, + getSkillPluginConflictWarning, + injectServerAuthIntoClient, + loadPluginConfig, + initializeOpenClaw, + isTmuxIntegrationEnabled, + startTmuxCheck, + createFirstMessageVariantGate, + createRuntimeTmuxConfig, + createModelCacheState, + createManagers, + createTools, + createHooks, + createPluginInterface, +} - injectServerAuthIntoClient(input.client) +export function createPluginModule(overrides: Partial = {}): PluginModule { + const deps = { ...defaultPluginModuleDeps, ...overrides } + const serverPlugin: Plugin = async (input, _options): Promise => { + deps.installAgentSortShim() + deps.initConfigContext("opencode", null) + deps.log("[oh-my-openagent] ENTRY - plugin loading", { + directory: input.directory, + }) + deps.logLegacyPluginStartupWarning() - const pluginConfig = loadPluginConfig(input.directory, input) - setAgentSortOrder(pluginConfig.agent_order) - - if (pluginConfig.openclaw) { - await initializeOpenClaw(pluginConfig.openclaw) - } - if (pluginConfig.team_mode?.enabled) { - const teamModeConfig = pluginConfig.team_mode - try { - const { ensureBaseDirs, resolveBaseDir } = await import("./features/team-mode/team-registry/paths") - const { checkTeamModeDependencies } = await import("./features/team-mode/deps") - await checkTeamModeDependencies(teamModeConfig) - await ensureBaseDirs(resolveBaseDir(teamModeConfig)) - if (pluginConfig.disabled_skills?.includes("team-mode")) { - console.warn( - "[team-mode] enabled=true but team-mode skill is disabled; skill docs hidden but tools still registered (D-29)", - ) - } - } catch (err) { - console.warn("[team-mode] init failed:", err) + const skillPluginCheck = deps.detectExternalSkillPlugin(input.directory) + if (skillPluginCheck.detected && skillPluginCheck.pluginName) { + console.warn(deps.getSkillPluginConflictWarning(skillPluginCheck.pluginName)) } - } - const tmuxIntegrationEnabled = isTmuxIntegrationEnabled(pluginConfig) - if (tmuxIntegrationEnabled) { - startTmuxCheck() - } - const disabledHooks = new Set(pluginConfig.disabled_hooks ?? []) - const isHookEnabled = (hookName: HookName): boolean => !disabledHooks.has(hookName) - const safeHookEnabled = pluginConfig.experimental?.safe_hook_creation ?? true + deps.injectServerAuthIntoClient(input.client) - const firstMessageVariantGate = createFirstMessageVariantGate() + const pluginConfig = deps.loadPluginConfig(input.directory, input) + deps.setAgentSortOrder(pluginConfig.agent_order) - const tmuxConfig = createRuntimeTmuxConfig(pluginConfig) + if (pluginConfig.openclaw) { + await deps.initializeOpenClaw(pluginConfig.openclaw) + } + if (pluginConfig.team_mode?.enabled) { + const teamModeConfig = pluginConfig.team_mode + try { + const { ensureBaseDirs, resolveBaseDir } = await import("./features/team-mode/team-registry/paths") + const { checkTeamModeDependencies } = await import("./features/team-mode/deps") + await checkTeamModeDependencies(teamModeConfig) + await ensureBaseDirs(resolveBaseDir(teamModeConfig)) + if (pluginConfig.disabled_skills?.includes("team-mode")) { + console.warn( + "[team-mode] enabled=true but team-mode skill is disabled; skill docs hidden but tools still registered (D-29)", + ) + } + } catch (err) { + console.warn("[team-mode] init failed:", err) + } + } + const tmuxIntegrationEnabled = deps.isTmuxIntegrationEnabled(pluginConfig) + if (tmuxIntegrationEnabled) { + deps.startTmuxCheck() + } + const disabledHooks = new Set(pluginConfig.disabled_hooks ?? []) - const modelCacheState = createModelCacheState() + const isHookEnabled = (hookName: HookName): boolean => !disabledHooks.has(hookName) + const safeHookEnabled = pluginConfig.experimental?.safe_hook_creation ?? true - const managers = createManagers({ - ctx: input, - pluginConfig, - tmuxConfig, - modelCacheState, - backgroundNotificationHookEnabled: isHookEnabled("background-notification"), - }) + const firstMessageVariantGate = deps.createFirstMessageVariantGate() - const toolsResult = await createTools({ - ctx: input, - pluginConfig, - managers, - }) + const tmuxConfig = deps.createRuntimeTmuxConfig(pluginConfig) - const hooks = createHooks({ - ctx: input, - pluginConfig, - modelCacheState, - backgroundManager: managers.backgroundManager, - modelFallbackControllerAccessor: managers.modelFallbackControllerAccessor, - isHookEnabled, - safeHookEnabled, - mergedSkills: toolsResult.mergedSkills, - availableSkills: toolsResult.availableSkills, - }) + const modelCacheState = deps.createModelCacheState() - const pluginInterface = createPluginInterface({ - ctx: input, - pluginConfig, - firstMessageVariantGate, - managers, - hooks, - tools: toolsResult.filteredTools, - }) + const managers = deps.createManagers({ + ctx: input, + pluginConfig, + tmuxConfig, + modelCacheState, + backgroundNotificationHookEnabled: isHookEnabled("background-notification"), + }) - const pluginHooks: HooksWithCompactionAutocontinue = { - ...pluginInterface, + const toolsResult = await deps.createTools({ + ctx: input, + pluginConfig, + managers, + }) - "experimental.session.compacting": createSessionCompactingHandler(hooks), + const hooks = deps.createHooks({ + ctx: input, + pluginConfig, + modelCacheState, + backgroundManager: managers.backgroundManager, + modelFallbackControllerAccessor: managers.modelFallbackControllerAccessor, + isHookEnabled, + safeHookEnabled, + mergedSkills: toolsResult.mergedSkills, + availableSkills: toolsResult.availableSkills, + }) - "experimental.compaction.autocontinue": createCompactionAutocontinueHandler(hooks), + const pluginInterface = deps.createPluginInterface({ + ctx: input, + pluginConfig, + firstMessageVariantGate, + managers, + hooks, + tools: toolsResult.filteredTools, + }) + + const pluginHooks: HooksWithCompactionAutocontinue = { + ...pluginInterface, + + "experimental.session.compacting": createSessionCompactingHandler(hooks), + + "experimental.compaction.autocontinue": createCompactionAutocontinueHandler(hooks), + } + + return pluginHooks } - return pluginHooks + return { + id: "oh-my-openagent", + server: serverPlugin, + } } -const pluginModule: PluginModule = { - id: "oh-my-openagent", - server: serverPlugin, -} +const pluginModule: PluginModule = createPluginModule() export default pluginModule diff --git a/src/shared/git-worktree/collect-git-diff-stats.ts b/src/shared/git-worktree/collect-git-diff-stats.ts index f4cb34339..402e3cab5 100644 --- a/src/shared/git-worktree/collect-git-diff-stats.ts +++ b/src/shared/git-worktree/collect-git-diff-stats.ts @@ -1,5 +1,5 @@ -import { execFileSync } from "node:child_process" -import { readFileSync } from "node:fs" +import * as childProcess from "node:child_process" +import * as fs from "node:fs" import { join } from "node:path" import { parseGitStatusPorcelain } from "./parse-status-porcelain" import { parseGitDiffNumstat } from "./parse-diff-numstat" @@ -7,21 +7,21 @@ import type { GitFileStat } from "./types" export function collectGitDiffStats(directory: string): GitFileStat[] { try { - const diffOutput = execFileSync("git", ["diff", "--numstat", "HEAD"], { + const diffOutput = childProcess.execFileSync("git", ["diff", "--numstat", "HEAD"], { cwd: directory, encoding: "utf-8", timeout: 5000, stdio: ["pipe", "pipe", "pipe"], }).trimEnd() - const statusOutput = execFileSync("git", ["status", "--porcelain"], { + const statusOutput = childProcess.execFileSync("git", ["status", "--porcelain"], { cwd: directory, encoding: "utf-8", timeout: 5000, stdio: ["pipe", "pipe", "pipe"], }).trimEnd() - const untrackedOutput = execFileSync("git", ["ls-files", "--others", "--exclude-standard"], { + const untrackedOutput = childProcess.execFileSync("git", ["ls-files", "--others", "--exclude-standard"], { cwd: directory, encoding: "utf-8", timeout: 5000, @@ -34,7 +34,7 @@ export function collectGitDiffStats(directory: string): GitFileStat[] { .filter(Boolean) .map((filePath) => { try { - const content = readFileSync(join(directory, filePath), "utf-8") + const content = fs.readFileSync(join(directory, filePath), "utf-8") const lineCount = content.split("\n").length - (content.endsWith("\n") ? 1 : 0) return `${lineCount}\t0\t${filePath}` } catch { diff --git a/src/shared/migrate-legacy-plugin-entry.ts b/src/shared/migrate-legacy-plugin-entry.ts index 0eeb1949d..ef3ed87dd 100644 --- a/src/shared/migrate-legacy-plugin-entry.ts +++ b/src/shared/migrate-legacy-plugin-entry.ts @@ -1,4 +1,4 @@ -import { closeSync, existsSync, fsyncSync, openSync, readFileSync, renameSync, writeFileSync } from "node:fs" +import * as fs from "node:fs" import { applyEdits, modify } from "jsonc-parser" @@ -36,10 +36,10 @@ function updateJsoncPluginArray(content: string, pluginEntries: string[]): strin } export function migrateLegacyPluginEntry(configPath: string): boolean { - if (!existsSync(configPath)) return false + if (!fs.existsSync(configPath)) return false try { - const content = readFileSync(configPath, "utf-8") + const content = fs.readFileSync(configPath, "utf-8") if (!content.includes(LEGACY_PLUGIN_NAME)) return false const parseResult = parseJsoncSafe(content) @@ -53,15 +53,15 @@ export function migrateLegacyPluginEntry(configPath: string): boolean { if (!updated || updated === content) return false const tempPath = `${configPath}.tmp` - writeFileSync(tempPath, updated, "utf-8") - const tempFileDescriptor = openSync(tempPath, "r+") + fs.writeFileSync(tempPath, updated, "utf-8") + const tempFileDescriptor = fs.openSync(tempPath, "r+") try { - fsyncSync(tempFileDescriptor) + fs.fsyncSync(tempFileDescriptor) } finally { - closeSync(tempFileDescriptor) + fs.closeSync(tempFileDescriptor) } - renameSync(tempPath, configPath) + fs.renameSync(tempPath, configPath) log("[migrateLegacyPluginEntry] Auto-migrated opencode.json plugin entry", { configPath, from: LEGACY_PLUGIN_NAME, diff --git a/src/shared/zauc-mocks-migrate-legacy-plugin/migrate-legacy-plugin-entry.test.ts b/src/shared/zauc-mocks-migrate-legacy-plugin/migrate-legacy-plugin-entry.test.ts index 6cd489062..fb9617e2a 100644 --- a/src/shared/zauc-mocks-migrate-legacy-plugin/migrate-legacy-plugin-entry.test.ts +++ b/src/shared/zauc-mocks-migrate-legacy-plugin/migrate-legacy-plugin-entry.test.ts @@ -1,6 +1,6 @@ /// -import { afterAll, afterEach, beforeEach, describe, expect, it, mock } from "bun:test" +import { afterAll, afterEach, beforeEach, describe, expect, it, mock, spyOn } from "bun:test" import { mkdirSync, readFileSync, rmSync, writeFileSync } from "node:fs" import { tmpdir } from "node:os" import { join } from "node:path" @@ -68,14 +68,9 @@ describe("migrateLegacyPluginEntry", () => { writeFileSync(configPath, originalContent) const fs = await import("node:fs") - const originalRenameSync = fs.renameSync - - mock.module("node:fs", () => ({ - ...fs, - renameSync: () => { - throw new Error("simulated rename failure") - }, - })) + const renameSyncSpy = spyOn(fs, "renameSync").mockImplementation(() => { + throw new Error("simulated rename failure") + }) try { const { migrateLegacyPluginEntry } = await importFreshMigrationModule() @@ -87,10 +82,7 @@ describe("migrateLegacyPluginEntry", () => { expect(readFileSync(tempPath, "utf-8")).toContain("oh-my-openagent@latest") expect(readFileSync(tempPath, "utf-8")).not.toContain("oh-my-opencode") } finally { - mock.module("node:fs", () => ({ - ...fs, - renameSync: originalRenameSync, - })) + renameSyncSpy.mockRestore() } }) }) @@ -106,13 +98,13 @@ describe("migrateLegacyPluginEntry", () => { const originalOpenSync = fs.openSync const openSyncCalls: string[] = [] - mock.module("node:fs", () => ({ - ...fs, - openSync: (path: Parameters[0], flags: Parameters[1]) => { + const openSyncSpy = spyOn(fs, "openSync").mockImplementation(( + path: Parameters[0], + flags: Parameters[1], + ) => { openSyncCalls.push(String(flags)) return originalOpenSync(path, flags) - }, - })) + }) try { const { migrateLegacyPluginEntry } = await importFreshMigrationModule() @@ -122,10 +114,7 @@ describe("migrateLegacyPluginEntry", () => { expect(result).toBe(true) expect(openSyncCalls).toContain("r+") } finally { - mock.module("node:fs", () => ({ - ...fs, - openSync: originalOpenSync, - })) + openSyncSpy.mockRestore() } }) }) diff --git a/src/tools/delegate-task/available-models.ts b/src/tools/delegate-task/available-models.ts index 9ec078cf7..d42a9dceb 100644 --- a/src/tools/delegate-task/available-models.ts +++ b/src/tools/delegate-task/available-models.ts @@ -1,7 +1,7 @@ import type { OpencodeClient } from "./types" import { log } from "../../shared/logger" import { isRecord } from "../../shared/record-type-guard" -import { readConnectedProvidersCache, readProviderModelsCache } from "../../shared/connected-providers-cache" +import * as connectedProvidersCache from "../../shared/connected-providers-cache" type ModelListClient = OpencodeClient & { model: { list: () => Promise } @@ -34,7 +34,7 @@ function addFromProviderModels( } export async function getAvailableModelsForDelegateTask(client: OpencodeClient): Promise> { - const providerModelsCache = readProviderModelsCache() + const providerModelsCache = connectedProvidersCache.readProviderModelsCache() if (providerModelsCache?.models) { const connected = new Set(providerModelsCache.connected) @@ -47,7 +47,7 @@ export async function getAvailableModelsForDelegateTask(client: OpencodeClient): return out } - const connectedProviders = readConnectedProvidersCache() + const connectedProviders = connectedProvidersCache.readConnectedProvidersCache() if (!connectedProviders || connectedProviders.length === 0) { return new Set() diff --git a/src/tools/delegate-task/model-selection.ts b/src/tools/delegate-task/model-selection.ts index 43fa4741b..9577dc266 100644 --- a/src/tools/delegate-task/model-selection.ts +++ b/src/tools/delegate-task/model-selection.ts @@ -2,7 +2,7 @@ import type { FallbackEntry } from "../../shared/model-requirements" import { normalizeModel } from "../../shared/model-normalization" import { fuzzyMatchModel } from "../../shared/model-availability" import { transformModelForProvider } from "../../shared/provider-model-id-transform" -import { hasConnectedProvidersCache, hasProviderModelsCache, readConnectedProvidersCache } from "../../shared/connected-providers-cache" +import * as connectedProvidersCache from "../../shared/connected-providers-cache" import { log } from "../../shared/logger" import { parseModelString, parseVariantFromModelID } from "../../shared/model-string-parser" @@ -63,11 +63,15 @@ export function resolveModelForDelegateTask(input: { return { model: userModel } } - const connectedProviders = input.availableModels.size === 0 ? readConnectedProvidersCache() : null + const connectedProviders = input.availableModels.size === 0 ? connectedProvidersCache.readConnectedProvidersCache() : null // Before provider cache is created (first run), skip model resolution entirely. // OpenCode will use its system default model when no model is specified in the prompt. - if (input.availableModels.size === 0 && !hasProviderModelsCache() && !hasConnectedProvidersCache()) { + if ( + input.availableModels.size === 0 && + !connectedProvidersCache.hasProviderModelsCache() && + !connectedProvidersCache.hasConnectedProvidersCache() + ) { return { skipped: true } }