From 3e7c5fd0e9f30efb2f83c347ac4cb093823f7b77 Mon Sep 17 00:00:00 2001 From: luvs01 Date: Sun, 30 Aug 2026 14:38:22 +0900 Subject: [PATCH 1/8] fix(oauth): clear the refresh intent after a transient Anthropic failure --- src/oauth/index.ts | 16 ++++++++++++++-- tests/oauth-refresh.test.ts | 25 +++++++++++++++++++++++++ 2 files changed, 39 insertions(+), 2 deletions(-) diff --git a/src/oauth/index.ts b/src/oauth/index.ts index 7a356df8c8..9945cc553c 100644 --- a/src/oauth/index.ts +++ b/src/oauth/index.ts @@ -694,8 +694,20 @@ export async function refreshAnthropicAccountWithLock( clearOAuthRefreshIntent(provider, accountId, generation); return fresh.access; } catch (error) { - if (error instanceof OAuthMutationBusyError) throw error; - if (!terminal(error)) throw error; + if (error instanceof OAuthMutationBusyError || error instanceof OAuthTokenRefreshStaleError) throw error; + if (!terminal(error)) { + // A non-terminal failure means the credential was never rejected, so the caller is + // told to retry. Leaving the intent behind contradicted that: the next attempt hit + // the pending-intent branch above and raised OAuthLoginRequiredError, so one 503 or + // timeout locked the account out of refresh entirely until manual re-auth — even + // once upstream recovered. Clear it so the promised retry can actually happen. + // + // The replay guard is preserved by `uncertain`: a refresh whose outcome is genuinely + // unknown surfaces as an uncertain intent from the store, which this path never + // clears, and a superseded owner still leaves through OAuthTokenRefreshStaleError. + clearOAuthRefreshIntent(provider, accountId, generation); + throw error; + } await markAccountNeedsReauthIfGeneration(provider, accountId, generation, writerGeneration); clearOAuthRefreshIntent(provider, accountId, generation); throw new OAuthLoginRequiredError(provider); diff --git a/tests/oauth-refresh.test.ts b/tests/oauth-refresh.test.ts index 0975e8285c..bdf74c3b64 100644 --- a/tests/oauth-refresh.test.ts +++ b/tests/oauth-refresh.test.ts @@ -517,6 +517,31 @@ describe("oauth refresh hardening", () => { expect(getAccountSet("anthropic")!.accounts[0]!.needsReauth).toBe(true); }); + /** + * A non-terminal refresh failure is reported as retryable, but the refresh intent outlived + * it. The next attempt then hit the pending-intent branch and raised OAuthLoginRequiredError, + * so a single 503 or timeout locked the account out of refresh until manual re-authentication + * even after upstream recovered. The replay guard is unaffected: an intent whose outcome is + * genuinely unknown is reported `uncertain` by the store and is still never cleared here. + */ + test("a transient Anthropic failure leaves the account refreshable", async () => { + await saveCredential("anthropic", { access: "old", refresh: "rt-old", expires: 1, accountId: "acct" }); + const id = getAccountSet("anthropic")!.activeAccountId; + const transient = new AnthropicTokenError("server", 503, undefined); + await expect(refreshAnthropicAccountWithLock("anthropic", id, { + ...OAUTH_PROVIDERS.anthropic!, + refresh: async () => { throw transient; }, + }, getAccountCredential("anthropic", id)!)).rejects.toBe(transient); + expect(readOAuthRefreshIntent("anthropic", id)).toBeUndefined(); + + // Upstream recovers: the retry the caller was promised must actually succeed. + await expect(refreshAnthropicAccountWithLock("anthropic", id, { + ...OAUTH_PROVIDERS.anthropic!, + refresh: async () => ({ access: "fresh", refresh: "rt-fresh", expires: Date.now() + 3_600_000 }), + }, getAccountCredential("anthropic", id)!)).resolves.toBe("fresh"); + expect(getAccountSet("anthropic")!.accounts.find(account => account.id === id)!.needsReauth).toBeUndefined(); + }); + test("Anthropic post-dispatch stale flight replacement stays retryable without replay or reauth", async () => { await saveCredential("anthropic", { access: "old", refresh: "rt-old", expires: 1, accountId: "acct" }); const id = getAccountSet("anthropic")!.activeAccountId; From 9b55278910e383d399877b6ed5ff9ca76fd38e28 Mon Sep 17 00:00:00 2001 From: luvs01 Date: Sun, 30 Aug 2026 16:22:26 +0900 Subject: [PATCH 2/8] fix(oauth): preserve Anthropic intent across uncertain refresh outcomes A timeout, lost response, unreadable body, or credential-store failure can happen after Anthropic consumed and rotated a refresh token. Clearing the durable intent lets the next attempt replay the old token, risking reuse handling and forced reauthentication. The store's uncertain flag does not cover these request outcomes. Track the pre-dispatch boundary explicitly. Clear a non-terminal intent only before dispatch or after the adapter reports an explicit non-success HTTP response; retain it after all other post-dispatch outcomes and after provider success followed by persistence failure. Intent cleanup is secondary to the refresh result. A failed unlink now leaves the replay guard in place without replacing the original provider error, pre-dispatch abort, terminal login result, or successfully persisted credential. Cover definite 503 retry, uncertain post-dispatch failure, pre-dispatch abort, cleanup failure, post-provider persistence failure, and post-persist cleanup failure. --- src/oauth/index.ts | 74 +++++++++++++++++---- tests/oauth-refresh.test.ts | 125 ++++++++++++++++++++++++++++++++++-- 2 files changed, 181 insertions(+), 18 deletions(-) diff --git a/src/oauth/index.ts b/src/oauth/index.ts index 9945cc553c..aa5460df88 100644 --- a/src/oauth/index.ts +++ b/src/oauth/index.ts @@ -562,6 +562,45 @@ function terminal(error:unknown):boolean{ if (error instanceof RefreshIntentIOError) return false; return isTerminalRefreshError(error); } + +/** + * True when the token endpoint definitively answered and rejected the request. + * + * The Anthropic adapter attaches an HTTP status only to an explicit non-success response, + * which is the retryable rejection this PR handles. Everything else (timeout, dropped + * connection, a body that could not be read or parsed, or a local persistence fault) leaves + * the outcome unknown: the server may already have rotated the token, and a blind replay + * could trip refresh-token-reuse revocation. Those cases must keep the refresh intent. + * + * Deliberately narrower than `terminal()`, which asks whether the CREDENTIAL is dead. + * This asks the different question of whether the ATTEMPT is known to have failed. + */ +function definitivelyAnswered(error: unknown): boolean { + if (error instanceof AnthropicTokenError) return error.httpStatus !== undefined; + return false; +} + +/** + * Intent cleanup is secondary to the refresh outcome it protects. + * + * A filesystem failure here must not replace a provider error, a pre-dispatch abort, or a + * successfully persisted credential. Leaving the intent in place is the conservative fallback: + * it blocks replay until the local persistence problem is repaired. + */ +function clearAnthropicRefreshIntentBestEffort( + provider: string, + accountId: string, + generation: string, +): boolean { + try { + return clearOAuthRefreshIntent(provider, accountId, generation); + } catch { + console.warn( + "[opencodex] Anthropic refresh intent cleanup failed; preserving the durable replay guard.", + ); + return false; + } +} function authoritative(stored:OAuthCredentials,active:boolean,now:()=>number):OAuthCredentials{if(stored.source!=="local-cli")return stored;const disk=detectGrokCliToken();if(!disk)return stored;const allowed=isSameGrokIdentity(stored,disk)||(active&&!hasComparableGrokIdentity(stored,disk));return allowed&&shouldAdoptGrokGeneration(stored,disk,now(),REFRESH_SKEW_MS)?disk:stored;} function merged(fresh: OAuthCredentials, previous: OAuthCredentials): OAuthCredentials { return { @@ -677,9 +716,14 @@ export async function refreshAnthropicAccountWithLock( return stored.access; } + let refreshMayHaveReachedProvider = false; try { writeOAuthRefreshIntent(provider, accountId, generation, now(), deps.flight?.flightId); if (deps.signal?.aborted) throw deps.signal.reason; + // From this point on, even a synchronous client error is conservatively post-dispatch: + // the provider may have received and rotated the refresh token before the caller learned + // the outcome. + refreshMayHaveReachedProvider = true; if (deps.flight) deps.flight.dispatched = true; const fresh = merged(await def.refresh(stored.refresh, deps.signal), stored); const outcome = await mergeAccountCredential(provider, accountId, fresh, { @@ -687,29 +731,35 @@ export async function refreshAnthropicAccountWithLock( afterPrePersistRead: deps.afterPrePersistRead, }); if (outcome.superseded) { - clearOAuthRefreshIntent(provider, accountId, generation); + clearAnthropicRefreshIntentBestEffort(provider, accountId, generation); if (outcome.stored.expires > now() + REFRESH_SKEW_MS) return outcome.stored.access; throw new OAuthLoginRequiredError(provider); } - clearOAuthRefreshIntent(provider, accountId, generation); + // The rotated credential is durable now. A cleanup failure must not turn that committed + // success into a refresh failure; the old-generation intent remains a conservative guard. + clearAnthropicRefreshIntentBestEffort(provider, accountId, generation); return fresh.access; } catch (error) { if (error instanceof OAuthMutationBusyError || error instanceof OAuthTokenRefreshStaleError) throw error; if (!terminal(error)) { - // A non-terminal failure means the credential was never rejected, so the caller is - // told to retry. Leaving the intent behind contradicted that: the next attempt hit - // the pending-intent branch above and raised OAuthLoginRequiredError, so one 503 or - // timeout locked the account out of refresh entirely until manual re-auth — even - // once upstream recovered. Clear it so the promised retry can actually happen. + // A non-terminal failure tells the caller to retry, but the intent outlived it, so + // the next attempt hit the pending-intent branch above and raised + // OAuthLoginRequiredError. One 503 locked the account out of refresh until manual + // re-auth even after upstream recovered. // - // The replay guard is preserved by `uncertain`: a refresh whose outcome is genuinely - // unknown surfaces as an uncertain intent from the store, which this path never - // clears, and a superseded owner still leaves through OAuthTokenRefreshStaleError. - clearOAuthRefreshIntent(provider, accountId, generation); + // Only clear the intent when the server DEFINITIVELY answered and rejected the + // request. The adapter attaches an HTTP status only to that explicit non-success + // response. A timeout, a dropped connection, or an unreadable/unparseable body + // carries no status: the server may already have + // rotated the token, and replaying it could trip refresh-token-reuse revocation. + // Those outcomes keep the intent so the guard still refuses a blind replay. + if (!refreshMayHaveReachedProvider || definitivelyAnswered(error)) { + clearAnthropicRefreshIntentBestEffort(provider, accountId, generation); + } throw error; } await markAccountNeedsReauthIfGeneration(provider, accountId, generation, writerGeneration); - clearOAuthRefreshIntent(provider, accountId, generation); + clearAnthropicRefreshIntentBestEffort(provider, accountId, generation); throw new OAuthLoginRequiredError(provider); } } finally { diff --git a/tests/oauth-refresh.test.ts b/tests/oauth-refresh.test.ts index bdf74c3b64..a817e76159 100644 --- a/tests/oauth-refresh.test.ts +++ b/tests/oauth-refresh.test.ts @@ -518,13 +518,12 @@ describe("oauth refresh hardening", () => { }); /** - * A non-terminal refresh failure is reported as retryable, but the refresh intent outlived - * it. The next attempt then hit the pending-intent branch and raised OAuthLoginRequiredError, - * so a single 503 or timeout locked the account out of refresh until manual re-authentication - * even after upstream recovered. The replay guard is unaffected: an intent whose outcome is - * genuinely unknown is reported `uncertain` by the store and is still never cleared here. + * A definitive non-terminal HTTP failure is retryable: the endpoint answered with a failure, + * so the durable intent must not turn that promised retry into OAuthLoginRequiredError. + * A timeout or unreadable response is different — the provider may already have rotated the + * token, so that post-dispatch intent remains as the replay guard. */ - test("a transient Anthropic failure leaves the account refreshable", async () => { + test("a definitive transient Anthropic HTTP failure leaves the account refreshable", async () => { await saveCredential("anthropic", { access: "old", refresh: "rt-old", expires: 1, accountId: "acct" }); const id = getAccountSet("anthropic")!.activeAccountId; const transient = new AnthropicTokenError("server", 503, undefined); @@ -542,6 +541,120 @@ describe("oauth refresh hardening", () => { expect(getAccountSet("anthropic")!.accounts.find(account => account.id === id)!.needsReauth).toBeUndefined(); }); + test("an Anthropic refresh with an uncertain post-dispatch outcome preserves its intent", async () => { + await saveCredential("anthropic", { access: "old", refresh: "rt-old", expires: 1, accountId: "acct" }); + const id = getAccountSet("anthropic")!.activeAccountId; + const credential = getAccountCredential("anthropic", id)!; + const generation = credentialGeneration(credential); + const timeout = new AnthropicTokenError("timeout", undefined, undefined); + + await expect(refreshAnthropicAccountWithLock("anthropic", id, { + ...OAUTH_PROVIDERS.anthropic!, + refresh: async () => { throw timeout; }, + }, credential)).rejects.toBe(timeout); + + expect(readOAuthRefreshIntent("anthropic", id)).toMatchObject({ generation }); + expect(getAccountSet("anthropic")!.accounts.find(account => account.id === id)!.needsReauth).toBeUndefined(); + }); + + test("a pre-dispatch Anthropic abort clears its unconsumed refresh intent", async () => { + await saveCredential("anthropic", { access: "old", refresh: "rt-old", expires: 1, accountId: "acct" }); + const id = getAccountSet("anthropic")!.activeAccountId; + const credential = getAccountCredential("anthropic", id)!; + const aborted = new Error("aborted before dispatch"); + const controller = new AbortController(); + controller.abort(aborted); + let calls = 0; + + await expect(refreshAnthropicAccountWithLock("anthropic", id, { + ...OAUTH_PROVIDERS.anthropic!, + refresh: async () => { calls += 1; throw new Error("must not run"); }, + }, credential, { signal: controller.signal })).rejects.toBe(aborted); + + expect(calls).toBe(0); + expect(readOAuthRefreshIntent("anthropic", id)).toBeUndefined(); + }); + + test("intent cleanup failure preserves the original Anthropic HTTP error", async () => { + await saveCredential("anthropic", { access: "old", refresh: "rt-old", expires: 1, accountId: "acct" }); + const id = getAccountSet("anthropic")!.activeAccountId; + const credential = getAccountCredential("anthropic", id)!; + const generation = credentialGeneration(credential); + const transient = new AnthropicTokenError("server", 503, undefined); + const clearFailure = new Error("intent unlink failed"); + const clearSpy = spyOn(storeModule, "clearOAuthRefreshIntent").mockImplementation(() => { + throw clearFailure; + }); + const warnSpy = spyOn(console, "warn").mockImplementation(() => {}); + try { + await expect(refreshAnthropicAccountWithLock("anthropic", id, { + ...OAUTH_PROVIDERS.anthropic!, + refresh: async () => { throw transient; }, + }, credential)).rejects.toBe(transient); + + expect(clearSpy).toHaveBeenCalled(); + expect(readOAuthRefreshIntent("anthropic", id)).toMatchObject({ generation }); + expect(getAccountSet("anthropic")!.accounts.find(account => account.id === id)!.needsReauth).toBeUndefined(); + } finally { + warnSpy.mockRestore(); + clearSpy.mockRestore(); + } + }); + + test("Anthropic persistence failure after provider success preserves the replay guard", async () => { + await saveCredential("anthropic", { access: "old", refresh: "rt-old", expires: 1, accountId: "acct" }); + const id = getAccountSet("anthropic")!.activeAccountId; + const credential = getAccountCredential("anthropic", id)!; + const generation = credentialGeneration(credential); + const persistenceFailure = new Error("credential persistence failed"); + const mergeSpy = spyOn(storeModule, "mergeAccountCredential").mockImplementation(async () => { + throw persistenceFailure; + }); + try { + await expect(refreshAnthropicAccountWithLock("anthropic", id, { + ...OAUTH_PROVIDERS.anthropic!, + refresh: async () => ({ + access: "fresh", + refresh: "rt-fresh", + expires: Date.now() + 3_600_000, + }), + }, credential)).rejects.toBe(persistenceFailure); + + expect(readOAuthRefreshIntent("anthropic", id)).toMatchObject({ generation }); + expect(getAccountCredential("anthropic", id)).toEqual(credential); + expect(getAccountSet("anthropic")!.accounts.find(account => account.id === id)!.needsReauth).toBeUndefined(); + } finally { + mergeSpy.mockRestore(); + } + }); + + test("post-persist intent cleanup failure does not turn Anthropic refresh success into failure", async () => { + await saveCredential("anthropic", { access: "old", refresh: "rt-old", expires: 1, accountId: "acct" }); + const id = getAccountSet("anthropic")!.activeAccountId; + const credential = getAccountCredential("anthropic", id)!; + const oldGeneration = credentialGeneration(credential); + const clearSpy = spyOn(storeModule, "clearOAuthRefreshIntent").mockImplementation(() => { + throw new Error("intent unlink failed"); + }); + const warnSpy = spyOn(console, "warn").mockImplementation(() => {}); + try { + await expect(refreshAnthropicAccountWithLock("anthropic", id, { + ...OAUTH_PROVIDERS.anthropic!, + refresh: async () => ({ + access: "fresh", + refresh: "rt-fresh", + expires: Date.now() + 3_600_000, + }), + }, credential)).resolves.toBe("fresh"); + + expect(getAccountCredential("anthropic", id)?.access).toBe("fresh"); + expect(readOAuthRefreshIntent("anthropic", id)).toMatchObject({ generation: oldGeneration }); + } finally { + warnSpy.mockRestore(); + clearSpy.mockRestore(); + } + }); + test("Anthropic post-dispatch stale flight replacement stays retryable without replay or reauth", async () => { await saveCredential("anthropic", { access: "old", refresh: "rt-old", expires: 1, accountId: "acct" }); const id = getAccountSet("anthropic")!.activeAccountId; From 97a547c6c3faa20a84ee1cc3e9f5a853b128e365 Mon Sep 17 00:00:00 2001 From: luvs01 Date: Sun, 30 Aug 2026 17:36:44 +0900 Subject: [PATCH 3/8] fix(oauth): resume retry-safe Anthropic intent cleanup Persist an attempt-scoped cleanup-pending marker before returning a definitive rejection or pre-dispatch abort. Serialize all refresh-intent mutations with the existing SQLite config transaction, refuse guard overwrites, and retry only the exact safe cleanup before provider redispatch. Preserve ordinary intents for timeouts and post-provider persistence uncertainty. --- src/oauth/index.ts | 156 ++++++++++++++-- src/oauth/store.ts | 258 ++++++++++++++++++++++++-- tests/oauth-refresh.test.ts | 360 +++++++++++++++++++++++++++++++++--- 3 files changed, 718 insertions(+), 56 deletions(-) diff --git a/src/oauth/index.ts b/src/oauth/index.ts index aa5460df88..1b3754dd21 100644 --- a/src/oauth/index.ts +++ b/src/oauth/index.ts @@ -4,7 +4,31 @@ import type { OcxConfig, OcxProviderConfig, RefreshPolicy } from "../types"; import { loadConfig, resolveEnvValue, saveConfig } from "../config"; import { maskEmail } from "../lib/privacy"; import { KiroTokenRefreshError, environmentKiroRoutingMetadata, loginKiro, refreshKiroToken, settleKiroLoginTransaction } from "./kiro"; -import { getAccountCredential, getAccountCredentialWithStatus, getAccountSet, removeAccount, saveAccountCredential, saveCredential, setActiveAccount, getCredential, credentialGeneration, createOAuthRefreshIntentLock, mergeAccountCredential, markAccountNeedsReauthIfGeneration, readOAuthRefreshIntent, writeOAuthRefreshIntent, markOAuthRefreshIntentStaleOwner, clearOAuthRefreshIntent, normalizeAuthStoreBuffer, OAuthMutationBusyError } from "./store"; +import { + OAuthMutationBusyError, + OAuthRefreshIntentIOError, + clearOAuthRefreshIntent, + clearOAuthRefreshIntentIfMatch, + createOAuthRefreshIntentLock, + credentialGeneration, + getAccountCredential, + getAccountCredentialWithStatus, + getAccountSet, + getCredential, + markAccountNeedsReauthIfGeneration, + markOAuthRefreshIntentCleanupPending, + markOAuthRefreshIntentStaleOwner, + mergeAccountCredential, + normalizeAuthStoreBuffer, + readOAuthRefreshIntent, + removeAccount, + saveAccountCredential, + saveCredential, + setActiveAccount, + writeOAuthRefreshIntent, + type OAuthRefreshIntent, + type OAuthRefreshIntentCleanupPending, +} from "./store"; import { loginXai, refreshXaiToken, XAI_LOCAL_CLI_DETACH_WARNING, XaiTokenRequestError } from "./xai"; import { ANTHROPIC_OAUTH_BETA, AnthropicTokenError, loginAnthropic, refreshAnthropicToken } from "./anthropic"; import { loginKimi, refreshKimiToken } from "./kimi"; @@ -559,7 +583,7 @@ function terminal(error:unknown):boolean{ // Local durable-write/read/cleanup failures are operational, not credential // death: the provider credential was never rejected or consumed. Never mark // the account needsReauth for broken local persistence infrastructure. - if (error instanceof RefreshIntentIOError) return false; + if (error instanceof RefreshIntentIOError || error instanceof OAuthRefreshIntentIOError) return false; return isTerminalRefreshError(error); } @@ -583,17 +607,19 @@ function definitivelyAnswered(error: unknown): boolean { /** * Intent cleanup is secondary to the refresh outcome it protects. * - * A filesystem failure here must not replace a provider error, a pre-dispatch abort, or a - * successfully persisted credential. Leaving the intent in place is the conservative fallback: - * it blocks replay until the local persistence problem is repaired. + * Once a credential is already durable, cleanup remains secondary and best-effort. A known + * failed attempt takes the stricter path below: its retry-safe marker must become durable before + * the original provider error can be returned. */ function clearAnthropicRefreshIntentBestEffort( provider: string, accountId: string, - generation: string, + expected: OAuthRefreshIntent, ): boolean { try { - return clearOAuthRefreshIntent(provider, accountId, generation); + return expected.attemptId + ? clearOAuthRefreshIntentIfMatch(provider, accountId, expected) + : clearOAuthRefreshIntent(provider, accountId, expected.generation); } catch { console.warn( "[opencodex] Anthropic refresh intent cleanup failed; preserving the durable replay guard.", @@ -601,6 +627,87 @@ function clearAnthropicRefreshIntentBestEffort( return false; } } + +function clearAnthropicRefreshIntentForKnownFailure( + provider: string, + accountId: string, + expected: OAuthRefreshIntent, + cleanupPending: OAuthRefreshIntentCleanupPending, + refreshError: unknown, +): boolean { + let marked: OAuthRefreshIntent | undefined; + try { + marked = markOAuthRefreshIntentCleanupPending( + provider, + accountId, + expected, + cleanupPending, + ); + } catch (cause) { + throw new OAuthRefreshIntentIOError( + "mark-cleanup-pending", + cause, + refreshError, + ); + } + if (!marked) { + throw new OAuthRefreshIntentIOError( + "mark-cleanup-pending", + new Error("Anthropic refresh intent changed before safe cleanup"), + refreshError, + ); + } + + let cleared: boolean; + try { + cleared = clearOAuthRefreshIntentIfMatch(provider, accountId, marked); + } catch { + console.warn( + "[opencodex] Anthropic refresh intent cleanup failed; retry-safe cleanup remains pending.", + ); + return false; + } + if (!cleared) { + throw new OAuthRefreshIntentIOError( + "clear-cleanup-pending", + new Error("Anthropic refresh intent changed during safe cleanup"), + refreshError, + ); + } + return true; +} + +function resumeAnthropicRefreshIntentCleanup( + provider: string, + accountId: string, + pendingIntent: OAuthRefreshIntent, +): void { + let cleared: boolean; + try { + cleared = clearOAuthRefreshIntentIfMatch(provider, accountId, pendingIntent); + } catch (cause) { + throw new OAuthRefreshIntentIOError( + "resume-cleanup", + cause, + ); + } + if (!cleared) { + throw new OAuthRefreshIntentIOError( + "resume-cleanup", + new Error("Pending Anthropic refresh intent changed before cleanup"), + ); + } +} + +function clearObservedAnthropicRefreshIntent( + provider: string, + accountId: string, + pendingIntent: OAuthRefreshIntent, +): boolean { + return pendingIntent.attemptId + ? clearOAuthRefreshIntentIfMatch(provider, accountId, pendingIntent) + : clearOAuthRefreshIntent(provider, accountId, pendingIntent.generation); +} function authoritative(stored:OAuthCredentials,active:boolean,now:()=>number):OAuthCredentials{if(stored.source!=="local-cli")return stored;const disk=detectGrokCliToken();if(!disk)return stored;const allowed=isSameGrokIdentity(stored,disk)||(active&&!hasComparableGrokIdentity(stored,disk));return allowed&&shouldAdoptGrokGeneration(stored,disk,now(),REFRESH_SKEW_MS)?disk:stored;} function merged(fresh: OAuthCredentials, previous: OAuthCredentials): OAuthCredentials { return { @@ -679,6 +786,10 @@ export async function refreshAnthropicAccountWithLock( const account = getAccountSet(provider)?.accounts.find(candidate => candidate.id === accountId); const generation = credentialGeneration(stored); let pendingIntent = readOAuthRefreshIntent(provider, accountId); + if (pendingIntent?.cleanupPending && pendingIntent.generation === generation) { + resumeAnthropicRefreshIntentCleanup(provider, accountId, pendingIntent); + pendingIntent = undefined; + } const disk = newerClaudeCredential(stored, now()); if (disk) { const outcome = await mergeAccountCredential(provider, accountId, disk, { @@ -686,11 +797,11 @@ export async function refreshAnthropicAccountWithLock( afterPrePersistRead: deps.afterPrePersistRead, }); if (outcome.superseded) { - if (pendingIntent) clearOAuthRefreshIntent(provider, accountId, pendingIntent.generation); + if (pendingIntent) clearObservedAnthropicRefreshIntent(provider, accountId, pendingIntent); if (outcome.stored.expires > now() + REFRESH_SKEW_MS) return outcome.stored.access; throw new OAuthLoginRequiredError(provider); } - if (pendingIntent) clearOAuthRefreshIntent(provider, accountId, pendingIntent.generation); + if (pendingIntent) clearObservedAnthropicRefreshIntent(provider, accountId, pendingIntent); return disk.access; } if (!pendingIntent?.uncertain && pendingIntent?.generation === generation) { @@ -700,7 +811,9 @@ export async function refreshAnthropicAccountWithLock( markOAuthRefreshIntentStaleOwner(provider, accountId, generation, deps.replacedStaleFlight.flightId); throw new OAuthTokenRefreshStaleError(); } - clearOAuthRefreshIntent(provider, accountId, generation); + if (!clearObservedAnthropicRefreshIntent(provider, accountId, pendingIntent)) { + throw new OAuthTokenRefreshStaleError(); + } pendingIntent = undefined; } } @@ -708,7 +821,9 @@ export async function refreshAnthropicAccountWithLock( await markAccountNeedsReauthIfGeneration(provider, accountId, generation, writerGeneration); throw new OAuthLoginRequiredError(provider); } - if (pendingIntent) clearOAuthRefreshIntent(provider, accountId, pendingIntent.generation); + if (pendingIntent && !clearObservedAnthropicRefreshIntent(provider, accountId, pendingIntent)) { + throw new OAuthTokenRefreshStaleError(); + } if (account?.needsReauth) { throw new OAuthLoginRequiredError(provider); } @@ -717,8 +832,9 @@ export async function refreshAnthropicAccountWithLock( } let refreshMayHaveReachedProvider = false; + let attemptIntent: OAuthRefreshIntent | undefined; try { - writeOAuthRefreshIntent(provider, accountId, generation, now(), deps.flight?.flightId); + attemptIntent = writeOAuthRefreshIntent(provider, accountId, generation, now(), deps.flight?.flightId); if (deps.signal?.aborted) throw deps.signal.reason; // From this point on, even a synchronous client error is conservatively post-dispatch: // the provider may have received and rotated the refresh token before the caller learned @@ -731,13 +847,13 @@ export async function refreshAnthropicAccountWithLock( afterPrePersistRead: deps.afterPrePersistRead, }); if (outcome.superseded) { - clearAnthropicRefreshIntentBestEffort(provider, accountId, generation); + if (attemptIntent) clearAnthropicRefreshIntentBestEffort(provider, accountId, attemptIntent); if (outcome.stored.expires > now() + REFRESH_SKEW_MS) return outcome.stored.access; throw new OAuthLoginRequiredError(provider); } // The rotated credential is durable now. A cleanup failure must not turn that committed // success into a refresh failure; the old-generation intent remains a conservative guard. - clearAnthropicRefreshIntentBestEffort(provider, accountId, generation); + if (attemptIntent) clearAnthropicRefreshIntentBestEffort(provider, accountId, attemptIntent); return fresh.access; } catch (error) { if (error instanceof OAuthMutationBusyError || error instanceof OAuthTokenRefreshStaleError) throw error; @@ -753,13 +869,19 @@ export async function refreshAnthropicAccountWithLock( // carries no status: the server may already have // rotated the token, and replaying it could trip refresh-token-reuse revocation. // Those outcomes keep the intent so the guard still refuses a blind replay. - if (!refreshMayHaveReachedProvider || definitivelyAnswered(error)) { - clearAnthropicRefreshIntentBestEffort(provider, accountId, generation); + if ((!refreshMayHaveReachedProvider || definitivelyAnswered(error)) && attemptIntent) { + clearAnthropicRefreshIntentForKnownFailure( + provider, + accountId, + attemptIntent, + refreshMayHaveReachedProvider ? "definitive-rejection" : "pre-dispatch", + error, + ); } throw error; } await markAccountNeedsReauthIfGeneration(provider, accountId, generation, writerGeneration); - clearAnthropicRefreshIntentBestEffort(provider, accountId, generation); + if (attemptIntent) clearAnthropicRefreshIntentBestEffort(provider, accountId, attemptIntent); throw new OAuthLoginRequiredError(provider); } } finally { diff --git a/src/oauth/store.ts b/src/oauth/store.ts index 755d86f3e2..842ee127fb 100644 --- a/src/oauth/store.ts +++ b/src/oauth/store.ts @@ -19,7 +19,7 @@ import { createHash, randomUUID } from "node:crypto"; import { chmodSync, closeSync, copyFileSync, existsSync, fstatSync, mkdirSync, openSync, readFileSync, statSync, unlinkSync, writeFileSync } from "node:fs"; import { join } from "node:path"; -import { getConfigDir, atomicWriteFile, backupInvalidConfig, hardenConfigDir, hardenExistingSecret } from "../config"; +import { getConfigDir, atomicWriteFile, backupInvalidConfig, hardenConfigDir, hardenExistingSecret, withConfigMutationLockSync } from "../config"; import { assertNotRealHomeUnderTest } from "../lib/test-home-guard"; import { recordOwnedConfigPath } from "../lib/config-ownership"; import { MAX_PENDING_OAUTH_MUTATIONS } from "../lib/translator-budget"; @@ -67,23 +67,113 @@ export function getAuthRefreshIntentLockPath(provider: string, accountId: string export function getAuthRefreshIntentPath(provider: string, accountId: string): string { return `${getAuthRefreshIntentLockPath(provider, accountId)}.json`; } -export interface OAuthRefreshIntent { version: 1; provider: string; accountId: string; generation: string; createdAt: number; flightId?: string; staleOwner?: true; uncertain?: true } +export type OAuthRefreshIntentCleanupPending = "pre-dispatch" | "definitive-rejection"; +export interface OAuthRefreshIntent { + version: 1; + provider: string; + accountId: string; + generation: string; + createdAt: number; + attemptId?: string; + flightId?: string; + staleOwner?: true; + uncertain?: true; + cleanupPending?: OAuthRefreshIntentCleanupPending; +} + +export class OAuthRefreshIntentIOError extends Error { + readonly code = "OAUTH_REFRESH_INTENT_IO"; + constructor( + readonly operation: "write-intent" | "mark-cleanup-pending" | "clear-cleanup-pending" | "resume-cleanup", + cause?: unknown, + readonly refreshError?: unknown, + ) { + super(`OAuth refresh intent ${operation} failed`, cause ? { cause } : undefined); + this.name = "OAuthRefreshIntentIOError"; + } +} + +interface OAuthRefreshIntentFileSnapshot { + raw: string; + dev: number; + ino: number; + mtimeMs: number; + size: number; +} + +export interface OAuthRefreshIntentMutationHooks { + beforeMarkCommit?: () => void; + beforeClearRecheck?: () => void; +} + +class OAuthRefreshIntentChangedError extends Error {} + +function uncertainOAuthRefreshIntent(provider: string, accountId: string): OAuthRefreshIntent { + return { version: 1, provider, accountId, generation: "", createdAt: 0, uncertain: true }; +} + +function snapshotOAuthRefreshIntentFile(path: string): OAuthRefreshIntentFileSnapshot { + const raw = readFileSync(path, "utf8"); + const stat = statSync(path); + return { raw, dev: stat.dev, ino: stat.ino, mtimeMs: stat.mtimeMs, size: stat.size }; +} + +function sameOAuthRefreshIntentFileSnapshot( + left: OAuthRefreshIntentFileSnapshot, + right: OAuthRefreshIntentFileSnapshot, +): boolean { + return left.raw === right.raw + && left.dev === right.dev + && left.ino === right.ino + && left.mtimeMs === right.mtimeMs + && left.size === right.size; +} + function parseOAuthRefreshIntent( provider: string, accountId: string, raw: string, ): OAuthRefreshIntent { - const value = JSON.parse(raw) as Partial; + const parsed: unknown = JSON.parse(raw); + if (!parsed || typeof parsed !== "object" || Array.isArray(parsed)) { + return uncertainOAuthRefreshIntent(provider, accountId); + } + const value = parsed as Partial; if ( value.version !== 1 || value.provider !== provider || value.accountId !== accountId || typeof value.generation !== "string" + || !/^[0-9a-f]{64}$/.test(value.generation) || typeof value.createdAt !== "number" - || (value.flightId !== undefined && typeof value.flightId !== "string") + || !Number.isFinite(value.createdAt) + || !Number.isInteger(value.createdAt) + || value.createdAt < 0 + || ( + value.attemptId !== undefined + && (typeof value.attemptId !== "string" || value.attemptId.length === 0 || value.attemptId.length > 128) + ) + || ( + value.flightId !== undefined + && (typeof value.flightId !== "string" || value.flightId.length === 0 || value.flightId.length > 128) + ) || (value.staleOwner !== undefined && value.staleOwner !== true) + || (value.uncertain !== undefined && value.uncertain !== true) + || ( + value.cleanupPending !== undefined + && value.cleanupPending !== "pre-dispatch" + && value.cleanupPending !== "definitive-rejection" + ) + || ( + value.cleanupPending !== undefined + && ( + value.attemptId === undefined + || value.uncertain === true + || value.staleOwner === true + ) + ) ) { - return { version: 1, provider, accountId, generation: "", createdAt: 0, uncertain: true }; + return uncertainOAuthRefreshIntent(provider, accountId); } return value as OAuthRefreshIntent; } @@ -96,7 +186,7 @@ export function readOAuthRefreshIntent(provider: string, accountId: string): OAu return parseOAuthRefreshIntent(provider, accountId, readFileSync(path, "utf8")); } catch (error) { if (errorCode(error) === "ENOENT") return undefined; - return { version: 1, provider, accountId, generation: "", createdAt: 0, uncertain: true }; + return uncertainOAuthRefreshIntent(provider, accountId); } } @@ -107,27 +197,159 @@ export function peekOAuthRefreshIntent(provider: string, accountId: string): OAu return parseOAuthRefreshIntent(provider, accountId, readFileSync(path, "utf8")); } catch (error) { if (errorCode(error) === "ENOENT") return undefined; - return { version: 1, provider, accountId, generation: "", createdAt: 0, uncertain: true }; + return uncertainOAuthRefreshIntent(provider, accountId); } } -export function writeOAuthRefreshIntent(provider: string, accountId: string, generation: string, createdAt = Date.now(), flightId?: string): void { +export function writeOAuthRefreshIntent(provider: string, accountId: string, generation: string, createdAt = Date.now(), flightId?: string): OAuthRefreshIntent { const dir = getConfigDir(); if (!existsSync(dir)) mkdirSync(dir, { recursive: true, mode: 0o700 }); hardenConfigDir(); - const intent: OAuthRefreshIntent = { version: 1, provider, accountId, generation, createdAt, ...(flightId ? { flightId } : {}) }; - atomicWriteFile(getAuthRefreshIntentPath(provider, accountId), `${JSON.stringify(intent)}\n`); + const intent: OAuthRefreshIntent = { + version: 1, + provider, + accountId, + generation, + createdAt, + attemptId: randomUUID(), + ...(flightId ? { flightId } : {}), + }; + return withConfigMutationLockSync(() => { + if (readOAuthRefreshIntent(provider, accountId) !== undefined) { + throw new OAuthRefreshIntentIOError( + "write-intent", + new Error("An OAuth refresh intent already exists; refusing to overwrite its replay guard"), + ); + } + atomicWriteFile(getAuthRefreshIntentPath(provider, accountId), `${JSON.stringify(intent)}\n`); + return intent; + }); +} + +function sameOAuthRefreshIntentAttempt(current: OAuthRefreshIntent, expected: OAuthRefreshIntent): boolean { + return current.provider === expected.provider + && current.accountId === expected.accountId + && current.generation === expected.generation + && current.createdAt === expected.createdAt + && current.attemptId === expected.attemptId + && current.flightId === expected.flightId; } + +function exactOAuthRefreshIntentFile( + provider: string, + accountId: string, +): { intent: OAuthRefreshIntent; snapshot: OAuthRefreshIntentFileSnapshot } | undefined { + const path = getAuthRefreshIntentPath(provider, accountId); + try { + hardenConfigDir(); + hardenExistingSecret(path); + const file = snapshotOAuthRefreshIntentFile(path); + let intent: OAuthRefreshIntent; + try { intent = parseOAuthRefreshIntent(provider, accountId, file.raw); } + catch { intent = uncertainOAuthRefreshIntent(provider, accountId); } + return { intent, snapshot: file }; + } catch (error) { + if (errorCode(error) === "ENOENT") return undefined; + throw error; + } +} + +export function markOAuthRefreshIntentCleanupPending( + provider: string, + accountId: string, + expected: OAuthRefreshIntent, + cleanupPending: OAuthRefreshIntentCleanupPending, + hooks: OAuthRefreshIntentMutationHooks = {}, +): OAuthRefreshIntent | undefined { + return withConfigMutationLockSync(() => { + const path = getAuthRefreshIntentPath(provider, accountId); + const observed = exactOAuthRefreshIntentFile(provider, accountId); + const current = observed?.intent; + if ( + !observed + || !current + || expected.attemptId === undefined + || current.uncertain + || current.staleOwner + || !sameOAuthRefreshIntentAttempt(current, expected) + || (current.cleanupPending !== undefined && current.cleanupPending !== cleanupPending) + ) return undefined; + const marked: OAuthRefreshIntent = { ...current, cleanupPending }; + try { + atomicWriteFile(path, `${JSON.stringify(marked)}\n`, undefined, { + beforeRename: hooks.beforeMarkCommit, + validateBeforeRename: targetPath => { + let latest: OAuthRefreshIntentFileSnapshot; + try { latest = snapshotOAuthRefreshIntentFile(targetPath); } + catch (error) { + throw new OAuthRefreshIntentChangedError("OAuth refresh intent changed before marker commit", { cause: error }); + } + if (!sameOAuthRefreshIntentFileSnapshot(observed.snapshot, latest)) { + throw new OAuthRefreshIntentChangedError("OAuth refresh intent changed before marker commit"); + } + }, + }); + } catch (error) { + if (error instanceof OAuthRefreshIntentChangedError) return undefined; + throw error; + } + return marked; + }); +} + +export function clearOAuthRefreshIntentIfMatch( + provider: string, + accountId: string, + expected: OAuthRefreshIntent, + hooks: OAuthRefreshIntentMutationHooks = {}, +): boolean { + return withConfigMutationLockSync(() => { + const path = getAuthRefreshIntentPath(provider, accountId); + const observed = exactOAuthRefreshIntentFile(provider, accountId); + if (!observed) return true; + const current = observed.intent; + if ( + current.uncertain + || expected.uncertain + || current.attemptId === undefined + || expected.attemptId === undefined + || current.uncertain !== expected.uncertain + || current.staleOwner !== expected.staleOwner + || current.cleanupPending !== expected.cleanupPending + || !sameOAuthRefreshIntentAttempt(current, expected) + ) return false; + hooks.beforeClearRecheck?.(); + let latest: OAuthRefreshIntentFileSnapshot; + try { latest = snapshotOAuthRefreshIntentFile(path); } + catch (error) { + if (errorCode(error) === "ENOENT") return true; + throw error; + } + if (!sameOAuthRefreshIntentFileSnapshot(observed.snapshot, latest)) return false; + try { unlinkSync(path); return true; } + catch (error) { if (errorCode(error) === "ENOENT") return true; throw error; } + }); +} + export function markOAuthRefreshIntentStaleOwner(provider: string, accountId: string, generation: string, flightId: string): boolean { - const current = readOAuthRefreshIntent(provider, accountId); - if (current?.uncertain || current?.generation !== generation || current.flightId !== flightId) return false; - atomicWriteFile(getAuthRefreshIntentPath(provider, accountId), `${JSON.stringify({ ...current, staleOwner: true })}\n`); - return true; + return withConfigMutationLockSync(() => { + const current = readOAuthRefreshIntent(provider, accountId); + if ( + current?.uncertain + || current?.cleanupPending + || current?.generation !== generation + || current.flightId !== flightId + ) return false; + atomicWriteFile(getAuthRefreshIntentPath(provider, accountId), `${JSON.stringify({ ...current, staleOwner: true })}\n`); + return true; + }); } export function clearOAuthRefreshIntent(provider: string, accountId: string, generation: string): boolean { - const current = readOAuthRefreshIntent(provider, accountId); - if (!current || current.generation !== generation) return false; - try { unlinkSync(getAuthRefreshIntentPath(provider, accountId)); return true; } - catch (error) { if (errorCode(error) === "ENOENT") return false; throw error; } + return withConfigMutationLockSync(() => { + const current = readOAuthRefreshIntent(provider, accountId); + if (!current || current.generation !== generation) return false; + try { unlinkSync(getAuthRefreshIntentPath(provider, accountId)); return true; } + catch (error) { if (errorCode(error) === "ENOENT") return false; throw error; } + }); } export function credentialGeneration(cred: OAuthCredentials): string { return createHash("sha256").update(JSON.stringify([cred.refresh, cred.access, cred.expires])).digest("hex"); diff --git a/tests/oauth-refresh.test.ts b/tests/oauth-refresh.test.ts index a817e76159..d4bbd435a6 100644 --- a/tests/oauth-refresh.test.ts +++ b/tests/oauth-refresh.test.ts @@ -7,7 +7,19 @@ import { getValidAccessToken, getValidAccessTokenForAccount, OAuthLoginRequiredE import { RefreshIntentIOError, nousRefreshIntentBlocksReplay } from "../src/oauth/nous"; import * as nousModule from "../src/oauth/nous"; import { AnthropicTokenError } from "../src/oauth/anthropic"; -import { credentialGeneration, getAccountCredential, getAccountSet, getAuthRefreshIntentPath, getCredential, markAccountNeedsReauth, readOAuthRefreshIntent, saveCredential, writeOAuthRefreshIntent } from "../src/oauth/store"; +import { + OAuthRefreshIntentIOError, + credentialGeneration, + getAccountCredential, + getAccountSet, + getAuthRefreshIntentPath, + getCredential, + markAccountNeedsReauth, + markOAuthRefreshIntentCleanupPending, + readOAuthRefreshIntent, + saveCredential, + writeOAuthRefreshIntent, +} from "../src/oauth/store"; import * as storeModule from "../src/oauth/store"; import * as configModule from "../src/config"; @@ -547,14 +559,25 @@ describe("oauth refresh hardening", () => { const credential = getAccountCredential("anthropic", id)!; const generation = credentialGeneration(credential); const timeout = new AnthropicTokenError("timeout", undefined, undefined); - - await expect(refreshAnthropicAccountWithLock("anthropic", id, { + let refreshCalls = 0; + const def = { ...OAUTH_PROVIDERS.anthropic!, - refresh: async () => { throw timeout; }, - }, credential)).rejects.toBe(timeout); + refresh: async () => { + refreshCalls += 1; + throw timeout; + }, + }; + + await expect(refreshAnthropicAccountWithLock("anthropic", id, def, credential)).rejects.toBe(timeout); - expect(readOAuthRefreshIntent("anthropic", id)).toMatchObject({ generation }); + const pending = readOAuthRefreshIntent("anthropic", id); + expect(pending).toMatchObject({ generation }); + expect(pending?.cleanupPending).toBeUndefined(); expect(getAccountSet("anthropic")!.accounts.find(account => account.id === id)!.needsReauth).toBeUndefined(); + await expect(refreshAnthropicAccountWithLock("anthropic", id, def, credential)) + .rejects.toBeInstanceOf(OAuthLoginRequiredError); + expect(refreshCalls).toBe(1); + expect(readOAuthRefreshIntent("anthropic", id)).toEqual(pending); }); test("a pre-dispatch Anthropic abort clears its unconsumed refresh intent", async () => { @@ -575,15 +598,146 @@ describe("oauth refresh hardening", () => { expect(readOAuthRefreshIntent("anthropic", id)).toBeUndefined(); }); - test("intent cleanup failure preserves the original Anthropic HTTP error", async () => { + test("a failed pre-dispatch cleanup remains safely retryable", async () => { + await saveCredential("anthropic", { access: "old", refresh: "rt-old", expires: 1, accountId: "acct" }); + const id = getAccountSet("anthropic")!.activeAccountId; + const credential = getAccountCredential("anthropic", id)!; + const controller = new AbortController(); + const aborted = new Error("aborted before dispatch"); + controller.abort(aborted); + const realClear = storeModule.clearOAuthRefreshIntentIfMatch; + let clearCalls = 0; + const clearSpy = spyOn(storeModule, "clearOAuthRefreshIntentIfMatch").mockImplementation((provider, accountId, expected) => { + clearCalls += 1; + if (clearCalls === 1) throw new Error("intent unlink failed"); + return realClear(provider, accountId, expected); + }); + const warnSpy = spyOn(console, "warn").mockImplementation(() => {}); + try { + await expect(refreshAnthropicAccountWithLock("anthropic", id, { + ...OAUTH_PROVIDERS.anthropic!, + refresh: async () => { throw new Error("must not dispatch"); }, + }, credential, { signal: controller.signal })).rejects.toBe(aborted); + expect(readOAuthRefreshIntent("anthropic", id)?.cleanupPending).toBe("pre-dispatch"); + + let retryCalls = 0; + await expect(refreshAnthropicAccountWithLock("anthropic", id, { + ...OAUTH_PROVIDERS.anthropic!, + refresh: async () => { + retryCalls += 1; + return { access: "fresh", refresh: "rt-fresh", expires: Date.now() + 3_600_000 }; + }, + }, credential)).resolves.toBe("fresh"); + expect(retryCalls).toBe(1); + expect(readOAuthRefreshIntent("anthropic", id)).toBeUndefined(); + } finally { + warnSpy.mockRestore(); + clearSpy.mockRestore(); + } + }); + + test("a cleanup-marker persistence failure surfaces a typed error with the provider outcome", async () => { + await saveCredential("anthropic", { access: "old", refresh: "rt-old", expires: 1, accountId: "acct" }); + const id = getAccountSet("anthropic")!.activeAccountId; + const credential = getAccountCredential("anthropic", id)!; + const transient = new AnthropicTokenError("server", 503, undefined); + const markerFailure = new Error("intent marker write failed"); + const markerSpy = spyOn(storeModule, "markOAuthRefreshIntentCleanupPending").mockImplementation(() => { + throw markerFailure; + }); + try { + let rejection: unknown; + try { + await refreshAnthropicAccountWithLock("anthropic", id, { + ...OAUTH_PROVIDERS.anthropic!, + refresh: async () => { throw transient; }, + }, credential); + } catch (error) { + rejection = error; + } + expect(rejection).toBeInstanceOf(OAuthRefreshIntentIOError); + expect(rejection).toMatchObject({ + operation: "mark-cleanup-pending", + code: "OAUTH_REFRESH_INTENT_IO", + cause: markerFailure, + refreshError: transient, + }); + const pending = readOAuthRefreshIntent("anthropic", id); + expect(pending?.cleanupPending).toBeUndefined(); + expect(getAccountSet("anthropic")!.accounts.find(account => account.id === id)!.needsReauth).toBeUndefined(); + } finally { + markerSpy.mockRestore(); + } + }); + + test("a failed definitive-rejection cleanup is retried before the next Anthropic request", async () => { await saveCredential("anthropic", { access: "old", refresh: "rt-old", expires: 1, accountId: "acct" }); const id = getAccountSet("anthropic")!.activeAccountId; const credential = getAccountCredential("anthropic", id)!; const generation = credentialGeneration(credential); const transient = new AnthropicTokenError("server", 503, undefined); const clearFailure = new Error("intent unlink failed"); - const clearSpy = spyOn(storeModule, "clearOAuthRefreshIntent").mockImplementation(() => { - throw clearFailure; + const realClear = storeModule.clearOAuthRefreshIntentIfMatch; + const events: string[] = []; + let clearCalls = 0; + const clearSpy = spyOn(storeModule, "clearOAuthRefreshIntentIfMatch").mockImplementation((provider, accountId, expected) => { + clearCalls += 1; + if (clearCalls === 1) { + events.push("clear-fail"); + throw clearFailure; + } + events.push(expected.cleanupPending ? "clear-recovery" : "clear-success"); + return realClear(provider, accountId, expected); + }); + const warnSpy = spyOn(console, "warn").mockImplementation(() => {}); + try { + await expect(refreshAnthropicAccountWithLock("anthropic", id, { + ...OAUTH_PROVIDERS.anthropic!, + refresh: async () => { + events.push("provider-503"); + throw transient; + }, + }, credential)).rejects.toBe(transient); + + expect(readOAuthRefreshIntent("anthropic", id)).toMatchObject({ + generation, + cleanupPending: "definitive-rejection", + }); + expect(getAccountSet("anthropic")!.accounts.find(account => account.id === id)!.needsReauth).toBeUndefined(); + + let retryCalls = 0; + await expect(refreshAnthropicAccountWithLock("anthropic", id, { + ...OAUTH_PROVIDERS.anthropic!, + refresh: async () => { + events.push("provider-retry"); + retryCalls += 1; + return { access: "fresh", refresh: "rt-fresh", expires: Date.now() + 3_600_000 }; + }, + }, getAccountCredential("anthropic", id)!)).resolves.toBe("fresh"); + expect(retryCalls).toBe(1); + expect(clearCalls).toBe(3); + expect(events).toEqual([ + "provider-503", + "clear-fail", + "clear-recovery", + "provider-retry", + "clear-success", + ]); + expect(readOAuthRefreshIntent("anthropic", id)).toBeUndefined(); + } finally { + warnSpy.mockRestore(); + clearSpy.mockRestore(); + } + }); + + test("a persistently blocked cleanup fails operationally without replay or reauth", async () => { + await saveCredential("anthropic", { access: "old", refresh: "rt-old", expires: 1, accountId: "acct" }); + const id = getAccountSet("anthropic")!.activeAccountId; + const credential = getAccountCredential("anthropic", id)!; + const transient = new AnthropicTokenError("server", 503, undefined); + const cleanupFailure = new Error("intent unlink failed"); + const clearSpy = spyOn(storeModule, "clearOAuthRefreshIntentIfMatch").mockImplementation(() => { + throw cleanupFailure; }); const warnSpy = spyOn(console, "warn").mockImplementation(() => {}); try { @@ -591,10 +745,31 @@ describe("oauth refresh hardening", () => { ...OAUTH_PROVIDERS.anthropic!, refresh: async () => { throw transient; }, }, credential)).rejects.toBe(transient); + expect(readOAuthRefreshIntent("anthropic", id)?.cleanupPending).toBe("definitive-rejection"); - expect(clearSpy).toHaveBeenCalled(); - expect(readOAuthRefreshIntent("anthropic", id)).toMatchObject({ generation }); + let retryCalls = 0; + let rejection: unknown; + try { + await refreshAnthropicAccountWithLock("anthropic", id, { + ...OAUTH_PROVIDERS.anthropic!, + refresh: async () => { + retryCalls += 1; + return { access: "must-not-run", refresh: "must-not-run", expires: Date.now() + 3_600_000 }; + }, + }, credential); + } catch (error) { + rejection = error; + } + + expect(retryCalls).toBe(0); + expect(rejection).toBeInstanceOf(OAuthRefreshIntentIOError); + expect(rejection).toMatchObject({ + operation: "resume-cleanup", + code: "OAUTH_REFRESH_INTENT_IO", + cause: cleanupFailure, + }); expect(getAccountSet("anthropic")!.accounts.find(account => account.id === id)!.needsReauth).toBeUndefined(); + expect(readOAuthRefreshIntent("anthropic", id)?.cleanupPending).toBe("definitive-rejection"); } finally { warnSpy.mockRestore(); clearSpy.mockRestore(); @@ -607,25 +782,39 @@ describe("oauth refresh hardening", () => { const credential = getAccountCredential("anthropic", id)!; const generation = credentialGeneration(credential); const persistenceFailure = new Error("credential persistence failed"); + let refreshCalls = 0; + const def = { + ...OAUTH_PROVIDERS.anthropic!, + refresh: async () => { + refreshCalls += 1; + return { + access: "fresh", + refresh: "rt-fresh", + expires: Date.now() + 3_600_000, + }; + }, + }; const mergeSpy = spyOn(storeModule, "mergeAccountCredential").mockImplementation(async () => { throw persistenceFailure; }); + let pendingAfterFailure: ReturnType; try { - await expect(refreshAnthropicAccountWithLock("anthropic", id, { - ...OAUTH_PROVIDERS.anthropic!, - refresh: async () => ({ - access: "fresh", - refresh: "rt-fresh", - expires: Date.now() + 3_600_000, - }), - }, credential)).rejects.toBe(persistenceFailure); + await expect(refreshAnthropicAccountWithLock("anthropic", id, def, credential)) + .rejects.toBe(persistenceFailure); - expect(readOAuthRefreshIntent("anthropic", id)).toMatchObject({ generation }); + const pending = readOAuthRefreshIntent("anthropic", id); + pendingAfterFailure = pending; + expect(pending).toMatchObject({ generation }); + expect(pending?.cleanupPending).toBeUndefined(); expect(getAccountCredential("anthropic", id)).toEqual(credential); expect(getAccountSet("anthropic")!.accounts.find(account => account.id === id)!.needsReauth).toBeUndefined(); } finally { mergeSpy.mockRestore(); } + await expect(refreshAnthropicAccountWithLock("anthropic", id, def, credential)) + .rejects.toBeInstanceOf(OAuthLoginRequiredError); + expect(refreshCalls).toBe(1); + expect(readOAuthRefreshIntent("anthropic", id)).toEqual(pendingAfterFailure); }); test("post-persist intent cleanup failure does not turn Anthropic refresh success into failure", async () => { @@ -633,7 +822,7 @@ describe("oauth refresh hardening", () => { const id = getAccountSet("anthropic")!.activeAccountId; const credential = getAccountCredential("anthropic", id)!; const oldGeneration = credentialGeneration(credential); - const clearSpy = spyOn(storeModule, "clearOAuthRefreshIntent").mockImplementation(() => { + const clearSpy = spyOn(storeModule, "clearOAuthRefreshIntentIfMatch").mockImplementation(() => { throw new Error("intent unlink failed"); }); const warnSpy = spyOn(console, "warn").mockImplementation(() => {}); @@ -776,6 +965,135 @@ describe("oauth refresh hardening", () => { expect(readOAuthRefreshIntent("anthropic", id)).toEqual(legacy); expect(readOAuthRefreshIntent("anthropic", id)?.uncertain).toBeUndefined(); + let refreshCalls = 0; + await expect(refreshAnthropicAccountWithLock("anthropic", id, { + ...OAUTH_PROVIDERS.anthropic!, + refresh: async () => { + refreshCalls += 1; + return { access: "must-not-run", refresh: "must-not-run", expires: Date.now() + 3_600_000 }; + }, + }, getAccountCredential("anthropic", id)!)).rejects.toBeInstanceOf(OAuthLoginRequiredError); + expect(refreshCalls).toBe(0); + expect(readOAuthRefreshIntent("anthropic", id)).toEqual(legacy); + }); + + test("cleanup-pending CAS never rewrites a foreign same-generation attempt", async () => { + await saveCredential("anthropic", { access: "old", refresh: "rt-old", expires: 1, accountId: "acct" }); + const id = getAccountSet("anthropic")!.activeAccountId; + const generation = credentialGeneration(getAccountCredential("anthropic", id)!); + const intent = writeOAuthRefreshIntent("anthropic", id, generation, Date.now()); + + for (const foreign of [ + { ...intent, generation: "foreign-generation" }, + { ...intent, attemptId: "foreign-attempt" }, + { ...intent, flightId: "foreign-flight" }, + { ...intent, createdAt: intent.createdAt + 1 }, + ]) { + expect(markOAuthRefreshIntentCleanupPending( + "anthropic", + id, + foreign, + "definitive-rejection", + )).toBeUndefined(); + expect(storeModule.clearOAuthRefreshIntentIfMatch("anthropic", id, foreign)).toBe(false); + expect(readOAuthRefreshIntent("anthropic", id)).toEqual(intent); + } + }); + + test("a replaced obsolete intent blocks Anthropic redispatch", async () => { + await saveCredential("anthropic", { access: "old", refresh: "rt-old", expires: 1, accountId: "acct" }); + const id = getAccountSet("anthropic")!.activeAccountId; + const credential = getAccountCredential("anthropic", id)!; + const obsolete = writeOAuthRefreshIntent("anthropic", id, "0".repeat(64)); + const replacement = { ...obsolete, attemptId: "foreign-attempt" }; + writeFileSync(getAuthRefreshIntentPath("anthropic", id), `${JSON.stringify(replacement)}\n`); + const clearSpy = spyOn(storeModule, "clearOAuthRefreshIntentIfMatch").mockReturnValue(false); + let refreshCalls = 0; + try { + await expect(refreshAnthropicAccountWithLock("anthropic", id, { + ...OAUTH_PROVIDERS.anthropic!, + refresh: async () => { + refreshCalls += 1; + return { access: "must-not-run", refresh: "must-not-run", expires: Date.now() + 3_600_000 }; + }, + }, credential)).rejects.toBeInstanceOf(OAuthTokenRefreshStaleError); + expect(refreshCalls).toBe(0); + expect(readOAuthRefreshIntent("anthropic", id)).toEqual(replacement); + } finally { + clearSpy.mockRestore(); + } + }); + + test("cleanup-pending marker commit preserves an attempt replaced during comparison", async () => { + await saveCredential("anthropic", { access: "old", refresh: "rt-old", expires: 1, accountId: "acct" }); + const id = getAccountSet("anthropic")!.activeAccountId; + const path = getAuthRefreshIntentPath("anthropic", id); + const generation = credentialGeneration(getAccountCredential("anthropic", id)!); + const original = writeOAuthRefreshIntent("anthropic", id, generation, Date.now(), "original-flight"); + const replacement = { + ...original, + attemptId: "replacement-attempt", + flightId: "replacement-flight", + }; + + expect(markOAuthRefreshIntentCleanupPending( + "anthropic", + id, + original, + "definitive-rejection", + { beforeMarkCommit: () => writeFileSync(path, `${JSON.stringify(replacement)}\n`) }, + )).toBeUndefined(); + expect(readOAuthRefreshIntent("anthropic", id)).toEqual(replacement); + }); + + test("exact cleanup preserves an attempt replaced before unlink", async () => { + await saveCredential("anthropic", { access: "old", refresh: "rt-old", expires: 1, accountId: "acct" }); + const id = getAccountSet("anthropic")!.activeAccountId; + const path = getAuthRefreshIntentPath("anthropic", id); + const generation = credentialGeneration(getAccountCredential("anthropic", id)!); + const original = writeOAuthRefreshIntent("anthropic", id, generation, Date.now(), "original-flight"); + const replacement = { + ...original, + attemptId: "replacement-attempt", + flightId: "replacement-flight", + }; + + expect(storeModule.clearOAuthRefreshIntentIfMatch( + "anthropic", + id, + original, + { beforeClearRecheck: () => writeFileSync(path, `${JSON.stringify(replacement)}\n`) }, + )).toBe(false); + expect(readOAuthRefreshIntent("anthropic", id)).toEqual(replacement); + }); + + test("an invalid cleanup-pending marker fails closed as uncertain", async () => { + await saveCredential("anthropic", { access: "old", refresh: "rt-old", expires: 1, accountId: "acct" }); + const id = getAccountSet("anthropic")!.activeAccountId; + const intent = writeOAuthRefreshIntent( + "anthropic", + id, + credentialGeneration(getAccountCredential("anthropic", id)!), + ); + const { attemptId: _attemptId, ...withoutAttemptId } = intent; + for (const invalid of [ + { ...intent, cleanupPending: "unsafe-retry" }, + { ...withoutAttemptId, cleanupPending: "definitive-rejection" }, + { ...intent, cleanupPending: "definitive-rejection", uncertain: true }, + { ...intent, cleanupPending: "definitive-rejection", staleOwner: true }, + ]) { + writeFileSync(getAuthRefreshIntentPath("anthropic", id), `${JSON.stringify(invalid)}\n`); + expect(readOAuthRefreshIntent("anthropic", id)?.uncertain).toBe(true); + } + let refreshCalls = 0; + await expect(refreshAnthropicAccountWithLock("anthropic", id, { + ...OAUTH_PROVIDERS.anthropic!, + refresh: async () => { + refreshCalls += 1; + return { access: "must-not-run", refresh: "must-not-run", expires: Date.now() + 3_600_000 }; + }, + }, getAccountCredential("anthropic", id)!)).rejects.toBeInstanceOf(OAuthLoginRequiredError); + expect(refreshCalls).toBe(0); }); test("Anthropic never replays an outstanding oauth-source generation across re-entry", async () => { From f45fb1ca308603d312aae309e1faf7cd7e9f524d Mon Sep 17 00:00:00 2001 From: luvs01 Date: Sun, 30 Aug 2026 19:26:27 +0900 Subject: [PATCH 4/8] fix(oauth): keep post-commit intent cleanup from masking a durable credential When Anthropic adopts a newer Claude Code credential, mergeAccountCredential persists it before the observed refresh intent is cleaned up. Both exact-match and generation-based cleanup rethrow non-ENOENT unlink errors, so a locked or read-only intent file turned a successful adoption into a rejected refresh even though the credential was already durable on disk. Those two post-commit sites now use the existing best-effort helper, which logs and preserves the replay guard instead of throwing. The two remaining throwing calls are unchanged: their return values gate stale-flight handling before a refresh is dispatched, so their failures must still surface. The refresh-intent file also carried its own snapshot type and identity check that duplicated the OAuth file lock's snapshot/sameSnapshot pair, differing only in a field name. Both are compare-and-swap decisions on secret-adjacent files, so they now share one definition rather than two copies that can drift. Regression: a cleanup failure during disk-credential adoption still resolves with the committed token, attempts cleanup, performs no network refresh, and does not mark the account needsReauth. Reverting the first hunk fails it with a rejected promise. --- src/oauth/index.ts | 6 ++++-- src/oauth/store.ts | 40 ++++++++++--------------------------- tests/oauth-refresh.test.ts | 35 ++++++++++++++++++++++++++++++++ 3 files changed, 49 insertions(+), 32 deletions(-) diff --git a/src/oauth/index.ts b/src/oauth/index.ts index 1b3754dd21..679df81d9c 100644 --- a/src/oauth/index.ts +++ b/src/oauth/index.ts @@ -797,11 +797,13 @@ export async function refreshAnthropicAccountWithLock( afterPrePersistRead: deps.afterPrePersistRead, }); if (outcome.superseded) { - if (pendingIntent) clearObservedAnthropicRefreshIntent(provider, accountId, pendingIntent); + // The disk credential is already durable here, so cleanup is secondary: an unlink + // failure must not mask a committed credential by throwing over the return below. + if (pendingIntent) clearAnthropicRefreshIntentBestEffort(provider, accountId, pendingIntent); if (outcome.stored.expires > now() + REFRESH_SKEW_MS) return outcome.stored.access; throw new OAuthLoginRequiredError(provider); } - if (pendingIntent) clearObservedAnthropicRefreshIntent(provider, accountId, pendingIntent); + if (pendingIntent) clearAnthropicRefreshIntentBestEffort(provider, accountId, pendingIntent); return disk.access; } if (!pendingIntent?.uncertain && pendingIntent?.generation === generation) { diff --git a/src/oauth/store.ts b/src/oauth/store.ts index 842ee127fb..bc9e0e11c1 100644 --- a/src/oauth/store.ts +++ b/src/oauth/store.ts @@ -93,13 +93,10 @@ export class OAuthRefreshIntentIOError extends Error { } } -interface OAuthRefreshIntentFileSnapshot { - raw: string; - dev: number; - ino: number; - mtimeMs: number; - size: number; -} +// Both compare-and-swap sites in this file — the OAuth file lock and the refresh-intent +// file — must agree on what "the same file" means, so they share one snapshot shape and +// one identity check (`snapshot`/`sameSnapshot` below) rather than two copies that can drift. +type OAuthRefreshIntentFileSnapshot = LockSnapshot; export interface OAuthRefreshIntentMutationHooks { beforeMarkCommit?: () => void; @@ -112,23 +109,6 @@ function uncertainOAuthRefreshIntent(provider: string, accountId: string): OAuth return { version: 1, provider, accountId, generation: "", createdAt: 0, uncertain: true }; } -function snapshotOAuthRefreshIntentFile(path: string): OAuthRefreshIntentFileSnapshot { - const raw = readFileSync(path, "utf8"); - const stat = statSync(path); - return { raw, dev: stat.dev, ino: stat.ino, mtimeMs: stat.mtimeMs, size: stat.size }; -} - -function sameOAuthRefreshIntentFileSnapshot( - left: OAuthRefreshIntentFileSnapshot, - right: OAuthRefreshIntentFileSnapshot, -): boolean { - return left.raw === right.raw - && left.dev === right.dev - && left.ino === right.ino - && left.mtimeMs === right.mtimeMs - && left.size === right.size; -} - function parseOAuthRefreshIntent( provider: string, accountId: string, @@ -242,9 +222,9 @@ function exactOAuthRefreshIntentFile( try { hardenConfigDir(); hardenExistingSecret(path); - const file = snapshotOAuthRefreshIntentFile(path); + const file = snapshot(path); let intent: OAuthRefreshIntent; - try { intent = parseOAuthRefreshIntent(provider, accountId, file.raw); } + try { intent = parseOAuthRefreshIntent(provider, accountId, file.bytes); } catch { intent = uncertainOAuthRefreshIntent(provider, accountId); } return { intent, snapshot: file }; } catch (error) { @@ -279,11 +259,11 @@ export function markOAuthRefreshIntentCleanupPending( beforeRename: hooks.beforeMarkCommit, validateBeforeRename: targetPath => { let latest: OAuthRefreshIntentFileSnapshot; - try { latest = snapshotOAuthRefreshIntentFile(targetPath); } + try { latest = snapshot(targetPath); } catch (error) { throw new OAuthRefreshIntentChangedError("OAuth refresh intent changed before marker commit", { cause: error }); } - if (!sameOAuthRefreshIntentFileSnapshot(observed.snapshot, latest)) { + if (!sameSnapshot(observed.snapshot, latest)) { throw new OAuthRefreshIntentChangedError("OAuth refresh intent changed before marker commit"); } }, @@ -319,12 +299,12 @@ export function clearOAuthRefreshIntentIfMatch( ) return false; hooks.beforeClearRecheck?.(); let latest: OAuthRefreshIntentFileSnapshot; - try { latest = snapshotOAuthRefreshIntentFile(path); } + try { latest = snapshot(path); } catch (error) { if (errorCode(error) === "ENOENT") return true; throw error; } - if (!sameOAuthRefreshIntentFileSnapshot(observed.snapshot, latest)) return false; + if (!sameSnapshot(observed.snapshot, latest)) return false; try { unlinkSync(path); return true; } catch (error) { if (errorCode(error) === "ENOENT") return true; throw error; } }); diff --git a/tests/oauth-refresh.test.ts b/tests/oauth-refresh.test.ts index d4bbd435a6..cfbc2b64c8 100644 --- a/tests/oauth-refresh.test.ts +++ b/tests/oauth-refresh.test.ts @@ -1201,6 +1201,41 @@ describe("oauth refresh hardening", () => { expect(getCredential("anthropic")?.refresh).toBe("rt-new"); }); + /** + * The disk credential is committed before the observed intent is cleaned up. Cleanup is a + * secondary durability concern, so an unlink failure must not throw over the committed + * credential and turn a successful adoption into a caller-visible refresh error. + */ + test("a cleanup failure after adopting a disk credential does not mask the committed token", async () => { + await saveCredential("anthropic", { access: "old", refresh: "rt-old", expires: 1, source: "local-cli" }); + const id = getAccountSet("anthropic")!.activeAccountId; + const stored = getAccountCredential("anthropic", id)!; + writeOAuthRefreshIntent("anthropic", id, credentialGeneration(stored)); + expect(readOAuthRefreshIntent("anthropic", id)).toBeDefined(); + + seedClaudeCredentials("disk", "rt-new", Date.now() + 3600_000); + const mock = mockRefreshFetch([new Response("unexpected", { status: 500 })]); + + // The intent carries an attemptId, so cleanup runs through the exact-match clear. + // Fail it the way a locked or read-only file would. + let cleanupAttempts = 0; + const cleanupSpy = spyOn(storeModule, "clearOAuthRefreshIntentIfMatch").mockImplementation(() => { + cleanupAttempts += 1; + throw new OAuthRefreshIntentIOError("clear-intent", new Error("forced cleanup failure (EROFS)")); + }); + try { + await expect(getValidAccessToken("anthropic")).resolves.toBe("disk"); + } finally { + cleanupSpy.mockRestore(); + } + + // The adoption committed and no network refresh was attempted; only the guard survives. + expect(cleanupAttempts).toBeGreaterThan(0); + expect(mock.count()).toBe(0); + expect(getCredential("anthropic")?.refresh).toBe("rt-new"); + expect(getAccountSet("anthropic")!.accounts[0]!.needsReauth).toBeUndefined(); + }); + test("marked Anthropic local-cli account lazily recovers only from a newer disk generation", async () => { await saveCredential("anthropic", { access: "old", refresh: "rt-old", expires: 1, source: "local-cli" }); const id = getAccountSet("anthropic")!.activeAccountId; From eb4639b658c37af8b5173363469973d897067d6a Mon Sep 17 00:00:00 2001 From: luvs01 Date: Sun, 30 Aug 2026 23:40:17 +0900 Subject: [PATCH 5/8] test(oauth): use declared cleanup operation --- tests/oauth-refresh.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/oauth-refresh.test.ts b/tests/oauth-refresh.test.ts index cfbc2b64c8..ef4991d836 100644 --- a/tests/oauth-refresh.test.ts +++ b/tests/oauth-refresh.test.ts @@ -1221,7 +1221,7 @@ describe("oauth refresh hardening", () => { let cleanupAttempts = 0; const cleanupSpy = spyOn(storeModule, "clearOAuthRefreshIntentIfMatch").mockImplementation(() => { cleanupAttempts += 1; - throw new OAuthRefreshIntentIOError("clear-intent", new Error("forced cleanup failure (EROFS)")); + throw new OAuthRefreshIntentIOError("clear-cleanup-pending", new Error("forced cleanup failure (EROFS)")); }); try { await expect(getValidAccessToken("anthropic")).resolves.toBe("disk"); From 5ca217211f8c9baa8644a09c6fa482a28c29b399 Mon Sep 17 00:00:00 2001 From: luvs01 Date: Mon, 31 Aug 2026 02:38:24 +0900 Subject: [PATCH 6/8] test(oauth): preserve refresh-intent spy hooks --- tests/oauth-refresh.test.ts | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/tests/oauth-refresh.test.ts b/tests/oauth-refresh.test.ts index ef4991d836..0699983195 100644 --- a/tests/oauth-refresh.test.ts +++ b/tests/oauth-refresh.test.ts @@ -607,10 +607,10 @@ describe("oauth refresh hardening", () => { controller.abort(aborted); const realClear = storeModule.clearOAuthRefreshIntentIfMatch; let clearCalls = 0; - const clearSpy = spyOn(storeModule, "clearOAuthRefreshIntentIfMatch").mockImplementation((provider, accountId, expected) => { + const clearSpy = spyOn(storeModule, "clearOAuthRefreshIntentIfMatch").mockImplementation((...args) => { clearCalls += 1; if (clearCalls === 1) throw new Error("intent unlink failed"); - return realClear(provider, accountId, expected); + return realClear(...args); }); const warnSpy = spyOn(console, "warn").mockImplementation(() => {}); try { @@ -680,14 +680,14 @@ describe("oauth refresh hardening", () => { const realClear = storeModule.clearOAuthRefreshIntentIfMatch; const events: string[] = []; let clearCalls = 0; - const clearSpy = spyOn(storeModule, "clearOAuthRefreshIntentIfMatch").mockImplementation((provider, accountId, expected) => { + const clearSpy = spyOn(storeModule, "clearOAuthRefreshIntentIfMatch").mockImplementation((...args) => { clearCalls += 1; if (clearCalls === 1) { events.push("clear-fail"); throw clearFailure; } - events.push(expected.cleanupPending ? "clear-recovery" : "clear-success"); - return realClear(provider, accountId, expected); + events.push(args[2].cleanupPending ? "clear-recovery" : "clear-success"); + return realClear(...args); }); const warnSpy = spyOn(console, "warn").mockImplementation(() => {}); try { From 66780c655182beffb250fa26425cdd01d47066e1 Mon Sep 17 00:00:00 2001 From: luvs01 Date: Mon, 31 Aug 2026 03:09:41 +0900 Subject: [PATCH 7/8] fix(oauth): adopt newer disk credentials before cleanup --- src/oauth/index.ts | 8 ++++---- tests/oauth-refresh.test.ts | 10 ++++++++-- 2 files changed, 12 insertions(+), 6 deletions(-) diff --git a/src/oauth/index.ts b/src/oauth/index.ts index 679df81d9c..f79578f1b4 100644 --- a/src/oauth/index.ts +++ b/src/oauth/index.ts @@ -786,10 +786,6 @@ export async function refreshAnthropicAccountWithLock( const account = getAccountSet(provider)?.accounts.find(candidate => candidate.id === accountId); const generation = credentialGeneration(stored); let pendingIntent = readOAuthRefreshIntent(provider, accountId); - if (pendingIntent?.cleanupPending && pendingIntent.generation === generation) { - resumeAnthropicRefreshIntentCleanup(provider, accountId, pendingIntent); - pendingIntent = undefined; - } const disk = newerClaudeCredential(stored, now()); if (disk) { const outcome = await mergeAccountCredential(provider, accountId, disk, { @@ -806,6 +802,10 @@ export async function refreshAnthropicAccountWithLock( if (pendingIntent) clearAnthropicRefreshIntentBestEffort(provider, accountId, pendingIntent); return disk.access; } + if (pendingIntent?.cleanupPending && pendingIntent.generation === generation) { + resumeAnthropicRefreshIntentCleanup(provider, accountId, pendingIntent); + pendingIntent = undefined; + } if (!pendingIntent?.uncertain && pendingIntent?.generation === generation) { if (pendingIntent.staleOwner) throw new OAuthTokenRefreshStaleError(); if (deps.replacedStaleFlight && pendingIntent.flightId === deps.replacedStaleFlight.flightId) { diff --git a/tests/oauth-refresh.test.ts b/tests/oauth-refresh.test.ts index 0699983195..279a84c40c 100644 --- a/tests/oauth-refresh.test.ts +++ b/tests/oauth-refresh.test.ts @@ -1210,8 +1210,13 @@ describe("oauth refresh hardening", () => { await saveCredential("anthropic", { access: "old", refresh: "rt-old", expires: 1, source: "local-cli" }); const id = getAccountSet("anthropic")!.activeAccountId; const stored = getAccountCredential("anthropic", id)!; - writeOAuthRefreshIntent("anthropic", id, credentialGeneration(stored)); - expect(readOAuthRefreshIntent("anthropic", id)).toBeDefined(); + const pending = writeOAuthRefreshIntent("anthropic", id, credentialGeneration(stored)); + expect(markOAuthRefreshIntentCleanupPending( + "anthropic", + id, + pending, + "definitive-rejection", + )).toMatchObject({ cleanupPending: "definitive-rejection" }); seedClaudeCredentials("disk", "rt-new", Date.now() + 3600_000); const mock = mockRefreshFetch([new Response("unexpected", { status: 500 })]); @@ -1234,6 +1239,7 @@ describe("oauth refresh hardening", () => { expect(mock.count()).toBe(0); expect(getCredential("anthropic")?.refresh).toBe("rt-new"); expect(getAccountSet("anthropic")!.accounts[0]!.needsReauth).toBeUndefined(); + expect(readOAuthRefreshIntent("anthropic", id)?.cleanupPending).toBe("definitive-rejection"); }); test("marked Anthropic local-cli account lazily recovers only from a newer disk generation", async () => { From 60c9d7604afcea41d203364b4216cda9d56788e3 Mon Sep 17 00:00:00 2001 From: luvs01 Date: Mon, 31 Aug 2026 13:35:35 +0900 Subject: [PATCH 8/8] fix(oauth): retry refresh intent marker contention --- src/oauth/index.ts | 55 +++++++++++++------ tests/oauth-refresh.test.ts | 104 +++++++++++++++++++++++++++++++++++- 2 files changed, 141 insertions(+), 18 deletions(-) diff --git a/src/oauth/index.ts b/src/oauth/index.ts index f79578f1b4..c4a682ee3f 100644 --- a/src/oauth/index.ts +++ b/src/oauth/index.ts @@ -1,7 +1,7 @@ import type { KiroOAuthMetadata, OAuthController, OAuthCredentials } from "./types"; import { parseCallbackInput } from "./callback-server"; import type { OcxConfig, OcxProviderConfig, RefreshPolicy } from "../types"; -import { loadConfig, resolveEnvValue, saveConfig } from "../config"; +import { ConfigMutationLockError, loadConfig, resolveEnvValue, saveConfig } from "../config"; import { maskEmail } from "../lib/privacy"; import { KiroTokenRefreshError, environmentKiroRoutingMetadata, loginKiro, refreshKiroToken, settleKiroLoginTransaction } from "./kiro"; import { @@ -628,27 +628,48 @@ function clearAnthropicRefreshIntentBestEffort( } } -function clearAnthropicRefreshIntentForKnownFailure( +const ANTHROPIC_INTENT_MARK_RETRY_DELAYS_MS = [10, 25, 50] as const; + +function isConfigMutationLockContention(error: unknown): boolean { + if (!(error instanceof ConfigMutationLockError)) return false; + const cause = error.cause; + const code = cause && typeof cause === "object" && "code" in cause + ? String((cause as { code?: unknown }).code) + : ""; + return code === "SQLITE_BUSY" || code === "SQLITE_LOCKED"; +} + +async function clearAnthropicRefreshIntentForKnownFailure( provider: string, accountId: string, expected: OAuthRefreshIntent, cleanupPending: OAuthRefreshIntentCleanupPending, refreshError: unknown, -): boolean { +): Promise { let marked: OAuthRefreshIntent | undefined; - try { - marked = markOAuthRefreshIntentCleanupPending( - provider, - accountId, - expected, - cleanupPending, - ); - } catch (cause) { - throw new OAuthRefreshIntentIOError( - "mark-cleanup-pending", - cause, - refreshError, - ); + for (let attempt = 0; attempt <= ANTHROPIC_INTENT_MARK_RETRY_DELAYS_MS.length; attempt += 1) { + try { + marked = markOAuthRefreshIntentCleanupPending( + provider, + accountId, + expected, + cleanupPending, + ); + break; + } catch (cause) { + const retryDelay = ANTHROPIC_INTENT_MARK_RETRY_DELAYS_MS[attempt]; + if (!isConfigMutationLockContention(cause) || retryDelay === undefined) { + throw new OAuthRefreshIntentIOError( + "mark-cleanup-pending", + cause, + refreshError, + ); + } + // The provider has definitively answered, so caller cancellation no longer changes the + // settlement obligation. Yield briefly while retaining the per-account refresh lock, then + // rerun the existing compare-and-swap marker against current disk state. + await Bun.sleep(retryDelay); + } } if (!marked) { throw new OAuthRefreshIntentIOError( @@ -872,7 +893,7 @@ export async function refreshAnthropicAccountWithLock( // rotated the token, and replaying it could trip refresh-token-reuse revocation. // Those outcomes keep the intent so the guard still refuses a blind replay. if ((!refreshMayHaveReachedProvider || definitivelyAnswered(error)) && attemptIntent) { - clearAnthropicRefreshIntentForKnownFailure( + await clearAnthropicRefreshIntentForKnownFailure( provider, accountId, attemptIntent, diff --git a/tests/oauth-refresh.test.ts b/tests/oauth-refresh.test.ts index 279a84c40c..3ba3e57ce8 100644 --- a/tests/oauth-refresh.test.ts +++ b/tests/oauth-refresh.test.ts @@ -641,8 +641,13 @@ describe("oauth refresh hardening", () => { const id = getAccountSet("anthropic")!.activeAccountId; const credential = getAccountCredential("anthropic", id)!; const transient = new AnthropicTokenError("server", 503, undefined); - const markerFailure = new Error("intent marker write failed"); + const markerFailure = new configModule.ConfigMutationLockError( + "Could not acquire config mutation transaction", + { cause: Object.assign(new Error("intent marker write failed"), { code: "EACCES" }) }, + ); + let markerCalls = 0; const markerSpy = spyOn(storeModule, "markOAuthRefreshIntentCleanupPending").mockImplementation(() => { + markerCalls += 1; throw markerFailure; }); try { @@ -662,6 +667,7 @@ describe("oauth refresh hardening", () => { cause: markerFailure, refreshError: transient, }); + expect(markerCalls).toBe(1); const pending = readOAuthRefreshIntent("anthropic", id); expect(pending?.cleanupPending).toBeUndefined(); expect(getAccountSet("anthropic")!.accounts.find(account => account.id === id)!.needsReauth).toBeUndefined(); @@ -670,6 +676,102 @@ describe("oauth refresh hardening", () => { } }); + test("transient cleanup-marker lock contention settles a retryable rejection without reauth", async () => { + await saveCredential("anthropic", { access: "old", refresh: "rt-old", expires: 1, accountId: "acct" }); + const id = getAccountSet("anthropic")!.activeAccountId; + const credential = getAccountCredential("anthropic", id)!; + const transient = new AnthropicTokenError("server", 503, undefined); + const realMark = storeModule.markOAuthRefreshIntentCleanupPending; + let markerCalls = 0; + const markerSpy = spyOn(storeModule, "markOAuthRefreshIntentCleanupPending").mockImplementation((...args) => { + markerCalls += 1; + if (markerCalls <= 2) { + throw new configModule.ConfigMutationLockError( + "Config mutation already in progress", + { cause: Object.assign(new Error("database is locked"), { code: "SQLITE_BUSY" }) }, + ); + } + return realMark(...args); + }); + try { + let providerCalls = 0; + await expect(refreshAnthropicAccountWithLock("anthropic", id, { + ...OAUTH_PROVIDERS.anthropic!, + refresh: async () => { + providerCalls += 1; + throw transient; + }, + }, credential)).rejects.toBe(transient); + + expect(providerCalls).toBe(1); + expect(markerCalls).toBe(3); + expect(readOAuthRefreshIntent("anthropic", id)).toBeUndefined(); + expect(getAccountSet("anthropic")!.accounts.find(account => account.id === id)!.needsReauth).toBeUndefined(); + + await expect(refreshAnthropicAccountWithLock("anthropic", id, { + ...OAUTH_PROVIDERS.anthropic!, + refresh: async () => ({ + access: "fresh", + refresh: "rt-fresh", + expires: Date.now() + 3_600_000, + }), + }, getAccountCredential("anthropic", id)!)).resolves.toBe("fresh"); + expect(getAccountCredential("anthropic", id)?.access).toBe("fresh"); + expect(readOAuthRefreshIntent("anthropic", id)).toBeUndefined(); + } finally { + markerSpy.mockRestore(); + } + }); + + test("persistent cleanup-marker lock contention stays bounded and preserves the replay guard", async () => { + await saveCredential("anthropic", { access: "old", refresh: "rt-old", expires: 1, accountId: "acct" }); + const id = getAccountSet("anthropic")!.activeAccountId; + const credential = getAccountCredential("anthropic", id)!; + const generation = credentialGeneration(credential); + const transient = new AnthropicTokenError("server", 503, undefined); + let markerCalls = 0; + let lastBusy: configModule.ConfigMutationLockError | undefined; + const markerSpy = spyOn(storeModule, "markOAuthRefreshIntentCleanupPending").mockImplementation(() => { + markerCalls += 1; + lastBusy = new configModule.ConfigMutationLockError( + "Config mutation already in progress", + { cause: Object.assign(new Error("database table is locked"), { code: "SQLITE_LOCKED" }) }, + ); + throw lastBusy; + }); + try { + let providerCalls = 0; + let rejection: unknown; + try { + await refreshAnthropicAccountWithLock("anthropic", id, { + ...OAUTH_PROVIDERS.anthropic!, + refresh: async () => { + providerCalls += 1; + throw transient; + }, + }, credential); + } catch (error) { + rejection = error; + } + + expect(providerCalls).toBe(1); + expect(markerCalls).toBe(4); + expect(rejection).toBeInstanceOf(OAuthRefreshIntentIOError); + expect(rejection).toMatchObject({ + operation: "mark-cleanup-pending", + code: "OAUTH_REFRESH_INTENT_IO", + cause: lastBusy, + refreshError: transient, + }); + expect(getAccountCredential("anthropic", id)).toEqual(credential); + expect(readOAuthRefreshIntent("anthropic", id)).toMatchObject({ generation }); + expect(readOAuthRefreshIntent("anthropic", id)?.cleanupPending).toBeUndefined(); + expect(getAccountSet("anthropic")!.accounts.find(account => account.id === id)!.needsReauth).toBeUndefined(); + } finally { + markerSpy.mockRestore(); + } + }); + test("a failed definitive-rejection cleanup is retried before the next Anthropic request", async () => { await saveCredential("anthropic", { access: "old", refresh: "rt-old", expires: 1, accountId: "acct" }); const id = getAccountSet("anthropic")!.activeAccountId;