diff --git a/lib/codex-manager/experimental-sync-target-entry.ts b/lib/codex-manager/experimental-sync-target-entry.ts index f72201a3..336d45c0 100644 --- a/lib/codex-manager/experimental-sync-target-entry.ts +++ b/lib/codex-manager/experimental-sync-target-entry.ts @@ -52,8 +52,6 @@ export async function loadExperimentalSyncTargetEntry(params: { "EBUSY", "EPERM", "EAGAIN", - "ENOTEMPTY", - "EACCES", ]), maxAttempts: 4, sleep: params.sleep, diff --git a/lib/codex-manager/settings-write-queue.ts b/lib/codex-manager/settings-write-queue.ts index dc89fc08..26cb5348 100644 --- a/lib/codex-manager/settings-write-queue.ts +++ b/lib/codex-manager/settings-write-queue.ts @@ -1,6 +1,6 @@ export const SETTINGS_WRITE_MAX_ATTEMPTS = 4; export const SETTINGS_WRITE_BASE_DELAY_MS = 50; -export const SETTINGS_WRITE_MAX_DELAY_MS = 1_000; +export const SETTINGS_WRITE_MAX_DELAY_MS = 30_000; export const RETRYABLE_SETTINGS_WRITE_CODES = new Set([ "EBUSY", "EPERM", diff --git a/lib/codex-manager/unified-settings-controller.ts b/lib/codex-manager/unified-settings-controller.ts index 21cb6e6c..866a32d8 100644 --- a/lib/codex-manager/unified-settings-controller.ts +++ b/lib/codex-manager/unified-settings-controller.ts @@ -10,54 +10,56 @@ export type SettingsHubActionType = | "backend" | "back"; +export type UnifiedSettingsControllerDeps = { + cloneDashboardSettings: ( + settings: DashboardDisplaySettings, + ) => DashboardDisplaySettings; + cloneBackendPluginConfig: (config: PluginConfig) => PluginConfig; + loadDashboardDisplaySettings: () => Promise; + loadPluginConfig: () => PluginConfig; + applyUiThemeFromDashboardSettings: ( + settings: DashboardDisplaySettings, + ) => void; + promptSettingsHub: ( + focus: SettingsHubActionType, + ) => Promise<{ type: SettingsHubActionType } | null>; + configureDashboardDisplaySettings: ( + current: DashboardDisplaySettings, + ) => Promise; + configureStatuslineSettings: ( + current: DashboardDisplaySettings, + ) => Promise; + promptBehaviorSettings: ( + current: DashboardDisplaySettings, + ) => Promise; + promptThemeSettings: ( + current: DashboardDisplaySettings, + ) => Promise; + dashboardSettingsEqual: ( + left: DashboardDisplaySettings, + right: DashboardDisplaySettings, + ) => boolean; + persistDashboardSettingsSelection: ( + selected: DashboardDisplaySettings, + keys: readonly (keyof DashboardDisplaySettings)[], + scope: string, + ) => Promise; + promptExperimentalSettings: ( + config: PluginConfig, + ) => Promise; + backendSettingsEqual: (left: PluginConfig, right: PluginConfig) => boolean; + persistBackendConfigSelection: ( + config: PluginConfig, + scope: string, + ) => Promise; + configureBackendSettings: (config: PluginConfig) => Promise; + BEHAVIOR_PANEL_KEYS: readonly (keyof DashboardDisplaySettings)[]; + THEME_PANEL_KEYS: readonly (keyof DashboardDisplaySettings)[]; +}; + export async function configureUnifiedSettingsController( initialSettings: DashboardDisplaySettings | undefined, - deps: { - cloneDashboardSettings: ( - settings: DashboardDisplaySettings, - ) => DashboardDisplaySettings; - cloneBackendPluginConfig: (config: PluginConfig) => PluginConfig; - loadDashboardDisplaySettings: () => Promise; - loadPluginConfig: () => PluginConfig; - applyUiThemeFromDashboardSettings: ( - settings: DashboardDisplaySettings, - ) => void; - promptSettingsHub: ( - focus: SettingsHubActionType, - ) => Promise<{ type: SettingsHubActionType } | null>; - configureDashboardDisplaySettings: ( - current: DashboardDisplaySettings, - ) => Promise; - configureStatuslineSettings: ( - current: DashboardDisplaySettings, - ) => Promise; - promptBehaviorSettings: ( - current: DashboardDisplaySettings, - ) => Promise; - promptThemeSettings: ( - current: DashboardDisplaySettings, - ) => Promise; - dashboardSettingsEqual: ( - left: DashboardDisplaySettings, - right: DashboardDisplaySettings, - ) => boolean; - persistDashboardSettingsSelection: ( - selected: DashboardDisplaySettings, - keys: readonly (keyof DashboardDisplaySettings)[], - scope: string, - ) => Promise; - promptExperimentalSettings: ( - config: PluginConfig, - ) => Promise; - backendSettingsEqual: (left: PluginConfig, right: PluginConfig) => boolean; - persistBackendConfigSelection: ( - config: PluginConfig, - scope: string, - ) => Promise; - configureBackendSettings: (config: PluginConfig) => Promise; - BEHAVIOR_PANEL_KEYS: readonly (keyof DashboardDisplaySettings)[]; - THEME_PANEL_KEYS: readonly (keyof DashboardDisplaySettings)[]; - }, + deps: UnifiedSettingsControllerDeps, ): Promise { let current = deps.cloneDashboardSettings( initialSettings ?? (await deps.loadDashboardDisplaySettings()), diff --git a/lib/codex-manager/unified-settings-entry.ts b/lib/codex-manager/unified-settings-entry.ts index b5591d53..c1f48a4b 100644 --- a/lib/codex-manager/unified-settings-entry.ts +++ b/lib/codex-manager/unified-settings-entry.ts @@ -1,109 +1,16 @@ import type { DashboardDisplaySettings } from "../dashboard-settings.js"; -import type { PluginConfig } from "../types.js"; -import type { SettingsHubActionType } from "./unified-settings-controller.js"; +import type { + UnifiedSettingsControllerDeps, +} from "./unified-settings-controller.js"; export async function configureUnifiedSettingsEntry( initialSettings: DashboardDisplaySettings | undefined, deps: { configureUnifiedSettingsController: ( initialSettings: DashboardDisplaySettings | undefined, - deps: { - cloneDashboardSettings: ( - settings: DashboardDisplaySettings, - ) => DashboardDisplaySettings; - cloneBackendPluginConfig: (config: PluginConfig) => PluginConfig; - loadDashboardDisplaySettings: () => Promise; - loadPluginConfig: () => PluginConfig; - applyUiThemeFromDashboardSettings: ( - settings: DashboardDisplaySettings, - ) => void; - promptSettingsHub: ( - focus: SettingsHubActionType, - ) => Promise<{ type: SettingsHubActionType } | null>; - configureDashboardDisplaySettings: ( - current: DashboardDisplaySettings, - ) => Promise; - configureStatuslineSettings: ( - current: DashboardDisplaySettings, - ) => Promise; - promptBehaviorSettings: ( - current: DashboardDisplaySettings, - ) => Promise; - promptThemeSettings: ( - current: DashboardDisplaySettings, - ) => Promise; - dashboardSettingsEqual: ( - left: DashboardDisplaySettings, - right: DashboardDisplaySettings, - ) => boolean; - persistDashboardSettingsSelection: ( - selected: DashboardDisplaySettings, - keys: readonly (keyof DashboardDisplaySettings)[], - scope: string, - ) => Promise; - promptExperimentalSettings: ( - config: PluginConfig, - ) => Promise; - backendSettingsEqual: ( - left: PluginConfig, - right: PluginConfig, - ) => boolean; - persistBackendConfigSelection: ( - config: PluginConfig, - scope: string, - ) => Promise; - configureBackendSettings: ( - config: PluginConfig, - ) => Promise; - BEHAVIOR_PANEL_KEYS: readonly (keyof DashboardDisplaySettings)[]; - THEME_PANEL_KEYS: readonly (keyof DashboardDisplaySettings)[]; - }, + deps: UnifiedSettingsControllerDeps, ) => Promise; - cloneDashboardSettings: ( - settings: DashboardDisplaySettings, - ) => DashboardDisplaySettings; - cloneBackendPluginConfig: (config: PluginConfig) => PluginConfig; - loadDashboardDisplaySettings: () => Promise; - loadPluginConfig: () => PluginConfig; - applyUiThemeFromDashboardSettings: ( - settings: DashboardDisplaySettings, - ) => void; - promptSettingsHub: ( - focus: SettingsHubActionType, - ) => Promise<{ type: SettingsHubActionType } | null>; - configureDashboardDisplaySettings: ( - current: DashboardDisplaySettings, - ) => Promise; - configureStatuslineSettings: ( - current: DashboardDisplaySettings, - ) => Promise; - promptBehaviorSettings: ( - current: DashboardDisplaySettings, - ) => Promise; - promptThemeSettings: ( - current: DashboardDisplaySettings, - ) => Promise; - dashboardSettingsEqual: ( - left: DashboardDisplaySettings, - right: DashboardDisplaySettings, - ) => boolean; - persistDashboardSettingsSelection: ( - selected: DashboardDisplaySettings, - keys: readonly (keyof DashboardDisplaySettings)[], - scope: string, - ) => Promise; - promptExperimentalSettings: ( - config: PluginConfig, - ) => Promise; - backendSettingsEqual: (left: PluginConfig, right: PluginConfig) => boolean; - persistBackendConfigSelection: ( - config: PluginConfig, - scope: string, - ) => Promise; - configureBackendSettings: (config: PluginConfig) => Promise; - BEHAVIOR_PANEL_KEYS: readonly (keyof DashboardDisplaySettings)[]; - THEME_PANEL_KEYS: readonly (keyof DashboardDisplaySettings)[]; - }, + } & UnifiedSettingsControllerDeps, ): Promise { return deps.configureUnifiedSettingsController(initialSettings, { cloneDashboardSettings: deps.cloneDashboardSettings, diff --git a/lib/storage/account-port.ts b/lib/storage/account-port.ts index a69b607f..b14f29f5 100644 --- a/lib/storage/account-port.ts +++ b/lib/storage/account-port.ts @@ -1,4 +1,4 @@ -import type { AccountStorageV3, FlaggedAccountStorageV1 } from "../storage.js"; +import type { AccountStorageV3 } from "../storage.js"; export async function exportAccountsSnapshot(params: { resolvedPath: string; @@ -103,35 +103,3 @@ export async function importAccountsSnapshot(params: { return result; } -export async function saveFlaggedAccountsEntry(params: { - storage: FlaggedAccountStorageV1; - withStorageLock: (fn: () => Promise) => Promise; - saveUnlocked: (storage: FlaggedAccountStorageV1) => Promise; -}): Promise { - return params.withStorageLock(async () => { - await params.saveUnlocked(params.storage); - }); -} - -export async function clearFlaggedAccountsEntry(params: { - path: string; - withStorageLock: (fn: () => Promise) => Promise; - markerPath: string; - getBackupPaths: () => Promise; - clearFlaggedAccountsOnDisk: (args: { - path: string; - markerPath: string; - backupPaths: string[]; - logError: (message: string, details: Record) => void; - }) => Promise; - logError: (message: string, details: Record) => void; -}): Promise { - return params.withStorageLock(async () => { - await params.clearFlaggedAccountsOnDisk({ - path: params.path, - markerPath: params.markerPath, - backupPaths: await params.getBackupPaths(), - logError: params.logError, - }); - }); -} diff --git a/lib/storage/account-save.ts b/lib/storage/account-save.ts index 121c3bcc..08986fcf 100644 --- a/lib/storage/account-save.ts +++ b/lib/storage/account-save.ts @@ -37,19 +37,13 @@ export async function saveAccountsToDisk( await params.ensureGitignore(); if (params.looksLikeSyntheticFixtureStorage(storage)) { - try { - const existing = await params.loadExistingStorage(); - if ( - existing && - existing.accounts.length > 0 && - !params.looksLikeSyntheticFixtureStorage(existing) - ) { - throw params.createSyntheticFixtureError(); - } - } catch (error) { - if (error instanceof Error && error.message.includes("synthetic")) { - throw error; - } + const existing = await params.loadExistingStorage(); + if ( + existing && + existing.accounts.length > 0 && + !params.looksLikeSyntheticFixtureStorage(existing) + ) { + throw params.createSyntheticFixtureError(); } } diff --git a/lib/storage/account-snapshot.ts b/lib/storage/account-snapshot.ts index 10d4330a..2fa28583 100644 --- a/lib/storage/account-snapshot.ts +++ b/lib/storage/account-snapshot.ts @@ -1,16 +1,13 @@ -import type { BackupSnapshotMetadata } from "./backup-metadata.js"; - -type SnapshotStats = { - exists: boolean; - bytes?: number; - mtimeMs?: number; -}; +import type { BackupSnapshotMetadata, SnapshotStats } from "./backup-metadata.js"; +/** + * Read size and mtime metadata for a backup candidate when it exists. + */ export async function statSnapshot( path: string, deps: { stat: typeof import("node:fs").promises.stat; - logWarn?: (message: string, meta: Record) => void; + logWarn: (message: string, meta: Record) => void; }, ): Promise { try { @@ -18,8 +15,15 @@ export async function statSnapshot( return { exists: true, bytes: stats.size, mtimeMs: stats.mtimeMs }; } catch (error) { const code = (error as NodeJS.ErrnoException).code; + if (code === "EBUSY" || code === "EPERM") { + deps.logWarn("Backup candidate is locked", { + path, + error: String(error), + }); + return { exists: true }; + } if (code !== "ENOENT") { - deps.logWarn?.("Failed to stat backup candidate", { + deps.logWarn("Failed to stat backup candidate", { path, error: String(error), }); @@ -28,6 +32,10 @@ export async function statSnapshot( } } +/** + * Build backup metadata for an account snapshot, treating ENOENT load races as invalid snapshots + * that were present when the initial stat succeeded. + */ export async function describeAccountSnapshot( path: string, kind: BackupSnapshotMetadata["kind"], @@ -39,7 +47,7 @@ export async function describeAccountSnapshot( schemaErrors: string[]; storedVersion?: unknown; }>; - logWarn?: (message: string, meta: Record) => void; + logWarn: (message: string, meta: Record) => void; }, ): Promise { const stats = await deps.statSnapshot(path); @@ -49,14 +57,26 @@ export async function describeAccountSnapshot( try { const { normalized, schemaErrors, storedVersion } = await deps.loadAccountsFromPath(path); + let resolvedStats = stats; + for ( + let attempt = 0; + attempt < 2 && + (resolvedStats.bytes === undefined || resolvedStats.mtimeMs === undefined); + attempt += 1 + ) { + const refreshedStats = await deps.statSnapshot(path); + if (refreshedStats.exists) { + resolvedStats = refreshedStats; + } + } return { kind, path, index: deps.index, exists: true, valid: !!normalized, - bytes: stats.bytes, - mtimeMs: stats.mtimeMs, + bytes: resolvedStats.bytes ?? 0, + mtimeMs: resolvedStats.mtimeMs ?? 0, version: typeof storedVersion === "number" ? storedVersion : undefined, accountCount: normalized?.accounts.length, schemaErrors: schemaErrors.length > 0 ? schemaErrors : undefined, @@ -64,7 +84,7 @@ export async function describeAccountSnapshot( } catch (error) { const code = (error as NodeJS.ErrnoException).code; if (code !== "ENOENT") { - deps.logWarn?.("Failed to inspect account snapshot", { + deps.logWarn("Failed to inspect account snapshot", { path, error: String(error), }); diff --git a/lib/storage/backup-metadata.ts b/lib/storage/backup-metadata.ts index 7eb4be8f..64b35107 100644 --- a/lib/storage/backup-metadata.ts +++ b/lib/storage/backup-metadata.ts @@ -52,3 +52,9 @@ export function buildMetadataSection( snapshots, }; } + +export type SnapshotStats = { + exists: boolean; + bytes?: number; + mtimeMs?: number; +}; diff --git a/lib/storage/flagged-entry.ts b/lib/storage/flagged-entry.ts index a06b9106..1c5c656a 100644 --- a/lib/storage/flagged-entry.ts +++ b/lib/storage/flagged-entry.ts @@ -1,14 +1,4 @@ -import type { FlaggedAccountStorageV1 } from "../storage.js"; - -export async function saveFlaggedAccountsEntry(params: { - storage: FlaggedAccountStorageV1; - withStorageLock: (fn: () => Promise) => Promise; - saveUnlocked: (storage: FlaggedAccountStorageV1) => Promise; -}): Promise { - return params.withStorageLock(async () => { - await params.saveUnlocked(params.storage); - }); -} +export { saveFlaggedAccountsEntry } from "./flagged-save-entry.js"; export async function clearFlaggedAccountsEntry(params: { path: string; diff --git a/lib/storage/flagged-storage-file.ts b/lib/storage/flagged-storage-file.ts index bf573a1d..50f7b663 100644 --- a/lib/storage/flagged-storage-file.ts +++ b/lib/storage/flagged-storage-file.ts @@ -1,13 +1,41 @@ import type { FlaggedAccountStorageV1 } from "../storage.js"; +import { sleep } from "../utils.js"; + +const RETRYABLE_READ_CODES = new Set(["EBUSY", "EAGAIN"]); + +function isRetryableReadError(error: unknown): boolean { + const code = (error as NodeJS.ErrnoException | undefined)?.code; + return typeof code === "string" && RETRYABLE_READ_CODES.has(code); +} + +async function readFileWithRetry( + path: string, + deps: { + readFile: typeof import("node:fs").promises.readFile; + sleep?: (ms: number) => Promise; + }, +): Promise { + for (let attempt = 0; ; attempt += 1) { + try { + return await deps.readFile(path, "utf-8"); + } catch (error) { + if (!isRetryableReadError(error) || attempt >= 3) { + throw error; + } + await (deps.sleep ?? sleep)(10 * 2 ** attempt); + } + } +} export async function loadFlaggedAccountsFromFile( path: string, deps: { readFile: typeof import("node:fs").promises.readFile; normalizeFlaggedStorage: (data: unknown) => FlaggedAccountStorageV1; + sleep?: (ms: number) => Promise; }, ): Promise { - const content = await deps.readFile(path, "utf-8"); + const content = await readFileWithRetry(path, deps); const data = JSON.parse(content) as unknown; return deps.normalizeFlaggedStorage(data); } diff --git a/lib/storage/snapshot-inspectors.ts b/lib/storage/snapshot-inspectors.ts index f2989ce2..03cdd61b 100644 --- a/lib/storage/snapshot-inspectors.ts +++ b/lib/storage/snapshot-inspectors.ts @@ -1,11 +1,5 @@ import type { FlaggedAccountStorageV1 } from "../storage.js"; -import type { BackupSnapshotMetadata } from "./backup-metadata.js"; - -type SnapshotStats = { - exists: boolean; - bytes?: number; - mtimeMs?: number; -}; +import type { BackupSnapshotMetadata, SnapshotStats } from "./backup-metadata.js"; export async function describeAccountsWalSnapshot( path: string, diff --git a/test/account-clear-entry.test.ts b/test/account-clear-entry.test.ts index 4981d5e7..0e906fa9 100644 --- a/test/account-clear-entry.test.ts +++ b/test/account-clear-entry.test.ts @@ -4,9 +4,10 @@ import { clearAccountsEntry } from "../lib/storage/account-clear-entry.js"; describe("account clear entry", () => { it("delegates clear through the storage lock and backup resolver", async () => { const clearAccountStorageArtifacts = vi.fn(async () => undefined); + const withStorageLock = vi.fn(async (fn: () => Promise) => fn()); await clearAccountsEntry({ path: "/tmp/accounts.json", - withStorageLock: async (fn) => fn(), + withStorageLock, resetMarkerPath: "/tmp/accounts.reset-intent", walPath: "/tmp/accounts.wal", getBackupPaths: async () => ["/tmp/accounts.json.bak"], @@ -14,6 +15,8 @@ describe("account clear entry", () => { logError: vi.fn(), }); + expect(withStorageLock).toHaveBeenCalledOnce(); + expect(clearAccountStorageArtifacts).toHaveBeenCalledTimes(1); expect(clearAccountStorageArtifacts).toHaveBeenCalledWith({ path: "/tmp/accounts.json", resetMarkerPath: "/tmp/accounts.reset-intent", @@ -22,4 +25,92 @@ describe("account clear entry", () => { logError: expect.any(Function), }); }); + + it("serializes concurrent clears through the shared storage lock", async () => { + const events: string[] = []; + let releaseFirst: (() => void) | undefined; + const firstGate = new Promise((resolve) => { + releaseFirst = resolve; + }); + let queue = Promise.resolve(); + const withStorageLock = vi.fn(async (fn: () => Promise) => { + const run = queue.then(fn); + queue = run.then( + () => undefined, + () => undefined, + ); + return run; + }); + const clearAccountStorageArtifacts = vi.fn( + async ({ path }: { path: string }) => { + events.push(`start:${path}`); + if (path === "/tmp/first.json") { + await firstGate; + } + events.push(`end:${path}`); + }, + ); + + const firstCall = clearAccountsEntry({ + path: "/tmp/first.json", + withStorageLock, + resetMarkerPath: "/tmp/first.reset-intent", + walPath: "/tmp/first.wal", + getBackupPaths: async () => ["/tmp/first.json.bak"], + clearAccountStorageArtifacts, + logError: vi.fn(), + }); + const secondCall = clearAccountsEntry({ + path: "/tmp/second.json", + withStorageLock, + resetMarkerPath: "/tmp/second.reset-intent", + walPath: "/tmp/second.wal", + getBackupPaths: async () => ["/tmp/second.json.bak"], + clearAccountStorageArtifacts, + logError: vi.fn(), + }); + + await Promise.resolve(); + await Promise.resolve(); + + expect(events).toEqual(["start:/tmp/first.json"]); + releaseFirst?.(); + await Promise.all([firstCall, secondCall]); + + expect(withStorageLock).toHaveBeenCalledTimes(2); + expect(events).toEqual([ + "start:/tmp/first.json", + "end:/tmp/first.json", + "start:/tmp/second.json", + "end:/tmp/second.json", + ]); + }); + + it("preserves windows-style paths when clearing storage artifacts", async () => { + const windowsPath = "C:\\codex\\accounts.json"; + const resetMarkerPath = "C:\\codex\\accounts.reset-intent"; + const walPath = "C:\\codex\\accounts.wal"; + const backupPath = "C:\\codex\\accounts.json.bak"; + const withStorageLock = vi.fn(async (fn: () => Promise) => fn()); + const clearAccountStorageArtifacts = vi.fn(async () => undefined); + + await clearAccountsEntry({ + path: windowsPath, + withStorageLock, + resetMarkerPath, + walPath, + getBackupPaths: async () => [backupPath], + clearAccountStorageArtifacts, + logError: vi.fn(), + }); + + expect(withStorageLock).toHaveBeenCalledOnce(); + expect(clearAccountStorageArtifacts).toHaveBeenCalledWith({ + path: windowsPath, + resetMarkerPath, + walPath, + backupPaths: [backupPath], + logError: expect.any(Function), + }); + }); }); diff --git a/test/account-port.test.ts b/test/account-port.test.ts index 49b4cb8c..4dc3ece1 100644 --- a/test/account-port.test.ts +++ b/test/account-port.test.ts @@ -1,35 +1,10 @@ import { describe, expect, it, vi } from "vitest"; import { - clearFlaggedAccountsEntry, exportAccountsSnapshot, importAccountsSnapshot, - saveFlaggedAccountsEntry, } from "../lib/storage/account-port.js"; describe("account port helpers", () => { - it("delegates flagged save through storage lock", async () => { - const saveUnlocked = vi.fn(async () => undefined); - await saveFlaggedAccountsEntry({ - storage: { version: 1, accounts: [] }, - withStorageLock: async (fn) => fn(), - saveUnlocked, - }); - expect(saveUnlocked).toHaveBeenCalled(); - }); - - it("delegates flagged clear through storage lock", async () => { - const clearFlaggedAccountsOnDisk = vi.fn(async () => undefined); - await clearFlaggedAccountsEntry({ - path: "/tmp/flagged.json", - withStorageLock: async (fn) => fn(), - markerPath: "/tmp/flagged.reset", - getBackupPaths: async () => ["/tmp/flagged.json.bak"], - clearFlaggedAccountsOnDisk, - logError: vi.fn(), - }); - expect(clearFlaggedAccountsOnDisk).toHaveBeenCalled(); - }); - it("exports transaction snapshot when active", async () => { const exportAccountsToFile = vi.fn(async () => undefined); await exportAccountsSnapshot({ diff --git a/test/account-save.test.ts b/test/account-save.test.ts new file mode 100644 index 00000000..dbb3d7e3 --- /dev/null +++ b/test/account-save.test.ts @@ -0,0 +1,83 @@ +import { describe, expect, it, vi } from "vitest"; +import { saveAccountsToDisk } from "../lib/storage/account-save.js"; + +function createStorage(): { + version: 3; + accounts: Array<{ refreshToken: string }>; + activeIndex: number; + activeIndexByFamily: Record; +} { + return { + version: 3, + accounts: [{ refreshToken: "rt-1" }], + activeIndex: 0, + activeIndexByFamily: {}, + }; +} + +function createParams(overrides?: Partial[1]>) { + return { + path: "/tmp/accounts.json", + resetMarkerPath: "/tmp/accounts.reset", + walPath: "/tmp/accounts.wal", + storageBackupEnabled: false, + ensureDirectory: vi.fn(async () => undefined), + ensureGitignore: vi.fn(async () => undefined), + looksLikeSyntheticFixtureStorage: vi.fn(() => true), + loadExistingStorage: vi.fn(async () => null), + createSyntheticFixtureError: vi.fn(() => new Error("synthetic fixture refusal")), + createRotatingAccountsBackup: vi.fn(async () => undefined), + computeSha256: vi.fn(() => "hash"), + writeJournal: vi.fn(async () => undefined), + writeTemp: vi.fn(async () => undefined), + statTemp: vi.fn(async () => ({ size: 10 })), + renameTempToPath: vi.fn(async () => undefined), + cleanupResetMarker: vi.fn(async () => undefined), + cleanupWal: vi.fn(async () => undefined), + cleanupTemp: vi.fn(async () => undefined), + onSaved: vi.fn(() => undefined), + logWarn: vi.fn(() => undefined), + logError: vi.fn(() => undefined), + createStorageError: vi.fn((error) => error as Error), + backupPath: "/tmp/accounts.backup", + createTempPath: vi.fn(() => "/tmp/accounts.tmp"), + ...overrides, + }; +} + +describe("account save helper", () => { + it("rethrows probe failures through createStorageError", async () => { + const probeError = new Error("Failed to read storage file"); + const params = createParams({ + loadExistingStorage: vi.fn(async () => { + throw probeError; + }), + }); + + await expect(saveAccountsToDisk(createStorage() as never, params)).rejects.toBe( + probeError, + ); + expect(params.createSyntheticFixtureError).not.toHaveBeenCalled(); + expect(params.createStorageError).toHaveBeenCalledWith(probeError); + expect(params.writeJournal).not.toHaveBeenCalled(); + }); + + it("refuses to overwrite live storage with a synthetic fixture payload", async () => { + const refusalError = new Error("synthetic fixture refusal"); + const params = createParams({ + loadExistingStorage: vi.fn(async () => createStorage() as never), + looksLikeSyntheticFixtureStorage: vi + .fn() + .mockReturnValueOnce(true) + .mockReturnValueOnce(false), + createSyntheticFixtureError: vi.fn(() => refusalError), + }); + + await expect(saveAccountsToDisk(createStorage() as never, params)).rejects.toBe( + refusalError, + ); + expect(params.createSyntheticFixtureError).toHaveBeenCalledOnce(); + expect(params.createStorageError).toHaveBeenCalledWith(refusalError); + expect(params.writeJournal).not.toHaveBeenCalled(); + }); +}); diff --git a/test/account-snapshot.test.ts b/test/account-snapshot.test.ts new file mode 100644 index 00000000..5479be35 --- /dev/null +++ b/test/account-snapshot.test.ts @@ -0,0 +1,271 @@ +import { describe, expect, it, vi } from "vitest"; +import { describeAccountSnapshot, statSnapshot } from "../lib/storage/account-snapshot.js"; + +describe("statSnapshot", () => { + it("returns size and mtime for accessible snapshots", async () => { + await expect( + statSnapshot("accounts.json", { + stat: vi.fn(async () => ({ size: 1234, mtimeMs: 5678 })), + logWarn: vi.fn(), + }), + ).resolves.toEqual({ exists: true, bytes: 1234, mtimeMs: 5678 }); + }); + + it("returns missing metadata for ENOENT", async () => { + await expect( + statSnapshot("missing.json", { + stat: vi.fn(async () => { + throw Object.assign(new Error("missing"), { code: "ENOENT" }); + }), + logWarn: vi.fn(), + }), + ).resolves.toEqual({ exists: false }); + }); + + it("logs and returns missing metadata for non-ENOENT stat failures", async () => { + const logWarn = vi.fn(); + await expect( + statSnapshot("denied.json", { + stat: vi.fn(async () => { + throw Object.assign(new Error("denied"), { code: "EACCES" }); + }), + logWarn, + }), + ).resolves.toEqual({ exists: false }); + expect(logWarn).toHaveBeenCalledWith( + "Failed to stat backup candidate", + expect.objectContaining({ path: "denied.json" }), + ); + }); + it("treats locked snapshots as existing when stat returns EBUSY", async () => { + const logWarn = vi.fn(); + await expect( + statSnapshot("locked.json", { + stat: vi.fn(async () => { + throw Object.assign(new Error("busy"), { code: "EBUSY" }); + }), + logWarn, + }), + ).resolves.toEqual({ exists: true }); + expect(logWarn).toHaveBeenCalledWith( + "Backup candidate is locked", + expect.objectContaining({ path: "locked.json" }), + ); + }); + + it("treats locked snapshots as existing when stat returns EPERM", async () => { + const logWarn = vi.fn(); + await expect( + statSnapshot("locked.json", { + stat: vi.fn(async () => { + throw Object.assign(new Error("perm"), { code: "EPERM" }); + }), + logWarn, + }), + ).resolves.toEqual({ exists: true }); + expect(logWarn).toHaveBeenCalledWith( + "Backup candidate is locked", + expect.objectContaining({ path: "locked.json" }), + ); + }); + +}); + +describe("describeAccountSnapshot", () => { + it("marks schema-error snapshots valid while preserving schema errors in metadata", async () => { + await expect( + describeAccountSnapshot("accounts.json", "accounts-primary", { + index: 0, + statSnapshot: vi.fn(async () => ({ exists: true, bytes: 12, mtimeMs: 34 })), + loadAccountsFromPath: vi.fn(async () => ({ normalized: { accounts: [{ id: 1 }] }, schemaErrors: ["bad"], storedVersion: 3 })), + logWarn: vi.fn(), + }), + ).resolves.toEqual({ + kind: "accounts-primary", + path: "accounts.json", + index: 0, + exists: true, + valid: true, + bytes: 12, + mtimeMs: 34, + version: 3, + accountCount: 1, + schemaErrors: ["bad"], + }); + }); + + it("marks null-normalized snapshots invalid while preserving metadata", async () => { + await expect( + describeAccountSnapshot("accounts.json", "accounts-primary", { + index: 0, + statSnapshot: vi.fn(async () => ({ exists: true, bytes: 12, mtimeMs: 34 })), + loadAccountsFromPath: vi.fn(async () => ({ normalized: null, schemaErrors: [], storedVersion: 3 })), + logWarn: vi.fn(), + }), + ).resolves.toEqual({ + kind: "accounts-primary", + path: "accounts.json", + index: 0, + exists: true, + valid: false, + bytes: 12, + mtimeMs: 34, + version: 3, + accountCount: undefined, + schemaErrors: undefined, + }); + }); + + it("returns metadata for valid snapshots", async () => { + await expect( + describeAccountSnapshot("accounts.json", "accounts-primary", { + index: 0, + statSnapshot: vi.fn(async () => ({ exists: true, bytes: 12, mtimeMs: 34 })), + loadAccountsFromPath: vi.fn(async () => ({ + normalized: { accounts: [{ id: 1 }, { id: 2 }] }, + schemaErrors: [], + storedVersion: 3, + })), + logWarn: vi.fn(), + }), + ).resolves.toEqual({ + kind: "accounts-primary", + path: "accounts.json", + index: 0, + exists: true, + valid: true, + bytes: 12, + mtimeMs: 34, + version: 3, + accountCount: 2, + }); + }); + + it("refreshes snapshot metadata after a transient stat lock", async () => { + const statSnapshot = vi + .fn() + .mockResolvedValueOnce({ exists: true }) + .mockResolvedValueOnce({ exists: true, bytes: 12, mtimeMs: 34 }); + + await expect( + describeAccountSnapshot("accounts.json", "accounts-primary", { + index: 0, + statSnapshot, + loadAccountsFromPath: vi.fn(async () => ({ + normalized: { accounts: [{ id: 1 }] }, + schemaErrors: [], + storedVersion: 3, + })), + logWarn: vi.fn(), + }), + ).resolves.toEqual({ + kind: "accounts-primary", + path: "accounts.json", + index: 0, + exists: true, + valid: true, + bytes: 12, + mtimeMs: 34, + version: 3, + accountCount: 1, + }); + + expect(statSnapshot).toHaveBeenCalledTimes(2); + }); + + it("falls back to zeroed metadata after repeated stat locks", async () => { + const statSnapshot = vi + .fn() + .mockResolvedValueOnce({ exists: true }) + .mockResolvedValueOnce({ exists: true }) + .mockResolvedValueOnce({ exists: true }); + + await expect( + describeAccountSnapshot("accounts.json", "accounts-primary", { + index: 0, + statSnapshot, + loadAccountsFromPath: vi.fn(async () => ({ + normalized: { accounts: [{ id: 1 }] }, + schemaErrors: [], + storedVersion: 3, + })), + logWarn: vi.fn(), + }), + ).resolves.toEqual({ + kind: "accounts-primary", + path: "accounts.json", + index: 0, + exists: true, + valid: true, + bytes: 0, + mtimeMs: 0, + version: 3, + accountCount: 1, + }); + + expect(statSnapshot).toHaveBeenCalledTimes(3); + }); + + it("returns invalid metadata when the loader fails", async () => { + const logWarn = vi.fn(); + await expect( + describeAccountSnapshot("accounts.json", "accounts-primary", { + statSnapshot: vi.fn(async () => ({ exists: true, bytes: 12, mtimeMs: 34 })), + loadAccountsFromPath: vi.fn(async () => { + throw Object.assign(new Error("boom"), { code: "EACCES" }); + }), + logWarn, + }), + ).resolves.toEqual({ + kind: "accounts-primary", + path: "accounts.json", + index: undefined, + exists: true, + valid: false, + bytes: 12, + mtimeMs: 34, + }); + expect(logWarn).toHaveBeenCalledWith( + "Failed to inspect account snapshot", + expect.objectContaining({ path: "accounts.json" }), + ); + }); + + it("returns missing metadata when stat reports the snapshot missing", async () => { + await expect( + describeAccountSnapshot("missing.json", "accounts-primary", { + index: 2, + statSnapshot: vi.fn(async () => ({ exists: false })), + loadAccountsFromPath: vi.fn(), + logWarn: vi.fn(), + }), + ).resolves.toEqual({ + kind: "accounts-primary", + path: "missing.json", + index: 2, + exists: false, + valid: false, + }); + }); + + it("treats ENOENT during load as an invalid existing snapshot", async () => { + await expect( + describeAccountSnapshot("gone.json", "accounts-primary", { + index: 1, + statSnapshot: vi.fn(async () => ({ exists: true, bytes: 12, mtimeMs: 34 })), + loadAccountsFromPath: vi.fn(async () => { + throw Object.assign(new Error("gone"), { code: "ENOENT" }); + }), + logWarn: vi.fn(), + }), + ).resolves.toEqual({ + kind: "accounts-primary", + path: "gone.json", + index: 1, + exists: true, + valid: false, + bytes: 12, + mtimeMs: 34, + }); + }); +}); diff --git a/test/dashboard-formatters.test.ts b/test/dashboard-formatters.test.ts new file mode 100644 index 00000000..806f7468 --- /dev/null +++ b/test/dashboard-formatters.test.ts @@ -0,0 +1,33 @@ +import { describe, expect, it } from "vitest"; +import { + formatDashboardSettingState, + formatMenuLayoutMode, + formatMenuQuotaTtl, + formatMenuSortMode, +} from "../lib/codex-manager/dashboard-formatters.js"; + +describe("dashboard-formatters", () => { + it("formats dashboard toggle states", () => { + expect(formatDashboardSettingState(true)).toBe("[x]"); + expect(formatDashboardSettingState(false)).toBe("[ ]"); + }); + + it("formats sort mode labels", () => { + expect(formatMenuSortMode("ready-first")).toBe("Ready-First"); + expect(formatMenuSortMode("manual")).toBe("Manual"); + }); + + it("formats layout mode labels", () => { + expect(formatMenuLayoutMode("expanded-rows")).toBe("Expanded Rows"); + expect(formatMenuLayoutMode("compact-details")).toBe( + "Compact + Details Pane", + ); + }); + + it("formats quota ttl values across minute, second, and millisecond branches", () => { + expect(formatMenuQuotaTtl(120_000)).toBe("2m"); + expect(formatMenuQuotaTtl(5_000)).toBe("5s"); + expect(formatMenuQuotaTtl(500)).toBe("500ms"); + expect(formatMenuQuotaTtl(1_500)).toBe("1500ms"); + }); +}); diff --git a/test/restore-backup-entry.test.ts b/test/restore-backup-entry.test.ts index c634b6e9..18214d07 100644 --- a/test/restore-backup-entry.test.ts +++ b/test/restore-backup-entry.test.ts @@ -3,12 +3,17 @@ import { restoreAccountsFromBackupEntry } from "../lib/storage/restore-backup-en describe("restore backup entry", () => { it("passes path, options, and injected deps through to the restore helper", async () => { - const restoreAccountsFromBackupPath = vi.fn(async () => ({ + const restoredStorage = { version: 3, accounts: [], activeIndex: 0, activeIndexByFamily: {}, - })); + }; + const realpath = vi.fn(async (path: string) => path); + const restoreAccountsFromBackupPath = vi.fn(async (path: string, options) => { + await options.realpath(path); + return restoredStorage; + }); const loadAccountsFromPath = vi.fn(async () => ({ normalized: null })); const saveAccounts = vi.fn(async () => undefined); @@ -18,7 +23,7 @@ describe("restore backup entry", () => { restoreAccountsFromBackupPath, getNamedBackupRoot: () => "/tmp/backups", getStoragePath: () => "/tmp/accounts.json", - realpath: vi.fn(async (path) => path), + realpath, loadAccountsFromPath, saveAccounts, }); @@ -28,15 +33,49 @@ describe("restore backup entry", () => { expect.objectContaining({ persist: false, backupRoot: "/tmp/backups", + realpath, loadAccountsFromPath, saveAccounts, }), ); - expect(result).toEqual({ - version: 3, - accounts: [], - activeIndex: 0, - activeIndexByFamily: {}, + expect(realpath).toHaveBeenCalledWith("/tmp/backup.json"); + expect(result).toEqual(restoredStorage); + }); + + it("keeps windows-style backup paths and realpath wiring intact", async () => { + const windowsBackupPath = "C:\\codex\\backups\\snapshot.json"; + const windowsBackupRoot = "C:\\codex\\backups"; + const realpath = vi.fn(async (path: string) => path); + const restoreAccountsFromBackupPath = vi.fn(async (path: string, options) => { + const resolvedPath = await options.realpath(path); + return { + version: 3, + accounts: [{ email: resolvedPath }], + activeIndex: 0, + activeIndexByFamily: {}, + }; + }); + + const result = await restoreAccountsFromBackupEntry({ + path: windowsBackupPath, + options: { persist: true }, + restoreAccountsFromBackupPath, + getNamedBackupRoot: () => windowsBackupRoot, + getStoragePath: () => "C:\\codex\\accounts.json", + realpath, + loadAccountsFromPath: vi.fn(async () => ({ normalized: null })), + saveAccounts: vi.fn(async () => undefined), }); + + expect(restoreAccountsFromBackupPath).toHaveBeenCalledWith( + windowsBackupPath, + expect.objectContaining({ + persist: true, + backupRoot: windowsBackupRoot, + realpath, + }), + ); + expect(realpath).toHaveBeenCalledWith(windowsBackupPath); + expect(result.accounts).toEqual([{ email: windowsBackupPath }]); }); }); diff --git a/test/settings-panels.test.ts b/test/settings-panels.test.ts index e6b77e49..2ad7c0e9 100644 --- a/test/settings-panels.test.ts +++ b/test/settings-panels.test.ts @@ -1,8 +1,13 @@ -import { describe, expect, it } from "vitest"; +import { describe, expect, it, vi } from "vitest"; import { formatAutoReturnDelayLabel, + promptBehaviorSettingsPanelEntry, + promptDashboardDisplaySettingsPanelEntry, + promptStatuslineSettingsPanelEntry, + promptThemeSettingsPanelEntry, reorderStatuslineField, } from "../lib/codex-manager/settings-panels.js"; +import { DEFAULT_DASHBOARD_DISPLAY_SETTINGS } from "../lib/dashboard-settings.js"; describe("settings panel helpers", () => { it("reorders statusline fields safely", () => { @@ -18,4 +23,153 @@ describe("settings panel helpers", () => { expect(formatAutoReturnDelayLabel(0)).toBe("Instant return"); expect(formatAutoReturnDelayLabel(4000)).toBe("4s auto-return"); }); + + it("passes dashboard display panel dependencies through and falls back to defaults", async () => { + const initial = { ...DEFAULT_DASHBOARD_DISPLAY_SETTINGS }; + const cloneDashboardSettings = vi.fn((value) => ({ ...value })); + const buildAccountListPreview = vi.fn(() => "preview"); + const formatDashboardSettingState = vi.fn(() => "state"); + const formatMenuSortMode = vi.fn(() => "sort"); + const resolveMenuLayoutMode = vi.fn((settings?: { menuLayoutMode?: string }) => + settings?.menuLayoutMode === "expanded-rows" + ? "expanded-rows" + : "compact-details", + ); + const formatMenuLayoutMode = vi.fn(() => "layout"); + const applyDashboardDefaultsForKeys = vi.fn(() => initial); + const promptDashboardDisplayPanel = vi.fn(async (_value, deps) => { + expect(_value).toEqual(initial); + expect(deps.cloneDashboardSettings).toBe(cloneDashboardSettings); + expect(deps.buildAccountListPreview).toBe(buildAccountListPreview); + expect(deps.formatDashboardSettingState).toBe(formatDashboardSettingState); + expect(deps.formatMenuSortMode).toBe(formatMenuSortMode); + expect(deps.formatMenuLayoutMode).toBe(formatMenuLayoutMode); + expect(deps.applyDashboardDefaultsForKeys).toBe(applyDashboardDefaultsForKeys); + expect(deps.resolveMenuLayoutMode(undefined)).toBe("compact-details"); + expect( + deps.resolveMenuLayoutMode({ menuLayoutMode: "expanded-rows" } as never), + ).toBe("expanded-rows"); + return initial; + }); + + const result = await promptDashboardDisplaySettingsPanelEntry({ + initial, + promptDashboardDisplayPanel, + cloneDashboardSettings, + buildAccountListPreview, + formatDashboardSettingState, + formatMenuSortMode, + resolveMenuLayoutMode, + formatMenuLayoutMode, + applyDashboardDefaultsForKeys, + DASHBOARD_DISPLAY_OPTIONS: [] as never, + ACCOUNT_LIST_PANEL_KEYS: [] as never, + UI_COPY: {} as never, + }); + + expect(promptDashboardDisplayPanel).toHaveBeenCalledOnce(); + expect(resolveMenuLayoutMode).toHaveBeenCalledWith( + DEFAULT_DASHBOARD_DISPLAY_SETTINGS, + ); + expect(result).toEqual(initial); + }); + + it("passes statusline panel dependencies through", async () => { + const initial = { ...DEFAULT_DASHBOARD_DISPLAY_SETTINGS }; + const cloneDashboardSettings = vi.fn((value) => ({ ...value })); + const buildAccountListPreview = vi.fn(() => "preview"); + const normalizeStatuslineFields = vi.fn((fields) => fields); + const formatDashboardSettingState = vi.fn(() => "state"); + const applyDashboardDefaultsForKeys = vi.fn(() => initial); + const promptStatuslineSettingsPanel = vi.fn(async (_value, deps) => { + expect(_value).toEqual(initial); + expect(deps.cloneDashboardSettings).toBe(cloneDashboardSettings); + expect(deps.buildAccountListPreview).toBe(buildAccountListPreview); + expect(deps.normalizeStatuslineFields).toBe(normalizeStatuslineFields); + expect(deps.formatDashboardSettingState).toBe(formatDashboardSettingState); + expect(deps.applyDashboardDefaultsForKeys).toBe(applyDashboardDefaultsForKeys); + expect(deps.reorderField(["last-used", "limits"], "limits", -1)).toEqual([ + "limits", + "last-used", + ]); + return null; + }); + + const result = await promptStatuslineSettingsPanelEntry({ + initial, + promptStatuslineSettingsPanel, + cloneDashboardSettings, + buildAccountListPreview, + normalizeStatuslineFields, + formatDashboardSettingState, + applyDashboardDefaultsForKeys, + STATUSLINE_FIELD_OPTIONS: [] as never, + STATUSLINE_PANEL_KEYS: [] as never, + UI_COPY: {} as never, + }); + + expect(promptStatuslineSettingsPanel).toHaveBeenCalledOnce(); + expect(result).toBeNull(); + }); + + it("passes behavior panel dependencies through", async () => { + const initial = { ...DEFAULT_DASHBOARD_DISPLAY_SETTINGS }; + const cloneDashboardSettings = vi.fn((value) => ({ ...value })); + const applyDashboardDefaultsForKeys = vi.fn(() => initial); + const formatMenuQuotaTtl = vi.fn(() => "ttl"); + const promptBehaviorSettingsPanel = vi.fn(async (_value, deps) => { + expect(_value).toEqual(initial); + expect(deps.cloneDashboardSettings).toBe(cloneDashboardSettings); + expect(deps.applyDashboardDefaultsForKeys).toBe(applyDashboardDefaultsForKeys); + expect(deps.formatMenuQuotaTtl).toBe(formatMenuQuotaTtl); + expect(deps.formatDelayLabel(4000)).toBe("4s auto-return"); + return initial; + }); + + const result = await promptBehaviorSettingsPanelEntry({ + initial, + promptBehaviorSettingsPanel, + cloneDashboardSettings, + applyDashboardDefaultsForKeys, + formatMenuQuotaTtl, + AUTO_RETURN_OPTIONS_MS: [] as never, + MENU_QUOTA_TTL_OPTIONS_MS: [] as never, + BEHAVIOR_PANEL_KEYS: [] as never, + UI_COPY: {} as never, + }); + + expect(promptBehaviorSettingsPanel).toHaveBeenCalledOnce(); + expect(result).toEqual(initial); + }); + + it("passes theme panel dependencies through", async () => { + const initial = { ...DEFAULT_DASHBOARD_DISPLAY_SETTINGS }; + const cloneDashboardSettings = vi.fn((value) => ({ ...value })); + const applyDashboardDefaultsForKeys = vi.fn(() => initial); + const applyUiThemeFromDashboardSettings = vi.fn(); + const promptThemeSettingsPanel = vi.fn(async (_value, deps) => { + expect(_value).toEqual(initial); + expect(deps.cloneDashboardSettings).toBe(cloneDashboardSettings); + expect(deps.applyDashboardDefaultsForKeys).toBe(applyDashboardDefaultsForKeys); + expect(deps.applyUiThemeFromDashboardSettings).toBe( + applyUiThemeFromDashboardSettings, + ); + return null; + }); + + const result = await promptThemeSettingsPanelEntry({ + initial, + promptThemeSettingsPanel, + cloneDashboardSettings, + applyDashboardDefaultsForKeys, + applyUiThemeFromDashboardSettings, + THEME_PRESET_OPTIONS: [] as never, + ACCENT_COLOR_OPTIONS: [] as never, + THEME_PANEL_KEYS: [] as never, + UI_COPY: {} as never, + }); + + expect(promptThemeSettingsPanel).toHaveBeenCalledOnce(); + expect(result).toBeNull(); + }); }); diff --git a/test/settings-persist-utils.test.ts b/test/settings-persist-utils.test.ts new file mode 100644 index 00000000..e7dfb8b4 --- /dev/null +++ b/test/settings-persist-utils.test.ts @@ -0,0 +1,87 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import { promises as fs } from "node:fs"; +import { formatPersistError, readFileWithRetry, resolvePluginConfigSavePathKey, warnPersistFailure } from "../lib/codex-manager/settings-persist-utils.js"; + +describe("settings persist utils", () => { + afterEach(() => { + delete process.env.CODEX_MULTI_AUTH_CONFIG_PATH; + vi.restoreAllMocks(); + }); + + it("prefers explicit config path over unified settings path", async () => { + process.env.CODEX_MULTI_AUTH_CONFIG_PATH = " /tmp/custom.json "; + const { resolvePluginConfigSavePathKey } = await import("../lib/codex-manager/settings-persist-utils.js"); + expect(resolvePluginConfigSavePathKey()).toBe("/tmp/custom.json"); + }); + + it("formats persist errors consistently", () => { + expect(formatPersistError(new Error("boom"))).toBe("boom"); + expect(formatPersistError("bad")).toBe("bad"); + }); + + it("warns with scope-aware message", () => { + const warn = vi.spyOn(console, "warn").mockImplementation(() => {}); + warnPersistFailure("settings", new Error("disk locked")); + expect(warn).toHaveBeenCalledWith("Settings save failed (settings) after retries: disk locked"); + }); + + it("returns data on the first successful read", async () => { + const readSpy = vi.spyOn(fs, "readFile").mockResolvedValueOnce("ok" as never); + await expect( + readFileWithRetry("settings.json", { retryableCodes: new Set(["EBUSY"]), maxAttempts: 4, sleep: vi.fn(async () => {}) }), + ).resolves.toBe("ok"); + expect(readSpy).toHaveBeenCalledTimes(1); + }); + + it("throws ENOENT immediately without retrying", async () => { + const readSpy = vi.spyOn(fs, "readFile").mockRejectedValueOnce(Object.assign(new Error("missing"), { code: "ENOENT" }) as never); + const sleep = vi.fn(async () => {}); + await expect( + readFileWithRetry("settings.json", { retryableCodes: new Set(["EBUSY"]), maxAttempts: 4, sleep }), + ).rejects.toThrow("missing"); + expect(readSpy).toHaveBeenCalledTimes(1); + expect(sleep).not.toHaveBeenCalled(); + }); + + it("throws non-retryable errors immediately", async () => { + const readSpy = vi.spyOn(fs, "readFile").mockRejectedValueOnce(Object.assign(new Error("bad fd"), { code: "EBADF" }) as never); + const sleep = vi.fn(async () => {}); + await expect( + readFileWithRetry("settings.json", { retryableCodes: new Set(["EBUSY"]), maxAttempts: 4, sleep }), + ).rejects.toThrow("bad fd"); + expect(readSpy).toHaveBeenCalledTimes(1); + expect(sleep).not.toHaveBeenCalled(); + }); + + it("retries retryable errors until max attempts then throws", async () => { + vi.useFakeTimers(); + const readSpy = vi.spyOn(fs, "readFile").mockRejectedValue(Object.assign(new Error("busy"), { code: "EBUSY" }) as never); + const sleep = vi.fn(async (ms: number) => { + await vi.advanceTimersByTimeAsync(ms); + }); + await expect( + readFileWithRetry("settings.json", { retryableCodes: new Set(["EBUSY"]), maxAttempts: 4, sleep }), + ).rejects.toThrow("busy"); + expect(readSpy).toHaveBeenCalledTimes(4); + expect(sleep).toHaveBeenNthCalledWith(1, 25); + expect(sleep).toHaveBeenNthCalledWith(2, 50); + expect(sleep).toHaveBeenNthCalledWith(3, 100); + vi.useRealTimers(); + }); + + it("succeeds on a later retry after EBUSY", async () => { + vi.useFakeTimers(); + const readSpy = vi.spyOn(fs, "readFile") + .mockRejectedValueOnce(Object.assign(new Error("busy"), { code: "EBUSY" }) as never) + .mockResolvedValueOnce("recovered" as never); + const sleep = vi.fn(async (ms: number) => { + await vi.advanceTimersByTimeAsync(ms); + }); + await expect( + readFileWithRetry("settings.json", { retryableCodes: new Set(["EBUSY"]), maxAttempts: 4, sleep }), + ).resolves.toBe("recovered"); + expect(readSpy).toHaveBeenCalledTimes(2); + expect(sleep).toHaveBeenCalledTimes(1); + vi.useRealTimers(); + }); +}); diff --git a/test/storage-flagged.test.ts b/test/storage-flagged.test.ts index c1d50877..8effd3df 100644 --- a/test/storage-flagged.test.ts +++ b/test/storage-flagged.test.ts @@ -11,6 +11,8 @@ import { saveFlaggedAccounts, setStoragePathDirect, } from "../lib/storage.js"; +import { loadFlaggedAccountsFromFile } from "../lib/storage/flagged-storage-file.js"; +import { describeFlaggedSnapshot } from "../lib/storage/snapshot-inspectors.js"; const RETRYABLE_REMOVE_CODES = new Set(["EBUSY", "EPERM", "ENOTEMPTY"]); @@ -537,3 +539,94 @@ describe("flagged account storage", () => { renameSpy.mockRestore(); }); }); + + +describe("flagged storage extracted helpers", () => { + it("retries transient Windows read locks before parsing", async () => { + const normalizeFlaggedStorage = vi.fn((data) => data as never); + const readFile = vi + .fn() + .mockRejectedValueOnce(Object.assign(new Error("busy"), { code: "EBUSY" })) + .mockRejectedValueOnce(Object.assign(new Error("again"), { code: "EAGAIN" })) + .mockResolvedValueOnce('{"version":1,"accounts":[]}'); + await expect( + loadFlaggedAccountsFromFile("flagged.json", { + readFile, + normalizeFlaggedStorage, + sleep: vi.fn(async () => {}), + }), + ).resolves.toEqual({ version: 1, accounts: [] }); + expect(readFile).toHaveBeenCalledTimes(3); + expect(normalizeFlaggedStorage).toHaveBeenCalledWith({ version: 1, accounts: [] }); + }); + + it("does not retry non-retryable permission errors", async () => { + const sleep = vi.fn(async () => {}); + const readFile = vi + .fn() + .mockRejectedValue(Object.assign(new Error("permission denied"), { code: "EPERM" })); + await expect( + loadFlaggedAccountsFromFile("flagged.json", { + readFile, + normalizeFlaggedStorage: vi.fn(), + sleep, + }), + ).rejects.toThrow("permission denied"); + expect(readFile).toHaveBeenCalledTimes(1); + expect(sleep).not.toHaveBeenCalled(); + }); + + it("rethrows after retry budget is exhausted for windows lock errors", async () => { + const sleep = vi.fn(async () => {}); + const readFile = vi + .fn() + .mockRejectedValue(Object.assign(new Error("locked"), { code: "EBUSY" })); + await expect( + loadFlaggedAccountsFromFile("flagged.json", { + readFile, + normalizeFlaggedStorage: vi.fn(), + sleep, + }), + ).rejects.toThrow("locked"); + expect(readFile).toHaveBeenCalledTimes(4); + expect(sleep).toHaveBeenNthCalledWith(1, 10); + expect(sleep).toHaveBeenNthCalledWith(2, 20); + expect(sleep).toHaveBeenNthCalledWith(3, 40); + }); + + + it("propagates malformed JSON parse errors", async () => { + await expect( + loadFlaggedAccountsFromFile("flagged.json", { + readFile: vi.fn(async () => "{"), + normalizeFlaggedStorage: vi.fn(), + }), + ).rejects.toBeInstanceOf(SyntaxError); + }); + + it("returns invalid existing metadata after transient read retries are exhausted", async () => { + const logWarn = vi.fn(); + await expect( + describeFlaggedSnapshot("flagged.json", "flagged-accounts", { + index: 0, + statSnapshot: vi.fn(async () => ({ exists: true, bytes: 12, mtimeMs: 34 })), + loadFlaggedAccountsFromPath: vi.fn(async () => { + throw Object.assign(new Error("locked"), { code: "EBUSY" }); + }), + logWarn, + }), + ).resolves.toEqual({ + kind: "flagged-accounts", + path: "flagged.json", + index: 0, + exists: true, + valid: false, + bytes: 12, + mtimeMs: 34, + }); + expect(logWarn).toHaveBeenCalledWith( + "Failed to inspect flagged snapshot", + expect.objectContaining({ path: "flagged.json" }), + ); + }); +}); diff --git a/test/unified-settings-entry.test.ts b/test/unified-settings-entry.test.ts index ae473f76..ff01a613 100644 --- a/test/unified-settings-entry.test.ts +++ b/test/unified-settings-entry.test.ts @@ -1,37 +1,61 @@ import { describe, expect, it, vi } from "vitest"; import { configureUnifiedSettingsEntry } from "../lib/codex-manager/unified-settings-entry.js"; +function createControllerDeps() { + return { + cloneDashboardSettings: vi.fn((settings) => settings), + cloneBackendPluginConfig: vi.fn((config) => config), + loadDashboardDisplaySettings: vi.fn(async () => ({ + menuShowStatusBadge: false, + })), + loadPluginConfig: vi.fn(() => ({ fetchTimeoutMs: 1000 })), + applyUiThemeFromDashboardSettings: vi.fn(), + promptSettingsHub: vi.fn(), + configureDashboardDisplaySettings: vi.fn(), + configureStatuslineSettings: vi.fn(), + promptBehaviorSettings: vi.fn(), + promptThemeSettings: vi.fn(), + dashboardSettingsEqual: vi.fn(), + persistDashboardSettingsSelection: vi.fn(), + promptExperimentalSettings: vi.fn(), + backendSettingsEqual: vi.fn(), + persistBackendConfigSelection: vi.fn(), + configureBackendSettings: vi.fn(), + BEHAVIOR_PANEL_KEYS: [], + THEME_PANEL_KEYS: [], + }; +} + describe("unified settings entry", () => { it("delegates to the unified settings controller with provided deps", async () => { const configureUnifiedSettingsController = vi.fn(async () => ({ menuShowStatusBadge: true, })); + const controllerDeps = createControllerDeps(); const result = await configureUnifiedSettingsEntry(undefined, { configureUnifiedSettingsController, - cloneDashboardSettings: vi.fn((settings) => settings), - cloneBackendPluginConfig: vi.fn((config) => config), - loadDashboardDisplaySettings: vi.fn(async () => ({ - menuShowStatusBadge: false, - })), - loadPluginConfig: vi.fn(() => ({ fetchTimeoutMs: 1000 })), - applyUiThemeFromDashboardSettings: vi.fn(), - promptSettingsHub: vi.fn(), - configureDashboardDisplaySettings: vi.fn(), - configureStatuslineSettings: vi.fn(), - promptBehaviorSettings: vi.fn(), - promptThemeSettings: vi.fn(), - dashboardSettingsEqual: vi.fn(), - persistDashboardSettingsSelection: vi.fn(), - promptExperimentalSettings: vi.fn(), - backendSettingsEqual: vi.fn(), - persistBackendConfigSelection: vi.fn(), - configureBackendSettings: vi.fn(), - BEHAVIOR_PANEL_KEYS: [], - THEME_PANEL_KEYS: [], + ...controllerDeps, }); - expect(configureUnifiedSettingsController).toHaveBeenCalled(); + expect(configureUnifiedSettingsController).toHaveBeenCalledWith( + undefined, + controllerDeps, + ); expect(result).toEqual({ menuShowStatusBadge: true }); }); + + it("propagates rejection from the controller", async () => { + const expectedError = new Error("controller failure"); + const configureUnifiedSettingsController = vi.fn(async () => { + throw expectedError; + }); + + await expect( + configureUnifiedSettingsEntry(undefined, { + configureUnifiedSettingsController, + ...createControllerDeps(), + }), + ).rejects.toThrow(expectedError); + }); });