Skip to content

Commit ba02944

Browse files
ndycodendycode
committed
Finish PR #78 review follow-ups
1 parent a160bc5 commit ba02944

4 files changed

Lines changed: 42 additions & 37 deletions

File tree

‎index.ts‎

Lines changed: 20 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -578,7 +578,7 @@ export const OpenAIOAuthPlugin: Plugin = async ({ client }: PluginInput) => {
578578
}
579579
// Unknown disabled reasons are treated as legacy/manual disables.
580580
// Explicit login revival is reserved for accounts we know were disabled by auth failure.
581-
return undefined;
581+
return "user" satisfies AccountDisabledReason;
582582
})();
583583
const targetCoolingDownUntil =
584584
typeof target.coolingDownUntil === "number" && Number.isFinite(target.coolingDownUntil)
@@ -1681,6 +1681,15 @@ export const OpenAIOAuthPlugin: Plugin = async ({ client }: PluginInput) => {
16811681
}
16821682
};
16831683

1684+
const setCachedAccountManager = (manager: AccountManager): AccountManager => {
1685+
if (cachedAccountManager && cachedAccountManager !== manager) {
1686+
staleAccountManagersForCleanup.add(cachedAccountManager);
1687+
}
1688+
cachedAccountManager = manager;
1689+
accountManagerPromise = Promise.resolve(manager);
1690+
return manager;
1691+
};
1692+
16841693
const invalidateAccountManagerCache = (): void => {
16851694
if (cachedAccountManager) {
16861695
staleAccountManagersForCleanup.add(cachedAccountManager);
@@ -1751,8 +1760,7 @@ export const OpenAIOAuthPlugin: Plugin = async ({ client }: PluginInput) => {
17511760
// refresh tokens with stale in-memory state.
17521761
if (cachedAccountManager) {
17531762
const reloadedManager = await AccountManager.loadFromDisk();
1754-
cachedAccountManager = reloadedManager;
1755-
accountManagerPromise = Promise.resolve(reloadedManager);
1763+
setCachedAccountManager(reloadedManager);
17561764
}
17571765

17581766
await showToast(`Switched to account ${index + 1}`, "info");
@@ -1821,8 +1829,7 @@ export const OpenAIOAuthPlugin: Plugin = async ({ client }: PluginInput) => {
18211829
if (!accountManagerPromise) {
18221830
accountManagerPromise = AccountManager.loadFromDisk(authFallback);
18231831
}
1824-
let accountManager = await accountManagerPromise;
1825-
cachedAccountManager = accountManager;
1832+
let accountManager = setCachedAccountManager(await accountManagerPromise);
18261833
const refreshToken = authFallback?.refresh ?? "";
18271834
const needsPersist =
18281835
refreshToken &&
@@ -4050,10 +4057,9 @@ while (attempted.size < Math.max(1, accountCount)) {
40504057
return `Switched to ${label} but failed to persist. Changes may be lost on restart.`;
40514058
}
40524059

4053-
if (cachedAccountManager) {
4060+
if (cachedAccountManager) {
40544061
const reloadedManager = await AccountManager.loadFromDisk();
4055-
cachedAccountManager = reloadedManager;
4056-
accountManagerPromise = Promise.resolve(reloadedManager);
4062+
setCachedAccountManager(reloadedManager);
40574063
}
40584064

40594065
const label = formatCommandAccountLabel(account, targetIndex);
@@ -5110,8 +5116,7 @@ while (attempted.size < Math.max(1, accountCount)) {
51105116

51115117
if (cachedAccountManager) {
51125118
const reloadedManager = await AccountManager.loadFromDisk();
5113-
cachedAccountManager = reloadedManager;
5114-
accountManagerPromise = Promise.resolve(reloadedManager);
5119+
setCachedAccountManager(reloadedManager);
51155120
}
51165121
}
51175122

@@ -5355,8 +5360,7 @@ while (attempted.size < Math.max(1, accountCount)) {
53555360

53565361
if (cachedAccountManager) {
53575362
const reloadedManager = await AccountManager.loadFromDisk();
5358-
cachedAccountManager = reloadedManager;
5359-
accountManagerPromise = Promise.resolve(reloadedManager);
5363+
setCachedAccountManager(reloadedManager);
53605364
}
53615365

53625366
const accountLabel = formatCommandAccountLabel(account, targetIndex);
@@ -5456,8 +5460,7 @@ while (attempted.size < Math.max(1, accountCount)) {
54565460

54575461
if (cachedAccountManager) {
54585462
const reloadedManager = await AccountManager.loadFromDisk();
5459-
cachedAccountManager = reloadedManager;
5460-
accountManagerPromise = Promise.resolve(reloadedManager);
5463+
setCachedAccountManager(reloadedManager);
54615464
}
54625465

54635466
const accountLabel = formatCommandAccountLabel(account, targetIndex);
@@ -5534,8 +5537,7 @@ while (attempted.size < Math.max(1, accountCount)) {
55345537

55355538
if (cachedAccountManager) {
55365539
const reloadedManager = await AccountManager.loadFromDisk();
5537-
cachedAccountManager = reloadedManager;
5538-
accountManagerPromise = Promise.resolve(reloadedManager);
5540+
setCachedAccountManager(reloadedManager);
55395541
}
55405542

55415543
const accountLabel = formatCommandAccountLabel(account, targetIndex);
@@ -5851,8 +5853,7 @@ while (attempted.size < Math.max(1, accountCount)) {
58515853

58525854
if (cachedAccountManager) {
58535855
const reloadedManager = await AccountManager.loadFromDisk();
5854-
cachedAccountManager = reloadedManager;
5855-
accountManagerPromise = Promise.resolve(reloadedManager);
5856+
setCachedAccountManager(reloadedManager);
58565857
}
58575858

58585859
const remaining = storage.accounts.length;
@@ -5946,8 +5947,7 @@ while (attempted.size < Math.max(1, accountCount)) {
59465947
await saveAccounts(storage);
59475948
if (cachedAccountManager) {
59485949
const reloadedManager = await AccountManager.loadFromDisk();
5949-
cachedAccountManager = reloadedManager;
5950-
accountManagerPromise = Promise.resolve(reloadedManager);
5950+
setCachedAccountManager(reloadedManager);
59515951
}
59525952
results.push("");
59535953
results.push(`Summary: ${refreshedCount} refreshed, ${failedCount} failed`);

‎lib/accounts.ts‎

Lines changed: 19 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -271,7 +271,7 @@ export class AccountManager {
271271
if (!changed) return;
272272

273273
try {
274-
await this.saveToDisk();
274+
await this.enqueueSave(() => this.saveToDisk());
275275
} catch (error) {
276276
log.debug("Failed to persist Codex CLI cache hydration", { error: String(error) });
277277
}
@@ -991,6 +991,22 @@ export class AccountManager {
991991
await saveAccounts(storage);
992992
}
993993

994+
private enqueueSave(saveOperation: () => Promise<void>): Promise<void> {
995+
const previousSave = this.pendingSave;
996+
const nextSave = (async () => {
997+
if (previousSave) {
998+
await previousSave;
999+
}
1000+
await saveOperation();
1001+
})().finally(() => {
1002+
if (this.pendingSave === nextSave) {
1003+
this.pendingSave = null;
1004+
}
1005+
});
1006+
this.pendingSave = nextSave;
1007+
return nextSave;
1008+
}
1009+
9941010
saveToDiskDebounced(delayMs = 500): void {
9951011
if (this.saveDebounceTimer) {
9961012
clearTimeout(this.saveDebounceTimer);
@@ -999,13 +1015,7 @@ export class AccountManager {
9991015
this.saveDebounceTimer = null;
10001016
const doSave = async () => {
10011017
try {
1002-
if (this.pendingSave) {
1003-
await this.pendingSave;
1004-
}
1005-
this.pendingSave = this.saveToDisk().finally(() => {
1006-
this.pendingSave = null;
1007-
});
1008-
await this.pendingSave;
1018+
await this.enqueueSave(() => this.saveToDisk());
10091019
} catch (error) {
10101020
log.warn("Debounced save failed", { error: error instanceof Error ? error.message : String(error) });
10111021
}
@@ -1045,12 +1055,7 @@ export class AccountManager {
10451055
if (this.saveDebounceTimer !== null || this.pendingSave !== null) {
10461056
continue;
10471057
}
1048-
const flushSave = this.saveToDisk().finally(() => {
1049-
if (this.pendingSave === flushSave) {
1050-
this.pendingSave = null;
1051-
}
1052-
});
1053-
this.pendingSave = flushSave;
1058+
const flushSave = this.enqueueSave(() => this.saveToDisk());
10541059
await flushSave;
10551060
// Drain saves that were queued while the flush save was in flight.
10561061
await Promise.resolve();

‎test/accounts.test.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -915,7 +915,7 @@ describe("AccountManager", () => {
915915
enabled: false,
916916
disabledReason: "user" as const,
917917
coolingDownUntil: now + 60_000,
918-
cooldownReason: "rate-limit" as const,
918+
cooldownReason: "network-error" as const,
919919
addedAt: now,
920920
lastUsed: now,
921921
},
@@ -939,7 +939,7 @@ describe("AccountManager", () => {
939939
enabled: false,
940940
disabledReason: "user",
941941
coolingDownUntil: now + 60_000,
942-
cooldownReason: "rate-limit",
942+
cooldownReason: "network-error",
943943
});
944944
expect(updated[2]).toMatchObject({
945945
accountId: "workspace-b",

‎test/index.test.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3272,7 +3272,7 @@ describe("OpenAIOAuthPlugin persistAccountPool", () => {
32723272
const mergedOrg = mergedOrgEntries[0];
32733273
expect(mergedOrg?.accountId).toBe("org-shared");
32743274
expect(mergedOrg?.enabled).toBe(false);
3275-
expect(mergedOrg?.disabledReason).toBeUndefined();
3275+
expect(mergedOrg?.disabledReason).toBe("user");
32763276
expect(mergedOrg?.coolingDownUntil).toBe(12_000);
32773277
expect(mergedOrg?.cooldownReason).toBe("auth-failure");
32783278
});

0 commit comments

Comments
 (0)