From 74f24ba75e73c8ed2a02d1c7e33363bcc80b91d2 Mon Sep 17 00:00:00 2001 From: ndycode Date: Sat, 14 Mar 2026 02:20:53 +0800 Subject: [PATCH 01/43] Disable invalid account groups instead of removing them --- index.ts | 56 +++++++++++++++---- lib/accounts.ts | 22 ++++++++ lib/constants.ts | 4 +- test/accounts.test.ts | 57 ++++++++++++++++++++ test/index.test.ts | 122 ++++++++++++++++++++++++++++++++++++++++-- 5 files changed, 243 insertions(+), 18 deletions(-) diff --git a/index.ts b/index.ts index e29c9179..519e1210 100644 --- a/index.ts +++ b/index.ts @@ -471,15 +471,27 @@ export const OpenAIOAuthPlugin: Plugin = async ({ client }: PluginInput) => { ); }; + type PersistAccountPoolOptions = { + reviveMatchingDisabledAccounts?: boolean; + }; + const persistAccountPool = async ( results: TokenSuccessWithAccount[], replaceAll: boolean = false, + options: PersistAccountPoolOptions = {}, ): Promise => { if (results.length === 0) return; await withAccountStorageTransaction(async (loadedStorage, persist) => { const now = Date.now(); const stored = replaceAll ? null : loadedStorage; let accounts = stored?.accounts ? [...stored.accounts] : []; + const refreshTokensToRevive = options.reviveMatchingDisabledAccounts + ? new Set( + results + .map((result) => result.refresh.trim()) + .filter((refreshToken) => refreshToken.length > 0), + ) + : null; const pushIndex = ( map: Map, @@ -1005,6 +1017,22 @@ export const OpenAIOAuthPlugin: Plugin = async ({ client }: PluginInput) => { pruneRefreshTokenCollisions(); + if (refreshTokensToRevive && refreshTokensToRevive.size > 0) { + accounts = accounts.map((account) => { + const refreshToken = account.refreshToken?.trim(); + if (!refreshToken || !refreshTokensToRevive.has(refreshToken)) { + return account; + } + + return { + ...account, + enabled: undefined, + coolingDownUntil: undefined, + cooldownReason: undefined, + }; + }); + } + if (accounts.length === 0) return; const resolveIndexByIdentityKeys = (identityKeys: string[] | undefined): number | undefined => { @@ -2161,11 +2189,11 @@ while (attempted.size < Math.max(1, accountCount)) { const failures = accountManager.incrementAuthFailures(account); const accountLabel = formatAccountLabel(account, account.index); - if (failures >= ACCOUNT_LIMITS.MAX_AUTH_FAILURES_BEFORE_REMOVAL) { - const removedCount = accountManager.removeAccountsWithSameRefreshToken(account); - if (removedCount <= 0) { + if (failures >= ACCOUNT_LIMITS.MAX_AUTH_FAILURES_BEFORE_DISABLE) { + const disabledCount = accountManager.disableAccountsWithSameRefreshToken(account); + if (disabledCount <= 0) { logWarn( - `[${PLUGIN_NAME}] Expected grouped account removal after auth failures, but removed ${removedCount}.`, + `[${PLUGIN_NAME}] Expected grouped account disable after auth failures, but disabled ${disabledCount}.`, ); const cooledCount = accountManager.markAccountsWithRefreshTokenCoolingDown( account.refreshToken, @@ -2181,11 +2209,11 @@ while (attempted.size < Math.max(1, accountCount)) { continue; } accountManager.saveToDiskDebounced(); - const removalMessage = removedCount > 1 - ? `Removed ${removedCount} accounts (same refresh token) after ${failures} consecutive auth failures. Run 'opencode auth login' to re-add.` - : `Removed ${accountLabel} after ${failures} consecutive auth failures. Run 'opencode auth login' to re-add.`; + const disableMessage = disabledCount > 1 + ? `Disabled ${disabledCount} accounts (same refresh token) after ${failures} consecutive auth failures. Run 'opencode auth login' to re-enable.` + : `Disabled ${accountLabel} after ${failures} consecutive auth failures. Run 'opencode auth login' to re-enable.`; await showToast( - removalMessage, + disableMessage, "error", { duration: toastDurationMs * 2 }, ); @@ -3507,7 +3535,9 @@ while (attempted.size < Math.max(1, accountCount)) { const { pkce, state, url } = await createAuthorizationFlow(); return buildManualOAuthFlow(pkce, url, state, async (selection) => { try { - await persistAccountPool(selection.variantsForPersistence, startFresh); + await persistAccountPool(selection.variantsForPersistence, startFresh, { + reviveMatchingDisabledAccounts: true, + }); invalidateAccountManagerCache(); } catch (err) { const storagePath = getStoragePath(); @@ -3582,7 +3612,9 @@ while (attempted.size < Math.max(1, accountCount)) { const isFirstAccount = accounts.length === 1; const entriesToPersist = variantsForPersistence.length > 0 ? variantsForPersistence : [resolved]; - await persistAccountPool(entriesToPersist, isFirstAccount && startFresh); + await persistAccountPool(entriesToPersist, isFirstAccount && startFresh, { + reviveMatchingDisabledAccounts: true, + }); invalidateAccountManagerCache(); } catch (err) { const storagePath = getStoragePath(); @@ -3667,7 +3699,9 @@ while (attempted.size < Math.max(1, accountCount)) { const { pkce, state, url } = await createAuthorizationFlow(); return buildManualOAuthFlow(pkce, url, state, async (selection) => { try { - await persistAccountPool(selection.variantsForPersistence, false); + await persistAccountPool(selection.variantsForPersistence, false, { + reviveMatchingDisabledAccounts: true, + }); } catch (err) { const storagePath = getStoragePath(); const errorCode = (err as NodeJS.ErrnoException)?.code || "UNKNOWN"; diff --git a/lib/accounts.ts b/lib/accounts.ts index 286bc6f2..f494b1d4 100644 --- a/lib/accounts.ts +++ b/lib/accounts.ts @@ -872,6 +872,28 @@ export class AccountManager { return this.removeAccount(account); } + /** + * Disable all accounts that share the same refreshToken as the given account. + * This keeps org/workspace variants visible in the pool while preventing reuse. + * @returns Number of accounts newly disabled + */ + disableAccountsWithSameRefreshToken(account: ManagedAccount): number { + const refreshToken = account.refreshToken; + let disabledCount = 0; + + for (const accountToDisable of this.accounts) { + if (accountToDisable.refreshToken !== refreshToken) continue; + if (accountToDisable.enabled === false) continue; + accountToDisable.enabled = false; + disabledCount++; + } + + // Clear stale auth failure state for this refresh token once the group is disabled. + this.authFailuresByRefreshToken.delete(refreshToken); + + return disabledCount; + } + /** * Remove all accounts that share the same refreshToken as the given account. * This is used when auth refresh fails to remove all org variants together. diff --git a/lib/constants.ts b/lib/constants.ts index 69e7f5bf..a601b7a4 100644 --- a/lib/constants.ts +++ b/lib/constants.ts @@ -89,6 +89,6 @@ export const ACCOUNT_LIMITS = { MAX_ACCOUNTS: 20, /** Cooldown period (ms) after auth failure before retrying account */ AUTH_FAILURE_COOLDOWN_MS: 30_000, - /** Number of consecutive auth failures before auto-removing account */ - MAX_AUTH_FAILURES_BEFORE_REMOVAL: 3, + /** Number of consecutive auth failures before auto-disabling account */ + MAX_AUTH_FAILURES_BEFORE_DISABLE: 3, } as const; diff --git a/test/accounts.test.ts b/test/accounts.test.ts index 68e6da9c..ecb8c960 100644 --- a/test/accounts.test.ts +++ b/test/accounts.test.ts @@ -856,6 +856,32 @@ describe("AccountManager", () => { expect(manager.getAccountsSnapshot()[0].refreshToken).toBe("token-2"); }); + it("disables all accounts with the same refreshToken", () => { + const now = Date.now(); + const stored = { + version: 3 as const, + activeIndex: 0, + accounts: [ + { refreshToken: "token-1", addedAt: now, lastUsed: now }, + { refreshToken: "token-1", organizationId: "org-1", addedAt: now, lastUsed: now }, + { refreshToken: "token-1", organizationId: "org-2", addedAt: now, lastUsed: now }, + { refreshToken: "token-2", addedAt: now, lastUsed: now }, + ], + }; + + const manager = new AccountManager(undefined, stored); + const accounts = manager.getAccountsSnapshot(); + expect(accounts).toHaveLength(4); + + const disabledCount = manager.disableAccountsWithSameRefreshToken(accounts[0]); + expect(disabledCount).toBe(3); + expect(manager.getAccountCount()).toBe(4); + + const updated = manager.getAccountsSnapshot(); + expect(updated.slice(0, 3).every((account) => account.enabled === false)).toBe(true); + expect(updated[3]?.enabled).not.toBe(false); + }); + it("clears auth failure counter when removing accounts with same refreshToken", () => { const now = Date.now(); const stored = { @@ -891,6 +917,37 @@ describe("AccountManager", () => { expect(failuresByRefreshToken.has("token-1")).toBe(false); expect(manager.incrementAuthFailures(account1)).toBe(1); }); + + it("clears auth failure counter when disabling accounts with same refreshToken", () => { + const now = Date.now(); + const stored = { + version: 3 as const, + activeIndex: 0, + accounts: [ + { refreshToken: "token-1", addedAt: now, lastUsed: now }, + { refreshToken: "token-1", organizationId: "org-1", addedAt: now, lastUsed: now }, + ], + }; + + const manager = new AccountManager(undefined, stored); + const accounts = manager.getAccountsSnapshot(); + expect(accounts).toHaveLength(2); + + expect(manager.incrementAuthFailures(accounts[0])).toBe(1); + expect(manager.incrementAuthFailures(accounts[1])).toBe(2); + expect(manager.incrementAuthFailures(accounts[0])).toBe(3); + + const disabledCount = manager.disableAccountsWithSameRefreshToken(accounts[0]); + expect(disabledCount).toBe(2); + expect(manager.getAccountsSnapshot().every((account) => account.enabled === false)).toBe(true); + + const failuresByRefreshToken = Reflect.get( + manager, + "authFailuresByRefreshToken", + ) as Map; + expect(failuresByRefreshToken.has("token-1")).toBe(false); + expect(manager.incrementAuthFailures(accounts[0])).toBe(1); + }); }); describe("getMinWaitTimeForFamily", () => { diff --git a/test/index.test.ts b/test/index.test.ts index daf55c6c..e65200c4 100644 --- a/test/index.test.ts +++ b/test/index.test.ts @@ -344,6 +344,7 @@ vi.mock("../lib/accounts.js", () => { refundToken() {} markSwitched() {} removeAccount() {} + disableAccountsWithSameRefreshToken() { return 1; } removeAccountsWithSameRefreshToken() { return 1; } getMinWaitTimeForFamily() { @@ -2038,7 +2039,7 @@ describe("OpenAIOAuthPlugin fetch handler", () => { expect(await response.text()).toContain("server errors or auth issues"); }); - it("cools down the account when grouped auth removal removes zero entries", async () => { + it("disables grouped accounts when auth failures hit the threshold", async () => { const fetchHelpers = await import("../lib/request/fetch-helpers.js"); const { AccountManager } = await import("../lib/accounts.js"); const { ACCOUNT_LIMITS } = await import("../lib/constants.js"); @@ -2049,9 +2050,56 @@ describe("OpenAIOAuthPlugin fetch handler", () => { ); const incrementAuthFailuresSpy = vi .spyOn(AccountManager.prototype, "incrementAuthFailures") - .mockReturnValue(ACCOUNT_LIMITS.MAX_AUTH_FAILURES_BEFORE_REMOVAL); - const removeGroupedAccountsSpy = vi - .spyOn(AccountManager.prototype, "removeAccountsWithSameRefreshToken") + .mockReturnValue(ACCOUNT_LIMITS.MAX_AUTH_FAILURES_BEFORE_DISABLE); + const disableGroupedAccountsSpy = vi + .spyOn(AccountManager.prototype, "disableAccountsWithSameRefreshToken") + .mockReturnValue(2); + vi.spyOn(AccountManager.prototype, "getCurrentOrNextForFamilyHybrid") + .mockImplementationOnce(() => ({ + index: 0, + email: "user1@example.com", + refreshToken: "refresh-1", + }) as never) + .mockImplementation(() => null); + + globalThis.fetch = vi.fn().mockResolvedValue( + new Response(JSON.stringify({ content: "should-not-fetch" }), { status: 200 }), + ); + + const { sdk, mockClient } = await setupPlugin(); + const response = await sdk.fetch!("https://api.openai.com/v1/chat", { + method: "POST", + body: JSON.stringify({ model: "gpt-5.1" }), + }); + + expect(response.status).toBe(503); + expect(globalThis.fetch).not.toHaveBeenCalled(); + expect(incrementAuthFailuresSpy).toHaveBeenCalledTimes(1); + expect(disableGroupedAccountsSpy).toHaveBeenCalledTimes(1); + expect(mockClient.tui.showToast).toHaveBeenCalledWith( + expect.objectContaining({ + body: expect.objectContaining({ + message: expect.stringContaining("Disabled 2 accounts"), + variant: "error", + }), + }), + ); + }); + + it("cools down the account when grouped auth disable updates zero entries", async () => { + const fetchHelpers = await import("../lib/request/fetch-helpers.js"); + const { AccountManager } = await import("../lib/accounts.js"); + const { ACCOUNT_LIMITS } = await import("../lib/constants.js"); + + vi.spyOn(fetchHelpers, "shouldRefreshToken").mockReturnValue(true); + vi.mocked(fetchHelpers.refreshAndUpdateToken).mockRejectedValue( + new Error("Token expired"), + ); + const incrementAuthFailuresSpy = vi + .spyOn(AccountManager.prototype, "incrementAuthFailures") + .mockReturnValue(ACCOUNT_LIMITS.MAX_AUTH_FAILURES_BEFORE_DISABLE); + const disableGroupedAccountsSpy = vi + .spyOn(AccountManager.prototype, "disableAccountsWithSameRefreshToken") .mockReturnValue(0); const markAccountsWithRefreshTokenCoolingDownSpy = vi.spyOn( AccountManager.prototype, @@ -2071,7 +2119,7 @@ describe("OpenAIOAuthPlugin fetch handler", () => { expect(response.status).toBe(503); expect(globalThis.fetch).not.toHaveBeenCalled(); expect(incrementAuthFailuresSpy).toHaveBeenCalledTimes(1); - expect(removeGroupedAccountsSpy).toHaveBeenCalledTimes(1); + expect(disableGroupedAccountsSpy).toHaveBeenCalledTimes(1); expect(markAccountsWithRefreshTokenCoolingDownSpy).toHaveBeenCalledWith( "refresh-1", ACCOUNT_LIMITS.AUTH_FAILURE_COOLDOWN_MS, @@ -3126,6 +3174,70 @@ describe("OpenAIOAuthPlugin persistAccountPool", () => { expect(mergedOrg?.cooldownReason).toBe("auth-failure"); }); + it("re-enables disabled same-refresh-token variants on explicit login", async () => { + const accountsModule = await import("../lib/accounts.js"); + const authModule = await import("../lib/auth/auth.js"); + + mockStorage.accounts = [ + { + accountId: "org-shared", + organizationId: "org-keep", + email: "org@example.com", + refreshToken: "shared-refresh", + enabled: false, + coolingDownUntil: 12_000, + cooldownReason: "auth-failure", + addedAt: 10, + lastUsed: 10, + }, + { + accountId: "org-sibling", + organizationId: "org-sibling", + email: "sibling@example.com", + refreshToken: "shared-refresh", + enabled: false, + coolingDownUntil: 15_000, + cooldownReason: "auth-failure", + addedAt: 11, + lastUsed: 11, + }, + ]; + + vi.mocked(authModule.exchangeAuthorizationCode).mockResolvedValueOnce({ + type: "success", + access: "access-shared-refresh", + refresh: "shared-refresh", + expires: Date.now() + 300_000, + idToken: "id-shared-refresh", + }); + vi.mocked(accountsModule.getAccountIdCandidates).mockReturnValueOnce([]); + vi.mocked(accountsModule.extractAccountId).mockImplementation((accessToken) => + accessToken === "access-shared-refresh" ? "org-shared" : "account-1", + ); + + const mockClient = createMockClient(); + const { OpenAIOAuthPlugin } = await import("../index.js"); + const plugin = (await OpenAIOAuthPlugin({ + client: mockClient, + } as never)) as unknown as PluginType; + const autoMethod = plugin.auth.methods[0] as unknown as { + authorize: (inputs?: Record) => Promise<{ instructions: string }>; + }; + + await autoMethod.authorize({ loginMode: "add", accountCount: "1" }); + + const revivedEntries = mockStorage.accounts.filter( + (account) => account.refreshToken === "shared-refresh", + ); + expect(revivedEntries).toHaveLength(2); + expect(revivedEntries.every((account) => account.enabled !== false)).toBe(true); + expect(revivedEntries.every((account) => account.coolingDownUntil === undefined)).toBe(true); + expect(revivedEntries.every((account) => account.cooldownReason === undefined)).toBe(true); + expect( + revivedEntries.find((account) => account.accountId === "org-shared")?.accessToken, + ).toBe("access-shared-refresh"); + }); + it("preserves same-organization entries when accountId differs", async () => { const accountsModule = await import("../lib/accounts.js"); const authModule = await import("../lib/auth/auth.js"); From 5a5d9c7445cbbe08de9d394254272475fcf90cb6 Mon Sep 17 00:00:00 2001 From: ndycode Date: Sat, 14 Mar 2026 23:51:43 +0800 Subject: [PATCH 02/43] Preserve manual disables when reviving accounts --- index.ts | 26 +++++- lib/accounts.ts | 8 ++ lib/schemas.ts | 6 ++ lib/storage.ts | 19 ++++- lib/storage/migrations.ts | 4 + test/accounts.test.ts | 46 +++++++++++ test/index.test.ts | 167 ++++++++++++++++++++++++++++++++++++-- 7 files changed, 266 insertions(+), 10 deletions(-) diff --git a/index.ts b/index.ts index 519e1210..2278ff08 100644 --- a/index.ts +++ b/index.ts @@ -118,6 +118,7 @@ import { loadFlaggedAccounts, saveFlaggedAccounts, clearFlaggedAccounts, + type AccountDisabledReason, StorageError, formatStorageErrorHint, type AccountStorageV3, @@ -560,6 +561,19 @@ export const OpenAIOAuthPlugin: Plugin = async ({ client }: PluginInput) => { target.enabled === false || source.enabled === false ? false : target.enabled ?? source.enabled; + const mergedDisabledReason = (() => { + if (mergedEnabled !== false) { + return undefined; + } + const reasons = [target.disabledReason, source.disabledReason]; + if (reasons.includes("user")) { + return "user" satisfies AccountDisabledReason; + } + if (reasons.includes("auth-failure")) { + return "auth-failure" satisfies AccountDisabledReason; + } + return undefined; + })(); const targetCoolingDownUntil = typeof target.coolingDownUntil === "number" && Number.isFinite(target.coolingDownUntil) ? target.coolingDownUntil @@ -597,6 +611,7 @@ export const OpenAIOAuthPlugin: Plugin = async ({ client }: PluginInput) => { accessToken: newer.accessToken || older.accessToken, expiresAt: newer.expiresAt ?? older.expiresAt, enabled: mergedEnabled, + disabledReason: mergedDisabledReason, addedAt: Math.max(target.addedAt ?? 0, source.addedAt ?? 0), lastUsed: Math.max(target.lastUsed ?? 0, source.lastUsed ?? 0), lastSwitchReason: target.lastSwitchReason ?? source.lastSwitchReason, @@ -1020,13 +1035,18 @@ export const OpenAIOAuthPlugin: Plugin = async ({ client }: PluginInput) => { if (refreshTokensToRevive && refreshTokensToRevive.size > 0) { accounts = accounts.map((account) => { const refreshToken = account.refreshToken?.trim(); - if (!refreshToken || !refreshTokensToRevive.has(refreshToken)) { + if ( + !refreshToken || + !refreshTokensToRevive.has(refreshToken) || + account.disabledReason !== "auth-failure" + ) { return account; } return { ...account, enabled: undefined, + disabledReason: undefined, coolingDownUntil: undefined, cooldownReason: undefined, }; @@ -3465,7 +3485,9 @@ while (attempted.size < Math.max(1, accountCount)) { if (typeof menuResult.toggleAccountIndex === "number") { const target = workingStorage.accounts[menuResult.toggleAccountIndex]; if (target) { - target.enabled = target.enabled === false ? true : false; + const shouldEnable = target.enabled === false; + target.enabled = shouldEnable ? true : false; + target.disabledReason = shouldEnable ? undefined : "user"; await saveAccounts(workingStorage); invalidateAccountManagerCache(); console.log( diff --git a/lib/accounts.ts b/lib/accounts.ts index f494b1d4..73c83d18 100644 --- a/lib/accounts.ts +++ b/lib/accounts.ts @@ -6,6 +6,7 @@ import { createLogger } from "./logger.js"; import { loadAccounts, saveAccounts, + type AccountDisabledReason, type AccountStorageV3, type CooldownReason, type RateLimitStateV3, @@ -181,6 +182,7 @@ export interface ManagedAccount { email?: string; refreshToken: string; enabled?: boolean; + disabledReason?: AccountDisabledReason; access?: string; expires?: number; addedAt: number; @@ -309,6 +311,7 @@ export class AccountManager { : sanitizeEmail(account.email), refreshToken, enabled: account.enabled !== false, + disabledReason: account.enabled === false ? account.disabledReason : undefined, access: matchesFallback && authFallback ? authFallback.access : account.accessToken, expires: matchesFallback && authFallback ? authFallback.expires : account.expiresAt, addedAt: clampNonNegativeInt(account.addedAt, baseNow), @@ -885,6 +888,7 @@ export class AccountManager { if (accountToDisable.refreshToken !== refreshToken) continue; if (accountToDisable.enabled === false) continue; accountToDisable.enabled = false; + accountToDisable.disabledReason = "auth-failure"; disabledCount++; } @@ -923,6 +927,9 @@ export class AccountManager { const account = this.accounts[index]; if (!account) return null; account.enabled = enabled; + if (enabled) { + delete account.disabledReason; + } return account; } @@ -949,6 +956,7 @@ export class AccountManager { accessToken: account.access, expiresAt: account.expires, enabled: account.enabled === false ? false : undefined, + disabledReason: account.enabled === false ? account.disabledReason : undefined, addedAt: account.addedAt, lastUsed: account.lastUsed, lastSwitchReason: account.lastSwitchReason, diff --git a/lib/schemas.ts b/lib/schemas.ts index 6028246d..2ba4c1ff 100644 --- a/lib/schemas.ts +++ b/lib/schemas.ts @@ -74,6 +74,10 @@ export const CooldownReasonSchema = z.enum(["auth-failure", "network-error"]); export type CooldownReasonFromSchema = z.infer; +export const DisabledReasonSchema = z.enum(["user", "auth-failure"]); + +export type DisabledReasonFromSchema = z.infer; + /** * Last switch reason for account rotation tracking. */ @@ -117,6 +121,7 @@ export const AccountMetadataV3Schema = z.object({ accessToken: z.string().optional(), expiresAt: z.number().optional(), enabled: z.boolean().optional(), + disabledReason: DisabledReasonSchema.optional(), addedAt: z.number(), lastUsed: z.number(), lastSwitchReason: SwitchReasonSchema.optional(), @@ -164,6 +169,7 @@ export const AccountMetadataV1Schema = z.object({ accessToken: z.string().optional(), expiresAt: z.number().optional(), enabled: z.boolean().optional(), + disabledReason: DisabledReasonSchema.optional(), addedAt: z.number(), lastUsed: z.number(), lastSwitchReason: SwitchReasonSchema.optional(), diff --git a/lib/storage.ts b/lib/storage.ts index 151e2213..c60f9ddb 100644 --- a/lib/storage.ts +++ b/lib/storage.ts @@ -8,6 +8,7 @@ import { AnyAccountStorageSchema, getValidationErrors } from "./schemas.js"; import { getConfigDir, getProjectConfigDir, getProjectGlobalConfigDir, findProjectRoot, resolvePath } from "./storage/paths.js"; import { migrateV1ToV3, + type AccountDisabledReason, type CooldownReason, type RateLimitStateV3, type AccountMetadataV1, @@ -16,7 +17,15 @@ import { type AccountStorageV3, } from "./storage/migrations.js"; -export type { CooldownReason, RateLimitStateV3, AccountMetadataV1, AccountStorageV1, AccountMetadataV3, AccountStorageV3 }; +export type { + AccountDisabledReason, + CooldownReason, + RateLimitStateV3, + AccountMetadataV1, + AccountStorageV1, + AccountMetadataV3, + AccountStorageV3, +}; const log = createLogger("storage"); const ACCOUNTS_FILE_NAME = "openai-codex-accounts.json"; @@ -941,6 +950,10 @@ function normalizeFlaggedStorage(data: unknown): FlaggedAccountStorageV1 { value: unknown, ): value is AccountMetadataV3["cooldownReason"] => value === "auth-failure" || value === "network-error"; + const isDisabledReason = ( + value: unknown, + ): value is AccountDisabledReason => + value === "user" || value === "auth-failure"; const normalizeTags = (value: unknown): string[] | undefined => { if (!Array.isArray(value)) return undefined; const normalized = value @@ -972,6 +985,9 @@ function normalizeFlaggedStorage(data: unknown): FlaggedAccountStorageV1 { const cooldownReason = isCooldownReason(rawAccount.cooldownReason) ? rawAccount.cooldownReason : undefined; + const disabledReason = isDisabledReason(rawAccount.disabledReason) + ? rawAccount.disabledReason + : undefined; const accountTags = normalizeTags(rawAccount.accountTags); const accountNote = typeof rawAccount.accountNote === "string" && rawAccount.accountNote.trim() @@ -991,6 +1007,7 @@ function normalizeFlaggedStorage(data: unknown): FlaggedAccountStorageV1 { accountNote, email: typeof rawAccount.email === "string" ? rawAccount.email : undefined, enabled: typeof rawAccount.enabled === "boolean" ? rawAccount.enabled : undefined, + disabledReason, lastSwitchReason, rateLimitResetTimes, coolingDownUntil: diff --git a/lib/storage/migrations.ts b/lib/storage/migrations.ts index 092782cd..53389a34 100644 --- a/lib/storage/migrations.ts +++ b/lib/storage/migrations.ts @@ -7,6 +7,7 @@ import { MODEL_FAMILIES, type ModelFamily } from "../prompts/codex.js"; import type { AccountIdSource } from "../types.js"; export type CooldownReason = "auth-failure" | "network-error"; +export type AccountDisabledReason = "user" | "auth-failure"; export interface RateLimitStateV3 { [key: string]: number | undefined; @@ -26,6 +27,7 @@ export interface AccountMetadataV1 { /** Optional access token expiry timestamp (ms since epoch). */ expiresAt?: number; enabled?: boolean; + disabledReason?: AccountDisabledReason; addedAt: number; lastUsed: number; lastSwitchReason?: "rate-limit" | "initial" | "rotation"; @@ -54,6 +56,7 @@ export interface AccountMetadataV3 { /** Optional access token expiry timestamp (ms since epoch). */ expiresAt?: number; enabled?: boolean; + disabledReason?: AccountDisabledReason; addedAt: number; lastUsed: number; lastSwitchReason?: "rate-limit" | "initial" | "rotation"; @@ -96,6 +99,7 @@ export function migrateV1ToV3(v1: AccountStorageV1): AccountStorageV3 { accessToken: account.accessToken, expiresAt: account.expiresAt, enabled: account.enabled, + disabledReason: account.enabled === false ? account.disabledReason : undefined, addedAt: account.addedAt, lastUsed: account.lastUsed, lastSwitchReason: account.lastSwitchReason, diff --git a/test/accounts.test.ts b/test/accounts.test.ts index ecb8c960..3115cffe 100644 --- a/test/accounts.test.ts +++ b/test/accounts.test.ts @@ -879,9 +879,52 @@ describe("AccountManager", () => { const updated = manager.getAccountsSnapshot(); expect(updated.slice(0, 3).every((account) => account.enabled === false)).toBe(true); + expect(updated.slice(0, 3).every((account) => account.disabledReason === "auth-failure")).toBe(true); expect(updated[3]?.enabled).not.toBe(false); }); + it("preserves a manual disable reason when auth-failure disabling sibling variants", () => { + const now = Date.now(); + const stored = { + version: 3 as const, + activeIndex: 0, + accounts: [ + { refreshToken: "token-1", accountId: "workspace-a", addedAt: now, lastUsed: now }, + { + refreshToken: "token-1", + accountId: "workspace-user-disabled", + enabled: false, + disabledReason: "user" as const, + addedAt: now, + lastUsed: now, + }, + { refreshToken: "token-1", accountId: "workspace-b", addedAt: now, lastUsed: now }, + ], + }; + + const manager = new AccountManager(undefined, stored); + const accounts = manager.getAccountsSnapshot(); + const disabledCount = manager.disableAccountsWithSameRefreshToken(accounts[0]); + + expect(disabledCount).toBe(2); + const updated = manager.getAccountsSnapshot(); + expect(updated[0]).toMatchObject({ + accountId: "workspace-a", + enabled: false, + disabledReason: "auth-failure", + }); + expect(updated[1]).toMatchObject({ + accountId: "workspace-user-disabled", + enabled: false, + disabledReason: "user", + }); + expect(updated[2]).toMatchObject({ + accountId: "workspace-b", + enabled: false, + disabledReason: "auth-failure", + }); + }); + it("clears auth failure counter when removing accounts with same refreshToken", () => { const now = Date.now(); const stored = { @@ -940,6 +983,9 @@ describe("AccountManager", () => { const disabledCount = manager.disableAccountsWithSameRefreshToken(accounts[0]); expect(disabledCount).toBe(2); expect(manager.getAccountsSnapshot().every((account) => account.enabled === false)).toBe(true); + expect( + manager.getAccountsSnapshot().every((account) => account.disabledReason === "auth-failure"), + ).toBe(true); const failuresByRefreshToken = Reflect.get( manager, diff --git a/test/index.test.ts b/test/index.test.ts index e65200c4..428cde97 100644 --- a/test/index.test.ts +++ b/test/index.test.ts @@ -204,6 +204,7 @@ const mockStorage = { accessToken?: string; expiresAt?: number; enabled?: boolean; + disabledReason?: string; addedAt?: number; lastUsed?: number; coolingDownUntil?: number; @@ -3170,11 +3171,78 @@ describe("OpenAIOAuthPlugin persistAccountPool", () => { const mergedOrg = mergedOrgEntries[0]; expect(mergedOrg?.accountId).toBe("org-shared"); expect(mergedOrg?.enabled).toBe(false); + expect(mergedOrg?.disabledReason).toBeUndefined(); expect(mergedOrg?.coolingDownUntil).toBe(12_000); expect(mergedOrg?.cooldownReason).toBe("auth-failure"); }); - it("re-enables disabled same-refresh-token variants on explicit login", async () => { + it("preserves manual disable reason over auth-failure when collapsing same-organization duplicates", async () => { + const accountsModule = await import("../lib/accounts.js"); + const authModule = await import("../lib/auth/auth.js"); + + mockStorage.accounts = [ + { + accountId: "org-shared", + organizationId: "org-keep", + email: "manual@example.com", + refreshToken: "shared-refresh", + enabled: false, + disabledReason: "user", + addedAt: 10, + lastUsed: 10, + }, + { + accountId: "org-shared", + organizationId: "org-keep", + email: "auth-failure@example.com", + refreshToken: "shared-refresh", + enabled: false, + disabledReason: "auth-failure", + coolingDownUntil: 12_000, + cooldownReason: "auth-failure", + addedAt: 20, + lastUsed: 20, + }, + ]; + + vi.mocked(authModule.exchangeAuthorizationCode).mockResolvedValueOnce({ + type: "success", + access: "access-unrelated-user-disabled", + refresh: "refresh-unrelated-user-disabled", + expires: Date.now() + 300_000, + idToken: "id-unrelated-user-disabled", + }); + vi.mocked(accountsModule.getAccountIdCandidates).mockReturnValueOnce([]); + vi.mocked(accountsModule.extractAccountId).mockImplementation((accessToken) => + accessToken === "access-unrelated-user-disabled" ? "unrelated-user-disabled" : "account-1", + ); + + const mockClient = createMockClient(); + const { OpenAIOAuthPlugin } = await import("../index.js"); + const plugin = (await OpenAIOAuthPlugin({ + client: mockClient, + } as never)) as unknown as PluginType; + const autoMethod = plugin.auth.methods[0] as unknown as { + authorize: (inputs?: Record) => Promise<{ instructions: string }>; + }; + + await autoMethod.authorize({ loginMode: "add", accountCount: "1" }); + + const mergedOrgEntries = mockStorage.accounts.filter( + (account) => account.organizationId === "org-keep", + ); + expect(mergedOrgEntries).toHaveLength(1); + const mergedOrg = mergedOrgEntries[0]; + expect(mergedOrg).toMatchObject({ + accountId: "org-shared", + enabled: false, + disabledReason: "user", + coolingDownUntil: 12_000, + cooldownReason: "auth-failure", + }); + }); + + it("preserves user-disabled and legacy-disabled variants while reviving auth-failure disables on explicit login", async () => { const accountsModule = await import("../lib/accounts.js"); const authModule = await import("../lib/auth/auth.js"); @@ -3185,6 +3253,7 @@ describe("OpenAIOAuthPlugin persistAccountPool", () => { email: "org@example.com", refreshToken: "shared-refresh", enabled: false, + disabledReason: "auth-failure", coolingDownUntil: 12_000, cooldownReason: "auth-failure", addedAt: 10, @@ -3196,11 +3265,21 @@ describe("OpenAIOAuthPlugin persistAccountPool", () => { email: "sibling@example.com", refreshToken: "shared-refresh", enabled: false, - coolingDownUntil: 15_000, - cooldownReason: "auth-failure", + disabledReason: "user", addedAt: 11, lastUsed: 11, }, + { + accountId: "org-legacy", + organizationId: "org-legacy", + email: "legacy@example.com", + refreshToken: "shared-refresh", + enabled: false, + coolingDownUntil: 18_000, + cooldownReason: "auth-failure", + addedAt: 12, + lastUsed: 12, + }, ]; vi.mocked(authModule.exchangeAuthorizationCode).mockResolvedValueOnce({ @@ -3229,10 +3308,34 @@ describe("OpenAIOAuthPlugin persistAccountPool", () => { const revivedEntries = mockStorage.accounts.filter( (account) => account.refreshToken === "shared-refresh", ); - expect(revivedEntries).toHaveLength(2); - expect(revivedEntries.every((account) => account.enabled !== false)).toBe(true); - expect(revivedEntries.every((account) => account.coolingDownUntil === undefined)).toBe(true); - expect(revivedEntries.every((account) => account.cooldownReason === undefined)).toBe(true); + expect(revivedEntries).toHaveLength(3); + expect( + revivedEntries.find((account) => account.accountId === "org-shared"), + ).toMatchObject({ + accountId: "org-shared", + enabled: undefined, + disabledReason: undefined, + coolingDownUntil: undefined, + cooldownReason: undefined, + }); + expect( + revivedEntries.find((account) => account.accountId === "org-sibling"), + ).toMatchObject({ + accountId: "org-sibling", + enabled: false, + disabledReason: "user", + }); + expect( + revivedEntries.find((account) => account.accountId === "org-legacy"), + ).toMatchObject({ + accountId: "org-legacy", + enabled: false, + coolingDownUntil: 18_000, + cooldownReason: "auth-failure", + }); + expect( + revivedEntries.find((account) => account.accountId === "org-legacy"), + ).not.toHaveProperty("disabledReason"); expect( revivedEntries.find((account) => account.accountId === "org-shared")?.accessToken, ).toBe("access-shared-refresh"); @@ -3425,6 +3528,56 @@ describe("OpenAIOAuthPlugin persistAccountPool", () => { accounts: [], }); }); + + it("writes user disable metadata from the auth manage menu and clears it on manual re-enable", async () => { + const cliModule = await import("../lib/cli.js"); + const storageModule = await import("../lib/storage.js"); + + mockStorage.accounts = [ + { + accountId: "workspace-managed", + email: "managed@example.com", + refreshToken: "refresh-managed", + addedAt: 10, + lastUsed: 10, + }, + ]; + + vi.mocked(cliModule.promptLoginMode) + .mockResolvedValueOnce({ mode: "manage", toggleAccountIndex: 0 }) + .mockResolvedValueOnce({ mode: "manage", toggleAccountIndex: 0 }) + .mockResolvedValueOnce({ mode: "cancel" }); + + const mockClient = createMockClient(); + const { OpenAIOAuthPlugin } = await import("../index.js"); + const plugin = (await OpenAIOAuthPlugin({ + client: mockClient, + } as never)) as unknown as PluginType; + const autoMethod = plugin.auth.methods[0] as unknown as { + authorize: (inputs?: Record) => Promise<{ instructions: string }>; + }; + + const authResult = await autoMethod.authorize(); + expect(authResult.instructions).toBe("Authentication cancelled"); + + const saveAccountsMock = vi.mocked(storageModule.saveAccounts); + expect(saveAccountsMock).toHaveBeenCalledTimes(2); + expect(saveAccountsMock.mock.calls[0]?.[0].accounts[0]).toMatchObject({ + accountId: "workspace-managed", + enabled: false, + disabledReason: "user", + }); + expect(saveAccountsMock.mock.calls[1]?.[0].accounts[0]).toMatchObject({ + accountId: "workspace-managed", + enabled: true, + disabledReason: undefined, + }); + expect(mockStorage.accounts[0]).toMatchObject({ + accountId: "workspace-managed", + enabled: true, + disabledReason: undefined, + }); + }); }); describe("OpenAIOAuthPlugin showToast error handling", () => { From a5df174e330764aaec30a0d88928c0fe7f1c5862 Mon Sep 17 00:00:00 2001 From: ndycode Date: Sun, 15 Mar 2026 00:09:56 +0800 Subject: [PATCH 03/43] Fix remaining auth disable review gaps --- index.ts | 4 ++- lib/accounts.ts | 8 +++++- lib/storage.ts | 2 +- lib/storage/migrations.ts | 5 +++- test/accounts.test.ts | 25 +++++++++++++++++ test/index.test.ts | 57 ++++++++++++++++++++++++++++++++------- test/storage.test.ts | 24 +++++++++++++++++ 7 files changed, 111 insertions(+), 14 deletions(-) diff --git a/index.ts b/index.ts index 2278ff08..8b053719 100644 --- a/index.ts +++ b/index.ts @@ -3371,7 +3371,9 @@ while (attempted.size < Math.max(1, accountCount)) { } if (restored.length > 0) { - await persistAccountPool(restored, false); + await persistAccountPool(restored, false, { + reviveMatchingDisabledAccounts: true, + }); invalidateAccountManagerCache(); } diff --git a/lib/accounts.ts b/lib/accounts.ts index 73c83d18..453082ec 100644 --- a/lib/accounts.ts +++ b/lib/accounts.ts @@ -921,7 +921,11 @@ export class AccountManager { return removedCount; } - setAccountEnabled(index: number, enabled: boolean): ManagedAccount | null { + setAccountEnabled( + index: number, + enabled: boolean, + reason?: AccountDisabledReason, + ): ManagedAccount | null { if (!Number.isFinite(index)) return null; if (index < 0 || index >= this.accounts.length) return null; const account = this.accounts[index]; @@ -929,6 +933,8 @@ export class AccountManager { account.enabled = enabled; if (enabled) { delete account.disabledReason; + } else if (reason) { + account.disabledReason = reason; } return account; } diff --git a/lib/storage.ts b/lib/storage.ts index c60f9ddb..1861833b 100644 --- a/lib/storage.ts +++ b/lib/storage.ts @@ -621,7 +621,7 @@ export function normalizeAccountStorage(data: unknown): AccountStorageV3 | null ? migrateV1ToV3(data as unknown as AccountStorageV1) : (data as unknown as AccountStorageV3); - const validAccounts = rawAccounts.filter( + const validAccounts = baseStorage.accounts.filter( (account): account is AccountMetadataV3 => isRecord(account) && typeof account.refreshToken === "string" && !!account.refreshToken.trim(), ); diff --git a/lib/storage/migrations.ts b/lib/storage/migrations.ts index 53389a34..04d3d4ed 100644 --- a/lib/storage/migrations.ts +++ b/lib/storage/migrations.ts @@ -99,7 +99,10 @@ export function migrateV1ToV3(v1: AccountStorageV1): AccountStorageV3 { accessToken: account.accessToken, expiresAt: account.expiresAt, enabled: account.enabled, - disabledReason: account.enabled === false ? account.disabledReason : undefined, + disabledReason: + account.enabled === false + ? account.disabledReason ?? "user" + : undefined, addedAt: account.addedAt, lastUsed: account.lastUsed, lastSwitchReason: account.lastSwitchReason, diff --git a/test/accounts.test.ts b/test/accounts.test.ts index 3115cffe..e49ca0f9 100644 --- a/test/accounts.test.ts +++ b/test/accounts.test.ts @@ -994,6 +994,31 @@ describe("AccountManager", () => { expect(failuresByRefreshToken.has("token-1")).toBe(false); expect(manager.incrementAuthFailures(accounts[0])).toBe(1); }); + + it("records disable reason when setAccountEnabled disables an account", () => { + const now = Date.now(); + const stored = { + version: 3 as const, + activeIndex: 0, + accounts: [ + { refreshToken: "token-1", addedAt: now, lastUsed: now }, + ], + }; + + const manager = new AccountManager(undefined, stored); + + const disabled = manager.setAccountEnabled(0, false, "user"); + expect(disabled).toMatchObject({ + enabled: false, + disabledReason: "user", + }); + + const reenabled = manager.setAccountEnabled(0, true); + expect(reenabled).toMatchObject({ + enabled: true, + }); + expect(reenabled?.disabledReason).toBeUndefined(); + }); }); describe("getMinWaitTimeForFamily", () => { diff --git a/test/index.test.ts b/test/index.test.ts index 428cde97..1f0b55a5 100644 --- a/test/index.test.ts +++ b/test/index.test.ts @@ -3247,6 +3247,16 @@ describe("OpenAIOAuthPlugin persistAccountPool", () => { const authModule = await import("../lib/auth/auth.js"); mockStorage.accounts = [ + { + accountId: "unrelated-disabled", + organizationId: "org-unrelated", + email: "unrelated@example.com", + refreshToken: "different-refresh", + enabled: false, + disabledReason: "user", + addedAt: 9, + lastUsed: 9, + }, { accountId: "org-shared", organizationId: "org-keep", @@ -3339,6 +3349,14 @@ describe("OpenAIOAuthPlugin persistAccountPool", () => { expect( revivedEntries.find((account) => account.accountId === "org-shared")?.accessToken, ).toBe("access-shared-refresh"); + expect( + mockStorage.accounts.find((account) => account.accountId === "unrelated-disabled"), + ).toMatchObject({ + accountId: "unrelated-disabled", + refreshToken: "different-refresh", + enabled: false, + disabledReason: "user", + }); }); it("preserves same-organization entries when accountId differs", async () => { @@ -3442,6 +3460,23 @@ describe("OpenAIOAuthPlugin persistAccountPool", () => { const accountsModule = await import("../lib/accounts.js"); const refreshQueueModule = await import("../lib/refresh-queue.js"); + mockStorage.accounts = [ + { + refreshToken: "refreshed-refresh", + organizationId: "org-refresh", + accountId: "flagged-live", + accountIdSource: "manual", + accountLabel: "Refresh Workspace", + email: "refresh@example.com", + enabled: false, + disabledReason: "auth-failure", + coolingDownUntil: 60_000, + cooldownReason: "auth-failure", + addedAt: Date.now() - 1500, + lastUsed: Date.now() - 1500, + }, + ]; + const flaggedAccounts = [ { refreshToken: "flagged-refresh-cache", @@ -3495,16 +3530,7 @@ describe("OpenAIOAuthPlugin persistAccountPool", () => { } return null; }); - vi.mocked(accountsModule.getAccountIdCandidates).mockReturnValue([ - { - accountId: "token-shared", - source: "token", - label: "Token Shared [id:shared]", - }, - ]); - vi.mocked(accountsModule.selectBestAccountCandidate).mockImplementation( - (candidates) => candidates[0] ?? null, - ); + vi.mocked(accountsModule.getAccountIdCandidates).mockReturnValue([]); const mockClient = createMockClient(); const { OpenAIOAuthPlugin } = await import("../index.js"); @@ -3523,6 +3549,17 @@ describe("OpenAIOAuthPlugin persistAccountPool", () => { expect(new Set(mockStorage.accounts.map((account) => account.organizationId))).toEqual( new Set(["org-cache", "org-refresh"]), ); + expect( + mockStorage.accounts.find((account) => account.organizationId === "org-refresh"), + ).toMatchObject({ + accountId: "flagged-live", + refreshToken: "refreshed-refresh", + enabled: undefined, + disabledReason: undefined, + coolingDownUntil: undefined, + cooldownReason: undefined, + accessToken: "refreshed-access", + }); expect(vi.mocked(storageModule.saveFlaggedAccounts)).toHaveBeenCalledWith({ version: 1, accounts: [], diff --git a/test/storage.test.ts b/test/storage.test.ts index 94d20141..67c578ed 100644 --- a/test/storage.test.ts +++ b/test/storage.test.ts @@ -1050,6 +1050,30 @@ describe("storage", () => { expect(result?.accounts).toHaveLength(1); }); + it("defaults migrated legacy disabled accounts to user disabledReason", () => { + const data = { + version: 1, + activeIndex: 0, + accounts: [ + { + refreshToken: "t1", + accountId: "A", + enabled: false, + addedAt: 1, + lastUsed: 1, + }, + ], + }; + + const result = normalizeAccountStorage(data); + expect(result?.version).toBe(3); + expect(result?.accounts[0]).toMatchObject({ + refreshToken: "t1", + enabled: false, + disabledReason: "user", + }); + }); + it("preserves activeIndexByFamily when valid", () => { const data = { version: 3, From 09eabd3b99ffb39fb2a7acbf4b61f61f7782fe19 Mon Sep 17 00:00:00 2001 From: ndycode Date: Sun, 15 Mar 2026 00:21:16 +0800 Subject: [PATCH 04/43] Harden disabled account persistence and retries --- index.ts | 19 +++++-- lib/accounts.ts | 5 ++ test/accounts.test.ts | 34 ++++++++----- test/index-retry.test.ts | 13 +++++ test/index.test.ts | 105 ++++++++++++++++++++++++++++++++++++++- 5 files changed, 158 insertions(+), 18 deletions(-) diff --git a/index.ts b/index.ts index 8b053719..9e22f315 100644 --- a/index.ts +++ b/index.ts @@ -88,6 +88,7 @@ import { } from "./lib/logger.js"; import { checkAndNotify } from "./lib/auto-update-checker.js"; import { handleContextOverflow } from "./lib/context-overflow.js"; +import { registerCleanup } from "./lib/shutdown.js"; import { AccountManager, type AccountSelectionExplainability, @@ -1680,6 +1681,10 @@ export const OpenAIOAuthPlugin: Plugin = async ({ client }: PluginInput) => { accountManagerPromise = null; }; + registerCleanup(async () => { + await cachedAccountManager?.flushPendingSave(); + }); + // Event handler for session recovery and account selection const eventHandler = async (input: { event: { type: string; properties?: unknown } }) => { try { @@ -2131,7 +2136,7 @@ export const OpenAIOAuthPlugin: Plugin = async ({ client }: PluginInput) => { } while (true) { - let accountCount = accountManager.getAccountCount(); + let accountCount = accountManager.getEnabledAccountCount(); const attempted = new Set(); let restartAccountTraversalWithFallback = false; @@ -2239,7 +2244,7 @@ while (attempted.size < Math.max(1, accountCount)) { ); // Restart traversal: clear attempted and refresh accountCount to avoid skipping healthy accounts attempted.clear(); - accountCount = accountManager.getAccountCount(); + accountCount = accountManager.getEnabledAccountCount(); continue; } @@ -2597,7 +2602,7 @@ while (attempted.size < Math.max(1, accountCount)) { ); if ( - accountManager.getAccountCount() > 1 && + accountManager.getEnabledAccountCount() > 1 && accountManager.shouldShowAccountToast( account.index, rateLimitToastDebounceMs, @@ -2680,7 +2685,7 @@ while (attempted.size < Math.max(1, accountCount)) { } const waitMs = accountManager.getMinWaitTimeForFamily(modelFamily, model); - const count = accountManager.getAccountCount(); + const count = accountManager.getEnabledAccountCount(); if ( retryAllAccountsRateLimited && @@ -3488,6 +3493,12 @@ while (attempted.size < Math.max(1, accountCount)) { const target = workingStorage.accounts[menuResult.toggleAccountIndex]; if (target) { const shouldEnable = target.enabled === false; + if (shouldEnable && target.disabledReason === "auth-failure") { + console.log( + "\nThis account was disabled after repeated auth failures. Run 'opencode auth login' to re-enable with fresh credentials.\n", + ); + continue; + } target.enabled = shouldEnable ? true : false; target.disabledReason = shouldEnable ? undefined : "user"; await saveAccounts(workingStorage); diff --git a/lib/accounts.ts b/lib/accounts.ts index 453082ec..b4ff188b 100644 --- a/lib/accounts.ts +++ b/lib/accounts.ts @@ -395,6 +395,10 @@ export class AccountManager { return this.accounts.length; } + getEnabledAccountCount(): number { + return this.accounts.filter((account) => account.enabled !== false).length; + } + getActiveIndex(): number { return this.getActiveIndexForFamily("codex"); } @@ -886,6 +890,7 @@ export class AccountManager { for (const accountToDisable of this.accounts) { if (accountToDisable.refreshToken !== refreshToken) continue; + this.clearAccountCooldown(accountToDisable); if (accountToDisable.enabled === false) continue; accountToDisable.enabled = false; accountToDisable.disabledReason = "auth-failure"; diff --git a/test/accounts.test.ts b/test/accounts.test.ts index e49ca0f9..249a13b3 100644 --- a/test/accounts.test.ts +++ b/test/accounts.test.ts @@ -953,11 +953,6 @@ describe("AccountManager", () => { const removedCount = manager.removeAccountsWithSameRefreshToken(account1); expect(removedCount).toBe(2); expect(manager.getAccountCount()).toBe(0); - const failuresByRefreshToken = Reflect.get( - manager, - "authFailuresByRefreshToken", - ) as Map; - expect(failuresByRefreshToken.has("token-1")).toBe(false); expect(manager.incrementAuthFailures(account1)).toBe(1); }); @@ -967,8 +962,21 @@ describe("AccountManager", () => { version: 3 as const, activeIndex: 0, accounts: [ - { refreshToken: "token-1", addedAt: now, lastUsed: now }, - { refreshToken: "token-1", organizationId: "org-1", addedAt: now, lastUsed: now }, + { + refreshToken: "token-1", + coolingDownUntil: now + 60_000, + cooldownReason: "auth-failure" as const, + addedAt: now, + lastUsed: now, + }, + { + refreshToken: "token-1", + organizationId: "org-1", + coolingDownUntil: now + 60_000, + cooldownReason: "auth-failure" as const, + addedAt: now, + lastUsed: now, + }, ], }; @@ -986,12 +994,12 @@ describe("AccountManager", () => { expect( manager.getAccountsSnapshot().every((account) => account.disabledReason === "auth-failure"), ).toBe(true); - - const failuresByRefreshToken = Reflect.get( - manager, - "authFailuresByRefreshToken", - ) as Map; - expect(failuresByRefreshToken.has("token-1")).toBe(false); + expect( + manager.getAccountsSnapshot().every((account) => account.coolingDownUntil === undefined), + ).toBe(true); + expect( + manager.getAccountsSnapshot().every((account) => account.cooldownReason === undefined), + ).toBe(true); expect(manager.incrementAuthFailures(accounts[0])).toBe(1); }); diff --git a/test/index-retry.test.ts b/test/index-retry.test.ts index e4268e6c..aca18064 100644 --- a/test/index-retry.test.ts +++ b/test/index-retry.test.ts @@ -1,4 +1,13 @@ import { describe, it, expect, vi, beforeEach, afterEach } from "vitest"; +import { runCleanup } from "../lib/shutdown.js"; + +beforeEach(async () => { + await runCleanup(); +}); + +afterEach(async () => { + await runCleanup(); +}); vi.mock("@opencode-ai/plugin/tool", () => { const makeSchema = () => ({ @@ -45,6 +54,10 @@ vi.mock("../lib/accounts.js", () => { return 1; } + getEnabledAccountCount() { + return 1; + } + getCurrentOrNextForFamily() { this.calls += 1; if (this.calls === 1) return null; diff --git a/test/index.test.ts b/test/index.test.ts index 1f0b55a5..67953057 100644 --- a/test/index.test.ts +++ b/test/index.test.ts @@ -1,4 +1,13 @@ import { describe, it, expect, vi, beforeEach, afterEach } from "vitest"; +import { runCleanup } from "../lib/shutdown.js"; + +beforeEach(async () => { + await runCleanup(); +}); + +afterEach(async () => { + await runCleanup(); +}); vi.mock("@opencode-ai/plugin/tool", () => { const makeSchema = () => ({ @@ -294,6 +303,10 @@ vi.mock("../lib/accounts.js", () => { return this.accounts.length; } + getEnabledAccountCount() { + return this.accounts.filter((account) => account.enabled !== false).length; + } + getCurrentOrNextForFamily() { return this.accounts[0] ?? null; } @@ -345,7 +358,20 @@ vi.mock("../lib/accounts.js", () => { refundToken() {} markSwitched() {} removeAccount() {} - disableAccountsWithSameRefreshToken() { return 1; } + disableAccountsWithSameRefreshToken(account: { refreshToken?: string }) { + const refreshToken = account.refreshToken; + let disabledCount = 0; + for (const storedAccount of this.accounts) { + if (storedAccount.refreshToken !== refreshToken) continue; + delete storedAccount.coolingDownUntil; + delete storedAccount.cooldownReason; + if (storedAccount.enabled === false) continue; + storedAccount.enabled = false; + storedAccount.disabledReason = "auth-failure"; + disabledCount++; + } + return disabledCount; + } removeAccountsWithSameRefreshToken() { return 1; } getMinWaitTimeForFamily() { @@ -2339,6 +2365,7 @@ describe("OpenAIOAuthPlugin fetch handler", () => { let fallbackSelection = 0; const customManager = { getAccountCount: () => 2, + getEnabledAccountCount: () => 2, getCurrentOrNextForFamilyHybrid: (_family: string, currentModel?: string) => { if (currentModel === "gpt-5-codex") { if (fallbackSelection === 0) { @@ -3566,6 +3593,34 @@ describe("OpenAIOAuthPlugin persistAccountPool", () => { }); }); + it("flushes pending account saves during shutdown cleanup", async () => { + const accountsModule = await import("../lib/accounts.js"); + const flushPendingSave = vi.fn(async () => {}); + const manager = await accountsModule.AccountManager.loadFromDisk(); + manager.flushPendingSave = flushPendingSave; + vi.spyOn(accountsModule.AccountManager, "loadFromDisk").mockResolvedValue(manager); + + const mockClient = createMockClient(); + const { OpenAIOAuthPlugin } = await import("../index.js"); + const plugin = (await OpenAIOAuthPlugin({ + client: mockClient, + } as never)) as unknown as PluginType; + + await plugin.auth.loader( + async () => ({ + type: "oauth", + access: "access-token", + refresh: "refresh-token", + expires: Date.now() + 60_000, + }) as never, + {}, + ); + + await runCleanup(); + + expect(flushPendingSave).toHaveBeenCalledTimes(1); + }); + it("writes user disable metadata from the auth manage menu and clears it on manual re-enable", async () => { const cliModule = await import("../lib/cli.js"); const storageModule = await import("../lib/storage.js"); @@ -3615,6 +3670,54 @@ describe("OpenAIOAuthPlugin persistAccountPool", () => { disabledReason: undefined, }); }); + + it("keeps auth-failure disables blocked in the auth manage menu until a fresh login", async () => { + const cliModule = await import("../lib/cli.js"); + const storageModule = await import("../lib/storage.js"); + const consoleLog = vi.spyOn(console, "log").mockImplementation(() => {}); + + mockStorage.accounts = [ + { + accountId: "workspace-auth-failure", + email: "blocked@example.com", + refreshToken: "refresh-blocked", + enabled: false, + disabledReason: "auth-failure", + coolingDownUntil: 30_000, + cooldownReason: "auth-failure", + addedAt: 10, + lastUsed: 10, + }, + ]; + + vi.mocked(cliModule.promptLoginMode) + .mockResolvedValueOnce({ mode: "manage", toggleAccountIndex: 0 }) + .mockResolvedValueOnce({ mode: "cancel" }); + + const mockClient = createMockClient(); + const { OpenAIOAuthPlugin } = await import("../index.js"); + const plugin = (await OpenAIOAuthPlugin({ + client: mockClient, + } as never)) as unknown as PluginType; + const autoMethod = plugin.auth.methods[0] as unknown as { + authorize: (inputs?: Record) => Promise<{ instructions: string }>; + }; + + const authResult = await autoMethod.authorize(); + expect(authResult.instructions).toBe("Authentication cancelled"); + + expect(vi.mocked(storageModule.saveAccounts)).not.toHaveBeenCalled(); + expect(mockStorage.accounts[0]).toMatchObject({ + accountId: "workspace-auth-failure", + enabled: false, + disabledReason: "auth-failure", + coolingDownUntil: 30_000, + cooldownReason: "auth-failure", + }); + expect(consoleLog).toHaveBeenCalledWith( + expect.stringContaining("Run 'opencode auth login' to re-enable with fresh credentials."), + ); + }); }); describe("OpenAIOAuthPlugin showToast error handling", () => { From d466201bd43ba79c361f0936a8eb35801624de67 Mon Sep 17 00:00:00 2001 From: ndycode Date: Sun, 15 Mar 2026 00:29:11 +0800 Subject: [PATCH 05/43] Clarify enabled account selection logging --- index.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/index.ts b/index.ts index 9e22f315..578b6037 100644 --- a/index.ts +++ b/index.ts @@ -2169,7 +2169,7 @@ while (attempted.size < Math.max(1, accountCount)) { } // Log account selection for debugging rotation logDebug( - `Using account ${account.index + 1}/${accountCount}: ${account.email ?? "unknown"} for ${modelFamily}`, + `Using account ${account.index + 1} (enabled ${attempted.size}/${accountCount}): ${account.email ?? "unknown"} for ${modelFamily}`, ); let accountAuth = accountManager.toAuthDetails(account) as OAuthAuthDetails; From c8538cfaa21df421856452d3967c9cfde4b6fbda Mon Sep 17 00:00:00 2001 From: ndycode Date: Sun, 15 Mar 2026 00:35:17 +0800 Subject: [PATCH 06/43] Fix enabled account selection toast positions --- index.ts | 2 +- test/index.test.ts | 74 ++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 75 insertions(+), 1 deletion(-) diff --git a/index.ts b/index.ts index 578b6037..c4f7d190 100644 --- a/index.ts +++ b/index.ts @@ -2289,7 +2289,7 @@ while (attempted.size < Math.max(1, accountCount)) { ) { const accountLabel = formatAccountLabel(account, account.index); await showToast( - `Using ${accountLabel} (${account.index + 1}/${accountCount})`, + `Using ${accountLabel} (enabled ${attempted.size}/${accountCount})`, "info", ); accountManager.markToastShown(account.index); diff --git a/test/index.test.ts b/test/index.test.ts index 67953057..bba740a6 100644 --- a/test/index.test.ts +++ b/test/index.test.ts @@ -2535,6 +2535,80 @@ describe("OpenAIOAuthPlugin fetch handler", () => { expect(response.status).toBe(200); }); + it("uses enabled-account positions in selection toasts", async () => { + const { AccountManager } = await import("../lib/accounts.js"); + + const account = { + index: 2, + accountId: "acc-3", + email: "user3@example.com", + refreshToken: "refresh-3", + }; + const markToastShown = vi.fn(); + const customManager = { + getAccountCount: () => 3, + getEnabledAccountCount: () => 2, + getCurrentOrNextForFamilyHybrid: (() => { + let selected = false; + return () => { + if (selected) { + return null; + } + selected = true; + return account; + }; + })(), + getSelectionExplainability: () => [], + toAuthDetails: (selectedAccount: { accountId?: string }) => ({ + type: "oauth" as const, + access: `access-${selectedAccount.accountId ?? "unknown"}`, + refresh: "refresh-token", + expires: Date.now() + 60_000, + }), + hasRefreshToken: () => true, + saveToDiskDebounced: () => {}, + updateFromAuth: () => {}, + clearAuthFailures: () => {}, + incrementAuthFailures: () => 1, + markAccountCoolingDown: () => {}, + markRateLimitedWithReason: () => {}, + recordRateLimit: () => {}, + consumeToken: () => true, + refundToken: () => {}, + markSwitched: () => {}, + removeAccount: () => {}, + recordFailure: () => {}, + recordSuccess: () => {}, + getMinWaitTimeForFamily: () => 0, + shouldShowAccountToast: () => true, + markToastShown, + setActiveIndex: () => account, + getAccountsSnapshot: () => [account], + }; + vi.spyOn(AccountManager, "loadFromDisk").mockResolvedValueOnce(customManager as never); + + globalThis.fetch = vi.fn().mockResolvedValue( + new Response(JSON.stringify({ content: "test" }), { status: 200 }), + ); + + const { sdk, mockClient } = await setupPlugin(); + const response = await sdk.fetch!("https://api.openai.com/v1/chat", { + method: "POST", + body: JSON.stringify({ model: "gpt-5.1" }), + }); + + expect(response.status).toBe(200); + expect(mockClient.tui.showToast).toHaveBeenCalledWith( + expect.objectContaining({ + body: expect.objectContaining({ + message: expect.stringContaining("enabled 1/2"), + variant: "info", + }), + }), + ); + expect(markToastShown).toHaveBeenCalledWith(2); + }); + it("handles empty body in request", async () => { globalThis.fetch = vi.fn().mockResolvedValue( new Response(JSON.stringify({ content: "test" }), { status: 200 }), From 208e5b2156f7fb81ef04456521e16c0e85dd9de8 Mon Sep 17 00:00:00 2001 From: ndycode Date: Sun, 15 Mar 2026 00:47:55 +0800 Subject: [PATCH 07/43] Preserve disabled sibling state during auth failure --- lib/accounts.ts | 2 +- lib/shutdown.ts | 33 ++++++++++++++++++++++++++++----- test/accounts.test.ts | 4 ++++ test/shutdown.test.ts | 22 ++++++++++++++++++++++ 4 files changed, 55 insertions(+), 6 deletions(-) diff --git a/lib/accounts.ts b/lib/accounts.ts index b4ff188b..7475ffd9 100644 --- a/lib/accounts.ts +++ b/lib/accounts.ts @@ -890,8 +890,8 @@ export class AccountManager { for (const accountToDisable of this.accounts) { if (accountToDisable.refreshToken !== refreshToken) continue; - this.clearAccountCooldown(accountToDisable); if (accountToDisable.enabled === false) continue; + this.clearAccountCooldown(accountToDisable); accountToDisable.enabled = false; accountToDisable.disabledReason = "auth-failure"; disabledCount++; diff --git a/lib/shutdown.ts b/lib/shutdown.ts index 66965627..3fa0390a 100644 --- a/lib/shutdown.ts +++ b/lib/shutdown.ts @@ -2,6 +2,9 @@ type CleanupFn = () => void | Promise; const cleanupFunctions: CleanupFn[] = []; let shutdownRegistered = false; +let sigintHandler: (() => void) | null = null; +let sigtermHandler: (() => void) | null = null; +let beforeExitHandler: (() => void) | null = null; export function registerCleanup(fn: CleanupFn): void { cleanupFunctions.push(fn); @@ -18,6 +21,8 @@ export function unregisterCleanup(fn: CleanupFn): void { export async function runCleanup(): Promise { const fns = [...cleanupFunctions]; cleanupFunctions.length = 0; + removeShutdownHandlers(); + shutdownRegistered = false; for (const fn of fns) { try { @@ -37,12 +42,30 @@ function ensureShutdownHandler(): void { process.exit(0); }); }; - - process.once("SIGINT", handleSignal); - process.once("SIGTERM", handleSignal); - process.once("beforeExit", () => { + sigintHandler = handleSignal; + sigtermHandler = handleSignal; + beforeExitHandler = () => { void runCleanup(); - }); + }; + + process.once("SIGINT", sigintHandler); + process.once("SIGTERM", sigtermHandler); + process.once("beforeExit", beforeExitHandler); +} + +function removeShutdownHandlers(): void { + if (sigintHandler) { + process.off("SIGINT", sigintHandler); + sigintHandler = null; + } + if (sigtermHandler) { + process.off("SIGTERM", sigtermHandler); + sigtermHandler = null; + } + if (beforeExitHandler) { + process.off("beforeExit", beforeExitHandler); + beforeExitHandler = null; + } } export function getCleanupCount(): number { diff --git a/test/accounts.test.ts b/test/accounts.test.ts index 249a13b3..ed3aa1a5 100644 --- a/test/accounts.test.ts +++ b/test/accounts.test.ts @@ -895,6 +895,8 @@ describe("AccountManager", () => { accountId: "workspace-user-disabled", enabled: false, disabledReason: "user" as const, + coolingDownUntil: now + 60_000, + cooldownReason: "rate-limit" as const, addedAt: now, lastUsed: now, }, @@ -917,6 +919,8 @@ describe("AccountManager", () => { accountId: "workspace-user-disabled", enabled: false, disabledReason: "user", + coolingDownUntil: now + 60_000, + cooldownReason: "rate-limit", }); expect(updated[2]).toMatchObject({ accountId: "workspace-b", diff --git a/test/shutdown.test.ts b/test/shutdown.test.ts index c64ecf41..8ae1230c 100644 --- a/test/shutdown.test.ts +++ b/test/shutdown.test.ts @@ -173,5 +173,27 @@ describe("Graceful shutdown", () => { processOnceSpy.mockRestore(); }); + + it("re-registers signal handlers after cleanup resets shutdown state", async () => { + const processOnceSpy = vi.spyOn(process, "once").mockImplementation(() => process); + const processOffSpy = vi.spyOn(process, "off").mockImplementation(() => process); + + vi.resetModules(); + const { + registerCleanup: freshRegister, + runCleanup: freshRunCleanup, + } = await import("../lib/shutdown.js"); + + freshRegister(() => {}); + expect(processOnceSpy).toHaveBeenCalledTimes(3); + + await freshRunCleanup(); + expect(processOffSpy).toHaveBeenCalledTimes(3); + freshRegister(() => {}); + expect(processOnceSpy).toHaveBeenCalledTimes(6); + + processOnceSpy.mockRestore(); + processOffSpy.mockRestore(); + }); }); }); From 51886eb5cfa8b3a4dfe66a83b838a5c3c8e01d44 Mon Sep 17 00:00:00 2001 From: ndycode Date: Sun, 15 Mar 2026 01:03:09 +0800 Subject: [PATCH 08/43] Harden save flushing and disable semantics --- lib/accounts.ts | 7 ++++- lib/shutdown.ts | 17 ++++++----- test/accounts.test.ts | 69 +++++++++++++++++++++++++++++++++++++++++++ test/shutdown.test.ts | 39 ++++++++++++++++++++++++ 4 files changed, 124 insertions(+), 8 deletions(-) diff --git a/lib/accounts.ts b/lib/accounts.ts index 7475ffd9..7a21a0e6 100644 --- a/lib/accounts.ts +++ b/lib/accounts.ts @@ -940,6 +940,8 @@ export class AccountManager { delete account.disabledReason; } else if (reason) { account.disabledReason = reason; + } else { + delete account.disabledReason; } return account; } @@ -1007,14 +1009,17 @@ export class AccountManager { } async flushPendingSave(): Promise { + const hadDebouncedSave = !!this.saveDebounceTimer; if (this.saveDebounceTimer) { clearTimeout(this.saveDebounceTimer); this.saveDebounceTimer = null; - await this.saveToDisk(); } if (this.pendingSave) { await this.pendingSave; } + if (hadDebouncedSave) { + await this.saveToDisk(); + } } } diff --git a/lib/shutdown.ts b/lib/shutdown.ts index 3fa0390a..0b574acb 100644 --- a/lib/shutdown.ts +++ b/lib/shutdown.ts @@ -21,15 +21,18 @@ export function unregisterCleanup(fn: CleanupFn): void { export async function runCleanup(): Promise { const fns = [...cleanupFunctions]; cleanupFunctions.length = 0; - removeShutdownHandlers(); - shutdownRegistered = false; - for (const fn of fns) { - try { - await fn(); - } catch { - // Ignore cleanup errors during shutdown + try { + for (const fn of fns) { + try { + await fn(); + } catch { + // Ignore cleanup errors during shutdown + } } + } finally { + removeShutdownHandlers(); + shutdownRegistered = false; } } diff --git a/test/accounts.test.ts b/test/accounts.test.ts index ed3aa1a5..9fae6616 100644 --- a/test/accounts.test.ts +++ b/test/accounts.test.ts @@ -1031,6 +1031,31 @@ describe("AccountManager", () => { }); expect(reenabled?.disabledReason).toBeUndefined(); }); + + it("clears stale disabledReason when disabling without an explicit reason", () => { + const now = Date.now(); + const stored = { + version: 3 as const, + activeIndex: 0, + accounts: [ + { + refreshToken: "token-1", + enabled: false, + disabledReason: "auth-failure" as const, + addedAt: now, + lastUsed: now, + }, + ], + }; + + const manager = new AccountManager(undefined, stored); + + const disabled = manager.setAccountEnabled(0, false); + expect(disabled).toMatchObject({ + enabled: false, + }); + expect(disabled?.disabledReason).toBeUndefined(); + }); }); describe("getMinWaitTimeForFamily", () => { @@ -1929,6 +1954,50 @@ describe("AccountManager", () => { await manager.flushPendingSave(); await savePromise; }); + + it("waits for in-flight save before flushing a pending debounce", async () => { + vi.useFakeTimers(); + try { + const { saveAccounts } = await import("../lib/storage.js"); + const mockSaveAccounts = vi.mocked(saveAccounts); + mockSaveAccounts.mockClear(); + + let resolveFirst!: () => void; + const firstSave = new Promise((resolve) => { + resolveFirst = resolve; + }); + mockSaveAccounts.mockImplementationOnce(() => firstSave); + mockSaveAccounts.mockResolvedValue(); + + const now = Date.now(); + const stored = { + version: 3 as const, + activeIndex: 0, + accounts: [ + { refreshToken: "token-1", addedAt: now, lastUsed: now }, + ], + }; + + const manager = new AccountManager(undefined, stored); + + manager.saveToDiskDebounced(50); + await vi.advanceTimersByTimeAsync(60); + expect(mockSaveAccounts).toHaveBeenCalledTimes(1); + + manager.saveToDiskDebounced(50); + const flushPromise = manager.flushPendingSave(); + await Promise.resolve(); + + expect(mockSaveAccounts).toHaveBeenCalledTimes(1); + + resolveFirst(); + await flushPromise; + + expect(mockSaveAccounts).toHaveBeenCalledTimes(2); + } finally { + vi.useRealTimers(); + } + }); }); describe("health and token tracking methods", () => { diff --git a/test/shutdown.test.ts b/test/shutdown.test.ts index 8ae1230c..211517ec 100644 --- a/test/shutdown.test.ts +++ b/test/shutdown.test.ts @@ -130,6 +130,45 @@ describe("Graceful shutdown", () => { processExitSpy.mockRestore(); }); + it("keeps shutdown handlers installed until async cleanup completes", async () => { + const capturedHandlers = new Map void>(); + + const processOnceSpy = vi.spyOn(process, "once").mockImplementation((event: string | symbol, handler: (...args: unknown[]) => void) => { + capturedHandlers.set(String(event), handler); + return process; + }); + const processOffSpy = vi.spyOn(process, "off").mockImplementation(() => process); + const processExitSpy = vi.spyOn(process, "exit").mockImplementation(() => undefined as never); + + vi.resetModules(); + const { registerCleanup: freshRegister, runCleanup: freshRunCleanup } = await import("../lib/shutdown.js"); + await freshRunCleanup(); + + let resolveCleanup!: () => void; + const cleanupPromise = new Promise((resolve) => { + resolveCleanup = resolve; + }); + freshRegister(async () => { + await cleanupPromise; + }); + + const sigtermHandler = capturedHandlers.get("SIGTERM"); + expect(sigtermHandler).toBeDefined(); + + sigtermHandler!(); + await Promise.resolve(); + expect(processOffSpy).not.toHaveBeenCalled(); + + resolveCleanup(); + await new Promise((r) => setTimeout(r, 10)); + expect(processOffSpy).toHaveBeenCalledTimes(3); + expect(processExitSpy).toHaveBeenCalledWith(0); + + processOnceSpy.mockRestore(); + processOffSpy.mockRestore(); + processExitSpy.mockRestore(); + }); + it("beforeExit handler runs cleanup without calling exit", async () => { const capturedHandlers = new Map void>(); From 323871e6b91a4424a655d4b18135ea6ee401fe27 Mon Sep 17 00:00:00 2001 From: ndycode Date: Sun, 15 Mar 2026 01:22:26 +0800 Subject: [PATCH 09/43] Drain pending saves without overlapping writes --- index.ts | 1 + lib/accounts.ts | 42 +++++++--- test/accounts.test.ts | 183 ++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 216 insertions(+), 10 deletions(-) diff --git a/index.ts b/index.ts index c4f7d190..a5949a70 100644 --- a/index.ts +++ b/index.ts @@ -1681,6 +1681,7 @@ export const OpenAIOAuthPlugin: Plugin = async ({ client }: PluginInput) => { accountManagerPromise = null; }; + // Plugin init can run more than once per process; runCleanup drains this module-level list. registerCleanup(async () => { await cachedAccountManager?.flushPendingSave(); }); diff --git a/lib/accounts.ts b/lib/accounts.ts index 7a21a0e6..d914d1eb 100644 --- a/lib/accounts.ts +++ b/lib/accounts.ts @@ -1009,16 +1009,38 @@ export class AccountManager { } async flushPendingSave(): Promise { - const hadDebouncedSave = !!this.saveDebounceTimer; - if (this.saveDebounceTimer) { - clearTimeout(this.saveDebounceTimer); - this.saveDebounceTimer = null; - } - if (this.pendingSave) { - await this.pendingSave; - } - if (hadDebouncedSave) { - await this.saveToDisk(); + while (true) { + const hadDebouncedSave = !!this.saveDebounceTimer; + if (this.saveDebounceTimer) { + clearTimeout(this.saveDebounceTimer); + this.saveDebounceTimer = null; + } + if (this.pendingSave) { + await this.pendingSave; + // Let debounced callbacks waiting on the completed save re-arm pendingSave. + await Promise.resolve(); + if (this.saveDebounceTimer !== null || this.pendingSave !== null) { + continue; + } + } + if (!hadDebouncedSave) { + return; + } + if (this.saveDebounceTimer !== null || this.pendingSave !== null) { + continue; + } + const flushSave = this.saveToDisk().finally(() => { + if (this.pendingSave === flushSave) { + this.pendingSave = null; + } + }); + this.pendingSave = flushSave; + await flushSave; + // Drain saves that were queued while the flush save was in flight. + await Promise.resolve(); + if (this.saveDebounceTimer === null && this.pendingSave === null) { + return; + } } } } diff --git a/test/accounts.test.ts b/test/accounts.test.ts index 9fae6616..7de605eb 100644 --- a/test/accounts.test.ts +++ b/test/accounts.test.ts @@ -1854,6 +1854,12 @@ describe("AccountManager", () => { }); describe("flushPendingSave", () => { + const drainMicrotasks = async (turns = 10): Promise => { + for (let turn = 0; turn < turns; turn++) { + await Promise.resolve(); + } + }; + it("flushes pending debounced save", async () => { const { saveAccounts } = await import("../lib/storage.js"); const mockSaveAccounts = vi.mocked(saveAccounts); @@ -1998,6 +2004,183 @@ describe("AccountManager", () => { vi.useRealTimers(); } }); + + it("drains saves queued during flush without overlapping writes", async () => { + vi.useFakeTimers(); + try { + const { saveAccounts } = await import("../lib/storage.js"); + const mockSaveAccounts = vi.mocked(saveAccounts); + mockSaveAccounts.mockClear(); + + let activeWrites = 0; + let maxConcurrentWrites = 0; + const settleWrites: Array<() => void> = []; + const writePromises: Promise[] = []; + + mockSaveAccounts.mockImplementation(() => { + activeWrites++; + maxConcurrentWrites = Math.max(maxConcurrentWrites, activeWrites); + const writePromise = new Promise((resolve) => { + settleWrites.push(() => { + activeWrites--; + resolve(); + }); + }); + writePromises.push(writePromise); + return writePromise; + }); + + const now = Date.now(); + const stored = { + version: 3 as const, + activeIndex: 0, + accounts: [ + { refreshToken: "token-1", addedAt: now, lastUsed: now }, + ], + }; + + const manager = new AccountManager(undefined, stored); + + manager.saveToDiskDebounced(50); + await vi.advanceTimersByTimeAsync(60); + expect(mockSaveAccounts).toHaveBeenCalledTimes(1); + + const armGapSave = writePromises[0]!.then(() => { + manager.saveToDiskDebounced(0); + }); + + manager.saveToDiskDebounced(50); + const flushPromise = manager.flushPendingSave(); + + settleWrites[0]!(); + await armGapSave; + await drainMicrotasks(); + + expect(mockSaveAccounts).toHaveBeenCalledTimes(2); + + manager.saveToDiskDebounced(0); + await vi.advanceTimersByTimeAsync(0); + expect(mockSaveAccounts).toHaveBeenCalledTimes(2); + + settleWrites[1]!(); + await drainMicrotasks(); + + expect(mockSaveAccounts).toHaveBeenCalledTimes(3); + + settleWrites[2]!(); + await flushPromise; + + expect(maxConcurrentWrites).toBe(1); + } finally { + vi.useRealTimers(); + } + }); + + it("persists disabled auth-failure state after flushing over an in-flight save", async () => { + vi.useFakeTimers(); + try { + const { saveAccounts } = await import("../lib/storage.js"); + const mockSaveAccounts = vi.mocked(saveAccounts); + mockSaveAccounts.mockClear(); + + let activeWrites = 0; + let maxConcurrentWrites = 0; + const settleWrites: Array<() => void> = []; + const savedSnapshots: Array< + Array<{ + refreshToken?: string; + enabled?: false; + disabledReason?: string; + coolingDownUntil?: number; + cooldownReason?: string; + }> + > = []; + + mockSaveAccounts.mockImplementation(async (storage) => { + activeWrites++; + maxConcurrentWrites = Math.max(maxConcurrentWrites, activeWrites); + savedSnapshots.push( + storage.accounts.map((account) => ({ + refreshToken: account.refreshToken, + enabled: account.enabled, + disabledReason: account.disabledReason, + coolingDownUntil: account.coolingDownUntil, + cooldownReason: account.cooldownReason, + })), + ); + await new Promise((resolve) => { + settleWrites.push(() => { + activeWrites--; + resolve(); + }); + }); + }); + + const now = Date.now(); + const stored = { + version: 3 as const, + activeIndex: 0, + accounts: [ + { + refreshToken: "shared-refresh", + addedAt: now, + lastUsed: now, + coolingDownUntil: now + 10_000, + cooldownReason: "auth-failure" as const, + }, + { + refreshToken: "shared-refresh", + addedAt: now + 1, + lastUsed: now + 1, + coolingDownUntil: now + 20_000, + cooldownReason: "auth-failure" as const, + }, + ], + }; + + const manager = new AccountManager(undefined, stored); + + manager.saveToDiskDebounced(50); + await vi.advanceTimersByTimeAsync(60); + expect(mockSaveAccounts).toHaveBeenCalledTimes(1); + + const account = manager.getAccountsSnapshot()[0]!; + manager.disableAccountsWithSameRefreshToken(account); + manager.saveToDiskDebounced(50); + + const flushPromise = manager.flushPendingSave(); + + settleWrites[0]!(); + await drainMicrotasks(); + + expect(mockSaveAccounts).toHaveBeenCalledTimes(2); + + const flushedSharedAccounts = savedSnapshots[1]!.filter( + (savedAccount) => savedAccount.refreshToken === "shared-refresh", + ); + expect(flushedSharedAccounts).toEqual([ + expect.objectContaining({ + enabled: false, + disabledReason: "auth-failure", + coolingDownUntil: undefined, + cooldownReason: undefined, + }), + expect.objectContaining({ + enabled: false, + disabledReason: "auth-failure", + coolingDownUntil: undefined, + cooldownReason: undefined, + }), + ]); + + settleWrites[1]!(); + await flushPromise; + + expect(maxConcurrentWrites).toBe(1); + } finally { + vi.useRealTimers(); + } + }); }); describe("health and token tracking methods", () => { From 113941f95e038456923900bec0d16dd5c387d4c7 Mon Sep 17 00:00:00 2001 From: ndycode Date: Sun, 15 Mar 2026 01:41:04 +0800 Subject: [PATCH 10/43] Cap flush loops and log shutdown save failures --- index.ts | 10 ++++++- lib/accounts.ts | 10 +++++++ lib/storage/migrations.ts | 1 + test/accounts.test.ts | 60 +++++++++++++++++++++++++++++++++++++++ test/index.test.ts | 36 +++++++++++++++++++++++ 5 files changed, 116 insertions(+), 1 deletion(-) diff --git a/index.ts b/index.ts index a5949a70..8e1a8aa1 100644 --- a/index.ts +++ b/index.ts @@ -573,6 +573,8 @@ export const OpenAIOAuthPlugin: Plugin = async ({ client }: PluginInput) => { if (reasons.includes("auth-failure")) { return "auth-failure" satisfies AccountDisabledReason; } + // Unknown disabled reasons are treated as legacy/manual disables. + // Explicit login revival is reserved for accounts we know were disabled by auth failure. return undefined; })(); const targetCoolingDownUntil = @@ -1683,7 +1685,13 @@ export const OpenAIOAuthPlugin: Plugin = async ({ client }: PluginInput) => { // Plugin init can run more than once per process; runCleanup drains this module-level list. registerCleanup(async () => { - await cachedAccountManager?.flushPendingSave(); + try { + await cachedAccountManager?.flushPendingSave(); + } catch (error) { + logWarn("[shutdown] flushPendingSave failed; disabled state may not be persisted", { + error: error instanceof Error ? error.message : String(error), + }); + } }); // Event handler for session recovery and account selection diff --git a/lib/accounts.ts b/lib/accounts.ts index d914d1eb..820ce133 100644 --- a/lib/accounts.ts +++ b/lib/accounts.ts @@ -1009,7 +1009,17 @@ export class AccountManager { } async flushPendingSave(): Promise { + const MAX_FLUSH_ITERATIONS = 20; + let flushIterations = 0; + while (true) { + flushIterations += 1; + if (flushIterations > MAX_FLUSH_ITERATIONS) { + log.warn("flushPendingSave exceeded max iterations; possible save loop", { + iterations: flushIterations - 1, + }); + return; + } const hadDebouncedSave = !!this.saveDebounceTimer; if (this.saveDebounceTimer) { clearTimeout(this.saveDebounceTimer); diff --git a/lib/storage/migrations.ts b/lib/storage/migrations.ts index 04d3d4ed..7b290384 100644 --- a/lib/storage/migrations.ts +++ b/lib/storage/migrations.ts @@ -27,6 +27,7 @@ export interface AccountMetadataV1 { /** Optional access token expiry timestamp (ms since epoch). */ expiresAt?: number; enabled?: boolean; + /** Never persisted by V1 storage; disabled V1 accounts migrate as user-disabled. */ disabledReason?: AccountDisabledReason; addedAt: number; lastUsed: number; diff --git a/test/accounts.test.ts b/test/accounts.test.ts index 7de605eb..4abd2292 100644 --- a/test/accounts.test.ts +++ b/test/accounts.test.ts @@ -13,6 +13,10 @@ import { import { getHealthTracker, getTokenTracker, resetTrackers } from "../lib/rotation.js"; import type { OAuthAuthDetails } from "../lib/types.js"; +const { accountLoggerWarn } = vi.hoisted(() => ({ + accountLoggerWarn: vi.fn(), +})); + vi.mock("../lib/storage.js", async (importOriginal) => { const actual = await importOriginal(); return { @@ -21,6 +25,21 @@ vi.mock("../lib/storage.js", async (importOriginal) => { }; }); +vi.mock("../lib/logger.js", () => ({ + createLogger: vi.fn(() => ({ + debug: vi.fn(), + info: vi.fn(), + warn: accountLoggerWarn, + error: vi.fn(), + time: vi.fn(() => vi.fn(() => 0)), + timeEnd: vi.fn(), + })), +})); + +beforeEach(() => { + accountLoggerWarn.mockClear(); +}); + describe("parseRateLimitReason", () => { it("returns quota for quota-related codes", () => { expect(parseRateLimitReason("usage_limit_reached")).toBe("quota"); @@ -2181,6 +2200,47 @@ describe("AccountManager", () => { vi.useRealTimers(); } }); + + it("warns and exits if flushPendingSave keeps discovering new saves", async () => { + vi.useFakeTimers(); + try { + const { saveAccounts } = await import("../lib/storage.js"); + const mockSaveAccounts = vi.mocked(saveAccounts); + mockSaveAccounts.mockClear(); + mockSaveAccounts.mockResolvedValue(); + + const now = Date.now(); + const stored = { + version: 3 as const, + activeIndex: 0, + accounts: [ + { refreshToken: "token-1", addedAt: now, lastUsed: now }, + ], + }; + + const manager = new AccountManager(undefined, stored); + const originalSaveToDisk = manager.saveToDisk.bind(manager); + let rearmedSaves = 0; + + vi.spyOn(manager, "saveToDisk").mockImplementation(async () => { + if (rearmedSaves < 25) { + rearmedSaves++; + manager.saveToDiskDebounced(0); + } + await originalSaveToDisk(); + }); + + manager.saveToDiskDebounced(0); + await manager.flushPendingSave(); + + expect(accountLoggerWarn).toHaveBeenCalledWith( + "flushPendingSave exceeded max iterations; possible save loop", + expect.objectContaining({ iterations: 20 }), + ); + } finally { + vi.useRealTimers(); + } + }); }); describe("health and token tracking methods", () => { diff --git a/test/index.test.ts b/test/index.test.ts index bba740a6..9dd284ed 100644 --- a/test/index.test.ts +++ b/test/index.test.ts @@ -3695,6 +3695,42 @@ describe("OpenAIOAuthPlugin persistAccountPool", () => { expect(flushPendingSave).toHaveBeenCalledTimes(1); }); + it("logs when shutdown cleanup flush fails", async () => { + const accountsModule = await import("../lib/accounts.js"); + const loggerModule = await import("../lib/logger.js"); + const flushPendingSave = vi.fn(async () => { + throw new Error("EBUSY"); + }); + const manager = await accountsModule.AccountManager.loadFromDisk(); + manager.flushPendingSave = flushPendingSave; + vi.spyOn(accountsModule.AccountManager, "loadFromDisk").mockResolvedValue(manager); + + const mockClient = createMockClient(); + const { OpenAIOAuthPlugin } = await import("../index.js"); + const plugin = (await OpenAIOAuthPlugin({ + client: mockClient, + } as never)) as unknown as PluginType; + + await plugin.auth.loader( + async () => ({ + type: "oauth", + access: "access-token", + refresh: "refresh-token", + expires: Date.now() + 60_000, + }) as never, + {}, + ); + + vi.mocked(loggerModule.logWarn).mockClear(); + await runCleanup(); + + expect(flushPendingSave).toHaveBeenCalledTimes(1); + expect(vi.mocked(loggerModule.logWarn)).toHaveBeenCalledWith( + "[shutdown] flushPendingSave failed; disabled state may not be persisted", + expect.objectContaining({ error: "EBUSY" }), + ); + }); + it("writes user disable metadata from the auth manage menu and clears it on manual re-enable", async () => { const cliModule = await import("../lib/cli.js"); const storageModule = await import("../lib/storage.js"); From 2b851ccdf64925e949743d3fc7f92d583162a329 Mon Sep 17 00:00:00 2001 From: ndycode Date: Sun, 15 Mar 2026 01:47:16 +0800 Subject: [PATCH 11/43] Address remaining PR follow-up comments --- index.ts | 13 +++++-- lib/accounts.ts | 8 +++- test/index.test.ts | 90 +++++++++++++++++++++++++------------------ test/shutdown.test.ts | 51 +++++++++++++----------- 4 files changed, 97 insertions(+), 65 deletions(-) diff --git a/index.ts b/index.ts index 8e1a8aa1..ee304413 100644 --- a/index.ts +++ b/index.ts @@ -88,7 +88,7 @@ import { } from "./lib/logger.js"; import { checkAndNotify } from "./lib/auto-update-checker.js"; import { handleContextOverflow } from "./lib/context-overflow.js"; -import { registerCleanup } from "./lib/shutdown.js"; +import { registerCleanup, unregisterCleanup } from "./lib/shutdown.js"; import { AccountManager, type AccountSelectionExplainability, @@ -201,6 +201,8 @@ import { * } * ``` */ +let accountManagerCleanupHook: (() => Promise) | null = null; + // eslint-disable-next-line @typescript-eslint/require-await export const OpenAIOAuthPlugin: Plugin = async ({ client }: PluginInput) => { initLogger(client); @@ -1683,8 +1685,10 @@ export const OpenAIOAuthPlugin: Plugin = async ({ client }: PluginInput) => { accountManagerPromise = null; }; - // Plugin init can run more than once per process; runCleanup drains this module-level list. - registerCleanup(async () => { + if (accountManagerCleanupHook) { + unregisterCleanup(accountManagerCleanupHook); + } + accountManagerCleanupHook = async () => { try { await cachedAccountManager?.flushPendingSave(); } catch (error) { @@ -1692,7 +1696,8 @@ export const OpenAIOAuthPlugin: Plugin = async ({ client }: PluginInput) => { error: error instanceof Error ? error.message : String(error), }); } - }); + }; + registerCleanup(accountManagerCleanupHook); // Event handler for session recovery and account selection const eventHandler = async (input: { event: { type: string; properties?: unknown } }) => { diff --git a/lib/accounts.ts b/lib/accounts.ts index 820ce133..ea8135f6 100644 --- a/lib/accounts.ts +++ b/lib/accounts.ts @@ -396,7 +396,13 @@ export class AccountManager { } getEnabledAccountCount(): number { - return this.accounts.filter((account) => account.enabled !== false).length; + let enabledCount = 0; + for (const account of this.accounts) { + if (account.enabled !== false) { + enabledCount += 1; + } + } + return enabledCount; } getActiveIndex(): number { diff --git a/test/index.test.ts b/test/index.test.ts index 9dd284ed..41724565 100644 --- a/test/index.test.ts +++ b/test/index.test.ts @@ -1,5 +1,5 @@ import { describe, it, expect, vi, beforeEach, afterEach } from "vitest"; -import { runCleanup } from "../lib/shutdown.js"; +import { getCleanupCount, runCleanup } from "../lib/shutdown.js"; beforeEach(async () => { await runCleanup(); @@ -3731,6 +3731,18 @@ describe("OpenAIOAuthPlugin persistAccountPool", () => { ); }); + it("replaces the account-save shutdown cleanup on plugin re-initialization", async () => { + const { OpenAIOAuthPlugin } = await import("../index.js"); + + expect(getCleanupCount()).toBe(0); + + await OpenAIOAuthPlugin({ client: createMockClient() } as never); + expect(getCleanupCount()).toBe(1); + + await OpenAIOAuthPlugin({ client: createMockClient() } as never); + expect(getCleanupCount()).toBe(1); + }); + it("writes user disable metadata from the auth manage menu and clears it on manual re-enable", async () => { const cliModule = await import("../lib/cli.js"); const storageModule = await import("../lib/storage.js"); @@ -3786,47 +3798,51 @@ describe("OpenAIOAuthPlugin persistAccountPool", () => { const storageModule = await import("../lib/storage.js"); const consoleLog = vi.spyOn(console, "log").mockImplementation(() => {}); - mockStorage.accounts = [ - { + try { + mockStorage.accounts = [ + { + accountId: "workspace-auth-failure", + email: "blocked@example.com", + refreshToken: "refresh-blocked", + enabled: false, + disabledReason: "auth-failure", + coolingDownUntil: 30_000, + cooldownReason: "auth-failure", + addedAt: 10, + lastUsed: 10, + }, + ]; + + vi.mocked(cliModule.promptLoginMode) + .mockResolvedValueOnce({ mode: "manage", toggleAccountIndex: 0 }) + .mockResolvedValueOnce({ mode: "cancel" }); + + const mockClient = createMockClient(); + const { OpenAIOAuthPlugin } = await import("../index.js"); + const plugin = (await OpenAIOAuthPlugin({ + client: mockClient, + } as never)) as unknown as PluginType; + const autoMethod = plugin.auth.methods[0] as unknown as { + authorize: (inputs?: Record) => Promise<{ instructions: string }>; + }; + + const authResult = await autoMethod.authorize(); + expect(authResult.instructions).toBe("Authentication cancelled"); + + expect(vi.mocked(storageModule.saveAccounts)).not.toHaveBeenCalled(); + expect(mockStorage.accounts[0]).toMatchObject({ accountId: "workspace-auth-failure", - email: "blocked@example.com", - refreshToken: "refresh-blocked", enabled: false, disabledReason: "auth-failure", coolingDownUntil: 30_000, cooldownReason: "auth-failure", - addedAt: 10, - lastUsed: 10, - }, - ]; - - vi.mocked(cliModule.promptLoginMode) - .mockResolvedValueOnce({ mode: "manage", toggleAccountIndex: 0 }) - .mockResolvedValueOnce({ mode: "cancel" }); - - const mockClient = createMockClient(); - const { OpenAIOAuthPlugin } = await import("../index.js"); - const plugin = (await OpenAIOAuthPlugin({ - client: mockClient, - } as never)) as unknown as PluginType; - const autoMethod = plugin.auth.methods[0] as unknown as { - authorize: (inputs?: Record) => Promise<{ instructions: string }>; - }; - - const authResult = await autoMethod.authorize(); - expect(authResult.instructions).toBe("Authentication cancelled"); - - expect(vi.mocked(storageModule.saveAccounts)).not.toHaveBeenCalled(); - expect(mockStorage.accounts[0]).toMatchObject({ - accountId: "workspace-auth-failure", - enabled: false, - disabledReason: "auth-failure", - coolingDownUntil: 30_000, - cooldownReason: "auth-failure", - }); - expect(consoleLog).toHaveBeenCalledWith( - expect.stringContaining("Run 'opencode auth login' to re-enable with fresh credentials."), - ); + }); + expect(consoleLog).toHaveBeenCalledWith( + expect.stringContaining("Run 'opencode auth login' to re-enable with fresh credentials."), + ); + } finally { + consoleLog.mockRestore(); + } }); }); diff --git a/test/shutdown.test.ts b/test/shutdown.test.ts index 211517ec..222af67a 100644 --- a/test/shutdown.test.ts +++ b/test/shutdown.test.ts @@ -152,21 +152,24 @@ describe("Graceful shutdown", () => { await cleanupPromise; }); - const sigtermHandler = capturedHandlers.get("SIGTERM"); - expect(sigtermHandler).toBeDefined(); - - sigtermHandler!(); - await Promise.resolve(); - expect(processOffSpy).not.toHaveBeenCalled(); - - resolveCleanup(); - await new Promise((r) => setTimeout(r, 10)); - expect(processOffSpy).toHaveBeenCalledTimes(3); - expect(processExitSpy).toHaveBeenCalledWith(0); - - processOnceSpy.mockRestore(); - processOffSpy.mockRestore(); - processExitSpy.mockRestore(); + try { + const sigtermHandler = capturedHandlers.get("SIGTERM"); + expect(sigtermHandler).toBeDefined(); + + sigtermHandler!(); + await Promise.resolve(); + expect(processOffSpy).not.toHaveBeenCalled(); + + resolveCleanup(); + await vi.waitFor(() => { + expect(processOffSpy).toHaveBeenCalledTimes(3); + expect(processExitSpy).toHaveBeenCalledWith(0); + }); + } finally { + processOnceSpy.mockRestore(); + processOffSpy.mockRestore(); + processExitSpy.mockRestore(); + } }); it("beforeExit handler runs cleanup without calling exit", async () => { @@ -188,14 +191,16 @@ describe("Graceful shutdown", () => { const beforeExitHandler = capturedHandlers.get("beforeExit"); expect(beforeExitHandler).toBeDefined(); - beforeExitHandler!(); - await new Promise((r) => setTimeout(r, 10)); - - expect(cleanupFn).toHaveBeenCalled(); - expect(processExitSpy).not.toHaveBeenCalled(); - - processOnceSpy.mockRestore(); - processExitSpy.mockRestore(); + try { + beforeExitHandler!(); + await vi.waitFor(() => { + expect(cleanupFn).toHaveBeenCalled(); + }); + expect(processExitSpy).not.toHaveBeenCalled(); + } finally { + processOnceSpy.mockRestore(); + processExitSpy.mockRestore(); + } }); it("signal handlers are only registered once", async () => { From 9a7f29bedc7c6647488c229e5c2e554f300ffa0c Mon Sep 17 00:00:00 2001 From: ndycode Date: Sun, 15 Mar 2026 02:11:29 +0800 Subject: [PATCH 12/43] Flush invalidated account managers on shutdown --- index.ts | 23 ++++++++++++---- test/index.test.ts | 69 +++++++++++++++++++++++++++++++++++++++++++++- 2 files changed, 85 insertions(+), 7 deletions(-) diff --git a/index.ts b/index.ts index ee304413..40689159 100644 --- a/index.ts +++ b/index.ts @@ -202,6 +202,7 @@ import { * ``` */ let accountManagerCleanupHook: (() => Promise) | null = null; +const staleAccountManagersForCleanup = new Set(); // eslint-disable-next-line @typescript-eslint/require-await export const OpenAIOAuthPlugin: Plugin = async ({ client }: PluginInput) => { @@ -1681,6 +1682,9 @@ export const OpenAIOAuthPlugin: Plugin = async ({ client }: PluginInput) => { }; const invalidateAccountManagerCache = (): void => { + if (cachedAccountManager) { + staleAccountManagersForCleanup.add(cachedAccountManager); + } cachedAccountManager = null; accountManagerPromise = null; }; @@ -1689,12 +1693,19 @@ export const OpenAIOAuthPlugin: Plugin = async ({ client }: PluginInput) => { unregisterCleanup(accountManagerCleanupHook); } accountManagerCleanupHook = async () => { - try { - await cachedAccountManager?.flushPendingSave(); - } catch (error) { - logWarn("[shutdown] flushPendingSave failed; disabled state may not be persisted", { - error: error instanceof Error ? error.message : String(error), - }); + const managersToFlush = new Set(staleAccountManagersForCleanup); + staleAccountManagersForCleanup.clear(); + if (cachedAccountManager) { + managersToFlush.add(cachedAccountManager); + } + for (const manager of managersToFlush) { + try { + await manager.flushPendingSave(); + } catch (error) { + logWarn("[shutdown] flushPendingSave failed; disabled state may not be persisted", { + error: error instanceof Error ? error.message : String(error), + }); + } } }; registerCleanup(accountManagerCleanupHook); diff --git a/test/index.test.ts b/test/index.test.ts index 41724565..ed5149cb 100644 --- a/test/index.test.ts +++ b/test/index.test.ts @@ -363,9 +363,9 @@ vi.mock("../lib/accounts.js", () => { let disabledCount = 0; for (const storedAccount of this.accounts) { if (storedAccount.refreshToken !== refreshToken) continue; + if (storedAccount.enabled === false) continue; delete storedAccount.coolingDownUntil; delete storedAccount.cooldownReason; - if (storedAccount.enabled === false) continue; storedAccount.enabled = false; storedAccount.disabledReason = "auth-failure"; disabledCount++; @@ -3731,6 +3731,73 @@ describe("OpenAIOAuthPlugin persistAccountPool", () => { ); }); + it("flushes stale invalidated managers alongside the current manager during shutdown cleanup", async () => { + const accountsModule = await import("../lib/accounts.js"); + const cliModule = await import("../lib/cli.js"); + const managerOne = await accountsModule.AccountManager.loadFromDisk(); + const managerTwo = await accountsModule.AccountManager.loadFromDisk(); + const flushManagerOne = vi.fn(async () => {}); + const flushManagerTwo = vi.fn(async () => {}); + managerOne.flushPendingSave = flushManagerOne; + managerTwo.flushPendingSave = flushManagerTwo; + vi.spyOn(accountsModule.AccountManager, "loadFromDisk") + .mockResolvedValueOnce(managerOne) + .mockResolvedValue(managerTwo); + + mockStorage.accounts = [ + { + accountId: "workspace-managed", + email: "managed@example.com", + refreshToken: "refresh-managed", + enabled: true, + addedAt: 10, + lastUsed: 10, + }, + ]; + + vi.mocked(cliModule.promptLoginMode) + .mockResolvedValueOnce({ mode: "manage", toggleAccountIndex: 0 }) + .mockResolvedValueOnce({ mode: "cancel" }); + + const mockClient = createMockClient(); + const { OpenAIOAuthPlugin } = await import("../index.js"); + const plugin = (await OpenAIOAuthPlugin({ + client: mockClient, + } as never)) as unknown as PluginType; + const autoMethod = plugin.auth.methods[0] as unknown as { + authorize: (inputs?: Record) => Promise<{ instructions: string }>; + }; + + await plugin.auth.loader( + async () => ({ + type: "oauth", + access: "access-token-1", + refresh: "refresh-token-1", + expires: Date.now() + 60_000, + }) as never, + {}, + ); + + flushManagerOne.mockClear(); + const authResult = await autoMethod.authorize(); + expect(authResult.instructions).toBe("Authentication cancelled"); + + await plugin.auth.loader( + async () => ({ + type: "oauth", + access: "access-token-2", + refresh: "refresh-token-2", + expires: Date.now() + 60_000, + }) as never, + {}, + ); + + await runCleanup(); + + expect(flushManagerOne).toHaveBeenCalledTimes(1); + expect(flushManagerTwo).toHaveBeenCalledTimes(1); + }); + it("replaces the account-save shutdown cleanup on plugin re-initialization", async () => { const { OpenAIOAuthPlugin } = await import("../index.js"); From dd37ad04555dc8fa4149c649f0d6f5f208dd57f4 Mon Sep 17 00:00:00 2001 From: ndycode Date: Sun, 15 Mar 2026 02:20:59 +0800 Subject: [PATCH 13/43] Finish PR #78 review follow-ups --- index.ts | 40 ++++++++++++++++++++-------------------- lib/accounts.ts | 33 +++++++++++++++++++-------------- test/accounts.test.ts | 4 ++-- test/index.test.ts | 2 +- 4 files changed, 42 insertions(+), 37 deletions(-) diff --git a/index.ts b/index.ts index 40689159..b02851e7 100644 --- a/index.ts +++ b/index.ts @@ -578,7 +578,7 @@ export const OpenAIOAuthPlugin: Plugin = async ({ client }: PluginInput) => { } // Unknown disabled reasons are treated as legacy/manual disables. // Explicit login revival is reserved for accounts we know were disabled by auth failure. - return undefined; + return "user" satisfies AccountDisabledReason; })(); const targetCoolingDownUntil = typeof target.coolingDownUntil === "number" && Number.isFinite(target.coolingDownUntil) @@ -1681,6 +1681,15 @@ export const OpenAIOAuthPlugin: Plugin = async ({ client }: PluginInput) => { } }; + const setCachedAccountManager = (manager: AccountManager): AccountManager => { + if (cachedAccountManager && cachedAccountManager !== manager) { + staleAccountManagersForCleanup.add(cachedAccountManager); + } + cachedAccountManager = manager; + accountManagerPromise = Promise.resolve(manager); + return manager; + }; + const invalidateAccountManagerCache = (): void => { if (cachedAccountManager) { staleAccountManagersForCleanup.add(cachedAccountManager); @@ -1751,8 +1760,7 @@ export const OpenAIOAuthPlugin: Plugin = async ({ client }: PluginInput) => { // refresh tokens with stale in-memory state. if (cachedAccountManager) { const reloadedManager = await AccountManager.loadFromDisk(); - cachedAccountManager = reloadedManager; - accountManagerPromise = Promise.resolve(reloadedManager); + setCachedAccountManager(reloadedManager); } await showToast(`Switched to account ${index + 1}`, "info"); @@ -1821,8 +1829,7 @@ export const OpenAIOAuthPlugin: Plugin = async ({ client }: PluginInput) => { if (!accountManagerPromise) { accountManagerPromise = AccountManager.loadFromDisk(authFallback); } - let accountManager = await accountManagerPromise; - cachedAccountManager = accountManager; + let accountManager = setCachedAccountManager(await accountManagerPromise); const refreshToken = authFallback?.refresh ?? ""; const needsPersist = refreshToken && @@ -4050,10 +4057,9 @@ while (attempted.size < Math.max(1, accountCount)) { return `Switched to ${label} but failed to persist. Changes may be lost on restart.`; } - if (cachedAccountManager) { + if (cachedAccountManager) { const reloadedManager = await AccountManager.loadFromDisk(); - cachedAccountManager = reloadedManager; - accountManagerPromise = Promise.resolve(reloadedManager); + setCachedAccountManager(reloadedManager); } const label = formatCommandAccountLabel(account, targetIndex); @@ -5110,8 +5116,7 @@ while (attempted.size < Math.max(1, accountCount)) { if (cachedAccountManager) { const reloadedManager = await AccountManager.loadFromDisk(); - cachedAccountManager = reloadedManager; - accountManagerPromise = Promise.resolve(reloadedManager); + setCachedAccountManager(reloadedManager); } } @@ -5355,8 +5360,7 @@ while (attempted.size < Math.max(1, accountCount)) { if (cachedAccountManager) { const reloadedManager = await AccountManager.loadFromDisk(); - cachedAccountManager = reloadedManager; - accountManagerPromise = Promise.resolve(reloadedManager); + setCachedAccountManager(reloadedManager); } const accountLabel = formatCommandAccountLabel(account, targetIndex); @@ -5456,8 +5460,7 @@ while (attempted.size < Math.max(1, accountCount)) { if (cachedAccountManager) { const reloadedManager = await AccountManager.loadFromDisk(); - cachedAccountManager = reloadedManager; - accountManagerPromise = Promise.resolve(reloadedManager); + setCachedAccountManager(reloadedManager); } const accountLabel = formatCommandAccountLabel(account, targetIndex); @@ -5534,8 +5537,7 @@ while (attempted.size < Math.max(1, accountCount)) { if (cachedAccountManager) { const reloadedManager = await AccountManager.loadFromDisk(); - cachedAccountManager = reloadedManager; - accountManagerPromise = Promise.resolve(reloadedManager); + setCachedAccountManager(reloadedManager); } const accountLabel = formatCommandAccountLabel(account, targetIndex); @@ -5851,8 +5853,7 @@ while (attempted.size < Math.max(1, accountCount)) { if (cachedAccountManager) { const reloadedManager = await AccountManager.loadFromDisk(); - cachedAccountManager = reloadedManager; - accountManagerPromise = Promise.resolve(reloadedManager); + setCachedAccountManager(reloadedManager); } const remaining = storage.accounts.length; @@ -5946,8 +5947,7 @@ while (attempted.size < Math.max(1, accountCount)) { await saveAccounts(storage); if (cachedAccountManager) { const reloadedManager = await AccountManager.loadFromDisk(); - cachedAccountManager = reloadedManager; - accountManagerPromise = Promise.resolve(reloadedManager); + setCachedAccountManager(reloadedManager); } results.push(""); results.push(`Summary: ${refreshedCount} refreshed, ${failedCount} failed`); diff --git a/lib/accounts.ts b/lib/accounts.ts index ea8135f6..e3b0663e 100644 --- a/lib/accounts.ts +++ b/lib/accounts.ts @@ -271,7 +271,7 @@ export class AccountManager { if (!changed) return; try { - await this.saveToDisk(); + await this.enqueueSave(() => this.saveToDisk()); } catch (error) { log.debug("Failed to persist Codex CLI cache hydration", { error: String(error) }); } @@ -991,6 +991,22 @@ export class AccountManager { await saveAccounts(storage); } + private enqueueSave(saveOperation: () => Promise): Promise { + const previousSave = this.pendingSave; + const nextSave = (async () => { + if (previousSave) { + await previousSave; + } + await saveOperation(); + })().finally(() => { + if (this.pendingSave === nextSave) { + this.pendingSave = null; + } + }); + this.pendingSave = nextSave; + return nextSave; + } + saveToDiskDebounced(delayMs = 500): void { if (this.saveDebounceTimer) { clearTimeout(this.saveDebounceTimer); @@ -999,13 +1015,7 @@ export class AccountManager { this.saveDebounceTimer = null; const doSave = async () => { try { - if (this.pendingSave) { - await this.pendingSave; - } - this.pendingSave = this.saveToDisk().finally(() => { - this.pendingSave = null; - }); - await this.pendingSave; + await this.enqueueSave(() => this.saveToDisk()); } catch (error) { log.warn("Debounced save failed", { error: error instanceof Error ? error.message : String(error) }); } @@ -1045,12 +1055,7 @@ export class AccountManager { if (this.saveDebounceTimer !== null || this.pendingSave !== null) { continue; } - const flushSave = this.saveToDisk().finally(() => { - if (this.pendingSave === flushSave) { - this.pendingSave = null; - } - }); - this.pendingSave = flushSave; + const flushSave = this.enqueueSave(() => this.saveToDisk()); await flushSave; // Drain saves that were queued while the flush save was in flight. await Promise.resolve(); diff --git a/test/accounts.test.ts b/test/accounts.test.ts index 4abd2292..1f981198 100644 --- a/test/accounts.test.ts +++ b/test/accounts.test.ts @@ -915,7 +915,7 @@ describe("AccountManager", () => { enabled: false, disabledReason: "user" as const, coolingDownUntil: now + 60_000, - cooldownReason: "rate-limit" as const, + cooldownReason: "network-error" as const, addedAt: now, lastUsed: now, }, @@ -939,7 +939,7 @@ describe("AccountManager", () => { enabled: false, disabledReason: "user", coolingDownUntil: now + 60_000, - cooldownReason: "rate-limit", + cooldownReason: "network-error", }); expect(updated[2]).toMatchObject({ accountId: "workspace-b", diff --git a/test/index.test.ts b/test/index.test.ts index ed5149cb..2f8aaf71 100644 --- a/test/index.test.ts +++ b/test/index.test.ts @@ -3272,7 +3272,7 @@ describe("OpenAIOAuthPlugin persistAccountPool", () => { const mergedOrg = mergedOrgEntries[0]; expect(mergedOrg?.accountId).toBe("org-shared"); expect(mergedOrg?.enabled).toBe(false); - expect(mergedOrg?.disabledReason).toBeUndefined(); + expect(mergedOrg?.disabledReason).toBe("user"); expect(mergedOrg?.coolingDownUntil).toBe(12_000); expect(mergedOrg?.cooldownReason).toBe("auth-failure"); }); From f78127b53744b3bfe17b7fab3e168a3a383215c6 Mon Sep 17 00:00:00 2001 From: ndycode Date: Sun, 15 Mar 2026 02:40:09 +0800 Subject: [PATCH 14/43] Harden account disable reason handling --- lib/accounts.ts | 9 ++++-- test/accounts.test.ts | 68 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 75 insertions(+), 2 deletions(-) diff --git a/lib/accounts.ts b/lib/accounts.ts index e3b0663e..8d09a156 100644 --- a/lib/accounts.ts +++ b/lib/accounts.ts @@ -311,7 +311,8 @@ export class AccountManager { : sanitizeEmail(account.email), refreshToken, enabled: account.enabled !== false, - disabledReason: account.enabled === false ? account.disabledReason : undefined, + disabledReason: + account.enabled === false ? account.disabledReason ?? "user" : undefined, access: matchesFallback && authFallback ? authFallback.access : account.accessToken, expires: matchesFallback && authFallback ? authFallback.expires : account.expiresAt, addedAt: clampNonNegativeInt(account.addedAt, baseNow), @@ -941,6 +942,9 @@ export class AccountManager { if (index < 0 || index >= this.accounts.length) return null; const account = this.accounts[index]; if (!account) return null; + if (enabled && account.disabledReason === "auth-failure") { + return null; + } account.enabled = enabled; if (enabled) { delete account.disabledReason; @@ -975,7 +979,8 @@ export class AccountManager { accessToken: account.access, expiresAt: account.expires, enabled: account.enabled === false ? false : undefined, - disabledReason: account.enabled === false ? account.disabledReason : undefined, + disabledReason: + account.enabled === false ? account.disabledReason ?? "user" : undefined, addedAt: account.addedAt, lastUsed: account.lastUsed, lastSwitchReason: account.lastSwitchReason, diff --git a/test/accounts.test.ts b/test/accounts.test.ts index 1f981198..8423613d 100644 --- a/test/accounts.test.ts +++ b/test/accounts.test.ts @@ -948,6 +948,47 @@ describe("AccountManager", () => { }); }); + it("defaults legacy disabled accounts to a user disable reason", async () => { + const { saveAccounts } = await import("../lib/storage.js"); + const mockSaveAccounts = vi.mocked(saveAccounts); + mockSaveAccounts.mockClear(); + mockSaveAccounts.mockResolvedValueOnce(); + + const now = Date.now(); + const stored = { + version: 3 as const, + activeIndex: 0, + accounts: [ + { + refreshToken: "token-1", + enabled: false, + addedAt: now, + lastUsed: now, + }, + ], + }; + + const manager = new AccountManager(undefined, stored); + + expect(manager.getAccountsSnapshot()[0]).toMatchObject({ + enabled: false, + disabledReason: "user", + }); + + await manager.saveToDisk(); + + expect(mockSaveAccounts).toHaveBeenCalledWith( + expect.objectContaining({ + accounts: [ + expect.objectContaining({ + enabled: false, + disabledReason: "user", + }), + ], + }), + ); + }); + it("clears auth failure counter when removing accounts with same refreshToken", () => { const now = Date.now(); const stored = { @@ -1075,6 +1116,32 @@ describe("AccountManager", () => { }); expect(disabled?.disabledReason).toBeUndefined(); }); + + it("blocks re-enabling auth-failure disabled accounts through setAccountEnabled", () => { + const now = Date.now(); + const stored = { + version: 3 as const, + activeIndex: 0, + accounts: [ + { + refreshToken: "token-1", + enabled: false, + disabledReason: "auth-failure" as const, + addedAt: now, + lastUsed: now, + }, + ], + }; + + const manager = new AccountManager(undefined, stored); + + const reenabled = manager.setAccountEnabled(0, true); + expect(reenabled).toBeNull(); + expect(manager.getAccountsSnapshot()[0]).toMatchObject({ + enabled: false, + disabledReason: "auth-failure", + }); + }); }); describe("getMinWaitTimeForFamily", () => { @@ -1461,6 +1528,7 @@ describe("AccountManager", () => { it("saves accounts with all fields", async () => { const { saveAccounts } = await import("../lib/storage.js"); const mockSaveAccounts = vi.mocked(saveAccounts); + mockSaveAccounts.mockClear(); mockSaveAccounts.mockResolvedValueOnce(); const now = Date.now(); From 2cd74eaf809ce03d20b11133784396495eeddeea Mon Sep 17 00:00:00 2001 From: ndycode Date: Sun, 15 Mar 2026 02:45:27 +0800 Subject: [PATCH 15/43] Track shutdown cleanup across plugin instances --- index.ts | 23 ++++++++++------------- test/index.test.ts | 47 ++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 57 insertions(+), 13 deletions(-) diff --git a/index.ts b/index.ts index b02851e7..7b1c77d2 100644 --- a/index.ts +++ b/index.ts @@ -201,8 +201,10 @@ import { * } * ``` */ +// Shared across plugin instances so shutdown cleanup can flush debounced saves +// even when multiple plugin objects coexist during reloads or tests. let accountManagerCleanupHook: (() => Promise) | null = null; -const staleAccountManagersForCleanup = new Set(); +const trackedAccountManagersForCleanup = new Set(); // eslint-disable-next-line @typescript-eslint/require-await export const OpenAIOAuthPlugin: Plugin = async ({ client }: PluginInput) => { @@ -1683,8 +1685,9 @@ export const OpenAIOAuthPlugin: Plugin = async ({ client }: PluginInput) => { const setCachedAccountManager = (manager: AccountManager): AccountManager => { if (cachedAccountManager && cachedAccountManager !== manager) { - staleAccountManagersForCleanup.add(cachedAccountManager); + trackedAccountManagersForCleanup.add(cachedAccountManager); } + trackedAccountManagersForCleanup.add(manager); cachedAccountManager = manager; accountManagerPromise = Promise.resolve(manager); return manager; @@ -1692,24 +1695,17 @@ export const OpenAIOAuthPlugin: Plugin = async ({ client }: PluginInput) => { const invalidateAccountManagerCache = (): void => { if (cachedAccountManager) { - staleAccountManagersForCleanup.add(cachedAccountManager); + trackedAccountManagersForCleanup.add(cachedAccountManager); } cachedAccountManager = null; accountManagerPromise = null; }; - if (accountManagerCleanupHook) { - unregisterCleanup(accountManagerCleanupHook); - } - accountManagerCleanupHook = async () => { - const managersToFlush = new Set(staleAccountManagersForCleanup); - staleAccountManagersForCleanup.clear(); - if (cachedAccountManager) { - managersToFlush.add(cachedAccountManager); - } - for (const manager of managersToFlush) { + accountManagerCleanupHook ??= async () => { + for (const manager of [...trackedAccountManagersForCleanup]) { try { await manager.flushPendingSave(); + trackedAccountManagersForCleanup.delete(manager); } catch (error) { logWarn("[shutdown] flushPendingSave failed; disabled state may not be persisted", { error: error instanceof Error ? error.message : String(error), @@ -1717,6 +1713,7 @@ export const OpenAIOAuthPlugin: Plugin = async ({ client }: PluginInput) => { } } }; + unregisterCleanup(accountManagerCleanupHook); registerCleanup(accountManagerCleanupHook); // Event handler for session recovery and account selection diff --git a/test/index.test.ts b/test/index.test.ts index 2f8aaf71..9b5d5df7 100644 --- a/test/index.test.ts +++ b/test/index.test.ts @@ -3798,6 +3798,53 @@ describe("OpenAIOAuthPlugin persistAccountPool", () => { expect(flushManagerTwo).toHaveBeenCalledTimes(1); }); + it("flushes tracked account managers from multiple plugin instances during shutdown cleanup", async () => { + const accountsModule = await import("../lib/accounts.js"); + const managerOne = await accountsModule.AccountManager.loadFromDisk(); + const managerTwo = await accountsModule.AccountManager.loadFromDisk(); + const flushManagerOne = vi.fn(async () => {}); + const flushManagerTwo = vi.fn(async () => {}); + managerOne.flushPendingSave = flushManagerOne; + managerTwo.flushPendingSave = flushManagerTwo; + vi.spyOn(accountsModule.AccountManager, "loadFromDisk") + .mockResolvedValueOnce(managerOne) + .mockResolvedValueOnce(managerTwo); + + const { OpenAIOAuthPlugin } = await import("../index.js"); + const firstPlugin = (await OpenAIOAuthPlugin({ + client: createMockClient(), + } as never)) as unknown as PluginType; + const secondPlugin = (await OpenAIOAuthPlugin({ + client: createMockClient(), + } as never)) as unknown as PluginType; + + await firstPlugin.auth.loader( + async () => ({ + type: "oauth", + access: "access-token-1", + refresh: "refresh-token-1", + expires: Date.now() + 60_000, + }) as never, + {}, + ); + await secondPlugin.auth.loader( + async () => ({ + type: "oauth", + access: "access-token-2", + refresh: "refresh-token-2", + expires: Date.now() + 60_000, + }) as never, + {}, + ); + + expect(getCleanupCount()).toBe(1); + + await runCleanup(); + + expect(flushManagerOne).toHaveBeenCalledTimes(1); + expect(flushManagerTwo).toHaveBeenCalledTimes(1); + }); + it("replaces the account-save shutdown cleanup on plugin re-initialization", async () => { const { OpenAIOAuthPlugin } = await import("../index.js"); From e4421d9fc94c71699bc546b36c22a9090085cabb Mon Sep 17 00:00:00 2001 From: ndycode Date: Sun, 15 Mar 2026 02:54:44 +0800 Subject: [PATCH 16/43] Add flush rejection regression coverage --- lib/storage/migrations.ts | 2 +- test/accounts.test.ts | 32 ++++++++++++++++++++++++++++++++ 2 files changed, 33 insertions(+), 1 deletion(-) diff --git a/lib/storage/migrations.ts b/lib/storage/migrations.ts index 7b290384..6469b990 100644 --- a/lib/storage/migrations.ts +++ b/lib/storage/migrations.ts @@ -27,7 +27,7 @@ export interface AccountMetadataV1 { /** Optional access token expiry timestamp (ms since epoch). */ expiresAt?: number; enabled?: boolean; - /** Never persisted by V1 storage; disabled V1 accounts migrate as user-disabled. */ + /** Never set by V1 storage; migration defaults disabled V1 accounts to "user". */ disabledReason?: AccountDisabledReason; addedAt: number; lastUsed: number; diff --git a/test/accounts.test.ts b/test/accounts.test.ts index 8423613d..09f56af7 100644 --- a/test/accounts.test.ts +++ b/test/accounts.test.ts @@ -2309,6 +2309,38 @@ describe("AccountManager", () => { vi.useRealTimers(); } }); + + it("rejects when saveToDisk throws during flush", async () => { + vi.useFakeTimers(); + try { + const { saveAccounts } = await import("../lib/storage.js"); + const mockSaveAccounts = vi.mocked(saveAccounts); + mockSaveAccounts.mockClear(); + mockSaveAccounts.mockRejectedValueOnce(new Error("EBUSY: file in use")); + mockSaveAccounts.mockResolvedValueOnce(); + + const now = Date.now(); + const stored = { + version: 3 as const, + activeIndex: 0, + accounts: [ + { refreshToken: "token-1", addedAt: now, lastUsed: now }, + ], + }; + + const manager = new AccountManager(undefined, stored); + manager.saveToDiskDebounced(0); + + await expect(manager.flushPendingSave()).rejects.toThrow("EBUSY"); + + manager.saveToDiskDebounced(0); + await vi.advanceTimersByTimeAsync(0); + + expect(mockSaveAccounts).toHaveBeenCalledTimes(2); + } finally { + vi.useRealTimers(); + } + }); }); describe("health and token tracking methods", () => { From e9b75daa9061b9852b1f517c83fc49f39406df57 Mon Sep 17 00:00:00 2001 From: ndycode Date: Sun, 15 Mar 2026 02:56:12 +0800 Subject: [PATCH 17/43] Preserve auth-failure disable locks --- lib/accounts.ts | 4 +++- test/accounts.test.ts | 4 ++-- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/lib/accounts.ts b/lib/accounts.ts index 8d09a156..2ce3dae5 100644 --- a/lib/accounts.ts +++ b/lib/accounts.ts @@ -951,7 +951,9 @@ export class AccountManager { } else if (reason) { account.disabledReason = reason; } else { - delete account.disabledReason; + if (account.disabledReason !== "auth-failure") { + delete account.disabledReason; + } } return account; } diff --git a/test/accounts.test.ts b/test/accounts.test.ts index 09f56af7..6b483d35 100644 --- a/test/accounts.test.ts +++ b/test/accounts.test.ts @@ -1092,7 +1092,7 @@ describe("AccountManager", () => { expect(reenabled?.disabledReason).toBeUndefined(); }); - it("clears stale disabledReason when disabling without an explicit reason", () => { + it("preserves auth-failure disabledReason when disabling without an explicit reason", () => { const now = Date.now(); const stored = { version: 3 as const, @@ -1113,8 +1113,8 @@ describe("AccountManager", () => { const disabled = manager.setAccountEnabled(0, false); expect(disabled).toMatchObject({ enabled: false, + disabledReason: "auth-failure", }); - expect(disabled?.disabledReason).toBeUndefined(); }); it("blocks re-enabling auth-failure disabled accounts through setAccountEnabled", () => { From e421a5c6314122b3b932f7d18535dbd001128c73 Mon Sep 17 00:00:00 2001 From: ndycode Date: Sun, 15 Mar 2026 03:18:17 +0800 Subject: [PATCH 18/43] Fix remaining review regressions --- index.ts | 15 +++++---- lib/shutdown.ts | 53 ++++++++++++++++++++---------- lib/storage.ts | 7 ++-- test/index.test.ts | 52 +++++++++++++++++++++++++++-- test/shutdown.test.ts | 76 ++++++++++++++++++++++++++++++++++--------- test/storage.test.ts | 25 ++++++++++++++ 6 files changed, 184 insertions(+), 44 deletions(-) diff --git a/index.ts b/index.ts index 7b1c77d2..920fad90 100644 --- a/index.ts +++ b/index.ts @@ -2714,11 +2714,12 @@ while (attempted.size < Math.max(1, accountCount)) { } const waitMs = accountManager.getMinWaitTimeForFamily(modelFamily, model); - const count = accountManager.getEnabledAccountCount(); + const enabledCount = accountManager.getEnabledAccountCount(); + const totalCount = accountManager.getAccountCount(); if ( retryAllAccountsRateLimited && - count > 0 && + enabledCount > 0 && waitMs > 0 && (retryAllAccountsMaxWaitMs === 0 || waitMs <= retryAllAccountsMaxWaitMs) && @@ -2728,7 +2729,7 @@ while (attempted.size < Math.max(1, accountCount)) { `All accounts rate-limited wait ${waitMs}ms`, ) ) { - const countdownMessage = `All ${count} account(s) rate-limited. Waiting`; + const countdownMessage = `All ${enabledCount} enabled account(s) rate-limited. Waiting`; await sleepWithCountdown(addJitter(waitMs, 0.2), countdownMessage); allRateLimitedRetries++; continue; @@ -2736,11 +2737,13 @@ while (attempted.size < Math.max(1, accountCount)) { const waitLabel = waitMs > 0 ? formatWaitTime(waitMs) : "a bit"; const message = - count === 0 + totalCount === 0 ? "No Codex accounts configured. Run `opencode auth login`." + : enabledCount === 0 + ? "All stored Codex accounts are disabled. Re-enable them from account management or run `opencode auth login` to restore auth-failure disables." : waitMs > 0 - ? `All ${count} account(s) are rate-limited. Try again in ${waitLabel} or add another account with \`opencode auth login\`.` - : `All ${count} account(s) failed (server errors or auth issues). Check account health with \`codex-health\`.`; + ? `All ${enabledCount} enabled account(s) are rate-limited. Try again in ${waitLabel} or add another account with \`opencode auth login\`.` + : `All ${enabledCount} enabled account(s) failed (server errors or auth issues). Check account health with \`codex-health\`.`; runtimeMetrics.failedRequests++; runtimeMetrics.lastError = message; runtimeMetrics.lastErrorCategory = waitMs > 0 ? "rate-limit" : "account-failure"; diff --git a/lib/shutdown.ts b/lib/shutdown.ts index 0b574acb..a3df654c 100644 --- a/lib/shutdown.ts +++ b/lib/shutdown.ts @@ -5,6 +5,8 @@ let shutdownRegistered = false; let sigintHandler: (() => void) | null = null; let sigtermHandler: (() => void) | null = null; let beforeExitHandler: (() => void) | null = null; +let cleanupInFlight: Promise | null = null; +let signalExitPending = false; export function registerCleanup(fn: CleanupFn): void { cleanupFunctions.push(fn); @@ -18,22 +20,33 @@ export function unregisterCleanup(fn: CleanupFn): void { } } -export async function runCleanup(): Promise { - const fns = [...cleanupFunctions]; - cleanupFunctions.length = 0; +export function runCleanup(): Promise { + if (cleanupInFlight) { + return cleanupInFlight; + } + + cleanupInFlight = (async () => { + const fns = [...cleanupFunctions]; + cleanupFunctions.length = 0; - try { - for (const fn of fns) { - try { - await fn(); - } catch { - // Ignore cleanup errors during shutdown + try { + for (const fn of fns) { + try { + await fn(); + } catch { + // Ignore cleanup errors during shutdown + } } + } finally { + removeShutdownHandlers(); + shutdownRegistered = false; } - } finally { - removeShutdownHandlers(); - shutdownRegistered = false; - } + })().finally(() => { + cleanupInFlight = null; + signalExitPending = false; + }); + + return cleanupInFlight; } function ensureShutdownHandler(): void { @@ -41,6 +54,10 @@ function ensureShutdownHandler(): void { shutdownRegistered = true; const handleSignal = () => { + if (signalExitPending) { + return; + } + signalExitPending = true; void runCleanup().finally(() => { process.exit(0); }); @@ -48,12 +65,14 @@ function ensureShutdownHandler(): void { sigintHandler = handleSignal; sigtermHandler = handleSignal; beforeExitHandler = () => { - void runCleanup(); + if (!cleanupInFlight) { + void runCleanup(); + } }; - process.once("SIGINT", sigintHandler); - process.once("SIGTERM", sigtermHandler); - process.once("beforeExit", beforeExitHandler); + process.on("SIGINT", sigintHandler); + process.on("SIGTERM", sigtermHandler); + process.on("beforeExit", beforeExitHandler); } function removeShutdownHandlers(): void { diff --git a/lib/storage.ts b/lib/storage.ts index 1861833b..78ff95fd 100644 --- a/lib/storage.ts +++ b/lib/storage.ts @@ -985,9 +985,10 @@ function normalizeFlaggedStorage(data: unknown): FlaggedAccountStorageV1 { const cooldownReason = isCooldownReason(rawAccount.cooldownReason) ? rawAccount.cooldownReason : undefined; - const disabledReason = isDisabledReason(rawAccount.disabledReason) - ? rawAccount.disabledReason - : undefined; + const disabledReason = + rawAccount.enabled === false && isDisabledReason(rawAccount.disabledReason) + ? rawAccount.disabledReason + : undefined; const accountTags = normalizeTags(rawAccount.accountTags); const accountNote = typeof rawAccount.accountNote === "string" && rawAccount.accountNote.trim() diff --git a/test/index.test.ts b/test/index.test.ts index 9b5d5df7..178ee46e 100644 --- a/test/index.test.ts +++ b/test/index.test.ts @@ -213,7 +213,7 @@ const mockStorage = { accessToken?: string; expiresAt?: number; enabled?: boolean; - disabledReason?: string; + disabledReason?: "user" | "auth-failure"; addedAt?: number; lastUsed?: number; coolingDownUntil?: number; @@ -2066,6 +2066,54 @@ describe("OpenAIOAuthPlugin fetch handler", () => { expect(await response.text()).toContain("server errors or auth issues"); }); + it("reports disabled pools separately from missing pools", async () => { + const { AccountManager } = await import("../lib/accounts.js"); + + const customManager = { + getAccountCount: () => 2, + getEnabledAccountCount: () => 0, + getCurrentOrNextForFamilyHybrid: () => null, + getSelectionExplainability: () => [], + toAuthDetails: () => ({ + type: "oauth" as const, + access: "access-disabled", + refresh: "refresh-disabled", + expires: Date.now() + 60_000, + }), + hasRefreshToken: () => true, + saveToDiskDebounced: () => {}, + updateFromAuth: () => {}, + clearAuthFailures: () => {}, + incrementAuthFailures: () => 1, + markAccountCoolingDown: () => {}, + markRateLimitedWithReason: () => {}, + recordRateLimit: () => {}, + consumeToken: () => true, + refundToken: () => {}, + markSwitched: () => {}, + removeAccount: () => {}, + recordFailure: () => {}, + recordSuccess: () => {}, + getMinWaitTimeForFamily: () => 0, + shouldShowAccountToast: () => false, + markToastShown: () => {}, + setActiveIndex: () => null, + getAccountsSnapshot: () => [], + }; + vi.spyOn(AccountManager, "loadFromDisk").mockResolvedValueOnce(customManager as never); + globalThis.fetch = vi.fn(); + + const { sdk } = await setupPlugin(); + const response = await sdk.fetch!("https://api.openai.com/v1/chat", { + method: "POST", + body: JSON.stringify({ model: "gpt-5.1" }), + }); + + expect(globalThis.fetch).not.toHaveBeenCalled(); + expect(response.status).toBe(503); + expect(await response.text()).toContain("All stored Codex accounts are disabled"); + }); + it("disables grouped accounts when auth failures hit the threshold", async () => { const fetchHelpers = await import("../lib/request/fetch-helpers.js"); const { AccountManager } = await import("../lib/accounts.js"); @@ -3631,7 +3679,7 @@ describe("OpenAIOAuthPlugin persistAccountPool", () => { } return null; }); - vi.mocked(accountsModule.getAccountIdCandidates).mockReturnValue([]); + vi.mocked(accountsModule.getAccountIdCandidates).mockReturnValueOnce([]); const mockClient = createMockClient(); const { OpenAIOAuthPlugin } = await import("../index.js"); diff --git a/test/shutdown.test.ts b/test/shutdown.test.ts index 222af67a..2ce0600a 100644 --- a/test/shutdown.test.ts +++ b/test/shutdown.test.ts @@ -75,7 +75,7 @@ describe("Graceful shutdown", () => { it("SIGINT handler runs cleanup and exits with code 0", async () => { const capturedHandlers = new Map void>(); - const processOnceSpy = vi.spyOn(process, "once").mockImplementation((event: string | symbol, handler: (...args: unknown[]) => void) => { + const processOnSpy = vi.spyOn(process, "on").mockImplementation((event: string | symbol, handler: (...args: unknown[]) => void) => { capturedHandlers.set(String(event), handler); return process; }); @@ -97,14 +97,14 @@ describe("Graceful shutdown", () => { expect(cleanupFn).toHaveBeenCalled(); expect(processExitSpy).toHaveBeenCalledWith(0); - processOnceSpy.mockRestore(); + processOnSpy.mockRestore(); processExitSpy.mockRestore(); }); it("SIGTERM handler runs cleanup and exits with code 0", async () => { const capturedHandlers = new Map void>(); - const processOnceSpy = vi.spyOn(process, "once").mockImplementation((event: string | symbol, handler: (...args: unknown[]) => void) => { + const processOnSpy = vi.spyOn(process, "on").mockImplementation((event: string | symbol, handler: (...args: unknown[]) => void) => { capturedHandlers.set(String(event), handler); return process; }); @@ -126,14 +126,14 @@ describe("Graceful shutdown", () => { expect(cleanupFn).toHaveBeenCalled(); expect(processExitSpy).toHaveBeenCalledWith(0); - processOnceSpy.mockRestore(); + processOnSpy.mockRestore(); processExitSpy.mockRestore(); }); it("keeps shutdown handlers installed until async cleanup completes", async () => { const capturedHandlers = new Map void>(); - const processOnceSpy = vi.spyOn(process, "once").mockImplementation((event: string | symbol, handler: (...args: unknown[]) => void) => { + const processOnSpy = vi.spyOn(process, "on").mockImplementation((event: string | symbol, handler: (...args: unknown[]) => void) => { capturedHandlers.set(String(event), handler); return process; }); @@ -166,16 +166,60 @@ describe("Graceful shutdown", () => { expect(processExitSpy).toHaveBeenCalledWith(0); }); } finally { - processOnceSpy.mockRestore(); + processOnSpy.mockRestore(); processOffSpy.mockRestore(); processExitSpy.mockRestore(); } }); + it("ignores repeated signals while async cleanup is already running", async () => { + const capturedHandlers = new Map void>(); + + const processOnSpy = vi.spyOn(process, "on").mockImplementation((event: string | symbol, handler: (...args: unknown[]) => void) => { + capturedHandlers.set(String(event), handler); + return process; + }); + const processExitSpy = vi.spyOn(process, "exit").mockImplementation(() => undefined as never); + + vi.resetModules(); + const { registerCleanup: freshRegister, runCleanup: freshRunCleanup } = await import("../lib/shutdown.js"); + await freshRunCleanup(); + + let resolveCleanup!: () => void; + const cleanupPromise = new Promise((resolve) => { + resolveCleanup = resolve; + }); + const cleanupFn = vi.fn(async () => { + await cleanupPromise; + }); + freshRegister(cleanupFn); + + try { + const sigtermHandler = capturedHandlers.get("SIGTERM"); + expect(sigtermHandler).toBeDefined(); + + sigtermHandler!(); + sigtermHandler!(); + await Promise.resolve(); + + expect(cleanupFn).toHaveBeenCalledTimes(1); + expect(processExitSpy).not.toHaveBeenCalled(); + + resolveCleanup(); + await vi.waitFor(() => { + expect(processExitSpy).toHaveBeenCalledTimes(1); + expect(processExitSpy).toHaveBeenCalledWith(0); + }); + } finally { + processOnSpy.mockRestore(); + processExitSpy.mockRestore(); + } + }); + it("beforeExit handler runs cleanup without calling exit", async () => { const capturedHandlers = new Map void>(); - const processOnceSpy = vi.spyOn(process, "once").mockImplementation((event: string | symbol, handler: (...args: unknown[]) => void) => { + const processOnSpy = vi.spyOn(process, "on").mockImplementation((event: string | symbol, handler: (...args: unknown[]) => void) => { capturedHandlers.set(String(event), handler); return process; }); @@ -198,28 +242,28 @@ describe("Graceful shutdown", () => { }); expect(processExitSpy).not.toHaveBeenCalled(); } finally { - processOnceSpy.mockRestore(); + processOnSpy.mockRestore(); processExitSpy.mockRestore(); } }); it("signal handlers are only registered once", async () => { - const processOnceSpy = vi.spyOn(process, "once").mockImplementation(() => process); + const processOnSpy = vi.spyOn(process, "on").mockImplementation(() => process); vi.resetModules(); const { registerCleanup: freshRegister } = await import("../lib/shutdown.js"); freshRegister(() => {}); - const firstCallCount = processOnceSpy.mock.calls.length; + const firstCallCount = processOnSpy.mock.calls.length; freshRegister(() => {}); - expect(processOnceSpy.mock.calls.length).toBe(firstCallCount); + expect(processOnSpy.mock.calls.length).toBe(firstCallCount); - processOnceSpy.mockRestore(); + processOnSpy.mockRestore(); }); it("re-registers signal handlers after cleanup resets shutdown state", async () => { - const processOnceSpy = vi.spyOn(process, "once").mockImplementation(() => process); + const processOnSpy = vi.spyOn(process, "on").mockImplementation(() => process); const processOffSpy = vi.spyOn(process, "off").mockImplementation(() => process); vi.resetModules(); @@ -229,14 +273,14 @@ describe("Graceful shutdown", () => { } = await import("../lib/shutdown.js"); freshRegister(() => {}); - expect(processOnceSpy).toHaveBeenCalledTimes(3); + expect(processOnSpy).toHaveBeenCalledTimes(3); await freshRunCleanup(); expect(processOffSpy).toHaveBeenCalledTimes(3); freshRegister(() => {}); - expect(processOnceSpy).toHaveBeenCalledTimes(6); + expect(processOnSpy).toHaveBeenCalledTimes(6); - processOnceSpy.mockRestore(); + processOnSpy.mockRestore(); processOffSpy.mockRestore(); }); }); diff --git a/test/storage.test.ts b/test/storage.test.ts index 67c578ed..285d5101 100644 --- a/test/storage.test.ts +++ b/test/storage.test.ts @@ -1494,6 +1494,31 @@ describe("storage", () => { expect(loaded.accounts[0]?.accountIdSource).toBe("id_token"); }); + it("strips stale disabledReason from enabled flagged records during load", async () => { + const flaggedPath = join(dirname(testStoragePath), "openai-codex-flagged-accounts.json"); + await fs.writeFile( + flaggedPath, + JSON.stringify({ + version: 1, + accounts: [ + { + refreshToken: "flagged-enabled", + flaggedAt: 123, + addedAt: 123, + lastUsed: 123, + enabled: true, + disabledReason: "auth-failure", + }, + ], + }), + ); + + const loaded = await loadFlaggedAccounts(); + expect(loaded.accounts).toHaveLength(1); + expect(loaded.accounts[0]?.enabled).toBe(true); + expect(loaded.accounts[0]?.disabledReason).toBeUndefined(); + }); + it("retries flagged storage rename on EBUSY and succeeds", async () => { const originalRename = fs.rename.bind(fs); let attemptCount = 0; From e2b313de8f301d271764b1069146dcee65645f5d Mon Sep 17 00:00:00 2001 From: ndycode Date: Sun, 15 Mar 2026 03:20:52 +0800 Subject: [PATCH 19/43] Stabilize shutdown signal tests --- test/shutdown.test.ts | 44 +++++++++++++++++++++++-------------------- 1 file changed, 24 insertions(+), 20 deletions(-) diff --git a/test/shutdown.test.ts b/test/shutdown.test.ts index 2ce0600a..2b75b3d4 100644 --- a/test/shutdown.test.ts +++ b/test/shutdown.test.ts @@ -88,17 +88,19 @@ describe("Graceful shutdown", () => { const cleanupFn = vi.fn(); freshRegister(cleanupFn); - const sigintHandler = capturedHandlers.get("SIGINT"); - expect(sigintHandler).toBeDefined(); - - sigintHandler!(); - await new Promise((r) => setTimeout(r, 10)); - - expect(cleanupFn).toHaveBeenCalled(); - expect(processExitSpy).toHaveBeenCalledWith(0); + try { + const sigintHandler = capturedHandlers.get("SIGINT"); + expect(sigintHandler).toBeDefined(); - processOnSpy.mockRestore(); - processExitSpy.mockRestore(); + sigintHandler!(); + await vi.waitFor(() => { + expect(cleanupFn).toHaveBeenCalled(); + expect(processExitSpy).toHaveBeenCalledWith(0); + }); + } finally { + processOnSpy.mockRestore(); + processExitSpy.mockRestore(); + } }); it("SIGTERM handler runs cleanup and exits with code 0", async () => { @@ -117,17 +119,19 @@ describe("Graceful shutdown", () => { const cleanupFn = vi.fn(); freshRegister(cleanupFn); - const sigtermHandler = capturedHandlers.get("SIGTERM"); - expect(sigtermHandler).toBeDefined(); - - sigtermHandler!(); - await new Promise((r) => setTimeout(r, 10)); - - expect(cleanupFn).toHaveBeenCalled(); - expect(processExitSpy).toHaveBeenCalledWith(0); + try { + const sigtermHandler = capturedHandlers.get("SIGTERM"); + expect(sigtermHandler).toBeDefined(); - processOnSpy.mockRestore(); - processExitSpy.mockRestore(); + sigtermHandler!(); + await vi.waitFor(() => { + expect(cleanupFn).toHaveBeenCalled(); + expect(processExitSpy).toHaveBeenCalledWith(0); + }); + } finally { + processOnSpy.mockRestore(); + processExitSpy.mockRestore(); + } }); it("keeps shutdown handlers installed until async cleanup completes", async () => { From fca1832729d767943653cb97e26407db07aa0450 Mon Sep 17 00:00:00 2001 From: ndycode Date: Sun, 15 Mar 2026 03:50:39 +0800 Subject: [PATCH 20/43] chore: retrigger review bots From 17e819de82082856168edfb64882a1018a9ac95e Mon Sep 17 00:00:00 2001 From: ndycode Date: Sun, 15 Mar 2026 04:08:15 +0800 Subject: [PATCH 21/43] Fix disabled-account persistence edge cases --- lib/accounts.ts | 32 +++++++---- lib/storage.ts | 33 +++++++---- test/accounts.test.ts | 125 +++++++++++++++++++++++++++++++++++++++--- test/storage.test.ts | 24 ++++++++ 4 files changed, 184 insertions(+), 30 deletions(-) diff --git a/lib/accounts.ts b/lib/accounts.ts index 2ce3dae5..b129991a 100644 --- a/lib/accounts.ts +++ b/lib/accounts.ts @@ -208,6 +208,12 @@ export interface AccountSelectionExplainability { lastUsed: number; } +export type SetAccountEnabledFailureReason = "auth-failure-blocked" | "invalid-index"; + +export type SetAccountEnabledResult = + | { ok: true; account: ManagedAccount } + | { ok: false; reason: SetAccountEnabledFailureReason }; + export class AccountManager { private accounts: ManagedAccount[] = []; private cursorByFamily: Record = initFamilyState(0); @@ -937,25 +943,23 @@ export class AccountManager { index: number, enabled: boolean, reason?: AccountDisabledReason, - ): ManagedAccount | null { - if (!Number.isFinite(index)) return null; - if (index < 0 || index >= this.accounts.length) return null; + ): SetAccountEnabledResult { + if (!Number.isFinite(index)) return { ok: false, reason: "invalid-index" }; + if (index < 0 || index >= this.accounts.length) return { ok: false, reason: "invalid-index" }; const account = this.accounts[index]; - if (!account) return null; + if (!account) return { ok: false, reason: "invalid-index" }; if (enabled && account.disabledReason === "auth-failure") { - return null; + return { ok: false, reason: "auth-failure-blocked" }; } account.enabled = enabled; if (enabled) { delete account.disabledReason; } else if (reason) { account.disabledReason = reason; - } else { - if (account.disabledReason !== "auth-failure") { - delete account.disabledReason; - } + } else if (account.disabledReason !== "auth-failure") { + account.disabledReason = "user"; } - return account; + return { ok: true, account }; } async saveToDisk(): Promise { @@ -1002,7 +1006,13 @@ export class AccountManager { const previousSave = this.pendingSave; const nextSave = (async () => { if (previousSave) { - await previousSave; + try { + await previousSave; + } catch (error) { + log.warn("Continuing queued save after previous save failure", { + error: error instanceof Error ? error.message : String(error), + }); + } } await saveOperation(); })().finally(() => { diff --git a/lib/storage.ts b/lib/storage.ts index 78ff95fd..045cab46 100644 --- a/lib/storage.ts +++ b/lib/storage.ts @@ -134,6 +134,17 @@ const WINDOWS_RENAME_RETRY_ATTEMPTS = 5; const WINDOWS_RENAME_RETRY_BASE_DELAY_MS = 10; const PRE_IMPORT_BACKUP_WRITE_TIMEOUT_MS = 3_000; +function isDisabledReason(value: unknown): value is AccountDisabledReason { + return value === "user" || value === "auth-failure"; +} + +function normalizeDisabledReason( + enabled: unknown, + disabledReason: unknown, +): AccountDisabledReason | undefined { + return enabled === false && isDisabledReason(disabledReason) ? disabledReason : undefined; +} + function isWindowsLockError(error: unknown): error is NodeJS.ErrnoException { const code = (error as NodeJS.ErrnoException)?.code; return code === "EPERM" || code === "EBUSY"; @@ -621,10 +632,15 @@ export function normalizeAccountStorage(data: unknown): AccountStorageV3 | null ? migrateV1ToV3(data as unknown as AccountStorageV1) : (data as unknown as AccountStorageV3); - const validAccounts = baseStorage.accounts.filter( - (account): account is AccountMetadataV3 => - isRecord(account) && typeof account.refreshToken === "string" && !!account.refreshToken.trim(), - ); + const validAccounts: AccountMetadataV3[] = baseStorage.accounts + .filter( + (account): account is AccountMetadataV3 => + isRecord(account) && typeof account.refreshToken === "string" && !!account.refreshToken.trim(), + ) + .map((account) => ({ + ...account, + disabledReason: normalizeDisabledReason(account.enabled, account.disabledReason), + })); const deduplicatedAccounts = deduplicateAccountsForStorage(validAccounts); @@ -950,10 +966,6 @@ function normalizeFlaggedStorage(data: unknown): FlaggedAccountStorageV1 { value: unknown, ): value is AccountMetadataV3["cooldownReason"] => value === "auth-failure" || value === "network-error"; - const isDisabledReason = ( - value: unknown, - ): value is AccountDisabledReason => - value === "user" || value === "auth-failure"; const normalizeTags = (value: unknown): string[] | undefined => { if (!Array.isArray(value)) return undefined; const normalized = value @@ -985,10 +997,7 @@ function normalizeFlaggedStorage(data: unknown): FlaggedAccountStorageV1 { const cooldownReason = isCooldownReason(rawAccount.cooldownReason) ? rawAccount.cooldownReason : undefined; - const disabledReason = - rawAccount.enabled === false && isDisabledReason(rawAccount.disabledReason) - ? rawAccount.disabledReason - : undefined; + const disabledReason = normalizeDisabledReason(rawAccount.enabled, rawAccount.disabledReason); const accountTags = normalizeTags(rawAccount.accountTags); const accountNote = typeof rawAccount.accountNote === "string" && rawAccount.accountNote.trim() diff --git a/test/accounts.test.ts b/test/accounts.test.ts index 6b483d35..921b0869 100644 --- a/test/accounts.test.ts +++ b/test/accounts.test.ts @@ -1081,15 +1081,46 @@ describe("AccountManager", () => { const disabled = manager.setAccountEnabled(0, false, "user"); expect(disabled).toMatchObject({ - enabled: false, - disabledReason: "user", + ok: true, + account: { + enabled: false, + disabledReason: "user", + }, }); const reenabled = manager.setAccountEnabled(0, true); expect(reenabled).toMatchObject({ - enabled: true, + ok: true, + account: { + enabled: true, + }, + }); + if (!reenabled.ok) { + throw new Error("expected setAccountEnabled to re-enable account"); + } + expect(reenabled.account.disabledReason).toBeUndefined(); + }); + + it("defaults disable reason to user when disabling without an explicit reason", () => { + const now = Date.now(); + const stored = { + version: 3 as const, + activeIndex: 0, + accounts: [ + { refreshToken: "token-1", addedAt: now, lastUsed: now }, + ], + }; + + const manager = new AccountManager(undefined, stored); + + const disabled = manager.setAccountEnabled(0, false); + expect(disabled).toMatchObject({ + ok: true, + account: { + enabled: false, + disabledReason: "user", + }, }); - expect(reenabled?.disabledReason).toBeUndefined(); }); it("preserves auth-failure disabledReason when disabling without an explicit reason", () => { @@ -1112,8 +1143,11 @@ describe("AccountManager", () => { const disabled = manager.setAccountEnabled(0, false); expect(disabled).toMatchObject({ - enabled: false, - disabledReason: "auth-failure", + ok: true, + account: { + enabled: false, + disabledReason: "auth-failure", + }, }); }); @@ -1136,7 +1170,10 @@ describe("AccountManager", () => { const manager = new AccountManager(undefined, stored); const reenabled = manager.setAccountEnabled(0, true); - expect(reenabled).toBeNull(); + expect(reenabled).toEqual({ + ok: false, + reason: "auth-failure-blocked", + }); expect(manager.getAccountsSnapshot()[0]).toMatchObject({ enabled: false, disabledReason: "auth-failure", @@ -1649,6 +1686,80 @@ describe("AccountManager", () => { expect(mockSaveAccounts).toHaveBeenCalledTimes(2); }); + + it("continues a queued save after the previous save fails", async () => { + const { saveAccounts } = await import("../lib/storage.js"); + const mockSaveAccounts = vi.mocked(saveAccounts); + mockSaveAccounts.mockClear(); + + const savedSnapshots: Array< + Array<{ + refreshToken?: string; + enabled?: false; + disabledReason?: string; + }> + > = []; + + let rejectFirstSave!: (error: Error) => void; + mockSaveAccounts.mockImplementationOnce(async (storage) => { + savedSnapshots.push( + storage.accounts.map((account) => ({ + refreshToken: account.refreshToken, + enabled: account.enabled, + disabledReason: account.disabledReason, + })), + ); + await new Promise((_, reject) => { + rejectFirstSave = reject; + }); + }); + mockSaveAccounts.mockImplementationOnce(async (storage) => { + savedSnapshots.push( + storage.accounts.map((account) => ({ + refreshToken: account.refreshToken, + enabled: account.enabled, + disabledReason: account.disabledReason, + })), + ); + }); + + const now = Date.now(); + const stored = { + version: 3 as const, + activeIndex: 0, + accounts: [ + { refreshToken: "token-1", addedAt: now, lastUsed: now }, + ], + }; + + const manager = new AccountManager(undefined, stored); + + manager.saveToDiskDebounced(50); + await vi.advanceTimersByTimeAsync(60); + expect(mockSaveAccounts).toHaveBeenCalledTimes(1); + + const account = manager.getAccountsSnapshot()[0]!; + manager.disableAccountsWithSameRefreshToken(account); + manager.saveToDiskDebounced(50); + await vi.advanceTimersByTimeAsync(60); + + expect(mockSaveAccounts).toHaveBeenCalledTimes(1); + + rejectFirstSave(Object.assign(new Error("EBUSY: file in use"), { code: "EBUSY" })); + await vi.advanceTimersByTimeAsync(0); + for (let turn = 0; turn < 6; turn += 1) { + await Promise.resolve(); + } + + expect(mockSaveAccounts).toHaveBeenCalledTimes(2); + expect(savedSnapshots[1]).toEqual([ + expect.objectContaining({ + refreshToken: "token-1", + enabled: false, + disabledReason: "auth-failure", + }), + ]); + }); }); describe("constructor edge cases", () => { diff --git a/test/storage.test.ts b/test/storage.test.ts index 285d5101..cf96ef4e 100644 --- a/test/storage.test.ts +++ b/test/storage.test.ts @@ -1074,6 +1074,30 @@ describe("storage", () => { }); }); + it("strips invalid disabledReason values from disabled v3 accounts", () => { + const data = { + version: 3, + activeIndex: 0, + accounts: [ + { + refreshToken: "t1", + accountId: "A", + enabled: false, + disabledReason: "future-reason", + addedAt: 1, + lastUsed: 1, + }, + ], + }; + + const result = normalizeAccountStorage(data); + expect(result?.accounts[0]).toMatchObject({ + refreshToken: "t1", + enabled: false, + }); + expect(result?.accounts[0]?.disabledReason).toBeUndefined(); + }); + it("preserves activeIndexByFamily when valid", () => { const data = { version: 3, From 943d36334ac6f584ae66c77f4468c2875ed193f4 Mon Sep 17 00:00:00 2001 From: ndycode Date: Sun, 15 Mar 2026 04:31:10 +0800 Subject: [PATCH 22/43] Tighten disabled-account shutdown persistence --- index.ts | 9 ++++++-- lib/accounts.ts | 4 +++- lib/shutdown.ts | 21 +++++++++++------- test/accounts.test.ts | 6 ++++-- test/index.test.ts | 8 +++---- test/shutdown.test.ts | 50 +++++++++++++++++++++++++++++++++++++++++++ 6 files changed, 81 insertions(+), 17 deletions(-) diff --git a/index.ts b/index.ts index 920fad90..abf57a13 100644 --- a/index.ts +++ b/index.ts @@ -3531,8 +3531,13 @@ while (attempted.size < Math.max(1, accountCount)) { ); continue; } - target.enabled = shouldEnable ? true : false; - target.disabledReason = shouldEnable ? undefined : "user"; + if (shouldEnable) { + delete target.enabled; + delete target.disabledReason; + } else { + target.enabled = false; + target.disabledReason = "user"; + } await saveAccounts(workingStorage); invalidateAccountManagerCache(); console.log( diff --git a/lib/accounts.ts b/lib/accounts.ts index b129991a..ff49332e 100644 --- a/lib/accounts.ts +++ b/lib/accounts.ts @@ -315,6 +315,7 @@ export class AccountManager { email: matchesFallback ? fallbackAccountEmail ?? sanitizeEmail(account.email) : sanitizeEmail(account.email), + // Storage only persists `enabled: false`; in memory we normalize to a concrete boolean. refreshToken, enabled: account.enabled !== false, disabledReason: @@ -984,6 +985,7 @@ export class AccountManager { refreshToken: account.refreshToken, accessToken: account.access, expiresAt: account.expires, + // Persist enabled accounts by omitting the flag to preserve the storage convention. enabled: account.enabled === false ? false : undefined, disabledReason: account.enabled === false ? account.disabledReason ?? "user" : undefined, @@ -1051,7 +1053,7 @@ export class AccountManager { log.warn("flushPendingSave exceeded max iterations; possible save loop", { iterations: flushIterations - 1, }); - return; + throw new Error("flushPendingSave: exceeded max flush iterations; save state may be incomplete"); } const hadDebouncedSave = !!this.saveDebounceTimer; if (this.saveDebounceTimer) { diff --git a/lib/shutdown.ts b/lib/shutdown.ts index a3df654c..70af03d2 100644 --- a/lib/shutdown.ts +++ b/lib/shutdown.ts @@ -26,15 +26,17 @@ export function runCleanup(): Promise { } cleanupInFlight = (async () => { - const fns = [...cleanupFunctions]; - cleanupFunctions.length = 0; - try { - for (const fn of fns) { - try { - await fn(); - } catch { - // Ignore cleanup errors during shutdown + while (cleanupFunctions.length > 0) { + const fns = [...cleanupFunctions]; + cleanupFunctions.length = 0; + + for (const fn of fns) { + try { + await fn(); + } catch { + // Ignore cleanup errors during shutdown + } } } } finally { @@ -44,6 +46,9 @@ export function runCleanup(): Promise { })().finally(() => { cleanupInFlight = null; signalExitPending = false; + if (cleanupFunctions.length > 0) { + ensureShutdownHandler(); + } }); return cleanupInFlight; diff --git a/test/accounts.test.ts b/test/accounts.test.ts index 921b0869..10453bba 100644 --- a/test/accounts.test.ts +++ b/test/accounts.test.ts @@ -2380,7 +2380,7 @@ describe("AccountManager", () => { } }); - it("warns and exits if flushPendingSave keeps discovering new saves", async () => { + it("warns and rejects if flushPendingSave keeps discovering new saves", async () => { vi.useFakeTimers(); try { const { saveAccounts } = await import("../lib/storage.js"); @@ -2410,7 +2410,9 @@ describe("AccountManager", () => { }); manager.saveToDiskDebounced(0); - await manager.flushPendingSave(); + await expect(manager.flushPendingSave()).rejects.toThrow( + "flushPendingSave: exceeded max flush iterations; save state may be incomplete", + ); expect(accountLoggerWarn).toHaveBeenCalledWith( "flushPendingSave exceeded max iterations; possible save loop", diff --git a/test/index.test.ts b/test/index.test.ts index 178ee46e..3c689ea7 100644 --- a/test/index.test.ts +++ b/test/index.test.ts @@ -3945,14 +3945,14 @@ describe("OpenAIOAuthPlugin persistAccountPool", () => { }); expect(saveAccountsMock.mock.calls[1]?.[0].accounts[0]).toMatchObject({ accountId: "workspace-managed", - enabled: true, - disabledReason: undefined, }); + expect(saveAccountsMock.mock.calls[1]?.[0].accounts[0]).not.toHaveProperty("enabled"); + expect(saveAccountsMock.mock.calls[1]?.[0].accounts[0]).not.toHaveProperty("disabledReason"); expect(mockStorage.accounts[0]).toMatchObject({ accountId: "workspace-managed", - enabled: true, - disabledReason: undefined, }); + expect(mockStorage.accounts[0]).not.toHaveProperty("enabled"); + expect(mockStorage.accounts[0]).not.toHaveProperty("disabledReason"); }); it("keeps auth-failure disables blocked in the auth manage menu until a fresh login", async () => { diff --git a/test/shutdown.test.ts b/test/shutdown.test.ts index 2b75b3d4..c08cbf1d 100644 --- a/test/shutdown.test.ts +++ b/test/shutdown.test.ts @@ -65,6 +65,21 @@ describe("Graceful shutdown", () => { expect(getCleanupCount()).toBe(0); }); + it("drains cleanup functions registered while cleanup is already running", async () => { + const order: number[] = []; + registerCleanup(async () => { + order.push(1); + registerCleanup(() => { + order.push(2); + }); + }); + + await runCleanup(); + + expect(order).toEqual([1, 2]); + expect(getCleanupCount()).toBe(0); + }); + it("unregister is no-op for non-registered function", () => { const fn = vi.fn(); unregisterCleanup(fn); @@ -287,5 +302,40 @@ describe("Graceful shutdown", () => { processOnSpy.mockRestore(); processOffSpy.mockRestore(); }); + + it("reinstalls signal handlers when cleanup is registered during teardown", async () => { + const processOnSpy = vi.spyOn(process, "on").mockImplementation(() => process); + + vi.resetModules(); + const { + registerCleanup: freshRegister, + runCleanup: freshRunCleanup, + getCleanupCount: freshGetCleanupCount, + } = await import("../lib/shutdown.js"); + await freshRunCleanup(); + + let registeredDuringTeardown = false; + const processOffSpy = vi.spyOn(process, "off").mockImplementation((event: string | symbol) => { + if (!registeredDuringTeardown && String(event) === "beforeExit") { + registeredDuringTeardown = true; + freshRegister(() => {}); + } + return process; + }); + + try { + freshRegister(() => {}); + expect(processOnSpy).toHaveBeenCalledTimes(3); + + await freshRunCleanup(); + + expect(freshGetCleanupCount()).toBe(1); + expect(processOffSpy).toHaveBeenCalledTimes(3); + expect(processOnSpy).toHaveBeenCalledTimes(6); + } finally { + processOnSpy.mockRestore(); + processOffSpy.mockRestore(); + } + }); }); }); From 321a9739a77ddfb04a11432c80fd3c45ccf16fef Mon Sep 17 00:00:00 2001 From: ndycode Date: Sun, 15 Mar 2026 05:01:13 +0800 Subject: [PATCH 23/43] Harden storage validation and cleanup tracking --- index.ts | 3 ++- lib/schemas.ts | 10 +++++++- test/index.test.ts | 56 ++++++++++++++++++++++++++++++++++++++++++++ test/schemas.test.ts | 29 +++++++++++++++++++++++ 4 files changed, 96 insertions(+), 2 deletions(-) diff --git a/index.ts b/index.ts index abf57a13..d9a722f6 100644 --- a/index.ts +++ b/index.ts @@ -1705,11 +1705,12 @@ export const OpenAIOAuthPlugin: Plugin = async ({ client }: PluginInput) => { for (const manager of [...trackedAccountManagersForCleanup]) { try { await manager.flushPendingSave(); - trackedAccountManagersForCleanup.delete(manager); } catch (error) { logWarn("[shutdown] flushPendingSave failed; disabled state may not be persisted", { error: error instanceof Error ? error.message : String(error), }); + } finally { + trackedAccountManagersForCleanup.delete(manager); } } }; diff --git a/lib/schemas.ts b/lib/schemas.ts index 2ba4c1ff..26e233da 100644 --- a/lib/schemas.ts +++ b/lib/schemas.ts @@ -74,7 +74,15 @@ export const CooldownReasonSchema = z.enum(["auth-failure", "network-error"]); export type CooldownReasonFromSchema = z.infer; -export const DisabledReasonSchema = z.enum(["user", "auth-failure"]); +const normalizeDisabledReason = (value: unknown): unknown => + value === "user" || value === "auth-failure" ? value : undefined; + +// Storage normalization strips unknown disabled reasons later; keep schema parsing +// lenient so legacy/downgraded files don't warn or fail before that step runs. +export const DisabledReasonSchema = z.preprocess( + normalizeDisabledReason, + z.enum(["user", "auth-failure"]).optional(), +); export type DisabledReasonFromSchema = z.infer; diff --git a/test/index.test.ts b/test/index.test.ts index 3c689ea7..54358838 100644 --- a/test/index.test.ts +++ b/test/index.test.ts @@ -3779,6 +3779,62 @@ describe("OpenAIOAuthPlugin persistAccountPool", () => { ); }); + it("drops failed tracked managers after shutdown cleanup logs the failure", async () => { + const accountsModule = await import("../lib/accounts.js"); + const loggerModule = await import("../lib/logger.js"); + const failingManager = await accountsModule.AccountManager.loadFromDisk(); + const succeedingManager = await accountsModule.AccountManager.loadFromDisk(); + const failingFlush = vi.fn(async () => { + throw new Error("EBUSY"); + }); + const succeedingFlush = vi.fn(async () => {}); + failingManager.flushPendingSave = failingFlush; + succeedingManager.flushPendingSave = succeedingFlush; + vi.spyOn(accountsModule.AccountManager, "loadFromDisk") + .mockResolvedValueOnce(failingManager) + .mockResolvedValueOnce(succeedingManager); + + const { OpenAIOAuthPlugin } = await import("../index.js"); + const firstPlugin = (await OpenAIOAuthPlugin({ + client: createMockClient(), + } as never)) as unknown as PluginType; + await firstPlugin.auth.loader( + async () => ({ + type: "oauth", + access: "access-token-1", + refresh: "refresh-token-1", + expires: Date.now() + 60_000, + }) as never, + {}, + ); + + vi.mocked(loggerModule.logWarn).mockClear(); + await runCleanup(); + + const secondPlugin = (await OpenAIOAuthPlugin({ + client: createMockClient(), + } as never)) as unknown as PluginType; + await secondPlugin.auth.loader( + async () => ({ + type: "oauth", + access: "access-token-2", + refresh: "refresh-token-2", + expires: Date.now() + 60_000, + }) as never, + {}, + ); + + vi.mocked(loggerModule.logWarn).mockClear(); + await runCleanup(); + + expect(failingFlush).toHaveBeenCalledTimes(1); + expect(succeedingFlush).toHaveBeenCalledTimes(1); + expect(vi.mocked(loggerModule.logWarn)).not.toHaveBeenCalledWith( + "[shutdown] flushPendingSave failed; disabled state may not be persisted", + expect.anything(), + ); + }); + it("flushes stale invalidated managers alongside the current manager during shutdown cleanup", async () => { const accountsModule = await import("../lib/accounts.js"); const cliModule = await import("../lib/cli.js"); diff --git a/test/schemas.test.ts b/test/schemas.test.ts index 81283aff..fb2e390d 100644 --- a/test/schemas.test.ts +++ b/test/schemas.test.ts @@ -155,6 +155,18 @@ describe("AccountMetadataV3Schema", () => { expect(result.success).toBe(false); }); + it("normalizes invalid disabledReason values to undefined", () => { + const result = AccountMetadataV3Schema.safeParse({ + ...validAccount, + enabled: false, + disabledReason: "future-reason", + }); + expect(result.success).toBe(true); + if (result.success) { + expect(result.data.disabledReason).toBeUndefined(); + } + }); + it("normalizes account tags by trimming, lowercasing, and filtering blanks", () => { const result = AccountMetadataV3Schema.safeParse({ ...validAccount, @@ -541,4 +553,21 @@ describe("getValidationErrors", () => { expect(errors.length).toBeGreaterThan(0); expect(errors[0]).not.toMatch(/^[a-zA-Z0-9_.]+: /); }); + + it("does not warn for unknown disabledReason values in account storage", () => { + const errors = getValidationErrors(AnyAccountStorageSchema, { + version: 3, + accounts: [ + { + refreshToken: "rt", + enabled: false, + disabledReason: "future-reason", + addedAt: 1, + lastUsed: 1, + }, + ], + activeIndex: 0, + }); + expect(errors).toEqual([]); + }); }); From fb1461f32acb47d645ec84ba975628788d3eef9b Mon Sep 17 00:00:00 2001 From: ndycode Date: Sun, 15 Mar 2026 05:05:16 +0800 Subject: [PATCH 24/43] Clear stale cooldowns on re-enable --- index.ts | 4 ++++ lib/accounts.ts | 1 + test/accounts.test.ts | 10 +++++++++- test/index.test.ts | 9 +++++++++ 4 files changed, 23 insertions(+), 1 deletion(-) diff --git a/index.ts b/index.ts index d9a722f6..55101215 100644 --- a/index.ts +++ b/index.ts @@ -3527,6 +3527,10 @@ while (attempted.size < Math.max(1, accountCount)) { if (target) { const shouldEnable = target.enabled === false; if (shouldEnable && target.disabledReason === "auth-failure") { + logWarn("[account-menu] blocked re-enable for auth-failure disabled account", { + accountId: target.accountId ?? null, + email: target.email ?? null, + }); console.log( "\nThis account was disabled after repeated auth failures. Run 'opencode auth login' to re-enable with fresh credentials.\n", ); diff --git a/lib/accounts.ts b/lib/accounts.ts index ff49332e..8b8dbc8e 100644 --- a/lib/accounts.ts +++ b/lib/accounts.ts @@ -955,6 +955,7 @@ export class AccountManager { account.enabled = enabled; if (enabled) { delete account.disabledReason; + this.clearAccountCooldown(account); } else if (reason) { account.disabledReason = reason; } else if (account.disabledReason !== "auth-failure") { diff --git a/test/accounts.test.ts b/test/accounts.test.ts index 10453bba..31c57b37 100644 --- a/test/accounts.test.ts +++ b/test/accounts.test.ts @@ -1073,7 +1073,13 @@ describe("AccountManager", () => { version: 3 as const, activeIndex: 0, accounts: [ - { refreshToken: "token-1", addedAt: now, lastUsed: now }, + { + refreshToken: "token-1", + coolingDownUntil: now + 60_000, + cooldownReason: "network-error" as const, + addedAt: now, + lastUsed: now, + }, ], }; @@ -1099,6 +1105,8 @@ describe("AccountManager", () => { throw new Error("expected setAccountEnabled to re-enable account"); } expect(reenabled.account.disabledReason).toBeUndefined(); + expect(reenabled.account.coolingDownUntil).toBeUndefined(); + expect(reenabled.account.cooldownReason).toBeUndefined(); }); it("defaults disable reason to user when disabling without an explicit reason", () => { diff --git a/test/index.test.ts b/test/index.test.ts index 54358838..31e21349 100644 --- a/test/index.test.ts +++ b/test/index.test.ts @@ -4013,6 +4013,7 @@ describe("OpenAIOAuthPlugin persistAccountPool", () => { it("keeps auth-failure disables blocked in the auth manage menu until a fresh login", async () => { const cliModule = await import("../lib/cli.js"); + const loggerModule = await import("../lib/logger.js"); const storageModule = await import("../lib/storage.js"); const consoleLog = vi.spyOn(console, "log").mockImplementation(() => {}); @@ -4044,6 +4045,7 @@ describe("OpenAIOAuthPlugin persistAccountPool", () => { authorize: (inputs?: Record) => Promise<{ instructions: string }>; }; + vi.mocked(loggerModule.logWarn).mockClear(); const authResult = await autoMethod.authorize(); expect(authResult.instructions).toBe("Authentication cancelled"); @@ -4058,6 +4060,13 @@ describe("OpenAIOAuthPlugin persistAccountPool", () => { expect(consoleLog).toHaveBeenCalledWith( expect.stringContaining("Run 'opencode auth login' to re-enable with fresh credentials."), ); + expect(vi.mocked(loggerModule.logWarn)).toHaveBeenCalledWith( + "[account-menu] blocked re-enable for auth-failure disabled account", + expect.objectContaining({ + accountId: "workspace-auth-failure", + email: "blocked@example.com", + }), + ); } finally { consoleLog.mockRestore(); } From f08847f6134e53dffb4c0c2bca8e3f71ea12e772 Mon Sep 17 00:00:00 2001 From: ndycode Date: Sun, 15 Mar 2026 05:24:58 +0800 Subject: [PATCH 25/43] Harden shutdown signal cleanup --- lib/shutdown.ts | 7 ++++++- test/shutdown.test.ts | 37 +++++++++++++++++++++++++++++++++++++ 2 files changed, 43 insertions(+), 1 deletion(-) diff --git a/lib/shutdown.ts b/lib/shutdown.ts index 70af03d2..61773e81 100644 --- a/lib/shutdown.ts +++ b/lib/shutdown.ts @@ -7,6 +7,7 @@ let sigtermHandler: (() => void) | null = null; let beforeExitHandler: (() => void) | null = null; let cleanupInFlight: Promise | null = null; let signalExitPending = false; +let shouldResetSignalExitPending = true; export function registerCleanup(fn: CleanupFn): void { cleanupFunctions.push(fn); @@ -45,7 +46,10 @@ export function runCleanup(): Promise { } })().finally(() => { cleanupInFlight = null; - signalExitPending = false; + if (shouldResetSignalExitPending) { + signalExitPending = false; + } + shouldResetSignalExitPending = true; if (cleanupFunctions.length > 0) { ensureShutdownHandler(); } @@ -63,6 +67,7 @@ function ensureShutdownHandler(): void { return; } signalExitPending = true; + shouldResetSignalExitPending = false; void runCleanup().finally(() => { process.exit(0); }); diff --git a/test/shutdown.test.ts b/test/shutdown.test.ts index c08cbf1d..73a7d3b8 100644 --- a/test/shutdown.test.ts +++ b/test/shutdown.test.ts @@ -235,6 +235,43 @@ describe("Graceful shutdown", () => { } }); + it("keeps later signals ignored after cleanup completes until process exit", async () => { + const capturedHandlers = new Map void>(); + + const processOnSpy = vi.spyOn(process, "on").mockImplementation((event: string | symbol, handler: (...args: unknown[]) => void) => { + capturedHandlers.set(String(event), handler); + return process; + }); + const processExitSpy = vi.spyOn(process, "exit").mockImplementation(() => undefined as never); + + vi.resetModules(); + const { registerCleanup: freshRegister, runCleanup: freshRunCleanup } = await import("../lib/shutdown.js"); + await freshRunCleanup(); + + const cleanupFn = vi.fn(); + freshRegister(cleanupFn); + + try { + const sigtermHandler = capturedHandlers.get("SIGTERM"); + expect(sigtermHandler).toBeDefined(); + + sigtermHandler!(); + await vi.waitFor(() => { + expect(cleanupFn).toHaveBeenCalledTimes(1); + expect(processExitSpy).toHaveBeenCalledTimes(1); + }); + + sigtermHandler!(); + await Promise.resolve(); + + expect(cleanupFn).toHaveBeenCalledTimes(1); + expect(processExitSpy).toHaveBeenCalledTimes(1); + } finally { + processOnSpy.mockRestore(); + processExitSpy.mockRestore(); + } + }); + it("beforeExit handler runs cleanup without calling exit", async () => { const capturedHandlers = new Map void>(); From b6742d2609069093869d251ab927f7faedab7391 Mon Sep 17 00:00:00 2001 From: ndycode Date: Sun, 15 Mar 2026 05:35:37 +0800 Subject: [PATCH 26/43] Clear cooldowns on menu re-enable --- index.ts | 2 ++ test/index.test.ts | 8 ++++++++ 2 files changed, 10 insertions(+) diff --git a/index.ts b/index.ts index 55101215..41f7e464 100644 --- a/index.ts +++ b/index.ts @@ -3539,6 +3539,8 @@ while (attempted.size < Math.max(1, accountCount)) { if (shouldEnable) { delete target.enabled; delete target.disabledReason; + delete target.coolingDownUntil; + delete target.cooldownReason; } else { target.enabled = false; target.disabledReason = "user"; diff --git a/test/index.test.ts b/test/index.test.ts index 31e21349..03daa041 100644 --- a/test/index.test.ts +++ b/test/index.test.ts @@ -3970,6 +3970,8 @@ describe("OpenAIOAuthPlugin persistAccountPool", () => { accountId: "workspace-managed", email: "managed@example.com", refreshToken: "refresh-managed", + coolingDownUntil: 30_000, + cooldownReason: "network-error", addedAt: 10, lastUsed: 10, }, @@ -3998,17 +4000,23 @@ describe("OpenAIOAuthPlugin persistAccountPool", () => { accountId: "workspace-managed", enabled: false, disabledReason: "user", + coolingDownUntil: 30_000, + cooldownReason: "network-error", }); expect(saveAccountsMock.mock.calls[1]?.[0].accounts[0]).toMatchObject({ accountId: "workspace-managed", }); expect(saveAccountsMock.mock.calls[1]?.[0].accounts[0]).not.toHaveProperty("enabled"); expect(saveAccountsMock.mock.calls[1]?.[0].accounts[0]).not.toHaveProperty("disabledReason"); + expect(saveAccountsMock.mock.calls[1]?.[0].accounts[0]).not.toHaveProperty("coolingDownUntil"); + expect(saveAccountsMock.mock.calls[1]?.[0].accounts[0]).not.toHaveProperty("cooldownReason"); expect(mockStorage.accounts[0]).toMatchObject({ accountId: "workspace-managed", }); expect(mockStorage.accounts[0]).not.toHaveProperty("enabled"); expect(mockStorage.accounts[0]).not.toHaveProperty("disabledReason"); + expect(mockStorage.accounts[0]).not.toHaveProperty("coolingDownUntil"); + expect(mockStorage.accounts[0]).not.toHaveProperty("cooldownReason"); }); it("keeps auth-failure disables blocked in the auth manage menu until a fresh login", async () => { From 067f24c4da8293fb64685b4f15490c2b46154162 Mon Sep 17 00:00:00 2001 From: ndycode Date: Sun, 15 Mar 2026 11:45:09 +0800 Subject: [PATCH 27/43] Harden flush recovery for disabled state --- index.ts | 2 +- lib/accounts.ts | 18 +++++++++- lib/storage.ts | 14 +++++--- test/accounts.test.ts | 77 +++++++++++++++++++++++++++++++++++++++++++ test/index.test.ts | 8 +++-- test/storage.test.ts | 1 + 6 files changed, 111 insertions(+), 9 deletions(-) diff --git a/index.ts b/index.ts index 41f7e464..91678186 100644 --- a/index.ts +++ b/index.ts @@ -2741,7 +2741,7 @@ while (attempted.size < Math.max(1, accountCount)) { totalCount === 0 ? "No Codex accounts configured. Run `opencode auth login`." : enabledCount === 0 - ? "All stored Codex accounts are disabled. Re-enable them from account management or run `opencode auth login` to restore auth-failure disables." + ? "All stored Codex accounts are disabled. Re-enable user-disabled accounts from account management, or run `opencode auth login` to restore auth-failure disables." : waitMs > 0 ? `All ${enabledCount} enabled account(s) are rate-limited. Try again in ${waitLabel} or add another account with \`opencode auth login\`.` : `All ${enabledCount} enabled account(s) failed (server errors or auth issues). Check account health with \`codex-health\`.`; diff --git a/lib/accounts.ts b/lib/accounts.ts index 8b8dbc8e..3c67057d 100644 --- a/lib/accounts.ts +++ b/lib/accounts.ts @@ -1062,12 +1062,28 @@ export class AccountManager { this.saveDebounceTimer = null; } if (this.pendingSave) { - await this.pendingSave; + let pendingSaveError: unknown; + try { + await this.pendingSave; + } catch (error) { + pendingSaveError = error; + } // Let debounced callbacks waiting on the completed save re-arm pendingSave. await Promise.resolve(); if (this.saveDebounceTimer !== null || this.pendingSave !== null) { continue; } + if (pendingSaveError) { + if (!hadDebouncedSave) { + throw pendingSaveError; + } + log.warn("flushPendingSave: ignoring pending-save failure to flush newer state", { + error: + pendingSaveError instanceof Error + ? pendingSaveError.message + : String(pendingSaveError), + }); + } } if (!hadDebouncedSave) { return; diff --git a/lib/storage.ts b/lib/storage.ts index 045cab46..185f11af 100644 --- a/lib/storage.ts +++ b/lib/storage.ts @@ -637,10 +637,16 @@ export function normalizeAccountStorage(data: unknown): AccountStorageV3 | null (account): account is AccountMetadataV3 => isRecord(account) && typeof account.refreshToken === "string" && !!account.refreshToken.trim(), ) - .map((account) => ({ - ...account, - disabledReason: normalizeDisabledReason(account.enabled, account.disabledReason), - })); + .map((account) => { + const normalizedAccount: AccountMetadataV3 = { ...account }; + const disabledReason = normalizeDisabledReason(account.enabled, account.disabledReason); + if (disabledReason === undefined) { + delete normalizedAccount.disabledReason; + } else { + normalizedAccount.disabledReason = disabledReason; + } + return normalizedAccount; + }); const deduplicatedAccounts = deduplicateAccountsForStorage(validAccounts); diff --git a/test/accounts.test.ts b/test/accounts.test.ts index 31c57b37..bcbb1cfc 100644 --- a/test/accounts.test.ts +++ b/test/accounts.test.ts @@ -2388,6 +2388,83 @@ describe("AccountManager", () => { } }); + it("flushes newer debounced state after an older pending save fails", async () => { + vi.useFakeTimers(); + try { + const { saveAccounts } = await import("../lib/storage.js"); + const mockSaveAccounts = vi.mocked(saveAccounts); + mockSaveAccounts.mockClear(); + + const savedSnapshots: Array< + Array<{ + refreshToken?: string; + enabled?: false; + disabledReason?: string; + }> + > = []; + + let rejectFirstSave: (error: Error) => void; + const firstSave = new Promise((_resolve, reject) => { + rejectFirstSave = reject; + }); + mockSaveAccounts.mockImplementationOnce(async (storage) => { + savedSnapshots.push( + storage.accounts.map((account) => ({ + refreshToken: account.refreshToken, + enabled: account.enabled, + disabledReason: account.disabledReason, + })), + ); + await firstSave; + }); + mockSaveAccounts.mockImplementationOnce(async (storage) => { + savedSnapshots.push( + storage.accounts.map((account) => ({ + refreshToken: account.refreshToken, + enabled: account.enabled, + disabledReason: account.disabledReason, + })), + ); + }); + + const now = Date.now(); + const stored = { + version: 3 as const, + activeIndex: 0, + accounts: [ + { refreshToken: "shared-refresh", addedAt: now, lastUsed: now }, + ], + }; + + const manager = new AccountManager(undefined, stored); + + manager.saveToDiskDebounced(0); + await vi.advanceTimersByTimeAsync(0); + expect(mockSaveAccounts).toHaveBeenCalledTimes(1); + + const account = manager.getAccountsSnapshot()[0]!; + manager.disableAccountsWithSameRefreshToken(account); + manager.saveToDiskDebounced(50); + + const flushPromise = manager.flushPendingSave(); + await Promise.resolve(); + + rejectFirstSave!(Object.assign(new Error("EBUSY: file in use"), { code: "EBUSY" })); + await flushPromise; + + expect(mockSaveAccounts).toHaveBeenCalledTimes(2); + expect(savedSnapshots[1]).toEqual([ + expect.objectContaining({ + refreshToken: "shared-refresh", + enabled: false, + disabledReason: "auth-failure", + }), + ]); + } finally { + vi.useRealTimers(); + } + }); + it("warns and rejects if flushPendingSave keeps discovering new saves", async () => { vi.useFakeTimers(); try { diff --git a/test/index.test.ts b/test/index.test.ts index 03daa041..b5bb6216 100644 --- a/test/index.test.ts +++ b/test/index.test.ts @@ -2111,7 +2111,9 @@ describe("OpenAIOAuthPlugin fetch handler", () => { expect(globalThis.fetch).not.toHaveBeenCalled(); expect(response.status).toBe(503); - expect(await response.text()).toContain("All stored Codex accounts are disabled"); + const responseText = await response.text(); + expect(responseText).toContain("All stored Codex accounts are disabled"); + expect(responseText).toContain("Re-enable user-disabled accounts from account management"); }); it("disables grouped accounts when auth failures hit the threshold", async () => { @@ -3493,8 +3495,8 @@ describe("OpenAIOAuthPlugin persistAccountPool", () => { cooldownReason: "auth-failure", }); expect( - revivedEntries.find((account) => account.accountId === "org-legacy"), - ).not.toHaveProperty("disabledReason"); + revivedEntries.find((account) => account.accountId === "org-legacy")?.disabledReason, + ).toBeUndefined(); expect( revivedEntries.find((account) => account.accountId === "org-shared")?.accessToken, ).toBe("access-shared-refresh"); diff --git a/test/storage.test.ts b/test/storage.test.ts index cf96ef4e..cb4b662a 100644 --- a/test/storage.test.ts +++ b/test/storage.test.ts @@ -1096,6 +1096,7 @@ describe("storage", () => { enabled: false, }); expect(result?.accounts[0]?.disabledReason).toBeUndefined(); + expect(result?.accounts[0]).not.toHaveProperty("disabledReason"); }); it("preserves activeIndexByFamily when valid", () => { From 8a2f988afbfce6733d6505c7e35712461146f621 Mon Sep 17 00:00:00 2001 From: ndycode Date: Sun, 15 Mar 2026 13:52:14 +0800 Subject: [PATCH 28/43] Prune idle shutdown manager tracking --- index.ts | 29 +++++++++++++++--- lib/accounts.ts | 7 ++++- lib/shutdown.ts | 32 ++++++++----------- lib/storage.ts | 4 ++- test/accounts.test.ts | 52 +++++++++++++++++++++++++++++++ test/index.test.ts | 71 +++++++++++++++++++++++++++++++++++++++++++ test/shutdown.test.ts | 2 +- test/storage.test.ts | 1 + 8 files changed, 172 insertions(+), 26 deletions(-) diff --git a/index.ts b/index.ts index 91678186..12754264 100644 --- a/index.ts +++ b/index.ts @@ -204,6 +204,7 @@ import { // Shared across plugin instances so shutdown cleanup can flush debounced saves // even when multiple plugin objects coexist during reloads or tests. let accountManagerCleanupHook: (() => Promise) | null = null; +const activeAccountManagersForCleanup = new Set(); const trackedAccountManagersForCleanup = new Set(); // eslint-disable-next-line @typescript-eslint/require-await @@ -1684,25 +1685,44 @@ export const OpenAIOAuthPlugin: Plugin = async ({ client }: PluginInput) => { }; const setCachedAccountManager = (manager: AccountManager): AccountManager => { + const managerHasPendingSave = (candidate: AccountManager): boolean => + typeof candidate.hasPendingSave === "function" ? candidate.hasPendingSave() : true; if (cachedAccountManager && cachedAccountManager !== manager) { - trackedAccountManagersForCleanup.add(cachedAccountManager); + activeAccountManagersForCleanup.delete(cachedAccountManager); + if (managerHasPendingSave(cachedAccountManager)) { + trackedAccountManagersForCleanup.add(cachedAccountManager); + } else { + trackedAccountManagersForCleanup.delete(cachedAccountManager); + } } - trackedAccountManagersForCleanup.add(manager); + activeAccountManagersForCleanup.add(manager); + trackedAccountManagersForCleanup.delete(manager); cachedAccountManager = manager; accountManagerPromise = Promise.resolve(manager); return manager; }; const invalidateAccountManagerCache = (): void => { + const managerHasPendingSave = (candidate: AccountManager): boolean => + typeof candidate.hasPendingSave === "function" ? candidate.hasPendingSave() : true; if (cachedAccountManager) { - trackedAccountManagersForCleanup.add(cachedAccountManager); + activeAccountManagersForCleanup.delete(cachedAccountManager); + if (managerHasPendingSave(cachedAccountManager)) { + trackedAccountManagersForCleanup.add(cachedAccountManager); + } else { + trackedAccountManagersForCleanup.delete(cachedAccountManager); + } } cachedAccountManager = null; accountManagerPromise = null; }; accountManagerCleanupHook ??= async () => { - for (const manager of [...trackedAccountManagersForCleanup]) { + const managersToFlush = new Set([ + ...activeAccountManagersForCleanup, + ...trackedAccountManagersForCleanup, + ]); + for (const manager of managersToFlush) { try { await manager.flushPendingSave(); } catch (error) { @@ -1710,6 +1730,7 @@ export const OpenAIOAuthPlugin: Plugin = async ({ client }: PluginInput) => { error: error instanceof Error ? error.message : String(error), }); } finally { + activeAccountManagersForCleanup.delete(manager); trackedAccountManagersForCleanup.delete(manager); } } diff --git a/lib/accounts.ts b/lib/accounts.ts index 3c67057d..7341efd7 100644 --- a/lib/accounts.ts +++ b/lib/accounts.ts @@ -911,7 +911,8 @@ export class AccountManager { disabledCount++; } - // Clear stale auth failure state for this refresh token once the group is disabled. + // Reset any accumulated auth failures for this token, even if matching accounts + // were already manually disabled, so a later manual re-enable starts clean. this.authFailuresByRefreshToken.delete(refreshToken); return disabledCount; @@ -1027,6 +1028,10 @@ export class AccountManager { return nextSave; } + hasPendingSave(): boolean { + return this.saveDebounceTimer !== null || this.pendingSave !== null; + } + saveToDiskDebounced(delayMs = 500): void { if (this.saveDebounceTimer) { clearTimeout(this.saveDebounceTimer); diff --git a/lib/shutdown.ts b/lib/shutdown.ts index 61773e81..4fd132b3 100644 --- a/lib/shutdown.ts +++ b/lib/shutdown.ts @@ -7,7 +7,6 @@ let sigtermHandler: (() => void) | null = null; let beforeExitHandler: (() => void) | null = null; let cleanupInFlight: Promise | null = null; let signalExitPending = false; -let shouldResetSignalExitPending = true; export function registerCleanup(fn: CleanupFn): void { cleanupFunctions.push(fn); @@ -26,31 +25,27 @@ export function runCleanup(): Promise { return cleanupInFlight; } + const keepHandlersInstalled = signalExitPending; cleanupInFlight = (async () => { - try { - while (cleanupFunctions.length > 0) { - const fns = [...cleanupFunctions]; - cleanupFunctions.length = 0; + while (cleanupFunctions.length > 0) { + const fns = [...cleanupFunctions]; + cleanupFunctions.length = 0; - for (const fn of fns) { - try { - await fn(); - } catch { - // Ignore cleanup errors during shutdown - } + for (const fn of fns) { + try { + await fn(); + } catch { + // Ignore cleanup errors during shutdown } } - } finally { - removeShutdownHandlers(); - shutdownRegistered = false; } })().finally(() => { cleanupInFlight = null; - if (shouldResetSignalExitPending) { - signalExitPending = false; + if (!keepHandlersInstalled) { + removeShutdownHandlers(); + shutdownRegistered = false; } - shouldResetSignalExitPending = true; - if (cleanupFunctions.length > 0) { + if (cleanupFunctions.length > 0 && !shutdownRegistered) { ensureShutdownHandler(); } }); @@ -67,7 +62,6 @@ function ensureShutdownHandler(): void { return; } signalExitPending = true; - shouldResetSignalExitPending = false; void runCleanup().finally(() => { process.exit(0); }); diff --git a/lib/storage.ts b/lib/storage.ts index 185f11af..ab7c23c2 100644 --- a/lib/storage.ts +++ b/lib/storage.ts @@ -1023,7 +1023,6 @@ function normalizeFlaggedStorage(data: unknown): FlaggedAccountStorageV1 { accountNote, email: typeof rawAccount.email === "string" ? rawAccount.email : undefined, enabled: typeof rawAccount.enabled === "boolean" ? rawAccount.enabled : undefined, - disabledReason, lastSwitchReason, rateLimitResetTimes, coolingDownUntil: @@ -1033,6 +1032,9 @@ function normalizeFlaggedStorage(data: unknown): FlaggedAccountStorageV1 { flaggedReason: typeof rawAccount.flaggedReason === "string" ? rawAccount.flaggedReason : undefined, lastError: typeof rawAccount.lastError === "string" ? rawAccount.lastError : undefined, }; + if (disabledReason !== undefined) { + normalized.disabledReason = disabledReason; + } byRefreshToken.set(refreshToken, normalized); } diff --git a/test/accounts.test.ts b/test/accounts.test.ts index bcbb1cfc..6169ff1e 100644 --- a/test/accounts.test.ts +++ b/test/accounts.test.ts @@ -1067,6 +1067,58 @@ describe("AccountManager", () => { expect(manager.incrementAuthFailures(accounts[0])).toBe(1); }); + it("clears auth failure counter even when matching accounts were already user-disabled", () => { + const now = Date.now(); + const stored = { + version: 3 as const, + activeIndex: 0, + accounts: [ + { + refreshToken: "token-1", + accountId: "workspace-a", + enabled: false, + disabledReason: "user" as const, + coolingDownUntil: now + 60_000, + cooldownReason: "network-error" as const, + addedAt: now, + lastUsed: now, + }, + { + refreshToken: "token-1", + accountId: "workspace-b", + enabled: false, + disabledReason: "user" as const, + addedAt: now, + lastUsed: now, + }, + ], + }; + + const manager = new AccountManager(undefined, stored); + const accounts = manager.getAccountsSnapshot(); + + expect(manager.incrementAuthFailures(accounts[0])).toBe(1); + expect(manager.incrementAuthFailures(accounts[1])).toBe(2); + + const disabledCount = manager.disableAccountsWithSameRefreshToken(accounts[0]); + expect(disabledCount).toBe(0); + expect(manager.getAccountsSnapshot()).toMatchObject([ + { + accountId: "workspace-a", + enabled: false, + disabledReason: "user", + coolingDownUntil: now + 60_000, + cooldownReason: "network-error", + }, + { + accountId: "workspace-b", + enabled: false, + disabledReason: "user", + }, + ]); + expect(manager.incrementAuthFailures(accounts[0])).toBe(1); + }); + it("records disable reason when setAccountEnabled disables an account", () => { const now = Date.now(); const stored = { diff --git a/test/index.test.ts b/test/index.test.ts index b5bb6216..be044dc5 100644 --- a/test/index.test.ts +++ b/test/index.test.ts @@ -3846,6 +3846,8 @@ describe("OpenAIOAuthPlugin persistAccountPool", () => { const flushManagerTwo = vi.fn(async () => {}); managerOne.flushPendingSave = flushManagerOne; managerTwo.flushPendingSave = flushManagerTwo; + managerOne.hasPendingSave = vi.fn(() => true); + managerTwo.hasPendingSave = vi.fn(() => false); vi.spyOn(accountsModule.AccountManager, "loadFromDisk") .mockResolvedValueOnce(managerOne) .mockResolvedValue(managerTwo); @@ -3904,6 +3906,75 @@ describe("OpenAIOAuthPlugin persistAccountPool", () => { expect(flushManagerTwo).toHaveBeenCalledTimes(1); }); + it("drops invalidated managers without pending saves from shutdown cleanup tracking", async () => { + const accountsModule = await import("../lib/accounts.js"); + const cliModule = await import("../lib/cli.js"); + const managerOne = await accountsModule.AccountManager.loadFromDisk(); + const managerTwo = await accountsModule.AccountManager.loadFromDisk(); + const flushManagerOne = vi.fn(async () => {}); + const flushManagerTwo = vi.fn(async () => {}); + managerOne.flushPendingSave = flushManagerOne; + managerTwo.flushPendingSave = flushManagerTwo; + managerOne.hasPendingSave = vi.fn(() => false); + managerTwo.hasPendingSave = vi.fn(() => false); + vi.spyOn(accountsModule.AccountManager, "loadFromDisk") + .mockResolvedValueOnce(managerOne) + .mockResolvedValue(managerTwo); + + mockStorage.accounts = [ + { + accountId: "workspace-managed", + email: "managed@example.com", + refreshToken: "refresh-managed", + enabled: true, + addedAt: 10, + lastUsed: 10, + }, + ]; + + vi.mocked(cliModule.promptLoginMode) + .mockResolvedValueOnce({ mode: "manage", toggleAccountIndex: 0 }) + .mockResolvedValueOnce({ mode: "cancel" }); + + const mockClient = createMockClient(); + const { OpenAIOAuthPlugin } = await import("../index.js"); + const plugin = (await OpenAIOAuthPlugin({ + client: mockClient, + } as never)) as unknown as PluginType; + const autoMethod = plugin.auth.methods[0] as unknown as { + authorize: (inputs?: Record) => Promise<{ instructions: string }>; + }; + + await plugin.auth.loader( + async () => ({ + type: "oauth", + access: "access-token-1", + refresh: "refresh-token-1", + expires: Date.now() + 60_000, + }) as never, + {}, + ); + + flushManagerOne.mockClear(); + const authResult = await autoMethod.authorize(); + expect(authResult.instructions).toBe("Authentication cancelled"); + + await plugin.auth.loader( + async () => ({ + type: "oauth", + access: "access-token-2", + refresh: "refresh-token-2", + expires: Date.now() + 60_000, + }) as never, + {}, + ); + + await runCleanup(); + + expect(flushManagerOne).not.toHaveBeenCalled(); + expect(flushManagerTwo).toHaveBeenCalledTimes(1); + }); + it("flushes tracked account managers from multiple plugin instances during shutdown cleanup", async () => { const accountsModule = await import("../lib/accounts.js"); const managerOne = await accountsModule.AccountManager.loadFromDisk(); diff --git a/test/shutdown.test.ts b/test/shutdown.test.ts index 73a7d3b8..d4651f45 100644 --- a/test/shutdown.test.ts +++ b/test/shutdown.test.ts @@ -181,7 +181,7 @@ describe("Graceful shutdown", () => { resolveCleanup(); await vi.waitFor(() => { - expect(processOffSpy).toHaveBeenCalledTimes(3); + expect(processOffSpy).not.toHaveBeenCalled(); expect(processExitSpy).toHaveBeenCalledWith(0); }); } finally { diff --git a/test/storage.test.ts b/test/storage.test.ts index cb4b662a..4c009e4c 100644 --- a/test/storage.test.ts +++ b/test/storage.test.ts @@ -1542,6 +1542,7 @@ describe("storage", () => { expect(loaded.accounts).toHaveLength(1); expect(loaded.accounts[0]?.enabled).toBe(true); expect(loaded.accounts[0]?.disabledReason).toBeUndefined(); + expect(loaded.accounts[0]).not.toHaveProperty("disabledReason"); }); it("retries flagged storage rename on EBUSY and succeeds", async () => { From 8da402963198f6b334462012afc4598e3fde77bb Mon Sep 17 00:00:00 2001 From: ndycode Date: Sun, 15 Mar 2026 15:08:47 +0800 Subject: [PATCH 29/43] Prune settled managers during cleanup tracking --- index.ts | 17 +++++++++++---- lib/schemas.ts | 4 ++-- lib/shutdown.ts | 3 +-- test/index.test.ts | 6 ++++-- test/shutdown.test.ts | 50 +++++++++++++++++++++++++++++++++++++++++++ 5 files changed, 70 insertions(+), 10 deletions(-) diff --git a/index.ts b/index.ts index 12754264..5e70bd33 100644 --- a/index.ts +++ b/index.ts @@ -1684,9 +1684,19 @@ export const OpenAIOAuthPlugin: Plugin = async ({ client }: PluginInput) => { } }; + const managerHasPendingSave = (candidate: AccountManager): boolean => + typeof candidate.hasPendingSave === "function" ? candidate.hasPendingSave() : true; + + const pruneTrackedAccountManagersForCleanup = (): void => { + for (const trackedManager of [...trackedAccountManagersForCleanup]) { + if (!managerHasPendingSave(trackedManager)) { + trackedAccountManagersForCleanup.delete(trackedManager); + } + } + }; + const setCachedAccountManager = (manager: AccountManager): AccountManager => { - const managerHasPendingSave = (candidate: AccountManager): boolean => - typeof candidate.hasPendingSave === "function" ? candidate.hasPendingSave() : true; + pruneTrackedAccountManagersForCleanup(); if (cachedAccountManager && cachedAccountManager !== manager) { activeAccountManagersForCleanup.delete(cachedAccountManager); if (managerHasPendingSave(cachedAccountManager)) { @@ -1703,8 +1713,7 @@ export const OpenAIOAuthPlugin: Plugin = async ({ client }: PluginInput) => { }; const invalidateAccountManagerCache = (): void => { - const managerHasPendingSave = (candidate: AccountManager): boolean => - typeof candidate.hasPendingSave === "function" ? candidate.hasPendingSave() : true; + pruneTrackedAccountManagersForCleanup(); if (cachedAccountManager) { activeAccountManagersForCleanup.delete(cachedAccountManager); if (managerHasPendingSave(cachedAccountManager)) { diff --git a/lib/schemas.ts b/lib/schemas.ts index 26e233da..083d586d 100644 --- a/lib/schemas.ts +++ b/lib/schemas.ts @@ -129,7 +129,7 @@ export const AccountMetadataV3Schema = z.object({ accessToken: z.string().optional(), expiresAt: z.number().optional(), enabled: z.boolean().optional(), - disabledReason: DisabledReasonSchema.optional(), + disabledReason: DisabledReasonSchema, addedAt: z.number(), lastUsed: z.number(), lastSwitchReason: SwitchReasonSchema.optional(), @@ -177,7 +177,7 @@ export const AccountMetadataV1Schema = z.object({ accessToken: z.string().optional(), expiresAt: z.number().optional(), enabled: z.boolean().optional(), - disabledReason: DisabledReasonSchema.optional(), + disabledReason: DisabledReasonSchema, addedAt: z.number(), lastUsed: z.number(), lastSwitchReason: SwitchReasonSchema.optional(), diff --git a/lib/shutdown.ts b/lib/shutdown.ts index 4fd132b3..5d362d6f 100644 --- a/lib/shutdown.ts +++ b/lib/shutdown.ts @@ -25,7 +25,6 @@ export function runCleanup(): Promise { return cleanupInFlight; } - const keepHandlersInstalled = signalExitPending; cleanupInFlight = (async () => { while (cleanupFunctions.length > 0) { const fns = [...cleanupFunctions]; @@ -41,7 +40,7 @@ export function runCleanup(): Promise { } })().finally(() => { cleanupInFlight = null; - if (!keepHandlersInstalled) { + if (!signalExitPending) { removeShutdownHandlers(); shutdownRegistered = false; } diff --git a/test/index.test.ts b/test/index.test.ts index be044dc5..ae0c31d9 100644 --- a/test/index.test.ts +++ b/test/index.test.ts @@ -3906,16 +3906,17 @@ describe("OpenAIOAuthPlugin persistAccountPool", () => { expect(flushManagerTwo).toHaveBeenCalledTimes(1); }); - it("drops invalidated managers without pending saves from shutdown cleanup tracking", async () => { + it("prunes tracked invalidated managers once their pending saves settle", async () => { const accountsModule = await import("../lib/accounts.js"); const cliModule = await import("../lib/cli.js"); const managerOne = await accountsModule.AccountManager.loadFromDisk(); const managerTwo = await accountsModule.AccountManager.loadFromDisk(); const flushManagerOne = vi.fn(async () => {}); const flushManagerTwo = vi.fn(async () => {}); + let managerOneHasPendingSave = true; managerOne.flushPendingSave = flushManagerOne; managerTwo.flushPendingSave = flushManagerTwo; - managerOne.hasPendingSave = vi.fn(() => false); + managerOne.hasPendingSave = vi.fn(() => managerOneHasPendingSave); managerTwo.hasPendingSave = vi.fn(() => false); vi.spyOn(accountsModule.AccountManager, "loadFromDisk") .mockResolvedValueOnce(managerOne) @@ -3959,6 +3960,7 @@ describe("OpenAIOAuthPlugin persistAccountPool", () => { const authResult = await autoMethod.authorize(); expect(authResult.instructions).toBe("Authentication cancelled"); + managerOneHasPendingSave = false; await plugin.auth.loader( async () => ({ type: "oauth", diff --git a/test/shutdown.test.ts b/test/shutdown.test.ts index d4651f45..49618d49 100644 --- a/test/shutdown.test.ts +++ b/test/shutdown.test.ts @@ -303,6 +303,56 @@ describe("Graceful shutdown", () => { } }); + it("keeps handlers installed when a signal arrives during beforeExit cleanup", async () => { + const capturedHandlers = new Map void>(); + + const processOnSpy = vi.spyOn(process, "on").mockImplementation((event: string | symbol, handler: (...args: unknown[]) => void) => { + capturedHandlers.set(String(event), handler); + return process; + }); + const processOffSpy = vi.spyOn(process, "off").mockImplementation(() => process); + const processExitSpy = vi.spyOn(process, "exit").mockImplementation(() => undefined as never); + + vi.resetModules(); + const { registerCleanup: freshRegister, runCleanup: freshRunCleanup } = await import("../lib/shutdown.js"); + await freshRunCleanup(); + + let resolveCleanup!: () => void; + const cleanupPromise = new Promise((resolve) => { + resolveCleanup = resolve; + }); + const cleanupFn = vi.fn(async () => { + await cleanupPromise; + }); + freshRegister(cleanupFn); + + try { + const beforeExitHandler = capturedHandlers.get("beforeExit"); + const sigtermHandler = capturedHandlers.get("SIGTERM"); + expect(beforeExitHandler).toBeDefined(); + expect(sigtermHandler).toBeDefined(); + + beforeExitHandler!(); + await Promise.resolve(); + sigtermHandler!(); + await Promise.resolve(); + + expect(cleanupFn).toHaveBeenCalledTimes(1); + expect(processExitSpy).not.toHaveBeenCalled(); + expect(processOffSpy).not.toHaveBeenCalled(); + + resolveCleanup(); + await vi.waitFor(() => { + expect(processExitSpy).toHaveBeenCalledWith(0); + }); + expect(processOffSpy).not.toHaveBeenCalled(); + } finally { + processOnSpy.mockRestore(); + processOffSpy.mockRestore(); + processExitSpy.mockRestore(); + } + }); + it("signal handlers are only registered once", async () => { const processOnSpy = vi.spyOn(process, "on").mockImplementation(() => process); From 766683726391e86e17a8929bc26403a017dcf21c Mon Sep 17 00:00:00 2001 From: ndycode Date: Sun, 15 Mar 2026 15:34:10 +0800 Subject: [PATCH 30/43] Fix storage canonicalization and account toggle API --- lib/accounts.ts | 11 +++++++++- lib/schemas.ts | 6 ++---- lib/storage.ts | 42 +++++++++++++++++++-------------------- lib/storage/migrations.ts | 30 +++++++++++++++++++++++----- test/accounts.test.ts | 38 ++++++++++++++++++++++++++++++----- test/storage.test.ts | 40 +++++++++++++++++++++++++++++++++++++ 6 files changed, 131 insertions(+), 36 deletions(-) diff --git a/lib/accounts.ts b/lib/accounts.ts index 7341efd7..5e042257 100644 --- a/lib/accounts.ts +++ b/lib/accounts.ts @@ -941,7 +941,7 @@ export class AccountManager { return removedCount; } - setAccountEnabled( + trySetAccountEnabled( index: number, enabled: boolean, reason?: AccountDisabledReason, @@ -965,6 +965,15 @@ export class AccountManager { return { ok: true, account }; } + setAccountEnabled( + index: number, + enabled: boolean, + reason?: AccountDisabledReason, + ): ManagedAccount | null { + const result = this.trySetAccountEnabled(index, enabled, reason); + return result.ok ? result.account : null; + } + async saveToDisk(): Promise { const activeIndexByFamily: Partial> = {}; for (const family of MODEL_FAMILIES) { diff --git a/lib/schemas.ts b/lib/schemas.ts index 083d586d..a89fa7a3 100644 --- a/lib/schemas.ts +++ b/lib/schemas.ts @@ -5,6 +5,7 @@ */ import { z } from "zod"; import { MODEL_FAMILIES, type ModelFamily } from "./prompts/codex.js"; +import { normalizeAccountDisabledReason } from "./storage/migrations.js"; // ============================================================================ // Plugin Configuration Schema @@ -74,13 +75,10 @@ export const CooldownReasonSchema = z.enum(["auth-failure", "network-error"]); export type CooldownReasonFromSchema = z.infer; -const normalizeDisabledReason = (value: unknown): unknown => - value === "user" || value === "auth-failure" ? value : undefined; - // Storage normalization strips unknown disabled reasons later; keep schema parsing // lenient so legacy/downgraded files don't warn or fail before that step runs. export const DisabledReasonSchema = z.preprocess( - normalizeDisabledReason, + normalizeAccountDisabledReason, z.enum(["user", "auth-failure"]).optional(), ); diff --git a/lib/storage.ts b/lib/storage.ts index ab7c23c2..e3ac36ee 100644 --- a/lib/storage.ts +++ b/lib/storage.ts @@ -7,14 +7,16 @@ import { MODEL_FAMILIES, type ModelFamily } from "./prompts/codex.js"; import { AnyAccountStorageSchema, getValidationErrors } from "./schemas.js"; import { getConfigDir, getProjectConfigDir, getProjectGlobalConfigDir, findProjectRoot, resolvePath } from "./storage/paths.js"; import { - migrateV1ToV3, - type AccountDisabledReason, - type CooldownReason, - type RateLimitStateV3, - type AccountMetadataV1, - type AccountStorageV1, - type AccountMetadataV3, - type AccountStorageV3, + migrateV1ToV3, + normalizeStoredAccountDisabledReason, + normalizeStoredEnabled, + type AccountDisabledReason, + type CooldownReason, + type RateLimitStateV3, + type AccountMetadataV1, + type AccountStorageV1, + type AccountMetadataV3, + type AccountStorageV3, } from "./storage/migrations.js"; export type { @@ -134,17 +136,6 @@ const WINDOWS_RENAME_RETRY_ATTEMPTS = 5; const WINDOWS_RENAME_RETRY_BASE_DELAY_MS = 10; const PRE_IMPORT_BACKUP_WRITE_TIMEOUT_MS = 3_000; -function isDisabledReason(value: unknown): value is AccountDisabledReason { - return value === "user" || value === "auth-failure"; -} - -function normalizeDisabledReason( - enabled: unknown, - disabledReason: unknown, -): AccountDisabledReason | undefined { - return enabled === false && isDisabledReason(disabledReason) ? disabledReason : undefined; -} - function isWindowsLockError(error: unknown): error is NodeJS.ErrnoException { const code = (error as NodeJS.ErrnoException)?.code; return code === "EPERM" || code === "EBUSY"; @@ -639,7 +630,13 @@ export function normalizeAccountStorage(data: unknown): AccountStorageV3 | null ) .map((account) => { const normalizedAccount: AccountMetadataV3 = { ...account }; - const disabledReason = normalizeDisabledReason(account.enabled, account.disabledReason); + const enabled = normalizeStoredEnabled(account.enabled); + const disabledReason = normalizeStoredAccountDisabledReason(account.enabled, account.disabledReason); + if (enabled === undefined) { + delete normalizedAccount.enabled; + } else { + normalizedAccount.enabled = enabled; + } if (disabledReason === undefined) { delete normalizedAccount.disabledReason; } else { @@ -1003,7 +1000,10 @@ function normalizeFlaggedStorage(data: unknown): FlaggedAccountStorageV1 { const cooldownReason = isCooldownReason(rawAccount.cooldownReason) ? rawAccount.cooldownReason : undefined; - const disabledReason = normalizeDisabledReason(rawAccount.enabled, rawAccount.disabledReason); + const disabledReason = normalizeStoredAccountDisabledReason( + rawAccount.enabled, + rawAccount.disabledReason, + ); const accountTags = normalizeTags(rawAccount.accountTags); const accountNote = typeof rawAccount.accountNote === "string" && rawAccount.accountNote.trim() diff --git a/lib/storage/migrations.ts b/lib/storage/migrations.ts index 6469b990..28af2eb2 100644 --- a/lib/storage/migrations.ts +++ b/lib/storage/migrations.ts @@ -9,6 +9,29 @@ import type { AccountIdSource } from "../types.js"; export type CooldownReason = "auth-failure" | "network-error"; export type AccountDisabledReason = "user" | "auth-failure"; +export function isAccountDisabledReason(value: unknown): value is AccountDisabledReason { + return value === "user" || value === "auth-failure"; +} + +export function normalizeAccountDisabledReason( + value: unknown, +): AccountDisabledReason | undefined { + return isAccountDisabledReason(value) ? value : undefined; +} + +export function normalizeStoredAccountDisabledReason( + enabled: unknown, + disabledReason: unknown, +): AccountDisabledReason | undefined { + return enabled === false + ? normalizeAccountDisabledReason(disabledReason) + : undefined; +} + +export function normalizeStoredEnabled(enabled: unknown): false | undefined { + return enabled === false ? false : undefined; +} + export interface RateLimitStateV3 { [key: string]: number | undefined; } @@ -99,11 +122,8 @@ export function migrateV1ToV3(v1: AccountStorageV1): AccountStorageV3 { refreshToken: account.refreshToken, accessToken: account.accessToken, expiresAt: account.expiresAt, - enabled: account.enabled, - disabledReason: - account.enabled === false - ? account.disabledReason ?? "user" - : undefined, + enabled: normalizeStoredEnabled(account.enabled), + disabledReason: account.enabled === false ? account.disabledReason ?? "user" : undefined, addedAt: account.addedAt, lastUsed: account.lastUsed, lastSwitchReason: account.lastSwitchReason, diff --git a/test/accounts.test.ts b/test/accounts.test.ts index 6169ff1e..5eaf6548 100644 --- a/test/accounts.test.ts +++ b/test/accounts.test.ts @@ -1137,7 +1137,7 @@ describe("AccountManager", () => { const manager = new AccountManager(undefined, stored); - const disabled = manager.setAccountEnabled(0, false, "user"); + const disabled = manager.trySetAccountEnabled(0, false, "user"); expect(disabled).toMatchObject({ ok: true, account: { @@ -1146,7 +1146,7 @@ describe("AccountManager", () => { }, }); - const reenabled = manager.setAccountEnabled(0, true); + const reenabled = manager.trySetAccountEnabled(0, true); expect(reenabled).toMatchObject({ ok: true, account: { @@ -1173,7 +1173,7 @@ describe("AccountManager", () => { const manager = new AccountManager(undefined, stored); - const disabled = manager.setAccountEnabled(0, false); + const disabled = manager.trySetAccountEnabled(0, false); expect(disabled).toMatchObject({ ok: true, account: { @@ -1201,7 +1201,7 @@ describe("AccountManager", () => { const manager = new AccountManager(undefined, stored); - const disabled = manager.setAccountEnabled(0, false); + const disabled = manager.trySetAccountEnabled(0, false); expect(disabled).toMatchObject({ ok: true, account: { @@ -1229,7 +1229,7 @@ describe("AccountManager", () => { const manager = new AccountManager(undefined, stored); - const reenabled = manager.setAccountEnabled(0, true); + const reenabled = manager.trySetAccountEnabled(0, true); expect(reenabled).toEqual({ ok: false, reason: "auth-failure-blocked", @@ -1239,6 +1239,34 @@ describe("AccountManager", () => { disabledReason: "auth-failure", }); }); + + it("preserves the legacy setAccountEnabled account-or-null contract", () => { + const now = Date.now(); + const stored = { + version: 3 as const, + activeIndex: 0, + accounts: [ + { + refreshToken: "token-1", + enabled: false, + disabledReason: "auth-failure" as const, + addedAt: now, + lastUsed: now, + }, + ], + }; + + const manager = new AccountManager(undefined, stored); + + expect(manager.setAccountEnabled(0, true)).toBeNull(); + expect(manager.setAccountEnabled(99, false)).toBeNull(); + + const userDisabled = manager.setAccountEnabled(0, false, "user"); + expect(userDisabled).toMatchObject({ + enabled: false, + disabledReason: "user", + }); + }); }); describe("getMinWaitTimeForFamily", () => { diff --git a/test/storage.test.ts b/test/storage.test.ts index 4c009e4c..d53efc6a 100644 --- a/test/storage.test.ts +++ b/test/storage.test.ts @@ -1074,6 +1074,26 @@ describe("storage", () => { }); }); + it("omits enabled for enabled accounts during v1 migration", () => { + const data = { + version: 1, + activeIndex: 0, + accounts: [ + { + refreshToken: "t1", + accountId: "A", + enabled: true, + addedAt: 1, + lastUsed: 1, + }, + ], + }; + + const result = normalizeAccountStorage(data); + expect(result?.accounts[0]?.enabled).toBeUndefined(); + expect(result?.accounts[0]).not.toHaveProperty("enabled"); + }); + it("strips invalid disabledReason values from disabled v3 accounts", () => { const data = { version: 3, @@ -1099,6 +1119,26 @@ describe("storage", () => { expect(result?.accounts[0]).not.toHaveProperty("disabledReason"); }); + it("strips enabled: true from v3 accounts during normalization", () => { + const data = { + version: 3, + activeIndex: 0, + accounts: [ + { + refreshToken: "t1", + accountId: "A", + enabled: true, + addedAt: 1, + lastUsed: 1, + }, + ], + }; + + const result = normalizeAccountStorage(data); + expect(result?.accounts[0]?.enabled).toBeUndefined(); + expect(result?.accounts[0]).not.toHaveProperty("enabled"); + }); + it("preserves activeIndexByFamily when valid", () => { const data = { version: 3, From e8c022705b45fb12d28dae2aa1e542b172e785fb Mon Sep 17 00:00:00 2001 From: ndycode Date: Sun, 15 Mar 2026 15:52:39 +0800 Subject: [PATCH 31/43] Preserve auth-failure disable invariants --- lib/accounts.ts | 8 ++++++++ lib/storage/migrations.ts | 5 ++++- test/accounts.test.ts | 38 +++++++++++++++++++++++++++++++++++++- test/storage.test.ts | 25 +++++++++++++++++++++++++ 4 files changed, 74 insertions(+), 2 deletions(-) diff --git a/lib/accounts.ts b/lib/accounts.ts index 5e042257..f7ce8775 100644 --- a/lib/accounts.ts +++ b/lib/accounts.ts @@ -953,6 +953,12 @@ export class AccountManager { if (enabled && account.disabledReason === "auth-failure") { return { ok: false, reason: "auth-failure-blocked" }; } + // Once an account is auth-failure disabled, callers must refresh credentials + // instead of downgrading the reason to a manually re-enableable state. + if (!enabled && account.disabledReason === "auth-failure") { + account.enabled = false; + return { ok: true, account }; + } account.enabled = enabled; if (enabled) { delete account.disabledReason; @@ -1065,6 +1071,8 @@ export class AccountManager { while (true) { flushIterations += 1; if (flushIterations > MAX_FLUSH_ITERATIONS) { + // This is intentionally far above realistic debounce re-arm chains; if we + // still haven't converged, shutdown callers log the failure and continue exit. log.warn("flushPendingSave exceeded max iterations; possible save loop", { iterations: flushIterations - 1, }); diff --git a/lib/storage/migrations.ts b/lib/storage/migrations.ts index 28af2eb2..37154007 100644 --- a/lib/storage/migrations.ts +++ b/lib/storage/migrations.ts @@ -123,7 +123,10 @@ export function migrateV1ToV3(v1: AccountStorageV1): AccountStorageV3 { accessToken: account.accessToken, expiresAt: account.expiresAt, enabled: normalizeStoredEnabled(account.enabled), - disabledReason: account.enabled === false ? account.disabledReason ?? "user" : undefined, + disabledReason: + account.enabled === false + ? normalizeAccountDisabledReason(account.disabledReason) ?? "user" + : undefined, addedAt: account.addedAt, lastUsed: account.lastUsed, lastSwitchReason: account.lastSwitchReason, diff --git a/test/accounts.test.ts b/test/accounts.test.ts index 5eaf6548..2a8232f4 100644 --- a/test/accounts.test.ts +++ b/test/accounts.test.ts @@ -1240,7 +1240,7 @@ describe("AccountManager", () => { }); }); - it("preserves the legacy setAccountEnabled account-or-null contract", () => { + it("ignores explicit disable-reason overrides for auth-failure disabled accounts", () => { const now = Date.now(); const stored = { version: 3 as const, @@ -1258,7 +1258,38 @@ describe("AccountManager", () => { const manager = new AccountManager(undefined, stored); + const disabled = manager.trySetAccountEnabled(0, false, "user"); + expect(disabled).toMatchObject({ + ok: true, + account: { + enabled: false, + disabledReason: "auth-failure", + }, + }); + + expect(manager.trySetAccountEnabled(0, true)).toEqual({ + ok: false, + reason: "auth-failure-blocked", + }); expect(manager.setAccountEnabled(0, true)).toBeNull(); + }); + + it("preserves the legacy setAccountEnabled account-or-null contract", () => { + const now = Date.now(); + const stored = { + version: 3 as const, + activeIndex: 0, + accounts: [ + { + refreshToken: "token-1", + addedAt: now, + lastUsed: now, + }, + ], + }; + + const manager = new AccountManager(undefined, stored); + expect(manager.setAccountEnabled(99, false)).toBeNull(); const userDisabled = manager.setAccountEnabled(0, false, "user"); @@ -1266,6 +1297,11 @@ describe("AccountManager", () => { enabled: false, disabledReason: "user", }); + + const reenabled = manager.setAccountEnabled(0, true); + expect(reenabled).toMatchObject({ + enabled: true, + }); }); }); diff --git a/test/storage.test.ts b/test/storage.test.ts index d53efc6a..5450269d 100644 --- a/test/storage.test.ts +++ b/test/storage.test.ts @@ -1074,6 +1074,31 @@ describe("storage", () => { }); }); + it("defaults invalid migrated legacy disabled reasons to user", () => { + const data = { + version: 1, + activeIndex: 0, + accounts: [ + { + refreshToken: "t1", + accountId: "A", + enabled: false, + disabledReason: "future-reason", + addedAt: 1, + lastUsed: 1, + }, + ], + }; + + const result = normalizeAccountStorage(data); + expect(result?.version).toBe(3); + expect(result?.accounts[0]).toMatchObject({ + refreshToken: "t1", + enabled: false, + disabledReason: "user", + }); + }); + it("omits enabled for enabled accounts during v1 migration", () => { const data = { version: 1, From b51d093c7d968d2ccbac3b08d89abb2c3c9d3596 Mon Sep 17 00:00:00 2001 From: ndycode Date: Sun, 15 Mar 2026 16:02:39 +0800 Subject: [PATCH 32/43] Prune tracked managers after saves settle --- index.ts | 24 ++++++++++++++++++++++++ lib/accounts.ts | 23 +++++++++++++++++++++++ test/index.test.ts | 27 +++++++++++---------------- 3 files changed, 58 insertions(+), 16 deletions(-) diff --git a/index.ts b/index.ts index 5e70bd33..0f67b42b 100644 --- a/index.ts +++ b/index.ts @@ -1686,6 +1686,7 @@ export const OpenAIOAuthPlugin: Plugin = async ({ client }: PluginInput) => { const managerHasPendingSave = (candidate: AccountManager): boolean => typeof candidate.hasPendingSave === "function" ? candidate.hasPendingSave() : true; + const trackedManagerSettledWaits = new WeakMap>(); const pruneTrackedAccountManagersForCleanup = (): void => { for (const trackedManager of [...trackedAccountManagersForCleanup]) { @@ -1695,12 +1696,34 @@ export const OpenAIOAuthPlugin: Plugin = async ({ client }: PluginInput) => { } }; + const scheduleTrackedManagerPrune = (manager: AccountManager): void => { + if ( + !managerHasPendingSave(manager) || + trackedManagerSettledWaits.has(manager) || + typeof manager.waitForPendingSaveToSettle !== "function" + ) { + return; + } + + const waitForSettle = manager + .waitForPendingSaveToSettle() + .finally(() => { + trackedManagerSettledWaits.delete(manager); + if (!managerHasPendingSave(manager) && !activeAccountManagersForCleanup.has(manager)) { + trackedAccountManagersForCleanup.delete(manager); + } + }); + + trackedManagerSettledWaits.set(manager, waitForSettle); + }; + const setCachedAccountManager = (manager: AccountManager): AccountManager => { pruneTrackedAccountManagersForCleanup(); if (cachedAccountManager && cachedAccountManager !== manager) { activeAccountManagersForCleanup.delete(cachedAccountManager); if (managerHasPendingSave(cachedAccountManager)) { trackedAccountManagersForCleanup.add(cachedAccountManager); + scheduleTrackedManagerPrune(cachedAccountManager); } else { trackedAccountManagersForCleanup.delete(cachedAccountManager); } @@ -1718,6 +1741,7 @@ export const OpenAIOAuthPlugin: Plugin = async ({ client }: PluginInput) => { activeAccountManagersForCleanup.delete(cachedAccountManager); if (managerHasPendingSave(cachedAccountManager)) { trackedAccountManagersForCleanup.add(cachedAccountManager); + scheduleTrackedManagerPrune(cachedAccountManager); } else { trackedAccountManagersForCleanup.delete(cachedAccountManager); } diff --git a/lib/accounts.ts b/lib/accounts.ts index f7ce8775..eb8f4418 100644 --- a/lib/accounts.ts +++ b/lib/accounts.ts @@ -223,6 +223,7 @@ export class AccountManager { private saveDebounceTimer: ReturnType | null = null; private pendingSave: Promise | null = null; private authFailuresByRefreshToken: Map = new Map(); + private pendingSaveSettledWaiters = new Set<() => void>(); static async loadFromDisk(authFallback?: OAuthAuthDetails): Promise { const stored = await loadAccounts(); @@ -1038,6 +1039,7 @@ export class AccountManager { if (this.pendingSave === nextSave) { this.pendingSave = null; } + this.resolvePendingSaveSettledWaiters(); }); this.pendingSave = nextSave; return nextSave; @@ -1047,6 +1049,27 @@ export class AccountManager { return this.saveDebounceTimer !== null || this.pendingSave !== null; } + waitForPendingSaveToSettle(): Promise { + if (!this.hasPendingSave()) { + return Promise.resolve(); + } + + return new Promise((resolve) => { + this.pendingSaveSettledWaiters.add(resolve); + }); + } + + private resolvePendingSaveSettledWaiters(): void { + if (this.hasPendingSave() || this.pendingSaveSettledWaiters.size === 0) { + return; + } + + for (const resolve of [...this.pendingSaveSettledWaiters]) { + this.pendingSaveSettledWaiters.delete(resolve); + resolve(); + } + } + saveToDiskDebounced(delayMs = 500): void { if (this.saveDebounceTimer) { clearTimeout(this.saveDebounceTimer); diff --git a/test/index.test.ts b/test/index.test.ts index ae0c31d9..6b2f8472 100644 --- a/test/index.test.ts +++ b/test/index.test.ts @@ -3910,17 +3910,19 @@ describe("OpenAIOAuthPlugin persistAccountPool", () => { const accountsModule = await import("../lib/accounts.js"); const cliModule = await import("../lib/cli.js"); const managerOne = await accountsModule.AccountManager.loadFromDisk(); - const managerTwo = await accountsModule.AccountManager.loadFromDisk(); const flushManagerOne = vi.fn(async () => {}); - const flushManagerTwo = vi.fn(async () => {}); let managerOneHasPendingSave = true; + let resolveManagerOneSettled: (() => void) | null = null; managerOne.flushPendingSave = flushManagerOne; - managerTwo.flushPendingSave = flushManagerTwo; managerOne.hasPendingSave = vi.fn(() => managerOneHasPendingSave); - managerTwo.hasPendingSave = vi.fn(() => false); + managerOne.waitForPendingSaveToSettle = vi.fn( + () => + new Promise((resolve) => { + resolveManagerOneSettled = resolve; + }), + ); vi.spyOn(accountsModule.AccountManager, "loadFromDisk") - .mockResolvedValueOnce(managerOne) - .mockResolvedValue(managerTwo); + .mockResolvedValue(managerOne); mockStorage.accounts = [ { @@ -3961,20 +3963,13 @@ describe("OpenAIOAuthPlugin persistAccountPool", () => { expect(authResult.instructions).toBe("Authentication cancelled"); managerOneHasPendingSave = false; - await plugin.auth.loader( - async () => ({ - type: "oauth", - access: "access-token-2", - refresh: "refresh-token-2", - expires: Date.now() + 60_000, - }) as never, - {}, - ); + resolveManagerOneSettled?.(); + await Promise.resolve(); + await Promise.resolve(); await runCleanup(); expect(flushManagerOne).not.toHaveBeenCalled(); - expect(flushManagerTwo).toHaveBeenCalledTimes(1); }); it("flushes tracked account managers from multiple plugin instances during shutdown cleanup", async () => { From b27305f46e149f57744ce32a610d287d876a250b Mon Sep 17 00:00:00 2001 From: ndycode Date: Sun, 15 Mar 2026 16:08:19 +0800 Subject: [PATCH 33/43] Harden shutdown cleanup and reauth messaging --- index.ts | 8 +++++++- lib/shutdown.ts | 19 ++++++++++++++++++- test/index.test.ts | 13 +++++++++++++ test/shutdown.test.ts | 42 ++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 80 insertions(+), 2 deletions(-) diff --git a/index.ts b/index.ts index 0f67b42b..7739991a 100644 --- a/index.ts +++ b/index.ts @@ -3581,12 +3581,18 @@ while (attempted.size < Math.max(1, accountCount)) { if (target) { const shouldEnable = target.enabled === false; if (shouldEnable && target.disabledReason === "auth-failure") { + const message = + "This account was disabled after repeated auth failures. Run 'opencode auth login' to re-enable with fresh credentials."; logWarn("[account-menu] blocked re-enable for auth-failure disabled account", { accountId: target.accountId ?? null, email: target.email ?? null, }); + logInfo("[account-menu] prompted re-auth for auth-failure disabled account", { + accountId: target.accountId ?? null, + }); + await showToast(message, "warning"); console.log( - "\nThis account was disabled after repeated auth failures. Run 'opencode auth login' to re-enable with fresh credentials.\n", + `\n${message}\n`, ); continue; } diff --git a/lib/shutdown.ts b/lib/shutdown.ts index 5d362d6f..8558612e 100644 --- a/lib/shutdown.ts +++ b/lib/shutdown.ts @@ -1,5 +1,7 @@ type CleanupFn = () => void | Promise; +const SIGNAL_CLEANUP_TIMEOUT_MS = 5_000; + const cleanupFunctions: CleanupFn[] = []; let shutdownRegistered = false; let sigintHandler: (() => void) | null = null; @@ -61,8 +63,23 @@ function ensureShutdownHandler(): void { return; } signalExitPending = true; - void runCleanup().finally(() => { + let exitRequested = false; + let timeoutHandle: ReturnType | null = setTimeout(() => { + requestExit(); + }, SIGNAL_CLEANUP_TIMEOUT_MS); + const requestExit = () => { + if (exitRequested) { + return; + } + exitRequested = true; + if (timeoutHandle) { + clearTimeout(timeoutHandle); + timeoutHandle = null; + } process.exit(0); + }; + void runCleanup().finally(() => { + requestExit(); }); }; sigintHandler = handleSignal; diff --git a/test/index.test.ts b/test/index.test.ts index 6b2f8472..e2fbfbfe 100644 --- a/test/index.test.ts +++ b/test/index.test.ts @@ -4138,6 +4138,19 @@ describe("OpenAIOAuthPlugin persistAccountPool", () => { expect(consoleLog).toHaveBeenCalledWith( expect.stringContaining("Run 'opencode auth login' to re-enable with fresh credentials."), ); + expect(mockClient.tui.showToast).toHaveBeenCalledWith({ + body: expect.objectContaining({ + message: + "This account was disabled after repeated auth failures. Run 'opencode auth login' to re-enable with fresh credentials.", + variant: "warning", + }), + }); + expect(vi.mocked(loggerModule.logInfo)).toHaveBeenCalledWith( + "[account-menu] prompted re-auth for auth-failure disabled account", + expect.objectContaining({ + accountId: "workspace-auth-failure", + }), + ); expect(vi.mocked(loggerModule.logWarn)).toHaveBeenCalledWith( "[account-menu] blocked re-enable for auth-failure disabled account", expect.objectContaining({ diff --git a/test/shutdown.test.ts b/test/shutdown.test.ts index 49618d49..464620d6 100644 --- a/test/shutdown.test.ts +++ b/test/shutdown.test.ts @@ -235,6 +235,48 @@ describe("Graceful shutdown", () => { } }); + it("forces exit if signal cleanup stalls past the timeout", async () => { + vi.useFakeTimers(); + const capturedHandlers = new Map void>(); + + const processOnSpy = vi.spyOn(process, "on").mockImplementation((event: string | symbol, handler: (...args: unknown[]) => void) => { + capturedHandlers.set(String(event), handler); + return process; + }); + const processExitSpy = vi.spyOn(process, "exit").mockImplementation(() => undefined as never); + + vi.resetModules(); + const { registerCleanup: freshRegister, runCleanup: freshRunCleanup } = await import("../lib/shutdown.js"); + await freshRunCleanup(); + + const cleanupFn = vi.fn(async () => { + await new Promise(() => { + // Intentionally never resolves so the timeout path can force exit. + }); + }); + freshRegister(cleanupFn); + + try { + const sigtermHandler = capturedHandlers.get("SIGTERM"); + expect(sigtermHandler).toBeDefined(); + + sigtermHandler!(); + await Promise.resolve(); + + expect(cleanupFn).toHaveBeenCalledTimes(1); + expect(processExitSpy).not.toHaveBeenCalled(); + + await vi.advanceTimersByTimeAsync(5_000); + + expect(processExitSpy).toHaveBeenCalledTimes(1); + expect(processExitSpy).toHaveBeenCalledWith(0); + } finally { + vi.useRealTimers(); + processOnSpy.mockRestore(); + processExitSpy.mockRestore(); + } + }); + it("keeps later signals ignored after cleanup completes until process exit", async () => { const capturedHandlers = new Map void>(); From c5add838b8973f9f57c41759b548074a65fb09fb Mon Sep 17 00:00:00 2001 From: ndycode Date: Sun, 15 Mar 2026 16:34:52 +0800 Subject: [PATCH 34/43] Address remaining review follow-ups --- index.ts | 1 - lib/accounts.ts | 10 ++++- lib/storage/migrations.ts | 2 + test/index.test.ts | 81 ++++++++++++++++++++++++++++++++++++++- 4 files changed, 90 insertions(+), 4 deletions(-) diff --git a/index.ts b/index.ts index 7739991a..b9fb1208 100644 --- a/index.ts +++ b/index.ts @@ -3585,7 +3585,6 @@ while (attempted.size < Math.max(1, accountCount)) { "This account was disabled after repeated auth failures. Run 'opencode auth login' to re-enable with fresh credentials."; logWarn("[account-menu] blocked re-enable for auth-failure disabled account", { accountId: target.accountId ?? null, - email: target.email ?? null, }); logInfo("[account-menu] prompted re-auth for auth-failure disabled account", { accountId: target.accountId ?? null, diff --git a/lib/accounts.ts b/lib/accounts.ts index eb8f4418..5fce82d8 100644 --- a/lib/accounts.ts +++ b/lib/accounts.ts @@ -66,6 +66,12 @@ import { const log = createLogger("accounts"); +async function drainSaveMicrotasks(): Promise { + // enqueueSave settles through an async IIFE and a trailing .finally(). + await Promise.resolve(); + await Promise.resolve(); +} + export type CodexCliTokenCacheEntry = { accessToken: string; expiresAt?: number; @@ -1114,7 +1120,7 @@ export class AccountManager { pendingSaveError = error; } // Let debounced callbacks waiting on the completed save re-arm pendingSave. - await Promise.resolve(); + await drainSaveMicrotasks(); if (this.saveDebounceTimer !== null || this.pendingSave !== null) { continue; } @@ -1139,7 +1145,7 @@ export class AccountManager { const flushSave = this.enqueueSave(() => this.saveToDisk()); await flushSave; // Drain saves that were queued while the flush save was in flight. - await Promise.resolve(); + await drainSaveMicrotasks(); if (this.saveDebounceTimer === null && this.pendingSave === null) { return; } diff --git a/lib/storage/migrations.ts b/lib/storage/migrations.ts index 37154007..498fbbbe 100644 --- a/lib/storage/migrations.ts +++ b/lib/storage/migrations.ts @@ -23,6 +23,8 @@ export function normalizeStoredAccountDisabledReason( enabled: unknown, disabledReason: unknown, ): AccountDisabledReason | undefined { + // Unlike V1 migration, V3 normalization intentionally leaves missing/invalid + // reasons unset so legacy-disabled accounts are not auto-revived as auth failures. return enabled === false ? normalizeAccountDisabledReason(disabledReason) : undefined; diff --git a/test/index.test.ts b/test/index.test.ts index e2fbfbfe..957e3291 100644 --- a/test/index.test.ts +++ b/test/index.test.ts @@ -2204,6 +2204,86 @@ describe("OpenAIOAuthPlugin fetch handler", () => { ); }); + it("surfaces a disabled-pool response when grouped disable and cooldown both no-op", async () => { + const fetchHelpers = await import("../lib/request/fetch-helpers.js"); + const { AccountManager } = await import("../lib/accounts.js"); + const { ACCOUNT_LIMITS } = await import("../lib/constants.js"); + + vi.spyOn(fetchHelpers, "shouldRefreshToken").mockReturnValue(true); + vi.mocked(fetchHelpers.refreshAndUpdateToken).mockRejectedValue( + new Error("Token expired"), + ); + + let selected = false; + const disableGroupedAccountsSpy = vi.fn(() => 0); + const markAccountsWithRefreshTokenCoolingDownSpy = vi.fn(() => 0); + const saveToDiskDebouncedSpy = vi.fn(); + const customManager = { + getAccountCount: () => 1, + getEnabledAccountCount: () => 0, + getCurrentOrNextForFamilyHybrid: () => { + if (selected) { + return null; + } + selected = true; + return { + index: 0, + email: "user1@example.com", + refreshToken: "refresh-1", + }; + }, + getSelectionExplainability: () => [], + toAuthDetails: () => ({ + type: "oauth" as const, + access: "access-disabled", + refresh: "refresh-disabled", + expires: Date.now() + 60_000, + }), + hasRefreshToken: () => true, + saveToDiskDebounced: saveToDiskDebouncedSpy, + updateFromAuth: () => {}, + clearAuthFailures: () => {}, + incrementAuthFailures: () => ACCOUNT_LIMITS.MAX_AUTH_FAILURES_BEFORE_DISABLE, + disableAccountsWithSameRefreshToken: disableGroupedAccountsSpy, + markAccountsWithRefreshTokenCoolingDown: markAccountsWithRefreshTokenCoolingDownSpy, + markAccountCoolingDown: () => {}, + markRateLimitedWithReason: () => {}, + recordRateLimit: () => {}, + consumeToken: () => true, + refundToken: () => {}, + markSwitched: () => {}, + removeAccount: () => {}, + recordFailure: () => {}, + recordSuccess: () => {}, + getMinWaitTimeForFamily: () => 0, + shouldShowAccountToast: () => false, + setActiveIndex: () => null, + getAccountsSnapshot: () => [], + }; + vi.spyOn(AccountManager, "loadFromDisk").mockResolvedValueOnce(customManager as never); + + globalThis.fetch = vi.fn().mockResolvedValue( + new Response(JSON.stringify({ content: "should-not-fetch" }), { status: 200 }), + ); + + const { sdk } = await setupPlugin(); + const response = await sdk.fetch!("https://api.openai.com/v1/chat", { + method: "POST", + body: JSON.stringify({ model: "gpt-5.1" }), + }); + + expect(response.status).toBe(503); + expect(globalThis.fetch).not.toHaveBeenCalled(); + expect(disableGroupedAccountsSpy).toHaveBeenCalledTimes(1); + expect(markAccountsWithRefreshTokenCoolingDownSpy).toHaveBeenCalledWith( + "refresh-1", + ACCOUNT_LIMITS.AUTH_FAILURE_COOLDOWN_MS, + "auth-failure", + ); + expect(saveToDiskDebouncedSpy).toHaveBeenCalledTimes(1); + expect(await response.text()).toContain("All stored Codex accounts are disabled"); + }); + it("skips fetch when local token bucket is depleted", async () => { const { AccountManager } = await import("../lib/accounts.js"); const consumeSpy = vi.spyOn(AccountManager.prototype, "consumeToken").mockReturnValue(false); @@ -4155,7 +4235,6 @@ describe("OpenAIOAuthPlugin persistAccountPool", () => { "[account-menu] blocked re-enable for auth-failure disabled account", expect.objectContaining({ accountId: "workspace-auth-failure", - email: "blocked@example.com", }), ); } finally { From 749eaaf13bc1b632aa5c4dcfdc9733bbd1f6ec0e Mon Sep 17 00:00:00 2001 From: ndycode Date: Sun, 15 Mar 2026 16:39:46 +0800 Subject: [PATCH 35/43] Tighten shutdown exit ordering --- lib/shutdown.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/shutdown.ts b/lib/shutdown.ts index 8558612e..de3d3d37 100644 --- a/lib/shutdown.ts +++ b/lib/shutdown.ts @@ -64,9 +64,6 @@ function ensureShutdownHandler(): void { } signalExitPending = true; let exitRequested = false; - let timeoutHandle: ReturnType | null = setTimeout(() => { - requestExit(); - }, SIGNAL_CLEANUP_TIMEOUT_MS); const requestExit = () => { if (exitRequested) { return; @@ -78,6 +75,9 @@ function ensureShutdownHandler(): void { } process.exit(0); }; + let timeoutHandle: ReturnType | null = setTimeout(() => { + requestExit(); + }, SIGNAL_CLEANUP_TIMEOUT_MS); void runCleanup().finally(() => { requestExit(); }); From 28406113059da50f145e6af3c65018fe596926af Mon Sep 17 00:00:00 2001 From: ndycode Date: Sun, 15 Mar 2026 17:12:34 +0800 Subject: [PATCH 36/43] Harden auth-failure save flush follow-ups --- index.ts | 15 ++++--- lib/accounts.ts | 41 ++++++++++++----- lib/shutdown.ts | 9 ++++ test/accounts.test.ts | 101 ++++++++++++++++++++++++++++++++++++++++++ test/index.test.ts | 64 ++++++++++++++++++++++++-- 5 files changed, 212 insertions(+), 18 deletions(-) diff --git a/index.ts b/index.ts index b9fb1208..bc9f71fc 100644 --- a/index.ts +++ b/index.ts @@ -1126,7 +1126,7 @@ export const OpenAIOAuthPlugin: Plugin = async ({ client }: PluginInput) => { message: string, variant: "info" | "success" | "warning" | "error" = "success", options?: { title?: string; duration?: number }, - ): Promise => { + ): Promise => { try { await client.tui.showToast({ body: { @@ -1136,8 +1136,10 @@ export const OpenAIOAuthPlugin: Plugin = async ({ client }: PluginInput) => { ...(options?.duration && { duration: options.duration }), }, }); + return true; } catch { // Ignore when TUI is not available. + return false; } }; @@ -1705,6 +1707,9 @@ export const OpenAIOAuthPlugin: Plugin = async ({ client }: PluginInput) => { return; } + // Re-registering this waiter after each settle is intentional: a manager can + // move from "debounce only" to "in-flight save" repeatedly, and we do not + // want a transient idle check to drop shutdown tracking before the next save. const waitForSettle = manager .waitForPendingSaveToSettle() .finally(() => { @@ -3589,10 +3594,10 @@ while (attempted.size < Math.max(1, accountCount)) { logInfo("[account-menu] prompted re-auth for auth-failure disabled account", { accountId: target.accountId ?? null, }); - await showToast(message, "warning"); - console.log( - `\n${message}\n`, - ); + const toastShown = await showToast(message, "warning"); + if (!toastShown) { + console.log("\nRun 'opencode auth login' to re-enable this account.\n"); + } continue; } if (shouldEnable) { diff --git a/lib/accounts.ts b/lib/accounts.ts index 5fce82d8..c57a2116 100644 --- a/lib/accounts.ts +++ b/lib/accounts.ts @@ -66,12 +66,6 @@ import { const log = createLogger("accounts"); -async function drainSaveMicrotasks(): Promise { - // enqueueSave settles through an async IIFE and a trailing .finally(). - await Promise.resolve(); - await Promise.resolve(); -} - export type CodexCliTokenCacheEntry = { accessToken: string; expiresAt?: number; @@ -230,6 +224,8 @@ export class AccountManager { private pendingSave: Promise | null = null; private authFailuresByRefreshToken: Map = new Map(); private pendingSaveSettledWaiters = new Set<() => void>(); + private saveFinalizationTick = 0; + private saveFinalizationWaiters = new Set<{ afterTick: number; resolve: () => void }>(); static async loadFromDisk(authFallback?: OAuthAuthDetails): Promise { const stored = await loadAccounts(); @@ -1046,6 +1042,7 @@ export class AccountManager { this.pendingSave = null; } this.resolvePendingSaveSettledWaiters(); + this.notifySaveFinalization(); }); this.pendingSave = nextSave; return nextSave; @@ -1060,11 +1057,35 @@ export class AccountManager { return Promise.resolve(); } + // This intentionally treats debounce-only timers as pending work too. Callers + // such as scheduleTrackedManagerPrune wait across those gaps so a later re-arm + // cannot drop shutdown tracking before the deferred save actually runs. return new Promise((resolve) => { this.pendingSaveSettledWaiters.add(resolve); }); } + private waitForSaveFinalization(afterTick: number): Promise { + if (this.saveFinalizationTick > afterTick) { + return Promise.resolve(); + } + + return new Promise((resolve) => { + this.saveFinalizationWaiters.add({ afterTick, resolve }); + }); + } + + private notifySaveFinalization(): void { + this.saveFinalizationTick += 1; + for (const waiter of [...this.saveFinalizationWaiters]) { + if (this.saveFinalizationTick <= waiter.afterTick) { + continue; + } + this.saveFinalizationWaiters.delete(waiter); + waiter.resolve(); + } + } + private resolvePendingSaveSettledWaiters(): void { if (this.hasPendingSave() || this.pendingSaveSettledWaiters.size === 0) { return; @@ -1113,14 +1134,14 @@ export class AccountManager { this.saveDebounceTimer = null; } if (this.pendingSave) { + const pendingSaveTick = this.saveFinalizationTick; let pendingSaveError: unknown; try { await this.pendingSave; } catch (error) { pendingSaveError = error; } - // Let debounced callbacks waiting on the completed save re-arm pendingSave. - await drainSaveMicrotasks(); + await this.waitForSaveFinalization(pendingSaveTick); if (this.saveDebounceTimer !== null || this.pendingSave !== null) { continue; } @@ -1142,10 +1163,10 @@ export class AccountManager { if (this.saveDebounceTimer !== null || this.pendingSave !== null) { continue; } + const flushSaveTick = this.saveFinalizationTick; const flushSave = this.enqueueSave(() => this.saveToDisk()); await flushSave; - // Drain saves that were queued while the flush save was in flight. - await drainSaveMicrotasks(); + await this.waitForSaveFinalization(flushSaveTick); if (this.saveDebounceTimer === null && this.pendingSave === null) { return; } diff --git a/lib/shutdown.ts b/lib/shutdown.ts index de3d3d37..38c1d98e 100644 --- a/lib/shutdown.ts +++ b/lib/shutdown.ts @@ -20,6 +20,15 @@ export function unregisterCleanup(fn: CleanupFn): void { if (index !== -1) { cleanupFunctions.splice(index, 1); } + if ( + cleanupFunctions.length === 0 && + !cleanupInFlight && + !signalExitPending && + shutdownRegistered + ) { + removeShutdownHandlers(); + shutdownRegistered = false; + } } export function runCleanup(): Promise { diff --git a/test/accounts.test.ts b/test/accounts.test.ts index 2a8232f4..55417785 100644 --- a/test/accounts.test.ts +++ b/test/accounts.test.ts @@ -1884,6 +1884,107 @@ describe("AccountManager", () => { }), ]); }); + + it("persists newer disabled state after two queued save failures once a later flush requeues it", async () => { + const { saveAccounts } = await import("../lib/storage.js"); + const mockSaveAccounts = vi.mocked(saveAccounts); + mockSaveAccounts.mockClear(); + + const savedSnapshots: Array< + Array<{ + refreshToken?: string; + enabled?: false; + disabledReason?: string; + }> + > = []; + + let rejectFirstSave!: (error: Error) => void; + let rejectSecondSave!: (error: Error) => void; + const firstSave = new Promise((_resolve, reject) => { + rejectFirstSave = reject; + }); + const secondSave = new Promise((_resolve, reject) => { + rejectSecondSave = reject; + }); + + mockSaveAccounts.mockImplementationOnce(async (storage) => { + savedSnapshots.push( + storage.accounts.map((account) => ({ + refreshToken: account.refreshToken, + enabled: account.enabled, + disabledReason: account.disabledReason, + })), + ); + await firstSave; + }); + mockSaveAccounts.mockImplementationOnce(async (storage) => { + savedSnapshots.push( + storage.accounts.map((account) => ({ + refreshToken: account.refreshToken, + enabled: account.enabled, + disabledReason: account.disabledReason, + })), + ); + await secondSave; + }); + mockSaveAccounts.mockImplementationOnce(async (storage) => { + savedSnapshots.push( + storage.accounts.map((account) => ({ + refreshToken: account.refreshToken, + enabled: account.enabled, + disabledReason: account.disabledReason, + })), + ); + }); + + const now = Date.now(); + const stored = { + version: 3 as const, + activeIndex: 0, + accounts: [ + { refreshToken: "token-1", addedAt: now, lastUsed: now }, + ], + }; + + const manager = new AccountManager(undefined, stored); + + manager.saveToDiskDebounced(0); + await vi.advanceTimersByTimeAsync(0); + expect(mockSaveAccounts).toHaveBeenCalledTimes(1); + + const account = manager.getAccountsSnapshot()[0]!; + manager.disableAccountsWithSameRefreshToken(account); + manager.saveToDiskDebounced(50); + await vi.advanceTimersByTimeAsync(60); + + rejectFirstSave(Object.assign(new Error("EBUSY: file in use"), { code: "EBUSY" })); + await vi.advanceTimersByTimeAsync(0); + for (let turn = 0; turn < 6; turn += 1) { + await Promise.resolve(); + } + + expect(mockSaveAccounts).toHaveBeenCalledTimes(2); + + rejectSecondSave(Object.assign(new Error("EBUSY: file in use"), { code: "EBUSY" })); + await vi.advanceTimersByTimeAsync(0); + for (let turn = 0; turn < 6; turn += 1) { + await Promise.resolve(); + } + + expect(mockSaveAccounts).toHaveBeenCalledTimes(2); + + manager.saveToDiskDebounced(0); + await manager.flushPendingSave(); + + expect(mockSaveAccounts).toHaveBeenCalledTimes(3); + expect(savedSnapshots[2]).toEqual([ + expect.objectContaining({ + refreshToken: "token-1", + enabled: false, + disabledReason: "auth-failure", + }), + ]); + }); }); describe("constructor edge cases", () => { diff --git a/test/index.test.ts b/test/index.test.ts index 957e3291..9d919261 100644 --- a/test/index.test.ts +++ b/test/index.test.ts @@ -4215,9 +4215,7 @@ describe("OpenAIOAuthPlugin persistAccountPool", () => { coolingDownUntil: 30_000, cooldownReason: "auth-failure", }); - expect(consoleLog).toHaveBeenCalledWith( - expect.stringContaining("Run 'opencode auth login' to re-enable with fresh credentials."), - ); + expect(consoleLog).not.toHaveBeenCalled(); expect(mockClient.tui.showToast).toHaveBeenCalledWith({ body: expect.objectContaining({ message: @@ -4241,6 +4239,66 @@ describe("OpenAIOAuthPlugin persistAccountPool", () => { consoleLog.mockRestore(); } }); + + it("falls back to a generic console hint when TUI toast is unavailable for auth-failure disables", async () => { + const cliModule = await import("../lib/cli.js"); + const loggerModule = await import("../lib/logger.js"); + const storageModule = await import("../lib/storage.js"); + const consoleLog = vi.spyOn(console, "log").mockImplementation(() => {}); + + try { + mockStorage.accounts = [ + { + accountId: "workspace-auth-failure", + email: "blocked@example.com", + refreshToken: "refresh-blocked", + enabled: false, + disabledReason: "auth-failure", + coolingDownUntil: 30_000, + cooldownReason: "auth-failure", + addedAt: 10, + lastUsed: 10, + }, + ]; + + vi.mocked(cliModule.promptLoginMode) + .mockResolvedValueOnce({ mode: "manage", toggleAccountIndex: 0 }) + .mockResolvedValueOnce({ mode: "cancel" }); + + const mockClient = createMockClient(); + mockClient.tui.showToast.mockRejectedValueOnce(new Error("TUI unavailable")); + const { OpenAIOAuthPlugin } = await import("../index.js"); + const plugin = (await OpenAIOAuthPlugin({ + client: mockClient, + } as never)) as unknown as PluginType; + const autoMethod = plugin.auth.methods[0] as unknown as { + authorize: (inputs?: Record) => Promise<{ instructions: string }>; + }; + + vi.mocked(loggerModule.logWarn).mockClear(); + const authResult = await autoMethod.authorize(); + expect(authResult.instructions).toBe("Authentication cancelled"); + + expect(vi.mocked(storageModule.saveAccounts)).not.toHaveBeenCalled(); + expect(consoleLog).toHaveBeenCalledWith( + "\nRun 'opencode auth login' to re-enable this account.\n", + ); + expect(vi.mocked(loggerModule.logInfo)).toHaveBeenCalledWith( + "[account-menu] prompted re-auth for auth-failure disabled account", + expect.objectContaining({ + accountId: "workspace-auth-failure", + }), + ); + expect(vi.mocked(loggerModule.logWarn)).toHaveBeenCalledWith( + "[account-menu] blocked re-enable for auth-failure disabled account", + expect.objectContaining({ + accountId: "workspace-auth-failure", + }), + ); + } finally { + consoleLog.mockRestore(); + } + }); }); describe("OpenAIOAuthPlugin showToast error handling", () => { From 66fb080708619d43bbb39da7b6b88ea8c9f6a2f7 Mon Sep 17 00:00:00 2001 From: ndycode Date: Sun, 15 Mar 2026 17:48:24 +0800 Subject: [PATCH 37/43] Tighten disabled-account review follow-ups --- index.ts | 2 + lib/schemas.ts | 16 +++++--- test/index.test.ts | 97 ++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 110 insertions(+), 5 deletions(-) diff --git a/index.ts b/index.ts index bc9f71fc..d9e65e1c 100644 --- a/index.ts +++ b/index.ts @@ -204,6 +204,8 @@ import { // Shared across plugin instances so shutdown cleanup can flush debounced saves // even when multiple plugin objects coexist during reloads or tests. let accountManagerCleanupHook: (() => Promise) | null = null; +// `active...` retains only the shared currently cached manager. Older managers move to +// `tracked...` only while they still have pending disk work and self-prune once settled. const activeAccountManagersForCleanup = new Set(); const trackedAccountManagersForCleanup = new Set(); diff --git a/lib/schemas.ts b/lib/schemas.ts index a89fa7a3..0405a402 100644 --- a/lib/schemas.ts +++ b/lib/schemas.ts @@ -75,14 +75,20 @@ export const CooldownReasonSchema = z.enum(["auth-failure", "network-error"]); export type CooldownReasonFromSchema = z.infer; +const AccountDisabledReasonValueSchema = z.enum(["user", "auth-failure"]); + // Storage normalization strips unknown disabled reasons later; keep schema parsing // lenient so legacy/downgraded files don't warn or fail before that step runs. -export const DisabledReasonSchema = z.preprocess( +export const AccountDisabledReasonSchema = z.preprocess( normalizeAccountDisabledReason, - z.enum(["user", "auth-failure"]).optional(), + AccountDisabledReasonValueSchema.optional(), ); -export type DisabledReasonFromSchema = z.infer; +// Preserve the older export name for callers while using a more specific schema name internally. +export const DisabledReasonSchema = AccountDisabledReasonSchema; + +export type AccountDisabledReasonFromSchema = z.infer; +export type DisabledReasonFromSchema = AccountDisabledReasonFromSchema; /** * Last switch reason for account rotation tracking. @@ -127,7 +133,7 @@ export const AccountMetadataV3Schema = z.object({ accessToken: z.string().optional(), expiresAt: z.number().optional(), enabled: z.boolean().optional(), - disabledReason: DisabledReasonSchema, + disabledReason: AccountDisabledReasonSchema, addedAt: z.number(), lastUsed: z.number(), lastSwitchReason: SwitchReasonSchema.optional(), @@ -175,7 +181,7 @@ export const AccountMetadataV1Schema = z.object({ accessToken: z.string().optional(), expiresAt: z.number().optional(), enabled: z.boolean().optional(), - disabledReason: DisabledReasonSchema, + disabledReason: AccountDisabledReasonSchema, addedAt: z.number(), lastUsed: z.number(), lastSwitchReason: SwitchReasonSchema.optional(), diff --git a/test/index.test.ts b/test/index.test.ts index 9d919261..fca4b744 100644 --- a/test/index.test.ts +++ b/test/index.test.ts @@ -2284,6 +2284,103 @@ describe("OpenAIOAuthPlugin fetch handler", () => { expect(await response.text()).toContain("All stored Codex accounts are disabled"); }); + it("surfaces the disabled-pool response when same-token variants were already user-disabled", async () => { + const fetchHelpers = await import("../lib/request/fetch-helpers.js"); + const { AccountManager } = await import("../lib/accounts.js"); + const { ACCOUNT_LIMITS } = await import("../lib/constants.js"); + + vi.spyOn(fetchHelpers, "shouldRefreshToken").mockReturnValue(true); + vi.mocked(fetchHelpers.refreshAndUpdateToken).mockRejectedValue( + new Error("Token expired"), + ); + + let selected = false; + const disableGroupedAccountsSpy = vi.fn(() => 0); + const markAccountsWithRefreshTokenCoolingDownSpy = vi.fn(() => 0); + const saveToDiskDebouncedSpy = vi.fn(); + const customManager = { + getAccountCount: () => 2, + getEnabledAccountCount: () => 0, + getCurrentOrNextForFamilyHybrid: () => { + if (selected) { + return null; + } + selected = true; + return { + index: 0, + email: "user1@example.com", + refreshToken: "refresh-shared", + }; + }, + getSelectionExplainability: () => [], + toAuthDetails: () => ({ + type: "oauth" as const, + access: "access-disabled", + refresh: "refresh-shared", + expires: Date.now() + 60_000, + }), + hasRefreshToken: () => true, + saveToDiskDebounced: saveToDiskDebouncedSpy, + updateFromAuth: () => {}, + clearAuthFailures: () => {}, + incrementAuthFailures: () => ACCOUNT_LIMITS.MAX_AUTH_FAILURES_BEFORE_DISABLE, + disableAccountsWithSameRefreshToken: disableGroupedAccountsSpy, + markAccountsWithRefreshTokenCoolingDown: markAccountsWithRefreshTokenCoolingDownSpy, + markAccountCoolingDown: () => {}, + markRateLimitedWithReason: () => {}, + recordRateLimit: () => {}, + consumeToken: () => true, + refundToken: () => {}, + markSwitched: () => {}, + removeAccount: () => {}, + recordFailure: () => {}, + recordSuccess: () => {}, + getMinWaitTimeForFamily: () => 0, + shouldShowAccountToast: () => false, + setActiveIndex: () => null, + getAccountsSnapshot: () => [ + { + index: 0, + email: "user1@example.com", + refreshToken: "refresh-shared", + enabled: false, + disabledReason: "user" as const, + }, + { + index: 1, + email: "user1@example.com", + refreshToken: "refresh-shared", + enabled: false, + disabledReason: "user" as const, + }, + ], + }; + vi.spyOn(AccountManager, "loadFromDisk").mockResolvedValueOnce(customManager as never); + + globalThis.fetch = vi.fn().mockResolvedValue( + new Response(JSON.stringify({ content: "should-not-fetch" }), { status: 200 }), + ); + + const { sdk } = await setupPlugin(); + const response = await sdk.fetch!("https://api.openai.com/v1/chat", { + method: "POST", + body: JSON.stringify({ model: "gpt-5.1" }), + }); + + expect(response.status).toBe(503); + expect(globalThis.fetch).not.toHaveBeenCalled(); + expect(disableGroupedAccountsSpy).toHaveBeenCalledTimes(1); + expect(markAccountsWithRefreshTokenCoolingDownSpy).toHaveBeenCalledWith( + "refresh-shared", + ACCOUNT_LIMITS.AUTH_FAILURE_COOLDOWN_MS, + "auth-failure", + ); + expect(saveToDiskDebouncedSpy).toHaveBeenCalledTimes(1); + const responseText = await response.text(); + expect(responseText).toContain("All stored Codex accounts are disabled"); + expect(responseText).toContain("Re-enable user-disabled accounts from account management"); + }); + it("skips fetch when local token bucket is depleted", async () => { const { AccountManager } = await import("../lib/accounts.js"); const consumeSpy = vi.spyOn(AccountManager.prototype, "consumeToken").mockReturnValue(false); From 5072f0e385fc0a2281989861a79a536ffedef080 Mon Sep 17 00:00:00 2001 From: ndycode Date: Sun, 15 Mar 2026 18:07:56 +0800 Subject: [PATCH 38/43] Refine disabled-pool guidance and test doubles --- index.ts | 19 +++++++- lib/accounts.ts | 4 +- test/index.test.ts | 109 +++++++++++++++++++++++++++++++++++++++++---- 3 files changed, 121 insertions(+), 11 deletions(-) diff --git a/index.ts b/index.ts index d9e65e1c..84087b4a 100644 --- a/index.ts +++ b/index.ts @@ -2798,11 +2798,28 @@ while (attempted.size < Math.max(1, accountCount)) { } const waitLabel = waitMs > 0 ? formatWaitTime(waitMs) : "a bit"; + const disabledSnapshot = + enabledCount === 0 ? accountManager.getAccountsSnapshot() : []; + const hasUserDisabled = disabledSnapshot.some( + (account) => + account.enabled === false && + account.disabledReason !== "auth-failure", + ); + const hasAuthFailureDisabled = disabledSnapshot.some( + (account) => + account.enabled === false && + account.disabledReason === "auth-failure", + ); + const disabledMessage = hasUserDisabled && hasAuthFailureDisabled + ? "All stored Codex accounts are disabled. Re-enable user-disabled accounts from account management, or run `opencode auth login` to restore auth-failure disables." + : hasAuthFailureDisabled + ? "All stored Codex accounts are disabled after repeated auth failures. Run `opencode auth login` to restore access." + : "All stored Codex accounts are user-disabled. Re-enable them from account management."; const message = totalCount === 0 ? "No Codex accounts configured. Run `opencode auth login`." : enabledCount === 0 - ? "All stored Codex accounts are disabled. Re-enable user-disabled accounts from account management, or run `opencode auth login` to restore auth-failure disables." + ? disabledMessage : waitMs > 0 ? `All ${enabledCount} enabled account(s) are rate-limited. Try again in ${waitLabel} or add another account with \`opencode auth login\`.` : `All ${enabledCount} enabled account(s) failed (server errors or auth issues). Check account health with \`codex-health\`.`; diff --git a/lib/accounts.ts b/lib/accounts.ts index c57a2116..62b5f8e4 100644 --- a/lib/accounts.ts +++ b/lib/accounts.ts @@ -1059,7 +1059,9 @@ export class AccountManager { // This intentionally treats debounce-only timers as pending work too. Callers // such as scheduleTrackedManagerPrune wait across those gaps so a later re-arm - // cannot drop shutdown tracking before the deferred save actually runs. + // cannot drop shutdown tracking before the deferred save actually runs. If a + // manager is abandoned before the debounce is replayed through flushPendingSave + // or the timer itself fires, cleanup-time pruning is the final backstop. return new Promise((resolve) => { this.pendingSaveSettledWaiters.add(resolve); }); diff --git a/test/index.test.ts b/test/index.test.ts index fca4b744..c412bcb8 100644 --- a/test/index.test.ts +++ b/test/index.test.ts @@ -2098,7 +2098,25 @@ describe("OpenAIOAuthPlugin fetch handler", () => { shouldShowAccountToast: () => false, markToastShown: () => {}, setActiveIndex: () => null, - getAccountsSnapshot: () => [], + hasPendingSave: () => false, + waitForPendingSaveToSettle: async () => {}, + flushPendingSave: async () => {}, + getAccountsSnapshot: () => [ + { + index: 0, + email: "user1@example.com", + refreshToken: "refresh-disabled-a", + enabled: false, + disabledReason: "user" as const, + }, + { + index: 1, + email: "user2@example.com", + refreshToken: "refresh-disabled-b", + enabled: false, + disabledReason: "user" as const, + }, + ], }; vi.spyOn(AccountManager, "loadFromDisk").mockResolvedValueOnce(customManager as never); globalThis.fetch = vi.fn(); @@ -2112,8 +2130,8 @@ describe("OpenAIOAuthPlugin fetch handler", () => { expect(globalThis.fetch).not.toHaveBeenCalled(); expect(response.status).toBe(503); const responseText = await response.text(); - expect(responseText).toContain("All stored Codex accounts are disabled"); - expect(responseText).toContain("Re-enable user-disabled accounts from account management"); + expect(responseText).toContain("All stored Codex accounts are user-disabled"); + expect(responseText).toContain("Re-enable them from account management"); }); it("disables grouped accounts when auth failures hit the threshold", async () => { @@ -2258,6 +2276,9 @@ describe("OpenAIOAuthPlugin fetch handler", () => { getMinWaitTimeForFamily: () => 0, shouldShowAccountToast: () => false, setActiveIndex: () => null, + hasPendingSave: () => false, + waitForPendingSaveToSettle: async () => {}, + flushPendingSave: async () => {}, getAccountsSnapshot: () => [], }; vi.spyOn(AccountManager, "loadFromDisk").mockResolvedValueOnce(customManager as never); @@ -2281,7 +2302,9 @@ describe("OpenAIOAuthPlugin fetch handler", () => { "auth-failure", ); expect(saveToDiskDebouncedSpy).toHaveBeenCalledTimes(1); - expect(await response.text()).toContain("All stored Codex accounts are disabled"); + const responseText = await response.text(); + expect(responseText).toContain("All stored Codex accounts are user-disabled"); + expect(responseText).toContain("Re-enable them from account management"); }); it("surfaces the disabled-pool response when same-token variants were already user-disabled", async () => { @@ -2338,6 +2361,9 @@ describe("OpenAIOAuthPlugin fetch handler", () => { getMinWaitTimeForFamily: () => 0, shouldShowAccountToast: () => false, setActiveIndex: () => null, + hasPendingSave: () => false, + waitForPendingSaveToSettle: async () => {}, + flushPendingSave: async () => {}, getAccountsSnapshot: () => [ { index: 0, @@ -2377,8 +2403,76 @@ describe("OpenAIOAuthPlugin fetch handler", () => { ); expect(saveToDiskDebouncedSpy).toHaveBeenCalledTimes(1); const responseText = await response.text(); - expect(responseText).toContain("All stored Codex accounts are disabled"); - expect(responseText).toContain("Re-enable user-disabled accounts from account management"); + expect(responseText).toContain("All stored Codex accounts are user-disabled"); + expect(responseText).toContain("Re-enable them from account management"); + }); + + it("surfaces an auth-failure-specific disabled-pool response when every account is auth-failure disabled", async () => { + const { AccountManager } = await import("../lib/accounts.js"); + + const customManager = { + getAccountCount: () => 2, + getEnabledAccountCount: () => 0, + getCurrentOrNextForFamilyHybrid: () => null, + getSelectionExplainability: () => [], + toAuthDetails: () => ({ + type: "oauth" as const, + access: "access-disabled", + refresh: "refresh-disabled", + expires: Date.now() + 60_000, + }), + hasRefreshToken: () => true, + saveToDiskDebounced: () => {}, + updateFromAuth: () => {}, + clearAuthFailures: () => {}, + incrementAuthFailures: () => 1, + markAccountCoolingDown: () => {}, + markRateLimitedWithReason: () => {}, + recordRateLimit: () => {}, + consumeToken: () => true, + refundToken: () => {}, + markSwitched: () => {}, + removeAccount: () => {}, + recordFailure: () => {}, + recordSuccess: () => {}, + getMinWaitTimeForFamily: () => 0, + shouldShowAccountToast: () => false, + markToastShown: () => {}, + setActiveIndex: () => null, + hasPendingSave: () => false, + waitForPendingSaveToSettle: async () => {}, + flushPendingSave: async () => {}, + getAccountsSnapshot: () => [ + { + index: 0, + email: "user1@example.com", + refreshToken: "refresh-disabled-a", + enabled: false, + disabledReason: "auth-failure" as const, + }, + { + index: 1, + email: "user2@example.com", + refreshToken: "refresh-disabled-b", + enabled: false, + disabledReason: "auth-failure" as const, + }, + ], + }; + vi.spyOn(AccountManager, "loadFromDisk").mockResolvedValueOnce(customManager as never); + globalThis.fetch = vi.fn(); + + const { sdk } = await setupPlugin(); + const response = await sdk.fetch!("https://api.openai.com/v1/chat", { + method: "POST", + body: JSON.stringify({ model: "gpt-5.1" }), + }); + + expect(globalThis.fetch).not.toHaveBeenCalled(); + expect(response.status).toBe(503); + const responseText = await response.text(); + expect(responseText).toContain("disabled after repeated auth failures"); + expect(responseText).toContain("Run `opencode auth login` to restore access"); }); it("skips fetch when local token bucket is depleted", async () => { @@ -3452,7 +3546,6 @@ describe("OpenAIOAuthPlugin persistAccountPool", () => { organizationId: "org-keep", email: "org@example.com", refreshToken: "shared-refresh", - enabled: true, addedAt: 10, lastUsed: 10, }, @@ -4034,7 +4127,6 @@ describe("OpenAIOAuthPlugin persistAccountPool", () => { accountId: "workspace-managed", email: "managed@example.com", refreshToken: "refresh-managed", - enabled: true, addedAt: 10, lastUsed: 10, }, @@ -4106,7 +4198,6 @@ describe("OpenAIOAuthPlugin persistAccountPool", () => { accountId: "workspace-managed", email: "managed@example.com", refreshToken: "refresh-managed", - enabled: true, addedAt: 10, lastUsed: 10, }, From 8f5f9cf68f3a6145498be3ebdb43c189650faeaf Mon Sep 17 00:00:00 2001 From: ndycode Date: Sun, 15 Mar 2026 18:31:12 +0800 Subject: [PATCH 39/43] Harden disabled account cleanup handling --- index.ts | 6 ++++-- lib/shutdown.ts | 20 +++++++++++++++++++- test/accounts.test.ts | 31 +++++++++++++++++++++++++++++++ test/index.test.ts | 5 +++-- test/shutdown.test.ts | 11 ++++++----- 5 files changed, 63 insertions(+), 10 deletions(-) diff --git a/index.ts b/index.ts index 84087b4a..d4a50981 100644 --- a/index.ts +++ b/index.ts @@ -2803,7 +2803,7 @@ while (attempted.size < Math.max(1, accountCount)) { const hasUserDisabled = disabledSnapshot.some( (account) => account.enabled === false && - account.disabledReason !== "auth-failure", + account.disabledReason === "user", ); const hasAuthFailureDisabled = disabledSnapshot.some( (account) => @@ -2814,7 +2814,9 @@ while (attempted.size < Math.max(1, accountCount)) { ? "All stored Codex accounts are disabled. Re-enable user-disabled accounts from account management, or run `opencode auth login` to restore auth-failure disables." : hasAuthFailureDisabled ? "All stored Codex accounts are disabled after repeated auth failures. Run `opencode auth login` to restore access." - : "All stored Codex accounts are user-disabled. Re-enable them from account management."; + : hasUserDisabled + ? "All stored Codex accounts are user-disabled. Re-enable them from account management." + : "All stored Codex accounts are disabled. Re-enable any manually disabled accounts from account management, or run `opencode auth login` to restore access."; const message = totalCount === 0 ? "No Codex accounts configured. Run `opencode auth login`." diff --git a/lib/shutdown.ts b/lib/shutdown.ts index 38c1d98e..2b6967af 100644 --- a/lib/shutdown.ts +++ b/lib/shutdown.ts @@ -63,6 +63,18 @@ export function runCleanup(): Promise { return cleanupInFlight; } +function resetSignalStateAfterInterceptedExit(): void { + if (!signalExitPending) { + return; + } + + signalExitPending = false; + if (cleanupFunctions.length === 0 && !cleanupInFlight && shutdownRegistered) { + removeShutdownHandlers(); + shutdownRegistered = false; + } +} + function ensureShutdownHandler(): void { if (shutdownRegistered) return; shutdownRegistered = true; @@ -88,7 +100,13 @@ function ensureShutdownHandler(): void { requestExit(); }, SIGNAL_CLEANUP_TIMEOUT_MS); void runCleanup().finally(() => { - requestExit(); + try { + requestExit(); + } finally { + // `process.exit()` normally terminates immediately. This only runs when exit + // is intercepted (for example in tests), so we need to unlatch signal state. + resetSignalStateAfterInterceptedExit(); + } }); }; sigintHandler = handleSignal; diff --git a/test/accounts.test.ts b/test/accounts.test.ts index 55417785..ddb9c955 100644 --- a/test/accounts.test.ts +++ b/test/accounts.test.ts @@ -2428,6 +2428,37 @@ describe("AccountManager", () => { } }); + it("resolves settle waiters when flushPendingSave cancels a pending debounce", async () => { + vi.useFakeTimers(); + try { + const { saveAccounts } = await import("../lib/storage.js"); + const mockSaveAccounts = vi.mocked(saveAccounts); + mockSaveAccounts.mockClear(); + mockSaveAccounts.mockResolvedValue(); + + const now = Date.now(); + const stored = { + version: 3 as const, + activeIndex: 0, + accounts: [ + { refreshToken: "token-1", addedAt: now, lastUsed: now }, + ], + }; + + const manager = new AccountManager(undefined, stored); + + manager.saveToDiskDebounced(50); + const settlePromise = manager.waitForPendingSaveToSettle(); + const flushPromise = manager.flushPendingSave(); + + await flushPromise; + await expect(settlePromise).resolves.toBeUndefined(); + expect(mockSaveAccounts).toHaveBeenCalledTimes(1); + } finally { + vi.useRealTimers(); + } + }); + it("drains saves queued during flush without overlapping writes", async () => { vi.useFakeTimers(); try { diff --git a/test/index.test.ts b/test/index.test.ts index c412bcb8..19a27352 100644 --- a/test/index.test.ts +++ b/test/index.test.ts @@ -2303,8 +2303,9 @@ describe("OpenAIOAuthPlugin fetch handler", () => { ); expect(saveToDiskDebouncedSpy).toHaveBeenCalledTimes(1); const responseText = await response.text(); - expect(responseText).toContain("All stored Codex accounts are user-disabled"); - expect(responseText).toContain("Re-enable them from account management"); + expect(responseText).toContain( + "All stored Codex accounts are disabled. Re-enable any manually disabled accounts from account management, or run `opencode auth login` to restore access.", + ); }); it("surfaces the disabled-pool response when same-token variants were already user-disabled", async () => { diff --git a/test/shutdown.test.ts b/test/shutdown.test.ts index 464620d6..273f0174 100644 --- a/test/shutdown.test.ts +++ b/test/shutdown.test.ts @@ -181,9 +181,9 @@ describe("Graceful shutdown", () => { resolveCleanup(); await vi.waitFor(() => { - expect(processOffSpy).not.toHaveBeenCalled(); expect(processExitSpy).toHaveBeenCalledWith(0); }); + expect(processOffSpy).toHaveBeenCalled(); } finally { processOnSpy.mockRestore(); processOffSpy.mockRestore(); @@ -277,7 +277,7 @@ describe("Graceful shutdown", () => { } }); - it("keeps later signals ignored after cleanup completes until process exit", async () => { + it("allows later signals again if process exit is intercepted", async () => { const capturedHandlers = new Map void>(); const processOnSpy = vi.spyOn(process, "on").mockImplementation((event: string | symbol, handler: (...args: unknown[]) => void) => { @@ -304,10 +304,11 @@ describe("Graceful shutdown", () => { }); sigtermHandler!(); - await Promise.resolve(); + await vi.waitFor(() => { + expect(processExitSpy).toHaveBeenCalledTimes(2); + }); expect(cleanupFn).toHaveBeenCalledTimes(1); - expect(processExitSpy).toHaveBeenCalledTimes(1); } finally { processOnSpy.mockRestore(); processExitSpy.mockRestore(); @@ -387,7 +388,7 @@ describe("Graceful shutdown", () => { await vi.waitFor(() => { expect(processExitSpy).toHaveBeenCalledWith(0); }); - expect(processOffSpy).not.toHaveBeenCalled(); + expect(processOffSpy).toHaveBeenCalled(); } finally { processOnSpy.mockRestore(); processOffSpy.mockRestore(); From b257c2a16c3bf9d04415fc57c56dafe34781d4e7 Mon Sep 17 00:00:00 2001 From: ndycode Date: Sun, 15 Mar 2026 20:23:34 +0800 Subject: [PATCH 40/43] Harden shutdown and flush retry coverage --- lib/accounts.ts | 21 ++++++++++- lib/schemas.ts | 4 +- lib/shutdown.ts | 4 +- test/accounts.test.ts | 65 +++++++++++++++++++++++++++++++++ test/index.test.ts | 85 +++++++++++++++++++++++++++++++++++++++++++ test/shutdown.test.ts | 6 ++- 6 files changed, 180 insertions(+), 5 deletions(-) diff --git a/lib/accounts.ts b/lib/accounts.ts index 62b5f8e4..5cb5d629 100644 --- a/lib/accounts.ts +++ b/lib/accounts.ts @@ -1167,8 +1167,27 @@ export class AccountManager { } const flushSaveTick = this.saveFinalizationTick; const flushSave = this.enqueueSave(() => this.saveToDisk()); - await flushSave; + let flushSaveError: unknown; + try { + await flushSave; + } catch (error) { + flushSaveError = error; + } await this.waitForSaveFinalization(flushSaveTick); + if (this.saveDebounceTimer !== null || this.pendingSave !== null) { + if (flushSaveError) { + log.warn("flushPendingSave: retrying after flush save failure while newer save is queued", { + error: + flushSaveError instanceof Error + ? flushSaveError.message + : String(flushSaveError), + }); + } + continue; + } + if (flushSaveError) { + throw flushSaveError; + } if (this.saveDebounceTimer === null && this.pendingSave === null) { return; } diff --git a/lib/schemas.ts b/lib/schemas.ts index 0405a402..28928bc7 100644 --- a/lib/schemas.ts +++ b/lib/schemas.ts @@ -133,7 +133,7 @@ export const AccountMetadataV3Schema = z.object({ accessToken: z.string().optional(), expiresAt: z.number().optional(), enabled: z.boolean().optional(), - disabledReason: AccountDisabledReasonSchema, + disabledReason: AccountDisabledReasonSchema.optional(), addedAt: z.number(), lastUsed: z.number(), lastSwitchReason: SwitchReasonSchema.optional(), @@ -181,7 +181,7 @@ export const AccountMetadataV1Schema = z.object({ accessToken: z.string().optional(), expiresAt: z.number().optional(), enabled: z.boolean().optional(), - disabledReason: AccountDisabledReasonSchema, + disabledReason: AccountDisabledReasonSchema.optional(), addedAt: z.number(), lastUsed: z.number(), lastSwitchReason: SwitchReasonSchema.optional(), diff --git a/lib/shutdown.ts b/lib/shutdown.ts index 2b6967af..312f4c22 100644 --- a/lib/shutdown.ts +++ b/lib/shutdown.ts @@ -1,6 +1,8 @@ type CleanupFn = () => void | Promise; -const SIGNAL_CLEANUP_TIMEOUT_MS = 5_000; +// Allow enough time for serialized manager flushes to finish on Windows hosts +// where AV/file-indexer contention can hold the accounts file for multiple seconds. +const SIGNAL_CLEANUP_TIMEOUT_MS = 10_000; const cleanupFunctions: CleanupFn[] = []; let shutdownRegistered = false; diff --git a/test/accounts.test.ts b/test/accounts.test.ts index ddb9c955..567d70cf 100644 --- a/test/accounts.test.ts +++ b/test/accounts.test.ts @@ -2713,6 +2713,71 @@ describe("AccountManager", () => { } }); + it("keeps retrying re-armed flush saves after EBUSY failures until a later write succeeds", async () => { + vi.useFakeTimers(); + try { + const { saveAccounts } = await import("../lib/storage.js"); + const mockSaveAccounts = vi.mocked(saveAccounts); + mockSaveAccounts.mockClear(); + + const savedSnapshots: Array< + Array<{ + refreshToken?: string; + enabled?: false; + disabledReason?: string; + }> + > = []; + + const now = Date.now(); + const stored = { + version: 3 as const, + activeIndex: 0, + accounts: [ + { refreshToken: "shared-refresh", addedAt: now, lastUsed: now }, + ], + }; + + const manager = new AccountManager(undefined, stored); + const account = manager.getAccountsSnapshot()[0]!; + manager.disableAccountsWithSameRefreshToken(account); + + let attempts = 0; + mockSaveAccounts.mockImplementation(async (storage) => { + savedSnapshots.push( + storage.accounts.map((savedAccount) => ({ + refreshToken: savedAccount.refreshToken, + enabled: savedAccount.enabled, + disabledReason: savedAccount.disabledReason, + })), + ); + attempts += 1; + if (attempts < 4) { + manager.saveToDiskDebounced(0); + throw Object.assign(new Error("EBUSY: file in use"), { code: "EBUSY" }); + } + }); + + manager.saveToDiskDebounced(0); + + await expect(manager.flushPendingSave()).resolves.toBeUndefined(); + + expect(mockSaveAccounts).toHaveBeenCalledTimes(4); + expect(savedSnapshots.at(-1)).toEqual([ + expect.objectContaining({ + refreshToken: "shared-refresh", + enabled: false, + disabledReason: "auth-failure", + }), + ]); + expect(accountLoggerWarn).toHaveBeenCalledWith( + "flushPendingSave: retrying after flush save failure while newer save is queued", + expect.objectContaining({ error: "EBUSY: file in use" }), + ); + } finally { + vi.useRealTimers(); + } + }); + it("warns and rejects if flushPendingSave keeps discovering new saves", async () => { vi.useFakeTimers(); try { diff --git a/test/index.test.ts b/test/index.test.ts index 19a27352..b8ed7368 100644 --- a/test/index.test.ts +++ b/test/index.test.ts @@ -4241,6 +4241,91 @@ describe("OpenAIOAuthPlugin persistAccountPool", () => { expect(flushManagerOne).not.toHaveBeenCalled(); }); + it("flushes a tracked debounce-only manager during shutdown before its settle waiter resolves naturally", async () => { + const accountsModule = await import("../lib/accounts.js"); + const cliModule = await import("../lib/cli.js"); + const managerOne = await accountsModule.AccountManager.loadFromDisk(); + const managerTwo = await accountsModule.AccountManager.loadFromDisk(); + const flushManagerOne = vi.fn(async () => { + managerOneHasPendingSave = false; + resolveManagerOneSettled?.(); + }); + const flushManagerTwo = vi.fn(async () => {}); + let managerOneHasPendingSave = true; + let resolveManagerOneSettled: (() => void) | null = null; + managerOne.flushPendingSave = flushManagerOne; + managerOne.hasPendingSave = vi.fn(() => managerOneHasPendingSave); + managerOne.waitForPendingSaveToSettle = vi.fn( + () => + new Promise((resolve) => { + resolveManagerOneSettled = resolve; + }), + ); + managerTwo.flushPendingSave = flushManagerTwo; + managerTwo.hasPendingSave = vi.fn(() => false); + managerTwo.waitForPendingSaveToSettle = vi.fn(async () => {}); + vi.spyOn(accountsModule.AccountManager, "loadFromDisk") + .mockResolvedValueOnce(managerOne) + .mockResolvedValue(managerTwo); + + mockStorage.accounts = [ + { + accountId: "workspace-managed", + email: "managed@example.com", + refreshToken: "refresh-managed", + addedAt: 10, + lastUsed: 10, + }, + ]; + + vi.mocked(cliModule.promptLoginMode) + .mockResolvedValueOnce({ mode: "manage", toggleAccountIndex: 0 }) + .mockResolvedValueOnce({ mode: "cancel" }); + + const mockClient = createMockClient(); + const { OpenAIOAuthPlugin } = await import("../index.js"); + const plugin = (await OpenAIOAuthPlugin({ + client: mockClient, + } as never)) as unknown as PluginType; + const autoMethod = plugin.auth.methods[0] as unknown as { + authorize: (inputs?: Record) => Promise<{ instructions: string }>; + }; + + await plugin.auth.loader( + async () => ({ + type: "oauth", + access: "access-token-1", + refresh: "refresh-token-1", + expires: Date.now() + 60_000, + }) as never, + {}, + ); + + flushManagerOne.mockClear(); + const authResult = await autoMethod.authorize(); + expect(authResult.instructions).toBe("Authentication cancelled"); + + await plugin.auth.loader( + async () => ({ + type: "oauth", + access: "access-token-2", + refresh: "refresh-token-2", + expires: Date.now() + 60_000, + }) as never, + {}, + ); + + expect(managerOne.waitForPendingSaveToSettle).toHaveBeenCalledTimes(1); + + await runCleanup(); + await Promise.resolve(); + await Promise.resolve(); + + expect(flushManagerOne).toHaveBeenCalledTimes(1); + expect(flushManagerTwo).toHaveBeenCalledTimes(1); + expect(managerOneHasPendingSave).toBe(false); + }); + it("flushes tracked account managers from multiple plugin instances during shutdown cleanup", async () => { const accountsModule = await import("../lib/accounts.js"); const managerOne = await accountsModule.AccountManager.loadFromDisk(); diff --git a/test/shutdown.test.ts b/test/shutdown.test.ts index 273f0174..4ee7f756 100644 --- a/test/shutdown.test.ts +++ b/test/shutdown.test.ts @@ -266,7 +266,11 @@ describe("Graceful shutdown", () => { expect(cleanupFn).toHaveBeenCalledTimes(1); expect(processExitSpy).not.toHaveBeenCalled(); - await vi.advanceTimersByTimeAsync(5_000); + await vi.advanceTimersByTimeAsync(9_999); + + expect(processExitSpy).not.toHaveBeenCalled(); + + await vi.advanceTimersByTimeAsync(1); expect(processExitSpy).toHaveBeenCalledTimes(1); expect(processExitSpy).toHaveBeenCalledWith(0); From 31a742732e37e5958b8f2870bc966f93aa9ee3ca Mon Sep 17 00:00:00 2001 From: ndycode Date: Sun, 15 Mar 2026 23:47:37 +0800 Subject: [PATCH 41/43] Fix shutdown and auth-disable follow-ups --- index.ts | 25 +++-- lib/accounts.ts | 32 +++++- lib/shutdown.ts | 15 ++- test/accounts.test.ts | 66 ++++++++++++ test/index.test.ts | 237 +++++++++++++++++++++++++++++------------- test/shutdown.test.ts | 35 +++++++ 6 files changed, 319 insertions(+), 91 deletions(-) diff --git a/index.ts b/index.ts index d4a50981..1d345ba0 100644 --- a/index.ts +++ b/index.ts @@ -1716,9 +1716,15 @@ export const OpenAIOAuthPlugin: Plugin = async ({ client }: PluginInput) => { .waitForPendingSaveToSettle() .finally(() => { trackedManagerSettledWaits.delete(manager); - if (!managerHasPendingSave(manager) && !activeAccountManagersForCleanup.has(manager)) { - trackedAccountManagersForCleanup.delete(manager); + if (activeAccountManagersForCleanup.has(manager)) { + return; } + if (managerHasPendingSave(manager)) { + trackedAccountManagersForCleanup.add(manager); + scheduleTrackedManagerPrune(manager); + return; + } + trackedAccountManagersForCleanup.delete(manager); }); trackedManagerSettledWaits.set(manager, waitForSettle); @@ -1757,14 +1763,16 @@ export const OpenAIOAuthPlugin: Plugin = async ({ client }: PluginInput) => { accountManagerPromise = null; }; - accountManagerCleanupHook ??= async () => { + accountManagerCleanupHook ??= async (context?: { deadlineMs?: number }) => { const managersToFlush = new Set([ ...activeAccountManagersForCleanup, ...trackedAccountManagersForCleanup, ]); for (const manager of managersToFlush) { try { - await manager.flushPendingSave(); + await manager.flushPendingSave( + context?.deadlineMs !== undefined ? { deadlineMs: context.deadlineMs } : undefined, + ); } catch (error) { logWarn("[shutdown] flushPendingSave failed; disabled state may not be persisted", { error: error instanceof Error ? error.message : String(error), @@ -3609,15 +3617,18 @@ while (attempted.size < Math.max(1, accountCount)) { if (shouldEnable && target.disabledReason === "auth-failure") { const message = "This account was disabled after repeated auth failures. Run 'opencode auth login' to re-enable with fresh credentials."; + const logContext = { + accountIndex: menuResult.toggleAccountIndex + 1, + }; logWarn("[account-menu] blocked re-enable for auth-failure disabled account", { - accountId: target.accountId ?? null, + ...logContext, }); logInfo("[account-menu] prompted re-auth for auth-failure disabled account", { - accountId: target.accountId ?? null, + ...logContext, }); const toastShown = await showToast(message, "warning"); if (!toastShown) { - console.log("\nRun 'opencode auth login' to re-enable this account.\n"); + process.stdout.write("\nRun 'opencode auth login' to re-enable this account.\n\n"); } continue; } diff --git a/lib/accounts.ts b/lib/accounts.ts index 5cb5d629..65724f2d 100644 --- a/lib/accounts.ts +++ b/lib/accounts.ts @@ -73,6 +73,10 @@ export type CodexCliTokenCacheEntry = { accountId?: string; }; +type FlushPendingSaveOptions = { + deadlineMs?: number; +}; + const CODEX_CLI_ACCOUNTS_PATH = join(homedir(), ".codex", "accounts.json"); const CODEX_CLI_CACHE_TTL_MS = 5_000; let codexCliTokenCache: Map | null = null; @@ -962,14 +966,21 @@ export class AccountManager { account.enabled = false; return { ok: true, account }; } - account.enabled = enabled; if (enabled) { - delete account.disabledReason; - this.clearAccountCooldown(account); + const wasDisabled = account.enabled === false; + account.enabled = true; + if (wasDisabled) { + delete account.disabledReason; + this.clearAccountCooldown(account); + } } else if (reason) { + account.enabled = false; account.disabledReason = reason; } else if (account.disabledReason !== "auth-failure") { + account.enabled = false; account.disabledReason = "user"; + } else { + account.enabled = false; } return { ok: true, account }; } @@ -1116,11 +1127,21 @@ export class AccountManager { }, delayMs); } - async flushPendingSave(): Promise { + async flushPendingSave(options?: FlushPendingSaveOptions): Promise { const MAX_FLUSH_ITERATIONS = 20; let flushIterations = 0; + const deadlineMs = + typeof options?.deadlineMs === "number" && Number.isFinite(options.deadlineMs) + ? options.deadlineMs + : undefined; + const throwIfDeadlineExceeded = (): void => { + if (deadlineMs !== undefined && Date.now() >= deadlineMs) { + throw new Error("flushPendingSave: shutdown deadline exceeded before save state settled"); + } + }; while (true) { + throwIfDeadlineExceeded(); flushIterations += 1; if (flushIterations > MAX_FLUSH_ITERATIONS) { // This is intentionally far above realistic debounce re-arm chains; if we @@ -1145,6 +1166,7 @@ export class AccountManager { } await this.waitForSaveFinalization(pendingSaveTick); if (this.saveDebounceTimer !== null || this.pendingSave !== null) { + throwIfDeadlineExceeded(); continue; } if (pendingSaveError) { @@ -1163,6 +1185,7 @@ export class AccountManager { return; } if (this.saveDebounceTimer !== null || this.pendingSave !== null) { + throwIfDeadlineExceeded(); continue; } const flushSaveTick = this.saveFinalizationTick; @@ -1183,6 +1206,7 @@ export class AccountManager { : String(flushSaveError), }); } + throwIfDeadlineExceeded(); continue; } if (flushSaveError) { diff --git a/lib/shutdown.ts b/lib/shutdown.ts index 312f4c22..2b0ad19b 100644 --- a/lib/shutdown.ts +++ b/lib/shutdown.ts @@ -1,4 +1,8 @@ -type CleanupFn = () => void | Promise; +export type CleanupContext = { + deadlineMs?: number; +}; + +type CleanupFn = (context?: CleanupContext) => void | Promise; // Allow enough time for serialized manager flushes to finish on Windows hosts // where AV/file-indexer contention can hold the accounts file for multiple seconds. @@ -33,7 +37,7 @@ export function unregisterCleanup(fn: CleanupFn): void { } } -export function runCleanup(): Promise { +export function runCleanup(context?: CleanupContext): Promise { if (cleanupInFlight) { return cleanupInFlight; } @@ -45,7 +49,7 @@ export function runCleanup(): Promise { for (const fn of fns) { try { - await fn(); + await fn(context); } catch { // Ignore cleanup errors during shutdown } @@ -86,6 +90,7 @@ function ensureShutdownHandler(): void { return; } signalExitPending = true; + const deadlineMs = Date.now() + SIGNAL_CLEANUP_TIMEOUT_MS; let exitRequested = false; const requestExit = () => { if (exitRequested) { @@ -100,8 +105,8 @@ function ensureShutdownHandler(): void { }; let timeoutHandle: ReturnType | null = setTimeout(() => { requestExit(); - }, SIGNAL_CLEANUP_TIMEOUT_MS); - void runCleanup().finally(() => { + }, Math.max(0, deadlineMs - Date.now())); + void runCleanup({ deadlineMs }).finally(() => { try { requestExit(); } finally { diff --git a/test/accounts.test.ts b/test/accounts.test.ts index 567d70cf..798544b9 100644 --- a/test/accounts.test.ts +++ b/test/accounts.test.ts @@ -1240,6 +1240,36 @@ describe("AccountManager", () => { }); }); + it("preserves cooldown metadata when enabling an already enabled account", () => { + const now = Date.now(); + const stored = { + version: 3 as const, + activeIndex: 0, + accounts: [ + { + refreshToken: "token-1", + enabled: true, + coolingDownUntil: now + 60_000, + cooldownReason: "network-error" as const, + addedAt: now, + lastUsed: now, + }, + ], + }; + + const manager = new AccountManager(undefined, stored); + + const result = manager.trySetAccountEnabled(0, true); + expect(result).toMatchObject({ + ok: true, + account: { + enabled: true, + coolingDownUntil: now + 60_000, + cooldownReason: "network-error", + }, + }); + }); + it("ignores explicit disable-reason overrides for auth-failure disabled accounts", () => { const now = Date.now(); const stored = { @@ -2821,6 +2851,42 @@ describe("AccountManager", () => { } }); + it("stops retrying flushes once the shutdown deadline is exceeded", async () => { + vi.useFakeTimers(); + try { + vi.setSystemTime(new Date(1_000)); + const { saveAccounts } = await import("../lib/storage.js"); + const mockSaveAccounts = vi.mocked(saveAccounts); + mockSaveAccounts.mockClear(); + + const now = Date.now(); + const stored = { + version: 3 as const, + activeIndex: 0, + accounts: [ + { refreshToken: "token-1", addedAt: now, lastUsed: now }, + ], + }; + + const manager = new AccountManager(undefined, stored); + + mockSaveAccounts.mockImplementation(async () => { + vi.setSystemTime(new Date(2_000)); + manager.saveToDiskDebounced(0); + }); + + manager.saveToDiskDebounced(0); + + await expect( + manager.flushPendingSave({ deadlineMs: 1_500 }), + ).rejects.toThrow("flushPendingSave: shutdown deadline exceeded before save state settled"); + expect(mockSaveAccounts).toHaveBeenCalledTimes(1); + } finally { + vi.clearAllTimers(); + vi.useRealTimers(); + } + }); + it("rejects when saveToDisk throws during flush", async () => { vi.useFakeTimers(); try { diff --git a/test/index.test.ts b/test/index.test.ts index b8ed7368..d7a9a754 100644 --- a/test/index.test.ts +++ b/test/index.test.ts @@ -4016,6 +4016,35 @@ describe("OpenAIOAuthPlugin persistAccountPool", () => { expect(flushPendingSave).toHaveBeenCalledTimes(1); }); + it("forwards the shutdown deadline to pending account save flushers", async () => { + const accountsModule = await import("../lib/accounts.js"); + const flushPendingSave = vi.fn(async () => {}); + const manager = await accountsModule.AccountManager.loadFromDisk(); + manager.flushPendingSave = flushPendingSave; + vi.spyOn(accountsModule.AccountManager, "loadFromDisk").mockResolvedValue(manager); + + const mockClient = createMockClient(); + const { OpenAIOAuthPlugin } = await import("../index.js"); + const plugin = (await OpenAIOAuthPlugin({ + client: mockClient, + } as never)) as unknown as PluginType; + + await plugin.auth.loader( + async () => ({ + type: "oauth", + access: "access-token", + refresh: "refresh-token", + expires: Date.now() + 60_000, + }) as never, + {}, + ); + + await runCleanup({ deadlineMs: 12_345 }); + + expect(flushPendingSave).toHaveBeenCalledTimes(1); + expect(flushPendingSave).toHaveBeenCalledWith({ deadlineMs: 12_345 }); + }); + it("logs when shutdown cleanup flush fails", async () => { const accountsModule = await import("../lib/accounts.js"); const loggerModule = await import("../lib/logger.js"); @@ -4326,6 +4355,74 @@ describe("OpenAIOAuthPlugin persistAccountPool", () => { expect(managerOneHasPendingSave).toBe(false); }); + it("re-arms tracked manager prune waiters when pending saves continue after a settle notification", async () => { + const accountsModule = await import("../lib/accounts.js"); + const cliModule = await import("../lib/cli.js"); + const managerOne = await accountsModule.AccountManager.loadFromDisk(); + const flushManagerOne = vi.fn(async () => { + managerOneHasPendingSave = false; + }); + let managerOneHasPendingSave = true; + const settleResolvers: Array<() => void> = []; + managerOne.flushPendingSave = flushManagerOne; + managerOne.hasPendingSave = vi.fn(() => managerOneHasPendingSave); + managerOne.waitForPendingSaveToSettle = vi.fn( + () => + new Promise((resolve) => { + settleResolvers.push(resolve); + }), + ); + vi.spyOn(accountsModule.AccountManager, "loadFromDisk").mockResolvedValue(managerOne); + + mockStorage.accounts = [ + { + accountId: "workspace-managed", + email: "managed@example.com", + refreshToken: "refresh-managed", + addedAt: 10, + lastUsed: 10, + }, + ]; + + vi.mocked(cliModule.promptLoginMode) + .mockResolvedValueOnce({ mode: "manage", toggleAccountIndex: 0 }) + .mockResolvedValueOnce({ mode: "cancel" }); + + const mockClient = createMockClient(); + const { OpenAIOAuthPlugin } = await import("../index.js"); + const plugin = (await OpenAIOAuthPlugin({ + client: mockClient, + } as never)) as unknown as PluginType; + const autoMethod = plugin.auth.methods[0] as unknown as { + authorize: (inputs?: Record) => Promise<{ instructions: string }>; + }; + + await plugin.auth.loader( + async () => ({ + type: "oauth", + access: "access-token-1", + refresh: "refresh-token-1", + expires: Date.now() + 60_000, + }) as never, + {}, + ); + + flushManagerOne.mockClear(); + const authResult = await autoMethod.authorize(); + expect(authResult.instructions).toBe("Authentication cancelled"); + expect(managerOne.waitForPendingSaveToSettle).toHaveBeenCalledTimes(1); + + settleResolvers[0]?.(); + await Promise.resolve(); + await Promise.resolve(); + + expect(managerOne.waitForPendingSaveToSettle).toHaveBeenCalledTimes(2); + + await runCleanup(); + + expect(flushManagerOne).toHaveBeenCalledTimes(1); + }); + it("flushes tracked account managers from multiple plugin instances during shutdown cleanup", async () => { const accountsModule = await import("../lib/accounts.js"); const managerOne = await accountsModule.AccountManager.loadFromDisk(); @@ -4447,78 +4544,70 @@ describe("OpenAIOAuthPlugin persistAccountPool", () => { const cliModule = await import("../lib/cli.js"); const loggerModule = await import("../lib/logger.js"); const storageModule = await import("../lib/storage.js"); - const consoleLog = vi.spyOn(console, "log").mockImplementation(() => {}); - try { - mockStorage.accounts = [ - { - accountId: "workspace-auth-failure", - email: "blocked@example.com", - refreshToken: "refresh-blocked", - enabled: false, - disabledReason: "auth-failure", - coolingDownUntil: 30_000, - cooldownReason: "auth-failure", - addedAt: 10, - lastUsed: 10, - }, - ]; - - vi.mocked(cliModule.promptLoginMode) - .mockResolvedValueOnce({ mode: "manage", toggleAccountIndex: 0 }) - .mockResolvedValueOnce({ mode: "cancel" }); - - const mockClient = createMockClient(); - const { OpenAIOAuthPlugin } = await import("../index.js"); - const plugin = (await OpenAIOAuthPlugin({ - client: mockClient, - } as never)) as unknown as PluginType; - const autoMethod = plugin.auth.methods[0] as unknown as { - authorize: (inputs?: Record) => Promise<{ instructions: string }>; - }; - - vi.mocked(loggerModule.logWarn).mockClear(); - const authResult = await autoMethod.authorize(); - expect(authResult.instructions).toBe("Authentication cancelled"); - - expect(vi.mocked(storageModule.saveAccounts)).not.toHaveBeenCalled(); - expect(mockStorage.accounts[0]).toMatchObject({ + mockStorage.accounts = [ + { accountId: "workspace-auth-failure", + email: "blocked@example.com", + refreshToken: "refresh-blocked", enabled: false, disabledReason: "auth-failure", coolingDownUntil: 30_000, cooldownReason: "auth-failure", - }); - expect(consoleLog).not.toHaveBeenCalled(); - expect(mockClient.tui.showToast).toHaveBeenCalledWith({ - body: expect.objectContaining({ - message: - "This account was disabled after repeated auth failures. Run 'opencode auth login' to re-enable with fresh credentials.", - variant: "warning", - }), - }); - expect(vi.mocked(loggerModule.logInfo)).toHaveBeenCalledWith( - "[account-menu] prompted re-auth for auth-failure disabled account", - expect.objectContaining({ - accountId: "workspace-auth-failure", - }), - ); - expect(vi.mocked(loggerModule.logWarn)).toHaveBeenCalledWith( - "[account-menu] blocked re-enable for auth-failure disabled account", - expect.objectContaining({ - accountId: "workspace-auth-failure", - }), - ); - } finally { - consoleLog.mockRestore(); - } + addedAt: 10, + lastUsed: 10, + }, + ]; + + vi.mocked(cliModule.promptLoginMode) + .mockResolvedValueOnce({ mode: "manage", toggleAccountIndex: 0 }) + .mockResolvedValueOnce({ mode: "cancel" }); + + const mockClient = createMockClient(); + const { OpenAIOAuthPlugin } = await import("../index.js"); + const plugin = (await OpenAIOAuthPlugin({ + client: mockClient, + } as never)) as unknown as PluginType; + const autoMethod = plugin.auth.methods[0] as unknown as { + authorize: (inputs?: Record) => Promise<{ instructions: string }>; + }; + + vi.mocked(loggerModule.logWarn).mockClear(); + const authResult = await autoMethod.authorize(); + expect(authResult.instructions).toBe("Authentication cancelled"); + + expect(vi.mocked(storageModule.saveAccounts)).not.toHaveBeenCalled(); + expect(mockStorage.accounts[0]).toMatchObject({ + accountId: "workspace-auth-failure", + enabled: false, + disabledReason: "auth-failure", + coolingDownUntil: 30_000, + cooldownReason: "auth-failure", + }); + expect(mockClient.tui.showToast).toHaveBeenCalledWith({ + body: expect.objectContaining({ + message: + "This account was disabled after repeated auth failures. Run 'opencode auth login' to re-enable with fresh credentials.", + variant: "warning", + }), + }); + const infoCall = vi + .mocked(loggerModule.logInfo) + .mock.calls.find(([message]) => message === "[account-menu] prompted re-auth for auth-failure disabled account"); + const warnCall = vi + .mocked(loggerModule.logWarn) + .mock.calls.find(([message]) => message === "[account-menu] blocked re-enable for auth-failure disabled account"); + expect(infoCall?.[1]).toEqual(expect.objectContaining({ accountIndex: 1 })); + expect(warnCall?.[1]).toEqual(expect.objectContaining({ accountIndex: 1 })); + expect(infoCall?.[1]).not.toHaveProperty("accountId"); + expect(warnCall?.[1]).not.toHaveProperty("accountId"); }); it("falls back to a generic console hint when TUI toast is unavailable for auth-failure disables", async () => { const cliModule = await import("../lib/cli.js"); const loggerModule = await import("../lib/logger.js"); const storageModule = await import("../lib/storage.js"); - const consoleLog = vi.spyOn(console, "log").mockImplementation(() => {}); + const stdoutWrite = vi.spyOn(process.stdout, "write").mockReturnValue(true); try { mockStorage.accounts = [ @@ -4554,23 +4643,21 @@ describe("OpenAIOAuthPlugin persistAccountPool", () => { expect(authResult.instructions).toBe("Authentication cancelled"); expect(vi.mocked(storageModule.saveAccounts)).not.toHaveBeenCalled(); - expect(consoleLog).toHaveBeenCalledWith( - "\nRun 'opencode auth login' to re-enable this account.\n", - ); - expect(vi.mocked(loggerModule.logInfo)).toHaveBeenCalledWith( - "[account-menu] prompted re-auth for auth-failure disabled account", - expect.objectContaining({ - accountId: "workspace-auth-failure", - }), - ); - expect(vi.mocked(loggerModule.logWarn)).toHaveBeenCalledWith( - "[account-menu] blocked re-enable for auth-failure disabled account", - expect.objectContaining({ - accountId: "workspace-auth-failure", - }), + expect(stdoutWrite).toHaveBeenCalledWith( + "\nRun 'opencode auth login' to re-enable this account.\n\n", ); + const infoCall = vi + .mocked(loggerModule.logInfo) + .mock.calls.find(([message]) => message === "[account-menu] prompted re-auth for auth-failure disabled account"); + const warnCall = vi + .mocked(loggerModule.logWarn) + .mock.calls.find(([message]) => message === "[account-menu] blocked re-enable for auth-failure disabled account"); + expect(infoCall?.[1]).toEqual(expect.objectContaining({ accountIndex: 1 })); + expect(warnCall?.[1]).toEqual(expect.objectContaining({ accountIndex: 1 })); + expect(infoCall?.[1]).not.toHaveProperty("accountId"); + expect(warnCall?.[1]).not.toHaveProperty("accountId"); } finally { - consoleLog.mockRestore(); + stdoutWrite.mockRestore(); } }); }); diff --git a/test/shutdown.test.ts b/test/shutdown.test.ts index 4ee7f756..7e6f4060 100644 --- a/test/shutdown.test.ts +++ b/test/shutdown.test.ts @@ -149,6 +149,41 @@ describe("Graceful shutdown", () => { } }); + it("passes the shutdown deadline to cleanup functions on signal exit", async () => { + const capturedHandlers = new Map void>(); + + const processOnSpy = vi.spyOn(process, "on").mockImplementation((event: string | symbol, handler: (...args: unknown[]) => void) => { + capturedHandlers.set(String(event), handler); + return process; + }); + const processExitSpy = vi.spyOn(process, "exit").mockImplementation(() => undefined as never); + + vi.resetModules(); + const { registerCleanup: freshRegister, runCleanup: freshRunCleanup } = await import("../lib/shutdown.js"); + await freshRunCleanup(); + + const cleanupFn = vi.fn(async () => {}); + freshRegister(cleanupFn); + + try { + const sigtermHandler = capturedHandlers.get("SIGTERM"); + expect(sigtermHandler).toBeDefined(); + + sigtermHandler!(); + await vi.waitFor(() => { + expect(cleanupFn).toHaveBeenCalledWith( + expect.objectContaining({ + deadlineMs: expect.any(Number), + }), + ); + expect(processExitSpy).toHaveBeenCalledWith(0); + }); + } finally { + processOnSpy.mockRestore(); + processExitSpy.mockRestore(); + } + }); + it("keeps shutdown handlers installed until async cleanup completes", async () => { const capturedHandlers = new Map void>(); From feb5f290a696a3c9311931d17ba0e6d832813e31 Mon Sep 17 00:00:00 2001 From: ndycode Date: Mon, 16 Mar 2026 00:52:54 +0800 Subject: [PATCH 42/43] Harden account save shutdown tracking --- index.ts | 22 ++++---------- lib/accounts.ts | 12 +++++--- test/accounts.test.ts | 43 +++++++++++++++++++++++++- test/index.test.ts | 70 +++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 126 insertions(+), 21 deletions(-) diff --git a/index.ts b/index.ts index 1d345ba0..8ca10655 100644 --- a/index.ts +++ b/index.ts @@ -1692,14 +1692,6 @@ export const OpenAIOAuthPlugin: Plugin = async ({ client }: PluginInput) => { typeof candidate.hasPendingSave === "function" ? candidate.hasPendingSave() : true; const trackedManagerSettledWaits = new WeakMap>(); - const pruneTrackedAccountManagersForCleanup = (): void => { - for (const trackedManager of [...trackedAccountManagersForCleanup]) { - if (!managerHasPendingSave(trackedManager)) { - trackedAccountManagersForCleanup.delete(trackedManager); - } - } - }; - const scheduleTrackedManagerPrune = (manager: AccountManager): void => { if ( !managerHasPendingSave(manager) || @@ -1731,14 +1723,13 @@ export const OpenAIOAuthPlugin: Plugin = async ({ client }: PluginInput) => { }; const setCachedAccountManager = (manager: AccountManager): AccountManager => { - pruneTrackedAccountManagersForCleanup(); if (cachedAccountManager && cachedAccountManager !== manager) { activeAccountManagersForCleanup.delete(cachedAccountManager); + // Detached managers can still enqueue their first debounced save after the + // cache switches. Keep them tracked until shutdown so that late work is flushed. + trackedAccountManagersForCleanup.add(cachedAccountManager); if (managerHasPendingSave(cachedAccountManager)) { - trackedAccountManagersForCleanup.add(cachedAccountManager); scheduleTrackedManagerPrune(cachedAccountManager); - } else { - trackedAccountManagersForCleanup.delete(cachedAccountManager); } } activeAccountManagersForCleanup.add(manager); @@ -1749,14 +1740,13 @@ export const OpenAIOAuthPlugin: Plugin = async ({ client }: PluginInput) => { }; const invalidateAccountManagerCache = (): void => { - pruneTrackedAccountManagersForCleanup(); if (cachedAccountManager) { activeAccountManagersForCleanup.delete(cachedAccountManager); + // Once detached, this manager may still persist state from in-flight request + // handlers. Keep it in the shutdown tracking set until cleanup runs. + trackedAccountManagersForCleanup.add(cachedAccountManager); if (managerHasPendingSave(cachedAccountManager)) { - trackedAccountManagersForCleanup.add(cachedAccountManager); scheduleTrackedManagerPrune(cachedAccountManager); - } else { - trackedAccountManagersForCleanup.delete(cachedAccountManager); } } cachedAccountManager = null; diff --git a/lib/accounts.ts b/lib/accounts.ts index 65724f2d..ae3e6568 100644 --- a/lib/accounts.ts +++ b/lib/accounts.ts @@ -284,7 +284,7 @@ export class AccountManager { if (!changed) return; try { - await this.enqueueSave(() => this.saveToDisk()); + await this.saveToDisk(); } catch (error) { log.debug("Failed to persist Codex CLI cache hydration", { error: String(error) }); } @@ -994,7 +994,7 @@ export class AccountManager { return result.ok ? result.account : null; } - async saveToDisk(): Promise { + private async persistToDisk(): Promise { const activeIndexByFamily: Partial> = {}; for (const family of MODEL_FAMILIES) { const raw = this.currentAccountIndexByFamily[family]; @@ -1035,6 +1035,10 @@ export class AccountManager { await saveAccounts(storage); } + async saveToDisk(): Promise { + return this.enqueueSave(() => this.persistToDisk()); + } + private enqueueSave(saveOperation: () => Promise): Promise { const previousSave = this.pendingSave; const nextSave = (async () => { @@ -1118,7 +1122,7 @@ export class AccountManager { this.saveDebounceTimer = null; const doSave = async () => { try { - await this.enqueueSave(() => this.saveToDisk()); + await this.saveToDisk(); } catch (error) { log.warn("Debounced save failed", { error: error instanceof Error ? error.message : String(error) }); } @@ -1189,7 +1193,7 @@ export class AccountManager { continue; } const flushSaveTick = this.saveFinalizationTick; - const flushSave = this.enqueueSave(() => this.saveToDisk()); + const flushSave = this.saveToDisk(); let flushSaveError: unknown; try { await flushSave; diff --git a/test/accounts.test.ts b/test/accounts.test.ts index 798544b9..f8689190 100644 --- a/test/accounts.test.ts +++ b/test/accounts.test.ts @@ -2409,11 +2409,52 @@ describe("AccountManager", () => { const savePromise = manager.saveToDisk(); queueMicrotask(() => resolveInFlight!()); - + await manager.flushPendingSave(); await savePromise; }); + it("tracks direct saveToDisk work so flushPendingSave waits for it", async () => { + const { saveAccounts } = await import("../lib/storage.js"); + const mockSaveAccounts = vi.mocked(saveAccounts); + mockSaveAccounts.mockClear(); + + let resolveInFlight: (() => void) | undefined; + const inFlightSave = new Promise((resolve) => { + resolveInFlight = resolve; + }); + mockSaveAccounts.mockImplementation(() => inFlightSave); + + const now = Date.now(); + const stored = { + version: 3 as const, + activeIndex: 0, + accounts: [ + { refreshToken: "token-1", addedAt: now, lastUsed: now }, + ], + }; + + const manager = new AccountManager(undefined, stored); + const savePromise = manager.saveToDisk(); + const flushPromise = manager.flushPendingSave(); + + expect(manager.hasPendingSave()).toBe(true); + + let flushSettled = false; + void flushPromise.then(() => { + flushSettled = true; + }); + await Promise.resolve(); + expect(flushSettled).toBe(false); + + resolveInFlight?.(); + await savePromise; + await flushPromise; + + expect(manager.hasPendingSave()).toBe(false); + expect(mockSaveAccounts).toHaveBeenCalledTimes(1); + }); + it("waits for in-flight save before flushing a pending debounce", async () => { vi.useFakeTimers(); try { diff --git a/test/index.test.ts b/test/index.test.ts index d7a9a754..89b75809 100644 --- a/test/index.test.ts +++ b/test/index.test.ts @@ -4205,6 +4205,76 @@ describe("OpenAIOAuthPlugin persistAccountPool", () => { expect(flushManagerTwo).toHaveBeenCalledTimes(1); }); + it("keeps idle detached managers tracked until shutdown in case they enqueue a later save", async () => { + const accountsModule = await import("../lib/accounts.js"); + const cliModule = await import("../lib/cli.js"); + const managerOne = await accountsModule.AccountManager.loadFromDisk(); + const managerTwo = await accountsModule.AccountManager.loadFromDisk(); + const flushManagerOne = vi.fn(async () => {}); + const flushManagerTwo = vi.fn(async () => {}); + let managerOneHasPendingSave = false; + managerOne.flushPendingSave = flushManagerOne; + managerTwo.flushPendingSave = flushManagerTwo; + managerOne.hasPendingSave = vi.fn(() => managerOneHasPendingSave); + managerTwo.hasPendingSave = vi.fn(() => false); + vi.spyOn(accountsModule.AccountManager, "loadFromDisk") + .mockResolvedValueOnce(managerOne) + .mockResolvedValue(managerTwo); + + mockStorage.accounts = [ + { + accountId: "workspace-managed", + email: "managed@example.com", + refreshToken: "refresh-managed", + addedAt: 10, + lastUsed: 10, + }, + ]; + + vi.mocked(cliModule.promptLoginMode) + .mockResolvedValueOnce({ mode: "manage", toggleAccountIndex: 0 }) + .mockResolvedValueOnce({ mode: "cancel" }); + + const mockClient = createMockClient(); + const { OpenAIOAuthPlugin } = await import("../index.js"); + const plugin = (await OpenAIOAuthPlugin({ + client: mockClient, + } as never)) as unknown as PluginType; + const autoMethod = plugin.auth.methods[0] as unknown as { + authorize: (inputs?: Record) => Promise<{ instructions: string }>; + }; + + await plugin.auth.loader( + async () => ({ + type: "oauth", + access: "access-token-1", + refresh: "refresh-token-1", + expires: Date.now() + 60_000, + }) as never, + {}, + ); + + flushManagerOne.mockClear(); + const authResult = await autoMethod.authorize(); + expect(authResult.instructions).toBe("Authentication cancelled"); + + await plugin.auth.loader( + async () => ({ + type: "oauth", + access: "access-token-2", + refresh: "refresh-token-2", + expires: Date.now() + 60_000, + }) as never, + {}, + ); + + managerOneHasPendingSave = true; + await runCleanup(); + + expect(flushManagerOne).toHaveBeenCalledTimes(1); + expect(flushManagerTwo).toHaveBeenCalledTimes(1); + }); + it("prunes tracked invalidated managers once their pending saves settle", async () => { const accountsModule = await import("../lib/accounts.js"); const cliModule = await import("../lib/cli.js"); From b979b685ffd300be8329125b946813ad5fbf99ac Mon Sep 17 00:00:00 2001 From: ndycode Date: Mon, 16 Mar 2026 02:21:53 +0800 Subject: [PATCH 43/43] Fix debounced save retry tracking --- index.ts | 1 + lib/accounts.ts | 17 ++++++++--- test/accounts.test.ts | 70 +++++++++++++++++++++++++++++++++++++++++++ test/index.test.ts | 47 +++++++++++++++++++++++++++++ 4 files changed, 131 insertions(+), 4 deletions(-) diff --git a/index.ts b/index.ts index 8ca10655..900621ec 100644 --- a/index.ts +++ b/index.ts @@ -3867,6 +3867,7 @@ while (attempted.size < Math.max(1, accountCount)) { await persistAccountPool(selection.variantsForPersistence, false, { reviveMatchingDisabledAccounts: true, }); + invalidateAccountManagerCache(); } catch (err) { const storagePath = getStoragePath(); const errorCode = (err as NodeJS.ErrnoException)?.code || "UNKNOWN"; diff --git a/lib/accounts.ts b/lib/accounts.ts index ae3e6568..eb7c67a6 100644 --- a/lib/accounts.ts +++ b/lib/accounts.ts @@ -226,6 +226,7 @@ export class AccountManager { private lastToastTime = 0; private saveDebounceTimer: ReturnType | null = null; private pendingSave: Promise | null = null; + private saveRetryNeeded = false; private authFailuresByRefreshToken: Map = new Map(); private pendingSaveSettledWaiters = new Set<() => void>(); private saveFinalizationTick = 0; @@ -1051,7 +1052,13 @@ export class AccountManager { }); } } - await saveOperation(); + try { + await saveOperation(); + this.saveRetryNeeded = false; + } catch (error) { + this.saveRetryNeeded = true; + throw error; + } })().finally(() => { if (this.pendingSave === nextSave) { this.pendingSave = null; @@ -1115,6 +1122,7 @@ export class AccountManager { } saveToDiskDebounced(delayMs = 500): void { + this.saveRetryNeeded = true; if (this.saveDebounceTimer) { clearTimeout(this.saveDebounceTimer); } @@ -1173,11 +1181,12 @@ export class AccountManager { throwIfDeadlineExceeded(); continue; } + const shouldRetryAfterPendingSave = hadDebouncedSave || this.saveRetryNeeded; if (pendingSaveError) { - if (!hadDebouncedSave) { + if (!shouldRetryAfterPendingSave) { throw pendingSaveError; } - log.warn("flushPendingSave: ignoring pending-save failure to flush newer state", { + log.warn("flushPendingSave: retrying after save failure to flush latest state", { error: pendingSaveError instanceof Error ? pendingSaveError.message @@ -1185,7 +1194,7 @@ export class AccountManager { }); } } - if (!hadDebouncedSave) { + if (!hadDebouncedSave && !this.saveRetryNeeded) { return; } if (this.saveDebounceTimer !== null || this.pendingSave !== null) { diff --git a/test/accounts.test.ts b/test/accounts.test.ts index f8689190..57135c9c 100644 --- a/test/accounts.test.ts +++ b/test/accounts.test.ts @@ -2530,6 +2530,76 @@ describe("AccountManager", () => { } }); + it("retries a failed debounced save during flush even after pending state clears", async () => { + vi.useFakeTimers(); + try { + const { saveAccounts } = await import("../lib/storage.js"); + const mockSaveAccounts = vi.mocked(saveAccounts); + mockSaveAccounts.mockClear(); + + const savedSnapshots: Array< + Array<{ + refreshToken?: string; + enabled?: false; + disabledReason?: string; + }> + > = []; + + mockSaveAccounts.mockImplementationOnce(async (storage) => { + savedSnapshots.push( + storage.accounts.map((account) => ({ + refreshToken: account.refreshToken, + enabled: account.enabled, + disabledReason: account.disabledReason, + })), + ); + throw Object.assign(new Error("EBUSY: file in use"), { code: "EBUSY" }); + }); + mockSaveAccounts.mockImplementationOnce(async (storage) => { + savedSnapshots.push( + storage.accounts.map((account) => ({ + refreshToken: account.refreshToken, + enabled: account.enabled, + disabledReason: account.disabledReason, + })), + ); + }); + + const now = Date.now(); + const stored = { + version: 3 as const, + activeIndex: 0, + accounts: [ + { refreshToken: "token-1", addedAt: now, lastUsed: now }, + ], + }; + + const manager = new AccountManager(undefined, stored); + const account = manager.getAccountsSnapshot()[0]!; + + manager.disableAccountsWithSameRefreshToken(account); + manager.saveToDiskDebounced(0); + await vi.advanceTimersByTimeAsync(0); + await drainMicrotasks(); + + expect(mockSaveAccounts).toHaveBeenCalledTimes(1); + expect(manager.hasPendingSave()).toBe(false); + + await manager.flushPendingSave(); + + expect(mockSaveAccounts).toHaveBeenCalledTimes(2); + expect(savedSnapshots[1]).toEqual([ + expect.objectContaining({ + refreshToken: "token-1", + enabled: false, + disabledReason: "auth-failure", + }), + ]); + } finally { + vi.useRealTimers(); + } + }); + it("drains saves queued during flush without overlapping writes", async () => { vi.useFakeTimers(); try { diff --git a/test/index.test.ts b/test/index.test.ts index 89b75809..e3308cdc 100644 --- a/test/index.test.ts +++ b/test/index.test.ts @@ -532,6 +532,53 @@ describe("OpenAIOAuthPlugin", () => { expect(result.reason).toBe("invalid_response"); expect(vi.mocked(authModule.exchangeAuthorizationCode)).not.toHaveBeenCalled(); }); + + it("invalidates cached account manager after manual OAuth persistence", async () => { + const { AccountManager } = await import("../lib/accounts.js"); + const authModule = await import("../lib/auth/auth.js"); + const accountsModule = await import("../lib/accounts.js"); + const loadFromDiskSpy = vi.spyOn(AccountManager, "loadFromDisk"); + + mockStorage.accounts = [{ refreshToken: "existing-refresh", email: "existing@example.com" }]; + + const getAuth = async () => ({ + type: "oauth" as const, + access: "existing-access", + refresh: "existing-refresh", + expires: Date.now() + 60_000, + multiAccount: true, + }); + + await plugin.auth.loader(getAuth, { options: {}, models: {} }); + loadFromDiskSpy.mockClear(); + + vi.mocked(authModule.exchangeAuthorizationCode).mockResolvedValueOnce({ + type: "success", + access: "manual-access", + refresh: "manual-refresh", + expires: Date.now() + 60_000, + idToken: "manual-id-token", + }); + vi.mocked(accountsModule.getAccountIdCandidates).mockReturnValueOnce([]); + + const manualMethod = plugin.auth.methods[1] as unknown as { + authorize: () => Promise<{ + callback: (input: string) => Promise<{ type: string }>; + }>; + }; + + const flow = await manualMethod.authorize(); + const result = await flow.callback( + "http://127.0.0.1:1455/auth/callback?code=manual-code&state=test-state", + ); + + expect(result.type).toBe("success"); + expect(mockStorage.accounts).toHaveLength(2); + + await plugin.auth.loader(getAuth, { options: {}, models: {} }); + + expect(loadFromDiskSpy).toHaveBeenCalledTimes(1); + }); }); describe("event handler", () => {