diff --git a/docs/reference/storage-paths.md b/docs/reference/storage-paths.md index 186ab1f5..e8f0e8fd 100644 --- a/docs/reference/storage-paths.md +++ b/docs/reference/storage-paths.md @@ -21,6 +21,7 @@ Override root: | File | Default path | | --- | --- | | Unified settings | `~/.codex/multi-auth/settings.json` | +| Unified settings backup | `~/.codex/multi-auth/settings.json.bak` | | Accounts | `~/.codex/multi-auth/openai-codex-accounts.json` | | Accounts backup | `~/.codex/multi-auth/openai-codex-accounts.json.bak` | | Accounts WAL | `~/.codex/multi-auth/openai-codex-accounts.json.wal` | @@ -48,6 +49,14 @@ Compatibility note: Backup metadata: - `getBackupMetadata()` reports deterministic snapshot lists for the canonical account pool (primary, WAL, `.bak`, `.bak.1`, `.bak.2`, and discovered manual backups) and flagged-account state (primary, `.bak`, `.bak.1`, `.bak.2`, and discovered manual backups). Cache-like artifacts and `.reset-intent` markers are excluded from recovery candidates. +- `settings.json.bak` stores the last valid unified settings snapshot before each write and is used as a recovery fallback when `settings.json` is unreadable. +- Flagged-account backup recovery is suppressed whenever the flagged reset marker is still present, so partial clears cannot revive previously cleared flagged entries. + +Upgrade note: + +- Restore workflows now distinguish between unreadable state and intentionally cleared state. `settings.json.bak` is only used when `settings.json` exists but cannot be read, while flagged-account backups stay suppressed whenever the reset marker survives a partial clear. +- Operators validating a restore or clear flow should use `codex auth verify-flagged`, `codex auth fix --dry-run`, and `codex auth doctor --fix` to confirm what will be recovered, what stays cleared, and whether manual repair is still needed. +- Maintainers validating the on-disk upgrade behavior can run `npm run build` plus `npm test -- --run test/unified-settings.test.ts test/storage-recovery-paths.test.ts test/storage-flagged.test.ts` before shipping backup or restore changes. --- diff --git a/lib/config.ts b/lib/config.ts index c68597fc..4e0a676c 100644 --- a/lib/config.ts +++ b/lib/config.ts @@ -41,6 +41,13 @@ const UNSUPPORTED_CODEX_POLICIES = new Set(["strict", "fallback"]); const emittedConfigWarnings = new Set(); const configSaveQueues = new Map>(); const RETRYABLE_FS_CODES = new Set(["EBUSY", "EPERM"]); +const RETRYABLE_CONFIG_READ_CODES = new Set(["EBUSY", "EPERM", "EAGAIN"]); + +type ConfigReadState = + | { status: "missing" } + | { status: "ok"; record: Record } + | { status: "invalid"; errorMessage: string } + | { status: "unreadable"; errorMessage: string }; export type UnsupportedCodexPolicy = "strict" | "fallback"; @@ -199,6 +206,11 @@ export const DEFAULT_PLUGIN_CONFIG: PluginConfig = { preemptiveQuotaMaxDeferralMs: 2 * 60 * 60_000, }; +const PLUGIN_CONFIG_FIELD_SCHEMAS = PluginConfigSchema.shape; +const PLUGIN_CONFIG_KEYS = Object.keys(DEFAULT_PLUGIN_CONFIG) as Array< + keyof PluginConfig +>; + /** * Return a shallow copy of the default plugin configuration. * @@ -238,6 +250,10 @@ export function loadPluginConfig(): PluginConfig { sourceKind = "file"; } + const normalizedUserConfig = sanitizePluginConfigRecord(userConfig, { + warnOnInvalid: true, + }); + const hasFallbackEnvOverride = process.env.CODEX_AUTH_FALLBACK_UNSUPPORTED_MODEL !== undefined || process.env.CODEX_AUTH_FALLBACK_GPT53_TO_GPT52 !== undefined; @@ -255,16 +271,9 @@ export function loadPluginConfig(): PluginConfig { } } - const schemaErrors = getValidationErrors(PluginConfigSchema, userConfig); - if (schemaErrors.length > 0) { - logConfigWarnOnce( - `Plugin config validation warnings: ${schemaErrors.slice(0, 3).join(", ")}`, - ); - } - if ( sourceKind === "file" && - isRecord(userConfig) && + normalizedUserConfig !== null && (process.env.CODEX_MULTI_AUTH_CONFIG_PATH ?? "").trim().length === 0 ) { logConfigWarnOnce( @@ -274,7 +283,7 @@ export function loadPluginConfig(): PluginConfig { return { ...DEFAULT_PLUGIN_CONFIG, - ...(userConfig as Partial), + ...(normalizedUserConfig ?? {}), }; } catch (error) { const configPath = resolvePluginConfigPath() ?? CONFIG_PATH; @@ -285,6 +294,69 @@ export function loadPluginConfig(): PluginConfig { } } +function sanitizePluginConfigRecord( + data: unknown, + options?: { warnOnInvalid?: boolean }, +): Partial | null { + if (!isRecord(data)) { + return null; + } + + if (options?.warnOnInvalid) { + const schemaErrors = getValidationErrors(PluginConfigSchema, data); + if (schemaErrors.length > 0) { + logConfigWarnOnce( + `Plugin config validation warnings: ${schemaErrors.slice(0, 3).join(", ")}`, + ); + } + } + + const sanitized: Record = {}; + for (const key of PLUGIN_CONFIG_KEYS) { + const value = data[key]; + if (value === undefined) { + continue; + } + const schema = PLUGIN_CONFIG_FIELD_SCHEMAS[key]; + const result = schema.safeParse(value); + if (result.success) { + sanitized[String(key)] = result.data; + } + } + + return sanitized as Partial; +} + +/** + * Sanitize a stored plugin-config record while preserving unknown keys. + * + * Known plugin-config keys are schema-validated before they are merged back + * into runtime state. Unknown keys are kept so legacy and forward-compatible + * settings survive env-path saves. + */ +function sanitizeStoredPluginConfigRecord( + data: unknown, +): Record | null { + if (!isRecord(data)) { + return null; + } + + const sanitized: Record = {}; + for (const [key, value] of Object.entries(data)) { + if (!PLUGIN_CONFIG_KEYS.includes(key as keyof PluginConfig)) { + sanitized[key] = value; + continue; + } + const schema = PLUGIN_CONFIG_FIELD_SCHEMAS[key as keyof PluginConfig]; + const result = schema.safeParse(value); + if (result.success) { + sanitized[key] = result.data; + } + } + + return sanitized; +} + /** * Remove a leading UTF‑8 byte order mark (BOM) from the given string if present. * @@ -391,6 +463,53 @@ function readConfigRecordFromPath( } } +async function readConfigRecordForSave( + configPath: string, +): Promise { + if (!existsSync(configPath)) { + return { status: "missing" }; + } + + for (let attempt = 0; attempt < 5; attempt += 1) { + try { + const fileContent = await fs.readFile(configPath, "utf-8"); + const normalizedFileContent = stripUtf8Bom(fileContent); + const parsed = JSON.parse(normalizedFileContent) as unknown; + if (!isRecord(parsed)) { + const errorMessage = `Config at ${configPath} must contain a JSON object at the root.`; + logConfigWarnOnce(errorMessage); + return { status: "invalid", errorMessage }; + } + return { status: "ok", record: parsed }; + } catch (error) { + const code = (error as NodeJS.ErrnoException | undefined)?.code; + if (code === "ENOENT") { + return { status: "missing" }; + } + if ( + typeof code === "string" && + RETRYABLE_CONFIG_READ_CODES.has(code) && + attempt < 4 + ) { + await sleep(10 * 2 ** attempt); + continue; + } + const errorMessage = `Failed to read config from ${configPath}: ${ + error instanceof Error ? error.message : String(error) + }`; + logConfigWarnOnce(errorMessage); + if (typeof code === "string" && RETRYABLE_CONFIG_READ_CODES.has(code)) { + return { status: "unreadable", errorMessage }; + } + return { status: "invalid", errorMessage }; + } + } + + const errorMessage = `Failed to read config from ${configPath}.`; + logConfigWarnOnce(errorMessage); + return { status: "unreadable", errorMessage }; +} + function resolveStoredPluginConfigRecord(): { configPath: string | null; storageKind: ConfigExplainStorageKind; @@ -442,9 +561,17 @@ function resolveStoredPluginConfigRecord(): { */ function sanitizePluginConfigForSave( config: Partial, -): Record { - const entries = Object.entries(config as Record); + ): { sanitized: Record; droppedKeys: string[] } { + const normalized = sanitizePluginConfigRecord(config as Record); + const entries = Object.entries((normalized ?? {}) as Record); const sanitized: Record = {}; + const inputRecord = isRecord(config) ? config : {}; + const droppedKeys = PLUGIN_CONFIG_KEYS.filter((key) => { + if (!(key in inputRecord) || inputRecord[key] === undefined) { + return false; + } + return !(key in (normalized ?? {})); + }); for (const [key, value] of entries) { if (value === undefined) continue; if (typeof value === "number" && !Number.isFinite(value)) continue; @@ -454,7 +581,7 @@ function sanitizePluginConfigForSave( } sanitized[key] = value; } - return sanitized; + return { sanitized, droppedKeys }; } /** @@ -473,13 +600,29 @@ function sanitizePluginConfigForSave( export async function savePluginConfig( configPatch: Partial, ): Promise { - const sanitizedPatch = sanitizePluginConfigForSave(configPatch); + const { sanitized: sanitizedPatch, droppedKeys } = + sanitizePluginConfigForSave(configPatch); + if (droppedKeys.length > 0) { + logConfigWarnOnce( + `Ignoring invalid plugin config field(s): ${droppedKeys.join(", ")}.`, + ); + } const envPath = (process.env.CODEX_MULTI_AUTH_CONFIG_PATH ?? "").trim(); if (envPath.length > 0) { await withConfigSaveLock(envPath, async () => { + const envConfigState = await readConfigRecordForSave(envPath); + if (envConfigState.status === "unreadable") { + throw new Error( + `Aborting config save because ${envPath} is unreadable.`, + ); + } + const existingConfig = + envConfigState.status === "ok" + ? sanitizeStoredPluginConfigRecord(envConfigState.record) + : null; const merged = { - ...(readConfigRecordFromPath(envPath) ?? {}), + ...(existingConfig ?? {}), ...sanitizedPatch, }; await writeJsonFileAtomicWithRetry(envPath, merged); @@ -489,11 +632,37 @@ export async function savePluginConfig( const unifiedPath = getUnifiedSettingsPath(); await withConfigSaveLock(unifiedPath, async () => { - const unifiedConfig = loadUnifiedPluginConfigSync(); - const legacyPath = unifiedConfig ? null : resolvePluginConfigPath(); + const unifiedConfigState = await readConfigRecordForSave(unifiedPath); + if (unifiedConfigState.status === "unreadable") { + throw new Error( + `Aborting config save because ${unifiedPath} is unreadable.`, + ); + } + const unifiedConfigRecord = + unifiedConfigState.status === "ok" + ? unifiedConfigState.record.pluginConfig + : loadUnifiedPluginConfigSync(); + const unifiedConfig = sanitizeStoredPluginConfigRecord(unifiedConfigRecord); + const legacyPath = + unifiedConfigState.status === "missing" || + (unifiedConfigState.status === "ok" && !unifiedConfig) + ? resolvePluginConfigPath() + : null; + const legacyConfigState = legacyPath + ? await readConfigRecordForSave(legacyPath) + : null; + if (legacyConfigState?.status === "unreadable") { + throw new Error( + `Aborting config save because ${legacyPath} is unreadable.`, + ); + } + const legacyConfig = + legacyConfigState?.status === "ok" + ? sanitizeStoredPluginConfigRecord(legacyConfigState.record) + : null; const merged = { ...(unifiedConfig ?? - (legacyPath ? readConfigRecordFromPath(legacyPath) : null) ?? + legacyConfig ?? {}), ...sanitizedPatch, }; @@ -510,12 +679,30 @@ export async function savePluginConfig( */ function parseBooleanEnv(value: string | undefined): boolean | undefined { if (value === undefined) return undefined; - return value === "1"; + const normalized = value.trim().toLowerCase(); + if (normalized.length === 0) return undefined; + if ( + normalized === "1" || + normalized === "true" || + normalized === "yes" + ) { + return true; + } + if ( + normalized === "0" || + normalized === "false" || + normalized === "no" + ) { + return false; + } + return undefined; } function parseNumberEnv(value: string | undefined): number | undefined { if (value === undefined) return undefined; - const parsed = Number(value); + const trimmed = value.trim(); + if (trimmed.length === 0) return undefined; + const parsed = Number(trimmed); if (!Number.isFinite(parsed)) return undefined; return parsed; } @@ -544,7 +731,17 @@ function resolveBooleanSetting( configValue: boolean | undefined, defaultValue: boolean, ): boolean { - const envValue = parseBooleanEnv(process.env[envName]); + const rawEnvValue = process.env[envName]; + const envValue = parseBooleanEnv(rawEnvValue); + if ( + rawEnvValue !== undefined && + rawEnvValue.trim().length > 0 && + envValue === undefined + ) { + logConfigWarnOnce( + `Ignoring invalid boolean env ${envName}. Expected 0/1, true/false, or yes/no.`, + ); + } if (envValue !== undefined) return envValue; return configValue ?? defaultValue; } @@ -566,7 +763,17 @@ function resolveNumberSetting( defaultValue: number, options?: { min?: number; max?: number }, ): number { - const envValue = parseNumberEnv(process.env[envName]); + const rawEnvValue = process.env[envName]; + const envValue = parseNumberEnv(rawEnvValue); + if ( + rawEnvValue !== undefined && + rawEnvValue.trim().length > 0 && + envValue === undefined + ) { + logConfigWarnOnce( + `Ignoring invalid numeric env ${envName}. Expected a finite number.`, + ); + } const candidate = envValue ?? configValue ?? defaultValue; const min = options?.min ?? Number.NEGATIVE_INFINITY; const max = options?.max ?? Number.POSITIVE_INFINITY; diff --git a/lib/storage/flagged-storage-io.ts b/lib/storage/flagged-storage-io.ts index 1788da2b..4c483793 100644 --- a/lib/storage/flagged-storage-io.ts +++ b/lib/storage/flagged-storage-io.ts @@ -2,6 +2,37 @@ import { existsSync, promises as fs } from "node:fs"; import { dirname } from "node:path"; import type { FlaggedAccountStorageV1 } from "../storage.js"; +const RETRYABLE_UNLINK_CODES = new Set(["EBUSY", "EAGAIN", "EPERM"]); + +/** + * Return the ordered backup paths consulted for flagged-account recovery. + */ +function getFlaggedBackupPaths(path: string): string[] { + return [`${path}.bak`, `${path}.bak.1`, `${path}.bak.2`]; +} + +async function unlinkWithRetry(candidatePath: string): Promise { + for (let attempt = 0; attempt < 5; attempt += 1) { + try { + await fs.unlink(candidatePath); + return; + } catch (error) { + const code = (error as NodeJS.ErrnoException).code; + if (code === "ENOENT") { + return; + } + if ( + !code || + !RETRYABLE_UNLINK_CODES.has(code) || + attempt >= 4 + ) { + throw error; + } + await new Promise((resolve) => setTimeout(resolve, 10 * 2 ** attempt)); + } + } +} + export async function loadFlaggedAccountsState(params: { path: string; legacyPath: string; @@ -12,6 +43,37 @@ export async function loadFlaggedAccountsState(params: { logInfo: (message: string, details: Record) => void; }): Promise { const empty: FlaggedAccountStorageV1 = { version: 1, accounts: [] }; + if (existsSync(params.resetMarkerPath)) { + return empty; + } + const loadFlaggedBackup = async (): Promise => { + for (const backupPath of getFlaggedBackupPaths(params.path)) { + if (!existsSync(backupPath)) { + continue; + } + try { + const backupContent = await fs.readFile(backupPath, "utf-8"); + const backupData = JSON.parse(backupContent) as unknown; + const recovered = params.normalizeFlaggedStorage(backupData); + if (existsSync(params.resetMarkerPath)) { + return empty; + } + params.logInfo("Recovered flagged account storage from backup", { + from: backupPath, + to: params.path, + accounts: recovered.accounts.length, + }); + return recovered; + } catch (backupError) { + params.logError("Failed to recover flagged account storage from backup", { + from: backupPath, + to: params.path, + error: String(backupError), + }); + } + } + return null; + }; try { const content = await fs.readFile(params.path, "utf-8"); @@ -28,10 +90,15 @@ export async function loadFlaggedAccountsState(params: { path: params.path, error: String(error), }); - return empty; + return (await loadFlaggedBackup()) ?? empty; } } + const recoveredBackup = await loadFlaggedBackup(); + if (recoveredBackup) { + return recoveredBackup; + } + if (!existsSync(params.legacyPath)) { return empty; } @@ -129,6 +196,7 @@ export async function clearFlaggedAccountsOnDisk(params: { backupPaths: string[]; logError: (message: string, details: Record) => void; }): Promise { + let keepResetMarker = false; try { await fs.writeFile(params.markerPath, "reset", { encoding: "utf-8", @@ -145,10 +213,9 @@ export async function clearFlaggedAccountsOnDisk(params: { for (const candidate of [ params.path, ...params.backupPaths, - params.markerPath, ]) { try { - await fs.unlink(candidate); + await unlinkWithRetry(candidate); } catch (error) { const code = (error as NodeJS.ErrnoException).code; if (code !== "ENOENT") { @@ -159,6 +226,21 @@ export async function clearFlaggedAccountsOnDisk(params: { if (candidate === params.path) { throw error; } + keepResetMarker = true; + } + } + } + if (!keepResetMarker) { + try { + await unlinkWithRetry(params.markerPath); + } catch (error) { + const code = (error as NodeJS.ErrnoException).code; + if (code !== "ENOENT") { + params.logError("Failed to clear flagged account storage", { + path: params.markerPath, + error: String(error), + }); + throw error; } } } diff --git a/lib/unified-settings.ts b/lib/unified-settings.ts index ff63d894..3444c01e 100644 --- a/lib/unified-settings.ts +++ b/lib/unified-settings.ts @@ -1,4 +1,5 @@ import { + copyFileSync, existsSync, mkdirSync, renameSync, @@ -16,9 +17,16 @@ type JsonRecord = Record; export const UNIFIED_SETTINGS_VERSION = 1 as const; const UNIFIED_SETTINGS_PATH = join(getCodexMultiAuthDir(), "settings.json"); +const UNIFIED_SETTINGS_BACKUP_PATH = `${UNIFIED_SETTINGS_PATH}.bak`; const RETRYABLE_FS_CODES = new Set(["EBUSY", "EPERM"]); +const TRANSIENT_READ_FS_CODES = new Set(["EBUSY", "EAGAIN"]); let settingsWriteQueue: Promise = Promise.resolve(); +type SettingsReadResult = { + record: JsonRecord | null; + usedBackup: boolean; +}; + function isRetryableFsError(error: unknown): boolean { const code = (error as NodeJS.ErrnoException | undefined)?.code; return typeof code === "string" && RETRYABLE_FS_CODES.has(code); @@ -45,6 +53,141 @@ function cloneRecord(value: unknown): JsonRecord | null { return { ...value }; } +function parseSettingsRecord(content: string): JsonRecord { + const parsed = cloneRecord(JSON.parse(content)); + if (!parsed) { + throw new Error("Unified settings must contain a JSON object at the root."); + } + return parsed; +} + +/** + * Read a unified-settings record from a specific path. + * + * Returns `null` when the file is absent and throws when the file exists but + * cannot be parsed into the expected object shape. + */ +function readSettingsRecordSyncFromPath(filePath: string): JsonRecord | null { + if (!existsSync(filePath)) { + return null; + } + return parseSettingsRecord(readFileSync(filePath, "utf8")); +} + +/** + * Async variant of `readSettingsRecordSyncFromPath`. + */ +async function readSettingsRecordAsyncFromPath( + filePath: string, +): Promise { + if (!existsSync(filePath)) { + return null; + } + return parseSettingsRecord(await fs.readFile(filePath, "utf8")); +} + +/** + * Best-effort backup reader for sync callers. + * + * Backup corruption is treated as an unavailable backup so callers can keep + * their legacy null-on-unavailable behavior. + */ +function readSettingsBackupSync(): JsonRecord | null { + try { + return readSettingsRecordSyncFromPath(UNIFIED_SETTINGS_BACKUP_PATH); + } catch { + return null; + } +} + +/** + * Best-effort backup reader for async callers. + */ +async function readSettingsBackupAsync(): Promise { + try { + return await readSettingsRecordAsyncFromPath(UNIFIED_SETTINGS_BACKUP_PATH); + } catch { + return null; + } +} + +/** + * Decide whether a primary settings read should fall back to the backup file. + * + * Missing primaries stay missing so callers do not merge stale backup state. + * Corrupt or unreadable primaries may fall back, while transient lock errors + * are rethrown so callers can retry. + */ +function shouldFallbackToSettingsBackup( + primaryExists: boolean, + error: unknown, +): boolean { + if (!primaryExists) { + return false; + } + const code = (error as NodeJS.ErrnoException | undefined)?.code; + if (code === "ENOENT") { + return false; + } + if (typeof code === "string" && TRANSIENT_READ_FS_CODES.has(code)) { + return false; + } + return true; +} + +/** + * Snapshot the primary settings file into `settings.json.bak` for sync writes. + * + * When the current in-memory state was recovered from backup, snapshotting is + * skipped so a corrupt primary cannot overwrite the only known-good backup. + */ +function trySnapshotUnifiedSettingsBackupSync(options?: { + skipBecauseBackupRead?: boolean; +}): void { + if (options?.skipBecauseBackupRead) { + return; + } + if (!existsSync(UNIFIED_SETTINGS_PATH)) { + return; + } + for (let attempt = 0; attempt < 5; attempt += 1) { + try { + copyFileSync(UNIFIED_SETTINGS_PATH, UNIFIED_SETTINGS_BACKUP_PATH); + return; + } catch (error) { + if (!isRetryableFsError(error) || attempt >= 4) { + return; + } + Atomics.wait(new Int32Array(new SharedArrayBuffer(4)), 0, 0, 10 * 2 ** attempt); + } + } +} + +/** + * Async variant of `trySnapshotUnifiedSettingsBackupSync`. + */ +async function trySnapshotUnifiedSettingsBackupAsync(options?: { + skipBecauseBackupRead?: boolean; +}): Promise { + if (options?.skipBecauseBackupRead) { + return; + } + if (!existsSync(UNIFIED_SETTINGS_PATH)) { + return; + } + for (let attempt = 0; attempt < 5; attempt += 1) { + try { + await fs.copyFile(UNIFIED_SETTINGS_PATH, UNIFIED_SETTINGS_BACKUP_PATH); + return; + } catch (error) { + if (!isRetryableFsError(error) || attempt >= 4) { + return; + } + await sleep(10 * 2 ** attempt); + } + } +} + /** * Reads and parses the unified settings JSON file from disk. * @@ -55,17 +198,29 @@ function cloneRecord(value: unknown): JsonRecord | null { * - Windows: file locking on Windows may cause reads to fail; in those cases this function returns `null`. * - Sensitive data: this function performs no token or secret redaction; any sensitive values present in the file are returned as-is and callers are responsible for redaction before logging or external exposure. */ -function readSettingsRecordSync(): JsonRecord | null { - if (!existsSync(UNIFIED_SETTINGS_PATH)) { - return null; +function readSettingsRecordSyncInternal(): SettingsReadResult { + const primaryExists = existsSync(UNIFIED_SETTINGS_PATH); + try { + const primaryRecord = readSettingsRecordSyncFromPath(UNIFIED_SETTINGS_PATH); + if (primaryRecord) { + return { record: primaryRecord, usedBackup: false }; + } + } catch (error) { + if (!shouldFallbackToSettingsBackup(primaryExists, error)) { + throw error; + } + const backupRecord = readSettingsBackupSync(); + if (backupRecord) { + return { record: backupRecord, usedBackup: true }; + } + throw error; } - const raw = readFileSync(UNIFIED_SETTINGS_PATH, "utf8"); - const parsed = cloneRecord(JSON.parse(raw)); - if (!parsed) { - throw new Error("Unified settings must contain a JSON object at the root."); - } - return parsed; + return { record: null, usedBackup: false }; +} + +function readSettingsRecordSync(): JsonRecord | null { + return readSettingsRecordSyncInternal().record; } /** @@ -75,17 +230,31 @@ function readSettingsRecordSync(): JsonRecord | null { * * @returns The parsed settings record as an object clone, or `null` if unavailable or invalid. */ -async function readSettingsRecordAsync(): Promise { - if (!existsSync(UNIFIED_SETTINGS_PATH)) { - return null; +async function readSettingsRecordAsyncInternal(): Promise { + const primaryExists = existsSync(UNIFIED_SETTINGS_PATH); + try { + const primaryRecord = await readSettingsRecordAsyncFromPath( + UNIFIED_SETTINGS_PATH, + ); + if (primaryRecord) { + return { record: primaryRecord, usedBackup: false }; + } + } catch (error) { + if (!shouldFallbackToSettingsBackup(primaryExists, error)) { + throw error; + } + const backupRecord = await readSettingsBackupAsync(); + if (backupRecord) { + return { record: backupRecord, usedBackup: true }; + } + throw error; } - const raw = await fs.readFile(UNIFIED_SETTINGS_PATH, "utf8"); - const parsed = cloneRecord(JSON.parse(raw)); - if (!parsed) { - throw new Error("Unified settings must contain a JSON object at the root."); - } - return parsed; + return { record: null, usedBackup: false }; +} + +async function readSettingsRecordAsync(): Promise { + return (await readSettingsRecordAsyncInternal()).record; } /** @@ -120,11 +289,17 @@ function normalizeForWrite(record: JsonRecord): JsonRecord { * * @param record - The settings object to persist; it will be normalized to include the unified settings version. */ -function writeSettingsRecordSync(record: JsonRecord): void { +function writeSettingsRecordSync( + record: JsonRecord, + options?: { skipBackupSnapshot?: boolean }, +): void { mkdirSync(getCodexMultiAuthDir(), { recursive: true }); const payload = normalizeForWrite(record); const data = `${JSON.stringify(payload, null, 2)}\n`; const tempPath = `${UNIFIED_SETTINGS_PATH}.${process.pid}.${Date.now()}.tmp`; + trySnapshotUnifiedSettingsBackupSync({ + skipBecauseBackupRead: options?.skipBackupSnapshot, + }); writeFileSync(tempPath, data, "utf8"); let moved = false; try { @@ -171,11 +346,17 @@ function writeSettingsRecordSync(record: JsonRecord): void { * * @param record - The settings object to persist; it will be normalized (version set) */ -async function writeSettingsRecordAsync(record: JsonRecord): Promise { +async function writeSettingsRecordAsync( + record: JsonRecord, + options?: { skipBackupSnapshot?: boolean }, +): Promise { await fs.mkdir(getCodexMultiAuthDir(), { recursive: true }); const payload = normalizeForWrite(record); const data = `${JSON.stringify(payload, null, 2)}\n`; const tempPath = `${UNIFIED_SETTINGS_PATH}.${process.pid}.${Date.now()}.${Math.random().toString(36).slice(2, 8)}.tmp`; + await trySnapshotUnifiedSettingsBackupAsync({ + skipBecauseBackupRead: options?.skipBackupSnapshot, + }); await fs.writeFile(tempPath, data, "utf8"); let moved = false; try { @@ -254,9 +435,12 @@ export function loadUnifiedPluginConfigSync(): JsonRecord | null { * @param pluginConfig - Key/value map representing plugin configuration to persist */ export function saveUnifiedPluginConfigSync(pluginConfig: JsonRecord): void { - const record = readSettingsRecordSync() ?? {}; - record.pluginConfig = { ...pluginConfig }; - writeSettingsRecordSync(record); + const { record, usedBackup } = readSettingsRecordSyncInternal(); + const nextRecord = record ?? {}; + nextRecord.pluginConfig = { ...pluginConfig }; + writeSettingsRecordSync(nextRecord, { + skipBackupSnapshot: usedBackup, + }); } /** @@ -271,9 +455,12 @@ export function saveUnifiedPluginConfigSync(pluginConfig: JsonRecord): void { */ export async function saveUnifiedPluginConfig(pluginConfig: JsonRecord): Promise { await enqueueSettingsWrite(async () => { - const record = await readSettingsRecordAsync() ?? {}; - record.pluginConfig = { ...pluginConfig }; - await writeSettingsRecordAsync(record); + const { record, usedBackup } = await readSettingsRecordAsyncInternal(); + const nextRecord = record ?? {}; + nextRecord.pluginConfig = { ...pluginConfig }; + await writeSettingsRecordAsync(nextRecord, { + skipBackupSnapshot: usedBackup, + }); }); } @@ -314,8 +501,11 @@ export async function saveUnifiedDashboardSettings( dashboardDisplaySettings: JsonRecord, ): Promise { await enqueueSettingsWrite(async () => { - const record = await readSettingsRecordAsync() ?? {}; - record.dashboardDisplaySettings = { ...dashboardDisplaySettings }; - await writeSettingsRecordAsync(record); + const { record, usedBackup } = await readSettingsRecordAsyncInternal(); + const nextRecord = record ?? {}; + nextRecord.dashboardDisplaySettings = { ...dashboardDisplaySettings }; + await writeSettingsRecordAsync(nextRecord, { + skipBackupSnapshot: usedBackup, + }); }); } diff --git a/test/accounts.test.ts b/test/accounts.test.ts index 1fb49029..3b011982 100644 --- a/test/accounts.test.ts +++ b/test/accounts.test.ts @@ -3085,7 +3085,7 @@ describe("AccountManager", () => { expect(getRuntimeTrackerKey(account)).toBe(trackerKey); expect(healthTracker.getScore(trackerKey, "codex:gpt-5.1")).toBeCloseTo( degradedScore, - 6, + 5, ); expect(tokenTracker.getTokens(trackerKey, "codex:gpt-5.1")).toBeLessThan(50); }); diff --git a/test/config-save.test.ts b/test/config-save.test.ts index 2064faeb..3b4222c9 100644 --- a/test/config-save.test.ts +++ b/test/config-save.test.ts @@ -126,19 +126,138 @@ describe("plugin config save paths", () => { it("writes through unified settings when env path is unset", async () => { delete process.env.CODEX_MULTI_AUTH_CONFIG_PATH; + const unifiedPath = join(tempDir, "settings.json"); + await fs.writeFile( + unifiedPath, + JSON.stringify({ + version: 1, + pluginConfig: { + preserved: 1, + codexMode: true, + }, + }), + "utf8", + ); - const { savePluginConfig, loadPluginConfig } = - await import("../lib/config.js"); - await savePluginConfig({ - codexMode: false, - parallelProbing: true, - parallelProbingMaxConcurrency: 7, + const logWarnMock = vi.fn(); + vi.doMock("../lib/logger.js", async () => { + const actual = + await vi.importActual( + "../lib/logger.js", + ); + return { + ...actual, + logWarn: logWarnMock, + }; + }); + + try { + const { savePluginConfig, loadPluginConfig } = + await import("../lib/config.js"); + await savePluginConfig({ + codexMode: false, + parallelProbing: true, + parallelProbingMaxConcurrency: 7, + }); + + const loaded = loadPluginConfig(); + expect(loaded.codexMode).toBe(false); + expect(loaded.parallelProbing).toBe(true); + expect(loaded.parallelProbingMaxConcurrency).toBe(2); + + const parsed = JSON.parse(await fs.readFile(unifiedPath, "utf8")) as { + pluginConfig?: Record; + }; + expect(parsed.pluginConfig).toEqual({ + preserved: 1, + codexMode: false, + parallelProbing: true, + }); + expect(logWarnMock).toHaveBeenCalledWith( + expect.stringContaining( + "Ignoring invalid plugin config field(s): parallelProbingMaxConcurrency.", + ), + ); + } finally { + vi.doUnmock("../lib/logger.js"); + } + }); + + it("does not overwrite an unreadable env-path config file", async () => { + const configPath = join(tempDir, "plugin-config.json"); + process.env.CODEX_MULTI_AUTH_CONFIG_PATH = configPath; + await fs.writeFile( + configPath, + JSON.stringify({ codexMode: true, preserved: 1 }), + "utf8", + ); + + const originalReadFile = fs.readFile.bind(fs); + const readSpy = vi.spyOn(fs, "readFile").mockImplementation(async (...args) => { + const [targetPath] = args; + if (targetPath === configPath) { + const error = new Error("busy") as NodeJS.ErrnoException; + error.code = "EBUSY"; + throw error; + } + return originalReadFile(...args); }); - const loaded = loadPluginConfig(); - expect(loaded.codexMode).toBe(false); - expect(loaded.parallelProbing).toBe(true); - expect(loaded.parallelProbingMaxConcurrency).toBe(7); + try { + const { savePluginConfig } = await import("../lib/config.js"); + await expect(savePluginConfig({ codexMode: false })).rejects.toThrow( + "unreadable", + ); + } finally { + readSpy.mockRestore(); + } + + const parsed = JSON.parse(await fs.readFile(configPath, "utf8")) as Record< + string, + unknown + >; + expect(parsed).toEqual({ codexMode: true, preserved: 1 }); + }); + + it("does not overwrite an unreadable unified settings file", async () => { + delete process.env.CODEX_MULTI_AUTH_CONFIG_PATH; + const unifiedPath = join(tempDir, "settings.json"); + await fs.writeFile( + unifiedPath, + JSON.stringify({ + version: 1, + pluginConfig: { codexMode: true, preserved: 1 }, + dashboardDisplaySettings: { uiThemePreset: "green" }, + }), + "utf8", + ); + + const originalReadFile = fs.readFile.bind(fs); + const readSpy = vi.spyOn(fs, "readFile").mockImplementation(async (...args) => { + const [targetPath] = args; + if (targetPath === unifiedPath) { + const error = new Error("busy") as NodeJS.ErrnoException; + error.code = "EBUSY"; + throw error; + } + return originalReadFile(...args); + }); + + try { + const { savePluginConfig } = await import("../lib/config.js"); + await expect(savePluginConfig({ codexMode: false })).rejects.toThrow( + "unreadable", + ); + } finally { + readSpy.mockRestore(); + } + + const parsed = JSON.parse(await fs.readFile(unifiedPath, "utf8")) as { + pluginConfig?: Record; + dashboardDisplaySettings?: Record; + }; + expect(parsed.pluginConfig).toEqual({ codexMode: true, preserved: 1 }); + expect(parsed.dashboardDisplaySettings).toEqual({ uiThemePreset: "green" }); }); it("resolves parallel probing settings and clamps concurrency", async () => { @@ -161,6 +280,128 @@ describe("plugin config save paths", () => { ).toBe(1); }); + it("does not warn for blank numeric env overrides", async () => { + process.env.CODEX_AUTH_PARALLEL_PROBING_MAX_CONCURRENCY = ""; + vi.resetModules(); + const logWarnMock = vi.fn(); + + vi.doMock("../lib/logger.js", async () => { + const actual = + await vi.importActual( + "../lib/logger.js", + ); + return { + ...actual, + logWarn: logWarnMock, + }; + }); + + try { + const { getParallelProbingMaxConcurrency } = await import( + "../lib/config.js" + ); + expect( + getParallelProbingMaxConcurrency({ parallelProbingMaxConcurrency: 4 }), + ).toBe(4); + expect(logWarnMock).not.toHaveBeenCalled(); + } finally { + vi.doUnmock("../lib/logger.js"); + } + }); + + it("does not warn for blank boolean env overrides", async () => { + process.env.CODEX_AUTH_PARALLEL_PROBING = ""; + vi.resetModules(); + const logWarnMock = vi.fn(); + + vi.doMock("../lib/logger.js", async () => { + const actual = + await vi.importActual( + "../lib/logger.js", + ); + return { + ...actual, + logWarn: logWarnMock, + }; + }); + + try { + const { getParallelProbing } = await import("../lib/config.js"); + expect(getParallelProbing({ parallelProbing: false })).toBe(false); + expect(logWarnMock).not.toHaveBeenCalled(); + } finally { + vi.doUnmock("../lib/logger.js"); + } + }); + + it("redacts raw values from invalid boolean env override warnings", async () => { + process.env.CODEX_AUTH_PARALLEL_PROBING = "secret-bool"; + vi.resetModules(); + const logWarnMock = vi.fn(); + + vi.doMock("../lib/logger.js", async () => { + const actual = + await vi.importActual( + "../lib/logger.js", + ); + return { + ...actual, + logWarn: logWarnMock, + }; + }); + + try { + const { getParallelProbing } = await import("../lib/config.js"); + expect(getParallelProbing({ parallelProbing: false })).toBe(false); + expect(logWarnMock).toHaveBeenCalledWith( + expect.stringContaining( + "Ignoring invalid boolean env CODEX_AUTH_PARALLEL_PROBING.", + ), + ); + expect(logWarnMock).not.toHaveBeenCalledWith( + expect.stringContaining("secret-bool"), + ); + } finally { + vi.doUnmock("../lib/logger.js"); + } + }); + + it("redacts raw values from invalid numeric env override warnings", async () => { + process.env.CODEX_AUTH_PARALLEL_PROBING_MAX_CONCURRENCY = "secret-value"; + vi.resetModules(); + const logWarnMock = vi.fn(); + + vi.doMock("../lib/logger.js", async () => { + const actual = + await vi.importActual( + "../lib/logger.js", + ); + return { + ...actual, + logWarn: logWarnMock, + }; + }); + + try { + const { getParallelProbingMaxConcurrency } = await import( + "../lib/config.js" + ); + expect( + getParallelProbingMaxConcurrency({ parallelProbingMaxConcurrency: 4 }), + ).toBe(4); + expect(logWarnMock).toHaveBeenCalledWith( + expect.stringContaining( + "Ignoring invalid numeric env CODEX_AUTH_PARALLEL_PROBING_MAX_CONCURRENCY.", + ), + ); + expect(logWarnMock).not.toHaveBeenCalledWith( + expect.stringContaining("secret-value"), + ); + } finally { + vi.doUnmock("../lib/logger.js"); + } + }); + it("normalizes fallback chain and drops invalid entries", async () => { const { getUnsupportedCodexFallbackChain } = await import("../lib/config.js"); diff --git a/test/plugin-config.test.ts b/test/plugin-config.test.ts index 59c5d233..d30c1f70 100644 --- a/test/plugin-config.test.ts +++ b/test/plugin-config.test.ts @@ -661,9 +661,9 @@ describe('Plugin Configuration', () => { expect(result).toBe(false); }); - it('should handle env var with any value other than "1" as false', () => { - process.env.CODEX_MODE = 'false'; - const config: PluginConfig = { codexMode: true }; + it('should ignore invalid env values and fall back to config/default', () => { + process.env.CODEX_MODE = 'maybe'; + const config: PluginConfig = { codexMode: false }; const result = getCodexMode(config); @@ -990,6 +990,23 @@ describe('Plugin Configuration', () => { expect.stringContaining('Plugin config validation warnings') ); }); + + it('drops invalid persisted values while keeping valid config keys', () => { + mockExistsSync.mockReturnValue(true); + mockReadFileSync.mockReturnValue(JSON.stringify({ + codexMode: 'not-a-boolean', + fetchTimeoutMs: 'oops', + streamStallTimeoutMs: 30000, + backgroundResponses: true, + })); + + const config = loadPluginConfig(); + + expect(config.codexMode).toBe(true); + expect(config.fetchTimeoutMs).toBe(60_000); + expect(config.streamStallTimeoutMs).toBe(30000); + expect(config.backgroundResponses).toBe(true); + }); }); describe('resolveNumberSetting without min option', () => { @@ -1015,6 +1032,13 @@ describe('Plugin Configuration', () => { expect(getFetchTimeoutMs(config)).toBe(120000); }); + it('should ignore empty numeric env values and fall back to config/default', () => { + process.env.CODEX_AUTH_FETCH_TIMEOUT_MS = ''; + expect(getFetchTimeoutMs({ fetchTimeoutMs: 90000 })).toBe(90000); + delete process.env.CODEX_AUTH_FETCH_TIMEOUT_MS; + expect(getFetchTimeoutMs({})).toBe(60000); + }); + it('should read stream stall timeout from env', () => { process.env.CODEX_AUTH_STREAM_STALL_TIMEOUT_MS = '30000'; expect(getStreamStallTimeoutMs({})).toBe(30000); diff --git a/test/rotation-integration.test.ts b/test/rotation-integration.test.ts index a16fe37a..4f6716ac 100644 --- a/test/rotation-integration.test.ts +++ b/test/rotation-integration.test.ts @@ -286,15 +286,24 @@ describe("Multi-Account Rotation Integration", () => { }); }); - describe("Storage mutex (file locking)", () => { - it("concurrent saves complete without corruption", async () => { - const storage = createStorageFromTestAccounts(TEST_ACCOUNTS.slice(0, 3)); - const manager = new AccountManager(undefined, storage); - - const saves = Array.from({ length: 10 }, () => manager.saveToDisk()); - await Promise.all(saves); - }); - }); + describe("Storage mutex (file locking)", () => { + it("concurrent saves complete without corruption", async () => { + const storage = createStorageFromTestAccounts(TEST_ACCOUNTS.slice(0, 3)); + const manager = new AccountManager(undefined, storage); + + const saves = Array.from({ length: 10 }, () => manager.saveToDisk()); + await Promise.all(saves); + + const parsed = JSON.parse( + await fs.readFile(testStoragePath, "utf8"), + ) as AccountStorageV3; + expect(parsed.version).toBe(3); + expect(parsed.accounts).toHaveLength(3); + expect(parsed.accounts.map((account) => account.email)).toEqual( + TEST_ACCOUNTS.slice(0, 3).map((account) => account.email), + ); + }, 15_000); + }); describe("Debounced save", () => { it("saveToDiskDebounced does not throw", () => { diff --git a/test/settings-hub-utils.test.ts b/test/settings-hub-utils.test.ts index 36ca1e3b..af3bc055 100644 --- a/test/settings-hub-utils.test.ts +++ b/test/settings-hub-utils.test.ts @@ -232,7 +232,7 @@ describe("settings-hub utility coverage", () => { expect(() => api.clampBackendNumber("unknown-setting", 5)).toThrow( "Unknown backend numeric setting key", ); - }); + }, 15_000); it("formats layout mode labels", async () => { const api = await loadSettingsHubTestApi(); diff --git a/test/storage-flagged.test.ts b/test/storage-flagged.test.ts index 8effd3df..2e624e22 100644 --- a/test/storage-flagged.test.ts +++ b/test/storage-flagged.test.ts @@ -292,7 +292,7 @@ describe("flagged account storage", () => { unlinkSpy.mockRestore(); }); - it("does not recover flagged backups when the primary file exists but read fails", async () => { + it("recovers flagged backups when the primary file exists but read fails", async () => { await saveFlaggedAccounts({ version: 1, accounts: [ @@ -334,12 +334,50 @@ describe("flagged account storage", () => { }); const flagged = await loadFlaggedAccounts(); - expect(flagged.accounts).toHaveLength(0); + expect(flagged.accounts).toHaveLength(1); + expect(flagged.accounts[0]?.refreshToken).toBe("primary-flagged"); expect(existsSync(flaggedPath)).toBe(true); readSpy.mockRestore(); }); + it("honors the reset marker even when it appears during backup recovery", async () => { + const backupPath = `${getFlaggedAccountsPath()}.bak`; + const resetMarkerPath = `${getFlaggedAccountsPath()}.reset-intent`; + await fs.writeFile( + backupPath, + JSON.stringify({ + version: 1, + accounts: [ + { + refreshToken: "backup-race", + flaggedAt: 1, + addedAt: 1, + lastUsed: 1, + }, + ], + }), + "utf8", + ); + + const originalReadFile = fs.readFile.bind(fs); + const readSpy = vi.spyOn(fs, "readFile").mockImplementation(async (...args) => { + const [targetPath] = args; + const result = await originalReadFile(...args); + if (targetPath === backupPath) { + await fs.writeFile(resetMarkerPath, "reset", "utf8"); + } + return result; + }); + + try { + const flagged = await loadFlaggedAccounts(); + expect(flagged.accounts).toHaveLength(0); + } finally { + readSpy.mockRestore(); + } + }); + it("clears discovered flagged backup artifacts so manual snapshots cannot revive after clear", async () => { await saveFlaggedAccounts({ version: 1, @@ -413,45 +451,6 @@ describe("flagged account storage", () => { unlinkSpy.mockRestore(); }); - it("suppresses flagged accounts when clear cannot delete the primary file after writing the reset marker", async () => { - await saveFlaggedAccounts({ - version: 1, - accounts: [ - { - refreshToken: "stale-primary", - flaggedAt: 1, - addedAt: 1, - lastUsed: 1, - }, - ], - }); - - const flaggedPath = getFlaggedAccountsPath(); - const originalUnlink = fs.unlink.bind(fs); - const unlinkSpy = vi - .spyOn(fs, "unlink") - .mockImplementation(async (targetPath) => { - if (targetPath === flaggedPath) { - const error = new Error( - "EPERM primary delete", - ) as NodeJS.ErrnoException; - error.code = "EPERM"; - throw error; - } - return originalUnlink(targetPath); - }); - - await expect(clearFlaggedAccounts()).rejects.toThrow( - "EPERM primary delete", - ); - - const flagged = await loadFlaggedAccounts(); - expect(existsSync(flaggedPath)).toBe(true); - expect(flagged.accounts).toHaveLength(0); - - unlinkSpy.mockRestore(); - }); - it("emits snapshot metadata for flagged account backups", async () => { await saveFlaggedAccounts({ version: 1, diff --git a/test/storage-recovery-paths.test.ts b/test/storage-recovery-paths.test.ts index 264bf494..f93d9e29 100644 --- a/test/storage-recovery-paths.test.ts +++ b/test/storage-recovery-paths.test.ts @@ -1,4 +1,4 @@ -import { describe, expect, it, beforeEach, afterEach } from "vitest"; +import { describe, expect, it, beforeEach, afterEach, vi } from "vitest"; import { promises as fs, existsSync } from "node:fs"; import { createHash } from "node:crypto"; import { join } from "node:path"; @@ -6,6 +6,7 @@ import { tmpdir } from "node:os"; import { removeWithRetry } from "./helpers/remove-with-retry.js"; import { loadAccounts, + loadFlaggedAccounts, getBackupMetadata, saveAccounts, setStorageBackupEnabled, @@ -109,6 +110,94 @@ describe("storage recovery paths", () => { expect(persisted.accounts?.[0]?.accountId).toBe("from-backup"); }); + it("recovers flagged accounts from backup file when primary storage is missing", async () => { + const flaggedPath = join(workDir, "openai-codex-flagged-accounts.json"); + await fs.writeFile( + `${flaggedPath}.bak`, + JSON.stringify({ + version: 1, + accounts: [ + { + refreshToken: "flagged-refresh", + email: "flagged@example.com", + addedAt: 7, + lastUsed: 7, + flaggedAt: 7, + }, + ], + }), + "utf-8", + ); + + const recovered = await loadFlaggedAccounts(); + expect(recovered.accounts).toHaveLength(1); + expect(recovered.accounts[0]?.email).toBe("flagged@example.com"); + }); + + it("recovers flagged accounts from backup file when primary storage is unreadable", async () => { + const flaggedPath = join(workDir, "openai-codex-flagged-accounts.json"); + await fs.writeFile(flaggedPath, "{broken-primary", "utf-8"); + await fs.writeFile( + `${flaggedPath}.bak`, + JSON.stringify({ + version: 1, + accounts: [ + { + refreshToken: "flagged-refresh-2", + email: "flagged2@example.com", + addedAt: 8, + lastUsed: 8, + flaggedAt: 8, + }, + ], + }), + "utf-8", + ); + + const recovered = await loadFlaggedAccounts(); + expect(recovered.accounts).toHaveLength(1); + expect(recovered.accounts[0]?.email).toBe("flagged2@example.com"); + }); + + it("suppresses flagged backup recovery when the reset marker appears mid-read", async () => { + const flaggedPath = join(workDir, "openai-codex-flagged-accounts.json"); + const backupPath = `${flaggedPath}.bak`; + const resetMarkerPath = `${flaggedPath}.reset-intent`; + await fs.writeFile( + backupPath, + JSON.stringify({ + version: 1, + accounts: [ + { + refreshToken: "flagged-race-refresh", + email: "race@example.com", + addedAt: 9, + lastUsed: 9, + flaggedAt: 9, + }, + ], + }), + "utf-8", + ); + + const originalReadFile = fs.readFile.bind(fs); + const readSpy = vi.spyOn(fs, "readFile").mockImplementation(async (...args) => { + const [targetPath] = args; + const result = await originalReadFile(...args); + if (targetPath === backupPath) { + await fs.writeFile(resetMarkerPath, "reset", "utf-8"); + } + return result; + }); + + try { + const recovered = await loadFlaggedAccounts(); + expect(recovered.accounts).toHaveLength(0); + } finally { + readSpy.mockRestore(); + } + }); + it("falls back to historical backup snapshots when the latest backup is unreadable", async () => { await fs.writeFile(storagePath, "{broken-primary", "utf-8"); await fs.writeFile(`${storagePath}.bak`, "{broken-latest-backup", "utf-8"); diff --git a/test/unified-settings.test.ts b/test/unified-settings.test.ts index 0d749cbd..852863e1 100644 --- a/test/unified-settings.test.ts +++ b/test/unified-settings.test.ts @@ -71,6 +71,163 @@ describe("unified settings", () => { expect(await loadUnifiedDashboardSettings()).toBeNull(); }); + it("returns null sections when both primary and backup settings files are invalid", async () => { + const { + getUnifiedSettingsPath, + loadUnifiedPluginConfigSync, + loadUnifiedDashboardSettings, + } = await import("../lib/unified-settings.js"); + + await fs.writeFile(getUnifiedSettingsPath(), "{ invalid json", "utf8"); + await fs.writeFile(`${getUnifiedSettingsPath()}.bak`, "{ invalid backup", "utf8"); + + expect(loadUnifiedPluginConfigSync()).toBeNull(); + expect(await loadUnifiedDashboardSettings()).toBeNull(); + }); + + it("does not load backup settings when the primary file is missing", async () => { + const { + getUnifiedSettingsPath, + loadUnifiedPluginConfigSync, + loadUnifiedDashboardSettings, + } = await import("../lib/unified-settings.js"); + + await fs.writeFile( + `${getUnifiedSettingsPath()}.bak`, + JSON.stringify({ + version: 1, + pluginConfig: { codexMode: true }, + dashboardDisplaySettings: { uiThemePreset: "green" }, + }), + "utf8", + ); + + expect(loadUnifiedPluginConfigSync()).toBeNull(); + expect(await loadUnifiedDashboardSettings()).toBeNull(); + }); + + it("recovers plugin config from backup when primary settings file is invalid", async () => { + const { + getUnifiedSettingsPath, + saveUnifiedPluginConfig, + loadUnifiedPluginConfigSync, + } = await import("../lib/unified-settings.js"); + + await saveUnifiedPluginConfig({ codexMode: true, fetchTimeoutMs: 45_000 }); + await saveUnifiedPluginConfig({ codexMode: false, fetchTimeoutMs: 90_000 }); + await fs.writeFile(getUnifiedSettingsPath(), "{ invalid json", "utf8"); + + expect(loadUnifiedPluginConfigSync()).toEqual({ + codexMode: true, + fetchTimeoutMs: 45_000, + }); + }); + + it("recovers dashboard settings from backup when primary settings file is invalid", async () => { + const { + getUnifiedSettingsPath, + saveUnifiedDashboardSettings, + loadUnifiedDashboardSettings, + } = await import("../lib/unified-settings.js"); + + await saveUnifiedDashboardSettings({ + menuShowLastUsed: true, + uiThemePreset: "green", + }); + await saveUnifiedDashboardSettings({ + menuShowLastUsed: false, + uiThemePreset: "blue", + }); + await fs.writeFile(getUnifiedSettingsPath(), "{ invalid json", "utf8"); + + expect(await loadUnifiedDashboardSettings()).toEqual({ + menuShowLastUsed: true, + uiThemePreset: "green", + }); + }); + + it("preserves the last good backup when a write fails after a backup-derived read", async () => { + const { + getUnifiedSettingsPath, + loadUnifiedPluginConfigSync, + saveUnifiedPluginConfig, + } = await import("../lib/unified-settings.js"); + + await saveUnifiedPluginConfig({ codexMode: true, fetchTimeoutMs: 45_000 }); + await saveUnifiedPluginConfig({ codexMode: false, fetchTimeoutMs: 90_000 }); + await fs.writeFile(getUnifiedSettingsPath(), "{ invalid json", "utf8"); + + expect(loadUnifiedPluginConfigSync()).toEqual({ + codexMode: true, + fetchTimeoutMs: 45_000, + }); + + const backupPath = `${getUnifiedSettingsPath()}.bak`; + const copySpy = vi.spyOn(fs, "copyFile"); + const renameSpy = vi.spyOn(fs, "rename"); + renameSpy.mockImplementation(async () => { + const error = new Error("busy") as NodeJS.ErrnoException; + error.code = "EBUSY"; + throw error; + }); + + try { + await expect( + saveUnifiedPluginConfig({ codexMode: true, fetchTimeoutMs: 120_000 }), + ).rejects.toThrow(); + } finally { + copySpy.mockRestore(); + renameSpy.mockRestore(); + } + + expect(copySpy).not.toHaveBeenCalledWith( + getUnifiedSettingsPath(), + backupPath, + ); + const backupRecord = JSON.parse( + await fs.readFile(backupPath, "utf8"), + ) as { pluginConfig?: Record }; + expect(backupRecord.pluginConfig).toEqual({ + codexMode: true, + fetchTimeoutMs: 45_000, + }); + }); + + it("resumes snapshotting after a backup-derived write succeeds", async () => { + const { + getUnifiedSettingsPath, + loadUnifiedPluginConfigSync, + saveUnifiedPluginConfig, + } = await import("../lib/unified-settings.js"); + + await saveUnifiedPluginConfig({ codexMode: true, fetchTimeoutMs: 45_000 }); + await saveUnifiedPluginConfig({ codexMode: false, fetchTimeoutMs: 90_000 }); + await fs.writeFile(getUnifiedSettingsPath(), "{ invalid json", "utf8"); + + expect(loadUnifiedPluginConfigSync()).toEqual({ + codexMode: true, + fetchTimeoutMs: 45_000, + }); + + await saveUnifiedPluginConfig({ codexMode: true, fetchTimeoutMs: 120_000 }); + let backupRecord = JSON.parse( + await fs.readFile(`${getUnifiedSettingsPath()}.bak`, "utf8"), + ) as { pluginConfig?: Record }; + expect(backupRecord.pluginConfig).toEqual({ + codexMode: true, + fetchTimeoutMs: 45_000, + }); + + await saveUnifiedPluginConfig({ codexMode: false, fetchTimeoutMs: 150_000 }); + backupRecord = JSON.parse( + await fs.readFile(`${getUnifiedSettingsPath()}.bak`, "utf8"), + ) as { pluginConfig?: Record }; + expect(backupRecord.pluginConfig).toEqual({ + codexMode: true, + fetchTimeoutMs: 120_000, + }); + }); + it("returns null when dashboard settings file is missing", async () => { const { loadUnifiedDashboardSettings } = await import( "../lib/unified-settings.js" @@ -379,7 +536,51 @@ describe("unified settings", () => { }); }); - it("refuses overwriting settings sections when a read fails", async () => { + it("falls back to backup when the primary settings file is unreadable", async () => { + const { + saveUnifiedPluginConfig, + saveUnifiedDashboardSettings, + getUnifiedSettingsPath, + } = await import("../lib/unified-settings.js"); + + await saveUnifiedPluginConfig({ codexMode: true, fetchTimeoutMs: 70_000 }); + await saveUnifiedDashboardSettings({ + menuShowLastUsed: false, + uiThemePreset: "blue", + }); + + const readSpy = vi.spyOn(fs, "readFile"); + readSpy.mockImplementationOnce(async () => { + const error = new Error("permission denied") as NodeJS.ErrnoException; + error.code = "EACCES"; + throw error; + }); + + try { + await saveUnifiedDashboardSettings({ + menuShowLastUsed: true, + uiThemePreset: "yellow", + }); + } finally { + readSpy.mockRestore(); + } + + const raw = await fs.readFile(getUnifiedSettingsPath(), "utf8"); + const parsed = JSON.parse(raw) as { + pluginConfig?: Record; + dashboardDisplaySettings?: Record; + }; + expect(parsed.pluginConfig).toEqual({ + codexMode: true, + fetchTimeoutMs: 70_000, + }); + expect(parsed.dashboardDisplaySettings).toEqual({ + menuShowLastUsed: true, + uiThemePreset: "yellow", + }); + }); + + it("rethrows transient primary read errors instead of falling back to backup", async () => { const { saveUnifiedPluginConfig, saveUnifiedDashboardSettings, @@ -405,7 +606,7 @@ describe("unified settings", () => { menuShowLastUsed: true, uiThemePreset: "yellow", }), - ).rejects.toThrow(); + ).rejects.toThrow("file locked"); } finally { readSpy.mockRestore(); }