Skip to content

Commit 60c9d76

Browse files
committed
fix(oauth): retry refresh intent marker contention
1 parent 66780c6 commit 60c9d76

2 files changed

Lines changed: 141 additions & 18 deletions

File tree

src/oauth/index.ts

Lines changed: 38 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import type { KiroOAuthMetadata, OAuthController, OAuthCredentials } from "./types";
22
import { parseCallbackInput } from "./callback-server";
33
import type { OcxConfig, OcxProviderConfig, RefreshPolicy } from "../types";
4-
import { loadConfig, resolveEnvValue, saveConfig } from "../config";
4+
import { ConfigMutationLockError, loadConfig, resolveEnvValue, saveConfig } from "../config";
55
import { maskEmail } from "../lib/privacy";
66
import { KiroTokenRefreshError, environmentKiroRoutingMetadata, loginKiro, refreshKiroToken, settleKiroLoginTransaction } from "./kiro";
77
import {
@@ -628,27 +628,48 @@ function clearAnthropicRefreshIntentBestEffort(
628628
}
629629
}
630630

631-
function clearAnthropicRefreshIntentForKnownFailure(
631+
const ANTHROPIC_INTENT_MARK_RETRY_DELAYS_MS = [10, 25, 50] as const;
632+
633+
function isConfigMutationLockContention(error: unknown): boolean {
634+
if (!(error instanceof ConfigMutationLockError)) return false;
635+
const cause = error.cause;
636+
const code = cause && typeof cause === "object" && "code" in cause
637+
? String((cause as { code?: unknown }).code)
638+
: "";
639+
return code === "SQLITE_BUSY" || code === "SQLITE_LOCKED";
640+
}
641+
642+
async function clearAnthropicRefreshIntentForKnownFailure(
632643
provider: string,
633644
accountId: string,
634645
expected: OAuthRefreshIntent,
635646
cleanupPending: OAuthRefreshIntentCleanupPending,
636647
refreshError: unknown,
637-
): boolean {
648+
): Promise<boolean> {
638649
let marked: OAuthRefreshIntent | undefined;
639-
try {
640-
marked = markOAuthRefreshIntentCleanupPending(
641-
provider,
642-
accountId,
643-
expected,
644-
cleanupPending,
645-
);
646-
} catch (cause) {
647-
throw new OAuthRefreshIntentIOError(
648-
"mark-cleanup-pending",
649-
cause,
650-
refreshError,
651-
);
650+
for (let attempt = 0; attempt <= ANTHROPIC_INTENT_MARK_RETRY_DELAYS_MS.length; attempt += 1) {
651+
try {
652+
marked = markOAuthRefreshIntentCleanupPending(
653+
provider,
654+
accountId,
655+
expected,
656+
cleanupPending,
657+
);
658+
break;
659+
} catch (cause) {
660+
const retryDelay = ANTHROPIC_INTENT_MARK_RETRY_DELAYS_MS[attempt];
661+
if (!isConfigMutationLockContention(cause) || retryDelay === undefined) {
662+
throw new OAuthRefreshIntentIOError(
663+
"mark-cleanup-pending",
664+
cause,
665+
refreshError,
666+
);
667+
}
668+
// The provider has definitively answered, so caller cancellation no longer changes the
669+
// settlement obligation. Yield briefly while retaining the per-account refresh lock, then
670+
// rerun the existing compare-and-swap marker against current disk state.
671+
await Bun.sleep(retryDelay);
672+
}
652673
}
653674
if (!marked) {
654675
throw new OAuthRefreshIntentIOError(
@@ -872,7 +893,7 @@ export async function refreshAnthropicAccountWithLock(
872893
// rotated the token, and replaying it could trip refresh-token-reuse revocation.
873894
// Those outcomes keep the intent so the guard still refuses a blind replay.
874895
if ((!refreshMayHaveReachedProvider || definitivelyAnswered(error)) && attemptIntent) {
875-
clearAnthropicRefreshIntentForKnownFailure(
896+
await clearAnthropicRefreshIntentForKnownFailure(
876897
provider,
877898
accountId,
878899
attemptIntent,

tests/oauth-refresh.test.ts

Lines changed: 103 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -641,8 +641,13 @@ describe("oauth refresh hardening", () => {
641641
const id = getAccountSet("anthropic")!.activeAccountId;
642642
const credential = getAccountCredential("anthropic", id)!;
643643
const transient = new AnthropicTokenError("server", 503, undefined);
644-
const markerFailure = new Error("intent marker write failed");
644+
const markerFailure = new configModule.ConfigMutationLockError(
645+
"Could not acquire config mutation transaction",
646+
{ cause: Object.assign(new Error("intent marker write failed"), { code: "EACCES" }) },
647+
);
648+
let markerCalls = 0;
645649
const markerSpy = spyOn(storeModule, "markOAuthRefreshIntentCleanupPending").mockImplementation(() => {
650+
markerCalls += 1;
646651
throw markerFailure;
647652
});
648653
try {
@@ -662,6 +667,7 @@ describe("oauth refresh hardening", () => {
662667
cause: markerFailure,
663668
refreshError: transient,
664669
});
670+
expect(markerCalls).toBe(1);
665671
const pending = readOAuthRefreshIntent("anthropic", id);
666672
expect(pending?.cleanupPending).toBeUndefined();
667673
expect(getAccountSet("anthropic")!.accounts.find(account => account.id === id)!.needsReauth).toBeUndefined();
@@ -670,6 +676,102 @@ describe("oauth refresh hardening", () => {
670676
}
671677
});
672678

679+
test("transient cleanup-marker lock contention settles a retryable rejection without reauth", async () => {
680+
await saveCredential("anthropic", { access: "old", refresh: "rt-old", expires: 1, accountId: "acct" });
681+
const id = getAccountSet("anthropic")!.activeAccountId;
682+
const credential = getAccountCredential("anthropic", id)!;
683+
const transient = new AnthropicTokenError("server", 503, undefined);
684+
const realMark = storeModule.markOAuthRefreshIntentCleanupPending;
685+
let markerCalls = 0;
686+
const markerSpy = spyOn(storeModule, "markOAuthRefreshIntentCleanupPending").mockImplementation((...args) => {
687+
markerCalls += 1;
688+
if (markerCalls <= 2) {
689+
throw new configModule.ConfigMutationLockError(
690+
"Config mutation already in progress",
691+
{ cause: Object.assign(new Error("database is locked"), { code: "SQLITE_BUSY" }) },
692+
);
693+
}
694+
return realMark(...args);
695+
});
696+
try {
697+
let providerCalls = 0;
698+
await expect(refreshAnthropicAccountWithLock("anthropic", id, {
699+
...OAUTH_PROVIDERS.anthropic!,
700+
refresh: async () => {
701+
providerCalls += 1;
702+
throw transient;
703+
},
704+
}, credential)).rejects.toBe(transient);
705+
706+
expect(providerCalls).toBe(1);
707+
expect(markerCalls).toBe(3);
708+
expect(readOAuthRefreshIntent("anthropic", id)).toBeUndefined();
709+
expect(getAccountSet("anthropic")!.accounts.find(account => account.id === id)!.needsReauth).toBeUndefined();
710+
711+
await expect(refreshAnthropicAccountWithLock("anthropic", id, {
712+
...OAUTH_PROVIDERS.anthropic!,
713+
refresh: async () => ({
714+
access: "fresh",
715+
refresh: "rt-fresh",
716+
expires: Date.now() + 3_600_000,
717+
}),
718+
}, getAccountCredential("anthropic", id)!)).resolves.toBe("fresh");
719+
expect(getAccountCredential("anthropic", id)?.access).toBe("fresh");
720+
expect(readOAuthRefreshIntent("anthropic", id)).toBeUndefined();
721+
} finally {
722+
markerSpy.mockRestore();
723+
}
724+
});
725+
726+
test("persistent cleanup-marker lock contention stays bounded and preserves the replay guard", async () => {
727+
await saveCredential("anthropic", { access: "old", refresh: "rt-old", expires: 1, accountId: "acct" });
728+
const id = getAccountSet("anthropic")!.activeAccountId;
729+
const credential = getAccountCredential("anthropic", id)!;
730+
const generation = credentialGeneration(credential);
731+
const transient = new AnthropicTokenError("server", 503, undefined);
732+
let markerCalls = 0;
733+
let lastBusy: configModule.ConfigMutationLockError | undefined;
734+
const markerSpy = spyOn(storeModule, "markOAuthRefreshIntentCleanupPending").mockImplementation(() => {
735+
markerCalls += 1;
736+
lastBusy = new configModule.ConfigMutationLockError(
737+
"Config mutation already in progress",
738+
{ cause: Object.assign(new Error("database table is locked"), { code: "SQLITE_LOCKED" }) },
739+
);
740+
throw lastBusy;
741+
});
742+
try {
743+
let providerCalls = 0;
744+
let rejection: unknown;
745+
try {
746+
await refreshAnthropicAccountWithLock("anthropic", id, {
747+
...OAUTH_PROVIDERS.anthropic!,
748+
refresh: async () => {
749+
providerCalls += 1;
750+
throw transient;
751+
},
752+
}, credential);
753+
} catch (error) {
754+
rejection = error;
755+
}
756+
757+
expect(providerCalls).toBe(1);
758+
expect(markerCalls).toBe(4);
759+
expect(rejection).toBeInstanceOf(OAuthRefreshIntentIOError);
760+
expect(rejection).toMatchObject({
761+
operation: "mark-cleanup-pending",
762+
code: "OAUTH_REFRESH_INTENT_IO",
763+
cause: lastBusy,
764+
refreshError: transient,
765+
});
766+
expect(getAccountCredential("anthropic", id)).toEqual(credential);
767+
expect(readOAuthRefreshIntent("anthropic", id)).toMatchObject({ generation });
768+
expect(readOAuthRefreshIntent("anthropic", id)?.cleanupPending).toBeUndefined();
769+
expect(getAccountSet("anthropic")!.accounts.find(account => account.id === id)!.needsReauth).toBeUndefined();
770+
} finally {
771+
markerSpy.mockRestore();
772+
}
773+
});
774+
673775
test("a failed definitive-rejection cleanup is retried before the next Anthropic request", async () => {
674776
await saveCredential("anthropic", { access: "old", refresh: "rt-old", expires: 1, accountId: "acct" });
675777
const id = getAccountSet("anthropic")!.activeAccountId;

0 commit comments

Comments
 (0)