Skip to content

Commit bfb6a33

Browse files
ndycodendycode
committed
fix: graduate refresh guardian failure penalties
1 parent 53488ef commit bfb6a33

2 files changed

Lines changed: 156 additions & 22 deletions

File tree

‎lib/refresh-guardian.ts‎

Lines changed: 37 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import { createLogger } from "./logger.js";
22
import { applyRefreshResult, refreshExpiringAccounts } from "./proactive-refresh.js";
33
import type { AccountManager } from "./accounts.js";
4+
import { ACCOUNT_LIMITS } from "./constants.js";
45
import type { CooldownReason } from "./storage.js";
56
import type { TokenResult } from "./types.js";
67

@@ -24,6 +25,7 @@ export interface RefreshGuardianStats {
2425
}
2526

2627
const DEFAULT_INTERVAL_MS = 60_000;
28+
const NETWORK_FAILURE_COOLDOWN_MS = 6_000;
2729

2830
export class RefreshGuardian {
2931
private readonly getAccountManager: () => AccountManager | null;
@@ -95,6 +97,15 @@ export class RefreshGuardian {
9597
return "network-error";
9698
}
9799

100+
private getNetworkFailureCooldownMs(): number {
101+
return Math.min(this.bufferMs, NETWORK_FAILURE_COOLDOWN_MS);
102+
}
103+
104+
private getAuthFailureCooldownMs(failureCount: number): number {
105+
const streak = Math.max(1, Math.floor(failureCount));
106+
return Math.min(this.bufferMs, ACCOUNT_LIMITS.AUTH_FAILURE_COOLDOWN_MS * streak);
107+
}
108+
98109
async tick(): Promise<void> {
99110
if (this.running) return;
100111
const manager = this.getAccountManager();
@@ -135,14 +146,33 @@ export class RefreshGuardian {
135146
manager.clearAuthFailures(account);
136147
this.stats.refreshed += 1;
137148
} else {
138-
manager.markAccountCoolingDown(account, this.bufferMs, "network-error");
149+
manager.markAccountCoolingDown(
150+
account,
151+
this.getNetworkFailureCooldownMs(),
152+
"network-error",
153+
);
139154
this.stats.failed += 1;
140155
this.stats.networkFailed += 1;
141156
}
142157
break;
143158
case "failed": {
144159
const cooldownReason = this.classifyFailureReason(result.tokenResult);
145-
manager.markAccountCoolingDown(account, this.bufferMs, cooldownReason);
160+
if (cooldownReason === "rate-limit") {
161+
manager.markRateLimited(account, this.bufferMs, "codex");
162+
} else if (cooldownReason === "auth-failure") {
163+
const failureCount = manager.incrementAuthFailures(account);
164+
manager.markAccountCoolingDown(
165+
account,
166+
this.getAuthFailureCooldownMs(failureCount),
167+
cooldownReason,
168+
);
169+
} else {
170+
manager.markAccountCoolingDown(
171+
account,
172+
this.getNetworkFailureCooldownMs(),
173+
cooldownReason,
174+
);
175+
}
146176
this.stats.failed += 1;
147177
if (cooldownReason === "rate-limit") this.stats.rateLimited += 1;
148178
else if (cooldownReason === "auth-failure") this.stats.authFailed += 1;
@@ -153,7 +183,11 @@ export class RefreshGuardian {
153183
this.stats.notNeeded += 1;
154184
break;
155185
case "no_refresh_token":
156-
manager.markAccountCoolingDown(account, this.bufferMs, "auth-failure");
186+
manager.markAccountCoolingDown(
187+
account,
188+
ACCOUNT_LIMITS.AUTH_FAILURE_COOLDOWN_MS,
189+
"auth-failure",
190+
);
157191
manager.setAccountEnabled(account.index, false);
158192
this.stats.noRefreshToken += 1;
159193
this.stats.failed += 1;

‎test/refresh-guardian.test.ts‎

Lines changed: 119 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -27,9 +27,34 @@ function createManagerMock(accounts: ManagedAccount[]): AccountManager {
2727
(index: number) =>
2828
accounts.find((account) => account.index === index) ?? null,
2929
),
30-
clearAuthFailures: vi.fn(),
31-
markAccountCoolingDown: vi.fn(),
32-
setAccountEnabled: vi.fn(),
30+
clearAuthFailures: vi.fn((account: ManagedAccount) => {
31+
account.consecutiveAuthFailures = 0;
32+
}),
33+
incrementAuthFailures: vi.fn((account: ManagedAccount) => {
34+
account.consecutiveAuthFailures = (account.consecutiveAuthFailures ?? 0) + 1;
35+
return account.consecutiveAuthFailures;
36+
}),
37+
markAccountCoolingDown: vi.fn(
38+
(
39+
account: ManagedAccount,
40+
cooldownMs: number,
41+
reason: "auth-failure" | "network-error" | "rate-limit",
42+
) => {
43+
account.coolingDownUntil = Date.now() + cooldownMs;
44+
account.cooldownReason = reason;
45+
},
46+
),
47+
markRateLimited: vi.fn(
48+
(account: ManagedAccount, retryAfterMs: number) => {
49+
account.rateLimitResetTimes.codex = Date.now() + retryAfterMs;
50+
},
51+
),
52+
setAccountEnabled: vi.fn((index: number, enabled: boolean) => {
53+
const account = accounts.find((candidate) => candidate.index === index);
54+
if (account) {
55+
account.enabled = enabled;
56+
}
57+
}),
3358
saveToDiskDebounced: vi.fn(),
3459
} as unknown as AccountManager;
3560
}
@@ -159,9 +184,12 @@ describe("refresh-guardian", () => {
159184
expect(
160185
manager.clearAuthFailures as ReturnType<typeof vi.fn>,
161186
).toHaveBeenCalledWith(accountA);
187+
expect(
188+
manager.incrementAuthFailures as ReturnType<typeof vi.fn>,
189+
).toHaveBeenCalledWith(accountB);
162190
expect(
163191
manager.markAccountCoolingDown as ReturnType<typeof vi.fn>,
164-
).toHaveBeenCalledWith(accountB, 60_000, "auth-failure");
192+
).toHaveBeenCalledWith(accountB, 30_000, "auth-failure");
165193
expect(
166194
manager.saveToDiskDebounced as ReturnType<typeof vi.fn>,
167195
).toHaveBeenCalledTimes(1);
@@ -232,8 +260,15 @@ describe("refresh-guardian", () => {
232260
(index: number) =>
233261
[liveB, liveA].find((account) => account.index === index) ?? null,
234262
),
235-
clearAuthFailures: vi.fn(),
263+
clearAuthFailures: vi.fn((account: ManagedAccount) => {
264+
account.consecutiveAuthFailures = 0;
265+
}),
266+
incrementAuthFailures: vi.fn((account: ManagedAccount) => {
267+
account.consecutiveAuthFailures = (account.consecutiveAuthFailures ?? 0) + 1;
268+
return account.consecutiveAuthFailures;
269+
}),
236270
markAccountCoolingDown: vi.fn(),
271+
markRateLimited: vi.fn(),
237272
saveToDiskDebounced: vi.fn(),
238273
} as unknown as AccountManager;
239274
const { RefreshGuardian } = await import("../lib/refresh-guardian.js");
@@ -343,16 +378,19 @@ describe("refresh-guardian", () => {
343378
).toHaveBeenCalledWith(1, false);
344379
expect(
345380
manager.markAccountCoolingDown as ReturnType<typeof vi.fn>,
346-
).toHaveBeenNthCalledWith(1, accountB, 60_000, "auth-failure");
381+
).toHaveBeenNthCalledWith(1, accountB, 30_000, "auth-failure");
347382
expect(
348383
manager.markAccountCoolingDown as ReturnType<typeof vi.fn>,
349-
).toHaveBeenNthCalledWith(2, accountC, 60_000, "rate-limit");
384+
).toHaveBeenNthCalledWith(2, accountD, 6_000, "network-error");
350385
expect(
351386
manager.markAccountCoolingDown as ReturnType<typeof vi.fn>,
352-
).toHaveBeenNthCalledWith(3, accountD, 60_000, "network-error");
387+
).toHaveBeenNthCalledWith(3, accountE, 30_000, "auth-failure");
353388
expect(
354-
manager.markAccountCoolingDown as ReturnType<typeof vi.fn>,
355-
).toHaveBeenNthCalledWith(4, accountE, 60_000, "auth-failure");
389+
manager.markRateLimited as ReturnType<typeof vi.fn>,
390+
).toHaveBeenCalledWith(accountC, 60_000, "codex");
391+
expect(
392+
manager.incrementAuthFailures as ReturnType<typeof vi.fn>,
393+
).toHaveBeenNthCalledWith(1, accountE);
356394

