refactor: extract storage error hints

lib/errors.ts

@@ -164,3 +164,24 @@ export class CodexRateLimitError extends CodexError {
 		this.accountId = options?.accountId;
 	}
 }
+
+/**
+ * Storage-specific error with a filesystem code, target path, and user-facing hint.
+ */
+export class StorageError extends CodexError {
+	override readonly name = "StorageError";
+	readonly path: string;
+	readonly hint: string;
+
+	constructor(
+		message: string,
+		code: string,
+		path: string,
+		hint: string,
+		cause?: Error,
+	) {
+		super(message, { code, cause });
+		this.path = path;
+		this.hint = hint;
+	}
+}

lib/storage.ts

@@ -3,6 +3,7 @@ import { createHash } from "node:crypto";
 import { existsSync, promises as fs } from "node:fs";
 import { basename, dirname, isAbsolute, join, relative } from "node:path";
 import { ACCOUNT_LIMITS } from "./constants.js";
+import { StorageError } from "./errors.js";
 import { createLogger } from "./logger.js";
 import {
 	exportNamedBackupFile,
@@ -11,6 +12,12 @@ import {
 } from "./named-backup-export.js";
 import { MODEL_FAMILIES, type ModelFamily } from "./prompts/codex.js";
 import { AnyAccountStorageSchema, getValidationErrors } from "./schemas.js";
+
+import { toStorageError } from "./storage/error-hints.js";
+
+export { StorageError } from "./errors.js";
+export { formatStorageErrorHint } from "./storage/error-hints.js";
+
 import {
 	type AccountMetadataV1,
 	type AccountMetadataV3,
@@ -122,7 +129,9 @@ export interface NamedBackupSummary {
 	mtimeMs: number;
 }
 
-async function collectNamedBackups(storagePath: string): Promise<NamedBackupSummary[]> {
+async function collectNamedBackups(
+	storagePath: string,
+): Promise<NamedBackupSummary[]> {
 	const backupRoot = getNamedBackupRoot(storagePath);
 	let entries: Array<{ isFile(): boolean; name: string }>;
 	try {
@@ -147,12 +156,15 @@ async function collectNamedBackups(storagePath: string): Promise<NamedBackupSumm
 			if (!normalized || normalized.accounts.length === 0) continue;
 			const statsAfter = await fs.stat(candidatePath).catch(() => null);
 			if (statsAfter && statsAfter.mtimeMs !== statsBefore.mtimeMs) {
-				log.debug("backup file changed between stat and load, mtime may be stale", {
-					candidatePath,
-					fileName: entry.name,
-					beforeMtimeMs: statsBefore.mtimeMs,
-					afterMtimeMs: statsAfter.mtimeMs,
-				});
+				log.debug(
+					"backup file changed between stat and load, mtime may be stale",
+					{
+						candidatePath,
+						fileName: entry.name,
+						beforeMtimeMs: statsBefore.mtimeMs,
+						afterMtimeMs: statsAfter.mtimeMs,
+					},
+				);
 			}
 			candidates.push({
 				path: candidatePath,
@@ -161,17 +173,20 @@ async function collectNamedBackups(storagePath: string): Promise<NamedBackupSumm
 				mtimeMs: statsBefore.mtimeMs,
 			});
 		} catch (error) {
-			log.debug("Skipping named backup candidate after loadAccountsFromPath/fs.stat failure", {
-				candidatePath,
-				fileName: entry.name,
-				error: error instanceof Error
-					? {
-						message: error.message,
-						stack: error.stack,
-					}
-					: String(error),
-			});
-			continue;
+			log.debug(
+				"Skipping named backup candidate after loadAccountsFromPath/fs.stat failure",
+				{
+					candidatePath,
+					fileName: entry.name,
+					error:
+						error instanceof Error
+							? {
+									message: error.message,
+									stack: error.stack,
+								}
+							: String(error),
+				},
+			);
 		}
 	}
 
@@ -183,56 +198,6 @@ async function collectNamedBackups(storagePath: string): Promise<NamedBackupSumm
 	return candidates;
 }
 
-/**
- * Custom error class for storage operations with platform-aware hints.
- */
-export class StorageError extends Error {
-	readonly code: string;
-	readonly path: string;
-	readonly hint: string;
-
-	constructor(
-		message: string,
-		code: string,
-		path: string,
-		hint: string,
-		cause?: Error,
-	) {
-		super(message, { cause });
-		this.name = "StorageError";
-		this.code = code;
-		this.path = path;
-		this.hint = hint;
-	}
-}
-
-/**
- * Generate platform-aware troubleshooting hint based on error code.
- */
-export function formatStorageErrorHint(error: unknown, path: string): string {
-	const err = error as NodeJS.ErrnoException;
-	const code = err?.code || "UNKNOWN";
-	const isWindows = process.platform === "win32";
-
-	switch (code) {
-		case "EACCES":
-		case "EPERM":
-			return isWindows
-				? `Permission denied writing to ${path}. Check antivirus exclusions for this folder. Ensure you have write permissions.`
-				: `Permission denied writing to ${path}. Check folder permissions. Try: chmod 755 ~/.codex`;
-		case "EBUSY":
-			return `File is locked at ${path}. The file may be open in another program. Close any editors or processes accessing it.`;
-		case "ENOSPC":
-			return `Disk is full. Free up space and try again. Path: ${path}`;
-		case "EEMPTY":
-			return `File written but is empty. This may indicate a disk or filesystem issue. Path: ${path}`;
-		default:
-			return isWindows
-				? `Failed to write to ${path}. Check folder permissions and ensure path contains no special characters.`
-				: `Failed to write to ${path}. Check folder permissions and disk space.`;
-	}
-}
-
 let storageMutex: Promise<void> = Promise.resolve();
 const transactionSnapshotContext = new AsyncLocalStorage<{
 	snapshot: AccountStorageV3 | null;
@@ -949,7 +914,9 @@ export async function restoreAccountsFromBackup(
 		!relativePath.startsWith("..") &&
 		!isAbsolute(relativePath);
 	if (!isInsideBackupRoot) {
-		throw new Error(`Backup path must stay inside ${resolvedBackupRoot}: ${path}`);
+		throw new Error(
+			`Backup path must stay inside ${resolvedBackupRoot}: ${path}`,
+		);
 	}
 
 	const { normalized } = await (async () => {
@@ -958,15 +925,15 @@ export async function restoreAccountsFromBackup(
 		} catch (error) {
 			const code = (error as NodeJS.ErrnoException).code;
 			if (code === "ENOENT") {
-				throw new Error(
-					`Backup file no longer exists: ${path}`,
-				);
+				throw new Error(`Backup file no longer exists: ${path}`);
 			}
 			throw error;
 		}
 	})();
 	if (!normalized || normalized.accounts.length === 0) {
-		throw new Error(`Backup does not contain any accounts: ${resolvedBackupPath}`);
+		throw new Error(
+			`Backup does not contain any accounts: ${resolvedBackupPath}`,
+		);
 	}
 	if (options?.persist !== false) {
 		await saveAccounts(normalized);
@@ -1330,7 +1297,10 @@ function findCompatibleRefreshTokenMatchIndex<T extends AccountLike>(
 			matchingAccount = account;
 			continue;
 		}
-		const newest: T = selectNewestAccount(matchingAccount ?? undefined, account);
+		const newest: T = selectNewestAccount(
+			matchingAccount ?? undefined,
+			account,
+		);
 		if (newest === account) {
 			matchingIndex = i;
 			matchingAccount = account;
@@ -2034,23 +2004,20 @@ async function saveAccountsUnlocked(storage: AccountStorageV3): Promise<void> {
 		}
 
 		const err = error as NodeJS.ErrnoException;
-		const code = err?.code || "UNKNOWN";
-		const hint = formatStorageErrorHint(error, path);
+		const storageError = toStorageError(
+			`Failed to save accounts: ${err?.message || "Unknown error"}`,
+			error,
+			path,
+		);
 
 		log.error("Failed to save accounts", {
 			path,
-			code,
+			code: storageError.code,
 			message: err?.message,
-			hint,
+			hint: storageError.hint,
 		});
 
-		throw new StorageError(
-			`Failed to save accounts: ${err?.message || "Unknown error"}`,
-			code,
-			path,
-			hint,
-			err instanceof Error ? err : undefined,
-		);
+		throw storageError;
 	}
 }
 
@@ -2114,7 +2081,9 @@ export async function withAccountAndFlaggedStorageTransaction<T>(
 			accountStorage: AccountStorageV3,
 			flaggedStorage: FlaggedAccountStorageV1,
 		): Promise<void> => {
-			const previousAccounts = cloneAccountStorageForPersistence(state.snapshot);
+			const previousAccounts = cloneAccountStorageForPersistence(
+				state.snapshot,
+			);
 			const nextAccounts = cloneAccountStorageForPersistence(accountStorage);
 			await saveAccountsUnlocked(nextAccounts);
 			try {

lib/storage/error-hints.ts

@@ -0,0 +1,49 @@
+import { StorageError } from "../errors.js";
+
+function extractErrorCode(error: unknown): string {
+	const err = error as NodeJS.ErrnoException;
+	return err?.code || "UNKNOWN";
+}
+
+/**
+ * Format a user-facing hint for storage persistence failures based on errno code.
+ */
+export function formatStorageErrorHint(error: unknown, path: string): string {
+	const code = extractErrorCode(error);
+	const isWindows = process.platform === "win32";
+
+	switch (code) {
+		case "EACCES":
+		case "EPERM":
+			return isWindows
+				? `Permission denied writing to ${path}. Check antivirus exclusions for this folder. Ensure you have write permissions.`
+				: `Permission denied writing to ${path}. Check folder permissions. Try: chmod 755 ~/.codex`;
+		case "EBUSY":
+			return `File is locked at ${path}. The file may be open in another program. Close any editors or processes accessing it.`;
+		case "ENOENT":
+			return `Path does not exist: ${path}. Create the parent folder and try again.`;
+		case "ENOSPC":
+			return `Disk is full. Free up space and try again. Path: ${path}`;
+		default:
+			return isWindows
+				? `Failed to write to ${path}. Check folder permissions and ensure path contains no special characters.`
+				: `Failed to write to ${path}. Check folder permissions and disk space.`;
+	}
+}
+
+/**
+ * Wrap an arbitrary storage failure in a StorageError with a derived hint.
+ */
+export function toStorageError(
+	message: string,
+	error: unknown,
+	path: string,
+): StorageError {
+	return new StorageError(
+		message,
+		extractErrorCode(error),
+		path,
+		formatStorageErrorHint(error, path),
+		error instanceof Error ? error : undefined,
+	);
+}

test/storage.test.ts

@@ -27,6 +27,7 @@ import {
 	withAccountAndFlaggedStorageTransaction,
 	withAccountStorageTransaction,
 } from "../lib/storage.js";
+import { toStorageError } from "../lib/storage/error-hints.js";
 
 // Mocking the behavior we're about to implement for TDD
 // Since the functions aren't in lib/storage.ts yet, we'll need to mock them or
@@ -78,6 +79,21 @@ describe("storage", () => {
 			expect(error.hint).toContain("Permission denied writing");
 			expect(error.path).toBe("/tmp/openai-codex-accounts.json");
 		});
+
+		it("wraps unknown failures with a StorageError", () => {
+			const cause = Object.assign(new Error("file locked"), { code: "EBUSY" });
+			const error = toStorageError(
+				"failed to persist accounts",
+				cause,
+				"/tmp/openai-codex-accounts.json",
+			);
+
+			expect(error).toBeInstanceOf(StorageError);
+			expect(error.code).toBe("EBUSY");
+			expect(error.path).toBe("/tmp/openai-codex-accounts.json");
+			expect(error.hint).toContain("File is locked");
+			expect(error.cause).toBe(cause);
+		});
 	});
 	describe("deduplication", () => {
 		it("preserves activeIndexByFamily when shared accountId entries remain distinct without email", () => {
@@ -1223,13 +1239,6 @@ describe("storage", () => {
 				expect(hint).toContain("Disk is full");
 			});
 
-			it("should return empty file hint for EEMPTY", () => {
-				const err = { code: "EEMPTY" } as NodeJS.ErrnoException;
-				const hint = formatStorageErrorHint(err, testPath);
-
-				expect(hint).toContain("empty");
-			});
-
 			it("should return generic hint for unknown error codes", () => {
 				const err = { code: "UNKNOWN_CODE" } as NodeJS.ErrnoException;
 				const hint = formatStorageErrorHint(err, testPath);