From 2013dbef013cc9ab9250f190ebbadf52608be82a Mon Sep 17 00:00:00 2001 From: ndycode Date: Wed, 4 Mar 2026 05:47:49 +0800 Subject: [PATCH 01/13] refactor: extract shared account view helpers Introduce a shared account-view helper module and reuse it across the plugin entrypoint and CLI manager to remove duplicated formatting logic while preserving behavior. Add unit/regression tests to lock formatting and active-index fallback behavior. Co-authored-by: Codex --- index.ts | 54 ++------------------ lib/accounts/account-view.ts | 57 +++++++++++++++++++++ lib/codex-manager.ts | 49 ++---------------- test/account-view.test.ts | 93 ++++++++++++++++++++++++++++++++++ test/codex-manager-cli.test.ts | 26 ++++++++++ test/index.test.ts | 9 +++- 6 files changed, 193 insertions(+), 95 deletions(-) create mode 100644 lib/accounts/account-view.ts create mode 100644 test/account-view.test.ts diff --git a/index.ts b/index.ts index 7db8808..b755f40 100644 --- a/index.ts +++ b/index.ts @@ -117,6 +117,11 @@ import { lookupCodexCliTokensByEmail, isCodexCliSyncEnabled, } from "./lib/accounts.js"; +import { + resolveActiveIndex, + getRateLimitResetTimeForFamily, + formatRateLimitEntry, +} from "./lib/accounts/account-view.js"; import { getStoragePath, loadAccounts, @@ -676,21 +681,6 @@ export const OpenAIOAuthPlugin: Plugin = async ({ client }: PluginInput) => { } }; - const resolveActiveIndex = ( - storage: { - activeIndex: number; - activeIndexByFamily?: Partial>; - accounts: unknown[]; - }, - family: ModelFamily = "codex", - ): number => { - const total = storage.accounts.length; - if (total === 0) return 0; - const rawCandidate = storage.activeIndexByFamily?.[family] ?? storage.activeIndex; - const raw = Number.isFinite(rawCandidate) ? rawCandidate : 0; - return Math.max(0, Math.min(raw, total - 1)); - }; - const hydrateEmails = async ( storage: AccountStorageV3 | null, ): Promise => { @@ -755,40 +745,6 @@ export const OpenAIOAuthPlugin: Plugin = async ({ client }: PluginInput) => { return storage; }; - const getRateLimitResetTimeForFamily = ( - account: { rateLimitResetTimes?: Record }, - now: number, - family: ModelFamily, - ): number | null => { - const times = account.rateLimitResetTimes; - if (!times) return null; - - let minReset: number | null = null; - const prefix = `${family}:`; - for (const [key, value] of Object.entries(times)) { - if (typeof value !== "number") continue; - if (value <= now) continue; - if (key !== family && !key.startsWith(prefix)) continue; - if (minReset === null || value < minReset) { - minReset = value; - } - } - - return minReset; - }; - - const formatRateLimitEntry = ( - account: { rateLimitResetTimes?: Record }, - now: number, - family: ModelFamily = "codex", - ): string | null => { - const resetAt = getRateLimitResetTimeForFamily(account, now, family); - if (typeof resetAt !== "number") return null; - const remaining = resetAt - now; - if (remaining <= 0) return null; - return `resets in ${formatWaitTime(remaining)}`; - }; - const applyUiRuntimeFromConfig = ( pluginConfig: ReturnType, ): UiRuntimeOptions => { diff --git a/lib/accounts/account-view.ts b/lib/accounts/account-view.ts new file mode 100644 index 0000000..576711d --- /dev/null +++ b/lib/accounts/account-view.ts @@ -0,0 +1,57 @@ +import type { ModelFamily } from "../prompts/codex.js"; +import { formatWaitTime } from "./rate-limits.js"; + +export interface ActiveAccountStorageView { + activeIndex: number; + activeIndexByFamily?: Partial>; + accounts: unknown[]; +} + +export interface AccountRateLimitView { + rateLimitResetTimes?: Record; +} + +export function resolveActiveIndex( + storage: ActiveAccountStorageView, + family: ModelFamily = "codex", +): number { + const total = storage.accounts.length; + if (total === 0) return 0; + const rawCandidate = storage.activeIndexByFamily?.[family] ?? storage.activeIndex; + const raw = Number.isFinite(rawCandidate) ? rawCandidate : 0; + return Math.max(0, Math.min(raw, total - 1)); +} + +export function getRateLimitResetTimeForFamily( + account: AccountRateLimitView, + now: number, + family: ModelFamily, +): number | null { + const times = account.rateLimitResetTimes; + if (!times) return null; + + let minReset: number | null = null; + const prefix = `${family}:`; + for (const [key, value] of Object.entries(times)) { + if (typeof value !== "number") continue; + if (value <= now) continue; + if (key !== family && !key.startsWith(prefix)) continue; + if (minReset === null || value < minReset) { + minReset = value; + } + } + + return minReset; +} + +export function formatRateLimitEntry( + account: AccountRateLimitView, + now: number, + family: ModelFamily = "codex", +): string | null { + const resetAt = getRateLimitResetTimeForFamily(account, now, family); + if (typeof resetAt !== "number") return null; + const remaining = resetAt - now; + if (remaining <= 0) return null; + return `resets in ${formatWaitTime(remaining)}`; +} diff --git a/lib/codex-manager.ts b/lib/codex-manager.ts index 794eb7c..cbf0210 100644 --- a/lib/codex-manager.ts +++ b/lib/codex-manager.ts @@ -22,6 +22,10 @@ import { sanitizeEmail, selectBestAccountCandidate, } from "./accounts.js"; +import { + resolveActiveIndex, + formatRateLimitEntry, +} from "./accounts/account-view.js"; import { ACCOUNT_LIMITS } from "./constants.js"; import { loadDashboardDisplaySettings, @@ -362,51 +366,6 @@ function runFeaturesReport(): number { return 0; } -function resolveActiveIndex( - storage: AccountStorageV3, - family: ModelFamily = "codex", -): number { - const total = storage.accounts.length; - if (total === 0) return 0; - const rawCandidate = storage.activeIndexByFamily?.[family] ?? storage.activeIndex; - const raw = Number.isFinite(rawCandidate) ? rawCandidate : 0; - return Math.max(0, Math.min(raw, total - 1)); -} - -function getRateLimitResetTimeForFamily( - account: { rateLimitResetTimes?: Record }, - now: number, - family: ModelFamily, -): number | null { - const times = account.rateLimitResetTimes; - if (!times) return null; - - let minReset: number | null = null; - const prefix = `${family}:`; - for (const [key, value] of Object.entries(times)) { - if (typeof value !== "number") continue; - if (value <= now) continue; - if (key !== family && !key.startsWith(prefix)) continue; - if (minReset === null || value < minReset) { - minReset = value; - } - } - - return minReset; -} - -function formatRateLimitEntry( - account: { rateLimitResetTimes?: Record }, - now: number, - family: ModelFamily = "codex", -): string | null { - const resetAt = getRateLimitResetTimeForFamily(account, now, family); - if (typeof resetAt !== "number") return null; - const remaining = resetAt - now; - if (remaining <= 0) return null; - return `resets in ${formatWaitTime(remaining)}`; -} - function normalizeQuotaEmail(email: string | undefined): string | null { const normalized = sanitizeEmail(email); return normalized && normalized.length > 0 ? normalized : null; diff --git a/test/account-view.test.ts b/test/account-view.test.ts new file mode 100644 index 0000000..ce554f5 --- /dev/null +++ b/test/account-view.test.ts @@ -0,0 +1,93 @@ +import { describe, expect, it } from "vitest"; +import { + resolveActiveIndex, + getRateLimitResetTimeForFamily, + formatRateLimitEntry, +} from "../lib/accounts/account-view.js"; + +describe("account-view helpers", () => { + it("resolves active index from family mapping and clamps to bounds", () => { + expect( + resolveActiveIndex({ + activeIndex: 9, + activeIndexByFamily: { codex: 4 }, + accounts: [{}, {}], + }), + ).toBe(1); + }); + + it("falls back to storage active index and normalizes non-finite candidates", () => { + expect( + resolveActiveIndex({ + activeIndex: Number.NaN, + activeIndexByFamily: { codex: Number.NaN }, + accounts: [{}, {}, {}], + }), + ).toBe(0); + expect( + resolveActiveIndex({ + activeIndex: 2, + accounts: [{}, {}, {}], + }), + ).toBe(2); + }); + + it("returns earliest future reset for matching family keys", () => { + const now = 1_000_000; + expect( + getRateLimitResetTimeForFamily( + { + rateLimitResetTimes: { + codex: now + 90_000, + "codex:gpt-5.2": now + 30_000, + "gpt-5.1": now + 10_000, + "codex:bad": undefined, + }, + }, + now, + "codex", + ), + ).toBe(now + 30_000); + }); + + it("returns null for missing or expired family reset state", () => { + const now = 5_000; + expect(getRateLimitResetTimeForFamily({}, now, "codex")).toBeNull(); + expect( + getRateLimitResetTimeForFamily( + { + rateLimitResetTimes: { + codex: now, + "codex:gpt-5.2": now - 1, + }, + }, + now, + "codex", + ), + ).toBeNull(); + }); + + it("formats wait labels only when a positive reset window exists", () => { + const now = 42_000; + expect( + formatRateLimitEntry( + { + rateLimitResetTimes: { + codex: now + 90_000, + }, + }, + now, + ), + ).toBe("resets in 1m 30s"); + expect( + formatRateLimitEntry( + { + rateLimitResetTimes: { + codex: now, + }, + }, + now, + ), + ).toBeNull(); + }); +}); diff --git a/test/codex-manager-cli.test.ts b/test/codex-manager-cli.test.ts index 27261cd..46af34d 100644 --- a/test/codex-manager-cli.test.ts +++ b/test/codex-manager-cli.test.ts @@ -499,6 +499,32 @@ describe("codex manager cli commands", () => { expect(logSpy.mock.calls.some((call) => String(call[0]).includes("live session OK"))).toBe(true); }); + it("shows rate-limit reset details in auth status output", async () => { + const now = Date.now(); + loadAccountsMock.mockResolvedValueOnce({ + version: 3, + activeIndex: 0, + activeIndexByFamily: { codex: 0 }, + accounts: [ + { + email: "rate-limited@example.com", + refreshToken: "refresh-rate-limited", + addedAt: now - 1_000, + lastUsed: now - 1_000, + rateLimitResetTimes: { + codex: now + 120_000, + }, + }, + ], + }); + const logSpy = vi.spyOn(console, "log").mockImplementation(() => {}); + const { runCodexMultiAuthCli } = await import("../lib/codex-manager.js"); + + const exitCode = await runCodexMultiAuthCli(["auth", "status"]); + expect(exitCode).toBe(0); + expect(logSpy.mock.calls.some((call) => String(call[0]).includes("rate-limited"))).toBe(true); + }); + it("runs fix apply mode and returns a switch recommendation", async () => { const now = Date.now(); loadAccountsMock.mockResolvedValueOnce({ diff --git a/test/index.test.ts b/test/index.test.ts index d6d9549..5593107 100644 --- a/test/index.test.ts +++ b/test/index.test.ts @@ -731,13 +731,20 @@ describe("OpenAIOAuthPlugin", () => { }); it("shows detailed status for accounts", async () => { + const now = Date.now(); mockStorage.accounts = [ - { refreshToken: "r1", email: "user@example.com", lastUsed: Date.now() - 60000 }, + { + refreshToken: "r1", + email: "user@example.com", + lastUsed: now - 60000, + rateLimitResetTimes: { codex: now + 120000 }, + }, ]; mockStorage.activeIndexByFamily = { codex: 0 }; const result = await plugin.tool["codex-status"].execute(); expect(result).toContain("Account Status"); expect(result).toContain("Active index by model family"); + expect(result).toContain("resets in"); }); }); From 00dba6efcc7e8976a2903eade39ee87825cb18d5 Mon Sep 17 00:00:00 2001 From: ndycode Date: Wed, 4 Mar 2026 06:00:36 +0800 Subject: [PATCH 02/13] refactor: centralize active-index family mutations Add shared helpers for per-family active-index map creation and bulk updates, then reuse them across plugin, CLI manager, and codex-cli sync flows to reduce duplicated state-mutation logic. Extend tests to lock switch and selection behavior for all model-family indexes. Co-authored-by: Codex --- index.ts | 13 ++------- lib/accounts/active-index.ts | 29 +++++++++++++++++++ lib/codex-cli/sync.ts | 7 ++--- lib/codex-manager.ts | 28 ++++++------------- test/active-index.test.ts | 54 ++++++++++++++++++++++++++++++++++++ test/index.test.ts | 10 +++++++ 6 files changed, 107 insertions(+), 34 deletions(-) create mode 100644 lib/accounts/active-index.ts create mode 100644 test/active-index.test.ts diff --git a/index.ts b/index.ts index b755f40..5ac326f 100644 --- a/index.ts +++ b/index.ts @@ -122,6 +122,7 @@ import { getRateLimitResetTimeForFamily, formatRateLimitEntry, } from "./lib/accounts/account-view.js"; +import { setActiveIndexForAllFamilies } from "./lib/accounts/active-index.js"; import { getStoragePath, loadAccounts, @@ -955,11 +956,7 @@ export const OpenAIOAuthPlugin: Plugin = async ({ client }: PluginInput) => { account.lastUsed = now; account.lastSwitchReason = "rotation"; } - storage.activeIndex = index; - storage.activeIndexByFamily = storage.activeIndexByFamily ?? {}; - for (const family of MODEL_FAMILIES) { - storage.activeIndexByFamily[family] = index; - } + setActiveIndexForAllFamilies(storage, index); await saveAccounts(storage); if (cachedAccountManager) { @@ -3522,11 +3519,7 @@ while (attempted.size < Math.max(1, accountCount)) { account.lastSwitchReason = "rotation"; } - storage.activeIndex = targetIndex; - storage.activeIndexByFamily = storage.activeIndexByFamily ?? {}; - for (const family of MODEL_FAMILIES) { - storage.activeIndexByFamily[family] = targetIndex; - } + setActiveIndexForAllFamilies(storage, targetIndex); try { await saveAccounts(storage); } catch (saveError) { diff --git a/lib/accounts/active-index.ts b/lib/accounts/active-index.ts new file mode 100644 index 0000000..f95f19e --- /dev/null +++ b/lib/accounts/active-index.ts @@ -0,0 +1,29 @@ +import { MODEL_FAMILIES, type ModelFamily } from "../prompts/codex.js"; + +export interface ActiveIndexFamilyStorage { + activeIndex: number; + activeIndexByFamily?: Partial>; +} + +export function createActiveIndexByFamily( + index: number, + families: readonly ModelFamily[] = MODEL_FAMILIES, +): Partial> { + const byFamily: Partial> = {}; + for (const family of families) { + byFamily[family] = index; + } + return byFamily; +} + +export function setActiveIndexForAllFamilies( + storage: ActiveIndexFamilyStorage, + index: number, + families: readonly ModelFamily[] = MODEL_FAMILIES, +): void { + storage.activeIndex = index; + storage.activeIndexByFamily = storage.activeIndexByFamily ?? {}; + for (const family of families) { + storage.activeIndexByFamily[family] = index; + } +} diff --git a/lib/codex-cli/sync.ts b/lib/codex-cli/sync.ts index 38e6605..83d7f54 100644 --- a/lib/codex-cli/sync.ts +++ b/lib/codex-cli/sync.ts @@ -4,6 +4,7 @@ import { type AccountStorageV3, } from "../storage.js"; import { MODEL_FAMILIES, type ModelFamily } from "../prompts/codex.js"; +import { setActiveIndexForAllFamilies } from "../accounts/active-index.js"; import { createLogger } from "../logger.js"; import { loadCodexCliState, type CodexCliAccountSnapshot } from "./state.js"; import { @@ -162,11 +163,7 @@ function writeFamilyIndexes( storage: AccountStorageV3, index: number, ): void { - storage.activeIndex = index; - storage.activeIndexByFamily = storage.activeIndexByFamily ?? {}; - for (const family of MODEL_FAMILIES) { - storage.activeIndexByFamily[family] = index; - } + setActiveIndexForAllFamilies(storage, index); } /** diff --git a/lib/codex-manager.ts b/lib/codex-manager.ts index cbf0210..d289ce0 100644 --- a/lib/codex-manager.ts +++ b/lib/codex-manager.ts @@ -26,6 +26,10 @@ import { resolveActiveIndex, formatRateLimitEntry, } from "./accounts/account-view.js"; +import { + createActiveIndexByFamily, + setActiveIndexForAllFamilies, +} from "./accounts/active-index.js"; import { ACCOUNT_LIMITS } from "./constants.js"; import { loadDashboardDisplaySettings, @@ -40,7 +44,7 @@ import { summarizeForecast, type ForecastAccountResult, } from "./forecast.js"; -import { MODEL_FAMILIES, type ModelFamily } from "./prompts/codex.js"; +import { MODEL_FAMILIES } from "./prompts/codex.js"; import { fetchCodexQuotaSnapshot, formatQuotaSnapshotLine, @@ -1331,10 +1335,7 @@ async function persistAccountPool( : selectedAccountIndex === null ? fallbackActiveIndex : Math.max(0, Math.min(selectedAccountIndex, accounts.length - 1)); - const activeIndexByFamily: Partial> = {}; - for (const family of MODEL_FAMILIES) { - activeIndexByFamily[family] = nextActiveIndex; - } + const activeIndexByFamily = createActiveIndexByFamily(nextActiveIndex); await saveAccounts({ version: 3, @@ -2345,10 +2346,7 @@ interface VerifyFlaggedReport { } function createEmptyAccountStorage(): AccountStorageV3 { - const activeIndexByFamily: Partial> = {}; - for (const family of MODEL_FAMILIES) { - activeIndexByFamily[family] = 0; - } + const activeIndexByFamily = createActiveIndexByFamily(0); return { version: 3, accounts: [], @@ -3631,11 +3629,7 @@ async function handleManageAction( const idx = menuResult.deleteAccountIndex; if (idx >= 0 && idx < storage.accounts.length) { storage.accounts.splice(idx, 1); - storage.activeIndex = 0; - storage.activeIndexByFamily = {}; - for (const family of MODEL_FAMILIES) { - storage.activeIndexByFamily[family] = 0; - } + setActiveIndexForAllFamilies(storage, 0); await saveAccounts(storage); console.log(`Deleted account ${idx + 1}.`); } @@ -3856,11 +3850,7 @@ async function runSwitch(args: string[]): Promise { return 1; } - storage.activeIndex = targetIndex; - storage.activeIndexByFamily = storage.activeIndexByFamily ?? {}; - for (const family of MODEL_FAMILIES) { - storage.activeIndexByFamily[family] = targetIndex; - } + setActiveIndexForAllFamilies(storage, targetIndex); const wasDisabled = account.enabled === false; if (wasDisabled) { account.enabled = true; diff --git a/test/active-index.test.ts b/test/active-index.test.ts new file mode 100644 index 0000000..bc7768d --- /dev/null +++ b/test/active-index.test.ts @@ -0,0 +1,54 @@ +import { describe, expect, it } from "vitest"; +import { MODEL_FAMILIES } from "../lib/prompts/codex.js"; +import { + createActiveIndexByFamily, + setActiveIndexForAllFamilies, +} from "../lib/accounts/active-index.js"; + +describe("active-index helpers", () => { + it("creates a per-family active index map", () => { + expect(createActiveIndexByFamily(2)).toEqual({ + "gpt-5-codex": 2, + "codex-max": 2, + codex: 2, + "gpt-5.2": 2, + "gpt-5.1": 2, + }); + }); + + it("sets active index across all model families", () => { + const storage: { + activeIndex: number; + activeIndexByFamily?: Partial>; + } = { + activeIndex: 0, + }; + + setActiveIndexForAllFamilies(storage, 3); + + expect(storage.activeIndex).toBe(3); + for (const family of MODEL_FAMILIES) { + expect(storage.activeIndexByFamily?.[family]).toBe(3); + } + }); + + it("preserves unrelated keys when reusing existing family map objects", () => { + const storage: { + activeIndex: number; + activeIndexByFamily?: Record; + } = { + activeIndex: 1, + activeIndexByFamily: { + legacy: 99, + codex: 1, + }, + }; + + setActiveIndexForAllFamilies(storage, 0); + + expect(storage.activeIndexByFamily?.legacy).toBe(99); + for (const family of MODEL_FAMILIES) { + expect(storage.activeIndexByFamily?.[family]).toBe(0); + } + }); +}); diff --git a/test/index.test.ts b/test/index.test.ts index 5593107..76f35bf 100644 --- a/test/index.test.ts +++ b/test/index.test.ts @@ -696,6 +696,11 @@ describe("OpenAIOAuthPlugin", () => { { refreshToken: "r2", email: "user2@example.com" }, ]; const result = await plugin.tool["codex-switch"].execute({ index: 2 }); + const { MODEL_FAMILIES } = await import("../lib/prompts/codex.js"); + expect(mockStorage.activeIndex).toBe(1); + for (const family of MODEL_FAMILIES) { + expect(mockStorage.activeIndexByFamily[family]).toBe(1); + } expect(result).toContain("Switched to account"); }); @@ -1906,6 +1911,11 @@ describe("OpenAIOAuthPlugin event handler edge cases", () => { await plugin.event({ event: { type: "account.select", properties: { accountIndex: 1 } }, }); + const { MODEL_FAMILIES } = await import("../lib/prompts/codex.js"); + expect(mockStorage.activeIndex).toBe(1); + for (const family of MODEL_FAMILIES) { + expect(mockStorage.activeIndexByFamily[family]).toBe(1); + } }); it("reloads account manager from disk when handling account.select", async () => { From 0d96ab3a2535870d18cfd4c3afeace1b027b02f1 Mon Sep 17 00:00:00 2001 From: ndycode Date: Wed, 4 Mar 2026 06:26:50 +0800 Subject: [PATCH 03/13] refactor: unify active-index normalization flow Extract active-index normalization into a shared helper and reuse it in plugin account-flow clamping, codex-cli sync, and doctor normalization paths. Add focused tests covering bounded normalization and empty-account map handling to preserve behavior across both clear and fill modes. Co-authored-by: Codex --- index.ts | 22 ++++--------- lib/accounts/active-index.ts | 53 +++++++++++++++++++++++++++++ lib/codex-cli/sync.ts | 21 ++++-------- lib/codex-manager.ts | 22 ++----------- test/active-index.test.ts | 64 ++++++++++++++++++++++++++++++++++++ test/index.test.ts | 4 +++ 6 files changed, 136 insertions(+), 50 deletions(-) diff --git a/index.ts b/index.ts index 5ac326f..e0c9aaf 100644 --- a/index.ts +++ b/index.ts @@ -122,7 +122,10 @@ import { getRateLimitResetTimeForFamily, formatRateLimitEntry, } from "./lib/accounts/account-view.js"; -import { setActiveIndexForAllFamilies } from "./lib/accounts/active-index.js"; +import { + setActiveIndexForAllFamilies, + normalizeActiveIndexByFamily, +} from "./lib/accounts/active-index.js"; import { getStoragePath, loadAccounts, @@ -2385,20 +2388,9 @@ while (attempted.size < Math.max(1, accountCount)) { let refreshAccountIndex: number | undefined; const clampActiveIndices = (storage: AccountStorageV3): void => { - const count = storage.accounts.length; - if (count === 0) { - storage.activeIndex = 0; - storage.activeIndexByFamily = {}; - return; - } - storage.activeIndex = Math.max(0, Math.min(storage.activeIndex, count - 1)); - storage.activeIndexByFamily = storage.activeIndexByFamily ?? {}; - for (const family of MODEL_FAMILIES) { - const raw = storage.activeIndexByFamily[family]; - const candidate = - typeof raw === "number" && Number.isFinite(raw) ? raw : storage.activeIndex; - storage.activeIndexByFamily[family] = Math.max(0, Math.min(candidate, count - 1)); - } + normalizeActiveIndexByFamily(storage, storage.accounts.length, { + clearFamilyMapWhenEmpty: true, + }); }; const isFlaggableFailure = (failure: Extract): boolean => { diff --git a/lib/accounts/active-index.ts b/lib/accounts/active-index.ts index f95f19e..40171f1 100644 --- a/lib/accounts/active-index.ts +++ b/lib/accounts/active-index.ts @@ -5,6 +5,16 @@ export interface ActiveIndexFamilyStorage { activeIndexByFamily?: Partial>; } +interface NormalizeActiveIndexOptions { + clearFamilyMapWhenEmpty?: boolean; + families?: readonly ModelFamily[]; +} + +function clampIndex(index: number, count: number): number { + if (count <= 0) return 0; + return Math.max(0, Math.min(index, count - 1)); +} + export function createActiveIndexByFamily( index: number, families: readonly ModelFamily[] = MODEL_FAMILIES, @@ -27,3 +37,46 @@ export function setActiveIndexForAllFamilies( storage.activeIndexByFamily[family] = index; } } + +export function normalizeActiveIndexByFamily( + storage: ActiveIndexFamilyStorage, + accountCount: number, + options: NormalizeActiveIndexOptions = {}, +): boolean { + const families = options.families ?? MODEL_FAMILIES; + const clearFamilyMapWhenEmpty = options.clearFamilyMapWhenEmpty === true; + let changed = false; + + const nextActiveIndex = clampIndex(storage.activeIndex, accountCount); + if (storage.activeIndex !== nextActiveIndex) { + storage.activeIndex = nextActiveIndex; + changed = true; + } + + if (accountCount === 0 && clearFamilyMapWhenEmpty) { + const hadKeys = !!storage.activeIndexByFamily && Object.keys(storage.activeIndexByFamily).length > 0; + if (storage.activeIndexByFamily === undefined || hadKeys) { + storage.activeIndexByFamily = {}; + changed = true; + } + return changed; + } + + if (!storage.activeIndexByFamily) { + storage.activeIndexByFamily = {}; + changed = true; + } + + for (const family of families) { + const raw = storage.activeIndexByFamily[family]; + const fallback = storage.activeIndex; + const candidate = typeof raw === "number" && Number.isFinite(raw) ? raw : fallback; + const nextValue = accountCount === 0 ? 0 : clampIndex(candidate, accountCount); + if (storage.activeIndexByFamily[family] !== nextValue) { + storage.activeIndexByFamily[family] = nextValue; + changed = true; + } + } + + return changed; +} diff --git a/lib/codex-cli/sync.ts b/lib/codex-cli/sync.ts index 83d7f54..787fd55 100644 --- a/lib/codex-cli/sync.ts +++ b/lib/codex-cli/sync.ts @@ -3,8 +3,11 @@ import { type AccountMetadataV3, type AccountStorageV3, } from "../storage.js"; -import { MODEL_FAMILIES, type ModelFamily } from "../prompts/codex.js"; -import { setActiveIndexForAllFamilies } from "../accounts/active-index.js"; +import { type ModelFamily } from "../prompts/codex.js"; +import { + setActiveIndexForAllFamilies, + normalizeActiveIndexByFamily, +} from "../accounts/active-index.js"; import { createLogger } from "../logger.js"; import { loadCodexCliState, type CodexCliAccountSnapshot } from "./state.js"; import { @@ -182,19 +185,7 @@ function writeFamilyIndexes( * @param storage - The account storage object whose indexes will be normalized and clamped */ function normalizeStoredFamilyIndexes(storage: AccountStorageV3): void { - const count = storage.accounts.length; - const clamped = count === 0 ? 0 : Math.max(0, Math.min(storage.activeIndex, count - 1)); - if (storage.activeIndex !== clamped) { - storage.activeIndex = clamped; - } - storage.activeIndexByFamily = storage.activeIndexByFamily ?? {}; - for (const family of MODEL_FAMILIES) { - const raw = storage.activeIndexByFamily[family]; - const resolved = - typeof raw === "number" && Number.isFinite(raw) ? raw : storage.activeIndex; - storage.activeIndexByFamily[family] = - count === 0 ? 0 : Math.max(0, Math.min(resolved, count - 1)); - } + normalizeActiveIndexByFamily(storage, storage.accounts.length); } /** diff --git a/lib/codex-manager.ts b/lib/codex-manager.ts index d289ce0..b9b2ece 100644 --- a/lib/codex-manager.ts +++ b/lib/codex-manager.ts @@ -29,6 +29,7 @@ import { import { createActiveIndexByFamily, setActiveIndexForAllFamilies, + normalizeActiveIndexByFamily, } from "./accounts/active-index.js"; import { ACCOUNT_LIMITS } from "./constants.js"; import { @@ -44,7 +45,6 @@ import { summarizeForecast, type ForecastAccountResult, } from "./forecast.js"; -import { MODEL_FAMILIES } from "./prompts/codex.js"; import { fetchCodexQuotaSnapshot, formatQuotaSnapshotLine, @@ -3051,25 +3051,7 @@ function hasPlaceholderEmail(value: string | undefined): boolean { } function normalizeDoctorIndexes(storage: AccountStorageV3): boolean { - const total = storage.accounts.length; - const nextActive = total === 0 ? 0 : Math.max(0, Math.min(storage.activeIndex, total - 1)); - let changed = false; - if (storage.activeIndex !== nextActive) { - storage.activeIndex = nextActive; - changed = true; - } - storage.activeIndexByFamily = storage.activeIndexByFamily ?? {}; - for (const family of MODEL_FAMILIES) { - const raw = storage.activeIndexByFamily[family]; - const fallback = storage.activeIndex; - const candidate = typeof raw === "number" && Number.isFinite(raw) ? raw : fallback; - const clamped = total === 0 ? 0 : Math.max(0, Math.min(candidate, total - 1)); - if (storage.activeIndexByFamily[family] !== clamped) { - storage.activeIndexByFamily[family] = clamped; - changed = true; - } - } - return changed; + return normalizeActiveIndexByFamily(storage, storage.accounts.length); } function applyDoctorFixes(storage: AccountStorageV3): { changed: boolean; actions: DoctorFixAction[] } { diff --git a/test/active-index.test.ts b/test/active-index.test.ts index bc7768d..15389e4 100644 --- a/test/active-index.test.ts +++ b/test/active-index.test.ts @@ -3,6 +3,7 @@ import { MODEL_FAMILIES } from "../lib/prompts/codex.js"; import { createActiveIndexByFamily, setActiveIndexForAllFamilies, + normalizeActiveIndexByFamily, } from "../lib/accounts/active-index.js"; describe("active-index helpers", () => { @@ -51,4 +52,67 @@ describe("active-index helpers", () => { expect(storage.activeIndexByFamily?.[family]).toBe(0); } }); + + it("normalizes active and per-family indexes within account bounds", () => { + const storage: { + activeIndex: number; + activeIndexByFamily?: Partial>; + } = { + activeIndex: 99, + activeIndexByFamily: { + codex: Number.NaN, + "gpt-5.1": -3, + }, + }; + + const changed = normalizeActiveIndexByFamily(storage, 2); + + expect(changed).toBe(true); + expect(storage.activeIndex).toBe(1); + for (const family of MODEL_FAMILIES) { + expect(storage.activeIndexByFamily?.[family]).toBeGreaterThanOrEqual(0); + expect(storage.activeIndexByFamily?.[family]).toBeLessThanOrEqual(1); + } + expect(storage.activeIndexByFamily?.codex).toBe(1); + expect(storage.activeIndexByFamily?.["gpt-5.1"]).toBe(0); + }); + + it("clears family map when empty accounts are requested to clear", () => { + const storage: { + activeIndex: number; + activeIndexByFamily?: Record; + } = { + activeIndex: 5, + activeIndexByFamily: { + codex: 2, + legacy: 9, + }, + }; + + const changed = normalizeActiveIndexByFamily(storage, 0, { + clearFamilyMapWhenEmpty: true, + }); + + expect(changed).toBe(true); + expect(storage.activeIndex).toBe(0); + expect(storage.activeIndexByFamily).toEqual({}); + }); + + it("fills model-family indexes with zero for empty account sets by default", () => { + const storage: { + activeIndex: number; + activeIndexByFamily?: Partial>; + } = { + activeIndex: 3, + activeIndexByFamily: {}, + }; + + const changed = normalizeActiveIndexByFamily(storage, 0); + + expect(changed).toBe(true); + expect(storage.activeIndex).toBe(0); + for (const family of MODEL_FAMILIES) { + expect(storage.activeIndexByFamily?.[family]).toBe(0); + } + }); }); diff --git a/test/index.test.ts b/test/index.test.ts index 76f35bf..f4e3188 100644 --- a/test/index.test.ts +++ b/test/index.test.ts @@ -803,9 +803,13 @@ describe("OpenAIOAuthPlugin", () => { it("handles removal of last account", async () => { mockStorage.accounts = [{ refreshToken: "r1", email: "user@example.com" }]; + mockStorage.activeIndex = 3; + mockStorage.activeIndexByFamily = { codex: 3, "gpt-5.1": 3 }; const result = await plugin.tool["codex-remove"].execute({ index: 1 }); expect(result).toContain("Removed"); expect(result).toContain("No accounts remaining"); + expect(mockStorage.activeIndex).toBe(0); + expect(mockStorage.activeIndexByFamily).toEqual({}); }); }); From a0b9d72f099f0c571a791c4825a1d3f114260193 Mon Sep 17 00:00:00 2001 From: ndycode Date: Wed, 4 Mar 2026 06:31:29 +0800 Subject: [PATCH 04/13] refactor: share family rate-limit status formatting Extract model-family rate-limit status rendering into account-view helpers and replace duplicate status mapping logic in the plugin status output paths. Add unit coverage for mixed limited/ok family labels to protect output behavior. Co-authored-by: Codex --- index.ts | 14 +++----------- lib/accounts/account-view.ts | 14 +++++++++++++- test/account-view.test.ts | 17 +++++++++++++++++ 3 files changed, 33 insertions(+), 12 deletions(-) diff --git a/index.ts b/index.ts index e0c9aaf..6f2e09b 100644 --- a/index.ts +++ b/index.ts @@ -119,8 +119,8 @@ import { } from "./lib/accounts.js"; import { resolveActiveIndex, - getRateLimitResetTimeForFamily, formatRateLimitEntry, + formatRateLimitStatusByFamily, } from "./lib/accounts/account-view.js"; import { setActiveIndexForAllFamilies, @@ -3598,11 +3598,7 @@ while (attempted.size < Math.max(1, accountCount)) { lines.push(""); lines.push(...formatUiSection(ui, "Rate limits by model family (per account)")); storage.accounts.forEach((account, index) => { - const statuses = MODEL_FAMILIES.map((family) => { - const resetAt = getRateLimitResetTimeForFamily(account, now, family); - if (typeof resetAt !== "number") return `${family}=ok`; - return `${family}=${formatWaitTime(resetAt - now)}`; - }); + const statuses = formatRateLimitStatusByFamily(account, now); lines.push(formatUiItem(ui, `Account ${index + 1}: ${statuses.join(" | ")}`)); }); @@ -3651,11 +3647,7 @@ while (attempted.size < Math.max(1, accountCount)) { lines.push(""); lines.push("Rate limits by model family (per account):"); storage.accounts.forEach((account, index) => { - const statuses = MODEL_FAMILIES.map((family) => { - const resetAt = getRateLimitResetTimeForFamily(account, now, family); - if (typeof resetAt !== "number") return `${family}=ok`; - return `${family}=${formatWaitTime(resetAt - now)}`; - }); + const statuses = formatRateLimitStatusByFamily(account, now); lines.push(` Account ${index + 1}: ${statuses.join(" | ")}`); }); diff --git a/lib/accounts/account-view.ts b/lib/accounts/account-view.ts index 576711d..025ab1d 100644 --- a/lib/accounts/account-view.ts +++ b/lib/accounts/account-view.ts @@ -1,4 +1,4 @@ -import type { ModelFamily } from "../prompts/codex.js"; +import { MODEL_FAMILIES, type ModelFamily } from "../prompts/codex.js"; import { formatWaitTime } from "./rate-limits.js"; export interface ActiveAccountStorageView { @@ -55,3 +55,15 @@ export function formatRateLimitEntry( if (remaining <= 0) return null; return `resets in ${formatWaitTime(remaining)}`; } + +export function formatRateLimitStatusByFamily( + account: AccountRateLimitView, + now: number, + families: readonly ModelFamily[] = MODEL_FAMILIES, +): string[] { + return families.map((family) => { + const resetAt = getRateLimitResetTimeForFamily(account, now, family); + if (typeof resetAt !== "number") return `${family}=ok`; + return `${family}=${formatWaitTime(resetAt - now)}`; + }); +} diff --git a/test/account-view.test.ts b/test/account-view.test.ts index ce554f5..f1c3b01 100644 --- a/test/account-view.test.ts +++ b/test/account-view.test.ts @@ -3,6 +3,7 @@ import { resolveActiveIndex, getRateLimitResetTimeForFamily, formatRateLimitEntry, + formatRateLimitStatusByFamily, } from "../lib/accounts/account-view.js"; describe("account-view helpers", () => { @@ -90,4 +91,20 @@ describe("account-view helpers", () => { ), ).toBeNull(); }); + + it("formats model-family status labels with ok and remaining times", () => { + const now = 1_000_000; + expect( + formatRateLimitStatusByFamily( + { + rateLimitResetTimes: { + codex: now + 30_000, + "gpt-5.1": now + 70_000, + }, + }, + now, + ["codex", "gpt-5.1", "codex-max"], + ), + ).toEqual(["codex=30s", "gpt-5.1=1m 10s", "codex-max=ok"]); + }); }); From f8bf733fddcbd9e3e458f55ebdca67b82e49f996 Mon Sep 17 00:00:00 2001 From: ndycode Date: Wed, 4 Mar 2026 06:33:33 +0800 Subject: [PATCH 05/13] refactor: share family index label formatting Move active-index-by-family label rendering into shared account-view helpers and reuse it in both v2 and legacy codex status output paths. Add helper tests to lock 1-based labels and placeholder output for missing families. Co-authored-by: Codex --- index.ts | 15 +++++---------- lib/accounts/account-view.ts | 13 +++++++++++++ test/account-view.test.ts | 16 ++++++++++++++++ 3 files changed, 34 insertions(+), 10 deletions(-) diff --git a/index.ts b/index.ts index 6f2e09b..7116a50 100644 --- a/index.ts +++ b/index.ts @@ -120,6 +120,7 @@ import { import { resolveActiveIndex, formatRateLimitEntry, + formatActiveIndexByFamilyLabels, formatRateLimitStatusByFamily, } from "./lib/accounts/account-view.js"; import { @@ -3588,11 +3589,8 @@ while (attempted.size < Math.max(1, accountCount)) { lines.push(""); lines.push(...formatUiSection(ui, "Active index by model family")); - for (const family of MODEL_FAMILIES) { - const idx = storage.activeIndexByFamily?.[family]; - const familyIndexLabel = - typeof idx === "number" && Number.isFinite(idx) ? String(idx + 1) : "-"; - lines.push(formatUiItem(ui, `${family}: ${familyIndexLabel}`)); + for (const line of formatActiveIndexByFamilyLabels(storage.activeIndexByFamily)) { + lines.push(formatUiItem(ui, line)); } lines.push(""); @@ -3637,11 +3635,8 @@ while (attempted.size < Math.max(1, accountCount)) { lines.push(""); lines.push("Active index by model family:"); - for (const family of MODEL_FAMILIES) { - const idx = storage.activeIndexByFamily?.[family]; - const familyIndexLabel = - typeof idx === "number" && Number.isFinite(idx) ? String(idx + 1) : "-"; - lines.push(` ${family}: ${familyIndexLabel}`); + for (const line of formatActiveIndexByFamilyLabels(storage.activeIndexByFamily)) { + lines.push(` ${line}`); } lines.push(""); diff --git a/lib/accounts/account-view.ts b/lib/accounts/account-view.ts index 025ab1d..4fec315 100644 --- a/lib/accounts/account-view.ts +++ b/lib/accounts/account-view.ts @@ -11,6 +11,8 @@ export interface AccountRateLimitView { rateLimitResetTimes?: Record; } +export type ActiveIndexByFamilyView = Partial> | undefined; + export function resolveActiveIndex( storage: ActiveAccountStorageView, family: ModelFamily = "codex", @@ -67,3 +69,14 @@ export function formatRateLimitStatusByFamily( return `${family}=${formatWaitTime(resetAt - now)}`; }); } + +export function formatActiveIndexByFamilyLabels( + activeIndexByFamily: ActiveIndexByFamilyView, + families: readonly ModelFamily[] = MODEL_FAMILIES, +): string[] { + return families.map((family) => { + const idx = activeIndexByFamily?.[family]; + const label = typeof idx === "number" && Number.isFinite(idx) ? String(idx + 1) : "-"; + return `${family}: ${label}`; + }); +} diff --git a/test/account-view.test.ts b/test/account-view.test.ts index f1c3b01..b33166d 100644 --- a/test/account-view.test.ts +++ b/test/account-view.test.ts @@ -3,6 +3,7 @@ import { resolveActiveIndex, getRateLimitResetTimeForFamily, formatRateLimitEntry, + formatActiveIndexByFamilyLabels, formatRateLimitStatusByFamily, } from "../lib/accounts/account-view.js"; @@ -107,4 +108,19 @@ describe("account-view helpers", () => { ), ).toEqual(["codex=30s", "gpt-5.1=1m 10s", "codex-max=ok"]); }); + + it("formats active-index labels with 1-based values and fallback placeholders", () => { + expect( + formatActiveIndexByFamilyLabels({ + codex: 0, + "gpt-5.1": 2, + }), + ).toEqual([ + "gpt-5-codex: -", + "codex-max: -", + "codex: 1", + "gpt-5.2: -", + "gpt-5.1: 3", + ]); + }); }); From 0e0edf87d17952b4a9e79493e7fd3c9bf81e5921 Mon Sep 17 00:00:00 2001 From: ndycode Date: Wed, 4 Mar 2026 06:36:11 +0800 Subject: [PATCH 06/13] refactor: extract account-removal index reconciliation Move codex-remove index/family reindexing into shared active-index helpers to centralize the mutation policy and reduce inline branching in the plugin entrypoint. Add helper tests for in-range and out-of-range removal behavior. Co-authored-by: Codex --- index.ts | 27 ++--------------------- lib/accounts/active-index.ts | 42 ++++++++++++++++++++++++++++++++++++ test/active-index.test.ts | 41 +++++++++++++++++++++++++++++++++++ 3 files changed, 85 insertions(+), 25 deletions(-) diff --git a/index.ts b/index.ts index 7116a50..aa88f6e 100644 --- a/index.ts +++ b/index.ts @@ -126,6 +126,7 @@ import { import { setActiveIndexForAllFamilies, normalizeActiveIndexByFamily, + removeAccountAndReconcileActiveIndexes, } from "./lib/accounts/active-index.js"; import { getStoragePath, @@ -3861,31 +3862,7 @@ while (attempted.size < Math.max(1, accountCount)) { const label = formatAccountLabel(account, targetIndex); - storage.accounts.splice(targetIndex, 1); - - if (storage.accounts.length === 0) { - storage.activeIndex = 0; - storage.activeIndexByFamily = {}; - } else { - if (storage.activeIndex >= storage.accounts.length) { - storage.activeIndex = 0; - } else if (storage.activeIndex > targetIndex) { - storage.activeIndex -= 1; - } - - if (storage.activeIndexByFamily) { - for (const family of MODEL_FAMILIES) { - const idx = storage.activeIndexByFamily[family]; - if (typeof idx === "number") { - if (idx >= storage.accounts.length) { - storage.activeIndexByFamily[family] = 0; - } else if (idx > targetIndex) { - storage.activeIndexByFamily[family] = idx - 1; - } - } - } - } - } + removeAccountAndReconcileActiveIndexes(storage, targetIndex); try { await saveAccounts(storage); diff --git a/lib/accounts/active-index.ts b/lib/accounts/active-index.ts index 40171f1..e0bbb8a 100644 --- a/lib/accounts/active-index.ts +++ b/lib/accounts/active-index.ts @@ -5,6 +5,10 @@ export interface ActiveIndexFamilyStorage { activeIndexByFamily?: Partial>; } +export interface AccountListActiveIndexStorage extends ActiveIndexFamilyStorage { + accounts: unknown[]; +} + interface NormalizeActiveIndexOptions { clearFamilyMapWhenEmpty?: boolean; families?: readonly ModelFamily[]; @@ -80,3 +84,41 @@ export function normalizeActiveIndexByFamily( return changed; } + +export function removeAccountAndReconcileActiveIndexes( + storage: AccountListActiveIndexStorage, + targetIndex: number, + families: readonly ModelFamily[] = MODEL_FAMILIES, +): boolean { + if (targetIndex < 0 || targetIndex >= storage.accounts.length) { + return false; + } + + storage.accounts.splice(targetIndex, 1); + + if (storage.accounts.length === 0) { + storage.activeIndex = 0; + storage.activeIndexByFamily = {}; + return true; + } + + if (storage.activeIndex >= storage.accounts.length) { + storage.activeIndex = 0; + } else if (storage.activeIndex > targetIndex) { + storage.activeIndex -= 1; + } + + if (storage.activeIndexByFamily) { + for (const family of families) { + const idx = storage.activeIndexByFamily[family]; + if (typeof idx !== "number") continue; + if (idx >= storage.accounts.length) { + storage.activeIndexByFamily[family] = 0; + } else if (idx > targetIndex) { + storage.activeIndexByFamily[family] = idx - 1; + } + } + } + + return true; +} diff --git a/test/active-index.test.ts b/test/active-index.test.ts index 15389e4..5ab421e 100644 --- a/test/active-index.test.ts +++ b/test/active-index.test.ts @@ -4,6 +4,7 @@ import { createActiveIndexByFamily, setActiveIndexForAllFamilies, normalizeActiveIndexByFamily, + removeAccountAndReconcileActiveIndexes, } from "../lib/accounts/active-index.js"; describe("active-index helpers", () => { @@ -115,4 +116,44 @@ describe("active-index helpers", () => { expect(storage.activeIndexByFamily?.[family]).toBe(0); } }); + + it("removes accounts and reconciles active indexes for remaining entries", () => { + const storage: { + accounts: unknown[]; + activeIndex: number; + activeIndexByFamily?: Partial>; + } = { + accounts: [{ id: 1 }, { id: 2 }, { id: 3 }], + activeIndex: 2, + activeIndexByFamily: { + codex: 2, + "gpt-5.1": 2, + }, + }; + + const changed = removeAccountAndReconcileActiveIndexes(storage, 0); + + expect(changed).toBe(true); + expect(storage.accounts).toHaveLength(2); + expect(storage.activeIndex).toBe(0); + expect(storage.activeIndexByFamily?.codex).toBe(0); + expect(storage.activeIndexByFamily?.["gpt-5.1"]).toBe(0); + }); + + it("returns false and keeps storage unchanged for out-of-range removals", () => { + const storage: { + accounts: unknown[]; + activeIndex: number; + activeIndexByFamily?: Record; + } = { + accounts: [{ id: 1 }], + activeIndex: 0, + activeIndexByFamily: { codex: 0 }, + }; + + expect(removeAccountAndReconcileActiveIndexes(storage, 2)).toBe(false); + expect(storage.accounts).toHaveLength(1); + expect(storage.activeIndex).toBe(0); + expect(storage.activeIndexByFamily).toEqual({ codex: 0 }); + }); }); From 6b7b2e8926504eba67fc4cba0ac5af4abd63a998 Mon Sep 17 00:00:00 2001 From: ndycode Date: Wed, 4 Mar 2026 07:27:38 +0800 Subject: [PATCH 07/13] refactor: centralize account storage clone/empty factories Introduce shared account-storage view helpers for creating empty v3 storage and cloning mutable working copies, then reuse them across plugin login/check flow, codex-cli sync, and manager reset paths. Add unit tests to lock clone isolation and family-index initialization behavior. Co-authored-by: Codex --- index.ts | 24 ++++++------------ lib/accounts/storage-view.ts | 29 +++++++++++++++++++++ lib/codex-cli/sync.ts | 26 ++++--------------- lib/codex-manager.ts | 18 ++++--------- test/account-storage-view.test.ts | 42 +++++++++++++++++++++++++++++++ 5 files changed, 89 insertions(+), 50 deletions(-) create mode 100644 lib/accounts/storage-view.ts create mode 100644 test/account-storage-view.test.ts diff --git a/index.ts b/index.ts index aa88f6e..54a55a3 100644 --- a/index.ts +++ b/index.ts @@ -123,6 +123,10 @@ import { formatActiveIndexByFamilyLabels, formatRateLimitStatusByFamily, } from "./lib/accounts/account-view.js"; +import { + cloneAccountStorage, + createEmptyAccountStorage, +} from "./lib/accounts/storage-view.js"; import { setActiveIndexForAllFamilies, normalizeActiveIndexByFamily, @@ -2653,14 +2657,8 @@ while (attempted.size < Math.max(1, accountCount)) { const runAccountCheck = async (deepProbe: boolean): Promise => { const loadedStorage = await hydrateEmails(await loadAccounts()); const workingStorage = loadedStorage - ? { - ...loadedStorage, - accounts: loadedStorage.accounts.map((account) => ({ ...account })), - activeIndexByFamily: loadedStorage.activeIndexByFamily - ? { ...loadedStorage.activeIndexByFamily } - : {}, - } - : { version: 3 as const, accounts: [], activeIndex: 0, activeIndexByFamily: {} }; + ? cloneAccountStorage(loadedStorage) + : createEmptyAccountStorage(); if (workingStorage.accounts.length === 0) { console.log("\nNo accounts to check.\n"); @@ -3006,14 +3004,8 @@ while (attempted.size < Math.max(1, accountCount)) { while (true) { const loadedStorage = await hydrateEmails(await loadAccounts()); const workingStorage = loadedStorage - ? { - ...loadedStorage, - accounts: loadedStorage.accounts.map((account) => ({ ...account })), - activeIndexByFamily: loadedStorage.activeIndexByFamily - ? { ...loadedStorage.activeIndexByFamily } - : {}, - } - : { version: 3 as const, accounts: [], activeIndex: 0, activeIndexByFamily: {} }; + ? cloneAccountStorage(loadedStorage) + : createEmptyAccountStorage(); const flaggedStorage = await loadFlaggedAccounts(); if (workingStorage.accounts.length === 0 && flaggedStorage.accounts.length === 0) { diff --git a/lib/accounts/storage-view.ts b/lib/accounts/storage-view.ts new file mode 100644 index 0000000..ef5feb4 --- /dev/null +++ b/lib/accounts/storage-view.ts @@ -0,0 +1,29 @@ +import type { AccountStorageV3 } from "../storage.js"; +import { createActiveIndexByFamily } from "./active-index.js"; + +interface CreateEmptyAccountStorageOptions { + initializeFamilyIndexes?: boolean; +} + +export function createEmptyAccountStorage( + options: CreateEmptyAccountStorageOptions = {}, +): AccountStorageV3 { + const initializeFamilyIndexes = options.initializeFamilyIndexes === true; + return { + version: 3, + accounts: [], + activeIndex: 0, + activeIndexByFamily: initializeFamilyIndexes ? createActiveIndexByFamily(0) : {}, + }; +} + +export function cloneAccountStorage(storage: AccountStorageV3): AccountStorageV3 { + return { + version: 3, + accounts: storage.accounts.map((account) => ({ ...account })), + activeIndex: storage.activeIndex, + activeIndexByFamily: storage.activeIndexByFamily + ? { ...storage.activeIndexByFamily } + : {}, + }; +} diff --git a/lib/codex-cli/sync.ts b/lib/codex-cli/sync.ts index 787fd55..af880fa 100644 --- a/lib/codex-cli/sync.ts +++ b/lib/codex-cli/sync.ts @@ -8,6 +8,10 @@ import { setActiveIndexForAllFamilies, normalizeActiveIndexByFamily, } from "../accounts/active-index.js"; +import { + cloneAccountStorage, + createEmptyAccountStorage, +} from "../accounts/storage-view.js"; import { createLogger } from "../logger.js"; import { loadCodexCliState, type CodexCliAccountSnapshot } from "./state.js"; import { @@ -24,26 +28,6 @@ function normalizeEmail(value: string | undefined): string | undefined { return trimmed.length > 0 ? trimmed : undefined; } -function createEmptyStorage(): AccountStorageV3 { - return { - version: 3, - accounts: [], - activeIndex: 0, - activeIndexByFamily: {}, - }; -} - -function cloneStorage(storage: AccountStorageV3): AccountStorageV3 { - return { - version: 3, - accounts: storage.accounts.map((account) => ({ ...account })), - activeIndex: storage.activeIndex, - activeIndexByFamily: storage.activeIndexByFamily - ? { ...storage.activeIndexByFamily } - : {}, - }; -} - function buildIndexByAccountId(accounts: AccountMetadataV3[]): Map { const map = new Map(); for (let i = 0; i < accounts.length; i += 1) { @@ -268,7 +252,7 @@ export async function syncAccountStorageFromCodexCli( return { storage: current, changed: false }; } - const next = current ? cloneStorage(current) : createEmptyStorage(); + const next = current ? cloneAccountStorage(current) : createEmptyAccountStorage(); let changed = false; for (const snapshot of state.accounts) { diff --git a/lib/codex-manager.ts b/lib/codex-manager.ts index b9b2ece..a8f5c56 100644 --- a/lib/codex-manager.ts +++ b/lib/codex-manager.ts @@ -31,6 +31,9 @@ import { setActiveIndexForAllFamilies, normalizeActiveIndexByFamily, } from "./accounts/active-index.js"; +import { + createEmptyAccountStorage as createEmptyAccountStorageBase, +} from "./accounts/storage-view.js"; import { ACCOUNT_LIMITS } from "./constants.js"; import { loadDashboardDisplaySettings, @@ -2346,13 +2349,7 @@ interface VerifyFlaggedReport { } function createEmptyAccountStorage(): AccountStorageV3 { - const activeIndexByFamily = createActiveIndexByFamily(0); - return { - version: 3, - accounts: [], - activeIndex: 0, - activeIndexByFamily, - }; + return createEmptyAccountStorageBase({ initializeFamilyIndexes: true }); } function findExistingAccountIndexForFlagged( @@ -3589,12 +3586,7 @@ async function runDoctor(args: string[]): Promise { } async function clearAccountsAndReset(): Promise { - await saveAccounts({ - version: 3, - accounts: [], - activeIndex: 0, - activeIndexByFamily: {}, - }); + await saveAccounts(createEmptyAccountStorageBase()); } async function handleManageAction( diff --git a/test/account-storage-view.test.ts b/test/account-storage-view.test.ts new file mode 100644 index 0000000..4c1ac5b --- /dev/null +++ b/test/account-storage-view.test.ts @@ -0,0 +1,42 @@ +import { describe, expect, it } from "vitest"; +import { MODEL_FAMILIES } from "../lib/prompts/codex.js"; +import { + cloneAccountStorage, + createEmptyAccountStorage, +} from "../lib/accounts/storage-view.js"; + +describe("account storage view helpers", () => { + it("creates default empty storage without prefilled family indexes", () => { + expect(createEmptyAccountStorage()).toEqual({ + version: 3, + accounts: [], + activeIndex: 0, + activeIndexByFamily: {}, + }); + }); + + it("can create empty storage with per-family indexes initialized", () => { + const storage = createEmptyAccountStorage({ initializeFamilyIndexes: true }); + for (const family of MODEL_FAMILIES) { + expect(storage.activeIndexByFamily?.[family]).toBe(0); + } + }); + + it("clones account storage to isolated mutable copies", () => { + const original = { + version: 3 as const, + accounts: [{ refreshToken: "r1", email: "a@example.com", enabled: true }], + activeIndex: 0, + activeIndexByFamily: { codex: 0 }, + }; + + const clone = cloneAccountStorage(original); + expect(clone).toEqual(original); + + clone.accounts[0]!.email = "b@example.com"; + clone.activeIndexByFamily.codex = 1; + + expect(original.accounts[0]!.email).toBe("a@example.com"); + expect(original.activeIndexByFamily.codex).toBe(0); + }); +}); From c953709fccc2dd53b095f9f8b70ffcf0f17f5bf1 Mon Sep 17 00:00:00 2001 From: ndycode Date: Thu, 5 Mar 2026 10:33:40 +0800 Subject: [PATCH 08/13] fix: resolve PR33 review issues on index normalization and account reconciliation - normalize event and helper indexes to safe integer bounds\n- ignore non-finite rate-limit reset timestamps\n- deep-clone account rate-limit maps in storage views\n- reconcile active selection when deleting accounts instead of forcing index 0\n- add regression coverage for fractional/non-finite inputs and manager delete flow\n\nCo-authored-by: Codex --- index.ts | 5 +-- lib/accounts/account-view.ts | 4 +-- lib/accounts/active-index.ts | 41 +++++++++++++++------- lib/accounts/storage-view.ts | 11 ++++-- lib/codex-manager.ts | 9 ++--- test/account-storage-view.test.ts | 11 +++++- test/account-view.test.ts | 49 ++++++++++++++++++++++++++ test/active-index.test.ts | 49 +++++++++++++++++++++++--- test/codex-manager-cli.test.ts | 57 +++++++++++++++++++++++++++++-- test/index.test.ts | 35 +++++++++++++++---- 10 files changed, 234 insertions(+), 37 deletions(-) diff --git a/index.ts b/index.ts index 54a55a3..f30a3ee 100644 --- a/index.ts +++ b/index.ts @@ -952,8 +952,9 @@ export const OpenAIOAuthPlugin: Plugin = async ({ client }: PluginInput) => { return; } - const index = props.index ?? props.accountIndex; - if (typeof index === "number") { + const rawIndex = props.index ?? props.accountIndex; + if (typeof rawIndex === "number" && Number.isFinite(rawIndex)) { + const index = Math.floor(rawIndex); const storage = await loadAccounts(); if (!storage || index < 0 || index >= storage.accounts.length) { return; diff --git a/lib/accounts/account-view.ts b/lib/accounts/account-view.ts index 4fec315..d549b6b 100644 --- a/lib/accounts/account-view.ts +++ b/lib/accounts/account-view.ts @@ -20,7 +20,7 @@ export function resolveActiveIndex( const total = storage.accounts.length; if (total === 0) return 0; const rawCandidate = storage.activeIndexByFamily?.[family] ?? storage.activeIndex; - const raw = Number.isFinite(rawCandidate) ? rawCandidate : 0; + const raw = Number.isFinite(rawCandidate) ? Math.floor(rawCandidate) : 0; return Math.max(0, Math.min(raw, total - 1)); } @@ -35,7 +35,7 @@ export function getRateLimitResetTimeForFamily( let minReset: number | null = null; const prefix = `${family}:`; for (const [key, value] of Object.entries(times)) { - if (typeof value !== "number") continue; + if (typeof value !== "number" || !Number.isFinite(value)) continue; if (value <= now) continue; if (key !== family && !key.startsWith(prefix)) continue; if (minReset === null || value < minReset) { diff --git a/lib/accounts/active-index.ts b/lib/accounts/active-index.ts index e0bbb8a..5d51809 100644 --- a/lib/accounts/active-index.ts +++ b/lib/accounts/active-index.ts @@ -14,9 +14,14 @@ interface NormalizeActiveIndexOptions { families?: readonly ModelFamily[]; } +function toFiniteInteger(index: number): number { + return Number.isFinite(index) ? Math.trunc(index) : 0; +} + function clampIndex(index: number, count: number): number { if (count <= 0) return 0; - return Math.max(0, Math.min(index, count - 1)); + const normalized = toFiniteInteger(index); + return Math.max(0, Math.min(normalized, count - 1)); } export function createActiveIndexByFamily( @@ -35,10 +40,11 @@ export function setActiveIndexForAllFamilies( index: number, families: readonly ModelFamily[] = MODEL_FAMILIES, ): void { - storage.activeIndex = index; + const normalized = Math.max(0, toFiniteInteger(index)); + storage.activeIndex = normalized; storage.activeIndexByFamily = storage.activeIndexByFamily ?? {}; for (const family of families) { - storage.activeIndexByFamily[family] = index; + storage.activeIndexByFamily[family] = normalized; } } @@ -90,10 +96,12 @@ export function removeAccountAndReconcileActiveIndexes( targetIndex: number, families: readonly ModelFamily[] = MODEL_FAMILIES, ): boolean { - if (targetIndex < 0 || targetIndex >= storage.accounts.length) { + if (!Number.isInteger(targetIndex) || targetIndex < 0 || targetIndex >= storage.accounts.length) { return false; } + const previousCount = storage.accounts.length; + const previousActive = clampIndex(storage.activeIndex, previousCount); storage.accounts.splice(targetIndex, 1); if (storage.accounts.length === 0) { @@ -102,20 +110,27 @@ export function removeAccountAndReconcileActiveIndexes( return true; } - if (storage.activeIndex >= storage.accounts.length) { - storage.activeIndex = 0; - } else if (storage.activeIndex > targetIndex) { - storage.activeIndex -= 1; + if (previousActive > targetIndex) { + storage.activeIndex = previousActive - 1; + } else if (previousActive === targetIndex) { + storage.activeIndex = Math.min(targetIndex, storage.accounts.length - 1); + } else { + storage.activeIndex = previousActive; } if (storage.activeIndexByFamily) { for (const family of families) { const idx = storage.activeIndexByFamily[family]; - if (typeof idx !== "number") continue; - if (idx >= storage.accounts.length) { - storage.activeIndexByFamily[family] = 0; - } else if (idx > targetIndex) { - storage.activeIndexByFamily[family] = idx - 1; + const base = typeof idx === "number" + ? clampIndex(idx, previousCount) + : previousActive; + const next = base > targetIndex + ? base - 1 + : base === targetIndex + ? Math.min(targetIndex, storage.accounts.length - 1) + : base; + if (storage.activeIndexByFamily[family] !== next) { + storage.activeIndexByFamily[family] = next; } } } diff --git a/lib/accounts/storage-view.ts b/lib/accounts/storage-view.ts index ef5feb4..9ab2258 100644 --- a/lib/accounts/storage-view.ts +++ b/lib/accounts/storage-view.ts @@ -1,4 +1,4 @@ -import type { AccountStorageV3 } from "../storage.js"; +import type { AccountMetadataV3, AccountStorageV3 } from "../storage.js"; import { createActiveIndexByFamily } from "./active-index.js"; interface CreateEmptyAccountStorageOptions { @@ -18,9 +18,16 @@ export function createEmptyAccountStorage( } export function cloneAccountStorage(storage: AccountStorageV3): AccountStorageV3 { + const cloneAccount = (account: AccountMetadataV3): AccountMetadataV3 => ({ + ...account, + rateLimitResetTimes: account.rateLimitResetTimes + ? { ...account.rateLimitResetTimes } + : undefined, + }); + return { version: 3, - accounts: storage.accounts.map((account) => ({ ...account })), + accounts: storage.accounts.map(cloneAccount), activeIndex: storage.activeIndex, activeIndexByFamily: storage.activeIndexByFamily ? { ...storage.activeIndexByFamily } diff --git a/lib/codex-manager.ts b/lib/codex-manager.ts index a8f5c56..fc58a85 100644 --- a/lib/codex-manager.ts +++ b/lib/codex-manager.ts @@ -30,6 +30,7 @@ import { createActiveIndexByFamily, setActiveIndexForAllFamilies, normalizeActiveIndexByFamily, + removeAccountAndReconcileActiveIndexes, } from "./accounts/active-index.js"; import { createEmptyAccountStorage as createEmptyAccountStorageBase, @@ -1390,13 +1391,14 @@ async function showAccountStatus(): Promise { if (account.enabled === false) markers.push("disabled"); const rateLimit = formatRateLimitEntry(account, now, "codex"); if (rateLimit) markers.push("rate-limited"); + const rateLimitDetail = rateLimit ? ` (${rateLimit})` : ""; const cooldown = formatCooldown(account, now); if (cooldown) markers.push(`cooldown:${cooldown}`); const markerLabel = markers.length > 0 ? ` [${markers.join(", ")}]` : ""; const lastUsed = typeof account.lastUsed === "number" && account.lastUsed > 0 ? `used ${formatWaitTime(now - account.lastUsed)} ago` : "never used"; - console.log(`${i + 1}. ${label}${markerLabel} ${lastUsed}`); + console.log(`${i + 1}. ${label}${markerLabel}${rateLimitDetail} ${lastUsed}`); } } @@ -3586,7 +3588,7 @@ async function runDoctor(args: string[]): Promise { } async function clearAccountsAndReset(): Promise { - await saveAccounts(createEmptyAccountStorageBase()); + await saveAccounts(createEmptyAccountStorage()); } async function handleManageAction( @@ -3602,8 +3604,7 @@ async function handleManageAction( if (typeof menuResult.deleteAccountIndex === "number") { const idx = menuResult.deleteAccountIndex; if (idx >= 0 && idx < storage.accounts.length) { - storage.accounts.splice(idx, 1); - setActiveIndexForAllFamilies(storage, 0); + removeAccountAndReconcileActiveIndexes(storage, idx); await saveAccounts(storage); console.log(`Deleted account ${idx + 1}.`); } diff --git a/test/account-storage-view.test.ts b/test/account-storage-view.test.ts index 4c1ac5b..9d090f2 100644 --- a/test/account-storage-view.test.ts +++ b/test/account-storage-view.test.ts @@ -25,7 +25,14 @@ describe("account storage view helpers", () => { it("clones account storage to isolated mutable copies", () => { const original = { version: 3 as const, - accounts: [{ refreshToken: "r1", email: "a@example.com", enabled: true }], + accounts: [ + { + refreshToken: "r1", + email: "a@example.com", + enabled: true, + rateLimitResetTimes: { codex: 123_000 }, + }, + ], activeIndex: 0, activeIndexByFamily: { codex: 0 }, }; @@ -34,9 +41,11 @@ describe("account storage view helpers", () => { expect(clone).toEqual(original); clone.accounts[0]!.email = "b@example.com"; + clone.accounts[0]!.rateLimitResetTimes!.codex = 999_000; clone.activeIndexByFamily.codex = 1; expect(original.accounts[0]!.email).toBe("a@example.com"); + expect(original.accounts[0]!.rateLimitResetTimes?.codex).toBe(123_000); expect(original.activeIndexByFamily.codex).toBe(0); }); }); diff --git a/test/account-view.test.ts b/test/account-view.test.ts index b33166d..65c20e1 100644 --- a/test/account-view.test.ts +++ b/test/account-view.test.ts @@ -19,6 +19,13 @@ describe("account-view helpers", () => { }); it("falls back to storage active index and normalizes non-finite candidates", () => { + expect( + resolveActiveIndex({ + activeIndex: 5, + activeIndexByFamily: { codex: 3 }, + accounts: [], + }), + ).toBe(0); expect( resolveActiveIndex({ activeIndex: Number.NaN, @@ -34,6 +41,16 @@ describe("account-view helpers", () => { ).toBe(2); }); + it("coerces fractional active indexes to bounded integers", () => { + const resolved = resolveActiveIndex({ + activeIndex: 1.8, + activeIndexByFamily: { codex: 1.2 }, + accounts: [{}, {}], + }); + expect(resolved).toBe(1); + expect(Number.isInteger(resolved)).toBe(true); + }); + it("returns earliest future reset for matching family keys", () => { const now = 1_000_000; expect( @@ -52,6 +69,38 @@ describe("account-view helpers", () => { ).toBe(now + 30_000); }); + it("ignores non-finite reset timestamps when selecting family reset times", () => { + const now = 20_000; + expect( + getRateLimitResetTimeForFamily( + { + rateLimitResetTimes: { + codex: Number.NaN, + "codex:gpt-5.2": Number.POSITIVE_INFINITY, + "codex:gpt-5.1": Number.NEGATIVE_INFINITY, + "codex:gpt-5.3": now + 4_000, + }, + }, + now, + "codex", + ), + ).toBe(now + 4_000); + expect( + formatRateLimitEntry( + { + rateLimitResetTimes: { + codex: Number.NaN, + "codex:gpt-5.2": Number.POSITIVE_INFINITY, + "codex:gpt-5.1": Number.NEGATIVE_INFINITY, + "codex:gpt-5.3": now + 4_000, + }, + }, + now, + "codex", + ), + ).toBe("resets in 4s"); + }); + it("returns null for missing or expired family reset state", () => { const now = 5_000; expect(getRateLimitResetTimeForFamily({}, now, "codex")).toBeNull(); diff --git a/test/active-index.test.ts b/test/active-index.test.ts index 5ab421e..4245fde 100644 --- a/test/active-index.test.ts +++ b/test/active-index.test.ts @@ -34,6 +34,27 @@ describe("active-index helpers", () => { } }); + it("normalizes non-finite and fractional set-active values before writing", () => { + const storage: { + activeIndex: number; + activeIndexByFamily?: Partial>; + } = { + activeIndex: 0, + }; + + setActiveIndexForAllFamilies(storage, 1.8); + expect(storage.activeIndex).toBe(1); + for (const family of MODEL_FAMILIES) { + expect(storage.activeIndexByFamily?.[family]).toBe(1); + } + + setActiveIndexForAllFamilies(storage, Number.NaN); + expect(storage.activeIndex).toBe(0); + for (const family of MODEL_FAMILIES) { + expect(storage.activeIndexByFamily?.[family]).toBe(0); + } + }); + it("preserves unrelated keys when reusing existing family map objects", () => { const storage: { activeIndex: number; @@ -61,8 +82,8 @@ describe("active-index helpers", () => { } = { activeIndex: 99, activeIndexByFamily: { - codex: Number.NaN, - "gpt-5.1": -3, + codex: Number.POSITIVE_INFINITY, + "gpt-5.1": -3.2, }, }; @@ -73,6 +94,7 @@ describe("active-index helpers", () => { for (const family of MODEL_FAMILIES) { expect(storage.activeIndexByFamily?.[family]).toBeGreaterThanOrEqual(0); expect(storage.activeIndexByFamily?.[family]).toBeLessThanOrEqual(1); + expect(Number.isInteger(storage.activeIndexByFamily?.[family] ?? Number.NaN)).toBe(true); } expect(storage.activeIndexByFamily?.codex).toBe(1); expect(storage.activeIndexByFamily?.["gpt-5.1"]).toBe(0); @@ -135,9 +157,9 @@ describe("active-index helpers", () => { expect(changed).toBe(true); expect(storage.accounts).toHaveLength(2); - expect(storage.activeIndex).toBe(0); - expect(storage.activeIndexByFamily?.codex).toBe(0); - expect(storage.activeIndexByFamily?.["gpt-5.1"]).toBe(0); + expect(storage.activeIndex).toBe(1); + expect(storage.activeIndexByFamily?.codex).toBe(1); + expect(storage.activeIndexByFamily?.["gpt-5.1"]).toBe(1); }); it("returns false and keeps storage unchanged for out-of-range removals", () => { @@ -156,4 +178,21 @@ describe("active-index helpers", () => { expect(storage.activeIndex).toBe(0); expect(storage.activeIndexByFamily).toEqual({ codex: 0 }); }); + + it("returns false and keeps storage unchanged for non-integer removals", () => { + const storage: { + accounts: unknown[]; + activeIndex: number; + activeIndexByFamily?: Record; + } = { + accounts: [{ id: 1 }, { id: 2 }], + activeIndex: 1, + activeIndexByFamily: { codex: 1 }, + }; + + expect(removeAccountAndReconcileActiveIndexes(storage, 0.5)).toBe(false); + expect(storage.accounts).toEqual([{ id: 1 }, { id: 2 }]); + expect(storage.activeIndex).toBe(1); + expect(storage.activeIndexByFamily).toEqual({ codex: 1 }); + }); }); diff --git a/test/codex-manager-cli.test.ts b/test/codex-manager-cli.test.ts index 46af34d..9371dde 100644 --- a/test/codex-manager-cli.test.ts +++ b/test/codex-manager-cli.test.ts @@ -501,7 +501,7 @@ describe("codex manager cli commands", () => { it("shows rate-limit reset details in auth status output", async () => { const now = Date.now(); - loadAccountsMock.mockResolvedValueOnce({ + const storage = { version: 3, activeIndex: 0, activeIndexByFamily: { codex: 0 }, @@ -516,13 +516,16 @@ describe("codex manager cli commands", () => { }, }, ], - }); + }; + loadAccountsMock.mockResolvedValueOnce(storage); const logSpy = vi.spyOn(console, "log").mockImplementation(() => {}); const { runCodexMultiAuthCli } = await import("../lib/codex-manager.js"); + const { formatRateLimitEntry } = await import("../lib/accounts/account-view.js"); const exitCode = await runCodexMultiAuthCli(["auth", "status"]); expect(exitCode).toBe(0); expect(logSpy.mock.calls.some((call) => String(call[0]).includes("rate-limited"))).toBe(true); + expect(formatRateLimitEntry(storage.accounts[0]!, now, "codex")).toContain("resets in"); }); it("runs fix apply mode and returns a switch recommendation", async () => { @@ -1968,6 +1971,56 @@ describe("codex manager cli commands", () => { expect(saveAccountsMock.mock.calls[0]?.[0]?.accounts?.[0]?.email).toBe("first@example.com"); }); + it("preserves active selection semantics when deleting a non-active account", async () => { + const now = Date.now(); + loadAccountsMock.mockResolvedValue({ + version: 3, + activeIndex: 2, + activeIndexByFamily: { + codex: 2, + "gpt-5.1": 2, + }, + accounts: [ + { + email: "first@example.com", + refreshToken: "refresh-first", + addedAt: now - 3_000, + lastUsed: now - 3_000, + enabled: true, + }, + { + email: "second@example.com", + refreshToken: "refresh-second", + addedAt: now - 2_000, + lastUsed: now - 2_000, + enabled: true, + }, + { + email: "third@example.com", + refreshToken: "refresh-third", + addedAt: now - 1_000, + lastUsed: now - 1_000, + enabled: true, + }, + ], + }); + promptLoginModeMock + .mockResolvedValueOnce({ mode: "manage", deleteAccountIndex: 0 }) + .mockResolvedValueOnce({ mode: "cancel" }); + + const { runCodexMultiAuthCli } = await import("../lib/codex-manager.js"); + const exitCode = await runCodexMultiAuthCli(["auth", "login"]); + + expect(exitCode).toBe(0); + expect(saveAccountsMock).toHaveBeenCalledTimes(1); + const saved = saveAccountsMock.mock.calls[0]?.[0]; + expect(saved?.accounts).toHaveLength(2); + expect(saved?.accounts?.[0]?.email).toBe("second@example.com"); + expect(saved?.activeIndex).toBe(1); + expect(saved?.activeIndexByFamily?.codex).toBe(1); + expect(saved?.activeIndexByFamily?.["gpt-5.1"]).toBe(2); + }); + it("toggles account enabled state from manage mode", async () => { const now = Date.now(); loadAccountsMock.mockResolvedValue({ diff --git a/test/index.test.ts b/test/index.test.ts index f4e3188..69e8cd1 100644 --- a/test/index.test.ts +++ b/test/index.test.ts @@ -987,8 +987,8 @@ describe("OpenAIOAuthPlugin edge cases", () => { }); it("adjusts activeIndex when removing account before it", async () => { - // When activeIndex=2 and we remove index 0 (1-based: 1), the remaining accounts - // have length 2. Since activeIndex (2) >= length (2), it resets to 0. + // When activeIndex=2 and we remove index 0 (1-based: 1), the active account + // should remain selected after reindexing (new index 1). mockStorage.accounts = [ { refreshToken: "r1", email: "user1@example.com" }, { refreshToken: "r2", email: "user2@example.com" }, @@ -1003,10 +1003,8 @@ describe("OpenAIOAuthPlugin edge cases", () => { const plugin = await OpenAIOAuthPlugin({ client: mockClient } as never) as unknown as PluginType; await plugin.tool["codex-remove"].execute({ index: 1 }); - // After removing account at 0-based index 0, length is 2. - // activeIndex (2) >= length (2), so it resets to 0 - expect(mockStorage.activeIndex).toBe(0); - expect(mockStorage.activeIndexByFamily.codex).toBe(0); + expect(mockStorage.activeIndex).toBe(1); + expect(mockStorage.activeIndexByFamily.codex).toBe(1); }); it("resets activeIndex when removing active account at end", async () => { @@ -1922,6 +1920,31 @@ describe("OpenAIOAuthPlugin event handler edge cases", () => { } }); + it("coerces fractional account.select indexes before applying selection", async () => { + const mockClient = createMockClient(); + const { OpenAIOAuthPlugin } = await import("../index.js"); + const plugin = await OpenAIOAuthPlugin({ client: mockClient } as never) as unknown as PluginType; + + const getAuth = async () => ({ + type: "oauth" as const, + access: "access-token", + refresh: "refresh-token", + expires: Date.now() + 60_000, + multiAccount: true, + }); + + await plugin.auth.loader(getAuth, { options: {}, models: {} }); + + await plugin.event({ + event: { type: "account.select", properties: { index: 1.9 } }, + }); + const { MODEL_FAMILIES } = await import("../lib/prompts/codex.js"); + expect(mockStorage.activeIndex).toBe(1); + for (const family of MODEL_FAMILIES) { + expect(mockStorage.activeIndexByFamily[family]).toBe(1); + } + }); + it("reloads account manager from disk when handling account.select", async () => { const mockClient = createMockClient(); const { OpenAIOAuthPlugin } = await import("../index.js"); From 3135264b8141d8271445f1338f7f8fdbae132e6d Mon Sep 17 00:00:00 2001 From: ndycode Date: Thu, 5 Mar 2026 10:44:05 +0800 Subject: [PATCH 09/13] test: align PR33 family reindex assertion with helper behavior Update the mocked model-family set and deletion assertion so the test validates that non-codex family indexes are reindexed correctly after account removal.\n\nCo-authored-by: Codex --- test/codex-manager-cli.test.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/codex-manager-cli.test.ts b/test/codex-manager-cli.test.ts index 9371dde..d4339f6 100644 --- a/test/codex-manager-cli.test.ts +++ b/test/codex-manager-cli.test.ts @@ -51,7 +51,7 @@ vi.mock("../lib/cli.js", () => ({ })); vi.mock("../lib/prompts/codex.js", () => ({ - MODEL_FAMILIES: ["codex"] as const, + MODEL_FAMILIES: ["codex", "gpt-5.1"] as const, })); vi.mock("../lib/accounts.js", () => ({ @@ -2018,7 +2018,7 @@ describe("codex manager cli commands", () => { expect(saved?.accounts?.[0]?.email).toBe("second@example.com"); expect(saved?.activeIndex).toBe(1); expect(saved?.activeIndexByFamily?.codex).toBe(1); - expect(saved?.activeIndexByFamily?.["gpt-5.1"]).toBe(2); + expect(saved?.activeIndexByFamily?.["gpt-5.1"]).toBe(1); }); it("toggles account enabled state from manage mode", async () => { From 281738460e5dc86dfd26d87404d7f11e6fd47cbf Mon Sep 17 00:00:00 2001 From: ndycode Date: Thu, 5 Mar 2026 11:02:12 +0800 Subject: [PATCH 10/13] fix: resolve PR33 deletion reindex and manage-delete guards - use removal reconciliation (not clamp-only) in plugin delete flows to keep per-family selection stable - guard manage delete persistence/logging on actual removal result - align index test model-family mock with production families and add multi-family reindex regression - add codex-manager CLI regression for fractional delete index handling Co-authored-by: Codex --- index.ts | 41 +++++++++++++++++++++++------ lib/codex-manager.ts | 6 ++++- test/codex-manager-cli.test.ts | 47 ++++++++++++++++++++++++++++++++++ test/index.test.ts | 32 ++++++++++++++++++++++- 4 files changed, 116 insertions(+), 10 deletions(-) diff --git a/index.ts b/index.ts index f30a3ee..585e125 100644 --- a/index.ts +++ b/index.ts @@ -2394,10 +2394,18 @@ while (attempted.size < Math.max(1, accountCount)) { let startFresh = explicitLoginMode === "fresh"; let refreshAccountIndex: number | undefined; - const clampActiveIndices = (storage: AccountStorageV3): void => { + const reconcileActiveIndicesAfterRemoval = ( + storage: AccountStorageV3, + targetIndex: number, + ): boolean => { + const removed = removeAccountAndReconcileActiveIndexes(storage, targetIndex); + if (!removed) { + return false; + } normalizeActiveIndexByFamily(storage, storage.accounts.length, { clearFamilyMapWhenEmpty: true, }); + return true; }; const isFlaggableFailure = (failure: Extract): boolean => { @@ -2881,11 +2889,23 @@ while (attempted.size < Math.max(1, accountCount)) { } if (removeFromActive.size > 0) { - workingStorage.accounts = workingStorage.accounts.filter( - (account) => !removeFromActive.has(account.refreshToken), - ); - clampActiveIndices(workingStorage); - storageChanged = true; + const indexesToRemove: number[] = []; + for (let i = 0; i < workingStorage.accounts.length; i += 1) { + const account = workingStorage.accounts[i]; + if (account && removeFromActive.has(account.refreshToken)) { + indexesToRemove.push(i); + } + } + indexesToRemove.sort((a, b) => b - a); + let removedAny = false; + for (const indexToRemove of indexesToRemove) { + if (reconcileActiveIndicesAfterRemoval(workingStorage, indexToRemove)) { + removedAny = true; + } + } + if (removedAny) { + storageChanged = true; + } } if (storageChanged) { @@ -3077,8 +3097,13 @@ while (attempted.size < Math.max(1, accountCount)) { if (typeof menuResult.deleteAccountIndex === "number") { const target = workingStorage.accounts[menuResult.deleteAccountIndex]; if (target) { - workingStorage.accounts.splice(menuResult.deleteAccountIndex, 1); - clampActiveIndices(workingStorage); + const removed = reconcileActiveIndicesAfterRemoval( + workingStorage, + menuResult.deleteAccountIndex, + ); + if (!removed) { + continue; + } await saveAccounts(workingStorage); await saveFlaggedAccounts({ version: 1, diff --git a/lib/codex-manager.ts b/lib/codex-manager.ts index fc58a85..0cfa321 100644 --- a/lib/codex-manager.ts +++ b/lib/codex-manager.ts @@ -3604,7 +3604,11 @@ async function handleManageAction( if (typeof menuResult.deleteAccountIndex === "number") { const idx = menuResult.deleteAccountIndex; if (idx >= 0 && idx < storage.accounts.length) { - removeAccountAndReconcileActiveIndexes(storage, idx); + const removed = removeAccountAndReconcileActiveIndexes(storage, idx); + if (!removed) { + console.error(`Invalid account index: ${idx + 1}.`); + return; + } await saveAccounts(storage); console.log(`Deleted account ${idx + 1}.`); } diff --git a/test/codex-manager-cli.test.ts b/test/codex-manager-cli.test.ts index d4339f6..245672f 100644 --- a/test/codex-manager-cli.test.ts +++ b/test/codex-manager-cli.test.ts @@ -2021,6 +2021,53 @@ describe("codex manager cli commands", () => { expect(saved?.activeIndexByFamily?.["gpt-5.1"]).toBe(1); }); + it("does not persist or log delete success for fractional manage index", async () => { + const now = Date.now(); + loadAccountsMock.mockResolvedValue({ + version: 3, + activeIndex: 0, + activeIndexByFamily: { codex: 0 }, + accounts: [ + { + email: "first@example.com", + refreshToken: "refresh-first", + addedAt: now - 2_000, + lastUsed: now - 2_000, + enabled: true, + }, + { + email: "second@example.com", + refreshToken: "refresh-second", + addedAt: now - 1_000, + lastUsed: now - 1_000, + enabled: true, + }, + ], + }); + promptLoginModeMock + .mockResolvedValueOnce({ mode: "manage", deleteAccountIndex: 0.5 }) + .mockResolvedValueOnce({ mode: "cancel" }); + + const logSpy = vi.spyOn(console, "log").mockImplementation(() => {}); + const errorSpy = vi.spyOn(console, "error").mockImplementation(() => {}); + try { + const { runCodexMultiAuthCli } = await import("../lib/codex-manager.js"); + const exitCode = await runCodexMultiAuthCli(["auth", "login"]); + + expect(exitCode).toBe(0); + expect(saveAccountsMock).not.toHaveBeenCalled(); + expect( + logSpy.mock.calls.some((call) => String(call[0]).includes("Deleted account")), + ).toBe(false); + expect( + errorSpy.mock.calls.some((call) => String(call[0]).includes("Invalid account index")), + ).toBe(true); + } finally { + logSpy.mockRestore(); + errorSpy.mockRestore(); + } + }); + it("toggles account enabled state from manage mode", async () => { const now = Date.now(); loadAccountsMock.mockResolvedValue({ diff --git a/test/index.test.ts b/test/index.test.ts index 69e8cd1..a98ea17 100644 --- a/test/index.test.ts +++ b/test/index.test.ts @@ -171,12 +171,14 @@ vi.mock("../lib/rotation.js", () => ({ vi.mock("../lib/prompts/codex.js", () => ({ getModelFamily: (model: string) => { + if (model.includes("gpt-5-codex")) return "gpt-5-codex"; if (model.includes("codex-max")) return "codex-max"; + if (model.includes("gpt-5.2")) return "gpt-5.2"; if (model.includes("codex")) return "codex"; return "gpt-5.1"; }, getCodexInstructions: vi.fn(async () => "test instructions"), - MODEL_FAMILIES: ["codex-max", "codex", "gpt-5.1"] as const, + MODEL_FAMILIES: ["gpt-5-codex", "codex-max", "codex", "gpt-5.2", "gpt-5.1"] as const, prewarmCodexInstructions: vi.fn(), })); @@ -1007,6 +1009,34 @@ describe("OpenAIOAuthPlugin edge cases", () => { expect(mockStorage.activeIndexByFamily.codex).toBe(1); }); + it("reindexes per-family active indices when removing index 0", async () => { + mockStorage.accounts = [ + { refreshToken: "r1", email: "user1@example.com" }, + { refreshToken: "r2", email: "user2@example.com" }, + { refreshToken: "r3", email: "user3@example.com" }, + ]; + mockStorage.activeIndex = 2; + mockStorage.activeIndexByFamily = { + "gpt-5-codex": 0, + "codex-max": 2, + codex: 2, + "gpt-5.2": 2, + "gpt-5.1": 1, + }; + + const mockClient = createMockClient(); + const { OpenAIOAuthPlugin } = await import("../index.js"); + const plugin = await OpenAIOAuthPlugin({ client: mockClient } as never) as unknown as PluginType; + + await plugin.tool["codex-remove"].execute({ index: 1 }); + expect(mockStorage.activeIndex).toBe(1); + expect(mockStorage.activeIndexByFamily["gpt-5-codex"]).toBe(0); + expect(mockStorage.activeIndexByFamily["codex-max"]).toBe(1); + expect(mockStorage.activeIndexByFamily.codex).toBe(1); + expect(mockStorage.activeIndexByFamily["gpt-5.2"]).toBe(1); + expect(mockStorage.activeIndexByFamily["gpt-5.1"]).toBe(0); + }); + it("resets activeIndex when removing active account at end", async () => { mockStorage.accounts = [ { refreshToken: "r1", email: "user1@example.com" }, From 786a5c05017ead02373cf1beba5e1d7f283b44bc Mon Sep 17 00:00:00 2001 From: ndycode Date: Thu, 5 Mar 2026 11:18:26 +0800 Subject: [PATCH 11/13] fix: normalize codex-remove family indices after deletion - normalize activeIndexByFamily after codex-remove reconciliation to align with other delete paths - add regression for removal when activeIndexByFamily is undefined Co-authored-by: Codex --- index.ts | 8 +++++++- test/index.test.ts | 20 ++++++++++++++++++++ 2 files changed, 27 insertions(+), 1 deletion(-) diff --git a/index.ts b/index.ts index 585e125..593c772 100644 --- a/index.ts +++ b/index.ts @@ -3880,7 +3880,13 @@ while (attempted.size < Math.max(1, accountCount)) { const label = formatAccountLabel(account, targetIndex); - removeAccountAndReconcileActiveIndexes(storage, targetIndex); + const removed = removeAccountAndReconcileActiveIndexes(storage, targetIndex); + if (!removed) { + return `Account ${index} not found.`; + } + normalizeActiveIndexByFamily(storage, storage.accounts.length, { + clearFamilyMapWhenEmpty: true, + }); try { await saveAccounts(storage); diff --git a/test/index.test.ts b/test/index.test.ts index a98ea17..c3ab402 100644 --- a/test/index.test.ts +++ b/test/index.test.ts @@ -813,6 +813,26 @@ describe("OpenAIOAuthPlugin", () => { expect(mockStorage.activeIndex).toBe(0); expect(mockStorage.activeIndexByFamily).toEqual({}); }); + + it("normalizes family active-index map after remove when map is undefined", async () => { + mockStorage.accounts = [ + { refreshToken: "r1", email: "user1@example.com" }, + { refreshToken: "r2", email: "user2@example.com" }, + { refreshToken: "r3", email: "user3@example.com" }, + ]; + mockStorage.activeIndex = 2; + mockStorage.activeIndexByFamily = undefined as unknown as Record; + + const mockClient = createMockClient(); + const { OpenAIOAuthPlugin } = await import("../index.js"); + const plugin = await OpenAIOAuthPlugin({ client: mockClient } as never) as unknown as PluginType; + + await plugin.tool["codex-remove"].execute({ index: 1 }); + const { MODEL_FAMILIES } = await import("../lib/prompts/codex.js"); + for (const family of MODEL_FAMILIES) { + expect(mockStorage.activeIndexByFamily[family]).toBe(1); + } + }); }); describe("codex-refresh tool", () => { From ce93aa07840d12d27c6a36ecffa5d6522edfd067 Mon Sep 17 00:00:00 2001 From: ndycode Date: Thu, 5 Mar 2026 11:38:20 +0800 Subject: [PATCH 12/13] fix: reconcile family indexes on legacy account deletion Ensure removeAccountAndReconcileActiveIndexes initializes and updates activeIndexByFamily even when legacy storage omits that field. Add regression coverage for deletion from legacy-shaped storage. Co-authored-by: Codex --- lib/accounts/active-index.ts | 28 ++++++++++++++-------------- test/active-index.test.ts | 20 ++++++++++++++++++++ 2 files changed, 34 insertions(+), 14 deletions(-) diff --git a/lib/accounts/active-index.ts b/lib/accounts/active-index.ts index 5d51809..6ac9ce0 100644 --- a/lib/accounts/active-index.ts +++ b/lib/accounts/active-index.ts @@ -118,20 +118,20 @@ export function removeAccountAndReconcileActiveIndexes( storage.activeIndex = previousActive; } - if (storage.activeIndexByFamily) { - for (const family of families) { - const idx = storage.activeIndexByFamily[family]; - const base = typeof idx === "number" - ? clampIndex(idx, previousCount) - : previousActive; - const next = base > targetIndex - ? base - 1 - : base === targetIndex - ? Math.min(targetIndex, storage.accounts.length - 1) - : base; - if (storage.activeIndexByFamily[family] !== next) { - storage.activeIndexByFamily[family] = next; - } + const activeIndexByFamily = storage.activeIndexByFamily ?? {}; + storage.activeIndexByFamily = activeIndexByFamily; + for (const family of families) { + const idx = activeIndexByFamily[family]; + const base = typeof idx === "number" + ? clampIndex(idx, previousCount) + : previousActive; + const next = base > targetIndex + ? base - 1 + : base === targetIndex + ? Math.min(targetIndex, storage.accounts.length - 1) + : base; + if (activeIndexByFamily[family] !== next) { + activeIndexByFamily[family] = next; } } diff --git a/test/active-index.test.ts b/test/active-index.test.ts index 4245fde..d8018a3 100644 --- a/test/active-index.test.ts +++ b/test/active-index.test.ts @@ -162,6 +162,26 @@ describe("active-index helpers", () => { expect(storage.activeIndexByFamily?.["gpt-5.1"]).toBe(1); }); + it("initializes model-family indexes when removing from legacy storage without family map", () => { + const storage: { + accounts: unknown[]; + activeIndex: number; + activeIndexByFamily?: Partial>; + } = { + accounts: [{ id: 1 }, { id: 2 }, { id: 3 }], + activeIndex: 1, + }; + + const changed = removeAccountAndReconcileActiveIndexes(storage, 0); + + expect(changed).toBe(true); + expect(storage.accounts).toHaveLength(2); + expect(storage.activeIndex).toBe(0); + for (const family of MODEL_FAMILIES) { + expect(storage.activeIndexByFamily?.[family]).toBe(0); + } + }); + it("returns false and keeps storage unchanged for out-of-range removals", () => { const storage: { accounts: unknown[]; From d818232a19e3577e995f2a0b4a6931199b3b238b Mon Sep 17 00:00:00 2001 From: ndycode Date: Thu, 5 Mar 2026 11:48:57 +0800 Subject: [PATCH 13/13] fix: preserve undefined family map in storage cloning Keep cloneAccountStorage from normalizing undefined activeIndexByFamily to an empty object so legacy-shaped snapshots do not trigger spurious changed-state signals. Add regression coverage. Co-authored-by: Codex --- lib/accounts/storage-view.ts | 2 +- test/account-storage-view.test.ts | 11 +++++++++++ 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/lib/accounts/storage-view.ts b/lib/accounts/storage-view.ts index 9ab2258..734da77 100644 --- a/lib/accounts/storage-view.ts +++ b/lib/accounts/storage-view.ts @@ -31,6 +31,6 @@ export function cloneAccountStorage(storage: AccountStorageV3): AccountStorageV3 activeIndex: storage.activeIndex, activeIndexByFamily: storage.activeIndexByFamily ? { ...storage.activeIndexByFamily } - : {}, + : undefined, }; } diff --git a/test/account-storage-view.test.ts b/test/account-storage-view.test.ts index 9d090f2..4e27852 100644 --- a/test/account-storage-view.test.ts +++ b/test/account-storage-view.test.ts @@ -48,4 +48,15 @@ describe("account storage view helpers", () => { expect(original.accounts[0]!.rateLimitResetTimes?.codex).toBe(123_000); expect(original.activeIndexByFamily.codex).toBe(0); }); + + it("preserves undefined family index map when cloning legacy-shaped storage", () => { + const clone = cloneAccountStorage({ + version: 3, + accounts: [], + activeIndex: 0, + activeIndexByFamily: undefined, + }); + + expect(clone.activeIndexByFamily).toBeUndefined(); + }); });