357395
const stats = guardian.getStats();
358396
expect(stats.runs).toBe(1);
@@ -395,8 +433,15 @@ describe("refresh-guardian", () => {
395433
(index: number) =>
396434
liveSnapshot.find((account) => account.index === index) ?? null,
397435
),
398-
clearAuthFailures: vi.fn(),
436+
clearAuthFailures: vi.fn((account: ManagedAccount) => {
437+
account.consecutiveAuthFailures = 0;
438+
}),
439+
incrementAuthFailures: vi.fn((account: ManagedAccount) => {
440+
account.consecutiveAuthFailures = (account.consecutiveAuthFailures ?? 0) + 1;
441+
return account.consecutiveAuthFailures;
442+
}),
399443
markAccountCoolingDown: vi.fn(),
444+
markRateLimited: vi.fn(),
400445
setAccountEnabled: vi.fn(),
401446
saveToDiskDebounced: vi.fn(),
402447
} as unknown as AccountManager;
@@ -499,19 +544,25 @@ describe("refresh-guardian", () => {
499544
).toHaveBeenCalledTimes(5);
500545
expect(
501546
manager.markAccountCoolingDown as ReturnType<typeof vi.fn>,
502-
).toHaveBeenNthCalledWith(1, accountA, 60_000, "network-error");
547+
).toHaveBeenNthCalledWith(1, accountA, 6_000, "network-error");
503548
expect(
504549
manager.markAccountCoolingDown as ReturnType<typeof vi.fn>,
505-
).toHaveBeenNthCalledWith(2, accountB, 60_000, "network-error");
550+
).toHaveBeenNthCalledWith(2, accountB, 6_000, "network-error");
506551
expect(
507552
manager.markAccountCoolingDown as ReturnType<typeof vi.fn>,
508-
).toHaveBeenNthCalledWith(3, accountC, 60_000, "auth-failure");
553+
).toHaveBeenNthCalledWith(3, accountC, 30_000, "auth-failure");
509554
expect(
510555
manager.markAccountCoolingDown as ReturnType<typeof vi.fn>,
511-
).toHaveBeenNthCalledWith(4, accountD, 60_000, "auth-failure");
556+
).toHaveBeenNthCalledWith(4, accountD, 30_000, "auth-failure");
512557
expect(
513558
manager.markAccountCoolingDown as ReturnType<typeof vi.fn>,
514-
).toHaveBeenNthCalledWith(5, accountE, 60_000, "network-error");
559+
).toHaveBeenNthCalledWith(5, accountE, 6_000, "network-error");
560+
expect(
561+
manager.incrementAuthFailures as ReturnType<typeof vi.fn>,
562+
).toHaveBeenNthCalledWith(1, accountC);
563+
expect(
564+
manager.incrementAuthFailures as ReturnType<typeof vi.fn>,
565+
).toHaveBeenNthCalledWith(2, accountD);
515566
expect(
516567
manager.saveToDiskDebounced as ReturnType<typeof vi.fn>,
517568
).toHaveBeenCalledTimes(1);
@@ -545,6 +596,48 @@ describe("refresh-guardian", () => {
545596
expect(Reflect.get(guardian, "running")).toBe(false);
546597
});
547598

