Skip to content

Commit 16af48b

Browse files
authored
Merge pull request #337 from ndycode/audit/pr1-pool-health
fix report live token freshness handling
2 parents b9c9273 + 294104d commit 16af48b

3 files changed

Lines changed: 142 additions & 65 deletions

File tree

lib/codex-manager.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3224,6 +3224,7 @@ export async function runCodexMultiAuthCli(rawArgs: string[]): Promise<number> {
32243224
loadAccounts,
32253225
saveAccounts,
32263226
resolveActiveIndex,
3227+
hasUsableAccessToken,
32273228
queuedRefresh,
32283229
fetchCodexQuotaSnapshot,
32293230
formatRateLimitEntry,

lib/codex-manager/commands/report.ts

Lines changed: 75 additions & 65 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,10 @@ export interface ReportCommandDeps {
5858
loadAccounts: () => Promise<AccountStorageV3 | null>;
5959
saveAccounts: (storage: AccountStorageV3) => Promise<void>;
6060
resolveActiveIndex: (storage: AccountStorageV3, family?: "codex") => number;
61+
hasUsableAccessToken: (
62+
account: Pick<AccountMetadataV3, "accessToken" | "expiresAt">,
63+
now: number,
64+
) => boolean;
6165
queuedRefresh: (refreshToken: string) => Promise<TokenResult>;
6266
fetchCodexQuotaSnapshot: (input: {
6367
accountId: string;
@@ -362,73 +366,79 @@ export async function runReportCommand(
362366
const account = storage.accounts[i];
363367
if (!account || account.enabled === false) continue;
364368

365-
const refreshResult = await deps.queuedRefresh(account.refreshToken);
366-
if (refreshResult.type !== "success") {
367-
refreshFailures.set(i, {
368-
...refreshResult,
369-
message: deps.normalizeFailureDetail(
370-
refreshResult.message,
371-
refreshResult.reason,
372-
),
373-
});
374-
continue;
375-
}
376-
377-
const refreshedEmail = sanitizeEmail(
378-
extractAccountEmail(refreshResult.access, refreshResult.idToken),
379-
);
380-
const tokenDerivedAccountId = extractAccountId(refreshResult.access);
381-
const refreshedAccountId = account.accountId ?? tokenDerivedAccountId;
382-
const previousRefreshToken = account.refreshToken;
383-
const previousAccessToken = account.accessToken;
384-
const previousExpiresAt = account.expiresAt;
385-
const previousEmail = account.email;
386-
const previousAccountId = account.accountId;
387-
const refreshPatch: RefreshedAccountPatch = {
388-
refreshToken: refreshResult.refresh,
389-
accessToken: refreshResult.access,
390-
expiresAt: refreshResult.expires,
391-
};
392-
if (refreshedEmail) {
393-
refreshPatch.email = refreshedEmail;
394-
}
395-
if (tokenDerivedAccountId) {
396-
refreshPatch.accountId = tokenDerivedAccountId;
397-
refreshPatch.accountIdSource = "token";
398-
}
399-
const accountMatch: AccountIdentityMatch = {
400-
refreshToken: previousRefreshToken,
401-
email: previousEmail,
402-
accountId: previousAccountId,
403-
};
404-
applyRefreshedAccountPatch(account, refreshPatch);
405-
if (
406-
previousRefreshToken !== refreshPatch.refreshToken ||
407-
previousAccessToken !== refreshPatch.accessToken ||
408-
previousExpiresAt !== refreshPatch.expiresAt ||
409-
previousEmail !== account.email ||
410-
previousAccountId !== account.accountId
411-
) {
412-
try {
413-
await persistRefreshedAccountPatch(
414-
storage,
415-
accountMatch,
416-
refreshPatch,
417-
deps.loadAccounts,
418-
deps.saveAccounts,
419-
);
420-
} catch (error) {
421-
const message = deps.normalizeFailureDetail(
422-
error instanceof Error ? error.message : String(error),
423-
undefined,
424-
);
425-
probeErrors.push(`${formatAccountLabel(account, i)}: ${message}`);
369+
let probeAccessToken = account.accessToken;
370+
let probeAccountId =
371+
account.accountId ?? extractAccountId(account.accessToken);
372+
if (!deps.hasUsableAccessToken(account, now)) {
373+
const refreshResult = await deps.queuedRefresh(account.refreshToken);
374+
if (refreshResult.type !== "success") {
375+
refreshFailures.set(i, {
376+
...refreshResult,
377+
message: deps.normalizeFailureDetail(
378+
refreshResult.message,
379+
refreshResult.reason,
380+
),
381+
});
426382
continue;
427383
}
384+
385+
const refreshedEmail = sanitizeEmail(
386+
extractAccountEmail(refreshResult.access, refreshResult.idToken),
387+
);
388+
const tokenDerivedAccountId = extractAccountId(refreshResult.access);
389+
const refreshedAccountId = account.accountId ?? tokenDerivedAccountId;
390+
const previousRefreshToken = account.refreshToken;
391+
const previousAccessToken = account.accessToken;
392+
const previousExpiresAt = account.expiresAt;
393+
const previousEmail = account.email;
394+
const previousAccountId = account.accountId;
395+
const refreshPatch: RefreshedAccountPatch = {
396+
refreshToken: refreshResult.refresh,
397+
accessToken: refreshResult.access,
398+
expiresAt: refreshResult.expires,
399+
};
400+
if (refreshedEmail) {
401+
refreshPatch.email = refreshedEmail;
402+
}
403+
if (tokenDerivedAccountId) {
404+
refreshPatch.accountId = tokenDerivedAccountId;
405+
refreshPatch.accountIdSource = "token";
406+
}
407+
const accountMatch: AccountIdentityMatch = {
408+
refreshToken: previousRefreshToken,
409+
email: previousEmail,
410+
accountId: previousAccountId,
411+
};
412+
applyRefreshedAccountPatch(account, refreshPatch);
413+
probeAccessToken = refreshResult.access;
414+
probeAccountId = account.accountId ?? refreshedAccountId;
415+
if (
416+
previousRefreshToken !== refreshPatch.refreshToken ||
417+
previousAccessToken !== refreshPatch.accessToken ||
418+
previousExpiresAt !== refreshPatch.expiresAt ||
419+
previousEmail !== account.email ||
420+
previousAccountId !== account.accountId
421+
) {
422+
try {
423+
await persistRefreshedAccountPatch(
424+
storage,
425+
accountMatch,
426+
refreshPatch,
427+
deps.loadAccounts,
428+
deps.saveAccounts,
429+
);
430+
} catch (error) {
431+
const message = deps.normalizeFailureDetail(
432+
error instanceof Error ? error.message : String(error),
433+
undefined,
434+
);
435+
probeErrors.push(`${formatAccountLabel(account, i)}: ${message}`);
436+
continue;
437+
}
438+
}
428439
}
429440

430-
const accountId = account.accountId ?? refreshedAccountId;
431-
if (!accountId) {
441+
if (!probeAccessToken || !probeAccountId) {
432442
probeErrors.push(
433443
`${formatAccountLabel(account, i)}: missing accountId for live probe`,
434444
);
@@ -437,8 +447,8 @@ export async function runReportCommand(
437447

438448
try {
439449
const liveQuota = await deps.fetchCodexQuotaSnapshot({
440-
accountId,
441-
accessToken: refreshResult.access,
450+
accountId: probeAccountId,
451+
accessToken: probeAccessToken,
442452
model: modelInspection.normalized,
443453
});
444454
liveQuotaByIndex.set(i, liveQuota);

test/codex-manager-report-command.test.ts

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@ function createDeps(
3535
loadAccounts: vi.fn(async () => createStorage()),
3636
saveAccounts: vi.fn(async () => undefined),
3737
resolveActiveIndex: vi.fn(() => 0),
38+
hasUsableAccessToken: vi.fn(() => false),
3839
queuedRefresh: vi.fn(async () => ({
3940
type: "success",
4041
access: "access-token-1",
@@ -192,6 +193,71 @@ describe("runReportCommand", () => {
192193
expect(jsonOutput.forecast.accounts[3]?.liveQuota?.planType).toBe("pro");
193194
});
194195

196+
it("reuses usable access tokens for live probes without forcing refresh", async () => {
197+
const deps = createDeps({
198+
hasUsableAccessToken: vi.fn(() => true),
199+
loadAccounts: vi.fn(async () =>
200+
createStorage([
201+
{
202+
email: "one@example.com",
203+
accountId: "acct-live",
204+
refreshToken: "refresh-token-1",
205+
accessToken: "access-token-1",
206+
expiresAt: 5_000,
207+
addedAt: 1,
208+
lastUsed: 1,
209+
enabled: true,
210+
},
211+
]),
212+
),
213+
});
214+
215+
const result = await runReportCommand(["--live", "--json"], deps);
216+
217+
expect(result).toBe(0);
218+
expect(deps.queuedRefresh).not.toHaveBeenCalled();
219+
expect(deps.saveAccounts).not.toHaveBeenCalled();
220+
expect(deps.fetchCodexQuotaSnapshot).toHaveBeenCalledWith({
221+
accountId: "acct-live",
222+
accessToken: "access-token-1",
223+
model: "gpt-5-codex",
224+
});
225+
});
226+
227+
it("records probe error when usable token exists but account id is missing", async () => {
228+
const deps = createDeps({
229+
hasUsableAccessToken: vi.fn(() => true),
230+
loadAccounts: vi.fn(async () =>
231+
createStorage([
232+
{
233+
email: "missing-id@example.com",
234+
refreshToken: "refresh-token-1",
235+
accessToken: "not-a-jwt",
236+
expiresAt: 5_000,
237+
addedAt: 1,
238+
lastUsed: 1,
239+
enabled: true,
240+
},
241+
]),
242+
),
243+
});
244+
245+
const result = await runReportCommand(["--live", "--json"], deps);
246+
247+
expect(result).toBe(0);
248+
expect(deps.queuedRefresh).not.toHaveBeenCalled();
249+
expect(deps.saveAccounts).not.toHaveBeenCalled();
250+
expect(deps.fetchCodexQuotaSnapshot).not.toHaveBeenCalled();
251+
const jsonOutput = JSON.parse(
252+
(deps.logInfo as ReturnType<typeof vi.fn>).mock.calls.at(-1)?.[0] ?? "{}",
253+
) as { forecast: { probeErrors: string[] } };
254+
expect(jsonOutput.forecast.probeErrors).toEqual(
255+
expect.arrayContaining([
256+
expect.stringContaining("missing accountId for live probe"),
257+
]),
258+
);
259+
});
260+
195261
it("persists refreshed probe tokens before report live probes", async () => {
196262
const storage = createStorage([
197263
{

0 commit comments

Comments
 (0)