From e9fe149d0cd0e196a6c5a707065038c0c1ef84b7 Mon Sep 17 00:00:00 2001 From: ndycode Date: Thu, 12 Mar 2026 13:57:51 +0800 Subject: [PATCH 1/9] feat(auth): add backup restore manager --- docs/reference/storage-paths.md | 2 + lib/cli.ts | 3 + lib/codex-manager.ts | 109 ++ lib/storage.ts | 2009 +++++++++++++++++++------------ lib/ui/auth-menu.ts | 13 + lib/ui/copy.ts | 2 + test/storage.test.ts | 90 ++ 7 files changed, 1443 insertions(+), 785 deletions(-) diff --git a/docs/reference/storage-paths.md b/docs/reference/storage-paths.md index bae76b8..6496572 100644 --- a/docs/reference/storage-paths.md +++ b/docs/reference/storage-paths.md @@ -22,6 +22,7 @@ Override root: | --- | --- | | Unified settings | `~/.codex/multi-auth/settings.json` | | Accounts | `~/.codex/multi-auth/openai-codex-accounts.json` | +| Named backups | `~/.codex/multi-auth/backups/.json` | | Accounts backup | `~/.codex/multi-auth/openai-codex-accounts.json.bak` | | Accounts WAL | `~/.codex/multi-auth/openai-codex-accounts.json.wal` | | Flagged accounts | `~/.codex/multi-auth/openai-codex-flagged-accounts.json` | @@ -43,6 +44,7 @@ Ownership note: When project-scoped behavior is enabled: - `~/.codex/multi-auth/projects//openai-codex-accounts.json` +- `~/.codex/multi-auth/projects//backups/.json` `` is derived as: diff --git a/lib/cli.ts b/lib/cli.ts index 59b66f7..c6522f4 100644 --- a/lib/cli.ts +++ b/lib/cli.ts @@ -57,6 +57,7 @@ export type LoginMode = | "check" | "deep-check" | "verify-flagged" + | "restore-backup" | "cancel"; export interface ExistingAccountInfo { @@ -279,6 +280,8 @@ export async function promptLoginMode( return { mode: "deep-check" }; case "verify-flagged": return { mode: "verify-flagged" }; + case "restore-backup": + return { mode: "restore-backup" }; case "select-account": { const accountAction = await showAccountDetails(action.account); if (accountAction === "delete") { diff --git a/lib/codex-manager.ts b/lib/codex-manager.ts index d967ffb..52b09ec 100644 --- a/lib/codex-manager.ts +++ b/lib/codex-manager.ts @@ -73,17 +73,22 @@ import { queuedRefresh } from "./refresh-queue.js"; import { type AccountMetadataV3, type AccountStorageV3, + assessNamedBackupRestore, type FlaggedAccountMetadataV1, type FlaggedAccountStorageV1, + getNamedBackupsDirectoryPath, getStoragePath, + listNamedBackups, loadAccounts, loadFlaggedAccounts, + restoreNamedBackup, saveAccounts, saveFlaggedAccounts, setStoragePath, } from "./storage.js"; import type { AccountIdSource, TokenFailure, TokenResult } from "./types.js"; import { ANSI } from "./ui/ansi.js"; +import { confirm } from "./ui/confirm.js"; import { UI_COPY } from "./ui/copy.js"; import { paintUiText, quotaToneFromLeftPercent } from "./ui/format.js"; import { getUiRuntimeOptions } from "./ui/runtime.js"; @@ -130,6 +135,17 @@ function formatReasonLabel(reason: string | undefined): string | undefined { return normalized.length > 0 ? normalized : undefined; } +function formatRelativeDateShort( + timestamp: number | null | undefined, +): string | null { + if (!timestamp) return null; + const days = Math.floor((Date.now() - timestamp) / 86_400_000); + if (days <= 0) return "today"; + if (days === 1) return "yesterday"; + if (days < 7) return `${days}d ago`; + return new Date(timestamp).toLocaleDateString(); +} + function extractErrorMessageFromPayload(payload: unknown): string | undefined { if (!payload || typeof payload !== "object") return undefined; const record = payload as Record; @@ -4069,6 +4085,95 @@ async function handleManageAction( } } +type BackupMenuAction = + | { + type: "restore"; + assessment: Awaited>; + } + | { type: "back" }; + +async function runBackupRestoreManager( + displaySettings: DashboardDisplaySettings, +): Promise { + const backupDir = getNamedBackupsDirectoryPath(); + const backups = await listNamedBackups(); + if (backups.length === 0) { + console.log(`No named backups found. Place backup files in ${backupDir}.`); + return; + } + + const currentStorage = await loadAccounts(); + const assessments = await Promise.all( + backups.map((backup) => + assessNamedBackupRestore(backup.name, { currentStorage }), + ), + ); + + const items: MenuItem[] = assessments.map((assessment) => { + const status = + assessment.valid && !assessment.wouldExceedLimit + ? "ready" + : assessment.wouldExceedLimit + ? "limit" + : "invalid"; + const lastUpdated = formatRelativeDateShort(assessment.backup.updatedAt); + const parts = [ + assessment.backup.accountCount !== null + ? `${assessment.backup.accountCount} account${assessment.backup.accountCount === 1 ? "" : "s"}` + : undefined, + lastUpdated ? `updated ${lastUpdated}` : undefined, + assessment.wouldExceedLimit + ? `would exceed ${ACCOUNT_LIMITS.MAX_ACCOUNTS}` + : undefined, + assessment.error ?? assessment.backup.loadError, + ].filter( + (value): value is string => + typeof value === "string" && value.trim().length > 0, + ); + + return { + label: assessment.backup.name, + hint: parts.length > 0 ? parts.join(" | ") : undefined, + value: { type: "restore", assessment }, + color: + status === "ready" ? "green" : status === "limit" ? "red" : "yellow", + disabled: !assessment.valid || assessment.wouldExceedLimit, + }; + }); + + items.push({ label: "Back", value: { type: "back" } }); + + const ui = getUiRuntimeOptions(); + const selection = await select(items, { + message: "Restore From Backup", + subtitle: backupDir, + help: UI_COPY.mainMenu.helpCompact, + clearScreen: true, + selectedEmphasis: "minimal", + focusStyle: displaySettings.menuFocusStyle ?? "row-invert", + theme: ui.theme, + }); + + if (!selection || selection.type === "back") { + return; + } + + const assessment = selection.assessment; + if (!assessment.valid || assessment.wouldExceedLimit) { + console.log(assessment.error ?? "Backup is not eligible for restore."); + return; + } + + const confirmMessage = `Restore backup "${assessment.backup.name}"? This will merge ${assessment.backup.accountCount ?? 0} account(s) into ${assessment.currentAccountCount} current (${assessment.mergedAccountCount ?? assessment.currentAccountCount} after dedupe).`; + const confirmed = await confirm(confirmMessage); + if (!confirmed) return; + + const result = await restoreNamedBackup(assessment.backup.name); + console.log( + `Restored backup "${assessment.backup.name}". Imported ${result.imported}, skipped ${result.skipped}, total ${result.total}.`, + ); +} + async function runAuthLogin(): Promise { setStoragePath(null); let pendingMenuQuotaRefresh: Promise | null = null; @@ -4192,6 +4297,10 @@ async function runAuthLogin(): Promise { ); continue; } + if (menuResult.mode === "restore-backup") { + await runBackupRestoreManager(displaySettings); + continue; + } if (menuResult.mode === "fresh" && menuResult.deleteAll) { await runActionPanel( DESTRUCTIVE_ACTION_COPY.deleteSavedAccounts.label, diff --git a/lib/storage.ts b/lib/storage.ts index 3453a42..344f97b 100644 --- a/lib/storage.ts +++ b/lib/storage.ts @@ -1,29 +1,36 @@ -import { promises as fs, existsSync } from "node:fs"; -import { basename, dirname, join } from "node:path"; import { createHash } from "node:crypto"; +import { existsSync, promises as fs } from "node:fs"; +import { basename, dirname, join } from "node:path"; import { ACCOUNT_LIMITS } from "./constants.js"; import { createLogger } from "./logger.js"; import { MODEL_FAMILIES, type ModelFamily } from "./prompts/codex.js"; import { AnyAccountStorageSchema, getValidationErrors } from "./schemas.js"; import { + type AccountMetadataV1, + type AccountMetadataV3, + type AccountStorageV1, + type AccountStorageV3, + type CooldownReason, + migrateV1ToV3, + type RateLimitStateV3, +} from "./storage/migrations.js"; +import { + findProjectRoot, getConfigDir, getProjectConfigDir, getProjectGlobalConfigDir, - findProjectRoot, resolvePath, resolveProjectStorageIdentityRoot, } from "./storage/paths.js"; -import { - migrateV1ToV3, - type CooldownReason, - type RateLimitStateV3, - type AccountMetadataV1, - type AccountStorageV1, - type AccountMetadataV3, - type AccountStorageV3, -} from "./storage/migrations.js"; -export type { CooldownReason, RateLimitStateV3, AccountMetadataV1, AccountStorageV1, AccountMetadataV3, AccountStorageV3 }; +export type { + CooldownReason, + RateLimitStateV3, + AccountMetadataV1, + AccountStorageV1, + AccountMetadataV3, + AccountStorageV3, +}; const log = createLogger("storage"); const ACCOUNTS_FILE_NAME = "openai-codex-accounts.json"; @@ -34,6 +41,8 @@ const ACCOUNTS_WAL_SUFFIX = ".wal"; const ACCOUNTS_BACKUP_HISTORY_DEPTH = 3; const BACKUP_COPY_MAX_ATTEMPTS = 5; const BACKUP_COPY_BASE_DELAY_MS = 10; +const NAMED_BACKUP_DIRECTORY = "backups"; +const NAMED_BACKUP_EXTENSION = ".json"; let storageBackupEnabled = true; let lastAccountsSaveTimestamp = 0; @@ -53,72 +62,84 @@ export interface FlaggedAccountStorageV1 { * 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; - } + 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.`; - } + 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 = Promise.resolve(); function withStorageLock(fn: () => Promise): Promise { - const previousMutex = storageMutex; - let releaseLock: () => void; - storageMutex = new Promise((resolve) => { - releaseLock = resolve; - }); - return previousMutex.then(fn).finally(() => releaseLock()); + const previousMutex = storageMutex; + let releaseLock: () => void; + storageMutex = new Promise((resolve) => { + releaseLock = resolve; + }); + return previousMutex.then(fn).finally(() => releaseLock()); } type AnyAccountStorage = AccountStorageV1 | AccountStorageV3; type AccountLike = { - accountId?: string; - email?: string; - refreshToken: string; - addedAt?: number; - lastUsed?: number; + accountId?: string; + email?: string; + refreshToken: string; + addedAt?: number; + lastUsed?: number; }; function looksLikeSyntheticFixtureAccount(account: AccountMetadataV3): boolean { - const email = typeof account.email === "string" ? account.email.trim().toLowerCase() : ""; + const email = + typeof account.email === "string" ? account.email.trim().toLowerCase() : ""; const refreshToken = - typeof account.refreshToken === "string" ? account.refreshToken.trim().toLowerCase() : ""; - const accountId = typeof account.accountId === "string" ? account.accountId.trim().toLowerCase() : ""; + typeof account.refreshToken === "string" + ? account.refreshToken.trim().toLowerCase() + : ""; + const accountId = + typeof account.accountId === "string" + ? account.accountId.trim().toLowerCase() + : ""; if (!/^account\d+@example\.com$/.test(email)) { return false; } @@ -134,39 +155,51 @@ function looksLikeSyntheticFixtureAccount(account: AccountMetadataV3): boolean { return /^acc(_|-)?\d+$/.test(accountId); } -function looksLikeSyntheticFixtureStorage(storage: AccountStorageV3 | null): boolean { +function looksLikeSyntheticFixtureStorage( + storage: AccountStorageV3 | null, +): boolean { if (!storage || storage.accounts.length === 0) return false; - return storage.accounts.every((account) => looksLikeSyntheticFixtureAccount(account)); + return storage.accounts.every((account) => + looksLikeSyntheticFixtureAccount(account), + ); } async function ensureGitignore(storagePath: string): Promise { - if (!currentStoragePath) return; - - const configDir = dirname(storagePath); - const inferredProjectRoot = dirname(configDir); - const candidateRoots = [currentProjectRoot, inferredProjectRoot].filter( - (root): root is string => typeof root === "string" && root.length > 0, - ); - const projectRoot = candidateRoots.find((root) => existsSync(join(root, ".git"))); - if (!projectRoot) return; - const gitignorePath = join(projectRoot, ".gitignore"); - - try { - let content = ""; - if (existsSync(gitignorePath)) { - content = await fs.readFile(gitignorePath, "utf-8"); - const lines = content.split("\n").map((l) => l.trim()); - if (lines.includes(".codex") || lines.includes(".codex/") || lines.includes("/.codex") || lines.includes("/.codex/")) { - return; - } - } - - const newContent = content.endsWith("\n") || content === "" ? content : content + "\n"; - await fs.writeFile(gitignorePath, newContent + ".codex/\n", "utf-8"); - log.debug("Added .codex to .gitignore", { path: gitignorePath }); - } catch (error) { - log.warn("Failed to update .gitignore", { error: String(error) }); - } + if (!currentStoragePath) return; + + const configDir = dirname(storagePath); + const inferredProjectRoot = dirname(configDir); + const candidateRoots = [currentProjectRoot, inferredProjectRoot].filter( + (root): root is string => typeof root === "string" && root.length > 0, + ); + const projectRoot = candidateRoots.find((root) => + existsSync(join(root, ".git")), + ); + if (!projectRoot) return; + const gitignorePath = join(projectRoot, ".gitignore"); + + try { + let content = ""; + if (existsSync(gitignorePath)) { + content = await fs.readFile(gitignorePath, "utf-8"); + const lines = content.split("\n").map((l) => l.trim()); + if ( + lines.includes(".codex") || + lines.includes(".codex/") || + lines.includes("/.codex") || + lines.includes("/.codex/") + ) { + return; + } + } + + const newContent = + content.endsWith("\n") || content === "" ? content : content + "\n"; + await fs.writeFile(gitignorePath, newContent + ".codex/\n", "utf-8"); + log.debug("Added .codex to .gitignore", { path: gitignorePath }); + } catch (error) { + log.warn("Failed to update .gitignore", { error: String(error) }); + } } let currentStoragePath: string | null = null; @@ -197,7 +230,9 @@ function getAccountsBackupRecoveryCandidates(path: string): string[] { return candidates; } -async function getAccountsBackupRecoveryCandidatesWithDiscovery(path: string): Promise { +async function getAccountsBackupRecoveryCandidatesWithDiscovery( + path: string, +): Promise { const knownCandidates = getAccountsBackupRecoveryCandidates(path); const discoveredCandidates = new Set(); const candidatePrefix = `${basename(path)}.`; @@ -265,7 +300,10 @@ async function copyFileWithRetry( } } -async function renameFileWithRetry(sourcePath: string, destinationPath: string): Promise { +async function renameFileWithRetry( + sourcePath: string, + destinationPath: string, +): Promise { for (let attempt = 0; attempt < BACKUP_COPY_MAX_ATTEMPTS; attempt += 1) { try { await fs.rename(sourcePath, destinationPath); @@ -280,7 +318,10 @@ async function renameFileWithRetry(sourcePath: string, destinationPath: string): } const jitterMs = Math.floor(Math.random() * BACKUP_COPY_BASE_DELAY_MS); await new Promise((resolve) => - setTimeout(resolve, BACKUP_COPY_BASE_DELAY_MS * 2 ** attempt + jitterMs), + setTimeout( + resolve, + BACKUP_COPY_BASE_DELAY_MS * 2 ** attempt + jitterMs, + ), ); } } @@ -301,7 +342,9 @@ async function createRotatingAccountsBackup(path: string): Promise { continue; } const stagedPath = buildStagedPath(currentPath, `slot-${i}`); - await copyFileWithRetry(previousPath, stagedPath, { allowMissingSource: true }); + await copyFileWithRetry(previousPath, stagedPath, { + allowMissingSource: true, + }); if (existsSync(stagedPath)) { stagedWrites.push({ targetPath: currentPath, stagedPath }); } @@ -314,7 +357,10 @@ async function createRotatingAccountsBackup(path: string): Promise { const latestStagedPath = buildStagedPath(latestBackupPath, "latest"); await copyFileWithRetry(path, latestStagedPath); if (existsSync(latestStagedPath)) { - stagedWrites.push({ targetPath: latestBackupPath, stagedPath: latestStagedPath }); + stagedWrites.push({ + targetPath: latestBackupPath, + stagedPath: latestStagedPath, + }); } for (const stagedWrite of stagedWrites) { @@ -334,9 +380,15 @@ async function createRotatingAccountsBackup(path: string): Promise { } } -function isRotatingBackupTempArtifact(storagePath: string, candidatePath: string): boolean { +function isRotatingBackupTempArtifact( + storagePath: string, + candidatePath: string, +): boolean { const backupPrefix = `${storagePath}${ACCOUNTS_BACKUP_SUFFIX}`; - if (!candidatePath.startsWith(backupPrefix) || !candidatePath.endsWith(".tmp")) { + if ( + !candidatePath.startsWith(backupPrefix) || + !candidatePath.endsWith(".tmp") + ) { return false; } @@ -354,10 +406,14 @@ function isRotatingBackupTempArtifact(storagePath: string, candidatePath: string return true; } -async function cleanupStaleRotatingBackupArtifacts(path: string): Promise { +async function cleanupStaleRotatingBackupArtifacts( + path: string, +): Promise { const directoryPath = dirname(path); try { - const directoryEntries = await fs.readdir(directoryPath, { withFileTypes: true }); + const directoryEntries = await fs.readdir(directoryPath, { + withFileTypes: true, + }); const staleArtifacts = directoryEntries .filter((entry) => entry.isFile()) .map((entry) => join(directoryPath, entry.name)) @@ -391,6 +447,45 @@ function computeSha256(value: string): string { return createHash("sha256").update(value).digest("hex"); } +function normalizeBackupName(rawName: string): string { + const trimmed = rawName.trim().replace(/\.(json|bak)$/i, ""); + const collapsedWhitespace = trimmed.replace(/\s+/g, "-"); + const sanitized = collapsedWhitespace + .replace(/[^a-zA-Z0-9._-]+/g, "-") + .replace(/-{2,}/g, "-") + .replace(/^-+|-+$/g, "") + .slice(0, 120); + if (!sanitized) { + throw new Error(`Invalid backup name: ${rawName}`); + } + return sanitized; +} + +function getNamedBackupsDirectory(): string { + return join(dirname(getStoragePath()), NAMED_BACKUP_DIRECTORY); +} + +async function ensureNamedBackupsDirectory(): Promise { + const directory = getNamedBackupsDirectory(); + await fs.mkdir(directory, { recursive: true }); + return directory; +} + +function resolveNamedBackupPath(name: string): string { + const normalizedName = normalizeBackupName(name); + return join( + getNamedBackupsDirectory(), + `${normalizedName}${NAMED_BACKUP_EXTENSION}`, + ); +} + +function deriveBackupNameFromFile(fileName: string): string { + if (fileName.endsWith(NAMED_BACKUP_EXTENSION)) { + return fileName.slice(0, -NAMED_BACKUP_EXTENSION.length); + } + return normalizeBackupName(fileName); +} + type AccountsJournalEntry = { version: 1; createdAt: number; @@ -399,44 +494,76 @@ type AccountsJournalEntry = { content: string; }; +export interface NamedBackupMetadata { + name: string; + path: string; + createdAt: number | null; + updatedAt: number | null; + sizeBytes: number | null; + version: number | null; + accountCount: number | null; + schemaErrors: string[]; + valid: boolean; + loadError?: string; +} + +export interface BackupRestoreAssessment { + backup: NamedBackupMetadata; + currentAccountCount: number; + mergedAccountCount: number | null; + imported: number | null; + skipped: number | null; + wouldExceedLimit: boolean; + valid: boolean; + error?: string; +} + export function getLastAccountsSaveTimestamp(): number { return lastAccountsSaveTimestamp; } export function setStoragePath(projectPath: string | null): void { - if (!projectPath) { - currentStoragePath = null; - currentLegacyProjectStoragePath = null; - currentLegacyWorktreeStoragePath = null; - currentProjectRoot = null; - return; - } - - const projectRoot = findProjectRoot(projectPath); - if (projectRoot) { - currentProjectRoot = projectRoot; - const identityRoot = resolveProjectStorageIdentityRoot(projectRoot); - currentStoragePath = join(getProjectGlobalConfigDir(identityRoot), ACCOUNTS_FILE_NAME); - currentLegacyProjectStoragePath = join(getProjectConfigDir(projectRoot), ACCOUNTS_FILE_NAME); - const previousWorktreeScopedPath = join( - getProjectGlobalConfigDir(projectRoot), - ACCOUNTS_FILE_NAME, - ); - currentLegacyWorktreeStoragePath = - previousWorktreeScopedPath !== currentStoragePath ? previousWorktreeScopedPath : null; - } else { - currentStoragePath = null; - currentLegacyProjectStoragePath = null; - currentLegacyWorktreeStoragePath = null; - currentProjectRoot = null; - } + if (!projectPath) { + currentStoragePath = null; + currentLegacyProjectStoragePath = null; + currentLegacyWorktreeStoragePath = null; + currentProjectRoot = null; + return; + } + + const projectRoot = findProjectRoot(projectPath); + if (projectRoot) { + currentProjectRoot = projectRoot; + const identityRoot = resolveProjectStorageIdentityRoot(projectRoot); + currentStoragePath = join( + getProjectGlobalConfigDir(identityRoot), + ACCOUNTS_FILE_NAME, + ); + currentLegacyProjectStoragePath = join( + getProjectConfigDir(projectRoot), + ACCOUNTS_FILE_NAME, + ); + const previousWorktreeScopedPath = join( + getProjectGlobalConfigDir(projectRoot), + ACCOUNTS_FILE_NAME, + ); + currentLegacyWorktreeStoragePath = + previousWorktreeScopedPath !== currentStoragePath + ? previousWorktreeScopedPath + : null; + } else { + currentStoragePath = null; + currentLegacyProjectStoragePath = null; + currentLegacyWorktreeStoragePath = null; + currentProjectRoot = null; + } } export function setStoragePathDirect(path: string | null): void { - currentStoragePath = path; - currentLegacyProjectStoragePath = null; - currentLegacyWorktreeStoragePath = null; - currentProjectRoot = null; + currentStoragePath = path; + currentLegacyProjectStoragePath = null; + currentLegacyWorktreeStoragePath = null; + currentProjectRoot = null; } /** @@ -444,10 +571,10 @@ export function setStoragePathDirect(path: string | null): void { * @returns Absolute path to the accounts.json file */ export function getStoragePath(): string { - if (currentStoragePath) { - return currentStoragePath; - } - return join(getConfigDir(), ACCOUNTS_FILE_NAME); + if (currentStoragePath) { + return currentStoragePath; + } + return join(getConfigDir(), ACCOUNTS_FILE_NAME); } export function getFlaggedAccountsPath(): string { @@ -459,176 +586,196 @@ function getLegacyFlaggedAccountsPath(): string { } async function migrateLegacyProjectStorageIfNeeded( - persist: (storage: AccountStorageV3) => Promise = saveAccounts, + persist: (storage: AccountStorageV3) => Promise = saveAccounts, ): Promise { - if (!currentStoragePath) { - return null; - } - - const candidatePaths = [currentLegacyWorktreeStoragePath, currentLegacyProjectStoragePath] - .filter( - (path): path is string => typeof path === "string" && path.length > 0 && path !== currentStoragePath, - ) - .filter((path, index, all) => all.indexOf(path) === index); - - if (candidatePaths.length === 0) { - return null; - } - - const existingCandidatePaths = candidatePaths.filter((legacyPath) => existsSync(legacyPath)); - if (existingCandidatePaths.length === 0) { - return null; - } - - let targetStorage = await loadNormalizedStorageFromPath(currentStoragePath, "current account storage"); - let migrated = false; - - for (const legacyPath of existingCandidatePaths) { - const legacyStorage = await loadNormalizedStorageFromPath(legacyPath, "legacy account storage"); - if (!legacyStorage) { - continue; - } - - const mergedStorage = mergeStorageForMigration(targetStorage, legacyStorage); - const fallbackStorage = targetStorage ?? legacyStorage; - - try { - await persist(mergedStorage); - targetStorage = mergedStorage; - migrated = true; - } catch (error) { - targetStorage = fallbackStorage; - log.warn("Failed to persist migrated account storage", { - from: legacyPath, - to: currentStoragePath, - error: String(error), - }); - continue; - } - - try { - await fs.unlink(legacyPath); - log.info("Removed legacy account storage file after migration", { - path: legacyPath, - }); - } catch (unlinkError) { - const code = (unlinkError as NodeJS.ErrnoException).code; - if (code !== "ENOENT") { - log.warn("Failed to remove legacy account storage file after migration", { - path: legacyPath, - error: String(unlinkError), - }); - } - } - - log.info("Migrated legacy project account storage", { - from: legacyPath, - to: currentStoragePath, - accounts: mergedStorage.accounts.length, - }); - } - - if (migrated) { - return targetStorage; - } - if (targetStorage && !existsSync(currentStoragePath)) { - return targetStorage; - } - return null; + if (!currentStoragePath) { + return null; + } + + const candidatePaths = [ + currentLegacyWorktreeStoragePath, + currentLegacyProjectStoragePath, + ] + .filter( + (path): path is string => + typeof path === "string" && + path.length > 0 && + path !== currentStoragePath, + ) + .filter((path, index, all) => all.indexOf(path) === index); + + if (candidatePaths.length === 0) { + return null; + } + + const existingCandidatePaths = candidatePaths.filter((legacyPath) => + existsSync(legacyPath), + ); + if (existingCandidatePaths.length === 0) { + return null; + } + + let targetStorage = await loadNormalizedStorageFromPath( + currentStoragePath, + "current account storage", + ); + let migrated = false; + + for (const legacyPath of existingCandidatePaths) { + const legacyStorage = await loadNormalizedStorageFromPath( + legacyPath, + "legacy account storage", + ); + if (!legacyStorage) { + continue; + } + + const mergedStorage = mergeStorageForMigration( + targetStorage, + legacyStorage, + ); + const fallbackStorage = targetStorage ?? legacyStorage; + + try { + await persist(mergedStorage); + targetStorage = mergedStorage; + migrated = true; + } catch (error) { + targetStorage = fallbackStorage; + log.warn("Failed to persist migrated account storage", { + from: legacyPath, + to: currentStoragePath, + error: String(error), + }); + continue; + } + + try { + await fs.unlink(legacyPath); + log.info("Removed legacy account storage file after migration", { + path: legacyPath, + }); + } catch (unlinkError) { + const code = (unlinkError as NodeJS.ErrnoException).code; + if (code !== "ENOENT") { + log.warn( + "Failed to remove legacy account storage file after migration", + { + path: legacyPath, + error: String(unlinkError), + }, + ); + } + } + + log.info("Migrated legacy project account storage", { + from: legacyPath, + to: currentStoragePath, + accounts: mergedStorage.accounts.length, + }); + } + + if (migrated) { + return targetStorage; + } + if (targetStorage && !existsSync(currentStoragePath)) { + return targetStorage; + } + return null; } async function loadNormalizedStorageFromPath( - path: string, - label: string, + path: string, + label: string, ): Promise { - try { - const { normalized, schemaErrors } = await loadAccountsFromPath(path); - if (schemaErrors.length > 0) { - log.warn(`${label} schema validation warnings`, { - path, - errors: schemaErrors.slice(0, 5), - }); - } - return normalized; - } catch (error) { - const code = (error as NodeJS.ErrnoException).code; - if (code !== "ENOENT") { - log.warn(`Failed to load ${label}`, { - path, - error: String(error), - }); - } - return null; - } + try { + const { normalized, schemaErrors } = await loadAccountsFromPath(path); + if (schemaErrors.length > 0) { + log.warn(`${label} schema validation warnings`, { + path, + errors: schemaErrors.slice(0, 5), + }); + } + return normalized; + } catch (error) { + const code = (error as NodeJS.ErrnoException).code; + if (code !== "ENOENT") { + log.warn(`Failed to load ${label}`, { + path, + error: String(error), + }); + } + return null; + } } function mergeStorageForMigration( - current: AccountStorageV3 | null, - incoming: AccountStorageV3, + current: AccountStorageV3 | null, + incoming: AccountStorageV3, ): AccountStorageV3 { - if (!current) { - return incoming; - } - - const merged = normalizeAccountStorage({ - version: 3, - activeIndex: current.activeIndex, - activeIndexByFamily: current.activeIndexByFamily, - accounts: [...current.accounts, ...incoming.accounts], - }); - if (!merged) { - return current; - } - return merged; + if (!current) { + return incoming; + } + + const merged = normalizeAccountStorage({ + version: 3, + activeIndex: current.activeIndex, + activeIndexByFamily: current.activeIndexByFamily, + accounts: [...current.accounts, ...incoming.accounts], + }); + if (!merged) { + return current; + } + return merged; } function selectNewestAccount( - current: T | undefined, - candidate: T, + current: T | undefined, + candidate: T, ): T { - if (!current) return candidate; - const currentLastUsed = current.lastUsed || 0; - const candidateLastUsed = candidate.lastUsed || 0; - if (candidateLastUsed > currentLastUsed) return candidate; - if (candidateLastUsed < currentLastUsed) return current; - const currentAddedAt = current.addedAt || 0; - const candidateAddedAt = candidate.addedAt || 0; - return candidateAddedAt >= currentAddedAt ? candidate : current; + if (!current) return candidate; + const currentLastUsed = current.lastUsed || 0; + const candidateLastUsed = candidate.lastUsed || 0; + if (candidateLastUsed > currentLastUsed) return candidate; + if (candidateLastUsed < currentLastUsed) return current; + const currentAddedAt = current.addedAt || 0; + const candidateAddedAt = candidate.addedAt || 0; + return candidateAddedAt >= currentAddedAt ? candidate : current; } function deduplicateAccountsByKey(accounts: T[]): T[] { - const keyToIndex = new Map(); - const indicesToKeep = new Set(); - - for (let i = 0; i < accounts.length; i += 1) { - const account = accounts[i]; - if (!account) continue; - const key = account.accountId || account.refreshToken; - if (!key) continue; - - const existingIndex = keyToIndex.get(key); - if (existingIndex === undefined) { - keyToIndex.set(key, i); - continue; - } - - const existing = accounts[existingIndex]; - const newest = selectNewestAccount(existing, account); - keyToIndex.set(key, newest === account ? i : existingIndex); - } - - for (const idx of keyToIndex.values()) { - indicesToKeep.add(idx); - } - - const result: T[] = []; - for (let i = 0; i < accounts.length; i += 1) { - if (indicesToKeep.has(i)) { - const account = accounts[i]; - if (account) result.push(account); - } - } - return result; + const keyToIndex = new Map(); + const indicesToKeep = new Set(); + + for (let i = 0; i < accounts.length; i += 1) { + const account = accounts[i]; + if (!account) continue; + const key = account.accountId || account.refreshToken; + if (!key) continue; + + const existingIndex = keyToIndex.get(key); + if (existingIndex === undefined) { + keyToIndex.set(key, i); + continue; + } + + const existing = accounts[existingIndex]; + const newest = selectNewestAccount(existing, account); + keyToIndex.set(key, newest === account ? i : existingIndex); + } + + for (const idx of keyToIndex.values()) { + indicesToKeep.add(idx); + } + + const result: T[] = []; + for (let i = 0; i < accounts.length; i += 1) { + if (indicesToKeep.has(i)) { + const account = accounts[i]; + if (account) result.push(account); + } + } + return result; } /** @@ -637,10 +784,15 @@ function deduplicateAccountsByKey(accounts: T[]): T[] { * @param accounts - Array of accounts to deduplicate * @returns New array with duplicates removed */ -export function deduplicateAccounts( - accounts: T[], -): T[] { - return deduplicateAccountsByKey(accounts); +export function deduplicateAccounts< + T extends { + accountId?: string; + refreshToken: string; + lastUsed?: number; + addedAt?: number; + }, +>(accounts: T[]): T[] { + return deduplicateAccountsByKey(accounts); } /** @@ -649,98 +801,105 @@ export function deduplicateAccounts( - accounts: T[], -): T[] { - - const emailToNewestIndex = new Map(); - const indicesToKeep = new Set(); - - for (let i = 0; i < accounts.length; i += 1) { - const account = accounts[i]; - if (!account) continue; - - const email = normalizeEmailKey(account.email); - if (!email) { - indicesToKeep.add(i); - continue; - } - - const existingIndex = emailToNewestIndex.get(email); - if (existingIndex === undefined) { - emailToNewestIndex.set(email, i); - continue; - } - - const existing = accounts[existingIndex]; - // istanbul ignore next -- defensive code: existingIndex always refers to valid account - if (!existing) { - emailToNewestIndex.set(email, i); - continue; - } - - const existingLastUsed = existing.lastUsed || 0; - const candidateLastUsed = account.lastUsed || 0; - const existingAddedAt = existing.addedAt || 0; - const candidateAddedAt = account.addedAt || 0; - - const isNewer = - candidateLastUsed > existingLastUsed || - (candidateLastUsed === existingLastUsed && candidateAddedAt > existingAddedAt); - - if (isNewer) { - emailToNewestIndex.set(email, i); - } - } - - for (const idx of emailToNewestIndex.values()) { - indicesToKeep.add(idx); - } - - const result: T[] = []; - for (let i = 0; i < accounts.length; i += 1) { - if (indicesToKeep.has(i)) { - const account = accounts[i]; - if (account) result.push(account); - } - } - return result; +export function normalizeEmailKey( + email: string | undefined, +): string | undefined { + if (!email) return undefined; + const trimmed = email.trim(); + if (!trimmed) return undefined; + return trimmed.toLowerCase(); +} + +export function deduplicateAccountsByEmail< + T extends { email?: string; lastUsed?: number; addedAt?: number }, +>(accounts: T[]): T[] { + const emailToNewestIndex = new Map(); + const indicesToKeep = new Set(); + + for (let i = 0; i < accounts.length; i += 1) { + const account = accounts[i]; + if (!account) continue; + + const email = normalizeEmailKey(account.email); + if (!email) { + indicesToKeep.add(i); + continue; + } + + const existingIndex = emailToNewestIndex.get(email); + if (existingIndex === undefined) { + emailToNewestIndex.set(email, i); + continue; + } + + const existing = accounts[existingIndex]; + // istanbul ignore next -- defensive code: existingIndex always refers to valid account + if (!existing) { + emailToNewestIndex.set(email, i); + continue; + } + + const existingLastUsed = existing.lastUsed || 0; + const candidateLastUsed = account.lastUsed || 0; + const existingAddedAt = existing.addedAt || 0; + const candidateAddedAt = account.addedAt || 0; + + const isNewer = + candidateLastUsed > existingLastUsed || + (candidateLastUsed === existingLastUsed && + candidateAddedAt > existingAddedAt); + + if (isNewer) { + emailToNewestIndex.set(email, i); + } + } + + for (const idx of emailToNewestIndex.values()) { + indicesToKeep.add(idx); + } + + const result: T[] = []; + for (let i = 0; i < accounts.length; i += 1) { + if (indicesToKeep.has(i)) { + const account = accounts[i]; + if (account) result.push(account); + } + } + return result; } function isRecord(value: unknown): value is Record { - return !!value && typeof value === "object" && !Array.isArray(value); + return !!value && typeof value === "object" && !Array.isArray(value); } function clampIndex(index: number, length: number): number { - if (length <= 0) return 0; - return Math.max(0, Math.min(index, length - 1)); + if (length <= 0) return 0; + return Math.max(0, Math.min(index, length - 1)); } -function toAccountKey(account: Pick): string { - return account.accountId || account.refreshToken; +function toAccountKey( + account: Pick, +): string { + return account.accountId || account.refreshToken; } -function extractActiveKey(accounts: unknown[], activeIndex: number): string | undefined { - const candidate = accounts[activeIndex]; - if (!isRecord(candidate)) return undefined; +function extractActiveKey( + accounts: unknown[], + activeIndex: number, +): string | undefined { + const candidate = accounts[activeIndex]; + if (!isRecord(candidate)) return undefined; - const accountId = - typeof candidate.accountId === "string" && candidate.accountId.trim() - ? candidate.accountId - : undefined; - const refreshToken = - typeof candidate.refreshToken === "string" && candidate.refreshToken.trim() - ? candidate.refreshToken - : undefined; + const accountId = + typeof candidate.accountId === "string" && candidate.accountId.trim() + ? candidate.accountId + : undefined; + const refreshToken = + typeof candidate.refreshToken === "string" && candidate.refreshToken.trim() + ? candidate.refreshToken + : undefined; - return accountId || refreshToken; + return accountId || refreshToken; } /** @@ -749,95 +908,99 @@ function extractActiveKey(accounts: unknown[], activeIndex: number): string | un * @param data - Raw storage data (unknown format) * @returns Normalized AccountStorageV3 or null if invalid */ -export function normalizeAccountStorage(data: unknown): AccountStorageV3 | null { - if (!isRecord(data)) { - log.warn("Invalid storage format, ignoring"); - return null; - } - - if (data.version !== 1 && data.version !== 3) { - log.warn("Unknown storage version, ignoring", { - version: (data as { version?: unknown }).version, - }); - return null; - } - - const rawAccounts = data.accounts; - if (!Array.isArray(rawAccounts)) { - log.warn("Invalid storage format, ignoring"); - return null; - } - - const activeIndexValue = - typeof data.activeIndex === "number" && Number.isFinite(data.activeIndex) - ? data.activeIndex - : 0; - - const rawActiveIndex = clampIndex(activeIndexValue, rawAccounts.length); - const activeKey = extractActiveKey(rawAccounts, rawActiveIndex); - - const fromVersion = data.version as AnyAccountStorage["version"]; - const baseStorage: AccountStorageV3 = - fromVersion === 1 - ? migrateV1ToV3(data as unknown as AccountStorageV1) - : (data as unknown as AccountStorageV3); - - const validAccounts = rawAccounts.filter( - (account): account is AccountMetadataV3 => - isRecord(account) && typeof account.refreshToken === "string" && !!account.refreshToken.trim(), - ); - - const deduplicatedAccounts = deduplicateAccountsByEmail( - deduplicateAccountsByKey(validAccounts), - ); - - const activeIndex = (() => { - if (deduplicatedAccounts.length === 0) return 0; - - if (activeKey) { - const mappedIndex = deduplicatedAccounts.findIndex( - (account) => toAccountKey(account) === activeKey, - ); - if (mappedIndex >= 0) return mappedIndex; - } - - return clampIndex(rawActiveIndex, deduplicatedAccounts.length); - })(); - - const activeIndexByFamily: Partial> = {}; - const rawFamilyIndices = isRecord(baseStorage.activeIndexByFamily) - ? (baseStorage.activeIndexByFamily as Record) - : {}; - - for (const family of MODEL_FAMILIES) { - const rawIndexValue = rawFamilyIndices[family]; - const rawIndex = - typeof rawIndexValue === "number" && Number.isFinite(rawIndexValue) - ? rawIndexValue - : rawActiveIndex; - - const clampedRawIndex = clampIndex(rawIndex, rawAccounts.length); - const familyKey = extractActiveKey(rawAccounts, clampedRawIndex); - - let mappedIndex = clampIndex(rawIndex, deduplicatedAccounts.length); - if (familyKey && deduplicatedAccounts.length > 0) { - const idx = deduplicatedAccounts.findIndex( - (account) => toAccountKey(account) === familyKey, - ); - if (idx >= 0) { - mappedIndex = idx; - } - } - - activeIndexByFamily[family] = mappedIndex; - } - - return { - version: 3, - accounts: deduplicatedAccounts, - activeIndex, - activeIndexByFamily, - }; +export function normalizeAccountStorage( + data: unknown, +): AccountStorageV3 | null { + if (!isRecord(data)) { + log.warn("Invalid storage format, ignoring"); + return null; + } + + if (data.version !== 1 && data.version !== 3) { + log.warn("Unknown storage version, ignoring", { + version: (data as { version?: unknown }).version, + }); + return null; + } + + const rawAccounts = data.accounts; + if (!Array.isArray(rawAccounts)) { + log.warn("Invalid storage format, ignoring"); + return null; + } + + const activeIndexValue = + typeof data.activeIndex === "number" && Number.isFinite(data.activeIndex) + ? data.activeIndex + : 0; + + const rawActiveIndex = clampIndex(activeIndexValue, rawAccounts.length); + const activeKey = extractActiveKey(rawAccounts, rawActiveIndex); + + const fromVersion = data.version as AnyAccountStorage["version"]; + const baseStorage: AccountStorageV3 = + fromVersion === 1 + ? migrateV1ToV3(data as unknown as AccountStorageV1) + : (data as unknown as AccountStorageV3); + + const validAccounts = rawAccounts.filter( + (account): account is AccountMetadataV3 => + isRecord(account) && + typeof account.refreshToken === "string" && + !!account.refreshToken.trim(), + ); + + const deduplicatedAccounts = deduplicateAccountsByEmail( + deduplicateAccountsByKey(validAccounts), + ); + + const activeIndex = (() => { + if (deduplicatedAccounts.length === 0) return 0; + + if (activeKey) { + const mappedIndex = deduplicatedAccounts.findIndex( + (account) => toAccountKey(account) === activeKey, + ); + if (mappedIndex >= 0) return mappedIndex; + } + + return clampIndex(rawActiveIndex, deduplicatedAccounts.length); + })(); + + const activeIndexByFamily: Partial> = {}; + const rawFamilyIndices = isRecord(baseStorage.activeIndexByFamily) + ? (baseStorage.activeIndexByFamily as Record) + : {}; + + for (const family of MODEL_FAMILIES) { + const rawIndexValue = rawFamilyIndices[family]; + const rawIndex = + typeof rawIndexValue === "number" && Number.isFinite(rawIndexValue) + ? rawIndexValue + : rawActiveIndex; + + const clampedRawIndex = clampIndex(rawIndex, rawAccounts.length); + const familyKey = extractActiveKey(rawAccounts, clampedRawIndex); + + let mappedIndex = clampIndex(rawIndex, deduplicatedAccounts.length); + if (familyKey && deduplicatedAccounts.length > 0) { + const idx = deduplicatedAccounts.findIndex( + (account) => toAccountKey(account) === familyKey, + ); + if (idx >= 0) { + mappedIndex = idx; + } + } + + activeIndexByFamily[family] = mappedIndex; + } + + return { + version: 3, + accounts: deduplicatedAccounts, + activeIndex, + activeIndexByFamily, + }; } /** @@ -846,7 +1009,7 @@ export function normalizeAccountStorage(data: unknown): AccountStorageV3 | null * @returns AccountStorageV3 if file exists and is valid, null otherwise */ export async function loadAccounts(): Promise { - return loadAccountsInternal(saveAccounts); + return loadAccountsInternal(saveAccounts); } function parseAndNormalizeStorage(data: unknown): { @@ -856,7 +1019,9 @@ function parseAndNormalizeStorage(data: unknown): { } { const schemaErrors = getValidationErrors(AnyAccountStorageSchema, data); const normalized = normalizeAccountStorage(data); - const storedVersion = isRecord(data) ? (data as { version?: unknown }).version : undefined; + const storedVersion = isRecord(data) + ? (data as { version?: unknown }).version + : undefined; return { normalized, storedVersion, schemaErrors }; } @@ -870,7 +1035,27 @@ async function loadAccountsFromPath(path: string): Promise<{ return parseAndNormalizeStorage(data); } -async function loadAccountsFromJournal(path: string): Promise { +async function loadBackupCandidate(path: string): Promise<{ + normalized: AccountStorageV3 | null; + storedVersion: unknown; + schemaErrors: string[]; + error?: string; +}> { + try { + return await loadAccountsFromPath(path); + } catch (error) { + return { + normalized: null, + storedVersion: undefined, + schemaErrors: [], + error: String(error), + }; + } +} + +async function loadAccountsFromJournal( + path: string, +): Promise { const walPath = getAccountsWalPath(path); try { const raw = await fs.readFile(walPath, "utf-8"); @@ -878,7 +1063,8 @@ async function loadAccountsFromJournal(path: string): Promise; if (entry.version !== 1) return null; - if (typeof entry.content !== "string" || typeof entry.checksum !== "string") return null; + if (typeof entry.content !== "string" || typeof entry.checksum !== "string") + return null; const computed = computeSha256(entry.content); if (computed !== entry.checksum) { log.warn("Account journal checksum mismatch", { path: walPath }); @@ -892,14 +1078,17 @@ async function loadAccountsFromJournal(path: string): Promise Promise) | null, + persistMigration: ((storage: AccountStorageV3) => Promise) | null, ): Promise { const path = getStoragePath(); await cleanupStaleRotatingBackupArtifacts(path); @@ -907,249 +1096,429 @@ async function loadAccountsInternal( ? await migrateLegacyProjectStorageIfNeeded(persistMigration) : null; - try { - const { normalized, storedVersion, schemaErrors } = await loadAccountsFromPath(path); - if (schemaErrors.length > 0) { - log.warn("Account storage schema validation warnings", { errors: schemaErrors.slice(0, 5) }); - } - if (normalized && storedVersion !== normalized.version) { - log.info("Migrating account storage to v3", { from: storedVersion, to: normalized.version }); - if (persistMigration) { - try { - await persistMigration(normalized); - } catch (saveError) { - log.warn("Failed to persist migrated storage", { error: String(saveError) }); - } - } - } - - const primaryLooksSynthetic = looksLikeSyntheticFixtureStorage(normalized); - if (storageBackupEnabled && normalized && primaryLooksSynthetic) { - const backupCandidates = await getAccountsBackupRecoveryCandidatesWithDiscovery(path); - for (const backupPath of backupCandidates) { - if (backupPath === path) continue; - try { - const backup = await loadAccountsFromPath(backupPath); - if (!backup.normalized) continue; - if (looksLikeSyntheticFixtureStorage(backup.normalized)) continue; - if (backup.normalized.accounts.length <= 0) continue; - log.warn("Detected synthetic primary account storage; promoting backup", { - path, - backupPath, - primaryAccounts: normalized.accounts.length, - backupAccounts: backup.normalized.accounts.length, - }); - if (persistMigration) { - try { - await persistMigration(backup.normalized); - } catch (persistError) { - log.warn("Failed to persist promoted backup storage", { - path, - error: String(persistError), - }); - } - } - return backup.normalized; - } catch (backupError) { - const backupCode = (backupError as NodeJS.ErrnoException).code; - if (backupCode !== "ENOENT") { - log.warn("Failed to load candidate backup for synthetic-primary promotion", { - path: backupPath, - error: String(backupError), + try { + const { normalized, storedVersion, schemaErrors } = + await loadAccountsFromPath(path); + if (schemaErrors.length > 0) { + log.warn("Account storage schema validation warnings", { + errors: schemaErrors.slice(0, 5), + }); + } + if (normalized && storedVersion !== normalized.version) { + log.info("Migrating account storage to v3", { + from: storedVersion, + to: normalized.version, + }); + if (persistMigration) { + try { + await persistMigration(normalized); + } catch (saveError) { + log.warn("Failed to persist migrated storage", { + error: String(saveError), }); } } } - } - - return normalized; - } catch (error) { - const code = (error as NodeJS.ErrnoException).code; - if (code === "ENOENT" && migratedLegacyStorage) { - return migratedLegacyStorage; - } - - const recoveredFromWal = await loadAccountsFromJournal(path); - if (recoveredFromWal) { - if (persistMigration) { - try { - await persistMigration(recoveredFromWal); - } catch (persistError) { - log.warn("Failed to persist WAL-recovered storage", { - path, - error: String(persistError), - }); - } - } - return recoveredFromWal; - } - if (storageBackupEnabled) { - const backupCandidates = await getAccountsBackupRecoveryCandidatesWithDiscovery(path); - for (const backupPath of backupCandidates) { - try { - const backup = await loadAccountsFromPath(backupPath); - if (backup.schemaErrors.length > 0) { - log.warn("Backup account storage schema validation warnings", { - path: backupPath, - errors: backup.schemaErrors.slice(0, 5), - }); - } - if (backup.normalized) { - log.warn("Recovered account storage from backup file", { path, backupPath }); + const primaryLooksSynthetic = looksLikeSyntheticFixtureStorage(normalized); + if (storageBackupEnabled && normalized && primaryLooksSynthetic) { + const backupCandidates = + await getAccountsBackupRecoveryCandidatesWithDiscovery(path); + for (const backupPath of backupCandidates) { + if (backupPath === path) continue; + try { + const backup = await loadAccountsFromPath(backupPath); + if (!backup.normalized) continue; + if (looksLikeSyntheticFixtureStorage(backup.normalized)) continue; + if (backup.normalized.accounts.length <= 0) continue; + log.warn( + "Detected synthetic primary account storage; promoting backup", + { + path, + backupPath, + primaryAccounts: normalized.accounts.length, + backupAccounts: backup.normalized.accounts.length, + }, + ); if (persistMigration) { try { await persistMigration(backup.normalized); } catch (persistError) { - log.warn("Failed to persist recovered backup storage", { + log.warn("Failed to persist promoted backup storage", { path, error: String(persistError), }); } } return backup.normalized; + } catch (backupError) { + const backupCode = (backupError as NodeJS.ErrnoException).code; + if (backupCode !== "ENOENT") { + log.warn( + "Failed to load candidate backup for synthetic-primary promotion", + { + path: backupPath, + error: String(backupError), + }, + ); + } } - } catch (backupError) { - const backupCode = (backupError as NodeJS.ErrnoException).code; - if (backupCode !== "ENOENT") { - log.warn("Failed to load backup account storage", { - path: backupPath, - error: String(backupError), + } + } + + return normalized; + } catch (error) { + const code = (error as NodeJS.ErrnoException).code; + if (code === "ENOENT" && migratedLegacyStorage) { + return migratedLegacyStorage; + } + + const recoveredFromWal = await loadAccountsFromJournal(path); + if (recoveredFromWal) { + if (persistMigration) { + try { + await persistMigration(recoveredFromWal); + } catch (persistError) { + log.warn("Failed to persist WAL-recovered storage", { + path, + error: String(persistError), }); } } + return recoveredFromWal; + } + + if (storageBackupEnabled) { + const backupCandidates = + await getAccountsBackupRecoveryCandidatesWithDiscovery(path); + for (const backupPath of backupCandidates) { + try { + const backup = await loadAccountsFromPath(backupPath); + if (backup.schemaErrors.length > 0) { + log.warn("Backup account storage schema validation warnings", { + path: backupPath, + errors: backup.schemaErrors.slice(0, 5), + }); + } + if (backup.normalized) { + log.warn("Recovered account storage from backup file", { + path, + backupPath, + }); + if (persistMigration) { + try { + await persistMigration(backup.normalized); + } catch (persistError) { + log.warn("Failed to persist recovered backup storage", { + path, + error: String(persistError), + }); + } + } + return backup.normalized; + } + } catch (backupError) { + const backupCode = (backupError as NodeJS.ErrnoException).code; + if (backupCode !== "ENOENT") { + log.warn("Failed to load backup account storage", { + path: backupPath, + error: String(backupError), + }); + } + } + } + } + + if (code !== "ENOENT") { + log.error("Failed to load account storage", { error: String(error) }); + } + return null; + } +} + +async function buildNamedBackupMetadata( + name: string, + path: string, + opts: { candidate?: Awaited> } = {}, +): Promise { + const candidate = opts.candidate ?? (await loadBackupCandidate(path)); + let stats: { + size?: number; + mtimeMs?: number; + birthtimeMs?: number; + ctimeMs?: number; + } | null = null; + try { + stats = await fs.stat(path); + } catch (error) { + const code = (error as NodeJS.ErrnoException).code; + if (code !== "ENOENT") { + log.warn("Failed to stat named backup", { path, error: String(error) }); } } - if (code !== "ENOENT") { - log.error("Failed to load account storage", { error: String(error) }); - } - return null; - } + const version = + candidate.normalized?.version ?? + (typeof candidate.storedVersion === "number" + ? candidate.storedVersion + : null); + const accountCount = candidate.normalized?.accounts.length ?? null; + const createdAt = stats?.birthtimeMs ?? stats?.ctimeMs ?? null; + const updatedAt = stats?.mtimeMs ?? null; + + return { + name, + path, + createdAt, + updatedAt, + sizeBytes: typeof stats?.size === "number" ? stats?.size : null, + version, + accountCount, + schemaErrors: candidate.schemaErrors, + valid: !!candidate.normalized, + loadError: candidate.error, + }; +} + +export async function listNamedBackups(): Promise { + const backupDir = getNamedBackupsDirectory(); + try { + const entries = await fs.readdir(backupDir, { withFileTypes: true }); + const backups: NamedBackupMetadata[] = []; + for (const entry of entries) { + if (!entry.isFile()) continue; + if (!entry.name.endsWith(NAMED_BACKUP_EXTENSION)) continue; + + const name = deriveBackupNameFromFile(entry.name); + const path = join(backupDir, entry.name); + const candidate = await loadBackupCandidate(path); + backups.push(await buildNamedBackupMetadata(name, path, { candidate })); + } + + return backups.sort((a, b) => (b.updatedAt ?? 0) - (a.updatedAt ?? 0)); + } catch (error) { + const code = (error as NodeJS.ErrnoException).code; + if (code !== "ENOENT") { + log.warn("Failed to list named backups", { + path: backupDir, + error: String(error), + }); + } + return []; + } +} + +export function getNamedBackupsDirectoryPath(): string { + return getNamedBackupsDirectory(); +} + +export async function createNamedBackup( + name: string, + options: { force?: boolean } = {}, +): Promise { + const normalizedName = normalizeBackupName(name); + const backupPath = resolveNamedBackupPath(normalizedName); + await ensureNamedBackupsDirectory(); + await exportAccounts(backupPath, options.force ?? false); + const candidate = await loadBackupCandidate(backupPath); + return buildNamedBackupMetadata(normalizedName, backupPath, { candidate }); +} + +export async function assessNamedBackupRestore( + name: string, + options: { currentStorage?: AccountStorageV3 | null } = {}, +): Promise { + const normalizedName = normalizeBackupName(name); + const backupPath = resolveNamedBackupPath(normalizedName); + const candidate = await loadBackupCandidate(backupPath); + const backup = await buildNamedBackupMetadata(normalizedName, backupPath, { + candidate, + }); + const currentStorage = options.currentStorage ?? (await loadAccounts()); + const currentAccounts = currentStorage?.accounts ?? []; + + if ( + !candidate.normalized || + !backup.accountCount || + backup.accountCount <= 0 + ) { + return { + backup, + currentAccountCount: currentAccounts.length, + mergedAccountCount: null, + imported: null, + skipped: null, + wouldExceedLimit: false, + valid: false, + error: backup.loadError ?? "Backup is empty or invalid", + }; + } + + const mergedAccounts = deduplicateAccountsByEmail( + deduplicateAccounts([...currentAccounts, ...candidate.normalized.accounts]), + ); + const wouldExceedLimit = mergedAccounts.length > ACCOUNT_LIMITS.MAX_ACCOUNTS; + const imported = wouldExceedLimit + ? null + : mergedAccounts.length - currentAccounts.length; + const skipped = wouldExceedLimit + ? null + : Math.max(0, candidate.normalized.accounts.length - (imported ?? 0)); + + return { + backup, + currentAccountCount: currentAccounts.length, + mergedAccountCount: mergedAccounts.length, + imported, + skipped, + wouldExceedLimit, + valid: !wouldExceedLimit, + error: wouldExceedLimit + ? `Restore would exceed maximum of ${ACCOUNT_LIMITS.MAX_ACCOUNTS} accounts` + : undefined, + }; +} + +export async function restoreNamedBackup( + name: string, +): Promise<{ imported: number; total: number; skipped: number }> { + const normalizedName = normalizeBackupName(name); + const backupPath = resolveNamedBackupPath(normalizedName); + return importAccounts(backupPath); } async function saveAccountsUnlocked(storage: AccountStorageV3): Promise { - const path = getStoragePath(); - const uniqueSuffix = `${Date.now()}.${Math.random().toString(36).slice(2, 8)}`; - const tempPath = `${path}.${uniqueSuffix}.tmp`; - const walPath = getAccountsWalPath(path); + const path = getStoragePath(); + const uniqueSuffix = `${Date.now()}.${Math.random().toString(36).slice(2, 8)}`; + const tempPath = `${path}.${uniqueSuffix}.tmp`; + const walPath = getAccountsWalPath(path); - try { - await fs.mkdir(dirname(path), { recursive: true }); - await ensureGitignore(path); + try { + await fs.mkdir(dirname(path), { recursive: true }); + await ensureGitignore(path); - if (looksLikeSyntheticFixtureStorage(storage)) { - try { - const existing = await loadNormalizedStorageFromPath(path, "existing account storage"); - if (existing && existing.accounts.length > 0 && !looksLikeSyntheticFixtureStorage(existing)) { - throw new StorageError( - "Refusing to overwrite non-synthetic account storage with synthetic fixture payload", - "EINVALID", + if (looksLikeSyntheticFixtureStorage(storage)) { + try { + const existing = await loadNormalizedStorageFromPath( path, - "Detected synthetic fixture-like account payload. Use explicit account import/login commands instead.", + "existing account storage", ); + if ( + existing && + existing.accounts.length > 0 && + !looksLikeSyntheticFixtureStorage(existing) + ) { + throw new StorageError( + "Refusing to overwrite non-synthetic account storage with synthetic fixture payload", + "EINVALID", + path, + "Detected synthetic fixture-like account payload. Use explicit account import/login commands instead.", + ); + } + } catch (error) { + if (error instanceof StorageError) { + throw error; + } + // Ignore existing-file probe failures and continue with normal save flow. } - } catch (error) { - if (error instanceof StorageError) { - throw error; + } + + if (storageBackupEnabled && existsSync(path)) { + try { + await createRotatingAccountsBackup(path); + } catch (backupError) { + log.warn("Failed to create account storage backup", { + path, + backupPath: getAccountsBackupPath(path), + error: String(backupError), + }); } - // Ignore existing-file probe failures and continue with normal save flow. } - } - if (storageBackupEnabled && existsSync(path)) { - try { - await createRotatingAccountsBackup(path); - } catch (backupError) { - log.warn("Failed to create account storage backup", { - path, - backupPath: getAccountsBackupPath(path), - error: String(backupError), - }); + const content = JSON.stringify(storage, null, 2); + const journalEntry: AccountsJournalEntry = { + version: 1, + createdAt: Date.now(), + path, + checksum: computeSha256(content), + content, + }; + await fs.writeFile(walPath, JSON.stringify(journalEntry), { + encoding: "utf-8", + mode: 0o600, + }); + await fs.writeFile(tempPath, content, { encoding: "utf-8", mode: 0o600 }); + + const stats = await fs.stat(tempPath); + if (stats.size === 0) { + const emptyError = Object.assign( + new Error("File written but size is 0"), + { code: "EEMPTY" }, + ); + throw emptyError; } - } - const content = JSON.stringify(storage, null, 2); - const journalEntry: AccountsJournalEntry = { - version: 1, - createdAt: Date.now(), - path, - checksum: computeSha256(content), - content, - }; - await fs.writeFile(walPath, JSON.stringify(journalEntry), { - encoding: "utf-8", - mode: 0o600, - }); - await fs.writeFile(tempPath, content, { encoding: "utf-8", mode: 0o600 }); - - const stats = await fs.stat(tempPath); - if (stats.size === 0) { - const emptyError = Object.assign(new Error("File written but size is 0"), { code: "EEMPTY" }); - throw emptyError; - } - - // Retry rename with exponential backoff for Windows EPERM/EBUSY - let lastError: NodeJS.ErrnoException | null = null; - for (let attempt = 0; attempt < 5; attempt++) { - try { - await fs.rename(tempPath, path); - lastAccountsSaveTimestamp = Date.now(); + // Retry rename with exponential backoff for Windows EPERM/EBUSY + let lastError: NodeJS.ErrnoException | null = null; + for (let attempt = 0; attempt < 5; attempt++) { + try { + await fs.rename(tempPath, path); + lastAccountsSaveTimestamp = Date.now(); + try { + await fs.unlink(walPath); + } catch { + // Best effort cleanup. + } + return; + } catch (renameError) { + const code = (renameError as NodeJS.ErrnoException).code; + if (code === "EPERM" || code === "EBUSY") { + lastError = renameError as NodeJS.ErrnoException; + await new Promise((r) => setTimeout(r, 10 * 2 ** attempt)); + continue; + } + throw renameError; + } + } + if (lastError) throw lastError; + } catch (error) { try { - await fs.unlink(walPath); + await fs.unlink(tempPath); } catch { - // Best effort cleanup. + // Ignore cleanup failure. } - return; - } catch (renameError) { - const code = (renameError as NodeJS.ErrnoException).code; - if (code === "EPERM" || code === "EBUSY") { - lastError = renameError as NodeJS.ErrnoException; - await new Promise(r => setTimeout(r, 10 * Math.pow(2, attempt))); - continue; - } - throw renameError; - } - } - if (lastError) throw lastError; - } catch (error) { - try { - await fs.unlink(tempPath); - } catch { - // Ignore cleanup failure. - } - - const err = error as NodeJS.ErrnoException; - const code = err?.code || "UNKNOWN"; - const hint = formatStorageErrorHint(error, path); - - log.error("Failed to save accounts", { - path, - code, - message: err?.message, - hint, - }); - - throw new StorageError( - `Failed to save accounts: ${err?.message || "Unknown error"}`, - code, - path, - hint, - err instanceof Error ? err : undefined - ); - } + + const err = error as NodeJS.ErrnoException; + const code = err?.code || "UNKNOWN"; + const hint = formatStorageErrorHint(error, path); + + log.error("Failed to save accounts", { + path, + code, + message: err?.message, + hint, + }); + + throw new StorageError( + `Failed to save accounts: ${err?.message || "Unknown error"}`, + code, + path, + hint, + err instanceof Error ? err : undefined, + ); + } } export async function withAccountStorageTransaction( - handler: ( - current: AccountStorageV3 | null, - persist: (storage: AccountStorageV3) => Promise, - ) => Promise, + handler: ( + current: AccountStorageV3 | null, + persist: (storage: AccountStorageV3) => Promise, + ) => Promise, ): Promise { - return withStorageLock(async () => { - const current = await loadAccountsInternal(saveAccountsUnlocked); - return handler(current, saveAccountsUnlocked); - }); + return withStorageLock(async () => { + const current = await loadAccountsInternal(saveAccountsUnlocked); + return handler(current, saveAccountsUnlocked); + }); } /** @@ -1160,9 +1529,9 @@ export async function withAccountStorageTransaction( * @throws StorageError with platform-aware hints on failure */ export async function saveAccounts(storage: AccountStorageV3): Promise { - return withStorageLock(async () => { - await saveAccountsUnlocked(storage); - }); + return withStorageLock(async () => { + await saveAccountsUnlocked(storage); + }); } /** @@ -1170,30 +1539,34 @@ export async function saveAccounts(storage: AccountStorageV3): Promise { * Silently ignores if file doesn't exist. */ export async function clearAccounts(): Promise { - return withStorageLock(async () => { - const path = getStoragePath(); - const walPath = getAccountsWalPath(path); - const backupPaths = getAccountsBackupRecoveryCandidates(path); - const clearPath = async (targetPath: string): Promise => { - try { - await fs.unlink(targetPath); - } catch (error) { - const code = (error as NodeJS.ErrnoException).code; - if (code !== "ENOENT") { - log.error("Failed to clear account storage artifact", { - path: targetPath, - error: String(error), - }); - } - } - }; - - try { - await Promise.all([clearPath(path), clearPath(walPath), ...backupPaths.map(clearPath)]); - } catch { - // Individual path cleanup is already best-effort with per-artifact logging. - } - }); + return withStorageLock(async () => { + const path = getStoragePath(); + const walPath = getAccountsWalPath(path); + const backupPaths = getAccountsBackupRecoveryCandidates(path); + const clearPath = async (targetPath: string): Promise => { + try { + await fs.unlink(targetPath); + } catch (error) { + const code = (error as NodeJS.ErrnoException).code; + if (code !== "ENOENT") { + log.error("Failed to clear account storage artifact", { + path: targetPath, + error: String(error), + }); + } + } + }; + + try { + await Promise.all([ + clearPath(path), + clearPath(walPath), + ...backupPaths.map(clearPath), + ]); + } catch { + // Individual path cleanup is already best-effort with per-artifact logging. + } + }); } function normalizeFlaggedStorage(data: unknown): FlaggedAccountStorageV1 { @@ -1205,14 +1578,22 @@ function normalizeFlaggedStorage(data: unknown): FlaggedAccountStorageV1 { for (const rawAccount of data.accounts) { if (!isRecord(rawAccount)) continue; const refreshToken = - typeof rawAccount.refreshToken === "string" ? rawAccount.refreshToken.trim() : ""; + typeof rawAccount.refreshToken === "string" + ? rawAccount.refreshToken.trim() + : ""; if (!refreshToken) continue; - const flaggedAt = typeof rawAccount.flaggedAt === "number" ? rawAccount.flaggedAt : Date.now(); + const flaggedAt = + typeof rawAccount.flaggedAt === "number" + ? rawAccount.flaggedAt + : Date.now(); const isAccountIdSource = ( value: unknown, ): value is AccountMetadataV3["accountIdSource"] => - value === "token" || value === "id_token" || value === "org" || value === "manual"; + value === "token" || + value === "id_token" || + value === "org" || + value === "manual"; const isSwitchReason = ( value: unknown, ): value is AccountMetadataV3["lastSwitchReason"] => @@ -1220,12 +1601,18 @@ function normalizeFlaggedStorage(data: unknown): FlaggedAccountStorageV1 { const isCooldownReason = ( value: unknown, ): value is AccountMetadataV3["cooldownReason"] => - value === "auth-failure" || value === "network-error" || value === "rate-limit"; + value === "auth-failure" || + value === "network-error" || + value === "rate-limit"; - let rateLimitResetTimes: AccountMetadataV3["rateLimitResetTimes"] | undefined; + let rateLimitResetTimes: + | AccountMetadataV3["rateLimitResetTimes"] + | undefined; if (isRecord(rawAccount.rateLimitResetTimes)) { const normalizedRateLimits: Record = {}; - for (const [key, value] of Object.entries(rawAccount.rateLimitResetTimes)) { + for (const [key, value] of Object.entries( + rawAccount.rateLimitResetTimes, + )) { if (typeof value === "number") { normalizedRateLimits[key] = value; } @@ -1247,21 +1634,43 @@ function normalizeFlaggedStorage(data: unknown): FlaggedAccountStorageV1 { const normalized: FlaggedAccountMetadataV1 = { refreshToken, - addedAt: typeof rawAccount.addedAt === "number" ? rawAccount.addedAt : flaggedAt, - lastUsed: typeof rawAccount.lastUsed === "number" ? rawAccount.lastUsed : flaggedAt, - accountId: typeof rawAccount.accountId === "string" ? rawAccount.accountId : undefined, + addedAt: + typeof rawAccount.addedAt === "number" ? rawAccount.addedAt : flaggedAt, + lastUsed: + typeof rawAccount.lastUsed === "number" + ? rawAccount.lastUsed + : flaggedAt, + accountId: + typeof rawAccount.accountId === "string" + ? rawAccount.accountId + : undefined, accountIdSource, - accountLabel: typeof rawAccount.accountLabel === "string" ? rawAccount.accountLabel : undefined, - email: typeof rawAccount.email === "string" ? rawAccount.email : undefined, - enabled: typeof rawAccount.enabled === "boolean" ? rawAccount.enabled : undefined, + accountLabel: + typeof rawAccount.accountLabel === "string" + ? rawAccount.accountLabel + : undefined, + email: + typeof rawAccount.email === "string" ? rawAccount.email : undefined, + enabled: + typeof rawAccount.enabled === "boolean" + ? rawAccount.enabled + : undefined, lastSwitchReason, rateLimitResetTimes, coolingDownUntil: - typeof rawAccount.coolingDownUntil === "number" ? rawAccount.coolingDownUntil : undefined, + typeof rawAccount.coolingDownUntil === "number" + ? rawAccount.coolingDownUntil + : undefined, cooldownReason, flaggedAt, - flaggedReason: typeof rawAccount.flaggedReason === "string" ? rawAccount.flaggedReason : undefined, - lastError: typeof rawAccount.lastError === "string" ? rawAccount.lastError : undefined, + flaggedReason: + typeof rawAccount.flaggedReason === "string" + ? rawAccount.flaggedReason + : undefined, + lastError: + typeof rawAccount.lastError === "string" + ? rawAccount.lastError + : undefined, }; byRefreshToken.set(refreshToken, normalized); } @@ -1283,7 +1692,10 @@ export async function loadFlaggedAccounts(): Promise { } catch (error) { const code = (error as NodeJS.ErrnoException).code; if (code !== "ENOENT") { - log.error("Failed to load flagged account storage", { path, error: String(error) }); + log.error("Failed to load flagged account storage", { + path, + error: String(error), + }); return empty; } } @@ -1321,7 +1733,9 @@ export async function loadFlaggedAccounts(): Promise { } } -export async function saveFlaggedAccounts(storage: FlaggedAccountStorageV1): Promise { +export async function saveFlaggedAccounts( + storage: FlaggedAccountStorageV1, +): Promise { return withStorageLock(async () => { const path = getFlaggedAccountsPath(); const uniqueSuffix = `${Date.now()}.${Math.random().toString(36).slice(2, 8)}`; @@ -1338,7 +1752,10 @@ export async function saveFlaggedAccounts(storage: FlaggedAccountStorageV1): Pro } catch { // Ignore cleanup failures. } - log.error("Failed to save flagged account storage", { path, error: String(error) }); + log.error("Failed to save flagged account storage", { + path, + error: String(error), + }); throw error; } }); @@ -1351,7 +1768,9 @@ export async function clearFlaggedAccounts(): Promise { } catch (error) { const code = (error as NodeJS.ErrnoException).code; if (code !== "ENOENT") { - log.error("Failed to clear flagged account storage", { error: String(error) }); + log.error("Failed to clear flagged account storage", { + error: String(error), + }); } } }); @@ -1363,23 +1782,31 @@ export async function clearFlaggedAccounts(): Promise { * @param force - If true, overwrite existing file (default: true) * @throws Error if file exists and force is false, or if no accounts to export */ -export async function exportAccounts(filePath: string, force = true): Promise { - const resolvedPath = resolvePath(filePath); - - if (!force && existsSync(resolvedPath)) { - throw new Error(`File already exists: ${resolvedPath}`); - } - - const storage = await withAccountStorageTransaction((current) => Promise.resolve(current)); - if (!storage || storage.accounts.length === 0) { - throw new Error("No accounts to export"); - } - - await fs.mkdir(dirname(resolvedPath), { recursive: true }); - - const content = JSON.stringify(storage, null, 2); - await fs.writeFile(resolvedPath, content, { encoding: "utf-8", mode: 0o600 }); - log.info("Exported accounts", { path: resolvedPath, count: storage.accounts.length }); +export async function exportAccounts( + filePath: string, + force = true, +): Promise { + const resolvedPath = resolvePath(filePath); + + if (!force && existsSync(resolvedPath)) { + throw new Error(`File already exists: ${resolvedPath}`); + } + + const storage = await withAccountStorageTransaction((current) => + Promise.resolve(current), + ); + if (!storage || storage.accounts.length === 0) { + throw new Error("No accounts to export"); + } + + await fs.mkdir(dirname(resolvedPath), { recursive: true }); + + const content = JSON.stringify(storage, null, 2); + await fs.writeFile(resolvedPath, content, { encoding: "utf-8", mode: 0o600 }); + log.info("Exported accounts", { + path: resolvedPath, + count: storage.accounts.length, + }); } /** @@ -1388,61 +1815,73 @@ export async function exportAccounts(filePath: string, force = true): Promise { - const resolvedPath = resolvePath(filePath); - - // Check file exists with friendly error - if (!existsSync(resolvedPath)) { - throw new Error(`Import file not found: ${resolvedPath}`); - } - - const content = await fs.readFile(resolvedPath, "utf-8"); - - let imported: unknown; - try { - imported = JSON.parse(content); - } catch { - throw new Error(`Invalid JSON in import file: ${resolvedPath}`); - } - - const normalized = normalizeAccountStorage(imported); - if (!normalized) { - throw new Error("Invalid account storage format"); - } - - const { imported: importedCount, total, skipped: skippedCount } = - await withAccountStorageTransaction(async (existing, persist) => { - const existingAccounts = existing?.accounts ?? []; - const existingActiveIndex = existing?.activeIndex ?? 0; - - const merged = [...existingAccounts, ...normalized.accounts]; - - if (merged.length > ACCOUNT_LIMITS.MAX_ACCOUNTS) { - const deduped = deduplicateAccountsByEmail(deduplicateAccounts(merged)); - if (deduped.length > ACCOUNT_LIMITS.MAX_ACCOUNTS) { - throw new Error( - `Import would exceed maximum of ${ACCOUNT_LIMITS.MAX_ACCOUNTS} accounts (would have ${deduped.length})` - ); - } - } - - const deduplicatedAccounts = deduplicateAccountsByEmail(deduplicateAccounts(merged)); - - const newStorage: AccountStorageV3 = { - version: 3, - accounts: deduplicatedAccounts, - activeIndex: existingActiveIndex, - activeIndexByFamily: existing?.activeIndexByFamily, - }; - - await persist(newStorage); - - const imported = deduplicatedAccounts.length - existingAccounts.length; - const skipped = normalized.accounts.length - imported; - return { imported, total: deduplicatedAccounts.length, skipped }; - }); - - log.info("Imported accounts", { path: resolvedPath, imported: importedCount, skipped: skippedCount, total }); - - return { imported: importedCount, total, skipped: skippedCount }; +export async function importAccounts( + filePath: string, +): Promise<{ imported: number; total: number; skipped: number }> { + const resolvedPath = resolvePath(filePath); + + // Check file exists with friendly error + if (!existsSync(resolvedPath)) { + throw new Error(`Import file not found: ${resolvedPath}`); + } + + const content = await fs.readFile(resolvedPath, "utf-8"); + + let imported: unknown; + try { + imported = JSON.parse(content); + } catch { + throw new Error(`Invalid JSON in import file: ${resolvedPath}`); + } + + const normalized = normalizeAccountStorage(imported); + if (!normalized) { + throw new Error("Invalid account storage format"); + } + + const { + imported: importedCount, + total, + skipped: skippedCount, + } = await withAccountStorageTransaction(async (existing, persist) => { + const existingAccounts = existing?.accounts ?? []; + const existingActiveIndex = existing?.activeIndex ?? 0; + + const merged = [...existingAccounts, ...normalized.accounts]; + + if (merged.length > ACCOUNT_LIMITS.MAX_ACCOUNTS) { + const deduped = deduplicateAccountsByEmail(deduplicateAccounts(merged)); + if (deduped.length > ACCOUNT_LIMITS.MAX_ACCOUNTS) { + throw new Error( + `Import would exceed maximum of ${ACCOUNT_LIMITS.MAX_ACCOUNTS} accounts (would have ${deduped.length})`, + ); + } + } + + const deduplicatedAccounts = deduplicateAccountsByEmail( + deduplicateAccounts(merged), + ); + + const newStorage: AccountStorageV3 = { + version: 3, + accounts: deduplicatedAccounts, + activeIndex: existingActiveIndex, + activeIndexByFamily: existing?.activeIndexByFamily, + }; + + await persist(newStorage); + + const imported = deduplicatedAccounts.length - existingAccounts.length; + const skipped = normalized.accounts.length - imported; + return { imported, total: deduplicatedAccounts.length, skipped }; + }); + + log.info("Imported accounts", { + path: resolvedPath, + imported: importedCount, + skipped: skippedCount, + total, + }); + + return { imported: importedCount, total, skipped: skippedCount }; } diff --git a/lib/ui/auth-menu.ts b/lib/ui/auth-menu.ts index de2546a..c6e4536 100644 --- a/lib/ui/auth-menu.ts +++ b/lib/ui/auth-menu.ts @@ -65,6 +65,7 @@ export type AuthMenuAction = | { type: "check" } | { type: "deep-check" } | { type: "verify-flagged" } + | { type: "restore-backup" } | { type: "select-account"; account: AccountInfo } | { type: "set-current-account"; account: AccountInfo } | { type: "refresh-account"; account: AccountInfo } @@ -515,6 +516,7 @@ function authMenuFocusKey(action: AuthMenuAction): string { case "check": case "deep-check": case "verify-flagged": + case "restore-backup": case "search": case "delete-all": case "cancel": @@ -653,6 +655,17 @@ export async function showAuthMenu( ); } + items.push({ label: "", value: { type: "cancel" }, separator: true }); + items.push({ + label: UI_COPY.mainMenu.recovery, + value: { type: "cancel" }, + kind: "heading", + }); + items.push({ + label: UI_COPY.mainMenu.restoreBackup, + value: { type: "restore-backup" }, + color: "yellow", + }); items.push({ label: "", value: { type: "cancel" }, separator: true }); items.push({ label: UI_COPY.mainMenu.dangerZone, diff --git a/lib/ui/copy.ts b/lib/ui/copy.ts index fe6280e..8aec4ff 100644 --- a/lib/ui/copy.ts +++ b/lib/ui/copy.ts @@ -14,6 +14,8 @@ export const UI_COPY = { accounts: "Saved Accounts", loadingLimits: "Fetching account limits...", noSearchMatches: "No accounts match your search", + recovery: "Recovery", + restoreBackup: "Restore From Backup", dangerZone: "Danger Zone", removeAllAccounts: "Delete Saved Accounts", resetLocalState: "Reset Local State", diff --git a/test/storage.test.ts b/test/storage.test.ts index 18e3e00..811329b 100644 --- a/test/storage.test.ts +++ b/test/storage.test.ts @@ -5,15 +5,19 @@ import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; import { clearQuotaCache, getQuotaCachePath } from "../lib/quota-cache.js"; import { getConfigDir, getProjectStorageKey } from "../lib/storage/paths.js"; import { + assessNamedBackupRestore, clearAccounts, + createNamedBackup, deduplicateAccounts, deduplicateAccountsByEmail, exportAccounts, formatStorageErrorHint, getStoragePath, importAccounts, + listNamedBackups, loadAccounts, normalizeAccountStorage, + restoreNamedBackup, StorageError, saveAccounts, setStoragePath, @@ -294,6 +298,92 @@ describe("storage", () => { }); }); + describe("named backups", () => { + const testWorkDir = join( + tmpdir(), + "codex-backup-" + Math.random().toString(36).slice(2), + ); + let testStoragePath = ""; + + beforeEach(async () => { + await fs.mkdir(testWorkDir, { recursive: true }); + testStoragePath = join(testWorkDir, "openai-codex-accounts.json"); + setStoragePathDirect(testStoragePath); + }); + + afterEach(async () => { + setStoragePathDirect(null); + await fs.rm(testWorkDir, { recursive: true, force: true }); + }); + + it("creates and lists named backups with metadata", async () => { + await saveAccounts({ + version: 3, + activeIndex: 0, + accounts: [ + { + accountId: "acct-backup", + refreshToken: "ref-backup", + addedAt: 1, + lastUsed: 2, + }, + ], + }); + + const backup = await createNamedBackup("My Backup 1"); + expect(backup.name).toBe("My-Backup-1"); + const backups = await listNamedBackups(); + expect(backups.length).toBeGreaterThan(0); + expect(backups[0]?.accountCount).toBe(1); + const backupPath = join(testWorkDir, "backups", `${backup.name}.json`); + expect(existsSync(backupPath)).toBe(true); + }); + + it("assesses eligibility and restores a named backup", async () => { + await saveAccounts({ + version: 3, + activeIndex: 0, + accounts: [ + { + accountId: "primary", + refreshToken: "ref-primary", + addedAt: 1, + lastUsed: 1, + }, + ], + }); + + await createNamedBackup("restore-me"); + await clearAccounts(); + + const assessment = await assessNamedBackupRestore("restore-me"); + expect(assessment.valid).toBe(true); + expect(assessment.wouldExceedLimit).toBe(false); + + const restoreResult = await restoreNamedBackup("restore-me"); + expect(restoreResult.total).toBe(1); + + const restored = await loadAccounts(); + expect(restored?.accounts[0]?.accountId).toBe("primary"); + }); + + it("rejects invalid backup names", async () => { + await saveAccounts({ + version: 3, + activeIndex: 0, + accounts: [ + { + accountId: "primary", + refreshToken: "ref-primary", + addedAt: 1, + lastUsed: 1, + }, + ], + }); + await expect(createNamedBackup(" ")).rejects.toThrow(/Invalid backup name/); + }); + }); + describe("filename migration (TDD)", () => { it("should migrate from old filename to new filename", async () => { // This test is tricky because it depends on the internal state of getStoragePath() From c10e66ba9c70aadafb0ddee4cca339e2c3e67a62 Mon Sep 17 00:00:00 2001 From: ndycode Date: Thu, 12 Mar 2026 15:40:13 +0800 Subject: [PATCH 2/9] test(auth): cover backup restore manager flow --- test/codex-manager-cli.test.ts | 107 +++++++++++++++++++++++++++++++++ 1 file changed, 107 insertions(+) diff --git a/test/codex-manager-cli.test.ts b/test/codex-manager-cli.test.ts index 334a2bf..31d03bb 100644 --- a/test/codex-manager-cli.test.ts +++ b/test/codex-manager-cli.test.ts @@ -6,6 +6,10 @@ const saveAccountsMock = vi.fn(); const saveFlaggedAccountsMock = vi.fn(); const setStoragePathMock = vi.fn(); const getStoragePathMock = vi.fn(() => "/mock/openai-codex-accounts.json"); +const listNamedBackupsMock = vi.fn(); +const assessNamedBackupRestoreMock = vi.fn(); +const getNamedBackupsDirectoryPathMock = vi.fn(); +const restoreNamedBackupMock = vi.fn(); const queuedRefreshMock = vi.fn(); const setCodexCliActiveSelectionMock = vi.fn(); const promptAddAnotherAccountMock = vi.fn(); @@ -88,6 +92,10 @@ vi.mock("../lib/storage.js", () => ({ saveFlaggedAccounts: saveFlaggedAccountsMock, setStoragePath: setStoragePathMock, getStoragePath: getStoragePathMock, + listNamedBackups: listNamedBackupsMock, + assessNamedBackupRestore: assessNamedBackupRestoreMock, + getNamedBackupsDirectoryPath: getNamedBackupsDirectoryPathMock, + restoreNamedBackup: restoreNamedBackupMock, })); vi.mock("../lib/refresh-queue.js", () => ({ @@ -249,6 +257,38 @@ describe("codex manager cli commands", () => { version: 1, accounts: [], }); + listNamedBackupsMock.mockReset(); + assessNamedBackupRestoreMock.mockReset(); + getNamedBackupsDirectoryPathMock.mockReset(); + restoreNamedBackupMock.mockReset(); + listNamedBackupsMock.mockResolvedValue([]); + assessNamedBackupRestoreMock.mockResolvedValue({ + backup: { + name: "named-backup", + path: "/mock/backups/named-backup.json", + createdAt: null, + updatedAt: null, + sizeBytes: null, + version: 3, + accountCount: 1, + schemaErrors: [], + valid: true, + loadError: undefined, + }, + currentAccountCount: 0, + mergedAccountCount: 1, + imported: 1, + skipped: 0, + wouldExceedLimit: false, + valid: true, + error: undefined, + }); + getNamedBackupsDirectoryPathMock.mockReturnValue("/mock/backups"); + restoreNamedBackupMock.mockResolvedValue({ + imported: 1, + skipped: 0, + total: 1, + }); loadDashboardDisplaySettingsMock.mockResolvedValue({ showPerAccountRows: true, showQuotaDetails: true, @@ -1518,6 +1558,73 @@ describe("codex manager cli commands", () => { ); }); + it("restores a named backup from the login recovery menu", async () => { + setInteractiveTTY(true); + const now = Date.now(); + loadAccountsMock.mockResolvedValue({ + version: 3, + activeIndex: 0, + activeIndexByFamily: { codex: 0 }, + accounts: [ + { + email: "settings@example.com", + accountId: "acc_settings", + refreshToken: "refresh-settings", + accessToken: "access-settings", + expiresAt: now + 3_600_000, + addedAt: now - 1_000, + lastUsed: now - 1_000, + enabled: true, + }, + ], + }); + const assessment = { + backup: { + name: "named-backup", + path: "/mock/backups/named-backup.json", + createdAt: null, + updatedAt: now, + sizeBytes: 128, + version: 3, + accountCount: 1, + schemaErrors: [], + valid: true, + loadError: undefined, + }, + currentAccountCount: 1, + mergedAccountCount: 2, + imported: 1, + skipped: 0, + wouldExceedLimit: false, + valid: true, + error: undefined, + }; + listNamedBackupsMock.mockResolvedValue([assessment.backup]); + assessNamedBackupRestoreMock.mockResolvedValue(assessment); + promptLoginModeMock + .mockResolvedValueOnce({ mode: "restore-backup" }) + .mockResolvedValueOnce({ mode: "cancel" }); + selectMock.mockResolvedValueOnce({ type: "restore", assessment }); + const confirmMock = vi.fn().mockResolvedValue(true); + vi.doMock("../lib/ui/confirm.js", () => ({ confirm: confirmMock })); + vi.resetModules(); + const { runCodexMultiAuthCli } = await import("../lib/codex-manager.js"); + + const exitCode = await runCodexMultiAuthCli(["auth", "login"]); + + expect(exitCode).toBe(0); + expect(listNamedBackupsMock).toHaveBeenCalledTimes(1); + expect(assessNamedBackupRestoreMock).toHaveBeenCalledWith( + "named-backup", + expect.objectContaining({ + currentStorage: expect.objectContaining({ + accounts: expect.any(Array), + }), + }), + ); + expect(restoreNamedBackupMock).toHaveBeenCalledWith("named-backup"); + }); + it.each([ { panel: "account-list", mode: "windows-ebusy" }, { panel: "summary-fields", mode: "windows-ebusy" }, From bd8060be3487e1bb3d4f6643df34c33937ca89c3 Mon Sep 17 00:00:00 2001 From: ndycode Date: Fri, 13 Mar 2026 10:04:22 +0800 Subject: [PATCH 3/9] fix(auth): harden named backup restore paths --- lib/storage.ts | 25 +++++++++----- test/codex-manager-cli.test.ts | 1 + test/storage.test.ts | 63 ++++++++++++++++++++++++++++++++-- 3 files changed, 78 insertions(+), 11 deletions(-) diff --git a/lib/storage.ts b/lib/storage.ts index 344f97b..fe4fa56 100644 --- a/lib/storage.ts +++ b/lib/storage.ts @@ -472,11 +472,19 @@ async function ensureNamedBackupsDirectory(): Promise { } function resolveNamedBackupPath(name: string): string { - const normalizedName = normalizeBackupName(name); - return join( - getNamedBackupsDirectory(), - `${normalizedName}${NAMED_BACKUP_EXTENSION}`, + const backupDir = getNamedBackupsDirectory(); + const trimmedName = name.trim(); + const directPath = join( + backupDir, + trimmedName.endsWith(NAMED_BACKUP_EXTENSION) + ? trimmedName + : `${trimmedName}${NAMED_BACKUP_EXTENSION}`, ); + if (existsSync(directPath)) { + return directPath; + } + const normalizedName = normalizeBackupName(trimmedName); + return join(backupDir, `${normalizedName}${NAMED_BACKUP_EXTENSION}`); } function deriveBackupNameFromFile(fileName: string): string { @@ -1328,10 +1336,10 @@ export async function assessNamedBackupRestore( name: string, options: { currentStorage?: AccountStorageV3 | null } = {}, ): Promise { - const normalizedName = normalizeBackupName(name); - const backupPath = resolveNamedBackupPath(normalizedName); + const backupPath = resolveNamedBackupPath(name); + const backupName = deriveBackupNameFromFile(basename(backupPath)); const candidate = await loadBackupCandidate(backupPath); - const backup = await buildNamedBackupMetadata(normalizedName, backupPath, { + const backup = await buildNamedBackupMetadata(backupName, backupPath, { candidate, }); const currentStorage = options.currentStorage ?? (await loadAccounts()); @@ -1382,8 +1390,7 @@ export async function assessNamedBackupRestore( export async function restoreNamedBackup( name: string, ): Promise<{ imported: number; total: number; skipped: number }> { - const normalizedName = normalizeBackupName(name); - const backupPath = resolveNamedBackupPath(normalizedName); + const backupPath = resolveNamedBackupPath(name); return importAccounts(backupPath); } diff --git a/test/codex-manager-cli.test.ts b/test/codex-manager-cli.test.ts index 31d03bb..171fb7e 100644 --- a/test/codex-manager-cli.test.ts +++ b/test/codex-manager-cli.test.ts @@ -1623,6 +1623,7 @@ describe("codex manager cli commands", () => { }), ); expect(restoreNamedBackupMock).toHaveBeenCalledWith("named-backup"); + vi.doUnmock("../lib/ui/confirm.js"); }); it.each([ diff --git a/test/storage.test.ts b/test/storage.test.ts index 811329b..1fff276 100644 --- a/test/storage.test.ts +++ b/test/storage.test.ts @@ -127,7 +127,23 @@ describe("storage", () => { afterEach(async () => { setStoragePathDirect(null); - await fs.rm(testWorkDir, { recursive: true, force: true }); + for (let attempt = 0; attempt < 5; attempt += 1) { + try { + await fs.rm(testWorkDir, { recursive: true, force: true }); + break; + } catch (error) { + const code = (error as NodeJS.ErrnoException).code; + if ( + (code !== "EBUSY" && code !== "EPERM" && code !== "ENOTEMPTY") || + attempt === 4 + ) { + throw error; + } + await new Promise((resolve) => + setTimeout(resolve, 25 * 2 ** attempt), + ); + } + } }); it("should export accounts to a file", async () => { @@ -367,6 +383,47 @@ describe("storage", () => { expect(restored?.accounts[0]?.accountId).toBe("primary"); }); + it("restores manually named backups without normalized filenames", async () => { + await saveAccounts({ + version: 3, + activeIndex: 0, + accounts: [ + { + accountId: "manual", + refreshToken: "ref-manual", + addedAt: 1, + lastUsed: 1, + }, + ], + }); + + const backupsDir = join(testWorkDir, "backups"); + await fs.mkdir(backupsDir, { recursive: true }); + await fs.writeFile( + join(backupsDir, "Manual Backup.json"), + JSON.stringify({ + version: 3, + activeIndex: 0, + accounts: [ + { + accountId: "manual", + refreshToken: "ref-manual", + addedAt: 1, + lastUsed: 1, + }, + ], + }), + "utf-8", + ); + + await clearAccounts(); + const assessment = await assessNamedBackupRestore("Manual Backup"); + expect(assessment.valid).toBe(true); + expect(assessment.backup.name).toBe("Manual Backup"); + const restoreResult = await restoreNamedBackup("Manual Backup"); + expect(restoreResult.total).toBe(1); + }); + it("rejects invalid backup names", async () => { await saveAccounts({ version: 3, @@ -380,7 +437,9 @@ describe("storage", () => { }, ], }); - await expect(createNamedBackup(" ")).rejects.toThrow(/Invalid backup name/); + await expect(createNamedBackup(" ")).rejects.toThrow( + /Invalid backup name/, + ); }); }); From f5f39450dc82f34740e3f79bc3c6f7109fcf8d41 Mon Sep 17 00:00:00 2001 From: ndycode Date: Sat, 14 Mar 2026 01:32:03 +0800 Subject: [PATCH 4/9] fix(storage): cover restore backup edge cases --- lib/storage.ts | 29 +++++++------ test/storage.test.ts | 96 ++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 110 insertions(+), 15 deletions(-) diff --git a/lib/storage.ts b/lib/storage.ts index 037067a..3b4817a 100644 --- a/lib/storage.ts +++ b/lib/storage.ts @@ -1581,21 +1581,20 @@ export async function listNamedBackups(): Promise { const backupRoot = getNamedBackupRoot(getStoragePath()); try { const entries = await fs.readdir(backupRoot, { withFileTypes: true }); - const backups: NamedBackupMetadata[] = []; - for (const entry of entries) { - if (!entry.isFile() || entry.isSymbolicLink()) continue; - if (!entry.name.toLowerCase().endsWith(".json")) continue; - - const path = resolvePath(join(backupRoot, entry.name)); - const candidate = await loadBackupCandidate(path); - backups.push( - await buildNamedBackupMetadata( - entry.name.slice(0, -".json".length), - path, - { candidate }, - ), - ); - } + const backups = await Promise.all( + entries + .filter((entry) => entry.isFile() && !entry.isSymbolicLink()) + .filter((entry) => entry.name.toLowerCase().endsWith(".json")) + .map(async (entry) => { + const path = resolvePath(join(backupRoot, entry.name)); + const candidate = await loadBackupCandidate(path); + return buildNamedBackupMetadata( + entry.name.slice(0, -".json".length), + path, + { candidate }, + ); + }), + ); return backups.sort((left, right) => (right.updatedAt ?? 0) - (left.updatedAt ?? 0)); } catch (error) { const code = (error as NodeJS.ErrnoException).code; diff --git a/test/storage.test.ts b/test/storage.test.ts index 77c323f..e1af932 100644 --- a/test/storage.test.ts +++ b/test/storage.test.ts @@ -1176,6 +1176,102 @@ describe("storage", () => { expect(restoreResult.total).toBe(1); }); + it("throws when a named backup is deleted after assessment", async () => { + await saveAccounts({ + version: 3, + activeIndex: 0, + accounts: [ + { + accountId: "deleted-backup", + refreshToken: "ref-deleted-backup", + addedAt: 1, + lastUsed: 1, + }, + ], + }); + + const backup = await createNamedBackup("deleted-after-assessment"); + await clearAccounts(); + + const assessment = await assessNamedBackupRestore("deleted-after-assessment"); + expect(assessment.valid).toBe(true); + + await fs.unlink(backup.path); + + await expect( + restoreNamedBackup("deleted-after-assessment"), + ).rejects.toThrow(/Import file not found/); + }); + + it("throws when a named backup becomes invalid JSON after assessment", async () => { + await saveAccounts({ + version: 3, + activeIndex: 0, + accounts: [ + { + accountId: "invalid-backup", + refreshToken: "ref-invalid-backup", + addedAt: 1, + lastUsed: 1, + }, + ], + }); + + const backup = await createNamedBackup("invalid-after-assessment"); + await clearAccounts(); + + const assessment = await assessNamedBackupRestore("invalid-after-assessment"); + expect(assessment.valid).toBe(true); + + await fs.writeFile(backup.path, "not valid json {[", "utf-8"); + + await expect( + restoreNamedBackup("invalid-after-assessment"), + ).rejects.toThrow(/Invalid JSON in import file/); + }); + + it("throws when restoring would exceed the account limit after assessment", async () => { + const { ACCOUNT_LIMITS } = await import("../lib/constants.js"); + + await saveAccounts({ + version: 3, + activeIndex: 0, + accounts: [ + { + accountId: "limit-backup", + refreshToken: "ref-limit-backup", + addedAt: 1, + lastUsed: 1, + }, + ], + }); + + await createNamedBackup("limit-after-assessment"); + await clearAccounts(); + + const assessment = await assessNamedBackupRestore("limit-after-assessment"); + expect(assessment.valid).toBe(true); + expect(assessment.wouldExceedLimit).toBe(false); + + await saveAccounts({ + version: 3, + activeIndex: 0, + accounts: Array.from( + { length: ACCOUNT_LIMITS.MAX_ACCOUNTS }, + (_, index) => ({ + accountId: `current-${index}`, + refreshToken: `ref-current-${index}`, + addedAt: index + 1, + lastUsed: index + 1, + }), + ), + }); + + await expect( + restoreNamedBackup("limit-after-assessment"), + ).rejects.toThrow(/Import would exceed maximum of/); + }); + it.each(["../openai-codex-accounts", String.raw`..\openai-codex-accounts`])( "rejects backup names that escape the backups directory: %s", async (input) => { From c659e7255f49c6bf60d9b6881e1e6d46fdcb8c83 Mon Sep 17 00:00:00 2001 From: ndycode Date: Sat, 14 Mar 2026 01:35:27 +0800 Subject: [PATCH 5/9] fix(auth): harden backup restore follow-up --- lib/codex-manager.ts | 15 +++++-- test/storage.test.ts | 105 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 116 insertions(+), 4 deletions(-) diff --git a/lib/codex-manager.ts b/lib/codex-manager.ts index 9b4cae8..d905fd2 100644 --- a/lib/codex-manager.ts +++ b/lib/codex-manager.ts @@ -4284,10 +4284,17 @@ async function runBackupRestoreManager( const confirmed = await confirm(confirmMessage); if (!confirmed) return; - const result = await restoreNamedBackup(assessment.backup.name); - console.log( - `Restored backup "${assessment.backup.name}". Imported ${result.imported}, skipped ${result.skipped}, total ${result.total}.`, - ); + try { + const result = await restoreNamedBackup(assessment.backup.name); + console.log( + `Restored backup "${assessment.backup.name}". Imported ${result.imported}, skipped ${result.skipped}, total ${result.total}.`, + ); + } catch (error) { + const message = error instanceof Error ? error.message : String(error); + console.error( + `Restore failed: ${collapseWhitespace(message) || "unknown error"}`, + ); + } } export async function runCodexMultiAuthCli(rawArgs: string[]): Promise { diff --git a/test/storage.test.ts b/test/storage.test.ts index e1af932..8c9a440 100644 --- a/test/storage.test.ts +++ b/test/storage.test.ts @@ -2,6 +2,7 @@ import { existsSync, promises as fs } from "node:fs"; import { tmpdir } from "node:os"; import { dirname, join } from "node:path"; import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import { ACCOUNT_LIMITS } from "../lib/constants.js"; import { clearQuotaCache, getQuotaCachePath } from "../lib/quota-cache.js"; import { getConfigDir, getProjectStorageKey } from "../lib/storage/paths.js"; import { removeWithRetry } from "./helpers/remove-with-retry.js"; @@ -1279,6 +1280,110 @@ describe("storage", () => { await expect(restoreNamedBackup(input)).rejects.toThrow(); }, ); + + it("throws when a named backup is deleted after assessment", async () => { + await saveAccounts({ + version: 3, + activeIndex: 0, + accounts: [ + { + accountId: "deleted-backup", + refreshToken: "ref-deleted-backup", + addedAt: 1, + lastUsed: 1, + }, + ], + }); + await createNamedBackup("deleted-backup"); + await clearAccounts(); + + const assessment = await assessNamedBackupRestore("deleted-backup"); + expect(assessment.valid).toBe(true); + + await fs.rm(buildNamedBackupPath("deleted-backup"), { force: true }); + + await expect(restoreNamedBackup("deleted-backup")).rejects.toThrow( + /Import file not found/, + ); + }); + + it("throws when a named backup becomes invalid JSON before restore", async () => { + await saveAccounts({ + version: 3, + activeIndex: 0, + accounts: [ + { + accountId: "corrupt-backup", + refreshToken: "ref-corrupt-backup", + addedAt: 1, + lastUsed: 1, + }, + ], + }); + await createNamedBackup("corrupt-backup"); + await clearAccounts(); + + const assessment = await assessNamedBackupRestore("corrupt-backup"); + expect(assessment.valid).toBe(true); + + await fs.writeFile(buildNamedBackupPath("corrupt-backup"), "{", "utf-8"); + + await expect(restoreNamedBackup("corrupt-backup")).rejects.toThrow( + /Invalid JSON in import file/, + ); + }); + + it("throws when current accounts exceed the limit after assessment", async () => { + await saveAccounts({ + version: 3, + activeIndex: 0, + accounts: [ + { + accountId: "limit-backup", + refreshToken: "ref-limit-backup", + addedAt: 1, + lastUsed: 1, + }, + ], + }); + await createNamedBackup("limit-backup"); + + await saveAccounts({ + version: 3, + activeIndex: 0, + accounts: Array.from( + { length: ACCOUNT_LIMITS.MAX_ACCOUNTS - 1 }, + (_, index) => ({ + accountId: `current-${index}`, + refreshToken: `ref-current-${index}`, + addedAt: index + 1, + lastUsed: index + 1, + }), + ), + }); + + const assessment = await assessNamedBackupRestore("limit-backup"); + expect(assessment.valid).toBe(true); + expect(assessment.wouldExceedLimit).toBe(false); + + await saveAccounts({ + version: 3, + activeIndex: 0, + accounts: Array.from( + { length: ACCOUNT_LIMITS.MAX_ACCOUNTS }, + (_, index) => ({ + accountId: `grown-${index}`, + refreshToken: `ref-grown-${index}`, + addedAt: index + 1, + lastUsed: index + 1, + }), + ), + }); + + await expect(restoreNamedBackup("limit-backup")).rejects.toThrow( + `Import would exceed maximum of ${ACCOUNT_LIMITS.MAX_ACCOUNTS} accounts`, + ); + }); }); describe("filename migration (TDD)", () => { From 11f8c45899d20fde8443883e612c472a23329fdc Mon Sep 17 00:00:00 2001 From: ndycode Date: Sat, 14 Mar 2026 01:37:52 +0800 Subject: [PATCH 6/9] fix(storage): guard backup restore edge cases --- lib/storage.ts | 40 ++++++++-- test/storage.test.ts | 176 ++++++++++++++++++++++++++++++++----------- 2 files changed, 166 insertions(+), 50 deletions(-) diff --git a/lib/storage.ts b/lib/storage.ts index 3b4817a..4ecda7b 100644 --- a/lib/storage.ts +++ b/lib/storage.ts @@ -1776,7 +1776,29 @@ async function resolveNamedBackupRestorePath(name: string): Promise { if (existingPath) { return existingPath; } - return buildNamedBackupPath(name); + const requested = (name ?? "").trim(); + const backupRoot = getNamedBackupRoot(getStoragePath()); + const requestedWithExtension = requested.toLowerCase().endsWith(".json") + ? requested + : `${requested}.json`; + try { + return buildNamedBackupPath(name); + } catch (error) { + const baseName = requestedWithExtension.toLowerCase().endsWith(".json") + ? requestedWithExtension.slice(0, -".json".length) + : requestedWithExtension; + if ( + requested.length > 0 && + basename(requestedWithExtension) === requestedWithExtension && + !requestedWithExtension.includes("..") && + !/^[A-Za-z0-9_-]+$/.test(baseName) + ) { + throw new Error( + `Import file not found: ${resolvePath(join(backupRoot, requestedWithExtension))}`, + ); + } + throw error; + } } async function loadAccountsFromJournal( @@ -2594,13 +2616,15 @@ export async function exportAccounts( const transactionState = transactionSnapshotContext.getStore(); const currentStoragePath = getStoragePath(); - const storage = - transactionState?.active && - transactionState.storagePath === currentStoragePath - ? transactionState.snapshot - : await withAccountStorageTransaction((current) => - Promise.resolve(current), - ); + const storage = transactionState?.active + ? transactionState.storagePath === currentStoragePath + ? transactionState.snapshot + : (() => { + throw new Error( + "exportAccounts called inside an active transaction for a different storage path", + ); + })() + : await withAccountStorageTransaction((current) => Promise.resolve(current)); if (!storage || storage.accounts.length === 0) { throw new Error("No accounts to export"); } diff --git a/test/storage.test.ts b/test/storage.test.ts index 8c9a440..781fb73 100644 --- a/test/storage.test.ts +++ b/test/storage.test.ts @@ -369,6 +369,34 @@ describe("storage", () => { ); }); + it("throws when exporting inside an active transaction for a different storage path", async () => { + await saveAccounts({ + version: 3, + activeIndex: 0, + accounts: [ + { + accountId: "transactional-export", + refreshToken: "ref-transactional-export", + addedAt: 1, + lastUsed: 1, + }, + ], + }); + + const alternateStoragePath = join(testWorkDir, "alternate-accounts.json"); + + await expect( + withAccountStorageTransaction(async () => { + setStoragePathDirect(alternateStoragePath); + try { + await exportAccounts(exportPath); + } finally { + setStoragePathDirect(testStoragePath); + } + }), + ).rejects.toThrow(/different storage path/); + }); + it("should import accounts from a file and merge", async () => { // @ts-expect-error const { importAccounts } = await import("../lib/storage.js"); @@ -1231,48 +1259,6 @@ describe("storage", () => { ).rejects.toThrow(/Invalid JSON in import file/); }); - it("throws when restoring would exceed the account limit after assessment", async () => { - const { ACCOUNT_LIMITS } = await import("../lib/constants.js"); - - await saveAccounts({ - version: 3, - activeIndex: 0, - accounts: [ - { - accountId: "limit-backup", - refreshToken: "ref-limit-backup", - addedAt: 1, - lastUsed: 1, - }, - ], - }); - - await createNamedBackup("limit-after-assessment"); - await clearAccounts(); - - const assessment = await assessNamedBackupRestore("limit-after-assessment"); - expect(assessment.valid).toBe(true); - expect(assessment.wouldExceedLimit).toBe(false); - - await saveAccounts({ - version: 3, - activeIndex: 0, - accounts: Array.from( - { length: ACCOUNT_LIMITS.MAX_ACCOUNTS }, - (_, index) => ({ - accountId: `current-${index}`, - refreshToken: `ref-current-${index}`, - addedAt: index + 1, - lastUsed: index + 1, - }), - ), - }); - - await expect( - restoreNamedBackup("limit-after-assessment"), - ).rejects.toThrow(/Import would exceed maximum of/); - }); - it.each(["../openai-codex-accounts", String.raw`..\openai-codex-accounts`])( "rejects backup names that escape the backups directory: %s", async (input) => { @@ -1307,6 +1293,41 @@ describe("storage", () => { ); }); + it("throws file-not-found when a manually named backup disappears after assessment", async () => { + const backupPath = join( + dirname(testStoragePath), + "backups", + "Manual Backup.json", + ); + await fs.mkdir(dirname(backupPath), { recursive: true }); + await fs.writeFile( + backupPath, + JSON.stringify({ + version: 3, + activeIndex: 0, + accounts: [ + { + accountId: "manual-missing", + refreshToken: "ref-manual-missing", + addedAt: 1, + lastUsed: 1, + }, + ], + }), + "utf-8", + ); + await clearAccounts(); + + const assessment = await assessNamedBackupRestore("Manual Backup"); + expect(assessment.valid).toBe(true); + + await fs.rm(backupPath, { force: true }); + + await expect(restoreNamedBackup("Manual Backup")).rejects.toThrow( + /Import file not found/, + ); + }); + it("throws when a named backup becomes invalid JSON before restore", async () => { await saveAccounts({ version: 3, @@ -1384,6 +1405,77 @@ describe("storage", () => { `Import would exceed maximum of ${ACCOUNT_LIMITS.MAX_ACCOUNTS} accounts`, ); }); + + it("serializes concurrent restores so only one succeeds when the limit is tight", async () => { + await saveAccounts({ + version: 3, + activeIndex: 0, + accounts: [ + { + accountId: "backup-a-account", + refreshToken: "ref-backup-a-account", + addedAt: 1, + lastUsed: 1, + }, + ], + }); + await createNamedBackup("backup-a"); + + await saveAccounts({ + version: 3, + activeIndex: 0, + accounts: [ + { + accountId: "backup-b-account", + refreshToken: "ref-backup-b-account", + addedAt: 1, + lastUsed: 1, + }, + ], + }); + await createNamedBackup("backup-b"); + + await saveAccounts({ + version: 3, + activeIndex: 0, + accounts: Array.from( + { length: ACCOUNT_LIMITS.MAX_ACCOUNTS - 1 }, + (_, index) => ({ + accountId: `current-${index}`, + refreshToken: `ref-current-${index}`, + addedAt: index + 1, + lastUsed: index + 1, + }), + ), + }); + + const assessmentA = await assessNamedBackupRestore("backup-a"); + const assessmentB = await assessNamedBackupRestore("backup-b"); + expect(assessmentA.valid).toBe(true); + expect(assessmentB.valid).toBe(true); + + const results = await Promise.allSettled([ + restoreNamedBackup("backup-a"), + restoreNamedBackup("backup-b"), + ]); + const succeeded = results.filter( + (result): result is PromiseFulfilledResult<{ + imported: number; + skipped: number; + total: number; + }> => result.status === "fulfilled", + ); + const failed = results.filter( + (result): result is PromiseRejectedResult => result.status === "rejected", + ); + + expect(succeeded).toHaveLength(1); + expect(failed).toHaveLength(1); + expect(String(failed[0]?.reason)).toContain("Import would exceed maximum"); + + const restored = await loadAccounts(); + expect(restored?.accounts).toHaveLength(ACCOUNT_LIMITS.MAX_ACCOUNTS); + }); }); describe("filename migration (TDD)", () => { From bf93df9dab4674a61f8596d1bb8ed7ab332056ae Mon Sep 17 00:00:00 2001 From: ndycode Date: Sat, 14 Mar 2026 02:16:46 +0800 Subject: [PATCH 7/9] fix(auth): close backup restore review gaps --- docs/reference/storage-paths.md | 8 + lib/cli.ts | 8 + lib/codex-manager.ts | 302 ++++++++++++++++---------------- lib/storage.ts | 43 ++++- lib/ui/copy.ts | 4 +- test/cli.test.ts | 14 ++ test/codex-manager-cli.test.ts | 111 ++++++++++++ test/storage.test.ts | 139 +++++++-------- 8 files changed, 391 insertions(+), 238 deletions(-) diff --git a/docs/reference/storage-paths.md b/docs/reference/storage-paths.md index f45e1d9..cf0747d 100644 --- a/docs/reference/storage-paths.md +++ b/docs/reference/storage-paths.md @@ -102,6 +102,13 @@ Rules: - `.rotate.`, `.tmp`, and `.wal` names are rejected - existing files are not overwritten unless a lower-level force path is used explicitly +Restore workflow: + +1. Run `codex auth login`. +2. Open the `Recovery` section. +3. Choose `Restore From Backup`. +4. Pick a backup and confirm the merge summary before import. + --- ## oc-chatgpt Target Paths @@ -117,6 +124,7 @@ Experimental sync targets the companion `oc-chatgpt-multi-auth` storage layout: ## Verification Commands ```bash +codex auth login codex auth status codex auth list ``` diff --git a/lib/cli.ts b/lib/cli.ts index e2b0b80..67c304d 100644 --- a/lib/cli.ts +++ b/lib/cli.ts @@ -234,6 +234,14 @@ async function promptLoginModeFallback( ) { return { mode: "verify-flagged" }; } + if ( + normalized === "u" || + normalized === "backup" || + normalized === "restore" || + normalized === "restore-backup" + ) { + return { mode: "restore-backup" }; + } if (normalized === "q" || normalized === "quit") return { mode: "cancel" }; console.log(UI_COPY.fallback.invalidModePrompt); diff --git a/lib/codex-manager.ts b/lib/codex-manager.ts index d905fd2..1f6bf16 100644 --- a/lib/codex-manager.ts +++ b/lib/codex-manager.ts @@ -3821,173 +3821,167 @@ async function runAuthLogin(): Promise { let menuQuotaRefreshStatus: string | undefined; loginFlow: while (true) { - let existingStorage = await loadAccounts(); - if (existingStorage && existingStorage.accounts.length > 0) { - while (true) { - existingStorage = await loadAccounts(); - if (!existingStorage || existingStorage.accounts.length === 0) { - break; - } - const currentStorage = existingStorage; - const displaySettings = await loadDashboardDisplaySettings(); - applyUiThemeFromDashboardSettings(displaySettings); - const quotaCache = await loadQuotaCache(); - const shouldAutoFetchLimits = displaySettings.menuAutoFetchLimits ?? true; - const showFetchStatus = displaySettings.menuShowFetchStatus ?? true; - const quotaTtlMs = displaySettings.menuQuotaTtlMs ?? DEFAULT_MENU_QUOTA_REFRESH_TTL_MS; - if (shouldAutoFetchLimits && !pendingMenuQuotaRefresh) { - const staleCount = countMenuQuotaRefreshTargets(currentStorage, quotaCache, quotaTtlMs); - if (staleCount > 0) { - if (showFetchStatus) { - menuQuotaRefreshStatus = `${UI_COPY.mainMenu.loadingLimits} [0/${staleCount}]`; - } - pendingMenuQuotaRefresh = refreshQuotaCacheForMenu( - currentStorage, - quotaCache, - quotaTtlMs, - (current, total) => { - if (!showFetchStatus) return; - menuQuotaRefreshStatus = `${UI_COPY.mainMenu.loadingLimits} [${current}/${total}]`; - }, - ) - .then(() => undefined) - .catch(() => undefined) - .finally(() => { - menuQuotaRefreshStatus = undefined; - pendingMenuQuotaRefresh = null; - }); + while (true) { + const existingStorage = await loadAccounts(); + const currentStorage = existingStorage ?? createEmptyAccountStorage(); + const displaySettings = await loadDashboardDisplaySettings(); + applyUiThemeFromDashboardSettings(displaySettings); + const quotaCache = await loadQuotaCache(); + const shouldAutoFetchLimits = displaySettings.menuAutoFetchLimits ?? true; + const showFetchStatus = displaySettings.menuShowFetchStatus ?? true; + const quotaTtlMs = displaySettings.menuQuotaTtlMs ?? DEFAULT_MENU_QUOTA_REFRESH_TTL_MS; + if (shouldAutoFetchLimits && !pendingMenuQuotaRefresh) { + const staleCount = countMenuQuotaRefreshTargets(currentStorage, quotaCache, quotaTtlMs); + if (staleCount > 0) { + if (showFetchStatus) { + menuQuotaRefreshStatus = `${UI_COPY.mainMenu.loadingLimits} [0/${staleCount}]`; } + pendingMenuQuotaRefresh = refreshQuotaCacheForMenu( + currentStorage, + quotaCache, + quotaTtlMs, + (current, total) => { + if (!showFetchStatus) return; + menuQuotaRefreshStatus = `${UI_COPY.mainMenu.loadingLimits} [${current}/${total}]`; + }, + ) + .then(() => undefined) + .catch(() => undefined) + .finally(() => { + menuQuotaRefreshStatus = undefined; + pendingMenuQuotaRefresh = null; + }); } - const flaggedStorage = await loadFlaggedAccounts(); + } + const flaggedStorage = await loadFlaggedAccounts(); - const menuResult = await promptLoginMode( - toExistingAccountInfo(currentStorage, quotaCache, displaySettings), - { - flaggedCount: flaggedStorage.accounts.length, - statusMessage: showFetchStatus ? () => menuQuotaRefreshStatus : undefined, - }, - ); + const menuResult = await promptLoginMode( + toExistingAccountInfo(currentStorage, quotaCache, displaySettings), + { + flaggedCount: flaggedStorage.accounts.length, + statusMessage: showFetchStatus ? () => menuQuotaRefreshStatus : undefined, + }, + ); - if (menuResult.mode === "cancel") { - console.log("Cancelled."); - return 0; - } - if (menuResult.mode === "check") { - await runActionPanel("Quick Check", "Checking local session + live status", async () => { - await runHealthCheck({ forceRefresh: false, liveProbe: true }); - }, displaySettings); - continue; - } - if (menuResult.mode === "deep-check") { - await runActionPanel("Deep Check", "Refreshing and testing all accounts", async () => { - await runHealthCheck({ forceRefresh: true, liveProbe: true }); - }, displaySettings); - continue; - } - if (menuResult.mode === "forecast") { - await runActionPanel("Best Account", "Comparing accounts", async () => { - await runForecast(["--live"]); - }, displaySettings); - continue; - } - if (menuResult.mode === "fix") { - await runActionPanel("Auto-Fix", "Checking and fixing common issues", async () => { - await runFix(["--live"]); - }, displaySettings); - continue; - } - if (menuResult.mode === "settings") { - await configureUnifiedSettings(displaySettings); - continue; + if (menuResult.mode === "cancel") { + console.log("Cancelled."); + return 0; + } + if (menuResult.mode === "check") { + await runActionPanel("Quick Check", "Checking local session + live status", async () => { + await runHealthCheck({ forceRefresh: false, liveProbe: true }); + }, displaySettings); + continue; + } + if (menuResult.mode === "deep-check") { + await runActionPanel("Deep Check", "Refreshing and testing all accounts", async () => { + await runHealthCheck({ forceRefresh: true, liveProbe: true }); + }, displaySettings); + continue; + } + if (menuResult.mode === "forecast") { + await runActionPanel("Best Account", "Comparing accounts", async () => { + await runForecast(["--live"]); + }, displaySettings); + continue; + } + if (menuResult.mode === "fix") { + await runActionPanel("Auto-Fix", "Checking and fixing common issues", async () => { + await runFix(["--live"]); + }, displaySettings); + continue; + } + if (menuResult.mode === "settings") { + await configureUnifiedSettings(displaySettings); + continue; + } + if (menuResult.mode === "verify-flagged") { + await runActionPanel("Problem Account Check", "Checking problem accounts", async () => { + await runVerifyFlagged([]); + }, displaySettings); + continue; + } + if (menuResult.mode === "restore-backup") { + try { + await runBackupRestoreManager(displaySettings); + } catch (error) { + const message = + error instanceof Error ? error.message : String(error); + console.error( + `Restore failed: ${collapseWhitespace(message) || "unknown error"}`, + ); } - if (menuResult.mode === "verify-flagged") { - await runActionPanel("Problem Account Check", "Checking problem accounts", async () => { - await runVerifyFlagged([]); - }, displaySettings); + continue; + } + if (menuResult.mode === "fresh" && menuResult.deleteAll) { + if (destructiveActionInFlight) { + console.log("Another destructive action is already running. Wait for it to finish."); continue; } - if (menuResult.mode === "restore-backup") { - try { - await runBackupRestoreManager(displaySettings); - } catch (error) { - const message = - error instanceof Error ? error.message : String(error); - console.error( - `Restore failed: ${collapseWhitespace(message) || "unknown error"}`, - ); - } - continue; + destructiveActionInFlight = true; + try { + await runActionPanel( + DESTRUCTIVE_ACTION_COPY.deleteSavedAccounts.label, + DESTRUCTIVE_ACTION_COPY.deleteSavedAccounts.stage, + async () => { + const result = await deleteSavedAccounts(); + console.log( + result.accountsCleared + ? DESTRUCTIVE_ACTION_COPY.deleteSavedAccounts.completed + : "Delete saved accounts completed with warnings. Some saved account artifacts could not be removed; see logs.", + ); + }, + displaySettings, + ); + } finally { + destructiveActionInFlight = false; } - if (menuResult.mode === "fresh" && menuResult.deleteAll) { - if (destructiveActionInFlight) { - console.log("Another destructive action is already running. Wait for it to finish."); - continue; - } - destructiveActionInFlight = true; - try { - await runActionPanel( - DESTRUCTIVE_ACTION_COPY.deleteSavedAccounts.label, - DESTRUCTIVE_ACTION_COPY.deleteSavedAccounts.stage, - async () => { - const result = await deleteSavedAccounts(); - console.log( - result.accountsCleared - ? DESTRUCTIVE_ACTION_COPY.deleteSavedAccounts.completed - : "Delete saved accounts completed with warnings. Some saved account artifacts could not be removed; see logs.", - ); - }, - displaySettings, - ); - } finally { - destructiveActionInFlight = false; - } + continue; + } + if (menuResult.mode === "reset") { + if (destructiveActionInFlight) { + console.log("Another destructive action is already running. Wait for it to finish."); continue; } - if (menuResult.mode === "reset") { - if (destructiveActionInFlight) { - console.log("Another destructive action is already running. Wait for it to finish."); - continue; - } - destructiveActionInFlight = true; - try { - await runActionPanel( - DESTRUCTIVE_ACTION_COPY.resetLocalState.label, - DESTRUCTIVE_ACTION_COPY.resetLocalState.stage, - async () => { - const pendingQuotaRefresh = pendingMenuQuotaRefresh; - if (pendingQuotaRefresh) { - await pendingQuotaRefresh; - } - const result = await resetLocalState(); - console.log( - result.accountsCleared && - result.flaggedCleared && - result.quotaCacheCleared - ? DESTRUCTIVE_ACTION_COPY.resetLocalState.completed - : "Reset local state completed with warnings. Some local artifacts could not be removed; see logs.", - ); - }, - displaySettings, - ); - } finally { - destructiveActionInFlight = false; - } - continue; + destructiveActionInFlight = true; + try { + await runActionPanel( + DESTRUCTIVE_ACTION_COPY.resetLocalState.label, + DESTRUCTIVE_ACTION_COPY.resetLocalState.stage, + async () => { + const pendingQuotaRefresh = pendingMenuQuotaRefresh; + if (pendingQuotaRefresh) { + await pendingQuotaRefresh; + } + const result = await resetLocalState(); + console.log( + result.accountsCleared && + result.flaggedCleared && + result.quotaCacheCleared + ? DESTRUCTIVE_ACTION_COPY.resetLocalState.completed + : "Reset local state completed with warnings. Some local artifacts could not be removed; see logs.", + ); + }, + displaySettings, + ); + } finally { + destructiveActionInFlight = false; } - if (menuResult.mode === "manage") { - const requiresInteractiveOAuth = typeof menuResult.refreshAccountIndex === "number"; - if (requiresInteractiveOAuth) { - await handleManageAction(currentStorage, menuResult); - continue; - } - await runActionPanel("Applying Change", "Updating selected account", async () => { - await handleManageAction(currentStorage, menuResult); - }, displaySettings); + continue; + } + if (menuResult.mode === "manage") { + const requiresInteractiveOAuth = typeof menuResult.refreshAccountIndex === "number"; + if (requiresInteractiveOAuth) { + await handleManageAction(currentStorage, menuResult); continue; } - if (menuResult.mode === "add") { - break; - } + await runActionPanel("Applying Change", "Updating selected account", async () => { + await handleManageAction(currentStorage, menuResult); + }, displaySettings); + continue; + } + if (menuResult.mode === "add") { + break; } } diff --git a/lib/storage.ts b/lib/storage.ts index 4ecda7b..d6820a7 100644 --- a/lib/storage.ts +++ b/lib/storage.ts @@ -1598,16 +1598,41 @@ export async function listNamedBackups(): Promise { return backups.sort((left, right) => (right.updatedAt ?? 0) - (left.updatedAt ?? 0)); } catch (error) { const code = (error as NodeJS.ErrnoException).code; - if (code !== "ENOENT") { - log.warn("Failed to list named backups", { - path: backupRoot, - error: String(error), - }); + if (code === "ENOENT") { + return []; } - return []; + log.warn("Failed to list named backups", { + path: backupRoot, + error: String(error), + }); + throw error; } } +function isRetryableFilesystemErrorCode( + code: string | undefined, +): code is "EPERM" | "EBUSY" | "EAGAIN" { + return code === "EPERM" || code === "EBUSY" || code === "EAGAIN"; +} + +async function retryTransientFilesystemOperation( + operation: () => Promise, +): Promise { + for (let attempt = 0; attempt < 5; attempt += 1) { + try { + return await operation(); + } catch (error) { + const code = (error as NodeJS.ErrnoException).code; + if (!isRetryableFilesystemErrorCode(code) || attempt === 4) { + throw error; + } + await new Promise((resolve) => setTimeout(resolve, 10 * 2 ** attempt)); + } + } + + throw new Error("Retry loop exhausted unexpectedly"); +} + export function getNamedBackupsDirectoryPath(): string { return getNamedBackupRoot(getStoragePath()); } @@ -1715,7 +1740,9 @@ async function loadBackupCandidate(path: string): Promise<{ error?: string; }> { try { - return await loadAccountsFromPath(path); + return await retryTransientFilesystemOperation(() => + loadAccountsFromPath(path), + ); } catch (error) { return { normalized: null, @@ -2022,7 +2049,7 @@ async function buildNamedBackupMetadata( ctimeMs?: number; } | null = null; try { - stats = await fs.stat(path); + stats = await retryTransientFilesystemOperation(() => fs.stat(path)); } catch (error) { const code = (error as NodeJS.ErrnoException).code; if (code !== "ENOENT") { diff --git a/lib/ui/copy.ts b/lib/ui/copy.ts index 91bdf60..10f123e 100644 --- a/lib/ui/copy.ts +++ b/lib/ui/copy.ts @@ -133,8 +133,8 @@ export const UI_COPY = { addAnotherQuestion: (count: number) => `Add another account? (${count} added) (y/n): `, selectModePrompt: - "(a) add, (c) check, (b) best, fi(x), (s) settings, (d) deep, (g) problem, (f) fresh, (r) reset, (q) back [a/c/b/x/s/d/g/f/r/q]: ", - invalidModePrompt: "Use one of: a, c, b, x, s, d, g, f, r, q.", + "(a) add, (c) check, (b) best, fi(x), (s) settings, (d) deep, (g) problem, (u) restore backup, (f) fresh, (r) reset, (q) back [a/c/b/x/s/d/g/u/f/r/q]: ", + invalidModePrompt: "Use one of: a, c, b, x, s, d, g, u, f, r, q.", }, } as const; diff --git a/test/cli.test.ts b/test/cli.test.ts index a275084..269a0eb 100644 --- a/test/cli.test.ts +++ b/test/cli.test.ts @@ -704,6 +704,20 @@ describe("CLI Module", () => { }); }); + it("returns restore-backup for fallback restore aliases", async () => { + const { promptLoginMode } = await import("../lib/cli.js"); + + mockRl.question.mockResolvedValueOnce("u"); + await expect(promptLoginMode([{ index: 0 }])).resolves.toEqual({ + mode: "restore-backup", + }); + + mockRl.question.mockResolvedValueOnce("restore"); + await expect(promptLoginMode([{ index: 0 }])).resolves.toEqual({ + mode: "restore-backup", + }); + }); + it("evaluates CODEX_TUI/CODEX_DESKTOP/TERM_PROGRAM/ELECTRON branches when TTY is true", async () => { delete process.env.FORCE_INTERACTIVE_MODE; const { stdin, stdout } = await import("node:process"); diff --git a/test/codex-manager-cli.test.ts b/test/codex-manager-cli.test.ts index 15df9ac..57a7aa5 100644 --- a/test/codex-manager-cli.test.ts +++ b/test/codex-manager-cli.test.ts @@ -2427,6 +2427,117 @@ describe("codex manager cli commands", () => { expect(restoreNamedBackupMock).toHaveBeenCalledWith("named-backup"); }); + it("offers backup restore from the login menu when no accounts are saved", async () => { + setInteractiveTTY(true); + const now = Date.now(); + loadAccountsMock.mockResolvedValue(null); + const assessment = { + backup: { + name: "named-backup", + path: "/mock/backups/named-backup.json", + createdAt: null, + updatedAt: now, + sizeBytes: 128, + version: 3, + accountCount: 1, + schemaErrors: [], + valid: true, + loadError: undefined, + }, + currentAccountCount: 0, + mergedAccountCount: 1, + imported: 1, + skipped: 0, + wouldExceedLimit: false, + valid: true, + error: undefined, + }; + listNamedBackupsMock.mockResolvedValue([assessment.backup]); + assessNamedBackupRestoreMock.mockResolvedValue(assessment); + promptLoginModeMock + .mockResolvedValueOnce({ mode: "restore-backup" }) + .mockResolvedValueOnce({ mode: "cancel" }); + selectMock.mockResolvedValueOnce({ type: "restore", assessment }); + + const { runCodexMultiAuthCli } = await import("../lib/codex-manager.js"); + const exitCode = await runCodexMultiAuthCli(["auth", "login"]); + + expect(exitCode).toBe(0); + expect(promptLoginModeMock.mock.calls[0]?.[0]).toEqual([]); + expect(listNamedBackupsMock).toHaveBeenCalledTimes(1); + expect(assessNamedBackupRestoreMock).toHaveBeenCalledWith( + "named-backup", + expect.objectContaining({ currentStorage: null }), + ); + expect(restoreNamedBackupMock).toHaveBeenCalledWith("named-backup"); + }); + + it("does not restore a named backup when confirmation is declined", async () => { + setInteractiveTTY(true); + const now = Date.now(); + loadAccountsMock.mockResolvedValue({ + version: 3, + activeIndex: 0, + activeIndexByFamily: { codex: 0 }, + accounts: [ + { + email: "settings@example.com", + accountId: "acc_settings", + refreshToken: "refresh-settings", + accessToken: "access-settings", + expiresAt: now + 3_600_000, + addedAt: now - 1_000, + lastUsed: now - 1_000, + enabled: true, + }, + ], + }); + const assessment = { + backup: { + name: "named-backup", + path: "/mock/backups/named-backup.json", + createdAt: null, + updatedAt: now, + sizeBytes: 128, + version: 3, + accountCount: 1, + schemaErrors: [], + valid: true, + loadError: undefined, + }, + currentAccountCount: 1, + mergedAccountCount: 2, + imported: 1, + skipped: 0, + wouldExceedLimit: false, + valid: true, + error: undefined, + }; + listNamedBackupsMock.mockResolvedValue([assessment.backup]); + assessNamedBackupRestoreMock.mockResolvedValue(assessment); + confirmMock.mockResolvedValueOnce(false); + promptLoginModeMock + .mockResolvedValueOnce({ mode: "restore-backup" }) + .mockResolvedValueOnce({ mode: "cancel" }); + selectMock.mockResolvedValueOnce({ type: "restore", assessment }); + + const { runCodexMultiAuthCli } = await import("../lib/codex-manager.js"); + const exitCode = await runCodexMultiAuthCli(["auth", "login"]); + + expect(exitCode).toBe(0); + expect(listNamedBackupsMock).toHaveBeenCalledTimes(1); + expect(assessNamedBackupRestoreMock).toHaveBeenCalledWith( + "named-backup", + expect.objectContaining({ + currentStorage: expect.objectContaining({ + accounts: expect.any(Array), + }), + }), + ); + expect(confirmMock).toHaveBeenCalledOnce(); + expect(restoreNamedBackupMock).not.toHaveBeenCalled(); + }); + it("catches restore failures and returns to the login menu", async () => { setInteractiveTTY(true); const now = Date.now(); diff --git a/test/storage.test.ts b/test/storage.test.ts index 781fb73..140b38c 100644 --- a/test/storage.test.ts +++ b/test/storage.test.ts @@ -1267,30 +1267,17 @@ describe("storage", () => { }, ); - it("throws when a named backup is deleted after assessment", async () => { - await saveAccounts({ - version: 3, - activeIndex: 0, - accounts: [ - { - accountId: "deleted-backup", - refreshToken: "ref-deleted-backup", - addedAt: 1, - lastUsed: 1, - }, - ], - }); - await createNamedBackup("deleted-backup"); - await clearAccounts(); - - const assessment = await assessNamedBackupRestore("deleted-backup"); - expect(assessment.valid).toBe(true); - - await fs.rm(buildNamedBackupPath("deleted-backup"), { force: true }); + it("rethrows unreadable backup directory errors while listing backups", async () => { + const readdirSpy = vi.spyOn(fs, "readdir"); + const error = new Error("backup directory locked") as NodeJS.ErrnoException; + error.code = "EPERM"; + readdirSpy.mockRejectedValueOnce(error); - await expect(restoreNamedBackup("deleted-backup")).rejects.toThrow( - /Import file not found/, - ); + try { + await expect(listNamedBackups()).rejects.toMatchObject({ code: "EPERM" }); + } finally { + readdirSpy.mockRestore(); + } }); it("throws file-not-found when a manually named backup disappears after assessment", async () => { @@ -1328,82 +1315,86 @@ describe("storage", () => { ); }); - it("throws when a named backup becomes invalid JSON before restore", async () => { + it("retries transient backup read errors while listing backups", async () => { await saveAccounts({ version: 3, activeIndex: 0, accounts: [ { - accountId: "corrupt-backup", - refreshToken: "ref-corrupt-backup", + accountId: "retry-read", + refreshToken: "ref-retry-read", addedAt: 1, lastUsed: 1, }, ], }); - await createNamedBackup("corrupt-backup"); - await clearAccounts(); - - const assessment = await assessNamedBackupRestore("corrupt-backup"); - expect(assessment.valid).toBe(true); - - await fs.writeFile(buildNamedBackupPath("corrupt-backup"), "{", "utf-8"); + const backup = await createNamedBackup("retry-read"); + const originalReadFile = fs.readFile.bind(fs); + let busyFailures = 0; + const readFileSpy = vi + .spyOn(fs, "readFile") + .mockImplementation(async (...args) => { + const [path] = args; + if (String(path) === backup.path && busyFailures === 0) { + busyFailures += 1; + const error = new Error("backup file busy") as NodeJS.ErrnoException; + error.code = "EBUSY"; + throw error; + } + return originalReadFile(...(args as Parameters)); + }); - await expect(restoreNamedBackup("corrupt-backup")).rejects.toThrow( - /Invalid JSON in import file/, - ); + try { + const backups = await listNamedBackups(); + expect(backups).toEqual( + expect.arrayContaining([ + expect.objectContaining({ name: "retry-read", valid: true }), + ]), + ); + expect(busyFailures).toBe(1); + } finally { + readFileSpy.mockRestore(); + } }); - it("throws when current accounts exceed the limit after assessment", async () => { + it("retries transient backup stat errors while listing backups", async () => { await saveAccounts({ version: 3, activeIndex: 0, accounts: [ { - accountId: "limit-backup", - refreshToken: "ref-limit-backup", + accountId: "retry-stat", + refreshToken: "ref-retry-stat", addedAt: 1, lastUsed: 1, }, ], }); - await createNamedBackup("limit-backup"); - - await saveAccounts({ - version: 3, - activeIndex: 0, - accounts: Array.from( - { length: ACCOUNT_LIMITS.MAX_ACCOUNTS - 1 }, - (_, index) => ({ - accountId: `current-${index}`, - refreshToken: `ref-current-${index}`, - addedAt: index + 1, - lastUsed: index + 1, - }), - ), - }); - - const assessment = await assessNamedBackupRestore("limit-backup"); - expect(assessment.valid).toBe(true); - expect(assessment.wouldExceedLimit).toBe(false); - - await saveAccounts({ - version: 3, - activeIndex: 0, - accounts: Array.from( - { length: ACCOUNT_LIMITS.MAX_ACCOUNTS }, - (_, index) => ({ - accountId: `grown-${index}`, - refreshToken: `ref-grown-${index}`, - addedAt: index + 1, - lastUsed: index + 1, - }), - ), + const backup = await createNamedBackup("retry-stat"); + const originalStat = fs.stat.bind(fs); + let busyFailures = 0; + const statSpy = vi.spyOn(fs, "stat").mockImplementation(async (...args) => { + const [path] = args; + if (String(path) === backup.path && busyFailures === 0) { + busyFailures += 1; + const error = new Error("backup stat busy") as NodeJS.ErrnoException; + error.code = "EAGAIN"; + throw error; + } + return originalStat(...(args as Parameters)); }); - await expect(restoreNamedBackup("limit-backup")).rejects.toThrow( - `Import would exceed maximum of ${ACCOUNT_LIMITS.MAX_ACCOUNTS} accounts`, - ); + try { + const backups = await listNamedBackups(); + expect(backups).toEqual( + expect.arrayContaining([ + expect.objectContaining({ name: "retry-stat", valid: true }), + ]), + ); + expect(busyFailures).toBe(1); + } finally { + statSpy.mockRestore(); + } }); it("serializes concurrent restores so only one succeeds when the limit is tight", async () => { From d12e6dad3664c519d3c14e26070b1f53ce38fc17 Mon Sep 17 00:00:00 2001 From: ndycode Date: Sat, 14 Mar 2026 02:34:08 +0800 Subject: [PATCH 8/9] fix(auth): refresh restore eligibility before confirm --- lib/codex-manager.ts | 19 ++++--- lib/storage.ts | 6 +-- test/codex-manager-cli.test.ts | 96 ++++++++++++++++++++++++++++++++-- test/storage.test.ts | 14 ++--- 4 files changed, 112 insertions(+), 23 deletions(-) diff --git a/lib/codex-manager.ts b/lib/codex-manager.ts index 1f6bf16..dd269db 100644 --- a/lib/codex-manager.ts +++ b/lib/codex-manager.ts @@ -4221,7 +4221,7 @@ async function runBackupRestoreManager( const items: MenuItem[] = assessments.map((assessment) => { const status = - assessment.valid && !assessment.wouldExceedLimit + assessment.eligibleForRestore ? "ready" : assessment.wouldExceedLimit ? "limit" @@ -4247,7 +4247,7 @@ async function runBackupRestoreManager( value: { type: "restore", assessment }, color: status === "ready" ? "green" : status === "limit" ? "red" : "yellow", - disabled: !assessment.valid || assessment.wouldExceedLimit, + disabled: !assessment.eligibleForRestore, }; }); @@ -4268,20 +4268,23 @@ async function runBackupRestoreManager( return; } - const assessment = selection.assessment; - if (!assessment.valid || assessment.wouldExceedLimit) { - console.log(assessment.error ?? "Backup is not eligible for restore."); + const latestAssessment = await assessNamedBackupRestore( + selection.assessment.backup.name, + { currentStorage: await loadAccounts() }, + ); + if (!latestAssessment.eligibleForRestore) { + console.log(latestAssessment.error ?? "Backup is not eligible for restore."); return; } - const confirmMessage = `Restore backup "${assessment.backup.name}"? This will merge ${assessment.backup.accountCount ?? 0} account(s) into ${assessment.currentAccountCount} current (${assessment.mergedAccountCount ?? assessment.currentAccountCount} after dedupe).`; + const confirmMessage = `Restore backup "${latestAssessment.backup.name}"? This will merge ${latestAssessment.backup.accountCount ?? 0} account(s) into ${latestAssessment.currentAccountCount} current (${latestAssessment.mergedAccountCount ?? latestAssessment.currentAccountCount} after dedupe).`; const confirmed = await confirm(confirmMessage); if (!confirmed) return; try { - const result = await restoreNamedBackup(assessment.backup.name); + const result = await restoreNamedBackup(latestAssessment.backup.name); console.log( - `Restored backup "${assessment.backup.name}". Imported ${result.imported}, skipped ${result.skipped}, total ${result.total}.`, + `Restored backup "${latestAssessment.backup.name}". Imported ${result.imported}, skipped ${result.skipped}, total ${result.total}.`, ); } catch (error) { const message = error instanceof Error ? error.message : String(error); diff --git a/lib/storage.ts b/lib/storage.ts index d6820a7..680219c 100644 --- a/lib/storage.ts +++ b/lib/storage.ts @@ -135,7 +135,7 @@ export interface BackupRestoreAssessment { imported: number | null; skipped: number | null; wouldExceedLimit: boolean; - valid: boolean; + eligibleForRestore: boolean; error?: string; } @@ -1672,7 +1672,7 @@ export async function assessNamedBackupRestore( imported: null, skipped: null, wouldExceedLimit: false, - valid: false, + eligibleForRestore: false, error: backup.loadError ?? "Backup is empty or invalid", }; } @@ -1696,7 +1696,7 @@ export async function assessNamedBackupRestore( imported, skipped, wouldExceedLimit, - valid: !wouldExceedLimit, + eligibleForRestore: !wouldExceedLimit, error: wouldExceedLimit ? `Restore would exceed maximum of ${ACCOUNT_LIMITS.MAX_ACCOUNTS} accounts` : undefined, diff --git a/test/codex-manager-cli.test.ts b/test/codex-manager-cli.test.ts index 57a7aa5..59b3763 100644 --- a/test/codex-manager-cli.test.ts +++ b/test/codex-manager-cli.test.ts @@ -527,7 +527,7 @@ describe("codex manager cli commands", () => { imported: 1, skipped: 0, wouldExceedLimit: false, - valid: true, + eligibleForRestore: true, error: undefined, }); getNamedBackupsDirectoryPathMock.mockReturnValue("/mock/backups"); @@ -2400,7 +2400,7 @@ describe("codex manager cli commands", () => { imported: 1, skipped: 0, wouldExceedLimit: false, - valid: true, + eligibleForRestore: true, error: undefined, }; listNamedBackupsMock.mockResolvedValue([assessment.backup]); @@ -2449,7 +2449,7 @@ describe("codex manager cli commands", () => { imported: 1, skipped: 0, wouldExceedLimit: false, - valid: true, + eligibleForRestore: true, error: undefined, }; listNamedBackupsMock.mockResolvedValue([assessment.backup]); @@ -2510,7 +2510,7 @@ describe("codex manager cli commands", () => { imported: 1, skipped: 0, wouldExceedLimit: false, - valid: true, + eligibleForRestore: true, error: undefined, }; listNamedBackupsMock.mockResolvedValue([assessment.backup]); @@ -2576,7 +2576,7 @@ describe("codex manager cli commands", () => { imported: 1, skipped: 0, wouldExceedLimit: false, - valid: true, + eligibleForRestore: true, error: undefined, }; listNamedBackupsMock.mockResolvedValue([assessment.backup]); @@ -2610,6 +2610,92 @@ describe("codex manager cli commands", () => { } }); + it("reassesses a backup before confirmation so the merge summary stays current", async () => { + setInteractiveTTY(true); + const now = Date.now(); + loadAccountsMock.mockResolvedValue({ + version: 3, + activeIndex: 0, + activeIndexByFamily: { codex: 0 }, + accounts: [ + { + email: "settings@example.com", + accountId: "acc_settings", + refreshToken: "refresh-settings", + accessToken: "access-settings", + expiresAt: now + 3_600_000, + addedAt: now - 1_000, + lastUsed: now - 1_000, + enabled: true, + }, + ], + }); + const initialAssessment = { + backup: { + name: "named-backup", + path: "/mock/backups/named-backup.json", + createdAt: null, + updatedAt: now, + sizeBytes: 128, + version: 3, + accountCount: 1, + schemaErrors: [], + valid: true, + loadError: undefined, + }, + currentAccountCount: 1, + mergedAccountCount: 2, + imported: 1, + skipped: 0, + wouldExceedLimit: false, + eligibleForRestore: true, + error: undefined, + }; + const refreshedAssessment = { + ...initialAssessment, + currentAccountCount: 3, + mergedAccountCount: 4, + }; + listNamedBackupsMock.mockResolvedValue([initialAssessment.backup]); + assessNamedBackupRestoreMock + .mockResolvedValueOnce(initialAssessment) + .mockResolvedValueOnce(refreshedAssessment); + promptLoginModeMock + .mockResolvedValueOnce({ mode: "restore-backup" }) + .mockResolvedValueOnce({ mode: "cancel" }); + selectMock.mockResolvedValueOnce({ + type: "restore", + assessment: initialAssessment, + }); + + const { runCodexMultiAuthCli } = await import("../lib/codex-manager.js"); + const exitCode = await runCodexMultiAuthCli(["auth", "login"]); + + expect(exitCode).toBe(0); + expect(assessNamedBackupRestoreMock).toHaveBeenNthCalledWith( + 1, + "named-backup", + expect.objectContaining({ + currentStorage: expect.objectContaining({ + accounts: expect.any(Array), + }), + }), + ); + expect(assessNamedBackupRestoreMock).toHaveBeenNthCalledWith( + 2, + "named-backup", + expect.objectContaining({ + currentStorage: expect.objectContaining({ + accounts: expect.any(Array), + }), + }), + ); + expect(confirmMock).toHaveBeenCalledWith( + expect.stringContaining("into 3 current (4 after dedupe)"), + ); + expect(restoreNamedBackupMock).toHaveBeenCalledWith("named-backup"); + }); + it("shows experimental settings in the settings hub", async () => { const now = Date.now(); setupInteractiveSettingsLogin(createSettingsStorage(now)); diff --git a/test/storage.test.ts b/test/storage.test.ts index 140b38c..fab8d4f 100644 --- a/test/storage.test.ts +++ b/test/storage.test.ts @@ -1155,7 +1155,7 @@ describe("storage", () => { await clearAccounts(); const assessment = await assessNamedBackupRestore("restore-me"); - expect(assessment.valid).toBe(true); + expect(assessment.eligibleForRestore).toBe(true); expect(assessment.wouldExceedLimit).toBe(false); const restoreResult = await restoreNamedBackup("restore-me"); @@ -1198,7 +1198,7 @@ describe("storage", () => { await clearAccounts(); const assessment = await assessNamedBackupRestore("Manual Backup"); - expect(assessment.valid).toBe(true); + expect(assessment.eligibleForRestore).toBe(true); expect(assessment.backup.name).toBe("Manual Backup"); const restoreResult = await restoreNamedBackup("Manual Backup"); @@ -1223,7 +1223,7 @@ describe("storage", () => { await clearAccounts(); const assessment = await assessNamedBackupRestore("deleted-after-assessment"); - expect(assessment.valid).toBe(true); + expect(assessment.eligibleForRestore).toBe(true); await fs.unlink(backup.path); @@ -1250,7 +1250,7 @@ describe("storage", () => { await clearAccounts(); const assessment = await assessNamedBackupRestore("invalid-after-assessment"); - expect(assessment.valid).toBe(true); + expect(assessment.eligibleForRestore).toBe(true); await fs.writeFile(backup.path, "not valid json {[", "utf-8"); @@ -1306,7 +1306,7 @@ describe("storage", () => { await clearAccounts(); const assessment = await assessNamedBackupRestore("Manual Backup"); - expect(assessment.valid).toBe(true); + expect(assessment.eligibleForRestore).toBe(true); await fs.rm(backupPath, { force: true }); @@ -1442,8 +1442,8 @@ describe("storage", () => { const assessmentA = await assessNamedBackupRestore("backup-a"); const assessmentB = await assessNamedBackupRestore("backup-b"); - expect(assessmentA.valid).toBe(true); - expect(assessmentB.valid).toBe(true); + expect(assessmentA.eligibleForRestore).toBe(true); + expect(assessmentB.eligibleForRestore).toBe(true); const results = await Promise.allSettled([ restoreNamedBackup("backup-a"), From a0544fffd65a3795e7c22ff919b0b11bdfdbdd3a Mon Sep 17 00:00:00 2001 From: ndycode Date: Sat, 14 Mar 2026 15:59:58 +0800 Subject: [PATCH 9/9] fix(storage): honor null restore snapshots --- lib/codex-manager.ts | 2 +- lib/storage.ts | 33 +++++++++--- test/codex-manager-cli.test.ts | 41 +++++++++++++++ test/storage.test.ts | 91 ++++++++++++++++++++++++++++++++++ 4 files changed, 158 insertions(+), 9 deletions(-) diff --git a/lib/codex-manager.ts b/lib/codex-manager.ts index dd269db..a96cd6d 100644 --- a/lib/codex-manager.ts +++ b/lib/codex-manager.ts @@ -133,7 +133,7 @@ function formatReasonLabel(reason: string | undefined): string | undefined { function formatRelativeDateShort( timestamp: number | null | undefined, ): string | null { - if (!timestamp) return null; + if (timestamp == null) return null; const days = Math.floor((Date.now() - timestamp) / 86_400_000); if (days <= 0) return "today"; if (days === 1) return "yesterday"; diff --git a/lib/storage.ts b/lib/storage.ts index 680219c..5907319 100644 --- a/lib/storage.ts +++ b/lib/storage.ts @@ -47,6 +47,7 @@ const ACCOUNTS_WAL_SUFFIX = ".wal"; const ACCOUNTS_BACKUP_HISTORY_DEPTH = 3; const BACKUP_COPY_MAX_ATTEMPTS = 5; const BACKUP_COPY_BASE_DELAY_MS = 10; +const NAMED_BACKUP_LIST_CONCURRENCY = 8; const RESET_MARKER_SUFFIX = ".reset-intent"; let storageBackupEnabled = true; let lastAccountsSaveTimestamp = 0; @@ -1581,11 +1582,22 @@ export async function listNamedBackups(): Promise { const backupRoot = getNamedBackupRoot(getStoragePath()); try { const entries = await fs.readdir(backupRoot, { withFileTypes: true }); - const backups = await Promise.all( - entries - .filter((entry) => entry.isFile() && !entry.isSymbolicLink()) - .filter((entry) => entry.name.toLowerCase().endsWith(".json")) - .map(async (entry) => { + const backupEntries = entries + .filter((entry) => entry.isFile() && !entry.isSymbolicLink()) + .filter((entry) => entry.name.toLowerCase().endsWith(".json")); + const backups: NamedBackupMetadata[] = []; + for ( + let index = 0; + index < backupEntries.length; + index += NAMED_BACKUP_LIST_CONCURRENCY + ) { + const chunk = backupEntries.slice( + index, + index + NAMED_BACKUP_LIST_CONCURRENCY, + ); + backups.push( + ...(await Promise.all( + chunk.map(async (entry) => { const path = resolvePath(join(backupRoot, entry.name)); const candidate = await loadBackupCandidate(path); return buildNamedBackupMetadata( @@ -1593,8 +1605,10 @@ export async function listNamedBackups(): Promise { path, { candidate }, ); - }), - ); + }), + )), + ); + } return backups.sort((left, right) => (right.updatedAt ?? 0) - (left.updatedAt ?? 0)); } catch (error) { const code = (error as NodeJS.ErrnoException).code; @@ -1661,7 +1675,10 @@ export async function assessNamedBackupRestore( backupPath, { candidate }, ); - const currentStorage = options.currentStorage ?? (await loadAccounts()); + const currentStorage = + options.currentStorage !== undefined + ? options.currentStorage + : await loadAccounts(); const currentAccounts = currentStorage?.accounts ?? []; if (!candidate.normalized || !backup.accountCount || backup.accountCount <= 0) { diff --git a/test/codex-manager-cli.test.ts b/test/codex-manager-cli.test.ts index 59b3763..02ddb5c 100644 --- a/test/codex-manager-cli.test.ts +++ b/test/codex-manager-cli.test.ts @@ -2696,6 +2696,47 @@ describe("codex manager cli commands", () => { expect(restoreNamedBackupMock).toHaveBeenCalledWith("named-backup"); }); + it("shows epoch backup timestamps in restore hints", async () => { + setInteractiveTTY(true); + loadAccountsMock.mockResolvedValue(null); + const assessment = { + backup: { + name: "epoch-backup", + path: "/mock/backups/epoch-backup.json", + createdAt: null, + updatedAt: 0, + sizeBytes: 128, + version: 3, + accountCount: 1, + schemaErrors: [], + valid: true, + loadError: undefined, + }, + currentAccountCount: 0, + mergedAccountCount: 1, + imported: 1, + skipped: 0, + wouldExceedLimit: false, + eligibleForRestore: true, + error: undefined, + }; + listNamedBackupsMock.mockResolvedValue([assessment.backup]); + assessNamedBackupRestoreMock.mockResolvedValue(assessment); + promptLoginModeMock + .mockResolvedValueOnce({ mode: "restore-backup" }) + .mockResolvedValueOnce({ mode: "cancel" }); + selectMock.mockResolvedValueOnce({ type: "back" }); + + const { runCodexMultiAuthCli } = await import("../lib/codex-manager.js"); + const exitCode = await runCodexMultiAuthCli(["auth", "login"]); + + expect(exitCode).toBe(0); + const backupItems = selectMock.mock.calls[0]?.[0]; + expect(backupItems?.[0]?.hint).toContain( + `updated ${new Date(0).toLocaleDateString()}`, + ); + }); + it("shows experimental settings in the settings hub", async () => { const now = Date.now(); setupInteractiveSettingsLogin(createSettingsStorage(now)); diff --git a/test/storage.test.ts b/test/storage.test.ts index fab8d4f..12a03f9 100644 --- a/test/storage.test.ts +++ b/test/storage.test.ts @@ -1165,6 +1165,46 @@ describe("storage", () => { expect(restored?.accounts[0]?.accountId).toBe("primary"); }); + it("honors explicit null currentStorage when assessing a named backup", async () => { + await saveAccounts({ + version: 3, + activeIndex: 0, + accounts: [ + { + accountId: "backup-account", + refreshToken: "ref-backup-account", + addedAt: 1, + lastUsed: 1, + }, + ], + }); + await createNamedBackup("explicit-null-current-storage"); + + await saveAccounts({ + version: 3, + activeIndex: 0, + accounts: [ + { + accountId: "current-account", + refreshToken: "ref-current-account", + addedAt: 2, + lastUsed: 2, + }, + ], + }); + + const assessment = await assessNamedBackupRestore( + "explicit-null-current-storage", + { currentStorage: null }, + ); + + expect(assessment.currentAccountCount).toBe(0); + expect(assessment.mergedAccountCount).toBe(1); + expect(assessment.imported).toBe(1); + expect(assessment.skipped).toBe(0); + expect(assessment.eligibleForRestore).toBe(true); + }); + it("restores manually named backups that already exist inside the backups directory", async () => { const backupPath = join( dirname(testStoragePath), @@ -1397,6 +1437,57 @@ describe("storage", () => { } }); + it("limits concurrent backup reads while listing backups", async () => { + const backupPaths: string[] = []; + for (let index = 0; index < 12; index += 1) { + await saveAccounts({ + version: 3, + activeIndex: 0, + accounts: [ + { + accountId: `concurrency-${index}`, + refreshToken: `ref-concurrency-${index}`, + addedAt: index + 1, + lastUsed: index + 1, + }, + ], + }); + const backup = await createNamedBackup(`concurrency-${index}`); + backupPaths.push(backup.path); + } + + const originalReadFile = fs.readFile.bind(fs); + const delayedPaths = new Set(backupPaths); + let activeReads = 0; + let peakReads = 0; + const readFileSpy = vi + .spyOn(fs, "readFile") + .mockImplementation(async (...args) => { + const [path] = args; + if (delayedPaths.has(String(path))) { + activeReads += 1; + peakReads = Math.max(peakReads, activeReads); + try { + await new Promise((resolve) => setTimeout(resolve, 10)); + return await originalReadFile( + ...(args as Parameters), + ); + } finally { + activeReads -= 1; + } + } + return originalReadFile(...(args as Parameters)); + }); + + try { + const backups = await listNamedBackups(); + expect(backups).toHaveLength(12); + expect(peakReads).toBeLessThanOrEqual(8); + } finally { + readFileSpy.mockRestore(); + } + }); + it("serializes concurrent restores so only one succeeds when the limit is tight", async () => { await saveAccounts({ version: 3,