599+
it("escalates auth cooldowns across repeated guardian failures", async () => {
600+
const account = createManagedAccount(0);
601+
const manager = createManagerMock([account]);
602+
const { RefreshGuardian } = await import("../lib/refresh-guardian.js");
603+
const guardian = new RefreshGuardian(() => manager, {
604+
bufferMs: 120_000,
605+
intervalMs: 5_000,
606+
});
607+
608+
refreshExpiringAccountsMock.mockResolvedValue(
609+
new Map([
610+
[
611+
0,
612+
{
613+
refreshed: true,
614+
reason: "failed",
615+
tokenResult: {
616+
type: "failed",
617+
reason: "http_error",
618+
statusCode: 401,
619+
message: "expired",
620+
},
621+
},
622+
],
623+
]),
624+
);
625+
626+
await guardian.tick();
627+
await vi.advanceTimersByTimeAsync(30_001);
628+
await guardian.tick();
629+
630+
expect(
631+
manager.markAccountCoolingDown as ReturnType<typeof vi.fn>,
632+
).toHaveBeenNthCalledWith(1, account, 30_000, "auth-failure");
633+
expect(
634+
manager.markAccountCoolingDown as ReturnType<typeof vi.fn>,
635+
).toHaveBeenNthCalledWith(2, account, 60_000, "auth-failure");
636+
expect(
637+
manager.incrementAuthFailures as ReturnType<typeof vi.fn>,
638+
).toHaveBeenCalledTimes(2);
639+
});
640+
548641
it("handles account removal during tick without throwing", async () => {
549642
const originalA = createManagedAccount(0);
550643
const originalB = createManagedAccount(1);
@@ -560,8 +653,15 @@ describe("refresh-guardian", () => {
560653
getAccountByIndex: vi.fn(
561654
(index: number) => liveAfterRemoval[index] ?? null,
562655
),
563-
clearAuthFailures: vi.fn(),
656+
clearAuthFailures: vi.fn((account: ManagedAccount) => {
657+
account.consecutiveAuthFailures = 0;
658+
}),
659+
incrementAuthFailures: vi.fn((account: ManagedAccount) => {
660+
account.consecutiveAuthFailures = (account.consecutiveAuthFailures ?? 0) + 1;
661+
return account.consecutiveAuthFailures;
662+
}),
564663
markAccountCoolingDown: vi.fn(),
664+
markRateLimited: vi.fn(),
565665
setAccountEnabled: vi.fn(),
566666
saveToDiskDebounced: vi.fn(),
567667
} as unknown as AccountManager;
@@ -609,11 +709,11 @@ describe("refresh-guardian", () => {
609709
expect.anything(),
610710
);
611711
expect(
612-
manager.markAccountCoolingDown as ReturnType<typeof vi.fn>,
712+
manager.markRateLimited as ReturnType<typeof vi.fn>,
613713
).toHaveBeenCalledWith(
614714
expect.objectContaining({ refreshToken: originalB.refreshToken }),
615715
60_000,
616-
"rate-limit",
716+
"codex",
617717
);
618718
});
619719
});

0 commit comments

Comments
 (0)