From a9d6a8464db85211fdc3105dae388f29c245123f Mon Sep 17 00:00:00 2001 From: ndycode Date: Thu, 19 Mar 2026 20:34:18 +0800 Subject: [PATCH 1/4] feat(auth): harden opencode import flow --- docs/getting-started.md | 2 + lib/cli.ts | 4 + lib/codex-manager.ts | 58 ++- lib/storage.ts | 272 +++++++++++++- lib/ui/copy.ts | 4 +- test/cli.test.ts | 14 + test/codex-manager-cli.test.ts | 623 ++++++++++++++++++++------------- test/documentation.test.ts | 5 +- test/storage.test.ts | 289 ++++++++++++++- 9 files changed, 1010 insertions(+), 261 deletions(-) diff --git a/docs/getting-started.md b/docs/getting-started.md index 8caf3fb4..2d2fa281 100644 --- a/docs/getting-started.md +++ b/docs/getting-started.md @@ -82,6 +82,8 @@ Use the restore path when you already have named backup files and want to recove The restore manager shows each backup name, account count, freshness, and whether the restore would exceed the account limit before it lets you apply anything. +If you already have an OpenCode account pool on the same machine, use the recovery import option from the login dashboard to preview and import those saved accounts before creating new OAuth sessions. The detector checks the standard OpenCode account file and honors `CODEX_OPENCODE_POOL_PATH` when you need to point at a specific file. + --- ## Sync And Settings diff --git a/lib/cli.ts b/lib/cli.ts index dd728c9a..ec853ee9 100644 --- a/lib/cli.ts +++ b/lib/cli.ts @@ -62,6 +62,7 @@ export type LoginMode = | "check" | "deep-check" | "verify-flagged" + | "import-opencode" | "restore-backup" | "cancel"; @@ -248,6 +249,9 @@ async function promptLoginModeFallback( ) { return { mode: "restore-backup" }; } + if (normalized === "i" || normalized === "import-opencode") { + return { mode: "import-opencode" }; + } 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 2b69bbb9..829e2486 100644 --- a/lib/codex-manager.ts +++ b/lib/codex-manager.ts @@ -2,7 +2,7 @@ import { createHash } from "node:crypto"; import { createInterface } from "node:readline/promises"; import { stdin as input, stdout as output } from "node:process"; import { promises as fs, existsSync } from "node:fs"; -import { dirname, resolve } from "node:path"; +import { basename, dirname, resolve } from "node:path"; import { createAuthorizationFlow, exchangeAuthorizationCode, @@ -65,9 +65,13 @@ import { } from "./quota-cache.js"; import { assessNamedBackupRestore, + assessOpencodeAccountPool, + type BackupRestoreAssessment, + formatRedactedFilesystemError, getActionableNamedBackupRestores, getRedactedFilesystemErrorLabel, getNamedBackupsDirectoryPath, + importAccounts, listNamedBackups, listRotatingBackups, NAMED_BACKUP_LIST_CONCURRENCY, @@ -4448,6 +4452,58 @@ async function runAuthLogin(): Promise { } continue; } + if (menuResult.mode === "import-opencode") { + let assessment: BackupRestoreAssessment | null; + try { + assessment = await assessOpencodeAccountPool({ + currentStorage, + }); + } catch (error) { + const errorLabel = formatRedactedFilesystemError(error); + console.error(`Import assessment failed: ${errorLabel}`); + continue; + } + if (!assessment) { + console.log("No OpenCode account pool was detected."); + continue; + } + if (!assessment.backup.valid || !assessment.eligibleForRestore) { + const assessmentErrorLabel = + assessment.error || "OpenCode account pool is not importable."; + console.log(assessmentErrorLabel); + continue; + } + if (assessment.wouldExceedLimit) { + console.log( + `Import would exceed the account limit (${assessment.currentAccountCount ?? "?"} current, ${assessment.mergedAccountCount ?? "?"} after import). Remove accounts first.`, + ); + continue; + } + const backupLabel = basename(assessment.backup.path); + const confirmed = await confirm( + `Import OpenCode accounts from ${backupLabel}?`, + ); + if (!confirmed) { + continue; + } + try { + await runActionPanel( + "Import OpenCode Accounts", + `Importing from ${backupLabel}`, + async () => { + const imported = await importAccounts(assessment.backup.path); + console.log( + `Imported ${imported.imported} account${imported.imported === 1 ? "" : "s"}. Skipped ${imported.skipped}. Total accounts: ${imported.total}.`, + ); + }, + displaySettings, + ); + } catch (error) { + const errorLabel = formatRedactedFilesystemError(error); + console.error(`Import failed: ${errorLabel}`); + } + continue; + } if (menuResult.mode === "fresh" && menuResult.deleteAll) { if (destructiveActionInFlight) { console.log("Another destructive action is already running. Wait for it to finish."); diff --git a/lib/storage.ts b/lib/storage.ts index 481cbc64..0d2e92e8 100644 --- a/lib/storage.ts +++ b/lib/storage.ts @@ -1,7 +1,13 @@ import { AsyncLocalStorage } from "node:async_hooks"; import { createHash } from "node:crypto"; import { existsSync, promises as fs } from "node:fs"; -import { basename, dirname, join } from "node:path"; +import { homedir } from "node:os"; +import { + basename, + dirname, + join, + resolve as resolveFilesystemPath, +} from "node:path"; import { ACCOUNT_LIMITS } from "./constants.js"; import { createLogger } from "./logger.js"; import { @@ -49,6 +55,7 @@ const BACKUP_COPY_MAX_ATTEMPTS = 5; const BACKUP_COPY_BASE_DELAY_MS = 10; const ROTATING_BACKUP_STALE_ARTIFACT_MAX_AGE_MS = 60_000; export const NAMED_BACKUP_LIST_CONCURRENCY = 8; +export const ACCOUNT_SNAPSHOT_RETENTION_PER_REASON = 3; const RESET_MARKER_SUFFIX = ".reset-intent"; let storageBackupEnabled = true; let lastAccountsSaveTimestamp = 0; @@ -2000,6 +2007,11 @@ export async function listNamedBackups(): Promise { return scanResult.backups.map((entry) => entry.backup); } +export async function listAccountSnapshots(): Promise { + const backups = await listNamedBackups(); + return backups.filter((backup) => isAccountSnapshotName(backup.name)); +} + export async function listRotatingBackups(): Promise { let storagePath: string | null = null; try { @@ -2071,6 +2083,42 @@ export function getNamedBackupsDirectoryPath(): string { return getNamedBackupRoot(getStoragePath()); } +export function detectOpencodeAccountPoolPath(): string | null { + const candidates = new Set(); + const explicit = process.env.CODEX_OPENCODE_POOL_PATH; + if (explicit?.trim()) { + const explicitPath = resolvePath(explicit.trim()); + return existsSync(explicitPath) ? explicitPath : null; + } + + const appDataBases = [process.env.LOCALAPPDATA, process.env.APPDATA].filter( + (base): base is string => !!base && base.trim().length > 0, + ); + for (const base of appDataBases) { + candidates.add( + resolveFilesystemPath(join(base, "OpenCode", ACCOUNTS_FILE_NAME)), + ); + candidates.add( + resolveFilesystemPath(join(base, "opencode", ACCOUNTS_FILE_NAME)), + ); + } + + const home = homedir(); + if (home) { + candidates.add( + resolveFilesystemPath(join(home, ".opencode", ACCOUNTS_FILE_NAME)), + ); + } + + for (const candidate of candidates) { + if (existsSync(candidate)) { + return candidate; + } + } + + return null; +} + async function scanNamedBackupsForActionableRestores(): Promise { try { return await scanNamedBackups(); @@ -2268,6 +2316,147 @@ function buildAccountSnapshotName( return `accounts-${reason}-snapshot-${formatTimestampForSnapshot(timestamp)}`; } +export function isAccountSnapshotName(name: string): boolean { + return /^accounts-(?[a-z0-9-]+)-snapshot-\d{4}-\d{2}-\d{2}_\d{2}-\d{2}-\d{2}_\d{3}$/i.test( + name, + ); +} + +function getAccountSnapshotReason(name: string): string | null { + const match = name.match( + /^accounts-(?[a-z0-9-]+)-snapshot-\d{4}-\d{2}-\d{2}_\d{2}-\d{2}-\d{2}_\d{3}$/i, + ); + return match?.groups?.reason?.toLowerCase() ?? null; +} + +type AutoSnapshotDetails = { + backup: NamedBackupMetadata; + name: string; + reason: string; + sortTimestamp: number; +}; + +function parseAutoSnapshot( + backup: NamedBackupMetadata, +): AutoSnapshotDetails | null { + const reason = getAccountSnapshotReason(backup.name); + if (!reason) { + return null; + } + return { + backup, + name: backup.name, + reason, + sortTimestamp: backup.updatedAt ?? backup.createdAt ?? 0, + }; +} + +async function getLatestManualCodexCliRollbackSnapshotNames(): Promise< + Set +> { + try { + const syncHistoryModule = await import("./sync-history.js"); + if (typeof syncHistoryModule.readSyncHistory !== "function") { + return new Set(); + } + const history = await syncHistoryModule.readSyncHistory({ + kind: "codex-cli-sync", + }); + for (let index = history.length - 1; index >= 0; index -= 1) { + const entry = history[index]; + if (!entry || entry.kind !== "codex-cli-sync") { + continue; + } + const run = entry.run; + if (run?.trigger !== "manual" || run.outcome !== "changed") { + continue; + } + const snapshotName = run.rollbackSnapshot?.name?.trim(); + if (!snapshotName) { + break; + } + return new Set([snapshotName]); + } + } catch (error) { + log.debug("Failed to load rollback snapshot names for retention", { + error: String(error), + }); + } + return new Set(); +} + +export interface AutoSnapshotPruneOptions { + backups?: NamedBackupMetadata[]; + preserveNames?: Iterable; + keepLatestPerReason?: number; +} + +export interface AutoSnapshotPruneResult { + pruned: NamedBackupMetadata[]; + kept: NamedBackupMetadata[]; +} + +export async function pruneAutoGeneratedSnapshots( + options: AutoSnapshotPruneOptions = {}, +): Promise { + const backups = options.backups ?? (await listNamedBackups()); + const keepLatestPerReason = Math.max( + 1, + options.keepLatestPerReason ?? 1, + ); + const preserveNames = new Set(options.preserveNames ?? []); + for (const rollbackName of await getLatestManualCodexCliRollbackSnapshotNames()) { + preserveNames.add(rollbackName); + } + + const autoSnapshots = backups + .map((backup) => parseAutoSnapshot(backup)) + .filter((snapshot): snapshot is AutoSnapshotDetails => snapshot !== null); + if (autoSnapshots.length === 0) { + return { pruned: [], kept: [] }; + } + + const keepSet = new Set(preserveNames); + const snapshotsByReason = new Map(); + for (const snapshot of autoSnapshots) { + const bucket = snapshotsByReason.get(snapshot.reason) ?? []; + bucket.push(snapshot); + snapshotsByReason.set(snapshot.reason, bucket); + } + + for (const snapshots of snapshotsByReason.values()) { + snapshots.sort((left, right) => right.sortTimestamp - left.sortTimestamp); + for (const snapshot of snapshots.slice(0, keepLatestPerReason)) { + keepSet.add(snapshot.name); + } + } + + const keptNames = new Set(keepSet); + const pruned: NamedBackupMetadata[] = []; + for (const snapshot of autoSnapshots) { + if (keepSet.has(snapshot.name)) { + continue; + } + try { + await unlinkWithRetry(snapshot.backup.path); + pruned.push(snapshot.backup); + } catch (error) { + keptNames.add(snapshot.name); + log.warn("Failed to prune auto-generated snapshot", { + name: snapshot.name, + path: snapshot.backup.path, + error: String(error), + }); + } + } + + const kept = autoSnapshots + .filter((snapshot) => keptNames.has(snapshot.name)) + .map((snapshot) => snapshot.backup); + + return { pruned, kept }; +} + function extractPathTail(pathValue: string): string { const segments = pathValue.split(/[\\/]+/).filter(Boolean); return segments.at(-1) ?? pathValue; @@ -2275,11 +2464,25 @@ function extractPathTail(pathValue: string): string { function redactFilesystemDetails(value: string): string { return value.replace( - /(?:[A-Za-z]:)?[\\/][^"'`\r\n]+(?:[\\/][^"'`\r\n]+)+/g, + /(?:[A-Za-z]:)?[\\/][^"'`\r\n]+(?:[\\/][^"'`\r\n]+)*/g, (pathValue) => extractPathTail(pathValue), ); } +export function formatRedactedFilesystemError(error: unknown): string { + const code = + typeof (error as NodeJS.ErrnoException | undefined)?.code === "string" + ? (error as NodeJS.ErrnoException).code + : undefined; + const rawMessage = + error instanceof Error ? error.message : String(error ?? "unknown error"); + const redactedMessage = redactFilesystemDetails(rawMessage); + if (code && !redactedMessage.includes(code)) { + return `${code}: ${redactedMessage}`; + } + return redactedMessage; +} + function formatSnapshotErrorForLog(error: unknown): string { const code = typeof (error as NodeJS.ErrnoException | undefined)?.code === "string" @@ -2294,6 +2497,12 @@ function formatSnapshotErrorForLog(error: unknown): string { return redactedMessage; } +async function enforceSnapshotRetention(): Promise { + await pruneAutoGeneratedSnapshots({ + keepLatestPerReason: ACCOUNT_SNAPSHOT_RETENTION_PER_REASON, + }); +} + export async function snapshotAccountStorage( options: AccountSnapshotOptions, ): Promise { @@ -2316,8 +2525,9 @@ export async function snapshotAccountStorage( } const backupName = buildAccountSnapshotName(reason, now); + let snapshot: NamedBackupMetadata; try { - return await createBackup(backupName, { + snapshot = await createBackup(backupName, { force, storage: currentStorage, storagePath: resolvedStoragePath, @@ -2333,6 +2543,18 @@ export async function snapshotAccountStorage( }); return null; } + + try { + await enforceSnapshotRetention(); + } catch (error) { + log.warn("Failed to enforce account snapshot retention", { + reason, + backupName, + error: formatSnapshotErrorForLog(error), + }); + } + + return snapshot; } export async function assessNamedBackupRestore( @@ -2358,6 +2580,39 @@ export async function assessNamedBackupRestore( ); } +export async function assessOpencodeAccountPool( + options: { currentStorage?: AccountStorageV3 | null; path?: string } = {}, +): Promise { + const resolvedPath = options.path?.trim() + ? resolvePath(options.path.trim()) + : detectOpencodeAccountPoolPath(); + + if (!resolvedPath) { + return null; + } + + if (equalsResolvedStoragePath(resolvedPath, getStoragePath())) { + throw new Error("Import source cannot be the active storage file."); + } + + const candidate = await loadBackupCandidate(resolvedPath); + const baseBackup = await buildBackupFileMetadata(resolvedPath, { candidate }); + const backup: NamedBackupMetadata = { + name: basename(resolvedPath), + ...baseBackup, + }; + const currentStorage = + options.currentStorage !== undefined + ? options.currentStorage + : await loadAccounts(); + return assessNamedBackupRestoreCandidate( + backup, + candidate, + currentStorage, + candidate.rawAccounts ?? [], + ); +} + function assessNamedBackupRestoreCandidate( backup: NamedBackupMetadata, candidate: LoadedBackupCandidate, @@ -2799,7 +3054,7 @@ async function loadBackupCandidate(path: string): Promise storedVersion: undefined, schemaErrors: [], rawAccounts: [], - error: String(error), + error: formatRedactedFilesystemError(error), errorCode, }; } @@ -2811,6 +3066,12 @@ function equalsNamedBackupEntry(left: string, right: string): boolean { : left === right; } +function equalsResolvedStoragePath(left: string, right: string): boolean { + return process.platform === "win32" + ? left.toLowerCase() === right.toLowerCase() + : left === right; +} + function stripNamedBackupJsonExtension(name: string): string { return name.toLowerCase().endsWith(".json") ? name.slice(0, -".json".length) @@ -3860,6 +4121,9 @@ export async function importAccounts( filePath: string, ): Promise<{ imported: number; total: number; skipped: number }> { const resolvedPath = resolvePath(filePath); + if (equalsResolvedStoragePath(resolvedPath, getStoragePath())) { + throw new Error("Import source cannot be the active storage file."); + } const candidate = await loadImportableBackupCandidate(resolvedPath); return importNormalizedAccounts(candidate.normalized, resolvedPath, { snapshotReason: "import-accounts", diff --git a/lib/ui/copy.ts b/lib/ui/copy.ts index b4fcebad..25e5b998 100644 --- a/lib/ui/copy.ts +++ b/lib/ui/copy.ts @@ -158,8 +158,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, (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.", + "(a) add, (c) check, (b) best, fi(x), (s) settings, (d) deep, (g) problem, (u) restore backup, (i) import OpenCode, (f) fresh, (r) reset, (q) back [a/c/b/x/s/d/g/u/i/f/r/q]: ", + invalidModePrompt: "Use one of: a, c, b, x, s, d, g, u, i, f, r, q.", }, } as const; diff --git a/test/cli.test.ts b/test/cli.test.ts index efbffdce..f035efa9 100644 --- a/test/cli.test.ts +++ b/test/cli.test.ts @@ -728,6 +728,20 @@ describe("CLI Module", () => { }); }); + it("returns import-opencode for fallback import aliases", async () => { + const { promptLoginMode } = await import("../lib/cli.js"); + + mockRl.question.mockResolvedValueOnce("i"); + await expect(promptLoginMode([{ index: 0 }])).resolves.toEqual({ + mode: "import-opencode", + }); + + mockRl.question.mockResolvedValueOnce("import-opencode"); + await expect(promptLoginMode([{ index: 0 }])).resolves.toEqual({ + mode: "import-opencode", + }); + }); + 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 ba024eaf..dc72006b 100644 --- a/test/codex-manager-cli.test.ts +++ b/test/codex-manager-cli.test.ts @@ -13,7 +13,9 @@ const getActionableNamedBackupRestoresMock = vi.fn(); const listNamedBackupsMock = vi.fn(); const listRotatingBackupsMock = vi.fn(); const assessNamedBackupRestoreMock = vi.fn(); +const assessOpencodeAccountPoolMock = vi.fn(); const getNamedBackupsDirectoryPathMock = vi.fn(); +const importAccountsMock = vi.fn(); const restoreNamedBackupMock = vi.fn(); const queuedRefreshMock = vi.fn(); const setCodexCliActiveSelectionMock = vi.fn(); @@ -152,7 +154,9 @@ vi.mock("../lib/storage.js", async () => { listNamedBackups: listNamedBackupsMock, listRotatingBackups: listRotatingBackupsMock, assessNamedBackupRestore: assessNamedBackupRestoreMock, + assessOpencodeAccountPool: assessOpencodeAccountPoolMock, getNamedBackupsDirectoryPath: getNamedBackupsDirectoryPathMock, + importAccounts: importAccountsMock, restoreNamedBackup: restoreNamedBackupMock, exportNamedBackup: exportNamedBackupMock, normalizeAccountStorage: normalizeAccountStorageMock, @@ -646,7 +650,9 @@ describe("codex manager cli commands", () => { listNamedBackupsMock.mockReset(); listRotatingBackupsMock.mockReset(); assessNamedBackupRestoreMock.mockReset(); + assessOpencodeAccountPoolMock.mockReset(); getNamedBackupsDirectoryPathMock.mockReset(); + importAccountsMock.mockReset(); restoreNamedBackupMock.mockReset(); confirmMock.mockReset(); getActionableNamedBackupRestoresMock.mockReset(); @@ -678,7 +684,13 @@ describe("codex manager cli commands", () => { eligibleForRestore: true, error: undefined, }); + assessOpencodeAccountPoolMock.mockResolvedValue(null); getNamedBackupsDirectoryPathMock.mockReturnValue("/mock/backups"); + importAccountsMock.mockResolvedValue({ + imported: 0, + skipped: 0, + total: 0, + }); restoreNamedBackupMock.mockResolvedValue({ imported: 1, skipped: 0, @@ -1065,6 +1077,54 @@ describe("codex manager cli commands", () => { ); }); + it("imports opencode accounts from the login menu", async () => { + setInteractiveTTY(true); + loadAccountsMock.mockResolvedValue({ + version: 3, + activeIndex: 0, + activeIndexByFamily: { codex: 0 }, + accounts: [], + }); + assessOpencodeAccountPoolMock.mockResolvedValue({ + backup: { + name: "OpenCode account pool", + path: "/mock/.opencode/openai-codex-accounts.json", + createdAt: null, + updatedAt: Date.now(), + sizeBytes: 128, + version: 3, + accountCount: 1, + schemaErrors: [], + valid: true, + loadError: undefined, + }, + eligibleForRestore: true, + wouldExceedLimit: false, + error: undefined, + }); + importAccountsMock.mockResolvedValue({ + imported: 1, + skipped: 0, + total: 2, + }); + confirmMock.mockResolvedValueOnce(true); + promptLoginModeMock + .mockResolvedValueOnce({ mode: "import-opencode" }) + .mockResolvedValueOnce({ mode: "cancel" }); + + const { runCodexMultiAuthCli } = await import("../lib/codex-manager.js"); + const exitCode = await runCodexMultiAuthCli(["auth", "login"]); + + expect(exitCode).toBe(0); + expect(assessOpencodeAccountPoolMock).toHaveBeenCalledTimes(1); + expect(importAccountsMock).toHaveBeenCalledWith( + "/mock/.opencode/openai-codex-accounts.json", + ); + expect(confirmMock).toHaveBeenCalledWith( + "Import OpenCode accounts from openai-codex-accounts.json?", + ); + }); + it("returns a non-zero exit code when the direct restore-backup command fails", async () => { setInteractiveTTY(true); const now = Date.now(); @@ -1123,214 +1183,94 @@ describe("codex manager cli commands", () => { ); }); - it("runs restore preview before applying a replace-only named backup", async () => { + it("skips OpenCode import when confirmation is declined", async () => { setInteractiveTTY(true); - const now = Date.now(); loadAccountsMock.mockResolvedValue({ version: 3, activeIndex: 0, activeIndexByFamily: { codex: 0 }, accounts: [ { - email: "current@example.com", - accountId: "current-account", - refreshToken: "refresh-current", - accessToken: "access-current", - expiresAt: now + 3_600_000, - addedAt: now - 2_000, - lastUsed: now - 2_000, - enabled: true, + email: "existing@example.com", + refreshToken: "existing-refresh", + addedAt: Date.now(), + lastUsed: Date.now(), }, ], }); - const assessment = { + assessOpencodeAccountPoolMock.mockResolvedValue({ backup: { - name: "named-backup", - path: "/mock/backups/named-backup.json", + name: "openai-codex-accounts.json", + path: "/mock/.opencode/openai-codex-accounts.json", createdAt: null, - updatedAt: now, - sizeBytes: 128, + updatedAt: Date.now(), + sizeBytes: 256, version: 3, accountCount: 1, schemaErrors: [], valid: true, - loadError: undefined, + loadError: "", }, - backupAccountCount: 1, - dedupedBackupAccountCount: 1, - conflictsWithinBackup: 0, - conflictsWithExisting: 0, - replacedExistingCount: 1, - keptExistingCount: 0, - keptBackupCount: 1, currentAccountCount: 1, - mergedAccountCount: 1, - imported: 0, + mergedAccountCount: 2, + imported: 1, skipped: 0, wouldExceedLimit: false, eligibleForRestore: true, nextActiveIndex: 0, - nextActiveEmail: "current@example.com", - nextActiveAccountId: "current-account", - currentActiveIndex: 0, - currentActiveEmail: "current@example.com", - currentActiveAccountId: "current-account", - activeAccountOutcome: "unchanged", + nextActiveEmail: "existing@example.com", + nextActiveAccountId: undefined, activeAccountChanged: false, - activeAccountPreview: { - current: { - index: 0, - email: "current@example.com", - accountId: "current-account", - }, - next: { - index: 0, - email: "current@example.com", - accountId: "current-account", - }, - outcome: "unchanged", - changed: false, - }, - namedBackupRestorePreview: { - conflicts: [ - { - conflict: { - backupIndex: 0, - currentIndex: 0, - reasons: ["accountId", "email"], - resolution: "backup-kept", - }, - backup: { - index: 0, - email: "current@example.com", - accountId: "current-account", - }, - current: { - index: 0, - email: "current@example.com", - accountId: "current-account", - }, - }, - ], - activeAccount: { - current: { - index: 0, - email: "current@example.com", - accountId: "current-account", - }, - next: { - index: 0, - email: "current@example.com", - accountId: "current-account", - }, - outcome: "unchanged", - changed: false, - }, - }, - error: undefined, - }; - listNamedBackupsMock.mockResolvedValue([assessment.backup]); - assessNamedBackupRestoreMock - .mockResolvedValueOnce(assessment) - .mockResolvedValueOnce(assessment); - selectMock - .mockResolvedValueOnce({ - type: "inspect", - entry: { - kind: "named", - label: assessment.backup.name, - backup: assessment.backup, - assessment, - }, - }) - .mockResolvedValueOnce("preview-restore"); - confirmMock.mockResolvedValueOnce(true); - restoreNamedBackupMock.mockResolvedValueOnce({ - imported: 0, + error: "", + }); + importAccountsMock.mockResolvedValue({ + imported: 1, skipped: 0, - total: 1, + total: 2, }); - const logSpy = vi.spyOn(console, "log").mockImplementation(() => {}); + confirmMock.mockResolvedValueOnce(false); + promptLoginModeMock + .mockResolvedValueOnce({ mode: "import-opencode" }) + .mockResolvedValueOnce({ mode: "cancel" }); + const { runCodexMultiAuthCli } = await import("../lib/codex-manager.js"); - try { - const { runCodexMultiAuthCli } = await import("../lib/codex-manager.js"); - const exitCode = await runCodexMultiAuthCli(["auth", "restore-backup"]); + const exitCode = await runCodexMultiAuthCli(["auth", "login"]); - expect(exitCode).toBe(0); - expect(listNamedBackupsMock).toHaveBeenCalledTimes(1); - expect(selectMock).toHaveBeenCalledTimes(2); - expect(selectMock.mock.calls[0]?.[1]).toMatchObject({ - message: "Backup Browser", - }); - expect(selectMock.mock.calls[1]?.[1]).toMatchObject({ - message: "Backup Actions", - subtitle: "named-backup", - }); - 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( - "Restore named-backup? Import 0 new accounts for 1 total. Replacing 1 current account.", - ); - expect(restoreNamedBackupMock).toHaveBeenCalledWith( - "named-backup", - expect.objectContaining({ assessment }), - ); - expect(logSpy).toHaveBeenCalledWith( - "Replaced 1 current account. Total accounts: 1.", - ); - } finally { - logSpy.mockRestore(); - } + expect(exitCode).toBe(0); + expect(assessOpencodeAccountPoolMock).toHaveBeenCalledTimes(1); + expect(confirmMock).toHaveBeenCalledOnce(); + expect(importAccountsMock).not.toHaveBeenCalled(); + expect(promptLoginModeMock).toHaveBeenCalledTimes(2); }); - it("returns a non-zero exit code when the direct restore-backup command fails", async () => { + it("returns to the dashboard when OpenCode import fails", async () => { setInteractiveTTY(true); - const now = Date.now(); - loadAccountsMock.mockResolvedValue({ + const currentStorage = { 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, + email: "existing@example.com", + refreshToken: "existing-refresh", + addedAt: Date.now(), + lastUsed: Date.now(), }, ], - }); + }; + loadAccountsMock.mockResolvedValue(currentStorage); const assessment = { backup: { - name: "named-backup", - path: "/mock/backups/named-backup.json", + name: "openai-codex-accounts.json", + path: "/mock/.opencode/openai-codex-accounts.json", createdAt: null, - updatedAt: now, - sizeBytes: 128, + updatedAt: Date.now(), + sizeBytes: 256, version: 3, accountCount: 1, schemaErrors: [], valid: true, - loadError: undefined, + loadError: "", }, currentAccountCount: 1, mergedAccountCount: 2, @@ -1338,81 +1278,246 @@ describe("codex manager cli commands", () => { skipped: 0, wouldExceedLimit: false, eligibleForRestore: true, - error: undefined, + nextActiveIndex: 0, + nextActiveEmail: "existing@example.com", + nextActiveAccountId: undefined, + activeAccountChanged: false, + error: "", }; - listNamedBackupsMock.mockResolvedValue([assessment.backup]); - assessNamedBackupRestoreMock.mockResolvedValue(assessment); - selectMock.mockResolvedValueOnce({ type: "restore", assessment }); - restoreNamedBackupMock.mockRejectedValueOnce(new Error("backup locked")); + assessOpencodeAccountPoolMock.mockResolvedValue(assessment); + const importError = makeErrnoError( + "operation not permitted, open 'C:\\Users\\alice\\AppData\\Local\\OpenCode\\openai-codex-accounts.json'", + "EPERM", + ); + importAccountsMock.mockRejectedValueOnce(importError); + confirmMock.mockResolvedValueOnce(true); + promptLoginModeMock + .mockResolvedValueOnce({ mode: "import-opencode" }) + .mockResolvedValueOnce({ mode: "cancel" }); + const errorSpy = vi.spyOn(console, "error").mockImplementation(() => {}); + const { runCodexMultiAuthCli } = await import("../lib/codex-manager.js"); + + const exitCode = await runCodexMultiAuthCli(["auth", "login"]); + expect(exitCode).toBe(0); + expect(importAccountsMock).toHaveBeenCalledWith( + "/mock/.opencode/openai-codex-accounts.json", + ); + expect(promptLoginModeMock).toHaveBeenCalledTimes(2); + expect(errorSpy).toHaveBeenCalledWith( + "Import failed: EPERM: operation not permitted, open 'openai-codex-accounts.json'", + ); + expect( + errorSpy.mock.calls.some(([message]) => + String(message).includes("C:\\Users\\alice\\AppData\\Local\\OpenCode"), + ), + ).toBe(false); + errorSpy.mockRestore(); + }); + + it("returns to the dashboard when OpenCode import assessment fails", async () => { + setInteractiveTTY(true); + loadAccountsMock.mockResolvedValue({ + version: 3, + activeIndex: 0, + activeIndexByFamily: { codex: 0 }, + accounts: [], + }); + assessOpencodeAccountPoolMock.mockRejectedValueOnce( + makeErrnoError("assessment locked", "EPERM"), + ); + promptLoginModeMock + .mockResolvedValueOnce({ mode: "import-opencode" }) + .mockResolvedValueOnce({ mode: "cancel" }); + const errorSpy = vi.spyOn(console, "error").mockImplementation(() => {}); const { runCodexMultiAuthCli } = await import("../lib/codex-manager.js"); - const exitCode = await runCodexMultiAuthCli(["auth", "restore-backup"]); - expect(exitCode).toBe(1); - expect(promptLoginModeMock).not.toHaveBeenCalled(); - expect(confirmMock).toHaveBeenCalledOnce(); - expect(restoreNamedBackupMock).toHaveBeenCalledWith("named-backup", { - assessment, + const exitCode = await runCodexMultiAuthCli(["auth", "login"]); + + expect(exitCode).toBe(0); + expect(assessOpencodeAccountPoolMock).toHaveBeenCalledTimes(1); + expect(confirmMock).not.toHaveBeenCalled(); + expect(importAccountsMock).not.toHaveBeenCalled(); + expect(promptLoginModeMock).toHaveBeenCalledTimes(2); + expect(errorSpy).toHaveBeenCalledWith( + "Import assessment failed: EPERM: assessment locked", + ); + errorSpy.mockRestore(); + }); + + it("returns to the dashboard when no OpenCode account pool is detected", async () => { + setInteractiveTTY(true); + loadAccountsMock.mockResolvedValue({ + version: 3, + activeIndex: 0, + activeIndexByFamily: { codex: 0 }, + accounts: [], }); + assessOpencodeAccountPoolMock.mockResolvedValueOnce(null); + promptLoginModeMock + .mockResolvedValueOnce({ mode: "import-opencode" }) + .mockResolvedValueOnce({ mode: "cancel" }); + const logSpy = vi.spyOn(console, "log").mockImplementation(() => {}); + const { runCodexMultiAuthCli } = await import("../lib/codex-manager.js"); + + const exitCode = await runCodexMultiAuthCli(["auth", "login"]); + + expect(exitCode).toBe(0); + expect(assessOpencodeAccountPoolMock).toHaveBeenCalledTimes(1); + expect(confirmMock).not.toHaveBeenCalled(); + expect(importAccountsMock).not.toHaveBeenCalled(); + expect(promptLoginModeMock).toHaveBeenCalledTimes(2); + expect(logSpy).toHaveBeenCalledWith( + "No OpenCode account pool was detected.", + ); + logSpy.mockRestore(); }); - it("returns a non-zero exit code when the direct restore-backup command fails", async () => { + it.each([ + { + label: "would exceed the account limit", + expectedLog: + "Import would exceed the account limit (2 current, 5 after import). Remove accounts first.", + assessment: { + backup: { + name: "openai-codex-accounts.json", + path: "/mock/.opencode/openai-codex-accounts.json", + createdAt: null, + updatedAt: Date.now(), + sizeBytes: 256, + version: 3, + accountCount: 3, + schemaErrors: [], + valid: true, + loadError: "", + }, + currentAccountCount: 2, + mergedAccountCount: 5, + imported: 3, + skipped: 0, + wouldExceedLimit: true, + eligibleForRestore: true, + nextActiveIndex: 0, + nextActiveEmail: "existing@example.com", + nextActiveAccountId: undefined, + activeAccountChanged: false, + error: "", + }, + }, + { + label: "is not eligible for restore", + expectedLog: "OpenCode account pool is not importable.", + assessment: { + backup: { + name: "openai-codex-accounts.json", + path: "/mock/.opencode/openai-codex-accounts.json", + createdAt: null, + updatedAt: Date.now(), + sizeBytes: 256, + version: 3, + accountCount: 1, + schemaErrors: [], + valid: true, + loadError: "", + }, + currentAccountCount: 1, + mergedAccountCount: null, + imported: null, + skipped: null, + wouldExceedLimit: false, + eligibleForRestore: false, + nextActiveIndex: null, + nextActiveEmail: undefined, + nextActiveAccountId: undefined, + activeAccountChanged: false, + error: "OpenCode account pool is not importable.", + }, + }, + ])( + "skips confirmation and import when the OpenCode assessment $label", + async ({ assessment, expectedLog }) => { + setInteractiveTTY(true); + loadAccountsMock.mockResolvedValue({ + version: 3, + activeIndex: 0, + activeIndexByFamily: { codex: 0 }, + accounts: [ + { + email: "existing@example.com", + refreshToken: "existing-refresh", + addedAt: Date.now(), + lastUsed: Date.now(), + }, + ], + }); + assessOpencodeAccountPoolMock.mockResolvedValue(assessment); + promptLoginModeMock + .mockResolvedValueOnce({ mode: "import-opencode" }) + .mockResolvedValueOnce({ mode: "cancel" }); + const logSpy = vi.spyOn(console, "log").mockImplementation(() => {}); + const { runCodexMultiAuthCli } = await import("../lib/codex-manager.js"); + + const exitCode = await runCodexMultiAuthCli(["auth", "login"]); + + expect(exitCode).toBe(0); + expect(assessOpencodeAccountPoolMock).toHaveBeenCalledTimes(1); + expect(confirmMock).not.toHaveBeenCalled(); + expect(importAccountsMock).not.toHaveBeenCalled(); + expect(promptLoginModeMock).toHaveBeenCalledTimes(2); + expect(logSpy).toHaveBeenCalledWith(expectedLog); + logSpy.mockRestore(); + }, + ); + + it("redacts filesystem details when the OpenCode assessment is invalid", 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, - }, - ], + accounts: [], }); - const assessment = { + assessOpencodeAccountPoolMock.mockResolvedValue({ backup: { - name: "named-backup", - path: "/mock/backups/named-backup.json", + name: "openai-codex-accounts.json", + path: "/mock/.opencode/openai-codex-accounts.json", createdAt: null, - updatedAt: now, - sizeBytes: 128, + updatedAt: Date.now(), + sizeBytes: 256, version: 3, - accountCount: 1, - schemaErrors: [], - valid: true, - loadError: undefined, + accountCount: 0, + schemaErrors: ["invalid"], + valid: false, + loadError: "ENOENT: openai-codex-accounts.json", }, - currentAccountCount: 1, - mergedAccountCount: 2, - imported: 1, - skipped: 0, + currentAccountCount: 0, + mergedAccountCount: null, + imported: null, + skipped: null, wouldExceedLimit: false, - eligibleForRestore: true, - error: undefined, - }; - listNamedBackupsMock.mockResolvedValue([assessment.backup]); - assessNamedBackupRestoreMock.mockResolvedValue(assessment); - selectMock.mockResolvedValueOnce({ type: "restore", assessment }); - restoreNamedBackupMock.mockRejectedValueOnce(new Error("backup locked")); - + eligibleForRestore: false, + nextActiveIndex: null, + nextActiveEmail: undefined, + nextActiveAccountId: undefined, + activeAccountChanged: false, + error: "ENOENT: openai-codex-accounts.json", + }); + promptLoginModeMock + .mockResolvedValueOnce({ mode: "import-opencode" }) + .mockResolvedValueOnce({ mode: "cancel" }); + const logSpy = vi.spyOn(console, "log").mockImplementation(() => {}); const { runCodexMultiAuthCli } = await import("../lib/codex-manager.js"); - const exitCode = await runCodexMultiAuthCli(["auth", "restore-backup"]); - expect(exitCode).toBe(1); - expect(promptLoginModeMock).not.toHaveBeenCalled(); - expect(confirmMock).toHaveBeenCalledOnce(); - expect(restoreNamedBackupMock).toHaveBeenCalledWith("named-backup", { - assessment, - }); - }); + const exitCode = await runCodexMultiAuthCli(["auth", "login"]); + expect(exitCode).toBe(0); + expect(confirmMock).not.toHaveBeenCalled(); + expect(importAccountsMock).not.toHaveBeenCalled(); + expect(logSpy).toHaveBeenCalledWith( + "ENOENT: openai-codex-accounts.json", + ); + logSpy.mockRestore(); + }); it("runs restore preview before applying a replace-only named backup", async () => { setInteractiveTTY(true); const now = Date.now(); @@ -1535,44 +1640,58 @@ describe("codex manager cli commands", () => { }) .mockResolvedValueOnce("preview-restore"); confirmMock.mockResolvedValueOnce(true); + restoreNamedBackupMock.mockResolvedValueOnce({ + imported: 0, + skipped: 0, + total: 1, + }); + const logSpy = vi.spyOn(console, "log").mockImplementation(() => {}); - const { runCodexMultiAuthCli } = await import("../lib/codex-manager.js"); - const exitCode = await runCodexMultiAuthCli(["auth", "restore-backup"]); + try { + const { runCodexMultiAuthCli } = await import("../lib/codex-manager.js"); + const exitCode = await runCodexMultiAuthCli(["auth", "restore-backup"]); - expect(exitCode).toBe(0); - expect(listNamedBackupsMock).toHaveBeenCalledTimes(1); - expect(selectMock).toHaveBeenCalledTimes(2); - expect(selectMock.mock.calls[0]?.[1]).toMatchObject({ - message: "Backup Browser", - }); - expect(selectMock.mock.calls[1]?.[1]).toMatchObject({ - message: "Backup Actions", - subtitle: "named-backup", - }); - expect(assessNamedBackupRestoreMock).toHaveBeenNthCalledWith( - 1, - "named-backup", - expect.objectContaining({ - currentStorage: expect.objectContaining({ - accounts: expect.any(Array), + expect(exitCode).toBe(0); + expect(listNamedBackupsMock).toHaveBeenCalledTimes(1); + expect(selectMock).toHaveBeenCalledTimes(2); + expect(selectMock.mock.calls[0]?.[1]).toMatchObject({ + message: "Backup Browser", + }); + expect(selectMock.mock.calls[1]?.[1]).toMatchObject({ + message: "Backup Actions", + subtitle: "named-backup", + }); + 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(assessNamedBackupRestoreMock).toHaveBeenNthCalledWith( + 2, + "named-backup", + expect.objectContaining({ + currentStorage: expect.objectContaining({ + accounts: expect.any(Array), + }), }), - }), - ); - expect(confirmMock).toHaveBeenCalledWith( - "Restore named-backup? Import 0 new accounts for 1 total. Replacing 1 current account.", - ); - expect(restoreNamedBackupMock).toHaveBeenCalledWith("named-backup", { - assessment, - }); + ); + expect(confirmMock).toHaveBeenCalledWith( + "Restore named-backup? Import 0 new accounts for 1 total. Replacing 1 current account.", + ); + expect(restoreNamedBackupMock).toHaveBeenCalledWith( + "named-backup", + expect.objectContaining({ assessment }), + ); + expect(logSpy).toHaveBeenCalledWith( + "Replaced 1 current account. Total accounts: 1.", + ); + } finally { + logSpy.mockRestore(); + } }); it("restores healthy flagged accounts into active storage", async () => { diff --git a/test/documentation.test.ts b/test/documentation.test.ts index eb06a930..abb5bdf7 100644 --- a/test/documentation.test.ts +++ b/test/documentation.test.ts @@ -166,7 +166,10 @@ describe("Documentation Integrity", () => { }); it("does not include opencode wording in user docs", () => { - const allowedOpencodeFiles = new Set(["docs/reference/storage-paths.md"]); + const allowedOpencodeFiles = new Set([ + "docs/getting-started.md", + "docs/reference/storage-paths.md", + ]); for (const filePath of userDocs) { const content = read(filePath).toLowerCase(); const hasLegacyHostWord = content.includes("opencode"); diff --git a/test/storage.test.ts b/test/storage.test.ts index 2470f97f..b6aabb4f 100644 --- a/test/storage.test.ts +++ b/test/storage.test.ts @@ -1,6 +1,6 @@ import { existsSync, promises as fs } from "node:fs"; import { tmpdir } from "node:os"; -import { dirname, join } from "node:path"; +import { dirname, join, relative, resolve as resolvePath } from "node:path"; import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; import { ACCOUNT_LIMITS } from "../lib/constants.js"; import { @@ -13,16 +13,19 @@ import { removeWithRetry } from "./helpers/remove-with-retry.js"; import { assessNamedBackupRestore, buildNamedBackupPath, + assessOpencodeAccountPool, clearAccounts, clearFlaggedAccounts, createNamedBackup, deduplicateAccounts, deduplicateAccountsByEmail, + detectOpencodeAccountPoolPath, exportAccounts, exportNamedBackup, findMatchingAccountIndex, formatStorageErrorHint, getActionableNamedBackupRestores, + formatRedactedFilesystemError, getFlaggedAccountsPath, getNamedBackupsDirectoryPath, NAMED_BACKUP_LIST_CONCURRENCY, @@ -1149,6 +1152,27 @@ describe("storage", () => { await expect(importAccounts(exportPath)).rejects.toThrow(/Invalid JSON/); }); + it("should fail import when file is the active storage file", async () => { + await fs.writeFile( + testStoragePath, + JSON.stringify({ + version: 3, + activeIndex: 0, + accounts: [ + { + accountId: "current-account", + refreshToken: "ref-current", + addedAt: 1, + lastUsed: 1, + }, + ], + }), + ); + await expect(importAccounts(testStoragePath)).rejects.toThrow( + /Import source cannot be the active storage file/, + ); + }); + it("should fail import when file contains invalid format", async () => { await fs.writeFile(exportPath, JSON.stringify({ invalid: "format" })); await expect(importAccounts(exportPath)).rejects.toThrow( @@ -5690,6 +5714,269 @@ describe("storage", () => { }); }); + describe("opencode account pool detection", () => { + const originalLocalAppData = process.env.LOCALAPPDATA; + const originalAppData = process.env.APPDATA; + const originalHome = process.env.HOME; + const originalPoolPath = process.env.CODEX_OPENCODE_POOL_PATH; + const originalUserProfile = process.env.USERPROFILE; + let tempRoot = ""; + let poolPath = ""; + + beforeEach(async () => { + tempRoot = join( + tmpdir(), + "opencode-pool-" + Math.random().toString(36).slice(2), + ); + poolPath = join(tempRoot, "OpenCode", "openai-codex-accounts.json"); + process.env.LOCALAPPDATA = tempRoot; + process.env.APPDATA = tempRoot; + delete process.env.CODEX_OPENCODE_POOL_PATH; + await fs.mkdir(dirname(poolPath), { recursive: true }); + setStoragePathDirect(join(tempRoot, "current-storage.json")); + }); + + afterEach(async () => { + if (originalLocalAppData === undefined) delete process.env.LOCALAPPDATA; + else process.env.LOCALAPPDATA = originalLocalAppData; + if (originalAppData === undefined) delete process.env.APPDATA; + else process.env.APPDATA = originalAppData; + if (originalHome === undefined) delete process.env.HOME; + else process.env.HOME = originalHome; + if (originalPoolPath === undefined) + delete process.env.CODEX_OPENCODE_POOL_PATH; + else process.env.CODEX_OPENCODE_POOL_PATH = originalPoolPath; + if (originalUserProfile === undefined) delete process.env.USERPROFILE; + else process.env.USERPROFILE = originalUserProfile; + setStoragePathDirect(null); + if (tempRoot) { + await fs.rm(tempRoot, { recursive: true, force: true }); + } + }); + + it("detects and assesses a valid opencode pool source", async () => { + const poolStorage = { + version: 3, + activeIndex: 0, + accounts: [ + { + accountId: "pool-account", + refreshToken: "ref-pool", + addedAt: 1, + lastUsed: 1, + }, + ], + }; + await fs.writeFile(poolPath, JSON.stringify(poolStorage)); + + const detected = detectOpencodeAccountPoolPath(); + expect(detected).toBe(poolPath); + + const assessment = await assessOpencodeAccountPool(); + expect(assessment).not.toBeNull(); + expect(assessment?.backup.path).toBe(poolPath); + expect(assessment?.backup.valid).toBe(true); + expect(assessment?.eligibleForRestore).toBe(true); + expect(assessment?.imported).toBe(1); + }); + + it("prefers an explicit CODEX_OPENCODE_POOL_PATH override", async () => { + const explicitPoolPath = join(tempRoot, "explicit", "pool.json"); + await fs.mkdir(dirname(explicitPoolPath), { recursive: true }); + await fs.writeFile( + explicitPoolPath, + JSON.stringify({ + version: 3, + activeIndex: 0, + accounts: [ + { + accountId: "explicit-account", + refreshToken: "ref-explicit", + addedAt: 1, + lastUsed: 1, + }, + ], + }), + ); + await fs.writeFile( + poolPath, + JSON.stringify({ + version: 3, + activeIndex: 0, + accounts: [ + { + accountId: "fallback-account", + refreshToken: "ref-fallback", + addedAt: 1, + lastUsed: 1, + }, + ], + }), + ); + process.env.CODEX_OPENCODE_POOL_PATH = ` ${explicitPoolPath} `; + + const detected = detectOpencodeAccountPoolPath(); + expect(detected).toBe(explicitPoolPath); + + const assessment = await assessOpencodeAccountPool(); + expect(assessment?.backup.path).toBe(explicitPoolPath); + expect(assessment?.imported).toBe(1); + }); + + it("does not fall back to auto-detection when an explicit CODEX_OPENCODE_POOL_PATH override is missing", async () => { + await fs.writeFile( + poolPath, + JSON.stringify({ + version: 3, + activeIndex: 0, + accounts: [ + { + accountId: "fallback-account", + refreshToken: "ref-fallback", + addedAt: 1, + lastUsed: 1, + }, + ], + }), + ); + process.env.CODEX_OPENCODE_POOL_PATH = join( + tempRoot, + "explicit", + "missing-pool.json", + ); + + expect(detectOpencodeAccountPoolPath()).toBeNull(); + await expect(assessOpencodeAccountPool()).resolves.toBeNull(); + }); + + it("falls back to homedir when LOCALAPPDATA and APPDATA are not set", async () => { + delete process.env.LOCALAPPDATA; + delete process.env.APPDATA; + const fakeHome = join(tempRoot, "fake-home"); + const homedirPoolPath = join( + fakeHome, + ".opencode", + "openai-codex-accounts.json", + ); + process.env.HOME = fakeHome; + process.env.USERPROFILE = fakeHome; + await fs.mkdir(dirname(homedirPoolPath), { recursive: true }); + await fs.writeFile( + homedirPoolPath, + JSON.stringify({ + version: 3, + activeIndex: 0, + accounts: [ + { + accountId: "homedir-account", + refreshToken: "ref-homedir", + addedAt: 1, + lastUsed: 1, + }, + ], + }), + ); + + expect(detectOpencodeAccountPoolPath()).toBe(homedirPoolPath); + const assessment = await assessOpencodeAccountPool(); + expect(assessment?.backup.path).toBe(homedirPoolPath); + expect(assessment?.eligibleForRestore).toBe(true); + expect(assessment?.imported).toBe(1); + }); + + it("resolves relative app data candidates before detection", async () => { + const relativeBase = relative( + process.cwd(), + join(tempRoot, "relative-appdata"), + ); + const resolvedPoolPath = resolvePath( + join(relativeBase, "OpenCode", "openai-codex-accounts.json"), + ); + delete process.env.LOCALAPPDATA; + process.env.APPDATA = relativeBase; + await fs.mkdir(dirname(resolvedPoolPath), { recursive: true }); + await fs.writeFile( + resolvedPoolPath, + JSON.stringify({ + version: 3, + activeIndex: 0, + accounts: [ + { + accountId: "relative-appdata-account", + refreshToken: "ref-relative", + addedAt: 1, + lastUsed: 1, + }, + ], + }), + ); + + expect(detectOpencodeAccountPoolPath()).toBe(resolvedPoolPath); + const assessment = await assessOpencodeAccountPool(); + expect(assessment?.backup.path).toBe(resolvedPoolPath); + expect(assessment?.eligibleForRestore).toBe(true); + expect(assessment?.imported).toBe(1); + }); + + it("redacts single-segment filesystem paths in error messages", async () => { + const error = new Error( + "EPERM: operation not permitted, open 'C:\\accounts.json'", + ) as NodeJS.ErrnoException; + error.code = "EPERM"; + + expect(formatRedactedFilesystemError(error)).toBe( + "EPERM: operation not permitted, open 'accounts.json'", + ); + }); + + it("refuses malformed opencode source before any mutation", async () => { + await fs.writeFile(poolPath, "not valid json"); + + const detected = detectOpencodeAccountPoolPath(); + expect(detected).toBe(poolPath); + + const assessment = await assessOpencodeAccountPool(); + expect(assessment).not.toBeNull(); + expect(assessment?.backup.valid).toBe(false); + expect(assessment?.eligibleForRestore).toBe(false); + expect(assessment?.imported).toBeNull(); + const current = await loadAccounts(); + expect(current).toMatchObject({ + version: 3, + activeIndex: 0, + activeIndexByFamily: {}, + accounts: [], + restoreEligible: true, + restoreReason: "missing-storage", + }); + }); + + it("rejects using the active storage file as the opencode import source", async () => { + const activeStoragePath = getStoragePath(); + await fs.writeFile( + activeStoragePath, + JSON.stringify({ + version: 3, + activeIndex: 0, + accounts: [ + { + accountId: "current-account", + refreshToken: "ref-current", + addedAt: 1, + lastUsed: 1, + }, + ], + }), + ); + process.env.CODEX_OPENCODE_POOL_PATH = activeStoragePath; + + expect(detectOpencodeAccountPoolPath()).toBe(activeStoragePath); + await expect(assessOpencodeAccountPool()).rejects.toThrow( + "Import source cannot be the active storage file.", + ); + }); + }); + describe("clearAccounts edge cases", () => { it("removes primary, backup, and wal artifacts", async () => { const now = Date.now(); From e4073506e285d140d860b7975b4c6aff6093b45f Mon Sep 17 00:00:00 2001 From: ndycode Date: Thu, 19 Mar 2026 23:55:01 +0800 Subject: [PATCH 2/4] fix(storage): harden opencode import retention and redaction --- lib/codex-manager.ts | 5 +- lib/storage.ts | 123 +++++++++++++++---- test/codex-manager-cli.test.ts | 63 +++++++++- test/storage.test.ts | 210 ++++++++++++++++++++++++++++++++- 4 files changed, 369 insertions(+), 32 deletions(-) diff --git a/lib/codex-manager.ts b/lib/codex-manager.ts index 829e2486..592e71bb 100644 --- a/lib/codex-manager.ts +++ b/lib/codex-manager.ts @@ -4468,8 +4468,9 @@ async function runAuthLogin(): Promise { continue; } if (!assessment.backup.valid || !assessment.eligibleForRestore) { - const assessmentErrorLabel = - assessment.error || "OpenCode account pool is not importable."; + const assessmentErrorLabel = formatRedactedFilesystemError( + assessment.error || "OpenCode account pool is not importable.", + ); console.log(assessmentErrorLabel); continue; } diff --git a/lib/storage.ts b/lib/storage.ts index 0d2e92e8..52c9d9ca 100644 --- a/lib/storage.ts +++ b/lib/storage.ts @@ -57,8 +57,10 @@ const ROTATING_BACKUP_STALE_ARTIFACT_MAX_AGE_MS = 60_000; export const NAMED_BACKUP_LIST_CONCURRENCY = 8; export const ACCOUNT_SNAPSHOT_RETENTION_PER_REASON = 3; const RESET_MARKER_SUFFIX = ".reset-intent"; +const AUTO_SNAPSHOT_MARKER_SUFFIX = ".snapshot-auto"; let storageBackupEnabled = true; let lastAccountsSaveTimestamp = 0; +const retentionEnforcementInFlight = new Set(); export interface FlaggedAccountMetadataV1 extends AccountMetadataV3 { flaggedAt: number; @@ -1880,8 +1882,9 @@ export async function getRestoreAssessment(): Promise { }; } -async function scanNamedBackups(): Promise { - const backupRoot = getNamedBackupRoot(getStoragePath()); +async function scanNamedBackups( + backupRoot = getNamedBackupRoot(getStoragePath()), +): Promise { try { const entries = await retryTransientFilesystemOperation(() => fs.readdir(backupRoot, { withFileTypes: true }), @@ -1949,8 +1952,9 @@ async function scanNamedBackups(): Promise { } } -async function listNamedBackupsWithoutLoading(): Promise { - const backupRoot = getNamedBackupRoot(getStoragePath()); +async function listNamedBackupsWithoutLoading( + backupRoot = getNamedBackupRoot(getStoragePath()), +): Promise { try { const entries = await retryTransientFilesystemOperation(() => fs.readdir(backupRoot, { withFileTypes: true }), @@ -2088,7 +2092,12 @@ export function detectOpencodeAccountPoolPath(): string | null { const explicit = process.env.CODEX_OPENCODE_POOL_PATH; if (explicit?.trim()) { const explicitPath = resolvePath(explicit.trim()); - return existsSync(explicitPath) ? explicitPath : null; + if (!existsSync(explicitPath)) { + throw new Error( + `CODEX_OPENCODE_POOL_PATH points to a file that does not exist: ${basename(explicitPath)}`, + ); + } + return explicitPath; } const appDataBases = [process.env.LOCALAPPDATA, process.env.APPDATA].filter( @@ -2322,6 +2331,43 @@ export function isAccountSnapshotName(name: string): boolean { ); } +function getAutoSnapshotMarkerPath(backupPath: string): string { + return `${backupPath}${AUTO_SNAPSHOT_MARKER_SUFFIX}`; +} + +function normalizeStorageComparisonKey(pathValue: string): string { + const resolvedPath = resolvePath(pathValue); + return process.platform === "win32" + ? resolvedPath.toLowerCase() + : resolvedPath; +} + +async function writeAutoSnapshotMarker( + backupPath: string, + reason: AccountSnapshotReason, +): Promise { + const markerPath = getAutoSnapshotMarkerPath(backupPath); + await retryTransientFilesystemOperation(() => + fs.writeFile(markerPath, reason, { encoding: "utf-8", mode: 0o600 }), + ); +} + +async function deleteAutoSnapshotMarker(backupPath: string): Promise { + const markerPath = getAutoSnapshotMarkerPath(backupPath); + try { + await unlinkWithRetry(markerPath); + } catch (error) { + const code = (error as NodeJS.ErrnoException).code; + if (code !== "ENOENT") { + throw error; + } + } +} + +function isMarkedAutoSnapshot(backupPath: string): boolean { + return existsSync(getAutoSnapshotMarkerPath(backupPath)); +} + function getAccountSnapshotReason(name: string): string | null { const match = name.match( /^accounts-(?[a-z0-9-]+)-snapshot-\d{4}-\d{2}-\d{2}_\d{2}-\d{2}-\d{2}_\d{3}$/i, @@ -2389,6 +2435,7 @@ export interface AutoSnapshotPruneOptions { backups?: NamedBackupMetadata[]; preserveNames?: Iterable; keepLatestPerReason?: number; + storagePath?: string; } export interface AutoSnapshotPruneResult { @@ -2399,7 +2446,13 @@ export interface AutoSnapshotPruneResult { export async function pruneAutoGeneratedSnapshots( options: AutoSnapshotPruneOptions = {}, ): Promise { - const backups = options.backups ?? (await listNamedBackups()); + const backups = + options.backups ?? + ( + await scanNamedBackups( + getNamedBackupRoot(options.storagePath ?? getStoragePath()), + ) + ).backups.map((entry) => entry.backup); const keepLatestPerReason = Math.max( 1, options.keepLatestPerReason ?? 1, @@ -2410,7 +2463,12 @@ export async function pruneAutoGeneratedSnapshots( } const autoSnapshots = backups - .map((backup) => parseAutoSnapshot(backup)) + .map((backup) => { + if (!isMarkedAutoSnapshot(backup.path)) { + return null; + } + return parseAutoSnapshot(backup); + }) .filter((snapshot): snapshot is AutoSnapshotDetails => snapshot !== null); if (autoSnapshots.length === 0) { return { pruned: [], kept: [] }; @@ -2439,6 +2497,7 @@ export async function pruneAutoGeneratedSnapshots( } try { await unlinkWithRetry(snapshot.backup.path); + await deleteAutoSnapshotMarker(snapshot.backup.path); pruned.push(snapshot.backup); } catch (error) { keptNames.add(snapshot.name); @@ -2484,23 +2543,25 @@ export function formatRedactedFilesystemError(error: unknown): string { } function formatSnapshotErrorForLog(error: unknown): string { - const code = - typeof (error as NodeJS.ErrnoException | undefined)?.code === "string" - ? (error as NodeJS.ErrnoException).code - : undefined; - const rawMessage = - error instanceof Error ? error.message : String(error ?? "unknown error"); - const redactedMessage = redactFilesystemDetails(rawMessage); - if (code && !redactedMessage.includes(code)) { - return `${code}: ${redactedMessage}`; - } - return redactedMessage; + return formatRedactedFilesystemError(error); } -async function enforceSnapshotRetention(): Promise { - await pruneAutoGeneratedSnapshots({ - keepLatestPerReason: ACCOUNT_SNAPSHOT_RETENTION_PER_REASON, - }); +async function enforceSnapshotRetention(storagePath?: string): Promise { + const resolvedStoragePath = normalizeStorageComparisonKey( + storagePath ?? getStoragePath(), + ); + if (retentionEnforcementInFlight.has(resolvedStoragePath)) { + return; + } + retentionEnforcementInFlight.add(resolvedStoragePath); + try { + await pruneAutoGeneratedSnapshots({ + keepLatestPerReason: ACCOUNT_SNAPSHOT_RETENTION_PER_REASON, + storagePath: resolvedStoragePath, + }); + } finally { + retentionEnforcementInFlight.delete(resolvedStoragePath); + } } export async function snapshotAccountStorage( @@ -2545,7 +2606,17 @@ export async function snapshotAccountStorage( } try { - await enforceSnapshotRetention(); + await writeAutoSnapshotMarker(snapshot.path, reason); + } catch (error) { + log.warn("Failed to mark account storage snapshot for retention", { + reason, + backupName, + error: formatSnapshotErrorForLog(error), + }); + } + + try { + await enforceSnapshotRetention(resolvedStoragePath); } catch (error) { log.warn("Failed to enforce account snapshot retention", { reason, @@ -3067,9 +3138,11 @@ function equalsNamedBackupEntry(left: string, right: string): boolean { } function equalsResolvedStoragePath(left: string, right: string): boolean { + const normalizedLeft = resolvePath(left); + const normalizedRight = resolvePath(right); return process.platform === "win32" - ? left.toLowerCase() === right.toLowerCase() - : left === right; + ? normalizedLeft.toLowerCase() === normalizedRight.toLowerCase() + : normalizedLeft === normalizedRight; } function stripNamedBackupJsonExtension(name: string): string { diff --git a/test/codex-manager-cli.test.ts b/test/codex-manager-cli.test.ts index dc72006b..07eee299 100644 --- a/test/codex-manager-cli.test.ts +++ b/test/codex-manager-cli.test.ts @@ -1125,6 +1125,58 @@ describe("codex manager cli commands", () => { ); }); + it("skips OpenCode import when the detected pool resolves to the active storage file", async () => { + setInteractiveTTY(true); + loadAccountsMock.mockResolvedValue({ + version: 3, + activeIndex: 0, + activeIndexByFamily: { codex: 0 }, + accounts: [], + }); + assessOpencodeAccountPoolMock.mockResolvedValue({ + backup: { + name: "openai-codex-accounts.json", + path: "C:\\mock\\openai-codex-accounts.json", + createdAt: null, + updatedAt: Date.now(), + sizeBytes: 128, + version: 3, + accountCount: 1, + schemaErrors: [], + valid: true, + loadError: "", + }, + currentAccountCount: 0, + mergedAccountCount: null, + imported: null, + skipped: null, + wouldExceedLimit: false, + eligibleForRestore: false, + nextActiveIndex: null, + nextActiveEmail: undefined, + nextActiveAccountId: undefined, + activeAccountChanged: false, + error: "Import source cannot be the active storage file.", + }); + getStoragePathMock.mockReturnValue("c:/mock/openai-codex-accounts.json"); + promptLoginModeMock + .mockResolvedValueOnce({ mode: "import-opencode" }) + .mockResolvedValueOnce({ mode: "cancel" }); + const logSpy = vi.spyOn(console, "log").mockImplementation(() => {}); + const { runCodexMultiAuthCli } = await import("../lib/codex-manager.js"); + + const exitCode = await runCodexMultiAuthCli(["auth", "login"]); + + expect(exitCode).toBe(0); + expect(assessOpencodeAccountPoolMock).toHaveBeenCalledTimes(1); + expect(confirmMock).not.toHaveBeenCalled(); + expect(importAccountsMock).not.toHaveBeenCalled(); + expect(logSpy).toHaveBeenCalledWith( + "Import source cannot be the active storage file.", + ); + logSpy.mockRestore(); + }); + it("returns a non-zero exit code when the direct restore-backup command fails", async () => { setInteractiveTTY(true); const now = Date.now(); @@ -1488,7 +1540,8 @@ describe("codex manager cli commands", () => { accountCount: 0, schemaErrors: ["invalid"], valid: false, - loadError: "ENOENT: openai-codex-accounts.json", + loadError: + "ENOENT: C:\\Users\\alice\\AppData\\Local\\OpenCode\\openai-codex-accounts.json", }, currentAccountCount: 0, mergedAccountCount: null, @@ -1500,7 +1553,8 @@ describe("codex manager cli commands", () => { nextActiveEmail: undefined, nextActiveAccountId: undefined, activeAccountChanged: false, - error: "ENOENT: openai-codex-accounts.json", + error: + "ENOENT: C:\\Users\\alice\\AppData\\Local\\OpenCode\\openai-codex-accounts.json", }); promptLoginModeMock .mockResolvedValueOnce({ mode: "import-opencode" }) @@ -1516,6 +1570,11 @@ describe("codex manager cli commands", () => { expect(logSpy).toHaveBeenCalledWith( "ENOENT: openai-codex-accounts.json", ); + expect( + logSpy.mock.calls.some(([message]) => + String(message).includes("C:\\Users\\alice\\AppData\\Local\\OpenCode"), + ), + ).toBe(false); logSpy.mockRestore(); }); it("runs restore preview before applying a replace-only named backup", async () => { diff --git a/test/storage.test.ts b/test/storage.test.ts index b6aabb4f..29846784 100644 --- a/test/storage.test.ts +++ b/test/storage.test.ts @@ -36,6 +36,7 @@ import { loadAccounts, loadFlaggedAccounts, normalizeAccountStorage, + pruneAutoGeneratedSnapshots, restoreNamedBackup, resolveAccountSelectionIndex, saveFlaggedAccounts, @@ -2408,6 +2409,175 @@ describe("storage", () => { ).toHaveLength(2); }); + it("keeps only the latest marked auto snapshots per reason while preserving manual snapshot-shaped backups", async () => { + await saveAccounts({ + version: 3, + activeIndex: 0, + accounts: [ + { + accountId: "primary", + refreshToken: "ref-primary", + addedAt: 1, + lastUsed: 1, + }, + ], + }); + + const manualSnapshotShapedBackup = await createNamedBackup( + "accounts-delete-saved-accounts-snapshot-2023-12-31_23-59-59_999", + ); + for (const millisecond of [1, 2, 3, 4]) { + await snapshotAccountStorage({ + reason: "delete-saved-accounts", + now: Date.UTC(2024, 0, 2, 3, 4, 5, millisecond), + }); + } + + const backups = await listNamedBackups(); + const matchingNames = backups + .filter((backup) => + backup.name.startsWith("accounts-delete-saved-accounts-snapshot-"), + ) + .map((backup) => backup.name); + + expect(matchingNames).toContain(manualSnapshotShapedBackup.name); + expect(matchingNames).toHaveLength(4); + expect(matchingNames).not.toContain( + "accounts-delete-saved-accounts-snapshot-2024-01-02_03-04-05_001", + ); + expect(matchingNames).toContain( + "accounts-delete-saved-accounts-snapshot-2024-01-02_03-04-05_002", + ); + expect(matchingNames).toContain( + "accounts-delete-saved-accounts-snapshot-2024-01-02_03-04-05_003", + ); + expect(matchingNames).toContain( + "accounts-delete-saved-accounts-snapshot-2024-01-02_03-04-05_004", + ); + }); + + it("pins snapshot retention to the captured storage root", async () => { + const alternateStoragePath = join( + testWorkDir, + "alternate", + "openai-codex-accounts.json", + ); + const alternateStorage = { + version: 3 as const, + activeIndex: 0, + accounts: [ + { + accountId: "alternate", + refreshToken: "ref-alternate", + addedAt: 1, + lastUsed: 1, + }, + ], + }; + + for (const millisecond of [1, 2, 3, 4]) { + await snapshotAccountStorage({ + reason: "import-accounts", + now: Date.UTC(2024, 0, 2, 3, 4, 5, millisecond), + storagePath: alternateStoragePath, + storage: alternateStorage, + }); + } + + const alternateBackupRoot = join(dirname(alternateStoragePath), "backups"); + const alternateEntries = await fs.readdir(alternateBackupRoot); + expect( + alternateEntries.filter( + (name) => + name.startsWith("accounts-import-accounts-snapshot-") && + name.endsWith(".json"), + ), + ).toHaveLength(3); + expect( + existsSync( + join( + alternateBackupRoot, + "accounts-import-accounts-snapshot-2024-01-02_03-04-05_001.json", + ), + ), + ).toBe(false); + }); + + it("honors preserveNames while pruning only marker-backed auto snapshots", async () => { + await saveAccounts({ + version: 3, + activeIndex: 0, + accounts: [ + { + accountId: "primary", + refreshToken: "ref-primary", + addedAt: 1, + lastUsed: 1, + }, + ], + }); + + const makeSnapshotLikeBackup = async ( + name: string, + updatedAt: number, + markedAuto: boolean, + ) => { + const backup = await createNamedBackup(name); + const timestamp = new Date(updatedAt); + await fs.utimes(backup.path, timestamp, timestamp); + if (markedAuto) { + await fs.writeFile(`${backup.path}.snapshot-auto`, "import-accounts"); + } + const listedBackup = (await listNamedBackups()).find( + (candidate) => candidate.name === backup.name, + ); + if (!listedBackup) { + throw new Error(`expected backup metadata for ${backup.name}`); + } + return listedBackup; + }; + + const preserved = await makeSnapshotLikeBackup( + "accounts-import-accounts-snapshot-2024-01-02_03-04-05_001", + Date.UTC(2024, 0, 2, 3, 4, 5, 1), + true, + ); + const middle = await makeSnapshotLikeBackup( + "accounts-import-accounts-snapshot-2024-01-02_03-04-05_002", + Date.UTC(2024, 0, 2, 3, 4, 5, 2), + true, + ); + const newest = await makeSnapshotLikeBackup( + "accounts-import-accounts-snapshot-2024-01-02_03-04-05_003", + Date.UTC(2024, 0, 2, 3, 4, 5, 3), + true, + ); + const newestPlus = await makeSnapshotLikeBackup( + "accounts-import-accounts-snapshot-2024-01-02_03-04-05_004", + Date.UTC(2024, 0, 2, 3, 4, 5, 4), + true, + ); + const manual = await makeSnapshotLikeBackup( + "accounts-import-accounts-snapshot-2024-01-02_03-04-05_999", + Date.UTC(2024, 0, 2, 3, 4, 5, 999), + false, + ); + + const result = await pruneAutoGeneratedSnapshots({ + backups: (await listNamedBackups()).filter((backup) => + backup.name.startsWith("accounts-import-accounts-snapshot-"), + ), + keepLatestPerReason: 2, + preserveNames: [preserved.name], + }); + + expect(result.pruned.map((backup) => backup.name)).toEqual([middle.name]); + expect(result.kept.map((backup) => backup.name)).toEqual( + expect.arrayContaining([preserved.name, newest.name, newestPlus.name]), + ); + expect(existsSync(manual.path)).toBe(true); + }); + it("skips snapshot when no accounts exist", async () => { const snapshot = await snapshotAccountStorage({ reason: "delete-saved-accounts", @@ -2768,7 +2938,10 @@ describe("storage", () => { .spyOn(fs, "writeFile") .mockImplementation(async (path, data, options) => { if ( - String(path).includes("accounts-delete-saved-accounts-snapshot-") + String(path).includes( + "accounts-delete-saved-accounts-snapshot-", + ) && + !String(path).endsWith(".snapshot-auto") ) { snapshotWriteAttempts += 1; if (snapshotWriteAttempts === 1) { @@ -5845,8 +6018,12 @@ describe("storage", () => { "missing-pool.json", ); - expect(detectOpencodeAccountPoolPath()).toBeNull(); - await expect(assessOpencodeAccountPool()).resolves.toBeNull(); + expect(() => detectOpencodeAccountPoolPath()).toThrow( + "CODEX_OPENCODE_POOL_PATH points to a file that does not exist: missing-pool.json", + ); + await expect(assessOpencodeAccountPool()).rejects.toThrow( + "CODEX_OPENCODE_POOL_PATH points to a file that does not exist: missing-pool.json", + ); }); it("falls back to homedir when LOCALAPPDATA and APPDATA are not set", async () => { @@ -5975,6 +6152,33 @@ describe("storage", () => { "Import source cannot be the active storage file.", ); }); + + it("rejects Windows-style aliases of the active storage file as the opencode import source", async () => { + const activeStoragePath = getStoragePath(); + await fs.writeFile( + activeStoragePath, + JSON.stringify({ + version: 3, + activeIndex: 0, + accounts: [ + { + accountId: "current-account", + refreshToken: "ref-current", + addedAt: 1, + lastUsed: 1, + }, + ], + }), + ); + const windowsAlias = activeStoragePath + .replace(/\//g, "\\") + .replace(/^([a-z]):/, (_, drive: string) => drive.toUpperCase() + ":"); + process.env.CODEX_OPENCODE_POOL_PATH = windowsAlias; + + await expect(assessOpencodeAccountPool()).rejects.toThrow( + "Import source cannot be the active storage file.", + ); + }); }); describe("clearAccounts edge cases", () => { From cc66e926ce2cd563c38ee43eb471c82036090637 Mon Sep 17 00:00:00 2001 From: ndycode Date: Fri, 20 Mar 2026 00:16:20 +0800 Subject: [PATCH 3/4] fix(storage): guard rollback retention scans and prune races --- lib/storage.ts | 155 ++++++++++++++++++++----------------- test/storage.test.ts | 177 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 261 insertions(+), 71 deletions(-) diff --git a/lib/storage.ts b/lib/storage.ts index 52c9d9ca..efe10b88 100644 --- a/lib/storage.ts +++ b/lib/storage.ts @@ -60,7 +60,10 @@ const RESET_MARKER_SUFFIX = ".reset-intent"; const AUTO_SNAPSHOT_MARKER_SUFFIX = ".snapshot-auto"; let storageBackupEnabled = true; let lastAccountsSaveTimestamp = 0; -const retentionEnforcementInFlight = new Set(); +const retentionEnforcementInFlight = new Map< + string, + Promise +>(); export interface FlaggedAccountMetadataV1 extends AccountMetadataV3 { flaggedAt: number; @@ -2419,7 +2422,7 @@ async function getLatestManualCodexCliRollbackSnapshotNames(): Promise< } const snapshotName = run.rollbackSnapshot?.name?.trim(); if (!snapshotName) { - break; + continue; } return new Set([snapshotName]); } @@ -2446,74 +2449,92 @@ export interface AutoSnapshotPruneResult { export async function pruneAutoGeneratedSnapshots( options: AutoSnapshotPruneOptions = {}, ): Promise { - const backups = - options.backups ?? - ( - await scanNamedBackups( - getNamedBackupRoot(options.storagePath ?? getStoragePath()), - ) - ).backups.map((entry) => entry.backup); - const keepLatestPerReason = Math.max( - 1, - options.keepLatestPerReason ?? 1, + const resolvedStoragePath = normalizeStorageComparisonKey( + options.storagePath ?? getStoragePath(), ); - const preserveNames = new Set(options.preserveNames ?? []); - for (const rollbackName of await getLatestManualCodexCliRollbackSnapshotNames()) { - preserveNames.add(rollbackName); + const inFlight = retentionEnforcementInFlight.get(resolvedStoragePath); + if (inFlight) { + return await inFlight; } - const autoSnapshots = backups - .map((backup) => { - if (!isMarkedAutoSnapshot(backup.path)) { - return null; - } - return parseAutoSnapshot(backup); - }) - .filter((snapshot): snapshot is AutoSnapshotDetails => snapshot !== null); - if (autoSnapshots.length === 0) { - return { pruned: [], kept: [] }; - } + const prunePromise = (async (): Promise => { + const backups = + options.backups ?? + ( + await scanNamedBackups( + getNamedBackupRoot(resolvedStoragePath), + ) + ).backups.map((entry) => entry.backup); + const keepLatestPerReason = Math.max( + 1, + options.keepLatestPerReason ?? 1, + ); + const preserveNames = new Set(options.preserveNames ?? []); + for (const rollbackName of await getLatestManualCodexCliRollbackSnapshotNames()) { + preserveNames.add(rollbackName); + } - const keepSet = new Set(preserveNames); - const snapshotsByReason = new Map(); - for (const snapshot of autoSnapshots) { - const bucket = snapshotsByReason.get(snapshot.reason) ?? []; - bucket.push(snapshot); - snapshotsByReason.set(snapshot.reason, bucket); - } + const autoSnapshots = backups + .map((backup) => { + if (!isMarkedAutoSnapshot(backup.path)) { + return null; + } + return parseAutoSnapshot(backup); + }) + .filter((snapshot): snapshot is AutoSnapshotDetails => snapshot !== null); + if (autoSnapshots.length === 0) { + return { pruned: [], kept: [] }; + } - for (const snapshots of snapshotsByReason.values()) { - snapshots.sort((left, right) => right.sortTimestamp - left.sortTimestamp); - for (const snapshot of snapshots.slice(0, keepLatestPerReason)) { - keepSet.add(snapshot.name); + const keepSet = new Set(preserveNames); + const snapshotsByReason = new Map(); + for (const snapshot of autoSnapshots) { + const bucket = snapshotsByReason.get(snapshot.reason) ?? []; + bucket.push(snapshot); + snapshotsByReason.set(snapshot.reason, bucket); } - } - const keptNames = new Set(keepSet); - const pruned: NamedBackupMetadata[] = []; - for (const snapshot of autoSnapshots) { - if (keepSet.has(snapshot.name)) { - continue; + for (const snapshots of snapshotsByReason.values()) { + snapshots.sort((left, right) => right.sortTimestamp - left.sortTimestamp); + for (const snapshot of snapshots.slice(0, keepLatestPerReason)) { + keepSet.add(snapshot.name); + } } - try { - await unlinkWithRetry(snapshot.backup.path); - await deleteAutoSnapshotMarker(snapshot.backup.path); - pruned.push(snapshot.backup); - } catch (error) { - keptNames.add(snapshot.name); - log.warn("Failed to prune auto-generated snapshot", { - name: snapshot.name, - path: snapshot.backup.path, - error: String(error), - }); + + const keptNames = new Set(keepSet); + const pruned: NamedBackupMetadata[] = []; + for (const snapshot of autoSnapshots) { + if (keepSet.has(snapshot.name)) { + continue; + } + try { + await unlinkWithRetry(snapshot.backup.path); + await deleteAutoSnapshotMarker(snapshot.backup.path); + pruned.push(snapshot.backup); + } catch (error) { + keptNames.add(snapshot.name); + log.warn("Failed to prune auto-generated snapshot", { + name: snapshot.name, + path: snapshot.backup.path, + error: String(error), + }); + } } - } - const kept = autoSnapshots - .filter((snapshot) => keptNames.has(snapshot.name)) - .map((snapshot) => snapshot.backup); + const kept = autoSnapshots + .filter((snapshot) => keptNames.has(snapshot.name)) + .map((snapshot) => snapshot.backup); - return { pruned, kept }; + return { pruned, kept }; + })(); + retentionEnforcementInFlight.set(resolvedStoragePath, prunePromise); + try { + return await prunePromise; + } finally { + if (retentionEnforcementInFlight.get(resolvedStoragePath) === prunePromise) { + retentionEnforcementInFlight.delete(resolvedStoragePath); + } + } } function extractPathTail(pathValue: string): string { @@ -2550,18 +2571,10 @@ async function enforceSnapshotRetention(storagePath?: string): Promise { const resolvedStoragePath = normalizeStorageComparisonKey( storagePath ?? getStoragePath(), ); - if (retentionEnforcementInFlight.has(resolvedStoragePath)) { - return; - } - retentionEnforcementInFlight.add(resolvedStoragePath); - try { - await pruneAutoGeneratedSnapshots({ - keepLatestPerReason: ACCOUNT_SNAPSHOT_RETENTION_PER_REASON, - storagePath: resolvedStoragePath, - }); - } finally { - retentionEnforcementInFlight.delete(resolvedStoragePath); - } + await pruneAutoGeneratedSnapshots({ + keepLatestPerReason: ACCOUNT_SNAPSHOT_RETENTION_PER_REASON, + storagePath: resolvedStoragePath, + }); } export async function snapshotAccountStorage( diff --git a/test/storage.test.ts b/test/storage.test.ts index 29846784..793f84ae 100644 --- a/test/storage.test.ts +++ b/test/storage.test.ts @@ -2578,6 +2578,183 @@ describe("storage", () => { expect(existsSync(manual.path)).toBe(true); }); + it("keeps scanning rollback history when the newest manual entry lacks a snapshot name", async () => { + await saveAccounts({ + version: 3, + activeIndex: 0, + accounts: [ + { + accountId: "primary", + refreshToken: "ref-primary", + addedAt: 1, + lastUsed: 1, + }, + ], + }); + + const protectedBackup = await createNamedBackup( + "accounts-manual-sync-snapshot-2024-01-02_03-04-05_001", + ); + await fs.writeFile(`${protectedBackup.path}.snapshot-auto`, "manual-sync"); + const extraBackup = await createNamedBackup( + "accounts-manual-sync-snapshot-2024-01-02_03-04-05_002", + ); + await fs.writeFile(`${extraBackup.path}.snapshot-auto`, "manual-sync"); + const backups = (await listNamedBackups()).filter((backup) => + backup.name.startsWith("accounts-manual-sync-snapshot-"), + ); + + vi.resetModules(); + vi.doMock("../lib/sync-history.js", async () => { + const actual = await vi.importActual( + "../lib/sync-history.js", + ); + return { + ...actual, + readSyncHistory: vi.fn().mockResolvedValue([ + { + kind: "codex-cli-sync", + recordedAt: 10, + run: { + trigger: "manual", + outcome: "changed", + runAt: 10, + sourcePath: "source-older.json", + targetPath: "target.json", + summary: { + sourceAccountCount: 1, + targetAccountCountBefore: 1, + targetAccountCountAfter: 1, + addedAccountCount: 0, + updatedAccountCount: 1, + unchangedAccountCount: 0, + destinationOnlyPreservedCount: 0, + selectionChanged: false, + }, + rollbackSnapshot: { + name: protectedBackup.name, + path: protectedBackup.path, + }, + }, + }, + { + kind: "codex-cli-sync", + recordedAt: 11, + run: { + trigger: "manual", + outcome: "changed", + runAt: 11, + sourcePath: "source-newer.json", + targetPath: "target.json", + summary: { + sourceAccountCount: 1, + targetAccountCountBefore: 1, + targetAccountCountAfter: 1, + addedAccountCount: 0, + updatedAccountCount: 0, + unchangedAccountCount: 1, + destinationOnlyPreservedCount: 0, + selectionChanged: false, + }, + rollbackSnapshot: { + name: " ", + path: "", + }, + }, + }, + ]), + }; + }); + + try { + const isolatedStorageModule = await import("../lib/storage.js"); + const result = await isolatedStorageModule.pruneAutoGeneratedSnapshots({ + backups, + keepLatestPerReason: 1, + }); + + expect(result.kept.map((backup) => backup.name)).toContain( + protectedBackup.name, + ); + } finally { + vi.doUnmock("../lib/sync-history.js"); + vi.resetModules(); + } + }); + + it("deduplicates concurrent prune requests for the same storage path", async () => { + await saveAccounts({ + version: 3, + activeIndex: 0, + accounts: [ + { + accountId: "primary", + refreshToken: "ref-primary", + addedAt: 1, + lastUsed: 1, + }, + ], + }); + + const oldBackup = await createNamedBackup( + "accounts-import-race-snapshot-2024-01-02_03-04-05_001", + ); + await fs.writeFile(`${oldBackup.path}.snapshot-auto`, "import-accounts"); + const newBackup = await createNamedBackup( + "accounts-import-race-snapshot-2024-01-02_03-04-05_002", + ); + await fs.writeFile(`${newBackup.path}.snapshot-auto`, "import-accounts"); + + const originalUnlink = fs.unlink.bind(fs); + const unlinkSpy = vi.spyOn(fs, "unlink"); + let releaseFirstUnlink!: () => void; + let resolveFirstUnlinkStarted!: () => void; + const firstUnlinkStarted = new Promise((resolve) => { + resolveFirstUnlinkStarted = resolve; + }); + const firstUnlinkGate = new Promise((resolve) => { + releaseFirstUnlink = resolve; + }); + let heldUnlink = false; + unlinkSpy.mockImplementation(async (path, ...rest) => { + if (!heldUnlink && path === oldBackup.path) { + heldUnlink = true; + resolveFirstUnlinkStarted(); + await firstUnlinkGate; + } + return await originalUnlink(path, ...rest); + }); + + const backups = (await listNamedBackups()).filter((backup) => + backup.name.startsWith("accounts-import-race-snapshot-"), + ); + const firstPrune = pruneAutoGeneratedSnapshots({ + backups, + keepLatestPerReason: 1, + }); + await firstUnlinkStarted; + const secondPrune = pruneAutoGeneratedSnapshots({ + backups, + keepLatestPerReason: 1, + }); + releaseFirstUnlink(); + + const [firstResult, secondResult] = await Promise.all([ + firstPrune, + secondPrune, + ]); + + expect(firstResult).toEqual(secondResult); + expect( + unlinkSpy.mock.calls.filter(([path]) => path === oldBackup.path), + ).toHaveLength(1); + expect( + unlinkSpy.mock.calls.filter( + ([path]) => path === `${oldBackup.path}.snapshot-auto`, + ), + ).toHaveLength(1); + }); + it("skips snapshot when no accounts exist", async () => { const snapshot = await snapshotAccountStorage({ reason: "delete-saved-accounts", From 25aeb38ade1b31ff03a138c6ccef30688a927d2b Mon Sep 17 00:00:00 2001 From: ndycode Date: Fri, 20 Mar 2026 00:42:03 +0800 Subject: [PATCH 4/4] test(login): cover self-import assessment failure path --- test/codex-manager-cli.test.ts | 36 +++++++--------------------------- 1 file changed, 7 insertions(+), 29 deletions(-) diff --git a/test/codex-manager-cli.test.ts b/test/codex-manager-cli.test.ts index 07eee299..d9729ad6 100644 --- a/test/codex-manager-cli.test.ts +++ b/test/codex-manager-cli.test.ts @@ -1133,36 +1133,14 @@ describe("codex manager cli commands", () => { activeIndexByFamily: { codex: 0 }, accounts: [], }); - assessOpencodeAccountPoolMock.mockResolvedValue({ - backup: { - name: "openai-codex-accounts.json", - path: "C:\\mock\\openai-codex-accounts.json", - createdAt: null, - updatedAt: Date.now(), - sizeBytes: 128, - version: 3, - accountCount: 1, - schemaErrors: [], - valid: true, - loadError: "", - }, - currentAccountCount: 0, - mergedAccountCount: null, - imported: null, - skipped: null, - wouldExceedLimit: false, - eligibleForRestore: false, - nextActiveIndex: null, - nextActiveEmail: undefined, - nextActiveAccountId: undefined, - activeAccountChanged: false, - error: "Import source cannot be the active storage file.", - }); + assessOpencodeAccountPoolMock.mockRejectedValueOnce( + new Error("Import source cannot be the active storage file."), + ); getStoragePathMock.mockReturnValue("c:/mock/openai-codex-accounts.json"); promptLoginModeMock .mockResolvedValueOnce({ mode: "import-opencode" }) .mockResolvedValueOnce({ mode: "cancel" }); - const logSpy = vi.spyOn(console, "log").mockImplementation(() => {}); + const errorSpy = vi.spyOn(console, "error").mockImplementation(() => {}); const { runCodexMultiAuthCli } = await import("../lib/codex-manager.js"); const exitCode = await runCodexMultiAuthCli(["auth", "login"]); @@ -1171,10 +1149,10 @@ describe("codex manager cli commands", () => { expect(assessOpencodeAccountPoolMock).toHaveBeenCalledTimes(1); expect(confirmMock).not.toHaveBeenCalled(); expect(importAccountsMock).not.toHaveBeenCalled(); - expect(logSpy).toHaveBeenCalledWith( - "Import source cannot be the active storage file.", + expect(errorSpy).toHaveBeenCalledWith( + "Import assessment failed: Import source cannot be the active storage file.", ); - logSpy.mockRestore(); + errorSpy.mockRestore(); }); it("returns a non-zero exit code when the direct restore-backup command fails", async () => {