Hardening: close hidden production landmines in auth/storage/recovery paths

index.ts

@@ -1602,6 +1602,7 @@ while (attempted.size < Math.max(1, accountCount)) {
 									{
 										model,
 										promptCacheKey: effectivePromptCacheKey,
+										idempotencyKey: requestCorrelationId,
 									},
 								);
 								const quotaScheduleKey = `${entitlementAccountKey}:${model ?? modelFamily}`;
@@ -2164,6 +2165,7 @@ while (attempted.size < Math.max(1, accountCount)) {
 											{
 												model,
 												promptCacheKey: effectivePromptCacheKey,
+												idempotencyKey: requestCorrelationId,
 											},
 										);
 

lib/accounts.ts

@@ -16,7 +16,7 @@ import {
 	type AccountWithMetrics,
 	type HybridSelectionOptions,
 } from "./rotation.js";
-import { nowMs } from "./utils.js";
+import { isRecord, nowMs, sleep } from "./utils.js";
 import {
 	loadCodexCliState,
 	type CodexCliTokenCacheEntry,
@@ -72,6 +72,8 @@ import {
 } from "./accounts/rate-limits.js";
 
 const log = createLogger("accounts");
+type StoredAccount = AccountStorageV3["accounts"][number];
+const DISK_PREFERRED_MERGE_KEYS = new Set(["refreshToken", "accessToken", "expiresAt"]);
 
 function initFamilyState(defaultValue: number): Record<ModelFamily, number> {
 	return Object.fromEntries(
@@ -724,7 +726,7 @@ export class AccountManager {
 		return account;
 	}
 
-	async saveToDisk(): Promise<void> {
+	private buildStorageSnapshot(): AccountStorageV3 {
 		const activeIndexByFamily: Partial<Record<ModelFamily, number>> = {};
 		for (const family of MODEL_FAMILIES) {
 			const raw = this.currentAccountIndexByFamily[family];
@@ -755,8 +757,197 @@ export class AccountManager {
 			activeIndex,
 			activeIndexByFamily,
 		};
+		return storage;
+	}
+
+	private isStorageConflictError(error: unknown): boolean {
+		const code = (error as NodeJS.ErrnoException | undefined)?.code;
+		return code === "ECONFLICT";
+	}
+
+	private mergeIntoLatestStorage(
+		latest: AccountStorageV3 | null,
+		local: AccountStorageV3,
+	): AccountStorageV3 {
+		if (!latest) {
+			return local;
+		}
+
+		const mergedAccounts = latest.accounts.map((account) => ({ ...account }));
+		const claimIndex = (candidate: StoredAccount): number => {
+			const token = candidate.refreshToken.trim();
+			const accountId = candidate.accountId?.trim();
+			const email = sanitizeEmail(candidate.email);
+
+			const byToken = mergedAccounts.findIndex(
+				(account) => account.refreshToken.trim() === token,
+			);
+			if (byToken >= 0) return byToken;
+
+			if (accountId) {
+				const byAccountId = mergedAccounts.findIndex(
+					(account) => (account.accountId?.trim() ?? "") === accountId,
+				);
+				if (byAccountId >= 0) return byAccountId;
+			}
+
+			if (email) {
+				const byEmail = mergedAccounts.findIndex(
+					(account) => sanitizeEmail(account.email) === email,
+				);
+				if (byEmail >= 0) return byEmail;
+			}
+
+			return -1;
+		};
+
+		for (const account of local.accounts) {
+			const idx = claimIndex(account);
+			if (idx >= 0) {
+				const current = mergedAccounts[idx];
+				if (current) {
+					mergedAccounts[idx] = this.mergeStoredAccountRecords(current, account);
+				}
+			} else {
+				mergedAccounts.push({ ...account });
+			}
+		}
+
+		const localActiveTokensByFamily = Object.fromEntries(
+			MODEL_FAMILIES.map((family) => {
+				const localIndex = local.activeIndexByFamily?.[family];
+				const token =
+					typeof localIndex === "number" && localIndex >= 0
+						? local.accounts[localIndex]?.refreshToken
+						: undefined;
+				return [family, token];
+			}),
+		) as Partial<Record<ModelFamily, string | undefined>>;
+
+		const mergedActiveIndexByFamily: Partial<Record<ModelFamily, number>> = {};
+		for (const family of MODEL_FAMILIES) {
+			const token = localActiveTokensByFamily[family];
+			if (token) {
+				const index = mergedAccounts.findIndex(
+					(account) => account.refreshToken === token,
+				);
+				if (index >= 0) {
+					mergedActiveIndexByFamily[family] = index;
+					continue;
+				}
+			}
+			mergedActiveIndexByFamily[family] = clampNonNegativeInt(
+				latest.activeIndexByFamily?.[family],
+				0,
+			);
+		}
+
+		return {
+			version: 3,
+			accounts: mergedAccounts,
+			activeIndex: clampNonNegativeInt(mergedActiveIndexByFamily.codex, 0),
+			activeIndexByFamily: mergedActiveIndexByFamily,
+		};
+	}
+
+	private mergeStoredAccountRecords(current: StoredAccount, incoming: StoredAccount): StoredAccount {
+		const next: StoredAccount = { ...current };
+		const nextRecord = next as unknown as Record<string, unknown>;
+		for (const [rawKey, rawValue] of Object.entries(incoming)) {
+			const value = rawValue as unknown;
+			if (value === undefined) {
+				continue;
+			}
+			const currentValue = nextRecord[rawKey];
+			if (DISK_PREFERRED_MERGE_KEYS.has(rawKey) && currentValue !== undefined) {
+				continue;
+			}
+			if (
+				(rawKey === "lastUsed" || rawKey === "addedAt" || rawKey === "coolingDownUntil") &&
+				typeof currentValue === "number" &&
+				typeof value === "number"
+			) {
+				nextRecord[rawKey] = Math.max(currentValue, value);
+				continue;
+			}
+			if (rawKey === "rateLimitResetTimes" && isRecord(currentValue) && isRecord(value)) {
+				const mergedRateLimits: Record<string, unknown> = { ...currentValue };
+				for (const [resetKey, resetValue] of Object.entries(value)) {
+					if (resetValue === undefined) {
+						continue;
+					}
+					const existingResetValue = mergedRateLimits[resetKey];
+					if (typeof existingResetValue === "number" && typeof resetValue === "number") {
+						mergedRateLimits[resetKey] = Math.max(existingResetValue, resetValue);
+						continue;
+					}
+					mergedRateLimits[resetKey] = resetValue;
+				}
+				nextRecord[rawKey] = mergedRateLimits;
+				continue;
+			}
+			if (isRecord(currentValue) && isRecord(value)) {
+				nextRecord[rawKey] = {
+					...currentValue,
+					...value,
+				};
+				continue;
+			}
+			nextRecord[rawKey] = value;
+		}
+		return next;
+	}
+
+	private applyPersistedStorageSnapshot(storage: AccountStorageV3): void {
+		const previousByRefreshToken = new Map(
+			this.accounts.map((account) => [account.refreshToken, account] as const),
+		);
+		const rehydrated = new AccountManager(undefined, storage);
+		this.accounts = rehydrated.accounts.map((account) => {
+			const previous = previousByRefreshToken.get(account.refreshToken);
+			if (!previous) {
+				return account;
+			}
+			return {
+				...account,
+				lastRateLimitReason: previous.lastRateLimitReason,
+				consecutiveAuthFailures: previous.consecutiveAuthFailures,
+			};
+		});
+		this.cursorByFamily = { ...rehydrated.cursorByFamily };
+		this.currentAccountIndexByFamily = {
+			...rehydrated.currentAccountIndexByFamily,
+		};
+	}
+
+	private async persistStorageWithConflictRecovery(storage?: AccountStorageV3): Promise<void> {
+		const maxAttempts = 3;
+		const baseStorage = storage ?? this.buildStorageSnapshot();
+		let mergedCandidate = baseStorage;
+		for (let attempt = 0; attempt < maxAttempts; attempt += 1) {
+			try {
+				await saveAccounts(mergedCandidate);
+				if (attempt > 0) {
+					this.applyPersistedStorageSnapshot(mergedCandidate);
+				}
+				return;
+			} catch (error) {
+				if (!this.isStorageConflictError(error) || attempt + 1 >= maxAttempts) {
+					throw error;
+				}
+				log.warn("Account save conflict detected; retrying with merged disk snapshot", {
+					attempt: attempt + 1,
+					maxAttempts,
+				});
+				const latest = await loadAccounts();
+				mergedCandidate = this.mergeIntoLatestStorage(latest, baseStorage);
+				await sleep(20 * 2 ** attempt);
+			}
+		}
+	}
 
-		await saveAccounts(storage);
+	async saveToDisk(): Promise<void> {
+		await this.persistStorageWithConflictRecovery(this.buildStorageSnapshot());
 	}
 
 	saveToDiskDebounced(delayMs = 500): void {

lib/auth/auth.ts

@@ -3,7 +3,7 @@ import { randomBytes } from "node:crypto";
 import type { PKCEPair, AuthorizationFlow, TokenResult, ParsedAuthInput, JWTPayload } from "../types.js";
 import { logError } from "../logger.js";
 import { safeParseOAuthTokenResponse } from "../schemas.js";
-import { isAbortError } from "../utils.js";
+import { fetchWithTimeout, isAbortError } from "../utils.js";
 
 // OAuth constants (from openai/codex)
 export const CLIENT_ID = "app_EMoamEEZ73f0CkXaXp7hrann";
@@ -18,6 +18,8 @@ const OAUTH_SENSITIVE_QUERY_PARAMS = [
 	"code_challenge",
 	"code_verifier",
 ] as const;
+const OAUTH_TOKEN_EXCHANGE_TIMEOUT_MS = 30_000;
+const OAUTH_REFRESH_TIMEOUT_MS = 30_000;
 
 function getOAuthResponseLogMetadata(rawResponse: unknown): Record<string, unknown> {
 	if (Array.isArray(rawResponse)) {
@@ -116,17 +118,26 @@ export async function exchangeAuthorizationCode(
 	verifier: string,
 	redirectUri: string = REDIRECT_URI,
 ): Promise<TokenResult> {
-	const res = await fetch(TOKEN_URL, {
-		method: "POST",
-		headers: { "Content-Type": "application/x-www-form-urlencoded" },
-		body: new URLSearchParams({
-			grant_type: "authorization_code",
-			client_id: CLIENT_ID,
-			code,
-			code_verifier: verifier,
-			redirect_uri: redirectUri,
-		}),
-	});
+	let res: Response;
+	try {
+		res = await fetchWithTimeout(TOKEN_URL, {
+			method: "POST",
+			headers: { "Content-Type": "application/x-www-form-urlencoded" },
+			body: new URLSearchParams({
+				grant_type: "authorization_code",
+				client_id: CLIENT_ID,
+				code,
+				code_verifier: verifier,
+				redirect_uri: redirectUri,
+			}),
+		}, OAUTH_TOKEN_EXCHANGE_TIMEOUT_MS);
+	} catch (error) {
+		const err = error instanceof Error ? error : new Error(String(error));
+		if (isAbortError(err) || /timeout/i.test(err.message)) {
+			return { type: "failed", reason: "unknown", message: err.message };
+		}
+		return { type: "failed", reason: "network_error", message: err.message };
+	}
 	if (!res.ok) {
 		const text = await res.text().catch(() => "");
 		logError(`code->token failed: ${res.status} ${text}`);
@@ -186,14 +197,15 @@ export function decodeJWT(token: string): JWTPayload | null {
  */
 type RefreshAccessTokenOptions = {
 	signal?: AbortSignal;
+	timeoutMs?: number;
 };
 
 export async function refreshAccessToken(
 	refreshToken: string,
 	options: RefreshAccessTokenOptions = {},
 ): Promise<TokenResult> {
 	try {
-		const response = await fetch(TOKEN_URL, {
+		const response = await fetchWithTimeout(TOKEN_URL, {
 			method: "POST",
 			headers: { "Content-Type": "application/x-www-form-urlencoded" },
 			signal: options?.signal,
@@ -202,7 +214,7 @@ export async function refreshAccessToken(
 				refresh_token: refreshToken,
 				client_id: CLIENT_ID,
 			}),
-		});
+		}, options.timeoutMs ?? OAUTH_REFRESH_TIMEOUT_MS);
 
 		if (!response.ok) {
 			const text = await response.text().catch(() => "");
@@ -233,8 +245,8 @@ export async function refreshAccessToken(
 			multiAccount: true,
 		};
 	} catch (error) {
-		const err = error as Error;
-		if (isAbortError(err)) {
+		const err = error instanceof Error ? error : new Error(String(error));
+		if (isAbortError(err) || /timeout/i.test(err.message)) {
 			return { type: "failed", reason: "unknown", message: err?.message ?? "Request aborted" };
 		}
 		logError("Token refresh error", err);