fix(codex-manager): close the menu quota-refresh write races

Two related races flagged by review on #540/#547, both pre-existing
before the login-machinery extraction:

- Last-write-wins clobber: refreshQuotaCacheForMenu probed against a
  snapshot clone and then saved it whole-file, silently discarding any
  entries a concurrent writer (deep check, second session) persisted
  while the probes ran. The save now reloads the freshest persisted
  cache and re-applies this run's successful probe results onto it,
  falling back to the clone if the reload fails.

- Orphaned in-flight refresh: leaving the dashboard (add-account,
  cancel, empty-storage onboarding) abandoned a running refresh whose
  background cache save could race the subsequent account-pool write
  (Windows EBUSY/EPERM on sibling files). The three exit paths now
  drain the pending refresh first; the wait is bounded by the per-probe
  HTTP timeouts and never rejects.

New regression suite pins the rebase-on-save behavior (concurrent entry
preserved, reload-failure fallback, no save when nothing changed). The
cli suite's loadQuotaCache mock now returns a fresh object per call
like the real fresh-disk-read implementation, and the two
refresh-orchestration tests settle pass 1 deterministically via the
statusMessage observable instead of relying on microtask counts.

https://claude.ai/code/session_01XNtnkLbBiXZxfQQYLMpucB

Author: claude

lib/codex-manager/login-flow.ts

@@ -86,6 +86,20 @@ function clearMenuQuotaAutoRefreshSkip(state: MenuQuotaRefreshState): void {
 	state.menuQuotaRefreshGeneration += 1;
 }
 
