Skip to content

Commit 5ddd970

Browse files
committed
Merge pull request #571 from ndycode/claude/audit-52-rotation-token-refresh-tests
test: cover ensureFreshAccessToken refresh, cooldown, and dedup paths
2 parents c781939 + 9480f3f commit 5ddd970

1 file changed

Lines changed: 321 additions & 0 deletions

File tree

Lines changed: 321 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,321 @@
1+
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
2+
import { AccountManager } from "../lib/accounts.js";
3+
import type { AccountStorageV3 } from "../lib/storage.js";
4+
5+
const { queuedRefreshMock, saveAccountsMock, withAccountStorageTransactionMock } =
6+
vi.hoisted(() => ({
7+
queuedRefreshMock: vi.fn(),
8+
saveAccountsMock: vi.fn(),
9+
withAccountStorageTransactionMock: vi.fn(),
10+
}));
11+
12+
vi.mock("../lib/refresh-queue.js", async (importOriginal) => {
13+
const actual = await importOriginal<typeof import("../lib/refresh-queue.js")>();
14+
return { ...actual, queuedRefresh: queuedRefreshMock };
15+
});
16+
17+
vi.mock("../lib/storage.js", async (importOriginal) => {
18+
const actual = await importOriginal<typeof import("../lib/storage.js")>();
19+
return {
20+
...actual,
21+
saveAccounts: saveAccountsMock,
22+
withAccountStorageTransaction: withAccountStorageTransactionMock,
23+
};
24+
});
25+
26+
vi.mock("../lib/codex-cli/writer.js", () => ({
27+
setCodexCliActiveSelection: vi.fn().mockResolvedValue(true),
28+
}));
29+
30+
const {
31+
applyMonotonicAuthCooldown,
32+
DEFAULT_AUTH_FAILURE_COOLDOWN_MS,
33+
ensureFreshAccessToken,
34+
} = await import("../lib/runtime/rotation-token-refresh.js");
35+
36+
const NOW = Date.now();
37+
const FAMILY = "gpt-5-codex" as const;
38+
const SKEW_MS = 30_000;
39+
const INVALIDATION_COOLDOWN_MS = 300_000;
40+
41+
function storageWith(expiresAt: number): AccountStorageV3 {
42+
return {
43+
version: 3,
44+
activeIndex: 0,
45+
activeIndexByFamily: { [FAMILY]: 0 },
46+
accounts: [
47+
{
48+
email: "account-1@example.com",
49+
accountId: "acc_1",
50+
refreshToken: "refresh-1",
51+
accessToken: "access-1",
52+
expiresAt,
53+
addedAt: NOW - 60_000,
54+
lastUsed: NOW - 60_000,
55+
enabled: true,
56+
},
57+
],
58+
};
59+
}
60+
61+
const FRESH_EXPIRES = NOW + 3_600_000;
62+
// Inside the refresh skew window: the proxy must refresh before using it.
63+
const STALE_EXPIRES = NOW + 10_000;
64+
65+
const openManagers: AccountManager[] = [];
66+
67+
function managerWith(expiresAt: number): AccountManager {
68+
const accountManager = new AccountManager(undefined, storageWith(expiresAt));
69+
openManagers.push(accountManager);
70+
return accountManager;
71+
}
72+
73+
function refreshParams(accountManager: AccountManager) {
74+
const account = accountManager.getAccountByIndex(0);
75+
if (!account) throw new Error("fixture account missing");
76+
return {
77+
accountManager,
78+
account,
79+
family: FAMILY,
80+
model: null,
81+
now: NOW,
82+
tokenRefreshSkewMs: SKEW_MS,
83+
tokenInvalidationCooldownMs: INVALIDATION_COOLDOWN_MS,
84+
};
85+
}
86+
87+
beforeEach(() => {
88+
vi.clearAllMocks();
89+
AccountManager.resetVolatileRuntimeState();
90+
saveAccountsMock.mockResolvedValue(undefined);
91+
withAccountStorageTransactionMock.mockImplementation(async (handler) =>
92+
handler(null, async () => undefined),
93+
);
94+
});
95+
96+
afterEach(async () => {
97+
for (const accountManager of openManagers.splice(0, openManagers.length)) {
98+
await accountManager.flushPendingSave();
99+
}
100+
});
101+
102+
describe("ensureFreshAccessToken", () => {
103+
it("uses a fresh token as-is without touching the refresh queue", async () => {
104+
const accountManager = managerWith(FRESH_EXPIRES);
105+
106+
const result = await ensureFreshAccessToken(refreshParams(accountManager));
107+
108+
expect(result).toMatchObject({ ok: true, accessToken: "access-1" });
109+
expect(queuedRefreshMock).not.toHaveBeenCalled();
110+
});
111+
112+
it("refreshes a stale token and commits the rotated credentials", async () => {
113+
const accountManager = managerWith(STALE_EXPIRES);
114+
queuedRefreshMock.mockResolvedValue({
115+
type: "success",
116+
access: "access-new",
117+
refresh: "refresh-new",
118+
expires: NOW + 7_200_000,
119+
});
120+
121+
const result = await ensureFreshAccessToken(refreshParams(accountManager));
122+
123+
expect(queuedRefreshMock).toHaveBeenCalledExactlyOnceWith("refresh-1");
124+
expect(result.ok).toBe(true);
125+
if (result.ok) {
126+
expect(result.accessToken).toBe("access-new");
127+
expect(result.account.access).toBe("access-new");
128+
}
129+
// The in-memory pool now carries the rotated credentials.
130+
expect(accountManager.getAccountByIndex(0)).toMatchObject({
131+
access: "access-new",
132+
refreshToken: "refresh-new",
133+
});
134+
});
135+
136+
it("falls back to the refresh result's token when the commit cannot resolve the account", async () => {
137+
const accountManager = managerWith(STALE_EXPIRES);
138+
const original = accountManager.getAccountByIndex(0);
139+
if (!original) throw new Error("fixture account missing");
140+
queuedRefreshMock.mockResolvedValue({
141+
type: "success",
142+
access: "access-new",
143+
refresh: "refresh-new",
144+
expires: NOW + 7_200_000,
145+
});
146+
// The account vanished from storage between refresh and persist: the
147+
// commit reports null and the caller must use the freshly refreshed
148+
// token, never the stale one on the original account object.
149+
vi.spyOn(accountManager, "commitRefreshedAuth").mockResolvedValue(null);
150+
151+
const result = await ensureFreshAccessToken(refreshParams(accountManager));
152+
153+
expect(result.ok).toBe(true);
154+
if (result.ok) {
155+
expect(result.accessToken).toBe("access-new");
156+
expect(result.account).toBe(original);
157+
}
158+
});
159+
160+
it("deduplicates concurrent commits for the same refreshed account", async () => {
161+
const accountManager = managerWith(STALE_EXPIRES);
162+
const commit = vi.spyOn(accountManager, "commitRefreshedAuth");
163+
queuedRefreshMock.mockResolvedValue({
164+
type: "success",
165+
access: "access-new",
166+
refresh: "refresh-new",
167+
expires: NOW + 7_200_000,
168+
});
169+
// Hold the first commit open so the second caller arrives while it is
170+
// still in flight — that is the window the dedup queue exists for.
171+
let releaseCommit!: () => void;
172+
const commitGate = new Promise<void>((resolve) => {
173+
releaseCommit = resolve;
174+
});
175+
withAccountStorageTransactionMock.mockImplementation(async (handler) => {
176+
await commitGate;
177+
return handler(null, async () => undefined);
178+
});
179+
const params = refreshParams(accountManager);
180+
181+
const firstPending = ensureFreshAccessToken(params);
182+
const secondPending = ensureFreshAccessToken({ ...params });
183+
await new Promise<void>((resolve) => setImmediate(resolve));
184+
releaseCommit();
185+
const [first, second] = await Promise.all([firstPending, secondPending]);
186+
187+
// Both callers share the single in-flight commit and the same token.
188+
expect(commit).toHaveBeenCalledTimes(1);
189+
expect(first).toMatchObject({ ok: true, accessToken: "access-new" });
190+
expect(second).toMatchObject({ ok: true, accessToken: "access-new" });
191+
});
192+
193+
it("applies the short cooldown on a non-retryable auth failure", async () => {
194+
const accountManager = managerWith(STALE_EXPIRES);
195+
queuedRefreshMock.mockResolvedValue({
196+
type: "failed",
197+
reason: "http_error",
198+
statusCode: 401,
199+
message: "token expired",
200+
});
201+
202+
const result = await ensureFreshAccessToken(refreshParams(accountManager));
203+
204+
expect(result).toMatchObject({
205+
ok: false,
206+
retryable: false,
207+
invalidated: false,
208+
});
209+
const coolingDownUntil =
210+
accountManager.getAccountByIndex(0)?.coolingDownUntil ?? 0;
211+
expect(coolingDownUntil).toBeGreaterThan(NOW);
212+
expect(coolingDownUntil).toBeLessThanOrEqual(
213+
Date.now() + DEFAULT_AUTH_FAILURE_COOLDOWN_MS,
214+
);
215+
});
216+
217+
it("marks transient refresh failures retryable", async () => {
218+
const accountManager = managerWith(STALE_EXPIRES);
219+
queuedRefreshMock.mockResolvedValue({
220+
type: "failed",
221+
reason: "network_error",
222+
message: "fetch failed",
223+
});
224+
225+
const result = await ensureFreshAccessToken(refreshParams(accountManager));
226+
227+
expect(result).toMatchObject({ ok: false, retryable: true });
228+
});
229+
230+
it("applies the long cooldown and signals invalidation on a revoked token", async () => {
231+
const accountManager = managerWith(STALE_EXPIRES);
232+
queuedRefreshMock.mockResolvedValue({
233+
type: "failed",
234+
reason: "http_error",
235+
statusCode: 401,
236+
message: "OAuth token has been invalidated",
237+
});
238+
239+
const result = await ensureFreshAccessToken(refreshParams(accountManager));
240+
241+
expect(result).toMatchObject({ ok: false, invalidated: true });
242+
const coolingDownUntil =
243+
accountManager.getAccountByIndex(0)?.coolingDownUntil ?? 0;
244+
// The invalidation cooldown is the long one, far beyond the 30s default.
245+
expect(coolingDownUntil).toBeGreaterThan(
246+
NOW + INVALIDATION_COOLDOWN_MS - 10_000,
247+
);
248+
});
249+
250+
it("never lets a later generic failure truncate an invalidation cooldown", async () => {
251+
const accountManager = managerWith(STALE_EXPIRES);
252+
const account = accountManager.getAccountByIndex(0);
253+
if (!account) throw new Error("fixture account missing");
254+
accountManager.markAccountCoolingDown(
255+
account,
256+
INVALIDATION_COOLDOWN_MS,
257+
"auth-failure",
258+
);
259+
const longDeadline =
260+
accountManager.getAccountByIndex(0)?.coolingDownUntil ?? 0;
261+
queuedRefreshMock.mockResolvedValue({
262+
type: "failed",
263+
reason: "http_error",
264+
statusCode: 401,
265+
message: "token expired",
266+
});
267+
268+
await ensureFreshAccessToken(refreshParams(accountManager));
269+
270+
// Monotonic guard: the 30s generic cooldown must not shorten the
271+
// 5-minute invalidation cooldown set by a concurrent request.
272+
expect(accountManager.getAccountByIndex(0)?.coolingDownUntil).toBe(
273+
longDeadline,
274+
);
275+
});
276+
277+
it("cools down and stays retryable when the commit itself fails", async () => {
278+
const accountManager = managerWith(STALE_EXPIRES);
279+
queuedRefreshMock.mockResolvedValue({
280+
type: "success",
281+
access: "access-new",
282+
refresh: "refresh-new",
283+
expires: NOW + 7_200_000,
284+
});
285+
// Once: the post-test debounced-save flush must still see the working
286+
// transaction implementation from beforeEach.
287+
withAccountStorageTransactionMock.mockRejectedValueOnce(
288+
Object.assign(new Error("locked"), { code: "EBUSY" }),
289+
);
290+
291+
const result = await ensureFreshAccessToken(refreshParams(accountManager));
292+
293+
expect(result).toMatchObject({ ok: false, retryable: true });
294+
expect(
295+
accountManager.getAccountByIndex(0)?.coolingDownUntil ?? 0,
296+
).toBeGreaterThan(NOW);
297+
});
298+
});
299+
300+
describe("applyMonotonicAuthCooldown", () => {
301+
it("extends an absent cooldown but never shortens an existing one", () => {
302+
const accountManager = managerWith(FRESH_EXPIRES);
303+
const account = accountManager.getAccountByIndex(0);
304+
if (!account) throw new Error("fixture account missing");
305+
306+
applyMonotonicAuthCooldown(accountManager, account, 60_000);
307+
const firstDeadline =
308+
accountManager.getAccountByIndex(0)?.coolingDownUntil ?? 0;
309+
expect(firstDeadline).toBeGreaterThan(NOW);
310+
311+
applyMonotonicAuthCooldown(accountManager, account, 1_000);
312+
expect(accountManager.getAccountByIndex(0)?.coolingDownUntil).toBe(
313+
firstDeadline,
314+
);
315+
316+
applyMonotonicAuthCooldown(accountManager, account, 600_000);
317+
expect(
318+
accountManager.getAccountByIndex(0)?.coolingDownUntil ?? 0,
319+
).toBeGreaterThan(firstDeadline);
320+
});
321+
});

0 commit comments

Comments
 (0)