From aa77ab75eff13d1d09248d2c88cd9af7602df753 Mon Sep 17 00:00:00 2001 From: ndycode Date: Sat, 21 Mar 2026 11:54:17 +0800 Subject: [PATCH 1/6] refactor: extract storage error hints --- lib/storage.ts | 97 ++++++++++++++++++-------------------- lib/storage/error-hints.ts | 41 ++++++++++++++++ test/storage.test.ts | 1 + 3 files changed, 87 insertions(+), 52 deletions(-) create mode 100644 lib/storage/error-hints.ts diff --git a/lib/storage.ts b/lib/storage.ts index 0509925d..aabbe776 100644 --- a/lib/storage.ts +++ b/lib/storage.ts @@ -11,6 +11,11 @@ import { } from "./named-backup-export.js"; import { MODEL_FAMILIES, type ModelFamily } from "./prompts/codex.js"; import { AnyAccountStorageSchema, getValidationErrors } from "./schemas.js"; + +import { formatStorageErrorHint } from "./storage/error-hints.js"; + +export { formatStorageErrorHint } from "./storage/error-hints.js"; + import { type AccountMetadataV1, type AccountMetadataV3, @@ -122,7 +127,9 @@ export interface NamedBackupSummary { mtimeMs: number; } -async function collectNamedBackups(storagePath: string): Promise { +async function collectNamedBackups( + storagePath: string, +): Promise { const backupRoot = getNamedBackupRoot(storagePath); let entries: Array<{ isFile(): boolean; name: string }>; try { @@ -147,12 +154,15 @@ async function collectNamedBackups(storagePath: string): Promise 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 +171,20 @@ async function collectNamedBackups(storagePath: string): Promise = Promise.resolve(); const transactionSnapshotContext = new AsyncLocalStorage<{ snapshot: AccountStorageV3 | null; @@ -949,7 +935,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 +946,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 +1318,10 @@ function findCompatibleRefreshTokenMatchIndex( matchingAccount = account; continue; } - const newest: T = selectNewestAccount(matchingAccount ?? undefined, account); + const newest: T = selectNewestAccount( + matchingAccount ?? undefined, + account, + ); if (newest === account) { matchingIndex = i; matchingAccount = account; @@ -2114,7 +2105,9 @@ export async function withAccountAndFlaggedStorageTransaction( accountStorage: AccountStorageV3, flaggedStorage: FlaggedAccountStorageV1, ): Promise => { - const previousAccounts = cloneAccountStorageForPersistence(state.snapshot); + const previousAccounts = cloneAccountStorageForPersistence( + state.snapshot, + ); const nextAccounts = cloneAccountStorageForPersistence(accountStorage); await saveAccountsUnlocked(nextAccounts); try { diff --git a/lib/storage/error-hints.ts b/lib/storage/error-hints.ts new file mode 100644 index 00000000..dd0b78ce --- /dev/null +++ b/lib/storage/error-hints.ts @@ -0,0 +1,41 @@ +import { StorageError } from "../storage.js"; + +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.`; + } +} + +export function toStorageError( + message: string, + error: unknown, + path: string, +): StorageError { + const err = error as NodeJS.ErrnoException; + const code = err?.code || "UNKNOWN"; + return new StorageError( + message, + code, + path, + formatStorageErrorHint(error, path), + error instanceof Error ? error : undefined, + ); +} diff --git a/test/storage.test.ts b/test/storage.test.ts index e69feac2..32480c74 100644 --- a/test/storage.test.ts +++ b/test/storage.test.ts @@ -27,6 +27,7 @@ import { withAccountAndFlaggedStorageTransaction, withAccountStorageTransaction, } from "../lib/storage.js"; +import { formatStorageErrorHint } 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 From a7b3cfbc79d6c8e480367057eca045259165c967 Mon Sep 17 00:00:00 2001 From: ndycode Date: Sat, 21 Mar 2026 12:41:54 +0800 Subject: [PATCH 2/6] fix: break storage error helper cycle --- lib/errors.ts | 23 +++++++++++++++++++++++ lib/storage.ts | 25 ++----------------------- lib/storage/error-hints.ts | 2 +- test/storage.test.ts | 17 ++++++++++++++++- 4 files changed, 42 insertions(+), 25 deletions(-) diff --git a/lib/errors.ts b/lib/errors.ts index a44081cd..334ea591 100644 --- a/lib/errors.ts +++ b/lib/errors.ts @@ -164,3 +164,26 @@ 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 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; + } +} diff --git a/lib/storage.ts b/lib/storage.ts index aabbe776..873a332f 100644 --- a/lib/storage.ts +++ b/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, @@ -14,6 +15,7 @@ import { AnyAccountStorageSchema, getValidationErrors } from "./schemas.js"; import { formatStorageErrorHint } from "./storage/error-hints.js"; +export { StorageError } from "./errors.js"; export { formatStorageErrorHint } from "./storage/error-hints.js"; import { @@ -196,29 +198,6 @@ async function collectNamedBackups( 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; - } -} - let storageMutex: Promise = Promise.resolve(); const transactionSnapshotContext = new AsyncLocalStorage<{ snapshot: AccountStorageV3 | null; diff --git a/lib/storage/error-hints.ts b/lib/storage/error-hints.ts index dd0b78ce..03cc4c86 100644 --- a/lib/storage/error-hints.ts +++ b/lib/storage/error-hints.ts @@ -1,4 +1,4 @@ -import { StorageError } from "../storage.js"; +import { StorageError } from "../errors.js"; export function formatStorageErrorHint(error: unknown, path: string): string { const err = error as NodeJS.ErrnoException; diff --git a/test/storage.test.ts b/test/storage.test.ts index 32480c74..46030255 100644 --- a/test/storage.test.ts +++ b/test/storage.test.ts @@ -27,7 +27,7 @@ import { withAccountAndFlaggedStorageTransaction, withAccountStorageTransaction, } from "../lib/storage.js"; -import { formatStorageErrorHint } from "../lib/storage/error-hints.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 @@ -79,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", () => { From e01437ef91695b497988ac75f4d4e675d0af89c2 Mon Sep 17 00:00:00 2001 From: ndycode Date: Sat, 21 Mar 2026 13:05:48 +0800 Subject: [PATCH 3/6] fix: align storage error review followups --- lib/errors.ts | 8 +++----- lib/storage/error-hints.ts | 14 +++++++------- test/storage.test.ts | 7 ------- 3 files changed, 10 insertions(+), 19 deletions(-) diff --git a/lib/errors.ts b/lib/errors.ts index 334ea591..d1b95e03 100644 --- a/lib/errors.ts +++ b/lib/errors.ts @@ -168,8 +168,8 @@ export class CodexRateLimitError extends CodexError { /** * Storage-specific error with a filesystem code, target path, and user-facing hint. */ -export class StorageError extends Error { - readonly code: string; +export class StorageError extends CodexError { + override readonly name = "StorageError"; readonly path: string; readonly hint: string; @@ -180,9 +180,7 @@ export class StorageError extends Error { hint: string, cause?: Error, ) { - super(message, { cause }); - this.name = "StorageError"; - this.code = code; + super(message, { code, cause }); this.path = path; this.hint = hint; } diff --git a/lib/storage/error-hints.ts b/lib/storage/error-hints.ts index 03cc4c86..e5c79532 100644 --- a/lib/storage/error-hints.ts +++ b/lib/storage/error-hints.ts @@ -1,8 +1,12 @@ import { StorageError } from "../errors.js"; -export function formatStorageErrorHint(error: unknown, path: string): string { +function extractErrorCode(error: unknown): string { const err = error as NodeJS.ErrnoException; - const code = err?.code || "UNKNOWN"; + return err?.code || "UNKNOWN"; +} + +export function formatStorageErrorHint(error: unknown, path: string): string { + const code = extractErrorCode(error); const isWindows = process.platform === "win32"; switch (code) { @@ -15,8 +19,6 @@ export function formatStorageErrorHint(error: unknown, path: string): string { 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.` @@ -29,11 +31,9 @@ export function toStorageError( error: unknown, path: string, ): StorageError { - const err = error as NodeJS.ErrnoException; - const code = err?.code || "UNKNOWN"; return new StorageError( message, - code, + extractErrorCode(error), path, formatStorageErrorHint(error, path), error instanceof Error ? error : undefined, diff --git a/test/storage.test.ts b/test/storage.test.ts index 46030255..7ea802b7 100644 --- a/test/storage.test.ts +++ b/test/storage.test.ts @@ -1239,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); From 8f18512175f331dcb3f8b591bb4e6b4aa33e4f59 Mon Sep 17 00:00:00 2001 From: ndycode Date: Sat, 21 Mar 2026 15:24:09 +0800 Subject: [PATCH 4/6] fix: reuse storage error factory in save path --- lib/storage.ts | 21 +++++++++------------ 1 file changed, 9 insertions(+), 12 deletions(-) diff --git a/lib/storage.ts b/lib/storage.ts index 873a332f..a529807f 100644 --- a/lib/storage.ts +++ b/lib/storage.ts @@ -13,7 +13,7 @@ import { import { MODEL_FAMILIES, type ModelFamily } from "./prompts/codex.js"; import { AnyAccountStorageSchema, getValidationErrors } from "./schemas.js"; -import { formatStorageErrorHint } from "./storage/error-hints.js"; +import { toStorageError } from "./storage/error-hints.js"; export { StorageError } from "./errors.js"; export { formatStorageErrorHint } from "./storage/error-hints.js"; @@ -2004,23 +2004,20 @@ async function saveAccountsUnlocked(storage: AccountStorageV3): Promise { } 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; } } From 2412b22f6f5fef1a2ad807eab9d17502751b1a7c Mon Sep 17 00:00:00 2001 From: ndycode Date: Sun, 22 Mar 2026 00:38:08 +0800 Subject: [PATCH 5/6] fix: add missing storage ENOENT hint --- lib/storage/error-hints.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/lib/storage/error-hints.ts b/lib/storage/error-hints.ts index e5c79532..04a63bc9 100644 --- a/lib/storage/error-hints.ts +++ b/lib/storage/error-hints.ts @@ -17,6 +17,8 @@ export function formatStorageErrorHint(error: unknown, path: string): string { : `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: From e0ee8847157234da9b502ae41889aa9c990d54ac Mon Sep 17 00:00:00 2001 From: ndycode Date: Sun, 22 Mar 2026 01:02:09 +0800 Subject: [PATCH 6/6] docs: describe storage error helpers --- lib/storage/error-hints.ts | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/lib/storage/error-hints.ts b/lib/storage/error-hints.ts index 04a63bc9..3d406adf 100644 --- a/lib/storage/error-hints.ts +++ b/lib/storage/error-hints.ts @@ -5,6 +5,9 @@ function extractErrorCode(error: unknown): string { 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"; @@ -28,6 +31,9 @@ export function formatStorageErrorHint(error: unknown, path: string): string { } } +/** + * Wrap an arbitrary storage failure in a StorageError with a derived hint. + */ export function toStorageError( message: string, error: unknown,