diff --git a/src/features/skill-mcp-manager/manager-oauth-retry.test.ts b/src/features/skill-mcp-manager/manager-oauth-retry.test.ts index f887e80e7..dedd68baa 100644 --- a/src/features/skill-mcp-manager/manager-oauth-retry.test.ts +++ b/src/features/skill-mcp-manager/manager-oauth-retry.test.ts @@ -1,30 +1,20 @@ -import { afterAll, beforeEach, describe, expect, it, mock } from "bun:test" +import { describe, expect, it, mock, spyOn } from "bun:test" import type { ClaudeCodeMcpServer } from "../claude-code-mcp-loader/types" import type { OAuthTokenData } from "../mcp-oauth/storage" -import type { SkillMcpClientInfo, SkillMcpServerContext } from "./types" +import { unsafeTestValue } from "../../../test-support/unsafe-test-value" +import { SkillMcpManager } from "./manager" +import type { McpClient, SkillMcpClientInfo, SkillMcpServerContext } from "./types" -const mockGetOrCreateClient = mock(async () => { - throw new Error("not used") -}) +type ManagerWithPrivateRetry = { + getOrCreateClientWithRetry: (info: SkillMcpClientInfo, config: ClaudeCodeMcpServer) => Promise +} -const mockGetOrCreateClientWithRetryImpl = mock(async () => ({ - callTool: mock(async () => ({ content: [{ type: "text", text: "unused" }] })), - close: mock(async () => {}), -})) - -type ManagerModule = typeof import("./manager") - -async function importFreshManagerModule(): Promise { - mock.module("./connection", () => ({ - getOrCreateClient: mockGetOrCreateClient, - getOrCreateClientWithRetryImpl: mockGetOrCreateClientWithRetryImpl, - })) - - mock.module("../mcp-oauth/provider", () => ({ - McpOAuthProvider: class MockMcpOAuthProvider {}, - })) - - return await import(new URL(`./manager.ts?oauth-retry-test=${Date.now()}-${Math.random()}`, import.meta.url).href) +function stubClientRetry(manager: SkillMcpManager, callTool: McpClient["callTool"]): void { + const client = unsafeTestValue({ + callTool, + close: mock(async () => {}), + }) + spyOn(unsafeTestValue(manager), "getOrCreateClientWithRetry").mockResolvedValue(client) } function createInfo(): SkillMcpClientInfo { @@ -46,19 +36,9 @@ function createContext(): SkillMcpServerContext { } } -afterAll(() => { - mock.restore() -}) - describe("SkillMcpManager post-request OAuth retry", () => { - beforeEach(() => { - mockGetOrCreateClient.mockClear() - mockGetOrCreateClientWithRetryImpl.mockClear() - }) - it("retries the operation after a 401 refresh succeeds", async () => { // given - const { SkillMcpManager } = await importFreshManagerModule() const refresh = mock(async () => ({ accessToken: "refreshed-token" } satisfies OAuthTokenData)) const manager = new SkillMcpManager({ createOAuthProvider: () => ({ @@ -74,7 +54,7 @@ describe("SkillMcpManager post-request OAuth retry", () => { return { content: [{ type: "text", text: "success" }] } }) - mockGetOrCreateClientWithRetryImpl.mockResolvedValue({ callTool, close: mock(async () => {}) }) + stubClientRetry(manager, callTool) // when const result = await manager.callTool(createInfo(), createContext(), "test-tool", {}) @@ -87,7 +67,6 @@ describe("SkillMcpManager post-request OAuth retry", () => { it("retries the operation after a 403 refresh succeeds without step-up scope", async () => { // given - const { SkillMcpManager } = await importFreshManagerModule() const refresh = mock(async () => ({ accessToken: "refreshed-token" } satisfies OAuthTokenData)) const manager = new SkillMcpManager({ createOAuthProvider: () => ({ @@ -103,7 +82,7 @@ describe("SkillMcpManager post-request OAuth retry", () => { return { content: [{ type: "text", text: "success" }] } }) - mockGetOrCreateClientWithRetryImpl.mockResolvedValue({ callTool, close: mock(async () => {}) }) + stubClientRetry(manager, callTool) // when const result = await manager.callTool(createInfo(), createContext(), "test-tool", {}) @@ -116,7 +95,6 @@ describe("SkillMcpManager post-request OAuth retry", () => { it("propagates the auth error without retry when refresh fails", async () => { // given - const { SkillMcpManager } = await importFreshManagerModule() const refresh = mock(async () => { throw new Error("refresh failed") }) @@ -130,7 +108,7 @@ describe("SkillMcpManager post-request OAuth retry", () => { const callTool = mock(async () => { throw new Error("401 Unauthorized") }) - mockGetOrCreateClientWithRetryImpl.mockResolvedValue({ callTool, close: mock(async () => {}) }) + stubClientRetry(manager, callTool) // when / then await expect(manager.callTool(createInfo(), createContext(), "test-tool", {})).rejects.toThrow("401 Unauthorized") @@ -140,7 +118,6 @@ describe("SkillMcpManager post-request OAuth retry", () => { it("only attempts one refresh when the retried operation returns 401 again", async () => { // given - const { SkillMcpManager } = await importFreshManagerModule() const refresh = mock(async () => ({ accessToken: "refreshed-token" } satisfies OAuthTokenData)) const manager = new SkillMcpManager({ createOAuthProvider: () => ({ @@ -152,7 +129,7 @@ describe("SkillMcpManager post-request OAuth retry", () => { const callTool = mock(async () => { throw new Error("401 Unauthorized") }) - mockGetOrCreateClientWithRetryImpl.mockResolvedValue({ callTool, close: mock(async () => {}) }) + stubClientRetry(manager, callTool) // when / then await expect(manager.callTool(createInfo(), createContext(), "test-tool", {})).rejects.toThrow("401 Unauthorized") diff --git a/src/features/skill-mcp-manager/oauth-handler.test.ts b/src/features/skill-mcp-manager/oauth-handler.test.ts index 35823c6ae..d4d447679 100644 --- a/src/features/skill-mcp-manager/oauth-handler.test.ts +++ b/src/features/skill-mcp-manager/oauth-handler.test.ts @@ -6,10 +6,6 @@ import type { OAuthProviderFactory, OAuthProviderLike } from "./types" type OAuthHandlerModule = typeof import("./oauth-handler") async function importFreshOAuthHandlerModule(): Promise { - mock.module("../mcp-oauth/provider", () => ({ - McpOAuthProvider: class MockMcpOAuthProvider {}, - })) - return await import(new URL(`./oauth-handler.ts?oauth-handler-test=${Date.now()}-${Math.random()}`, import.meta.url).href) } diff --git a/src/hooks/rules-injector/project-root-finder.test.ts b/src/hooks/rules-injector/project-root-finder.test.ts index 35d442290..edda25edf 100644 --- a/src/hooks/rules-injector/project-root-finder.test.ts +++ b/src/hooks/rules-injector/project-root-finder.test.ts @@ -1,47 +1,43 @@ -import { afterEach, describe, expect, it, mock } from "bun:test"; +import { afterEach, describe, expect, it } from "bun:test"; +import { mkdirSync, rmSync, unlinkSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; + +let testRoot = ""; describe("findProjectRoot", () => { - afterEach(async () => { - const actualFileSystem = await import("node:fs"); - mock.module("node:fs", () => actualFileSystem); + afterEach(() => { + if (testRoot) { + rmSync(testRoot, { recursive: true, force: true }); + testRoot = ""; + } }); it("memoizes repeated lookups for the same start path and resets on cache clear", async () => { // given - const actualFileSystem = await import("node:fs"); - const projectRoot = "/workspace/project"; - const startPath = `${projectRoot}/src/file.ts`; - const packageJsonPath = `${projectRoot}/package.json`; - - const existsSyncSpy = mock((path: string) => path === packageJsonPath); - const statSyncSpy = mock(() => ({ isDirectory: () => false })); - - mock.module("node:fs", () => ({ - ...actualFileSystem, - existsSync: existsSyncSpy, - statSync: statSyncSpy, - })); + testRoot = join(tmpdir(), `rules-project-root-${Date.now()}-${Math.random()}`); + const projectRoot = join(testRoot, "project"); + const sourceDirectory = join(projectRoot, "src"); + const startPath = join(sourceDirectory, "file.ts"); + const packageJsonPath = join(projectRoot, "package.json"); + mkdirSync(sourceDirectory, { recursive: true }); + writeFileSync(startPath, "export const value = 1;\n"); + writeFileSync(packageJsonPath, "{}\n"); const { clearProjectRootCache, findProjectRoot } = await import( - `./project-root-finder.ts?memoization=${Date.now()}` + `./project-root-finder.ts?memoization=${Date.now()}-${Math.random()}` ); // when const firstResult = findProjectRoot(startPath); - const firstExistsSyncCallCount = existsSyncSpy.mock.calls.length; - + unlinkSync(packageJsonPath); const secondResult = findProjectRoot(startPath); - const secondExistsSyncCallCount = existsSyncSpy.mock.calls.length; - clearProjectRootCache(); const thirdResult = findProjectRoot(startPath); // then expect(firstResult).toBe(projectRoot); expect(secondResult).toBe(projectRoot); - expect(thirdResult).toBe(projectRoot); - expect(firstExistsSyncCallCount).toBeGreaterThan(0); - expect(secondExistsSyncCallCount).toBe(firstExistsSyncCallCount); - expect(existsSyncSpy).toHaveBeenCalledTimes(firstExistsSyncCallCount * 2); + expect(thirdResult).toBeNull(); }); });