+// The menu quota refresh runs fire-and-forget behind the dashboard. On the
+// paths that leave the loop (add-account's storage write, process exit) an
+// in-flight refresh must finish first so its cache save cannot race the
+// account-pool write (Windows EBUSY/EPERM on sibling files) or be orphaned
+// mid-write. The chain never rejects (it ends in .catch/.finally), and the
+// wait is bounded by the per-probe HTTP timeouts.
+async function drainPendingMenuQuotaRefresh(
+	state: MenuQuotaRefreshState,
+): Promise<void> {
+	if (state.pendingMenuQuotaRefresh) {
+		await state.pendingMenuQuotaRefresh;
+	}
+}
+
 const log = createLogger("codex-manager");
 
 async function clearAccountsAndReset(): Promise<void> {
@@ -126,6 +140,7 @@ async function runLoginDashboardLoop(
 	while (true) {
 		const existingStorage = await loadAccounts();
 		if (!existingStorage || existingStorage.accounts.length === 0) {
+			await drainPendingMenuQuotaRefresh(menuState);
 			return "add-account";
 		}
 		const currentStorage = existingStorage;
@@ -203,6 +218,7 @@ async function runLoginDashboardLoop(
 
 		if (menuResult.mode === "cancel") {
 			console.log("Cancelled.");
+			await drainPendingMenuQuotaRefresh(menuState);
 			return "exit";
 		}
 		if (menuResult.mode === "check") {
@@ -304,6 +320,7 @@ async function runLoginDashboardLoop(
 			continue;
 		}
 		if (menuResult.mode === "add") {
+			await drainPendingMenuQuotaRefresh(menuState);
 			return "add-account";
 		}
 	}

lib/codex-manager/login-menu-data.ts

@@ -8,11 +8,15 @@ import {
 	DEFAULT_DASHBOARD_DISPLAY_SETTINGS,
 } from "../dashboard-settings.js";
 import {
+	loadQuotaCache,
 	type QuotaCacheData,
 	type QuotaCacheEntry,
 	saveQuotaCache,
 } from "../quota-cache.js";
-import { fetchCodexQuotaSnapshot } from "../quota-probe.js";
+import {
+	type CodexQuotaSnapshot,
+	fetchCodexQuotaSnapshot,
+} from "../quota-probe.js";
 import {
 	buildQuotaEmailFallbackState,
 	hasSafeQuotaEmailFallback,
@@ -181,6 +185,10 @@ export async function refreshQuotaCacheForMenu(
 	let processed = 0;
 	onProgress?.(processed, total);
 	let changed = false;
+	const appliedSnapshots: {
+		account: AccountMetadataV3;
+		snapshot: CodexQuotaSnapshot;
+	}[] = [];
 	for (const target of targets) {
 		processed += 1;
 		onProgress?.(processed, total);
@@ -191,22 +199,45 @@ export async function refreshQuotaCacheForMenu(
 				accessToken: target.accessToken,
 				model: MENU_QUOTA_REFRESH_MODEL,
 			});
-			changed =
-				updateQuotaCacheForAccount(
-					nextCache,
-					target.account,
-					snapshot,
-					storage.accounts,
-					emailFallbackState,
-				) || changed;
+			const applied = updateQuotaCacheForAccount(
+				nextCache,
+				target.account,
+				snapshot,
+				storage.accounts,
+				emailFallbackState,
+			);
+			if (applied) appliedSnapshots.push({ account: target.account, snapshot });
+			changed = applied || changed;
 		} catch {
 			// Keep existing cached values if probing fails.
 		}
 	}
 
 	if (changed) {
+		// The probes above ran against a snapshot clone of the cache the menu
+		// loaded; another writer (a deep check, a second session) may have saved
+		// entries meanwhile, and writing the stale clone back whole-file would
+		// silently discard them (last write wins). Re-apply this run's results
+		// onto the freshest persisted cache instead.
+		let cacheToSave = nextCache;
+		try {
+			const persisted = await loadQuotaCache();
+			for (const { account, snapshot } of appliedSnapshots) {
+				updateQuotaCacheForAccount(
+					persisted,
+					account,
+					snapshot,
+					storage.accounts,
+					emailFallbackState,
+				);
+			}
+			cacheToSave = persisted;
+		} catch {
+			// Fall back to the snapshot clone; saving slightly stale data beats
+			// dropping this run's probe results.
+		}
 		try {
-			await saveQuotaCache(nextCache);
+			await saveQuotaCache(cacheToSave);
 		} catch (error) {
 			// Quota cache is a derived artifact; a transient Windows EBUSY/EPERM
 			// here must not fail the menu refresh, but it should not vanish into
@@ -216,6 +247,7 @@ export async function refreshQuotaCacheForMenu(
 				`Quota cache save failed: ${error instanceof Error ? error.message : String(error)}`,
 			);
 		}
+		return cacheToSave;
 	}
 
 	return nextCache;

test/codex-manager-cli.test.ts

@@ -750,10 +750,14 @@ describe("codex manager cli commands", () => {
 			primary: {},
 			secondary: {},
 		});
-		loadQuotaCacheMock.mockResolvedValue({
+		// Fresh object per call, like the real loadQuotaCache (a fresh disk read
+		// each time): refreshQuotaCacheForMenu rebases onto and mutates the
+		// loaded cache before saving, and a shared singleton here would leak
+		// that mutation into every later load in the same test.
+		loadQuotaCacheMock.mockImplementation(async () => ({
 			byAccountId: {},
 			byEmail: {},
-		});
+		}));
 		loadFlaggedAccountsMock.mockResolvedValue({
 			version: 1,
 			accounts: [],
@@ -9073,7 +9077,7 @@ describe("codex manager cli commands", () => {
 
 		let promptCallCount = 0;
 		promptLoginModeMock
-			.mockImplementationOnce(async (accounts) => {
+			.mockImplementationOnce(async (accounts, options) => {
 				promptCallCount += 1;
 				expect(promptCallCount).toBe(1);
 				expect(
@@ -9086,6 +9090,14 @@ describe("codex manager cli commands", () => {
 				queueMicrotask(() => {
 					releaseFirstRefresh.resolve();
 				});
+				// Wait for the first refresh to fully settle (statusMessage clears in
+				// the same .finally that releases the pending slot) so the second
+				// menu pass deterministically starts its own refresh; the refresh
+				// chain gained an await (the save-time cache rebase), so returning
+				// immediately could reach the pass-2 guard while pass 1 is pending.
+				await vi.waitFor(() => {
+					expect(options?.statusMessage?.()).toBeUndefined();
+				});
 				return { mode: "manage", deleteAccountIndex: 99 };
 			})
 			.mockImplementationOnce(async (accounts, options) => {
@@ -9258,13 +9270,18 @@ describe("codex manager cli commands", () => {
 
 		let promptCallCount = 0;
 		promptLoginModeMock
-			.mockImplementationOnce(async () => {
+			.mockImplementationOnce(async (_accounts, options) => {
 				promptCallCount += 1;
 				expect(promptCallCount).toBe(1);
 
 				queueMicrotask(() => {
 					releaseFirstRefresh.resolve();
 				});
+				// See the stale-refresh test above: settle pass 1's refresh before
+				// returning so pass 2's auto-fetch guard sees a free slot.
+				await vi.waitFor(() => {
+					expect(options?.statusMessage?.()).toBeUndefined();
+				});
 				return { mode: "manage", deleteAccountIndex: 99 };
 			})
 			.mockImplementationOnce(async (_accounts, options) => {

test/codex-manager-login-menu-refresh.test.ts

@@ -0,0 +1,149 @@
+import { beforeEach, describe, expect, it, vi } from "vitest";
+import type { QuotaCacheData } from "../lib/quota-cache.js";
+import type { AccountStorageV3 } from "../lib/storage.js";
+
+const { loadQuotaCacheMock, saveQuotaCacheMock, fetchCodexQuotaSnapshotMock } =
+	vi.hoisted(() => ({
+		loadQuotaCacheMock: vi.fn(),
+		saveQuotaCacheMock: vi.fn(),
+		fetchCodexQuotaSnapshotMock: vi.fn(),
+	}));
+
+vi.mock("../lib/quota-cache.js", () => ({
+	loadQuotaCache: loadQuotaCacheMock,
+	saveQuotaCache: saveQuotaCacheMock,
+}));
+
+vi.mock("../lib/quota-probe.js", () => ({
+	fetchCodexQuotaSnapshot: fetchCodexQuotaSnapshotMock,
+}));
+
+const { refreshQuotaCacheForMenu } = await import(
+	"../lib/codex-manager/login-menu-data.js"
+);
+
+function createStorage(now: number): AccountStorageV3 {
+	return {
+		version: 3,
+		activeIndex: 0,
+		activeIndexByFamily: { codex: 0 },
+		accounts: [
+			{
+				email: "a@example.com",
+				accountId: "acc_a",
+				refreshToken: "refresh-a",
+				accessToken: "access-a",
+				expiresAt: now + 3_600_000,
+				addedAt: now - 60_000,
+				lastUsed: now - 60_000,
+			},
+			{
+				email: "b@example.com",
+				accountId: "acc_b",
+				refreshToken: "refresh-b",
+				accessToken: "access-b",
+				expiresAt: now + 3_600_000,
+				addedAt: now - 60_000,
+				lastUsed: now - 60_000,
+			},
+		],
+	};
+}
+
+function emptyCache(): QuotaCacheData {
+	return { byAccountId: {}, byEmail: {} };
+}
+
+function snapshotFor(model: string) {
+	return {
+		status: 200,
+		model,
+		primary: { usedPercent: 10, windowMinutes: 300, resetAtMs: 1 },
+		secondary: { usedPercent: 5, windowMinutes: 10080, resetAtMs: 2 },
+	};
+}
+
+const CONCURRENT_ENTRY = {
+	updatedAt: 1,
+	status: 429,
+	model: "gpt-5-codex",
+	primary: { usedPercent: 100, windowMinutes: 300, resetAtMs: 99 },
+	secondary: {},
+};
+
+beforeEach(() => {
+	loadQuotaCacheMock.mockReset();
+	saveQuotaCacheMock.mockReset();
+	fetchCodexQuotaSnapshotMock.mockReset();
+	saveQuotaCacheMock.mockResolvedValue(undefined);
+	fetchCodexQuotaSnapshotMock.mockResolvedValue(snapshotFor("gpt-5-codex"));
+});
+
+describe("refreshQuotaCacheForMenu", () => {
+	it("rebases its results onto the freshest persisted cache before saving", async () => {
+		// Regression for the last-write-wins clobber: a concurrent writer (deep
+		// check, second session) saved acc_concurrent while the menu refresh was
+		// probing against its stale snapshot clone. The whole-file save must keep
+		// that entry, not silently discard it.
+		loadQuotaCacheMock.mockResolvedValue({
+			byAccountId: { acc_concurrent: { ...CONCURRENT_ENTRY } },
+			byEmail: {},
+		});
+
+		const result = await refreshQuotaCacheForMenu(
+			createStorage(Date.now()),
+			emptyCache(),
+			60_000,
+		);
+
+		expect(saveQuotaCacheMock).toHaveBeenCalledTimes(1);
+		const saved = saveQuotaCacheMock.mock.calls[0][0] as QuotaCacheData;
+		expect(saved.byAccountId.acc_concurrent).toMatchObject({ status: 429 });
+		expect(saved.byAccountId.acc_a).toMatchObject({ status: 200 });
+		expect(saved.byAccountId.acc_b).toMatchObject({ status: 200 });
+		expect(result).toBe(saved);
+	});
+
+	it("falls back to saving its own snapshot clone when the reload fails", async () => {
+		loadQuotaCacheMock.mockRejectedValue(new Error("EBUSY"));
+
+		const result = await refreshQuotaCacheForMenu(
+			createStorage(Date.now()),
+			emptyCache(),
+			60_000,
+		);
+
+		expect(saveQuotaCacheMock).toHaveBeenCalledTimes(1);
+		const saved = saveQuotaCacheMock.mock.calls[0][0] as QuotaCacheData;
+		expect(saved.byAccountId.acc_a).toMatchObject({ status: 200 });
+		expect(saved.byAccountId.acc_b).toMatchObject({ status: 200 });
+		expect(result).toBe(saved);
+	});
+
+	it("does not reload or save when every probe fails", async () => {
+		fetchCodexQuotaSnapshotMock.mockRejectedValue(new Error("network"));
+
+		const cache = emptyCache();
+		const result = await refreshQuotaCacheForMenu(
+			createStorage(Date.now()),
+			cache,
+			60_000,
+		);
+
+		expect(loadQuotaCacheMock).not.toHaveBeenCalled();
+		expect(saveQuotaCacheMock).not.toHaveBeenCalled();
+		expect(result).toEqual(cache);
+	});
+
+	it("returns the input cache untouched when there are no accounts", async () => {
+		const cache = emptyCache();
+		const result = await refreshQuotaCacheForMenu(
+			{ version: 3, activeIndex: 0, activeIndexByFamily: {}, accounts: [] },
+			cache,
+			60_000,
+		);
+		expect(result).toBe(cache);
+		expect(fetchCodexQuotaSnapshotMock).not.toHaveBeenCalled();
+		expect(saveQuotaCacheMock).not.toHaveBeenCalled();
+	});
+});