diff --git a/src/agents/auth-profiles/oauth-lock-timeout-classification.test.ts b/src/agents/auth-profiles/oauth-lock-timeout-classification.test.ts index bce49a77333..2a2b3f835c8 100644 --- a/src/agents/auth-profiles/oauth-lock-timeout-classification.test.ts +++ b/src/agents/auth-profiles/oauth-lock-timeout-classification.test.ts @@ -1,51 +1,10 @@ -import path from "node:path"; -import { afterAll, afterEach, beforeAll, beforeEach, describe, expect, it, vi } from "vitest"; +import { describe, expect, it } from "vitest"; import { FILE_LOCK_TIMEOUT_ERROR_CODE, type FileLockTimeoutError } from "../../infra/file-lock.js"; -import { captureEnv } from "../../test-utils/env.js"; -import { getOAuthProviderRuntimeMocks } from "./oauth-common-mocks.test-support.js"; -import "./oauth-external-auth-passthrough.test-support.js"; import { - OAUTH_AGENT_ENV_KEYS, - createOAuthMainAgentDir, - createOAuthTestTempRoot, - createExpiredOauthStore, - removeOAuthTestTempRoot, - resolveApiKeyForProfileInTest, - resetOAuthProviderRuntimeMocks, -} from "./oauth-test-utils.js"; + buildRefreshContentionError, + isGlobalRefreshLockTimeoutError, +} from "./oauth-refresh-lock-errors.js"; import { resolveAuthStorePath, resolveOAuthRefreshLockPath } from "./paths.js"; -import { clearRuntimeAuthProfileStoreSnapshots, saveAuthProfileStore } from "./store.js"; - -const { - refreshProviderOAuthCredentialWithPluginMock, - formatProviderAuthProfileApiKeyWithPluginMock, -} = getOAuthProviderRuntimeMocks(); - -let resolveApiKeyForProfile: typeof import("./oauth.js").resolveApiKeyForProfile; -let resetOAuthRefreshQueuesForTest: typeof import("./oauth.js").resetOAuthRefreshQueuesForTest; - -const { withFileLockMock } = vi.hoisted(() => ({ - withFileLockMock: vi.fn( - async (_filePath: string, _options: unknown, run: () => Promise) => await run(), - ), -})); - -vi.mock("@mariozechner/pi-ai/oauth", () => ({ - getOAuthApiKey: vi.fn(async () => null), - getOAuthProviders: () => [{ id: "openai-codex" }], -})); - -vi.mock("../../infra/file-lock.js", () => ({ - FILE_LOCK_TIMEOUT_ERROR_CODE: "file_lock_timeout", - resetFileLockStateForTest: () => undefined, - withFileLock: withFileLockMock, -})); - -vi.mock("../../plugin-sdk/file-lock.js", () => ({ - FILE_LOCK_TIMEOUT_ERROR_CODE: "file_lock_timeout", - resetFileLockStateForTest: () => undefined, - withFileLock: withFileLockMock, -})); function createLockTimeoutError(lockPath: string): FileLockTimeoutError { return Object.assign(new Error(`file lock timeout for ${lockPath.slice(0, -5)}`), { @@ -55,105 +14,42 @@ function createLockTimeoutError(lockPath: string): FileLockTimeoutError { } describe("OAuth refresh lock timeout classification", () => { - const envSnapshot = captureEnv(OAUTH_AGENT_ENV_KEYS); - let tempRoot = ""; - let agentDir = ""; - let caseIndex = 0; - - beforeAll(async () => { - tempRoot = await createOAuthTestTempRoot("openclaw-oauth-lock-timeout-"); - ({ resolveApiKeyForProfile, resetOAuthRefreshQueuesForTest } = await import("./oauth.js")); - }); - - beforeEach(async () => { - resetOAuthProviderRuntimeMocks({ - refreshProviderOAuthCredentialWithPluginMock, - formatProviderAuthProfileApiKeyWithPluginMock, - }); - withFileLockMock.mockReset(); - withFileLockMock.mockImplementation( - async (_filePath: string, _options: unknown, run: () => Promise) => await run(), - ); - clearRuntimeAuthProfileStoreSnapshots(); - const caseRoot = path.join(tempRoot, `case-${++caseIndex}`); - agentDir = await createOAuthMainAgentDir(caseRoot); - resetOAuthRefreshQueuesForTest(); - }); - - afterEach(async () => { - envSnapshot.restore(); - clearRuntimeAuthProfileStoreSnapshots(); - resetOAuthRefreshQueuesForTest(); - }); - - afterAll(async () => { - await removeOAuthTestTempRoot(tempRoot); - }); - - it("maps only global refresh lock timeouts to refresh_contention", async () => { + it("matches only the global refresh lock path", () => { const profileId = "openai-codex:default"; const provider = "openai-codex"; - const store = createExpiredOauthStore({ profileId, provider }); - saveAuthProfileStore(store, agentDir); + const refreshLockPath = resolveOAuthRefreshLockPath(provider, profileId); + const authStoreLockPath = resolveAuthStorePath("/tmp/openclaw-oauth-lock-timeout/agent"); - const refreshLockPath = `${resolveOAuthRefreshLockPath(provider, profileId)}.lock`; - withFileLockMock.mockImplementationOnce(async () => { - throw createLockTimeoutError(refreshLockPath); - }); - - try { - await resolveApiKeyForProfileInTest(resolveApiKeyForProfile, { - store, - profileId, - agentDir, - }); - throw new Error("expected refresh contention error"); - } catch (error) { - expect((error as Error).message).toContain("another process is already refreshing"); - expect((error as Error).message).toContain( - "Please wait for the in-flight refresh to finish and retry.", - ); - expect((error as Error & { cause?: unknown }).cause).toMatchObject({ - code: "refresh_contention", - }); - expect( - ((error as Error & { cause?: { cause?: unknown } }).cause as { cause?: unknown }).cause, - ).toMatchObject({ - code: FILE_LOCK_TIMEOUT_ERROR_CODE, - lockPath: refreshLockPath, - }); - } + expect( + isGlobalRefreshLockTimeoutError( + createLockTimeoutError(`${refreshLockPath}.lock`), + refreshLockPath, + ), + ).toBe(true); + expect( + isGlobalRefreshLockTimeoutError( + createLockTimeoutError(`${authStoreLockPath}.lock`), + refreshLockPath, + ), + ).toBe(false); }); - it("preserves auth-store lock timeouts instead of remapping them to refresh_contention", async () => { + it("builds refresh_contention errors that preserve the file-lock cause", () => { const profileId = "openai-codex:default"; const provider = "openai-codex"; - const store = createExpiredOauthStore({ profileId, provider }); - saveAuthProfileStore(store, agentDir); + const refreshLockPath = resolveOAuthRefreshLockPath(provider, profileId); + const cause = createLockTimeoutError(`${refreshLockPath}.lock`); - const authStoreLockPath = `${resolveAuthStorePath(agentDir)}.lock`; - withFileLockMock - .mockImplementationOnce( - async (_filePath: string, _options: unknown, run: () => Promise) => await run(), - ) - .mockImplementationOnce(async () => { - throw createLockTimeoutError(authStoreLockPath); - }); + const error = buildRefreshContentionError({ provider, profileId, cause }); - try { - await resolveApiKeyForProfileInTest(resolveApiKeyForProfile, { - store, - profileId, - agentDir, - }); - throw new Error("expected auth-store lock timeout"); - } catch (error) { - expect((error as Error).message).toContain("file lock timeout"); - expect((error as Error).message).toContain("Please try again or re-authenticate."); - expect((error as Error & { cause?: unknown }).cause).toMatchObject({ + expect(error).toMatchObject({ + code: "refresh_contention", + cause: { code: FILE_LOCK_TIMEOUT_ERROR_CODE, - lockPath: authStoreLockPath, - }); - } + lockPath: `${refreshLockPath}.lock`, + }, + }); + expect(error.message).toContain("another process is already refreshing"); + expect(error.message).toContain("Please wait for the in-flight refresh to finish and retry."); }); }); diff --git a/src/agents/auth-profiles/oauth-manager.ts b/src/agents/auth-profiles/oauth-manager.ts index 7a6d58ad17b..088c097ecab 100644 --- a/src/agents/auth-profiles/oauth-manager.ts +++ b/src/agents/auth-profiles/oauth-manager.ts @@ -1,5 +1,5 @@ import { formatErrorMessage } from "../../infra/errors.js"; -import { FILE_LOCK_TIMEOUT_ERROR_CODE, withFileLock } from "../../infra/file-lock.js"; +import { withFileLock } from "../../infra/file-lock.js"; import { AUTH_STORE_LOCK_OPTIONS, OAUTH_REFRESH_CALL_TIMEOUT_MS, @@ -7,6 +7,10 @@ import { log, } from "./constants.js"; import { shouldMirrorRefreshedOAuthCredential } from "./oauth-identity.js"; +import { + buildRefreshContentionError, + isGlobalRefreshLockTimeoutError, +} from "./oauth-refresh-lock-errors.js"; import { areOAuthCredentialsEquivalent, hasUsableOAuthCredential, @@ -131,33 +135,6 @@ function hasOAuthCredentialChanged( ); } -function isGlobalRefreshLockTimeoutError(error: unknown, lockPath: string): boolean { - const candidate = - typeof error === "object" && error !== null - ? (error as { code?: unknown; lockPath?: unknown }) - : undefined; - return ( - candidate?.code === FILE_LOCK_TIMEOUT_ERROR_CODE && candidate.lockPath === `${lockPath}.lock` - ); -} - -function buildRefreshContentionError(params: { - provider: string; - profileId: string; - cause: unknown; -}): Error & { code: "refresh_contention"; cause: unknown } { - return Object.assign( - new Error( - `OAuth refresh failed (refresh_contention): another process is already refreshing ${params.provider} for ${params.profileId}. Please wait for the in-flight refresh to finish and retry.`, - { cause: params.cause }, - ), - { - code: "refresh_contention" as const, - cause: params.cause, - }, - ); -} - async function loadFreshStoredOAuthCredential(params: { profileId: string; agentDir?: string; diff --git a/src/agents/auth-profiles/oauth-refresh-lock-errors.ts b/src/agents/auth-profiles/oauth-refresh-lock-errors.ts new file mode 100644 index 00000000000..84409274dd1 --- /dev/null +++ b/src/agents/auth-profiles/oauth-refresh-lock-errors.ts @@ -0,0 +1,28 @@ +import { FILE_LOCK_TIMEOUT_ERROR_CODE } from "../../infra/file-lock.js"; + +export function isGlobalRefreshLockTimeoutError(error: unknown, lockPath: string): boolean { + const candidate = + typeof error === "object" && error !== null + ? (error as { code?: unknown; lockPath?: unknown }) + : undefined; + return ( + candidate?.code === FILE_LOCK_TIMEOUT_ERROR_CODE && candidate.lockPath === `${lockPath}.lock` + ); +} + +export function buildRefreshContentionError(params: { + provider: string; + profileId: string; + cause: unknown; +}): Error & { code: "refresh_contention"; cause: unknown } { + return Object.assign( + new Error( + `OAuth refresh failed (refresh_contention): another process is already refreshing ${params.provider} for ${params.profileId}. Please wait for the in-flight refresh to finish and retry.`, + { cause: params.cause }, + ), + { + code: "refresh_contention" as const, + cause: params.cause, + }, + ); +}