Skip to content

Commit 52c7c8b

Browse files
committed
fix: preserve live refresh state and terminal auth setter failures
1 parent fe363b1 commit 52c7c8b

6 files changed

Lines changed: 142 additions & 6 deletions

File tree

lib/accounts.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -804,9 +804,9 @@ export class AccountManager {
804804
const nextAccountId = extractAccountId(auth.access)?.trim() || undefined;
805805
const nextEmail = sanitizeEmail(extractAccountEmail(auth.access));
806806
try {
807-
return await withAccountStorageTransaction(async (current, persist) => {
807+
return await withAccountStorageTransaction(async (_current, persist) => {
808808
const nextStorage = structuredClone(
809-
current ?? this.buildStorageSnapshot(),
809+
this.buildStorageSnapshot(),
810810
) as AccountStorageV3;
811811
const storageIndex = findAccountIndexByIdentity(
812812
nextStorage.accounts,

lib/refresh-guardian.ts

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -124,7 +124,25 @@ export class RefreshGuardian {
124124
multiAccount: true,
125125
};
126126
try {
127-
await manager.commitRefreshedAuth(sourceAccount, refreshedAuth);
127+
const committedAccount = await manager.commitRefreshedAuth(
128+
sourceAccount,
129+
refreshedAuth,
130+
);
131+
if (!committedAccount) {
132+
const account =
133+
manager.getAccountByIdentity(sourceAccount, refreshedAuth) ??
134+
manager.getAccountByIdentity(sourceAccount);
135+
if (account) {
136+
manager.markAccountCoolingDown(
137+
account,
138+
this.bufferMs,
139+
"network-error",
140+
);
141+
}
142+
this.stats.failed += 1;
143+
this.stats.networkFailed += 1;
144+
return !!account;
145+
}
128146
} catch (error) {
129147
log.warn("Refresh guardian commit failed", {
130148
sourceIndex: sourceAccount.index,

lib/request/fetch-helpers.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -461,7 +461,9 @@ export async function refreshAndUpdateToken(
461461
): Promise<Auth> {
462462
const authSetter = (client as Partial<CodexAuthSetter>).auth;
463463
if (!authSetter || typeof authSetter.set !== "function") {
464-
throw new CodexAuthError(ERROR_MESSAGES.TOKEN_REFRESH_FAILED, { retryable: true });
464+
throw new CodexAuthError(ERROR_MESSAGES.TOKEN_REFRESH_FAILED, {
465+
retryable: false,
466+
});
465467
}
466468

467469
const refreshToken = currentAuth.type === "oauth" ? currentAuth.refresh : "";

test/accounts.test.ts

Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1159,6 +1159,76 @@ describe("AccountManager", () => {
11591159
expect(account.consecutiveAuthFailures).toBe(0);
11601160
});
11611161

1162+
it("preserves unsaved live pool state when persisting refreshed auth", async () => {
1163+
const { withAccountStorageTransaction } = await import("../lib/storage.js");
1164+
const mockWithAccountStorageTransaction = vi.mocked(
1165+
withAccountStorageTransaction,
1166+
);
1167+
const now = Date.now();
1168+
const stored = {
1169+
version: 3 as const,
1170+
activeIndex: 0,
1171+
activeIndexByFamily: {
1172+
codex: 0,
1173+
},
1174+
accounts: [
1175+
{
1176+
refreshToken: "old-refresh-a",
1177+
accessToken: "old-access-a",
1178+
expiresAt: now,
1179+
addedAt: now,
1180+
lastUsed: now,
1181+
email: "a@example.com",
1182+
},
1183+
{
1184+
refreshToken: "old-refresh-b",
1185+
accessToken: "old-access-b",
1186+
expiresAt: now,
1187+
addedAt: now,
1188+
lastUsed: now,
1189+
email: "b@example.com",
1190+
},
1191+
],
1192+
};
1193+
const manager = new AccountManager(undefined, stored as any);
1194+
const accountA = manager.getAccountByIndex(0)!;
1195+
const accountB = manager.getAccountByIndex(1)!;
1196+
manager.markSwitched(accountB, "rotation", "codex");
1197+
manager.markRateLimited(accountB, 45_000, "codex");
1198+
manager.markAccountCoolingDown(accountB, 30_000, "network-error");
1199+
1200+
const refreshedAuth: OAuthAuthDetails = {
1201+
type: "oauth",
1202+
access: "header.payload.signature",
1203+
refresh: "new-refresh-a",
1204+
expires: now + 3_600_000,
1205+
};
1206+
1207+
mockWithAccountStorageTransaction.mockImplementationOnce(async (handler) => {
1208+
const persist = vi.fn().mockResolvedValue(undefined);
1209+
const result = await handler(stored as any, persist);
1210+
1211+
expect(persist).toHaveBeenCalledTimes(1);
1212+
const persistedStorage = persist.mock.calls[0]?.[0];
1213+
expect(persistedStorage?.activeIndex).toBe(1);
1214+
expect(persistedStorage?.activeIndexByFamily?.codex).toBe(1);
1215+
expect(persistedStorage?.accounts[0]?.refreshToken).toBe("new-refresh-a");
1216+
expect(persistedStorage?.accounts[1]?.rateLimitResetTimes?.codex).toBeGreaterThan(
1217+
now,
1218+
);
1219+
expect(persistedStorage?.accounts[1]?.cooldownReason).toBe(
1220+
"network-error",
1221+
);
1222+
expect(persistedStorage?.accounts[1]?.coolingDownUntil).toBeGreaterThan(
1223+
now,
1224+
);
1225+
1226+
return result;
1227+
});
1228+
1229+
await manager.commitRefreshedAuth(accountA, refreshedAuth);
1230+
});
1231+
11621232
it("propagates storage write failure as retryable CodexAuthError", async () => {
11631233
const { withAccountStorageTransaction } = await import("../lib/storage.js");
11641234
const mockWithAccountStorageTransaction = vi.mocked(

test/fetch-helpers.test.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -86,7 +86,7 @@ describe('Fetch Helpers Module', () => {
8686
(err) => err as CodexAuthError,
8787
);
8888
expect(error).toBeInstanceOf(CodexAuthError);
89-
expect(error.retryable).toBe(true);
89+
expect(error.retryable).toBe(false);
9090
expect(refreshSpy).not.toHaveBeenCalled();
9191
});
9292

@@ -99,7 +99,7 @@ describe('Fetch Helpers Module', () => {
9999
(err) => err as CodexAuthError,
100100
);
101101
expect(error).toBeInstanceOf(CodexAuthError);
102-
expect(error.retryable).toBe(true);
102+
expect(error.retryable).toBe(false);
103103
expect(refreshSpy).not.toHaveBeenCalled();
104104
});
105105

test/refresh-guardian.test.ts

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -711,6 +711,52 @@ describe("refresh-guardian", () => {
711711
);
712712
});
713713

714+
it("treats null commit results as retryable network cooldowns", async () => {
715+
const accountA = createManagedAccount(0);
716+
const manager = createManagerMock([accountA]);
717+
const commitRefreshedAuthMock = manager
718+
.commitRefreshedAuth as ReturnType<typeof vi.fn>;
719+
commitRefreshedAuthMock.mockResolvedValueOnce(null);
720+
721+
const { RefreshGuardian } = await import("../lib/refresh-guardian.js");
722+
const guardian = new RefreshGuardian(() => manager, {
723+
bufferMs: 60_000,
724+
intervalMs: 5_000,
725+
});
726+
727+
refreshExpiringAccountsMock.mockResolvedValue(
728+
new Map([
729+
[
730+
0,
731+
{
732+
refreshed: true,
733+
reason: "success",
734+
tokenResult: {
735+
type: "success",
736+
access: "access-0",
737+
refresh: "refresh-0-new",
738+
expires: Date.now() + 3_600_000,
739+
},
740+
},
741+
],
742+
]),
743+
);
744+
745+
await guardian.tick();
746+
747+
expect(applyRefreshResultMock).not.toHaveBeenCalled();
748+
expect(commitRefreshedAuthMock).toHaveBeenCalledTimes(1);
749+
expect(
750+
manager.markAccountCoolingDown as ReturnType<typeof vi.fn>,
751+
).toHaveBeenCalledWith(accountA, 60_000, "network-error");
752+
753+
const stats = guardian.getStats();
754+
expect(stats.runs).toBe(1);
755+
expect(stats.refreshed).toBe(0);
756+
expect(stats.failed).toBe(1);
757+
expect(stats.networkFailed).toBe(1);
758+
});
759+
714760
it("treats commit failures as retryable network cooldowns and continues the batch", async () => {
715761
const accountA = createManagedAccount(0);
716762
const accountB = createManagedAccount(1);

0 commit comments

Comments
 (0)